[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <f77c33eb-2696-2aa4-9f70-f783a04cf1cc@quicinc.com>
Date: Fri, 30 Jun 2023 18:26:15 -0700
From: Jessica Zhang <quic_jesszhan@...cinc.com>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>,
Thomas Zimmermann <tzimmermann@...e.de>,
David Airlie <airlied@...il.com>,
Daniel Vetter <daniel@...ll.ch>,
Rob Clark <robdclark@...il.com>, Sean Paul <sean@...rly.run>,
Marijn Suijten <marijn.suijten@...ainline.org>
CC: <quic_abhinavk@...cinc.com>, <ppaalanen@...il.com>,
<contact@...rsion.fr>, <laurent.pinchart@...asonboard.com>,
<sebastian.wick@...hat.com>, <ville.syrjala@...ux.intel.com>,
<dri-devel@...ts.freedesktop.org>, <linux-kernel@...r.kernel.org>,
<linux-arm-msm@...r.kernel.org>, <freedreno@...ts.freedesktop.org>,
<wayland-devel@...ts.freedesktop.org>
Subject: Re: [PATCH RFC v4 7/7] drm/msm/dpu: Use DRM solid_fill property
On 6/29/2023 5:59 PM, Dmitry Baryshkov wrote:
> On 30/06/2023 03:25, Jessica Zhang 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@...cinc.com>
>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
>
> Minor suggestion below.
>
>> ---
>> 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)
>
> Please consider accepting drm_plane_state instead and handling alpha
> here. Then _dpu_color_fill can accept rgba colour instead of separate
> RGB and alpha values.
Hi Dmitry,
Do you mean adding a patch to refactor _dpu_plane_color_fill() to accept
an RGBA8888 color?
Since, currently, the `color` parameter gets truncated to a BGR888 value
and OR'd with the `alpha` parameter [1].
Thanks,
Jessica Zhang
[1]
https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c#L682
>
>> +{
>> + uint32_t ret = 0;
>> +
>> + ret |= ((uint8_t) solid_fill.b) << 16;
>> + ret |= ((uint8_t) solid_fill.g) << 8;
>> + ret |= ((uint8_t) solid_fill.r);
>> +
>> + 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 */
>>
>
> --
> With best wishes
> Dmitry
>
Powered by blists - more mailing lists