[Intel-gfx] [PATCH v3] drm/i915/bxt: eDP Panel Power sequencing
Changes for BXT - added a IS_BROXTON check to use the macro related to PPS registers for BXT. BXT does not have PP_DIV register. Making changes to handle this. Second set of PPS registers have been defined but will be used when VBT provides a selection between the 2 sets of registers. v2: [Jani] Added 2nd set of PPS registers and the macro Jani's review comments - remove reference in i915_suspend.c - Use BXT PP macro Squashing all PPS related patches into one. v3: Jani's review comments addressed - Use pp_ctl instead of pp - ironlake_get_pp_control() is not required for BXT - correct the use of && in the print statement - drop the shift in the print statement Signed-off-by: Vandana Kannan Signed-off-by: A.Sunil Kamath --- drivers/gpu/drm/i915/i915_reg.h | 13 drivers/gpu/drm/i915/intel_dp.c | 74 - 2 files changed, 72 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 1b31238..777c51a 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -6315,6 +6315,8 @@ enum skl_disp_power_wells { #define PCH_PP_CONTROL 0xc7204 #define PANEL_UNLOCK_REGS (0xabcd << 16) #define PANEL_UNLOCK_MASK (0x << 16) +#define BXT_POWER_CYCLE_DELAY_MASK(0x1f0) +#define BXT_POWER_CYCLE_DELAY_SHIFT 4 #define EDP_FORCE_VDD (1 << 3) #define EDP_BLC_ENABLE(1 << 2) #define PANEL_POWER_RESET (1 << 1) @@ -6343,6 +6345,17 @@ enum skl_disp_power_wells { #define PANEL_POWER_CYCLE_DELAY_MASK (0x1f) #define PANEL_POWER_CYCLE_DELAY_SHIFT 0 +/* BXT PPS changes - 2nd set of PPS registers */ +#define _BXT_PP_STATUS20xc7300 +#define _BXT_PP_CONTROL2 0xc7304 +#define _BXT_PP_ON_DELAYS2 0xc7308 +#define _BXT_PP_OFF_DELAYS20xc730c + +#define BXT_PP_STATUS(n) ((!n) ? PCH_PP_STATUS : _BXT_PP_STATUS2) +#define BXT_PP_CONTROL(n) ((!n) ? PCH_PP_CONTROL : _BXT_PP_CONTROL2) +#define BXT_PP_ON_DELAYS(n)((!n) ? PCH_PP_ON_DELAYS : _BXT_PP_ON_DELAYS2) +#define BXT_PP_OFF_DELAYS(n) ((!n) ? PCH_PP_OFF_DELAYS : _BXT_PP_OFF_DELAYS2) + #define PCH_DP_B 0xe4100 #define PCH_DPB_AUX_CH_CTL 0xe4110 #define PCH_DPB_AUX_CH_DATA1 0xe4114 diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index bacdec5..3082224 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -559,7 +559,9 @@ static u32 _pp_ctrl_reg(struct intel_dp *intel_dp) { struct drm_device *dev = intel_dp_to_dev(intel_dp); - if (HAS_PCH_SPLIT(dev)) + if (IS_BROXTON(dev)) + return BXT_PP_CONTROL(0); + else if (HAS_PCH_SPLIT(dev)) return PCH_PP_CONTROL; else return VLV_PIPE_PP_CONTROL(vlv_power_sequencer_pipe(intel_dp)); @@ -569,7 +571,9 @@ static u32 _pp_stat_reg(struct intel_dp *intel_dp) { struct drm_device *dev = intel_dp_to_dev(intel_dp); - if (HAS_PCH_SPLIT(dev)) + if (IS_BROXTON(dev)) + return BXT_PP_STATUS(0); + else if (HAS_PCH_SPLIT(dev)) return PCH_PP_STATUS; else return VLV_PIPE_PP_STATUS(vlv_power_sequencer_pipe(intel_dp)); @@ -4969,8 +4973,8 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev, struct drm_i915_private *dev_priv = dev->dev_private; struct edp_power_seq cur, vbt, spec, *final = &intel_dp->pps_delays; - u32 pp_on, pp_off, pp_div, pp; - int pp_ctrl_reg, pp_on_reg, pp_off_reg, pp_div_reg; + u32 pp_on, pp_off, pp_div = 0, pp_ctl = 0; + int pp_ctrl_reg, pp_on_reg, pp_off_reg, pp_div_reg = 0; lockdep_assert_held(&dev_priv->pps_mutex); @@ -4978,7 +4982,16 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev, if (final->t11_t12 != 0) return; - if (HAS_PCH_SPLIT(dev)) { + if (IS_BROXTON(dev)) { + /* +* TODO: BXT has 2 sets of PPS registers. +* Correct Register for Broxton need to be identified +* using VBT. hardcoding for now +*/ + pp_ctrl_reg = BXT_PP_CONTROL(0); + pp_on_reg = BXT_PP_ON_DELAYS(0); + pp_off_reg = BXT_PP_OFF_DELAYS(0); + } else if (HAS_PCH_SPLIT(dev)) { pp_ctrl_reg = PCH_PP_CONTROL; pp_on_reg = PCH_PP_ON_DELAYS; pp_off_reg = PCH_PP_OFF_DELAYS; @@ -4994,12 +5007,17 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev, /* Workaround: Need to write PP_CONTROL with the unlock key as * the very first thing. */ - pp = ironlake_get_pp_control(intel_dp); - I915_WRITE(pp_ctrl_reg, pp); + if (!IS_BROXTON(dev)) { + pp_ctl = ironlake_get_pp_control(intel_dp); + I915_W
Re: [Intel-gfx] [PATCH 6/8] pwm: crc: Add Crystalcove (CRC) PWM driver
On Wed, May 6, 2015 at 1:10 PM, Paul Bolle wrote: > On Tue, 2015-05-05 at 15:08 +0530, Shobhit Kumar wrote: >> The Crystalcove PMIC controls PWM signals and this driver exports that >> capability as a PWM chip driver. This is platform device implementtaion >> of the drivers/mfd cell device for CRC PMIC >> >> v2: Use the existing config callback with duty_ns and period_ns(Thierry) >> >> v3: Correct the subject line (Lee jones) >> >> CC: Samuel Ortiz >> Cc: Linus Walleij >> Cc: Alexandre Courbot >> Cc: Thierry Reding >> Signed-off-by: Shobhit Kumar > > The same comments can be made as for v2, see > http://lkml.kernel.org/r/1430428322.2187.24.camel@x220 . Maybe you > didn't receive that message. > > It could also be that you think my comments were invalid, or too vague, > or whatever. Please say so, because then I don't have to bother you > again when you send out v4. > Not at all, I just missed your comments and realise my mistake later after sending next update. Somehow the mailing list filters that I have setup are not working correctly. I will look into your comments. Regards Shobhit ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 6/8] pwm: crc: Add Crystalcove (CRC) PWM driver
On Wed, May 6, 2015 at 5:44 PM, Thierry Reding wrote: > On Tue, May 05, 2015 at 03:08:36PM +0530, Shobhit Kumar wrote: >> The Crystalcove PMIC controls PWM signals and this driver exports that > > You say signal_s_ here, but you only expose a single PWM device. Does > the PMIC really control more than one? If it isn't, this should probably > become: "controls a PWM output and this driver...". Actually it does support 3 of them but on the platform only one is being used and I exported only that as of now. Probably I should expand a little in the commit message indicating this. will re-post after fixing based on your other comments. Regards Shobhit > >> capability as a PWM chip driver. This is platform device implementtaion > > "implementation" > >> of the drivers/mfd cell device for CRC PMIC > > Sentences should end with a full stop. > >> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig >> index b1541f4..954da3e 100644 >> --- a/drivers/pwm/Kconfig >> +++ b/drivers/pwm/Kconfig >> @@ -183,6 +183,13 @@ config PWM_LPC32XX >> To compile this driver as a module, choose M here: the module >> will be called pwm-lpc32xx. >> >> +config PWM_CRC >> + bool "Intel Crystalcove (CRC) PWM support" >> + depends on X86 && INTEL_SOC_PMIC >> + help >> + Generic PWM framework driver for Crystalcove (CRC) PMIC based PWM >> + control. >> + > > This is badly sorted. Please keep the list sorted alphabetically. > >> config PWM_LPSS >> tristate "Intel LPSS PWM support" >> depends on X86 >> diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile >> index ec50eb5..3d38fed 100644 >> --- a/drivers/pwm/Makefile >> +++ b/drivers/pwm/Makefile >> @@ -35,3 +35,4 @@ obj-$(CONFIG_PWM_TIPWMSS) += pwm-tipwmss.o >> obj-$(CONFIG_PWM_TWL)+= pwm-twl.o >> obj-$(CONFIG_PWM_TWL_LED)+= pwm-twl-led.o >> obj-$(CONFIG_PWM_VT8500) += pwm-vt8500.o >> +obj-$(CONFIG_PWM_CRC)+= pwm-crc.o > > This too. > >> diff --git a/drivers/pwm/pwm-crc.c b/drivers/pwm/pwm-crc.c >> new file mode 100644 >> index 000..987f3b4 >> --- /dev/null >> +++ b/drivers/pwm/pwm-crc.c >> @@ -0,0 +1,171 @@ >> +/* >> + * pwm-crc.c - Intel Crystal Cove PWM Driver > > I think you can safely remove this line. You already know what file it > is when you open it in your editor, and the description is in the > MODULE_DESCRIPTION string already. > >> + * >> + * Copyright (C) 2015 Intel Corporation. All rights reserved. >> + * >> + * This program is free software; you can redistribute it and/or >> + * modify it under the terms of the GNU General Public License version >> + * 2 as published by the Free Software Foundation. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + * Author: Shobhit Kumar >> + */ >> + >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +#define PWM0_CLK_DIV 0x4B >> +#define PWM_OUTPUT_ENABLE (1<<7) > > Should have spaces around <<. > >> +#define PWM_DIV_CLK_0 0x00 /* DIVIDECLK = BASECLK */ >> +#define PWM_DIV_CLK_100 0x63 /* DIVIDECLK = BASECLK/100 */ >> +#define PWM_DIV_CLK_128 0x7F /* DIVIDECLK = BASECLK/128 */ >> + >> +#define PWM0_DUTY_CYCLE 0x4E >> +#define BACKLIGHT_EN 0x51 >> + >> +#define PWM_MAX_LEVEL0xFF >> + >> +#define PWM_BASE_CLK 6000/* 6 MHz */ > > This number is actually 6 KHz. I think it'd be better if you stuck with > one unit here. Or perhaps there's some other reason why you can't use > 600 here instead? > >> +#define PWM_MAX_PERIOD_NS21333 /* 46.875KHz */ >> + >> +/** >> + * struct crystalcove_pwm - Crystal Cove PWM controller >> + * @chip: the abstract pwm_chip structure. >> + * @regmap: the regmap from the parent device. >> + */ >> +struct crystalcove_pwm { >> + struct pwm_chip chip; >> + struct platform_device *pdev; > > I think I had at some point requested that you get rid of this and use > the chip.dev member instead. There's no kerneldoc for it and it isn't > (well, almost, see below) used anywhere else, so perhaps you forgot to > remove it here? > >> + struct regmap *regmap; >> +}; >> + >> +static inline struct crystalcove_pwm *to_crc_pwm(struct pwm_chip *pc) >> +{ >> + return container_of(pc, struct crystalcove_pwm, chip); >> +} >> + >> +static int crc_pwm_enable(struct pwm_chip *c, struct pwm_device *pwm) >> +{ >> + struct crystalcove_pwm *crc_pwm = to_crc_pwm(c); >> + >> + regmap_write(crc_pwm->regmap, BACKLIGHT_EN, 1); >> + >> + return 0; >> +} >> + >> +static void crc_pwm_disable(struct pwm_chip *c, struct pwm_device *pwm) >> +{ >> + struct crystalcove_pwm *crc_pwm = to_crc_pwm(c); >> + >> + regmap_write(crc_pwm->regmap, BACKLIGHT_EN, 0); >> +} >> + >> +s
Re: [Intel-gfx] [PATCH 1/8] gpiolib: Add support for removing registered consumer lookup table
On Tue, 05 May 2015, Daniel Vetter wrote: > On Tue, May 05, 2015 at 11:45:05AM +0100, Lee Jones wrote: > > This is not how we submit subsequent patch-sets. > > It is unfortunately how we handle patches on dri-devel&intel-gfx to be > able to cope with massive mail load. If everyone who submits to intel-gfx > would always resend the entire series for minor updates of som patches > we'd completely drown in the resulting flood. For one or two simple fix-ups in the set perhaps, but when submitting the entire set it needs to be threaded as a separate block, rather than seeing current and superseded patches inter-woven. This submission is already a rat's nest and I'm struggling to see which patches are which. I'm really not looking forward to v3 and v4! Attaching one version to another is a good way to keep control if you really are over-whelmed. For your use-case I would expect to see the following, which is achieved using --in-reply-to: [PATCH 0/2] Here is what I did... [PATCH 1/2] Clean up and tests [PATCH 2/2] Implementation [PATCH v2 0/3] Here is a reroll [PATCH v2 1/3] Clean up [PATCH v2 2/3] New tests [PATCH v2 3/3] Implementation The version numbers also need to be present and aren't in this re-submission. > > Please submit them as a whole, seperately from the first submission > > and with versioning information i.e. [PATCH v2 X/Y] Stuff ... > > > > > In case we unload and load a driver module again that is registering a > > > lookup table, without this it will result in multiple entries. Provide > > > an option to remove the lookup table on driver unload > > > > > > v2: Ccing maintainers > > > v3: Correct the subject line (Lee jones) > > > > Change logs should go underneth the '---' and above the diffstat found > > below. > > Again just style differences between subsystems, I generally want to have > those above the ---. For all commits? Then I'm guessing your Git history is all but unreadable. In the kernel, unless the changelog holds valuable historic information which influance key design decisions, we put the patch changelog *below* the '---'. Please read Documentation/SubmittingPatches: "14) The canonical patch format [...] The "---" marker line serves the essential purpose of marking for patch handling tools where the changelog message ends. [...] Other comments relevant only to the moment or the maintainer, not suitable for the permanent changelog, should also go here. A good example of such comments might be *"patch changelogs"* which describe what has changed between the v1 and v2 version of the patch." > > > Cc: Samuel Ortiz > > > Cc: Linus Walleij > > > Cc: Alexandre Courbot > > > Cc: Thierry Reding > > > Reviewed-by: Alexandre Courbot > > > Signed-off-by: Shobhit Kumar > > > --- > > > drivers/gpio/gpiolib.c | 13 + > > > include/linux/gpio/machine.h | 1 + > > > 2 files changed, 14 insertions(+) > > > > > > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c > > > index 59eaa23..2420af9 100644 > > > --- a/drivers/gpio/gpiolib.c > > > +++ b/drivers/gpio/gpiolib.c > > > @@ -1658,6 +1658,19 @@ void gpiod_add_lookup_table(struct > > > gpiod_lookup_table *table) > > > mutex_unlock(&gpio_lookup_lock); > > > } > > > > > > +/** > > > + * gpiod_remove_lookup_table() - unregister GPIO device consumers > > > + * @table: table of consumers to unregister > > > + */ > > > +void gpiod_remove_lookup_table(struct gpiod_lookup_table *table) > > > +{ > > > + mutex_lock(&gpio_lookup_lock); > > > + > > > + list_del(&table->list); > > > + > > > + mutex_unlock(&gpio_lookup_lock); > > > +} > > > + > > > static struct gpio_desc *of_find_gpio(struct device *dev, const char > > > *con_id, > > > unsigned int idx, > > > enum gpio_lookup_flags *flags) > > > diff --git a/include/linux/gpio/machine.h b/include/linux/gpio/machine.h > > > index e270614..c0d712d 100644 > > > --- a/include/linux/gpio/machine.h > > > +++ b/include/linux/gpio/machine.h > > > @@ -57,5 +57,6 @@ struct gpiod_lookup_table { > > > } > > > > > > void gpiod_add_lookup_table(struct gpiod_lookup_table *table); > > > +void gpiod_remove_lookup_table(struct gpiod_lookup_table *table); > > > > > > #endif /* __LINUX_GPIO_MACHINE_H */ > > > -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] dma-buf: add ref counting for module as exporter
Add reference counting on a kernel module that exports dma-buf and implements its operations. This prevents the module from being unloaded while DMABUF file is in use. The original patch [1] was submitted by Tomasz, but he's since shifted jobs and a ping didn't elicit any response. [tomasz: Original author] Signed-off-by: Tomasz Stanislawski Acked-by: Daniel Vetter [sumits: updated & rebased as per review comments] Signed-off-by: Sumit Semwal [1]: https://lkml.org/lkml/2012/8/8/163 --- drivers/dma-buf/dma-buf.c | 9 - drivers/gpu/drm/armada/armada_gem.c| 1 + drivers/gpu/drm/drm_prime.c| 1 + drivers/gpu/drm/exynos/exynos_drm_dmabuf.c | 1 + drivers/gpu/drm/i915/i915_gem_dmabuf.c | 1 + drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c | 1 + drivers/gpu/drm/tegra/gem.c| 1 + drivers/gpu/drm/udl/udl_dmabuf.c | 1 + drivers/gpu/drm/vmwgfx/vmwgfx_prime.c | 1 + drivers/media/v4l2-core/videobuf2-dma-contig.c | 1 + drivers/media/v4l2-core/videobuf2-dma-sg.c | 1 + drivers/media/v4l2-core/videobuf2-vmalloc.c| 1 + drivers/staging/android/ion/ion.c | 1 + include/linux/dma-buf.h| 2 ++ 14 files changed, 22 insertions(+), 1 deletion(-) diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index c5a9138a6a8d..9583eac0238f 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -29,6 +29,7 @@ #include #include #include +#include #include #include #include @@ -64,6 +65,7 @@ static int dma_buf_release(struct inode *inode, struct file *file) BUG_ON(dmabuf->cb_shared.active || dmabuf->cb_excl.active); dmabuf->ops->release(dmabuf); + module_put(dmabuf->ops->owner); mutex_lock(&db_list.lock); list_del(&dmabuf->list_node); @@ -302,9 +304,14 @@ struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info) return ERR_PTR(-EINVAL); } + if (!try_module_get(exp_info->ops->owner)) + return ERR_PTR(-ENOENT); + dmabuf = kzalloc(alloc_size, GFP_KERNEL); - if (dmabuf == NULL) + if (!dmabuf) { + module_put(exp_info->ops->owner); return ERR_PTR(-ENOMEM); + } dmabuf->priv = exp_info->priv; dmabuf->ops = exp_info->ops; diff --git a/drivers/gpu/drm/armada/armada_gem.c b/drivers/gpu/drm/armada/armada_gem.c index 580e10acaa3a..d2c5fc1269b7 100644 --- a/drivers/gpu/drm/armada/armada_gem.c +++ b/drivers/gpu/drm/armada/armada_gem.c @@ -524,6 +524,7 @@ armada_gem_dmabuf_mmap(struct dma_buf *buf, struct vm_area_struct *vma) } static const struct dma_buf_ops armada_gem_prime_dmabuf_ops = { + .owner = THIS_MODULE, .map_dma_buf= armada_gem_prime_map_dma_buf, .unmap_dma_buf = armada_gem_prime_unmap_dma_buf, .release= drm_gem_dmabuf_release, diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index 7fec191b45f7..ed4a6e35dd91 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -289,6 +289,7 @@ static int drm_gem_dmabuf_mmap(struct dma_buf *dma_buf, } static const struct dma_buf_ops drm_gem_prime_dmabuf_ops = { + .owner = THIS_MODULE, .attach = drm_gem_map_attach, .detach = drm_gem_map_detach, .map_dma_buf = drm_gem_map_dma_buf, diff --git a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c index cd485c091b30..35fa029353e9 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c +++ b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c @@ -169,6 +169,7 @@ static int exynos_gem_dmabuf_mmap(struct dma_buf *dma_buf, } static struct dma_buf_ops exynos_dmabuf_ops = { + .owner = THIS_MODULE, .attach = exynos_gem_attach_dma_buf, .detach = exynos_gem_detach_dma_buf, .map_dma_buf= exynos_gem_map_dma_buf, diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/i915_gem_dmabuf.c index 7998da27c500..b9355d8d4862 100644 --- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c +++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c @@ -213,6 +213,7 @@ static int i915_gem_begin_cpu_access(struct dma_buf *dma_buf, size_t start, size } static const struct dma_buf_ops i915_dmabuf_ops = { + .owner = THIS_MODULE, .map_dma_buf = i915_gem_map_dma_buf, .unmap_dma_buf = i915_gem_unmap_dma_buf, .release = drm_gem_dmabuf_release, diff --git a/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c b/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c index 344fd789170d..977ece9be62c 100644 --- a/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c +++ b/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c @@ -156,6 +156,7 @@ static int omap_gem_dmabuf_mmap(struct dma_buf *buffer, } static struct dma_buf_ops omap_dmabuf_ops = { + .owner = THIS_MO
Re: [Intel-gfx] [PATCH 06/10] drm/i915: Implement the intel_dp_autotest_edid function for DP EDID complaince tests
On Mon, May 04, 2015 at 07:48:20AM -0700, Todd Previte wrote: > Updates the EDID compliance test function to perform the analyze and react to > the EDID data read as a result of a hot plug event. The results of this > analysis are handed off to userspace so that the userspace app can set the > display mode appropriately for the test result/response. > > The compliance_test_active flag now appears at the end of the individual > test handling functions. This is so that the kernel-side operations can > be completed without the risk of interruption from the userspace app > that is polling on that flag. > > V2: > - Addressed mailing list feedback > - Removed excess debug messages > - Removed extraneous comments > - Fixed formatting issues (line length > 80) > - Updated the debug message in compute_edid_checksum to output hex values > instead of decimal > V3: > - Addressed more list feedback > - Added the test_active flag to the autotest function > - Removed test_active flag from handler > - Added failsafe check on the compliance test active flag > at the end of the test handler > - Fixed checkpatch.pl issues > V4: > - Removed the checksum computation function and its use as it has been > rendered superfluous by changes to the core DRM EDID functions > - Updated to use the raw header corruption detection mechanism > - Moved the declaration of the test_data variable here > V5: > - Update test active flag variable name to match the change in the > first patch of the series. > - Relocated the test active flag declaration and initialization > to this patch > V6: > - Updated to use the new flag for raw EDID header corruption > - Removed the extra EDID read from the autotest function > - Added the edid_checksum variable to struct intel_dp so that the > autotest function can write it to the sink device > - Moved the update to the hpd_pulse function to another patch > - Removed extraneous constants > V7: > - Fixed erroneous placement of the checksum assignment. In some cases > such as when the EDID read fails and is NULL, this causes a NULL ptr > dereference in the kernel. Bad news. Fixed now. > V8: > - Updated to support the kfree() on the EDID data added previously > V9: > - Updated for the long_hpd flag propagation > V10: > - Updated to use actual checksum from the EDID read that occurs during > normal hot plug path execution > - Removed variables from intel_dp struct that are no longer needed > - Updated the patch subject to more closely match the nature and contents > of the patch > - Fixed formatting problem (long line) > V11: > - Removed extra debug messages > - Updated comments to be more informative > - Removed extra variable > V12: > - Removed the 4 bit offset of the resolution setting in compliance data > - Changed to DRM_DEBUG_KMS instead of DRM_DEBUG_DRIVER > > Signed-off-by: Todd Previte > Reviewed-by: Paulo Zanoni Pulled in the remaining patches, thanks. Aside: Something seems to be amiss in your patch number when you resend. This one here is 6/10 in the original patch series which has only 5 patches ... -Daniel > --- > drivers/gpu/drm/i915/intel_dp.c | 42 > > drivers/gpu/drm/i915/intel_drv.h | 2 ++ > 2 files changed, 44 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index b249ee8..a59bd75 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -41,6 +41,12 @@ > > #define DP_LINK_CHECK_TIMEOUT(10 * 1000) > > +/* Compliance test status bits */ > +#define INTEL_DP_RESOLUTION_SHIFT_MASK 0 > +#define INTEL_DP_RESOLUTION_PREFERRED(1 << > INTEL_DP_RESOLUTION_SHIFT_MASK) > +#define INTEL_DP_RESOLUTION_STANDARD (2 << INTEL_DP_RESOLUTION_SHIFT_MASK) > +#define INTEL_DP_RESOLUTION_FAILSAFE (3 << INTEL_DP_RESOLUTION_SHIFT_MASK) > + > struct dp_link_dpll { > int link_bw; > struct dpll dpll; > @@ -3994,6 +4000,39 @@ static uint8_t intel_dp_autotest_video_pattern(struct > intel_dp *intel_dp) > static uint8_t intel_dp_autotest_edid(struct intel_dp *intel_dp) > { > uint8_t test_result = DP_TEST_NAK; > + struct intel_connector *intel_connector = intel_dp->attached_connector; > + struct drm_connector *connector = &intel_connector->base; > + > + if (intel_connector->detect_edid == NULL || > + connector->edid_corrupt == 1 || > + intel_dp->aux.i2c_defer_count > 6) { > + /* Check EDID read for NACKs, DEFERs and corruption > + * (DP CTS 1.2 Core r1.1) > + *4.2.2.4 : Failed EDID read, I2C_NAK > + *4.2.2.5 : Failed EDID read, I2C_DEFER > + *4.2.2.6 : EDID corruption detected > + * Use failsafe mode for all cases > + */ > + if (intel_dp->aux.i2c_nack_count > 0 || > + intel_dp->aux.i2c_defer_count > 0) > + DRM_DEBUG_KMS("EDID read had %d NACKs
Re: [Intel-gfx] [PATCH v3] drm/i915/bxt: eDP Panel Power sequencing
On Thu, 07 May 2015, Vandana Kannan wrote: > Changes for BXT - added a IS_BROXTON check to use the macro related to PPS > registers for BXT. > BXT does not have PP_DIV register. Making changes to handle this. > Second set of PPS registers have been defined but will be used when VBT > provides a selection between the 2 sets of registers. > > v2: > [Jani] Added 2nd set of PPS registers and the macro > Jani's review comments > - remove reference in i915_suspend.c > - Use BXT PP macro > Squashing all PPS related patches into one. > > v3: Jani's review comments addressed > - Use pp_ctl instead of pp > - ironlake_get_pp_control() is not required for BXT > - correct the use of && in the print statement > - drop the shift in the print statement > > Signed-off-by: Vandana Kannan > Signed-off-by: A.Sunil Kamath > --- > drivers/gpu/drm/i915/i915_reg.h | 13 > drivers/gpu/drm/i915/intel_dp.c | 74 > - > 2 files changed, 72 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 1b31238..777c51a 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -6315,6 +6315,8 @@ enum skl_disp_power_wells { > #define PCH_PP_CONTROL 0xc7204 > #define PANEL_UNLOCK_REGS (0xabcd << 16) > #define PANEL_UNLOCK_MASK (0x << 16) > +#define BXT_POWER_CYCLE_DELAY_MASK (0x1f0) > +#define BXT_POWER_CYCLE_DELAY_SHIFT 4 > #define EDP_FORCE_VDD (1 << 3) > #define EDP_BLC_ENABLE (1 << 2) > #define PANEL_POWER_RESET (1 << 1) > @@ -6343,6 +6345,17 @@ enum skl_disp_power_wells { > #define PANEL_POWER_CYCLE_DELAY_MASK(0x1f) > #define PANEL_POWER_CYCLE_DELAY_SHIFT 0 > > +/* BXT PPS changes - 2nd set of PPS registers */ > +#define _BXT_PP_STATUS2 0xc7300 > +#define _BXT_PP_CONTROL2 0xc7304 > +#define _BXT_PP_ON_DELAYS2 0xc7308 > +#define _BXT_PP_OFF_DELAYS2 0xc730c > + > +#define BXT_PP_STATUS(n) ((!n) ? PCH_PP_STATUS : _BXT_PP_STATUS2) > +#define BXT_PP_CONTROL(n)((!n) ? PCH_PP_CONTROL : _BXT_PP_CONTROL2) > +#define BXT_PP_ON_DELAYS(n) ((!n) ? PCH_PP_ON_DELAYS : _BXT_PP_ON_DELAYS2) > +#define BXT_PP_OFF_DELAYS(n) ((!n) ? PCH_PP_OFF_DELAYS : _BXT_PP_OFF_DELAYS2) > + > #define PCH_DP_B 0xe4100 > #define PCH_DPB_AUX_CH_CTL 0xe4110 > #define PCH_DPB_AUX_CH_DATA1 0xe4114 > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index bacdec5..3082224 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -559,7 +559,9 @@ static u32 _pp_ctrl_reg(struct intel_dp *intel_dp) > { > struct drm_device *dev = intel_dp_to_dev(intel_dp); > > - if (HAS_PCH_SPLIT(dev)) > + if (IS_BROXTON(dev)) > + return BXT_PP_CONTROL(0); > + else if (HAS_PCH_SPLIT(dev)) > return PCH_PP_CONTROL; > else > return VLV_PIPE_PP_CONTROL(vlv_power_sequencer_pipe(intel_dp)); > @@ -569,7 +571,9 @@ static u32 _pp_stat_reg(struct intel_dp *intel_dp) > { > struct drm_device *dev = intel_dp_to_dev(intel_dp); > > - if (HAS_PCH_SPLIT(dev)) > + if (IS_BROXTON(dev)) > + return BXT_PP_STATUS(0); > + else if (HAS_PCH_SPLIT(dev)) > return PCH_PP_STATUS; > else > return VLV_PIPE_PP_STATUS(vlv_power_sequencer_pipe(intel_dp)); > @@ -4969,8 +4973,8 @@ intel_dp_init_panel_power_sequencer(struct drm_device > *dev, > struct drm_i915_private *dev_priv = dev->dev_private; > struct edp_power_seq cur, vbt, spec, > *final = &intel_dp->pps_delays; > - u32 pp_on, pp_off, pp_div, pp; > - int pp_ctrl_reg, pp_on_reg, pp_off_reg, pp_div_reg; > + u32 pp_on, pp_off, pp_div = 0, pp_ctl = 0; > + int pp_ctrl_reg, pp_on_reg, pp_off_reg, pp_div_reg = 0; > > lockdep_assert_held(&dev_priv->pps_mutex); > > @@ -4978,7 +4982,16 @@ intel_dp_init_panel_power_sequencer(struct drm_device > *dev, > if (final->t11_t12 != 0) > return; > > - if (HAS_PCH_SPLIT(dev)) { > + if (IS_BROXTON(dev)) { > + /* > + * TODO: BXT has 2 sets of PPS registers. > + * Correct Register for Broxton need to be identified > + * using VBT. hardcoding for now > + */ > + pp_ctrl_reg = BXT_PP_CONTROL(0); > + pp_on_reg = BXT_PP_ON_DELAYS(0); > + pp_off_reg = BXT_PP_OFF_DELAYS(0); > + } else if (HAS_PCH_SPLIT(dev)) { > pp_ctrl_reg = PCH_PP_CONTROL; > pp_on_reg = PCH_PP_ON_DELAYS; > pp_off_reg = PCH_PP_OFF_DELAYS; > @@ -4994,12 +5007,17 @@ intel_dp_init_panel_power_sequencer(struct drm_device > *dev, > > /* Workaround: Need to write PP_CONTROL with the unlock key as >* the very first thing. */ > - pp = ironlake_ge
Re: [Intel-gfx] [PATCH 04/35] drm/i915: get rid of primary_enabled and use atomic state
On Tue, Apr 21, 2015 at 05:12:53PM +0300, Ander Conselvan de Oliveira wrote: > From: Maarten Lankhorst Generally a bit too terse commmit message. E.g. here a short summary of our irc discussion about why it should be ok to remove this feature is definitely needed. I added that. But I prefer more verbose commit messages in general. -Daniel > > Signed-off-by: Maarten Lankhorst > Reviewed-by: Ander Conselvan de Oliveira > > --- > drivers/gpu/drm/i915/intel_display.c | 50 > > drivers/gpu/drm/i915/intel_drv.h | 1 - > drivers/gpu/drm/i915/intel_fbc.c | 2 +- > 3 files changed, 29 insertions(+), 24 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index acf3494..40e3c62 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -2236,11 +2236,7 @@ static void intel_enable_primary_hw_plane(struct > drm_plane *plane, > > /* If the pipe isn't enabled, we can't pump pixels and may hang */ > assert_pipe_enabled(dev_priv, intel_crtc->pipe); > - > - if (intel_crtc->primary_enabled) > - return; > - > - intel_crtc->primary_enabled = true; > + to_intel_plane_state(plane->state)->visible = true; > > dev_priv->display.update_primary_plane(crtc, plane->fb, > crtc->x, crtc->y); > @@ -2661,6 +2657,8 @@ static void i9xx_update_primary_plane(struct drm_crtc > *crtc, > struct drm_device *dev = crtc->dev; > struct drm_i915_private *dev_priv = dev->dev_private; > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > + struct drm_plane *primary = crtc->primary; > + bool visible = to_intel_plane_state(primary->state)->visible; > struct drm_i915_gem_object *obj; > int plane = intel_crtc->plane; > unsigned long linear_offset; > @@ -2668,7 +2666,7 @@ static void i9xx_update_primary_plane(struct drm_crtc > *crtc, > u32 reg = DSPCNTR(plane); > int pixel_size; > > - if (!intel_crtc->primary_enabled || !fb) { > + if (!visible || !fb) { > I915_WRITE(reg, 0); > if (INTEL_INFO(dev)->gen >= 4) > I915_WRITE(DSPSURF(plane), 0); > @@ -2790,6 +2788,8 @@ static void ironlake_update_primary_plane(struct > drm_crtc *crtc, > struct drm_device *dev = crtc->dev; > struct drm_i915_private *dev_priv = dev->dev_private; > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > + struct drm_plane *primary = crtc->primary; > + bool visible = to_intel_plane_state(primary->state)->visible; > struct drm_i915_gem_object *obj; > int plane = intel_crtc->plane; > unsigned long linear_offset; > @@ -2797,7 +2797,7 @@ static void ironlake_update_primary_plane(struct > drm_crtc *crtc, > u32 reg = DSPCNTR(plane); > int pixel_size; > > - if (!intel_crtc->primary_enabled || !fb) { > + if (!visible || !fb) { > I915_WRITE(reg, 0); > I915_WRITE(DSPSURF(plane), 0); > POSTING_READ(reg); > @@ -2966,6 +2966,8 @@ static void skylake_update_primary_plane(struct > drm_crtc *crtc, > struct drm_device *dev = crtc->dev; > struct drm_i915_private *dev_priv = dev->dev_private; > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > + struct drm_plane *plane = crtc->primary; > + bool visible = to_intel_plane_state(plane->state)->visible; > struct drm_i915_gem_object *obj; > int pipe = intel_crtc->pipe; > u32 plane_ctl, stride_div, stride; > @@ -2973,9 +2975,8 @@ static void skylake_update_primary_plane(struct > drm_crtc *crtc, > unsigned int rotation; > int x_offset, y_offset; > unsigned long surf_addr; > - struct drm_plane *plane; > > - if (!intel_crtc->primary_enabled || !fb) { > + if (!visible || !fb) { > I915_WRITE(PLANE_CTL(pipe, 0), 0); > I915_WRITE(PLANE_SURF(pipe, 0), 0); > POSTING_READ(PLANE_CTL(pipe, 0)); > @@ -3035,7 +3036,6 @@ static void skylake_update_primary_plane(struct > drm_crtc *crtc, > > plane_ctl |= PLANE_CTL_PLANE_GAMMA_DISABLE; > > - plane = crtc->primary; > rotation = plane->state->rotation; > switch (rotation) { > case BIT(DRM_ROTATE_90): > @@ -4699,7 +4699,6 @@ static void intel_crtc_disable_planes(struct drm_crtc > *crtc) > hsw_disable_ips(intel_crtc); > > intel_crtc_dpms_overlay(intel_crtc, false); > - intel_crtc->primary_enabled = false; > for_each_intel_plane(dev, intel_plane) { > if (intel_plane->pipe == pipe) { > struct drm_crtc *from = intel_plane->base.crtc; > @@ -12803,6 +12802,9 @@ static int intel_crtc_set_config(struct drm_mode_set > *set) > } else if (config->fb_changed) { > struct intel_crtc *intel_crtc = to_intel_crtc(set->crtc); >
[Intel-gfx] [PULL] drm-intel-next
Hi Dave, drm-intel-next-2015-04-23: - dither support for ns2501 dvo (Thomas Richter) - some polish for the gtt code and fixes to finally enable the cmd parser on hsw - first pile of bxt stage 1 enabling (too many different people to list ...) - more psr fixes from Rodrigo - skl rotation support from Chandra - more atomic work from Ander and Matt - pile of cleanups and micro-ops for execlist from Chris drm-intel-next-2015-04-10: - cdclk handling cleanup and fixes from Ville - more prep patches for olr removal from John Harrison - gmbus pin naming rework from Jani (prep for bxt) - remove ->new_config from Ander (more atomic conversion work) - rps (boost) tuning and unification with byt/bsw from Chris - cmd parser batch bool tuning from Chris - gen8 dynamic pte allocation (Michel Thierry, based on work from Ben Widawsky) - execlist tuning (not yet all of it) from Chris - add drm_plane_from_index (Chandra) - various small things all over Plus a few patches Jani collected while I was on vacation. Cheers, Daniel The following changes since commit 1d8ac08d498d579aae36221a80b4b724b2f52f39: Merge tag 'v4.0-rc7' into drm-next (2015-04-09 07:48:27 +1000) are available in the git repository at: git://anongit.freedesktop.org/drm-intel tags/drm-intel-next-2015-04-23-fixed for you to fetch changes up to 93a96c6f049d047bc196890fc4284eff15b3770f: Merge commit '75d04a3773ecee617847de963ae4195d6aa74c28' into drm-intel-next-queued (2015-05-04 09:25:12 +0200) A.Sunil Kamath (1): drm/i915/bxt: Implement enable/disable for Display C9 state Ander Conselvan de Oliveira (10): drm/i915: Check lane sharing between pipes B & C using atomic state drm/i915: Set best_encoder field of connector_state also when disabling drm/i915: Don't use staged config for VLV cdclk calculations drm/i915: Don't use intel_crtc->new_config in pll calculation code drm/i915: Remove intel_crtc->new_config drm/i915: Don't use staged config in check_digital_port_conflicts() drm/i915: Don't use staged config in check_encoder_cloning() drm/i915: Don't use staged config in intel_mst_pre_enable_dp() drm/i915: Remove stale comment from __intel_set_mode() drm/i915: Allocate connector state together with the connectors Arun Siluvery (1): drm/i915: Do not set L3-LLC Coherency bit in ctx descriptor Ben Widawsky (3): drm/i915/bxt: add GEN8_HDCUNIT_CLOCK_GATE_DISABLE_HDCREQ workaround drm/i915/bxt: add WaDisableMaskBasedCammingInRCC workaround drm/i915/skl: add WaDisableMaskBasedCammingInRCC workaround Chandra Konduru (12): drm: Adding drm helper function drm_plane_from_index(). drm/i915: Register definitions for skylake scalers drm/i915: skylake scaler structure definitions drm/i915: Initialize plane colorkey to NONE drm/i915: Initialize skylake scalers drm/i915: Keep sprite plane src rect in 16.16 format drm/i915: Dump scaler_state too as part of dumping crtc_state drm/i915: Preserve scaler state when clearing crtc_state drm/i915: setup scalers for crtc_compute_config drm/i915: Ensure setting up scalers into staged crtc_state drm/i915: copy staged scaler state from drm state to crtc->config. drm/i915: skylake panel fitting using shared scalers Chris Wilson (35): drm/i915: Add i915_gem_request_unreference__unlocked drm/i915: Make debugfs/i915_gem_request more friendly drm/i915: Allow disabling the destination colorkey for overlay drm/i915: Cache last obj->pages location for i915_gem_object_get_page() drm/i915: Fix the flip synchronisation to consider mmioflips drm/i915: Agressive downclocking on Baytrail drm/i915: Fix computation of last_adjustment for RPS autotuning drm/i915: Boost GPU frequency if we detect outstanding pageflips drm/i915: Deminish contribution of wait-boosting from clients drm/i915: Re-enable RPS wait-boosting for all engines drm/i915: Split i915_gem_batch_pool into its own header drm/i915: Tidy batch pool logic drm/i915: Split the batch pool by engine drm/i915: Free batch pool when idle drm/i915: Split batch pool into size buckets drm/i915: Include active flag when describing objects in debugfs drm/i915: Suppress empty lines from debugfs/i915_gem_objects drm/i915: Record ring->start address in error state drm/i915: Use simpler form of spin_lock_irq(execlist_lock) drm/i915: Use the global runtime-pm wakelock for a busy GPU for execlists drm/i915: Remove vestigal DRI1 ring quiescing code drm/i915: Use a separate slab for requests drm/i915: Use a separate slab for vmas drm/i915: Reduce locking in execlist command submission drm/i915: Reduce locking in gen8 IRQ handler drm/i915: Tidy gen8 IRQ handler drm/i915: Prefer to check for idleness in worker ra
Re: [Intel-gfx] [PATCH] dma-buf: add ref counting for module as exporter
On Thu, May 07, 2015 at 01:00:52PM +0530, Sumit Semwal wrote: > Add reference counting on a kernel module that exports dma-buf and > implements its operations. This prevents the module from being unloaded > while DMABUF file is in use. > > The original patch [1] was submitted by Tomasz, but he's since shifted > jobs and a ping didn't elicit any response. > > [tomasz: Original author] > Signed-off-by: Tomasz Stanislawski > Acked-by: Daniel Vetter > [sumits: updated & rebased as per review comments] > Signed-off-by: Sumit Semwal > > [1]: https://lkml.org/lkml/2012/8/8/163 > --- > drivers/dma-buf/dma-buf.c | 9 - > drivers/gpu/drm/armada/armada_gem.c| 1 + > drivers/gpu/drm/drm_prime.c| 1 + > drivers/gpu/drm/exynos/exynos_drm_dmabuf.c | 1 + > drivers/gpu/drm/i915/i915_gem_dmabuf.c | 1 + > drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c | 1 + > drivers/gpu/drm/tegra/gem.c| 1 + > drivers/gpu/drm/udl/udl_dmabuf.c | 1 + > drivers/gpu/drm/vmwgfx/vmwgfx_prime.c | 1 + > drivers/media/v4l2-core/videobuf2-dma-contig.c | 1 + > drivers/media/v4l2-core/videobuf2-dma-sg.c | 1 + > drivers/media/v4l2-core/videobuf2-vmalloc.c| 1 + > drivers/staging/android/ion/ion.c | 1 + > include/linux/dma-buf.h| 2 ++ > 14 files changed, 22 insertions(+), 1 deletion(-) > > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c > index c5a9138a6a8d..9583eac0238f 100644 > --- a/drivers/dma-buf/dma-buf.c > +++ b/drivers/dma-buf/dma-buf.c > @@ -29,6 +29,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -64,6 +65,7 @@ static int dma_buf_release(struct inode *inode, struct file > *file) > BUG_ON(dmabuf->cb_shared.active || dmabuf->cb_excl.active); > > dmabuf->ops->release(dmabuf); > + module_put(dmabuf->ops->owner); > > mutex_lock(&db_list.lock); > list_del(&dmabuf->list_node); > @@ -302,9 +304,14 @@ struct dma_buf *dma_buf_export(const struct > dma_buf_export_info *exp_info) > return ERR_PTR(-EINVAL); > } > > + if (!try_module_get(exp_info->ops->owner)) > + return ERR_PTR(-ENOENT); > + > dmabuf = kzalloc(alloc_size, GFP_KERNEL); > - if (dmabuf == NULL) > + if (!dmabuf) { > + module_put(exp_info->ops->owner); > return ERR_PTR(-ENOMEM); > + } > > dmabuf->priv = exp_info->priv; > dmabuf->ops = exp_info->ops; > diff --git a/drivers/gpu/drm/armada/armada_gem.c > b/drivers/gpu/drm/armada/armada_gem.c > index 580e10acaa3a..d2c5fc1269b7 100644 > --- a/drivers/gpu/drm/armada/armada_gem.c > +++ b/drivers/gpu/drm/armada/armada_gem.c > @@ -524,6 +524,7 @@ armada_gem_dmabuf_mmap(struct dma_buf *buf, struct > vm_area_struct *vma) > } > > static const struct dma_buf_ops armada_gem_prime_dmabuf_ops = { > + .owner = THIS_MODULE, > .map_dma_buf= armada_gem_prime_map_dma_buf, > .unmap_dma_buf = armada_gem_prime_unmap_dma_buf, > .release= drm_gem_dmabuf_release, The "easier" way to do this is change the registration function to add the module owner automatically, which keeps you from having to modify all of the individual drivers. Look at how pci and usb do this for their driver registration functions. That should result in a much smaller patch, that always works properly for everyone (there's no way for driver to get it wrong.) thanks, greg k-h ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 00/35] Make legacy modeset a lot more atomic-like
On Tue, Apr 21, 2015 at 05:21:22PM +0200, Maarten Lankhorst wrote: > Op 21-04-15 om 16:12 schreef Ander Conselvan de Oliveira: > > Hi, > > > > This patch series changes the legacy modeset path to be a lot more > > atomic like. Among other things, it > > > > - unifies the flip-only and the modeset path; > > - implements a full state swap as part of the modeset; > > - gets rid of the recovery logic in case of a failed modeset; > > - replaces some i915 functions with drm atomic helper ones. > > > > Some of these patches were sent previously and the feedback so far has > > been addressed. I also rebased this on top Maarten's series that > > simplifies plane enabling/disabling. > > > > Thanks, > > Ander > > > > Ander Conselvan de Oliveira (28): > > drm/i915: Don't check for NULL before freeing state > > drm/i915: Call drm helpers when duplicating crtc and plane states > > drm/i915: Use for_each_connector_in_state helper macro > > drm/i915: Extract mode_changed computation out of > > stage_output_config() > > drm/i915: Add crtc states before calling compute_config() > > drm/i915: Don't pretend we can calculate multiple pipe_configs > > drm/i915: Calculate a new pipe_config based on new enabled state > > drm/i915: Remove all *_pipes flags from modeset > > drm/i915: Remove saved_mode from __intel_set_mode() > > drm/i915: Move compute part of __intel_set_mode() to separate function > > drm/i915: Simplify error handling in __intel_set_mode() > > drm/i915: Don't modeset with old mode when set_crtc fails > > drm/i915: Add primary plane to atomic state in legacy modeset > > drm/i915: Delete fb, x and y parameters from mode set functions > > drm/i915: Don't use struct intel_set_config *_changed flags > > drm/i915: Don't use staged config to calculate mode_changed flags > > drm/i915: Unify modeset and flip paths of intel_crtc_set_config() > > drm/i915: Simplify intel_set_config_compute_mode_changes() a bit > > drm/i915: Stage new modeset state straight into atomic state > > drm/i915: Remove save/restore logic from intel_crtc_set_config() > > drm/i915: Update crtc state active flag based on DPMS > > drm/atomic: Make mode_fixup() optional for check_modeset() > > drm/i915: Use atomic helpers for computing changed flags > > drm/i915: Take ownership of atomic state on success in > > intel_set_mode() > > drm/i915: Preserve shared DPLL information in new pipe_config > > drm/i915: Don't use plane update helper in legacy mode set > > drm/i915: Swap atomic state in legacy modeset > > drm/i915: Get rid of intel_crtc_set_state() > For the whole series: > Reviewed-by: Maarten Lankhorst > > \o/ fixing up atomic planes should be a lot more trivial now, killing the > last of the plane helpers. Pulled them all in now, thanks. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/i915: Sink rate read should be saved in deca-kHz
On Thu, 07 May 2015, Sonika Jindal wrote: > The sink rate read from supported link rate table is in KHz as per spec > while in drm, the saved clock is in deca-KHz. So divide the link rate by > 10 before storing. Please refer to the commit which broke things. git blame is your friend. We need the information to know which kernel version the bug has landed or is about to land to, so we can queue the fix in the same place. If developers don't do this, maintainers will have to anyway, which doesn't scale well, which leads to grumpy maintainers. ;) Thanks, Jani. > > Cc: Ville Syrjälä > Signed-off-by: Sonika Jindal > --- > Just resending it along with the other intermediate link rate patches > (It was posted orginally on 21st April) > > Thanks, > Sonika > > drivers/gpu/drm/i915/intel_dp.c |3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index bacdec5..6bd5afb 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -3906,7 +3906,8 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp) > if (val == 0) > break; > > - intel_dp->sink_rates[i] = val * 200; > + /* Value read is in kHz while drm clock is saved in > deca-kHz */ > + intel_dp->sink_rates[i] = (val * 200) / 10; > } > intel_dp->num_sink_rates = i; > } > -- > 1.7.10.4 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Demote hangcheck ring idle warning
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 6325 -Summary- Platform Delta drm-intel-nightly Series Applied PNV 276/276 276/276 ILK 302/302 302/302 SNB 316/316 316/316 IVB 342/342 342/342 BYT 286/286 286/286 BDW 321/321 321/321 -Detailed- Platform Testdrm-intel-nightly Series Applied Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/i915: Sink rate read should be saved in deca-kHz
On Thu, May 07, 2015 at 11:10:30AM +0300, Jani Nikula wrote: > On Thu, 07 May 2015, Sonika Jindal wrote: > > The sink rate read from supported link rate table is in KHz as per spec > > while in drm, the saved clock is in deca-KHz. So divide the link rate by > > 10 before storing. > > Please refer to the commit which broke things. git blame is your > friend. We need the information to know which kernel version the bug has > landed or is about to land to, so we can queue the fix in the same > place. > > If developers don't do this, maintainers will have to anyway, which > doesn't scale well, which leads to grumpy maintainers. ;) Indeed, now I wonder if I broke it during the refactoring or was it already like that before that. Hmm. Doesn't look like it was me, but I did fail to spot it during the review :( Anyway, with the commit msg amended to Jani's liking this is: Reviewed-by: Ville Syrjälä > > Thanks, > Jani. > > > > > > Cc: Ville Syrjälä > > Signed-off-by: Sonika Jindal > > --- > > Just resending it along with the other intermediate link rate patches > > (It was posted orginally on 21st April) > > > > Thanks, > > Sonika > > > > drivers/gpu/drm/i915/intel_dp.c |3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c > > b/drivers/gpu/drm/i915/intel_dp.c > > index bacdec5..6bd5afb 100644 > > --- a/drivers/gpu/drm/i915/intel_dp.c > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > @@ -3906,7 +3906,8 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp) > > if (val == 0) > > break; > > > > - intel_dp->sink_rates[i] = val * 200; > > + /* Value read is in kHz while drm clock is saved in > > deca-kHz */ > > + intel_dp->sink_rates[i] = (val * 200) / 10; > > } > > intel_dp->num_sink_rates = i; > > } > > -- > > 1.7.10.4 > > > > ___ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Jani Nikula, Intel Open Source Technology Center > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/3] drm/i915: Rename dp rates array as per platform
On Thu, May 07, 2015 at 09:52:08AM +0530, Sonika Jindal wrote: > Renaming gen9_rates to skl_rates because other platforms may have different > supported rates. > > Signed-off-by: Sonika Jindal Reviewed-by: Ville Syrjälä > --- > drivers/gpu/drm/i915/intel_dp.c | 10 +- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 6bd5afb..c9d50d1 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -84,8 +84,8 @@ static const struct dp_link_dpll chv_dpll[] = { > { DP_LINK_BW_5_4, /* m2_int = 27, m2_fraction = 0 */ > { .p1 = 2, .p2 = 1, .n = 1, .m1 = 2, .m2 = 0x6c0 } } > }; > -/* Skylake supports following rates */ > -static const int gen9_rates[] = { 162000, 216000, 27, > + > +static const int skl_rates[] = { 162000, 216000, 27, > 324000, 432000, 54 }; > static const int chv_rates[] = { 162000, 202500, 21, 216000, >243000, 27, 324000, 405000, > @@ -1161,9 +1161,9 @@ intel_dp_sink_rates(struct intel_dp *intel_dp, const > int **sink_rates) > static int > intel_dp_source_rates(struct drm_device *dev, const int **source_rates) > { > - if (INTEL_INFO(dev)->gen >= 9) { > - *source_rates = gen9_rates; > - return ARRAY_SIZE(gen9_rates); > + if (IS_SKYLAKE(dev)) { > + *source_rates = skl_rates; > + return ARRAY_SIZE(skl_rates); > } else if (IS_CHERRYVIEW(dev)) { > *source_rates = chv_rates; > return ARRAY_SIZE(chv_rates); > -- > 1.7.10.4 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Sink rate read should be saved in deca-kHz
The sink rate read from supported link rate table is in KHz as per spec while in drm, the saved clock is in deca-KHz. So divide the link rate by 10 before storing. Reading of rates was added by: commit fc0f8e25318f ("drm/i915/skl: Read sink supported rates from edp panel") Cc: Ville Syrjälä Signed-off-by: Sonika Jindal Reviewed-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_dp.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index bacdec5..6bd5afb 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -3906,7 +3906,8 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp) if (val == 0) break; - intel_dp->sink_rates[i] = val * 200; + /* Value read is in kHz while drm clock is saved in deca-kHz */ + intel_dp->sink_rates[i] = (val * 200) / 10; } intel_dp->num_sink_rates = i; } -- 1.7.10.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/3] drm/i915/bxt: edp1.4 Intermediate Freq support
On Thu, May 07, 2015 at 09:52:09AM +0530, Sonika Jindal wrote: > BXT supports following intermediate link rates for edp: > 2.16GHz, 2.43GHz, 3.24GHz, 4.32GHz. > Adding support for programming the intermediate rates. > > Signed-off-by: Sonika Jindal > --- > drivers/gpu/drm/i915/intel_ddi.c | 44 > -- > drivers/gpu/drm/i915/intel_dp.c |7 +- > 2 files changed, 48 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c > b/drivers/gpu/drm/i915/intel_ddi.c > index 9c1e74a..c0cb5f7 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -1397,8 +1397,7 @@ bxt_ddi_pll_select(struct intel_crtc *intel_crtc, > clk_div.lanestagger = 0x04; > else > clk_div.lanestagger = 0x02; > - } else if (intel_encoder->type == INTEL_OUTPUT_DISPLAYPORT || > - intel_encoder->type == INTEL_OUTPUT_EDP) { > + } else if (intel_encoder->type == INTEL_OUTPUT_DISPLAYPORT) { > struct drm_encoder *encoder = &intel_encoder->base; > struct intel_dp *intel_dp = enc_to_intel_dp(encoder); > > @@ -1416,8 +1415,49 @@ bxt_ddi_pll_select(struct intel_crtc *intel_crtc, > clk_div = bxt_dp_clk_val[0]; > DRM_ERROR("Unknown link rate\n"); > } > + } else if (intel_encoder->type == INTEL_OUTPUT_EDP) { > + struct drm_encoder *encoder = &intel_encoder->base; > + struct intel_dp *intel_dp = enc_to_intel_dp(encoder); > + int link_rate; > + > + /* > + * If edp1.4 intermediate frequency support is present, we set > + * link_bw to 0 and a valid rate index in rate_select. > + */ > + if (intel_dp->link_bw) > + link_rate = > drm_dp_bw_code_to_link_rate(intel_dp->link_bw); > + else > + link_rate = intel_dp->sink_rates[intel_dp->rate_select]; The chosen clock should already be passed in, so no there should be no need for this. I see the DP case does has the same issue. > + > + switch (link_rate) { > + case 162000: > + clk_div = bxt_dp_clk_val[0]; > + break; > + case 216000: > + clk_div = bxt_dp_clk_val[3]; > + break; > + case 243000: > + clk_div = bxt_dp_clk_val[4]; > + break; > + case 27: > + clk_div = bxt_dp_clk_val[1]; > + break; > + case 324000: > + clk_div = bxt_dp_clk_val[5]; > + break; > + case 432000: > + clk_div = bxt_dp_clk_val[6]; > + break; > + case 54: > + clk_div = bxt_dp_clk_val[2]; > + break; > + default: > + clk_div = bxt_dp_clk_val[0]; > + DRM_ERROR("Unknown link rate\n"); > + } This looks rather fragile. I would suggest storing the link rate in the bxt_clk_div structure and just looping through the array looking for the correct rate. That will also work for normal DP, so less code in the end. > } > > + Spurious whitespace. > crtc_state->dpll_hw_state.ebb0 = > PORT_PLL_P1(clk_div.p1) | PORT_PLL_P2(clk_div.p2); > crtc_state->dpll_hw_state.pll0 = clk_div.m2_int; > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index c9d50d1..e6ee7c6 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -85,6 +85,8 @@ static const struct dp_link_dpll chv_dpll[] = { > { .p1 = 2, .p2 = 1, .n = 1, .m1 = 2, .m2 = 0x6c0 } } > }; > > +static const int bxt_rates[] = { 162000, 216000, 243000, 27, > + 324000, 432000, 54 }; > static const int skl_rates[] = { 162000, 216000, 27, > 324000, 432000, 54 }; > static const int chv_rates[] = { 162000, 202500, 21, 216000, > @@ -1161,7 +1163,10 @@ intel_dp_sink_rates(struct intel_dp *intel_dp, const > int **sink_rates) > static int > intel_dp_source_rates(struct drm_device *dev, const int **source_rates) > { > - if (IS_SKYLAKE(dev)) { > + if (IS_BROXTON(dev)) { > + *source_rates = bxt_rates; > + return ARRAY_SIZE(bxt_rates); > + } else if (IS_SKYLAKE(dev)) { > *source_rates = skl_rates; > return ARRAY_SIZE(skl_rates); > } else if (IS_CHERRYVIEW(dev)) { > -- > 1.7.10.4 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC ___
Re: [Intel-gfx] [PATCH i-g-t 4/4] kms_plane_scaling: Find the image regardless how the test is run
On 05/06/2015 07:17 PM, Konduru, Chandra wrote: -Original Message- From: Tvrtko Ursulin [mailto:tvrtko.ursu...@linux.intel.com] Sent: Wednesday, May 06, 2015 2:29 AM To: Konduru, Chandra; Intel-gfx@lists.freedesktop.org Cc: Ursulin, Tvrtko; Wood, Thomas Subject: Re: [PATCH i-g-t 4/4] kms_plane_scaling: Find the image regardless how the test is run On 05/05/2015 06:22 PM, Konduru, Chandra wrote: -Original Message- From: Tvrtko Ursulin [mailto:tvrtko.ursu...@linux.intel.com] Sent: Tuesday, May 05, 2015 2:53 AM To: Intel-gfx@lists.freedesktop.org Cc: Ursulin, Tvrtko; Konduru, Chandra; Wood, Thomas Subject: [PATCH i-g-t 4/4] kms_plane_scaling: Find the image regardless how the test is run From: Tvrtko Ursulin As it stands running the test like "sudo tests/kms_plane_scaling" does not work. Fix it by using the same method igt_paint_image uses. As image size is required in other tests too, in recent patch I added igt_get_image_size() which can be used here instead duplication. Ok, please do, I only attempted to fix it along the way since it did not run for me. And at the time there was no igt_get_image_size. Yeah, you are right. Sure I have few things to changes to do anyway for adding rotation to kms_plane_scaling. Along with that, I will make above change. In that case, I presume you drop this patch. Dropping happily. Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Sink rate read should be saved in deca-kHz
On Thu, 07 May 2015, Sonika Jindal wrote: > The sink rate read from supported link rate table is in KHz as per spec > while in drm, the saved clock is in deca-KHz. So divide the link rate by > 10 before storing. > > Reading of rates was added by: > commit fc0f8e25318f ("drm/i915/skl: Read sink supported rates from edp > panel") Thanks; now this git spell $ git tag --contains fc0f8e25318f | grep ^v[0-9] | sort -V | head -n 1 v4.1-rc1 will tell me I need to queue this one to v4.1 through drm-intel-fixes while the rest is for Daniel to merge to drm-intel-next-queued. Pushed, thanks for the patch and review. BR, Jani. > > Cc: Ville Syrjälä > Signed-off-by: Sonika Jindal > Reviewed-by: Ville Syrjälä > --- > drivers/gpu/drm/i915/intel_dp.c |3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index bacdec5..6bd5afb 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -3906,7 +3906,8 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp) > if (val == 0) > break; > > - intel_dp->sink_rates[i] = val * 200; > + /* Value read is in kHz while drm clock is saved in > deca-kHz */ > + intel_dp->sink_rates[i] = (val * 200) / 10; > } > intel_dp->num_sink_rates = i; > } > -- > 1.7.10.4 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/3] drm/i915/bxt: edp1.4 Intermediate Freq support
On 5/7/2015 2:11 PM, Ville Syrjälä wrote: On Thu, May 07, 2015 at 09:52:09AM +0530, Sonika Jindal wrote: BXT supports following intermediate link rates for edp: 2.16GHz, 2.43GHz, 3.24GHz, 4.32GHz. Adding support for programming the intermediate rates. Signed-off-by: Sonika Jindal --- drivers/gpu/drm/i915/intel_ddi.c | 44 -- drivers/gpu/drm/i915/intel_dp.c |7 +- 2 files changed, 48 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index 9c1e74a..c0cb5f7 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -1397,8 +1397,7 @@ bxt_ddi_pll_select(struct intel_crtc *intel_crtc, clk_div.lanestagger = 0x04; else clk_div.lanestagger = 0x02; - } else if (intel_encoder->type == INTEL_OUTPUT_DISPLAYPORT || - intel_encoder->type == INTEL_OUTPUT_EDP) { + } else if (intel_encoder->type == INTEL_OUTPUT_DISPLAYPORT) { struct drm_encoder *encoder = &intel_encoder->base; struct intel_dp *intel_dp = enc_to_intel_dp(encoder); @@ -1416,8 +1415,49 @@ bxt_ddi_pll_select(struct intel_crtc *intel_crtc, clk_div = bxt_dp_clk_val[0]; DRM_ERROR("Unknown link rate\n"); } + } else if (intel_encoder->type == INTEL_OUTPUT_EDP) { + struct drm_encoder *encoder = &intel_encoder->base; + struct intel_dp *intel_dp = enc_to_intel_dp(encoder); + int link_rate; + + /* +* If edp1.4 intermediate frequency support is present, we set +* link_bw to 0 and a valid rate index in rate_select. +*/ + if (intel_dp->link_bw) + link_rate = drm_dp_bw_code_to_link_rate(intel_dp->link_bw); + else + link_rate = intel_dp->sink_rates[intel_dp->rate_select]; The chosen clock should already be passed in, so no there should be no need for this. I see the DP case does has the same issue. Yes it was copied from DP :) + + switch (link_rate) { + case 162000: + clk_div = bxt_dp_clk_val[0]; + break; + case 216000: + clk_div = bxt_dp_clk_val[3]; + break; + case 243000: + clk_div = bxt_dp_clk_val[4]; + break; + case 27: + clk_div = bxt_dp_clk_val[1]; + break; + case 324000: + clk_div = bxt_dp_clk_val[5]; + break; + case 432000: + clk_div = bxt_dp_clk_val[6]; + break; + case 54: + clk_div = bxt_dp_clk_val[2]; + break; + default: + clk_div = bxt_dp_clk_val[0]; + DRM_ERROR("Unknown link rate\n"); + } This looks rather fragile. I would suggest storing the link rate in the bxt_clk_div structure and just looping through the array looking for the correct rate. That will also work for normal DP, so less code in the end. Ok, I will do that. } + Spurious whitespace. :( crtc_state->dpll_hw_state.ebb0 = PORT_PLL_P1(clk_div.p1) | PORT_PLL_P2(clk_div.p2); crtc_state->dpll_hw_state.pll0 = clk_div.m2_int; diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index c9d50d1..e6ee7c6 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -85,6 +85,8 @@ static const struct dp_link_dpll chv_dpll[] = { { .p1 = 2, .p2 = 1, .n = 1, .m1 = 2, .m2 = 0x6c0 } } }; +static const int bxt_rates[] = { 162000, 216000, 243000, 27, + 324000, 432000, 54 }; static const int skl_rates[] = { 162000, 216000, 27, 324000, 432000, 54 }; static const int chv_rates[] = { 162000, 202500, 21, 216000, @@ -1161,7 +1163,10 @@ intel_dp_sink_rates(struct intel_dp *intel_dp, const int **sink_rates) static int intel_dp_source_rates(struct drm_device *dev, const int **source_rates) { - if (IS_SKYLAKE(dev)) { + if (IS_BROXTON(dev)) { + *source_rates = bxt_rates; + return ARRAY_SIZE(bxt_rates); + } else if (IS_SKYLAKE(dev)) { *source_rates = skl_rates; return ARRAY_SIZE(skl_rates); } else if (IS_CHERRYVIEW(dev)) { -- 1.7.10.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx _
Re: [Intel-gfx] [PATCH v2 i-g-t 1/4] igt_kms: Avoid NULL ptr deref when commiting disabled planes
On 05/06/2015 09:47 PM, Konduru, Chandra wrote: -Original Message- From: Tvrtko Ursulin [mailto:tvrtko.ursu...@linux.intel.com] Sent: Tuesday, May 05, 2015 2:53 AM To: Intel-gfx@lists.freedesktop.org Cc: Ursulin, Tvrtko; Konduru, Chandra; Wood, Thomas Subject: [PATCH v2 i-g-t 1/4] igt_kms: Avoid NULL ptr deref when commiting disabled planes From: Tvrtko Ursulin I think; commit a26f9f9ad0e679c7ce413a25d34f6914e1174151 Author: chandra konduru Date: Mon Mar 30 13:52:04 2015 -0700 i-g-t: Adding plane scaling test case introduced a condition where it attempts to update a disabled plane because of the newly introduced size_changed flag which is set for disabled frame buffers. Result is a NULL ptr deref in igt_drm_plane_commit (plane->fb->src_x). Start recognising this case as disabled plane and act accordingly. v2: Split out igt_plane_set_fb cleanup. (Thomas Wood) Signed-off-by: Tvrtko Ursulin Cc: chandra konduru Cc: Thomas Wood --- There might be a better fix, but this works for me. --- lib/igt_kms.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/igt_kms.c b/lib/igt_kms.c index b7d1e90..33d437d 100644 --- a/lib/igt_kms.c +++ b/lib/igt_kms.c @@ -1331,7 +1331,7 @@ static int igt_drm_plane_commit(igt_plane_t *plane, fb_id = igt_plane_get_fb_id(plane); crtc_id = output->config.crtc->crtc_id; - if (plane->fb_changed && fb_id == 0) { + if ((plane->fb_changed || plane->size_changed) && fb_id == 0) { Shouldn't this include plane->position_changed too? Like: if ((plane->fb_changed || plane->size_changed || plane->position_changed) && fb_id == 0) { When you added size_changed, state for position_changed and fb == NULL remained the same, while size_changed added new state for size_changed == true and fb == NULL. So I added handling for that and did not think much beyond it. It fixes a segfault so I moved on. Or in other words, I don't see how it would harm to merge this, it doesn't make anything worse. Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t 2/4] igt_kms: Merge condition in igt_plane_set_fb
On 05/06/2015 09:56 PM, Konduru, Chandra wrote: @@ -1765,14 +1765,6 @@ void igt_plane_set_fb(igt_plane_t *plane, struct igt_fb *fb) plane->fb = fb; /* hack to keep tests working that don't call igt_plane_set_size() */ if (fb) { - plane->crtc_w = fb->width; - plane->crtc_h = fb->height; - } else { - plane->crtc_w = 0; - plane->crtc_h = 0; - } - - if (fb) { /* set default plane pos/size as fb size */ plane->crtc_x = 0; plane->crtc_y = 0; @@ -1784,6 +1776,9 @@ void igt_plane_set_fb(igt_plane_t *plane, struct igt_fb *fb) fb->src_y = 0; fb->src_w = fb->width; fb->src_h = fb->height; + } else { + plane->crtc_w = 0; + plane->crtc_h = 0; } Existing code is simply setting fb src position and plane crtc position to 0s (top left) and src size as fb size and crtc size as plane size to start a fb with a plane. Then individual test can change them to whatever fb position/size and plane position/size as it wants. As I commented to 3/4 patch, if these initializations are removed, then all tests to be updated to explicitly set them. Not sure what you mean. I simply cleaned two "if (fb)" conditions one after another, into one. No functional changes. As a side note, is there any reason for having two patches 2/4 and 3/4 modifying same lines of code instead of a single patch? Because this is just a code cleanup and the other was a functional change. And because it doesn't matter - lets not spend hours going back and forth on trivial IGT fixes. Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fix screen flickering on X
On Thu, 23 Apr 2015, Chris Wilson wrote: > [cc'ing the authors] This has been posted earlier [1] and it has review to be addressed [2]. BR, Jani. [1] http://mid.gmane.org/1428790644-6812-1-git-send-email-ism...@iodev.co.uk [2] http://mid.gmane.org/1428928418.2654.8.ca...@gmail.com > > On Sat, Apr 11, 2015 at 07:40:34PM -0300, Ismael Luceno wrote: >> A bisect showed that commit 32b7eeec4d1e861230b09d437e95d76c86ff4a68 >> introduced the issue. >> >> The issue starts as soon as X takes control of the screen, even if just >> a plain X doing nothing, so based on the code touched by the commit I >> thought it had to be related to the so called "hardware cursor". I >> confirmed it when hiding the cursor made the flickering go away. >> >> The aforementioned commit removed some suspicious code, and the >> Programmer's Reference Manual confirmed my suspicion: >> >> "Incorrectly programmed watermark values can result in screen corruption. >> >> The watermarks should be calculated and programmed when any of the >> watermark calculation inputs change. This includes planes enabling or >> disabling, plane source format or size changing, etc." >> >> So I'm re-adding the few lines that update the watermarks after a cursor >> size change. >> >> Signed-off-by: Ismael Luceno >> --- >> drivers/gpu/drm/i915/intel_display.c | 7 ++- >> 1 file changed, 6 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_display.c >> b/drivers/gpu/drm/i915/intel_display.c >> index f75173c..e23f062 100644 >> --- a/drivers/gpu/drm/i915/intel_display.c >> +++ b/drivers/gpu/drm/i915/intel_display.c >> @@ -12258,6 +12258,7 @@ intel_commit_cursor_plane(struct drm_plane *plane, >> struct intel_crtc *intel_crtc; >> struct intel_plane *intel_plane = to_intel_plane(plane); >> struct drm_i915_gem_object *obj = intel_fb_obj(state->base.fb); >> +unsigned old_width; >> uint32_t addr; >> >> crtc = crtc ? crtc : plane->crtc; >> @@ -12282,11 +12283,15 @@ intel_commit_cursor_plane(struct drm_plane *plane, >> intel_crtc->cursor_addr = addr; >> intel_crtc->cursor_bo = obj; >> update: >> +old_width = intel_crtc->cursor_width; >> intel_crtc->cursor_width = state->base.crtc_w; >> intel_crtc->cursor_height = state->base.crtc_h; >> >> -if (intel_crtc->active) >> +if (intel_crtc->active) { >> +if (old_width != intel_crtc->cursor_width) >> +intel_update_watermarks(crtc); >> intel_crtc_update_cursor(crtc, state->visible); >> +} >> } >> >> static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev, >> -- >> 2.3.4 > > -- > Chris Wilson, Intel Open Source Technology Centre > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] igt/gem_create_stolen: Verifying extended gem_create ioctl
On Thu, May 07, 2015 at 08:52:54AM +0200, Daniel Vetter wrote: > On Wed, May 06, 2015 at 03:51:52PM +0530, ankitprasad.r.sha...@intel.com > wrote: > > From: Ankitprasad Sharma > > > > This patch adds the testcases for verifying the new extended > > gem_create ioctl. By means of this extended ioctl, memory > > placement of the GEM object can be specified, i.e. either > > shmem or stolen memory. > > These testcases include functional tests and interface tests for > > testing the gem_create ioctl call for stolen memory placement > > > > v2: Testing pread/pwrite functionality for stolen backed objects, > > added local struct for extended gem_create and gem_get_aperture, > > until headers catch up (Chris) > > > > v3: Removed get_aperture related functions, extended gem_pread > > to compare speeds for user pages with and without page faults, > > unexposed local_gem_create struct, changed gem_create_stolen > > usage (Chris) > > > > Signed-off-by: Ankitprasad Sharma > > An igt to check for invalid arguments of the gem create ioctl (especially > the newly added flags parameters) seems to be missing. If we do that, I would actually create gem_create.c to do the parameter testing of CREATE, and rename this to gem_stolen.c as this covers the functional side of using stolen (i.e. not limited to testing the CREATE API). And I want a pink pony. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t 3/4] igt_kms: Do not reset plane position on assigning a fb
On 05/06/2015 08:02 PM, Konduru, Chandra wrote: diff --git a/lib/igt_kms.c b/lib/igt_kms.c index b5ba273..0665d70 100644 --- a/lib/igt_kms.c +++ b/lib/igt_kms.c @@ -1765,9 +1765,7 @@ void igt_plane_set_fb(igt_plane_t *plane, struct igt_fb *fb) plane->fb = fb; /* hack to keep tests working that don't call igt_plane_set_size() */ if (fb) { - /* set default plane pos/size as fb size */ - plane->crtc_x = 0; - plane->crtc_y = 0; Reason for doing this is to make existing kms tests to run without modification. Otherwise every existing test has to explicitly call igt_plane_set_position to make sure it works. Can you make sure removing above 2 lines doesn't break existing tests? Hm, before your patch tests could set a plane position, change the fb, and plane position would remain the same. After your patch changing the fb resets the plane position. My patch reverts this behaviour to old one. So I am not sure in what cases resetting plane position on fb changes helps and which tests? Which tests do you think should be run? Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/skl: Add module parameter to select edp vswing table
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 6326 -Summary- Platform Delta drm-intel-nightly Series Applied PNV 276/276 276/276 ILK 302/302 302/302 SNB 316/316 316/316 IVB 342/342 342/342 BYT 286/286 286/286 BDW 321/321 321/321 -Detailed- Platform Testdrm-intel-nightly Series Applied Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 1/2] drm/i915/bxt: Port PLL programming BUN
On to, 2015-05-07 at 12:00 +0530, Vandana Kannan wrote: > BUN 1: prop_coeff, int_coeff, tdctargetcnt programming updated and tied to > VCO frequencies. Program i_lockthresh in PORT_PLL_9. > > VCO calculated based on the formula: > Desired Output = Port bit rate in MHz (DisplayPort HBR2 is 5400 MHz) > Fast Clock = Desired Output / 2 > VCO = Fast Clock * P1 * P2 > > Prop_coeff, int_coeff, and tdctargetcnt modified according to above > calculation. > > BUN 2: Port PLLs require additional programming at certain frequencies - > DCO amplitude in PORT_PLL_10 > > Review comments from Siva which were addressed in the initial version of the > patch. > - Change PORT_PLL_LOCK_THRESHOLD to PORT_PLL_LOCK_THRESHOLD_MASK > - Calculate for HDMI > - Correct values for vco = 5.4 > - return in case of invalid vco range > > v2: Imre's review comments addressed > - change dcoampovr_en to dcoampovr_en_h > - change PORT_PLL_DCO_AMP_OVR_EN to PORT_PLL_DCO_AMP_OVR_EN_H > - Correct lane stagger value for 324MHz > - Make coef common for HDMI and DP > - remove superfluous comments > > Signed-off-by: Vandana Kannan > Reviewed-by: Sivakumar Thulasimani > Cc: Sivakumar Thulasimani > --- > drivers/gpu/drm/i915/i915_drv.h | 2 +- > drivers/gpu/drm/i915/i915_reg.h | 6 > drivers/gpu/drm/i915/intel_ddi.c | 69 > ++-- > 3 files changed, 60 insertions(+), 17 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 136d42a..05a4e1c 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -304,7 +304,7 @@ struct intel_dpll_hw_state { > uint32_t cfgcr1, cfgcr2; > > /* bxt */ > - uint32_t ebb0, pll0, pll1, pll2, pll3, pll6, pll8, pcsdw12; > + uint32_t ebb0, pll0, pll1, pll2, pll3, pll6, pll8, pll10, pcsdw12; > }; > > struct intel_shared_dpll_config { > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 1b31238..b086d26 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -1185,6 +1185,12 @@ enum skl_disp_power_wells { > #define PORT_PLL_GAIN_CTL(x) ((x) << 16) > /* PORT_PLL_8_A */ > #define PORT_PLL_TARGET_CNT_MASK 0x3FF > +/* PORT_PLL_9_A */ > +#define PORT_PLL_LOCK_THRESHOLD_MASK0xe > +/* PORT_PLL_10_A */ > +#define PORT_PLL_DCO_AMP_OVR_EN_H (1<<27) > +#define PORT_PLL_DCO_AMP_MASK 0x3c00 > +#define PORT_PLL_DCO_AMP(x) (x<<10) > #define _PORT_PLL_BASE(port) _PORT3(port, _PORT_PLL_0_A, \ > _PORT_PLL_0_B, \ > _PORT_PLL_0_C) > diff --git a/drivers/gpu/drm/i915/intel_ddi.c > b/drivers/gpu/drm/i915/intel_ddi.c > index 9c1e74a..49b9fd8 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -1337,18 +1337,20 @@ struct bxt_clk_div { > uint32_t int_coef; > uint32_t gain_ctl; > uint32_t targ_cnt; > + uint32_t dcoampovr_en_h; > + uint32_t dco_amp; > uint32_t lanestagger; > }; > > /* pre-calculated values for DP linkrates */ > static struct bxt_clk_div bxt_dp_clk_val[7] = { > - /* 162 */ {4, 2, 32, 1677722, 1, 1, 5, 11, 2, 9, 0xd}, > - /* 270 */ {4, 1, 27, 0, 0, 1, 3, 8, 1, 9, 0xd}, > - /* 540 */ {2, 1, 27, 0, 0, 1, 3, 8, 1, 9, 0x18}, > - /* 216 */ {3, 2, 32, 1677722, 1, 1, 5, 11, 2, 9, 0xd}, > - /* 243 */ {4, 1, 24, 1258291, 1, 1, 5, 11, 2, 9, 0xd}, > - /* 324 */ {4, 1, 32, 1677722, 1, 1, 5, 11, 2, 9, 0xd}, > - /* 432 */ {3, 1, 32, 1677722, 1, 1, 5, 11, 2, 9, 0x18} > + /* 162 */ {4, 2, 32, 1677722, 1, 1, 4, 9, 3, 8, 0, 15, 0xd}, > + /* 270 */ {4, 1, 27, 0, 0, 1, 3, 8, 1, 9, 0, 15, 0xd}, > + /* 540 */ {2, 1, 27, 0, 0, 1, 3, 8, 1, 9, 0, 15, 0x18}, > + /* 216 */ {3, 2, 32, 1677722, 1, 1, 4, 9, 3, 8, 0, 15, 0xd}, > + /* 243 */ {4, 1, 24, 1258291, 1, 1, 5, 11, 3, 9, 1, 15, 0xd}, > + /* 324 */ {4, 1, 32, 1677722, 1, 1, 4, 9, 3, 8, 0, 15, 0x18}, > + /* 432 */ {3, 1, 32, 1677722, 1, 1, 4, 9, 3, 8, 0, 15, 0x18} We calculate all of prop_coef, int_coef, gain_ctl, targ_cnt, dcoapovr_en_h, dco_amp, so no need to have fixed values for these. The corresponding fields should be removed from bxt_clk_div and replaced with local vars in bxt_ddi_pll_select. > }; > > static bool > @@ -1359,6 +1361,7 @@ bxt_ddi_pll_select(struct intel_crtc *intel_crtc, > { > struct intel_shared_dpll *pll; > struct bxt_clk_div clk_div = {0}; > + int vco = 0; > > if (intel_encoder->type == INTEL_OUTPUT_HDMI) { > intel_clock_t best_clock; > @@ -1382,11 +1385,7 @@ bxt_ddi_pll_select(struct intel_crtc *intel_crtc, > clk_div.m2_frac = best_clock.m2 & ((1 << 22) - 1); > clk_div.m2_frac_en = clk_div.m2_frac != 0;
Re: [Intel-gfx] [PATCH] drm/i915: restore ggtt double-bind avoidance
On Wed, May 06, 2015 at 05:18:01PM +0200, Daniel Vetter wrote: > This was accidentally lost in > > commit 75d04a3773ecee617847de963ae4195d6aa74c28 > Author: Mika Kuoppala > Date: Tue Apr 28 17:56:17 2015 +0300 > > drm/i915/gtt: Allocate va range only if vma is not bound > > While at it implement an improved version suggested by Chris which > avoids the double-bind irrespective of what type of bind is done > first. > > Cc: Chris Wilson > Cc: Michel Thierry > Cc: Mika Kuoppala > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/i915/i915_gem_gtt.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c > b/drivers/gpu/drm/i915/i915_gem_gtt.c > index 8fee6789fae2..11a3b511ae64 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > @@ -1928,6 +1928,9 @@ static int ggtt_bind_vma(struct i915_vma *vma, > cache_level, pte_flags); > } > > + if (!dev_priv->mm.aliasing_ppgtt) > + vma->bound |= GLOBAL_BIND | LOCAL_BIND; > + > return 0; > } Meh, it wasn't quite what I had in mind. If we do it there, we may as well go whole hog and do the vma->bound manipulation for both ggtt and ppgtt in the bind_vma() Something like: ickle@crystalwell:/usr/src/linux$ vim drivers/gpu/drm/i915/i915_gem_gtt.c ickle@crystalwell:/usr/src/linux$ git diff diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem index 698a30b..59114af 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -148,7 +148,7 @@ static int sanitize_enable_ppgtt(struct drm_device *dev, int static int ppgtt_bind_vma(struct i915_vma *vma, enum i915_cache_level cache_level, - u32 unused) + u32 bind_flags) { u32 pte_flags = 0; @@ -159,6 +159,7 @@ static int ppgtt_bind_vma(struct i915_vma *vma, vma->vm->insert_entries(vma->vm, vma->obj->pages, vma->node.start, cache_level, pte_flags); + vma->bound |= bind_flags; return 0; } @@ -1956,6 +1957,7 @@ static int ggtt_bind_vma(struct i915_vma *vma, vma->vm->insert_entries(vma->vm, pages, : diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 698a30b..59114af 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -148,7 +148,7 @@ static int sanitize_enable_ppgtt(struct drm_device *dev, int enable_ppgtt) static int ppgtt_bind_vma(struct i915_vma *vma, enum i915_cache_level cache_level, - u32 unused) + u32 bind_flags) { u32 pte_flags = 0; @@ -159,6 +159,7 @@ static int ppgtt_bind_vma(struct i915_vma *vma, vma->vm->insert_entries(vma->vm, vma->obj->pages, vma->node.start, cache_level, pte_flags); + vma->bound |= bind_flags; return 0; } @@ -1956,6 +1957,7 @@ static int ggtt_bind_vma(struct i915_vma *vma, vma->vm->insert_entries(vma->vm, pages, vma->node.start, cache_level, pte_flags); + flags |= GLOBAL_BIND; } if (dev_priv->mm.aliasing_ppgtt && flags & LOCAL_BIND) { @@ -1965,6 +1967,7 @@ static int ggtt_bind_vma(struct i915_vma *vma, cache_level, pte_flags); } + vma->bound |= flags; return 0; } @@ -2945,10 +2948,5 @@ int i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level, } trace_i915_vma_bind(vma, bind_flags); - ret = vma->vm->bind_vma(vma, cache_level, bind_flags); - if (ret) - return ret; - - vma->bound |= bind_flags; - return 0; + return vma->vm->bind_vma(vma, cache_level, bind_flags); } -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Store device pointer in contexts for late tracepoint usafe
On Wed, May 06, 2015 at 01:16:30PM +0200, Daniel Vetter wrote: > On Tue, May 05, 2015 at 09:17:29AM +0100, Chris Wilson wrote: > > [ 1572.417121] BUG: unable to handle kernel NULL pointer dereference at > > (null) > > [ 1572.421010] IP: [] > > ftrace_raw_event_i915_context+0x5d/0x70 [i915] > > [ 1572.424970] PGD 1766a3067 PUD 1767a2067 PMD 0 > > [ 1572.428892] Oops: [#1] SMP > > [ 1572.432787] Modules linked in: ipv6 dm_mod iTCO_wdt iTCO_vendor_support > > snd_hda_codec_realtek snd_hda_codec_generic snd_hda_intel > > snd_hda_controller snd_hda_codec snd_hda_core snd_hwdep snd_pcm snd_timer > > snd soundcore serio_raw pcspkr lpc_ich i2c_i801 mfd_core battery ac > > acpi_cpufreq i915 button video drm_kms_helper drm > > [ 1572.441720] CPU: 2 PID: 18853 Comm: kworker/u8:0 Not tainted > > 4.0.0_kcloud_3f0360_20150429+ #588 > > [ 1572.446298] Workqueue: i915 i915_gem_retire_work_handler [i915] > > [ 1572.450876] task: 880002f428f0 ti: 880035724000 task.ti: > > 880035724000 > > [ 1572.47] RIP: 0010:[] [] > > ftrace_raw_event_i915_context+0x5d/0x70 [i915] > > [ 1572.460423] RSP: 0018:880035727ce8 EFLAGS: 00010286 > > [ 1572.465262] RAX: 880073f1643c RBX: 880002da9058 RCX: > > 880073e5db40 > > [ 1572.470179] RDX: RSI: RDI: > > 880035727ce8 > > [ 1572.475107] RBP: 88007bb11a00 R08: R09: > > > > [ 1572.480034] R10: 00362200 R11: 0008 R12: > > > > [ 1572.484952] R13: 880035727d78 R14: 880002dc1c98 R15: > > 880002dc1dc8 > > [ 1572.489886] FS: () GS:88017fd0() > > knlGS: > > [ 1572.494883] CS: 0010 DS: ES: CR0: 8005003b > > [ 1572.499859] CR2: CR3: 00017572a000 CR4: > > 001006e0 > > [ 1572.504842] Stack: > > [ 1572.509834] 88017b0090c0 880073f16438 880002da9058 > > 880073f1643c > > [ 1572.514904] 0246 8801 88007bb11a00 > > 880002ddeb10 > > [ 1572.519985] 8801759f79c0 a0092ff0 > > 88007bb11a00 > > [ 1572.525049] Call Trace: > > [ 1572.530093] [] ? i915_gem_context_free+0xa8/0xc1 > > [i915] > > [ 1572.535227] [] ? i915_gem_request_free+0x4e/0x50 > > [i915] > > [ 1572.540347] [] ? > > intel_execlists_retire_requests+0x14c/0x159 [i915] > > [ 1572.545500] [] ? i915_gem_retire_requests+0x9d/0xeb > > [i915] > > [ 1572.550664] [] ? > > i915_gem_retire_work_handler+0x4c/0x61 [i915] > > [ 1572.555825] [] ? process_one_work+0x1b2/0x31d > > [ 1572.560951] [] ? worker_thread+0x24d/0x339 > > [ 1572.566033] [] ? cancel_delayed_work_sync+0xa/0xa > > [ 1572.571140] [] ? kthread+0xce/0xd6 > > [ 1572.576191] [] ? kthread_create_on_node+0x162/0x162 > > [ 1572.581228] [] ? ret_from_fork+0x58/0x90 > > [ 1572.586259] [] ? kthread_create_on_node+0x162/0x162 > > [ 1572.591318] Code: de 48 89 e7 e8 09 4d 00 e1 48 85 c0 74 27 48 89 68 10 > > 48 8b 55 38 48 89 e7 48 89 50 18 48 8b 55 10 48 8b 12 48 8b 12 48 8b 52 38 > > <8b> 12 89 50 08 e8 95 4d 00 e1 48 83 c4 30 5b 5d 41 5c c3 41 55 > > [ 1572.596981] RIP [] > > ftrace_raw_event_i915_context+0x5d/0x70 [i915] > > [ 1572.602464] RSP > > [ 1572.607911] CR2: > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90112#c23 > > Signed-off-by: Chris Wilson > > Could we do an igt for this, maybe using the sysfs trace interface? I wonder if it is as simple as: tracefs=/sys/kernel/tracing function set_tracing { for i in `find $tracefs/i915 -name enable`; do $i echo $1 > $i done echo $1 > $tracefs/enable } set_tracing 1 ./gem_ctx_basic ./gem_foo_basic ./gem_bar_basic ./gem_baz_basic set_tracing 0 -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Drop PIPE-A quirk for 945GSE HP Mini
Since the introduction of BIOS fb preservation, circa 3.17, we began encountering a failure during boot when trying to use force-detect before GEM was initialised. That bug is from commit 7fad798e16fecddd41c6a91728a09f0b9507e40c Author: Daniel Vetter Date: Wed Jul 4 17:51:47 2012 +0200 drm/i915: ensure the force pipe A quirk is actually followed but investigation of the affected machine revealed that it was using a PIPE-A quirk even though it was a 945GSE and the quirk is only supposed to be used to workaround a hardware issue on 830/845. That quirk was added for this HP Mini in commit 6b93afc564a5e74b0eaaa46c95f557449951b3b9 Author: Bryce Harrington Date: Wed May 27 03:40:52 2009 -0700 add pipe a force quirk for Dell mini in order to workaround an issue with the BIOS behaving strangely during lid-close. Since then we have a much larger hammer to thwart the BIOS after opening the lid and the PIPE-A quirk is no longer required. Reported-any-tested-by: Apostolos B. References: https://bugs.freedesktop.org/show_bug.cgi?id=21960 Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=87521#c13 Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/intel_display.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 77929538ec0a..8de94e8dcecc 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -14542,9 +14542,6 @@ static const struct intel_dmi_quirk intel_dmi_quirks[] = { }; static struct intel_quirk intel_quirks[] = { - /* HP Mini needs pipe A force quirk (LP: #322104) */ - { 0x27ae, 0x103c, 0x361a, quirk_pipea_force }, - /* Toshiba Protege R-205, S-209 needs pipe A force quirk */ { 0x2592, 0x1179, 0x0001, quirk_pipea_force }, -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Add missing POSTING_READ()s to BXT dbuf enable sequence
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 6328 -Summary- Platform Delta drm-intel-nightly Series Applied PNV 276/276 276/276 ILK -1 302/302 301/302 SNB 316/316 316/316 IVB 342/342 342/342 BYT 286/286 286/286 BDW 321/321 321/321 -Detailed- Platform Testdrm-intel-nightly Series Applied *ILK igt@kms_pipe_crc_basic@bad-source PASS(3) DMESG_WARN(1)PASS(1) (dmesg patch applied)drm:drm_edid_block_valid[drm]]*ERROR*EDID_checksum_is_invalid,remainder_is@EDID checksum is .* remainder is Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] dma-buf: add ref counting for module as exporter
On 7 May 2015 at 13:40, Greg KH wrote: > On Thu, May 07, 2015 at 01:00:52PM +0530, Sumit Semwal wrote: >> Add reference counting on a kernel module that exports dma-buf and >> implements its operations. This prevents the module from being unloaded >> while DMABUF file is in use. >> >> The original patch [1] was submitted by Tomasz, but he's since shifted >> jobs and a ping didn't elicit any response. >> >> [tomasz: Original author] >> Signed-off-by: Tomasz Stanislawski >> Acked-by: Daniel Vetter >> [sumits: updated & rebased as per review comments] >> Signed-off-by: Sumit Semwal >> >> [1]: https://lkml.org/lkml/2012/8/8/163 >> --- >> drivers/dma-buf/dma-buf.c | 9 - >> drivers/gpu/drm/armada/armada_gem.c| 1 + >> drivers/gpu/drm/drm_prime.c| 1 + >> drivers/gpu/drm/exynos/exynos_drm_dmabuf.c | 1 + >> drivers/gpu/drm/i915/i915_gem_dmabuf.c | 1 + >> drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c | 1 + >> drivers/gpu/drm/tegra/gem.c| 1 + >> drivers/gpu/drm/udl/udl_dmabuf.c | 1 + >> drivers/gpu/drm/vmwgfx/vmwgfx_prime.c | 1 + >> drivers/media/v4l2-core/videobuf2-dma-contig.c | 1 + >> drivers/media/v4l2-core/videobuf2-dma-sg.c | 1 + >> drivers/media/v4l2-core/videobuf2-vmalloc.c| 1 + >> drivers/staging/android/ion/ion.c | 1 + >> include/linux/dma-buf.h| 2 ++ >> 14 files changed, 22 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c >> index c5a9138a6a8d..9583eac0238f 100644 >> --- a/drivers/dma-buf/dma-buf.c >> +++ b/drivers/dma-buf/dma-buf.c >> @@ -29,6 +29,7 @@ >> #include >> #include >> #include >> +#include >> #include >> #include >> #include >> @@ -64,6 +65,7 @@ static int dma_buf_release(struct inode *inode, struct >> file *file) >> BUG_ON(dmabuf->cb_shared.active || dmabuf->cb_excl.active); >> >> dmabuf->ops->release(dmabuf); >> + module_put(dmabuf->ops->owner); >> >> mutex_lock(&db_list.lock); >> list_del(&dmabuf->list_node); >> @@ -302,9 +304,14 @@ struct dma_buf *dma_buf_export(const struct >> dma_buf_export_info *exp_info) >> return ERR_PTR(-EINVAL); >> } >> >> + if (!try_module_get(exp_info->ops->owner)) >> + return ERR_PTR(-ENOENT); >> + >> dmabuf = kzalloc(alloc_size, GFP_KERNEL); >> - if (dmabuf == NULL) >> + if (!dmabuf) { >> + module_put(exp_info->ops->owner); >> return ERR_PTR(-ENOMEM); >> + } >> >> dmabuf->priv = exp_info->priv; >> dmabuf->ops = exp_info->ops; >> diff --git a/drivers/gpu/drm/armada/armada_gem.c >> b/drivers/gpu/drm/armada/armada_gem.c >> index 580e10acaa3a..d2c5fc1269b7 100644 >> --- a/drivers/gpu/drm/armada/armada_gem.c >> +++ b/drivers/gpu/drm/armada/armada_gem.c >> @@ -524,6 +524,7 @@ armada_gem_dmabuf_mmap(struct dma_buf *buf, struct >> vm_area_struct *vma) >> } >> >> static const struct dma_buf_ops armada_gem_prime_dmabuf_ops = { >> + .owner = THIS_MODULE, >> .map_dma_buf= armada_gem_prime_map_dma_buf, >> .unmap_dma_buf = armada_gem_prime_unmap_dma_buf, >> .release= drm_gem_dmabuf_release, > > The "easier" way to do this is change the registration function to add > the module owner automatically, which keeps you from having to modify > all of the individual drivers. Look at how pci and usb do this for > their driver registration functions. That should result in a much > smaller patch, that always works properly for everyone (there's no way > for driver to get it wrong.) > Thanks Greg; but of course, you're right! We already have a DEFINE_DMA_BUF_EXPORT_INFO macro, so this is far easier incorporated into that. I will spin up the (much simpler) patch and repost! > thanks, > > greg k-h Best regards, Sumit. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Remove locking for get-caching query
Reading a single value from the object, the locking only provides futile protection against userspace races. The locking is useless so remove it. Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/i915_gem.c | 17 - 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 2b2b74dbb446..d071d0af2a6c 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -3983,17 +3983,10 @@ int i915_gem_get_caching_ioctl(struct drm_device *dev, void *data, { struct drm_i915_gem_caching *args = data; struct drm_i915_gem_object *obj; - int ret; - - ret = i915_mutex_lock_interruptible(dev); - if (ret) - return ret; obj = to_intel_bo(drm_gem_object_lookup(dev, file, args->handle)); - if (&obj->base == NULL) { - ret = -ENOENT; - goto unlock; - } + if (&obj->base == NULL) + return -ENOENT; switch (obj->cache_level) { case I915_CACHE_LLC: @@ -4010,10 +4003,8 @@ int i915_gem_get_caching_ioctl(struct drm_device *dev, void *data, break; } - drm_gem_object_unreference(&obj->base); -unlock: - mutex_unlock(&dev->struct_mutex); - return ret; + drm_gem_object_unreference_unlocked(&obj->base); + return 0; } int i915_gem_set_caching_ioctl(struct drm_device *dev, void *data, -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/bxt: edp1.4 Intermediate Freq support
BXT supports following intermediate link rates for edp: 2.16GHz, 2.43GHz, 3.24GHz, 4.32GHz. Adding support for programming the intermediate rates. v2: Adding clock in bxt_clk_div struct and then look for the entry with required rate (Ville) Signed-off-by: Sonika Jindal --- drivers/gpu/drm/i915/intel_ddi.c | 45 +- drivers/gpu/drm/i915/intel_dp.c |7 +- 2 files changed, 31 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index 9c1e74a..7b9d226 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -1327,6 +1327,7 @@ skl_ddi_pll_select(struct intel_crtc *intel_crtc, /* bxt clock parameters */ struct bxt_clk_div { + int clock; uint32_t p1; uint32_t p2; uint32_t m2_int; @@ -1342,13 +1343,13 @@ struct bxt_clk_div { /* pre-calculated values for DP linkrates */ static struct bxt_clk_div bxt_dp_clk_val[7] = { - /* 162 */ {4, 2, 32, 1677722, 1, 1, 5, 11, 2, 9, 0xd}, - /* 270 */ {4, 1, 27, 0, 0, 1, 3, 8, 1, 9, 0xd}, - /* 540 */ {2, 1, 27, 0, 0, 1, 3, 8, 1, 9, 0x18}, - /* 216 */ {3, 2, 32, 1677722, 1, 1, 5, 11, 2, 9, 0xd}, - /* 243 */ {4, 1, 24, 1258291, 1, 1, 5, 11, 2, 9, 0xd}, - /* 324 */ {4, 1, 32, 1677722, 1, 1, 5, 11, 2, 9, 0xd}, - /* 432 */ {3, 1, 32, 1677722, 1, 1, 5, 11, 2, 9, 0x18} + {162000, 4, 2, 32, 1677722, 1, 1, 5, 11, 2, 9, 0xd}, + {27, 4, 1, 27, 0, 0, 1, 3, 8, 1, 9, 0xd}, + {54, 2, 1, 27, 0, 0, 1, 3, 8, 1, 9, 0x18}, + {216000, 3, 2, 32, 1677722, 1, 1, 5, 11, 2, 9, 0xd}, + {243000, 4, 1, 24, 1258291, 1, 1, 5, 11, 2, 9, 0xd}, + {324000, 4, 1, 32, 1677722, 1, 1, 5, 11, 2, 9, 0xd}, + {432000, 3, 1, 32, 1677722, 1, 1, 5, 11, 2, 9, 0x18} }; static bool @@ -1401,20 +1402,24 @@ bxt_ddi_pll_select(struct intel_crtc *intel_crtc, intel_encoder->type == INTEL_OUTPUT_EDP) { struct drm_encoder *encoder = &intel_encoder->base; struct intel_dp *intel_dp = enc_to_intel_dp(encoder); + int link_rate; + int i; - switch (intel_dp->link_bw) { - case DP_LINK_BW_1_62: - clk_div = bxt_dp_clk_val[0]; - break; - case DP_LINK_BW_2_7: - clk_div = bxt_dp_clk_val[1]; - break; - case DP_LINK_BW_5_4: - clk_div = bxt_dp_clk_val[2]; - break; - default: - clk_div = bxt_dp_clk_val[0]; - DRM_ERROR("Unknown link rate\n"); + /* +* If edp1.4 intermediate frequency support is present, we set +* link_bw to 0 and a valid rate index in rate_select. +*/ + if (intel_dp->link_bw) + link_rate = clock; + else + link_rate = intel_dp->sink_rates[intel_dp->rate_select]; + + clk_div = bxt_dp_clk_val[0]; + for (i = 0; i < 7; ++i) { + if (bxt_dp_clk_val[i].clock == link_rate) { + clk_div = bxt_dp_clk_val[i]; + break; + } } } diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index c9d50d1..e6ee7c6 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -85,6 +85,8 @@ static const struct dp_link_dpll chv_dpll[] = { { .p1 = 2, .p2 = 1, .n = 1, .m1 = 2, .m2 = 0x6c0 } } }; +static const int bxt_rates[] = { 162000, 216000, 243000, 27, + 324000, 432000, 54 }; static const int skl_rates[] = { 162000, 216000, 27, 324000, 432000, 54 }; static const int chv_rates[] = { 162000, 202500, 21, 216000, @@ -1161,7 +1163,10 @@ intel_dp_sink_rates(struct intel_dp *intel_dp, const int **sink_rates) static int intel_dp_source_rates(struct drm_device *dev, const int **source_rates) { - if (IS_SKYLAKE(dev)) { + if (IS_BROXTON(dev)) { + *source_rates = bxt_rates; + return ARRAY_SIZE(bxt_rates); + } else if (IS_SKYLAKE(dev)) { *source_rates = skl_rates; return ARRAY_SIZE(skl_rates); } else if (IS_CHERRYVIEW(dev)) { -- 1.7.10.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 4/4] drm/i915: Use partial view in mmap fault handler
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 6329 -Summary- Platform Delta drm-intel-nightly Series Applied PNV 276/276 276/276 ILK 302/302 302/302 SNB 316/316 316/316 IVB 342/342 342/342 BYT 286/286 286/286 BDW 321/321 321/321 -Detailed- Platform Testdrm-intel-nightly Series Applied Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/vblank: Fixup and document timestamp update/read barriers
On 05/06/2015 04:56 AM, Daniel Vetter wrote: > On Tue, May 05, 2015 at 11:57:42AM -0400, Peter Hurley wrote: >> On 05/05/2015 11:42 AM, Daniel Vetter wrote: >>> On Tue, May 05, 2015 at 10:36:24AM -0400, Peter Hurley wrote: On 05/04/2015 12:52 AM, Mario Kleiner wrote: > On 04/16/2015 03:03 PM, Daniel Vetter wrote: >> On Thu, Apr 16, 2015 at 08:30:55AM -0400, Peter Hurley wrote: >>> On 04/15/2015 01:31 PM, Daniel Vetter wrote: On Wed, Apr 15, 2015 at 09:00:04AM -0400, Peter Hurley wrote: > Hi Daniel, > > On 04/15/2015 03:17 AM, Daniel Vetter wrote: >> This was a bit too much cargo-culted, so lets make it solid: >> - vblank->count doesn't need to be an atomic, writes are always done >>under the protection of dev->vblank_time_lock. Switch to an >> unsigned >>long instead and update comments. Note that atomic_read is just a >>normal read of a volatile variable, so no need to audit all the >>read-side access specifically. >> >> - The barriers for the vblank counter seqlock weren't complete: The >>read-side was missing the first barrier between the counter read >> and >>the timestamp read, it only had a barrier between the ts and the >>counter read. We need both. >> >> - Barriers weren't properly documented. Since barriers only work if >>you have them on boths sides of the transaction it's prudent to >>reference where the other side is. To avoid duplicating the >>write-side comment 3 times extract a little store_vblank() helper. >>In that helper also assert that we do indeed hold >>dev->vblank_time_lock, since in some cases the lock is acquired a >>few functions up in the callchain. >> >> Spotted while reviewing a patch from Chris Wilson to add a fastpath >> to >> the vblank_wait ioctl. >> >> Cc: Chris Wilson >> Cc: Mario Kleiner >> Cc: Ville Syrjälä >> Cc: Michel Dänzer >> Signed-off-by: Daniel Vetter >> --- >> drivers/gpu/drm/drm_irq.c | 92 >> --- >> include/drm/drmP.h| 8 +++-- >> 2 files changed, 54 insertions(+), 46 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c >> index c8a34476570a..23bfbc61a494 100644 >> --- a/drivers/gpu/drm/drm_irq.c >> +++ b/drivers/gpu/drm/drm_irq.c >> @@ -74,6 +74,33 @@ module_param_named(vblankoffdelay, >> drm_vblank_offdelay, int, 0600); >> module_param_named(timestamp_precision_usec, >> drm_timestamp_precision, int, 0600); >> module_param_named(timestamp_monotonic, drm_timestamp_monotonic, >> int, 0600); >> >> +static void store_vblank(struct drm_device *dev, int crtc, >> + unsigned vblank_count_inc, >> + struct timeval *t_vblank) >> +{ >> +struct drm_vblank_crtc *vblank = &dev->vblank[crtc]; >> +u32 tslot; >> + >> +assert_spin_locked(&dev->vblank_time_lock); >> + >> +if (t_vblank) { >> +tslot = vblank->count + vblank_count_inc; >> +vblanktimestamp(dev, crtc, tslot) = *t_vblank; >> +} >> + >> +/* >> + * vblank timestamp updates are protected on the write side with >> + * vblank_time_lock, but on the read side done locklessly using >> a >> + * sequence-lock on the vblank counter. Ensure correct ordering >> using >> + * memory barrriers. We need the barrier both before and also >> after the >> + * counter update to synchronize with the next timestamp write. >> + * The read-side barriers for this are in >> drm_vblank_count_and_time. >> + */ >> +smp_wmb(); >> +vblank->count += vblank_count_inc; >> +smp_wmb(); > > The comment and the code are each self-contradictory. > > If vblank->count writes are always protected by vblank_time_lock > (something I > did not verify but that the comment above asserts), then the trailing > write > barrier is not required (and the assertion that it is in the comment > is incorrect). > > A spin unlock operation is always a write barrier. Hm yeah. Otoh to me that's bordering on "code too clever for my own good". That the spinlock is held I can assure. That no one goes around and does multiple vblank updates (because somehow that code raced with the hw itself) I can't easily assure w
Re: [Intel-gfx] [PATCH] drm/i915/skl: Add module parameter to select edp vswing table
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 6330 -Summary- Platform Delta drm-intel-nightly Series Applied PNV 276/276 276/276 ILK 302/302 302/302 SNB 316/316 316/316 IVB 342/342 342/342 BYT 286/286 286/286 BDW 321/321 321/321 -Detailed- Platform Testdrm-intel-nightly Series Applied Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Drop PIPE-A quirk for 945GSE HP Mini
On Thu, 07 May 2015, Chris Wilson wrote: > Since the introduction of BIOS fb preservation, circa 3.17, we began > encountering a failure during boot when trying to use force-detect > before GEM was initialised. That bug is from > > commit 7fad798e16fecddd41c6a91728a09f0b9507e40c > Author: Daniel Vetter > Date: Wed Jul 4 17:51:47 2012 +0200 > > drm/i915: ensure the force pipe A quirk is actually followed > > but investigation of the affected machine revealed that it was using a > PIPE-A quirk even though it was a 945GSE and the quirk is only supposed > to be used to workaround a hardware issue on 830/845. That quirk was > added for this HP Mini in > > commit 6b93afc564a5e74b0eaaa46c95f557449951b3b9 > Author: Bryce Harrington > Date: Wed May 27 03:40:52 2009 -0700 > > add pipe a force quirk for Dell mini > > in order to workaround an issue with the BIOS behaving strangely during > lid-close. Since then we have a much larger hammer to thwart the BIOS > after opening the lid and the PIPE-A quirk is no longer required. > > Reported-any-tested-by: Apostolos B. > References: https://bugs.freedesktop.org/show_bug.cgi?id=21960 > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=87521#c13 > Signed-off-by: Chris Wilson Pushed to drm-intel-fixes, thanks for the patch. BR, Jani. > --- > drivers/gpu/drm/i915/intel_display.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 77929538ec0a..8de94e8dcecc 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -14542,9 +14542,6 @@ static const struct intel_dmi_quirk > intel_dmi_quirks[] = { > }; > > static struct intel_quirk intel_quirks[] = { > - /* HP Mini needs pipe A force quirk (LP: #322104) */ > - { 0x27ae, 0x103c, 0x361a, quirk_pipea_force }, > - > /* Toshiba Protege R-205, S-209 needs pipe A force quirk */ > { 0x2592, 0x1179, 0x0001, quirk_pipea_force }, > > -- > 2.1.4 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC PATCH 2/2] drm/i915: initialize backlight max from VBT
Normally we determine the backlight PWM modulation frequency (which we also use as backlight max value) from the backlight registers at module load time, expecting the registers have been initialized by the BIOS. If this is not the case, we fail. The VBT contains the backlight modulation frequency in Hz. Add platform specific functions to convert the frequency in Hz to backlight PWM modulation frequency, and use them to initialize the backlight when the registers are not initialized by the BIOS. Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/i915_drv.h| 2 + drivers/gpu/drm/i915/i915_reg.h| 1 + drivers/gpu/drm/i915/intel_panel.c | 160 +++-- 3 files changed, 156 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 87fe4f7d5ce5..82f3d3120190 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -602,6 +602,8 @@ struct drm_i915_display_funcs { uint32_t level); void (*disable_backlight)(struct intel_connector *connector); void (*enable_backlight)(struct intel_connector *connector); + uint32_t (*backlight_hz_to_pwm)(struct intel_connector *connector, + uint32_t hz); }; enum forcewake_domain_id { diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index ebe55704fa0e..73bd94ea4d7d 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -6168,6 +6168,7 @@ enum skl_disp_power_wells { #define SOUTH_CHICKEN2 0xc2004 #define FDI_MPHY_IOSFSB_RESET_STATUS (1<<13) #define FDI_MPHY_IOSFSB_RESET_CTL (1<<12) +#define PWM_GRANULARITY (1<<5) /* LPT */ #define DPLS_EDP_PPS_FIX_DIS (1<<0) #define _FDI_RXA_CHICKEN 0xc200c diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c index 7d83527f95f7..319cd922f8f3 100644 --- a/drivers/gpu/drm/i915/intel_panel.c +++ b/drivers/gpu/drm/i915/intel_panel.c @@ -1166,10 +1166,125 @@ static void intel_backlight_device_unregister(struct intel_connector *connector) #endif /* CONFIG_BACKLIGHT_CLASS_DEVICE */ /* - * Note: The setup hooks can't assume pipe is set! + * HSW/BDW: This value represents the period of the PWM stream in clock periods + * multiplied by 128 (default increment) or 16 (alternate increment, selected in + * LPT SOUTH_CHICKEN2 register bit 5). * - * XXX: Query mode clock or hardware clock and program PWM modulation frequency - * appropriately when it's 0. Use VBT and/or sane defaults. + * XXX: This only works when driving the PCH PWM. When driving the CPU PWM on + * the utility pin, the granularity needs to be determined by BLC_PWM_CTL bit + * 27. + */ +static u32 hsw_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz) +{ + struct drm_device *dev = connector->base.dev; + struct drm_i915_private *dev_priv = dev->dev_private; + u32 mul, clock; + + if (I915_READ(SOUTH_CHICKEN2) & PWM_GRANULARITY) + mul = 16; + else + mul = 128; + + if (dev_priv->pch_id == INTEL_PCH_LPT_DEVICE_ID_TYPE) + clock = MHz(135); /* LPT:H */ + else + clock = MHz(24); /* LPT:LP */ + + return clock / (pwm_freq_hz * mul); +} + +/* + * ILK/SNB/IVB: This value represents the period of the PWM stream in PCH + * display raw clocks multiplied by 128. + */ +static u32 pch_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz) +{ + struct drm_device *dev = connector->base.dev; + int clock = MHz(intel_pch_rawclk(dev)); + + return clock / (pwm_freq_hz * 128); +} + +/* + * Gen2: This field determines the number of time base events (display core + * clock frequency/32) in total for a complete cycle of modulated backlight + * control. + * + * Gen3: A time base event equals the display core clock ([DevPNV] HRAW clock) + * divided by 32. + */ +static u32 i9xx_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz) +{ + struct drm_device *dev = connector->base.dev; + struct drm_i915_private *dev_priv = dev->dev_private; + int clock; + + if (IS_PINEVIEW(dev)) + clock = intel_hrawclk(dev); + else + clock = 1000 * dev_priv->display.get_display_clock_speed(dev); + + return clock / (pwm_freq_hz * 32); +} + +/* + * Gen4: This value represents the period of the PWM stream in display core + * clocks multiplied by 128. + */ +static u32 i965_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz) +{ + struct drm_device *dev = connector->base.dev; + struct drm_i915_private *dev_priv = dev->dev_private; + int clock = 1000 * dev_priv->display.get_display_clock_speed(dev); + + return clock / (pwm_freq_hz * 128); +} + +/* + * VLV: This value represents the period of the PWM stream in display core + * clocks ([DevCTG
[Intel-gfx] [RFC PATCH 1/2] drm/i915: move intel_hrawclk() to intel_display.c
Make it available outside of intel_dp.c. Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/intel_display.c | 33 + drivers/gpu/drm/i915/intel_dp.c | 34 -- drivers/gpu/drm/i915/intel_drv.h | 1 + 3 files changed, 34 insertions(+), 34 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index c297cdc136e5..6cf7ad29b378 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -142,6 +142,39 @@ intel_pch_rawclk(struct drm_device *dev) return I915_READ(PCH_RAWCLK_FREQ) & RAWCLK_FREQ_MASK; } +/* hrawclock is 1/4 the FSB frequency */ +int intel_hrawclk(struct drm_device *dev) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + uint32_t clkcfg; + + /* There is no CLKCFG reg in Valleyview. VLV hrawclk is 200 MHz */ + if (IS_VALLEYVIEW(dev)) + return 200; + + clkcfg = I915_READ(CLKCFG); + switch (clkcfg & CLKCFG_FSB_MASK) { + case CLKCFG_FSB_400: + return 100; + case CLKCFG_FSB_533: + return 133; + case CLKCFG_FSB_667: + return 166; + case CLKCFG_FSB_800: + return 200; + case CLKCFG_FSB_1067: + return 266; + case CLKCFG_FSB_1333: + return 333; + /* these two are just a guess; one of them might be right */ + case CLKCFG_FSB_1600: + case CLKCFG_FSB_1600_ALT: + return 400; + default: + return 133; + } +} + static inline u32 /* units of 100MHz */ intel_fdi_link_freq(struct drm_device *dev) { diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 5bd73ad310d4..9d5505ec5904 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -254,40 +254,6 @@ static void intel_dp_unpack_aux(uint32_t src, uint8_t *dst, int dst_bytes) dst[i] = src >> ((3-i) * 8); } -/* hrawclock is 1/4 the FSB frequency */ -static int -intel_hrawclk(struct drm_device *dev) -{ - struct drm_i915_private *dev_priv = dev->dev_private; - uint32_t clkcfg; - - /* There is no CLKCFG reg in Valleyview. VLV hrawclk is 200 MHz */ - if (IS_VALLEYVIEW(dev)) - return 200; - - clkcfg = I915_READ(CLKCFG); - switch (clkcfg & CLKCFG_FSB_MASK) { - case CLKCFG_FSB_400: - return 100; - case CLKCFG_FSB_533: - return 133; - case CLKCFG_FSB_667: - return 166; - case CLKCFG_FSB_800: - return 200; - case CLKCFG_FSB_1067: - return 266; - case CLKCFG_FSB_1333: - return 333; - /* these two are just a guess; one of them might be right */ - case CLKCFG_FSB_1600: - case CLKCFG_FSB_1600_ALT: - return 400; - default: - return 133; - } -} - static void intel_dp_init_panel_power_sequencer(struct drm_device *dev, struct intel_dp *intel_dp); diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 930ea6916d9f..3fc9c2df1689 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -981,6 +981,7 @@ void i915_audio_component_cleanup(struct drm_i915_private *dev_priv); extern const struct drm_plane_funcs intel_plane_funcs; bool intel_has_pending_fb_unpin(struct drm_device *dev); int intel_pch_rawclk(struct drm_device *dev); +int intel_hrawclk(struct drm_device *dev); void intel_mark_busy(struct drm_device *dev); void intel_mark_idle(struct drm_device *dev); void intel_crtc_restore_mode(struct drm_crtc *crtc); -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC PATCH 0/2] drm/i915: get backlight max from vbt
Found this while cleaning up some old branches I have in my repo... I'm posting this more as archival for posterity than as a serious attempt to get this merged. There was a bunch of archeology involved in figuring out the Hz to PWM modulation frequency conversion, so someone might find that useful. Particularly the vlv setup could use some cleanup and only falling back to the hardcoded value if vbt is not set. And the hardcoded default could now be expressed in Hz instead of raw pwm value. But meh until someone really needs this. BR, Jani. Jani Nikula (2): drm/i915: move intel_hrawclk() to intel_display.c drm/i915: initialize backlight max from VBT drivers/gpu/drm/i915/i915_drv.h | 2 + drivers/gpu/drm/i915/i915_reg.h | 1 + drivers/gpu/drm/i915/intel_display.c | 33 drivers/gpu/drm/i915/intel_dp.c | 34 drivers/gpu/drm/i915/intel_drv.h | 1 + drivers/gpu/drm/i915/intel_panel.c | 160 +-- 6 files changed, 190 insertions(+), 41 deletions(-) -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915/bxt: Move around lane stagger calculation
On to, 2015-05-07 at 12:00 +0530, Vandana Kannan wrote: > Making lane stagger calculation common for HDMI and DP > > Signed-off-by: Vandana Kannan > --- > drivers/gpu/drm/i915/intel_ddi.c | 21 +++-- > 1 file changed, 11 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c > b/drivers/gpu/drm/i915/intel_ddi.c > index 49b9fd8..144d544 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -1386,16 +1386,6 @@ bxt_ddi_pll_select(struct intel_crtc *intel_crtc, > clk_div.m2_frac_en = clk_div.m2_frac != 0; > > vco = best_clock.vco; > - if (clock > 27) > - clk_div.lanestagger = 0x18; > - else if (clock > 135000) > - clk_div.lanestagger = 0x0d; > - else if (clock > 67000) > - clk_div.lanestagger = 0x07; > - else if (clock > 33000) > - clk_div.lanestagger = 0x04; > - else > - clk_div.lanestagger = 0x02; > } else if (intel_encoder->type == INTEL_OUTPUT_DISPLAYPORT || > intel_encoder->type == INTEL_OUTPUT_EDP) { > struct drm_encoder *encoder = &intel_encoder->base; > @@ -1443,6 +1433,17 @@ bxt_ddi_pll_select(struct intel_crtc *intel_crtc, > return false; > } > > + if (clock > 27) > + clk_div.lanestagger = 0x18; > + else if (clock > 135000) > + clk_div.lanestagger = 0x0d; > + else if (clock > 67000) > + clk_div.lanestagger = 0x07; > + else if (clock > 33000) > + clk_div.lanestagger = 0x04; > + else > + clk_div.lanestagger = 0x02; > + Here as in patch 1/2 we don't need to have fixed values for lanestagger any more, so you can remove it from bxt_clk_div and use a local var instead. > crtc_state->dpll_hw_state.ebb0 = > PORT_PLL_P1(clk_div.p1) | PORT_PLL_P2(clk_div.p2); > crtc_state->dpll_hw_state.pll0 = clk_div.m2_int; ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] igt/gem_create_stolen: Verifying extended gem_create ioctl
On Thu, May 07, 2015 at 10:12:08AM +0100, Chris Wilson wrote: > On Thu, May 07, 2015 at 08:52:54AM +0200, Daniel Vetter wrote: > > On Wed, May 06, 2015 at 03:51:52PM +0530, ankitprasad.r.sha...@intel.com > > wrote: > > > From: Ankitprasad Sharma > > > > > > This patch adds the testcases for verifying the new extended > > > gem_create ioctl. By means of this extended ioctl, memory > > > placement of the GEM object can be specified, i.e. either > > > shmem or stolen memory. > > > These testcases include functional tests and interface tests for > > > testing the gem_create ioctl call for stolen memory placement > > > > > > v2: Testing pread/pwrite functionality for stolen backed objects, > > > added local struct for extended gem_create and gem_get_aperture, > > > until headers catch up (Chris) > > > > > > v3: Removed get_aperture related functions, extended gem_pread > > > to compare speeds for user pages with and without page faults, > > > unexposed local_gem_create struct, changed gem_create_stolen > > > usage (Chris) > > > > > > Signed-off-by: Ankitprasad Sharma > > > > An igt to check for invalid arguments of the gem create ioctl (especially > > the newly added flags parameters) seems to be missing. > > If we do that, I would actually create gem_create.c to do the parameter > testing of CREATE, and rename this to gem_stolen.c as this covers the > functional side of using stolen (i.e. not limited to testing the CREATE > API). And I want a pink pony. Yes, that would be even nicer. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/9] drm/i915/bxt: Mark workaround as for Skylake & Broxton
Signed-off-by: Nick Hoath --- drivers/gpu/drm/i915/intel_ringbuffer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 49e4610..cdbdf49 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -923,7 +923,7 @@ static int gen9_init_workarounds(struct intel_engine_cs *ring) WA_SET_BIT_MASKED(GEN8_ROW_CHICKEN, PARTIAL_INSTRUCTION_SHOOTDOWN_DISABLE); - /* Syncing dependencies between camera and graphics */ + /* Syncing dependencies between camera and graphics:skl,bxt */ WA_SET_BIT_MASKED(HALF_SLICE_CHICKEN3, GEN9_DISABLE_OCL_OOB_SUPPRESS_LOGIC); -- 2.1.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/9] drm/i915/bxt: Enable WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken for Broxton
Signed-off-by: Nick Hoath --- drivers/gpu/drm/i915/intel_ringbuffer.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 3f1a784..ac1ad44 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -935,8 +935,9 @@ static int gen9_init_workarounds(struct intel_engine_cs *ring) GEN9_DG_MIRROR_FIX_ENABLE); } - if (IS_SKYLAKE(dev) && INTEL_REVID(dev) <= SKL_REVID_B0) { - /* WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken:skl */ + if ((IS_SKYLAKE(dev) && INTEL_REVID(dev) <= SKL_REVID_B0) || + (IS_BROXTON(dev) && INTEL_REVID(dev) < BXT_REVID_B0)) { + /* WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken:skl,bxt */ WA_SET_BIT_MASKED(GEN7_COMMON_SLICE_CHICKEN1, GEN9_RHWO_OPTIMIZATION_DISABLE); WA_SET_BIT_MASKED(GEN9_SLICE_COMMON_ECO_CHICKEN0, -- 2.1.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/9] drm/i915/bxt: Enable WaDisableDgMirrorFixInHalfSliceChicken5 for Broxton
Signed-off-by: Nick Hoath --- drivers/gpu/drm/i915/intel_ringbuffer.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index cdbdf49..3f1a784 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -927,9 +927,10 @@ static int gen9_init_workarounds(struct intel_engine_cs *ring) WA_SET_BIT_MASKED(HALF_SLICE_CHICKEN3, GEN9_DISABLE_OCL_OOB_SUPPRESS_LOGIC); - if (INTEL_REVID(dev) == SKL_REVID_A0 || - INTEL_REVID(dev) == SKL_REVID_B0) { - /* WaDisableDgMirrorFixInHalfSliceChicken5:skl */ + if ((IS_SKYLAKE(dev) && (INTEL_REVID(dev) == SKL_REVID_A0 || + INTEL_REVID(dev) == SKL_REVID_B0)) || + (IS_BROXTON(dev) && INTEL_REVID(dev) < BXT_REVID_B0)) { + /* WaDisableDgMirrorFixInHalfSliceChicken5:skl,bxt */ WA_CLR_BIT_MASKED(GEN9_HALF_SLICE_CHICKEN5, GEN9_DG_MIRROR_FIX_ENABLE); } -- 2.1.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/9] drm/i915/bxt: Mark WaDisablePartialInstShootdown as for Broxton also.
Signed-off-by: Nick Hoath --- drivers/gpu/drm/i915/intel_ringbuffer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 7ef9a29..49e4610 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -919,7 +919,7 @@ static int gen9_init_workarounds(struct intel_engine_cs *ring) struct drm_device *dev = ring->dev; struct drm_i915_private *dev_priv = dev->dev_private; - /* WaDisablePartialInstShootdown:skl */ + /* WaDisablePartialInstShootdown:skl,bxt */ WA_SET_BIT_MASKED(GEN8_ROW_CHICKEN, PARTIAL_INSTRUCTION_SHOOTDOWN_DISABLE); -- 2.1.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 0/9] drm/i915/bxt Enable existing workarounds for Broxton
The following patch series either enables a workaround for Broxton, marks it as applicable to Broxton, or moves it in to the SoC specific initialisation. v2: Split out the changes as one patch per workaround (Requested by Imre) Removed unused additional register. Cleaned up whitespace. (Imre) Cleaned up revision ID usage (Imre) Nick Hoath (9): drm/i915/bxt: Mark WaDisablePartialInstShootdown as for Broxton also. drm/i915/bxt: Mark workaround as for Skylake & Broxton drm/i915/bxt: Enable WaDisableDgMirrorFixInHalfSliceChicken5 for Broxton drm/i915/bxt: Enable WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken for Broxton drm/i915/bxt: Enable WaEnableYV12BugFixInHalfSliceChicken7 for Broxton drm/i915/bxt: Move WaForceEnableNonCoherent to Skylake only drm/i915/bxt: Mark Wa4x4STCOptimizationDisable as for Broxton also. drm/i915/bxt: Mark WaDisablePartialResolveInVc as for Broxton also. drm/i915/bxt: Mark WaCcsTlbPrefetchDisable as for Broxton also. drivers/gpu/drm/i915/intel_ringbuffer.c | 49 + 1 file changed, 26 insertions(+), 23 deletions(-) -- 2.1.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 9/9] drm/i915/bxt: Mark WaCcsTlbPrefetchDisable as for Broxton also.
Signed-off-by: Nick Hoath --- drivers/gpu/drm/i915/intel_ringbuffer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index e5c9f9a..001343f 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -957,7 +957,7 @@ static int gen9_init_workarounds(struct intel_engine_cs *ring) /* WaDisablePartialResolveInVc:skl,bxt */ WA_SET_BIT_MASKED(CACHE_MODE_1, GEN9_PARTIAL_RESOLVE_IN_VC_DISABLE); - /* WaCcsTlbPrefetchDisable:skl */ + /* WaCcsTlbPrefetchDisable:skl,bxt */ WA_CLR_BIT_MASKED(GEN9_HALF_SLICE_CHICKEN5, GEN9_CCS_TLB_PREFETCH_ENABLE); -- 2.1.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 7/9] drm/i915/bxt: Mark Wa4x4STCOptimizationDisable as for Broxton also.
Signed-off-by: Nick Hoath --- drivers/gpu/drm/i915/intel_ringbuffer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index dec0e74..cf36c6b 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -951,7 +951,7 @@ static int gen9_init_workarounds(struct intel_engine_cs *ring) GEN9_ENABLE_YV12_BUGFIX); } - /* Wa4x4STCOptimizationDisable:skl */ + /* Wa4x4STCOptimizationDisable:skl,bxt */ WA_SET_BIT_MASKED(CACHE_MODE_1, GEN8_4x4_STC_OPTIMIZATION_DISABLE); /* WaDisablePartialResolveInVc:skl */ -- 2.1.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 6/9] drm/i915/bxt: Move WaForceEnableNonCoherent to Skylake only
Signed-off-by: Nick Hoath --- drivers/gpu/drm/i915/intel_ringbuffer.c | 22 +++--- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 076d3e5..dec0e74 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -951,17 +951,6 @@ static int gen9_init_workarounds(struct intel_engine_cs *ring) GEN9_ENABLE_YV12_BUGFIX); } - if (INTEL_REVID(dev) <= SKL_REVID_D0) { - /* -*Use Force Non-Coherent whenever executing a 3D context. This -* is a workaround for a possible hang in the unlikely event -* a TLB invalidation occurs during a PSD flush. -*/ - /* WaForceEnableNonCoherent:skl */ - WA_SET_BIT_MASKED(HDC_CHICKEN0, - HDC_FORCE_NON_COHERENT); - } - /* Wa4x4STCOptimizationDisable:skl */ WA_SET_BIT_MASKED(CACHE_MODE_1, GEN8_4x4_STC_OPTIMIZATION_DISABLE); @@ -1039,6 +1028,17 @@ static int skl_init_workarounds(struct intel_engine_cs *ring) WA_SET_BIT_MASKED(HIZ_CHICKEN, BDW_HIZ_POWER_COMPILER_CLOCK_GATING_DISABLE); + if (INTEL_REVID(dev) <= SKL_REVID_D0) { + /* +*Use Force Non-Coherent whenever executing a 3D context. This +* is a workaround for a possible hang in the unlikely event +* a TLB invalidation occurs during a PSD flush. +*/ + /* WaForceEnableNonCoherent:skl */ + WA_SET_BIT_MASKED(HDC_CHICKEN0, + HDC_FORCE_NON_COHERENT); + } + return skl_tune_iz_hashing(ring); } -- 2.1.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 5/9] drm/i915/bxt: Enable WaEnableYV12BugFixInHalfSliceChicken7 for Broxton
Signed-off-by: Nick Hoath --- drivers/gpu/drm/i915/intel_ringbuffer.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index ac1ad44..076d3e5 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -944,8 +944,9 @@ static int gen9_init_workarounds(struct intel_engine_cs *ring) DISABLE_PIXEL_MASK_CAMMING); } - if (INTEL_REVID(dev) >= SKL_REVID_C0) { - /* WaEnableYV12BugFixInHalfSliceChicken7:skl */ + if ((IS_SKYLAKE(dev) && INTEL_REVID(dev) >= SKL_REVID_C0) || + IS_BROXTON(dev)) { + /* WaEnableYV12BugFixInHalfSliceChicken7:skl,bxt */ WA_SET_BIT_MASKED(GEN9_HALF_SLICE_CHICKEN7, GEN9_ENABLE_YV12_BUGFIX); } -- 2.1.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/3] drm/i915: Rename dp rates array as per platform
On Thu, May 07, 2015 at 11:35:40AM +0300, Ville Syrjälä wrote: > On Thu, May 07, 2015 at 09:52:08AM +0530, Sonika Jindal wrote: > > Renaming gen9_rates to skl_rates because other platforms may have different > > supported rates. > > > > Signed-off-by: Sonika Jindal > > Reviewed-by: Ville Syrjälä Queued for -next, thanks for the patch. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 8/9] drm/i915/bxt: Mark WaDisablePartialResolveInVc as for Broxton also.
Signed-off-by: Nick Hoath --- drivers/gpu/drm/i915/intel_ringbuffer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index cf36c6b..e5c9f9a 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -954,7 +954,7 @@ static int gen9_init_workarounds(struct intel_engine_cs *ring) /* Wa4x4STCOptimizationDisable:skl,bxt */ WA_SET_BIT_MASKED(CACHE_MODE_1, GEN8_4x4_STC_OPTIMIZATION_DISABLE); - /* WaDisablePartialResolveInVc:skl */ + /* WaDisablePartialResolveInVc:skl,bxt */ WA_SET_BIT_MASKED(CACHE_MODE_1, GEN9_PARTIAL_RESOLVE_IN_VC_DISABLE); /* WaCcsTlbPrefetchDisable:skl */ -- 2.1.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Remove locking for get-caching query
On Thu, May 07, 2015 at 12:14:55PM +0100, Chris Wilson wrote: > Reading a single value from the object, the locking only provides futile > protection against userspace races. The locking is useless so remove it. > > Signed-off-by: Chris Wilson > --- > drivers/gpu/drm/i915/i915_gem.c | 17 - > 1 file changed, 4 insertions(+), 13 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 2b2b74dbb446..d071d0af2a6c 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -3983,17 +3983,10 @@ int i915_gem_get_caching_ioctl(struct drm_device > *dev, void *data, > { > struct drm_i915_gem_caching *args = data; > struct drm_i915_gem_object *obj; > - int ret; > - > - ret = i915_mutex_lock_interruptible(dev); > - if (ret) > - return ret; > > obj = to_intel_bo(drm_gem_object_lookup(dev, file, args->handle)); > - if (&obj->base == NULL) { > - ret = -ENOENT; > - goto unlock; > - } > + if (&obj->base == NULL) > + return -ENOENT; > > switch (obj->cache_level) { Wrap this in ACCESS_ONCE, just for documentation purpose? Can do while applying if you ack. -Daniel > case I915_CACHE_LLC: > @@ -4010,10 +4003,8 @@ int i915_gem_get_caching_ioctl(struct drm_device *dev, > void *data, > break; > } > > - drm_gem_object_unreference(&obj->base); > -unlock: > - mutex_unlock(&dev->struct_mutex); > - return ret; > + drm_gem_object_unreference_unlocked(&obj->base); > + return 0; > } > > int i915_gem_set_caching_ioctl(struct drm_device *dev, void *data, > -- > 2.1.4 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Remove locking for get-caching query
On Thu, May 07, 2015 at 03:22:41PM +0200, Daniel Vetter wrote: > On Thu, May 07, 2015 at 12:14:55PM +0100, Chris Wilson wrote: > > Reading a single value from the object, the locking only provides futile > > protection against userspace races. The locking is useless so remove it. > > > > Signed-off-by: Chris Wilson > > --- > > drivers/gpu/drm/i915/i915_gem.c | 17 - > > 1 file changed, 4 insertions(+), 13 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_gem.c > > b/drivers/gpu/drm/i915/i915_gem.c > > index 2b2b74dbb446..d071d0af2a6c 100644 > > --- a/drivers/gpu/drm/i915/i915_gem.c > > +++ b/drivers/gpu/drm/i915/i915_gem.c > > @@ -3983,17 +3983,10 @@ int i915_gem_get_caching_ioctl(struct drm_device > > *dev, void *data, > > { > > struct drm_i915_gem_caching *args = data; > > struct drm_i915_gem_object *obj; > > - int ret; > > - > > - ret = i915_mutex_lock_interruptible(dev); > > - if (ret) > > - return ret; > > > > obj = to_intel_bo(drm_gem_object_lookup(dev, file, args->handle)); > > - if (&obj->base == NULL) { > > - ret = -ENOENT; > > - goto unlock; > > - } > > + if (&obj->base == NULL) > > + return -ENOENT; > > > > switch (obj->cache_level) { > > Wrap this in ACCESS_ONCE, just for documentation purpose? Can do while > applying if you ack. I was going to but I thought obj->cache_level was another bitfield... It is :| -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/skl: Fix FF_SLICE_CS_CHICKEN2 offset
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 6331 -Summary- Platform Delta drm-intel-nightly Series Applied PNV 276/276 276/276 ILK 302/302 302/302 SNB 316/316 316/316 IVB 342/342 342/342 BYT 286/286 286/286 BDW 321/321 321/321 -Detailed- Platform Testdrm-intel-nightly Series Applied Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fix screen flickering on X
On Thu, May 07, 2015 at 12:12:18PM +0300, Jani Nikula wrote: > On Thu, 23 Apr 2015, Chris Wilson wrote: > > [cc'ing the authors] > > This has been posted earlier [1] and it has review to be addressed [2]. > > BR, > Jani. I agree with Ander's response in [2]...we can't call intel_update_watermarks() in the commit function because we're under vblank evasion. We should already be flagging the transaction as needing a watermark update in intel_check_cursor_plane(), and that flag will be acted upon immediately after the commit functions are done running, once we've re-enabled interrupts. Note that our current codebase looks a bit different since we've dropped intel_crtc->cursor_{width,height}. So the relevant check in intel_check_cursor_plane() now looks like: if (plane->state->crtc_w != state->base.crtc_w) intel_crtc->atomic.update_wm = true; Is there a bugzilla open on this issue with more details? Matt > > > [1] http://mid.gmane.org/1428790644-6812-1-git-send-email-ism...@iodev.co.uk > [2] http://mid.gmane.org/1428928418.2654.8.ca...@gmail.com > > > > > > On Sat, Apr 11, 2015 at 07:40:34PM -0300, Ismael Luceno wrote: > >> A bisect showed that commit 32b7eeec4d1e861230b09d437e95d76c86ff4a68 > >> introduced the issue. > >> > >> The issue starts as soon as X takes control of the screen, even if just > >> a plain X doing nothing, so based on the code touched by the commit I > >> thought it had to be related to the so called "hardware cursor". I > >> confirmed it when hiding the cursor made the flickering go away. > >> > >> The aforementioned commit removed some suspicious code, and the > >> Programmer's Reference Manual confirmed my suspicion: > >> > >> "Incorrectly programmed watermark values can result in screen corruption. > >> > >> The watermarks should be calculated and programmed when any of the > >> watermark calculation inputs change. This includes planes enabling or > >> disabling, plane source format or size changing, etc." > >> > >> So I'm re-adding the few lines that update the watermarks after a cursor > >> size change. > >> > >> Signed-off-by: Ismael Luceno > >> --- > >> drivers/gpu/drm/i915/intel_display.c | 7 ++- > >> 1 file changed, 6 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/gpu/drm/i915/intel_display.c > >> b/drivers/gpu/drm/i915/intel_display.c > >> index f75173c..e23f062 100644 > >> --- a/drivers/gpu/drm/i915/intel_display.c > >> +++ b/drivers/gpu/drm/i915/intel_display.c > >> @@ -12258,6 +12258,7 @@ intel_commit_cursor_plane(struct drm_plane *plane, > >>struct intel_crtc *intel_crtc; > >>struct intel_plane *intel_plane = to_intel_plane(plane); > >>struct drm_i915_gem_object *obj = intel_fb_obj(state->base.fb); > >> + unsigned old_width; > >>uint32_t addr; > >> > >>crtc = crtc ? crtc : plane->crtc; > >> @@ -12282,11 +12283,15 @@ intel_commit_cursor_plane(struct drm_plane > >> *plane, > >>intel_crtc->cursor_addr = addr; > >>intel_crtc->cursor_bo = obj; > >> update: > >> + old_width = intel_crtc->cursor_width; > >>intel_crtc->cursor_width = state->base.crtc_w; > >>intel_crtc->cursor_height = state->base.crtc_h; > >> > >> - if (intel_crtc->active) > >> + if (intel_crtc->active) { > >> + if (old_width != intel_crtc->cursor_width) > >> + intel_update_watermarks(crtc); > >>intel_crtc_update_cursor(crtc, state->visible); > >> + } > >> } > >> > >> static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev, > >> -- > >> 2.3.4 > > > > -- > > Chris Wilson, Intel Open Source Technology Centre > > ___ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Jani Nikula, Intel Open Source Technology Center -- Matt Roper Graphics Software Engineer IoTG Platform Enabling & Development Intel Corporation (916) 356-2795 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [BUG] i915: suspend by closing Laptop lid broken
Am 2015-05-04 um 13:24 schrieb Jani Nikula: > On Mon, 04 May 2015, Martin Kepplinger wrote: >> So. -rc1 broke suspending by closing my laptop lid and it's not fixed in >> -rc2. It works exactly *one* first time and every subsequent lid-closing >> is ignored. >> >> Biscted and tested first bad commit: >> 14aa02449064541217836b9f3d3295e241d5ae9c >> >> This pulls in i915 changes as well as ACPI changes. I don't know the >> driver but I'm sure you can find the mistake. I'm happy to test changes. >> >> There are no log differences. > > Any chance you could bisect into the merge? It would be helpful. > > BR, > Jani. > > My attempt to go into the merge was too much effort as the checkouts in between break random other stuff. We should be between 09d51602cf84a1264946711dd4ea0dddbac599a1 (good) and c0f404284192f2d4a0159a714372a8c8610c1f6d. Could you have a look and maybe you have guesses for me, what I should test? It's not sooo much anymore, maybe you even have a guess about the bug? thanks martin ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH RESEND 2/5] drm/i915/opregion: add new opregion stuff
Inluding extended didl and cpdl fields Present since opregion version 3.0. Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/intel_opregion.c | 15 --- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c index c4756a2d77bb..d05a504fd176 100644 --- a/drivers/gpu/drm/i915/intel_opregion.c +++ b/drivers/gpu/drm/i915/intel_opregion.c @@ -53,6 +53,7 @@ #define MBOX_ACPI (1<<0) #define MBOX_SWSCI (1<<1) #define MBOX_ASLE (1<<2) +#define MBOX_ASLE_EXT (1<<4) struct opregion_header { u8 signature[16]; @@ -62,7 +63,10 @@ struct opregion_header { u8 vbios_ver[16]; u8 driver_ver[16]; u32 mboxes; - u8 reserved[164]; + u32 driver_model; + u32 pcon; + u8 dver[32]; + u8 rsvd[124]; } __packed; /* OpRegion mailbox #1: public ACPI methods */ @@ -84,7 +88,9 @@ struct opregion_acpi { u32 evts; /* ASL supported events */ u32 cnot; /* current OS notification */ u32 nrdy; /* driver status */ - u8 rsvd2[60]; + u32 did2[7];/* extended supported display devices ID list */ + u32 cpd2[7];/* extended attached display devices list */ + u8 rsvd2[4]; } __packed; /* OpRegion mailbox #2: SWSCI */ @@ -113,7 +119,10 @@ struct opregion_asle { u32 pcft; /* power conservation features */ u32 srot; /* supported rotation angles */ u32 iuer; /* IUER events */ - u8 rsvd[86]; + u64 fdss; + u32 fdsp; + u32 stat; + u8 rsvd[70]; } __packed; /* Driver readiness indicator */ -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH RESEND 5/5] drm/i915/opregion: start using extended didl
Adding support for did2, or the extended support display devices ID list, increases the total to 15. Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/intel_opregion.c | 28 +--- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c index 6235d9acce45..7df916e914a4 100644 --- a/drivers/gpu/drm/i915/intel_opregion.c +++ b/drivers/gpu/drm/i915/intel_opregion.c @@ -669,7 +669,7 @@ static void intel_didl_outputs(struct drm_device *dev) struct acpi_device *acpi_dev, *acpi_cdev, *acpi_video_bus = NULL; unsigned long long device_id; acpi_status status; - u32 temp; + u32 temp, max_outputs; int i = 0; handle = ACPI_HANDLE(&dev->pdev->dev); @@ -692,9 +692,20 @@ static void intel_didl_outputs(struct drm_device *dev) return; } + /* +* In theory, did2, the extended didl, gets added at opregion version +* 3.0. In practice, however, we're supposed to set it for earlier +* versions as well, since a BIOS that doesn't understand did2 should +* not look at it anyway. Use a variable so we can tweak this if a need +* arises later. +*/ + max_outputs = ARRAY_SIZE(opregion->acpi->didl) + + ARRAY_SIZE(opregion->acpi->did2); + list_for_each_entry(acpi_cdev, &acpi_video_bus->children, node) { - if (i >= 8) { - DRM_DEBUG_KMS("More than 8 outputs detected via ACPI\n"); + if (i >= max_outputs) { + DRM_DEBUG_KMS("More than %u outputs detected via ACPI\n", + max_outputs); return; } status = acpi_evaluate_integer(acpi_cdev->handle, "_ADR", @@ -707,8 +718,10 @@ static void intel_didl_outputs(struct drm_device *dev) } end: - /* If fewer than 8 outputs, the list must be null terminated */ - if (i < 8) + DRM_DEBUG_KMS("%d outputs detected\n", i); + + /* If fewer than max outputs, the list must be null terminated */ + if (i < max_outputs) set_did(opregion, i, 0); return; @@ -716,8 +729,9 @@ blind_set: i = 0; list_for_each_entry(connector, &dev->mode_config.connector_list, head) { int output_type = ACPI_OTHER_OUTPUT; - if (i >= 8) { - DRM_DEBUG_KMS("More than 8 outputs in connector list\n"); + if (i >= max_outputs) { + DRM_DEBUG_KMS("More than %u outputs in connector list\n", + max_outputs); return; } switch (connector->connector_type) { -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH RESEND 3/5] drm/i915/opregion: prefer DRM logging functions over pr_warn and dev_dbg
Conform to same style as the rest of the driver. Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/intel_opregion.c | 10 +++--- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c index d05a504fd176..5ab75a6a4131 100644 --- a/drivers/gpu/drm/i915/intel_opregion.c +++ b/drivers/gpu/drm/i915/intel_opregion.c @@ -25,8 +25,6 @@ * */ -#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt - #include #include @@ -658,14 +656,13 @@ static void intel_didl_outputs(struct drm_device *dev) } if (!acpi_video_bus) { - pr_warn("No ACPI video bus found\n"); + DRM_ERROR("No ACPI video bus found\n"); return; } list_for_each_entry(acpi_cdev, &acpi_video_bus->children, node) { if (i >= 8) { - dev_dbg(&dev->pdev->dev, - "More than 8 outputs detected via ACPI\n"); + DRM_DEBUG_KMS("More than 8 outputs detected via ACPI\n"); return; } status = @@ -691,8 +688,7 @@ blind_set: list_for_each_entry(connector, &dev->mode_config.connector_list, head) { int output_type = ACPI_OTHER_OUTPUT; if (i >= 8) { - dev_dbg(&dev->pdev->dev, - "More than 8 outputs in connector list\n"); + DRM_DEBUG_KMS("More than 8 outputs in connector list\n"); return; } switch (connector->connector_type) { -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH RESEND 4/5] drm/i915/opregion: abstract didl and did2 getter and setter
Make it easier to handle the extended didl. No functional changes. Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/intel_opregion.c | 50 +++ 1 file changed, 39 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c index 5ab75a6a4131..6235d9acce45 100644 --- a/drivers/gpu/drm/i915/intel_opregion.c +++ b/drivers/gpu/drm/i915/intel_opregion.c @@ -628,6 +628,38 @@ static struct notifier_block intel_opregion_notifier = { * (version 3) */ +static u32 get_did(struct intel_opregion *opregion, int i) +{ + u32 did; + + if (i < ARRAY_SIZE(opregion->acpi->didl)) { + did = ioread32(&opregion->acpi->didl[i]); + } else { + i -= ARRAY_SIZE(opregion->acpi->didl); + + if (WARN_ON(i >= ARRAY_SIZE(opregion->acpi->did2))) + return 0; + + did = ioread32(&opregion->acpi->did2[i]); + } + + return did; +} + +static void set_did(struct intel_opregion *opregion, int i, u32 val) +{ + if (i < ARRAY_SIZE(opregion->acpi->didl)) { + iowrite32(val, &opregion->acpi->didl[i]); + } else { + i -= ARRAY_SIZE(opregion->acpi->didl); + + if (WARN_ON(i >= ARRAY_SIZE(opregion->acpi->did2))) + return; + + iowrite32(val, &opregion->acpi->did2[i]); + } +} + static void intel_didl_outputs(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; @@ -665,22 +697,19 @@ static void intel_didl_outputs(struct drm_device *dev) DRM_DEBUG_KMS("More than 8 outputs detected via ACPI\n"); return; } - status = - acpi_evaluate_integer(acpi_cdev->handle, "_ADR", - NULL, &device_id); + status = acpi_evaluate_integer(acpi_cdev->handle, "_ADR", + NULL, &device_id); if (ACPI_SUCCESS(status)) { if (!device_id) goto blind_set; - iowrite32((u32)(device_id & 0x0f0f), - &opregion->acpi->didl[i]); - i++; + set_did(opregion, i++, (u32)(device_id & 0x0f0f)); } } end: /* If fewer than 8 outputs, the list must be null terminated */ if (i < 8) - iowrite32(0, &opregion->acpi->didl[i]); + set_did(opregion, i, 0); return; blind_set: @@ -713,9 +742,8 @@ blind_set: output_type = ACPI_LVDS_OUTPUT; break; } - temp = ioread32(&opregion->acpi->didl[i]); - iowrite32(temp | (1<<31) | output_type | i, - &opregion->acpi->didl[i]); + temp = get_did(opregion, i); + set_did(opregion, i, temp | (1 << 31) | output_type | i); i++; } goto end; @@ -735,7 +763,7 @@ static void intel_setup_cadls(struct drm_device *dev) * display switching hotkeys. Just like DIDL, CADL is NULL-terminated if * there are less than eight devices. */ do { - disp_id = ioread32(&opregion->acpi->didl[i]); + disp_id = get_did(opregion, i); iowrite32(disp_id, &opregion->acpi->cadl[i]); } while (++i < 8 && disp_id != 0); } -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH RESEND 1/5] drm/i915/opregion: use BUILD_BUG_ON to verify mailbox struct sizes
Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/intel_opregion.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c index 71e87abdcae7..c4756a2d77bb 100644 --- a/drivers/gpu/drm/i915/intel_opregion.c +++ b/drivers/gpu/drm/i915/intel_opregion.c @@ -862,6 +862,11 @@ int intel_opregion_setup(struct drm_device *dev) char buf[sizeof(OPREGION_SIGNATURE)]; int err = 0; + BUILD_BUG_ON(sizeof(struct opregion_header) != 0x100); + BUILD_BUG_ON(sizeof(struct opregion_acpi) != 0x100); + BUILD_BUG_ON(sizeof(struct opregion_swsci) != 0x100); + BUILD_BUG_ON(sizeof(struct opregion_asle) != 0x100); + pci_read_config_dword(dev->pdev, PCI_ASLS, &asls); DRM_DEBUG_DRIVER("graphic opregion physical addr: 0x%x\n", asls); if (asls == 0) { -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fix screen flickering on X
On Thu, 07 May 2015, Matt Roper wrote: > On Thu, May 07, 2015 at 12:12:18PM +0300, Jani Nikula wrote: >> On Thu, 23 Apr 2015, Chris Wilson wrote: >> > [cc'ing the authors] >> >> This has been posted earlier [1] and it has review to be addressed [2]. >> >> BR, >> Jani. > > I agree with Ander's response in [2]...we can't call > intel_update_watermarks() in the commit function because we're under > vblank evasion. We should already be flagging the transaction as > needing a watermark update in intel_check_cursor_plane(), and that flag > will be acted upon immediately after the commit functions are done > running, once we've re-enabled interrupts. > > Note that our current codebase looks a bit different since we've dropped > intel_crtc->cursor_{width,height}. So the relevant check in > intel_check_cursor_plane() now looks like: > > if (plane->state->crtc_w != state->base.crtc_w) > intel_crtc->atomic.update_wm = true; > > Is there a bugzilla open on this issue with more details? Not that I know of. Ismael? BR, Jani. > > > Matt > >> >> >> [1] http://mid.gmane.org/1428790644-6812-1-git-send-email-ism...@iodev.co.uk >> [2] http://mid.gmane.org/1428928418.2654.8.ca...@gmail.com >> >> >> > >> > On Sat, Apr 11, 2015 at 07:40:34PM -0300, Ismael Luceno wrote: >> >> A bisect showed that commit 32b7eeec4d1e861230b09d437e95d76c86ff4a68 >> >> introduced the issue. >> >> >> >> The issue starts as soon as X takes control of the screen, even if just >> >> a plain X doing nothing, so based on the code touched by the commit I >> >> thought it had to be related to the so called "hardware cursor". I >> >> confirmed it when hiding the cursor made the flickering go away. >> >> >> >> The aforementioned commit removed some suspicious code, and the >> >> Programmer's Reference Manual confirmed my suspicion: >> >> >> >> "Incorrectly programmed watermark values can result in screen corruption. >> >> >> >> The watermarks should be calculated and programmed when any of the >> >> watermark calculation inputs change. This includes planes enabling or >> >> disabling, plane source format or size changing, etc." >> >> >> >> So I'm re-adding the few lines that update the watermarks after a cursor >> >> size change. >> >> >> >> Signed-off-by: Ismael Luceno >> >> --- >> >> drivers/gpu/drm/i915/intel_display.c | 7 ++- >> >> 1 file changed, 6 insertions(+), 1 deletion(-) >> >> >> >> diff --git a/drivers/gpu/drm/i915/intel_display.c >> >> b/drivers/gpu/drm/i915/intel_display.c >> >> index f75173c..e23f062 100644 >> >> --- a/drivers/gpu/drm/i915/intel_display.c >> >> +++ b/drivers/gpu/drm/i915/intel_display.c >> >> @@ -12258,6 +12258,7 @@ intel_commit_cursor_plane(struct drm_plane *plane, >> >> struct intel_crtc *intel_crtc; >> >> struct intel_plane *intel_plane = to_intel_plane(plane); >> >> struct drm_i915_gem_object *obj = intel_fb_obj(state->base.fb); >> >> + unsigned old_width; >> >> uint32_t addr; >> >> >> >> crtc = crtc ? crtc : plane->crtc; >> >> @@ -12282,11 +12283,15 @@ intel_commit_cursor_plane(struct drm_plane >> >> *plane, >> >> intel_crtc->cursor_addr = addr; >> >> intel_crtc->cursor_bo = obj; >> >> update: >> >> + old_width = intel_crtc->cursor_width; >> >> intel_crtc->cursor_width = state->base.crtc_w; >> >> intel_crtc->cursor_height = state->base.crtc_h; >> >> >> >> - if (intel_crtc->active) >> >> + if (intel_crtc->active) { >> >> + if (old_width != intel_crtc->cursor_width) >> >> + intel_update_watermarks(crtc); >> >> intel_crtc_update_cursor(crtc, state->visible); >> >> + } >> >> } >> >> >> >> static struct drm_plane *intel_cursor_plane_create(struct drm_device >> >> *dev, >> >> -- >> >> 2.3.4 >> > >> > -- >> > Chris Wilson, Intel Open Source Technology Centre >> > ___ >> > Intel-gfx mailing list >> > Intel-gfx@lists.freedesktop.org >> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx >> >> -- >> Jani Nikula, Intel Open Source Technology Center > > -- > Matt Roper > Graphics Software Engineer > IoTG Platform Enabling & Development > Intel Corporation > (916) 356-2795 -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Remove locking for get-caching query
Chris Wilson writes: > Reading a single value from the object, the locking only provides futile > protection against userspace races. The locking is useless so remove it. > > Signed-off-by: Chris Wilson Reviewed-by: Mika Kuoppala > --- > drivers/gpu/drm/i915/i915_gem.c | 17 - > 1 file changed, 4 insertions(+), 13 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 2b2b74dbb446..d071d0af2a6c 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -3983,17 +3983,10 @@ int i915_gem_get_caching_ioctl(struct drm_device > *dev, void *data, > { > struct drm_i915_gem_caching *args = data; > struct drm_i915_gem_object *obj; > - int ret; > - > - ret = i915_mutex_lock_interruptible(dev); > - if (ret) > - return ret; > > obj = to_intel_bo(drm_gem_object_lookup(dev, file, args->handle)); > - if (&obj->base == NULL) { > - ret = -ENOENT; > - goto unlock; > - } > + if (&obj->base == NULL) > + return -ENOENT; > > switch (obj->cache_level) { > case I915_CACHE_LLC: > @@ -4010,10 +4003,8 @@ int i915_gem_get_caching_ioctl(struct drm_device *dev, > void *data, > break; > } > > - drm_gem_object_unreference(&obj->base); > -unlock: > - mutex_unlock(&dev->struct_mutex); > - return ret; > + drm_gem_object_unreference_unlocked(&obj->base); > + return 0; > } > > int i915_gem_set_caching_ioctl(struct drm_device *dev, void *data, > -- > 2.1.4 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [BUG] i915: suspend by closing Laptop lid broken
On Thu, 07 May 2015, Martin Kepplinger wrote: > Am 2015-05-04 um 13:24 schrieb Jani Nikula: >> On Mon, 04 May 2015, Martin Kepplinger wrote: >>> So. -rc1 broke suspending by closing my laptop lid and it's not fixed in >>> -rc2. It works exactly *one* first time and every subsequent lid-closing >>> is ignored. >>> >>> Biscted and tested first bad commit: >>> 14aa02449064541217836b9f3d3295e241d5ae9c >>> >>> This pulls in i915 changes as well as ACPI changes. I don't know the >>> driver but I'm sure you can find the mistake. I'm happy to test changes. >>> >>> There are no log differences. >> >> Any chance you could bisect into the merge? It would be helpful. >> >> BR, >> Jani. >> >> > > My attempt to go into the merge was too much effort as the checkouts in > between break random other stuff. This should obviously *not* be the case. :( > We should be between 09d51602cf84a1264946711dd4ea0dddbac599a1 (good) and > c0f404284192f2d4a0159a714372a8c8610c1f6d. Could you have a look and > maybe you have guesses for me, what I should test? It's not sooo much > anymore, maybe you even have a guess about the bug? $ git log --oneline 09d51602cf..c0f4042841 | grep drm/i915 | wc -l 286 Oof, I eyeballed the list, but didn't spot any obvious candidates. If a commit is inconclusive or borken in other ways, you could try git bisect skip to try another commit... BR, Jani. -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: clean up dsi pll calculation
Improve readability. No functional changes. Signed-off-by: Jani Nikula --- Cleaning up old branches... :) --- drivers/gpu/drm/i915/intel_dsi_pll.c | 52 1 file changed, 17 insertions(+), 35 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c index 3622d0bafdf8..fc139946abee 100644 --- a/drivers/gpu/drm/i915/intel_dsi_pll.c +++ b/drivers/gpu/drm/i915/intel_dsi_pll.c @@ -162,53 +162,35 @@ static u32 dsi_clk_from_pclk(u32 pclk, int pixel_format, int lane_count) #endif -static int dsi_calc_mnp(u32 dsi_clk, struct dsi_mnp *dsi_mnp) +static int dsi_calc_mnp(u32 target_dsi_clk, struct dsi_mnp *dsi_mnp) { u32 m, n, p; - u32 ref_clk; - u32 error; - u32 tmp_error; - int target_dsi_clk; - int calc_dsi_clk; - u32 calc_m; - u32 calc_p; + u32 ref_clk = 25000; + u32 delta = target_dsi_clk; + u32 calc_m = 0; + u32 calc_p = 0; u32 m_seed; - /* dsi_clk is expected in KHZ */ - if (dsi_clk < 30 || dsi_clk > 115) { + /* target_dsi_clk is expected in KHZ */ + if (target_dsi_clk < 30 || target_dsi_clk > 115) { DRM_ERROR("DSI CLK Out of Range\n"); return -ECHRNG; } - ref_clk = 25000; - target_dsi_clk = dsi_clk; - error = 0x; - tmp_error = 0x; - calc_m = 0; - calc_p = 0; - - for (m = 62; m <= 92; m++) { - for (p = 2; p <= 6; p++) { - /* Find the optimal m and p divisors - with minimal error +/- the required clock */ - calc_dsi_clk = (m * ref_clk) / p; - if (calc_dsi_clk == target_dsi_clk) { - calc_m = m; - calc_p = p; - error = 0; - break; - } else - tmp_error = abs(target_dsi_clk - calc_dsi_clk); - - if (tmp_error < error) { - error = tmp_error; + for (m = 62; m <= 92 && delta; m++) { + for (p = 2; p <= 6 && delta; p++) { + /* +* Find the optimal m and p divisors with minimal delta +* +/- the required clock +*/ + int calc_dsi_clk = (m * ref_clk) / p; + u32 d = abs(target_dsi_clk - calc_dsi_clk); + if (d < delta) { + delta = d; calc_m = m; calc_p = p; } } - - if (error == 0) - break; } m_seed = lfsr_converts[calc_m - 62]; -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC PATCH 02/11] perf: Add PERF_PMU_CAP_IS_DEVICE flag
The PERF_PMU_CAP_IS_DEVICE flag provides pmu drivers a way to declare that they only monitor device specific metrics and since they don't monitor any cpu metrics then perf should bypass any cpu centric security checks, as well as disallow cpu centric attributes. Signed-off-by: Robert Bragg --- include/linux/perf_event.h | 1 + kernel/events/core.c | 39 +-- 2 files changed, 34 insertions(+), 6 deletions(-) diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h index 2b62198..1af35b4 100644 --- a/include/linux/perf_event.h +++ b/include/linux/perf_event.h @@ -166,6 +166,7 @@ struct perf_event; * pmu::capabilities flags */ #define PERF_PMU_CAP_NO_INTERRUPT 0x01 +#define PERF_PMU_CAP_IS_DEVICE 0x02 /** * struct pmu - generic performance monitoring unit diff --git a/kernel/events/core.c b/kernel/events/core.c index 38c240c..7218b01 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -3330,7 +3330,8 @@ find_get_context(struct pmu *pmu, struct task_struct *task, int cpu) if (!task) { /* Must be root to operate on a CPU event: */ - if (perf_paranoid_cpu() && !capable(CAP_SYS_ADMIN)) + if (!(pmu->capabilities & PERF_PMU_CAP_IS_DEVICE) && + perf_paranoid_cpu() && !capable(CAP_SYS_ADMIN)) return ERR_PTR(-EACCES); /* @@ -7475,11 +7476,6 @@ SYSCALL_DEFINE5(perf_event_open, if (err) return err; - if (!attr.exclude_kernel) { - if (perf_paranoid_kernel() && !capable(CAP_SYS_ADMIN)) - return -EACCES; - } - if (attr.freq) { if (attr.sample_freq > sysctl_perf_event_sample_rate) return -EINVAL; @@ -7538,6 +7534,37 @@ SYSCALL_DEFINE5(perf_event_open, goto err_cpus; } + if (event->pmu->capabilities & PERF_PMU_CAP_IS_DEVICE) { + + /* Don't allow cpu centric attributes... */ + if (event->attr.exclude_user || + event->attr.exclude_callchain_user || + event->attr.exclude_kernel || + event->attr.exclude_callchain_kernel || + event->attr.exclude_hv || + event->attr.exclude_idle || + event->attr.exclude_host || + event->attr.exclude_guest || + event->attr.mmap || + event->attr.comm || + event->attr.task) + return -EINVAL; + + if (attr.sample_type & + (PERF_SAMPLE_IP | +PERF_SAMPLE_TID | +PERF_SAMPLE_ADDR | +PERF_SAMPLE_CALLCHAIN | +PERF_SAMPLE_CPU | +PERF_SAMPLE_BRANCH_STACK | +PERF_SAMPLE_REGS_USER | +PERF_SAMPLE_STACK_USER)) + return -EINVAL; + } else if (!attr.exclude_kernel) { + if (perf_paranoid_kernel() && !capable(CAP_SYS_ADMIN)) + return -EACCES; + } + if (flags & PERF_FLAG_PID_CGROUP) { err = perf_cgroup_connect(pid, event, &attr, group_leader); if (err) { -- 2.3.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC PATCH 05/11] perf: allow drivers more control over event logging
This exports enough api to allow drivers to output their own PERF_RECORD_DEVICE events. Signed-off-by: Robert Bragg --- include/linux/perf_event.h | 7 +++ kernel/events/core.c| 2 ++ kernel/events/internal.h| 9 - kernel/events/ring_buffer.c | 3 +++ 4 files changed, 12 insertions(+), 9 deletions(-) diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h index 69a0cb9..293f041 100644 --- a/include/linux/perf_event.h +++ b/include/linux/perf_event.h @@ -633,6 +633,10 @@ struct perf_sample_data { PERF_MEM_S(LOCK, NA) |\ PERF_MEM_S(TLB, NA)) +extern void perf_event_header__init_id(struct perf_event_header *header, + struct perf_sample_data *data, + struct perf_event *event); + static inline void perf_sample_data_init(struct perf_sample_data *data, u64 addr, u64 period) { @@ -654,6 +658,9 @@ extern void perf_prepare_sample(struct perf_event_header *header, struct perf_sample_data *data, struct perf_event *event, struct pt_regs *regs); +extern void perf_event__output_id_sample(struct perf_event *event, +struct perf_output_handle *handle, +struct perf_sample_data *sample); extern int perf_event_overflow(struct perf_event *event, struct perf_sample_data *data, diff --git a/kernel/events/core.c b/kernel/events/core.c index 340deaa..26b84fc 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -4793,6 +4793,7 @@ void perf_event_header__init_id(struct perf_event_header *header, if (event->attr.sample_id_all) __perf_event_header__init_id(header, data, event); } +EXPORT_SYMBOL_GPL(perf_event_header__init_id); static void __perf_event__output_id_sample(struct perf_output_handle *handle, struct perf_sample_data *data) @@ -4825,6 +4826,7 @@ void perf_event__output_id_sample(struct perf_event *event, if (event->attr.sample_id_all) __perf_event__output_id_sample(handle, sample); } +EXPORT_SYMBOL_GPL(perf_event__output_id_sample); static void perf_output_read_one(struct perf_output_handle *handle, struct perf_event *event, diff --git a/kernel/events/internal.h b/kernel/events/internal.h index 569b2187..3c86bb3 100644 --- a/kernel/events/internal.h +++ b/kernel/events/internal.h @@ -44,15 +44,6 @@ extern struct ring_buffer * rb_alloc(int nr_pages, long watermark, int cpu, int flags); extern void perf_event_wakeup(struct perf_event *event); -extern void -perf_event_header__init_id(struct perf_event_header *header, - struct perf_sample_data *data, - struct perf_event *event); -extern void -perf_event__output_id_sample(struct perf_event *event, -struct perf_output_handle *handle, -struct perf_sample_data *sample); - extern struct page * perf_mmap_to_page(struct ring_buffer *rb, unsigned long pgoff); diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c index eadb95c..fa100d4 100644 --- a/kernel/events/ring_buffer.c +++ b/kernel/events/ring_buffer.c @@ -202,12 +202,14 @@ out: return -ENOSPC; } +EXPORT_SYMBOL_GPL(perf_output_begin); unsigned int perf_output_copy(struct perf_output_handle *handle, const void *buf, unsigned int len) { return __output_copy(handle, buf, len); } +EXPORT_SYMBOL_GPL(perf_output_copy); unsigned int perf_output_skip(struct perf_output_handle *handle, unsigned int len) @@ -220,6 +222,7 @@ void perf_output_end(struct perf_output_handle *handle) perf_output_put_handle(handle); rcu_read_unlock(); } +EXPORT_SYMBOL_GPL(perf_output_end); static void ring_buffer_init(struct ring_buffer *rb, long watermark, int flags) -- 2.3.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC PATCH 07/11] drm/i915: Expose PMU for Observation Architecture
Gen graphics hardware can be set up to periodically write snapshots of performance counters into a circular buffer and this patch exposes that capability to userspace via the perf interface. To start with this only enables the A (aggregating) counters with the simplest configuration requirements. Only Haswell is supported currently. Signed-off-by: Robert Bragg --- drivers/gpu/drm/i915/Makefile | 1 + drivers/gpu/drm/i915/i915_dma.c | 6 + drivers/gpu/drm/i915/i915_drv.h | 53 +++ drivers/gpu/drm/i915/i915_gem_context.c | 45 +- drivers/gpu/drm/i915/i915_oa_perf.c | 750 drivers/gpu/drm/i915/i915_reg.h | 68 +++ include/uapi/drm/i915_drm.h | 29 ++ 7 files changed, 942 insertions(+), 10 deletions(-) create mode 100644 drivers/gpu/drm/i915/i915_oa_perf.c diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index a69002e..8af9f4a 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -16,6 +16,7 @@ i915-y := i915_drv.o \ i915-$(CONFIG_COMPAT) += i915_ioc32.o i915-$(CONFIG_DEBUG_FS) += i915_debugfs.o +i915-$(CONFIG_PERF_EVENTS) += i915_oa_perf.o # GEM code i915-y += i915_cmd_parser.o \ diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index e44116f..1de6639 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -817,6 +817,11 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) mutex_init(&dev_priv->dpio_lock); mutex_init(&dev_priv->modeset_restore_lock); + /* Must at least be registered before trying to pin any context +* otherwise i915_oa_context_pin_notify() will lock an un-initialized +* spinlock, upsetting lockdep checks */ + i915_oa_pmu_register(dev); + intel_pm_setup(dev); intel_display_crc_init(dev); @@ -1062,6 +1067,7 @@ int i915_driver_unload(struct drm_device *dev) return ret; } + i915_oa_pmu_unregister(dev); intel_power_domains_fini(dev_priv); intel_gpu_ips_teardown(); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index f5020a8..3d8e4ed 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -49,6 +49,7 @@ #include #include #include +#include #include /* General customization: @@ -1816,6 +1817,35 @@ struct drm_i915_private { */ struct workqueue_struct *dp_wq; +#ifdef CONFIG_PERF_EVENTS + struct { + struct pmu pmu; + spinlock_t lock; + struct hrtimer timer; + struct pt_regs dummy_regs; + + struct perf_event *exclusive_event; + struct intel_context *specific_ctx; + bool event_active; + + bool periodic; + u32 period_exponent; + + u32 metrics_set; + + struct { + struct drm_i915_gem_object *obj; + u32 gtt_offset; + u8 *addr; + u32 head; + u32 tail; + int format; + int format_size; + spinlock_t flush_lock; + } oa_buffer; + } oa_pmu; +#endif + /* Abstract the submission mechanism (legacy ringbuffer or execlists) away */ struct { int (*execbuf_submit)(struct drm_device *dev, struct drm_file *file, @@ -2975,6 +3005,20 @@ int i915_gem_context_getparam_ioctl(struct drm_device *dev, void *data, int i915_gem_context_setparam_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv); +#ifdef CONFIG_PERF_EVENTS +void i915_oa_context_pin_notify(struct drm_i915_private *dev_priv, + struct intel_context *context); +void i915_oa_context_unpin_notify(struct drm_i915_private *dev_priv, + struct intel_context *context); +#else +static inline void +i915_oa_context_pin_notify(struct drm_i915_private *dev_priv, + struct intel_context *context) {} +static inline void +i915_oa_context_unpin_notify(struct drm_i915_private *dev_priv, +struct intel_context *context) {} +#endif + /* i915_gem_evict.c */ int __must_check i915_gem_evict_something(struct drm_device *dev, struct i915_address_space *vm, @@ -3084,6 +3128,15 @@ int i915_parse_cmds(struct intel_engine_cs *ring, u32 batch_len, bool is_master); +/* i915_oa_perf.c */ +#ifdef CONFIG_PERF_EVENTS +extern void i915_oa_pmu_register(struct drm_device *dev); +extern void i915_oa_pmu_unregister(struct drm_device *dev); +#else +static inline void i915_oa_pmu_register(struct drm_device *dev) {} +static inline void i915_
[Intel-gfx] [RFC PATCH 10/11] drm/i915: report OA buf overrun + report lost status
This adds two driver specific PERF_RECORD_DEVICE event types for reporting OA buffer overrun and report lost status bits to userspace. Signed-off-by: Robert Bragg --- drivers/gpu/drm/i915/i915_oa_perf.c | 53 - include/uapi/drm/i915_drm.h | 27 +++ 2 files changed, 62 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_oa_perf.c b/drivers/gpu/drm/i915/i915_oa_perf.c index c3e5059..d0dad5d 100644 --- a/drivers/gpu/drm/i915/i915_oa_perf.c +++ b/drivers/gpu/drm/i915/i915_oa_perf.c @@ -159,6 +159,34 @@ static u32 forward_oa_snapshots(struct drm_i915_private *dev_priv, return dev_priv->oa_pmu.oa_buffer.gtt_offset + head; } +static void log_oa_status(struct drm_i915_private *dev_priv, + enum drm_i915_oa_event_type status) +{ + struct { + struct perf_event_header header; + drm_i915_oa_event_header_t i915_oa_header; + } oa_event; + struct perf_output_handle handle; + struct perf_sample_data sample_data; + struct perf_event *event = dev_priv->oa_pmu.exclusive_event; + int ret; + + oa_event.header.size = sizeof(oa_event); + oa_event.header.type = PERF_RECORD_DEVICE; + oa_event.i915_oa_header.type = status; + oa_event.i915_oa_header.__reserved_1 = 0; + + perf_event_header__init_id(&oa_event.header, &sample_data, event); + + ret = perf_output_begin(&handle, event, oa_event.header.size); + if (ret) + return; + + perf_output_put(&handle, oa_event); + perf_event__output_id_sample(event, &handle, &sample_data); + perf_output_end(&handle); +} + static void flush_oa_snapshots(struct drm_i915_private *dev_priv, bool skip_if_flushing) { @@ -189,25 +217,14 @@ static void flush_oa_snapshots(struct drm_i915_private *dev_priv, head = oastatus2 & GEN7_OASTATUS2_HEAD_MASK; tail = oastatus1 & GEN7_OASTATUS1_TAIL_MASK; - if (oastatus1 & (GEN7_OASTATUS1_OABUFFER_OVERFLOW | -GEN7_OASTATUS1_REPORT_LOST)) { + if (unlikely(oastatus1 & (GEN7_OASTATUS1_OABUFFER_OVERFLOW | + GEN7_OASTATUS1_REPORT_LOST))) { - /* XXX: How can we convey report-lost errors to userspace? It -* doesn't look like perf's _REPORT_LOST mechanism is -* appropriate in this case; that's just for cases where we -* run out of space for samples in the perf circular buffer. -* -* Maybe we can claim a special report-id and use that to -* forward status flags? -*/ - pr_debug("OA buffer read error: addr = %p, head = %u, offset = %u, tail = %u cnt o'flow = %d, buf o'flow = %d, rpt lost = %d\n", -dev_priv->oa_pmu.oa_buffer.addr, -head, -head - dev_priv->oa_pmu.oa_buffer.gtt_offset, -tail, -oastatus1 & GEN7_OASTATUS1_COUNTER_OVERFLOW ? 1 : 0, -oastatus1 & GEN7_OASTATUS1_OABUFFER_OVERFLOW ? 1 : 0, -oastatus1 & GEN7_OASTATUS1_REPORT_LOST ? 1 : 0); + if (oastatus1 & GEN7_OASTATUS1_OABUFFER_OVERFLOW) + log_oa_status(dev_priv, I915_OA_RECORD_BUFFER_OVERFLOW); + + if (oastatus1 & GEN7_OASTATUS1_REPORT_LOST) + log_oa_status(dev_priv, I915_OA_RECORD_REPORT_LOST); I915_WRITE(GEN7_OASTATUS1, oastatus1 & ~(GEN7_OASTATUS1_OABUFFER_OVERFLOW | diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index 7aa1d33..2871922 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -89,6 +89,33 @@ typedef struct _drm_i915_oa_attr { __reserved_1 : 63; } drm_i915_oa_attr_t; +/* Header for PERF_RECORD_DEVICE type events */ +typedef struct _drm_i915_oa_event_header { + __u32 type; + __u32 __reserved_1; +} drm_i915_oa_event_header_t; + +enum drm_i915_oa_event_type { + + /* +* struct { +* struct perf_event_headerheader; +* drm_i915_oa_event_header_t i915_oa_header; +* }; +*/ + I915_OA_RECORD_BUFFER_OVERFLOW = 1, + + /* +* struct { +* struct perf_event_headerheader; +* drm_i915_oa_event_header_t i915_oa_header; +* }; +*/ + I915_OA_RECORD_REPORT_LOST = 2, + + I915_OA_RECORD_MAX, /* non-ABI */ +}; + /* Each region is a minimum of 16k, and there are at most 255 of them. */ #define I915_NR_TEX_REGIONS 255/* table size 2k - maximum due to use -- 2.3.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.
[Intel-gfx] [RFC PATCH 08/11] drm/i915: add OA config for 3D render counters
This enables access to some additional counters beyond the aggregating A counters, adding a '3D' metric set configuration useful while profiling 3D rendering workloads. Signed-off-by: Robert Bragg --- drivers/gpu/drm/i915/i915_drv.h | 7 + drivers/gpu/drm/i915/i915_oa_perf.c | 124 +-- drivers/gpu/drm/i915/i915_reg.h | 294 include/uapi/drm/i915_drm.h | 2 + 4 files changed, 385 insertions(+), 42 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 3d8e4ed..1e65dc2 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1564,6 +1564,13 @@ struct i915_virtual_gpu { bool active; }; +#ifdef CONFIG_PERF_EVENTS +struct i915_oa_reg { + u32 addr; + u32 value; +}; +#endif + struct drm_i915_private { struct drm_device *dev; struct kmem_cache *objects; diff --git a/drivers/gpu/drm/i915/i915_oa_perf.c b/drivers/gpu/drm/i915/i915_oa_perf.c index a1cf55f..d0e144c 100644 --- a/drivers/gpu/drm/i915/i915_oa_perf.c +++ b/drivers/gpu/drm/i915/i915_oa_perf.c @@ -23,6 +23,80 @@ static int hsw_perf_format_sizes[] = { 64 /* C4_B8_HSW */ }; + +/* A generated mux config to select counters useful for profiling 3D + * workloads */ +static struct i915_oa_reg hsw_profile_3d_mux_config[] = { + + { 0x253A4, 0x0160 }, + { 0x25440, 0x0010 }, + { 0x25128, 0x }, + { 0x2691C, 0x0800 }, + { 0x26AA0, 0x0150 }, + { 0x26B9C, 0x6000 }, + { 0x2791C, 0x0800 }, + { 0x27AA0, 0x0150 }, + { 0x27B9C, 0x6000 }, + { 0x2641C, 0x0400 }, + { 0x25380, 0x0010 }, + { 0x2538C, 0x }, + { 0x25384, 0x0800 }, + { 0x25400, 0x0004 }, + { 0x2540C, 0x06029000 }, + { 0x25410, 0x0002 }, + { 0x25404, 0x5C30 }, + { 0x25100, 0x0016 }, + { 0x25110, 0x0400 }, + { 0x25104, 0x }, + { 0x26804, 0x1211 }, + { 0x26884, 0x0100 }, + { 0x26900, 0x0002 }, + { 0x26908, 0x0070 }, + { 0x26904, 0x }, + { 0x26984, 0x1022 }, + { 0x26A04, 0x0011 }, + { 0x26A80, 0x0006 }, + { 0x26A88, 0x0C02 }, + { 0x26A84, 0x }, + { 0x26B04, 0x1000 }, + { 0x26B80, 0x0002 }, + { 0x26B8C, 0x0007 }, + { 0x26B84, 0x }, + { 0x27804, 0x4844 }, + { 0x27884, 0x0400 }, + { 0x27900, 0x0002 }, + { 0x27908, 0x0E00 }, + { 0x27904, 0x }, + { 0x27984, 0x4088 }, + { 0x27A04, 0x0044 }, + { 0x27A80, 0x0006 }, + { 0x27A88, 0x00018040 }, + { 0x27A84, 0x }, + { 0x27B04, 0x4000 }, + { 0x27B80, 0x0002 }, + { 0x27B8C, 0x00E0 }, + { 0x27B84, 0x }, + { 0x26104, 0x }, + { 0x26184, 0x0C00 }, + { 0x26284, 0x0400 }, + { 0x26304, 0x0400 }, + { 0x26400, 0x0002 }, + { 0x26410, 0x00A0 }, + { 0x26404, 0x }, + { 0x25420, 0x04108020 }, + { 0x25424, 0x1284A420 }, + { 0x2541C, 0x }, + { 0x25428, 0x00042049 }, +}; + +/* A corresponding B counter configuration for profiling 3D workloads */ +static struct i915_oa_reg hsw_profile_3d_b_counter_config[] = { + { 0x2724, 0x0080 }, + { 0x2720, 0x }, + { 0x2714, 0x0080 }, + { 0x2710, 0x }, +}; + static void forward_one_oa_snapshot_to_event(struct drm_i915_private *dev_priv, u8 *snapshot, struct perf_event *event) @@ -551,6 +625,19 @@ static void update_oacontrol(struct drm_i915_private *dev_priv) I915_WRITE(GEN7_OACONTROL, 0); } +static void config_oa_regs(struct drm_i915_private *dev_priv, + struct i915_oa_reg *regs, + int n_regs) +{ + int i; + + for (i = 0; i < n_regs; i++) { + struct i915_oa_reg *reg = regs + i; + + I915_WRITE(reg->addr, reg->value); + } +} + static void i915_oa_event_start(struct perf_event *event, int flags) { struct drm_i915_private *dev_priv = @@ -571,22 +658,31 @@ static void i915_oa_event_start(struct perf_event *event, int flags) WARN_ONCE(I915_READ(GEN6_UCGCTL3) & GEN6_OACSUNIT_CLOCK_GATE_DISABLE, "disabled OA unit level clock gating will result in incorrect per-context OA counters"); - /* XXX: On Haswell, when threshold disable mode is desired, -* instead of setting the threshold enable to '0', we need to -* program it to '1' and set OASTARTTRIG1 bits 15:0 to 0 -* (threshold value of 0) -*/ - I915_WRITE(OASTARTTRIG6, (OASTARTTRIG6_B4_TO_B7_THRESHOLD
[Intel-gfx] [RFC PATCH 01/11] perf: export perf_event_overflow
To support pmu drivers in loadable modules, such as the i915 driver Signed-off-by: Robert Bragg --- kernel/events/core.c | 1 + 1 file changed, 1 insertion(+) diff --git a/kernel/events/core.c b/kernel/events/core.c index 2fabc06..38c240c 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -5851,6 +5851,7 @@ int perf_event_overflow(struct perf_event *event, { return __perf_event_overflow(event, 1, data, regs); } +EXPORT_SYMBOL_GPL(perf_event_overflow); /* * Generic software event infrastructure -- 2.3.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC PATCH 11/11] WIP: drm/i915: constrain unit gating while using OA
We are still investigating the detailed requirements here, but there are some constraints we need to apply on unit level clock gating for reliable metrics (in particular for a reliable sampling period). Signed-off-by: Robert Bragg --- drivers/gpu/drm/i915/i915_oa_perf.c | 70 +++-- drivers/gpu/drm/i915/i915_reg.h | 3 ++ 2 files changed, 63 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_oa_perf.c b/drivers/gpu/drm/i915/i915_oa_perf.c index d0dad5d..2a4121b 100644 --- a/drivers/gpu/drm/i915/i915_oa_perf.c +++ b/drivers/gpu/drm/i915/i915_oa_perf.c @@ -257,20 +257,46 @@ oa_buffer_destroy(struct drm_i915_private *i915) static void i915_oa_event_destroy(struct perf_event *event) { - struct drm_i915_private *i915 = - container_of(event->pmu, typeof(*i915), oa_pmu.pmu); + struct drm_i915_private *dev_priv = + container_of(event->pmu, typeof(*dev_priv), oa_pmu.pmu); WARN_ON(event->parent); - oa_buffer_destroy(i915); + I915_WRITE(GEN6_UCGCTL1, (I915_READ(GEN6_UCGCTL1) & + ~GEN6_RCZUNIT_CLOCK_GATE_DISABLE)); + //I915_WRITE(GEN6_UCGCTL3, (I915_READ(GEN6_UCGCTL3) & + //~GEN6_OACSUNIT_CLOCK_GATE_DISABLE)); + I915_WRITE(GEN7_MISCCPCTL, (I915_READ(GEN7_MISCCPCTL) | + GEN7_DOP_CLOCK_GATE_ENABLE)); + + I915_WRITE(GEN7_ROW_CHICKEN2, + _MASKED_BIT_DISABLE(DOP_CLOCK_GATING_DISABLE)); + + //if (IS_HSW_GT2(dev_priv->dev)) { + if (1) { + I915_WRITE(HSW_ROW_CHICKEN2_GT2, + _MASKED_BIT_DISABLE(DOP_CLOCK_GATING_DISABLE)); + } + + if (IS_HSW_GT3(dev_priv->dev)) { + I915_WRITE(HSW_ROW_CHICKEN2_GT3_0, + _MASKED_BIT_DISABLE(DOP_CLOCK_GATING_DISABLE)); + I915_WRITE(HSW_ROW_CHICKEN2_GT3_1, + _MASKED_BIT_DISABLE(DOP_CLOCK_GATING_DISABLE)); + } - i915->oa_pmu.specific_ctx = NULL; + I915_WRITE(GDT_CHICKEN_BITS, (I915_READ(GDT_CHICKEN_BITS) & + ~GT_NOA_ENABLE)); + + oa_buffer_destroy(dev_priv); + + dev_priv->oa_pmu.specific_ctx = NULL; - BUG_ON(i915->oa_pmu.exclusive_event != event); - i915->oa_pmu.exclusive_event = NULL; + BUG_ON(dev_priv->oa_pmu.exclusive_event != event); + dev_priv->oa_pmu.exclusive_event = NULL; - intel_uncore_forcewake_put(i915, FORCEWAKE_ALL); - intel_runtime_pm_put(i915); + intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL); + intel_runtime_pm_put(dev_priv); } static void *vmap_oa_buffer(struct drm_i915_gem_object *obj) @@ -581,6 +607,32 @@ static int i915_oa_event_init(struct perf_event *event) BUG_ON(dev_priv->oa_pmu.exclusive_event); dev_priv->oa_pmu.exclusive_event = event; + + I915_WRITE(GDT_CHICKEN_BITS, GT_NOA_ENABLE); + + I915_WRITE(GEN6_UCGCTL1, (I915_READ(GEN6_UCGCTL1) | + GEN6_RCZUNIT_CLOCK_GATE_DISABLE)); + //I915_WRITE(GEN6_UCGCTL3, (I915_READ(GEN6_UCGCTL3) | + //GEN6_OACSUNIT_CLOCK_GATE_DISABLE)); + I915_WRITE(GEN7_MISCCPCTL, (I915_READ(GEN7_MISCCPCTL) & + ~GEN7_DOP_CLOCK_GATE_ENABLE)); + + I915_WRITE(GEN7_ROW_CHICKEN2, + _MASKED_BIT_ENABLE(DOP_CLOCK_GATING_DISABLE)); + + //if (IS_HSW_GT2(dev_priv->dev)) { + if (1) { + I915_WRITE(HSW_ROW_CHICKEN2_GT2, + _MASKED_BIT_ENABLE(DOP_CLOCK_GATING_DISABLE)); + } + + if (IS_HSW_GT3(dev_priv->dev)) { + I915_WRITE(HSW_ROW_CHICKEN2_GT3_0, + _MASKED_BIT_ENABLE(DOP_CLOCK_GATING_DISABLE)); + I915_WRITE(HSW_ROW_CHICKEN2_GT3_1, + _MASKED_BIT_ENABLE(DOP_CLOCK_GATING_DISABLE)); + } + event->destroy = i915_oa_event_destroy; /* PRM - observability performance counters: @@ -678,8 +730,6 @@ static void i915_oa_event_start(struct perf_event *event, int flags) WARN_ONCE(I915_READ(GEN6_UCGCTL3) & GEN6_OACSUNIT_CLOCK_GATE_DISABLE, "disabled OA unit level clock gating will result in incorrect per-context OA counters"); - I915_WRITE(GDT_CHICKEN_BITS, GT_NOA_ENABLE); - if (dev_priv->oa_pmu.metrics_set == I915_OA_METRICS_SET_3D) { config_oa_regs(dev_priv, hsw_profile_3d_mux_config, ARRAY_SIZE(hsw_profile_3d_mux_config)); diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index d94932a..518b34c 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -7036,6 +7036,9 @@ enum skl_disp_power_wells { #define GEN7_ROW_CHICKEN2 0xe4f4 #define GEN7_ROW_CHICKEN2_GT2 0xf4f4 +#define HS
[Intel-gfx] [RFC PATCH 06/11] drm/i915: rename OACONTROL GEN7_OACONTROL
OACONTROL changes quite a bit for gen8, with some bits split out into a per-context OACTXCONTROL register Signed-off-by: Robert Bragg --- drivers/gpu/drm/i915/i915_cmd_parser.c | 4 ++-- drivers/gpu/drm/i915/i915_reg.h| 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c index 9605ff8..f7ef20c 100644 --- a/drivers/gpu/drm/i915/i915_cmd_parser.c +++ b/drivers/gpu/drm/i915/i915_cmd_parser.c @@ -417,7 +417,7 @@ static const u32 gen7_render_regs[] = { REG64(CL_PRIMITIVES_COUNT), REG64(PS_INVOCATION_COUNT), REG64(PS_DEPTH_COUNT), - OACONTROL, /* Only allowed for LRI and SRM. See below. */ + GEN7_OACONTROL, /* Only allowed for LRI and SRM. See below. */ REG64(MI_PREDICATE_SRC0), REG64(MI_PREDICATE_SRC1), GEN7_3DPRIM_END_OFFSET, @@ -961,7 +961,7 @@ static bool check_cmd(const struct intel_engine_cs *ring, * that will be written to the register. Hence, limit * OACONTROL writes to only MI_LOAD_REGISTER_IMM commands. */ - if (reg_addr == OACONTROL) { + if (reg_addr == GEN7_OACONTROL) { if (desc->cmd.value == MI_LOAD_REGISTER_MEM) { DRM_DEBUG_DRIVER("CMD: Rejected LRM to OACONTROL\n"); return false; diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index d47afbc..2fa1669 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -515,7 +515,7 @@ #define GEN7_3DPRIM_START_INSTANCE 0x243C #define GEN7_3DPRIM_BASE_VERTEX 0x2440 -#define OACONTROL 0x2360 +#define GEN7_OACONTROL 0x2360 #define _GEN7_PIPEA_DE_LOAD_SL 0x70068 #define _GEN7_PIPEB_DE_LOAD_SL 0x71068 -- 2.3.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC PATCH 09/11] drm/i915: Add dev.i915.oa_event_paranoid sysctl option
Consistent with the kernel.perf_event_paranoid sysctl option that can allow non-root users to access system wide cpu metrics, this can optionally allow non-root users to access system wide OA counter metrics from Gen graphics hardware. Signed-off-by: Robert Bragg --- drivers/gpu/drm/i915/i915_drv.h | 2 ++ drivers/gpu/drm/i915/i915_oa_perf.c | 40 - 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 1e65dc2..7bc7243 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1831,6 +1831,8 @@ struct drm_i915_private { struct hrtimer timer; struct pt_regs dummy_regs; + struct ctl_table_header *sysctl_header; + struct perf_event *exclusive_event; struct intel_context *specific_ctx; bool event_active; diff --git a/drivers/gpu/drm/i915/i915_oa_perf.c b/drivers/gpu/drm/i915/i915_oa_perf.c index d0e144c..c3e5059 100644 --- a/drivers/gpu/drm/i915/i915_oa_perf.c +++ b/drivers/gpu/drm/i915/i915_oa_perf.c @@ -11,6 +11,8 @@ #define FREQUENCY 200 #define PERIOD max_t(u64, 1, NSEC_PER_SEC / FREQUENCY) +static u32 i915_oa_event_paranoid = true; + static int hsw_perf_format_sizes[] = { 64, /* A13_HSW */ 128, /* A29_HSW */ @@ -548,7 +550,8 @@ static int i915_oa_event_init(struct perf_event *event) } } - if (!dev_priv->oa_pmu.specific_ctx && !capable(CAP_SYS_ADMIN)) + if (!dev_priv->oa_pmu.specific_ctx && + i915_oa_event_paranoid && !capable(CAP_SYS_ADMIN)) return -EACCES; mutex_lock(&dev_priv->dev->struct_mutex); @@ -795,6 +798,37 @@ void i915_oa_context_unpin_notify(struct drm_i915_private *dev_priv, spin_unlock_irqrestore(&dev_priv->oa_pmu.lock, flags); } +static struct ctl_table oa_table[] = { + { +.procname = "oa_event_paranoid", +.data = &i915_oa_event_paranoid, +.maxlen = sizeof(i915_oa_event_paranoid), +.mode = 0644, +.proc_handler = proc_dointvec, +}, + {} +}; + +static struct ctl_table i915_root[] = { + { +.procname = "i915", +.maxlen = 0, +.mode = 0555, +.child = oa_table, +}, + {} +}; + +static struct ctl_table dev_root[] = { + { +.procname = "dev", +.maxlen = 0, +.mode = 0555, +.child = i915_root, +}, + {} +}; + void i915_oa_pmu_register(struct drm_device *dev) { struct drm_i915_private *i915 = to_i915(dev); @@ -802,6 +836,8 @@ void i915_oa_pmu_register(struct drm_device *dev) if (!IS_HASWELL(dev)) return; + i915->oa_pmu.sysctl_header = register_sysctl_table(dev_root); + /* We need to be careful about forwarding cpu metrics to * userspace considering that PERF_PMU_CAP_IS_DEVICE bypasses * the events/core security check that stops an unprivileged @@ -841,6 +877,8 @@ void i915_oa_pmu_unregister(struct drm_device *dev) if (i915->oa_pmu.pmu.event_init == NULL) return; + unregister_sysctl_table(i915->oa_pmu.sysctl_header); + perf_pmu_unregister(&i915->oa_pmu.pmu); i915->oa_pmu.pmu.event_init = NULL; } -- 2.3.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC PATCH 00/11] drm/i915: Expose OA metrics via perf PMU
This is an updated series adding support for an "i915_oa" perf PMU for configuring the Intel Gen graphics Observability unit (Haswell only to start with) and forwarding periodic counter reports as perf samples. Compared to the series I sent out last year: The driver is now hooked into context switching so we no longer require a context to be pinned for the full lifetime of a perf event fd (when profiling a specific context). Not visible in the series, but I can say we've also gained some experience from looking at enabling Broadwell within the same architecture. There are some fiddly challenges ahead with enabling Broadwell but I do feel comfortable that it can be supported in the same kind of way via perf. I haven't updated my Broadwell branches for a little while now but if anyone is interested I can share this code as a point of reference if that's helpful. As I've had interest from folks looking at developing tools based on this interface to not require root, but we are following the precedent of not exposing system wide metrics to unprivileged processes, I've added a sysctl directly comparable to kernel.perf_event_paranoid (dev.i915.oa_event_paranoid) that lets users optionally allow unprivileged access to system wide gpu metrics. This series is able to expose more than just the A (aggregating) counters and demonstrates selecting a more counters that are useful when benchmarking 3D render workloads. The expectation is to add further configurations later geared towards media or gpgpu workloads for example. I've changed the uapi for configuring the i915_oa specific attributes when calling perf_event_open(2) whereby instead of cramming lots of bitfields into the perf_event_attr config members, I'm now daisy-chaining a drm_i915_oa_event_attr_t structure off of a single config member that's extensible and validated in the same way as the perf_event_attr struct. I've found this much nicer to work with while being neatly extensible too. I've made a few more (small) changes to core perf infrastructure: I've added a PERF_EVENT_IOC_FLUSH ioctl that can be used to explicitly ask the driver to flush buffered samples. In our case this makes sure to forward all reports currently in the gpu mapped, circular, oabuffer as perf samples. This issue was discussed a bit on LKML last year and previously I was overloading our PMU's read() hook but decided that the cleaner approach would be to add a dedicated ioctl instead. To allow device-driver PMUs to define their own types for records written to the perf circular buffer I've introduced a PERF_RECORD_DEVICE type whereby drivers can then document their own header defining a driver specific scheme for sub-types. This is then used in the i915_oa driver for reporting hw status conditions such as OABUFFER overruns or report lost conditions from the hw. For examples of using the i915_oa driver I have a branch of Mesa that enables support for the INTEL_performance_query extension based on this: https://github.com/rib/drm wip/rib/oa-hsw-4.0.0 https://github.com/rib/mesa wip/rib/oa-hsw-4.0.0 For reference I sent out a corresponding series for the Mesa work for review yesterday: http://lists.freedesktop.org/archives/mesa-dev/2015-May/083519.html I also have a minimal gputop tool that can both test Mesa's INTEL_performance_query implementation to view per-context metrics or it can view system wide gpu metrics collected directly from perf (gputop/gputop-perf.c would be the main code of interest): https://github.com/rib/gputop If it's convenient for testing, my kernel patches can also be fetched from here: https://github.com/rib/linux wip/rib/oa-hsw-4.0.0 One specific patch comment: [RFC PATCH 11/11] WIP: drm/i915: constrain unit gating while using OA I didn't want to hold up getting feedback due to this issue that I'm currently investigating (since the effect on the driver should be trivial) but I've included a work-in-progress patch since it does address a known problem with collecting reliable periodic metrics. Besides the last patch, I feel like this series is in pretty good shape now and by testing it with Mesa and several profiling tools as well as starting the work to enable Broadwell I feel quite happy with our approach of leveraging perf infrastructure. I'd really appreciate any feedback on the core perf changes I've made, as well as general feedback on the PMU driver itself. Since it's been quite a long time since I last sent out patches for this work; in case it's helpful to refer back to some of the discussion last year: https://lkml.org/lkml/2014/10/22/462 For anyone interested to know more details about this hardware, this PRM is a good starting point: https://01.org/sites/default/files/documentation/ observability_performance_counters_haswell.pdf Kind regards, - Robert Robert Bragg (11): perf: export perf_event_overflow perf: Add PERF_PMU_CAP_IS_DEVICE flag perf: Add PERF_EVENT_IOC_FLUSH ioctl perf: Add a PERF_RECORD_DEVICE event
[Intel-gfx] [RFC PATCH 04/11] perf: Add a PERF_RECORD_DEVICE event type
To allow for more extensible, device specific, perf record types this adds a generic PERF_RECORD_DEVICE type that can be used by device drivers. Driver developers can then document some driver-specific header to further detail such a record's type. Signed-off-by: Robert Bragg --- include/uapi/linux/perf_event.h | 13 + 1 file changed, 13 insertions(+) diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h index a25967b..688e192 100644 --- a/include/uapi/linux/perf_event.h +++ b/include/uapi/linux/perf_event.h @@ -726,6 +726,19 @@ enum perf_event_type { */ PERF_RECORD_MMAP2 = 10, + /* +* The DEVICE record implies some device driver specific record that +* will have some further mechanism for describing the contents of +* the record (i.e. some driver specific event header). +* +* struct { +* struct perf_event_headerheader; +* +* struct DEVICE_EVENT_HEADER device_header; +* }; +*/ + PERF_RECORD_DEVICE = 11, + PERF_RECORD_MAX,/* non-ABI */ }; -- 2.3.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC PATCH 03/11] perf: Add PERF_EVENT_IOC_FLUSH ioctl
To allow for pmus that may have internal buffering (e.g. the hardware itself writes out data to its own circular buffer which is only periodically forwarded to userspace via perf) this ioctl enables userspace to explicitly ensure it has received all samples before a point in time. Signed-off-by: Robert Bragg --- include/linux/perf_event.h | 7 +++ include/uapi/linux/perf_event.h | 1 + kernel/events/core.c| 5 + 3 files changed, 13 insertions(+) diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h index 1af35b4..69a0cb9 100644 --- a/include/linux/perf_event.h +++ b/include/linux/perf_event.h @@ -266,6 +266,13 @@ struct pmu { * flush branch stack on context-switches (needed in cpu-wide mode) */ void (*flush_branch_stack) (void); + + /* +* Flush buffered samples (E.g. for pmu hardware that writes samples to +* some intermediate buffer) userspace may need to explicitly ensure +* such samples have been forwarded to perf. +*/ + void (*flush) (struct perf_event *event); /*optional */ }; /** diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h index 9b79abb..a25967b 100644 --- a/include/uapi/linux/perf_event.h +++ b/include/uapi/linux/perf_event.h @@ -360,6 +360,7 @@ struct perf_event_attr { #define PERF_EVENT_IOC_SET_OUTPUT _IO ('$', 5) #define PERF_EVENT_IOC_SET_FILTER _IOW('$', 6, char *) #define PERF_EVENT_IOC_ID _IOR('$', 7, __u64 *) +#define PERF_EVENT_IOC_FLUSH _IO ('$', 8) enum perf_event_ioc_flags { PERF_IOC_FLAG_GROUP = 1U << 0, diff --git a/kernel/events/core.c b/kernel/events/core.c index 7218b01..340deaa 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -3981,6 +3981,11 @@ static long _perf_ioctl(struct perf_event *event, unsigned int cmd, unsigned lon case PERF_EVENT_IOC_SET_FILTER: return perf_event_set_filter(event, (void __user *)arg); + case PERF_EVENT_IOC_FLUSH: + if (event->pmu->flush) + event->pmu->flush(event); + return 0; + default: return -ENOTTY; } -- 2.3.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fix screen flickering on X
On Thu, May 07, 2015 at 04:41:48PM +0300, Jani Nikula wrote: > On Thu, 07 May 2015, Matt Roper wrote: > > On Thu, May 07, 2015 at 12:12:18PM +0300, Jani Nikula wrote: > >> On Thu, 23 Apr 2015, Chris Wilson wrote: > >> > [cc'ing the authors] > >> > >> This has been posted earlier [1] and it has review to be addressed [2]. > >> > >> BR, > >> Jani. > > > > I agree with Ander's response in [2]...we can't call > > intel_update_watermarks() in the commit function because we're under > > vblank evasion. We should already be flagging the transaction as > > needing a watermark update in intel_check_cursor_plane(), and that flag > > will be acted upon immediately after the commit functions are done > > running, once we've re-enabled interrupts. > > > > Note that our current codebase looks a bit different since we've dropped > > intel_crtc->cursor_{width,height}. So the relevant check in > > intel_check_cursor_plane() now looks like: > > > > if (plane->state->crtc_w != state->base.crtc_w) > > intel_crtc->atomic.update_wm = true; > > > > Is there a bugzilla open on this issue with more details? > > Not that I know of. Ismael? > Probably: https://bugs.freedesktop.org/show_bug.cgi?id=88944 https://bugzilla.redhat.com/show_bug.cgi?id=1199890 are related. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC PATCH 03/11] perf: Add PERF_EVENT_IOC_FLUSH ioctl
On Thu, May 07, 2015 at 03:15:46PM +0100, Robert Bragg wrote: > To allow for pmus that may have internal buffering (e.g. the hardware > itself writes out data to its own circular buffer which is only > periodically forwarded to userspace via perf) this ioctl enables > userspace to explicitly ensure it has received all samples before a > point in time. > > Signed-off-by: Robert Bragg > --- > include/linux/perf_event.h | 7 +++ > include/uapi/linux/perf_event.h | 1 + > kernel/events/core.c| 5 + > 3 files changed, 13 insertions(+) > > diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h > index 1af35b4..69a0cb9 100644 > --- a/include/linux/perf_event.h > +++ b/include/linux/perf_event.h > @@ -266,6 +266,13 @@ struct pmu { >* flush branch stack on context-switches (needed in cpu-wide mode) >*/ > void (*flush_branch_stack) (void); > + > + /* > + * Flush buffered samples (E.g. for pmu hardware that writes samples to > + * some intermediate buffer) userspace may need to explicitly ensure > + * such samples have been forwarded to perf. > + */ > + void (*flush) (struct perf_event *event); /*optional > */ void return? -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/skl: Fix WaDisableChickenBitTSGBarrierAckForFFSliceCS
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 6332 -Summary- Platform Delta drm-intel-nightly Series Applied PNV 276/276 276/276 ILK 302/302 302/302 SNB 316/316 316/316 IVB 342/342 342/342 BYT 286/286 286/286 BDW 321/321 321/321 -Detailed- Platform Testdrm-intel-nightly Series Applied Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC PATCH 07/11] drm/i915: Expose PMU for Observation Architecture
On Thu, May 07, 2015 at 03:15:50PM +0100, Robert Bragg wrote: > +static int init_oa_buffer(struct perf_event *event) > +{ > + struct drm_i915_private *dev_priv = > + container_of(event->pmu, typeof(*dev_priv), oa_pmu.pmu); > + struct drm_i915_gem_object *bo; > + int ret; > + > + BUG_ON(!IS_HASWELL(dev_priv->dev)); > + BUG_ON(!mutex_is_locked(&dev_priv->dev->struct_mutex)); > + BUG_ON(dev_priv->oa_pmu.oa_buffer.obj); > + > + spin_lock_init(&dev_priv->oa_pmu.oa_buffer.flush_lock); > + > + /* NB: We over allocate the OA buffer due to the way raw sample data > + * gets copied from the gpu mapped circular buffer into the perf > + * circular buffer so that only one copy is required. > + * > + * For each perf sample (raw->size + 4) needs to be 8 byte aligned, > + * where the 4 corresponds to the 32bit raw->size member that's > + * added to the sample header that userspace sees. > + * > + * Due to the + 4 for the size member: when we copy a report to the > + * userspace facing perf buffer we always copy an additional 4 bytes > + * from the subsequent report to make up for the miss alignment, but > + * when a report is at the end of the gpu mapped buffer we need to > + * read 4 bytes past the end of the buffer. > + */ > + bo = i915_gem_alloc_object(dev_priv->dev, OA_BUFFER_SIZE + PAGE_SIZE); > + if (bo == NULL) { > + DRM_ERROR("Failed to allocate OA buffer\n"); > + ret = -ENOMEM; > + goto err; > + } > + dev_priv->oa_pmu.oa_buffer.obj = bo; > + > + ret = i915_gem_object_set_cache_level(bo, I915_CACHE_LLC); > + if (ret) > + goto err_unref; > + > + /* PreHSW required 512K alignment, HSW requires 16M */ > + ret = i915_gem_obj_ggtt_pin(bo, SZ_16M, 0); > + if (ret) > + goto err_unref; > + > + dev_priv->oa_pmu.oa_buffer.gtt_offset = i915_gem_obj_ggtt_offset(bo); > + dev_priv->oa_pmu.oa_buffer.addr = vmap_oa_buffer(bo); You can look forward to both i915_gem_object_create_internal() and i915_gem_object_pin_vmap() > + > + /* Pre-DevBDW: OABUFFER must be set with counters off, > + * before OASTATUS1, but after OASTATUS2 */ > + I915_WRITE(GEN7_OASTATUS2, dev_priv->oa_pmu.oa_buffer.gtt_offset | > +GEN7_OASTATUS2_GGTT); /* head */ > + I915_WRITE(GEN7_OABUFFER, dev_priv->oa_pmu.oa_buffer.gtt_offset); > + I915_WRITE(GEN7_OASTATUS1, dev_priv->oa_pmu.oa_buffer.gtt_offset | > +GEN7_OASTATUS1_OABUFFER_SIZE_16M); /* tail */ > + > + DRM_DEBUG_DRIVER("OA Buffer initialized, gtt offset = 0x%x, vaddr = %p", > + dev_priv->oa_pmu.oa_buffer.gtt_offset, > + dev_priv->oa_pmu.oa_buffer.addr); > + > + return 0; > + > +err_unref: > + drm_gem_object_unreference_unlocked(&bo->base); But what I really what to say was: mutex deadlock^^^ -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC PATCH 07/11] drm/i915: Expose PMU for Observation Architecture
On Thu, May 07, 2015 at 03:15:50PM +0100, Robert Bragg wrote: > + /* We bypass the default perf core perf_paranoid_cpu() || > + * CAP_SYS_ADMIN check by using the PERF_PMU_CAP_IS_DEVICE > + * flag and instead authenticate based on whether the current > + * pid owns the specified context, or require CAP_SYS_ADMIN > + * when collecting cross-context metrics. > + */ > + dev_priv->oa_pmu.specific_ctx = NULL; > + if (oa_attr.single_context) { > + u32 ctx_id = oa_attr.ctx_id; > + unsigned int drm_fd = oa_attr.drm_fd; > + struct fd fd = fdget(drm_fd); > + > + if (fd.file) { Specify a ctx and not providing the right fd should be its own error, either EBADF or EINVAL. > + dev_priv->oa_pmu.specific_ctx = > + lookup_context(dev_priv, fd.file, ctx_id); > + } Missing fdput > + } > + > + if (!dev_priv->oa_pmu.specific_ctx && !capable(CAP_SYS_ADMIN)) > + return -EACCES; > + > + mutex_lock(&dev_priv->dev->struct_mutex); i915_mutex_interruptible, probably best to couple into the GPU error handling here as well especially as init_oa_buffer() will go onto touch GPU internals. > + ret = init_oa_buffer(event); > + mutex_unlock(&dev_priv->dev->struct_mutex); > + > + if (ret) > + return ret; > + > + BUG_ON(dev_priv->oa_pmu.exclusive_event); > + dev_priv->oa_pmu.exclusive_event = event; > + > + event->destroy = i915_oa_event_destroy; > + > + /* PRM - observability performance counters: > + * > + * OACONTROL, performance counter enable, note: > + * > + * "When this bit is set, in order to have coherent counts, > + * RC6 power state and trunk clock gating must be disabled. > + * This can be achieved by programming MMIO registers as > + * 0xA094=0 and 0xA090[31]=1" > + * > + * In our case we are expected that taking pm + FORCEWAKE > + * references will effectively disable RC6 and trunk clock > + * gating. > + */ > + intel_runtime_pm_get(dev_priv); > + intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL); That is a nuisance. Aside: Why isn't OA inside the powerctx? Is a subset valid with forcewake? It does perturb the system greatly to disable rc6, so I wonder if it could be made optional? > + > + return 0; > +} > + > +static void update_oacontrol(struct drm_i915_private *dev_priv) > +{ > + BUG_ON(!spin_is_locked(&dev_priv->oa_pmu.lock)); > + > + if (dev_priv->oa_pmu.event_active) { > + unsigned long ctx_id = 0; > + bool pinning_ok = false; > + > + if (dev_priv->oa_pmu.specific_ctx) { > + struct intel_context *ctx = > + dev_priv->oa_pmu.specific_ctx; > + struct drm_i915_gem_object *obj = > + ctx->legacy_hw_ctx.rcs_state; If only there was ctx->legacy_hw_ctx.rcs_vma... > + > + if (i915_gem_obj_is_pinned(obj)) { > + ctx_id = i915_gem_obj_ggtt_offset(obj); > + pinning_ok = true; > + } > + } > + > + if ((ctx_id == 0 || pinning_ok)) { > + bool periodic = dev_priv->oa_pmu.periodic; > + u32 period_exponent = dev_priv->oa_pmu.period_exponent; > + u32 report_format = dev_priv->oa_pmu.oa_buffer.format; > + > + I915_WRITE(GEN7_OACONTROL, > +(ctx_id & GEN7_OACONTROL_CTX_MASK) | > +(period_exponent << > + GEN7_OACONTROL_TIMER_PERIOD_SHIFT) | > +(periodic ? > + GEN7_OACONTROL_TIMER_ENABLE : 0) | > +(report_format << > + GEN7_OACONTROL_FORMAT_SHIFT) | > +(ctx_id ? > + GEN7_OACONTROL_PER_CTX_ENABLE : 0) | > +GEN7_OACONTROL_ENABLE); I notice you don't use any write barriers... -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/9] drm/i915/bxt Enable existing workarounds for Broxton
On to, 2015-05-07 at 14:15 +0100, Nick Hoath wrote: > The following patch series either enables a workaround for Broxton, marks > it as applicable to Broxton, or moves it in to the SoC specific > initialisation. > > v2: Split out the changes as one patch per workaround (Requested by Imre) > Removed unused additional register. > Cleaned up whitespace. (Imre) > Cleaned up revision ID usage (Imre) > > Nick Hoath (9): > drm/i915/bxt: Mark WaDisablePartialInstShootdown as for Broxton also. > drm/i915/bxt: Mark workaround as for Skylake & Broxton > drm/i915/bxt: Enable WaDisableDgMirrorFixInHalfSliceChicken5 for > Broxton > drm/i915/bxt: Enable > WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken for Broxton > drm/i915/bxt: Enable WaEnableYV12BugFixInHalfSliceChicken7 for Broxton > drm/i915/bxt: Move WaForceEnableNonCoherent to Skylake only > drm/i915/bxt: Mark Wa4x4STCOptimizationDisable as for Broxton also. > drm/i915/bxt: Mark WaDisablePartialResolveInVc as for Broxton also. > drm/i915/bxt: Mark WaCcsTlbPrefetchDisable as for Broxton also. > > drivers/gpu/drm/i915/intel_ringbuffer.c | 49 > + > 1 file changed, 26 insertions(+), 23 deletions(-) Looks ok, on the patchset: Reviewed-by: Imre Deak ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: restore ggtt double-bind avoidance
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 6333 -Summary- Platform Delta drm-intel-nightly Series Applied PNV 276/276 276/276 ILK 302/302 302/302 SNB 316/316 316/316 IVB 342/342 342/342 BYT 286/286 286/286 BDW 321/321 321/321 -Detailed- Platform Testdrm-intel-nightly Series Applied Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/bxt: edp1.4 Intermediate Freq support
On Thu, May 07, 2015 at 04:36:48PM +0530, Sonika Jindal wrote: > BXT supports following intermediate link rates for edp: > 2.16GHz, 2.43GHz, 3.24GHz, 4.32GHz. > Adding support for programming the intermediate rates. > > v2: Adding clock in bxt_clk_div struct and then look for the entry with > required rate (Ville) > > Signed-off-by: Sonika Jindal > --- > drivers/gpu/drm/i915/intel_ddi.c | 45 > +- > drivers/gpu/drm/i915/intel_dp.c |7 +- > 2 files changed, 31 insertions(+), 21 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c > b/drivers/gpu/drm/i915/intel_ddi.c > index 9c1e74a..7b9d226 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -1327,6 +1327,7 @@ skl_ddi_pll_select(struct intel_crtc *intel_crtc, > > /* bxt clock parameters */ > struct bxt_clk_div { > + int clock; > uint32_t p1; > uint32_t p2; > uint32_t m2_int; > @@ -1342,13 +1343,13 @@ struct bxt_clk_div { > > /* pre-calculated values for DP linkrates */ > static struct bxt_clk_div bxt_dp_clk_val[7] = { > - /* 162 */ {4, 2, 32, 1677722, 1, 1, 5, 11, 2, 9, 0xd}, > - /* 270 */ {4, 1, 27, 0, 0, 1, 3, 8, 1, 9, 0xd}, > - /* 540 */ {2, 1, 27, 0, 0, 1, 3, 8, 1, 9, 0x18}, > - /* 216 */ {3, 2, 32, 1677722, 1, 1, 5, 11, 2, 9, 0xd}, > - /* 243 */ {4, 1, 24, 1258291, 1, 1, 5, 11, 2, 9, 0xd}, > - /* 324 */ {4, 1, 32, 1677722, 1, 1, 5, 11, 2, 9, 0xd}, > - /* 432 */ {3, 1, 32, 1677722, 1, 1, 5, 11, 2, 9, 0x18} > + {162000, 4, 2, 32, 1677722, 1, 1, 5, 11, 2, 9, 0xd}, > + {27, 4, 1, 27, 0, 0, 1, 3, 8, 1, 9, 0xd}, > + {54, 2, 1, 27, 0, 0, 1, 3, 8, 1, 9, 0x18}, > + {216000, 3, 2, 32, 1677722, 1, 1, 5, 11, 2, 9, 0xd}, > + {243000, 4, 1, 24, 1258291, 1, 1, 5, 11, 2, 9, 0xd}, > + {324000, 4, 1, 32, 1677722, 1, 1, 5, 11, 2, 9, 0xd}, > + {432000, 3, 1, 32, 1677722, 1, 1, 5, 11, 2, 9, 0x18} > }; > > static bool > @@ -1401,20 +1402,24 @@ bxt_ddi_pll_select(struct intel_crtc *intel_crtc, > intel_encoder->type == INTEL_OUTPUT_EDP) { > struct drm_encoder *encoder = &intel_encoder->base; > struct intel_dp *intel_dp = enc_to_intel_dp(encoder); > + int link_rate; > + int i; > > - switch (intel_dp->link_bw) { > - case DP_LINK_BW_1_62: > - clk_div = bxt_dp_clk_val[0]; > - break; > - case DP_LINK_BW_2_7: > - clk_div = bxt_dp_clk_val[1]; > - break; > - case DP_LINK_BW_5_4: > - clk_div = bxt_dp_clk_val[2]; > - break; > - default: > - clk_div = bxt_dp_clk_val[0]; > - DRM_ERROR("Unknown link rate\n"); > + /* > + * If edp1.4 intermediate frequency support is present, we set > + * link_bw to 0 and a valid rate index in rate_select. > + */ > + if (intel_dp->link_bw) > + link_rate = clock; > + else > + link_rate = intel_dp->sink_rates[intel_dp->rate_select]; 'clock' should be correct in either case. > + > + clk_div = bxt_dp_clk_val[0]; > + for (i = 0; i < 7; ++i) { > + if (bxt_dp_clk_val[i].clock == link_rate) { > + clk_div = bxt_dp_clk_val[i]; > + break; > + } > } > } > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index c9d50d1..e6ee7c6 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -85,6 +85,8 @@ static const struct dp_link_dpll chv_dpll[] = { > { .p1 = 2, .p2 = 1, .n = 1, .m1 = 2, .m2 = 0x6c0 } } > }; > > +static const int bxt_rates[] = { 162000, 216000, 243000, 27, > + 324000, 432000, 54 }; > static const int skl_rates[] = { 162000, 216000, 27, > 324000, 432000, 54 }; > static const int chv_rates[] = { 162000, 202500, 21, 216000, > @@ -1161,7 +1163,10 @@ intel_dp_sink_rates(struct intel_dp *intel_dp, const > int **sink_rates) > static int > intel_dp_source_rates(struct drm_device *dev, const int **source_rates) > { > - if (IS_SKYLAKE(dev)) { > + if (IS_BROXTON(dev)) { > + *source_rates = bxt_rates; > + return ARRAY_SIZE(bxt_rates); > + } else if (IS_SKYLAKE(dev)) { > *source_rates = skl_rates; > return ARRAY_SIZE(skl_rates); > } else if (IS_CHERRYVIEW(dev)) { > -- > 1.7.10.4 -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/i
Re: [Intel-gfx] [RFC 0/2] strace/drm: Add i915 ioctls to strace
On 05/06/2015 07:48 AM, Patrik Jakobsson wrote: > This patch set aims to make strace more useful when tracing i915 ioctls. > The ioctl type is first checked for being drm and then the driver > backing the opened device is identified by looking at sysfs. Other > drivers than i915 can easily be added. > > Only a subset of the i915 ioctls are included. I will extend this patch > set if the approach looks ok. The generic drm ioctls are also missing. > > Give it a spin with: > strace -e trace=ioctl -p `pidof X` > > Patrik Jakobsson (2): > strace/drm: Print extended info for drm and i915 ioctls > strace/drm: Print args for most common i915 ioctls > > Makefile.am| 2 + > defs.h | 2 + > drm.c | 104 + > drm_i915.c | 278 > + > ioctl.c| 5 + > xlat/drm_i915_getparams.in | 28 + > xlat/drm_i915_ioctls.in| 51 + > xlat/drm_i915_setparams.in | 4 + > 8 files changed, 474 insertions(+) > create mode 100644 drm.c > create mode 100644 drm_i915.c > create mode 100644 xlat/drm_i915_getparams.in > create mode 100644 xlat/drm_i915_ioctls.in > create mode 100644 xlat/drm_i915_setparams.in Yeah, this looks pretty cool to me. I'm not familiar with strace internals, but the split and extensible design looks reasonable; should make it easy to add other drivers and such in the future. Who on the strace side can pick this up? Thanks, Jesse ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 09/11] drm/i915: Update compute_baseline_bpp for NV12.
> -Original Message- > From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter > Sent: Wednesday, May 06, 2015 11:59 PM > To: Konduru, Chandra > Cc: intel-gfx@lists.freedesktop.org; Vetter, Daniel; Syrjala, Ville > Subject: Re: [Intel-gfx] [PATCH 09/11] drm/i915: Update compute_baseline_bpp > for NV12. > > On Thu, Apr 30, 2015 at 08:43:13PM -0700, Chandra Konduru wrote: > > This patch updates baseline bpp when primary plane is running with > > NV12. It is same as 8-bpc RGB formats because NV12 also has 8-bits per > > channel. > > > > Signed-off-by: Chandra Konduru > > This code was removed in > > commit d328c9d78d64ca11e744fe227096990430a88477 > Author: Daniel Vetter > Date: Fri Apr 10 16:22:37 2015 +0200 > > drm/i915: Select starting pipe bpp irrespective or the primary plane > > Please rebase onto latest -nightly before submitting to check for such things. Will do. This patch (09/11) can be ignored as we are no more setting baseline bpp based on pixel format. > > Thanks, Daniel > > --- > > drivers/gpu/drm/i915/intel_display.c |1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c > > b/drivers/gpu/drm/i915/intel_display.c > > index 6e693c4..e747766 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -10845,6 +10845,7 @@ compute_baseline_pipe_bpp(struct intel_crtc > *crtc, > > return -EINVAL; > > case DRM_FORMAT_XRGB: > > case DRM_FORMAT_ARGB: > > + case DRM_FORMAT_NV12: > > bpp = 8*3; > > break; > > case DRM_FORMAT_XRGB2101010: > > -- > > 1.7.9.5 > > > > ___ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/9] drm/i915/bxt Enable existing workarounds for Broxton
On Thu, May 07, 2015 at 06:17:03PM +0300, Imre Deak wrote: > On to, 2015-05-07 at 14:15 +0100, Nick Hoath wrote: > > The following patch series either enables a workaround for Broxton, marks > > it as applicable to Broxton, or moves it in to the SoC specific > > initialisation. > > > > v2: Split out the changes as one patch per workaround (Requested by Imre) > > Removed unused additional register. > > Cleaned up whitespace. (Imre) > > Cleaned up revision ID usage (Imre) > > > > Nick Hoath (9): > > drm/i915/bxt: Mark WaDisablePartialInstShootdown as for Broxton also. > > drm/i915/bxt: Mark workaround as for Skylake & Broxton > > drm/i915/bxt: Enable WaDisableDgMirrorFixInHalfSliceChicken5 for > > Broxton > > drm/i915/bxt: Enable > > WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken for Broxton > > drm/i915/bxt: Enable WaEnableYV12BugFixInHalfSliceChicken7 for Broxton > > drm/i915/bxt: Move WaForceEnableNonCoherent to Skylake only > > drm/i915/bxt: Mark Wa4x4STCOptimizationDisable as for Broxton also. > > drm/i915/bxt: Mark WaDisablePartialResolveInVc as for Broxton also. > > drm/i915/bxt: Mark WaCcsTlbPrefetchDisable as for Broxton also. > > > > drivers/gpu/drm/i915/intel_ringbuffer.c | 49 > > + > > 1 file changed, 26 insertions(+), 23 deletions(-) > > Looks ok, on the patchset: > Reviewed-by: Imre Deak Entire series merged, thanks. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 4/5] drm/i915: Add a partial GGTT view type
On Wed, May 06, 2015 at 02:40:48PM +0300, Joonas Lahtinen wrote: > On ke, 2015-05-06 at 12:20 +0200, Daniel Vetter wrote: > > On Thu, Apr 30, 2015 at 01:16:30PM +0100, Tvrtko Ursulin wrote: > > > On 04/30/2015 12:20 PM, Joonas Lahtinen wrote: > > > >@@ -495,7 +503,10 @@ i915_ggtt_view_equal(const struct i915_ggtt_view *a, > > > > if (WARN_ON(!a || !b)) > > > > return false; > > > > > > > >-return a->type == b->type; > > > >+if (a->type != b->type) > > > >+return false; > > > >+ > > > >+return !memcmp(&a->params, &b->params, sizeof(a->params)); > > > > > > Still don't like this, would this be so bad: > > > > > > if (a->type != PARTIAL) > > > return a->type == b->type; > > > else > > > return !memcmp(...) > > > > Why do we even need this? memcmp implies comparing just one part of the > > struct, doesn't it? Of course this means we need to clear it, but imo > > that's the rtdt anyway. > > I only noticed this comment now (just sent v3 of the series out), I'm > fine with leaving it like it was in the revision 1, which means that the > last line is return !memcmp(..), so if the patch is otherwise OK, shall > I make a one more revision, or will you merge manually? Already pulled in, please do a follow-up patch. Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 3/4] drm/i915: Add a partial GGTT view type
On Wed, May 06, 2015 at 01:24:18PM +0100, Tvrtko Ursulin wrote: > > > On 05/06/2015 12:35 PM, Joonas Lahtinen wrote: > > > >Partial view type allows manipulating parts of huge BOs through the GGTT, > >which was not previously possible due to constraint that whole object had > >to be mapped for any access to it through GGTT. > > > >v2: > >- Retain error value from sg_alloc_table (Tvrtko Ursulin) > >- Do not zero already zeroed variable (Tvrtko Ursulin) > >- Use more common variable types for page size/offset (Tvrtko Ursulin) > >v3: > >- Only compare additional view parameters when need to (Tvrtko Ursulin) > >v4: > >- Do zero out the variable that needs to be (bug introduced in v2). > > Reviewed-by: Tvrtko Ursulin Pulled in entire series, thanks. Imo we have now an even bigger chaos between i915_gem.c and i915_gem_gtt.c around vma handling. But as discussed this is something we can clean up once the dust has settled a bit I think. I guess we could extract an i915_gem_vma.c for all this code, leaving only low-level gtt handling to i915_gem_gtt.c. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 0/5] Unit tests for the new SKL HDMI DPLLs code
The kernel code will follow with all the explanations, but here is the testing code. It's quite handy to have this in userspace to be a bit more exhaustive in the testing that what we can do in the kernel. -- Damien Damien Lespiau (5): compute_wrpll: Rename ddi_compute_wrpll to hsw_compute_wrpll skl_compute_wrpll: Add a way to test the SKL WRPLL algorithm skl_compute_wrpll: Make sure we respect the DCO frequency constraints skl_compute_wrpll: Count how many even/odd dividers we compute skl_compute_wrpll: Prefer even dividers tools/.gitignore | 3 +- tools/Makefile.sources | 3 +- tools/{ddi_compute_wrpll.c => hsw_compute_wrpll.c} | 0 tools/skl_compute_wrpll.c | 910 + 4 files changed, 914 insertions(+), 2 deletions(-) rename tools/{ddi_compute_wrpll.c => hsw_compute_wrpll.c} (100%) create mode 100644 tools/skl_compute_wrpll.c -- 2.1.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 5/5] skl_compute_wrpll: Prefer even dividers
Signed-off-by: Damien Lespiau --- tools/skl_compute_wrpll.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/tools/skl_compute_wrpll.c b/tools/skl_compute_wrpll.c index a3a6e58..55f2df4 100644 --- a/tools/skl_compute_wrpll.c +++ b/tools/skl_compute_wrpll.c @@ -431,6 +431,13 @@ skl_ddi_calculate_wrpll2(int clock /* in Hz */, dco_freq, p); } + + /* +* If a solution is found with an even divider, prefer +* this one. +*/ + if (d == 0 && ctx.p) + break; } } -- 2.1.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 2/5] skl_compute_wrpll: Add a way to test the SKL WRPLL algorithm
I had various problems (infinite loops, unable to compute dividers for certain frequencies) after implementing a BSpec update. Much easier to debug that in userspace. Signed-off-by: Damien Lespiau --- tools/.gitignore | 1 + tools/Makefile.sources| 1 + tools/skl_compute_wrpll.c | 848 ++ 3 files changed, 850 insertions(+) create mode 100644 tools/skl_compute_wrpll.c diff --git a/tools/.gitignore b/tools/.gitignore index f8f04d0..b47fafd 100644 --- a/tools/.gitignore +++ b/tools/.gitignore @@ -31,4 +31,5 @@ intel_stepping intel_vga_read intel_vga_write intel_watermark +skl_compute_wrpll skl_ddb_allocation diff --git a/tools/Makefile.sources b/tools/Makefile.sources index ae60a31..b07a71c 100644 --- a/tools/Makefile.sources +++ b/tools/Makefile.sources @@ -1,5 +1,6 @@ noinst_PROGRAMS = \ hsw_compute_wrpll \ + skl_compute_wrpll \ skl_ddb_allocation \ $(NULL) diff --git a/tools/skl_compute_wrpll.c b/tools/skl_compute_wrpll.c new file mode 100644 index 000..195163c --- /dev/null +++ b/tools/skl_compute_wrpll.c @@ -0,0 +1,848 @@ +/* + * Copyright © 2015 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER + * DEALINGS IN THE SOFTWARE. + */ + +#include +#include +#include +#include +#include +#include +#include + +#define ARRAY_SIZE(arr) (sizeof(arr)/sizeof(arr[0])) + +#define WARN(cond, msg)printf(msg) + +#define KHz(x) (1000 * (x)) +#define MHz(x) KHz(1000 * (x)) + +#define abs_diff(a, b) ({ \ + typeof(a) __a = (a);\ + typeof(b) __b = (b);\ + (void) (&__a == &__b); \ + __a > __b ? (__a - __b) : (__b - __a); }) + +static inline uint64_t div64_u64(uint64_t dividend, uint64_t divisor) +{ + return dividend / divisor; +} + +static inline uint64_t div_u64(uint64_t dividend, uint32_t divisor) +{ + return dividend / divisor; +} + +struct skl_wrpll_params { + uint32_tdco_fraction; + uint32_tdco_integer; + uint32_tqdiv_ratio; + uint32_tqdiv_mode; + uint32_tkdiv; + uint32_tpdiv; + uint32_tcentral_freq; +}; + +static bool +skl_ddi_calculate_wrpll1(int clock /* in Hz */, +struct skl_wrpll_params *wrpll_params) +{ + uint64_t afe_clock = clock * 5; /* AFE Clock is 5x Pixel clock */ + uint64_t dco_central_freq[3] = {84ULL, + 90ULL, + 96ULL}; + uint32_t min_dco_pdeviation = 100; /* DCO freq must be within +1%/-6% */ + uint32_t min_dco_ndeviation = 600; /* of the DCO central freq */ + uint32_t min_dco_index = 3; + uint32_t P0[4] = {1, 2, 3, 7}; + uint32_t P2[4] = {1, 2, 3, 5}; + bool found = false; + uint32_t candidate_p = 0; + uint32_t candidate_p0[3] = {0}, candidate_p1[3] = {0}; + uint32_t candidate_p2[3] = {0}; + uint32_t dco_central_freq_deviation[3]; + uint32_t i, P1, k, dco_count; + bool retry_with_odd = false; + + /* Determine P0, P1 or P2 */ + for (dco_count = 0; dco_count < 3; dco_count++) { + found = false; + candidate_p = + div64_u64(dco_central_freq[dco_count], afe_clock); + if (retry_with_odd == false) + candidate_p = (candidate_p % 2 == 0 ? + candidate_p : candidate_p + 1); + + for (P1 = 1; P1 < candidate_p; P1++) { + for (i = 0; i < 4; i++) { + if (!(P0[i] != 1 || P1 == 1)) + continue; + + for (k = 0; k < 4; k++) { + if (P1 != 1 && P2[k] != 2)
[Intel-gfx] [PATCH i-g-t 1/5] compute_wrpll: Rename ddi_compute_wrpll to hsw_compute_wrpll
We're going to add the SKL version, time to rename the HSW/BDW one. Signed-off-by: Damien Lespiau --- tools/.gitignore | 2 +- tools/Makefile.sources | 2 +- tools/{ddi_compute_wrpll.c => hsw_compute_wrpll.c} | 0 3 files changed, 2 insertions(+), 2 deletions(-) rename tools/{ddi_compute_wrpll.c => hsw_compute_wrpll.c} (100%) diff --git a/tools/.gitignore b/tools/.gitignore index 562e3cd..f8f04d0 100644 --- a/tools/.gitignore +++ b/tools/.gitignore @@ -1,5 +1,5 @@ # Please keep sorted alphabetically -ddi_compute_wrpll +hsw_compute_wrpll intel_audio_dump intel_backlight intel_bios_dumper diff --git a/tools/Makefile.sources b/tools/Makefile.sources index 1df1653..ae60a31 100644 --- a/tools/Makefile.sources +++ b/tools/Makefile.sources @@ -1,5 +1,5 @@ noinst_PROGRAMS = \ - ddi_compute_wrpll \ + hsw_compute_wrpll \ skl_ddb_allocation \ $(NULL) diff --git a/tools/ddi_compute_wrpll.c b/tools/hsw_compute_wrpll.c similarity index 100% rename from tools/ddi_compute_wrpll.c rename to tools/hsw_compute_wrpll.c -- 2.1.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 4/5] skl_compute_wrpll: Count how many even/odd dividers we compute
Signed-off-by: Damien Lespiau --- tools/skl_compute_wrpll.c | 15 ++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/tools/skl_compute_wrpll.c b/tools/skl_compute_wrpll.c index 4f7ea9a..a3a6e58 100644 --- a/tools/skl_compute_wrpll.c +++ b/tools/skl_compute_wrpll.c @@ -836,10 +836,12 @@ struct test_ops { static void test_run(struct test_ops *test) { unsigned int m; + unsigned p_odd_even[2] = { 0, 0 }; for (m = 0; m < ARRAY_SIZE(modes); m++) { struct skl_wrpll_params params = {}; int clock = modes[m].clock; + unsigned int p; if (!test->compute(clock, ¶ms)) { fprintf(stderr, "Couldn't compute divider for %dHz\n", @@ -847,12 +849,13 @@ static void test_run(struct test_ops *test) continue; } + p = params.p0 * params.p1 * params.p2; + /* * make sure we respect the +1%/-6% contraint around the * central frequency */ { - unsigned int p = params.p0 * params.p1 * params.p2; uint64_t dco_freq = (uint64_t)p * clock * 5; uint64_t central_freq = params.central_freq_hz; uint64_t deviation; @@ -871,7 +874,17 @@ static void test_run(struct test_ops *test) "deviation=%"PRIu64"\n", clock, deviation); } + + /* +* count how many even/odd dividers we have through the whole +* list of tested frequencies +*/ + { + p_odd_even[p % 2]++; + } } + + printf("even/odd dividers: %d/%d\n", p_odd_even[0], p_odd_even[1]); } int main(int argc, char **argv) -- 2.1.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 3/5] skl_compute_wrpll: Make sure we respect the DCO frequency constraints
We might as well verify that we have a semblance of all being in order by making sure the DCO frequency is within the expected bounds. Signed-off-by: Damien Lespiau --- tools/skl_compute_wrpll.c | 42 ++ 1 file changed, 42 insertions(+) diff --git a/tools/skl_compute_wrpll.c b/tools/skl_compute_wrpll.c index 195163c..4f7ea9a 100644 --- a/tools/skl_compute_wrpll.c +++ b/tools/skl_compute_wrpll.c @@ -60,6 +60,10 @@ struct skl_wrpll_params { uint32_tkdiv; uint32_tpdiv; uint32_tcentral_freq; + + /* for this test code only */ + uint64_t central_freq_hz; + unsigned int p0, p1, p2; }; static bool @@ -239,6 +243,12 @@ found: } + /* for this unit test only */ + wrpll_params->central_freq_hz = dco_central_freq[min_dco_index]; + wrpll_params->p0 = candidate_p0[min_dco_index]; + wrpll_params->p1 = candidate_p1[min_dco_index]; + wrpll_params->p2 = candidate_p2[min_dco_index]; + return true; } @@ -406,6 +416,7 @@ skl_ddi_calculate_wrpll2(int clock /* in Hz */, }; struct skl_wrpll_context ctx; unsigned int dco, d, i; + unsigned int p0, p1, p2; skl_wrpll_context_init(&ctx); @@ -428,6 +439,12 @@ skl_ddi_calculate_wrpll2(int clock /* in Hz */, skl_wrpll_get_multipliers(ctx.p, &p0, &p1, &p2); + /* for this unit test only */ + wrpll_params->central_freq_hz = ctx.central_freq; + wrpll_params->p0 = p0; + wrpll_params->p1 = p1; + wrpll_params->p2 = p2; + return true; } @@ -829,6 +846,31 @@ static void test_run(struct test_ops *test) clock); continue; } + + /* +* make sure we respect the +1%/-6% contraint around the +* central frequency +*/ + { + unsigned int p = params.p0 * params.p1 * params.p2; + uint64_t dco_freq = (uint64_t)p * clock * 5; + uint64_t central_freq = params.central_freq_hz; + uint64_t deviation; + uint64_t diff; + + diff = abs_diff(dco_freq, central_freq); + deviation = div64_u64(1 * diff, central_freq); + + if (dco_freq > central_freq) { + if (deviation > 100) + printf("failed constraint for %dHz " + "deviation=%"PRIu64"\n", clock, + deviation); + } else if (deviation > 600) + printf("failed constraint for %dHz " + "deviation=%"PRIu64"\n", clock, + deviation); + } } } -- 2.1.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/vblank: Fixup and document timestamp update/read barriers
On 05/07/2015 01:56 PM, Peter Hurley wrote: On 05/06/2015 04:56 AM, Daniel Vetter wrote: On Tue, May 05, 2015 at 11:57:42AM -0400, Peter Hurley wrote: On 05/05/2015 11:42 AM, Daniel Vetter wrote: On Tue, May 05, 2015 at 10:36:24AM -0400, Peter Hurley wrote: On 05/04/2015 12:52 AM, Mario Kleiner wrote: On 04/16/2015 03:03 PM, Daniel Vetter wrote: On Thu, Apr 16, 2015 at 08:30:55AM -0400, Peter Hurley wrote: On 04/15/2015 01:31 PM, Daniel Vetter wrote: On Wed, Apr 15, 2015 at 09:00:04AM -0400, Peter Hurley wrote: Hi Daniel, On 04/15/2015 03:17 AM, Daniel Vetter wrote: This was a bit too much cargo-culted, so lets make it solid: - vblank->count doesn't need to be an atomic, writes are always done under the protection of dev->vblank_time_lock. Switch to an unsigned long instead and update comments. Note that atomic_read is just a normal read of a volatile variable, so no need to audit all the read-side access specifically. - The barriers for the vblank counter seqlock weren't complete: The read-side was missing the first barrier between the counter read and the timestamp read, it only had a barrier between the ts and the counter read. We need both. - Barriers weren't properly documented. Since barriers only work if you have them on boths sides of the transaction it's prudent to reference where the other side is. To avoid duplicating the write-side comment 3 times extract a little store_vblank() helper. In that helper also assert that we do indeed hold dev->vblank_time_lock, since in some cases the lock is acquired a few functions up in the callchain. Spotted while reviewing a patch from Chris Wilson to add a fastpath to the vblank_wait ioctl. Cc: Chris Wilson Cc: Mario Kleiner Cc: Ville Syrjälä Cc: Michel Dänzer Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_irq.c | 92 --- include/drm/drmP.h| 8 +++-- 2 files changed, 54 insertions(+), 46 deletions(-) diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index c8a34476570a..23bfbc61a494 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -74,6 +74,33 @@ module_param_named(vblankoffdelay, drm_vblank_offdelay, int, 0600); module_param_named(timestamp_precision_usec, drm_timestamp_precision, int, 0600); module_param_named(timestamp_monotonic, drm_timestamp_monotonic, int, 0600); +static void store_vblank(struct drm_device *dev, int crtc, + unsigned vblank_count_inc, + struct timeval *t_vblank) +{ +struct drm_vblank_crtc *vblank = &dev->vblank[crtc]; +u32 tslot; + +assert_spin_locked(&dev->vblank_time_lock); + +if (t_vblank) { +tslot = vblank->count + vblank_count_inc; +vblanktimestamp(dev, crtc, tslot) = *t_vblank; +} + +/* + * vblank timestamp updates are protected on the write side with + * vblank_time_lock, but on the read side done locklessly using a + * sequence-lock on the vblank counter. Ensure correct ordering using + * memory barrriers. We need the barrier both before and also after the + * counter update to synchronize with the next timestamp write. + * The read-side barriers for this are in drm_vblank_count_and_time. + */ +smp_wmb(); +vblank->count += vblank_count_inc; +smp_wmb(); The comment and the code are each self-contradictory. If vblank->count writes are always protected by vblank_time_lock (something I did not verify but that the comment above asserts), then the trailing write barrier is not required (and the assertion that it is in the comment is incorrect). A spin unlock operation is always a write barrier. Hm yeah. Otoh to me that's bordering on "code too clever for my own good". That the spinlock is held I can assure. That no one goes around and does multiple vblank updates (because somehow that code raced with the hw itself) I can't easily assure with a simple assert or something similar. It's not the case right now, but that can changes. The algorithm would be broken if multiple updates for the same vblank count were allowed; that's why it checks to see if the vblank count has not advanced before storing a new timestamp. Otherwise, the read side would not be able to determine that the timestamp is valid by double-checking that the vblank count has not changed. And besides, even if the code looped without dropping the spinlock, the correct write order would still be observed because it would still be executing on the same cpu. My objection to the write memory barrier is not about optimization; it's about correct code. Well diff=0 is not allowed, I guess I could enforce this with some WARN_ON. And I still think my point of non-local correctness is solid. With the smp_wmb() removed the following still works correctly: spin_lock(vblank_time_lock); store_vblank(dev, crtc, 1, ts1); spin_unlock(vblank_time_lock); spin_lock(vblank_time_lock); st