Re: [PATCH 1/2] drm/edid: adjust double-clocked cea modes

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, May 15, 2012 at 11:33:43AM -0400, Adam Jackson wrote:
> On 5/14/12 3:43 PM, Paulo Zanoni wrote:
> 
> >Also, I think flag DRM_MODE_FLAG_DBLCLK does not sound correct for
> >them, so we would need to create flags:
> >- DRM_MODE_FLAG_PR_1_to_10
> >- DRM_MODE_FLAG_PR_1_or_2
> >- DRM_MODE_FLAG_PR_1_or_2_or_4
> >
> >Or any other more creative names. And then we should cross our fingers
> >in the hope that they don't start creating modes with other possible
> >variations of PR :)
> >Anyway, how will the user be able to choose the wanted PR? What about
> >the aspect ratio? Yet Another Connector Property?
> 
> Well, one thing at a time.
> 
> Right now we don't have the concept of a mode property.  Therefore,
> modes exposed to userspace should all be unique.  Exposing the pixel
> repeat to userspace as a connector property is wrong because it
> doesn't apply to all modes for a connector.
> 
> So my initial inclination would be to do something like this in the
> flags field:
> 
> --- a/include/drm/drm_mode.h
> +++ b/include/drm/drm_mode.h
> @@ -58,6 +58,9 @@
>  #define DRM_MODE_FLAG_PIXMUX   (1<<11)
>  #define DRM_MODE_FLAG_DBLCLK   (1<<12)
>  #define DRM_MODE_FLAG_CLKDIV2  (1<<13)
> +/* begin non-xorg definitions */
> +#define DRM_PIXREP_MASK                (15 << 14)
> +#define DRM_MODE_FLAG_PIXREP(x) (((x) << 14) & DRM_PIXMULT_MASK)
> 
>  /* DPMS flags */
>  /* bit compatible with the xorg definitions. */
> 
> The timings for the variably-repeated modes won't change (right?) so
> this is the only way to get the desired pixel repeat passed into the
> modesettinng path.  And then the "one to ten" and "one two or four"
> modes simply need to be added to the list multiple times, once for
> each possible pixel replication, and with varying names depending on
> the effective framebuffer size.
> 
> Problems with this approach are that it means rewriting the CEA mode
> list or the VIC table walk or both, and that it means creating a
> class of modes that userspace can set but not create (which means,
> if your X driver is parsing EDID itself instead of just using the
> damn kernel mode list, that many of the modes will be inaccessible
> to X).  Neither is insoluble, just nngh.

For the variable pixel-repeat modes I'd propose that we mark them up with
a flag and flat-out reject them int the cea edid parser. All these modes
are super-low-res interlaced things likely only good to upscale sdtv
material. At least on Intel we might as well use the hw scaler for them.

The double-clocked ones look slightly more sane, so I guess we could keep
these. If I manage to get them to work anywhere else than on my machine
here :(

> The dual-aspect-ratio modes are, well, ugly.  Ideally we'd have
> scaler hardware on all digital output routes so we could just make
> that a connector property, and then hide the choice of mechanism
> inside the drm (either in the monitor if the infoframe says so / if
> it's controllable with DDC/CI or something, or in the GPU if not).
> I don't know if all the digital outputs in the world live up to that
> lofty goal.  Failing that we could encode the widescreen-ness in the
> mode name?  All quite horrible.  TVs really are the worst thing.

tbh our current avi infoframe support is ridiculously lacking - we don't
even properly work the overscan properties, set the VIC number and all
kinds of stuff. I think we can defer worrying about these multi-aspect
ratio modes until everything else works somewhat.

Cheers, Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48