Re: [PATCH v4 3/6] drm/msm/dpu: check mode against PINGPONG or DSC max width

[Mahadevan P <mahadevan.p@oss.qualcomm.com>]

On 10/12/2026 5:44 PM, Dmitry Baryshkov wrote:
> On Thu, Oct 08, 2026 at 10:53:49PM +0530, Mahadevan P wrote:
>> From: Jessica Zhang <jesszhan0024@gmail.com>
>>
>> LM block doesn't have a hardware buffer (unlike PINGPONG and DSC
>> encoders). As such, don't use ephemeral max_mixer_width and
>> MAX_HDISPLAY_SPLIT to validate requested modes. Instead use PP and DSC
>> buffer widths.
>>
>> While on the DPU 8.x+ supports a max linewidth of 8960 for PINGPONG_0,
>> there is some additional logic that needs to be added to the resource
>> manager to specifically try and reserve PINGPONG_0 for modes that are
>> greater than 5k.
>>
>> The layer-mixer count for a merge-capable, non-DSC mode is chosen by
>> dpu_crtc_num_lm_for_mode(); feed the PINGPONG/DSC derived width to it as
>> the split threshold in place of the removed MAX_HDISPLAY_SPLIT, so the
>> pixel-rate floor added earlier keeps high-refresh modes that now fit
>> within a single PINGPONG buffer split across two mixers.
>>
>> Signed-off-by: Jessica Zhang <jessica.zhang@oss.qualcomm.com>
>> Tested-by: Xilin Wu <sophon@radxa.com> # qcs6490-radxa-dragon-q6a
>> [DB: reworked to drop catalog changes, updated commit message]
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
>> [MP: rebased on msm-next; fed PINGPONG/DSC width into
>>   dpu_crtc_num_lm_for_mode() instead of open-coding the width test]
>> Signed-off-by: Mahadevan P <mahadevan.p@oss.qualcomm.com>
>> ---
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c       | 54 +++++++++++++++++++++-----
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h |  7 ++++
>>   2 files changed, 51 insertions(+), 10 deletions(-)
>>
>> @@ -765,13 +781,14 @@ static int _dpu_crtc_check_and_setup_lm_bounds(struct drm_crtc *crtc,
>>   	struct drm_display_mode *adj_mode = &state->adjusted_mode;
>>   	u32 crtc_split_width = adj_mode->hdisplay / cstate->num_mixers;
>>   	struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc);
>> +	int max_pingpong_width = msm_display_get_max_pingpong_width(dpu_kms);
>>   	int i;
>>   
>>   	/* if we cannot merge 2 LMs (no 3d mux) better to fail earlier
>>   	 * before even checking the width after the split
>>   	 */
>>   	if (!dpu_kms->catalog->caps->has_3d_merge &&
>> -	    adj_mode->hdisplay > dpu_kms->catalog->caps->max_mixer_width)
>> +	    adj_mode->hdisplay > max_pingpong_width)
>>   		return -E2BIG;
> I guess Sashiko is correct here, we need to check DSC width.
Agreed, the PINGPONG bounds check alone does not enforce the DSC encoder 
width. The next revision validates the width per selected DSC encoder 
before resource reservation.
>
>>   
>>   	for (i = 0; i < cstate->num_mixers; i++) {
>> @@ -783,7 +800,7 @@ static int _dpu_crtc_check_and_setup_lm_bounds(struct drm_crtc *crtc,
>>   
>>   		trace_dpu_crtc_setup_lm_bounds(DRMID(crtc), i, r);
>>   
>> -		if (drm_rect_width(r) > dpu_kms->catalog->caps->max_mixer_width)
>> +		if (drm_rect_width(r) > max_pingpong_width)
>>   			return -E2BIG;
>>   	}
>>   
>> @@ -1374,17 +1391,17 @@ static int dpu_crtc_reassign_planes(struct drm_crtc *crtc, struct drm_crtc_state
>>   }
>>   
>>   #define MAX_CHANNELS_PER_CRTC PIPES_PER_PLANE
>> -#define MAX_HDISPLAY_SPLIT 1080
>>   
>>   static u32 dpu_crtc_num_lm_for_mode(struct dpu_kms *dpu_kms,
>> -				    const struct drm_display_mode *mode)
>> +				    const struct drm_display_mode *mode,
>> +				    u32 max_hdisplay_split)
>>   {
>>   	u64 mode_clk;
>>   
>>   	if (!dpu_kms->catalog->caps->has_3d_merge)
>>   		return 1;
>>   
>> -	if (mode->hdisplay > MAX_HDISPLAY_SPLIT)
>> +	if (mode->hdisplay > max_hdisplay_split)
>>   		return 2;
>>   
>>   	mode_clk = dpu_core_perf_adjusted_mode_clk(mode->clock,
>> @@ -1403,6 +1420,7 @@ static struct msm_display_topology dpu_crtc_get_topology(
>>   	struct drm_display_mode *mode = &crtc_state->adjusted_mode;
>>   	struct msm_display_topology topology = {0};
>>   	struct drm_encoder *drm_enc;
>> +	u32 max_hdisplay_split;
>>   
>>   	drm_for_each_encoder_mask(drm_enc, crtc->dev, crtc_state->encoder_mask)
>>   		dpu_encoder_update_topology(drm_enc, &topology, crtc_state->state,
>> @@ -1410,6 +1428,19 @@ static struct msm_display_topology dpu_crtc_get_topology(
>>   
>>   	topology.cwb_enabled = drm_crtc_in_clone_mode(crtc_state);
>>   
>> +	max_hdisplay_split = msm_display_get_max_pingpong_width(dpu_kms);
>> +
>> +	if (topology.num_dsc > 0) {
>> +		u32 max_dsc_encoder_width;
>> +
>> +		if (dpu_kms->catalog->mdss_ver->core_major_ver < 6)
>> +			max_dsc_encoder_width = DPU_1_x_MAX_DSC_ENCODER_WIDTH;
>> +		else
>> +			max_dsc_encoder_width = DPU_8_x_MAX_DSC_ENCODER_WIDTH;
>> +
>> +		max_hdisplay_split = min(max_hdisplay_split, max_dsc_encoder_width);
> Should this be min() rather than max()?
The code already uses min() here, as the width must fit both the 
PINGPONG and DSC encoder limits. If the variable name is unclear, I can 
rename it to "single_lm_width".
>
>> +	}
>> +
>>   	/*
>>   	 * Datapath topology selection
>>   	 *
Thanks,
Mahadevan