[PATCH] drm/i915: make edp optimize config

[PATCH] drm/i915: make edp optimize config

[matthew.s.atwood]

From: Matt Atwood <matthew.s.atwood@intel.com>

Previously it was assumed that eDP panels would advertise the lowest link
rate required for their singular mode to function. With the introduction
of more advanced features there are advantages to a panel advertising a
higher rate then it needs for a its given mode. For panels that did, the
driver previously used a higher rate then necessary for that mode.

Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index a2eeede..aa6d77d 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1758,16 +1758,6 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 				      dev_priv->vbt.edp.bpp);
 			bpp = dev_priv->vbt.edp.bpp;
 		}
-
-		/*
-		 * Use the maximum clock and number of lanes the eDP panel
-		 * advertizes being capable of. The panels are generally
-		 * designed to support only a single clock and lane
-		 * configuration, and typically these values correspond to the
-		 * native resolution of the panel.
-		 */
-		min_lane_count = max_lane_count;
-		min_clock = max_clock;
 	}
 
 	for (; bpp >= 6*3; bpp -= 2*3) {
-- 
2.7.4

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

✓ Fi.CI.BAT: success for drm/i915: make edp optimize config

[Patchwork]

== Series Details ==

Series: drm/i915: make edp optimize config
URL   : https://patchwork.freedesktop.org/series/39652/
State : success

== Summary ==

Series 39652v1 drm/i915: make edp optimize config
https://patchwork.freedesktop.org/api/1.0/series/39652/revisions/1/mbox/

---- Known issues:

Test gem_mmap_gtt:
        Subgroup basic-small-bo-tiledx:
                pass       -> FAIL       (fi-gdg-551) fdo#102575
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                pass       -> DMESG-WARN (fi-cnl-y3) fdo#103191

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

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:422s
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:370s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:499s
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:486s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:492s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:477s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:464s
fi-cfl-8700k     total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:405s
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:578s
fi-cnl-y3        total:288  pass:261  dwarn:1   dfail:0   fail:0   skip:26  time:573s
fi-elk-e7500     total:288  pass:229  dwarn:0   dfail:0   fail:0   skip:59  time:408s
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:518s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:395s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:413s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:462s
fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:420s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:470s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:461s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:513s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:590s
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:519s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:532s
fi-skl-6700k2    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:502s
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:422s
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:388s
Blacklisted hosts:
fi-cnl-drrs      total:288  pass:257  dwarn:3   dfail:0   fail:0   skip:19  time:504s

469c28df8d66d3cc0a4a2e4e12433a5c92102022 drm-tip: 2018y-03m-08d-22h-40m-12s UTC integration manifest
955496d81a15 drm/i915: make edp optimize config

== Logs ==

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

Re: [PATCH] drm/i915: make edp optimize config

[Benson Leung]

[-- Attachment #1.1: Type: text/plain, Size: 1713 bytes --]

Hi Matt,

Minor commit message nits.

On Thu, Mar 08, 2018 at 05:17:38PM -0800, matthew.s.atwood@intel.com wrote:
> From: Matt Atwood <matthew.s.atwood@intel.com>
> 
> Previously it was assumed that eDP panels would advertise the lowest link
> rate required for their singular mode to function. With the introduction
> of more advanced features there are advantages to a panel advertising a
> higher rate then
s/then/than

> it needs for a its given mode. 

Don't need "its" here.

> For panels that did, the
> driver previously used a higher rate then necessary for that mode.
>
s/then/than

 
> Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>

Tested-by: Benson Leung <bleung@chromium.org>

Tested this on a panel and system with the following source and sink rates:


[    1.623225] [drm:intel_dp_print_rates] source rates: 162000, 216000, 270000, 324000, 432000, 540000
[    1.623230] [drm:intel_dp_print_rates] sink rates: 162000, 216000, 243000, 270000, 324000, 432000, 540000
[    1.623234] [drm:intel_dp_print_rates] common rates: 162000, 216000, 270000, 324000, 432000, 540000

Prior to this patch, the driver would pick and train at 540000:
[    2.865653] [drm:intel_dp_start_link_train] [CONNECTOR:76:eDP-1] Link Training Passed at Link Rate = 540000, Lane count = 4

After this patch, the driver picks and trains at 324000, which is enough
for the panel's native mode:
[    5.359499] [drm:intel_dp_start_link_train] [CONNECTOR:76:eDP-1] Link Training Passed at Link Rate = 324000, Lane count = 4

Thanks!
Benson

-- 
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
bleung@google.com
Chromium OS Project
bleung@chromium.org

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

✗ Fi.CI.IGT: warning for drm/i915: make edp optimize config

[Patchwork]

== Series Details ==

Series: drm/i915: make edp optimize config
URL   : https://patchwork.freedesktop.org/series/39652/
State : warning

== Summary ==

---- Possible new issues:

Test kms_busy:
        Subgroup extended-pageflip-hang-oldfb-render-a:
                pass       -> SKIP       (shard-snb)
Test kms_vblank:
        Subgroup pipe-a-ts-continuation-modeset:
                pass       -> SKIP       (shard-snb)
        Subgroup pipe-a-wait-busy-hang:
                pass       -> SKIP       (shard-snb)

---- Known issues:

Test gem_eio:
        Subgroup in-flight-external:
                pass       -> INCOMPLETE (shard-apl) fdo#105341
Test kms_cursor_crc:
        Subgroup cursor-256x256-suspend:
                pass       -> INCOMPLETE (shard-hsw) fdo#103375
Test kms_setmode:
        Subgroup basic:
                fail       -> PASS       (shard-hsw) fdo#99912

fdo#105341 https://bugs.freedesktop.org/show_bug.cgi?id=105341
fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912

shard-apl        total:3381 pass:1779 dwarn:1   dfail:0   fail:9   skip:1591 time:11672s
shard-hsw        total:3383 pass:1724 dwarn:1   dfail:0   fail:0   skip:1656 time:11145s
shard-snb        total:3467 pass:1360 dwarn:1   dfail:0   fail:2   skip:2104 time:6846s
Blacklisted hosts:
shard-kbl        total:3308 pass:1857 dwarn:1   dfail:0   fail:9   skip:1439 time:8635s

== Logs ==

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

Re: [PATCH] drm/i915: make edp optimize config

[Jani Nikula]

On Thu, 08 Mar 2018, matthew.s.atwood@intel.com wrote:
> From: Matt Atwood <matthew.s.atwood@intel.com>
>
> Previously it was assumed that eDP panels would advertise the lowest link
> rate required for their singular mode to function. With the introduction
> of more advanced features there are advantages to a panel advertising a
> higher rate then it needs for a its given mode. For panels that did, the
> driver previously used a higher rate then necessary for that mode.
>
> Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 10 ----------
>  1 file changed, 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index a2eeede..aa6d77d 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1758,16 +1758,6 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  				      dev_priv->vbt.edp.bpp);
>  			bpp = dev_priv->vbt.edp.bpp;
>  		}
> -
> -		/*
> -		 * Use the maximum clock and number of lanes the eDP panel
> -		 * advertizes being capable of. The panels are generally
> -		 * designed to support only a single clock and lane
> -		 * configuration, and typically these values correspond to the
> -		 * native resolution of the panel.
> -		 */
> -		min_lane_count = max_lane_count;
> -		min_clock = max_clock;

Please see my reply to Manasi's identical patch [1]. If we apply this
as-is, it will regress and will be reverted.

BR,
Jani.


[1] http://patchwork.freedesktop.org/patch/msgid/1520579339-14745-1-git-send-email-manasi.d.navare@intel.com


>  	}
>  
>  	for (; bpp >= 6*3; bpp -= 2*3) {

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

Re: [PATCH] drm/i915: make edp optimize config

[Atwood, Matthew S]

On Fri, 2018-03-09 at 14:05 +0200, Jani Nikula wrote:
> On Thu, 08 Mar 2018, matthew.s.atwood@intel.com wrote:
> > 
> > From: Matt Atwood <matthew.s.atwood@intel.com>
> > 
> > Previously it was assumed that eDP panels would advertise the
> > lowest link
> > rate required for their singular mode to function. With the
> > introduction
> > of more advanced features there are advantages to a panel
> > advertising a
> > higher rate then it needs for a its given mode. For panels that
> > did, the
> > driver previously used a higher rate then necessary for that mode.
> > 
> > Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c | 10 ----------
> >  1 file changed, 10 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index a2eeede..aa6d77d 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -1758,16 +1758,6 @@ intel_dp_compute_config(struct intel_encoder
> > *encoder,
> >  				      dev_priv->vbt.edp.bpp);
> >  			bpp = dev_priv->vbt.edp.bpp;
> >  		}
> > -
> > -		/*
> > -		 * Use the maximum clock and number of lanes the
> > eDP panel
> > -		 * advertizes being capable of. The panels are
> > generally
> > -		 * designed to support only a single clock and
> > lane
> > -		 * configuration, and typically these values
> > correspond to the
> > -		 * native resolution of the panel.
> > -		 */
> > -		min_lane_count = max_lane_count;
> > -		min_clock = max_clock;
> Please see my reply to Manasi's identical patch [1]. If we apply this
> as-is, it will regress and will be reverted.
> 
> BR,
> Jani.
> 
> 
> [1] http://patchwork.freedesktop.org/patch/msgid/1520579339-14745-1-
> git-send-email-manasi.d.navare@intel.com
to consolidate some of the information the bug that's referenced in
Manasi's patch is https://bugs.freedesktop.org/show_bug.cgi?id=73539. I
understand this bug as the following some panels may advertise a mode
that requires less then MAX_LANE_COUNT, however those panels would fail
if less lanes were used.

When this bug was filed the compute config inner for loop iterated on
rate and the outer iterated on lanes; this is now flipped. It *should*
be the case that the lowest frequency with the max amount of lanes is
preferred. Given the opposite behavior of the alogorithm to select I
dont think we'd come across this again. Even if I'm wrong we could
still min_lane_count = max_lane count and let the clock optimize
itself.

I guess my question is, is there also a bug where if MAX_RATE wasnt
used we saw a panel fail as well?

Matt
> 
> 
> > 
> >  	}
> >  
> >  	for (; bpp >= 6*3; bpp -= 2*3) {
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: make edp optimize config

[Rodrigo Vivi]

On Sat, Mar 10, 2018 at 12:07:34AM +0000, Atwood, Matthew S wrote:
> On Fri, 2018-03-09 at 14:05 +0200, Jani Nikula wrote:
> > On Thu, 08 Mar 2018, matthew.s.atwood@intel.com wrote:
> > > 
> > > From: Matt Atwood <matthew.s.atwood@intel.com>
> > > 
> > > Previously it was assumed that eDP panels would advertise the
> > > lowest link
> > > rate required for their singular mode to function. With the
> > > introduction
> > > of more advanced features there are advantages to a panel
> > > advertising a
> > > higher rate then it needs for a its given mode. For panels that
> > > did, the
> > > driver previously used a higher rate then necessary for that mode.
> > > 
> > > Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_dp.c | 10 ----------
> > >  1 file changed, 10 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > > b/drivers/gpu/drm/i915/intel_dp.c
> > > index a2eeede..aa6d77d 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -1758,16 +1758,6 @@ intel_dp_compute_config(struct intel_encoder
> > > *encoder,
> > >  				      dev_priv->vbt.edp.bpp);
> > >  			bpp = dev_priv->vbt.edp.bpp;
> > >  		}
> > > -
> > > -		/*
> > > -		 * Use the maximum clock and number of lanes the
> > > eDP panel
> > > -		 * advertizes being capable of. The panels are
> > > generally
> > > -		 * designed to support only a single clock and
> > > lane
> > > -		 * configuration, and typically these values
> > > correspond to the
> > > -		 * native resolution of the panel.
> > > -		 */
> > > -		min_lane_count = max_lane_count;
> > > -		min_clock = max_clock;
> > Please see my reply to Manasi's identical patch [1]. If we apply this
> > as-is, it will regress and will be reverted.
> > 
> > BR,
> > Jani.
> > 
> > 
> > [1] http://patchwork.freedesktop.org/patch/msgid/1520579339-14745-1-
> > git-send-email-manasi.d.navare@intel.com
> to consolidate some of the information the bug that's referenced in
> Manasi's patch is https://bugs.freedesktop.org/show_bug.cgi?id=73539. I
> understand this bug as the following some panels may advertise a mode
> that requires less then MAX_LANE_COUNT, however those panels would fail
> if less lanes were used.
> 
> When this bug was filed the compute config inner for loop iterated on
> rate and the outer iterated on lanes; this is now flipped. It *should*
> be the case that the lowest frequency with the max amount of lanes is
> preferred. Given the opposite behavior of the alogorithm to select I
> dont think we'd come across this again. Even if I'm wrong we could
> still min_lane_count = max_lane count and let the clock optimize
> itself.
> 
> I guess my question is, is there also a bug where if MAX_RATE wasnt
> used we saw a panel fail as well?

Looking to eDP 1.4 spec I'm convinced that max link rate approach is
the ideal for eDP 1.3 and older, regardless of the issues we had on
previous attempts.

But eDP 1.4 seems to introduce the new link rates flexibility and selection
in order to provide improved system-specific link rate optimization
and power efficiency.

I believe what we want here is:

-		min_lane_count = max_lane_count;
-		min_clock = max_clock;
+		if (intel_dp->edp_dpcd[0] <= DP_EDP_13) {
+			min_lane_count = max_lane_count;
+			min_clock = max_clock;
+		}

for me it seems that eDP 1.4 brings eDP link rates handling close to
DP ones. So imho this move here should be safe.

Besides, with this difference in place,
we don't break the world and if new bugs appear we can
work on those individually to figure out if we are missing something
else and this block was only masking other issues.

Thoughts?

> 
> Matt
> > 
> > 
> > > 
> > >  	}
> > >  
> > >  	for (; bpp >= 6*3; bpp -= 2*3) {
> _______________________________________________
> 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