[PATCH v6 03/14] drm/msm/dpu: switch RM to use crtc_id rather than enc_id for allocation

[Jessica Zhang]

On 2/27/2025 7:07 AM, Dmitry Baryshkov wrote:
> On Fri, Feb 14, 2025 at 04:14:26PM -0800, Jessica Zhang wrote:
>> From: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
>>
>> Up to now the driver has been using encoder to allocate hardware
>> resources. Switch it to use CRTC id in preparation for the next step.
>>
>> Reviewed-by: Abhinav Kumar <quic_abhinavk at quicinc.com>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
>> Signed-off-by: Jessica Zhang <quic_jesszhan at quicinc.com>
>> ---
>> Changes in v6:
>> - Drop duplicate cstate initialization code and unnecessary memset
>> Changes in v5:
>> - Reordered to prevent breaking CI and upon partial application
>>
>> Changes in v4 (due to rebase):
>> - moved *_get_assigned_resources() changes for DSPP and LM from
>>    encoder *_virt_atomic_mode_set() to *_assign_crtc_resources()
>> ---
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c    |  18 +--
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c |  10 +-
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h     |  12 +-
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c      | 189 ++++++++++++++--------------
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h      |   7 +-
>>   5 files changed, 110 insertions(+), 126 deletions(-)
> 
> This commit breaks several tests in CI:
> - sc7180-trogdor-kingoftown:
>    - kms_cursor_crc at cursor-dpms
>    - kms_pipe_crc_basic at disable-crc-after-crtc
> - sc7180-trogdor-lazor-limozeen
>    - kms_cursor_crc at cursor-dpms
>    - kms_pipe_crc_basic at disable-crc-after-crtc

Hey Dmitry,

Thanks for catching this. Looks like this was exposed due to a recent 
IGT uprev that included  dc2d7fb4f978 ("lib/igt_kms: move setting 
DRM_CLIENT_CAP_WRITEBACK_CONNECTORS to kms_writeback").

The issue itself is that when DPMS is toggled, it is possible for RM to 
reserve new HW resources but skip the atomic_enable() due to the checks 
here [1]. This means that the change in HW block reservation won't be 
propagated to encoder if DPMS is set to off.

I've posted a fix for this here [2].

Thanks,

Jessica Zhang

[1] 
https://elixir.bootlin.com/linux/v6.14-rc4/source/drivers/gpu/drm/drm_atomic_helper.c#L1502
[2] https://patchwork.freedesktop.org/series/145735/

> 
> Corresponding pipeline is available at [1]
> 
> As I had to rebase your changes on top of msm-next, corresponding tree
> is available at [2]. It might be possible that the regression is
> introduced by my rebase.
> 
> [1] https://gitlab.freedesktop.org/drm/msm/-/pipelines/1374165
> 
> [2] https://gitlab.freedesktop.org/lumag/msm/-/commits/msm-next-lumag-cwb
> 
> -- 
> With best wishes
> Dmitry