All of lore.kernel.org
 help / color / mirror / Atom feed
From: Doug Anderson <dianders@chromium.org>
To: Maxime Ripard <maxime@cerno.tech>
Cc: Abhinav Kumar <quic_abhinavk@quicinc.com>,
	Rob Clark <robdclark@gmail.com>,
	dri-devel <dri-devel@lists.freedesktop.org>,
	freedreno <freedreno@lists.freedesktop.org>,
	Daniel Vetter <daniel@ffwll.ch>, David Airlie <airlied@linux.ie>,
	Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	Thomas Zimmermann <tzimmermann@suse.de>,
	LKML <linux-kernel@vger.kernel.org>,
	Sankeerth Billakanti <quic_sbillaka@quicinc.com>
Subject: Re: [RFC PATCH] drm/edid: Make 144 Hz not preferred on Sharp LQ140M1JW46
Date: Fri, 29 Jul 2022 12:57:40 -0700	[thread overview]
Message-ID: <CAD=FV=UKYksHjuVR27DPdUFFtJrQKB2KbT08qjeYLNW_3y_Mfg@mail.gmail.com> (raw)
In-Reply-To: <20220729164136.opucqg64qz4ypmvo@penduick>

Hi,

On Fri, Jul 29, 2022 at 9:41 AM Maxime Ripard <maxime@cerno.tech> 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@cerno.tech> 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@quicinc.com> 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@chromium.org> wrote:
> > > > > >>
> > > > > >> Hi,
> > > > > >>
> > > > > >> On Fri, Jul 22, 2022 at 9:37 AM Abhinav Kumar <quic_abhinavk@quicinc.com> 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.

-Doug

WARNING: multiple messages have this Message-ID (diff)
From: Doug Anderson <dianders@chromium.org>
To: Maxime Ripard <maxime@cerno.tech>
Cc: Sankeerth Billakanti <quic_sbillaka@quicinc.com>,
	Thomas Zimmermann <tzimmermann@suse.de>,
	David Airlie <airlied@linux.ie>,
	Abhinav Kumar <quic_abhinavk@quicinc.com>,
	dri-devel <dri-devel@lists.freedesktop.org>,
	LKML <linux-kernel@vger.kernel.org>,
	freedreno <freedreno@lists.freedesktop.org>
Subject: Re: [RFC PATCH] drm/edid: Make 144 Hz not preferred on Sharp LQ140M1JW46
Date: Fri, 29 Jul 2022 12:57:40 -0700	[thread overview]
Message-ID: <CAD=FV=UKYksHjuVR27DPdUFFtJrQKB2KbT08qjeYLNW_3y_Mfg@mail.gmail.com> (raw)
In-Reply-To: <20220729164136.opucqg64qz4ypmvo@penduick>

Hi,

On Fri, Jul 29, 2022 at 9:41 AM Maxime Ripard <maxime@cerno.tech> 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@cerno.tech> 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@quicinc.com> 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@chromium.org> wrote:
> > > > > >>
> > > > > >> Hi,
> > > > > >>
> > > > > >> On Fri, Jul 22, 2022 at 9:37 AM Abhinav Kumar <quic_abhinavk@quicinc.com> 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.

-Doug

  reply	other threads:[~2022-07-29 19:58 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-21 22:23 [RFC PATCH] drm/edid: Make 144 Hz not preferred on Sharp LQ140M1JW46 Douglas Anderson
2022-07-21 22:23 ` Douglas Anderson
2022-07-22 16:37 ` Abhinav Kumar
2022-07-22 16:37   ` Abhinav Kumar
2022-07-22 16:48   ` Doug Anderson
2022-07-22 16:48     ` Doug Anderson
2022-07-22 17:36     ` Rob Clark
2022-07-22 17:36       ` Rob Clark
2022-07-28 17:34       ` Abhinav Kumar
2022-07-28 17:34         ` Abhinav Kumar
2022-07-28 21:18         ` Doug Anderson
2022-07-28 21:18           ` Doug Anderson
2022-07-29  7:51           ` Maxime Ripard
2022-07-29  7:51             ` Maxime Ripard
2022-07-29 14:50             ` Doug Anderson
2022-07-29 14:50               ` Doug Anderson
2022-07-29 16:41               ` Maxime Ripard
2022-07-29 16:41                 ` Maxime Ripard
2022-07-29 19:57                 ` Doug Anderson [this message]
2022-07-29 19:57                   ` Doug Anderson
2022-08-15  6:45                   ` Maxime Ripard
2022-08-15  6:45                     ` Maxime Ripard
2022-08-17 23:45                     ` Doug Anderson
2022-08-17 23:45                       ` Doug Anderson
2022-08-18 10:31                       ` Jani Nikula
2022-08-18 10:31                         ` Jani Nikula

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAD=FV=UKYksHjuVR27DPdUFFtJrQKB2KbT08qjeYLNW_3y_Mfg@mail.gmail.com' \
    --to=dianders@chromium.org \
    --cc=airlied@linux.ie \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=freedreno@lists.freedesktop.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=maxime@cerno.tech \
    --cc=quic_abhinavk@quicinc.com \
    --cc=quic_sbillaka@quicinc.com \
    --cc=robdclark@gmail.com \
    --cc=tzimmermann@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.