| Message ID | 20230104234036.636-4-quic_jesszhan@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | Support for Solid Fill Planes | expand |
On 05/01/2023 01:40, Jessica Zhang wrote: > Initialize and use the color_fill properties for planes in DPU driver. In > addition, relax framebuffer requirements within atomic commit path and > add checks for NULL framebuffers. Finally, drop DPU_PLANE_COLOR_FILL_FLAG > as it's unused. > > Changes since V2: > - Fixed dropped 'const' warning > - Dropped use of solid_fill_format > - Switched to using drm_plane_solid_fill_enabled helper method > - Added helper to convert color fill to BGR888 (Rob) > - Added support for solid fill on planes of varying sizes > - Removed DPU_PLANE_COLOR_FILL_FLAG > > Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 9 +++- > drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 65 ++++++++++++++--------- > 2 files changed, 49 insertions(+), 25 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > index 13ce321283ff..0695b70ea1b7 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > @@ -409,6 +409,7 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc, > struct drm_plane_state *state; > struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc->state); > struct dpu_plane_state *pstate = NULL; > + const struct msm_format *fmt; > struct dpu_format *format; > struct dpu_hw_ctl *ctl = mixer->lm_ctl; > > @@ -441,7 +442,13 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc, > sspp_idx - SSPP_VIG0, > state->fb ? state->fb->base.id : -1); > > - format = to_dpu_format(msm_framebuffer_format(pstate->base.fb)); > + if (pstate->base.fb) > + fmt = msm_framebuffer_format(pstate->base.fb); > + else > + fmt = dpu_get_msm_format(&_dpu_crtc_get_kms(crtc)->base, > + DRM_FORMAT_ABGR8888, 0); > + > + format = to_dpu_format(fmt); > > if (pstate->stage == DPU_STAGE_BASE && format->alpha_enable) > bg_alpha_enable = true; > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c > index 86719020afe2..51a7507373f7 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c > @@ -44,7 +44,6 @@ > > #define DPU_NAME_SIZE 12 > > -#define DPU_PLANE_COLOR_FILL_FLAG BIT(31) > #define DPU_ZPOS_MAX 255 > > /* multirect rect index */ > @@ -105,7 +104,6 @@ struct dpu_plane { > enum dpu_sspp pipe; > > struct dpu_hw_pipe *pipe_hw; > - uint32_t color_fill; > bool is_error; > bool is_rt_pipe; > const struct dpu_mdss_cfg *catalog; > @@ -678,6 +676,17 @@ static void _dpu_plane_setup_scaler(struct dpu_plane *pdpu, > &scaler3_cfg); > } > > +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); > + > + return ret; > +} > + > /** > * _dpu_plane_color_fill - enables color fill on plane > * @pdpu: Pointer to DPU plane object > @@ -1001,12 +1010,17 @@ static int dpu_plane_atomic_check(struct drm_plane *plane, > > dst = drm_plane_state_dest(new_plane_state); > > - fb_rect.x2 = new_plane_state->fb->width; > - fb_rect.y2 = new_plane_state->fb->height; > + if (new_plane_state->fb) { > + fb_rect.x2 = new_plane_state->fb->width; > + fb_rect.y2 = new_plane_state->fb->height; > + } > > max_linewidth = pdpu->catalog->caps->max_linewidth; > > - fmt = to_dpu_format(msm_framebuffer_format(new_plane_state->fb)); > + if (new_plane_state->fb) > + fmt = to_dpu_format(msm_framebuffer_format(new_plane_state->fb)); > + else > + fmt = dpu_get_dpu_format(DRM_FORMAT_ABGR8888); I think this should be more explicit: if (solid_fill) fmt = dpu_get_dpu_format(...) else fmt = to_dpu_format(msm_framebuffer_format(...). And in the _dpu_crtc_blend_setup_mixer() too. Maybe the code can be extracted to a helper. > > min_src_size = DPU_FORMAT_IS_YUV(fmt) ? 2 : 1; > > @@ -1018,7 +1032,7 @@ static int dpu_plane_atomic_check(struct drm_plane *plane, > return -EINVAL; > > /* check src bounds */ > - } else if (!dpu_plane_validate_src(&src, &fb_rect, min_src_size)) { > + } else if (new_plane_state->fb && !dpu_plane_validate_src(&src, &fb_rect, min_src_size)) { > DPU_DEBUG_PLANE(pdpu, "invalid source " DRM_RECT_FMT "\n", > DRM_RECT_ARG(&src)); > return -E2BIG; > @@ -1086,9 +1100,10 @@ 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) > + else if (!(plane->state->fb) && drm_plane_solid_fill_enabled(plane->state)) > /* force 100% alpha */ > - _dpu_plane_color_fill(pdpu, pdpu->color_fill, 0xFF); > + _dpu_plane_color_fill(pdpu, _dpu_plane_get_fill_color(plane->state->solid_fill), > + 0xFF); I'd push alpha into _dpu_plane_get_fill_color(). Then adding alpha support would be more transparent. > else if (pdpu->pipe_hw && pdpu->pipe_hw->ops.setup_csc) { > const struct dpu_format *fmt = to_dpu_format(msm_framebuffer_format(plane->state->fb)); > const struct dpu_csc_cfg *csc_ptr = _dpu_plane_get_csc(pdpu, fmt); > @@ -1127,23 +1142,30 @@ static void dpu_plane_sspp_atomic_update(struct drm_plane *plane) > struct drm_crtc *crtc = state->crtc; > struct drm_framebuffer *fb = state->fb; > bool is_rt_pipe, update_qos_remap; > - const struct dpu_format *fmt = > - to_dpu_format(msm_framebuffer_format(fb)); > + const struct dpu_format *fmt; > struct dpu_hw_pipe_cfg pipe_cfg; > > - memset(&pipe_cfg, 0, sizeof(struct dpu_hw_pipe_cfg)); > - > - _dpu_plane_set_scanout(plane, pstate, &pipe_cfg, fb); > - > pstate->pending = true; > > is_rt_pipe = (dpu_crtc_get_client_type(crtc) != NRT_CLIENT); > _dpu_plane_set_qos_ctrl(plane, false, DPU_PLANE_QOS_PANIC_CTRL); > > - DPU_DEBUG_PLANE(pdpu, "FB[%u] " DRM_RECT_FP_FMT "->crtc%u " DRM_RECT_FMT > - ", %4.4s ubwc %d\n", fb->base.id, DRM_RECT_FP_ARG(&state->src), > - crtc->base.id, DRM_RECT_ARG(&state->dst), > - (char *)&fmt->base.pixel_format, DPU_FORMAT_IS_UBWC(fmt)); > + /* override for color fill */ > + if (!fb && drm_plane_solid_fill_enabled(plane->state)) { > + /* skip remaining processing on color fill */ > + return; > + } > + > + memset(&pipe_cfg, 0, sizeof(struct dpu_hw_pipe_cfg)); > + > + fmt = to_dpu_format(msm_framebuffer_format(fb)); > + _dpu_plane_set_scanout(plane, pstate, &pipe_cfg, fb); This change deserves a comment somewhere (in the commit message?). Beforehand the driver tried to set the scanout/layout for the COLOR_FILL case. You have changed this. > + > + if (fb) > + DPU_DEBUG_PLANE(pdpu, "FB[%u] " DRM_RECT_FP_FMT "->crtc%u " DRM_RECT_FMT > + ", %4.4s ubwc %d\n", fb->base.id, DRM_RECT_FP_ARG(&state->src), > + crtc->base.id, DRM_RECT_ARG(&state->dst), > + (char *)&fmt->base.pixel_format, DPU_FORMAT_IS_UBWC(fmt)); > > pipe_cfg.src_rect = state->src; > > @@ -1155,12 +1177,6 @@ static void dpu_plane_sspp_atomic_update(struct drm_plane *plane) > > pipe_cfg.dst_rect = state->dst; > > - /* override for color fill */ > - if (pdpu->color_fill & DPU_PLANE_COLOR_FILL_FLAG) { > - /* skip remaining processing on color fill */ > - return; > - } > - > if (pdpu->pipe_hw->ops.setup_rects) { > pdpu->pipe_hw->ops.setup_rects(pdpu->pipe_hw, > &pipe_cfg, > @@ -1511,6 +1527,7 @@ struct drm_plane *dpu_plane_init(struct drm_device *dev, > DPU_ERROR("failed to install zpos property, rc = %d\n", ret); > > drm_plane_create_alpha_property(plane); > + drm_plane_create_solid_fill_property(plane); > drm_plane_create_blend_mode_property(plane, > BIT(DRM_MODE_BLEND_PIXEL_NONE) | > BIT(DRM_MODE_BLEND_PREMULTI) |
On 1/4/2023 6:16 PM, Dmitry Baryshkov wrote: > On 05/01/2023 01:40, Jessica Zhang wrote: >> Initialize and use the color_fill properties for planes in DPU driver. In >> addition, relax framebuffer requirements within atomic commit path and >> add checks for NULL framebuffers. Finally, drop DPU_PLANE_COLOR_FILL_FLAG >> as it's unused. >> >> Changes since V2: >> - Fixed dropped 'const' warning >> - Dropped use of solid_fill_format >> - Switched to using drm_plane_solid_fill_enabled helper method >> - Added helper to convert color fill to BGR888 (Rob) >> - Added support for solid fill on planes of varying sizes >> - Removed DPU_PLANE_COLOR_FILL_FLAG >> >> Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> >> --- >> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 9 +++- >> drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 65 ++++++++++++++--------- >> 2 files changed, 49 insertions(+), 25 deletions(-) >> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c >> index 13ce321283ff..0695b70ea1b7 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c >> @@ -409,6 +409,7 @@ static void _dpu_crtc_blend_setup_mixer(struct >> drm_crtc *crtc, >> struct drm_plane_state *state; >> struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc->state); >> struct dpu_plane_state *pstate = NULL; >> + const struct msm_format *fmt; >> struct dpu_format *format; >> struct dpu_hw_ctl *ctl = mixer->lm_ctl; >> @@ -441,7 +442,13 @@ static void _dpu_crtc_blend_setup_mixer(struct >> drm_crtc *crtc, >> sspp_idx - SSPP_VIG0, >> state->fb ? state->fb->base.id : -1); >> - format = to_dpu_format(msm_framebuffer_format(pstate->base.fb)); >> + if (pstate->base.fb) >> + fmt = msm_framebuffer_format(pstate->base.fb); >> + else >> + fmt = dpu_get_msm_format(&_dpu_crtc_get_kms(crtc)->base, >> + DRM_FORMAT_ABGR8888, 0); >> + >> + format = to_dpu_format(fmt); >> if (pstate->stage == DPU_STAGE_BASE && format->alpha_enable) >> bg_alpha_enable = true; >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c >> index 86719020afe2..51a7507373f7 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c >> @@ -44,7 +44,6 @@ >> #define DPU_NAME_SIZE 12 >> -#define DPU_PLANE_COLOR_FILL_FLAG BIT(31) >> #define DPU_ZPOS_MAX 255 >> /* multirect rect index */ >> @@ -105,7 +104,6 @@ struct dpu_plane { >> enum dpu_sspp pipe; >> struct dpu_hw_pipe *pipe_hw; >> - uint32_t color_fill; >> bool is_error; >> bool is_rt_pipe; >> const struct dpu_mdss_cfg *catalog; >> @@ -678,6 +676,17 @@ static void _dpu_plane_setup_scaler(struct >> dpu_plane *pdpu, >> &scaler3_cfg); >> } >> +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); >> + >> + return ret; >> +} >> + >> /** >> * _dpu_plane_color_fill - enables color fill on plane >> * @pdpu: Pointer to DPU plane object >> @@ -1001,12 +1010,17 @@ static int dpu_plane_atomic_check(struct >> drm_plane *plane, >> dst = drm_plane_state_dest(new_plane_state); >> - fb_rect.x2 = new_plane_state->fb->width; >> - fb_rect.y2 = new_plane_state->fb->height; >> + if (new_plane_state->fb) { >> + fb_rect.x2 = new_plane_state->fb->width; >> + fb_rect.y2 = new_plane_state->fb->height; >> + } >> max_linewidth = pdpu->catalog->caps->max_linewidth; >> - fmt = to_dpu_format(msm_framebuffer_format(new_plane_state->fb)); >> + if (new_plane_state->fb) >> + fmt = >> to_dpu_format(msm_framebuffer_format(new_plane_state->fb)); >> + else >> + fmt = dpu_get_dpu_format(DRM_FORMAT_ABGR8888); > > I think this should be more explicit: > > if (solid_fill) > fmt = dpu_get_dpu_format(...) > else > fmt = to_dpu_format(msm_framebuffer_format(...). > > And in the _dpu_crtc_blend_setup_mixer() too. Hi Dmitry, Noted. > > Maybe the code can be extracted to a helper. > >> min_src_size = DPU_FORMAT_IS_YUV(fmt) ? 2 : 1; >> @@ -1018,7 +1032,7 @@ static int dpu_plane_atomic_check(struct >> drm_plane *plane, >> return -EINVAL; >> /* check src bounds */ >> - } else if (!dpu_plane_validate_src(&src, &fb_rect, min_src_size)) { >> + } else if (new_plane_state->fb && !dpu_plane_validate_src(&src, >> &fb_rect, min_src_size)) { >> DPU_DEBUG_PLANE(pdpu, "invalid source " DRM_RECT_FMT "\n", >> DRM_RECT_ARG(&src)); >> return -E2BIG; >> @@ -1086,9 +1100,10 @@ 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) >> + else if (!(plane->state->fb) && >> drm_plane_solid_fill_enabled(plane->state)) >> /* force 100% alpha */ >> - _dpu_plane_color_fill(pdpu, pdpu->color_fill, 0xFF); >> + _dpu_plane_color_fill(pdpu, >> _dpu_plane_get_fill_color(plane->state->solid_fill), >> + 0xFF); > > I'd push alpha into _dpu_plane_get_fill_color(). Then adding alpha > support would be more transparent. Acked. > >> else if (pdpu->pipe_hw && pdpu->pipe_hw->ops.setup_csc) { >> const struct dpu_format *fmt = >> to_dpu_format(msm_framebuffer_format(plane->state->fb)); >> const struct dpu_csc_cfg *csc_ptr = _dpu_plane_get_csc(pdpu, >> fmt); >> @@ -1127,23 +1142,30 @@ static void >> dpu_plane_sspp_atomic_update(struct drm_plane *plane) >> struct drm_crtc *crtc = state->crtc; >> struct drm_framebuffer *fb = state->fb; >> bool is_rt_pipe, update_qos_remap; >> - const struct dpu_format *fmt = >> - to_dpu_format(msm_framebuffer_format(fb)); >> + const struct dpu_format *fmt; >> struct dpu_hw_pipe_cfg pipe_cfg; >> - memset(&pipe_cfg, 0, sizeof(struct dpu_hw_pipe_cfg)); >> - >> - _dpu_plane_set_scanout(plane, pstate, &pipe_cfg, fb); >> - >> pstate->pending = true; >> is_rt_pipe = (dpu_crtc_get_client_type(crtc) != NRT_CLIENT); >> _dpu_plane_set_qos_ctrl(plane, false, DPU_PLANE_QOS_PANIC_CTRL); >> - DPU_DEBUG_PLANE(pdpu, "FB[%u] " DRM_RECT_FP_FMT "->crtc%u " >> DRM_RECT_FMT >> - ", %4.4s ubwc %d\n", fb->base.id, >> DRM_RECT_FP_ARG(&state->src), >> - crtc->base.id, DRM_RECT_ARG(&state->dst), >> - (char *)&fmt->base.pixel_format, DPU_FORMAT_IS_UBWC(fmt)); >> + /* override for color fill */ >> + if (!fb && drm_plane_solid_fill_enabled(plane->state)) { >> + /* skip remaining processing on color fill */ >> + return; >> + } >> + >> + memset(&pipe_cfg, 0, sizeof(struct dpu_hw_pipe_cfg)); >> + >> + fmt = to_dpu_format(msm_framebuffer_format(fb)); >> + _dpu_plane_set_scanout(plane, pstate, &pipe_cfg, fb); > > This change deserves a comment somewhere (in the commit message?). > Beforehand the driver tried to set the scanout/layout for the COLOR_FILL > case. You have changed this. Sounds good. Thanks, Jessica Zhang > >> + >> + if (fb) >> + DPU_DEBUG_PLANE(pdpu, "FB[%u] " DRM_RECT_FP_FMT "->crtc%u " >> DRM_RECT_FMT >> + ", %4.4s ubwc %d\n", fb->base.id, >> DRM_RECT_FP_ARG(&state->src), >> + crtc->base.id, DRM_RECT_ARG(&state->dst), >> + (char *)&fmt->base.pixel_format, >> DPU_FORMAT_IS_UBWC(fmt)); >> pipe_cfg.src_rect = state->src; >> @@ -1155,12 +1177,6 @@ static void dpu_plane_sspp_atomic_update(struct >> drm_plane *plane) >> pipe_cfg.dst_rect = state->dst; >> - /* override for color fill */ >> - if (pdpu->color_fill & DPU_PLANE_COLOR_FILL_FLAG) { >> - /* skip remaining processing on color fill */ >> - return; >> - } >> - >> if (pdpu->pipe_hw->ops.setup_rects) { >> pdpu->pipe_hw->ops.setup_rects(pdpu->pipe_hw, >> &pipe_cfg, >> @@ -1511,6 +1527,7 @@ struct drm_plane *dpu_plane_init(struct >> drm_device *dev, >> DPU_ERROR("failed to install zpos property, rc = %d\n", ret); >> drm_plane_create_alpha_property(plane); >> + drm_plane_create_solid_fill_property(plane); >> drm_plane_create_blend_mode_property(plane, >> BIT(DRM_MODE_BLEND_PIXEL_NONE) | >> BIT(DRM_MODE_BLEND_PREMULTI) | > > -- > With best wishes > Dmitry >
On Fri, 6 Jan 2023 at 22:57, Jessica Zhang <quic_jesszhan@quicinc.com> wrote: > On 1/4/2023 6:16 PM, Dmitry Baryshkov wrote: > > On 05/01/2023 01:40, Jessica Zhang wrote: > >> Initialize and use the color_fill properties for planes in DPU driver. In > >> addition, relax framebuffer requirements within atomic commit path and > >> add checks for NULL framebuffers. Finally, drop DPU_PLANE_COLOR_FILL_FLAG > >> as it's unused. > >> > >> Changes since V2: > >> - Fixed dropped 'const' warning > >> - Dropped use of solid_fill_format > >> - Switched to using drm_plane_solid_fill_enabled helper method > >> - Added helper to convert color fill to BGR888 (Rob) > >> - Added support for solid fill on planes of varying sizes > >> - Removed DPU_PLANE_COLOR_FILL_FLAG > >> > >> Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> > >> --- > >> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 9 +++- > >> drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 65 ++++++++++++++--------- > >> 2 files changed, 49 insertions(+), 25 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > >> index 13ce321283ff..0695b70ea1b7 100644 > >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > >> @@ -409,6 +409,7 @@ static void _dpu_crtc_blend_setup_mixer(struct > >> drm_crtc *crtc, > >> struct drm_plane_state *state; > >> struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc->state); > >> struct dpu_plane_state *pstate = NULL; > >> + const struct msm_format *fmt; > >> struct dpu_format *format; > >> struct dpu_hw_ctl *ctl = mixer->lm_ctl; > >> @@ -441,7 +442,13 @@ static void _dpu_crtc_blend_setup_mixer(struct > >> drm_crtc *crtc, > >> sspp_idx - SSPP_VIG0, > >> state->fb ? state->fb->base.id : -1); > >> - format = to_dpu_format(msm_framebuffer_format(pstate->base.fb)); > >> + if (pstate->base.fb) > >> + fmt = msm_framebuffer_format(pstate->base.fb); > >> + else > >> + fmt = dpu_get_msm_format(&_dpu_crtc_get_kms(crtc)->base, > >> + DRM_FORMAT_ABGR8888, 0); > >> + > >> + format = to_dpu_format(fmt); > >> if (pstate->stage == DPU_STAGE_BASE && format->alpha_enable) > >> bg_alpha_enable = true; > >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c > >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c > >> index 86719020afe2..51a7507373f7 100644 > >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c > >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c > >> @@ -44,7 +44,6 @@ > >> #define DPU_NAME_SIZE 12 > >> -#define DPU_PLANE_COLOR_FILL_FLAG BIT(31) > >> #define DPU_ZPOS_MAX 255 > >> /* multirect rect index */ > >> @@ -105,7 +104,6 @@ struct dpu_plane { > >> enum dpu_sspp pipe; > >> struct dpu_hw_pipe *pipe_hw; > >> - uint32_t color_fill; > >> bool is_error; > >> bool is_rt_pipe; > >> const struct dpu_mdss_cfg *catalog; > >> @@ -678,6 +676,17 @@ static void _dpu_plane_setup_scaler(struct > >> dpu_plane *pdpu, > >> &scaler3_cfg); > >> } > >> +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); > >> + > >> + return ret; > >> +} > >> + > >> /** > >> * _dpu_plane_color_fill - enables color fill on plane > >> * @pdpu: Pointer to DPU plane object > >> @@ -1001,12 +1010,17 @@ static int dpu_plane_atomic_check(struct > >> drm_plane *plane, > >> dst = drm_plane_state_dest(new_plane_state); > >> - fb_rect.x2 = new_plane_state->fb->width; > >> - fb_rect.y2 = new_plane_state->fb->height; > >> + if (new_plane_state->fb) { > >> + fb_rect.x2 = new_plane_state->fb->width; > >> + fb_rect.y2 = new_plane_state->fb->height; > >> + } > >> max_linewidth = pdpu->catalog->caps->max_linewidth; > >> - fmt = to_dpu_format(msm_framebuffer_format(new_plane_state->fb)); > >> + if (new_plane_state->fb) > >> + fmt = > >> to_dpu_format(msm_framebuffer_format(new_plane_state->fb)); > >> + else > >> + fmt = dpu_get_dpu_format(DRM_FORMAT_ABGR8888); > > > > I think this should be more explicit: > > > > if (solid_fill) > > fmt = dpu_get_dpu_format(...) > > else > > fmt = to_dpu_format(msm_framebuffer_format(...). > > > > And in the _dpu_crtc_blend_setup_mixer() too. > > Hi Dmitry, > > Noted. > > > > > Maybe the code can be extracted to a helper. > > > >> min_src_size = DPU_FORMAT_IS_YUV(fmt) ? 2 : 1; > >> @@ -1018,7 +1032,7 @@ static int dpu_plane_atomic_check(struct > >> drm_plane *plane, > >> return -EINVAL; > >> /* check src bounds */ > >> - } else if (!dpu_plane_validate_src(&src, &fb_rect, min_src_size)) { > >> + } else if (new_plane_state->fb && !dpu_plane_validate_src(&src, > >> &fb_rect, min_src_size)) { > >> DPU_DEBUG_PLANE(pdpu, "invalid source " DRM_RECT_FMT "\n", > >> DRM_RECT_ARG(&src)); > >> return -E2BIG; > >> @@ -1086,9 +1100,10 @@ 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) > >> + else if (!(plane->state->fb) && > >> drm_plane_solid_fill_enabled(plane->state)) > >> /* force 100% alpha */ > >> - _dpu_plane_color_fill(pdpu, pdpu->color_fill, 0xFF); > >> + _dpu_plane_color_fill(pdpu, > >> _dpu_plane_get_fill_color(plane->state->solid_fill), > >> + 0xFF); > > > > I'd push alpha into _dpu_plane_get_fill_color(). Then adding alpha > > support would be more transparent. > > Acked. Actually after our discussion I wanted to discuss this with you. We pass the plane's alpha value and blending mode using LM_BLEND* registers. Does that integrate correctly with the alpha part of SSPP_SRC_CONSTANT_COLOR?
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c index 13ce321283ff..0695b70ea1b7 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c @@ -409,6 +409,7 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc, struct drm_plane_state *state; struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc->state); struct dpu_plane_state *pstate = NULL; + const struct msm_format *fmt; struct dpu_format *format; struct dpu_hw_ctl *ctl = mixer->lm_ctl; @@ -441,7 +442,13 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc, sspp_idx - SSPP_VIG0, state->fb ? state->fb->base.id : -1); - format = to_dpu_format(msm_framebuffer_format(pstate->base.fb)); + if (pstate->base.fb) + fmt = msm_framebuffer_format(pstate->base.fb); + else + fmt = dpu_get_msm_format(&_dpu_crtc_get_kms(crtc)->base, + DRM_FORMAT_ABGR8888, 0); + + format = to_dpu_format(fmt); if (pstate->stage == DPU_STAGE_BASE && format->alpha_enable) bg_alpha_enable = true; diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c index 86719020afe2..51a7507373f7 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c @@ -44,7 +44,6 @@ #define DPU_NAME_SIZE 12 -#define DPU_PLANE_COLOR_FILL_FLAG BIT(31) #define DPU_ZPOS_MAX 255 /* multirect rect index */ @@ -105,7 +104,6 @@ struct dpu_plane { enum dpu_sspp pipe; struct dpu_hw_pipe *pipe_hw; - uint32_t color_fill; bool is_error; bool is_rt_pipe; const struct dpu_mdss_cfg *catalog; @@ -678,6 +676,17 @@ static void _dpu_plane_setup_scaler(struct dpu_plane *pdpu, &scaler3_cfg); } +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); + + return ret; +} + /** * _dpu_plane_color_fill - enables color fill on plane * @pdpu: Pointer to DPU plane object @@ -1001,12 +1010,17 @@ static int dpu_plane_atomic_check(struct drm_plane *plane, dst = drm_plane_state_dest(new_plane_state); - fb_rect.x2 = new_plane_state->fb->width; - fb_rect.y2 = new_plane_state->fb->height; + if (new_plane_state->fb) { + fb_rect.x2 = new_plane_state->fb->width; + fb_rect.y2 = new_plane_state->fb->height; + } max_linewidth = pdpu->catalog->caps->max_linewidth; - fmt = to_dpu_format(msm_framebuffer_format(new_plane_state->fb)); + if (new_plane_state->fb) + fmt = to_dpu_format(msm_framebuffer_format(new_plane_state->fb)); + else + fmt = dpu_get_dpu_format(DRM_FORMAT_ABGR8888); min_src_size = DPU_FORMAT_IS_YUV(fmt) ? 2 : 1; @@ -1018,7 +1032,7 @@ static int dpu_plane_atomic_check(struct drm_plane *plane, return -EINVAL; /* check src bounds */ - } else if (!dpu_plane_validate_src(&src, &fb_rect, min_src_size)) { + } else if (new_plane_state->fb && !dpu_plane_validate_src(&src, &fb_rect, min_src_size)) { DPU_DEBUG_PLANE(pdpu, "invalid source " DRM_RECT_FMT "\n", DRM_RECT_ARG(&src)); return -E2BIG; @@ -1086,9 +1100,10 @@ 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) + else if (!(plane->state->fb) && drm_plane_solid_fill_enabled(plane->state)) /* force 100% alpha */ - _dpu_plane_color_fill(pdpu, pdpu->color_fill, 0xFF); + _dpu_plane_color_fill(pdpu, _dpu_plane_get_fill_color(plane->state->solid_fill), + 0xFF); else if (pdpu->pipe_hw && pdpu->pipe_hw->ops.setup_csc) { const struct dpu_format *fmt = to_dpu_format(msm_framebuffer_format(plane->state->fb)); const struct dpu_csc_cfg *csc_ptr = _dpu_plane_get_csc(pdpu, fmt); @@ -1127,23 +1142,30 @@ static void dpu_plane_sspp_atomic_update(struct drm_plane *plane) struct drm_crtc *crtc = state->crtc; struct drm_framebuffer *fb = state->fb; bool is_rt_pipe, update_qos_remap; - const struct dpu_format *fmt = - to_dpu_format(msm_framebuffer_format(fb)); + const struct dpu_format *fmt; struct dpu_hw_pipe_cfg pipe_cfg; - memset(&pipe_cfg, 0, sizeof(struct dpu_hw_pipe_cfg)); - - _dpu_plane_set_scanout(plane, pstate, &pipe_cfg, fb); - pstate->pending = true; is_rt_pipe = (dpu_crtc_get_client_type(crtc) != NRT_CLIENT); _dpu_plane_set_qos_ctrl(plane, false, DPU_PLANE_QOS_PANIC_CTRL); - DPU_DEBUG_PLANE(pdpu, "FB[%u] " DRM_RECT_FP_FMT "->crtc%u " DRM_RECT_FMT - ", %4.4s ubwc %d\n", fb->base.id, DRM_RECT_FP_ARG(&state->src), - crtc->base.id, DRM_RECT_ARG(&state->dst), - (char *)&fmt->base.pixel_format, DPU_FORMAT_IS_UBWC(fmt)); + /* override for color fill */ + if (!fb && drm_plane_solid_fill_enabled(plane->state)) { + /* skip remaining processing on color fill */ + return; + } + + memset(&pipe_cfg, 0, sizeof(struct dpu_hw_pipe_cfg)); + + fmt = to_dpu_format(msm_framebuffer_format(fb)); + _dpu_plane_set_scanout(plane, pstate, &pipe_cfg, fb); + + if (fb) + DPU_DEBUG_PLANE(pdpu, "FB[%u] " DRM_RECT_FP_FMT "->crtc%u " DRM_RECT_FMT + ", %4.4s ubwc %d\n", fb->base.id, DRM_RECT_FP_ARG(&state->src), + crtc->base.id, DRM_RECT_ARG(&state->dst), + (char *)&fmt->base.pixel_format, DPU_FORMAT_IS_UBWC(fmt)); pipe_cfg.src_rect = state->src; @@ -1155,12 +1177,6 @@ static void dpu_plane_sspp_atomic_update(struct drm_plane *plane) pipe_cfg.dst_rect = state->dst; - /* override for color fill */ - if (pdpu->color_fill & DPU_PLANE_COLOR_FILL_FLAG) { - /* skip remaining processing on color fill */ - return; - } - if (pdpu->pipe_hw->ops.setup_rects) { pdpu->pipe_hw->ops.setup_rects(pdpu->pipe_hw, &pipe_cfg, @@ -1511,6 +1527,7 @@ struct drm_plane *dpu_plane_init(struct drm_device *dev, DPU_ERROR("failed to install zpos property, rc = %d\n", ret); drm_plane_create_alpha_property(plane); + drm_plane_create_solid_fill_property(plane); drm_plane_create_blend_mode_property(plane, BIT(DRM_MODE_BLEND_PIXEL_NONE) | BIT(DRM_MODE_BLEND_PREMULTI) |
Initialize and use the color_fill properties for planes in DPU driver. In addition, relax framebuffer requirements within atomic commit path and add checks for NULL framebuffers. Finally, drop DPU_PLANE_COLOR_FILL_FLAG as it's unused. Changes since V2: - Fixed dropped 'const' warning - Dropped use of solid_fill_format - Switched to using drm_plane_solid_fill_enabled helper method - Added helper to convert color fill to BGR888 (Rob) - Added support for solid fill on planes of varying sizes - Removed DPU_PLANE_COLOR_FILL_FLAG Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> --- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 9 +++- drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 65 ++++++++++++++--------- 2 files changed, 49 insertions(+), 25 deletions(-)