Re: [PATCH 2/3] drm: Make sure at least one plane supports the fb format

[Eric Anholt <eric@anholt.net>]

[-- Attachment #1.1: Type: text/plain, Size: 2524 bytes --]

Ville Syrjala <ville.syrjala@linux.intel.com> writes:

> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> To make life easier for drivers, let's have the core check that the
> requested pixel format is supported by at least one plane when creating
> a new framebuffer.
>
> This eases the burden on drivers by making sure they'll never get
> requests to create fbs with unsupported pixel formats. Thanks to the
> new .fb_modifier() hook this check can now be done whether the request
> specifies the modifier directly or driver has to deduce it from the
> gem bo tiling (or via any other method really).
>
> v0: Accept anyformat if the driver doesn't do planes (Eric)
>     s/planes_have_format/any_plane_has_format/ (Eric)
>     Check the modifier as well since we already have a function
>     that does both
> v3: Don't do the check in the core since we may not know the
>     modifier yet, instead export the function and let drivers
>     call it themselves
> v4: Unexport the functiona and put the format_default check back
>     since this will again be called by the core, ie. undo v3 ;)
>
> Cc: Eric Anholt <eric@anholt.net>
> Testcase: igt/kms_addfb_basic/expected-formats
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/drm_framebuffer.c | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c
> index 21d3d51eb261..e618a6b728d4 100644
> --- a/drivers/gpu/drm/drm_framebuffer.c
> +++ b/drivers/gpu/drm/drm_framebuffer.c
> @@ -152,6 +152,26 @@ static int fb_plane_height(int height,
>  	return DIV_ROUND_UP(height, format->vsub);
>  }
>  
> +static bool any_plane_has_format(struct drm_device *dev,
> +				 u32 format, u64 modifier)
> +{
> +	struct drm_plane *plane;
> +
> +	drm_for_each_plane(plane, dev) {
> +		/*
> +		 * In case the driver doesn't really do
> +		 * planes we have to accept any format here.
> +		 */
> +		if (plane->format_default)
> +			return true;
> +
> +		if (drm_plane_check_pixel_format(plane, format, modifier) == 0)
> +			return true;
> +	}
> +
> +	return false;
> +}

This drm_plane_check_pixel_format() will always fail for VC4's SAND
modifiers or VC5's UIF modifiers, where we're using the middle 48 bits
as a bit of metadata (like how we have horizontal stride passed outside
of the modifier) and you can't list all of the possible values in an
array on the plane.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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