Re: [PATCH] drm/i915/dsi: Stop using pixel_format_from_register_bits() to parse VBT

[Jani Nikula <jani.nikula@linux.intel.com>]

On Thu, 07 Nov 2024, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Introduce a proper VBT->enum mipi_dsi_pixel_format converter
> instead of abusing pixel_format_from_register_bits() (whose
> job is to parse the pixel format from some pre-ICL DSI
> hardware register).

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

I'd also happily take a patch renaming the VBT macros

PIXEL_FORMAT_RGB666 -> PIXEL_FORMAT_RGB666_PACKED
PIXEL_FORMAT_RGB666_LOOSELY_PACKED -> PIXEL_FORMAT_RGB666

because it's such a PITA that it's the opposite of what the register
macros and MIPI_DSI_* macros have.

>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dsi_vbt.c | 20 ++++++++++++++++++--
>  drivers/gpu/drm/i915/display/vlv_dsi.c       |  3 +--
>  drivers/gpu/drm/i915/display/vlv_dsi.h       |  7 -------
>  3 files changed, 19 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
> index e8129a720210..b2b78f39cfd3 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
> +++ b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
> @@ -745,6 +745,23 @@ void intel_dsi_log_params(struct intel_dsi *intel_dsi)
>  		    str_enabled_disabled(!(intel_dsi->video_frmt_cfg_bits & DISABLE_VIDEO_BTA)));
>  }
>  
> +static enum mipi_dsi_pixel_format vbt_to_dsi_pixel_format(unsigned int format)
> +{
> +	switch (format) {
> +	case PIXEL_FORMAT_RGB888:
> +		return MIPI_DSI_FMT_RGB888;
> +	case PIXEL_FORMAT_RGB666_LOOSELY_PACKED:
> +		return MIPI_DSI_FMT_RGB666;
> +	case PIXEL_FORMAT_RGB666:
> +		return MIPI_DSI_FMT_RGB666_PACKED;
> +	case PIXEL_FORMAT_RGB565:
> +		return MIPI_DSI_FMT_RGB565;
> +	default:
> +		MISSING_CASE(format);
> +		return MIPI_DSI_FMT_RGB666;
> +	}
> +}
> +
>  bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
>  {
>  	struct drm_device *dev = intel_dsi->base.base.dev;
> @@ -762,8 +779,7 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
>  	intel_dsi->clock_stop = mipi_config->enable_clk_stop ? 1 : 0;
>  	intel_dsi->lane_count = mipi_config->lane_cnt + 1;
>  	intel_dsi->pixel_format =
> -			pixel_format_from_register_bits(
> -				mipi_config->videomode_color_format << 7);
> +		vbt_to_dsi_pixel_format(mipi_config->videomode_color_format);
>  
>  	intel_dsi->dual_link = mipi_config->dual_link;
>  	intel_dsi->pixel_overlap = mipi_config->pixel_overlap;
> diff --git a/drivers/gpu/drm/i915/display/vlv_dsi.c b/drivers/gpu/drm/i915/display/vlv_dsi.c
> index 9383eedee2d4..587e98d32053 100644
> --- a/drivers/gpu/drm/i915/display/vlv_dsi.c
> +++ b/drivers/gpu/drm/i915/display/vlv_dsi.c
> @@ -67,9 +67,8 @@ static u16 pixels_from_txbyteclkhs(u16 clk_hs, int bpp, int lane_count,
>  						(bpp * burst_mode_ratio));
>  }
>  
> -enum mipi_dsi_pixel_format pixel_format_from_register_bits(u32 fmt)
> +static enum mipi_dsi_pixel_format pixel_format_from_register_bits(u32 fmt)
>  {
> -	/* It just so happens the VBT matches register contents. */
>  	switch (fmt) {
>  	case VID_MODE_FORMAT_RGB888:
>  		return MIPI_DSI_FMT_RGB888;
> diff --git a/drivers/gpu/drm/i915/display/vlv_dsi.h b/drivers/gpu/drm/i915/display/vlv_dsi.h
> index cf9d7b82f288..a02fd7bd2f2a 100644
> --- a/drivers/gpu/drm/i915/display/vlv_dsi.h
> +++ b/drivers/gpu/drm/i915/display/vlv_dsi.h
> @@ -6,24 +6,17 @@
>  #ifndef __VLV_DSI_H__
>  #define __VLV_DSI_H__
>  
> -#include <linux/types.h>
> -
>  enum port;
>  struct drm_i915_private;
>  struct intel_dsi;
>  
>  #ifdef I915
>  void vlv_dsi_wait_for_fifo_empty(struct intel_dsi *intel_dsi, enum port port);
> -enum mipi_dsi_pixel_format pixel_format_from_register_bits(u32 fmt);
>  void vlv_dsi_init(struct drm_i915_private *dev_priv);
>  #else
>  static inline void vlv_dsi_wait_for_fifo_empty(struct intel_dsi *intel_dsi, enum port port)
>  {
>  }
> -static inline enum mipi_dsi_pixel_format pixel_format_from_register_bits(u32 fmt)
> -{
> -	return 0;
> -}
>  static inline void vlv_dsi_init(struct drm_i915_private *dev_priv)
>  {
>  }

-- 
Jani Nikula, Intel