Re: [PATCH RFC v4 7/7] drm/msm/dpu: Use DRM solid_fill property

Pekka Paalanen
2023-06-30T08:27:04Z

[-- Attachment #1: Type: text/plain, Size: 3256 bytes --]

On Thu, 29 Jun 2023 17:25:06 -0700
Jessica Zhang <quic_jesszhan@quicinc.com> wrote:

> Drop DPU_PLANE_COLOR_FILL_FLAG and check the DRM solid_fill property to
> determine if the plane is solid fill. In addition drop the DPU plane
> color_fill field as we can now use drm_plane_state.solid_fill instead,
> and pass in drm_plane_state.alpha to _dpu_plane_color_fill_pipe() to
> allow userspace to configure the alpha value for the solid fill color.
>
> Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 21 +++++++++++++++------
> 1 file changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> index 4476722f03bb..11d4fb771a1f 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> @@ -42,7 +42,6 @@
> #define SHARP_SMOOTH_THR_DEFAULT 8
> #define SHARP_NOISE_THR_DEFAULT 2
>
> -#define DPU_PLANE_COLOR_FILL_FLAG BIT(31)
> #define DPU_ZPOS_MAX 255
>
> /*
> @@ -82,7 +81,6 @@ struct dpu_plane {
>
> enum dpu_sspp pipe;
>
> - uint32_t color_fill;
> bool is_error;
> bool is_rt_pipe;
> const struct dpu_mdss_cfg *catalog;
> @@ -606,6 +604,17 @@ static void _dpu_plane_color_fill_pipe(struct dpu_plane_state *pstate,
> _dpu_plane_setup_scaler(pipe, fmt, true, &pipe_cfg, pstate->rotation);
> }
>
> +static uint32_t _dpu_plane_get_fill_color(struct drm_solid_fill solid_fill)
> +{
> + uint32_t ret = 0;
> +
> + ret |= ((uint8_t) solid_fill.b) << 16;
> + ret |= ((uint8_t) solid_fill.g) << 8;
> + ret |= ((uint8_t) solid_fill.r);

solid_fill.r, g and b are uint32_t, yes?

What's the value encoding in the UAPI? That doc was missing.

I wouldn't expect the UAPI to use 32-bit variables if it was
essentially 8-bit, so this conversion looks wrong.

Nominal color value 1.0 in u8 is 0xff. The same in u32 is probably
0xffffffff? So a simple cast to u8 won't work. You'd want to take the
upper 8 bits instead.

Thanks,
pq

> +
> + return ret;
> +}
> +
> /**
> * _dpu_plane_color_fill - enables color fill on plane
> * @pdpu: Pointer to DPU plane object
> @@ -977,9 +986,9 @@ void dpu_plane_flush(struct drm_plane *plane)
> if (pdpu->is_error)
> /* force white frame with 100% alpha pipe output on error */
> _dpu_plane_color_fill(pdpu, 0xFFFFFF, 0xFF);
> - else if (pdpu->color_fill & DPU_PLANE_COLOR_FILL_FLAG)
> - /* force 100% alpha */
> - _dpu_plane_color_fill(pdpu, pdpu->color_fill, 0xFF);
> + else if (drm_plane_solid_fill_enabled(plane->state))
> + _dpu_plane_color_fill(pdpu, _dpu_plane_get_fill_color(plane->state->solid_fill),
> + plane->state->alpha);
> else {
> dpu_plane_flush_csc(pdpu, &pstate->pipe);
> dpu_plane_flush_csc(pdpu, &pstate->r_pipe);
> @@ -1024,7 +1033,7 @@ static void dpu_plane_sspp_update_pipe(struct drm_plane *plane,
> }
>
> /* override for color fill */
> - if (pdpu->color_fill & DPU_PLANE_COLOR_FILL_FLAG) {
> + if (drm_plane_solid_fill_enabled(plane->state)) {
> _dpu_plane_set_qos_ctrl(plane, pipe, false);
>
> /* skip remaining processing on color fill */
>

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]