[PATCHv3 19/30] drm/omap: fix replication logic

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Tuesday 28 Mar 2017 16:08:05 Tomi Valkeinen wrote:
> DSS uses "replication logic" to convert color components from smaller
> bit widths to bigger bit widths. Without replication logic, the color
> component would be shifted and the least significant bits would be left
> at 0, whereas with replication logic, the least significat bits will be
> filled with the most significant bit.
> 
> For some reason omapdrm leaves replication logic always to off, which,
> in the most common case, means that showing 16-bit RGB colors on 24-bit
> display gives a bit darker colors than expected.
> 
> The reset value of the replication logic bit is enabled, and it should
> always be set, as there's no practical reason to disable it. This patch
> removes the option to disable it from omapdrm, and always sets it to
> enabled in dispc.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen at ti.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> ---
>  drivers/gpu/drm/omapdrm/dss/dispc.c   | 7 ++++---
>  drivers/gpu/drm/omapdrm/dss/omapdss.h | 3 +--
>  drivers/gpu/drm/omapdrm/omap_plane.c  | 2 +-
>  3 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c
> b/drivers/gpu/drm/omapdrm/dss/dispc.c index e9d98046a97f..83241052df6b
> 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dispc.c
> +++ b/drivers/gpu/drm/omapdrm/dss/dispc.c
> @@ -2823,11 +2823,12 @@ static int dispc_ovl_setup_common(enum omap_plane
> plane, }
> 
>  static int dispc_ovl_setup(enum omap_plane plane, const struct
> omap_overlay_info *oi, -		bool replication, const struct 
videomode *vm,
> bool mem_to_mem)
> +		const struct videomode *vm, bool mem_to_mem)
>  {
>  	int r;
>  	enum omap_overlay_caps caps = dss_feat_get_overlay_caps(plane);
>  	enum omap_channel channel;
> +	const bool replication = true;
> 
>  	channel = dispc_ovl_get_channel_out(plane);
> 
> @@ -2854,7 +2855,7 @@ int dispc_wb_setup(const struct
> omap_dss_writeback_info *wi, enum omap_plane plane = OMAP_DSS_WB;
>  	const int pos_x = 0, pos_y = 0;
>  	const u8 zorder = 0, global_alpha = 0;
> -	const bool replication = false;
> +	const bool replication = true;
>  	bool truncation;
>  	int in_width = vm->hactive;
>  	int in_height = vm->vactive;
> @@ -4288,7 +4289,7 @@ static void dispc_errata_i734_wa(void)
> 
>  	/* Setup and enable GFX plane */
>  	dispc_ovl_set_channel_out(OMAP_DSS_GFX, OMAP_DSS_CHANNEL_LCD);
> -	dispc_ovl_setup(OMAP_DSS_GFX, &ovli, false, &i734.vm, false);
> +	dispc_ovl_setup(OMAP_DSS_GFX, &ovli, &i734.vm, false);
>  	dispc_ovl_enable(OMAP_DSS_GFX, true);
> 
>  	/* Set up and enable display manager for LCD1 */
> diff --git a/drivers/gpu/drm/omapdrm/dss/omapdss.h
> b/drivers/gpu/drm/omapdrm/dss/omapdss.h index 3e61f4b2d1ce..4545f1824e66
> 100644
> --- a/drivers/gpu/drm/omapdrm/dss/omapdss.h
> +++ b/drivers/gpu/drm/omapdrm/dss/omapdss.h
> @@ -928,8 +928,7 @@ struct dispc_ops {
>  	void (*ovl_set_channel_out)(enum omap_plane plane,
>  			enum omap_channel channel);
>  	int (*ovl_setup)(enum omap_plane plane, const struct omap_overlay_info
> *oi, -			bool replication, const struct videomode *vm,
> -			bool mem_to_mem);
> +			const struct videomode *vm, bool mem_to_mem);
> 
>  	enum omap_color_mode (*ovl_get_color_modes)(enum omap_plane plane);
>  };
> diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c
> b/drivers/gpu/drm/omapdrm/omap_plane.c index bdd74692e0cd..78a92422ede1
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_plane.c
> +++ b/drivers/gpu/drm/omapdrm/omap_plane.c
> @@ -128,7 +128,7 @@ static void omap_plane_atomic_update(struct drm_plane
> *plane, omap_crtc_channel(state->crtc));
> 
>  	/* and finally, update omapdss: */
> -	ret = priv->dispc_ops->ovl_setup(omap_plane->id, &info, false,
> +	ret = priv->dispc_ops->ovl_setup(omap_plane->id, &info,
>  			      omap_crtc_timings(state->crtc), false);
>  	if (ret) {
>  		dev_err(plane->dev->dev, "Failed to setup plane %s\n",

-- 
Regards,

Laurent Pinchart