On 2026-03-12 14:18:43, AngeloGioacchino Del Regno wrote:
...
> Marijn et al,
> 
> I don't really know this hardware specifically, but I had a very fast look
> at this patch, and I believe that however you put it, this looks like being
> either plain wrong or very weird.

I would say I also don't know the hardware here very well, and which parameters
apply when and were.  Most importantly, just be aware that the code we're
looking at is for **DSC**, i.e. when we're no longer transfering single pixels
per pclk, but compressed slices of a given number of bytes that can loosely be
converted back to a "number of pixels".

> I think this should be, instead....
> 
>       /* (don't add this comment) assuming RGB888/666, this will be 24 for 
> both Command 
> and Video modes */
>       bits_per_pclk = mipi_dsi_pixel_format_to_bpp(msm_host->format);

That would make sense, if it weren't for DSC.  The older downstream kernel I'm
looking at -for at least CMD mode- disregards the format entirely, and just
divides the "width" (computed like msm_dsc_get_bytes_per_line()) by 3 or 6
depending on widebus.

In other words that's clearly demonstrating that input/output "bits per
component" (in uncompressed space) are irrelevant when we're transmitting
compressed pixels.

>       /* Can send twice the bits per clock if WideBus with Video Mode */
>       if (wide_bus_enabled && msm_host->mode_flags & MIPI_DSI_MODE_VIDEO)
>               bits_per_pclk *= 2;

And this is where the original patch got things wrong too; widebus isn't limited
to VIDEO mode, in fact we were specifically only allowing CMD mode to use it
thus far.  That comment and condition should be inverted.

In this old downstream, widebus also doesn't seem to affect the pclk rate for
VIDEO mode, only the ratio between bits per component and the chroma format
(making up the total size of the pixel) and the configured "bits per pixel"
value are taken into account; that ratio is generally 3.

> ...because, unless there's a hardware quirk (and that should be really 
> clarified
> with a big comment stating that), I don't see why command mode should always 
> be
> 24 bits per clock, and I also don't see why a widebus case would do 48 bits 
> per
> clock even in the RGB666_PACKED and RGB565 cases (which may not even be 
> possible
> but *meh*).
> 
> Just my two cents.
> 
> Reminding you all again that I don't know this HW, so I may have said 
> something
> wrong here.
>
> Cheers!
> Angelo

Kind regards,
Marijn

Reply via email to