Re: [PATCH 01/86] drm/fbdev-helper: Move color-mode lookup into 4CC format helper

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

Hi Thomas,

Thank you for the patch.

On Fri, Aug 16, 2024 at 02:22:27PM +0200, Thomas Zimmermann wrote:
> The color mode specified on the kernel command line gives the user's
> preferred color depth and number of bits per pixel. Move the
> color-mode-to-format conversion form fbdev helpers into a 4CC helper,

s/form/from/

> so that is can be shared among DRM clients.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>

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

> ---
>  drivers/gpu/drm/drm_fb_helper.c | 70 +++++++--------------------------
>  drivers/gpu/drm/drm_fourcc.c    | 30 +++++++++++++-
>  include/drm/drm_fourcc.h        |  1 +
>  3 files changed, 45 insertions(+), 56 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 29c53f9f449c..af1fe79c701d 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -1441,67 +1441,27 @@ int drm_fb_helper_pan_display(struct fb_var_screeninfo *var,
>  EXPORT_SYMBOL(drm_fb_helper_pan_display);
>  
>  static uint32_t drm_fb_helper_find_format(struct drm_fb_helper *fb_helper, const uint32_t *formats,
> -					  size_t format_count, uint32_t bpp, uint32_t depth)
> +					  size_t format_count, unsigned int color_mode)
>  {
>  	struct drm_device *dev = fb_helper->dev;
>  	uint32_t format;
>  	size_t i;
>  
> -	/*
> -	 * Do not consider YUV or other complicated formats
> -	 * for framebuffers. This means only legacy formats
> -	 * are supported (fmt->depth is a legacy field), but
> -	 * the framebuffer emulation can only deal with such
> -	 * formats, specifically RGB/BGA formats.
> -	 */
> -	format = drm_mode_legacy_fb_format(bpp, depth);
> -	if (!format)
> -		goto err;
> +	format = drm_driver_color_mode_format(dev, color_mode);
> +	if (!format) {
> +		drm_info(dev, "unsupported color mode of %d\n", color_mode);
> +		return DRM_FORMAT_INVALID;
> +	}
>  
>  	for (i = 0; i < format_count; ++i) {
>  		if (formats[i] == format)
>  			return format;
>  	}
> -
> -err:
> -	/* We found nothing. */
> -	drm_warn(dev, "bpp/depth value of %u/%u not supported\n", bpp, depth);
> +	drm_warn(dev, "format %p4cc not supported\n", &format);
>  
>  	return DRM_FORMAT_INVALID;
>  }
>  
> -static uint32_t drm_fb_helper_find_color_mode_format(struct drm_fb_helper *fb_helper,
> -						     const uint32_t *formats, size_t format_count,
> -						     unsigned int color_mode)
> -{
> -	struct drm_device *dev = fb_helper->dev;
> -	uint32_t bpp, depth;
> -
> -	switch (color_mode) {
> -	case 1:
> -	case 2:
> -	case 4:
> -	case 8:
> -	case 16:
> -	case 24:
> -		bpp = depth = color_mode;
> -		break;
> -	case 15:
> -		bpp = 16;
> -		depth = 15;
> -		break;
> -	case 32:
> -		bpp = 32;
> -		depth = 24;
> -		break;
> -	default:
> -		drm_info(dev, "unsupported color mode of %d\n", color_mode);
> -		return DRM_FORMAT_INVALID;
> -	}
> -
> -	return drm_fb_helper_find_format(fb_helper, formats, format_count, bpp, depth);
> -}
> -
>  static int __drm_fb_helper_find_sizes(struct drm_fb_helper *fb_helper,
>  				      struct drm_fb_helper_surface_size *sizes)
>  {
> @@ -1531,10 +1491,10 @@ static int __drm_fb_helper_find_sizes(struct drm_fb_helper *fb_helper,
>  			if (!cmdline_mode->bpp_specified)
>  				continue;
>  
> -			surface_format = drm_fb_helper_find_color_mode_format(fb_helper,
> -									      plane->format_types,
> -									      plane->format_count,
> -									      cmdline_mode->bpp);
> +			surface_format = drm_fb_helper_find_format(fb_helper,
> +								   plane->format_types,
> +								   plane->format_count,
> +								   cmdline_mode->bpp);
>  			if (surface_format != DRM_FORMAT_INVALID)
>  				break; /* found supported format */
>  		}
> @@ -1544,10 +1504,10 @@ static int __drm_fb_helper_find_sizes(struct drm_fb_helper *fb_helper,
>  			break; /* found supported format */
>  
>  		/* try preferred color mode */
> -		surface_format = drm_fb_helper_find_color_mode_format(fb_helper,
> -								      plane->format_types,
> -								      plane->format_count,
> -								      fb_helper->preferred_bpp);
> +		surface_format = drm_fb_helper_find_format(fb_helper,
> +							   plane->format_types,
> +							   plane->format_count,
> +							   fb_helper->preferred_bpp);
>  		if (surface_format != DRM_FORMAT_INVALID)
>  			break; /* found supported format */
>  	}
> diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> index 193cf8ed7912..3a94ca211f9c 100644
> --- a/drivers/gpu/drm/drm_fourcc.c
> +++ b/drivers/gpu/drm/drm_fourcc.c
> @@ -36,7 +36,6 @@
>   * @depth: bit depth per pixel
>   *
>   * Computes a drm fourcc pixel format code for the given @bpp/@depth values.
> - * Useful in fbdev emulation code, since that deals in those values.
>   */
>  uint32_t drm_mode_legacy_fb_format(uint32_t bpp, uint32_t depth)
>  {
> @@ -140,6 +139,35 @@ uint32_t drm_driver_legacy_fb_format(struct drm_device *dev,
>  }
>  EXPORT_SYMBOL(drm_driver_legacy_fb_format);
>  
> +/**
> + * drm_driver_color_mode_format - Compute DRM 4CC code from color mode
> + * @dev: DRM device
> + * @color_mode: command-line color mode
> + *
> + * Computes a DRM 4CC pixel format code for the given color mode using
> + * drm_driver_color_mode(). The color mode is in the format used and the
> + * kernel command line. It specifies the number of bits per pixel
> + * and color depth in a single value.
> + *
> + * Useful in fbdev emulation code, since that deals in those values. The
> + * helper does not consider YUV or other complicated formats. This means
> + * only legacy formats are supported (fmt->depth is a legacy field), but
> + * the framebuffer emulation can only deal with such formats, specifically
> + * RGB/BGA formats.
> + */
> +uint32_t drm_driver_color_mode_format(struct drm_device *dev, unsigned int color_mode)
> +{
> +	switch (color_mode) {
> +	case 15:
> +		return drm_driver_legacy_fb_format(dev, 16, 15);
> +	case 32:
> +		return drm_driver_legacy_fb_format(dev, 32, 24);
> +	default:
> +		return drm_driver_legacy_fb_format(dev, color_mode, color_mode);
> +	}
> +}
> +EXPORT_SYMBOL(drm_driver_color_mode_format);
> +
>  /*
>   * Internal function to query information for a given format. See
>   * drm_format_info() for the public API.
> diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
> index ccf91daa4307..c3f4405d6662 100644
> --- a/include/drm/drm_fourcc.h
> +++ b/include/drm/drm_fourcc.h
> @@ -313,6 +313,7 @@ drm_get_format_info(struct drm_device *dev,
>  uint32_t drm_mode_legacy_fb_format(uint32_t bpp, uint32_t depth);
>  uint32_t drm_driver_legacy_fb_format(struct drm_device *dev,
>  				     uint32_t bpp, uint32_t depth);
> +uint32_t drm_driver_color_mode_format(struct drm_device *dev, unsigned int color_mode);
>  unsigned int drm_format_info_block_width(const struct drm_format_info *info,
>  					 int plane);
>  unsigned int drm_format_info_block_height(const struct drm_format_info *info,
> -- 
> 2.46.0
> 

-- 
Regards,

Laurent Pinchart