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

From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: dri-devel@lists.freedesktop.org
Cc: Jose Abreu <Jose.Abreu@synopsys.com>, Jia <lin.a.jia@intel.com>,
	Akashdeep Sharma <akashdeep.sharma@intel.com>,
	Lin@freedesktop.org, Emil Velikov <emil.l.velikov@gmail.com>,
	Daniel Vetter <daniel.vetter@ffwll.ch>,
	Jim Bride <jim.bride@linux.intel.com>
Subject: Re: [PATCH 1/2] Revert "drm: Add and handle new aspect ratios in DRM layer"
Date: Fri, 11 Nov 2016 19:00:17 +0200	[thread overview]
Message-ID: <20161111170017.GM31595@intel.com> (raw)
In-Reply-To: <1478176304-6743-1-git-send-email-ville.syrjala@linux.intel.com>

On Thu, Nov 03, 2016 at 02:31:43PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> This reverts commit a68362fe3e84fcbedd49939aa200519aa5410135.
> 
> Adding new mode flags willy nilly breaks existing userspace. We need to
> coordinate this better, potentially with a new client cap that only
> exposes the aspect ratio flags when userspace is prepared for them
> (similar to what we do with stereo 3D modes).

As a demonstration here's the change in the xrandr mode list after doing
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  

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.

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.

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