Re: [PATCH v3 1/2] drm/komeda: Add rotation support on Komeda driver

[james qian wang (Arm Technology China)]

On Tue, May 28, 2019 at 11:57:00AM +0800, Lowry Li (Arm Technology China) wrote:
> From: "Lowry Li (Arm Technology China)" <Lowry.Li@xxxxxxx>
> 
> - Adds rotation property to plane.
> - Komeda display rotation support diverges from the specific formats,
> so need to check the user required rotation type with the format caps
> and reject the commit if it can not be supported.
> - In the layer validate flow, sets the rotation value to the layer
> state. If r90 or r270, swap the width and height of the data flow
> for next stage.
> 
> Signed-off-by: Lowry Li (Arm Technology China) <lowry.li@xxxxxxx>
> ---
>  drivers/gpu/drm/arm/display/komeda/komeda_format_caps.h  | 11 +++++++++++
>  .../gpu/drm/arm/display/komeda/komeda_pipeline_state.c   |  7 +++++++
>  drivers/gpu/drm/arm/display/komeda/komeda_plane.c        | 16 ++++++++++++++++
>  3 files changed, 34 insertions(+)
> 
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_format_caps.h b/drivers/gpu/drm/arm/display/komeda/komeda_format_caps.h
> index bc3b2df36..96de22e 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_format_caps.h
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_format_caps.h
> @@ -79,6 +79,17 @@ struct komeda_format_caps_table {
>  
>  extern u64 komeda_supported_modifiers[];
>  
> +static inline const char *komeda_get_format_name(u32 fourcc, u64 modifier)
> +{
> +	struct drm_format_name_buf buf;
> +	static char name[64];
> +
> +	snprintf(name, sizeof(name), "%s with modifier: 0x%llx.",
> +		 drm_get_format_name(fourcc, &buf), modifier);
> +
> +	return name;
> +}
> +
>  const struct komeda_format_caps *
>  komeda_get_format_caps(struct komeda_format_caps_table *table,
>  		       u32 fourcc, u64 modifier);
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> index db34ea2..34737c0 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> @@ -339,6 +339,13 @@ struct komeda_pipeline_state *
>  	/* update the data flow for the next stage */
>  	komeda_component_set_output(&dflow->input, &layer->base, 0);
>  
> +	/*
> +	 * The rotation has been handled by layer, so adjusted the data flow for
> +	 * the next stage.
> +	 */
> +	if (drm_rotation_90_or_270(st->rot))
> +		swap(dflow->in_h, dflow->in_w);
> +
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_plane.c b/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
> index 9b87c25..c9f37ff 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
> @@ -10,6 +10,7 @@
>  #include <drm/drm_print.h>
>  #include "komeda_dev.h"
>  #include "komeda_kms.h"
> +#include "komeda_framebuffer.h"
>  
>  static int
>  komeda_plane_init_data_flow(struct drm_plane_state *st,
> @@ -17,6 +18,7 @@
>  {
>  	struct komeda_plane_state *kplane_st = to_kplane_st(st);
>  	struct drm_framebuffer *fb = st->fb;
> +	const struct komeda_format_caps *caps = to_kfb(fb)->format_caps;
>  
>  	memset(dflow, 0, sizeof(*dflow));
>  
> @@ -37,6 +39,15 @@
>  	dflow->in_w = st->src_w >> 16;
>  	dflow->in_h = st->src_h >> 16;
>  
> +	dflow->rot = drm_rotation_simplify(st->rotation, caps->supported_rots);
> +	if (!has_bits(dflow->rot, caps->supported_rots)) {
> +		DRM_DEBUG_ATOMIC("rotation(0x%x) isn't supported by %s.\n",
> +				 dflow->rot,
> +				 komeda_get_format_name(caps->fourcc,
> +							fb->modifier));
> +		return -EINVAL;
> +	}
> +
>  	dflow->en_img_enhancement = kplane_st->img_enhancement;
>  
>  	komeda_complete_data_flow_cfg(dflow);
> @@ -303,6 +314,11 @@ static int komeda_plane_add(struct komeda_kms_dev *kms,
>  
>  	drm_plane_helper_add(plane, &komeda_plane_helper_funcs);
>  
> +	err = drm_plane_create_rotation_property(plane, DRM_MODE_ROTATE_0,
> +						 layer->supported_rots);
> +	if (err)
> +		goto cleanup;
> +
>  	err = drm_plane_create_alpha_property(plane);
>  	if (err)
>  		goto cleanup;
> -- 
> 1.9.1
> 

Looks good to me.

Reviewed-by: James Qian Wang (Arm Technology China) <james.qian.wang@xxxxxxx>