Re: [PATCH v2] drm: fb-helper: Reject all pixel format changing requests

[Daniel Vetter]

On Wed, Oct 03, 2018 at 07:45:38PM +0300, Eugeniy Paltsev wrote:
> drm fbdev emulation doesn't support changing the pixel format at all,
> so reject all pixel format changing requests.

For next time around: Please keep the note here why we need this and what
the impact is. Otherwise it's not immediately clear why we should backport
this patch to all stable kernels.

I'll gather a few acks and then apply.
-Daniel
> 
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@xxxxxxxxxxxx>
> ---
> Changes v1->v2:
>  * Reject all pixel format changing request, not just the invalid ones.
> 
>  drivers/gpu/drm/drm_fb_helper.c | 91 ++++++++++++-----------------------------
>  1 file changed, 26 insertions(+), 65 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 16ec93b75dbf..48598d7f673f 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -1580,6 +1580,25 @@ int drm_fb_helper_ioctl(struct fb_info *info, unsigned int cmd,
>  }
>  EXPORT_SYMBOL(drm_fb_helper_ioctl);
>  
> +static bool drm_fb_pixel_format_equal(const struct fb_var_screeninfo *var_1,
> +				      const struct fb_var_screeninfo *var_2)
> +{
> +	return var_1->bits_per_pixel == var_2->bits_per_pixel &&
> +	       var_1->grayscale == var_2->grayscale &&
> +	       var_1->red.offset == var_2->red.offset &&
> +	       var_1->red.length == var_2->red.length &&
> +	       var_1->red.msb_right == var_2->red.msb_right &&
> +	       var_1->green.offset == var_2->green.offset &&
> +	       var_1->green.length == var_2->green.length &&
> +	       var_1->green.msb_right == var_2->green.msb_right &&
> +	       var_1->blue.offset == var_2->blue.offset &&
> +	       var_1->blue.length == var_2->blue.length &&
> +	       var_1->blue.msb_right == var_2->blue.msb_right &&
> +	       var_1->transp.offset == var_2->transp.offset &&
> +	       var_1->transp.length == var_2->transp.length &&
> +	       var_1->transp.msb_right == var_2->transp.msb_right;
> +}
> +
>  /**
>   * drm_fb_helper_check_var - implementation for &fb_ops.fb_check_var
>   * @var: screeninfo to check
> @@ -1590,7 +1609,6 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo *var,
>  {
>  	struct drm_fb_helper *fb_helper = info->par;
>  	struct drm_framebuffer *fb = fb_helper->fb;
> -	int depth;
>  
>  	if (var->pixclock != 0 || in_dbg_master())
>  		return -EINVAL;
> @@ -1610,72 +1628,15 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo *var,
>  		return -EINVAL;
>  	}
>  
> -	switch (var->bits_per_pixel) {
> -	case 16:
> -		depth = (var->green.length == 6) ? 16 : 15;
> -		break;
> -	case 32:
> -		depth = (var->transp.length > 0) ? 32 : 24;
> -		break;
> -	default:
> -		depth = var->bits_per_pixel;
> -		break;
> -	}
> -
> -	switch (depth) {
> -	case 8:
> -		var->red.offset = 0;
> -		var->green.offset = 0;
> -		var->blue.offset = 0;
> -		var->red.length = 8;
> -		var->green.length = 8;
> -		var->blue.length = 8;
> -		var->transp.length = 0;
> -		var->transp.offset = 0;
> -		break;
> -	case 15:
> -		var->red.offset = 10;
> -		var->green.offset = 5;
> -		var->blue.offset = 0;
> -		var->red.length = 5;
> -		var->green.length = 5;
> -		var->blue.length = 5;
> -		var->transp.length = 1;
> -		var->transp.offset = 15;
> -		break;
> -	case 16:
> -		var->red.offset = 11;
> -		var->green.offset = 5;
> -		var->blue.offset = 0;
> -		var->red.length = 5;
> -		var->green.length = 6;
> -		var->blue.length = 5;
> -		var->transp.length = 0;
> -		var->transp.offset = 0;
> -		break;
> -	case 24:
> -		var->red.offset = 16;
> -		var->green.offset = 8;
> -		var->blue.offset = 0;
> -		var->red.length = 8;
> -		var->green.length = 8;
> -		var->blue.length = 8;
> -		var->transp.length = 0;
> -		var->transp.offset = 0;
> -		break;
> -	case 32:
> -		var->red.offset = 16;
> -		var->green.offset = 8;
> -		var->blue.offset = 0;
> -		var->red.length = 8;
> -		var->green.length = 8;
> -		var->blue.length = 8;
> -		var->transp.length = 8;
> -		var->transp.offset = 24;
> -		break;
> -	default:
> +	/*
> +	 * drm fbdev emulation doesn't support changing the pixel format at all,
> +	 * so reject all pixel format changing requests.
> +	 */
> +	if (!drm_fb_pixel_format_equal(var, &info->var)) {
> +		DRM_DEBUG("fbdev emulation doesn't support changing the pixel format\n");
>  		return -EINVAL;
>  	}
> +
>  	return 0;
>  }
>  EXPORT_SYMBOL(drm_fb_helper_check_var);
> -- 
> 2.14.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch