[PATCH v3 2/3] drm/i915: Refactor icl_is_hdr_plane

[Ville Syrjälä]

On Tue, Feb 05, 2019 at 07:29:21PM -0800, Kevin Strasser wrote:
> Change the api in order to enable callers that can't supply a valid
> intel_plane pointer, as would be the case prior to calling
> drm_universal_plane_init.
> 
> Cc: Uma Shankar <uma.shankar at intel.com>
> Cc: Shashank Sharma <shashank.sharma at intel.com>
> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Cc: David Airlie <airlied at linux.ie>
> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> Cc: dri-devel at lists.freedesktop.org
> Signed-off-by: Kevin Strasser <kevin.strasser at intel.com>
> ---
>  drivers/gpu/drm/i915/intel_atomic.c  | 4 +++-
>  drivers/gpu/drm/i915/intel_display.c | 5 +++--
>  drivers/gpu/drm/i915/intel_drv.h     | 7 ++++---
>  drivers/gpu/drm/i915/intel_sprite.c  | 6 +++---
>  4 files changed, 13 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
> index 16263ad..7f824fd 100644
> --- a/drivers/gpu/drm/i915/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> @@ -210,6 +210,7 @@ static void intel_atomic_setup_scaler(struct intel_crtc_scaler_state *scaler_sta
>  				      int *scaler_id)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
> +	struct intel_plane *intel_plane;

s/intel_plane/plane/ here please. I'm trying to make our variable
naming to be more consistent. Still ways to go as we can see from
the later hunks, but at least new code should use consistent names.

Also the declaration can be moved into the if block itself.

>  	int j;
>  	u32 mode;
>  
> @@ -232,10 +233,11 @@ static void intel_atomic_setup_scaler(struct intel_crtc_scaler_state *scaler_sta
>  	if (plane_state && plane_state->base.fb &&
>  	    plane_state->base.fb->format->is_yuv &&
>  	    plane_state->base.fb->format->num_planes > 1) {
> +		intel_plane = to_intel_plane(plane_state->base.plane);
>  		if (IS_GEN(dev_priv, 9) &&
>  		    !IS_GEMINILAKE(dev_priv)) {
>  			mode = SKL_PS_SCALER_MODE_NV12;
> -		} else if (icl_is_hdr_plane(to_intel_plane(plane_state->base.plane))) {
> +		} else if (icl_is_hdr_plane(dev_priv, intel_plane->id)) {
>  			/*
>  			 * On gen11+'s HDR planes we only use the scaler for
>  			 * scaling. They have a dedicated chroma upsampler, so
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 4d5ec92..2d9639d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3726,7 +3726,7 @@ u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state,
>  	plane_color_ctl |= PLANE_COLOR_PLANE_GAMMA_DISABLE;
>  	plane_color_ctl |= glk_plane_color_ctl_alpha(plane_state);
>  
> -	if (fb->format->is_yuv && !icl_is_hdr_plane(plane)) {
> +	if (fb->format->is_yuv && !icl_is_hdr_plane(dev_priv, plane->id)) {
>  		if (plane_state->base.color_encoding == DRM_COLOR_YCBCR_BT709)
>  			plane_color_ctl |= PLANE_COLOR_CSC_MODE_YUV709_TO_RGB709;
>  		else
> @@ -5052,13 +5052,14 @@ static int skl_update_scaler_plane(struct intel_crtc_state *crtc_state,
>  {
>  	struct intel_plane *intel_plane =
>  		to_intel_plane(plane_state->base.plane);
> +	struct drm_i915_private *dev_priv = to_i915(intel_plane->base.dev);
>  	struct drm_framebuffer *fb = plane_state->base.fb;
>  	int ret;
>  	bool force_detach = !fb || !plane_state->base.visible;
>  	bool need_scaler = false;
>  
>  	/* Pre-gen11 and SDR planes always need a scaler for planar formats. */
> -	if (!icl_is_hdr_plane(intel_plane) &&
> +	if (!icl_is_hdr_plane(dev_priv, intel_plane->id) &&
>  	    fb && fb->format->format == DRM_FORMAT_NV12)
>  		need_scaler = true;
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 90ba543..154901f 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -2313,12 +2313,13 @@ static inline bool icl_is_nv12_y_plane(enum plane_id id)
>  	return false;
>  }
>  
> -static inline bool icl_is_hdr_plane(struct intel_plane *plane)
> +static inline bool icl_is_hdr_plane(struct drm_i915_private *dev_priv,
> +				    enum plane_id id)

s/id/plane_id/ please. Again, going for consistency.

With those this is
Reviewed-by: Ville Syrjälä <ville.syrjala at linux.intel.com>

>  {
> -	if (INTEL_GEN(to_i915(plane->base.dev)) < 11)
> +	if (INTEL_GEN(dev_priv) < 11)
>  		return false;
>  
> -	return plane->id < PLANE_SPRITE2;
> +	return id < PLANE_SPRITE2;
>  }
>  
>  /* intel_tv.c */
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index cd42e81..10b37e8 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -336,7 +336,7 @@ skl_program_scaler(struct intel_plane *plane,
>  
>  	/* TODO: handle sub-pixel coordinates */
>  	if (plane_state->base.fb->format->format == DRM_FORMAT_NV12 &&
> -	    !icl_is_hdr_plane(plane)) {
> +	    !icl_is_hdr_plane(dev_priv, plane->id)) {
>  		y_hphase = skl_scaler_calc_phase(1, hscale, false);
>  		y_vphase = skl_scaler_calc_phase(1, vscale, false);
>  
> @@ -511,7 +511,7 @@ skl_program_plane(struct intel_plane *plane,
>  	I915_WRITE_FW(PLANE_AUX_DIST(pipe, plane_id),
>  		      (plane_state->color_plane[1].offset - surf_addr) | aux_stride);
>  
> -	if (icl_is_hdr_plane(plane)) {
> +	if (icl_is_hdr_plane(dev_priv, plane_id)) {
>  		u32 cus_ctl = 0;
>  
>  		if (linked) {
> @@ -536,7 +536,7 @@ skl_program_plane(struct intel_plane *plane,
>  		I915_WRITE_FW(PLANE_COLOR_CTL(pipe, plane_id),
>  			      plane_state->color_ctl);
>  
> -	if (fb->format->is_yuv && icl_is_hdr_plane(plane))
> +	if (fb->format->is_yuv && icl_is_hdr_plane(dev_priv, plane_id))
>  		icl_program_input_csc(plane, crtc_state, plane_state);
>  
>  	skl_write_plane_wm(plane, crtc_state);
> -- 
> 2.7.4

-- 
Ville Syrjälä
Intel