Re: [PATCH v3 4/6] drm: Extend framebuffer_check to handle formats with cpp/char_per_block 0

From: Daniel Vetter <daniel@ffwll.ch>
To: Alexandru-Cosmin Gheorghe <Alexandru-Cosmin.Gheorghe@arm.com>
Cc: Charles Xu <Charles.Xu@arm.com>, nd <nd@arm.com>,
	Matt Szczesiak <matt.szczesiak@arm.com>,
	"airlied@linux.ie" <airlied@linux.ie>,
	Liviu Dudau <Liviu.Dudau@arm.com>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>,
	David Garbett <David.Garbett@arm.com>,
	"seanpaul@chromium.org" <seanpaul@chromium.org>,
	Lisa Wu <lisa.wu@arm.com>,
	"daniel.vetter@ffwll.ch" <daniel.vetter@ffwll.ch>,
	"malidp@foss.arm.com" <malidp@foss.arm.com>,
	Ayan Halder <Ayan.Halder@arm.com>,
	"james qian wang (Arm Technology China)"
	<james.qian.wang@arm.com>
Subject: Re: [PATCH v3 4/6] drm: Extend framebuffer_check to handle formats with cpp/char_per_block 0
Date: Fri, 5 Oct 2018 16:53:33 +0200	[thread overview]
Message-ID: <20181005145333.GG31561@phenom.ffwll.local> (raw)
In-Reply-To: <20181005092606.21100-5-alexandru-cosmin.gheorghe@arm.com>

On Fri, Oct 05, 2018 at 09:26:57AM +0000, Alexandru-Cosmin Gheorghe wrote:
> For formats that are supported only with non-linear modifiers it
> doesn't make to much sense to define cpp or char_per_block, so that
> will be set to 0.
> 
> This patch adds a restriction to force having a modifier attached when
> cpp/char_per_block is 0, and to bypass checking the pitch restriction.
> 
> This had been discussed here.
> [1] https://people.freedesktop.org/~cbrill/dri-log/?channel=dri-devel&highlight_names=&date=2018-09-13&show_html=true
> 
> Signed-off-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@arm.com>
> Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>

Can we require that the driver supplies a more accurate drm_format_info
through get_format_into in these cases? I'm hoping that works, and the
afbc stuff isn't entirely random alignment :-)

Then we wouldn't need these escape hatches here I think.
-Daniel

> ---
>  drivers/gpu/drm/drm_framebuffer.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c
> index 4e14013788cd..66ec126b7faf 100644
> --- a/drivers/gpu/drm/drm_framebuffer.c
> +++ b/drivers/gpu/drm/drm_framebuffer.c
> @@ -200,6 +200,10 @@ static int framebuffer_check(struct drm_device *dev,
>  					drm_format_info_block_width(info, i) *
>  					drm_format_info_block_height(info, i));
>  
> +		if (!block_size && (r->modifier[i] == DRM_FORMAT_MOD_LINEAR)) {
> +			DRM_DEBUG_KMS("Format requires non-linear modifier for plane %d\n", i);
> +			return -EINVAL;
> +		}
>  		if (!r->handles[i]) {
>  			DRM_DEBUG_KMS("no buffer object handle for plane %d\n", i);
>  			return -EINVAL;
> @@ -211,7 +215,7 @@ static int framebuffer_check(struct drm_device *dev,
>  		if ((uint64_t) height * r->pitches[i] + r->offsets[i] > UINT_MAX)
>  			return -ERANGE;
>  
> -		if (r->pitches[i] < min_pitch) {
> +		if (block_size && r->pitches[i] < min_pitch) {
>  			DRM_DEBUG_KMS("bad pitch %u for plane %d\n", r->pitches[i], i);
>  			return -EINVAL;
>  		}
> -- 
> 2.18.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel