Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Message-ID: <61d98a8a-1c0f-346a-1e66-2e647d2f3088@quicinc.com>
Date:   Tue, 26 Apr 2022 14:21:16 -0700
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.6.2
Subject: Re: [RFC PATCH] drm/edid: drm_add_modes_noedid() should set lowest
 resolution as preferred
Content-Language: en-US
To:     Doug Anderson <dianders@chromium.org>
CC:     Sankeerth Billakanti <quic_sbillaka@quicinc.com>,
        Thomas Zimmermann <tzimmermann@suse.de>,
        David Airlie <airlied@linux.ie>,
        linux-arm-msm <linux-arm-msm@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        dri-devel <dri-devel@lists.freedesktop.org>,
        Stephen Boyd <swboyd@chromium.org>,
        Dmitry Baryshkov <dmitry.baryshkov@linaro.org>,
        "Aravind Venkateswaran (QUIC)" <quic_aravindh@quicinc.com>,
        "Kuogee Hsieh (QUIC)" <quic_khsieh@quicinc.com>
References: <20220426132121.RFC.1.I31ec454f8d4ffce51a7708a8092f8a6f9c929092@changeid>
 <a21a6ad5-5ed3-6207-8af7-655d19197041@quicinc.com>
 <CAD=FV=XOWfz39imimoijNM14dUJNiwr8_aqPFCR=LmgH7yYzQQ@mail.gmail.com>
From:   Abhinav Kumar <quic_abhinavk@quicinc.com>
In-Reply-To: <CAD=FV=XOWfz39imimoijNM14dUJNiwr8_aqPFCR=LmgH7yYzQQ@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 4/26/2022 1:52 PM, Doug Anderson wrote:
> Hi,
> 
> On Tue, Apr 26, 2022 at 1:46 PM Abhinav Kumar <quic_abhinavk@quicinc.com> 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@chromium.org>
>>
>> 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@quicinc.com>

We will test this one also out with our equipment, then give tested-by tags.

Thanks

Abhinav