Re: [PATCH 3/6] drm/omap: remove ovl_set_channel_out

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

Hi Tomi,

Thank you for the patch.

On Thursday 27 Apr 2017 13:27:51 Tomi Valkeinen wrote:
> At the moment we have ovl_set_channel_out() to configure the output
> channel of an overlay. It makes sense to have this configuration as part
> of the reset of overlay configuration, and in DSS6+ we need the output

Do you mean s/reset/rest/ ?

> channel when doing the other overlay configuration.
> 
> This patch adds 'channel' parameter to ovl_setup(), so that all overlay

s/adds/adds a/

> configuration is done via the same function, and removes the
> ovl_set_channel_out().
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>  drivers/gpu/drm/omapdrm/dss/dispc.c   | 13 ++++++-------
>  drivers/gpu/drm/omapdrm/dss/omapdss.h |  3 ++-
>  drivers/gpu/drm/omapdrm/omap_plane.c  |  6 ++----
>  3 files changed, 10 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c
> b/drivers/gpu/drm/omapdrm/dss/dispc.c index 9fba92973d0e..97f4a1163fbe
> 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dispc.c
> +++ b/drivers/gpu/drm/omapdrm/dss/dispc.c
> @@ -2834,21 +2834,21 @@ static int dispc_ovl_setup_common(enum omap_plane_id
> plane,
> 
>  static int dispc_ovl_setup(enum omap_plane_id plane,
>  		const struct omap_overlay_info *oi,
> -		const struct videomode *vm, bool mem_to_mem)
> +		const struct videomode *vm, bool mem_to_mem,
> +		enum omap_channel channel)
>  {
>  	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);
> -
>  	DSSDBG("dispc_ovl_setup %d, pa %pad, pa_uv %pad, sw %d, %d,%d, %dx%d -
>"
>  		" %dx%d, cmode %x, rot %d, mir %d, chan %d repl %d\n",
>  		plane, &oi->paddr, &oi->p_uv_addr, oi->screen_width, oi-
>pos_x,
>  		oi->pos_y, oi->width, oi->height, oi->out_width, oi-
>out_height,
>  		oi->color_mode, oi->rotation, oi->mirror, channel, 
replication);
> 
> +	dispc_ovl_set_channel_out(plane, channel);
> +
>  	r = dispc_ovl_setup_common(plane, caps, oi->paddr, oi->p_uv_addr,
>  		oi->screen_width, oi->pos_x, oi->pos_y, oi->width, oi->height,
>  		oi->out_width, oi->out_height, oi->color_mode, oi->rotation,
> @@ -4304,8 +4304,8 @@ static void dispc_errata_i734_wa(void)
>  	REG_FLD_MOD(DISPC_CONFIG, 0x1f, 8, 4);
> 
>  	/* Setup and enable GFX plane */
> -	dispc_ovl_set_channel_out(OMAP_DSS_GFX, OMAP_DSS_CHANNEL_LCD);
> -	dispc_ovl_setup(OMAP_DSS_GFX, &ovli, &i734.vm, false);
> +	dispc_ovl_setup(OMAP_DSS_GFX, &ovli, &i734.vm, false,
> +		OMAP_DSS_CHANNEL_LCD);
>  	dispc_ovl_enable(OMAP_DSS_GFX, true);
> 
>  	/* Set up and enable display manager for LCD1 */
> @@ -4372,7 +4372,6 @@ static const struct dispc_ops dispc_ops = {
>  	.mgr_set_gamma = dispc_mgr_set_gamma,
> 
>  	.ovl_enable = dispc_ovl_enable,
> -	.ovl_set_channel_out = dispc_ovl_set_channel_out,
>  	.ovl_setup = dispc_ovl_setup,
>  	.ovl_get_color_modes = dispc_ovl_get_color_modes,
>  };
> diff --git a/drivers/gpu/drm/omapdrm/dss/omapdss.h
> b/drivers/gpu/drm/omapdrm/dss/omapdss.h index d9ad33df0455..a2b9b7f29472
> 100644
> --- a/drivers/gpu/drm/omapdrm/dss/omapdss.h
> +++ b/drivers/gpu/drm/omapdrm/dss/omapdss.h
> @@ -917,7 +917,8 @@ struct dispc_ops {
>  			enum omap_channel channel);

Shouldn't you also remove the .ovl_set_channel_out() operation from this 
structure ?

Apart from that,

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

>  	int (*ovl_setup)(enum omap_plane_id plane,
>  			 const struct omap_overlay_info *oi,
> -			const struct videomode *vm, bool mem_to_mem);
> +			const struct videomode *vm, bool mem_to_mem,
> +			enum omap_channel channel);
> 
>  	enum omap_color_mode (*ovl_get_color_modes)(enum omap_plane_id plane);
>  };
> diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c
> b/drivers/gpu/drm/omapdrm/omap_plane.c index 9168154d749e..81d9822f6f4a
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_plane.c
> +++ b/drivers/gpu/drm/omapdrm/omap_plane.c
> @@ -118,12 +118,10 @@ static void omap_plane_atomic_update(struct drm_plane
> *plane, DBG("%d,%d %pad %pad", info.pos_x, info.pos_y,
>  			&info.paddr, &info.p_uv_addr);
> 
> -	priv->dispc_ops->ovl_set_channel_out(omap_plane->id,
> -				  omap_crtc_channel(state->crtc));
> -
>  	/* and finally, update omapdss: */
>  	ret = priv->dispc_ops->ovl_setup(omap_plane->id, &info,
> -			      omap_crtc_timings(state->crtc), false);
> +			      omap_crtc_timings(state->crtc), false,
> +			      omap_crtc_channel(state->crtc));
>  	if (ret) {
>  		dev_err(plane->dev->dev, "Failed to setup plane %s\n",
>  			omap_plane->name);

-- 
Regards,

Laurent Pinchart

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