On Fri, Aug 01, 2025 at 09:19:41AM +0800, Jun Nie wrote: > Dmitry Baryshkov <[email protected]> 于2025年8月1日周五 01:08写道： > > > > On 31/07/2025 18:37, Jun Nie wrote: > > > Dmitry Baryshkov <[email protected]> 于2025年7月31日周四 > > > 22:22写道： > > >> > > >> On 31/07/2025 13:52, Jun Nie wrote: > > >>> Dmitry Baryshkov <[email protected]> 于2025年7月31日周四 > > >>> 02:52写道： > > >>>> > > >>>> On Mon, Jul 28, 2025 at 09:14:34PM +0800, Jun Nie wrote: > > >>>>> Currently, SSPPs are assigned to a maximum of two pipes. However, > > >>>>> quad-pipe usage scenarios require four pipes and involve configuring > > >>>>> two stages. In quad-pipe case, the first two pipes share a set of > > >>>>> mixer configurations and enable multi-rect mode when certain > > >>>>> conditions are met. The same applies to the subsequent two pipes. > > >>>>> > > >>>>> Assign SSPPs to the pipes in each stage using a unified method and > > >>>>> to loop the stages accordingly. > > >>>>> > > >>>>> Signed-off-by: Jun Nie <[email protected]> > > >>>>> --- > > >>>>> drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 160 > > >>>>> ++++++++++++++++++------------ > > >>>>> 1 file changed, 99 insertions(+), 61 deletions(-) > > >>>>> > > >>>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c > > >>>>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c > > >>>>> index > > >>>>> 55429f29a4b95594771d930efe42aaa4126f6f07..e1e16a8d5ac55ba52a0f460d62901dced65e3a9e > > >>>>> 100644 > > >>>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c > > >>>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c > > >>>>> @@ -959,6 +959,30 @@ static int > > >>>>> dpu_plane_is_multirect_parallel_capable(struct dpu_hw_sspp *sspp, > > >>>>> } > > >>>>> > > >>>>> > > >>>>> +static bool dpu_plane_get_single_pipe_in_stage(struct > > >>>>> dpu_plane_state *pstate, > > >>>>> + struct dpu_sw_pipe > > >>>>> **single_pipe, > > >>>>> + struct dpu_sw_pipe_cfg > > >>>>> **single_pipe_cfg, > > >>>>> + int stage_index) > > >>>>> +{ > > >>>>> + int pipe_idx, i, valid_pipe = 0; > > >>>>> + > > >>>>> + for (i = 0; i < PIPES_PER_STAGE; i++) { > > >>>> > > >>>> Why do you need to loop here? Is there a case when pipe 0 is not > > >>>> assigned, but pipe 1 is? > > >>> > > >>> Loop the pipe in a stage to count the valid pipes. If there are 2 valid > > >>> pipes in stage of the current plane, this function will return false. > > >>> Or you prefer the below coding? > > >>> > > >>> 1029 pipe_idx = stage_index * PIPES_PER_STAGE; > > >>> 1030 if (drm_rect_width(&pstate->pipe_cfg[pipe_idx].src_rect) > > >>> != 0 && > > >>> 1031 drm_rect_width(&pstate->pipe_cfg[pipe_idx + > > >>> 1].src_rect) == 0) { > > >> > > >> Yes, this is better from my POV. But the bigger question is why do you > > >> need it at all? What is wrong with the existing check? Can it be that > > >> pipe0 is not used, but pipe1 is used? > > > > > > There is no case that pipe0 is not used, but pipe1 is used. Existing check > > > does not filter the plane which contains single pipe in a stage, which can > > > be a candidate for SSPP sharing. If the stage contains 2 valid pipes or > > > no valid pipes, it is skipped in dpu_plane_try_multirect_shared(), or it > > > is > > > skipped to be stored in prev_adjacent_plane_state[]. > > > > Oh, I see, you need to check both pipes because you might need to skip > > it completely. I'd really prefer to have more explicit code: > > > > - check for pipe0, skip this part of the plane if there is none > > - check for pipe1, if there is none, it's a candidate for sharing. > > Yeah, this is the logic of the current implementation. So the your only > concern is the loop. Will remove the loop as above code.
Ok. -- With best wishes Dmitry
