All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915/psr: Update PSR2 resolution check for Cannonlake
@ 2018-03-01 20:38 Dhinakaran Pandiyan
  2018-03-01 20:47 ` Ville Syrjälä
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Dhinakaran Pandiyan @ 2018-03-01 20:38 UTC (permalink / raw)
  To: intel-gfx; +Cc: Dhinakaran Pandiyan, Rodrigo Vivi

In fact, apply the Cannonlake resolution check for all > Gen-9 platforms to
be safe.

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Elio Martinez Monroy <elio.martinez.monroy@intel.com>
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 drivers/gpu/drm/i915/intel_psr.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 05770790a4e9..2a2c696c4109 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -451,8 +451,8 @@ static bool intel_psr2_config_valid(struct intel_dp *intel_dp,
 {
 	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
 	struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
-	const struct drm_display_mode *adjusted_mode =
-		&crtc_state->base.adjusted_mode;
+	int crtc_h = crtc_state->base.adjusted_mode.crtc_hdisplay;
+	int crtc_v = crtc_state->base.adjusted_mode.crtc_vdisplay;
 
 	/*
 	 * FIXME psr2_support is messed up. It's both computed
@@ -462,9 +462,10 @@ static bool intel_psr2_config_valid(struct intel_dp *intel_dp,
 	if (!dev_priv->psr.psr2_support)
 		return false;
 
-	/* PSR2 is restricted to work with panel resolutions up to 3640x2304 */
-	if (adjusted_mode->crtc_hdisplay > 3640 ||
-	    adjusted_mode->crtc_vdisplay > 2304) {
+	if (crtc_h > 4096 || crtc_v > 2304) {
+		DRM_DEBUG_KMS("PSR2 not enabled, panel resolution too big\n");
+		return false;
+	} else if (IS_GEN9(dev_priv) && (crtc_h > 3640 || crtc_v > 2304)) {
 		DRM_DEBUG_KMS("PSR2 not enabled, panel resolution too big\n");
 		return false;
 	}
-- 
2.14.1

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

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

* Re: [PATCH] drm/i915/psr: Update PSR2 resolution check for Cannonlake
  2018-03-01 20:38 [PATCH] drm/i915/psr: Update PSR2 resolution check for Cannonlake Dhinakaran Pandiyan
@ 2018-03-01 20:47 ` Ville Syrjälä
  2018-03-01 20:50   ` Pandiyan, Dhinakaran
                     ` (2 more replies)
  2018-03-01 21:03 ` ✗ Fi.CI.BAT: failure for " Patchwork
                   ` (2 subsequent siblings)
  3 siblings, 3 replies; 11+ messages in thread
From: Ville Syrjälä @ 2018-03-01 20:47 UTC (permalink / raw)
  To: Dhinakaran Pandiyan; +Cc: intel-gfx, Rodrigo Vivi

On Thu, Mar 01, 2018 at 12:38:58PM -0800, Dhinakaran Pandiyan wrote:
> In fact, apply the Cannonlake resolution check for all > Gen-9 platforms to
> be safe.
> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Elio Martinez Monroy <elio.martinez.monroy@intel.com>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_psr.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 05770790a4e9..2a2c696c4109 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -451,8 +451,8 @@ static bool intel_psr2_config_valid(struct intel_dp *intel_dp,
>  {
>  	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
>  	struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
> -	const struct drm_display_mode *adjusted_mode =
> -		&crtc_state->base.adjusted_mode;
> +	int crtc_h = crtc_state->base.adjusted_mode.crtc_hdisplay;
> +	int crtc_v = crtc_state->base.adjusted_mode.crtc_vdisplay;
>  
>  	/*
>  	 * FIXME psr2_support is messed up. It's both computed
> @@ -462,9 +462,10 @@ static bool intel_psr2_config_valid(struct intel_dp *intel_dp,
>  	if (!dev_priv->psr.psr2_support)
>  		return false;
>  
> -	/* PSR2 is restricted to work with panel resolutions up to 3640x2304 */
> -	if (adjusted_mode->crtc_hdisplay > 3640 ||
> -	    adjusted_mode->crtc_vdisplay > 2304) {
> +	if (crtc_h > 4096 || crtc_v > 2304) {
> +		DRM_DEBUG_KMS("PSR2 not enabled, panel resolution too big\n");
> +		return false;
> +	} else if (IS_GEN9(dev_priv) && (crtc_h > 3640 || crtc_v > 2304)) {

I would suggest introducing max_w/h or something similar so that you
don't have to duplicate the actual code.

When debugging things from logs one might want to know both the requested
size and the max. So printing those out might be a good idea.

>  		DRM_DEBUG_KMS("PSR2 not enabled, panel resolution too big\n");
>  		return false;
>  	}
> -- 
> 2.14.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

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

* Re: [PATCH] drm/i915/psr: Update PSR2 resolution check for Cannonlake
  2018-03-01 20:47 ` Ville Syrjälä
@ 2018-03-01 20:50   ` Pandiyan, Dhinakaran
  2018-03-01 21:27   ` [PATCH v2] " Dhinakaran Pandiyan
  2018-03-01 22:09   ` [PATCH] " Rodrigo Vivi
  2 siblings, 0 replies; 11+ messages in thread
From: Pandiyan, Dhinakaran @ 2018-03-01 20:50 UTC (permalink / raw)
  To: ville.syrjala@linux.intel.com
  Cc: intel-gfx@lists.freedesktop.org, Vivi, Rodrigo

On Thu, 2018-03-01 at 22:47 +0200, Ville Syrjälä wrote:
> On Thu, Mar 01, 2018 at 12:38:58PM -0800, Dhinakaran Pandiyan wrote:
> > In fact, apply the Cannonlake resolution check for all > Gen-9 platforms to
> > be safe.
> > 
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Elio Martinez Monroy <elio.martinez.monroy@intel.com>
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_psr.c | 11 ++++++-----
> >  1 file changed, 6 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> > index 05770790a4e9..2a2c696c4109 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -451,8 +451,8 @@ static bool intel_psr2_config_valid(struct intel_dp *intel_dp,
> >  {
> >  	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> >  	struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
> > -	const struct drm_display_mode *adjusted_mode =
> > -		&crtc_state->base.adjusted_mode;
> > +	int crtc_h = crtc_state->base.adjusted_mode.crtc_hdisplay;
> > +	int crtc_v = crtc_state->base.adjusted_mode.crtc_vdisplay;
> >  
> >  	/*
> >  	 * FIXME psr2_support is messed up. It's both computed
> > @@ -462,9 +462,10 @@ static bool intel_psr2_config_valid(struct intel_dp *intel_dp,
> >  	if (!dev_priv->psr.psr2_support)
> >  		return false;
> >  
> > -	/* PSR2 is restricted to work with panel resolutions up to 3640x2304 */
> > -	if (adjusted_mode->crtc_hdisplay > 3640 ||
> > -	    adjusted_mode->crtc_vdisplay > 2304) {
> > +	if (crtc_h > 4096 || crtc_v > 2304) {
> > +		DRM_DEBUG_KMS("PSR2 not enabled, panel resolution too big\n");
> > +		return false;
> > +	} else if (IS_GEN9(dev_priv) && (crtc_h > 3640 || crtc_v > 2304)) {
> 
> I would suggest introducing max_w/h or something similar so that you
> don't have to duplicate the actual code.
> 
> When debugging things from logs one might want to know both the requested
> size and the max. So printing those out might be a good idea.

Agreed on both points.


> 
> >  		DRM_DEBUG_KMS("PSR2 not enabled, panel resolution too big\n");
> >  		return false;
> >  	}
> > -- 
> > 2.14.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* ✗ Fi.CI.BAT: failure for drm/i915/psr: Update PSR2 resolution check for Cannonlake
  2018-03-01 20:38 [PATCH] drm/i915/psr: Update PSR2 resolution check for Cannonlake Dhinakaran Pandiyan
  2018-03-01 20:47 ` Ville Syrjälä
@ 2018-03-01 21:03 ` Patchwork
  2018-03-01 22:08 ` ✓ Fi.CI.BAT: success for drm/i915/psr: Update PSR2 resolution check for Cannonlake (rev2) Patchwork
  2018-03-02  3:00 ` ✓ Fi.CI.IGT: " Patchwork
  3 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2018-03-01 21:03 UTC (permalink / raw)
  To: Pandiyan, Dhinakaran; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/psr: Update PSR2 resolution check for Cannonlake
URL   : https://patchwork.freedesktop.org/series/39238/
State : failure

== Summary ==

Series 39238v1 drm/i915/psr: Update PSR2 resolution check for Cannonlake
https://patchwork.freedesktop.org/api/1.0/series/39238/revisions/1/mbox/

---- Possible new issues:

Test gem_ctx_switch:
        Subgroup basic-default:
                pass       -> INCOMPLETE (fi-cnl-y3)

---- Known issues:

Test gem_mmap_gtt:
        Subgroup basic-small-bo-tiledx:
                pass       -> FAIL       (fi-gdg-551) fdo#102575

fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:416s
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:425s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:380s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:491s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:281s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:484s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:484s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:473s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:461s
fi-cfl-8700k     total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:392s
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:567s
fi-cnl-y3        total:21   pass:20   dwarn:0   dfail:0   fail:0   skip:0  
fi-elk-e7500     total:288  pass:229  dwarn:0   dfail:0   fail:0   skip:59  time:409s
fi-gdg-551       total:288  pass:179  dwarn:0   dfail:0   fail:1   skip:108 time:289s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:511s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:386s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:410s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:446s
fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:411s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:457s
fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:489s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:453s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:493s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:591s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:424s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:501s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:519s
fi-skl-6700k2    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:490s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:481s
fi-skl-guc       total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:411s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:430s
fi-snb-2520m     total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:522s
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:392s
Blacklisted hosts:
fi-cfl-u         total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:500s

7fe823aaeff7c50eeaf9b238a179b41771e08f9e drm-tip: 2018y-03m-01d-15h-58m-23s UTC integration manifest
e3f41cf6527e drm/i915/psr: Update PSR2 resolution check for Cannonlake

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8204/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* [PATCH v2] drm/i915/psr: Update PSR2 resolution check for Cannonlake
  2018-03-01 20:47 ` Ville Syrjälä
  2018-03-01 20:50   ` Pandiyan, Dhinakaran
@ 2018-03-01 21:27   ` Dhinakaran Pandiyan
  2018-03-01 21:47     ` Ville Syrjälä
  2018-03-01 22:09   ` [PATCH] " Rodrigo Vivi
  2 siblings, 1 reply; 11+ messages in thread
From: Dhinakaran Pandiyan @ 2018-03-01 21:27 UTC (permalink / raw)
  To: intel-gfx; +Cc: Dhinakaran Pandiyan, Rodrigo Vivi

In fact, apply the Cannonlake resolution check for all >= Gen-10 platforms
to be safe.

v2: Use local variables for resolution limits and print them (Ville)

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Elio Martinez Monroy <elio.martinez.monroy@intel.com>
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 drivers/gpu/drm/i915/intel_psr.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 05770790a4e9..66d04a8dd99e 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -451,8 +451,9 @@ static bool intel_psr2_config_valid(struct intel_dp *intel_dp,
 {
 	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
 	struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
-	const struct drm_display_mode *adjusted_mode =
-		&crtc_state->base.adjusted_mode;
+	int crtc_h = crtc_state->base.adjusted_mode.crtc_hdisplay;
+	int crtc_v = crtc_state->base.adjusted_mode.crtc_vdisplay;
+	int max_h, max_v;
 
 	/*
 	 * FIXME psr2_support is messed up. It's both computed
@@ -462,10 +463,11 @@ static bool intel_psr2_config_valid(struct intel_dp *intel_dp,
 	if (!dev_priv->psr.psr2_support)
 		return false;
 
-	/* PSR2 is restricted to work with panel resolutions up to 3640x2304 */
-	if (adjusted_mode->crtc_hdisplay > 3640 ||
-	    adjusted_mode->crtc_vdisplay > 2304) {
-		DRM_DEBUG_KMS("PSR2 not enabled, panel resolution too big\n");
+	max_h = INTEL_GEN(dev_priv) >= 10 ? 4096 : 3640;
+	max_v = 2304;
+	if (crtc_h > max_h || crtc_v > max_v) {
+		DRM_DEBUG_KMS("PSR2 not enabled, resolution %dx%d > max supported %dx%d\n",
+			      crtc_h, crtc_v, max_h, max_v);
 		return false;
 	}
 
-- 
2.14.1

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

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

* Re: [PATCH v2] drm/i915/psr: Update PSR2 resolution check for Cannonlake
  2018-03-01 21:27   ` [PATCH v2] " Dhinakaran Pandiyan
@ 2018-03-01 21:47     ` Ville Syrjälä
  2018-03-01 22:04       ` Pandiyan, Dhinakaran
  0 siblings, 1 reply; 11+ messages in thread
From: Ville Syrjälä @ 2018-03-01 21:47 UTC (permalink / raw)
  To: Dhinakaran Pandiyan; +Cc: intel-gfx, Rodrigo Vivi

On Thu, Mar 01, 2018 at 01:27:09PM -0800, Dhinakaran Pandiyan wrote:
> In fact, apply the Cannonlake resolution check for all >= Gen-10 platforms
> to be safe.
> 
> v2: Use local variables for resolution limits and print them (Ville)
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Elio Martinez Monroy <elio.martinez.monroy@intel.com>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_psr.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 05770790a4e9..66d04a8dd99e 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -451,8 +451,9 @@ static bool intel_psr2_config_valid(struct intel_dp *intel_dp,
>  {
>  	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
>  	struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
> -	const struct drm_display_mode *adjusted_mode =
> -		&crtc_state->base.adjusted_mode;
> +	int crtc_h = crtc_state->base.adjusted_mode.crtc_hdisplay;
> +	int crtc_v = crtc_state->base.adjusted_mode.crtc_vdisplay;

I'd probably call these hdisp/vdisp or something like that. "crtc_h" makes
me think it's a height of a plane in crtc (pipe source) coordinates.

> +	int max_h, max_v;
>  
>  	/*
>  	 * FIXME psr2_support is messed up. It's both computed
> @@ -462,10 +463,11 @@ static bool intel_psr2_config_valid(struct intel_dp *intel_dp,
>  	if (!dev_priv->psr.psr2_support)
>  		return false;
>  
> -	/* PSR2 is restricted to work with panel resolutions up to 3640x2304 */
> -	if (adjusted_mode->crtc_hdisplay > 3640 ||
> -	    adjusted_mode->crtc_vdisplay > 2304) {
> -		DRM_DEBUG_KMS("PSR2 not enabled, panel resolution too big\n");
> +	max_h = INTEL_GEN(dev_priv) >= 10 ? 4096 : 3640;
> +	max_v = 2304;

GLK should use the higher limit too no?

Looking at the *future* stuff for this it looks like we'll be getting
different limits again soon. So I'd prep for that day by making this 
a full blown if ladder from the start.

> +	if (crtc_h > max_h || crtc_v > max_v) {
> +		DRM_DEBUG_KMS("PSR2 not enabled, resolution %dx%d > max supported %dx%d\n",
> +			      crtc_h, crtc_v, max_h, max_v);
>  		return false;
>  	}
>  
> -- 
> 2.14.1

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

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

* Re: [PATCH v2] drm/i915/psr: Update PSR2 resolution check for Cannonlake
  2018-03-01 21:47     ` Ville Syrjälä
@ 2018-03-01 22:04       ` Pandiyan, Dhinakaran
  2018-03-02  8:13         ` Ville Syrjälä
  0 siblings, 1 reply; 11+ messages in thread
From: Pandiyan, Dhinakaran @ 2018-03-01 22:04 UTC (permalink / raw)
  To: ville.syrjala@linux.intel.com
  Cc: intel-gfx@lists.freedesktop.org, Vivi, Rodrigo




On Thu, 2018-03-01 at 23:47 +0200, Ville Syrjälä wrote:
> On Thu, Mar 01, 2018 at 01:27:09PM -0800, Dhinakaran Pandiyan wrote:
> > In fact, apply the Cannonlake resolution check for all >= Gen-10 platforms
> > to be safe.
> > 
> > v2: Use local variables for resolution limits and print them (Ville)
> > 
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Elio Martinez Monroy <elio.martinez.monroy@intel.com>
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_psr.c | 14 ++++++++------
> >  1 file changed, 8 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> > index 05770790a4e9..66d04a8dd99e 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -451,8 +451,9 @@ static bool intel_psr2_config_valid(struct intel_dp *intel_dp,
> >  {
> >  	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> >  	struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
> > -	const struct drm_display_mode *adjusted_mode =
> > -		&crtc_state->base.adjusted_mode;
> > +	int crtc_h = crtc_state->base.adjusted_mode.crtc_hdisplay;
> > +	int crtc_v = crtc_state->base.adjusted_mode.crtc_vdisplay;
> 
> I'd probably call these hdisp/vdisp or something like that. "crtc_h" makes
> me think it's a height of a plane in crtc (pipe source) coordinates.

and h/vdisplay are specifically related to the mode?

> 
> > +	int max_h, max_v;
I guess this is okay then?

> >  
> >  	/*
> >  	 * FIXME psr2_support is messed up. It's both computed
> > @@ -462,10 +463,11 @@ static bool intel_psr2_config_valid(struct intel_dp *intel_dp,
> >  	if (!dev_priv->psr.psr2_support)
> >  		return false;
> >  
> > -	/* PSR2 is restricted to work with panel resolutions up to 3640x2304 */
> > -	if (adjusted_mode->crtc_hdisplay > 3640 ||
> > -	    adjusted_mode->crtc_vdisplay > 2304) {
> > -		DRM_DEBUG_KMS("PSR2 not enabled, panel resolution too big\n");
> > +	max_h = INTEL_GEN(dev_priv) >= 10 ? 4096 : 3640;
> > +	max_v = 2304;
> 
> GLK should use the higher limit too no?

Yeah, I just checked and it makes sense to update GLK too.

> 
> Looking at the *future* stuff for this it looks like we'll be getting
> different limits again soon. So I'd prep for that day by making this 
> a full blown if ladder from the start.
> 
> > +	if (crtc_h > max_h || crtc_v > max_v) {
> > +		DRM_DEBUG_KMS("PSR2 not enabled, resolution %dx%d > max supported %dx%d\n",
> > +			      crtc_h, crtc_v, max_h, max_v);
> >  		return false;
> >  	}
> >  
> > -- 
> > 2.14.1
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* ✓ Fi.CI.BAT: success for drm/i915/psr: Update PSR2 resolution check for Cannonlake (rev2)
  2018-03-01 20:38 [PATCH] drm/i915/psr: Update PSR2 resolution check for Cannonlake Dhinakaran Pandiyan
  2018-03-01 20:47 ` Ville Syrjälä
  2018-03-01 21:03 ` ✗ Fi.CI.BAT: failure for " Patchwork
@ 2018-03-01 22:08 ` Patchwork
  2018-03-02  3:00 ` ✓ Fi.CI.IGT: " Patchwork
  3 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2018-03-01 22:08 UTC (permalink / raw)
  To: Pandiyan, Dhinakaran; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/psr: Update PSR2 resolution check for Cannonlake (rev2)
URL   : https://patchwork.freedesktop.org/series/39238/
State : success

== Summary ==

Series 39238v2 drm/i915/psr: Update PSR2 resolution check for Cannonlake
https://patchwork.freedesktop.org/api/1.0/series/39238/revisions/2/mbox/

---- Known issues:

Test debugfs_test:
        Subgroup read_all_entries:
                pass       -> INCOMPLETE (fi-snb-2520m) fdo#103713

fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:414s
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:419s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:372s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:478s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:279s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:477s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:478s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:465s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:453s
fi-cfl-8700k     total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:389s
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:567s
fi-elk-e7500     total:288  pass:229  dwarn:0   dfail:0   fail:0   skip:59  time:407s
fi-gdg-551       total:288  pass:180  dwarn:0   dfail:0   fail:0   skip:108 time:291s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:510s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:384s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:406s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:449s
fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:422s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:451s
fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:487s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:447s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:491s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:583s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:432s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:502s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:521s
fi-skl-6700k2    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:486s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:482s
fi-skl-guc       total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:404s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:426s
fi-snb-2520m     total:3    pass:2    dwarn:0   dfail:0   fail:0   skip:0  
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:389s
Blacklisted hosts:
fi-cfl-u         total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:504s

7fe823aaeff7c50eeaf9b238a179b41771e08f9e drm-tip: 2018y-03m-01d-15h-58m-23s UTC integration manifest
5a3ccdd925b6 drm/i915/psr: Update PSR2 resolution check for Cannonlake

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8205/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH] drm/i915/psr: Update PSR2 resolution check for Cannonlake
  2018-03-01 20:47 ` Ville Syrjälä
  2018-03-01 20:50   ` Pandiyan, Dhinakaran
  2018-03-01 21:27   ` [PATCH v2] " Dhinakaran Pandiyan
@ 2018-03-01 22:09   ` Rodrigo Vivi
  2 siblings, 0 replies; 11+ messages in thread
From: Rodrigo Vivi @ 2018-03-01 22:09 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx, Dhinakaran Pandiyan

On Thu, Mar 01, 2018 at 10:47:05PM +0200, Ville Syrjälä wrote:
> On Thu, Mar 01, 2018 at 12:38:58PM -0800, Dhinakaran Pandiyan wrote:
> > In fact, apply the Cannonlake resolution check for all > Gen-9 platforms to
> > be safe.
> > 
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Elio Martinez Monroy <elio.martinez.monroy@intel.com>
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_psr.c | 11 ++++++-----
> >  1 file changed, 6 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> > index 05770790a4e9..2a2c696c4109 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -451,8 +451,8 @@ static bool intel_psr2_config_valid(struct intel_dp *intel_dp,
> >  {
> >  	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> >  	struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
> > -	const struct drm_display_mode *adjusted_mode =
> > -		&crtc_state->base.adjusted_mode;
> > +	int crtc_h = crtc_state->base.adjusted_mode.crtc_hdisplay;
> > +	int crtc_v = crtc_state->base.adjusted_mode.crtc_vdisplay;
> >  
> >  	/*
> >  	 * FIXME psr2_support is messed up. It's both computed
> > @@ -462,9 +462,10 @@ static bool intel_psr2_config_valid(struct intel_dp *intel_dp,
> >  	if (!dev_priv->psr.psr2_support)
> >  		return false;
> >  
> > -	/* PSR2 is restricted to work with panel resolutions up to 3640x2304 */
> > -	if (adjusted_mode->crtc_hdisplay > 3640 ||
> > -	    adjusted_mode->crtc_vdisplay > 2304) {
> > +	if (crtc_h > 4096 || crtc_v > 2304) {
> > +		DRM_DEBUG_KMS("PSR2 not enabled, panel resolution too big\n");
> > +		return false;
> > +	} else if (IS_GEN9(dev_priv) && (crtc_h > 3640 || crtc_v > 2304)) {
> 
> I would suggest introducing max_w/h or something similar so that you
> don't have to duplicate the actual code.

I was going to suggest same thing. So it gets more clear that it is
per platform based and avoid duplication of code

> 
> When debugging things from logs one might want to know both the requested
> size and the max. So printing those out might be a good idea.
> 
> >  		DRM_DEBUG_KMS("PSR2 not enabled, panel resolution too big\n");
> >  		return false;
> >  	}
> > -- 
> > 2.14.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* ✓ Fi.CI.IGT: success for drm/i915/psr: Update PSR2 resolution check for Cannonlake (rev2)
  2018-03-01 20:38 [PATCH] drm/i915/psr: Update PSR2 resolution check for Cannonlake Dhinakaran Pandiyan
                   ` (2 preceding siblings ...)
  2018-03-01 22:08 ` ✓ Fi.CI.BAT: success for drm/i915/psr: Update PSR2 resolution check for Cannonlake (rev2) Patchwork
@ 2018-03-02  3:00 ` Patchwork
  3 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2018-03-02  3:00 UTC (permalink / raw)
  To: Pandiyan, Dhinakaran; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/psr: Update PSR2 resolution check for Cannonlake (rev2)
URL   : https://patchwork.freedesktop.org/series/39238/
State : success

== Summary ==

---- Possible new issues:

Test kms_fence_pin_leak:
                incomplete -> PASS       (shard-apl)

---- Known issues:

Test gem_eio:
        Subgroup in-flight:
                pass       -> INCOMPLETE (shard-apl) fdo#104945 +1
Test kms_chv_cursor_fail:
        Subgroup pipe-b-256x256-right-edge:
                dmesg-warn -> PASS       (shard-snb) fdo#105185 +2
Test kms_flip:
        Subgroup 2x-plain-flip-ts-check:
                pass       -> FAIL       (shard-hsw) fdo#100368 +1
        Subgroup dpms-vs-vblank-race:
                pass       -> FAIL       (shard-hsw) fdo#103060

fdo#104945 https://bugs.freedesktop.org/show_bug.cgi?id=104945
fdo#105185 https://bugs.freedesktop.org/show_bug.cgi?id=105185
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060

shard-apl        total:3300 pass:1733 dwarn:1   dfail:0   fail:6   skip:1557 time:11316s
shard-hsw        total:3461 pass:1765 dwarn:1   dfail:0   fail:4   skip:1690 time:11685s
shard-snb        total:3461 pass:1357 dwarn:4   dfail:0   fail:1   skip:2099 time:6638s
Blacklisted hosts:
shard-kbl        total:3461 pass:1946 dwarn:1   dfail:0   fail:7   skip:1507 time:9518s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8205/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH v2] drm/i915/psr: Update PSR2 resolution check for Cannonlake
  2018-03-01 22:04       ` Pandiyan, Dhinakaran
@ 2018-03-02  8:13         ` Ville Syrjälä
  0 siblings, 0 replies; 11+ messages in thread
From: Ville Syrjälä @ 2018-03-02  8:13 UTC (permalink / raw)
  To: Pandiyan, Dhinakaran; +Cc: intel-gfx@lists.freedesktop.org, Vivi, Rodrigo

On Thu, Mar 01, 2018 at 10:04:04PM +0000, Pandiyan, Dhinakaran wrote:
> 
> 
> 
> On Thu, 2018-03-01 at 23:47 +0200, Ville Syrjälä wrote:
> > On Thu, Mar 01, 2018 at 01:27:09PM -0800, Dhinakaran Pandiyan wrote:
> > > In fact, apply the Cannonlake resolution check for all >= Gen-10 platforms
> > > to be safe.
> > > 
> > > v2: Use local variables for resolution limits and print them (Ville)
> > > 
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Cc: Elio Martinez Monroy <elio.martinez.monroy@intel.com>
> > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_psr.c | 14 ++++++++------
> > >  1 file changed, 8 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> > > index 05770790a4e9..66d04a8dd99e 100644
> > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > @@ -451,8 +451,9 @@ static bool intel_psr2_config_valid(struct intel_dp *intel_dp,
> > >  {
> > >  	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> > >  	struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
> > > -	const struct drm_display_mode *adjusted_mode =
> > > -		&crtc_state->base.adjusted_mode;
> > > +	int crtc_h = crtc_state->base.adjusted_mode.crtc_hdisplay;
> > > +	int crtc_v = crtc_state->base.adjusted_mode.crtc_vdisplay;
> > 
> > I'd probably call these hdisp/vdisp or something like that. "crtc_h" makes
> > me think it's a height of a plane in crtc (pipe source) coordinates.
> 
> and h/vdisplay are specifically related to the mode?

They are the active portion of the timings.

> 
> > 
> > > +	int max_h, max_v;
> I guess this is okay then?

max_h does make me immediately think "max height" so they could probably
use a few more characters as well.

> 
> > >  
> > >  	/*
> > >  	 * FIXME psr2_support is messed up. It's both computed
> > > @@ -462,10 +463,11 @@ static bool intel_psr2_config_valid(struct intel_dp *intel_dp,
> > >  	if (!dev_priv->psr.psr2_support)
> > >  		return false;
> > >  
> > > -	/* PSR2 is restricted to work with panel resolutions up to 3640x2304 */
> > > -	if (adjusted_mode->crtc_hdisplay > 3640 ||
> > > -	    adjusted_mode->crtc_vdisplay > 2304) {
> > > -		DRM_DEBUG_KMS("PSR2 not enabled, panel resolution too big\n");
> > > +	max_h = INTEL_GEN(dev_priv) >= 10 ? 4096 : 3640;
> > > +	max_v = 2304;
> > 
> > GLK should use the higher limit too no?
> 
> Yeah, I just checked and it makes sense to update GLK too.
> 
> > 
> > Looking at the *future* stuff for this it looks like we'll be getting
> > different limits again soon. So I'd prep for that day by making this 
> > a full blown if ladder from the start.
> > 
> > > +	if (crtc_h > max_h || crtc_v > max_v) {
> > > +		DRM_DEBUG_KMS("PSR2 not enabled, resolution %dx%d > max supported %dx%d\n",
> > > +			      crtc_h, crtc_v, max_h, max_v);
> > >  		return false;
> > >  	}
> > >  
> > > -- 
> > > 2.14.1
> > 

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

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

end of thread, other threads:[~2018-03-02  8:14 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-03-01 20:38 [PATCH] drm/i915/psr: Update PSR2 resolution check for Cannonlake Dhinakaran Pandiyan
2018-03-01 20:47 ` Ville Syrjälä
2018-03-01 20:50   ` Pandiyan, Dhinakaran
2018-03-01 21:27   ` [PATCH v2] " Dhinakaran Pandiyan
2018-03-01 21:47     ` Ville Syrjälä
2018-03-01 22:04       ` Pandiyan, Dhinakaran
2018-03-02  8:13         ` Ville Syrjälä
2018-03-01 22:09   ` [PATCH] " Rodrigo Vivi
2018-03-01 21:03 ` ✗ Fi.CI.BAT: failure for " Patchwork
2018-03-01 22:08 ` ✓ Fi.CI.BAT: success for drm/i915/psr: Update PSR2 resolution check for Cannonlake (rev2) Patchwork
2018-03-02  3:00 ` ✓ Fi.CI.IGT: " Patchwork

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.