[PATCH v5 16/32] drm/msm/dpu: move the rest of plane checks to dpu_plane_atomic_check()

[Dmitry Baryshkov]

On 14/03/2023 06:02, Abhinav Kumar wrote:
> 
> 
> On 3/9/2023 4:56 PM, Dmitry Baryshkov wrote:
>> Move plane state updates from dpu_crtc_atomic_check() to the function
>> where they belong: to dpu_plane_atomic_check().
>>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
> 
> Can you please elaborate a bit on this comment?
> 
> https://patchwork.freedesktop.org/patch/521356/?series=99909&rev=4#comment_949772
> 
> Is something still missing? I dont see the multirects being cleared in 
> dpu_plane_atomic_disable() before or even now.

It is done in the last patch, where it belongs. There is no need to 
clear multirect setup until we support RECT_1.

> 
>> ---
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c  | 18 +-----------------
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 18 ++++++++++--------
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h |  6 ------
>>   3 files changed, 11 insertions(+), 31 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c 
>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
>> index 0260c4d6ded7..bd09bb319a58 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
>> @@ -1129,7 +1129,6 @@ static int dpu_crtc_atomic_check(struct drm_crtc 
>> *crtc,
>>                                         crtc);
>>       struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
>>       struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc_state);
>> -    struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc);
>>       const struct drm_plane_state *pstate;
>>       struct drm_plane *plane;
>> @@ -1161,11 +1160,10 @@ static int dpu_crtc_atomic_check(struct 
>> drm_crtc *crtc,
>>       crtc_rect.x2 = mode->hdisplay;
>>       crtc_rect.y2 = mode->vdisplay;
>> -     /* get plane state for all drm planes associated with crtc state */
>> +    /* FIXME: move this to dpu_plane_atomic_check? */
>>       drm_atomic_crtc_state_for_each_plane_state(plane, pstate, 
>> crtc_state) {
>>           struct dpu_plane_state *dpu_pstate = 
>> to_dpu_plane_state(pstate);
>>           struct drm_rect dst, clip = crtc_rect;
>> -        int stage;
>>           if (IS_ERR_OR_NULL(pstate)) {
>>               rc = PTR_ERR(pstate);
>> @@ -1179,8 +1177,6 @@ static int dpu_crtc_atomic_check(struct drm_crtc 
>> *crtc,
>>           dpu_pstate->needs_dirtyfb = needs_dirtyfb;
>> -        dpu_plane_clear_multirect(pstate);
>> -
>>           dst = drm_plane_state_dest(pstate);
>>           if (!drm_rect_intersect(&clip, &dst)) {
>>               DPU_ERROR("invalid vertical/horizontal destination\n");
>> @@ -1189,18 +1185,6 @@ static int dpu_crtc_atomic_check(struct 
>> drm_crtc *crtc,
>>                     DRM_RECT_ARG(&dst));
>>               return -E2BIG;
>>           }
>> -
>> -        /* verify stage setting before using it */
>> -        stage = DPU_STAGE_0 + pstate->normalized_zpos;
>> -        if (stage >= dpu_kms->catalog->caps->max_mixer_blendstages) {
>> -            DPU_ERROR("> %d plane stages assigned\n",
>> -                  dpu_kms->catalog->caps->max_mixer_blendstages - 
>> DPU_STAGE_0);
>> -            return -EINVAL;
>> -        }
>> -
>> -        to_dpu_plane_state(pstate)->stage = stage;
>> -        DRM_DEBUG_ATOMIC("%s: stage %d\n", dpu_crtc->name, stage);
>> -
>>       }
>>       atomic_inc(&_dpu_crtc_get_kms(crtc)->bandwidth_ref);
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c 
>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>> index ce01a602cbc9..3fba63fbbd78 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>> @@ -733,14 +733,6 @@ static int _dpu_plane_color_fill(struct dpu_plane 
>> *pdpu,
>>       return 0;
>>   }
>> -void dpu_plane_clear_multirect(const struct drm_plane_state *drm_state)
>> -{
>> -    struct dpu_plane_state *pstate = to_dpu_plane_state(drm_state);
>> -
>> -    pstate->pipe.multirect_index = DPU_SSPP_RECT_SOLO;
>> -    pstate->pipe.multirect_mode = DPU_SSPP_MULTIRECT_NONE;
>> -}
>> -
>>   int dpu_plane_validate_multirect_v2(struct 
>> dpu_multirect_plane_states *plane)
>>   {
>>       struct dpu_plane_state *pstate[R_MAX];
>> @@ -994,6 +986,16 @@ static int dpu_plane_atomic_check(struct 
>> drm_plane *plane,
>>       if (!new_plane_state->visible)
>>           return 0;
>> +    pstate->pipe.multirect_index = DPU_SSPP_RECT_SOLO;
>> +    pstate->pipe.multirect_mode = DPU_SSPP_MULTIRECT_NONE;
>> +
>> +    pstate->stage = DPU_STAGE_0 + pstate->base.normalized_zpos;
>> +    if (pstate->stage >= pdpu->catalog->caps->max_mixer_blendstages) {
>> +        DPU_ERROR("> %d plane stages assigned\n",
>> +              pdpu->catalog->caps->max_mixer_blendstages - DPU_STAGE_0);
>> +        return -EINVAL;
>> +    }
>> +
>>       src.x1 = new_plane_state->src_x >> 16;
>>       src.y1 = new_plane_state->src_y >> 16;
>>       src.x2 = src.x1 + (new_plane_state->src_w >> 16);
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h 
>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
>> index 228db401e905..a08b0539513b 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
>> @@ -88,12 +88,6 @@ struct drm_plane *dpu_plane_init(struct drm_device 
>> *dev,
>>    */
>>   int dpu_plane_validate_multirect_v2(struct 
>> dpu_multirect_plane_states *plane);
>> -/**
>> - * dpu_plane_clear_multirect - clear multirect bits for the given pipe
>> - * @drm_state: Pointer to DRM plane state
>> - */
>> -void dpu_plane_clear_multirect(const struct drm_plane_state *drm_state);
>> -
>>   /**
>>    * dpu_plane_color_fill - enables color fill on plane
>>    * @plane:  Pointer to DRM plane object

-- 
With best wishes
Dmitry