Messages in this thread | | | Date | Mon, 7 Aug 2023 09:59:46 -0700 | Subject | Re: [PATCH RFC v5 09/10] drm/msm/dpu: Use DRM solid_fill property | From | Jessica Zhang <> |
| |
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@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.
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 >
| |