Re: [PATCH 14/16] drm/omap: remove omap_framebuffer_get_formats()

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

Hi Tomi,

Thank you for the patch.

On Thursday 04 May 2017 13:23:30 Tomi Valkeinen wrote:
> We now get a fourcc array from dispc when asking for a plane's supported
> pixel formats, so we can drop omap_framebuffer_get_formats() which was
> used to convert between DSS and DRM pixel formats.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>  drivers/gpu/drm/omapdrm/omap_drv.h   |  2 --
>  drivers/gpu/drm/omapdrm/omap_fb.c    | 22 ----------------------
>  drivers/gpu/drm/omapdrm/omap_plane.c | 15 +++++++--------
>  3 files changed, 7 insertions(+), 32 deletions(-)

[snip]

> diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c
> b/drivers/gpu/drm/omapdrm/omap_plane.c index 6cabbda5ec57..6e2ea83b560c
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_plane.c
> +++ b/drivers/gpu/drm/omapdrm/omap_plane.c

[snip]

> @@ -346,6 +343,8 @@ struct drm_plane *omap_plane_init(struct drm_device
> *dev, struct omap_plane *omap_plane;
>  	enum omap_plane_id id;
>  	int ret;
> +	u32 nformats;
> +	const u32 *formats;
> 
>  	if (WARN_ON(idx >= ARRAY_SIZE(plane_idx_to_id)))
>  		return ERR_PTR(-EINVAL);
> @@ -358,17 +357,17 @@ struct drm_plane *omap_plane_init(struct drm_device
> *dev, if (!omap_plane)
>  		return ERR_PTR(-ENOMEM);
> 
> -	omap_plane->nformats = omap_framebuffer_get_formats(
> -			omap_plane->formats, ARRAY_SIZE(omap_plane->formats),
> -			priv->dispc_ops->ovl_get_color_modes(id));
> +	formats = priv->dispc_ops->ovl_get_color_modes(id);
> +	for (nformats = 0; formats[nformats]; ++nformats)
> +		;

Wouldn't it make sense to modify the way supported formats are stored 
internally to store both the array and its size ? We could then return both 
from ovl_get_color_modes(), which would save us from computing the size at 
runtime. This can be done in a subsequent patch, so

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

>  	omap_plane->id = id;
>  	omap_plane->name = plane_id_to_name[id];
> 
>  	plane = &omap_plane->base;
> 
>  	ret = drm_universal_plane_init(dev, plane, possible_crtcs,
> -				       &omap_plane_funcs, omap_plane->formats,
> -				       omap_plane->nformats, type, NULL);
> +				       &omap_plane_funcs, formats,
> +				       nformats, type, NULL);
>  	if (ret < 0)
>  		goto error;

-- 
Regards,

Laurent Pinchart

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