HDMI Aspect Ratio

HDMI Aspect Ratio

[Lloyd Atkinson]

Hi folks,

We're looking at 4K HDMI mode support, and noticed aspect ratio support
is in flux.

Aspect ratio parsing was added to the mode:
https://patchwork.kernel.org/patch/9271401/

But later reverted:
https://patchwork.kernel.org/patch/9410765/

We're finding that since aspect ratio information isn't supported, modes
conflict, and the userspace isn't able to see the full list of modes.

Similarly, there's a case where two modes differ only by aspect_ratio
and vrefresh. In drm_edid.c, within drm_display_mode_from_vic_index,
newmode->vrefresh is forcibly being cleared to 0. Since refresh rate is
forced to 0, and aspect ratio isn’t considered in general, the modes
collide and we can’t advertise one of the modes.

https://github.com/torvalds/linux/blob/v4.14-rc4/drivers/gpu/drm/drm_edid.c#L3153

Have there been any follow-up discussions on this topic?

What is the reason to clear the vrefresh within
drm_display_mode_from_vic_index?

Thanks,
Lloyd Atkinson

-- 
QUALCOMM Canada, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: HDMI Aspect Ratio

[Rob Clark]

On Wed, Oct 18, 2017 at 4:06 PM, Lloyd Atkinson <latkinso@codeaurora.org> wrote:
> Hi folks,
>
> We're looking at 4K HDMI mode support, and noticed aspect ratio support
> is in flux.
>
> Aspect ratio parsing was added to the mode:
> https://patchwork.kernel.org/patch/9271401/
>
> But later reverted:
> https://patchwork.kernel.org/patch/9410765/
>
> We're finding that since aspect ratio information isn't supported, modes
> conflict, and the userspace isn't able to see the full list of modes.
>
> Similarly, there's a case where two modes differ only by aspect_ratio
> and vrefresh. In drm_edid.c, within drm_display_mode_from_vic_index,
> newmode->vrefresh is forcibly being cleared to 0. Since refresh rate is
> forced to 0, and aspect ratio isn’t considered in general, the modes
> collide and we can’t advertise one of the modes.
>
> https://github.com/torvalds/linux/blob/v4.14-rc4/drivers/gpu/drm/drm_edid.c#L3153
>
> Have there been any follow-up discussions on this topic?
>
> What is the reason to clear the vrefresh within
> drm_display_mode_from_vic_index?

+Ville who might remember the history..

I don't remember if we found a userspace app that was broken, or if
reverting was an "abundance of caution" thing (which is equally valid,
it is basically impossible to track down and test "all of
userspace"..)

Adding a cap to let userspace opt-in to the new behaviour seems like a
sensible approach, and one that we've used before.

BR,
-R

> Thanks,
> Lloyd Atkinson
>
> --
> QUALCOMM Canada, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: HDMI Aspect Ratio

[Ville Syrjälä]

On Thu, Oct 19, 2017 at 12:32:53PM -0400, Rob Clark wrote:
> On Wed, Oct 18, 2017 at 4:06 PM, Lloyd Atkinson <latkinso@codeaurora.org> wrote:
> > Hi folks,
> >
> > We're looking at 4K HDMI mode support, and noticed aspect ratio support
> > is in flux.
> >
> > Aspect ratio parsing was added to the mode:
> > https://patchwork.kernel.org/patch/9271401/
> >
> > But later reverted:
> > https://patchwork.kernel.org/patch/9410765/
> >
> > We're finding that since aspect ratio information isn't supported, modes
> > conflict, and the userspace isn't able to see the full list of modes.
> >
> > Similarly, there's a case where two modes differ only by aspect_ratio
> > and vrefresh. In drm_edid.c, within drm_display_mode_from_vic_index,
> > newmode->vrefresh is forcibly being cleared to 0. Since refresh rate is
> > forced to 0, and aspect ratio isn’t considered in general, the modes
> > collide and we can’t advertise one of the modes.
> >
> > https://github.com/torvalds/linux/blob/v4.14-rc4/drivers/gpu/drm/drm_edid.c#L3153
> >
> > Have there been any follow-up discussions on this topic?
> >
> > What is the reason to clear the vrefresh within
> > drm_display_mode_from_vic_index?
> 
> +Ville who might remember the history..
> 
> I don't remember if we found a userspace app that was broken, or if
> reverting was an "abundance of caution" thing (which is equally valid,
> it is basically impossible to track down and test "all of
> userspace"..)

IIRC it was just me being paranoid because we used to have 0 there, and
I was worried something might get confused if it comes in pre-populated.

I don't think this should have any user space impact because the probe
helper will anyway populate it with drm_mode_vrefresh() before we
expose the mode to anyone. So the only time when you might even see the
zero is if you manually duplicate a cea/vic mode for some reason.

So I guess we could just remove the zeroing and see what happens. Or
we might even do something like WARN_ON(vrefresh != drm_mode_vrefresh())
to make sure we've correctly populate the cea/hdmi mode lists.

Note that we really shouldn't be using vrefresh for anything real.
It's there just for informational purposes and you really shouldn't
use it for anything important. drm_mode_equal() & co. ignore it
correctly.

If you have a need to know the actual refresh rate for something
important then 1 Hz accurace seem very much insufficient. We never
use vrefresh for anything in i915 code for instance and instead we
compute things based on the actual timings as needed.

Also I think it's rather unfortunate that the user visible mode even
has vrefresh. It has zero benefits and looks like we have no sanity
checking to make sure the user provided mode vrefresh even agrees
with the rest of the timings. We should probably just set
'mode->vrefresh = drm_mode_vrefres()' in drm_mode_convert_umode() to
make sure it's consistent from the start. Not sure we dare start
rejecting user modes on account of inconsistent vrefresh. That might
really break some userspace applications.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: HDMI Aspect Ratio

[Alex Deucher]

On Thu, Oct 19, 2017 at 12:32 PM, Rob Clark <robdclark@gmail.com> wrote:
> On Wed, Oct 18, 2017 at 4:06 PM, Lloyd Atkinson <latkinso@codeaurora.org> wrote:
>> Hi folks,
>>
>> We're looking at 4K HDMI mode support, and noticed aspect ratio support
>> is in flux.
>>
>> Aspect ratio parsing was added to the mode:
>> https://patchwork.kernel.org/patch/9271401/
>>
>> But later reverted:
>> https://patchwork.kernel.org/patch/9410765/
>>
>> We're finding that since aspect ratio information isn't supported, modes
>> conflict, and the userspace isn't able to see the full list of modes.
>>
>> Similarly, there's a case where two modes differ only by aspect_ratio
>> and vrefresh. In drm_edid.c, within drm_display_mode_from_vic_index,
>> newmode->vrefresh is forcibly being cleared to 0. Since refresh rate is
>> forced to 0, and aspect ratio isn’t considered in general, the modes
>> collide and we can’t advertise one of the modes.
>>
>> https://github.com/torvalds/linux/blob/v4.14-rc4/drivers/gpu/drm/drm_edid.c#L3153
>>
>> Have there been any follow-up discussions on this topic?
>>
>> What is the reason to clear the vrefresh within
>> drm_display_mode_from_vic_index?
>
> +Ville who might remember the history..
>
> I don't remember if we found a userspace app that was broken, or if
> reverting was an "abundance of caution" thing (which is equally valid,
> it is basically impossible to track down and test "all of
> userspace"..)
>

Wasn't there an issue with SNA?

Alex

> Adding a cap to let userspace opt-in to the new behaviour seems like a
> sensible approach, and one that we've used before.>
> BR,
> -R
>
>> Thanks,
>> Lloyd Atkinson
>>
>> --
>> QUALCOMM Canada, on behalf of Qualcomm Innovation Center, Inc. is a member
>> of Code Aurora Forum, hosted by The Linux Foundation
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: HDMI Aspect Ratio

[Ville Syrjälä]

On Thu, Oct 19, 2017 at 01:08:05PM -0400, Alex Deucher wrote:
> On Thu, Oct 19, 2017 at 12:32 PM, Rob Clark <robdclark@gmail.com> wrote:
> > On Wed, Oct 18, 2017 at 4:06 PM, Lloyd Atkinson <latkinso@codeaurora.org> wrote:
> >> Hi folks,
> >>
> >> We're looking at 4K HDMI mode support, and noticed aspect ratio support
> >> is in flux.
> >>
> >> Aspect ratio parsing was added to the mode:
> >> https://patchwork.kernel.org/patch/9271401/
> >>
> >> But later reverted:
> >> https://patchwork.kernel.org/patch/9410765/
> >>
> >> We're finding that since aspect ratio information isn't supported, modes
> >> conflict, and the userspace isn't able to see the full list of modes.
> >>
> >> Similarly, there's a case where two modes differ only by aspect_ratio
> >> and vrefresh. In drm_edid.c, within drm_display_mode_from_vic_index,
> >> newmode->vrefresh is forcibly being cleared to 0. Since refresh rate is
> >> forced to 0, and aspect ratio isn’t considered in general, the modes
> >> collide and we can’t advertise one of the modes.
> >>
> >> https://github.com/torvalds/linux/blob/v4.14-rc4/drivers/gpu/drm/drm_edid.c#L3153
> >>
> >> Have there been any follow-up discussions on this topic?
> >>
> >> What is the reason to clear the vrefresh within
> >> drm_display_mode_from_vic_index?
> >
> > +Ville who might remember the history..
> >
> > I don't remember if we found a userspace app that was broken, or if
> > reverting was an "abundance of caution" thing (which is equally valid,
> > it is basically impossible to track down and test "all of
> > userspace"..)
> >
> 
> Wasn't there an issue with SNA?

Exposing the aspect ratio flags broke sna, at least.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel