Re: [PATCH 03/36] drm/omap: add crtc background property

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Tomi,

Thank you for the patch.

On Wednesday 30 Nov 2016 13:17:05 Tomi Valkeinen wrote:
> Add DRM property for crtc background color property. Background
> color is shown on areas where there are no planes.

The background property is useful for the rcar-du driver as well, and I assume 
for a bunch of other display engines too. Could you please standardize it ? 
And for the usual bikeshedding required in those circumstances, should it be 
called background-color or bgcolor instead ?

> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>  drivers/gpu/drm/omapdrm/omap_crtc.c | 60 +++++++++++++++++++++++++++-------
>  drivers/gpu/drm/omapdrm/omap_drv.c  |  9 ++++++
>  drivers/gpu/drm/omapdrm/omap_drv.h  |  5 +++-
>  3 files changed, 63 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c
> b/drivers/gpu/drm/omapdrm/omap_crtc.c index ff1570c096f8..c0e30a4cb2a9
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_crtc.c
> +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
> @@ -48,6 +48,8 @@ struct omap_crtc {
> 
>  struct omap_crtc_state {
>  	struct drm_crtc_state base;
> +
> +	u32 default_color;
>  };
> 
>  /*
> ---------------------------------------------------------------------------
> -- @@ -196,15 +198,7 @@ static void omap_crtc_set_enabled(struct drm_crtc
> *crtc, bool enable) static int omap_crtc_dss_enable(enum omap_channel
> channel)
>  {
>  	struct omap_crtc *omap_crtc = omap_crtcs[channel];
> -	struct omap_overlay_manager_info info;
> -
> -	memset(&info, 0, sizeof(info));
> -	info.default_color = 0x00000000;
> -	info.trans_key = 0x00000000;
> -	info.trans_key_type = OMAP_DSS_COLOR_KEY_GFX_DST;
> -	info.trans_enabled = false;
> 
> -	dispc_mgr_setup(omap_crtc->channel, &info);
>  	dispc_mgr_set_timings(omap_crtc->channel,
>  			&omap_crtc->vm);
>  	omap_crtc_set_enabled(&omap_crtc->base, true);
> @@ -319,6 +313,20 @@ static void omap_crtc_vblank_irq(struct omap_drm_irq
> *irq, uint32_t irqstatus) wake_up(&omap_crtc->pending_wait);
>  }
> 
> +static void omap_crtc_write_crtc_properties(struct drm_crtc *crtc)
> +{
> +	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
> +	struct omap_overlay_manager_info info;
> +	const struct omap_crtc_state *omap_state =
> +		to_omap_crtc_state(crtc->state);
> +
> +	memset(&info, 0, sizeof(info));
> +	info.default_color = omap_state->default_color;
> +	info.trans_enabled = false;
> +
> +	dispc_mgr_setup(omap_crtc->channel, &info);
> +}
> +
>  /*
> ---------------------------------------------------------------------------
> -- * CRTC Functions
>   */
> @@ -362,6 +370,8 @@ static void omap_crtc_reset(struct drm_crtc *crtc)
>  	if (omap_state == NULL)
>  		return;
> 
> +	omap_state->default_color = 0;
> +
>  	crtc->state = &omap_state->base;
>  	crtc->state->crtc = crtc;
>  }
> @@ -462,6 +472,8 @@ static void omap_crtc_atomic_flush(struct drm_crtc
> *crtc, dispc_mgr_set_gamma(omap_crtc->channel, lut, length);
>  	}
> 
> +	omap_crtc_write_crtc_properties(crtc);
> +
>  	if (dispc_mgr_is_enabled(omap_crtc->channel)) {
> 
>  		DBG("%s: GO", omap_crtc->name);
> @@ -491,6 +503,10 @@ static int omap_crtc_atomic_set_property(struct
> drm_crtc *crtc, struct drm_property *property,
>  					 uint64_t val)
>  {
> +	struct drm_device *dev = crtc->dev;
> +	struct omap_drm_private *priv = dev->dev_private;
> +	struct omap_crtc_state *omap_state = to_omap_crtc_state(state);
> +
>  	if (omap_crtc_is_plane_prop(crtc, property)) {
>  		struct drm_plane_state *plane_state;
>  		struct drm_plane *plane = crtc->primary;
> @@ -508,7 +524,12 @@ static int omap_crtc_atomic_set_property(struct
> drm_crtc *crtc, property, val);
>  	}
> 
> -	return -EINVAL;
> +	if (property == priv->background_color_prop)
> +		omap_state->default_color = val;
> +	else
> +		return -EINVAL;
> +
> +	return 0;
>  }
> 
>  static int omap_crtc_atomic_get_property(struct drm_crtc *crtc,
> @@ -516,6 +537,10 @@ static int omap_crtc_atomic_get_property(struct
> drm_crtc *crtc, struct drm_property *property,
>  					 uint64_t *val)
>  {
> +	struct drm_device *dev = crtc->dev;
> +	struct omap_drm_private *priv = dev->dev_private;
> +	struct omap_crtc_state *omap_state = to_omap_crtc_state(state);
> +
>  	if (omap_crtc_is_plane_prop(crtc, property)) {
>  		/*
>  		 * Delegate property get to the primary plane. The
> @@ -527,7 +552,12 @@ static int omap_crtc_atomic_get_property(struct
> drm_crtc *crtc, property, val);
>  	}
> 
> -	return -EINVAL;
> +	if (property == priv->background_color_prop)
> +		*val = omap_state->default_color;
> +	else
> +		return -EINVAL;
> +
> +	return 0;
>  }
> 
>  static const struct drm_crtc_funcs omap_crtc_funcs = {
> @@ -573,6 +603,15 @@ void omap_crtc_pre_uninit(void)
>  	dss_uninstall_mgr_ops();
>  }
> 
> +static void omap_crtc_install_properties(struct drm_crtc *crtc)
> +{
> +	struct drm_device *dev = crtc->dev;
> +	struct drm_mode_object *obj = &crtc->base;
> +	struct omap_drm_private *priv = dev->dev_private;
> +
> +	drm_object_attach_property(obj, priv->background_color_prop, 0);
> +}
> +
>  /* initialize crtc */
>  struct drm_crtc *omap_crtc_init(struct drm_device *dev,
>  		struct drm_plane *plane, enum omap_channel channel, int id)
> @@ -625,6 +664,7 @@ struct drm_crtc *omap_crtc_init(struct drm_device *dev,
>  		drm_mode_crtc_set_gamma_size(crtc, gamma_lut_size);
>  	}
> 
> +	omap_crtc_install_properties(crtc);
>  	omap_plane_install_properties(crtc->primary, &crtc->base);
> 
>  	omap_crtcs[channel] = omap_crtc;
> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c
> b/drivers/gpu/drm/omapdrm/omap_drv.c index 39c5312b466c..8c58ca7db382
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_drv.c
> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
> @@ -293,10 +293,19 @@ static int omap_modeset_init_properties(struct
> drm_device *dev) {
>  	struct omap_drm_private *priv = dev->dev_private;
> 
> +	/* plane properties */
> +
>  	priv->zorder_prop = drm_property_create_range(dev, 0, "zorder", 0, 3);
>  	if (!priv->zorder_prop)
>  		return -ENOMEM;
> 
> +	/* crtc properties */
> +
> +	priv->background_color_prop = drm_property_create_range(dev, 0,
> +		"background", 0, 0xffffff);
> +	if (!priv->background_color_prop)
> +		return -ENOMEM;
> +
>  	return 0;
>  }
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.h
> b/drivers/gpu/drm/omapdrm/omap_drv.h index 4c51135eb9a6..0d16294e8145
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_drv.h
> +++ b/drivers/gpu/drm/omapdrm/omap_drv.h
> @@ -97,9 +97,12 @@ struct omap_drm_private {
>  	struct omap_drm_usergart *usergart;
>  	bool has_dmm;
> 
> -	/* properties: */
> +	/* plane properties */
>  	struct drm_property *zorder_prop;
> 
> +	/* crtc properties */
> +	struct drm_property *background_color_prop;
> +
>  	/* irq handling: */
>  	struct list_head irq_list;    /* list of omap_drm_irq */
>  	uint32_t vblank_mask;         /* irq bits set for userspace vblank */

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel