Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <23cf0a04-7988-9d83-7411-eec1ff270fad@linaro.org>
Date:   Tue, 1 Aug 2023 03:52:25 +0300
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.13.0
Subject: Re: [PATCH RFC v5 09/10] drm/msm/dpu: Use DRM solid_fill property
Content-Language: en-GB
To:     Jessica Zhang <quic_jesszhan@quicinc.com>,
        Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
        Maxime Ripard <mripard@kernel.org>,
        Thomas Zimmermann <tzimmermann@suse.de>,
        David Airlie <airlied@gmail.com>,
        Daniel Vetter <daniel@ffwll.ch>,
        Rob Clark <robdclark@gmail.com>, Sean Paul <sean@poorly.run>,
        Marijn Suijten <marijn.suijten@somainline.org>
Cc:     quic_abhinavk@quicinc.com, ppaalanen@gmail.com,
        contact@emersion.fr, laurent.pinchart@ideasonboard.com,
        sebastian.wick@redhat.com, ville.syrjala@linux.intel.com,
        dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
        linux-arm-msm@vger.kernel.org, freedreno@lists.freedesktop.org,
        wayland-devel@lists.freedesktop.org
References: <20230728-solid-fill-v5-0-053dbefa909c@quicinc.com>
 <20230728-solid-fill-v5-9-053dbefa909c@quicinc.com>
 <de406c53-023d-f477-17f2-7d6dabf838bd@linaro.org>
 <b2c55511-f44d-1c5a-a59b-108d57a07864@quicinc.com>
From:   Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
In-Reply-To: <b2c55511-f44d-1c5a-a59b-108d57a07864@quicinc.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

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@linaro.org>
>>> Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.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.

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