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.
Signed-off-by: Alexander Koskovich <akoskovich@pm.me>
---
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);
+
/*
* 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;
timing->xres = timing->width;
}
}
--
2.53.0Hi,
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.
>
> Signed-off-by: Alexander Koskovich <akoskovich@pm.me>
> ---
> 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.
> timing->xres = timing->width;
> }
> }
>
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@pm.me>
> > ---
> > 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
On Thursday, March 19th, 2026 at 10:13 AM, Neil Armstrong <neil.armstrong@linaro.org> 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.
>
> >
> > Signed-off-by: Alexander Koskovich <akoskovich@pm.me>
> > ---
> > 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.
Can you clarify what the 3 represents? My panel should have a 3.75:1 compression
ratio (30/8) so the final divisor of 24 would be wrong for my panel if its the
compression ratio?
>
> > timing->xres = timing->width;
> > }
> > }
> >
>
>
>
Thanks,
Alex
On 3/19/26 15:40, Alexander Koskovich wrote:
> On Thursday, March 19th, 2026 at 10:13 AM, Neil Armstrong <neil.armstrong@linaro.org> 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.
>>
>>>
>>> Signed-off-by: Alexander Koskovich <akoskovich@pm.me>
>>> ---
>>> 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.
>
> Can you clarify what the 3 represents? My panel should have a 3.75:1 compression
> ratio (30/8) so the final divisor of 24 would be wrong for my panel if its the
> compression ratio?
So my guess is that while the exact ratio on the DSI lanes is 3.75:1, the ratio
used to calculate the INTF timings is 3, then the DSC encoder probably does the
magic to feed 10bpp into a 3.75:1 ratio over the DSI lanes.
In dsi_adjust_pclk_for_compression, the pclk is adjusted to take in account bits_per_component,
so I presume the actual DSI pclk _is_ timing->width * drm_dsc_get_bpp_int(dsc) / (dsc->bits_per_component * 3),
which is your 3.75:1, but the INTF needs to generate timing->width * drm_dsc_get_bpp_int(dsc) / (8 * 3) pixels
to the DSC encoder which will emit timing->width * drm_dsc_get_bpp_int(dsc) / (dsc->bits_per_component * 3) pixels.
In any case, 24 _is_ 3 * 8, 3 being the DSC compression ratio on the INTF side.
Dmitry, Konrad, could you help confirming this ?
Neil
>
>>
>>> timing->xres = timing->width;
>>> }
>>> }
>>>
>>
>>
>>
>
> Thanks,
> Alex
On 3/19/26 10:54 AM, Neil Armstrong wrote:
> On 3/19/26 15:40, Alexander Koskovich wrote:
>> On Thursday, March 19th, 2026 at 10:13 AM, Neil Armstrong
>> <neil.armstrong@linaro.org> 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.
>>>
DSI widebus is implemented, and always used when available. The
adjustment for DSI widebus just doesn't happen in this function.
>>>>
>>>> Signed-off-by: Alexander Koskovich <akoskovich@pm.me>
>>>> ---
>>>> 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.
>>
>> Can you clarify what the 3 represents? My panel should have a 3.75:1
>> compression
>> ratio (30/8) so the final divisor of 24 would be wrong for my panel if
>> its the
>> compression ratio?
>
> So my guess is that while the exact ratio on the DSI lanes is 3.75:1,
> the ratio
> used to calculate the INTF timings is 3, then the DSC encoder probably
> does the
> magic to feed 10bpp into a 3.75:1 ratio over the DSI lanes.
>
That's not how it works. INTF (which feeds DSI) is after DSC compression.
INTF timings are always in RGB888 (24-bit) units. Ignoring widebus
details, the INTF timing should match what is programmed on the DSI side
(hdisplay, which is calculated as bytes per line / 3).
(fwiw, the current "timing->width = ..." calculation here blames to me,
but what I wrote originally was just "timing->width = timing->width / 3"
with a comment about being incomplete.)
> In dsi_adjust_pclk_for_compression, the pclk is adjusted to take in
> account bits_per_component,
> so I presume the actual DSI pclk _is_ timing->width *
> drm_dsc_get_bpp_int(dsc) / (dsc->bits_per_component * 3),
> which is your 3.75:1, but the INTF needs to generate timing->width *
> drm_dsc_get_bpp_int(dsc) / (8 * 3) pixels
> to the DSC encoder which will emit timing->width *
> drm_dsc_get_bpp_int(dsc) / (dsc->bits_per_component * 3) pixels.
>
The hdisplay calculation in dsi_adjust_pclk_for_compression (which only
affects the clock rate) seems to be wrong, and I think Alexander's panel
must be running at a 20% lower clock because of it. dsi_timing_setup has
the right hdisplay adjustment.
> In any case, 24 _is_ 3 * 8, 3 being the DSC compression ratio on the
> INTF side.
>
> Dmitry, Konrad, could you help confirming this ?
>
> Neil
>
>>
>>>
>>>> timing->xres = timing->width;
>>>> }
>>>> }
>>>>
>>>
>>>
>>>
>>
>> Thanks,
>> Alex
>
On Thu, Mar 19, 2026 at 01:23:03PM -0400, Jonathan Marek wrote:
> On 3/19/26 10:54 AM, Neil Armstrong wrote:
> > On 3/19/26 15:40, Alexander Koskovich wrote:
> > > On Thursday, March 19th, 2026 at 10:13 AM, Neil Armstrong
> > > <neil.armstrong@linaro.org> 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.
> > > >
>
> DSI widebus is implemented, and always used when available. The adjustment
> for DSI widebus just doesn't happen in this function.
>
> > > > >
> > > > > Signed-off-by: Alexander Koskovich <akoskovich@pm.me>
> > > > > ---
> > > > > 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.
> > >
> > > Can you clarify what the 3 represents? My panel should have a 3.75:1
> > > compression
> > > ratio (30/8) so the final divisor of 24 would be wrong for my panel
> > > if its the
> > > compression ratio?
> >
> > So my guess is that while the exact ratio on the DSI lanes is 3.75:1,
> > the ratio
> > used to calculate the INTF timings is 3, then the DSC encoder probably
> > does the
> > magic to feed 10bpp into a 3.75:1 ratio over the DSI lanes.
> >
>
> That's not how it works. INTF (which feeds DSI) is after DSC compression.
>
> INTF timings are always in RGB888 (24-bit) units. Ignoring widebus details,
> the INTF timing should match what is programmed on the DSI side (hdisplay,
> which is calculated as bytes per line / 3).
>
> (fwiw, the current "timing->width = ..." calculation here blames to me, but
> what I wrote originally was just "timing->width = timing->width / 3" with a
> comment about being incomplete.)
>
Okay. After reading the docs (sorry, it took a while).
- When widebus is not enabled, the transfer is always 24 bit of
compressed data. Thus if it is not in play, pclk and timing->width
should be scaled by source_pixel_depth / compression_ratio / 24. In
case of the code it is 'drm_dsc_get_bpp_int(dsc) / 24'.
For RGB101010 / 8bpp DSC this should result in the PCLK being lowered
by the factor of 3 (= 24 / (30 / 3.75))
- When widebus is in play (MDSS 6.x+, DSI 2.4+), the transfer takes
more than 24 bits. In this case the PCLK and timing->width should be
scaled exactly by the DSC compression ratio, which is
'drm_dsc_get_bpp_int(dsc) / (3 * dsc->bits_per_component).
So, this piece of code needs to be adjusted to check for the widebus
being enabled or not.
> > In dsi_adjust_pclk_for_compression, the pclk is adjusted to take in
> > account bits_per_component,
> > so I presume the actual DSI pclk _is_ timing->width *
> > drm_dsc_get_bpp_int(dsc) / (dsc->bits_per_component * 3),
> > which is your 3.75:1, but the INTF needs to generate timing->width *
> > drm_dsc_get_bpp_int(dsc) / (8 * 3) pixels
> > to the DSC encoder which will emit timing->width *
> > drm_dsc_get_bpp_int(dsc) / (dsc->bits_per_component * 3) pixels.
> >
>
> The hdisplay calculation in dsi_adjust_pclk_for_compression (which only
> affects the clock rate) seems to be wrong, and I think Alexander's panel
> must be running at a 20% lower clock because of it. dsi_timing_setup has the
> right hdisplay adjustment.
That function also needs to be adjusted accordingly. I think only the
dsi_timing_setup() is correct at this point. Note, widebus / not-widebus
cases should be handled separately.
> > In any case, 24 _is_ 3 * 8, 3 being the DSC compression ratio on the
> > INTF side.
In this case DSC compression ratio is 3.75, so it's not true.
--
With best wishes
Dmitry
On 3/19/26 9:45 PM, Dmitry Baryshkov wrote: > On Thu, Mar 19, 2026 at 01:23:03PM -0400, Jonathan Marek wrote: ... >> >> That's not how it works. INTF (which feeds DSI) is after DSC compression. >> >> INTF timings are always in RGB888 (24-bit) units. Ignoring widebus details, >> the INTF timing should match what is programmed on the DSI side (hdisplay, >> which is calculated as bytes per line / 3). >> >> (fwiw, the current "timing->width = ..." calculation here blames to me, but >> what I wrote originally was just "timing->width = timing->width / 3" with a >> comment about being incomplete.) >> > Okay. After reading the docs (sorry, it took a while). > > - When widebus is not enabled, the transfer is always 24 bit of > compressed data. Thus if it is not in play, pclk and timing->width > should be scaled by source_pixel_depth / compression_ratio / 24. In > case of the code it is 'drm_dsc_get_bpp_int(dsc) / 24'. > > For RGB101010 / 8bpp DSC this should result in the PCLK being lowered > by the factor of 3 (= 24 / (30 / 3.75)) > > - When widebus is in play (MDSS 6.x+, DSI 2.4+), the transfer takes > more than 24 bits. In this case the PCLK and timing->width should be > scaled exactly by the DSC compression ratio, which is > 'drm_dsc_get_bpp_int(dsc) / (3 * dsc->bits_per_component). > > So, this piece of code needs to be adjusted to check for the widebus > being enabled or not. > The widebus adjustment on the MDP/INTF side is already in dpu_hw_intf_setup_timing_engine: the "data width" is divided by 2 for 48-bit widebus instead of 24-bit. there shouldn't be any other adjustment (downstream doesn't have any other adjustment). a relevant downstream comment: "In DATABUS-WIDEN mode, MDP always sends out 48-bit compressed data per pclk and on average, DSI consumes an amount of compressed data equivalent to the uncompressed pixel depth per pclk." Based on that comment, this patch is correct, and the ''drm_dsc_get_bpp_int(dsc) / (3 * dsc->bits_per_component)' adjustment only applies to DSI. (note: newer downstream looks like it would divide by 3.75 here, which doesn't make sense. older downstream would divide by 3 here. I guess downstream is broken now and video mode + 10-bit dsc doesn't get tested?) on DSI side, "uncompressed pixel depth" shouldn't matter either: DSI only sees the compressed data. But based on that comment, when widebus is enabled, by setting DSI_VID_CFG0_DST_FORMAT(?) to 30bpp, then the DSI pclk is in 30-bit units instead of 24-bits. And with this series DSI side ends up with the right result if 30bpp format and widebus is enabled.
On Fri, Mar 20, 2026 at 12:25:00AM -0400, Jonathan Marek wrote: > On 3/19/26 9:45 PM, Dmitry Baryshkov wrote: > > On Thu, Mar 19, 2026 at 01:23:03PM -0400, Jonathan Marek wrote: > ... > > > > > > That's not how it works. INTF (which feeds DSI) is after DSC compression. > > > > > > INTF timings are always in RGB888 (24-bit) units. Ignoring widebus details, > > > the INTF timing should match what is programmed on the DSI side (hdisplay, > > > which is calculated as bytes per line / 3). > > > > > > (fwiw, the current "timing->width = ..." calculation here blames to me, but > > > what I wrote originally was just "timing->width = timing->width / 3" with a > > > comment about being incomplete.) > > > > > Okay. After reading the docs (sorry, it took a while). > > > > - When widebus is not enabled, the transfer is always 24 bit of > > compressed data. Thus if it is not in play, pclk and timing->width > > should be scaled by source_pixel_depth / compression_ratio / 24. In > > case of the code it is 'drm_dsc_get_bpp_int(dsc) / 24'. > > > > For RGB101010 / 8bpp DSC this should result in the PCLK being lowered > > by the factor of 3 (= 24 / (30 / 3.75)) > > > > - When widebus is in play (MDSS 6.x+, DSI 2.4+), the transfer takes > > more than 24 bits. In this case the PCLK and timing->width should be > > scaled exactly by the DSC compression ratio, which is > > 'drm_dsc_get_bpp_int(dsc) / (3 * dsc->bits_per_component). > > > > So, this piece of code needs to be adjusted to check for the widebus > > being enabled or not. > > > > The widebus adjustment on the MDP/INTF side is already in > dpu_hw_intf_setup_timing_engine: the "data width" is divided by 2 for 48-bit > widebus instead of 24-bit. there shouldn't be any other adjustment > (downstream doesn't have any other adjustment). > > a relevant downstream comment: "In DATABUS-WIDEN mode, MDP always sends out > 48-bit compressed data per pclk and on average, DSI consumes an amount of > compressed data equivalent to the uncompressed pixel depth per pclk." > > Based on that comment, this patch is correct, and the > ''drm_dsc_get_bpp_int(dsc) / (3 * dsc->bits_per_component)' adjustment only > applies to DSI. (note: newer downstream looks like it would divide by 3.75 > here, which doesn't make sense. older downstream would divide by 3 here. I > guess downstream is broken now and video mode + 10-bit dsc doesn't get > tested?) > For the reference, the commit in question is [1]. > on DSI side, "uncompressed pixel depth" shouldn't matter either: DSI only > sees the compressed data. But based on that comment, when widebus is > enabled, by setting DSI_VID_CFG0_DST_FORMAT(?) to 30bpp, then the DSI pclk > is in 30-bit units instead of 24-bits. And with this series DSI side ends up > with the right result if 30bpp format and widebus is enabled. Nevertheless, this matches the docs that I'm looking at. [1] https://git.codelinaro.org/clo/la/platform/vendor/opensource/display-drivers/-/commit/7b4616f157e67e593fae13684237f96da351e877 -- With best wishes Dmitry
On Fri, Mar 20, 2026 at 12:25:00AM -0400, Jonathan Marek wrote: > On 3/19/26 9:45 PM, Dmitry Baryshkov wrote: > > On Thu, Mar 19, 2026 at 01:23:03PM -0400, Jonathan Marek wrote: > ... > > > > > > That's not how it works. INTF (which feeds DSI) is after DSC compression. > > > > > > INTF timings are always in RGB888 (24-bit) units. Ignoring widebus details, > > > the INTF timing should match what is programmed on the DSI side (hdisplay, > > > which is calculated as bytes per line / 3). > > > > > > (fwiw, the current "timing->width = ..." calculation here blames to me, but > > > what I wrote originally was just "timing->width = timing->width / 3" with a > > > comment about being incomplete.) > > > > > Okay. After reading the docs (sorry, it took a while). > > > > - When widebus is not enabled, the transfer is always 24 bit of > > compressed data. Thus if it is not in play, pclk and timing->width > > should be scaled by source_pixel_depth / compression_ratio / 24. In > > case of the code it is 'drm_dsc_get_bpp_int(dsc) / 24'. > > > > For RGB101010 / 8bpp DSC this should result in the PCLK being lowered > > by the factor of 3 (= 24 / (30 / 3.75)) > > > > - When widebus is in play (MDSS 6.x+, DSI 2.4+), the transfer takes > > more than 24 bits. In this case the PCLK and timing->width should be > > scaled exactly by the DSC compression ratio, which is > > 'drm_dsc_get_bpp_int(dsc) / (3 * dsc->bits_per_component). > > > > So, this piece of code needs to be adjusted to check for the widebus > > being enabled or not. > > > > The widebus adjustment on the MDP/INTF side is already in > dpu_hw_intf_setup_timing_engine: the "data width" is divided by 2 for 48-bit > widebus instead of 24-bit. there shouldn't be any other adjustment > (downstream doesn't have any other adjustment). > > a relevant downstream comment: "In DATABUS-WIDEN mode, MDP always sends out > 48-bit compressed data per pclk and on average, DSI consumes an amount of > compressed data equivalent to the uncompressed pixel depth per pclk." > > Based on that comment, this patch is correct, and the > ''drm_dsc_get_bpp_int(dsc) / (3 * dsc->bits_per_component)' adjustment only > applies to DSI. (note: newer downstream looks like it would divide by 3.75 > here, which doesn't make sense. older downstream would divide by 3 here. I > guess downstream is broken now and video mode + 10-bit dsc doesn't get > tested?) I guess, the downstream might be broken wrt. the widebus being enabled or not. > > on DSI side, "uncompressed pixel depth" shouldn't matter either: DSI only > sees the compressed data. But based on that comment, when widebus is > enabled, by setting DSI_VID_CFG0_DST_FORMAT(?) to 30bpp, then the DSI pclk > is in 30-bit units instead of 24-bits. And with this series DSI side ends up > with the right result if 30bpp format and widebus is enabled. -- With best wishes Dmitry
On Friday, March 20th, 2026 at 12:25 AM, Jonathan Marek <jonathan@marek.ca> wrote:
> On 3/19/26 9:45 PM, Dmitry Baryshkov wrote:
> > On Thu, Mar 19, 2026 at 01:23:03PM -0400, Jonathan Marek wrote:
> ...
> >>
> >> That's not how it works. INTF (which feeds DSI) is after DSC compression.
> >>
> >> INTF timings are always in RGB888 (24-bit) units. Ignoring widebus details,
> >> the INTF timing should match what is programmed on the DSI side (hdisplay,
> >> which is calculated as bytes per line / 3).
> >>
> >> (fwiw, the current "timing->width = ..." calculation here blames to me, but
> >> what I wrote originally was just "timing->width = timing->width / 3" with a
> >> comment about being incomplete.)
> >>
> > Okay. After reading the docs (sorry, it took a while).
> >
> > - When widebus is not enabled, the transfer is always 24 bit of
> > compressed data. Thus if it is not in play, pclk and timing->width
> > should be scaled by source_pixel_depth / compression_ratio / 24. In
> > case of the code it is 'drm_dsc_get_bpp_int(dsc) / 24'.
> >
> > For RGB101010 / 8bpp DSC this should result in the PCLK being lowered
> > by the factor of 3 (= 24 / (30 / 3.75))
> >
> > - When widebus is in play (MDSS 6.x+, DSI 2.4+), the transfer takes
> > more than 24 bits. In this case the PCLK and timing->width should be
> > scaled exactly by the DSC compression ratio, which is
> > 'drm_dsc_get_bpp_int(dsc) / (3 * dsc->bits_per_component).
> >
> > So, this piece of code needs to be adjusted to check for the widebus
> > being enabled or not.
> >
>
> The widebus adjustment on the MDP/INTF side is already in
> dpu_hw_intf_setup_timing_engine: the "data width" is divided by 2 for
> 48-bit widebus instead of 24-bit. there shouldn't be any other
> adjustment (downstream doesn't have any other adjustment).
>
> a relevant downstream comment: "In DATABUS-WIDEN mode, MDP always sends
> out 48-bit compressed data per pclk and on average, DSI consumes an
> amount of compressed data equivalent to the uncompressed pixel depth per
> pclk."
>
> Based on that comment, this patch is correct, and the
> ''drm_dsc_get_bpp_int(dsc) / (3 * dsc->bits_per_component)' adjustment
> only applies to DSI.
If I keep the INTF side at /24 and change the DSI side to:
if (wide_bus)
new_hdisplay = DIV_ROUND_UP(mode->hdisplay * drm_dsc_get_bpp_int(dsc), dsc->bits_per_component * 3);
else
new_hdisplay = DIV_ROUND_UP(mode->hdisplay * drm_dsc_get_bpp_int(dsc), 24);
This also works on my panel.
Should I send this in a v4 for this series or just leave it for a seperate
change as panel seems to work with /24 here anyways?
> (note: newer downstream looks like it would divide
> by 3.75 here, which doesn't make sense. older downstream would divide by
> 3 here. I guess downstream is broken now and video mode + 10-bit dsc
> doesn't get tested?)
>
> on DSI side, "uncompressed pixel depth" shouldn't matter either: DSI
> only sees the compressed data. But based on that comment, when widebus
> is enabled, by setting DSI_VID_CFG0_DST_FORMAT(?) to 30bpp, then the DSI
> pclk is in 30-bit units instead of 24-bits. And with this series DSI
> side ends up with the right result if 30bpp format and widebus is enabled.
>
>
Thanks,
Alex
On Fri, Mar 20, 2026 at 04:46:02AM +0000, Alexander Koskovich wrote: > On Friday, March 20th, 2026 at 12:25 AM, Jonathan Marek <jonathan@marek.ca> wrote: > > > On 3/19/26 9:45 PM, Dmitry Baryshkov wrote: > > > On Thu, Mar 19, 2026 at 01:23:03PM -0400, Jonathan Marek wrote: > > ... > > >> > > >> That's not how it works. INTF (which feeds DSI) is after DSC compression. > > >> > > >> INTF timings are always in RGB888 (24-bit) units. Ignoring widebus details, > > >> the INTF timing should match what is programmed on the DSI side (hdisplay, > > >> which is calculated as bytes per line / 3). > > >> > > >> (fwiw, the current "timing->width = ..." calculation here blames to me, but > > >> what I wrote originally was just "timing->width = timing->width / 3" with a > > >> comment about being incomplete.) > > >> > > > Okay. After reading the docs (sorry, it took a while). > > > > > > - When widebus is not enabled, the transfer is always 24 bit of > > > compressed data. Thus if it is not in play, pclk and timing->width > > > should be scaled by source_pixel_depth / compression_ratio / 24. In > > > case of the code it is 'drm_dsc_get_bpp_int(dsc) / 24'. > > > > > > For RGB101010 / 8bpp DSC this should result in the PCLK being lowered > > > by the factor of 3 (= 24 / (30 / 3.75)) > > > > > > - When widebus is in play (MDSS 6.x+, DSI 2.4+), the transfer takes > > > more than 24 bits. In this case the PCLK and timing->width should be > > > scaled exactly by the DSC compression ratio, which is > > > 'drm_dsc_get_bpp_int(dsc) / (3 * dsc->bits_per_component). > > > > > > So, this piece of code needs to be adjusted to check for the widebus > > > being enabled or not. > > > > > > > The widebus adjustment on the MDP/INTF side is already in > > dpu_hw_intf_setup_timing_engine: the "data width" is divided by 2 for > > 48-bit widebus instead of 24-bit. there shouldn't be any other > > adjustment (downstream doesn't have any other adjustment). > > > > a relevant downstream comment: "In DATABUS-WIDEN mode, MDP always sends > > out 48-bit compressed data per pclk and on average, DSI consumes an > > amount of compressed data equivalent to the uncompressed pixel depth per > > pclk." > > > > Based on that comment, this patch is correct, and the > > ''drm_dsc_get_bpp_int(dsc) / (3 * dsc->bits_per_component)' adjustment > > only applies to DSI. > > If I keep the INTF side at /24 and change the DSI side to: > > if (wide_bus) > new_hdisplay = DIV_ROUND_UP(mode->hdisplay * drm_dsc_get_bpp_int(dsc), dsc->bits_per_component * 3); > else > new_hdisplay = DIV_ROUND_UP(mode->hdisplay * drm_dsc_get_bpp_int(dsc), 24); Please check the actual fps (I'm usually using a vblank-synced GL, e.g. kmscube). At least this matches my expectations. > > This also works on my panel. > > Should I send this in a v4 for this series or just leave it for a seperate > change as panel seems to work with /24 here anyways? > > > (note: newer downstream looks like it would divide > > by 3.75 here, which doesn't make sense. older downstream would divide by > > 3 here. I guess downstream is broken now and video mode + 10-bit dsc > > doesn't get tested?) > > > > on DSI side, "uncompressed pixel depth" shouldn't matter either: DSI > > only sees the compressed data. But based on that comment, when widebus > > is enabled, by setting DSI_VID_CFG0_DST_FORMAT(?) to 30bpp, then the DSI > > pclk is in 30-bit units instead of 24-bits. And with this series DSI > > side ends up with the right result if 30bpp format and widebus is enabled. > > > > > > Thanks, > Alex -- With best wishes Dmitry
On Friday, March 20th, 2026 at 2:38 AM, Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> wrote: > On Fri, Mar 20, 2026 at 04:46:02AM +0000, Alexander Koskovich wrote: > > On Friday, March 20th, 2026 at 12:25 AM, Jonathan Marek <jonathan@marek.ca> wrote: > > > > > On 3/19/26 9:45 PM, Dmitry Baryshkov wrote: > > > > On Thu, Mar 19, 2026 at 01:23:03PM -0400, Jonathan Marek wrote: > > > ... > > > >> > > > >> That's not how it works. INTF (which feeds DSI) is after DSC compression. > > > >> > > > >> INTF timings are always in RGB888 (24-bit) units. Ignoring widebus details, > > > >> the INTF timing should match what is programmed on the DSI side (hdisplay, > > > >> which is calculated as bytes per line / 3). > > > >> > > > >> (fwiw, the current "timing->width = ..." calculation here blames to me, but > > > >> what I wrote originally was just "timing->width = timing->width / 3" with a > > > >> comment about being incomplete.) > > > >> > > > > Okay. After reading the docs (sorry, it took a while). > > > > > > > > - When widebus is not enabled, the transfer is always 24 bit of > > > > compressed data. Thus if it is not in play, pclk and timing->width > > > > should be scaled by source_pixel_depth / compression_ratio / 24. In > > > > case of the code it is 'drm_dsc_get_bpp_int(dsc) / 24'. > > > > > > > > For RGB101010 / 8bpp DSC this should result in the PCLK being lowered > > > > by the factor of 3 (= 24 / (30 / 3.75)) > > > > > > > > - When widebus is in play (MDSS 6.x+, DSI 2.4+), the transfer takes > > > > more than 24 bits. In this case the PCLK and timing->width should be > > > > scaled exactly by the DSC compression ratio, which is > > > > 'drm_dsc_get_bpp_int(dsc) / (3 * dsc->bits_per_component). > > > > > > > > So, this piece of code needs to be adjusted to check for the widebus > > > > being enabled or not. > > > > > > > > > > The widebus adjustment on the MDP/INTF side is already in > > > dpu_hw_intf_setup_timing_engine: the "data width" is divided by 2 for > > > 48-bit widebus instead of 24-bit. there shouldn't be any other > > > adjustment (downstream doesn't have any other adjustment). > > > > > > a relevant downstream comment: "In DATABUS-WIDEN mode, MDP always sends > > > out 48-bit compressed data per pclk and on average, DSI consumes an > > > amount of compressed data equivalent to the uncompressed pixel depth per > > > pclk." > > > > > > Based on that comment, this patch is correct, and the > > > ''drm_dsc_get_bpp_int(dsc) / (3 * dsc->bits_per_component)' adjustment > > > only applies to DSI. > > > > If I keep the INTF side at /24 and change the DSI side to: > > > > if (wide_bus) > > new_hdisplay = DIV_ROUND_UP(mode->hdisplay * drm_dsc_get_bpp_int(dsc), dsc->bits_per_component * 3); > > else > > new_hdisplay = DIV_ROUND_UP(mode->hdisplay * drm_dsc_get_bpp_int(dsc), 24); > > Please check the actual fps (I'm usually using a vblank-synced GL, e.g. > kmscube). > > At least this matches my expectations. Hmmm with kmscube I am getting 120FPS with 24 and 100FPS with 30. So I guess that's a no go > > > > > This also works on my panel. > > > > Should I send this in a v4 for this series or just leave it for a seperate > > change as panel seems to work with /24 here anyways? > > > > > (note: newer downstream looks like it would divide > > > by 3.75 here, which doesn't make sense. older downstream would divide by > > > 3 here. I guess downstream is broken now and video mode + 10-bit dsc > > > doesn't get tested?) > > > > > > on DSI side, "uncompressed pixel depth" shouldn't matter either: DSI > > > only sees the compressed data. But based on that comment, when widebus > > > is enabled, by setting DSI_VID_CFG0_DST_FORMAT(?) to 30bpp, then the DSI > > > pclk is in 30-bit units instead of 24-bits. And with this series DSI > > > side ends up with the right result if 30bpp format and widebus is enabled. > > > > > > > > > > Thanks, > > Alex > > -- > With best wishes > Dmitry >
On Friday, March 20th, 2026 at 2:50 AM, Alexander Koskovich <akoskovich@pm.me> wrote: > On Friday, March 20th, 2026 at 2:38 AM, Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> wrote: > > > On Fri, Mar 20, 2026 at 04:46:02AM +0000, Alexander Koskovich wrote: > > > On Friday, March 20th, 2026 at 12:25 AM, Jonathan Marek <jonathan@marek.ca> wrote: > > > > > > > On 3/19/26 9:45 PM, Dmitry Baryshkov wrote: > > > > > On Thu, Mar 19, 2026 at 01:23:03PM -0400, Jonathan Marek wrote: > > > > ... > > > > >> > > > > >> That's not how it works. INTF (which feeds DSI) is after DSC compression. > > > > >> > > > > >> INTF timings are always in RGB888 (24-bit) units. Ignoring widebus details, > > > > >> the INTF timing should match what is programmed on the DSI side (hdisplay, > > > > >> which is calculated as bytes per line / 3). > > > > >> > > > > >> (fwiw, the current "timing->width = ..." calculation here blames to me, but > > > > >> what I wrote originally was just "timing->width = timing->width / 3" with a > > > > >> comment about being incomplete.) > > > > >> > > > > > Okay. After reading the docs (sorry, it took a while). > > > > > > > > > > - When widebus is not enabled, the transfer is always 24 bit of > > > > > compressed data. Thus if it is not in play, pclk and timing->width > > > > > should be scaled by source_pixel_depth / compression_ratio / 24. In > > > > > case of the code it is 'drm_dsc_get_bpp_int(dsc) / 24'. > > > > > > > > > > For RGB101010 / 8bpp DSC this should result in the PCLK being lowered > > > > > by the factor of 3 (= 24 / (30 / 3.75)) > > > > > > > > > > - When widebus is in play (MDSS 6.x+, DSI 2.4+), the transfer takes > > > > > more than 24 bits. In this case the PCLK and timing->width should be > > > > > scaled exactly by the DSC compression ratio, which is > > > > > 'drm_dsc_get_bpp_int(dsc) / (3 * dsc->bits_per_component). > > > > > > > > > > So, this piece of code needs to be adjusted to check for the widebus > > > > > being enabled or not. > > > > > > > > > > > > > The widebus adjustment on the MDP/INTF side is already in > > > > dpu_hw_intf_setup_timing_engine: the "data width" is divided by 2 for > > > > 48-bit widebus instead of 24-bit. there shouldn't be any other > > > > adjustment (downstream doesn't have any other adjustment). > > > > > > > > a relevant downstream comment: "In DATABUS-WIDEN mode, MDP always sends > > > > out 48-bit compressed data per pclk and on average, DSI consumes an > > > > amount of compressed data equivalent to the uncompressed pixel depth per > > > > pclk." > > > > > > > > Based on that comment, this patch is correct, and the > > > > ''drm_dsc_get_bpp_int(dsc) / (3 * dsc->bits_per_component)' adjustment > > > > only applies to DSI. > > > > > > If I keep the INTF side at /24 and change the DSI side to: > > > > > > if (wide_bus) > > > new_hdisplay = DIV_ROUND_UP(mode->hdisplay * drm_dsc_get_bpp_int(dsc), dsc->bits_per_component * 3); > > > else > > > new_hdisplay = DIV_ROUND_UP(mode->hdisplay * drm_dsc_get_bpp_int(dsc), 24); > > > > Please check the actual fps (I'm usually using a vblank-synced GL, e.g. > > kmscube). > > > > At least this matches my expectations. > > Hmmm with kmscube I am getting 120FPS with 24 and 100FPS with 30. So I guess that's a no go Although it was using dsc->bits_per_component * 3 regardless before for dsi_adjust_pclk_for_compression so I guess that's what Jonathan was referring to earlier... > > > > > > > > > This also works on my panel. > > > > > > Should I send this in a v4 for this series or just leave it for a seperate > > > change as panel seems to work with /24 here anyways? > > > > > > > (note: newer downstream looks like it would divide > > > > by 3.75 here, which doesn't make sense. older downstream would divide by > > > > 3 here. I guess downstream is broken now and video mode + 10-bit dsc > > > > doesn't get tested?) > > > > > > > > on DSI side, "uncompressed pixel depth" shouldn't matter either: DSI > > > > only sees the compressed data. But based on that comment, when widebus > > > > is enabled, by setting DSI_VID_CFG0_DST_FORMAT(?) to 30bpp, then the DSI > > > > pclk is in 30-bit units instead of 24-bits. And with this series DSI > > > > side ends up with the right result if 30bpp format and widebus is enabled. > > > > > > > > > > > > > > Thanks, > > > Alex > > > > -- > > With best wishes > > Dmitry > > > >
On Fri, Mar 20, 2026 at 07:02:54AM +0000, Alexander Koskovich wrote: > On Friday, March 20th, 2026 at 2:50 AM, Alexander Koskovich <akoskovich@pm.me> wrote: > > > On Friday, March 20th, 2026 at 2:38 AM, Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> wrote: > > > > > On Fri, Mar 20, 2026 at 04:46:02AM +0000, Alexander Koskovich wrote: > > > > On Friday, March 20th, 2026 at 12:25 AM, Jonathan Marek <jonathan@marek.ca> wrote: > > > > > > > > > On 3/19/26 9:45 PM, Dmitry Baryshkov wrote: > > > > > > On Thu, Mar 19, 2026 at 01:23:03PM -0400, Jonathan Marek wrote: > > > > > ... > > > > > >> > > > > > >> That's not how it works. INTF (which feeds DSI) is after DSC compression. > > > > > >> > > > > > >> INTF timings are always in RGB888 (24-bit) units. Ignoring widebus details, > > > > > >> the INTF timing should match what is programmed on the DSI side (hdisplay, > > > > > >> which is calculated as bytes per line / 3). > > > > > >> > > > > > >> (fwiw, the current "timing->width = ..." calculation here blames to me, but > > > > > >> what I wrote originally was just "timing->width = timing->width / 3" with a > > > > > >> comment about being incomplete.) > > > > > >> > > > > > > Okay. After reading the docs (sorry, it took a while). > > > > > > > > > > > > - When widebus is not enabled, the transfer is always 24 bit of > > > > > > compressed data. Thus if it is not in play, pclk and timing->width > > > > > > should be scaled by source_pixel_depth / compression_ratio / 24. In > > > > > > case of the code it is 'drm_dsc_get_bpp_int(dsc) / 24'. > > > > > > > > > > > > For RGB101010 / 8bpp DSC this should result in the PCLK being lowered > > > > > > by the factor of 3 (= 24 / (30 / 3.75)) > > > > > > > > > > > > - When widebus is in play (MDSS 6.x+, DSI 2.4+), the transfer takes > > > > > > more than 24 bits. In this case the PCLK and timing->width should be > > > > > > scaled exactly by the DSC compression ratio, which is > > > > > > 'drm_dsc_get_bpp_int(dsc) / (3 * dsc->bits_per_component). > > > > > > > > > > > > So, this piece of code needs to be adjusted to check for the widebus > > > > > > being enabled or not. > > > > > > > > > > > > > > > > The widebus adjustment on the MDP/INTF side is already in > > > > > dpu_hw_intf_setup_timing_engine: the "data width" is divided by 2 for > > > > > 48-bit widebus instead of 24-bit. there shouldn't be any other > > > > > adjustment (downstream doesn't have any other adjustment). > > > > > > > > > > a relevant downstream comment: "In DATABUS-WIDEN mode, MDP always sends > > > > > out 48-bit compressed data per pclk and on average, DSI consumes an > > > > > amount of compressed data equivalent to the uncompressed pixel depth per > > > > > pclk." > > > > > > > > > > Based on that comment, this patch is correct, and the > > > > > ''drm_dsc_get_bpp_int(dsc) / (3 * dsc->bits_per_component)' adjustment > > > > > only applies to DSI. > > > > > > > > If I keep the INTF side at /24 and change the DSI side to: > > > > > > > > if (wide_bus) > > > > new_hdisplay = DIV_ROUND_UP(mode->hdisplay * drm_dsc_get_bpp_int(dsc), dsc->bits_per_component * 3); > > > > else > > > > new_hdisplay = DIV_ROUND_UP(mode->hdisplay * drm_dsc_get_bpp_int(dsc), 24); > > > > > > Please check the actual fps (I'm usually using a vblank-synced GL, e.g. > > > kmscube). > > > > > > At least this matches my expectations. > > > > Hmmm with kmscube I am getting 120FPS with 24 and 100FPS with 30. So I guess that's a no go > > Although it was using dsc->bits_per_component * 3 regardless before for > dsi_adjust_pclk_for_compression so I guess that's what Jonathan was > referring to earlier... Do you have any of the patches by Marijn or Pengyu? - https://lore.kernel.org/linux-arm-msm/20260311-dsi-dsc-regresses-again-v1-1-6a422141eeea@somainline.org/ - https://lore.kernel.org/linux-arm-msm/20260307111250.105772-1-mitltlatltl@gmail.com/ -- With best wishes Dmitry
On Friday, March 20th, 2026 at 3:36 AM, Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> wrote: > On Fri, Mar 20, 2026 at 07:02:54AM +0000, Alexander Koskovich wrote: > > On Friday, March 20th, 2026 at 2:50 AM, Alexander Koskovich <akoskovich@pm.me> wrote: > > > > > On Friday, March 20th, 2026 at 2:38 AM, Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> wrote: > > > > > > > On Fri, Mar 20, 2026 at 04:46:02AM +0000, Alexander Koskovich wrote: > > > > > On Friday, March 20th, 2026 at 12:25 AM, Jonathan Marek <jonathan@marek.ca> wrote: > > > > > > > > > > > On 3/19/26 9:45 PM, Dmitry Baryshkov wrote: > > > > > > > On Thu, Mar 19, 2026 at 01:23:03PM -0400, Jonathan Marek wrote: > > > > > > ... > > > > > > >> > > > > > > >> That's not how it works. INTF (which feeds DSI) is after DSC compression. > > > > > > >> > > > > > > >> INTF timings are always in RGB888 (24-bit) units. Ignoring widebus details, > > > > > > >> the INTF timing should match what is programmed on the DSI side (hdisplay, > > > > > > >> which is calculated as bytes per line / 3). > > > > > > >> > > > > > > >> (fwiw, the current "timing->width = ..." calculation here blames to me, but > > > > > > >> what I wrote originally was just "timing->width = timing->width / 3" with a > > > > > > >> comment about being incomplete.) > > > > > > >> > > > > > > > Okay. After reading the docs (sorry, it took a while). > > > > > > > > > > > > > > - When widebus is not enabled, the transfer is always 24 bit of > > > > > > > compressed data. Thus if it is not in play, pclk and timing->width > > > > > > > should be scaled by source_pixel_depth / compression_ratio / 24. In > > > > > > > case of the code it is 'drm_dsc_get_bpp_int(dsc) / 24'. > > > > > > > > > > > > > > For RGB101010 / 8bpp DSC this should result in the PCLK being lowered > > > > > > > by the factor of 3 (= 24 / (30 / 3.75)) > > > > > > > > > > > > > > - When widebus is in play (MDSS 6.x+, DSI 2.4+), the transfer takes > > > > > > > more than 24 bits. In this case the PCLK and timing->width should be > > > > > > > scaled exactly by the DSC compression ratio, which is > > > > > > > 'drm_dsc_get_bpp_int(dsc) / (3 * dsc->bits_per_component). > > > > > > > > > > > > > > So, this piece of code needs to be adjusted to check for the widebus > > > > > > > being enabled or not. > > > > > > > > > > > > > > > > > > > The widebus adjustment on the MDP/INTF side is already in > > > > > > dpu_hw_intf_setup_timing_engine: the "data width" is divided by 2 for > > > > > > 48-bit widebus instead of 24-bit. there shouldn't be any other > > > > > > adjustment (downstream doesn't have any other adjustment). > > > > > > > > > > > > a relevant downstream comment: "In DATABUS-WIDEN mode, MDP always sends > > > > > > out 48-bit compressed data per pclk and on average, DSI consumes an > > > > > > amount of compressed data equivalent to the uncompressed pixel depth per > > > > > > pclk." > > > > > > > > > > > > Based on that comment, this patch is correct, and the > > > > > > ''drm_dsc_get_bpp_int(dsc) / (3 * dsc->bits_per_component)' adjustment > > > > > > only applies to DSI. > > > > > > > > > > If I keep the INTF side at /24 and change the DSI side to: > > > > > > > > > > if (wide_bus) > > > > > new_hdisplay = DIV_ROUND_UP(mode->hdisplay * drm_dsc_get_bpp_int(dsc), dsc->bits_per_component * 3); > > > > > else > > > > > new_hdisplay = DIV_ROUND_UP(mode->hdisplay * drm_dsc_get_bpp_int(dsc), 24); > > > > > > > > Please check the actual fps (I'm usually using a vblank-synced GL, e.g. > > > > kmscube). > > > > > > > > At least this matches my expectations. > > > > > > Hmmm with kmscube I am getting 120FPS with 24 and 100FPS with 30. So I guess that's a no go > > > > Although it was using dsc->bits_per_component * 3 regardless before for > > dsi_adjust_pclk_for_compression so I guess that's what Jonathan was > > referring to earlier... > > Do you have any of the patches by Marijn or Pengyu? > > - https://lore.kernel.org/linux-arm-msm/20260311-dsi-dsc-regresses-again-v1-1-6a422141eeea@somainline.org/ > > - https://lore.kernel.org/linux-arm-msm/20260307111250.105772-1-mitltlatltl@gmail.com/ Nope, I can test with them though if you'd like > > -- > With best wishes > Dmitry >
On Friday, March 20th, 2026 at 3:48 AM, Alexander Koskovich <akoskovich@pm.me> wrote: > > On Friday, March 20th, 2026 at 3:36 AM, Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> wrote: > > > On Fri, Mar 20, 2026 at 07:02:54AM +0000, Alexander Koskovich wrote: > > > On Friday, March 20th, 2026 at 2:50 AM, Alexander Koskovich <akoskovich@pm.me> wrote: > > > > > > > On Friday, March 20th, 2026 at 2:38 AM, Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> wrote: > > > > > > > > > On Fri, Mar 20, 2026 at 04:46:02AM +0000, Alexander Koskovich wrote: > > > > > > On Friday, March 20th, 2026 at 12:25 AM, Jonathan Marek <jonathan@marek.ca> wrote: > > > > > > > > > > > > > On 3/19/26 9:45 PM, Dmitry Baryshkov wrote: > > > > > > > > On Thu, Mar 19, 2026 at 01:23:03PM -0400, Jonathan Marek wrote: > > > > > > > ... > > > > > > > >> > > > > > > > >> That's not how it works. INTF (which feeds DSI) is after DSC compression. > > > > > > > >> > > > > > > > >> INTF timings are always in RGB888 (24-bit) units. Ignoring widebus details, > > > > > > > >> the INTF timing should match what is programmed on the DSI side (hdisplay, > > > > > > > >> which is calculated as bytes per line / 3). > > > > > > > >> > > > > > > > >> (fwiw, the current "timing->width = ..." calculation here blames to me, but > > > > > > > >> what I wrote originally was just "timing->width = timing->width / 3" with a > > > > > > > >> comment about being incomplete.) > > > > > > > >> > > > > > > > > Okay. After reading the docs (sorry, it took a while). > > > > > > > > > > > > > > > > - When widebus is not enabled, the transfer is always 24 bit of > > > > > > > > compressed data. Thus if it is not in play, pclk and timing->width > > > > > > > > should be scaled by source_pixel_depth / compression_ratio / 24. In > > > > > > > > case of the code it is 'drm_dsc_get_bpp_int(dsc) / 24'. > > > > > > > > > > > > > > > > For RGB101010 / 8bpp DSC this should result in the PCLK being lowered > > > > > > > > by the factor of 3 (= 24 / (30 / 3.75)) > > > > > > > > > > > > > > > > - When widebus is in play (MDSS 6.x+, DSI 2.4+), the transfer takes > > > > > > > > more than 24 bits. In this case the PCLK and timing->width should be > > > > > > > > scaled exactly by the DSC compression ratio, which is > > > > > > > > 'drm_dsc_get_bpp_int(dsc) / (3 * dsc->bits_per_component). > > > > > > > > > > > > > > > > So, this piece of code needs to be adjusted to check for the widebus > > > > > > > > being enabled or not. > > > > > > > > > > > > > > > > > > > > > > The widebus adjustment on the MDP/INTF side is already in > > > > > > > dpu_hw_intf_setup_timing_engine: the "data width" is divided by 2 for > > > > > > > 48-bit widebus instead of 24-bit. there shouldn't be any other > > > > > > > adjustment (downstream doesn't have any other adjustment). > > > > > > > > > > > > > > a relevant downstream comment: "In DATABUS-WIDEN mode, MDP always sends > > > > > > > out 48-bit compressed data per pclk and on average, DSI consumes an > > > > > > > amount of compressed data equivalent to the uncompressed pixel depth per > > > > > > > pclk." > > > > > > > > > > > > > > Based on that comment, this patch is correct, and the > > > > > > > ''drm_dsc_get_bpp_int(dsc) / (3 * dsc->bits_per_component)' adjustment > > > > > > > only applies to DSI. > > > > > > > > > > > > If I keep the INTF side at /24 and change the DSI side to: > > > > > > > > > > > > if (wide_bus) > > > > > > new_hdisplay = DIV_ROUND_UP(mode->hdisplay * drm_dsc_get_bpp_int(dsc), dsc->bits_per_component * 3); > > > > > > else > > > > > > new_hdisplay = DIV_ROUND_UP(mode->hdisplay * drm_dsc_get_bpp_int(dsc), 24); > > > > > > > > > > Please check the actual fps (I'm usually using a vblank-synced GL, e.g. > > > > > kmscube). > > > > > > > > > > At least this matches my expectations. > > > > > > > > Hmmm with kmscube I am getting 120FPS with 24 and 100FPS with 30. So I guess that's a no go > > > > > > Although it was using dsc->bits_per_component * 3 regardless before for > > > dsi_adjust_pclk_for_compression so I guess that's what Jonathan was > > > referring to earlier... > > > > Do you have any of the patches by Marijn or Pengyu? > > > > - https://lore.kernel.org/linux-arm-msm/20260311-dsi-dsc-regresses-again-v1-1-6a422141eeea@somainline.org/ > > > > - https://lore.kernel.org/linux-arm-msm/20260307111250.105772-1-mitltlatltl@gmail.com/ > > Nope, I can test with them though if you'd like Tested the following 3 patches on top of this series: drm/msm/dsi: fix hdisplay calculation when programming dsi registers drm/msm/dsi: fix bits_per_pclk drm/msm/dsi: fix hdisplay calculation for CMD mode panel Getting constant FIFO errors with them applied: [ 64.851635] dsi_err_worker: status=4 [ 64.851692] dsi_err_worker: status=4 [ 64.851730] dsi_err_worker: status=4 [ 64.851766] dsi_err_worker: status=4 [ 64.851802] dsi_err_worker: status=4 [ 64.851837] dsi_err_worker: status=4 [ 64.851903] dsi_err_worker: status=4 [ 64.851940] dsi_err_worker: status=4 [ 64.851976] dsi_err_worker: status=4 [ 64.852011] dsi_err_worker: status=4 > > > > > -- > > With best wishes > > Dmitry > > > >
On Thursday, March 19th, 2026 at 9:45 PM, Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> wrote:
> On Thu, Mar 19, 2026 at 01:23:03PM -0400, Jonathan Marek wrote:
> > On 3/19/26 10:54 AM, Neil Armstrong wrote:
> > > On 3/19/26 15:40, Alexander Koskovich wrote:
> > > > On Thursday, March 19th, 2026 at 10:13 AM, Neil Armstrong
> > > > <neil.armstrong@linaro.org> 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.
> > > > >
> >
> > DSI widebus is implemented, and always used when available. The adjustment
> > for DSI widebus just doesn't happen in this function.
> >
> > > > > >
> > > > > > Signed-off-by: Alexander Koskovich <akoskovich@pm.me>
> > > > > > ---
> > > > > > 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.
> > > >
> > > > Can you clarify what the 3 represents? My panel should have a 3.75:1
> > > > compression
> > > > ratio (30/8) so the final divisor of 24 would be wrong for my panel
> > > > if its the
> > > > compression ratio?
> > >
> > > So my guess is that while the exact ratio on the DSI lanes is 3.75:1,
> > > the ratio
> > > used to calculate the INTF timings is 3, then the DSC encoder probably
> > > does the
> > > magic to feed 10bpp into a 3.75:1 ratio over the DSI lanes.
> > >
> >
> > That's not how it works. INTF (which feeds DSI) is after DSC compression.
> >
> > INTF timings are always in RGB888 (24-bit) units. Ignoring widebus details,
> > the INTF timing should match what is programmed on the DSI side (hdisplay,
> > which is calculated as bytes per line / 3).
> >
> > (fwiw, the current "timing->width = ..." calculation here blames to me, but
> > what I wrote originally was just "timing->width = timing->width / 3" with a
> > comment about being incomplete.)
> >
> Okay. After reading the docs (sorry, it took a while).
>
> - When widebus is not enabled, the transfer is always 24 bit of
> compressed data. Thus if it is not in play, pclk and timing->width
> should be scaled by source_pixel_depth / compression_ratio / 24. In
> case of the code it is 'drm_dsc_get_bpp_int(dsc) / 24'.
>
> For RGB101010 / 8bpp DSC this should result in the PCLK being lowered
> by the factor of 3 (= 24 / (30 / 3.75))
>
> - When widebus is in play (MDSS 6.x+, DSI 2.4+), the transfer takes
> more than 24 bits. In this case the PCLK and timing->width should be
> scaled exactly by the DSC compression ratio, which is
> 'drm_dsc_get_bpp_int(dsc) / (3 * dsc->bits_per_component).
>
> So, this piece of code needs to be adjusted to check for the widebus
> being enabled or not.
>
Modified drm_mode_to_intf_timing_params & dsi_adjust_pclk_for_compression to account for widebus, but the hdisplay I get is different from stock with widebus factored in. Getting 288 instead of 360 now which produces constant FIFO errors.
This is with widebus enabled, using 3 * dsc->bits_per_component. Should it be 24 regardless of widebus?
> > > In dsi_adjust_pclk_for_compression, the pclk is adjusted to take in
> > > account bits_per_component,
> > > so I presume the actual DSI pclk _is_ timing->width *
> > > drm_dsc_get_bpp_int(dsc) / (dsc->bits_per_component * 3),
> > > which is your 3.75:1, but the INTF needs to generate timing->width *
> > > drm_dsc_get_bpp_int(dsc) / (8 * 3) pixels
> > > to the DSC encoder which will emit timing->width *
> > > drm_dsc_get_bpp_int(dsc) / (dsc->bits_per_component * 3) pixels.
> > >
> >
> > The hdisplay calculation in dsi_adjust_pclk_for_compression (which only
> > affects the clock rate) seems to be wrong, and I think Alexander's panel
> > must be running at a 20% lower clock because of it. dsi_timing_setup has the
> > right hdisplay adjustment.
>
> That function also needs to be adjusted accordingly. I think only the
> dsi_timing_setup() is correct at this point. Note, widebus / not-widebus
> cases should be handled separately.
>
> > > In any case, 24 _is_ 3 * 8, 3 being the DSC compression ratio on the
> > > INTF side.
>
> In this case DSC compression ratio is 3.75, so it's not true.
>
> --
> With best wishes
> Dmitry
>
Thanks,
Alex
On Thursday, March 19th, 2026 at 1:23 PM, Jonathan Marek <jonathan@marek.ca> wrote:
> On 3/19/26 10:54 AM, Neil Armstrong wrote:
> > On 3/19/26 15:40, Alexander Koskovich wrote:
> >> On Thursday, March 19th, 2026 at 10:13 AM, Neil Armstrong
> >> <neil.armstrong@linaro.org> 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.
> >>>
>
> DSI widebus is implemented, and always used when available. The
> adjustment for DSI widebus just doesn't happen in this function.
>
> >>>>
> >>>> Signed-off-by: Alexander Koskovich <akoskovich@pm.me>
> >>>> ---
> >>>> 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.
> >>
> >> Can you clarify what the 3 represents? My panel should have a 3.75:1
> >> compression
> >> ratio (30/8) so the final divisor of 24 would be wrong for my panel if
> >> its the
> >> compression ratio?
> >
> > So my guess is that while the exact ratio on the DSI lanes is 3.75:1,
> > the ratio
> > used to calculate the INTF timings is 3, then the DSC encoder probably
> > does the
> > magic to feed 10bpp into a 3.75:1 ratio over the DSI lanes.
> >
>
> That's not how it works. INTF (which feeds DSI) is after DSC compression.
>
> INTF timings are always in RGB888 (24-bit) units. Ignoring widebus
> details, the INTF timing should match what is programmed on the DSI side
> (hdisplay, which is calculated as bytes per line / 3).
>
> (fwiw, the current "timing->width = ..." calculation here blames to me,
> but what I wrote originally was just "timing->width = timing->width / 3"
> with a comment about being incomplete.)
>
> > In dsi_adjust_pclk_for_compression, the pclk is adjusted to take in
> > account bits_per_component,
> > so I presume the actual DSI pclk _is_ timing->width *
> > drm_dsc_get_bpp_int(dsc) / (dsc->bits_per_component * 3),
> > which is your 3.75:1, but the INTF needs to generate timing->width *
> > drm_dsc_get_bpp_int(dsc) / (8 * 3) pixels
> > to the DSC encoder which will emit timing->width *
> > drm_dsc_get_bpp_int(dsc) / (dsc->bits_per_component * 3) pixels.
> >
>
> The hdisplay calculation in dsi_adjust_pclk_for_compression (which only
> affects the clock rate) seems to be wrong, and I think Alexander's panel
> must be running at a 20% lower clock because of it. dsi_timing_setup has
> the right hdisplay adjustment.
Checked against downstream and the clocks seem to match more or less:
downstream:
pclk: 110070156
byte: 103190771
upstream:
pclk: 110073457
byte: 103193865
>
> > In any case, 24 _is_ 3 * 8, 3 being the DSC compression ratio on the
> > INTF side.
> >
> > Dmitry, Konrad, could you help confirming this ?
> >
> > Neil
> >
> >>
> >>>
> >>>> timing->xres = timing->width;
> >>>> }
> >>>> }
> >>>>
> >>>
> >>>
> >>>
> >>
> >> Thanks,
> >> Alex
> >
>
On 3/19/26 1:31 PM, Alexander Koskovich wrote: > On Thursday, March 19th, 2026 at 1:23 PM, Jonathan Marek <jonathan@marek.ca> wrote: > ...>> >> The hdisplay calculation in dsi_adjust_pclk_for_compression (which only >> affects the clock rate) seems to be wrong, and I think Alexander's panel >> must be running at a 20% lower clock because of it. dsi_timing_setup has >> the right hdisplay adjustment. > > Checked against downstream and the clocks seem to match more or less: > > downstream: > pclk: 110070156 > byte: 103190771 > > upstream: > pclk: 110073457 > byte: 103193865 > I was curious about this and looked into it a bit (without testing any HW): - using MIPI_DSI_FMT_RGB101010 dsi_byte_clk_get_rate cancels the effect of adjusting with bits_per_component for the byte clk, so the byte clock ends up being right - using DST_FORMAT_RGB101010 DSI pclk is in 30-bit units instead of 24-bit units, so the pclk ends up being right too (but that only works if widebus is enabled) a recent commit (ac47870fd795) changed the hdisplay calculation in dsi_timing_setup to match that in dsi_adjust_pclk_for_compression, but only when widebus is enabled. So things work out if widebus is enabled and MIPI_DSI_FMT_RGB101010 is used (note: looks like the only upstream 10-bit panel uses MIPI_DSI_FMT_RGB888), otherwise its broken. AFAICT if you revert ac47870fd795, use MIPI_DSI_FMT_RGB888, and change dsi_adjust_pclk_for_compression to divide by 24 instead of 30, then it should also work (and won't be dependent on widebus).
© 2016 - 2026 Red Hat, Inc.