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

[Lowry Li (Arm Technology China)]

On Wed, Jun 05, 2019 at 05:22:22PM +0800, Liviu Dudau wrote:
> Hi Lowry,
> 
> On Mon, Apr 22, 2019 at 04:16:26AM +0100, Lowry Li (Arm Technology China) wrote:
> > - 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 at arm.com>
> > ---
> >  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;
> > +}
> 
> Can you roll the content of this function inside the if () {.....} part? We
> only have one use for it, I don't see the need to split it into a separate
> function.
> 
> With that: Reviewed-by: Liviu Dudau <liviu.dudau at arm.com>
> 
> Best regards,
> Liviu
> 
Hi Liviu,

Thanks a lot for your comments. This function is generic one which
will be called from many places further, like print the format string
log and etc. So we split it out in komeda_format_caps header file
here :)

Best regards,
Lowry

> > +
> >  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 9b29e9a..8c133e4 100644
> > --- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> > @@ -317,6 +317,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 14d6861..5e5bfdb 100644
> > --- a/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
> > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
> > @@ -9,12 +9,14 @@
> >  #include <drm/drm_plane_helper.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,
> >  			    struct komeda_data_flow_cfg *dflow)
> >  {
> >  	struct drm_framebuffer *fb = st->fb;
> > +	const struct komeda_format_caps *caps = to_kfb(fb)->format_caps;
> >  
> >  	memset(dflow, 0, sizeof(*dflow));
> >  
> > @@ -35,6 +37,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;
> > +	}
> > +
> >  	return 0;
> >  }
> >  
> > @@ -233,6 +244,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
> > 
> 
> -- 
> ====================
> | I would like to |
> | fix the world,  |
> | but they're not |
> | giving me the   |
>  \ source code!  /
>   ---------------
>     ¯\_(ツ)_/¯

-- 
Regards,
Lowry