Re: [PATCH 3/3] drm/panel-edp: Choose correct preferred mode

[Jani Nikula]

On Mon, 06 Nov 2023, Maxime Ripard <mripard@xxxxxxxxxx> wrote:
> On Thu, Nov 02, 2023 at 07:33:48AM -0700, Doug Anderson wrote:
>> Hi,
>> 
>> On Wed, Nov 1, 2023 at 11:31 PM Dmitry Baryshkov
>> <dmitry.baryshkov@xxxxxxxxxx> wrote:
>> >
>> > On Wed, 1 Nov 2023 at 23:26, Hsin-Yi Wang <hsinyi@xxxxxxxxxxxx> wrote:
>> > >
>> > > If a non generic edp-panel is under aux-bus, the mode read from edid would
>> > > still be selected as preferred and results in multiple preferred modes,
>> > > which is ambiguous.
>> > >
>> > > If a hard-coded mode is present, unset the preferred bit of the modes read
>> > > from edid.
>> >
>> > Can we skip the EDID completely if the hardcoded override is present?
>> 
>> Yeah, I wondered about that too. The blending of the hardcoded with
>> the EDID predates my involvement with the driver. You can see even as
>> of commit 280921de7241 ("drm/panel: Add simple panel support") that
>> the driver would start with the EDID modes (if it had them) and then
>> go onto add the hardcoded modes. At least for eDP panels, though,
>> nobody (or almost nobody?) actually provided panel-simple a DDC bus at
>> the same time it was given a hardcoded panel.
>> 
>> I guess I could go either way, but I have a slight bias to adding the
>> extra modes and just making it clear to userspace that none of them
>> are "preferred". That seems like it would give userspace the most
>> flexibility
>
> I disagree. "Flexibility" here just means "the way to shoot itself in
> the foot without knowing it's aiming at its foot".
>
> If a mode is broken, we shouldn't expose it, just like we don't for all
> modes that require a maximum frequency higher than what the controller
> can provide on HDMI for example.

Agreed. This is exactly what the ->mode_valid connector helper callback
is for.

>
>> and also is closer to what we've historically done (though,
>> historically, we just allowed there to be more than one "preferred"
>> mode).
>
> I have no idea what history you're referring to here
>
>> One thing we definitely want to do, though, is to still expose the
>> EDID to userspace even if we're using a hardcoded mode. I believe
>> that, at least on ChromeOS, there are some tools that look at the EDID
>> directly for some reason or another.
>
> If the EDID is known to be broken and unreliable, what's the point?

I don't think it's unheard of to have bogus modes in the EDID while
other stuff is required.

I think the current agreement pretty much is that the kernel handles the
modes, either from the EDID or some other channel, and the userspace
does not look at the EDID for modes.

BR,
Jani.



-- 
Jani Nikula, Intel