[PATCHv3 1/8] drm: Add the lacking DRM_MODE_FLAG_* for matching the DISPLAY_FLAGS_*

Russell King - ARM Linux linux at arm.linux.org.uk
Tue Nov 12 09:04:55 PST 2013


On Tue, Nov 12, 2013 at 05:49:18PM +0100, Denis Carikli wrote:
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index fc2adb6..586c12f 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -537,6 +537,15 @@ int drm_display_mode_from_videomode(const struct videomode *vm,
>  		dmode->flags |= DRM_MODE_FLAG_DBLSCAN;
>  	if (vm->flags & DISPLAY_FLAGS_DOUBLECLK)
>  		dmode->flags |= DRM_MODE_FLAG_DBLCLK;
> +	if (vm->flags & DISPLAY_FLAGS_DE_LOW)
> +		dmode->flags |= DRM_MODE_FLAG_DE_LOW;
> +	if (vm->flags & DISPLAY_FLAGS_DE_HIGH)
> +		dmode->flags |= DRM_MODE_FLAG_DE_HIGH;
> +	if (vm->flags & DISPLAY_FLAGS_PIXDATA_POSEDGE)
> +		dmode->flags |= DRM_MODE_FLAG_PIXDATA_POSEDGE;
> +	if (vm->flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE)
> +		dmode->flags |= DRM_MODE_FLAG_PIXDATA_NEGEDGE;
> +

I'm still not convinced that these should be exposed in *any* way to
userspace - I'm with Ville Syrjälä on this point.

Yes, you've moved their definition out of a uapi header file, but
they're still leaking out of kernel space via calls (and with Xorg,
they'll leak into the DisplayMode structures within the X server.)

Now, here's the thing... The polarity of the display enable signal.
That's a property of the connected device right?  It doesn't change
with respect to the displayed mode unlike the hsync/vsync signals.
If that's true, it should not be here.

Same goes for the pixel clock edge.  If it's a property of the
connected device and doesn't have a dependence on the displayed
mode, then it should not be in the DRM mode structure.


More information about the dri-devel mailing list