Re: [PATCH v3 4/4] drm/msm/dpu: fix video mode DSC INTF timing width calculation

[Dmitry Baryshkov]

On Thu, Mar 19, 2026 at 03:09:13PM +0100, Neil Armstrong wrote:
> Hi,
> 
> On 3/19/26 12:58, Alexander Koskovich wrote:
> > Using bits_per_component * 3 as the divisor for the compressed INTF
> > timing width produces constant FIFO errors for the BOE BF068MWM-TD0
> > panel due to bits_per_component being 10 which results in a divisor
> > of 30 instead of 24.
> > 
> > Regardless of the compression ratio and pixel depth, 24 bits of
> > compressed data are transferred per pclk, so the divisor should
> > always be 24.
> 
> Not true with widebus, specify why 24 and because DSI widebus is not implemented yet.

Support for the widebus is implemented and enable for DSI >= 2.5.0

> 
> > 
> > Signed-off-by: Alexander Koskovich <akoskovich@xxxxx>
> > ---
> >   drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 9 ++++-----
> >   1 file changed, 4 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> > index 0ba777bda253..5419ef0be137 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> > @@ -122,19 +122,18 @@ static void drm_mode_to_intf_timing_params(
> >   	}
> >   	/*
> > -	 * for DSI, if compression is enabled, then divide the horizonal active
> > -	 * timing parameters by compression ratio. bits of 3 components(R/G/B)
> > -	 * is compressed into bits of 1 pixel.
> > +	 * For DSI, if DSC is enabled, 24 bits of compressed data are
> > +	 * transferred per pclk regardless of the source pixel depth.
> >   	 */
> >   	if (phys_enc->hw_intf->cap->type != INTF_DP && timing->compression_en) {
> >   		struct drm_dsc_config *dsc =
> >   		       dpu_encoder_get_dsc_config(phys_enc->parent);
> > +
> Drop this change
> 
> >   		/*
> >   		 * TODO: replace drm_dsc_get_bpp_int with logic to handle
> >   		 * fractional part if there is fraction
> >   		 */
> > -		timing->width = timing->width * drm_dsc_get_bpp_int(dsc) /
> > -				(dsc->bits_per_component * 3);
> > +		timing->width = timing->width * drm_dsc_get_bpp_int(dsc) / 24;
> 
> It would be helpful to somehow show that 24 is 8 * 3, 8 being the byte width and 3 the compression ratio.

Otherwise I'd have assumed that it is bus width.

> 
> >   		timing->xres = timing->width;
> >   	}
> >   }
> > 
> 

-- 
With best wishes
Dmitry