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

From: Liviu Dudau
Date: Wed Jun 05 2019 - 05:26:24 EST


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@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;
> +}

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@xxxxxxx>

Best regards,
Liviu

> +
> 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! /
---------------
Â\_(ã)_/Â