Re: [RFC PATCH 02/10] drm/i915: make transcoder_name return a string

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Tue, Mar 15, 2016 at 09:51:10PM +0200, Jani Nikula wrote:
> Nicer for eDP (actually "eDP" instead of "D"), and makes future
> expansion for DSI transcoders easier.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h            | 17 ++++++++++++++++-
>  drivers/gpu/drm/i915/intel_display.c       |  4 ++--
>  drivers/gpu/drm/i915/intel_fifo_underrun.c |  6 +++---
>  3 files changed, 21 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 8ef3c88d0ed2..5e4a42d996d8 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -125,7 +125,22 @@ enum transcoder {
>  	TRANSCODER_EDP,
>  	I915_MAX_TRANSCODERS
>  };
> -#define transcoder_name(t) ((t) + 'A')
> +
> +static inline const char *transcoder_name(enum transcoder transcoder)
> +{
> +	switch (transcoder) {
> +	case TRANSCODER_A:
> +		return "A";
> +	case TRANSCODER_B:
> +		return "B";
> +	case TRANSCODER_C:
> +		return "C";
> +	case TRANSCODER_EDP:
> +		return "eDP";

It's all caps "EDP" in Bspec.

with that
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>


> +	default:
> +		return "<invalid>";
> +	}
> +}
>  
>  /*
>   * I915_MAX_PLANES in the enum below is the maximum (across all platforms)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index ce55f0b683c6..7977a818326d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12096,7 +12096,7 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
>  	DRM_DEBUG_KMS("[CRTC:%d]%s config %p for pipe %c\n", crtc->base.base.id,
>  		      context, pipe_config, pipe_name(crtc->pipe));
>  
> -	DRM_DEBUG_KMS("cpu_transcoder: %c\n", transcoder_name(pipe_config->cpu_transcoder));
> +	DRM_DEBUG_KMS("cpu_transcoder: %s\n", transcoder_name(pipe_config->cpu_transcoder));
>  	DRM_DEBUG_KMS("pipe bpp: %i, dithering: %i\n",
>  		      pipe_config->pipe_bpp, pipe_config->dither);
>  	DRM_DEBUG_KMS("fdi/pch: %i, lanes: %i, gmch_m: %u, gmch_n: %u, link_m: %u, link_n: %u, tu: %u\n",
> @@ -16227,7 +16227,7 @@ intel_display_print_error_state(struct drm_i915_error_state_buf *m,
>  	}
>  
>  	for (i = 0; i < error->num_transcoders; i++) {
> -		err_printf(m, "CPU transcoder: %c\n",
> +		err_printf(m, "CPU transcoder: %s\n",
>  			   transcoder_name(error->transcoder[i].cpu_transcoder));
>  		err_printf(m, "  Power: %s\n",
>  			   onoff(error->transcoder[i].power_domain_on));
> diff --git a/drivers/gpu/drm/i915/intel_fifo_underrun.c b/drivers/gpu/drm/i915/intel_fifo_underrun.c
> index bda526660e20..19e50fdf9a91 100644
> --- a/drivers/gpu/drm/i915/intel_fifo_underrun.c
> +++ b/drivers/gpu/drm/i915/intel_fifo_underrun.c
> @@ -212,7 +212,7 @@ static void cpt_check_pch_fifo_underruns(struct intel_crtc *crtc)
>  	I915_WRITE(SERR_INT, SERR_INT_TRANS_FIFO_UNDERRUN(pch_transcoder));
>  	POSTING_READ(SERR_INT);
>  
> -	DRM_ERROR("pch fifo underrun on pch transcoder %c\n",
> +	DRM_ERROR("pch fifo underrun on pch transcoder %s\n",
>  		  transcoder_name(pch_transcoder));
>  }
>  
> @@ -235,7 +235,7 @@ static void cpt_set_fifo_underrun_reporting(struct drm_device *dev,
>  
>  		if (old && I915_READ(SERR_INT) &
>  		    SERR_INT_TRANS_FIFO_UNDERRUN(pch_transcoder)) {
> -			DRM_ERROR("uncleared pch fifo underrun on pch transcoder %c\n",
> +			DRM_ERROR("uncleared pch fifo underrun on pch transcoder %s\n",
>  				  transcoder_name(pch_transcoder));
>  		}
>  	}
> @@ -386,7 +386,7 @@ void intel_pch_fifo_underrun_irq_handler(struct drm_i915_private *dev_priv,
>  {
>  	if (intel_set_pch_fifo_underrun_reporting(dev_priv, pch_transcoder,
>  						  false))
> -		DRM_ERROR("PCH transcoder %c FIFO underrun\n",
> +		DRM_ERROR("PCH transcoder %s FIFO underrun\n",
>  			  transcoder_name(pch_transcoder));

We're a bit lazy with the PCH transcoders. Sometimes we use enum
transcoder sometimes enum pipe. In practice it doesn't make any
difference though since it's a 1:1 mapping. But at some point I
was arguing with myself whether I should try to change it all to
use one or the other, or even introduce a separate pch_transcoder
enum for clarity.

>  }
>  
> -- 
> 2.1.4

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx