public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Marijn Suijten <marijn.suijten@somainline.org>
To: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Cc: Rob Clark <robin.clark@oss.qualcomm.com>,
	 Dmitry Baryshkov <lumag@kernel.org>,
	Abhinav Kumar <abhinav.kumar@linux.dev>,
	 Jessica Zhang <jesszhan0024@gmail.com>,
	Sean Paul <sean@poorly.run>, David Airlie <airlied@gmail.com>,
	 Simona Vetter <simona@ffwll.ch>,
	Pengyu Luo <mitltlatltl@gmail.com>,
	 ~postmarketos/upstreaming@lists.sr.ht,
	Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>,
	 Martin Botka <martin.botka@somainline.org>,
	Jami Kettunen <jami.kettunen@somainline.org>,
	 Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>,
	linux-arm-msm@vger.kernel.org, dri-devel@lists.freedesktop.org,
	 freedreno@lists.freedesktop.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] drm/msm/dsi: Restore widebus calculation for CMDMode panels
Date: Fri, 13 Mar 2026 00:08:15 +0100	[thread overview]
Message-ID: <abNCphH1cGeora16@SoMainline.org> (raw)
In-Reply-To: <f88c030f-ebdb-49d1-8334-62f1f85154e7@collabora.com>

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	other threads:[~2026-03-12 23:33 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-11 22:31 [PATCH] drm/msm/dsi: Restore widebus calculation for CMDMode panels Marijn Suijten
2026-03-12  5:10 ` Pengyu Luo
2026-03-20  1:59   ` Dmitry Baryshkov
2026-03-20 11:26     ` Pengyu Luo
2026-03-20 13:06       ` Dmitry Baryshkov
2026-03-20 13:15         ` Pengyu Luo
2026-03-12 13:18 ` AngeloGioacchino Del Regno
2026-03-12 23:08   ` Marijn Suijten [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=abNCphH1cGeora16@SoMainline.org \
    --to=marijn.suijten@somainline.org \
    --cc=abhinav.kumar@linux.dev \
    --cc=airlied@gmail.com \
    --cc=angelogioacchino.delregno@collabora.com \
    --cc=dmitry.baryshkov@oss.qualcomm.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=freedreno@lists.freedesktop.org \
    --cc=jami.kettunen@somainline.org \
    --cc=jesszhan0024@gmail.com \
    --cc=konrad.dybcio@oss.qualcomm.com \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lumag@kernel.org \
    --cc=martin.botka@somainline.org \
    --cc=mitltlatltl@gmail.com \
    --cc=robin.clark@oss.qualcomm.com \
    --cc=sean@poorly.run \
    --cc=simona@ffwll.ch \
    --cc=~postmarketos/upstreaming@lists.sr.ht \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox