Messages in this thread | | | Date | Tue, 11 Jul 2023 17:01:42 -0700 | Subject | Re: [PATCH RFC v4 7/7] drm/msm/dpu: Use DRM solid_fill property | From | Jessica Zhang <> |
| |
On 7/3/2023 12:42 AM, Pekka Paalanen wrote: > On Fri, 30 Jun 2023 11:26:49 +0300 > Pekka Paalanen <ppaalanen@gmail.com> wrote: > >> 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.
Hi Pekka,
The solid fill blob will accept an RGB323232 value -- will document this in the drm_solid_fill_info struct
>> >> 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.
Acked.
>> >> >> Thanks, >> pq >> >>> + >>> + return ret; > > Btw. if your driver format is ABGR, then this function leaves alpha as > zero. That's confusing. > > It would be nice to mention the exact pixel format in the function name > so the consistency is easier to check in both here and in callers.
Acked.
Thanks,
Jessica Zhang
> > > Thanks, > pq > >>> +} >>> + >>> /** >>> * _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 */ >>> >> >
| |