Re: [PATCH 1/2] drm/i915: Move 90/270 rotation validity check into its own function

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

On Fri, Nov 03, 2017 at 04:37:57PM +0200, Juha-Pekka Heikkila wrote:
> This makes intel_plane_atomic_check_with_state() generally shorter.
> 
> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
> ---
>  drivers/gpu/drm/i915/intel_atomic_plane.c | 53 +++++++++++++++++--------------
>  1 file changed, 30 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
> index 8e6dc15..6c4c82e2d 100644
> --- a/drivers/gpu/drm/i915/intel_atomic_plane.c
> +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
> @@ -107,6 +107,35 @@ intel_plane_destroy_state(struct drm_plane *plane,
>  	drm_atomic_helper_plane_destroy_state(plane, state);
>  }
>  
> +static bool intel_valid_rotation(const struct drm_plane_state *state)
> +{
> +	struct drm_format_name_buf format_name;
> +
> +	if (state->fb->modifier != I915_FORMAT_MOD_Y_TILED &&
> +	    state->fb->modifier != I915_FORMAT_MOD_Yf_TILED) {
> +		DRM_DEBUG_KMS("Y/Yf tiling required for 90/270!\n");
> +		return false;
> +	}
> +
> +	/*
> +	 * 90/270 is not allowed with RGB64 16:16:16:16,
> +	 * RGB 16-bit 5:6:5, and Indexed 8-bit.
> +	 * TBD: Add RGB64 case once its added in supported format list.
> +	 */
> +	switch (state->fb->format->format) {
> +	case DRM_FORMAT_C8:
> +	case DRM_FORMAT_RGB565:
> +		DRM_DEBUG_KMS("Unsupported pixel format %s for 90/270!\n",
> +			      drm_get_format_name(state->fb->format->format,
> +						  &format_name));
> +		return false;
> +
> +	default:
> +		break;
> +	}

Usually there's an empty line after the final return.

> +	return true;
> +}
> +
>  int intel_plane_atomic_check_with_state(const struct intel_crtc_state *old_crtc_state,
>  					struct intel_crtc_state *crtc_state,
>  					const struct intel_plane_state *old_plane_state,
> @@ -138,30 +167,8 @@ int intel_plane_atomic_check_with_state(const struct intel_crtc_state *old_crtc_
>  		crtc_state->base.enable ? crtc_state->pipe_src_h : 0;
>  
>  	if (state->fb && drm_rotation_90_or_270(state->rotation)) {

I'd pull these checks into the new function as well (as an early
return so that we don't have to needlessly indent the whole
function body). Otherwise the function name doesn't really match
the implementation.

> -		struct drm_format_name_buf format_name;
> -
> -		if (state->fb->modifier != I915_FORMAT_MOD_Y_TILED &&
> -		    state->fb->modifier != I915_FORMAT_MOD_Yf_TILED) {
> -			DRM_DEBUG_KMS("Y/Yf tiling required for 90/270!\n");
> +		if (!intel_valid_rotation(state))
>  			return -EINVAL;
> -		}
> -
> -		/*
> -		 * 90/270 is not allowed with RGB64 16:16:16:16,
> -		 * RGB 16-bit 5:6:5, and Indexed 8-bit.
> -		 * TBD: Add RGB64 case once its added in supported format list.
> -		 */
> -		switch (state->fb->format->format) {
> -		case DRM_FORMAT_C8:
> -		case DRM_FORMAT_RGB565:
> -			DRM_DEBUG_KMS("Unsupported pixel format %s for 90/270!\n",
> -			              drm_get_format_name(state->fb->format->format,
> -			                                  &format_name));
> -			return -EINVAL;
> -
> -		default:
> -			break;
> -		}
>  	}
>  
>  	/* CHV ignores the mirror bit when the rotate bit is set :( */
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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