On 4/26/2022 1:52 PM, Doug Anderson wrote:
> Hi,
>
> On Tue, Apr 26, 2022 at 1:46 PM Abhinav Kumar <[email protected]> wrote:
>>
>> On 4/26/2022 1:21 PM, Douglas Anderson wrote:
>>> If we're unable to read the EDID for a display because it's corrupt /
>>> bogus / invalid then we'll add a set of standard modes for the
>>> display. When userspace looks at these modes it doesn't really have a
>>> good concept for which mode to pick and it'll likely pick the highest
>>> resolution one by default. That's probably not ideal because the modes
>>> were purely guesses on the part of the Linux kernel.
>>>
>>> Let's instead set 640x480 as the "preferred" mode when we have no EDID.
>>>
>>> Signed-off-by: Douglas Anderson <[email protected]>
>>
>> drm_dmt_modes array is sorted but you are also relying on this check to
>> eliminate the non-60fps modes
>>
>> 5611 if (drm_mode_vrefresh(ptr) > 61)
>> 5612 continue;
>>
>> I am not sure why we filter out the modes > 61 vrefresh.
>>
>> If that check will remain this is okay.
>>
>> If its not, its not reliable that the first mode will be 640x480@60
>
> I suspect that the check will remain. I guess I could try to do
> something fancier if people want, but I'd be interested in _what_
> fancier thing I should do if so. Do we want the rule to remain that we
> always prefer 640x480, or do we want to prefer the lowest resolution?
> ...do we want to prefer 60 Hz or the lowest refresh rate? Do we do
> this only for DP (which explicitly calls out 640x480 @60Hz as the best
> failsafe) or for everything?
>
> For now, the way it's coded up seems reasonable (to me). It's the
> lowest resolution _and_ it's 640x480 just because of the current
> values of the table. I suspect that extra lower resolution failsafe
> modes won't be added, but we can always change the rules here if/when
> they are.
>
> -Doug
Alright, agreed. The way the API is today, I dont see anything getting
broken with this.
So typically, as per spec, when a preferred mode is not set by the sink,
the first entry becomes the preferred mode.
This also aligns with that expectation.
So FWIW,
Reviewed-by: Abhinav Kumar <[email protected]>
We will test this one also out with our equipment, then give tested-by tags.
Thanks
Abhinav
Tested-by: Kuogee Hsieh <[email protected]>
On 4/26/2022 2:21 PM, Abhinav Kumar wrote:
>
>
> On 4/26/2022 1:52 PM, Doug Anderson wrote:
>> Hi,
>>
>> On Tue, Apr 26, 2022 at 1:46 PM Abhinav Kumar
>> <[email protected]> wrote:
>>>
>>> On 4/26/2022 1:21 PM, Douglas Anderson wrote:
>>>> If we're unable to read the EDID for a display because it's corrupt /
>>>> bogus / invalid then we'll add a set of standard modes for the
>>>> display. When userspace looks at these modes it doesn't really have a
>>>> good concept for which mode to pick and it'll likely pick the highest
>>>> resolution one by default. That's probably not ideal because the modes
>>>> were purely guesses on the part of the Linux kernel.
>>>>
>>>> Let's instead set 640x480 as the "preferred" mode when we have no
>>>> EDID.
>>>>
>>>> Signed-off-by: Douglas Anderson <[email protected]>
>>>
>>> drm_dmt_modes array is sorted but you are also relying on this check to
>>> eliminate the non-60fps modes
>>>
>>> 5611 if (drm_mode_vrefresh(ptr) > 61)
>>> 5612 continue;
>>>
>>> I am not sure why we filter out the modes > 61 vrefresh.
>>>
>>> If that check will remain this is okay.
>>>
>>> If its not, its not reliable that the first mode will be 640x480@60
>>
>> I suspect that the check will remain. I guess I could try to do
>> something fancier if people want, but I'd be interested in _what_
>> fancier thing I should do if so. Do we want the rule to remain that we
>> always prefer 640x480, or do we want to prefer the lowest resolution?
>> ...do we want to prefer 60 Hz or the lowest refresh rate? Do we do
>> this only for DP (which explicitly calls out 640x480 @60Hz as the best
>> failsafe) or for everything?
>>
>> For now, the way it's coded up seems reasonable (to me). It's the
>> lowest resolution _and_ it's 640x480 just because of the current
>> values of the table. I suspect that extra lower resolution failsafe
>> modes won't be added, but we can always change the rules here if/when
>> they are.
>>
>> -Doug
>
> Alright, agreed. The way the API is today, I dont see anything getting
> broken with this.
>
> So typically, as per spec, when a preferred mode is not set by the
> sink, the first entry becomes the preferred mode.
>
> This also aligns with that expectation.
>
> So FWIW,
>
> Reviewed-by: Abhinav Kumar <[email protected]>
>
> We will test this one also out with our equipment, then give tested-by
> tags.
>
> Thanks
>
> Abhinav
>