Re: [RFC PATCH] drm/edid: drm_add_modes_noedid() should set lowest resolution as preferred

[Kuogee Hsieh]

Tested-by: Kuogee Hsieh <quic_khsieh@xxxxxxxxxxx>


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 
> > <quic_abhinavk@xxxxxxxxxxx> 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 <dianders@xxxxxxxxxxxx>
> > >
> > > 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 <quic_abhinavk@xxxxxxxxxxx>
>
> We will test this one also out with our equipment, then give tested-by 
> tags.
>
> Thanks
>
> Abhinav