diff mbox

drm/i915/gen9: Give one extra block per line for SKL plane WM calculations

Message ID 1470344880-27394-1-git-send-email-matthew.d.roper@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Matt Roper Aug. 4, 2016, 9:08 p.m. UTC
The bspec was updated a couple weeks ago to add an extra block per line
to plane watermark calculations for linear pixel formats.

Bspec update 115327 description:
  "Gen9+ - Updated the plane blocks per line calculation for linear
  cases. Adds +1 for all linear cases to handle the non-block aligned
  stride cases."

Cc: Lyude <cpaul@redhat.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

cpaul@redhat.com Aug. 4, 2016, 11:36 p.m. UTC | #1
Reviewed-by: Lyude <cpaul@redhat.com>

On Thu, 2016-08-04 at 14:08 -0700, Matt Roper wrote:
> The bspec was updated a couple weeks ago to add an extra block per
> line
> to plane watermark calculations for linear pixel formats.
> 
> Bspec update 115327 description:
>   "Gen9+ - Updated the plane blocks per line calculation for linear
>   cases. Adds +1 for all linear cases to handle the non-block aligned
>   stride cases."
> 
> Cc: Lyude <cpaul@redhat.com>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c
> b/drivers/gpu/drm/i915/intel_pm.c
> index 4317cdf..6bd352a 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3352,6 +3352,8 @@ static uint32_t skl_wm_method2(uint32_t
> pixel_rate, uint32_t pipe_htotal,
>  		plane_bytes_per_line *= 4;
>  		plane_blocks_per_line =
> DIV_ROUND_UP(plane_bytes_per_line, 512);
>  		plane_blocks_per_line /= 4;
> +	} else if (tiling == DRM_FORMAT_MOD_NONE) {
> +		plane_blocks_per_line =
> DIV_ROUND_UP(plane_bytes_per_line, 512) + 1;
>  	} else {
>  		plane_blocks_per_line =
> DIV_ROUND_UP(plane_bytes_per_line, 512);
>  	}
Matt Roper Aug. 4, 2016, 11:51 p.m. UTC | #2
On Thu, Aug 04, 2016 at 07:36:15PM -0400, Lyude wrote:
> Reviewed-by: Lyude <cpaul@redhat.com>

Merged to dinq.  Thanks for the quick review.


Matt

> 
> On Thu, 2016-08-04 at 14:08 -0700, Matt Roper wrote:
> > The bspec was updated a couple weeks ago to add an extra block per
> > line
> > to plane watermark calculations for linear pixel formats.
> > 
> > Bspec update 115327 description:
> >   "Gen9+ - Updated the plane blocks per line calculation for linear
> >   cases. Adds +1 for all linear cases to handle the non-block aligned
> >   stride cases."
> > 
> > Cc: Lyude <cpaul@redhat.com>
> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_pm.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c
> > b/drivers/gpu/drm/i915/intel_pm.c
> > index 4317cdf..6bd352a 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -3352,6 +3352,8 @@ static uint32_t skl_wm_method2(uint32_t
> > pixel_rate, uint32_t pipe_htotal,
> >  		plane_bytes_per_line *= 4;
> >  		plane_blocks_per_line =
> > DIV_ROUND_UP(plane_bytes_per_line, 512);
> >  		plane_blocks_per_line /= 4;
> > +	} else if (tiling == DRM_FORMAT_MOD_NONE) {
> > +		plane_blocks_per_line =
> > DIV_ROUND_UP(plane_bytes_per_line, 512) + 1;
> >  	} else {
> >  		plane_blocks_per_line =
> > DIV_ROUND_UP(plane_bytes_per_line, 512);
> >  	}
> -- 
> Cheers,
> 	Lyude
Zanoni, Paulo R Aug. 8, 2016, 6:25 p.m. UTC | #3
Em Qui, 2016-08-04 às 16:51 -0700, Matt Roper escreveu:
> On Thu, Aug 04, 2016 at 07:36:15PM -0400, Lyude wrote:

> > 

> > Reviewed-by: Lyude <cpaul@redhat.com>

> 

> Merged to dinq.  Thanks for the quick review.


Regression? This patch makes my SKL machine fail any modesets. I now
boot to a blinking screen where X keeps trying to start and fails.

Xorg.0.log gives me:
[   273.512] (EE) modeset(0): failed to set mode: Invalid argument


On the dmesg side, these are the more suspicious messages:

[  273.583659] [drm:skl_compute_plane_wm] Requested display
configuration exceeds system watermark limitations
[  273.583663] [drm:skl_compute_plane_wm] Plane 1.0: blocks required =
4/0, lines required = 1/31


I tried applying Lyude's series to nightly to see if it fixes
something, but it looks like patch 2 doesn't apply.

> 

> 

> Matt

> 

> > 

> > 

> > On Thu, 2016-08-04 at 14:08 -0700, Matt Roper wrote:

> > > 

> > > The bspec was updated a couple weeks ago to add an extra block

> > > per

> > > line

> > > to plane watermark calculations for linear pixel formats.

> > > 

> > > Bspec update 115327 description:

> > >   "Gen9+ - Updated the plane blocks per line calculation for

> > > linear

> > >   cases. Adds +1 for all linear cases to handle the non-block

> > > aligned

> > >   stride cases."

> > > 

> > > Cc: Lyude <cpaul@redhat.com>

> > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>

> > > ---

> > >  drivers/gpu/drm/i915/intel_pm.c | 2 ++

> > >  1 file changed, 2 insertions(+)

> > > 

> > > diff --git a/drivers/gpu/drm/i915/intel_pm.c

> > > b/drivers/gpu/drm/i915/intel_pm.c

> > > index 4317cdf..6bd352a 100644

> > > --- a/drivers/gpu/drm/i915/intel_pm.c

> > > +++ b/drivers/gpu/drm/i915/intel_pm.c

> > > @@ -3352,6 +3352,8 @@ static uint32_t skl_wm_method2(uint32_t

> > > pixel_rate, uint32_t pipe_htotal,

> > >  		plane_bytes_per_line *= 4;

> > >  		plane_blocks_per_line =

> > > DIV_ROUND_UP(plane_bytes_per_line, 512);

> > >  		plane_blocks_per_line /= 4;

> > > +	} else if (tiling == DRM_FORMAT_MOD_NONE) {

> > > +		plane_blocks_per_line =

> > > DIV_ROUND_UP(plane_bytes_per_line, 512) + 1;

> > >  	} else {

> > >  		plane_blocks_per_line =

> > > DIV_ROUND_UP(plane_bytes_per_line, 512);

> > >  	}

> > -- 

> > Cheers,

> > 	Lyude

>
Chris Wilson Aug. 8, 2016, 6:35 p.m. UTC | #4
On Mon, Aug 08, 2016 at 06:25:49PM +0000, Zanoni, Paulo R wrote:
> Em Qui, 2016-08-04 às 16:51 -0700, Matt Roper escreveu:
> > On Thu, Aug 04, 2016 at 07:36:15PM -0400, Lyude wrote:
> > > 
> > > Reviewed-by: Lyude <cpaul@redhat.com>
> > 
> > Merged to dinq.  Thanks for the quick review.
> 
> Regression? This patch makes my SKL machine fail any modesets. I now
> boot to a blinking screen where X keeps trying to start and fails.

-intel has been fixing up failed multi-CRTC modesets since seemingly
forever on skl, that fail due to WM being exceeded. And why would
modesetting even be trying to use a non-tiled framebuffer?
-Chris
Zanoni, Paulo R Aug. 8, 2016, 6:36 p.m. UTC | #5
Em Seg, 2016-08-08 às 18:25 +0000, Zanoni, Paulo R escreveu:
> Em Qui, 2016-08-04 às 16:51 -0700, Matt Roper escreveu:

> > 

> > On Thu, Aug 04, 2016 at 07:36:15PM -0400, Lyude wrote:

> > > 

> > > 

> > > Reviewed-by: Lyude <cpaul@redhat.com>

> > 

> > Merged to dinq.  Thanks for the quick review.

> 

> Regression? This patch makes my SKL machine fail any modesets. I now

> boot to a blinking screen where X keeps trying to start and fails.

> 

> Xorg.0.log gives me:

> [   273.512] (EE) modeset(0): failed to set mode: Invalid argument

> 

> 

> On the dmesg side, these are the more suspicious messages:

> 

> [  273.583659] [drm:skl_compute_plane_wm] Requested display

> configuration exceeds system watermark limitations

> [  273.583663] [drm:skl_compute_plane_wm] Plane 1.0: blocks required

> =

> 4/0, lines required = 1/31

> 

> 

> I tried applying Lyude's series to nightly to see if it fixes

> something, but it looks like patch 2 doesn't apply.


The patches do apply, I was confused by the email threading and mixed
patch 6 v9 with patch 2. Lyude's series fix the regression :).

> 

> > 

> > 

> > 

> > Matt

> > 

> > > 

> > > 

> > > 

> > > On Thu, 2016-08-04 at 14:08 -0700, Matt Roper wrote:

> > > > 

> > > > 

> > > > The bspec was updated a couple weeks ago to add an extra block

> > > > per

> > > > line

> > > > to plane watermark calculations for linear pixel formats.

> > > > 

> > > > Bspec update 115327 description:

> > > >   "Gen9+ - Updated the plane blocks per line calculation for

> > > > linear

> > > >   cases. Adds +1 for all linear cases to handle the non-block

> > > > aligned

> > > >   stride cases."

> > > > 

> > > > Cc: Lyude <cpaul@redhat.com>

> > > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>

> > > > ---

> > > >  drivers/gpu/drm/i915/intel_pm.c | 2 ++

> > > >  1 file changed, 2 insertions(+)

> > > > 

> > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c

> > > > b/drivers/gpu/drm/i915/intel_pm.c

> > > > index 4317cdf..6bd352a 100644

> > > > --- a/drivers/gpu/drm/i915/intel_pm.c

> > > > +++ b/drivers/gpu/drm/i915/intel_pm.c

> > > > @@ -3352,6 +3352,8 @@ static uint32_t skl_wm_method2(uint32_t

> > > > pixel_rate, uint32_t pipe_htotal,

> > > >  		plane_bytes_per_line *= 4;

> > > >  		plane_blocks_per_line =

> > > > DIV_ROUND_UP(plane_bytes_per_line, 512);

> > > >  		plane_blocks_per_line /= 4;

> > > > +	} else if (tiling == DRM_FORMAT_MOD_NONE) {

> > > > +		plane_blocks_per_line =

> > > > DIV_ROUND_UP(plane_bytes_per_line, 512) + 1;

> > > >  	} else {

> > > >  		plane_blocks_per_line =

> > > > DIV_ROUND_UP(plane_bytes_per_line, 512);

> > > >  	}

> > > -- 

> > > Cheers,

> > > 	Lyude

> > 

> _______________________________________________

> Intel-gfx mailing list

> Intel-gfx@lists.freedesktop.org

> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Zanoni, Paulo R Aug. 8, 2016, 7 p.m. UTC | #6
Em Seg, 2016-08-08 às 19:35 +0100, Chris Wilson escreveu:
> On Mon, Aug 08, 2016 at 06:25:49PM +0000, Zanoni, Paulo R wrote:

> > 

> > Em Qui, 2016-08-04 às 16:51 -0700, Matt Roper escreveu:

> > > 

> > > On Thu, Aug 04, 2016 at 07:36:15PM -0400, Lyude wrote:

> > > > 

> > > > 

> > > > Reviewed-by: Lyude <cpaul@redhat.com>

> > > 

> > > Merged to dinq.  Thanks for the quick review.

> > 

> > Regression? This patch makes my SKL machine fail any modesets. I

> > now

> > boot to a blinking screen where X keeps trying to start and fails.

> 

> -intel has been fixing up failed multi-CRTC modesets since seemingly

> forever on skl, that fail due to WM being exceeded. And why would

> modesetting even be trying to use a non-tiled framebuffer?


I'm just using my distro's default driver, and Debian uses modesetting
now.

I did switch to xf86-video-intel and I found something interesting: the
machine boots correctly, but then if I stop+restart lightdm, I get a
black screen. The difference here is that X doesn't abort, it tries to
keep working despite the black screen:

[    46.483] (EE) intel(0): failed to set mode: Invalid argument [22]
[    46.485] (II) intel(0): EDID vendor "SDC", prod id 16970
[    46.485] (II) intel(0): Printing DDC gathered Modelines:
[    46.485] (II) intel(0): Modeline "3200x1800"x0.0  361.31  3200 3248
3280 3316  1800 1802 1807 1816 -hsync -vsync (109.0 kHz eP)
[    46.485] (II) intel(0): Modeline "3200x1800"x0.0  361.31  3200 3248
3280 3680  1800 1802 1807 2045 -hsync -vsync (98.2 kHz e)
[    46.794] (--) intel(0): HDMI max TMDS frequency 225000KHz
[    46.969] (EE) intel(0): failed to set mode: Invalid argument [22]

And dmesg has the same message as when using xf86-video-modesetting:

[   46.928018] [drm:skl_compute_plane_wm] Requested display
configuration exceeds system watermark limitations
[   46.928021] [drm:skl_compute_plane_wm] Plane 1.0: blocks required =
4/0, lines required = 1/31

Notice that this is the distro's driver version:
2:2.99.917+git20160706-1

So it looks like switching back to xf86-video-intel won't be a perfect
fix.

Anyway, while using the DDX to work around Kernel bugs may have some
benefits, it's probably best to try to push for an appropriate Kernel
fix, especially now that xf86-video-modesetting is gaining some market
share...

> -Chris

>
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 4317cdf..6bd352a 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3352,6 +3352,8 @@  static uint32_t skl_wm_method2(uint32_t pixel_rate, uint32_t pipe_htotal,
 		plane_bytes_per_line *= 4;
 		plane_blocks_per_line = DIV_ROUND_UP(plane_bytes_per_line, 512);
 		plane_blocks_per_line /= 4;
+	} else if (tiling == DRM_FORMAT_MOD_NONE) {
+		plane_blocks_per_line = DIV_ROUND_UP(plane_bytes_per_line, 512) + 1;
 	} else {
 		plane_blocks_per_line = DIV_ROUND_UP(plane_bytes_per_line, 512);
 	}