| Message ID | 20241025-dpu-virtual-wide-v6-2-0310fd519765@linaro.org |
|---|---|
| State | Accepted |
| Commit | 31f7148fd3704e0981b4eb6c6d13cf584da606c4 |
| Headers | show |
| Series | drm/msm/dpu: support virtual wide planes | expand |
On Fri, Oct 25, 2024 at 12:00:20PM -0700, Abhinav Kumar wrote: > > > On 10/24/2024 5:20 PM, Dmitry Baryshkov wrote: > > In preparation for virtualized planes support, move pstate->pipe > > initialization from dpu_plane_reset() to dpu_plane_atomic_check(). In > > case of virtual planes the plane's pipe will not be known up to the > > point of atomic_check() callback. > > > > I had R-bed this in v5. Did anything change in v6? No, nothing. I'm sorry for forgetting to run `b4 trailers -u`. > But one comment below. > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > --- > > drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 25 +++++++++++-------------- > > 1 file changed, 11 insertions(+), 14 deletions(-) > > > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c > > index 37faf5b238b0..725c9a5826fd 100644 > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c > > @@ -797,13 +797,22 @@ static int dpu_plane_atomic_check(struct drm_plane *plane, > > uint32_t max_linewidth; > > unsigned int rotation; > > uint32_t supported_rotations; > > - const struct dpu_sspp_cfg *pipe_hw_caps = pstate->pipe.sspp->cap; > > - const struct dpu_sspp_sub_blks *sblk = pstate->pipe.sspp->cap->sblk; > > + const struct dpu_sspp_cfg *pipe_hw_caps; > > + const struct dpu_sspp_sub_blks *sblk; > > if (new_plane_state->crtc) > > crtc_state = drm_atomic_get_new_crtc_state(state, > > new_plane_state->crtc); > > + pipe->sspp = dpu_rm_get_sspp(&kms->rm, pdpu->pipe); > > + r_pipe->sspp = NULL; > > + > > + if (!pipe->sspp) > > + return -EINVAL; > > + > > + pipe_hw_caps = pipe->sspp->cap; > > + sblk = pipe->sspp->cap->sblk; > > + > > min_scale = FRAC_16_16(1, sblk->maxupscale); > > ret = drm_atomic_helper_check_plane_state(new_plane_state, crtc_state, > > min_scale, > > Do you think it will be better to move the get_sspp() call after the > drm_atomic_helper_check_plane_state()? I'd say, it makes no difference. I'll check your suggestion if I have to send another iteration. > > > @@ -820,7 +829,6 @@ static int dpu_plane_atomic_check(struct drm_plane *plane, > > pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE; > > r_pipe->multirect_index = DPU_SSPP_RECT_SOLO; > > r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE; > > - r_pipe->sspp = NULL; > > pstate->stage = DPU_STAGE_0 + pstate->base.normalized_zpos; > > if (pstate->stage >= pdpu->catalog->caps->max_mixer_blendstages) { > > @@ -1286,7 +1294,6 @@ static void dpu_plane_reset(struct drm_plane *plane) > > { > > struct dpu_plane *pdpu; > > struct dpu_plane_state *pstate; > > - struct dpu_kms *dpu_kms = _dpu_plane_get_kms(plane); > > if (!plane) { > > DPU_ERROR("invalid plane\n"); > > @@ -1308,16 +1315,6 @@ static void dpu_plane_reset(struct drm_plane *plane) > > return; > > } > > - /* > > - * Set the SSPP here until we have proper virtualized DPU planes. > > - * This is the place where the state is allocated, so fill it fully. > > - */ > > - pstate->pipe.sspp = dpu_rm_get_sspp(&dpu_kms->rm, pdpu->pipe); > > - pstate->pipe.multirect_index = DPU_SSPP_RECT_SOLO; > > - pstate->pipe.multirect_mode = DPU_SSPP_MULTIRECT_NONE; > > - > > - pstate->r_pipe.sspp = NULL; > > - > > __drm_atomic_helper_plane_reset(plane, &pstate->base); > > } > >
On 10/28/2024 3:46 AM, Dmitry Baryshkov wrote: > On Fri, Oct 25, 2024 at 12:00:20PM -0700, Abhinav Kumar wrote: >> >> >> On 10/24/2024 5:20 PM, Dmitry Baryshkov wrote: >>> In preparation for virtualized planes support, move pstate->pipe >>> initialization from dpu_plane_reset() to dpu_plane_atomic_check(). In >>> case of virtual planes the plane's pipe will not be known up to the >>> point of atomic_check() callback. >>> >> >> I had R-bed this in v5. Did anything change in v6? > > No, nothing. I'm sorry for forgetting to run `b4 trailers -u`. > >> But one comment below. >> >>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >>> --- >>> drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 25 +++++++++++-------------- >>> 1 file changed, 11 insertions(+), 14 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c >>> index 37faf5b238b0..725c9a5826fd 100644 >>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c >>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c >>> @@ -797,13 +797,22 @@ static int dpu_plane_atomic_check(struct drm_plane *plane, >>> uint32_t max_linewidth; >>> unsigned int rotation; >>> uint32_t supported_rotations; >>> - const struct dpu_sspp_cfg *pipe_hw_caps = pstate->pipe.sspp->cap; >>> - const struct dpu_sspp_sub_blks *sblk = pstate->pipe.sspp->cap->sblk; >>> + const struct dpu_sspp_cfg *pipe_hw_caps; >>> + const struct dpu_sspp_sub_blks *sblk; >>> if (new_plane_state->crtc) >>> crtc_state = drm_atomic_get_new_crtc_state(state, >>> new_plane_state->crtc); >>> + pipe->sspp = dpu_rm_get_sspp(&kms->rm, pdpu->pipe); >>> + r_pipe->sspp = NULL; >>> + >>> + if (!pipe->sspp) >>> + return -EINVAL; >>> + >>> + pipe_hw_caps = pipe->sspp->cap; >>> + sblk = pipe->sspp->cap->sblk; >>> + >>> min_scale = FRAC_16_16(1, sblk->maxupscale); >>> ret = drm_atomic_helper_check_plane_state(new_plane_state, crtc_state, >>> min_scale, >> >> Do you think it will be better to move the get_sspp() call after the >> drm_atomic_helper_check_plane_state()? > > I'd say, it makes no difference. I'll check your suggestion if I have to > send another iteration. > Ok, its a minor comment, I am fine with this change otherwise, Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com> If you do need to push another version, you can explore that. >> >>> @@ -820,7 +829,6 @@ static int dpu_plane_atomic_check(struct drm_plane *plane, >>> pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE; >>> r_pipe->multirect_index = DPU_SSPP_RECT_SOLO; >>> r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE; >>> - r_pipe->sspp = NULL; >>> pstate->stage = DPU_STAGE_0 + pstate->base.normalized_zpos; >>> if (pstate->stage >= pdpu->catalog->caps->max_mixer_blendstages) { >>> @@ -1286,7 +1294,6 @@ static void dpu_plane_reset(struct drm_plane *plane) >>> { >>> struct dpu_plane *pdpu; >>> struct dpu_plane_state *pstate; >>> - struct dpu_kms *dpu_kms = _dpu_plane_get_kms(plane); >>> if (!plane) { >>> DPU_ERROR("invalid plane\n"); >>> @@ -1308,16 +1315,6 @@ static void dpu_plane_reset(struct drm_plane *plane) >>> return; >>> } >>> - /* >>> - * Set the SSPP here until we have proper virtualized DPU planes. >>> - * This is the place where the state is allocated, so fill it fully. >>> - */ >>> - pstate->pipe.sspp = dpu_rm_get_sspp(&dpu_kms->rm, pdpu->pipe); >>> - pstate->pipe.multirect_index = DPU_SSPP_RECT_SOLO; >>> - pstate->pipe.multirect_mode = DPU_SSPP_MULTIRECT_NONE; >>> - >>> - pstate->r_pipe.sspp = NULL; >>> - >>> __drm_atomic_helper_plane_reset(plane, &pstate->base); >>> } >>> >
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c index 37faf5b238b0..725c9a5826fd 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c @@ -797,13 +797,22 @@ static int dpu_plane_atomic_check(struct drm_plane *plane, uint32_t max_linewidth; unsigned int rotation; uint32_t supported_rotations; - const struct dpu_sspp_cfg *pipe_hw_caps = pstate->pipe.sspp->cap; - const struct dpu_sspp_sub_blks *sblk = pstate->pipe.sspp->cap->sblk; + const struct dpu_sspp_cfg *pipe_hw_caps; + const struct dpu_sspp_sub_blks *sblk; if (new_plane_state->crtc) crtc_state = drm_atomic_get_new_crtc_state(state, new_plane_state->crtc); + pipe->sspp = dpu_rm_get_sspp(&kms->rm, pdpu->pipe); + r_pipe->sspp = NULL; + + if (!pipe->sspp) + return -EINVAL; + + pipe_hw_caps = pipe->sspp->cap; + sblk = pipe->sspp->cap->sblk; + min_scale = FRAC_16_16(1, sblk->maxupscale); ret = drm_atomic_helper_check_plane_state(new_plane_state, crtc_state, min_scale, @@ -820,7 +829,6 @@ static int dpu_plane_atomic_check(struct drm_plane *plane, pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE; r_pipe->multirect_index = DPU_SSPP_RECT_SOLO; r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE; - r_pipe->sspp = NULL; pstate->stage = DPU_STAGE_0 + pstate->base.normalized_zpos; if (pstate->stage >= pdpu->catalog->caps->max_mixer_blendstages) { @@ -1286,7 +1294,6 @@ static void dpu_plane_reset(struct drm_plane *plane) { struct dpu_plane *pdpu; struct dpu_plane_state *pstate; - struct dpu_kms *dpu_kms = _dpu_plane_get_kms(plane); if (!plane) { DPU_ERROR("invalid plane\n"); @@ -1308,16 +1315,6 @@ static void dpu_plane_reset(struct drm_plane *plane) return; } - /* - * Set the SSPP here until we have proper virtualized DPU planes. - * This is the place where the state is allocated, so fill it fully. - */ - pstate->pipe.sspp = dpu_rm_get_sspp(&dpu_kms->rm, pdpu->pipe); - pstate->pipe.multirect_index = DPU_SSPP_RECT_SOLO; - pstate->pipe.multirect_mode = DPU_SSPP_MULTIRECT_NONE; - - pstate->r_pipe.sspp = NULL; - __drm_atomic_helper_plane_reset(plane, &pstate->base); }
In preparation for virtualized planes support, move pstate->pipe initialization from dpu_plane_reset() to dpu_plane_atomic_check(). In case of virtual planes the plane's pipe will not be known up to the point of atomic_check() callback. Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-)