Re: [Intel-gfx] [PATCH] drm/i915/chv: calculate rc6 residency correctly
On Wed, Jul 09, 2014 at 02:55:56PM +0300, Mika Kuoppala wrote:
> The register to read cz count is different from vlv. Also
> the counts returned from CCK_CTL1 for BSW are (ticks in 30ns - 1).
> czcount_30ns of value 1 is a special case for 320Mhz.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=80703
> Suggested-by: Deepak S
> Cc: Jesse Barnes
> Signed-off-by: Mika Kuoppala
> Tested-by: Guo Jinxian
> Reviewed-by: Deepak S
Queued for -next, thanks for the patch.
-Daniel
> ---
> drivers/gpu/drm/i915/i915_reg.h |2 +-
> drivers/gpu/drm/i915/i915_sysfs.c | 39
> +
> 2 files changed, 32 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 190d4bb..5a7be63 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2281,7 +2281,7 @@ enum punit_power_well {
> /* Same as Haswell, but 72064 bytes now. */
> #define GEN8_CXT_TOTAL_SIZE (18 * PAGE_SIZE)
>
> -
> +#define CHV_CLK_CTL1 0x101100
> #define VLV_CLK_CTL2 0x101104
> #define CLK_CTL2_CZCOUNT_30NS_SHIFT28
>
> diff --git a/drivers/gpu/drm/i915/i915_sysfs.c
> b/drivers/gpu/drm/i915/i915_sysfs.c
> index 86ce39a..a1d8940 100644
> --- a/drivers/gpu/drm/i915/i915_sysfs.c
> +++ b/drivers/gpu/drm/i915/i915_sysfs.c
> @@ -47,22 +47,45 @@ static u32 calc_residency(struct drm_device *dev, const
> u32 reg)
>
> intel_runtime_pm_get(dev_priv);
>
> - /* On VLV, residency time is in CZ units rather than 1.28us */
> + /* On VLV and CHV, residency time is in CZ units rather than 1.28us */
> if (IS_VALLEYVIEW(dev)) {
> - u32 clkctl2;
> + u32 reg, czcount_30ns;
>
> - clkctl2 = I915_READ(VLV_CLK_CTL2) >>
> - CLK_CTL2_CZCOUNT_30NS_SHIFT;
> - if (!clkctl2) {
> - WARN(!clkctl2, "bogus CZ count value");
> + if (IS_CHERRYVIEW(dev))
> + reg = CHV_CLK_CTL1;
> + else
> + reg = VLV_CLK_CTL2;
> +
> + czcount_30ns = I915_READ(reg) >> CLK_CTL2_CZCOUNT_30NS_SHIFT;
> +
> + if (!czcount_30ns) {
> + WARN(!czcount_30ns, "bogus CZ count value");
> ret = 0;
> goto out;
> }
> - units = DIV_ROUND_UP_ULL(30ULL * bias, (u64)clkctl2);
> +
> + units = 0;
> + div = 100ULL;
> +
> + if (IS_CHERRYVIEW(dev)) {
> + /* Special case for 320Mhz */
> + if (czcount_30ns == 1) {
> + div = 1000ULL;
> + units = 3125ULL;
> + } else {
> + /* chv counts are one less */
> + czcount_30ns += 1;
> + }
> + }
> +
> + if (units == 0)
> + units = DIV_ROUND_UP_ULL(30ULL * bias,
> + (u64)czcount_30ns);
> +
> if (I915_READ(VLV_COUNTER_CONTROL) & VLV_COUNT_RANGE_HIGH)
> units <<= 8;
>
> - div = 100ULL * bias;
> + div = div * bias;
> }
>
> raw_time = I915_READ(reg) * units;
> --
> 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
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [drm-intel:for-linux-next 190/192] drivers/gpu/drm/i915/intel_pm.c:6929:5: sparse: symbol 'byt_gpu_freq' was not declared. Should it be static?
On Sat, Jul 12, 2014 at 01:54:34AM +0800, kbuild test robot wrote:
> tree: git://anongit.freedesktop.org/drm-intel for-linux-next
> head: 7707df4ad6c73e005098c4b4db2f86494e9d404d
> commit: 22b1b2f866b2089d8264e367121c9c9ee0689da4 [190/192] drm/i915: CHV GPU
> frequency to opcode functions
> reproduce: make C=1 CF=-D__CHECK_ENDIAN__
>
>
> sparse warnings: (new ones prefixed by >>)
>
> >> drivers/gpu/drm/i915/intel_pm.c:6929:5: sparse: symbol 'byt_gpu_freq' was
> >> not declared. Should it be static?
> >> drivers/gpu/drm/i915/intel_pm.c:6951:5: sparse: symbol 'byt_freq_opcode'
> >> was not declared. Should it be static?
> >> drivers/gpu/drm/i915/intel_pm.c:6973:5: sparse: symbol 'chv_gpu_freq' was
> >> not declared. Should it be static?
> >> drivers/gpu/drm/i915/intel_pm.c:6998:5: sparse: symbol 'chv_freq_opcode'
> >> was not declared. Should it be static?
>
> Please consider folding the attached diff :-)
Oh, missed that when fixing up missing static from this series.
Queued for -next, thanks for the patch.
-Daniel
>
> ---
> 0-DAY kernel build testing backend Open Source Technology Center
> http://lists.01.org/mailman/listinfo/kbuild Intel Corporation
> From: Fengguang Wu
> Subject: [PATCH drm-intel] drm/i915: byt_gpu_freq() can be static
> TO: Deepak S
> CC: Daniel Vetter
> CC: intel-gfx@lists.freedesktop.org
> CC: dri-de...@lists.freedesktop.org
> CC: linux-ker...@vger.kernel.org
>
> CC: Deepak S
> CC: Daniel Vetter
> Signed-off-by: Fengguang Wu
> ---
> intel_pm.c |8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 1ec777a..656f951 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -6926,7 +6926,7 @@ int sandybridge_pcode_write(struct drm_i915_private
> *dev_priv, u8 mbox, u32 val)
> return 0;
> }
>
> -int byt_gpu_freq(struct drm_i915_private *dev_priv, int val)
> +static int byt_gpu_freq(struct drm_i915_private *dev_priv, int val)
> {
> int div;
>
> @@ -6948,7 +6948,7 @@ int byt_gpu_freq(struct drm_i915_private *dev_priv, int
> val)
> return DIV_ROUND_CLOSEST(dev_priv->mem_freq * (val + 6 - 0xbd), 4 *
> div);
> }
>
> -int byt_freq_opcode(struct drm_i915_private *dev_priv, int val)
> +static int byt_freq_opcode(struct drm_i915_private *dev_priv, int val)
> {
> int mul;
>
> @@ -6970,7 +6970,7 @@ int byt_freq_opcode(struct drm_i915_private *dev_priv,
> int val)
> return DIV_ROUND_CLOSEST(4 * mul * val, dev_priv->mem_freq) + 0xbd - 6;
> }
>
> -int chv_gpu_freq(struct drm_i915_private *dev_priv, int val)
> +static int chv_gpu_freq(struct drm_i915_private *dev_priv, int val)
> {
> int div, freq;
>
> @@ -6995,7 +6995,7 @@ int chv_gpu_freq(struct drm_i915_private *dev_priv, int
> val)
> return freq;
> }
>
> -int chv_freq_opcode(struct drm_i915_private *dev_priv, int val)
> +static int chv_freq_opcode(struct drm_i915_private *dev_priv, int val)
> {
> int mul, opcode;
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC v2 1/1] drm/i915: Power gating display wells during i915_pm_suspend
On Sat, Jul 12, 2014 at 10:02:27AM +0530, sagar.a.kam...@intel.com wrote:
> From: Borun Fu
>
> On VLV, after i915_pm_suspend display power wells are staying
> power ungated. So, after initiating mem sleep "echo mem > /sys/power/state"
> Display is staing D0 State. There might be better way/place to power gate
> these wells. Also, we need to make sure that if wells are power gated due to
> DPMS OFF sequence, they need not be turned off by i915_pm_suspend again.
>
> v2: Extracted helper for intel_crtc_disable and power gating CRTC power wells.
> [Daniel]
>
> Cc: Imre Deak
> Cc: Paulo Zanoni
> Cc: Daniel Vetter
> Cc: Jani Nikula
> Change-Id: I34c80da66aa24c423a5576c68aa1f3a8d0f43848
> Signed-off-by: Sagar Kamble
intel_crtc_control looks like a neat idea - I've started also thinking
about the enable side and noticed that we'll have similar issues there.
But complicated with modeset_global_resources and the differences between
the dpms paths and modeset paths.
But that's work for another day. Queued for -next, thanks for the patch.
-Daniel
> ---
> drivers/gpu/drm/i915/i915_drv.c | 7 +++
> drivers/gpu/drm/i915/i915_drv.h | 4
> drivers/gpu/drm/i915/intel_display.c | 30 +-
> drivers/gpu/drm/i915/intel_drv.h | 1 +
> 4 files changed, 25 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 83cb43a..5e4fefd 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -524,12 +524,11 @@ static int i915_drm_freeze(struct drm_device *dev)
>
> /*
>* Disable CRTCs directly since we want to preserve sw state
> - * for _thaw.
> + * for _thaw. Also, power gate the CRTC power wells.
>*/
> drm_modeset_lock_all(dev);
> - for_each_crtc(dev, crtc) {
> - dev_priv->display.crtc_disable(crtc);
> - }
> + for_each_crtc(dev, crtc)
> + intel_crtc_control(crtc, false);
> drm_modeset_unlock_all(dev);
>
> intel_modeset_suspend_hw(dev);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index a89c912..54f98d3 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -179,6 +179,10 @@ enum hpd_pin {
> list_for_each_entry((intel_connector),
> &(dev)->mode_config.connector_list, base.head) \
> if ((intel_connector)->base.encoder == (__encoder))
>
> +#define for_each_power_domain(domain, mask) \
> + for ((domain) = 0; (domain) < POWER_DOMAIN_NUM; (domain)++) \
> + if ((1 << (domain)) & (mask))
> +
> struct drm_i915_private;
> struct i915_mmu_object;
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index fe6f1db..7a1f14f 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4300,10 +4300,6 @@ static void i9xx_pfit_enable(struct intel_crtc *crtc)
> I915_WRITE(BCLRPAT(crtc->pipe), 0);
> }
>
> -#define for_each_power_domain(domain, mask) \
> - for ((domain) = 0; (domain) < POWER_DOMAIN_NUM; (domain)++) \
> - if ((1 << (domain)) & (mask))
> -
> enum intel_display_power_domain
> intel_display_port_power_domain(struct intel_encoder *intel_encoder)
> {
> @@ -4872,21 +4868,14 @@ static void intel_crtc_update_sarea(struct drm_crtc
> *crtc,
> }
> }
>
> -/**
> - * Sets the power management mode of the pipe and plane.
> - */
> -void intel_crtc_update_dpms(struct drm_crtc *crtc)
> +/* Master function to enable/disable CRTC and corresponding power wells */
> +void intel_crtc_control(struct drm_crtc *crtc, bool enable)
> {
> 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 intel_encoder *intel_encoder;
> enum intel_display_power_domain domain;
> unsigned long domains;
> - bool enable = false;
> -
> - for_each_encoder_on_crtc(dev, crtc, intel_encoder)
> - enable |= intel_encoder->connectors_active;
>
> if (enable) {
> if (!intel_crtc->active) {
> @@ -4907,6 +4896,21 @@ void intel_crtc_update_dpms(struct drm_crtc *crtc)
> intel_crtc->enabled_power_domains = 0;
> }
> }
> +}
> +
> +/**
> + * Sets the power management mode of the pipe and plane.
> + */
> +void intel_crtc_update_dpms(struct drm_crtc *crtc)
> +{
> + struct drm_device *dev = crtc->dev;
> + struct intel_encoder *intel_encoder;
> + bool enable = false;
> +
> + for_each_encoder_on_crtc(dev, crtc, intel_encoder)
> + enable |= intel_encoder->connectors_active;
> +
> + intel_crtc_control(crtc, ena
Re: [Intel-gfx] [PATCH 11/11] drm/i915: Enable PSR by default.
On Fri, Jul 11, 2014 at 10:30:19AM -0700, Rodrigo Vivi wrote:
> Panel Self Refresh is an eDP power saving feature specified by VESA's eDP
> v1.3,
> that allows some panel componets to shutdown while you still see static
> images on
> the screen. Besides being supported on the platform it must be supported by
> the
> eDP panel itself.
>
> Now that we have the propper frontbuffer tracking support and correct locks
> on place
> we can enabled this feature by default.
>
> Signed-off-by: Rodrigo Vivi
Yay!
Entire series pulled in, thanks a lot for the testing and review.
Besides byt psr the big chunk is now polishing the testcase. Iirc I've
written down a detailed description of the task in internal jira and
assigned it to you, right?
I want to have the tests before we enable byt psr to make sure we really
catch all the regressions that might crop up.
Thanks, Daniel
> ---
> drivers/gpu/drm/i915/i915_params.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_params.c
> b/drivers/gpu/drm/i915/i915_params.c
> index 8145729..bbdee21 100644
> --- a/drivers/gpu/drm/i915/i915_params.c
> +++ b/drivers/gpu/drm/i915/i915_params.c
> @@ -37,7 +37,7 @@ struct i915_params i915 __read_mostly = {
> .enable_fbc = -1,
> .enable_hangcheck = true,
> .enable_ppgtt = -1,
> - .enable_psr = 0,
> + .enable_psr = 1,
> .preliminary_hw_support =
> IS_ENABLED(CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT),
> .disable_power_well = 1,
> .enable_ips = 1,
> @@ -118,7 +118,7 @@ MODULE_PARM_DESC(enable_ppgtt,
> "(-1=auto [default], 0=disabled, 1=aliasing, 2=full)");
>
> module_param_named(enable_psr, i915.enable_psr, int, 0600);
> -MODULE_PARM_DESC(enable_psr, "Enable PSR (default: false)");
> +MODULE_PARM_DESC(enable_psr, "Enable PSR (default: true)");
>
> module_param_named(preliminary_hw_support, i915.preliminary_hw_support, int,
> 0600);
> MODULE_PARM_DESC(preliminary_hw_support,
> --
> 1.9.3
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - 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: Ignore VBT backlight presence check on HP Chromebook 14
On Fri, Jul 11, 2014 at 10:16:30PM +, Scot Doyle wrote:
> commit c675949ec58ca50d5a3ae3c757892f1560f6e896
> drm/i915: do not setup backlight if not available according to VBT
>
> caused a regression on the HP Chromebook 14 (with Celeron 2955U CPU),
> which has a misconfigured VBT. Apply quirk to ignore the VBT backlight
> presence check during backlight setup.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=79813
> Signed-off-by: Scot Doyle
> Tested-by: Stefan Nagy
> Cc: Jani Nikula
> Cc: Daniel Vetter
Picked up for -fixes, thanks for the patch.
-Daniel
> ---
> drivers/gpu/drm/i915/intel_display.c |3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index fe6f1db..5d70f02 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12440,6 +12440,9 @@ static struct intel_quirk intel_quirks[] = {
>
> /* Toshiba CB35 Chromebook (Celeron 2955U) */
> { 0x0a06, 0x1179, 0x0a88, quirk_backlight_present },
> +
> + /* HP Chromebook 14 (Celeron 2955U) */
> + { 0x0a06, 0x103c, 0x21ed, quirk_backlight_present },
> };
>
> static void intel_init_quirks(struct drm_device *dev)
> --
> 1.7.10.4
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - 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] igt/gem_partial_pwrite_pread: Add set-cache subtest to validate JIRA VIZ-3721
On Fri, Jul 11, 2014 at 05:55:41PM +, Reese, Armin C wrote:
> Which of the I-G-Ts would be the best to house the set-cache subtest?
> Is gem_partial_pwrite_pread a good candidate?
I guess you could just create a new test binary if there's nothing
suitable. Especially for tests I'm leaning towards "code reuse is
overrated". And with igt_simple_main there's really no boilerplate
(besides the obligatory license header) left.
-Daniel
> - Armin
>
> -Original Message-
> From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter
> Sent: Friday, June 13, 2014 12:14 AM
> To: Reese, Armin C
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH] igt/gem_partial_pwrite_pread: Add set-cache
> subtest to validate JIRA VIZ-3721
>
> On Thu, Jun 12, 2014 at 02:18:37PM -0700, armin.c.re...@intel.com wrote:
> > From: Armin Reese
> >
> > This subtest forces the driver down the code path in
> > i915_gem_object_set_cache_level() which unbinds VMAs and triggers the
> > kernel panic described in VIZ-3721. This test will pass if the bug fix is
> > in place.
> >
> > Bugzilla: https://bugs.fredesktop.org/show_bug.cgi?id=76384
> > Signed-off-by: Armin Reese
> > ---
> > tests/gem_partial_pwrite_pread.c | 11 +++
> > 1 file changed, 11 insertions(+)
> >
> > diff --git a/tests/gem_partial_pwrite_pread.c
> > b/tests/gem_partial_pwrite_pread.c
> > index 7945ba7..3e3ecc9 100644
> > --- a/tests/gem_partial_pwrite_pread.c
> > +++ b/tests/gem_partial_pwrite_pread.c
> > @@ -131,6 +131,14 @@ static void test_partial_reads(void)
> >
> > }
> >
> > +static void test_set_cache(void)
> > +{
> > + igt_info("checking set-cache\n");
> > +
> > + blt_bo_fill(staging_bo, scratch_bo, 0);
> > +
> > +}
>
> Ok, my apologies for the unclear JIRA description. My idea was to have a
> tailor-made testcase with a minimal reproduction scenario (i.e. map into 2
> ppgtt or ggtt + ppgtt, then do a set_caching ioctl).
>
> This here blows up because we do the set_caching as part of the fixture, so
> has a high risk that when someone decides to refactor the testcase we'll no
> longer test this bug. Hence the task to create a new, specific testcase.
> -Daniel
>
> > +
> > static void test_partial_writes(void) {
> > int i, j;
> > @@ -244,6 +252,9 @@ static void do_tests(int cache_level, const char
> > *suffix)
> > gem_set_caching(fd, scratch_bo->handle, cache_level);
> > }
> >
> > + igt_subtest_f("set-cache%s", suffix)
> > + test_set_cache();
> > +
> > igt_subtest_f("reads%s", suffix)
> > test_partial_reads();
> >
> > --
> > 1.9.1
> >
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [Xen-devel] [RFC][PATCH] gpu:drm:i915:intel_detect_pch: back to check devfn instead of check class type
On Fri, Jul 11, 2014 at 08:30:59PM +, Tian, Kevin wrote:
> > From: Konrad Rzeszutek Wilk [mailto:konrad.w...@oracle.com]
> > Sent: Friday, July 11, 2014 12:42 PM
> >
> > On Fri, Jul 11, 2014 at 08:29:56AM +0200, Daniel Vetter wrote:
> > > On Thu, Jul 10, 2014 at 09:08:24PM +, Tian, Kevin wrote:
> > > > actually I'm curious whether it's still necessary to __detect__ PCH.
> > > > Could
> > > > we assume a 1:1 mapping between GPU and PCH, e.g. BDW already hard
> > > > code the knowledge:
> > > >
> > > > } else if (IS_BROADWELL(dev)) {
> > > > dev_priv->pch_type = PCH_LPT;
> > > > dev_priv->pch_id =
> > > >
> > INTEL_PCH_LPT_LP_DEVICE_ID_TYPE;
> > > > DRM_DEBUG_KMS("This is Broadwell,
> > assuming "
> > > > "LynxPoint LP PCH\n");
> > > >
> > > > Or if there is real usage on non-fixed mapping (not majority), could it
> > > > be a
> > > > better option to have fixed mapping as a fallback instead of leaving as
> > > > PCH_NONE? Then even when Qemu doesn't provide a special tweaked
> > PCH,
> > > > the majority case just works.
> > >
> > > I guess we can do it, at least I haven't seen any strange combinations in
> > > the wild outside of Intel ...
> >
> > How big is the QA matrix for this? Would it make sense to just
> > include the latest hardware (say going two generations back)
> > and ignore the older one?
>
> suppose minimal or no QA effort on bare metal, if we only conservatively
> change the fallback path which is today not supposed to function with
> PCH_NONE. so it's only same amount of QA effort as whatever else is
> proposed in this passthru upstreaming task. I agree no need to cover
> older model, possibly just snb, ivb and hsw, but will leave Tiejun to answer
> the overall goal.
Yeah, I'd be ok with the approach of using defaults if we can't recognize
the pch - if anyone screams we can either quirk or figure something else
out.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - 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 11/11] drm/i915: Enable PSR by default.
On Sat, Jul 12, 2014 at 12:05:13PM +0200, Daniel Vetter wrote: > On Fri, Jul 11, 2014 at 10:30:19AM -0700, Rodrigo Vivi wrote: > > Panel Self Refresh is an eDP power saving feature specified by VESA's eDP > > v1.3, > > that allows some panel componets to shutdown while you still see static > > images on > > the screen. Besides being supported on the platform it must be supported by > > the > > eDP panel itself. > > > > Now that we have the propper frontbuffer tracking support and correct locks > > on place > > we can enabled this feature by default. > > > > Signed-off-by: Rodrigo Vivi > > Yay! > > Entire series pulled in, thanks a lot for the testing and review. Is there any chance we can have the output property to know when PSR is available and active on the eDP? -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/2] Low hanging fruit to reduce the driver size
On Fri, Jul 11, 2014 at 06:34:12PM +0100, Damien Lespiau wrote: > Being able to target a single platform to reduce the driver size has been > voiced a few times. These patches provide a Kconfig option to provide the > opportunity. > > Let's start small, and, along side the generic "Multi-platform" option, only > present Haswell and Broadwell in the list of platforms to choose from. When > selecting any of those those two platforms, (S)DVO and MIPI DSI code is not > compiled. > > The driver .text size is reduced by 67Kb (8.8%) when selecting HSW/BDW. Iirc my poor-man's-lto (include everything into one file) hack with a similar scenario and hard-coding intel_info plus adding a few hacks to help along gcc saved around 20-25% percent. And I'm really not terribly convinced that kb shaving is worth the effort overall, especially if it comes at a fairly steep maintainance burden of piles of new config options and #defines. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - 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/2] Low hanging fruit to reduce the driver size
On Sat, Jul 12, 2014 at 01:01:42PM +0200, Daniel Vetter wrote: > And I'm really not terribly convinced that kb shaving is worth the effort > overall, especially if it comes at a fairly steep maintainance burden of > piles of new config options and #defines. I wanted to try, I tried, probably not worth it, next! -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 11/11] drm/i915: Enable PSR by default.
On Sat, Jul 12, 2014 at 12:42 PM, Chris Wilson wrote: > On Sat, Jul 12, 2014 at 12:05:13PM +0200, Daniel Vetter wrote: >> On Fri, Jul 11, 2014 at 10:30:19AM -0700, Rodrigo Vivi wrote: >> > Panel Self Refresh is an eDP power saving feature specified by VESA's eDP >> > v1.3, >> > that allows some panel componets to shutdown while you still see static >> > images on >> > the screen. Besides being supported on the platform it must be supported >> > by the >> > eDP panel itself. >> > >> > Now that we have the propper frontbuffer tracking support and correct >> > locks on place >> > we can enabled this feature by default. >> > >> > Signed-off-by: Rodrigo Vivi >> >> Yay! >> >> Entire series pulled in, thanks a lot for the testing and review. > > Is there any chance we can have the output property to know when PSR is > available and active on the eDP? Oh right, forgotten about this. I guess now that the hw stuff is there we can resurrect this. And with the universal planes support we could move the "prefer backbuffer rendering" hint to planes even where it imo belongs (since e.g. fbc is on the plane). Or maybe we don't care about that level of granularity and just have one per pipe. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - 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/3] drm/i915: wait for all DSI FIFOs to be empty
Ensure that the DSI packets for a particular sequence are completely
sent before going ahead in the enabling or disabling of the panel
Signed-off-by: Shobhit Kumar
---
drivers/gpu/drm/i915/intel_dsi.c | 8
drivers/gpu/drm/i915/intel_dsi_cmd.c | 16
drivers/gpu/drm/i915/intel_dsi_cmd.h | 1 +
3 files changed, 25 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 61da0e5..98c78ab 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -152,6 +152,8 @@ static void intel_dsi_enable(struct intel_encoder *encoder)
if (intel_dsi->dev.dev_ops->enable)
intel_dsi->dev.dev_ops->enable(&intel_dsi->dev);
+ wait_for_dsi_fifo_empty(intel_dsi);
+
/* assert ip_tg_enable signal */
temp = I915_READ(MIPI_PORT_CTRL(pipe)) &
~LANE_CONFIGURATION_MASK;
temp = temp | intel_dsi->port_bits;
@@ -192,6 +194,8 @@ static void intel_dsi_pre_enable(struct intel_encoder
*encoder)
if (intel_dsi->dev.dev_ops->send_otp_cmds)
intel_dsi->dev.dev_ops->send_otp_cmds(&intel_dsi->dev);
+ wait_for_dsi_fifo_empty(intel_dsi);
+
/* Enable port in pre-enable phase itself because as per hw team
* recommendation, port should be enabled befor plane & pipe */
intel_dsi_enable(encoder);
@@ -232,6 +236,8 @@ static void intel_dsi_disable(struct intel_encoder *encoder)
DRM_DEBUG_KMS("\n");
if (is_vid_mode(intel_dsi)) {
+ wait_for_dsi_fifo_empty(intel_dsi);
+
/* de-assert ip_tg_enable signal */
temp = I915_READ(MIPI_PORT_CTRL(pipe));
I915_WRITE(MIPI_PORT_CTRL(pipe), temp & ~DPI_ENABLE);
@@ -261,6 +267,8 @@ static void intel_dsi_disable(struct intel_encoder *encoder)
* some next enable sequence send turn on packet error is observed */
if (intel_dsi->dev.dev_ops->disable)
intel_dsi->dev.dev_ops->disable(&intel_dsi->dev);
+
+ wait_for_dsi_fifo_empty(intel_dsi);
}
static void intel_dsi_clear_device_ready(struct intel_encoder *encoder)
diff --git a/drivers/gpu/drm/i915/intel_dsi_cmd.c
b/drivers/gpu/drm/i915/intel_dsi_cmd.c
index 933c863..7f1430a 100644
--- a/drivers/gpu/drm/i915/intel_dsi_cmd.c
+++ b/drivers/gpu/drm/i915/intel_dsi_cmd.c
@@ -419,3 +419,19 @@ int dpi_send_cmd(struct intel_dsi *intel_dsi, u32 cmd,
bool hs)
return 0;
}
+
+void wait_for_dsi_fifo_empty(struct intel_dsi *intel_dsi)
+{
+ struct drm_encoder *encoder = &intel_dsi->base.base;
+ struct drm_device *dev = encoder->dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
+ enum pipe pipe = intel_crtc->pipe;
+ u32 mask;
+
+ mask = LP_CTRL_FIFO_EMPTY | HS_CTRL_FIFO_EMPTY |
+ LP_DATA_FIFO_EMPTY | HS_DATA_FIFO_EMPTY;
+
+ if (wait_for((I915_READ(MIPI_GEN_FIFO_STAT(pipe)) & mask) == mask, 100))
+ DRM_ERROR("DPI FIFOs are not empty\n");
+}
diff --git a/drivers/gpu/drm/i915/intel_dsi_cmd.h
b/drivers/gpu/drm/i915/intel_dsi_cmd.h
index 9a18cbf..46aa1ac 100644
--- a/drivers/gpu/drm/i915/intel_dsi_cmd.h
+++ b/drivers/gpu/drm/i915/intel_dsi_cmd.h
@@ -51,6 +51,7 @@ int dsi_vc_generic_read(struct intel_dsi *intel_dsi, int
channel,
u8 *reqdata, int reqlen, u8 *buf, int buflen);
int dpi_send_cmd(struct intel_dsi *intel_dsi, u32 cmd, bool hs);
+void wait_for_dsi_fifo_empty(struct intel_dsi *intel_dsi);
/* XXX: questionable write helpers */
static inline int dsi_vc_dcs_write_0(struct intel_dsi *intel_dsi,
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 0/3] Fixing last of few known issues in DSI and Burst mode Support
Hi, This pacth set addresses a couple WARN dumps as DSI encoder did not yet implement the ->get_config for state tracking. This is tried to be addressed here. Most likely I have missed few things but atleast all WARNs are taken care of. Also better check is added to ensure all data has been sent to the panel before going ahead in the sequence of enabling/disabling of the panel by waiting for all FIFOs to be empty. Another new feature is DSI BURST mode support. With this single link DSI Video mode support in our driver will be complete. Next plan is Dual link video mode support. Regards Shobhit Shobhit Kumar (3): drm/i915: Add get_config implementation for DSI encoder drm/i915: wait for all DSI FIFOs to be empty drm/i915: Add support for Video Burst Mode for MIPI DSI drivers/gpu/drm/i915/intel_bios.h | 3 +- drivers/gpu/drm/i915/intel_display.c | 7 +-- drivers/gpu/drm/i915/intel_dsi.c | 75 ++ drivers/gpu/drm/i915/intel_dsi.h | 5 ++ drivers/gpu/drm/i915/intel_dsi_cmd.c | 16 +++ drivers/gpu/drm/i915/intel_dsi_cmd.h | 1 + drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 38 ++- drivers/gpu/drm/i915/intel_dsi_pll.c | 13 +++--- 8 files changed, 137 insertions(+), 21 deletions(-) -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/3] drm/i915: Add support for Video Burst Mode for MIPI DSI
Signed-off-by: Shobhit Kumar
---
drivers/gpu/drm/i915/intel_bios.h | 3 ++-
drivers/gpu/drm/i915/intel_dsi.c | 22 ++---
drivers/gpu/drm/i915/intel_dsi.h | 2 ++
drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 38 --
drivers/gpu/drm/i915/intel_dsi_pll.c | 9 +++
5 files changed, 57 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_bios.h
b/drivers/gpu/drm/i915/intel_bios.h
index b986677..905999b 100644
--- a/drivers/gpu/drm/i915/intel_bios.h
+++ b/drivers/gpu/drm/i915/intel_bios.h
@@ -802,7 +802,8 @@ struct mipi_config {
u16 rsvd4;
- u8 rsvd5[5];
+ u8 rsvd5;
+ u32 target_burst_mode_freq;
u32 dsi_ddr_clk;
u32 bridge_ref_clk;
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 98c78ab..732d96b 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -449,9 +449,11 @@ static u16 txclkesc(u32 divider, unsigned int us)
}
/* return pixels in terms of txbyteclkhs */
-static u16 txbyteclkhs(u16 pixels, int bpp, int lane_count)
+static u16 txbyteclkhs(u16 pixels, int bpp, int lane_count,
+ u16 burst_mode_ratio)
{
- return DIV_ROUND_UP(DIV_ROUND_UP(pixels * bpp, 8), lane_count);
+ return DIV_ROUND_UP(DIV_ROUND_UP(pixels * bpp * burst_mode_ratio,
+ 8 * 100), lane_count);
}
static void set_dsi_timings(struct drm_encoder *encoder,
@@ -477,10 +479,12 @@ static void set_dsi_timings(struct drm_encoder *encoder,
vbp = mode->vtotal - mode->vsync_end;
/* horizontal values are in terms of high speed byte clock */
- hactive = txbyteclkhs(hactive, bpp, lane_count);
- hfp = txbyteclkhs(hfp, bpp, lane_count);
- hsync = txbyteclkhs(hsync, bpp, lane_count);
- hbp = txbyteclkhs(hbp, bpp, lane_count);
+ hactive = txbyteclkhs(hactive, bpp, lane_count,
+ intel_dsi->burst_mode_ratio);
+ hfp = txbyteclkhs(hfp, bpp, lane_count, intel_dsi->burst_mode_ratio);
+ hsync = txbyteclkhs(hsync, bpp, lane_count,
+ intel_dsi->burst_mode_ratio);
+ hbp = txbyteclkhs(hbp, bpp, lane_count, intel_dsi->burst_mode_ratio);
I915_WRITE(MIPI_HACTIVE_AREA_COUNT(pipe), hactive);
I915_WRITE(MIPI_HFP_COUNT(pipe), hfp);
@@ -567,12 +571,14 @@ static void intel_dsi_prepare(struct intel_encoder
*intel_encoder)
intel_dsi->video_mode_format == VIDEO_MODE_BURST) {
I915_WRITE(MIPI_HS_TX_TIMEOUT(pipe),
txbyteclkhs(adjusted_mode->htotal, bpp,
- intel_dsi->lane_count) + 1);
+ intel_dsi->lane_count,
+ intel_dsi->burst_mode_ratio) + 1);
} else {
I915_WRITE(MIPI_HS_TX_TIMEOUT(pipe),
txbyteclkhs(adjusted_mode->vtotal *
adjusted_mode->htotal,
- bpp, intel_dsi->lane_count) + 1);
+ bpp, intel_dsi->lane_count,
+ intel_dsi->burst_mode_ratio) + 1);
}
I915_WRITE(MIPI_LP_RX_TIMEOUT(pipe), intel_dsi->lp_rx_timeout);
I915_WRITE(MIPI_TURN_AROUND_TIMEOUT(pipe), intel_dsi->turn_arnd_val);
diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
index e0c16b0..a34ffa4 100644
--- a/drivers/gpu/drm/i915/intel_dsi.h
+++ b/drivers/gpu/drm/i915/intel_dsi.h
@@ -116,6 +116,8 @@ struct intel_dsi {
u16 clk_hs_to_lp_count;
u16 init_count;
+ u32 pclk;
+ u16 burst_mode_ratio;
/* all delays in ms */
u16 backlight_off_delay;
diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
index 47c7584..1f5abb4 100644
--- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
+++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
@@ -271,6 +271,8 @@ static bool generic_init(struct intel_dsi_device *dsi)
u32 ths_prepare_ns, tclk_trail_ns;
u32 tclk_prepare_clkzero, ths_prepare_hszero;
u32 lp_to_hs_switch, hs_to_lp_switch;
+ u32 pclk, computed_ddr;
+ u16 burst_mode_ratio;
DRM_DEBUG_KMS("\n");
@@ -284,8 +286,6 @@ static bool generic_init(struct intel_dsi_device *dsi)
else if (intel_dsi->pixel_format == VID_MODE_FORMAT_RGB565)
bits_per_pixel = 16;
- bitrate = (mode->clock * bits_per_pixel) / intel_dsi->lane_count;
-
intel_dsi->operation_mode = mipi_config->is_cmd_mode;
intel_dsi->video_mode_format = mipi_config->video_transfer_mode;
intel_dsi->escape_clk_div = mipi_config->byte_clk_sel;
@@ -297,6 +297,40 @@ static bool generic_init(struct intel_dsi_devi
[Intel-gfx] [PATCH 1/3] drm/i915: Add get_config implementation for DSI encoder
Call to vlv_crtc_clock_get is not needed for DSI and was causing dpio
read WARN dumps as well. Absence of ->get_config was casuing othet WARN
dumps as well. With this the last of the known WARN dumps for DSI should
be fixed.
Signed-off-by: Shobhit Kumar
---
drivers/gpu/drm/i915/intel_display.c | 7 +++---
drivers/gpu/drm/i915/intel_dsi.c | 45
drivers/gpu/drm/i915/intel_dsi.h | 3 +++
drivers/gpu/drm/i915/intel_dsi_pll.c | 4 +++-
4 files changed, 55 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index fe6f1db..3d0ea7c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6309,9 +6309,10 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
if (IS_CHERRYVIEW(dev))
chv_crtc_clock_get(crtc, pipe_config);
- else if (IS_VALLEYVIEW(dev))
- vlv_crtc_clock_get(crtc, pipe_config);
- else
+ else if (IS_VALLEYVIEW(dev)) {
+ if (!intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_DSI))
+ vlv_crtc_clock_get(crtc, pipe_config);
+ } else
i9xx_crtc_clock_get(crtc, pipe_config);
return true;
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index bfcefbf..61da0e5 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -351,9 +351,54 @@ static bool intel_dsi_get_hw_state(struct intel_encoder
*encoder,
static void intel_dsi_get_config(struct intel_encoder *encoder,
struct intel_crtc_config *pipe_config)
{
+ struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
+ struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
+ u32 dsi_clock, pclk;
+ u32 pll_ctl, pll_div;
+ u32 m = 0, p = 0;
+ int refclk = 25000;
+ int i;
+
DRM_DEBUG_KMS("\n");
/* XXX: read flags, set to adjusted_mode */
+ pipe_config->quirks = 1;
+
+ memset(&pipe_config->dpll_hw_state, 0,
+ sizeof(pipe_config->dpll_hw_state));
+
+ mutex_lock(&dev_priv->dpio_lock);
+ pll_ctl = vlv_cck_read(dev_priv, CCK_REG_DSI_PLL_CONTROL);
+ pll_div = vlv_cck_read(dev_priv, CCK_REG_DSI_PLL_DIVIDER);
+ mutex_unlock(&dev_priv->dpio_lock);
+
+ pll_ctl &= ~(DSI_PLL_CLK_GATE_DSI0_DSIPLL | DSI_PLL_VCO_EN);
+ pll_ctl = pll_ctl >> (DSI_PLL_P1_POST_DIV_SHIFT - 2);
+
+ while (pll_ctl) {
+ pll_ctl = pll_ctl >> 1;
+ p++;
+ }
+ p--;
+
+ for (i = 0; i < num_lfsr_converts; i++) {
+ if (lfsr_converts[i] == pll_div)
+ break;
+ }
+
+ if (i == num_lfsr_converts) {
+ DRM_ERROR("wrong m_seed programmed\n");
+ return;
+ }
+
+ m = i + 62;
+
+ dsi_clock = (m * refclk) / p;
+ pclk = DIV_ROUND_CLOSEST(dsi_clock * intel_dsi->lane_count,
+pipe_config->pipe_bpp);
+
+ pipe_config->adjusted_mode.crtc_clock = pclk;
+ pipe_config->port_clock = pclk;
}
static enum drm_mode_status
diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
index 31db33d..e0c16b0 100644
--- a/drivers/gpu/drm/i915/intel_dsi.h
+++ b/drivers/gpu/drm/i915/intel_dsi.h
@@ -130,6 +130,9 @@ static inline struct intel_dsi *enc_to_intel_dsi(struct
drm_encoder *encoder)
return container_of(encoder, struct intel_dsi, base.base);
}
+extern const u32 lfsr_converts[];
+extern const int num_lfsr_converts;
+
extern void vlv_enable_dsi_pll(struct intel_encoder *encoder);
extern void vlv_disable_dsi_pll(struct intel_encoder *encoder);
diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c
b/drivers/gpu/drm/i915/intel_dsi_pll.c
index ba79ec1..78449ea 100644
--- a/drivers/gpu/drm/i915/intel_dsi_pll.c
+++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
@@ -43,13 +43,15 @@ struct dsi_mnp {
u32 dsi_pll_div;
};
-static const u32 lfsr_converts[] = {
+const u32 lfsr_converts[] = {
426, 469, 234, 373, 442, 221, 110, 311, 411,/* 62 - 70 */
461, 486, 243, 377, 188, 350, 175, 343, 427, 213, /* 71 - 80 */
106, 53, 282, 397, 354, 227, 113, 56, 284, 142, /* 81 - 90 */
71, 35 /* 91 - 92 */
};
+const int num_lfsr_converts = sizeof(lfsr_converts) / sizeof(lfsr_converts[0]);
+
#ifdef DSI_CLK_FROM_RR
static u32 dsi_rr_formula(const struct drm_display_mode *mode,
--
1.9.1
___
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: Add get_config implementation for DSI encoder
On Sat, Jul 12, 2014 at 1:47 PM, Shobhit Kumar wrote:
> Call to vlv_crtc_clock_get is not needed for DSI and was causing dpio
> read WARN dumps as well. Absence of ->get_config was casuing othet WARN
> dumps as well. With this the last of the known WARN dumps for DSI should
> be fixed.
>
> Signed-off-by: Shobhit Kumar
> ---
> drivers/gpu/drm/i915/intel_display.c | 7 +++---
> drivers/gpu/drm/i915/intel_dsi.c | 45
>
> drivers/gpu/drm/i915/intel_dsi.h | 3 +++
> drivers/gpu/drm/i915/intel_dsi_pll.c | 4 +++-
> 4 files changed, 55 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index fe6f1db..3d0ea7c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6309,9 +6309,10 @@ static bool i9xx_get_pipe_config(struct intel_crtc
> *crtc,
>
> if (IS_CHERRYVIEW(dev))
> chv_crtc_clock_get(crtc, pipe_config);
> - else if (IS_VALLEYVIEW(dev))
> - vlv_crtc_clock_get(crtc, pipe_config);
> - else
> + else if (IS_VALLEYVIEW(dev)) {
> + if (!intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_DSI))
> + vlv_crtc_clock_get(crtc, pipe_config);
If I understand the logic correctly we don't even enable the DPLL for
dsi, i.e. bit31 is clear. So instead of this sw-side check (which is
fragile since it depends upon corrected encoder->pipe links in our
data structures) we should instead check bit 31 in vlv_crtc_clock_get
and just bail out without reading out hw settings.
The goal of the hw state cross-check is to check the hw state, so
wherever possible we should rely 100% on available information in the
hw and not on software tracking.
> + } else
> i9xx_crtc_clock_get(crtc, pipe_config);
>
> return true;
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c
> b/drivers/gpu/drm/i915/intel_dsi.c
> index bfcefbf..61da0e5 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -351,9 +351,54 @@ static bool intel_dsi_get_hw_state(struct intel_encoder
> *encoder,
> static void intel_dsi_get_config(struct intel_encoder *encoder,
> struct intel_crtc_config *pipe_config)
> {
> + struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> + struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> + u32 dsi_clock, pclk;
> + u32 pll_ctl, pll_div;
> + u32 m = 0, p = 0;
> + int refclk = 25000;
> + int i;
> +
> DRM_DEBUG_KMS("\n");
>
> /* XXX: read flags, set to adjusted_mode */
> + pipe_config->quirks = 1;
Nack. First you need to use one of the symbolic quirk definitions
(there's a bunch of them). Second this needs a comment why exactly we
need the quirk (which really only should be used if there's no way to
read a given piece of state back from the hw).
> +
> + memset(&pipe_config->dpll_hw_state, 0,
> + sizeof(pipe_config->dpll_hw_state));
> +
> + mutex_lock(&dev_priv->dpio_lock);
> + pll_ctl = vlv_cck_read(dev_priv, CCK_REG_DSI_PLL_CONTROL);
> + pll_div = vlv_cck_read(dev_priv, CCK_REG_DSI_PLL_DIVIDER);
> + mutex_unlock(&dev_priv->dpio_lock);
> +
> + pll_ctl &= ~(DSI_PLL_CLK_GATE_DSI0_DSIPLL | DSI_PLL_VCO_EN);
> + pll_ctl = pll_ctl >> (DSI_PLL_P1_POST_DIV_SHIFT - 2);
> +
> + while (pll_ctl) {
> + pll_ctl = pll_ctl >> 1;
> + p++;
> + }
> + p--;
> +
> + for (i = 0; i < num_lfsr_converts; i++) {
> + if (lfsr_converts[i] == pll_div)
> + break;
> + }
> +
> + if (i == num_lfsr_converts) {
> + DRM_ERROR("wrong m_seed programmed\n");
> + return;
> + }
> +
> + m = i + 62;
> +
> + dsi_clock = (m * refclk) / p;
> + pclk = DIV_ROUND_CLOSEST(dsi_clock * intel_dsi->lane_count,
> +pipe_config->pipe_bpp);
> +
> + pipe_config->adjusted_mode.crtc_clock = pclk;
> + pipe_config->port_clock = pclk;
> }
>
> static enum drm_mode_status
> diff --git a/drivers/gpu/drm/i915/intel_dsi.h
> b/drivers/gpu/drm/i915/intel_dsi.h
> index 31db33d..e0c16b0 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.h
> +++ b/drivers/gpu/drm/i915/intel_dsi.h
> @@ -130,6 +130,9 @@ static inline struct intel_dsi *enc_to_intel_dsi(struct
> drm_encoder *encoder)
> return container_of(encoder, struct intel_dsi, base.base);
> }
>
> +extern const u32 lfsr_converts[];
> +extern const int num_lfsr_converts;
> +
> extern void vlv_enable_dsi_pll(struct intel_encoder *encoder);
> extern void vlv_disable_dsi_pll(struct intel_encoder *encoder);
>
> diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c
> b/drivers/gpu/drm/i915/intel_dsi_pll.c
> index ba79ec1..78449ea 100644
> --- a/drivers
Re: [Intel-gfx] [PATCH 11/11] drm/i915: Enable PSR by default.
On Sat, Jul 12, 2014 at 01:20:10PM +0200, Daniel Vetter wrote: > On Sat, Jul 12, 2014 at 12:42 PM, Chris Wilson > wrote: > > On Sat, Jul 12, 2014 at 12:05:13PM +0200, Daniel Vetter wrote: > >> On Fri, Jul 11, 2014 at 10:30:19AM -0700, Rodrigo Vivi wrote: > >> > Panel Self Refresh is an eDP power saving feature specified by VESA's > >> > eDP v1.3, > >> > that allows some panel componets to shutdown while you still see static > >> > images on > >> > the screen. Besides being supported on the platform it must be supported > >> > by the > >> > eDP panel itself. > >> > > >> > Now that we have the propper frontbuffer tracking support and correct > >> > locks on place > >> > we can enabled this feature by default. > >> > > >> > Signed-off-by: Rodrigo Vivi > >> > >> Yay! > >> > >> Entire series pulled in, thanks a lot for the testing and review. > > > > Is there any chance we can have the output property to know when PSR is > > available and active on the eDP? > > Oh right, forgotten about this. I guess now that the hw stuff is there > we can resurrect this. And with the universal planes support we could > move the "prefer backbuffer rendering" hint to planes even where it > imo belongs (since e.g. fbc is on the plane). Or maybe we don't care > about that level of granularity and just have one per pipe. But PSR is definitely an output property... Please-dont-touch-me is indeed a plane property. -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 11/11] drm/i915: Enable PSR by default.
On Sat, Jul 12, 2014 at 2:02 PM, Chris Wilson wrote: > But PSR is definitely an output property... Please-dont-touch-me is > indeed a plane property. Well I guess we also want this for fbc, and fbc isn't an output property. I guess in the end it doesn't really matter that much as long as it's there, but traditional userspace exposes all output properties to userspace (on X), hence why I think we should hide it a bit ;-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - 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 11/11] drm/i915: Enable PSR by default.
On Sat, Jul 12, 2014 at 07:14:33PM +0200, Daniel Vetter wrote: > On Sat, Jul 12, 2014 at 2:02 PM, Chris Wilson > wrote: > > But PSR is definitely an output property... Please-dont-touch-me is > > indeed a plane property. > > Well I guess we also want this for fbc, and fbc isn't an output > property. I guess in the end it doesn't really matter that much as > long as it's there, but traditional userspace exposes all output > properties to userspace (on X), hence why I think we should hide it a > bit ;-) Hence why I want PSR as an output property. I want to see the status exposed in xrandr. -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 11/11] drm/i915: Enable PSR by default.
On Sat, Jul 12, 2014 at 7:51 PM, Chris Wilson wrote: > On Sat, Jul 12, 2014 at 07:14:33PM +0200, Daniel Vetter wrote: >> On Sat, Jul 12, 2014 at 2:02 PM, Chris Wilson >> wrote: >> > But PSR is definitely an output property... Please-dont-touch-me is >> > indeed a plane property. >> >> Well I guess we also want this for fbc, and fbc isn't an output >> property. I guess in the end it doesn't really matter that much as >> long as it's there, but traditional userspace exposes all output >> properties to userspace (on X), hence why I think we should hide it a >> bit ;-) > > Hence why I want PSR as an output property. I want to see the status > exposed in xrandr. Hm, why that? I've thought we only want this to give hints to the compositor/ddx? -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - 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 11/11] drm/i915: Enable PSR by default.
On Sat, Jul 12, 2014 at 08:15:42PM +0200, Daniel Vetter wrote: > On Sat, Jul 12, 2014 at 7:51 PM, Chris Wilson > wrote: > > On Sat, Jul 12, 2014 at 07:14:33PM +0200, Daniel Vetter wrote: > >> On Sat, Jul 12, 2014 at 2:02 PM, Chris Wilson > >> wrote: > >> > But PSR is definitely an output property... Please-dont-touch-me is > >> > indeed a plane property. > >> > >> Well I guess we also want this for fbc, and fbc isn't an output > >> property. I guess in the end it doesn't really matter that much as > >> long as it's there, but traditional userspace exposes all output > >> properties to userspace (on X), hence why I think we should hide it a > >> bit ;-) > > > > Hence why I want PSR as an output property. I want to see the status > > exposed in xrandr. > > Hm, why that? I've thought we only want this to give hints to the > compositor/ddx? An interesting factoid to present to the user. Also makes it easy for me to check everything works in X. -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 11/11] drm/i915: Enable PSR by default.
On Sat, Jul 12, 2014 at 8:55 PM, Chris Wilson wrote: > On Sat, Jul 12, 2014 at 08:15:42PM +0200, Daniel Vetter wrote: >> On Sat, Jul 12, 2014 at 7:51 PM, Chris Wilson >> wrote: >> > On Sat, Jul 12, 2014 at 07:14:33PM +0200, Daniel Vetter wrote: >> >> On Sat, Jul 12, 2014 at 2:02 PM, Chris Wilson >> >> wrote: >> >> > But PSR is definitely an output property... Please-dont-touch-me is >> >> > indeed a plane property. >> >> >> >> Well I guess we also want this for fbc, and fbc isn't an output >> >> property. I guess in the end it doesn't really matter that much as >> >> long as it's there, but traditional userspace exposes all output >> >> properties to userspace (on X), hence why I think we should hide it a >> >> bit ;-) >> > >> > Hence why I want PSR as an output property. I want to see the status >> > exposed in xrandr. >> >> Hm, why that? I've thought we only want this to give hints to the >> compositor/ddx? > > An interesting factoid to present to the user. Also makes it easy for me > to check everything works in X. Hm not sure we want users to know this stuff. Next they ask to adjust it and then we have the same mess as with kernel options ;-) But I don't care strongly enough really either way. In-kernel I still think we want to keep this on the crtc so that fbc and psr work the same way. The connector property would then just chase the connected crtc to read out the property. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - 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 11/11] drm/i915: Enable PSR by default.
On Sat, Jul 12, 2014 at 10:48:30PM +0200, Daniel Vetter wrote: > On Sat, Jul 12, 2014 at 8:55 PM, Chris Wilson > wrote: > > On Sat, Jul 12, 2014 at 08:15:42PM +0200, Daniel Vetter wrote: > >> On Sat, Jul 12, 2014 at 7:51 PM, Chris Wilson > >> wrote: > >> > On Sat, Jul 12, 2014 at 07:14:33PM +0200, Daniel Vetter wrote: > >> >> On Sat, Jul 12, 2014 at 2:02 PM, Chris Wilson > >> >> wrote: > >> >> > But PSR is definitely an output property... Please-dont-touch-me is > >> >> > indeed a plane property. > >> >> > >> >> Well I guess we also want this for fbc, and fbc isn't an output > >> >> property. I guess in the end it doesn't really matter that much as > >> >> long as it's there, but traditional userspace exposes all output > >> >> properties to userspace (on X), hence why I think we should hide it a > >> >> bit ;-) > >> > > >> > Hence why I want PSR as an output property. I want to see the status > >> > exposed in xrandr. > >> > >> Hm, why that? I've thought we only want this to give hints to the > >> compositor/ddx? > > > > An interesting factoid to present to the user. Also makes it easy for me > > to check everything works in X. > > Hm not sure we want users to know this stuff. Next they ask to adjust > it and then we have the same mess as with kernel options ;-) But I > don't care strongly enough really either way. In-kernel I still think > we want to keep this on the crtc so that fbc and psr work the same > way. The connector property would then just chase the connected crtc > to read out the property. It can't be crtc as it is an connector properrty... And it is immutable. I think it will be one of those useful things that people would like to check infrequently to make sure everything is functioning as intended (like powertop). I think there may be a few properties like this we can expose. And if people ask why PSR isn't active, then we should do a better job at making sure it stays enabled. -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
