| Message ID | 20241130-dpu-virtual-wide-v7-2-991053fcf63c@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | drm/msm/dpu: support virtual wide planes | expand |
On 11/29/2024 5:55 PM, Dmitry Baryshkov wrote: > Virtual wide planes give high amount of flexibility, but it is not > always enough: > > In parallel multirect case only the half of the usual width is supported > for tiled formats. Thus the whole width of two tiled multirect > rectangles can not be greater than max_linewidth, which is not enough > for some platforms/compositors. > > Another example is as simple as wide YUV plane. YUV planes can not use > multirect, so currently they are limited to max_linewidth too. > > Now that the planes are fully virtualized, add support for allocating > two SSPP blocks to drive a single DRM plane. This fixes both mentioned > cases and allows all planes to go up to 2*max_linewidth (at the cost of > making some of the planes unavailable to the user). > Overall looks so much cleaner after unification! One small nit below, You can still have, Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com> Note: we have started testing this series with sc7180 CrOS, and will report our findings/ give tested-by this week. <snip> > +static bool dpu_plane_try_multirect_parallel(struct dpu_sw_pipe *pipe, struct dpu_sw_pipe_cfg *pipe_cfg, > + struct dpu_sw_pipe *r_pipe, struct dpu_sw_pipe_cfg *r_pipe_cfg, > + struct dpu_hw_sspp *sspp, const struct msm_format *fmt, > + uint32_t max_linewidth) > +{ > + r_pipe->sspp = NULL; > + > + pipe->multirect_index = DPU_SSPP_RECT_SOLO; > + pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE; > + > + r_pipe->multirect_index = DPU_SSPP_RECT_SOLO; > + r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE; > + There are two places where the multirect_index and multirect_mode are reset. Would it be better to just have a small api dpu_plane_reset_multirect() and do this there?
On Wed, Dec 11, 2024 at 01:51:51PM -0800, Abhinav Kumar wrote: > > > On 11/29/2024 5:55 PM, Dmitry Baryshkov wrote: > > Virtual wide planes give high amount of flexibility, but it is not > > always enough: > > > > In parallel multirect case only the half of the usual width is supported > > for tiled formats. Thus the whole width of two tiled multirect > > rectangles can not be greater than max_linewidth, which is not enough > > for some platforms/compositors. > > > > Another example is as simple as wide YUV plane. YUV planes can not use > > multirect, so currently they are limited to max_linewidth too. > > > > Now that the planes are fully virtualized, add support for allocating > > two SSPP blocks to drive a single DRM plane. This fixes both mentioned > > cases and allows all planes to go up to 2*max_linewidth (at the cost of > > making some of the planes unavailable to the user). > > > > Overall looks so much cleaner after unification! > > One small nit below, > > > You can still have, > > Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com> > > Note: we have started testing this series with sc7180 CrOS, and will report > our findings/ give tested-by this week. > > > <snip> > > > +static bool dpu_plane_try_multirect_parallel(struct dpu_sw_pipe *pipe, struct dpu_sw_pipe_cfg *pipe_cfg, > > + struct dpu_sw_pipe *r_pipe, struct dpu_sw_pipe_cfg *r_pipe_cfg, > > + struct dpu_hw_sspp *sspp, const struct msm_format *fmt, > > + uint32_t max_linewidth) > > +{ > > + r_pipe->sspp = NULL; > > + > > + pipe->multirect_index = DPU_SSPP_RECT_SOLO; > > + pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE; > > + > > + r_pipe->multirect_index = DPU_SSPP_RECT_SOLO; > > + r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE; > > + > > > There are two places where the multirect_index and multirect_mode are reset. > Would it be better to just have a small api dpu_plane_reset_multirect() and > do this there? I'm not sure, what's the benefit. We can add an API to reset one pipe (to also be able to use it in _dpu_plane_atomic_disable()), but then it's just deduplication for the sake of deduplication.
On 12/11/2024 2:24 PM, Dmitry Baryshkov wrote: > On Wed, Dec 11, 2024 at 01:51:51PM -0800, Abhinav Kumar wrote: >> >> >> On 11/29/2024 5:55 PM, Dmitry Baryshkov wrote: >>> Virtual wide planes give high amount of flexibility, but it is not >>> always enough: >>> >>> In parallel multirect case only the half of the usual width is supported >>> for tiled formats. Thus the whole width of two tiled multirect >>> rectangles can not be greater than max_linewidth, which is not enough >>> for some platforms/compositors. >>> >>> Another example is as simple as wide YUV plane. YUV planes can not use >>> multirect, so currently they are limited to max_linewidth too. >>> >>> Now that the planes are fully virtualized, add support for allocating >>> two SSPP blocks to drive a single DRM plane. This fixes both mentioned >>> cases and allows all planes to go up to 2*max_linewidth (at the cost of >>> making some of the planes unavailable to the user). >>> >> >> Overall looks so much cleaner after unification! >> >> One small nit below, >> >> >> You can still have, >> >> Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com> >> >> Note: we have started testing this series with sc7180 CrOS, and will report >> our findings/ give tested-by this week. >> >> >> <snip> >> >>> +static bool dpu_plane_try_multirect_parallel(struct dpu_sw_pipe *pipe, struct dpu_sw_pipe_cfg *pipe_cfg, >>> + struct dpu_sw_pipe *r_pipe, struct dpu_sw_pipe_cfg *r_pipe_cfg, >>> + struct dpu_hw_sspp *sspp, const struct msm_format *fmt, >>> + uint32_t max_linewidth) >>> +{ >>> + r_pipe->sspp = NULL; >>> + >>> + pipe->multirect_index = DPU_SSPP_RECT_SOLO; >>> + pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE; >>> + >>> + r_pipe->multirect_index = DPU_SSPP_RECT_SOLO; >>> + r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE; >>> + >> >> >> There are two places where the multirect_index and multirect_mode are reset. >> Would it be better to just have a small api dpu_plane_reset_multirect() and >> do this there? > > I'm not sure, what's the benefit. We can add an API to reset one pipe > (to also be able to use it in _dpu_plane_atomic_disable()), but then > it's just deduplication for the sake of deduplication. > Yeah I was thinking something like dpu_plane_reset_multirect(pipe); dpu_plane_reset_multirect(r_pipe); But its only a minor benefit, hence as I wrote it as a nit. We can keep things as it is, if its unnecessary in your opinion. >
On Thu, 12 Dec 2024 at 00:38, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: > > > > On 12/11/2024 2:24 PM, Dmitry Baryshkov wrote: > > On Wed, Dec 11, 2024 at 01:51:51PM -0800, Abhinav Kumar wrote: > >> > >> > >> On 11/29/2024 5:55 PM, Dmitry Baryshkov wrote: > >>> Virtual wide planes give high amount of flexibility, but it is not > >>> always enough: > >>> > >>> In parallel multirect case only the half of the usual width is supported > >>> for tiled formats. Thus the whole width of two tiled multirect > >>> rectangles can not be greater than max_linewidth, which is not enough > >>> for some platforms/compositors. > >>> > >>> Another example is as simple as wide YUV plane. YUV planes can not use > >>> multirect, so currently they are limited to max_linewidth too. > >>> > >>> Now that the planes are fully virtualized, add support for allocating > >>> two SSPP blocks to drive a single DRM plane. This fixes both mentioned > >>> cases and allows all planes to go up to 2*max_linewidth (at the cost of > >>> making some of the planes unavailable to the user). > >>> > >> > >> Overall looks so much cleaner after unification! > >> > >> One small nit below, > >> > >> > >> You can still have, > >> > >> Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com> > >> > >> Note: we have started testing this series with sc7180 CrOS, and will report > >> our findings/ give tested-by this week. > >> > >> > >> <snip> > >> > >>> +static bool dpu_plane_try_multirect_parallel(struct dpu_sw_pipe *pipe, struct dpu_sw_pipe_cfg *pipe_cfg, > >>> + struct dpu_sw_pipe *r_pipe, struct dpu_sw_pipe_cfg *r_pipe_cfg, > >>> + struct dpu_hw_sspp *sspp, const struct msm_format *fmt, > >>> + uint32_t max_linewidth) > >>> +{ > >>> + r_pipe->sspp = NULL; > >>> + > >>> + pipe->multirect_index = DPU_SSPP_RECT_SOLO; > >>> + pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE; > >>> + > >>> + r_pipe->multirect_index = DPU_SSPP_RECT_SOLO; > >>> + r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE; > >>> + > >> > >> > >> There are two places where the multirect_index and multirect_mode are reset. > >> Would it be better to just have a small api dpu_plane_reset_multirect() and > >> do this there? > > > > I'm not sure, what's the benefit. We can add an API to reset one pipe > > (to also be able to use it in _dpu_plane_atomic_disable()), but then > > it's just deduplication for the sake of deduplication. > > > > Yeah I was thinking something like > > dpu_plane_reset_multirect(pipe); > dpu_plane_reset_multirect(r_pipe); > > But its only a minor benefit, hence as I wrote it as a nit. We can keep > things as it is, if its unnecessary in your opinion. Well, granted that I hope to be able to drop non-virtual planes after a few releases, I don't think it makes real sense.
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c index 309a8cdaafcf3dacd1c7ba4e0cddb7f3f56423c0..098abc2c0003cde90ce6219c97ee18fa055a92a5 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c @@ -20,7 +20,6 @@ #include "msm_drv.h" #include "msm_mdss.h" #include "dpu_kms.h" -#include "dpu_formats.h" #include "dpu_hw_sspp.h" #include "dpu_hw_util.h" #include "dpu_trace.h" @@ -888,6 +887,32 @@ static int dpu_plane_atomic_check_nosspp(struct drm_plane *plane, return 0; } +static int dpu_plane_is_multirect_parallel_capable(struct dpu_hw_sspp *sspp, + struct dpu_sw_pipe_cfg *pipe_cfg, + const struct msm_format *fmt, + uint32_t max_linewidth) +{ + if (drm_rect_width(&pipe_cfg->src_rect) != drm_rect_width(&pipe_cfg->dst_rect) || + drm_rect_height(&pipe_cfg->src_rect) != drm_rect_height(&pipe_cfg->dst_rect)) + return false; + + if (pipe_cfg->rotation & DRM_MODE_ROTATE_90) + return false; + + if (MSM_FORMAT_IS_YUV(fmt)) + return false; + + if (MSM_FORMAT_IS_UBWC(fmt) && + drm_rect_width(&pipe_cfg->src_rect) > max_linewidth / 2) + return false; + + if (!test_bit(DPU_SSPP_SMART_DMA_V1, &sspp->cap->features) && + !test_bit(DPU_SSPP_SMART_DMA_V2, &sspp->cap->features)) + return false; + + return true; +} + static int dpu_plane_atomic_check_sspp(struct drm_plane *plane, struct drm_atomic_state *state, const struct drm_crtc_state *crtc_state) @@ -901,7 +926,6 @@ static int dpu_plane_atomic_check_sspp(struct drm_plane *plane, const struct msm_format *fmt; struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg; struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->r_pipe_cfg; - uint32_t max_linewidth; uint32_t supported_rotations; const struct dpu_sspp_cfg *pipe_hw_caps; const struct dpu_sspp_sub_blks *sblk; @@ -923,8 +947,6 @@ static int dpu_plane_atomic_check_sspp(struct drm_plane *plane, fmt = msm_framebuffer_format(new_plane_state->fb); - max_linewidth = pdpu->catalog->caps->max_linewidth; - supported_rotations = DRM_MODE_REFLECT_MASK | DRM_MODE_ROTATE_0; if (pipe_hw_caps->features & BIT(DPU_SSPP_INLINE_ROTATION)) @@ -940,48 +962,43 @@ static int dpu_plane_atomic_check_sspp(struct drm_plane *plane, return ret; if (drm_rect_width(&r_pipe_cfg->src_rect) != 0) { - /* - * In parallel multirect case only the half of the usual width - * is supported for tiled formats. If we are here, we know that - * full width is more than max_linewidth, thus each rect is - * wider than allowed. - */ - if (MSM_FORMAT_IS_UBWC(fmt) && - drm_rect_width(&pipe_cfg->src_rect) > max_linewidth) { - DPU_DEBUG_PLANE(pdpu, "invalid src " DRM_RECT_FMT " line:%u, tiled format\n", - DRM_RECT_ARG(&pipe_cfg->src_rect), max_linewidth); - return -E2BIG; - } + ret = dpu_plane_atomic_check_pipe(pdpu, r_pipe, r_pipe_cfg, fmt, + &crtc_state->adjusted_mode); + if (ret) + return ret; + } - if (drm_rect_width(&pipe_cfg->src_rect) != drm_rect_width(&pipe_cfg->dst_rect) || - drm_rect_height(&pipe_cfg->src_rect) != drm_rect_height(&pipe_cfg->dst_rect) || - (!test_bit(DPU_SSPP_SMART_DMA_V1, &pipe->sspp->cap->features) && - !test_bit(DPU_SSPP_SMART_DMA_V2, &pipe->sspp->cap->features)) || - pipe_cfg->rotation & DRM_MODE_ROTATE_90 || - MSM_FORMAT_IS_YUV(fmt)) { - DPU_DEBUG_PLANE(pdpu, "invalid src " DRM_RECT_FMT " line:%u, can't use split source\n", - DRM_RECT_ARG(&pipe_cfg->src_rect), max_linewidth); - return -E2BIG; - } + return 0; +} + +static bool dpu_plane_try_multirect_parallel(struct dpu_sw_pipe *pipe, struct dpu_sw_pipe_cfg *pipe_cfg, + struct dpu_sw_pipe *r_pipe, struct dpu_sw_pipe_cfg *r_pipe_cfg, + struct dpu_hw_sspp *sspp, const struct msm_format *fmt, + uint32_t max_linewidth) +{ + r_pipe->sspp = NULL; + + pipe->multirect_index = DPU_SSPP_RECT_SOLO; + pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE; + + r_pipe->multirect_index = DPU_SSPP_RECT_SOLO; + r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE; + + if (drm_rect_width(&r_pipe_cfg->src_rect) != 0) { + if (!dpu_plane_is_multirect_parallel_capable(pipe->sspp, pipe_cfg, fmt, max_linewidth) || + !dpu_plane_is_multirect_parallel_capable(pipe->sspp, r_pipe_cfg, fmt, max_linewidth)) + return false; + + r_pipe->sspp = pipe->sspp; - /* - * Use multirect for wide plane. We do not support dynamic - * assignment of SSPPs, so we know the configuration. - */ pipe->multirect_index = DPU_SSPP_RECT_0; pipe->multirect_mode = DPU_SSPP_MULTIRECT_PARALLEL; - r_pipe->sspp = pipe->sspp; r_pipe->multirect_index = DPU_SSPP_RECT_1; r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_PARALLEL; - - ret = dpu_plane_atomic_check_pipe(pdpu, r_pipe, r_pipe_cfg, fmt, - &crtc_state->adjusted_mode); - if (ret) - return ret; } - return 0; + return true; } static int dpu_plane_atomic_check(struct drm_plane *plane, @@ -995,16 +1012,16 @@ static int dpu_plane_atomic_check(struct drm_plane *plane, struct dpu_kms *dpu_kms = _dpu_plane_get_kms(plane); struct dpu_sw_pipe *pipe = &pstate->pipe; struct dpu_sw_pipe *r_pipe = &pstate->r_pipe; + struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg; + struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->r_pipe_cfg; const struct drm_crtc_state *crtc_state = NULL; + uint32_t max_linewidth = dpu_kms->catalog->caps->max_linewidth; if (new_plane_state->crtc) crtc_state = drm_atomic_get_new_crtc_state(state, new_plane_state->crtc); - if (pdpu->pipe != SSPP_NONE) { - pipe->sspp = dpu_rm_get_sspp(&dpu_kms->rm, pdpu->pipe); - r_pipe->sspp = NULL; - } + pipe->sspp = dpu_rm_get_sspp(&dpu_kms->rm, pdpu->pipe); if (!pipe->sspp) return -EINVAL; @@ -1016,10 +1033,17 @@ static int dpu_plane_atomic_check(struct drm_plane *plane, if (!new_plane_state->visible) return 0; - pipe->multirect_index = DPU_SSPP_RECT_SOLO; - pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE; - r_pipe->multirect_index = DPU_SSPP_RECT_SOLO; - r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE; + if (!dpu_plane_try_multirect_parallel(pipe, pipe_cfg, r_pipe, r_pipe_cfg, + pipe->sspp, + msm_framebuffer_format(new_plane_state->fb), + max_linewidth)) { + DPU_DEBUG_PLANE(pdpu, "invalid " DRM_RECT_FMT " /" DRM_RECT_FMT + " max_line:%u, can't use split source\n", + DRM_RECT_ARG(&pipe_cfg->src_rect), + DRM_RECT_ARG(&r_pipe_cfg->src_rect), + max_linewidth); + return -E2BIG; + } return dpu_plane_atomic_check_sspp(plane, state, crtc_state); } @@ -1054,8 +1078,16 @@ static int dpu_plane_virtual_atomic_check(struct drm_plane *plane, return 0; } - /* force resource reallocation if the format of FB has changed */ + /* + * Force resource reallocation if the format of FB or src/dst have + * changed. We might need to allocate different SSPP or SSPPs for this + * plane than the one used previously. + */ if (!old_plane_state || !old_plane_state->fb || + old_plane_state->src_w != plane_state->src_w || + old_plane_state->src_h != plane_state->src_h || + old_plane_state->src_w != plane_state->src_w || + old_plane_state->crtc_h != plane_state->crtc_h || msm_framebuffer_format(old_plane_state->fb) != msm_framebuffer_format(plane_state->fb)) crtc_state->planes_changed = true; @@ -1075,6 +1107,8 @@ static int dpu_plane_virtual_assign_resources(struct drm_crtc *crtc, struct dpu_plane_state *pstate; struct dpu_sw_pipe *pipe; struct dpu_sw_pipe *r_pipe; + struct dpu_sw_pipe_cfg *pipe_cfg; + struct dpu_sw_pipe_cfg *r_pipe_cfg; const struct msm_format *fmt; if (plane_state->crtc) @@ -1084,6 +1118,8 @@ static int dpu_plane_virtual_assign_resources(struct drm_crtc *crtc, pstate = to_dpu_plane_state(plane_state); pipe = &pstate->pipe; r_pipe = &pstate->r_pipe; + pipe_cfg = &pstate->pipe_cfg; + r_pipe_cfg = &pstate->r_pipe_cfg; pipe->sspp = NULL; r_pipe->sspp = NULL; @@ -1102,6 +1138,22 @@ static int dpu_plane_virtual_assign_resources(struct drm_crtc *crtc, if (!pipe->sspp) return -ENODEV; + if (!dpu_plane_try_multirect_parallel(pipe, pipe_cfg, r_pipe, r_pipe_cfg, + pipe->sspp, + msm_framebuffer_format(plane_state->fb), + dpu_kms->catalog->caps->max_linewidth)) { + /* multirect is not possible, use two SSPP blocks */ + r_pipe->sspp = dpu_rm_reserve_sspp(&dpu_kms->rm, global_state, crtc, &reqs); + if (!r_pipe->sspp) + return -ENODEV; + + pipe->multirect_index = DPU_SSPP_RECT_SOLO; + pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE; + + r_pipe->multirect_index = DPU_SSPP_RECT_SOLO; + r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE; + } + return dpu_plane_atomic_check_sspp(plane, state, crtc_state); }
Virtual wide planes give high amount of flexibility, but it is not always enough: In parallel multirect case only the half of the usual width is supported for tiled formats. Thus the whole width of two tiled multirect rectangles can not be greater than max_linewidth, which is not enough for some platforms/compositors. Another example is as simple as wide YUV plane. YUV planes can not use multirect, so currently they are limited to max_linewidth too. Now that the planes are fully virtualized, add support for allocating two SSPP blocks to drive a single DRM plane. This fixes both mentioned cases and allows all planes to go up to 2*max_linewidth (at the cost of making some of the planes unavailable to the user). Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 144 ++++++++++++++++++++---------- 1 file changed, 98 insertions(+), 46 deletions(-)