On Fri, Jul 29, 2022 at 12:57:40PM -0700, Doug Anderson wrote: > Hi, > > On Fri, Jul 29, 2022 at 9:41 AM Maxime Ripard <maxime@xxxxxxxxxx> wrote: > > > > On Fri, Jul 29, 2022 at 07:50:20AM -0700, Doug Anderson wrote: > > > On Fri, Jul 29, 2022 at 12:51 AM Maxime Ripard <maxime@xxxxxxxxxx> wrote: > > > > > > > > On Thu, Jul 28, 2022 at 02:18:38PM -0700, Doug Anderson wrote: > > > > > Hi, > > > > > > > > > > On Thu, Jul 28, 2022 at 10:34 AM Abhinav Kumar > > > > > <quic_abhinavk@xxxxxxxxxxx> wrote: > > > > > > > > > > > > Hi Rob and Doug > > > > > > > > > > > > On 7/22/2022 10:36 AM, Rob Clark wrote: > > > > > > > On Fri, Jul 22, 2022 at 9:48 AM Doug Anderson <dianders@xxxxxxxxxxxx> wrote: > > > > > > >> > > > > > > >> Hi, > > > > > > >> > > > > > > >> On Fri, Jul 22, 2022 at 9:37 AM Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx> wrote: > > > > > > >>> > > > > > > >>> + sankeerth > > > > > > >>> > > > > > > >>> Hi Doug > > > > > > >>> > > > > > > >>> On 7/21/2022 3:23 PM, Douglas Anderson wrote: > > > > > > >>>> The Sharp LQ140M1JW46 panel is on the Qualcomm sc7280 CRD reference > > > > > > >>>> board. This panel supports 144 Hz and 60 Hz. In the EDID, the 144 Hz > > > > > > >>>> mode is listed first and thus is marked preferred. The EDID decode I > > > > > > >>>> ran says: > > > > > > >>>> > > > > > > >>>> First detailed timing includes the native pixel format and preferred > > > > > > >>>> refresh rate. > > > > > > >>>> > > > > > > >>>> ... > > > > > > >>>> > > > > > > >>>> Detailed Timing Descriptors: > > > > > > >>>> DTD 1: 1920x1080 143.981 Hz 16:9 166.587 kHz 346.500 MHz > > > > > > >>>> Hfront 48 Hsync 32 Hback 80 Hpol N > > > > > > >>>> Vfront 3 Vsync 5 Vback 69 Vpol N > > > > > > >>>> DTD 2: 1920x1080 59.990 Hz 16:9 69.409 kHz 144.370 MHz > > > > > > >>>> Hfront 48 Hsync 32 Hback 80 Hpol N > > > > > > >>>> Vfront 3 Vsync 5 Vback 69 Vpol N > > > > > > >>>> > > > > > > >>>> I'm proposing here that the above is actually a bug and that the 60 Hz > > > > > > >>>> mode really should be considered preferred by Linux. > > > > > > > > > > > > Its a bit tricky to say that this is a bug but I think we can certainly > > > > > > add here that for an internal display we would have ideally had the > > > > > > lower resolution first to indicate it as default. > > > > > > > > > > Yeah, it gets into the vagueness of the EDID spec in general. As far > > > > > as I can find it's really up to the monitor to decide by what means it > > > > > chooses the "preferred" refresh rate if the monitor can support many. > > > > > Some displays may decide that the normal rate is "preferred" and some > > > > > may decide that the high refresh rate is "preferred". Neither display > > > > > is "wrong" per say, but it's nice to have some consistency here and to > > > > > make it so that otherwise "dumb" userspace will get something > > > > > reasonable by default. I'll change it to say: > > > > > > > > > > While the EDID spec appears to allow a display to use any criteria for > > > > > picking which refresh mode is "preferred" or "optimal", that vagueness > > > > > is a bit annoying. From Linux's point of view let's choose the 60 Hz > > > > > one as the default. > > > > > > > > And if we start making that decision, it should be for all panels with a > > > > similar constraint, so most likely handled by the core, and the new > > > > policy properly documented. > > > > > > > > Doing that just for a single panel is weird. > > > > > > Yeah, though having a "general policy" in the core can be problematic. > > > > > > In general I think panel EDIDs are only trustworthy as far as you can > > > throw them. They are notorious for having wrong and incorrect > > > information, which is why the EDID quirk list exists to begin with. > > > Trying to change how we're going to interpret all EDIDs, even all > > > EDIDs for eDP panels, seems like it will break someone somewhere. > > > Maybe there are EDIDs out there that were only ever validated at the > > > higher refresh rate and they don't work / flicker / cause digitizer > > > noise at the lower refresh rate. Heck, we've seen eDP panel vendors > > > that can't even get their checksum correct, so I'm not sure I want to > > > make a global assertion that all panels validated their "secondary" > > > display mode. > > > > > > In this particular case, we have validated that this particular Sharp > > > panel works fine at the lower refresh rate. > > > > > > I would also note that, as far as I understand it, ODMs actually can > > > request different EDIDs from the panel vendors. In the past we have > > > been able to get panel vendors to change EDIDs. Thus for most panels > > > I'd expect that we would discover this early, change the EDID default, > > > and be done with it. The case here is a little unusual in that by the > > > time we got involved and started digging into this panel too many were > > > created and nobody wants to throw away those old panels. This is why > > > I'm treating it as a quirk/bug. Really: we should have updated the > > > EDID of the panel but we're unable to in this case. > > > > You raise some good points, but most of the discussion around that patch > > were mostly around performances, power consumption and so on. > > > > This is very much a policy decision, and if there is some panel where > > the EDID reports 60Hz but is broken, then that panel should be the > > exception to the policy > > > > But doing it for a single panel is just odd > > OK, fair enough. I'll abandon this patch at least as far as mainline > is concerned, then. That wasn't really my point though :) If you think that this change is needed, then we should totally discuss it and I'm not opposed to it. What I don't really like about this patch is that it's about a single panel: if we're doing it we should do it for all the panels. Where we do it can also be discussed, but we should remain consistent there. Maxime
Attachment:
signature.asc
Description: PGP signature
