linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH] drm/edid: Make 144 Hz not preferred on Sharp LQ140M1JW46
@ 2022-07-21 22:23 Douglas Anderson
  2022-07-22 16:37 ` Abhinav Kumar
  0 siblings, 1 reply; 13+ messages in thread
From: Douglas Anderson @ 2022-07-21 22:23 UTC (permalink / raw)
  To: dri-devel
  Cc: Rob Clark, Abhinav Kumar, freedreno, Douglas Anderson,
	Daniel Vetter, David Airlie, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, linux-kernel

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.

The argument here is that this is a laptop panel and on a laptop we
know power will always be a concern. Presumably even if someone using
this panel wanted to use 144 Hz for some use cases they would only do
so dynamically and would still want the default to be 60 Hz.

Let's change the default to 60 Hz using a standard quirk.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

 drivers/gpu/drm/drm_edid.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index bbc25e3b7220..06ff8ba216af 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -160,6 +160,9 @@ static const struct edid_quirk {
 	EDID_QUIRK('S', 'A', 'M', 596, EDID_QUIRK_PREFER_LARGE_60),
 	EDID_QUIRK('S', 'A', 'M', 638, EDID_QUIRK_PREFER_LARGE_60),
 
+	/* 144 Hz should only be used when needed; it wastes power */
+	EDID_QUIRK('S', 'H', 'P', 0x1523, EDID_QUIRK_PREFER_LARGE_60),
+
 	/* Sony PVM-2541A does up to 12 bpc, but only reports max 8 bpc */
 	EDID_QUIRK('S', 'N', 'Y', 0x2541, EDID_QUIRK_FORCE_12BPC),
 
-- 
2.37.1.359.gd136c6c3e2-goog


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [RFC PATCH] drm/edid: Make 144 Hz not preferred on Sharp LQ140M1JW46
  2022-07-21 22:23 [RFC PATCH] drm/edid: Make 144 Hz not preferred on Sharp LQ140M1JW46 Douglas Anderson
@ 2022-07-22 16:37 ` Abhinav Kumar
  2022-07-22 16:48   ` Doug Anderson
  0 siblings, 1 reply; 13+ messages in thread
From: Abhinav Kumar @ 2022-07-22 16:37 UTC (permalink / raw)
  To: Douglas Anderson, dri-devel
  Cc: Rob Clark, freedreno, Daniel Vetter, David Airlie,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	linux-kernel, Sankeerth Billakanti

+ 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.
> 
> The argument here is that this is a laptop panel and on a laptop we
> know power will always be a concern. Presumably even if someone using
> this panel wanted to use 144 Hz for some use cases they would only do
> so dynamically and would still want the default to be 60 Hz.
> 
> Let's change the default to 60 Hz using a standard quirk.
> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org>

Yes, we were aware that 144Hz was getting picked. We found that while 
debugging the screen corruption issue.

Well, yes power would be less with 60Hz but so will be the performance.

The test teams have been validating with 144Hz so far so we are checking 
internally with the team whether its OKAY to goto 60Hz now since that 
kind of invalidates the testing they have been doing.

Will update this thread once we finalize.


> ---
> 
>   drivers/gpu/drm/drm_edid.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index bbc25e3b7220..06ff8ba216af 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -160,6 +160,9 @@ static const struct edid_quirk {
>   	EDID_QUIRK('S', 'A', 'M', 596, EDID_QUIRK_PREFER_LARGE_60),
>   	EDID_QUIRK('S', 'A', 'M', 638, EDID_QUIRK_PREFER_LARGE_60),
>   
> +	/* 144 Hz should only be used when needed; it wastes power */
> +	EDID_QUIRK('S', 'H', 'P', 0x1523, EDID_QUIRK_PREFER_LARGE_60),
> +
>   	/* Sony PVM-2541A does up to 12 bpc, but only reports max 8 bpc */
>   	EDID_QUIRK('S', 'N', 'Y', 0x2541, EDID_QUIRK_FORCE_12BPC),
>   

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC PATCH] drm/edid: Make 144 Hz not preferred on Sharp LQ140M1JW46
  2022-07-22 16:37 ` Abhinav Kumar
@ 2022-07-22 16:48   ` Doug Anderson
  2022-07-22 17:36     ` Rob Clark
  0 siblings, 1 reply; 13+ messages in thread
From: Doug Anderson @ 2022-07-22 16:48 UTC (permalink / raw)
  To: Abhinav Kumar
  Cc: dri-devel, Rob Clark, freedreno, Daniel Vetter, David Airlie,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, LKML,
	Sankeerth Billakanti

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.
> >
> > The argument here is that this is a laptop panel and on a laptop we
> > know power will always be a concern. Presumably even if someone using
> > this panel wanted to use 144 Hz for some use cases they would only do
> > so dynamically and would still want the default to be 60 Hz.
> >
> > Let's change the default to 60 Hz using a standard quirk.
> >
> > Signed-off-by: Douglas Anderson <dianders@chromium.org>
>
> Yes, we were aware that 144Hz was getting picked. We found that while
> debugging the screen corruption issue.
>
> Well, yes power would be less with 60Hz but so will be the performance.

What performance specifically will be less with 60 Hz? In general the
sc7280 CPU is a bit memory-bandwidth constrained and the LCD refresh
from memory is a non-trivial part of that. Reducing to 60 Hz will
relieve some of the memory bandwidth pressure and will actually allow
tasks on the CPU to run _faster_. I guess the downside is that some
animations might be a little less smooth...


> The test teams have been validating with 144Hz so far so we are checking
> internally with the team whether its OKAY to goto 60Hz now since that
> kind of invalidates the testing they have been doing.

You're worried that the panel itself won't work well at 60 Hz, or
something else about the system won't? The whole system in general
needs to work well with 60 Hz displays and I expect them to be much
more common than 144 Hz displays. Quite honestly if switching to 60 Hz
uncovers a problem that would be a huge benefit of landing this patch
because it would mean we'd find it now rather than down the road when
someone hooks up a different panel.

-Doug

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC PATCH] drm/edid: Make 144 Hz not preferred on Sharp LQ140M1JW46
  2022-07-22 16:48   ` Doug Anderson
@ 2022-07-22 17:36     ` Rob Clark
  2022-07-28 17:34       ` Abhinav Kumar
  0 siblings, 1 reply; 13+ messages in thread
From: Rob Clark @ 2022-07-22 17:36 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Abhinav Kumar, dri-devel, freedreno, Daniel Vetter, David Airlie,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, LKML,
	Sankeerth Billakanti

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.
> > >
> > > The argument here is that this is a laptop panel and on a laptop we
> > > know power will always be a concern. Presumably even if someone using
> > > this panel wanted to use 144 Hz for some use cases they would only do
> > > so dynamically and would still want the default to be 60 Hz.
> > >
> > > Let's change the default to 60 Hz using a standard quirk.
> > >
> > > Signed-off-by: Douglas Anderson <dianders@chromium.org>
> >
> > Yes, we were aware that 144Hz was getting picked. We found that while
> > debugging the screen corruption issue.
> >
> > Well, yes power would be less with 60Hz but so will be the performance.
>
> What performance specifically will be less with 60 Hz? In general the
> sc7280 CPU is a bit memory-bandwidth constrained and the LCD refresh
> from memory is a non-trivial part of that. Reducing to 60 Hz will
> relieve some of the memory bandwidth pressure and will actually allow
> tasks on the CPU to run _faster_. I guess the downside is that some
> animations might be a little less smooth...

I guess he is referring to something that is vblank sync'd running
faster than 60fps.

but OTOH it is a bit of a waste for fbcon to be using 144Hz.  And
there are enough android games that limit themselves to 30fps to save
your "phone" battery.  So it seems a lot more sane to default to 60Hz
and let userspace that knows it wants more pick the 144Hz rate when
needed.

BR,
-R

>
>
> > The test teams have been validating with 144Hz so far so we are checking
> > internally with the team whether its OKAY to goto 60Hz now since that
> > kind of invalidates the testing they have been doing.
>
> You're worried that the panel itself won't work well at 60 Hz, or
> something else about the system won't? The whole system in general
> needs to work well with 60 Hz displays and I expect them to be much
> more common than 144 Hz displays. Quite honestly if switching to 60 Hz
> uncovers a problem that would be a huge benefit of landing this patch
> because it would mean we'd find it now rather than down the road when
> someone hooks up a different panel.
>
> -Doug

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC PATCH] drm/edid: Make 144 Hz not preferred on Sharp LQ140M1JW46
  2022-07-22 17:36     ` Rob Clark
@ 2022-07-28 17:34       ` Abhinav Kumar
  2022-07-28 21:18         ` Doug Anderson
  0 siblings, 1 reply; 13+ messages in thread
From: Abhinav Kumar @ 2022-07-28 17:34 UTC (permalink / raw)
  To: Rob Clark, Doug Anderson
  Cc: dri-devel, freedreno, Daniel Vetter, David Airlie,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, LKML,
	Sankeerth Billakanti

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.

>>>>
>>>> The argument here is that this is a laptop panel and on a laptop we
>>>> know power will always be a concern. Presumably even if someone using
>>>> this panel wanted to use 144 Hz for some use cases they would only do
>>>> so dynamically and would still want the default to be 60 Hz.
>>>>
>>>> Let's change the default to 60 Hz using a standard quirk.
>>>>
>>>> Signed-off-by: Douglas Anderson <dianders@chromium.org>
>>>
>>> Yes, we were aware that 144Hz was getting picked. We found that while
>>> debugging the screen corruption issue.
>>>
>>> Well, yes power would be less with 60Hz but so will be the performance.
>>
>> What performance specifically will be less with 60 Hz? In general the
>> sc7280 CPU is a bit memory-bandwidth constrained and the LCD refresh
>> from memory is a non-trivial part of that. Reducing to 60 Hz will
>> relieve some of the memory bandwidth pressure and will actually allow
>> tasks on the CPU to run _faster_. I guess the downside is that some
>> animations might be a little less smooth...
> 
> I guess he is referring to something that is vblank sync'd running
> faster than 60fps.
> 
> but OTOH it is a bit of a waste for fbcon to be using 144Hz.  And
> there are enough android games that limit themselves to 30fps to save
> your "phone" battery.  So it seems a lot more sane to default to 60Hz
> and let userspace that knows it wants more pick the 144Hz rate when
> needed.
> 
> BR,
> -R

Yes i was referring to vblank synced apps.

> 
>>
>>
>>> The test teams have been validating with 144Hz so far so we are checking
>>> internally with the team whether its OKAY to goto 60Hz now since that
>>> kind of invalidates the testing they have been doing.
>>
>> You're worried that the panel itself won't work well at 60 Hz, or
>> something else about the system won't? The whole system in general
>> needs to work well with 60 Hz displays and I expect them to be much
>> more common than 144 Hz displays. Quite honestly if switching to 60 Hz
>> uncovers a problem that would be a huge benefit of landing this patch
>> because it would mean we'd find it now rather than down the road when
>> someone hooks up a different panel.

I was worried that it will invalidate the testing they did so far but 
since you have confirmed that you would prefer 60Hz to be more 
thoroughly tested than 144Hz, I have informed the internal teams of this 
change and given the heads up.

You can have my R-b for this change,

Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>

I would also wait to see if others have different thought about this.

>>
>> -Doug

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC PATCH] drm/edid: Make 144 Hz not preferred on Sharp LQ140M1JW46
  2022-07-28 17:34       ` Abhinav Kumar
@ 2022-07-28 21:18         ` Doug Anderson
  2022-07-29  7:51           ` Maxime Ripard
  0 siblings, 1 reply; 13+ messages in thread
From: Doug Anderson @ 2022-07-28 21:18 UTC (permalink / raw)
  To: Abhinav Kumar
  Cc: Rob Clark, dri-devel, freedreno, Daniel Vetter, David Airlie,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, LKML,
	Sankeerth Billakanti

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.


> >>>> The argument here is that this is a laptop panel and on a laptop we
> >>>> know power will always be a concern. Presumably even if someone using
> >>>> this panel wanted to use 144 Hz for some use cases they would only do
> >>>> so dynamically and would still want the default to be 60 Hz.
> >>>>
> >>>> Let's change the default to 60 Hz using a standard quirk.
> >>>>
> >>>> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> >>>
> >>> Yes, we were aware that 144Hz was getting picked. We found that while
> >>> debugging the screen corruption issue.
> >>>
> >>> Well, yes power would be less with 60Hz but so will be the performance.
> >>
> >> What performance specifically will be less with 60 Hz? In general the
> >> sc7280 CPU is a bit memory-bandwidth constrained and the LCD refresh
> >> from memory is a non-trivial part of that. Reducing to 60 Hz will
> >> relieve some of the memory bandwidth pressure and will actually allow
> >> tasks on the CPU to run _faster_. I guess the downside is that some
> >> animations might be a little less smooth...
> >
> > I guess he is referring to something that is vblank sync'd running
> > faster than 60fps.
> >
> > but OTOH it is a bit of a waste for fbcon to be using 144Hz.  And
> > there are enough android games that limit themselves to 30fps to save
> > your "phone" battery.  So it seems a lot more sane to default to 60Hz
> > and let userspace that knows it wants more pick the 144Hz rate when
> > needed.
> >
> > BR,
> > -R
>
> Yes i was referring to vblank synced apps.
>
> >
> >>
> >>
> >>> The test teams have been validating with 144Hz so far so we are checking
> >>> internally with the team whether its OKAY to goto 60Hz now since that
> >>> kind of invalidates the testing they have been doing.
> >>
> >> You're worried that the panel itself won't work well at 60 Hz, or
> >> something else about the system won't? The whole system in general
> >> needs to work well with 60 Hz displays and I expect them to be much
> >> more common than 144 Hz displays. Quite honestly if switching to 60 Hz
> >> uncovers a problem that would be a huge benefit of landing this patch
> >> because it would mean we'd find it now rather than down the road when
> >> someone hooks up a different panel.
>
> I was worried that it will invalidate the testing they did so far but
> since you have confirmed that you would prefer 60Hz to be more
> thoroughly tested than 144Hz, I have informed the internal teams of this
> change and given the heads up.
>
> You can have my R-b for this change,
>
> Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
>
> I would also wait to see if others have different thought about this.

Thanks! I'll probably wait another week or so then I'll land in
drm-misc-next with your tag.

-Doug

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC PATCH] drm/edid: Make 144 Hz not preferred on Sharp LQ140M1JW46
  2022-07-28 21:18         ` Doug Anderson
@ 2022-07-29  7:51           ` Maxime Ripard
  2022-07-29 14:50             ` Doug Anderson
  0 siblings, 1 reply; 13+ messages in thread
From: Maxime Ripard @ 2022-07-29  7:51 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Abhinav Kumar, Rob Clark, dri-devel, freedreno, Daniel Vetter,
	David Airlie, Maarten Lankhorst, Thomas Zimmermann, LKML,
	Sankeerth Billakanti

[-- Attachment #1: Type: text/plain, Size: 2848 bytes --]

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.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC PATCH] drm/edid: Make 144 Hz not preferred on Sharp LQ140M1JW46
  2022-07-29  7:51           ` Maxime Ripard
@ 2022-07-29 14:50             ` Doug Anderson
  2022-07-29 16:41               ` Maxime Ripard
  0 siblings, 1 reply; 13+ messages in thread
From: Doug Anderson @ 2022-07-29 14:50 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Abhinav Kumar, Rob Clark, dri-devel, freedreno, Daniel Vetter,
	David Airlie, Maarten Lankhorst, Thomas Zimmermann, LKML,
	Sankeerth Billakanti

Hi,

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.

-Doug

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC PATCH] drm/edid: Make 144 Hz not preferred on Sharp LQ140M1JW46
  2022-07-29 14:50             ` Doug Anderson
@ 2022-07-29 16:41               ` Maxime Ripard
  2022-07-29 19:57                 ` Doug Anderson
  0 siblings, 1 reply; 13+ messages in thread
From: Maxime Ripard @ 2022-07-29 16:41 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Abhinav Kumar, Rob Clark, dri-devel, freedreno, Daniel Vetter,
	David Airlie, Maarten Lankhorst, Thomas Zimmermann, LKML,
	Sankeerth Billakanti

[-- Attachment #1: Type: text/plain, Size: 5161 bytes --]

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

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC PATCH] drm/edid: Make 144 Hz not preferred on Sharp LQ140M1JW46
  2022-07-29 16:41               ` Maxime Ripard
@ 2022-07-29 19:57                 ` Doug Anderson
  2022-08-15  6:45                   ` Maxime Ripard
  0 siblings, 1 reply; 13+ messages in thread
From: Doug Anderson @ 2022-07-29 19:57 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Abhinav Kumar, Rob Clark, dri-devel, freedreno, Daniel Vetter,
	David Airlie, Maarten Lankhorst, Thomas Zimmermann, LKML,
	Sankeerth Billakanti

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC PATCH] drm/edid: Make 144 Hz not preferred on Sharp LQ140M1JW46
  2022-07-29 19:57                 ` Doug Anderson
@ 2022-08-15  6:45                   ` Maxime Ripard
  2022-08-17 23:45                     ` Doug Anderson
  0 siblings, 1 reply; 13+ messages in thread
From: Maxime Ripard @ 2022-08-15  6:45 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Abhinav Kumar, Rob Clark, dri-devel, freedreno, Daniel Vetter,
	David Airlie, Maarten Lankhorst, Thomas Zimmermann, LKML,
	Sankeerth Billakanti

[-- Attachment #1: Type: text/plain, Size: 6185 bytes --]

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@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.

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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC PATCH] drm/edid: Make 144 Hz not preferred on Sharp LQ140M1JW46
  2022-08-15  6:45                   ` Maxime Ripard
@ 2022-08-17 23:45                     ` Doug Anderson
  2022-08-18 10:31                       ` Jani Nikula
  0 siblings, 1 reply; 13+ messages in thread
From: Doug Anderson @ 2022-08-17 23:45 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Abhinav Kumar, Rob Clark, dri-devel, freedreno, Daniel Vetter,
	David Airlie, Maarten Lankhorst, Thomas Zimmermann, LKML,
	Sankeerth Billakanti

Hi,

On Sun, Aug 14, 2022 at 11:46 PM Maxime Ripard <maxime@cerno.tech> wrote:
>
> 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@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.
>
> 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.

I was never massively confident about it, which is why I added the
"RFC" tag to begin with. ;-) In general I suspect that either change
will make people upset. In other words, if we programmatically always
try to put the "high refresh rate" first for all displays then people
will be upset and if we programmatically always try to put the "60 Hz
rate" first then people will be upset. Unless someone wants to stand
up and say that one side or the other is wrong, I think we're going to
simply leave this up to the whim of individual panels. Someone could
stand up and demand that it go one way or the other, but I certainly
don't have that clout.

The spec, as far as I can tell, says that it's up to the panel vendor
to use whatever means they want to decide on the "preferred" refresh
rate. Thus, as far as the spec is concerned this decision is made on
an individual panel basis. ;-) This was really the justification for
why my patch was just on one panel.

In any case, as I said I'm OK w/ dropping this. We'll find other ways
to work around the issue.

-Doug

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC PATCH] drm/edid: Make 144 Hz not preferred on Sharp LQ140M1JW46
  2022-08-17 23:45                     ` Doug Anderson
@ 2022-08-18 10:31                       ` Jani Nikula
  0 siblings, 0 replies; 13+ messages in thread
From: Jani Nikula @ 2022-08-18 10:31 UTC (permalink / raw)
  To: Doug Anderson, Maxime Ripard
  Cc: Sankeerth Billakanti, Thomas Zimmermann, David Airlie,
	Abhinav Kumar, dri-devel, LKML, freedreno

On Wed, 17 Aug 2022, Doug Anderson <dianders@chromium.org> wrote:
> Hi,
>
> On Sun, Aug 14, 2022 at 11:46 PM Maxime Ripard <maxime@cerno.tech> wrote:
>>
>> 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@cerno.tech> wrote:
>> > > 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.
>
> I was never massively confident about it, which is why I added the
> "RFC" tag to begin with. ;-) In general I suspect that either change
> will make people upset. In other words, if we programmatically always
> try to put the "high refresh rate" first for all displays then people
> will be upset and if we programmatically always try to put the "60 Hz
> rate" first then people will be upset. Unless someone wants to stand
> up and say that one side or the other is wrong, I think we're going to
> simply leave this up to the whim of individual panels. Someone could
> stand up and demand that it go one way or the other, but I certainly
> don't have that clout.
>
> The spec, as far as I can tell, says that it's up to the panel vendor
> to use whatever means they want to decide on the "preferred" refresh
> rate. Thus, as far as the spec is concerned this decision is made on
> an individual panel basis. ;-) This was really the justification for
> why my patch was just on one panel.
>
> In any case, as I said I'm OK w/ dropping this. We'll find other ways
> to work around the issue.

FWIW, if the EDID reported preferred mode works, I also think that's
what we should prefer.

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2022-08-18 10:31 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-21 22:23 [RFC PATCH] drm/edid: Make 144 Hz not preferred on Sharp LQ140M1JW46 Douglas Anderson
2022-07-22 16:37 ` Abhinav Kumar
2022-07-22 16:48   ` Doug Anderson
2022-07-22 17:36     ` Rob Clark
2022-07-28 17:34       ` Abhinav Kumar
2022-07-28 21:18         ` Doug Anderson
2022-07-29  7:51           ` Maxime Ripard
2022-07-29 14:50             ` Doug Anderson
2022-07-29 16:41               ` Maxime Ripard
2022-07-29 19:57                 ` Doug Anderson
2022-08-15  6:45                   ` Maxime Ripard
2022-08-17 23:45                     ` Doug Anderson
2022-08-18 10:31                       ` Jani Nikula

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).