| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <2dace097-affe-4bc6-ac7c-b5606bd86a89@quicinc.com>
Date: Wed, 18 Oct 2023 17:59:22 -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 v6 08/10] drm/msm/dpu: Allow NULL FBs in atomic commit
On 9/24/2023 3:29 AM, Dmitry Baryshkov wrote:
> On 29/08/2023 03:05, Jessica Zhang wrote:
>> Since solid fill planes allow for a NULL framebuffer in a valid commit,
>> add NULL framebuffer checks to atomic commit calls within DPU.
>>
>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
>> Signed-off-by: Jessica Zhang <quic_jesszhan@...cinc.com>
>> ---
>> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 9 ++++++-
>> drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 41
>> ++++++++++++++++++++-----------
>> 2 files changed, 34 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
>> index 8ce7586e2ddf..5e845510e8c1 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
>> @@ -451,6 +451,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;
>> @@ -470,7 +471,13 @@ static void _dpu_crtc_blend_setup_mixer(struct
>> drm_crtc *crtc,
>> pstate = to_dpu_plane_state(state);
>> fb = state->fb;
>> - format = to_dpu_format(msm_framebuffer_format(pstate->base.fb));
>> + if (drm_plane_solid_fill_enabled(state))
>> + fmt = dpu_get_msm_format(&_dpu_crtc_get_kms(crtc)->base,
>> + DRM_FORMAT_ABGR8888, 0);
>> + else
>> + fmt = msm_framebuffer_format(pstate->base.fb);
>> +
>> + 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 c2aaaded07ed..114c803ff99b 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>> @@ -837,8 +837,13 @@ static int dpu_plane_atomic_check(struct
>> drm_plane *plane,
>> pipe_cfg->dst_rect = new_plane_state->dst;
>> - fb_rect.x2 = new_plane_state->fb->width;
>> - fb_rect.y2 = new_plane_state->fb->height;
>> + if (drm_plane_solid_fill_enabled(new_plane_state))
>> + return 0;
>
> This would skip all the width checks, dpu_plane_atomic_check_pipe(),
> etc. Could you please confirm that all of those checks are irrelevant
> for solid fill?
Hi Dmitry,
Good point -- I think it would be good to keep the rect and pipe checks
for solid fill.
>
>> +
>> + if (new_plane_state->pixel_source == DRM_PLANE_PIXEL_SOURCE_FB &&
>> new_plane_state->fb) {
>> + fb_rect.x2 = new_plane_state->fb->width;
>> + fb_rect.y2 = new_plane_state->fb->height;
>> + }
>> /* Ensure fb size is supported */
>> if (drm_rect_width(&fb_rect) > MAX_IMG_WIDTH ||
>> @@ -1082,21 +1087,32 @@ 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;
>> - const struct dpu_format *fmt =
>> - to_dpu_format(msm_framebuffer_format(fb));
>> + const struct dpu_format *fmt;
>> struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg;
>> struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->r_pipe_cfg;
>> struct dpu_kms *kms = _dpu_plane_get_kms(&pdpu->base);
>> struct msm_gem_address_space *aspace = kms->base.aspace;
>> struct dpu_hw_fmt_layout layout;
>> bool layout_valid = false;
>> - int ret;
>> - ret = dpu_format_populate_layout(aspace, fb, &layout);
>> - if (ret)
>> - DPU_ERROR_PLANE(pdpu, "failed to get format layout, %d\n", ret);
>> - else
>> - layout_valid = true;
>> + if (state->pixel_source == DRM_PLANE_PIXEL_SOURCE_FB && fb) {
>> + int ret;
>> +
>> + fmt = to_dpu_format(msm_framebuffer_format(fb));
>> +
>> + ret = dpu_format_populate_layout(aspace, fb, &layout);
>> + if (ret)
>> + DPU_ERROR_PLANE(pdpu, "failed to get format layout,
>> %d\n", ret);
>> + else
>> + layout_valid = true;
>> +
>> + 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));
>> + } else {
>> + fmt = dpu_get_dpu_format(DRM_FORMAT_ABGR8888);
>
> #define DPU_SOLID_FILL_FORMAT ?
Acked.
>
> Also, I don't think that solid_fill planes consume bandwidth, so this
> likely needs to be fixed too.
You're right -- I think we can actually return early here if solid fill
is enabled.
Thanks,
Jessica Zhang
>
>
>> + }
>> pstate->pending = true;
>> @@ -1104,11 +1120,6 @@ static void dpu_plane_sspp_atomic_update(struct
>> drm_plane *plane)
>> pstate->needs_qos_remap |= (is_rt_pipe != pdpu->is_rt_pipe);
>> pdpu->is_rt_pipe = is_rt_pipe;
>> - 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));
>> -
>> dpu_plane_sspp_update_pipe(plane, pipe, pipe_cfg, fmt,
>> drm_mode_vrefresh(&crtc->mode),
>> layout_valid ? &layout : NULL);
>>
>
> --
> With best wishes
> Dmitry
>
Powered by blists - more mailing lists