[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <705fbc71-7610-1567-42cd-b52caa94a12a@quicinc.com>
Date: Mon, 7 Aug 2023 09:59:46 -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 v5 09/10] drm/msm/dpu: Use DRM solid_fill property
On 7/31/2023 5:52 PM, Dmitry Baryshkov wrote:
> On 01/08/2023 03:39, Jessica Zhang wrote:
>>
>>
>> On 7/30/2023 9:15 PM, Dmitry Baryshkov wrote:
>>> On 28/07/2023 20:02, 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.
>>>>
>>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
>>>> Signed-off-by: Jessica Zhang <quic_jesszhan@...cinc.com>
>>>> ---
>>>> drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 24
>>>> ++++++++++++++++++------
>>>> 1 file changed, 18 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 114c803ff99b..95fc0394d13e 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,20 @@ 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_bgr_fill_color(struct drm_solid_fill
>>>> solid_fill)
>>>
>>> As I commented for v4 (please excuse me for not responding to your
>>> email at thattime), we can return abgr here, taking
>>> plane->state->alpha into account.
>>
>> Hi Dmitry,
>>
>> Since it seems that this comment wasn't resolved, I can drop your R-B
>> tag in the next revision.
>
> It's a minor issue, so no need to drop the tag.
>
>>
>> From my previous response, I pointed out that the color parameter
>> expects an RGB value [1].
>>
>> So is the intention here to refactor _dpu_plane_color_fill() to accept
>> an ABGR8888 color?
>
> That's what I'm suggesting.
Hi Dmitry,
Got it, sounds good to me.
Thanks,
Jessica Zhang
>
>>
>> Thanks,
>>
>> Jessica Zhang
>>
>> [1]
>> https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c#L676
>>
>>>
>>>> +{
>>>> + uint32_t ret = 0;
>>>> + uint8_t b = solid_fill.b >> 24;
>>>> + uint8_t g = solid_fill.g >> 24;
>>>> + uint8_t r = solid_fill.r >> 24;
>>>> +
>>>> + ret |= b << 16;
>>>> + ret |= g << 8;
>>>> + ret |= r;
>>>> +
>>>> + return ret;
>>>> +}
>>>> +
>>>> /**
>>>> * _dpu_plane_color_fill - enables color fill on plane
>>>> * @pdpu: Pointer to DPU plane object
>>>> @@ -977,9 +989,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_bgr_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 +1036,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
>>>
>
> --
> With best wishes
> Dmitry
>
Powered by blists - more mailing lists