linux-kernel - Re: [PATCH v3 10/11] drm/msm/dpu: Refactor SSPP to compatible DPU 13.0.0

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aUIZXiL+JY27phny@yuanjiey.ap.qualcomm.com>
Date: Wed, 17 Dec 2025 10:45:50 +0800
From: yuanjiey <yuanjie.yang@....qualcomm.com>
To: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
Cc: robin.clark@....qualcomm.com, lumag@...nel.org, abhinav.kumar@...ux.dev,
        jesszhan0024@...il.com, sean@...rly.run, marijn.suijten@...ainline.org,
        airlied@...il.com, simona@...ll.ch, maarten.lankhorst@...ux.intel.com,
        mripard@...nel.org, tzimmermann@...e.de, robh@...nel.org,
        krzk+dt@...nel.org, conor+dt@...nel.org, neil.armstrong@...aro.org,
        yongxing.mou@....qualcomm.com, konrad.dybcio@....qualcomm.com,
        linux-arm-msm@...r.kernel.org, dri-devel@...ts.freedesktop.org,
        freedreno@...ts.freedesktop.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, tingwei.zhang@....qualcomm.com,
        aiqun.yu@....qualcomm.com
Subject: Re: [PATCH v3 10/11] drm/msm/dpu: Refactor SSPP to compatible DPU
 13.0.0

On Tue, Dec 16, 2025 at 04:22:32PM +0200, Dmitry Baryshkov wrote:
> On Tue, Dec 16, 2025 at 02:56:31PM +0800, yuanjiey wrote:
> > On Mon, Dec 15, 2025 at 10:08:22PM +0200, Dmitry Baryshkov wrote:
> > > On Mon, Dec 15, 2025 at 04:38:53PM +0800, yuanjie yang wrote:
> > > > From: Yuanjie Yang <yuanjie.yang@....qualcomm.com>
> > > > 
> > > > DPU version 13.0.0 introduces structural changes including
> > > > register additions, removals, and relocations.
> > > > 
> > > > Refactor SSPP-related code to be compatible with DPU 13.0.0
> > > > modifications.
> > > > 
> > > > Co-developed-by: Yongxing Mou <yongxing.mou@....qualcomm.com>
> > > > Signed-off-by: Yongxing Mou <yongxing.mou@....qualcomm.com>
> > > > Signed-off-by: Yuanjie Yang <yuanjie.yang@....qualcomm.com>
> > > > ---
> > > 
> > > We've fixed the order of the interrupts patch. Now you are adding SSPP
> > > customization for 13.x _after_ adding the first 13.x support. Is that
> > > supposed to work?
> > 
> > Yes, will reorganize order.
> 
> And after comparing with v2, I'm really surprised. It was better before
> and then you changed the order of the patches. Why? You were asked to
> split it, but not to move it to the end.

I make the mistake.
Sure, I will keep the v2 patch order in next patch.
 
> > 
> >  
> > > >  .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h    |  15 +-
> > > >  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c   | 155 ++++++++++--------
> > > >  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h   |  52 ++++++
> > > >  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c   |  18 ++
> > > >  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h   |   3 +
> > > >  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c     |  17 +-
> > > >  6 files changed, 191 insertions(+), 69 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> > > >  		switch (ctx->ubwc->ubwc_enc_version) {
> > > >  		case UBWC_1_0:
> > > >  			fast_clear = fmt->alpha_enable ? BIT(31) : 0;
> > > > -			DPU_REG_WRITE(c, SSPP_UBWC_STATIC_CTRL,
> > > > -					fast_clear | (ctx->ubwc->ubwc_swizzle & 0x1) |
> > > > -					BIT(8) |
> > > > -					(ctx->ubwc->highest_bank_bit << 4));
> > > > +			DPU_REG_WRITE(c, ubwc_ctrl_off,
> > > > +				      fast_clear | (ctx->ubwc->ubwc_swizzle & 0x1) |
> > > > +				      BIT(8) |
> > > > +				     (ctx->ubwc->highest_bank_bit << 4));
> > > 
> > > I have asked to drop unrelated changes. You didn't. Why? You are
> > > changing whitespaces for no reason. It's just a noise which hides the
> > > actual change here.
> > 
> > here ubwc reg layout change in DPU 13.
> > 
> > ubwc_ctrl_off
> > veriosn < 13 
> > reg: SSPP_UBWC_STATIC_CTRL
> > verison >= 13 
> > reg: SSPP_REC_UBWC_STATIC_CTRL
> > 
> > So I do some fix.
> 
> What does it have to do with the whitespaces? Fix _one_ line.
get it, will drop unrelated whitespaces.

> > 
> > > >  			break;
> > > >  		case UBWC_2_0:
> > > >  			fast_clear = fmt->alpha_enable ? BIT(31) : 0;
> > > > -			DPU_REG_WRITE(c, SSPP_UBWC_STATIC_CTRL,
> > > > -					fast_clear | (ctx->ubwc->ubwc_swizzle) |
> > > > -					(ctx->ubwc->highest_bank_bit << 4));
> > > > +			DPU_REG_WRITE(c, ubwc_ctrl_off,
> > > > +				      fast_clear | (ctx->ubwc->ubwc_swizzle) |
> > > > +				     (ctx->ubwc->highest_bank_bit << 4));
> > > >  			break;
> > > >  		case UBWC_3_0:
> > > > -			DPU_REG_WRITE(c, SSPP_UBWC_STATIC_CTRL,
> > > > -					BIT(30) | (ctx->ubwc->ubwc_swizzle) |
> > > > -					(ctx->ubwc->highest_bank_bit << 4));
> > > > +			DPU_REG_WRITE(c, ubwc_ctrl_off,
> > > > +				      BIT(30) | (ctx->ubwc->ubwc_swizzle) |
> > > > +				     (ctx->ubwc->highest_bank_bit << 4));
> > > >  			break;
> > > >  		case UBWC_4_0:
> > > > -			DPU_REG_WRITE(c, SSPP_UBWC_STATIC_CTRL,
> > > > -					MSM_FORMAT_IS_YUV(fmt) ? 0 : BIT(30));
> > > > +			DPU_REG_WRITE(c, ubwc_ctrl_off,
> > > > +				      MSM_FORMAT_IS_YUV(fmt) ? 0 : BIT(30));
> > > >  			break;
> > > >  		}
> > > >  	}
> > > > @@ -313,19 +337,18 @@ static void dpu_hw_sspp_setup_format(struct dpu_sw_pipe *pipe,
> > > >  
> > > >  	/* update scaler opmode, if appropriate */
> > > >  	if (test_bit(DPU_SSPP_CSC, &ctx->cap->features))
> > > > -		_sspp_setup_opmode(ctx, VIG_OP_CSC_EN | VIG_OP_CSC_SRC_DATAFMT,
> > > > -			MSM_FORMAT_IS_YUV(fmt));
> > > > +		dpu_hw_sspp_setup_opmode(ctx, VIG_OP_CSC_EN | VIG_OP_CSC_SRC_DATAFMT,
> > > > +					 MSM_FORMAT_IS_YUV(fmt));
> > > >  	else if (test_bit(DPU_SSPP_CSC_10BIT, &ctx->cap->features))
> > > > -		_sspp_setup_csc10_opmode(ctx,
> > > > -			VIG_CSC_10_EN | VIG_CSC_10_SRC_DATAFMT,
> > > > -			MSM_FORMAT_IS_YUV(fmt));
> > > > +		dpu_hw_sspp_setup_csc10_opmode(ctx,
> > > > +					       VIG_CSC_10_EN | VIG_CSC_10_SRC_DATAFMT,
> > > > +					       MSM_FORMAT_IS_YUV(fmt));
> > > 
> > > Again, useless whitespace changes.
> > checkpatch.pl says here is alignment issuse, so I do this fix.
> 
> The issue was present before your patch. If you want to fix it, fix it
> in the separate patch or ignore it.
get it, will drop unrelated whitespaces.

> > 
> > > >  
> > > >  	DPU_REG_WRITE(c, format_off, src_format);
> > > >  	DPU_REG_WRITE(c, unpack_pat_off, unpack);
> > > >  	DPU_REG_WRITE(c, op_mode_off, opmode);
> > > > -
> > > 
> > > Why?
> > 
> > yes, will drop "-" diff.
> > 
> > > >  	/* clear previous UBWC error */
> > > > -	DPU_REG_WRITE(c, SSPP_UBWC_ERROR_STATUS, BIT(31));
> > > > +	DPU_REG_WRITE(c, ubwc_err_off, BIT(31));
> > > >  }
> > > >  
> > > >  static void dpu_hw_sspp_setup_pe_config(struct dpu_hw_sspp *ctx,
> > > > @@ -385,9 +408,9 @@ static void dpu_hw_sspp_setup_pe_config(struct dpu_hw_sspp *ctx,
> > > >  			tot_req_pixels[3]);
> > > >  }
> > > >  
> > > > -static void _dpu_hw_sspp_setup_scaler3(struct dpu_hw_sspp *ctx,
> > > > -		struct dpu_hw_scaler3_cfg *scaler3_cfg,
> > > > -		const struct msm_format *format)
> > > > +void dpu_hw_sspp_setup_scaler3(struct dpu_hw_sspp *ctx,
> > > > +			       struct dpu_hw_scaler3_cfg *scaler3_cfg,
> > > > +			       const struct msm_format *format)
> > > 
> > > And here...
> > checkpatch.pl says here is alignment issuse, so I do this fix.
> 
> And I'm asking you to don't do it. Don't clutter the patch with
> unrelated changes (and whitespace / alignment changes are generally
> unrelated).
>
> -- 
> With best wishes
> Dmitry

Thanks,
Yuanjie