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

[Jani Nikula <jani.nikula@linux.intel.com>]

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