All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Pretend cursor is always on for ILK-style WM calculations
@ 2016-02-01  9:26 Matt Roper
  2016-02-01  9:47 ` ✗ Fi.CI.BAT: failure for " Patchwork
  2016-02-01 14:22 ` [PATCH] " Ville Syrjälä
  0 siblings, 2 replies; 14+ messages in thread
From: Matt Roper @ 2016-02-01  9:26 UTC (permalink / raw)
  To: intel-gfx; +Cc: simdev11, manfred.kitzbichler

Due to our lack of two-step watermark programming, our driver has
historically pretended that the cursor plane is always on for the
purpose of watermark calculations; this helps avoid serious flickering
when the cursor turns off/on (e.g., when the user moves the mouse
pointer to a different screen).  That workaround was accidentally
dropped as we started working toward atomic watermark updates.  Since we
still aren't quite there yet with two-stage updates, we need to
resurrect the workaround and treat the cursor as always active.

Cc: simdev11@outlook.com
Cc: manfred.kitzbichler@gmail.com
Reported-by: simdev11@outlook.com
Reported-by: manfred.kitzbichler@gmail.com
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93892
Fixes: 43d59eda1 ("drm/i915: Eliminate usage of plane_wm_parameters from ILK-style WM code (v2)")
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
I'm traveling at the moment and don't have any HW access, so this patch is
completely untested, but I think it will solve the issue being reported.

 drivers/gpu/drm/i915/intel_pm.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 31bc4ea..a53c018 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1799,16 +1799,21 @@ static uint32_t ilk_compute_cur_wm(const struct intel_crtc_state *cstate,
 				   const struct intel_plane_state *pstate,
 				   uint32_t mem_value)
 {
-	int cpp = pstate->base.fb ?
-		drm_format_plane_cpp(pstate->base.fb->pixel_format, 0) : 0;
+	/*
+	 * We treat the cursor plane as always-on for the purposes of watermark
+	 * calculation.  Until we have two-stage watermark programming merged,
+	 * this is necessary to avoid flickering.
+	 */
+	int cpp = 4;
+	int width = drm_rect_width(&pstate->dst) ?: 64;
 
-	if (!cstate->base.active || !pstate->visible)
+	if (!cstate->base.active)
 		return 0;
 
+
 	return ilk_wm_method2(ilk_pipe_pixel_rate(cstate),
 			      cstate->base.adjusted_mode.crtc_htotal,
-			      drm_rect_width(&pstate->dst),
-			      cpp, mem_value);
+			      width, cpp, mem_value);
 }
 
 /* Only for WM_LP. */
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* ✗ Fi.CI.BAT: failure for drm/i915: Pretend cursor is always on for ILK-style WM calculations
  2016-02-01  9:26 [PATCH] drm/i915: Pretend cursor is always on for ILK-style WM calculations Matt Roper
@ 2016-02-01  9:47 ` Patchwork
  2016-02-01 14:22 ` [PATCH] " Ville Syrjälä
  1 sibling, 0 replies; 14+ messages in thread
From: Patchwork @ 2016-02-01  9:47 UTC (permalink / raw)
  To: Matt Roper; +Cc: intel-gfx

== Summary ==

Series 2975v1 drm/i915: Pretend cursor is always on for ILK-style WM calculations
http://patchwork.freedesktop.org/api/1.0/series/2975/revisions/1/mbox/

Test drv_module_reload_basic:
                pass       -> DMESG-WARN (ilk-hp8440p)
Test kms_flip:
        Subgroup basic-flip-vs-dpms:
                pass       -> DMESG-WARN (ilk-hp8440p) UNSTABLE
        Subgroup basic-plain-flip:
                pass       -> DMESG-WARN (ilk-hp8440p)
Test kms_pipe_crc_basic:
        Subgroup nonblocking-crc-pipe-a:
                pass       -> DMESG-WARN (ilk-hp8440p)
        Subgroup nonblocking-crc-pipe-a-frame-sequence:
                pass       -> DMESG-WARN (ilk-hp8440p)
        Subgroup nonblocking-crc-pipe-b:
                pass       -> DMESG-WARN (ilk-hp8440p)
        Subgroup nonblocking-crc-pipe-b-frame-sequence:
                pass       -> DMESG-WARN (ilk-hp8440p)
        Subgroup read-crc-pipe-b-frame-sequence:
                pass       -> DMESG-WARN (ilk-hp8440p)
        Subgroup suspend-read-crc-pipe-a:
                pass       -> FAIL       (byt-nuc)
        Subgroup suspend-read-crc-pipe-b:
                pass       -> DMESG-WARN (ilk-hp8440p)
                pass       -> FAIL       (byt-nuc)
        Subgroup suspend-read-crc-pipe-c:
                dmesg-warn -> PASS       (bsw-nuc-2)
                skip       -> FAIL       (byt-nuc)

bdw-nuci7        total:156  pass:147  dwarn:0   dfail:0   fail:0   skip:9  
bdw-ultra        total:159  pass:147  dwarn:0   dfail:0   fail:0   skip:12 
bsw-nuc-2        total:159  pass:129  dwarn:0   dfail:0   fail:0   skip:30 
byt-nuc          total:159  pass:134  dwarn:0   dfail:0   fail:3   skip:22 
hsw-brixbox      total:159  pass:146  dwarn:0   dfail:0   fail:0   skip:13 
ilk-hp8440p      total:159  pass:102  dwarn:9   dfail:0   fail:0   skip:48 
ivb-t430s        total:159  pass:145  dwarn:0   dfail:0   fail:0   skip:14 
skl-i5k-2        total:159  pass:144  dwarn:1   dfail:0   fail:0   skip:14 
snb-dellxps      total:159  pass:137  dwarn:0   dfail:0   fail:0   skip:22 
snb-x220t        total:159  pass:137  dwarn:0   dfail:0   fail:1   skip:21 

Results at /archive/results/CI_IGT_test/Patchwork_1333/

6b1049b84dcd979f631d15b2ada325d8e5b2c4e1 drm-intel-nightly: 2016y-01m-29d-22h-50m-57s UTC integration manifest
d2803db19c01d0f2f48793263da10ef0be3deebd drm/i915: Pretend cursor is always on for ILK-style WM calculations

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] drm/i915: Pretend cursor is always on for ILK-style WM calculations
  2016-02-01  9:26 [PATCH] drm/i915: Pretend cursor is always on for ILK-style WM calculations Matt Roper
  2016-02-01  9:47 ` ✗ Fi.CI.BAT: failure for " Patchwork
@ 2016-02-01 14:22 ` Ville Syrjälä
  2016-02-01 14:43   ` Manfred G Kitzbichler
  2016-02-02  3:31   ` Matt Roper
  1 sibling, 2 replies; 14+ messages in thread
From: Ville Syrjälä @ 2016-02-01 14:22 UTC (permalink / raw)
  To: Matt Roper; +Cc: simdev11, intel-gfx, manfred.kitzbichler

On Mon, Feb 01, 2016 at 01:26:14AM -0800, Matt Roper wrote:
> Due to our lack of two-step watermark programming, our driver has
> historically pretended that the cursor plane is always on for the
> purpose of watermark calculations; this helps avoid serious flickering
> when the cursor turns off/on (e.g., when the user moves the mouse
> pointer to a different screen).  That workaround was accidentally
> dropped as we started working toward atomic watermark updates.  Since we
> still aren't quite there yet with two-stage updates, we need to
> resurrect the workaround and treat the cursor as always active.
> 
> Cc: simdev11@outlook.com
> Cc: manfred.kitzbichler@gmail.com
> Reported-by: simdev11@outlook.com
> Reported-by: manfred.kitzbichler@gmail.com
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93892
> Fixes: 43d59eda1 ("drm/i915: Eliminate usage of plane_wm_parameters from ILK-style WM code (v2)")
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
> I'm traveling at the moment and don't have any HW access, so this patch is
> completely untested, but I think it will solve the issue being reported.
> 
>  drivers/gpu/drm/i915/intel_pm.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 31bc4ea..a53c018 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -1799,16 +1799,21 @@ static uint32_t ilk_compute_cur_wm(const struct intel_crtc_state *cstate,
>  				   const struct intel_plane_state *pstate,
>  				   uint32_t mem_value)
>  {
> -	int cpp = pstate->base.fb ?
> -		drm_format_plane_cpp(pstate->base.fb->pixel_format, 0) : 0;
> +	/*
> +	 * We treat the cursor plane as always-on for the purposes of watermark
> +	 * calculation.  Until we have two-stage watermark programming merged,
> +	 * this is necessary to avoid flickering.
> +	 */
> +	int cpp = 4;
> +	int width = drm_rect_width(&pstate->dst) ?: 64;

I don't think we can rely on that being correct when the plane is not
visible. Also for the cursor, I'm not sure we should be even using the
visible width of the plane when it's partially visible.

>  
> -	if (!cstate->base.active || !pstate->visible)
> +	if (!cstate->base.active)
>  		return 0;
>  
> +
>  	return ilk_wm_method2(ilk_pipe_pixel_rate(cstate),
>  			      cstate->base.adjusted_mode.crtc_htotal,
> -			      drm_rect_width(&pstate->dst),
> -			      cpp, mem_value);
> +			      width, cpp, mem_value);
>  }
>  
>  /* Only for WM_LP. */
> -- 
> 2.1.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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] drm/i915: Pretend cursor is always on for ILK-style WM calculations
  2016-02-01 14:22 ` [PATCH] " Ville Syrjälä
@ 2016-02-01 14:43   ` Manfred G Kitzbichler
  2016-02-02  3:31   ` Matt Roper
  1 sibling, 0 replies; 14+ messages in thread
From: Manfred G Kitzbichler @ 2016-02-01 14:43 UTC (permalink / raw)
  To: Ville Syrjälä, Matt Roper; +Cc: simdev11, intel-gfx

I can confirm that the proposed patch fixes the flicker problem on my 
system (openSuse Factory kernel 4.4.0-2-default), even though I had to 
slightly modify it to apply it to the current openSuse Factory sources).



On 02/01/2016 02:22 PM, Ville Syrjälä wrote:
> On Mon, Feb 01, 2016 at 01:26:14AM -0800, Matt Roper wrote:
>> Due to our lack of two-step watermark programming, our driver has
>> historically pretended that the cursor plane is always on for the
>> purpose of watermark calculations; this helps avoid serious flickering
>> when the cursor turns off/on (e.g., when the user moves the mouse
>> pointer to a different screen).  That workaround was accidentally
>> dropped as we started working toward atomic watermark updates.  Since we
>> still aren't quite there yet with two-stage updates, we need to
>> resurrect the workaround and treat the cursor as always active.
>>
>> Cc: simdev11@outlook.com
>> Cc: manfred.kitzbichler@gmail.com
>> Reported-by: simdev11@outlook.com
>> Reported-by: manfred.kitzbichler@gmail.com
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93892
>> Fixes: 43d59eda1 ("drm/i915: Eliminate usage of plane_wm_parameters from ILK-style WM code (v2)")
>> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
>> ---
>> I'm traveling at the moment and don't have any HW access, so this patch is
>> completely untested, but I think it will solve the issue being reported.
>>
>>   drivers/gpu/drm/i915/intel_pm.c | 15 ++++++++++-----
>>   1 file changed, 10 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>> index 31bc4ea..a53c018 100644
>> --- a/drivers/gpu/drm/i915/intel_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>> @@ -1799,16 +1799,21 @@ static uint32_t ilk_compute_cur_wm(const struct intel_crtc_state *cstate,
>>   				   const struct intel_plane_state *pstate,
>>   				   uint32_t mem_value)
>>   {
>> -	int cpp = pstate->base.fb ?
>> -		drm_format_plane_cpp(pstate->base.fb->pixel_format, 0) : 0;
>> +	/*
>> +	 * We treat the cursor plane as always-on for the purposes of watermark
>> +	 * calculation.  Until we have two-stage watermark programming merged,
>> +	 * this is necessary to avoid flickering.
>> +	 */
>> +	int cpp = 4;
>> +	int width = drm_rect_width(&pstate->dst) ?: 64;
> I don't think we can rely on that being correct when the plane is not
> visible. Also for the cursor, I'm not sure we should be even using the
> visible width of the plane when it's partially visible.
>
>>   
>> -	if (!cstate->base.active || !pstate->visible)
>> +	if (!cstate->base.active)
>>   		return 0;
>>   
>> +
>>   	return ilk_wm_method2(ilk_pipe_pixel_rate(cstate),
>>   			      cstate->base.adjusted_mode.crtc_htotal,
>> -			      drm_rect_width(&pstate->dst),
>> -			      cpp, mem_value);
>> +			      width, cpp, mem_value);
>>   }
>>   
>>   /* Only for WM_LP. */
>> -- 
>> 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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] drm/i915: Pretend cursor is always on for ILK-style WM calculations
  2016-02-01 14:22 ` [PATCH] " Ville Syrjälä
  2016-02-01 14:43   ` Manfred G Kitzbichler
@ 2016-02-02  3:31   ` Matt Roper
  2016-02-02 16:30     ` Ville Syrjälä
  2016-02-08 19:05     ` Matt Roper
  1 sibling, 2 replies; 14+ messages in thread
From: Matt Roper @ 2016-02-02  3:31 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: simdev11, intel-gfx, manfred.kitzbichler

On Mon, Feb 01, 2016 at 04:22:22PM +0200, Ville Syrjälä wrote:
> On Mon, Feb 01, 2016 at 01:26:14AM -0800, Matt Roper wrote:
> > Due to our lack of two-step watermark programming, our driver has
> > historically pretended that the cursor plane is always on for the
> > purpose of watermark calculations; this helps avoid serious flickering
> > when the cursor turns off/on (e.g., when the user moves the mouse
> > pointer to a different screen).  That workaround was accidentally
> > dropped as we started working toward atomic watermark updates.  Since we
> > still aren't quite there yet with two-stage updates, we need to
> > resurrect the workaround and treat the cursor as always active.
> > 
> > Cc: simdev11@outlook.com
> > Cc: manfred.kitzbichler@gmail.com
> > Reported-by: simdev11@outlook.com
> > Reported-by: manfred.kitzbichler@gmail.com
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93892
> > Fixes: 43d59eda1 ("drm/i915: Eliminate usage of plane_wm_parameters from ILK-style WM code (v2)")
> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> > ---
> > I'm traveling at the moment and don't have any HW access, so this patch is
> > completely untested, but I think it will solve the issue being reported.
> > 
> >  drivers/gpu/drm/i915/intel_pm.c | 15 ++++++++++-----
> >  1 file changed, 10 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index 31bc4ea..a53c018 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -1799,16 +1799,21 @@ static uint32_t ilk_compute_cur_wm(const struct intel_crtc_state *cstate,
> >  				   const struct intel_plane_state *pstate,
> >  				   uint32_t mem_value)
> >  {
> > -	int cpp = pstate->base.fb ?
> > -		drm_format_plane_cpp(pstate->base.fb->pixel_format, 0) : 0;
> > +	/*
> > +	 * We treat the cursor plane as always-on for the purposes of watermark
> > +	 * calculation.  Until we have two-stage watermark programming merged,
> > +	 * this is necessary to avoid flickering.
> > +	 */
> > +	int cpp = 4;
> > +	int width = drm_rect_width(&pstate->dst) ?: 64;
> 
> I don't think we can rely on that being correct when the plane is not
> visible. Also for the cursor, I'm not sure we should be even using the
> visible width of the plane when it's partially visible.
> 

But would this cause problems?  I thought higher values were always
"safe" (as long as they don't go over the maximums), just non-optimal
since they translate to the level of FIFO data at which we start
fetching more memory.  Acording to the bspec we don't even need to ever
program watermarks from the values setup by the BIOS if we're willing to
live with non-optimal power/memory bandwidth usage.  

The proper fix is definitely to re-merge the two-step watermark
programming, but we still haven't tracked down why that patch upset the
CI system yet (and I won't have time to look at that in depth until I'm
done traveling).  This seemed like a fairly safe/non-invasive temporary
workaround for the user-visible issues.  I'm open to other workarounds
though if you have an idea for something that will work better.

Thanks.


Matt

> >  
> > -	if (!cstate->base.active || !pstate->visible)
> > +	if (!cstate->base.active)
> >  		return 0;
> >  
> > +
> >  	return ilk_wm_method2(ilk_pipe_pixel_rate(cstate),
> >  			      cstate->base.adjusted_mode.crtc_htotal,
> > -			      drm_rect_width(&pstate->dst),
> > -			      cpp, mem_value);
> > +			      width, cpp, mem_value);
> >  }
> >  
> >  /* Only for WM_LP. */
> > -- 
> > 2.1.4
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC

-- 
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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] drm/i915: Pretend cursor is always on for ILK-style WM calculations
  2016-02-02  3:31   ` Matt Roper
@ 2016-02-02 16:30     ` Ville Syrjälä
  2016-02-03  6:06       ` [PATCH] drm/i915: Pretend cursor is always on for ILK-style WM calculations (v2) Matt Roper
  2016-02-08 19:05     ` Matt Roper
  1 sibling, 1 reply; 14+ messages in thread
From: Ville Syrjälä @ 2016-02-02 16:30 UTC (permalink / raw)
  To: Matt Roper; +Cc: simdev11, intel-gfx, manfred.kitzbichler

On Mon, Feb 01, 2016 at 07:31:47PM -0800, Matt Roper wrote:
> On Mon, Feb 01, 2016 at 04:22:22PM +0200, Ville Syrjälä wrote:
> > On Mon, Feb 01, 2016 at 01:26:14AM -0800, Matt Roper wrote:
> > > Due to our lack of two-step watermark programming, our driver has
> > > historically pretended that the cursor plane is always on for the
> > > purpose of watermark calculations; this helps avoid serious flickering
> > > when the cursor turns off/on (e.g., when the user moves the mouse
> > > pointer to a different screen).  That workaround was accidentally
> > > dropped as we started working toward atomic watermark updates.  Since we
> > > still aren't quite there yet with two-stage updates, we need to
> > > resurrect the workaround and treat the cursor as always active.
> > > 
> > > Cc: simdev11@outlook.com
> > > Cc: manfred.kitzbichler@gmail.com
> > > Reported-by: simdev11@outlook.com
> > > Reported-by: manfred.kitzbichler@gmail.com
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93892
> > > Fixes: 43d59eda1 ("drm/i915: Eliminate usage of plane_wm_parameters from ILK-style WM code (v2)")
> > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> > > ---
> > > I'm traveling at the moment and don't have any HW access, so this patch is
> > > completely untested, but I think it will solve the issue being reported.
> > > 
> > >  drivers/gpu/drm/i915/intel_pm.c | 15 ++++++++++-----
> > >  1 file changed, 10 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > > index 31bc4ea..a53c018 100644
> > > --- a/drivers/gpu/drm/i915/intel_pm.c
> > > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > > @@ -1799,16 +1799,21 @@ static uint32_t ilk_compute_cur_wm(const struct intel_crtc_state *cstate,
> > >  				   const struct intel_plane_state *pstate,
> > >  				   uint32_t mem_value)
> > >  {
> > > -	int cpp = pstate->base.fb ?
> > > -		drm_format_plane_cpp(pstate->base.fb->pixel_format, 0) : 0;
> > > +	/*
> > > +	 * We treat the cursor plane as always-on for the purposes of watermark
> > > +	 * calculation.  Until we have two-stage watermark programming merged,
> > > +	 * this is necessary to avoid flickering.
> > > +	 */
> > > +	int cpp = 4;
> > > +	int width = drm_rect_width(&pstate->dst) ?: 64;
> > 
> > I don't think we can rely on that being correct when the plane is not
> > visible. Also for the cursor, I'm not sure we should be even using the
> > visible width of the plane when it's partially visible.
> > 
> 
> But would this cause problems?  I thought higher values were always
> "safe" (as long as they don't go over the maximums), just non-optimal
> since they translate to the level of FIFO data at which we start
> fetching more memory.  Acording to the bspec we don't even need to ever
> program watermarks from the values setup by the BIOS if we're willing to
> live with non-optimal power/memory bandwidth usage.  

I'd probablty just make it
width = visible ? crtc_w : 64;
to make it close to where we were before.

> 
> The proper fix is definitely to re-merge the two-step watermark
> programming, but we still haven't tracked down why that patch upset the
> CI system yet (and I won't have time to look at that in depth until I'm
> done traveling).  This seemed like a fairly safe/non-invasive temporary
> workaround for the user-visible issues.  I'm open to other workarounds
> though if you have an idea for something that will work better.
> 
> Thanks.
> 
> 
> Matt
> 
> > >  
> > > -	if (!cstate->base.active || !pstate->visible)
> > > +	if (!cstate->base.active)
> > >  		return 0;
> > >  
> > > +
> > >  	return ilk_wm_method2(ilk_pipe_pixel_rate(cstate),
> > >  			      cstate->base.adjusted_mode.crtc_htotal,
> > > -			      drm_rect_width(&pstate->dst),
> > > -			      cpp, mem_value);
> > > +			      width, cpp, mem_value);
> > >  }
> > >  
> > >  /* Only for WM_LP. */
> > > -- 
> > > 2.1.4
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> > -- 
> > Ville Syrjälä
> > Intel OTC
> 
> -- 
> Matt Roper
> Graphics Software Engineer
> IoTG Platform Enabling & Development
> Intel Corporation
> (916) 356-2795

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH] drm/i915: Pretend cursor is always on for ILK-style WM calculations (v2)
  2016-02-02 16:30     ` Ville Syrjälä
@ 2016-02-03  6:06       ` Matt Roper
  2016-02-03 11:16         ` Ville Syrjälä
  0 siblings, 1 reply; 14+ messages in thread
From: Matt Roper @ 2016-02-03  6:06 UTC (permalink / raw)
  To: intel-gfx; +Cc: simdev11, manfred.kitzbichler

Due to our lack of two-step watermark programming, our driver has
historically pretended that the cursor plane is always on for the
purpose of watermark calculations; this helps avoid serious flickering
when the cursor turns off/on (e.g., when the user moves the mouse
pointer to a different screen).  That workaround was accidentally
dropped as we started working toward atomic watermark updates.  Since we
still aren't quite there yet with two-stage updates, we need to
resurrect the workaround and treat the cursor as always active.

v2: Tweak cursor width calculations slightly to more closely match the
    logic we used before the atomic overhaul began.  (Ville)

Cc: simdev11@outlook.com
Cc: manfred.kitzbichler@gmail.com
Reported-by: simdev11@outlook.com
Reported-by: manfred.kitzbichler@gmail.com
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93892
Fixes: 43d59eda1 ("drm/i915: Eliminate usage of plane_wm_parameters from ILK-style WM code (v2)")
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 31bc4ea..5888632 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1799,16 +1799,21 @@ static uint32_t ilk_compute_cur_wm(const struct intel_crtc_state *cstate,
 				   const struct intel_plane_state *pstate,
 				   uint32_t mem_value)
 {
-	int cpp = pstate->base.fb ?
-		drm_format_plane_cpp(pstate->base.fb->pixel_format, 0) : 0;
+	/*
+	 * We treat the cursor plane as always-on for the purposes of watermark
+	 * calculation.  Until we have two-stage watermark programming merged,
+	 * this is necessary to avoid flickering.
+	 */
+	int cpp = 4;
+	int width = pstate->visible ? pstate->base.crtc_w : 64;
 
-	if (!cstate->base.active || !pstate->visible)
+	if (!cstate->base.active)
 		return 0;
 
+
 	return ilk_wm_method2(ilk_pipe_pixel_rate(cstate),
 			      cstate->base.adjusted_mode.crtc_htotal,
-			      drm_rect_width(&pstate->dst),
-			      cpp, mem_value);
+			      width, cpp, mem_value);
 }
 
 /* Only for WM_LP. */
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH] drm/i915: Pretend cursor is always on for ILK-style WM calculations (v2)
  2016-02-03  6:06       ` [PATCH] drm/i915: Pretend cursor is always on for ILK-style WM calculations (v2) Matt Roper
@ 2016-02-03 11:16         ` Ville Syrjälä
  2016-02-03 14:05           ` Matt Roper
  0 siblings, 1 reply; 14+ messages in thread
From: Ville Syrjälä @ 2016-02-03 11:16 UTC (permalink / raw)
  To: Matt Roper; +Cc: simdev11, intel-gfx, manfred.kitzbichler

On Tue, Feb 02, 2016 at 10:06:51PM -0800, Matt Roper wrote:
> Due to our lack of two-step watermark programming, our driver has
> historically pretended that the cursor plane is always on for the
> purpose of watermark calculations; this helps avoid serious flickering
> when the cursor turns off/on (e.g., when the user moves the mouse
> pointer to a different screen).  That workaround was accidentally
> dropped as we started working toward atomic watermark updates.  Since we
> still aren't quite there yet with two-stage updates, we need to
> resurrect the workaround and treat the cursor as always active.
> 
> v2: Tweak cursor width calculations slightly to more closely match the
>     logic we used before the atomic overhaul began.  (Ville)
> 
> Cc: simdev11@outlook.com
> Cc: manfred.kitzbichler@gmail.com
> Reported-by: simdev11@outlook.com
> Reported-by: manfred.kitzbichler@gmail.com
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93892
> Fixes: 43d59eda1 ("drm/i915: Eliminate usage of plane_wm_parameters from ILK-style WM code (v2)")
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 31bc4ea..5888632 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -1799,16 +1799,21 @@ static uint32_t ilk_compute_cur_wm(const struct intel_crtc_state *cstate,
>  				   const struct intel_plane_state *pstate,
>  				   uint32_t mem_value)
>  {
> -	int cpp = pstate->base.fb ?
> -		drm_format_plane_cpp(pstate->base.fb->pixel_format, 0) : 0;
> +	/*
> +	 * We treat the cursor plane as always-on for the purposes of watermark
> +	 * calculation.  Until we have two-stage watermark programming merged,
> +	 * this is necessary to avoid flickering.
> +	 */
> +	int cpp = 4;
> +	int width = pstate->visible ? pstate->base.crtc_w : 64;
>  
> -	if (!cstate->base.active || !pstate->visible)
> +	if (!cstate->base.active)
>  		return 0;
>  
> +

Stray whitespace.

Otherwise lgtm
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  	return ilk_wm_method2(ilk_pipe_pixel_rate(cstate),
>  			      cstate->base.adjusted_mode.crtc_htotal,
> -			      drm_rect_width(&pstate->dst),
> -			      cpp, mem_value);
> +			      width, cpp, mem_value);
>  }
>  
>  /* Only for WM_LP. */
> -- 
> 2.1.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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] drm/i915: Pretend cursor is always on for ILK-style WM calculations (v2)
  2016-02-03 11:16         ` Ville Syrjälä
@ 2016-02-03 14:05           ` Matt Roper
  2016-02-11  8:50             ` Daniel Vetter
  0 siblings, 1 reply; 14+ messages in thread
From: Matt Roper @ 2016-02-03 14:05 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: simdev11, intel-gfx, manfred.kitzbichler

On Wed, Feb 03, 2016 at 01:16:38PM +0200, Ville Syrjälä wrote:
> On Tue, Feb 02, 2016 at 10:06:51PM -0800, Matt Roper wrote:
> > Due to our lack of two-step watermark programming, our driver has
> > historically pretended that the cursor plane is always on for the
> > purpose of watermark calculations; this helps avoid serious flickering
> > when the cursor turns off/on (e.g., when the user moves the mouse
> > pointer to a different screen).  That workaround was accidentally
> > dropped as we started working toward atomic watermark updates.  Since we
> > still aren't quite there yet with two-stage updates, we need to
> > resurrect the workaround and treat the cursor as always active.
> > 
> > v2: Tweak cursor width calculations slightly to more closely match the
> >     logic we used before the atomic overhaul began.  (Ville)
> > 
> > Cc: simdev11@outlook.com
> > Cc: manfred.kitzbichler@gmail.com
> > Reported-by: simdev11@outlook.com
> > Reported-by: manfred.kitzbichler@gmail.com
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93892
> > Fixes: 43d59eda1 ("drm/i915: Eliminate usage of plane_wm_parameters from ILK-style WM code (v2)")
> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_pm.c | 15 ++++++++++-----
> >  1 file changed, 10 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index 31bc4ea..5888632 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -1799,16 +1799,21 @@ static uint32_t ilk_compute_cur_wm(const struct intel_crtc_state *cstate,
> >  				   const struct intel_plane_state *pstate,
> >  				   uint32_t mem_value)
> >  {
> > -	int cpp = pstate->base.fb ?
> > -		drm_format_plane_cpp(pstate->base.fb->pixel_format, 0) : 0;
> > +	/*
> > +	 * We treat the cursor plane as always-on for the purposes of watermark
> > +	 * calculation.  Until we have two-stage watermark programming merged,
> > +	 * this is necessary to avoid flickering.
> > +	 */
> > +	int cpp = 4;
> > +	int width = pstate->visible ? pstate->base.crtc_w : 64;
> >  
> > -	if (!cstate->base.active || !pstate->visible)
> > +	if (!cstate->base.active)
> >  		return 0;
> >  
> > +
> 
> Stray whitespace.
> 
> Otherwise lgtm
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Whitespace fixed and pushed to dinq.  Also added a Cc: for -fixes since
this is relevant to 4.5.

Thanks for the review.


Matt

> 
> >  	return ilk_wm_method2(ilk_pipe_pixel_rate(cstate),
> >  			      cstate->base.adjusted_mode.crtc_htotal,
> > -			      drm_rect_width(&pstate->dst),
> > -			      cpp, mem_value);
> > +			      width, cpp, mem_value);
> >  }
> >  
> >  /* Only for WM_LP. */
> > -- 
> > 2.1.4
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC

-- 
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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH] drm/i915: Pretend cursor is always on for ILK-style WM calculations (v2)
  2016-02-02  3:31   ` Matt Roper
  2016-02-02 16:30     ` Ville Syrjälä
@ 2016-02-08 19:05     ` Matt Roper
  2016-02-09  9:36       ` Jani Nikula
  1 sibling, 1 reply; 14+ messages in thread
From: Matt Roper @ 2016-02-08 19:05 UTC (permalink / raw)
  To: intel-gfx; +Cc: drm-intel-fixes

Due to our lack of two-step watermark programming, our driver has
historically pretended that the cursor plane is always on for the
purpose of watermark calculations; this helps avoid serious flickering
when the cursor turns off/on (e.g., when the user moves the mouse
pointer to a different screen).  That workaround was accidentally
dropped as we started working toward atomic watermark updates.  Since we
still aren't quite there yet with two-stage updates, we need to
resurrect the workaround and treat the cursor as always active.

v2: Tweak cursor width calculations slightly to more closely match the
    logic we used before the atomic overhaul began.  (Ville)

Cc: simdev11@outlook.com
Cc: manfred.kitzbichler@gmail.com
Cc: drm-intel-fixes@lists.freedesktop.org
Reported-by: simdev11@outlook.com
Reported-by: manfred.kitzbichler@gmail.com
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93892
Fixes: 43d59eda1 ("drm/i915: Eliminate usage of plane_wm_parameters from ILK-style WM code (v2)")
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1454479611-6804-1-git-send-email-matthew.d.roper@intel.com
(cherry picked from commit b2435692dbb709d4c8ff3b2f2815c9b8423b72bb)
---
This patch is already merged to dinq, but did not cherry-pick cleanly to -fixes
due to dependence on a separate s/bpp/cpp/ standardization patch.  Here's a
version of the patch that should apply to -fixes cleanly.

 drivers/gpu/drm/i915/intel_pm.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index eb5fa05..a234687 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1783,16 +1783,20 @@ static uint32_t ilk_compute_cur_wm(const struct intel_crtc_state *cstate,
 				   const struct intel_plane_state *pstate,
 				   uint32_t mem_value)
 {
-	int bpp = pstate->base.fb ? pstate->base.fb->bits_per_pixel / 8 : 0;
+	/*
+	 * We treat the cursor plane as always-on for the purposes of watermark
+	 * calculation.  Until we have two-stage watermark programming merged,
+	 * this is necessary to avoid flickering.
+	 */
+	int cpp = 4;
+	int width = pstate->visible ? pstate->base.crtc_w : 64;
 
-	if (!cstate->base.active || !pstate->visible)
+	if (!cstate->base.active)
 		return 0;
 
 	return ilk_wm_method2(ilk_pipe_pixel_rate(cstate),
 			      cstate->base.adjusted_mode.crtc_htotal,
-			      drm_rect_width(&pstate->dst),
-			      bpp,
-			      mem_value);
+			      width, cpp, mem_value);
 }
 
 /* Only for WM_LP. */
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH] drm/i915: Pretend cursor is always on for ILK-style WM calculations (v2)
  2016-02-08 19:05     ` Matt Roper
@ 2016-02-09  9:36       ` Jani Nikula
  0 siblings, 0 replies; 14+ messages in thread
From: Jani Nikula @ 2016-02-09  9:36 UTC (permalink / raw)
  To: Matt Roper, intel-gfx; +Cc: drm-intel-fixes

On Mon, 08 Feb 2016, Matt Roper <matthew.d.roper@intel.com> wrote:
> Due to our lack of two-step watermark programming, our driver has
> historically pretended that the cursor plane is always on for the
> purpose of watermark calculations; this helps avoid serious flickering
> when the cursor turns off/on (e.g., when the user moves the mouse
> pointer to a different screen).  That workaround was accidentally
> dropped as we started working toward atomic watermark updates.  Since we
> still aren't quite there yet with two-stage updates, we need to
> resurrect the workaround and treat the cursor as always active.
>
> v2: Tweak cursor width calculations slightly to more closely match the
>     logic we used before the atomic overhaul began.  (Ville)
>
> Cc: simdev11@outlook.com
> Cc: manfred.kitzbichler@gmail.com
> Cc: drm-intel-fixes@lists.freedesktop.org
> Reported-by: simdev11@outlook.com
> Reported-by: manfred.kitzbichler@gmail.com
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93892
> Fixes: 43d59eda1 ("drm/i915: Eliminate usage of plane_wm_parameters from ILK-style WM code (v2)")
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> Link: http://patchwork.freedesktop.org/patch/msgid/1454479611-6804-1-git-send-email-matthew.d.roper@intel.com
> (cherry picked from commit b2435692dbb709d4c8ff3b2f2815c9b8423b72bb)
> ---
> This patch is already merged to dinq, but did not cherry-pick cleanly to -fixes
> due to dependence on a separate s/bpp/cpp/ standardization patch.  Here's a
> version of the patch that should apply to -fixes cleanly.

Pushed to drm-intel-fixes, thanks for the patch.

BR,
Jani.

>
>  drivers/gpu/drm/i915/intel_pm.c | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index eb5fa05..a234687 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -1783,16 +1783,20 @@ static uint32_t ilk_compute_cur_wm(const struct intel_crtc_state *cstate,
>  				   const struct intel_plane_state *pstate,
>  				   uint32_t mem_value)
>  {
> -	int bpp = pstate->base.fb ? pstate->base.fb->bits_per_pixel / 8 : 0;
> +	/*
> +	 * We treat the cursor plane as always-on for the purposes of watermark
> +	 * calculation.  Until we have two-stage watermark programming merged,
> +	 * this is necessary to avoid flickering.
> +	 */
> +	int cpp = 4;
> +	int width = pstate->visible ? pstate->base.crtc_w : 64;
>  
> -	if (!cstate->base.active || !pstate->visible)
> +	if (!cstate->base.active)
>  		return 0;
>  
>  	return ilk_wm_method2(ilk_pipe_pixel_rate(cstate),
>  			      cstate->base.adjusted_mode.crtc_htotal,
> -			      drm_rect_width(&pstate->dst),
> -			      bpp,
> -			      mem_value);
> +			      width, cpp, mem_value);
>  }
>  
>  /* Only for WM_LP. */

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] drm/i915: Pretend cursor is always on for ILK-style WM calculations (v2)
  2016-02-03 14:05           ` Matt Roper
@ 2016-02-11  8:50             ` Daniel Vetter
  2016-02-11 15:12               ` Matt Roper
  0 siblings, 1 reply; 14+ messages in thread
From: Daniel Vetter @ 2016-02-11  8:50 UTC (permalink / raw)
  To: Matt Roper; +Cc: simdev11, intel-gfx, manfred.kitzbichler

On Wed, Feb 03, 2016 at 06:05:01AM -0800, Matt Roper wrote:
> On Wed, Feb 03, 2016 at 01:16:38PM +0200, Ville Syrjälä wrote:
> > On Tue, Feb 02, 2016 at 10:06:51PM -0800, Matt Roper wrote:
> > > Due to our lack of two-step watermark programming, our driver has
> > > historically pretended that the cursor plane is always on for the
> > > purpose of watermark calculations; this helps avoid serious flickering
> > > when the cursor turns off/on (e.g., when the user moves the mouse
> > > pointer to a different screen).  That workaround was accidentally
> > > dropped as we started working toward atomic watermark updates.  Since we
> > > still aren't quite there yet with two-stage updates, we need to
> > > resurrect the workaround and treat the cursor as always active.
> > > 
> > > v2: Tweak cursor width calculations slightly to more closely match the
> > >     logic we used before the atomic overhaul began.  (Ville)
> > > 
> > > Cc: simdev11@outlook.com
> > > Cc: manfred.kitzbichler@gmail.com
> > > Reported-by: simdev11@outlook.com
> > > Reported-by: manfred.kitzbichler@gmail.com
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93892
> > > Fixes: 43d59eda1 ("drm/i915: Eliminate usage of plane_wm_parameters from ILK-style WM code (v2)")
> > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_pm.c | 15 ++++++++++-----
> > >  1 file changed, 10 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > > index 31bc4ea..5888632 100644
> > > --- a/drivers/gpu/drm/i915/intel_pm.c
> > > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > > @@ -1799,16 +1799,21 @@ static uint32_t ilk_compute_cur_wm(const struct intel_crtc_state *cstate,
> > >  				   const struct intel_plane_state *pstate,
> > >  				   uint32_t mem_value)
> > >  {
> > > -	int cpp = pstate->base.fb ?
> > > -		drm_format_plane_cpp(pstate->base.fb->pixel_format, 0) : 0;
> > > +	/*
> > > +	 * We treat the cursor plane as always-on for the purposes of watermark
> > > +	 * calculation.  Until we have two-stage watermark programming merged,
> > > +	 * this is necessary to avoid flickering.
> > > +	 */
> > > +	int cpp = 4;
> > > +	int width = pstate->visible ? pstate->base.crtc_w : 64;
> > >  
> > > -	if (!cstate->base.active || !pstate->visible)
> > > +	if (!cstate->base.active)
> > >  		return 0;
> > >  
> > > +
> > 
> > Stray whitespace.
> > 
> > Otherwise lgtm
> > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Whitespace fixed and pushed to dinq.  Also added a Cc: for -fixes since
> this is relevant to 4.5.
> 
> Thanks for the review.

Hm, I don't see the CI result for v2 of this patch anywhere. Is my mailer
lossy?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] drm/i915: Pretend cursor is always on for ILK-style WM calculations (v2)
  2016-02-11  8:50             ` Daniel Vetter
@ 2016-02-11 15:12               ` Matt Roper
  0 siblings, 0 replies; 14+ messages in thread
From: Matt Roper @ 2016-02-11 15:12 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

On Thu, Feb 11, 2016 at 09:50:45AM +0100, Daniel Vetter wrote:
> On Wed, Feb 03, 2016 at 06:05:01AM -0800, Matt Roper wrote:
> > On Wed, Feb 03, 2016 at 01:16:38PM +0200, Ville Syrjälä wrote:
> > > On Tue, Feb 02, 2016 at 10:06:51PM -0800, Matt Roper wrote:
> > > > Due to our lack of two-step watermark programming, our driver has
> > > > historically pretended that the cursor plane is always on for the
> > > > purpose of watermark calculations; this helps avoid serious flickering
> > > > when the cursor turns off/on (e.g., when the user moves the mouse
> > > > pointer to a different screen).  That workaround was accidentally
> > > > dropped as we started working toward atomic watermark updates.  Since we
> > > > still aren't quite there yet with two-stage updates, we need to
> > > > resurrect the workaround and treat the cursor as always active.
> > > > 
> > > > v2: Tweak cursor width calculations slightly to more closely match the
> > > >     logic we used before the atomic overhaul began.  (Ville)
> > > > 
> > > > Cc: simdev11@outlook.com
> > > > Cc: manfred.kitzbichler@gmail.com
> > > > Reported-by: simdev11@outlook.com
> > > > Reported-by: manfred.kitzbichler@gmail.com
> > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93892
> > > > Fixes: 43d59eda1 ("drm/i915: Eliminate usage of plane_wm_parameters from ILK-style WM code (v2)")
> > > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_pm.c | 15 ++++++++++-----
> > > >  1 file changed, 10 insertions(+), 5 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > > > index 31bc4ea..5888632 100644
> > > > --- a/drivers/gpu/drm/i915/intel_pm.c
> > > > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > > > @@ -1799,16 +1799,21 @@ static uint32_t ilk_compute_cur_wm(const struct intel_crtc_state *cstate,
> > > >  				   const struct intel_plane_state *pstate,
> > > >  				   uint32_t mem_value)
> > > >  {
> > > > -	int cpp = pstate->base.fb ?
> > > > -		drm_format_plane_cpp(pstate->base.fb->pixel_format, 0) : 0;
> > > > +	/*
> > > > +	 * We treat the cursor plane as always-on for the purposes of watermark
> > > > +	 * calculation.  Until we have two-stage watermark programming merged,
> > > > +	 * this is necessary to avoid flickering.
> > > > +	 */
> > > > +	int cpp = 4;
> > > > +	int width = pstate->visible ? pstate->base.crtc_w : 64;
> > > >  
> > > > -	if (!cstate->base.active || !pstate->visible)
> > > > +	if (!cstate->base.active)
> > > >  		return 0;
> > > >  
> > > > +
> > > 
> > > Stray whitespace.
> > > 
> > > Otherwise lgtm
> > > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Whitespace fixed and pushed to dinq.  Also added a Cc: for -fixes since
> > this is relevant to 4.5.
> > 
> > Thanks for the review.
> 
> Hm, I don't see the CI result for v2 of this patch anywhere. Is my mailer
> lossy?
> -Daniel

Looks like they were here:

https://lists.freedesktop.org/archives/intel-gfx/2016-February/086953.html


Matt



> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH] drm/i915: Pretend cursor is always on for ILK-style WM calculations (v2)
@ 2016-02-03  6:54 Matt Roper
  0 siblings, 0 replies; 14+ messages in thread
From: Matt Roper @ 2016-02-03  6:54 UTC (permalink / raw)
  To: intel-gfx; +Cc: simdev11, manfred.kitzbichler

Due to our lack of two-step watermark programming, our driver has
historically pretended that the cursor plane is always on for the
purpose of watermark calculations; this helps avoid serious flickering
when the cursor turns off/on (e.g., when the user moves the mouse
pointer to a different screen).  That workaround was accidentally
dropped as we started working toward atomic watermark updates.  Since we
still aren't quite there yet with two-stage updates, we need to
resurrect the workaround and treat the cursor as always active.

v2: Tweak cursor width calculations slightly to more closely match the
    logic we used before the atomic overhaul began.  (Ville)

Cc: simdev11@outlook.com
Cc: manfred.kitzbichler@gmail.com
Reported-by: simdev11@outlook.com
Reported-by: manfred.kitzbichler@gmail.com
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93892
Fixes: 43d59eda1 ("drm/i915: Eliminate usage of plane_wm_parameters from ILK-style WM code (v2)")
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 31bc4ea..5888632 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1799,16 +1799,21 @@ static uint32_t ilk_compute_cur_wm(const struct intel_crtc_state *cstate,
 				   const struct intel_plane_state *pstate,
 				   uint32_t mem_value)
 {
-	int cpp = pstate->base.fb ?
-		drm_format_plane_cpp(pstate->base.fb->pixel_format, 0) : 0;
+	/*
+	 * We treat the cursor plane as always-on for the purposes of watermark
+	 * calculation.  Until we have two-stage watermark programming merged,
+	 * this is necessary to avoid flickering.
+	 */
+	int cpp = 4;
+	int width = pstate->visible ? pstate->base.crtc_w : 64;
 
-	if (!cstate->base.active || !pstate->visible)
+	if (!cstate->base.active)
 		return 0;
 
+
 	return ilk_wm_method2(ilk_pipe_pixel_rate(cstate),
 			      cstate->base.adjusted_mode.crtc_htotal,
-			      drm_rect_width(&pstate->dst),
-			      cpp, mem_value);
+			      width, cpp, mem_value);
 }
 
 /* Only for WM_LP. */
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2016-02-11 15:12 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-01  9:26 [PATCH] drm/i915: Pretend cursor is always on for ILK-style WM calculations Matt Roper
2016-02-01  9:47 ` ✗ Fi.CI.BAT: failure for " Patchwork
2016-02-01 14:22 ` [PATCH] " Ville Syrjälä
2016-02-01 14:43   ` Manfred G Kitzbichler
2016-02-02  3:31   ` Matt Roper
2016-02-02 16:30     ` Ville Syrjälä
2016-02-03  6:06       ` [PATCH] drm/i915: Pretend cursor is always on for ILK-style WM calculations (v2) Matt Roper
2016-02-03 11:16         ` Ville Syrjälä
2016-02-03 14:05           ` Matt Roper
2016-02-11  8:50             ` Daniel Vetter
2016-02-11 15:12               ` Matt Roper
2016-02-08 19:05     ` Matt Roper
2016-02-09  9:36       ` Jani Nikula
2016-02-03  6:54 Matt Roper

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.