Re: [PATCH 1/2] Revert "drm: Add and handle new aspect ratios in DRM layer"

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Mon, Nov 14, 2016 at 08:14:34PM +0530, Sharma, Shashank wrote:
> Regards
> Shashank
> > the revert:
> >
> >   HDMI2 connected 1920x1080+0+0 (normal left inverted right x axis y axis) 700mm x 390mm
> > -   1920x1080     60.00*+
> > -   1920x1080i    60.00    50.00
> > +   1920x1080     60.00*+  50.00    59.94    30.00    25.00    24.00    29.97    23.98
> > +   1920x1080i    60.00    50.00    59.94
> >      1600x1200     60.00
> >      1680x1050     59.88
> >      1280x1024     75.02    60.02
> > @@ -13,30 +13,29 @@
> >      1360x768      60.02
> >      1280x800      59.91
> >      1152x864      75.00
> > -   1280x720      60.00    50.00
> > +   1280x720      60.00    50.00    59.94
> >      1024x768      75.03    70.07    60.00
> >      832x624       74.55
> >      800x600       72.19    75.00    60.32
> > -   640x480       75.00    72.81    66.67    59.94
> > +   720x576       50.00
> > +   720x480       60.00    59.94
> > +   640x480       75.00    72.81    66.67    60.00    59.94
> >      720x400       70.08
> None of these aspect ratios are new modes / new aspect ratios from HDMI 
> 2.0/CEA-861-F
> These are the existing modes, and should be independent of reverted 
> patches.

They're affected because your patches changed them by adding the aspect
ratio flags to them.

> > This was with sna, which does this:
> >   #define KNOWN_MODE_FLAGS ((1<<14)-1)
> >   if (mode->status == MODE_OK && kmode->flags & ~KNOWN_MODE_FLAGS)
> >   	mode->status = MODE_BAD; /* unknown flags => unhandled */
> > so all the modes with an aspect ratio just vanished.
> >
> > -modesetting and -ati on the other hand just copy over the unknown
> > bits into the xrandr mode structure, which sounds dubious at best:
> >   mode->Flags = kmode->flags; //& FLAG_BITS;
> > I've not checked what damage it can actually cause.
> >
> >
> > It looks like a few modes disappeared from the kernel's mode list
> > as well, presumably because some cea modes in the list originated from
> > DTDs and whanot so they don't have an aspect ratio and that causes
> > add_alternate_cea_modes() to ignore them. So not populating an aspect
> > ratio for cea modes originating from a source other than
> > edid_cea_modes[] looks like another bug to me as well.
> I am writing a patch series to cap the aspect ratio implementation under 
> a drm_cap_hdmi2_aspect_ratios
> This is how its going to work (inspired from the 2D/stereo series from 
> damien L)
> 
> - Add a new capability hdmi2_ar

It should be just a generic "expose aspect ratio flags to userspace?"

> - by default parsing the new hdmi 2.0 aspect ratio will be disabled 
> under check of this cap
> - during bootup time, while initializing the display, a userspace can 
> get_cap on the hdmi2_aspect_ratio
> - If it wants HDMI 2.0 aspect ratio support, it will set the cap, and 
> kernel will expose these aspect ratios
> > Another bug I think might be the ordering of the modes with aspect ratio
> > specified. IIRC the spec says that the preferred aspect ratio should be
> > listed first in the EDID, but I don't think we preserve that ordering
> > in the final mode list. I guess we could fix that by somehow noting
> > which aspect ratio is preferred and sort based on that, or we try to
> > preserve the order from the EDID until we're ready to sort, and then do
> > the sorting with a stable algorithm.
> AFAIK The mode order and priority is decided and arranged in userspace, 
> based on various factors like
> - preferred mode.
> - previously applied mode in previous sessions (like for android tvs)
> - Bigger h/w vs better refresh rate ?
> - Xserver applies its own algorithms to decide which mode should be 
> shown first.

Xorg does sort on its own. But since it doesn't know anything about
aspect ratios and whatnot I wouldn't rely on that for anything. I
also wouldn't expect eg. wayland compositors to do their own sorting.
And yeah, looks like weston at least doesn't do any sorting whatsoever.

> 
> I dont think kernel needs to bother about it.

So I'm going to say that we in fact do need to bother.

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