[PATCH] drm/i915/edp: Only use alternate fixed mode when requested

[PATCH] drm/i915/edp: Only use alternate fixed mode when requested

[clinton.a.taylor]

From: Clint Taylor <clinton.a.taylor@intel.com>

In commit dc911f5bd8aa ("drm/i915/edp: Allow alternate fixed mode for eDP
if available."), the patch was always selecting the alternate refresh rate
even though user space was asking for the higher rate. This patch adds a
check for vrefresh rate as well as the rest of the mode geometry.

Fixes: dc911f5bd8aac ("Allow alternate fixed mode for eDP if available.")
Cc: David Weinehall <david.weinehall@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 62f82c4..c4c0e79 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1681,7 +1681,8 @@ static bool intel_edp_compare_alt_mode(struct drm_display_mode *m1,
 			m1->vdisplay == m2->vdisplay &&
 			m1->vsync_start == m2->vsync_start &&
 			m1->vsync_end == m2->vsync_end &&
-			m1->vtotal == m2->vtotal);
+			m1->vtotal == m2->vtotal &&
+			m1->vrefresh == m2->vrefresh);
 	return bres;
 }
 
-- 
1.9.1

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

✗ Fi.CI.BAT: warning for drm/i915/edp: Only use alternate fixed mode when requested

[Patchwork]

== Series Details ==

Series: drm/i915/edp: Only use alternate fixed mode when requested
URL   : https://patchwork.freedesktop.org/series/41492/
State : warning

== Summary ==

Series 41492v1 drm/i915/edp: Only use alternate fixed mode when requested
https://patchwork.freedesktop.org/api/1.0/series/41492/revisions/1/mbox/

---- Possible new issues:

Test gem_exec_gttfill:
        Subgroup basic:
                pass       -> SKIP       (fi-pnv-d510)

---- 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-c:
                pass       -> INCOMPLETE (fi-bxt-dsi) fdo#103927
Test prime_vgem:
        Subgroup basic-fence-flip:
                pass       -> FAIL       (fi-ilk-650) fdo#104008

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

fi-bdw-5557u     total:285  pass:264  dwarn:0   dfail:0   fail:0   skip:21  time:431s
fi-bdw-gvtdvm    total:285  pass:261  dwarn:0   dfail:0   fail:0   skip:24  time:445s
fi-blb-e6850     total:285  pass:220  dwarn:1   dfail:0   fail:0   skip:64  time:382s
fi-bsw-n3050     total:285  pass:239  dwarn:0   dfail:0   fail:0   skip:46  time:548s
fi-bwr-2160      total:285  pass:180  dwarn:0   dfail:0   fail:0   skip:105 time:298s
fi-bxt-dsi       total:243  pass:216  dwarn:0   dfail:0   fail:0   skip:26 
fi-bxt-j4205     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:515s
fi-byt-j1900     total:285  pass:250  dwarn:0   dfail:0   fail:0   skip:35  time:527s
fi-byt-n2820     total:285  pass:246  dwarn:0   dfail:0   fail:0   skip:39  time:513s
fi-cfl-8700k     total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:413s
fi-cfl-s3        total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:563s
fi-cfl-u         total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:515s
fi-elk-e7500     total:285  pass:226  dwarn:0   dfail:0   fail:0   skip:59  time:424s
fi-gdg-551       total:285  pass:176  dwarn:0   dfail:0   fail:1   skip:108 time:314s
fi-glk-1         total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:542s
fi-glk-j4005     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:490s
fi-hsw-4770      total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:403s
fi-ilk-650       total:285  pass:224  dwarn:0   dfail:0   fail:1   skip:60  time:423s
fi-ivb-3520m     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:461s
fi-ivb-3770      total:285  pass:252  dwarn:0   dfail:0   fail:0   skip:33  time:433s
fi-kbl-7500u     total:285  pass:260  dwarn:1   dfail:0   fail:0   skip:24  time:471s
fi-kbl-7567u     total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:462s
fi-kbl-r         total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:510s
fi-pnv-d510      total:285  pass:219  dwarn:1   dfail:0   fail:0   skip:65  time:632s
fi-skl-6260u     total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:446s
fi-skl-6600u     total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:531s
fi-skl-6700k2    total:285  pass:261  dwarn:0   dfail:0   fail:0   skip:24  time:510s
fi-skl-6770hq    total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:509s
fi-skl-guc       total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:432s
fi-skl-gvtdvm    total:285  pass:262  dwarn:0   dfail:0   fail:0   skip:23  time:448s
fi-snb-2520m     total:285  pass:245  dwarn:0   dfail:0   fail:0   skip:40  time:568s
fi-snb-2600      total:285  pass:245  dwarn:0   dfail:0   fail:0   skip:40  time:404s

df88a0c8d65abb40b39bd5f19b5290dac6746cb9 drm-tip: 2018y-04m-10d-14h-31m-39s UTC integration manifest
04feae3dcd19 drm/i915/edp: Only use alternate fixed mode when requested

== Logs ==

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

Re: [PATCH] drm/i915/edp: Only use alternate fixed mode when requested

[Chris Wilson]

Quoting clinton.a.taylor@intel.com (2018-04-10 19:33:36)
> From: Clint Taylor <clinton.a.taylor@intel.com>
> 
> In commit dc911f5bd8aa ("drm/i915/edp: Allow alternate fixed mode for eDP
> if available."), the patch was always selecting the alternate refresh rate
> even though user space was asking for the higher rate. This patch adds a
> check for vrefresh rate as well as the rest of the mode geometry.
> 
> Fixes: dc911f5bd8aac ("Allow alternate fixed mode for eDP if available.")
> Cc: David Weinehall <david.weinehall@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>

Please see 20180313092855.32376-1-maarten.lankhorst@linux.intel.com in
particular the part where Ville has to fix vrefresh to be meaningful...
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.IGT: warning for drm/i915/edp: Only use alternate fixed mode when requested

[Patchwork]

== Series Details ==

Series: drm/i915/edp: Only use alternate fixed mode when requested
URL   : https://patchwork.freedesktop.org/series/41492/
State : warning

== Summary ==

= CI Bug Log - changes from CI_DRM_4041_full -> Patchwork_8661_full =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_8661_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_8661_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce the CI noise.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8661/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_8661_full:

  === IGT changes ===

    ==== Warnings ====

    igt@gem_mocs_settings@mocs-rc6-bsd2:
      shard-kbl:          SKIP -> PASS

    
== Known issues ==

  Here are the changes found in Patchwork_8661_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy:
      shard-hsw:          PASS -> FAIL (fdo#104873)

    igt@kms_flip@2x-blocking-wf_vblank:
      shard-hsw:          PASS -> FAIL (fdo#100368)

    igt@kms_flip@flip-vs-expired-vblank-interruptible:
      shard-hsw:          PASS -> FAIL (fdo#105189)

    igt@kms_flip@flip-vs-modeset-interruptible:
      shard-hsw:          PASS -> DMESG-WARN (fdo#102614)

    
    ==== Possible fixes ====

    igt@kms_flip@2x-flip-vs-absolute-wf_vblank-interruptible:
      shard-hsw:          FAIL (fdo#103928) -> PASS

    igt@kms_frontbuffer_tracking@fbc-rgb101010-draw-mmap-wc:
      shard-apl:          FAIL (fdo#103167) -> PASS

    igt@kms_sysfs_edid_timing:
      shard-apl:          WARN (fdo#100047) -> PASS

    
  fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103928 https://bugs.freedesktop.org/show_bug.cgi?id=103928
  fdo#104873 https://bugs.freedesktop.org/show_bug.cgi?id=104873
  fdo#105189 https://bugs.freedesktop.org/show_bug.cgi?id=105189


== Participating hosts (6 -> 4) ==

  Missing    (2): shard-glk shard-glkb 


== Build changes ==

    * Linux: CI_DRM_4041 -> Patchwork_8661

  CI_DRM_4041: df88a0c8d65abb40b39bd5f19b5290dac6746cb9 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4418: 7c474e011548d35df6b80ceed81d3e6ca560c71d @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_8661: 04feae3dcd1986ad0d2cac9db0f791af399fbc8c @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4418: 45e115f293fd6acc0c9647cf2d3b76be78819ba5 @ git://anongit.freedesktop.org/piglit

== Logs ==

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

Re: [PATCH] drm/i915/edp: Only use alternate fixed mode when requested

[Chris Wilson]

Quoting clinton.a.taylor@intel.com (2018-04-12 00:13:26)
> From: Clint Taylor <clinton.a.taylor@intel.com>
> 
> In commit dc911f5bd8aa ("drm/i915/edp: Allow alternate fixed mode for eDP
> if available."), the patch was always selecting the alternate refresh rate
> even though user space was asking for the higher rate. This patch adds a
> check for vrefresh rate as well as the rest of the mode geometry.
> 
> V2: use clock instead of vrefresh for compare.
> 
> Fixes: dc911f5bd8aac ("Allow alternate fixed mode for eDP if available.")
> Cc: David Weinehall <david.weinehall@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>

Still leaves the other discussion point in the other thread unresolved.
The fields are supplied by the user and can be arbitrary, so if they
request a clock for a 30Hz mode, instead of using the 40Hz alternative,
we use the 60Hz normal mode (by way of example). Is equality always the
best choice here?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] drm/i915/edp: Only use alternate fixed mode when requested

[clinton.a.taylor]

From: Clint Taylor <clinton.a.taylor@intel.com>

In commit dc911f5bd8aa ("drm/i915/edp: Allow alternate fixed mode for eDP
if available."), the patch was always selecting the alternate refresh rate
even though user space was asking for the higher rate. This patch adds a
check for vrefresh rate as well as the rest of the mode geometry.

V2: use clock instead of vrefresh for compare.

Fixes: dc911f5bd8aac ("Allow alternate fixed mode for eDP if available.")
Cc: David Weinehall <david.weinehall@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 62f82c4..6a2e256 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1681,7 +1681,8 @@ static bool intel_edp_compare_alt_mode(struct drm_display_mode *m1,
 			m1->vdisplay == m2->vdisplay &&
 			m1->vsync_start == m2->vsync_start &&
 			m1->vsync_end == m2->vsync_end &&
-			m1->vtotal == m2->vtotal);
+			m1->vtotal == m2->vtotal &&
+			m1->clock == m2->clock);
 	return bres;
 }
 
-- 
1.9.1

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

✓ Fi.CI.BAT: success for drm/i915/edp: Only use alternate fixed mode when requested (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915/edp: Only use alternate fixed mode when requested (rev2)
URL   : https://patchwork.freedesktop.org/series/41492/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4047 -> Patchwork_8676 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8676/

== Known issues ==

  Here are the changes found in Patchwork_8676 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-snb-2520m:       PASS -> INCOMPLETE (fdo#103713)

    igt@prime_vgem@basic-fence-flip:
      fi-ilk-650:         PASS -> FAIL (fdo#104008)

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


== Participating hosts (35 -> 31) ==

  Missing    (4): fi-ctg-p8600 fi-ilk-m540 fi-skl-6700hq fi-pnv-d510 


== Build changes ==

    * Linux: CI_DRM_4047 -> Patchwork_8676

  CI_DRM_4047: b21f4ebec3a255026a780ee4a35ec5f8d9ede6cd @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4423: d502f055ac4500cada758876a512ac4f14b34851 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_8676: 34c1726dec5c3db194806bc58bb9db63c3d0a157 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4423: 45e115f293fd6acc0c9647cf2d3b76be78819ba5 @ git://anongit.freedesktop.org/piglit


== Linux commits ==

34c1726dec5c drm/i915/edp: Only use alternate fixed mode when requested

== Logs ==

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

✗ Fi.CI.IGT: failure for drm/i915/edp: Only use alternate fixed mode when requested (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915/edp: Only use alternate fixed mode when requested (rev2)
URL   : https://patchwork.freedesktop.org/series/41492/
State : failure

== Summary ==

= CI Bug Log - changes from CI_DRM_4047_full -> Patchwork_8676_full =

== Summary - FAILURE ==

  Serious unknown changes coming with Patchwork_8676_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_8676_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8676/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_8676_full:

  === IGT changes ===

    ==== Possible regressions ====

    igt@gem_exec_capture@capture-bsd1:
      shard-kbl:          PASS -> FAIL

    
    ==== Warnings ====

    igt@gem_mocs_settings@mocs-rc6-blt:
      shard-kbl:          SKIP -> PASS

    
== Known issues ==

  Here are the changes found in Patchwork_8676_full that come from known issues:

  === IGT changes ===

    ==== Possible fixes ====

    igt@kms_flip@2x-plain-flip-fb-recreate-interruptible:
      shard-hsw:          FAIL (fdo#100368) -> PASS

    igt@perf_pmu@rc6-runtime-pm-long:
      shard-kbl:          FAIL (fdo#105010) -> PASS

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


== Participating hosts (6 -> 4) ==

  Missing    (2): shard-glk shard-glkb 


== Build changes ==

    * Linux: CI_DRM_4047 -> Patchwork_8676

  CI_DRM_4047: b21f4ebec3a255026a780ee4a35ec5f8d9ede6cd @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4423: d502f055ac4500cada758876a512ac4f14b34851 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_8676: 34c1726dec5c3db194806bc58bb9db63c3d0a157 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4423: 45e115f293fd6acc0c9647cf2d3b76be78819ba5 @ git://anongit.freedesktop.org/piglit

== Logs ==

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

Re: [PATCH] drm/i915/edp: Only use alternate fixed mode when requested

[Clint Taylor]

On 04/11/2018 04:11 PM, Chris Wilson wrote:
> Quoting clinton.a.taylor@intel.com (2018-04-12 00:13:26)
>> From: Clint Taylor <clinton.a.taylor@intel.com>
>>
>> In commit dc911f5bd8aa ("drm/i915/edp: Allow alternate fixed mode for eDP
>> if available."), the patch was always selecting the alternate refresh rate
>> even though user space was asking for the higher rate. This patch adds a
>> check for vrefresh rate as well as the rest of the mode geometry.
>>
>> V2: use clock instead of vrefresh for compare.
>>
>> Fixes: dc911f5bd8aac ("Allow alternate fixed mode for eDP if available.")
>> Cc: David Weinehall <david.weinehall@linux.intel.com>
>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
> Still leaves the other discussion point in the other thread unresolved.
> The fields are supplied by the user and can be arbitrary, so if they
> request a clock for a 30Hz mode, instead of using the 40Hz alternative,
> we use the 60Hz normal mode (by way of example). Is equality always the
> best choice here?
This feature is for testing PSR panels that don't support single frame 
setup times in their preferred timing. The down-clocked mode is the 
timing that the panel specifically states is supported. If a customer 
specifies a custom mode either it should be rejected or the eDP fixed 
mode (preferred) should be used. If we want to allow the users to set a 
custom timing to their eDP panels then we should get rid of the fixed 
mode feature for eDP panels.

-Clint

> -Chris

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

Re: [PATCH] drm/i915/edp: Only use alternate fixed mode when requested

[Vivi, Rodrigo]

> On Apr 12, 2018, at 2:21 PM, Taylor, Clinton A <clinton.a.taylor@intel.com> wrote:
> 
> 
> 
>> On 04/11/2018 04:11 PM, Chris Wilson wrote:
>> Quoting clinton.a.taylor@intel.com (2018-04-12 00:13:26)
>>> From: Clint Taylor <clinton.a.taylor@intel.com>
>>> 
>>> In commit dc911f5bd8aa ("drm/i915/edp: Allow alternate fixed mode for eDP
>>> if available."), the patch was always selecting the alternate refresh rate
>>> even though user space was asking for the higher rate. This patch adds a
>>> check for vrefresh rate as well as the rest of the mode geometry.
>>> 
>>> V2: use clock instead of vrefresh for compare.
>>> 
>>> Fixes: dc911f5bd8aac ("Allow alternate fixed mode for eDP if available.")
>>> Cc: David Weinehall <david.weinehall@linux.intel.com>
>>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>> Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
>> Still leaves the other discussion point in the other thread unresolved.
>> The fields are supplied by the user and can be arbitrary, so if they
>> request a clock for a 30Hz mode, instead of using the 40Hz alternative,
>> we use the 60Hz normal mode (by way of example). Is equality always the
>> best choice here?
> This feature is for testing PSR panels that don't support single frame setup times in their preferred timing. The down-clocked mode is the timing that the panel specifically states is supported. If a customer specifies a custom mode either it should be rejected or the eDP fixed mode (preferred) should be used. If we want to allow the users to set a custom timing to their eDP panels then we should get rid of the fixed mode feature for eDP panels.

But that was the goal of dc911f5bd8aac because
most of the panels we had here by that time with PSR had that short vblank periods for higher mode...
So the idea was to remove the fixed mode allowing the alternate one with lower rate and consequently higher vblank period.

But the idea was never to respect the arbitrary user request. All panels we were targeting here had both modes listed as supported... one with 60Hz and one with 48 Hz. Both same resolution.

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

Re: [PATCH] drm/i915/edp: Only use alternate fixed mode when requested

[Jani Nikula]

On Sat, 14 Apr 2018, "Vivi, Rodrigo" <rodrigo.vivi@intel.com> wrote:
>> On Apr 12, 2018, at 2:21 PM, Taylor, Clinton A <clinton.a.taylor@intel.com> wrote:
>> 
>> 
>> 
>>> On 04/11/2018 04:11 PM, Chris Wilson wrote:
>>> Quoting clinton.a.taylor@intel.com (2018-04-12 00:13:26)
>>>> From: Clint Taylor <clinton.a.taylor@intel.com>
>>>> 
>>>> In commit dc911f5bd8aa ("drm/i915/edp: Allow alternate fixed mode for eDP
>>>> if available."), the patch was always selecting the alternate refresh rate
>>>> even though user space was asking for the higher rate. This patch adds a
>>>> check for vrefresh rate as well as the rest of the mode geometry.
>>>> 
>>>> V2: use clock instead of vrefresh for compare.
>>>> 
>>>> Fixes: dc911f5bd8aac ("Allow alternate fixed mode for eDP if available.")
>>>> Cc: David Weinehall <david.weinehall@linux.intel.com>
>>>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>>> Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
>>> Still leaves the other discussion point in the other thread unresolved.
>>> The fields are supplied by the user and can be arbitrary, so if they
>>> request a clock for a 30Hz mode, instead of using the 40Hz alternative,
>>> we use the 60Hz normal mode (by way of example). Is equality always the
>>> best choice here?
>> This feature is for testing PSR panels that don't support single frame setup times in their preferred timing. The down-clocked mode is the timing that the panel specifically states is supported. If a customer specifies a custom mode either it should be rejected or the eDP fixed mode (preferred) should be used. If we want to allow the users to set a custom timing to their eDP panels then we should get rid of the fixed mode feature for eDP panels.
>
> But that was the goal of dc911f5bd8aac because
> most of the panels we had here by that time with PSR had that short vblank periods for higher mode...
> So the idea was to remove the fixed mode allowing the alternate one with lower rate and consequently higher vblank period.
>
> But the idea was never to respect the arbitrary user request. All panels we were targeting here had both modes listed as supported... one with 60Hz and one with 48 Hz. Both same resolution.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105469

Either we finally get a fix in with Cc: stable, or we revert
dc911f5bd8aa ("drm/i915/edp: Allow alternate fixed mode for eDP if
available.")

BR,
Jani.



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

-- 
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/edp: Only use alternate fixed mode when requested

[Clint Taylor]

On 04/30/2018 12:49 AM, Jani Nikula wrote:
> On Sat, 14 Apr 2018, "Vivi, Rodrigo" <rodrigo.vivi@intel.com> wrote:
>>> On Apr 12, 2018, at 2:21 PM, Taylor, Clinton A <clinton.a.taylor@intel.com> wrote:
>>>
>>>
>>>
>>>> On 04/11/2018 04:11 PM, Chris Wilson wrote:
>>>> Quoting clinton.a.taylor@intel.com (2018-04-12 00:13:26)
>>>>> From: Clint Taylor <clinton.a.taylor@intel.com>
>>>>>
>>>>> In commit dc911f5bd8aa ("drm/i915/edp: Allow alternate fixed mode for eDP
>>>>> if available."), the patch was always selecting the alternate refresh rate
>>>>> even though user space was asking for the higher rate. This patch adds a
>>>>> check for vrefresh rate as well as the rest of the mode geometry.
>>>>>
>>>>> V2: use clock instead of vrefresh for compare.
>>>>>
>>>>> Fixes: dc911f5bd8aac ("Allow alternate fixed mode for eDP if available.")
>>>>> Cc: David Weinehall <david.weinehall@linux.intel.com>
>>>>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>>>> Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
>>>> Still leaves the other discussion point in the other thread unresolved.
>>>> The fields are supplied by the user and can be arbitrary, so if they
>>>> request a clock for a 30Hz mode, instead of using the 40Hz alternative,
>>>> we use the 60Hz normal mode (by way of example). Is equality always the
>>>> best choice here?
>>> This feature is for testing PSR panels that don't support single frame setup times in their preferred timing. The down-clocked mode is the timing that the panel specifically states is supported. If a customer specifies a custom mode either it should be rejected or the eDP fixed mode (preferred) should be used. If we want to allow the users to set a custom timing to their eDP panels then we should get rid of the fixed mode feature for eDP panels.
>> But that was the goal of dc911f5bd8aac because
>> most of the panels we had here by that time with PSR had that short vblank periods for higher mode...
>> So the idea was to remove the fixed mode allowing the alternate one with lower rate and consequently higher vblank period.
>>
>> But the idea was never to respect the arbitrary user request. All panels we were targeting here had both modes listed as supported... one with 60Hz and one with 48 Hz. Both same resolution.
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105469
>
> Either we finally get a fix in with Cc: stable, or we revert
> dc911f5bd8aa ("drm/i915/edp: Allow alternate fixed mode for eDP if
> available.")

I have V3 of the patch in flight now. Though I have no problem with the 
revert as well.

-Clint


>
> BR,
> Jani.
>
>
>
>>> -Clint
>>>
>>>> -Chris
>> _______________________________________________
>> 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

✗ Fi.CI.CHECKPATCH: warning for drm/i915/edp: Only use alternate fixed mode when requested (rev3)

[Patchwork]

== Series Details ==

Series: drm/i915/edp: Only use alternate fixed mode when requested (rev3)
URL   : https://patchwork.freedesktop.org/series/41492/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
467a1ee16409 drn/i915/edp: Only use alternate fixed mode when requested
-:30: CHECK:UNNECESSARY_PARENTHESES: Unnecessary parentheses around '!i915_modparams.enable_psr'
#30: FILE: drivers/gpu/drm/i915/intel_dp.c:1867:
+		if ((!intel_edp_compare_alt_mode(req_mode, panel_mode) &&
+		    intel_psr_setup_time_valid(intel_dp, req_mode)) ||
+		    (!i915_modparams.enable_psr))

-:31: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#31: FILE: drivers/gpu/drm/i915/intel_dp.c:1868:
+		if ((!intel_edp_compare_alt_mode(req_mode, panel_mode) &&
+		    intel_psr_setup_time_valid(intel_dp, req_mode)) ||

-:45: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#45: FILE: drivers/gpu/drm/i915/intel_drv.h:1912:
+bool intel_psr_setup_time_valid(struct intel_dp *intel_dp,
+				    const struct drm_display_mode *mode);

total: 0 errors, 0 warnings, 3 checks, 71 lines checked

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

[PATCH V3] drn/i915/edp: Only use alternate fixed mode when requested

[clinton.a.taylor]

From: Clint Taylor <clinton.a.taylor@intel.com>

In commit dc911f5bd8aa ("drm/i915/edp: Allow alternate fixed mode for eDP
if available."), the patch was always selecting the alternate refresh rate
even though user space was asking for the higher rate. This patch confirms
the alt mode setup time meets requirements and only uses the alt mode if
psr is enable for the platform.

V2: use clock instead of vrefresh for compare.
V3: Confirm PSR is enabled and alt mode setup time is valid

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105469

Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c  |  4 +++-
 drivers/gpu/drm/i915/intel_drv.h |  2 ++
 drivers/gpu/drm/i915/intel_psr.c | 35 +++++++++++++++++++++++------------
 3 files changed, 28 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 83da50b..59b8a5c 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1864,7 +1864,9 @@ static bool intel_edp_compare_alt_mode(struct drm_display_mode *m1,
 			intel_connector->panel.alt_fixed_mode;
 		struct drm_display_mode *req_mode = &pipe_config->base.mode;
 
-		if (!intel_edp_compare_alt_mode(req_mode, panel_mode))
+		if ((!intel_edp_compare_alt_mode(req_mode, panel_mode) &&
+		    intel_psr_setup_time_valid(intel_dp, req_mode)) ||
+		    (!i915_modparams.enable_psr))
 			panel_mode = intel_connector->panel.fixed_mode;
 
 		drm_mode_debug_printmodeline(panel_mode);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 11a1932..e69ce10 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1908,6 +1908,8 @@ void intel_psr_flush(struct drm_i915_private *dev_priv,
 void intel_psr_init(struct drm_i915_private *dev_priv);
 void intel_psr_single_frame_update(struct drm_i915_private *dev_priv,
 				   unsigned frontbuffer_bits);
+bool intel_psr_setup_time_valid(struct intel_dp *intel_dp,
+				    const struct drm_display_mode *mode);
 void intel_psr_compute_config(struct intel_dp *intel_dp,
 			      struct intel_crtc_state *crtc_state);
 void intel_psr_irq_control(struct drm_i915_private *dev_priv, bool debug);
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 6233a32..9cc4109 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -578,6 +578,27 @@ static bool intel_psr2_config_valid(struct intel_dp *intel_dp,
 	return true;
 }
 
+bool intel_psr_setup_time_valid(struct intel_dp *intel_dp, const struct drm_display_mode *mode)
+{
+	int psr_setup_time = 0;
+
+	psr_setup_time = drm_dp_psr_setup_time(intel_dp->psr_dpcd);
+	if (psr_setup_time < 0) {
+		DRM_DEBUG_KMS("PSR condition failed: Invalid PSR setup time (0x%02x)\n",
+			      intel_dp->psr_dpcd[1]);
+		return false;
+	}
+
+	if (intel_usecs_to_scanlines(mode, psr_setup_time) >
+	    mode->crtc_vtotal - mode->crtc_vdisplay - 1) {
+		DRM_DEBUG_KMS("PSR condition failed: PSR setup time (%d us) too long\n",
+			      psr_setup_time);
+		return false;
+	}
+
+	return true;
+}
+
 void intel_psr_compute_config(struct intel_dp *intel_dp,
 			      struct intel_crtc_state *crtc_state)
 {
@@ -585,7 +606,6 @@ void intel_psr_compute_config(struct intel_dp *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 psr_setup_time;
 
 	if (!CAN_PSR(dev_priv))
 		return;
@@ -626,17 +646,8 @@ void intel_psr_compute_config(struct intel_dp *intel_dp,
 		return;
 	}
 
-	psr_setup_time = drm_dp_psr_setup_time(intel_dp->psr_dpcd);
-	if (psr_setup_time < 0) {
-		DRM_DEBUG_KMS("PSR condition failed: Invalid PSR setup time (0x%02x)\n",
-			      intel_dp->psr_dpcd[1]);
-		return;
-	}
-
-	if (intel_usecs_to_scanlines(adjusted_mode, psr_setup_time) >
-	    adjusted_mode->crtc_vtotal - adjusted_mode->crtc_vdisplay - 1) {
-		DRM_DEBUG_KMS("PSR condition failed: PSR setup time (%d us) too long\n",
-			      psr_setup_time);
+	if (!intel_psr_setup_time_valid(intel_dp, adjusted_mode)) {
+		DRM_DEBUG_KMS("PSR Setup invalid\n");
 		return;
 	}
 
-- 
1.9.1

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

✓ Fi.CI.BAT: success for drm/i915/edp: Only use alternate fixed mode when requested (rev3)

[Patchwork]

== Series Details ==

Series: drm/i915/edp: Only use alternate fixed mode when requested (rev3)
URL   : https://patchwork.freedesktop.org/series/41492/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4116 -> Patchwork_8850 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/41492/revisions/3/mbox/

== Known issues ==

  Here are the changes found in Patchwork_8850 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_mmap_gtt@basic-small-bo-tiledx:
      fi-gdg-551:         PASS -> FAIL (fdo#102575)

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-snb-2520m:       PASS -> INCOMPLETE (fdo#103713)
      fi-ivb-3520m:       PASS -> DMESG-WARN (fdo#106084)

    
    ==== Possible fixes ====

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
      fi-bxt-dsi:         INCOMPLETE (fdo#103927) -> PASS

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


== Participating hosts (38 -> 35) ==

  Missing    (3): fi-ctg-p8600 fi-ilk-m540 fi-skl-6700hq 


== Build changes ==

    * Linux: CI_DRM_4116 -> Patchwork_8850

  CI_DRM_4116: 7ff375cb94000d93f7a9d541cb0c8180fbea80f2 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4453: 29ae12bd764e3b1876356e7628a32192b4ec9066 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_8850: 467a1ee16409f0a76e0e9a21a6ce38e9eed4e0b9 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4453: d0a0bca2194a673c4d9a70a2256837c59213c64b @ git://anongit.freedesktop.org/piglit


== Linux commits ==

467a1ee16409 drn/i915/edp: Only use alternate fixed mode when requested

== Logs ==

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

✓ Fi.CI.IGT: success for drm/i915/edp: Only use alternate fixed mode when requested (rev3)

[Patchwork]

== Series Details ==

Series: drm/i915/edp: Only use alternate fixed mode when requested (rev3)
URL   : https://patchwork.freedesktop.org/series/41492/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4116_full -> Patchwork_8850_full =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_8850_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_8850_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/41492/revisions/3/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_8850_full:

  === IGT changes ===

    ==== Warnings ====

    igt@gem_exec_schedule@deep-bsd2:
      shard-kbl:          SKIP -> PASS

    
== Known issues ==

  Here are the changes found in Patchwork_8850_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_cursor_legacy@cursor-vs-flip-toggle:
      shard-hsw:          PASS -> FAIL (fdo#103355)

    igt@kms_flip@2x-dpms-vs-vblank-race:
      shard-hsw:          PASS -> FAIL (fdo#103060)

    igt@kms_flip@absolute-wf_vblank-interruptible:
      shard-glk:          PASS -> FAIL (fdo#106087)

    igt@kms_flip@modeset-vs-vblank-race-interruptible:
      shard-glk:          PASS -> FAIL (fdo#103060)

    igt@kms_flip@wf_vblank-ts-check-interruptible:
      shard-apl:          PASS -> FAIL (fdo#105312, fdo#103933)

    igt@kms_frontbuffer_tracking@fbc-rgb101010-draw-render:
      shard-apl:          PASS -> FAIL (fdo#103167)

    
    ==== Possible fixes ====

    igt@gem_workarounds@suspend-resume:
      shard-kbl:          INCOMPLETE (fdo#103665) -> PASS

    igt@kms_flip@2x-plain-flip-ts-check:
      shard-hsw:          FAIL (fdo#103928) -> PASS

    igt@kms_flip@flip-vs-blocking-wf-vblank:
      shard-kbl:          DMESG-FAIL (fdo#103558) -> PASS +1

    igt@kms_flip@flip-vs-expired-vblank-interruptible:
      shard-glk:          FAIL (fdo#102887) -> PASS

    igt@kms_flip@modeset-vs-vblank-race-interruptible:
      shard-kbl:          DMESG-WARN (fdo#105602, fdo#103558, fdo#103313) -> PASS

    igt@kms_flip@plain-flip-fb-recreate-interruptible:
      shard-glk:          FAIL (fdo#100368) -> PASS +2

    igt@kms_flip_event_leak:
      shard-kbl:          DMESG-WARN (fdo#105602, fdo#103558) -> PASS +33

    
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
  fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103313 https://bugs.freedesktop.org/show_bug.cgi?id=103313
  fdo#103355 https://bugs.freedesktop.org/show_bug.cgi?id=103355
  fdo#103558 https://bugs.freedesktop.org/show_bug.cgi?id=103558
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#103928 https://bugs.freedesktop.org/show_bug.cgi?id=103928
  fdo#103933 https://bugs.freedesktop.org/show_bug.cgi?id=103933
  fdo#105312 https://bugs.freedesktop.org/show_bug.cgi?id=105312
  fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602
  fdo#106087 https://bugs.freedesktop.org/show_bug.cgi?id=106087


== Participating hosts (9 -> 8) ==

  Missing    (1): shard-glkb 


== Build changes ==

    * Linux: CI_DRM_4116 -> Patchwork_8850

  CI_DRM_4116: 7ff375cb94000d93f7a9d541cb0c8180fbea80f2 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4453: 29ae12bd764e3b1876356e7628a32192b4ec9066 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_8850: 467a1ee16409f0a76e0e9a21a6ce38e9eed4e0b9 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4453: d0a0bca2194a673c4d9a70a2256837c59213c64b @ git://anongit.freedesktop.org/piglit

== Logs ==

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

Re: [PATCH V3] drn/i915/edp: Only use alternate fixed mode when requested

[Jani Nikula]

On Mon, 30 Apr 2018, clinton.a.taylor@intel.com wrote:
> From: Clint Taylor <clinton.a.taylor@intel.com>
>
> In commit dc911f5bd8aa ("drm/i915/edp: Allow alternate fixed mode for eDP
> if available."), the patch was always selecting the alternate refresh rate
> even though user space was asking for the higher rate. This patch confirms
> the alt mode setup time meets requirements and only uses the alt mode if
> psr is enable for the platform.

I'm confused.

If the panel supports two refresh rates, why would PSR have to be a
requirement for selecting one of them? If we now have bugs about only
being able to select one, won't we then get bugs about only being able
to select the other?

I don't think this should be tied to PSR.

BR,
Jani.

>
> V2: use clock instead of vrefresh for compare.
> V3: Confirm PSR is enabled and alt mode setup time is valid
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105469
>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c  |  4 +++-
>  drivers/gpu/drm/i915/intel_drv.h |  2 ++
>  drivers/gpu/drm/i915/intel_psr.c | 35 +++++++++++++++++++++++------------
>  3 files changed, 28 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 83da50b..59b8a5c 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1864,7 +1864,9 @@ static bool intel_edp_compare_alt_mode(struct drm_display_mode *m1,
>  			intel_connector->panel.alt_fixed_mode;
>  		struct drm_display_mode *req_mode = &pipe_config->base.mode;
>  
> -		if (!intel_edp_compare_alt_mode(req_mode, panel_mode))
> +		if ((!intel_edp_compare_alt_mode(req_mode, panel_mode) &&
> +		    intel_psr_setup_time_valid(intel_dp, req_mode)) ||
> +		    (!i915_modparams.enable_psr))
>  			panel_mode = intel_connector->panel.fixed_mode;
>  
>  		drm_mode_debug_printmodeline(panel_mode);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 11a1932..e69ce10 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1908,6 +1908,8 @@ void intel_psr_flush(struct drm_i915_private *dev_priv,
>  void intel_psr_init(struct drm_i915_private *dev_priv);
>  void intel_psr_single_frame_update(struct drm_i915_private *dev_priv,
>  				   unsigned frontbuffer_bits);
> +bool intel_psr_setup_time_valid(struct intel_dp *intel_dp,
> +				    const struct drm_display_mode *mode);
>  void intel_psr_compute_config(struct intel_dp *intel_dp,
>  			      struct intel_crtc_state *crtc_state);
>  void intel_psr_irq_control(struct drm_i915_private *dev_priv, bool debug);
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 6233a32..9cc4109 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -578,6 +578,27 @@ static bool intel_psr2_config_valid(struct intel_dp *intel_dp,
>  	return true;
>  }
>  
> +bool intel_psr_setup_time_valid(struct intel_dp *intel_dp, const struct drm_display_mode *mode)
> +{
> +	int psr_setup_time = 0;
> +
> +	psr_setup_time = drm_dp_psr_setup_time(intel_dp->psr_dpcd);
> +	if (psr_setup_time < 0) {
> +		DRM_DEBUG_KMS("PSR condition failed: Invalid PSR setup time (0x%02x)\n",
> +			      intel_dp->psr_dpcd[1]);
> +		return false;
> +	}
> +
> +	if (intel_usecs_to_scanlines(mode, psr_setup_time) >
> +	    mode->crtc_vtotal - mode->crtc_vdisplay - 1) {
> +		DRM_DEBUG_KMS("PSR condition failed: PSR setup time (%d us) too long\n",
> +			      psr_setup_time);
> +		return false;
> +	}
> +
> +	return true;
> +}
> +
>  void intel_psr_compute_config(struct intel_dp *intel_dp,
>  			      struct intel_crtc_state *crtc_state)
>  {
> @@ -585,7 +606,6 @@ void intel_psr_compute_config(struct intel_dp *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 psr_setup_time;
>  
>  	if (!CAN_PSR(dev_priv))
>  		return;
> @@ -626,17 +646,8 @@ void intel_psr_compute_config(struct intel_dp *intel_dp,
>  		return;
>  	}
>  
> -	psr_setup_time = drm_dp_psr_setup_time(intel_dp->psr_dpcd);
> -	if (psr_setup_time < 0) {
> -		DRM_DEBUG_KMS("PSR condition failed: Invalid PSR setup time (0x%02x)\n",
> -			      intel_dp->psr_dpcd[1]);
> -		return;
> -	}
> -
> -	if (intel_usecs_to_scanlines(adjusted_mode, psr_setup_time) >
> -	    adjusted_mode->crtc_vtotal - adjusted_mode->crtc_vdisplay - 1) {
> -		DRM_DEBUG_KMS("PSR condition failed: PSR setup time (%d us) too long\n",
> -			      psr_setup_time);
> +	if (!intel_psr_setup_time_valid(intel_dp, adjusted_mode)) {
> +		DRM_DEBUG_KMS("PSR Setup invalid\n");
>  		return;
>  	}

-- 
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 V3] drn/i915/edp: Only use alternate fixed mode when requested

[Rodrigo Vivi]

On Wed, May 02, 2018 at 10:22:16AM +0300, Jani Nikula wrote:
> On Mon, 30 Apr 2018, clinton.a.taylor@intel.com wrote:
> > From: Clint Taylor <clinton.a.taylor@intel.com>
> >
> > In commit dc911f5bd8aa ("drm/i915/edp: Allow alternate fixed mode for eDP
> > if available."), the patch was always selecting the alternate refresh rate
> > even though user space was asking for the higher rate. This patch confirms
> > the alt mode setup time meets requirements and only uses the alt mode if
> > psr is enable for the platform.
> 
> I'm confused.
> 
> If the panel supports two refresh rates, why would PSR have to be a
> requirement for selecting one of them? If we now have bugs about only
> being able to select one, won't we then get bugs about only being able
> to select the other?
> 
> I don't think this should be tied to PSR.

The biggest reason behind the initial test was to allow us to select
the 48Hz mode in order to test PSR in most of the panels we had around here
since with 60Hz the vblank period was to short for PSR.

But it seems we introduced a bad bug by doing that. Probably a revert is
the best option here since I agree with Jani that we shouldn't do any
mode selection based on a feture like PSR enabled.

Well, I honestly believe there should be an easy way... If 48Hz is available and
requested we should respect that and if we don't select anything we stick with
the preferred one that is probably 60Hz. But since we are not able to find
the correct way of allowing that in a clean way let's just revert the original
patch.

Thanks,
Rodrigo.

> 
> BR,
> Jani.
> 
> >
> > V2: use clock instead of vrefresh for compare.
> > V3: Confirm PSR is enabled and alt mode setup time is valid
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105469
> >
> > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c  |  4 +++-
> >  drivers/gpu/drm/i915/intel_drv.h |  2 ++
> >  drivers/gpu/drm/i915/intel_psr.c | 35 +++++++++++++++++++++++------------
> >  3 files changed, 28 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 83da50b..59b8a5c 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -1864,7 +1864,9 @@ static bool intel_edp_compare_alt_mode(struct drm_display_mode *m1,
> >  			intel_connector->panel.alt_fixed_mode;
> >  		struct drm_display_mode *req_mode = &pipe_config->base.mode;
> >  
> > -		if (!intel_edp_compare_alt_mode(req_mode, panel_mode))
> > +		if ((!intel_edp_compare_alt_mode(req_mode, panel_mode) &&
> > +		    intel_psr_setup_time_valid(intel_dp, req_mode)) ||
> > +		    (!i915_modparams.enable_psr))
> >  			panel_mode = intel_connector->panel.fixed_mode;
> >  
> >  		drm_mode_debug_printmodeline(panel_mode);
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 11a1932..e69ce10 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1908,6 +1908,8 @@ void intel_psr_flush(struct drm_i915_private *dev_priv,
> >  void intel_psr_init(struct drm_i915_private *dev_priv);
> >  void intel_psr_single_frame_update(struct drm_i915_private *dev_priv,
> >  				   unsigned frontbuffer_bits);
> > +bool intel_psr_setup_time_valid(struct intel_dp *intel_dp,
> > +				    const struct drm_display_mode *mode);
> >  void intel_psr_compute_config(struct intel_dp *intel_dp,
> >  			      struct intel_crtc_state *crtc_state);
> >  void intel_psr_irq_control(struct drm_i915_private *dev_priv, bool debug);
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> > index 6233a32..9cc4109 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -578,6 +578,27 @@ static bool intel_psr2_config_valid(struct intel_dp *intel_dp,
> >  	return true;
> >  }
> >  
> > +bool intel_psr_setup_time_valid(struct intel_dp *intel_dp, const struct drm_display_mode *mode)
> > +{
> > +	int psr_setup_time = 0;
> > +
> > +	psr_setup_time = drm_dp_psr_setup_time(intel_dp->psr_dpcd);
> > +	if (psr_setup_time < 0) {
> > +		DRM_DEBUG_KMS("PSR condition failed: Invalid PSR setup time (0x%02x)\n",
> > +			      intel_dp->psr_dpcd[1]);
> > +		return false;
> > +	}
> > +
> > +	if (intel_usecs_to_scanlines(mode, psr_setup_time) >
> > +	    mode->crtc_vtotal - mode->crtc_vdisplay - 1) {
> > +		DRM_DEBUG_KMS("PSR condition failed: PSR setup time (%d us) too long\n",
> > +			      psr_setup_time);
> > +		return false;
> > +	}
> > +
> > +	return true;
> > +}
> > +
> >  void intel_psr_compute_config(struct intel_dp *intel_dp,
> >  			      struct intel_crtc_state *crtc_state)
> >  {
> > @@ -585,7 +606,6 @@ void intel_psr_compute_config(struct intel_dp *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 psr_setup_time;
> >  
> >  	if (!CAN_PSR(dev_priv))
> >  		return;
> > @@ -626,17 +646,8 @@ void intel_psr_compute_config(struct intel_dp *intel_dp,
> >  		return;
> >  	}
> >  
> > -	psr_setup_time = drm_dp_psr_setup_time(intel_dp->psr_dpcd);
> > -	if (psr_setup_time < 0) {
> > -		DRM_DEBUG_KMS("PSR condition failed: Invalid PSR setup time (0x%02x)\n",
> > -			      intel_dp->psr_dpcd[1]);
> > -		return;
> > -	}
> > -
> > -	if (intel_usecs_to_scanlines(adjusted_mode, psr_setup_time) >
> > -	    adjusted_mode->crtc_vtotal - adjusted_mode->crtc_vdisplay - 1) {
> > -		DRM_DEBUG_KMS("PSR condition failed: PSR setup time (%d us) too long\n",
> > -			      psr_setup_time);
> > +	if (!intel_psr_setup_time_valid(intel_dp, adjusted_mode)) {
> > +		DRM_DEBUG_KMS("PSR Setup invalid\n");
> >  		return;
> >  	}
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx