On 16/06/2023 15:25, Marijn Suijten wrote:
On 2023-06-16 12:41:52, Dmitry Baryshkov wrote:
Provide actual documentation for the pclk and hdisplay calculations in
the case of DSC compression being used.
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
---
drivers/gpu/drm/msm/dsi/dsi_host.c | 35 ++++++++++++++++++++++++++++--
1 file changed, 33 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
index 3f6dfb4f9d5a..72c377c9c7be 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -528,6 +528,21 @@ void dsi_link_clk_disable_v2(struct msm_dsi_host *msm_host)
clk_disable_unprepare(msm_host->byte_clk);
}
+/*
+ * Adjust the pclk rate by calculating a new hdisplay proportional to
Make this a kerneldoc with:
Ack
/**
* dsi_adjust_pclk_for_compression() - Adjust ...
+ * the compression ratio such that:
+ * new_hdisplay = old_hdisplay * compressed_bpp / uncompressed_bpp
+ *
+ * Porches do not need to be adjusted:
+ * - For the VIDEO mode they are not compressed by DSC and are passed as is.
as-is
Cambridge dictionary gives this "as is", without dash.
Though this was never tested nor confirmed by QUIC, but we can assume it
is the case for now?
+ * - For the CMD mode the are no actual porches. Instead they represent the
the are no -> these are not
they currently* represent.
Ack
Let's make sure that folks read the FIXME
below by perhaps rewording it right into this entry?
I kept it separately, so that the FIXME can be removed once CMD handling
is reworked.
+ * overhead to the image data transfer. As such, they are calculated for the
+ * final mode parameters (after the compression) and are not to be adjusted
+ * too.
+ *
+ * FIXME: Reconsider this if/when CMD mode handling is rewritten to use
+ * refresh rate and data overhead as a starting point of the calculations.
+ */
static unsigned long dsi_adjust_pclk_for_compression(const struct drm_display_mode *mode,
const struct drm_dsc_config *dsc)
{
@@ -926,8 +941,24 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi)
if (ret)
return;
- /* Divide the display by 3 but keep back/font porch and
- * pulse width same
+ /*
+ * DPU sends 3 bytes per pclk cycle to DSI. If compression is
+ * not used, a single pixel is transferred at each pulse, no
+ * matter what bpp or pixel format is used. In case of DSC
+ * compression this results (due to data alignment
+ * requirements) in a transfer of 3 compressed pixel per pclk
3 compressed bytes*, not pixels.
No, that's the point. With 6bpp one can think that 4 pixels would fit,
but they don't.
+ * cycle.
+ *
+ * If widebus is enabled, bus width is extended to 6 bytes.
+ * This way the DPU can transfer 6 compressed pixels with bpp
pixels -> bytes?
Same comment, no.
+ * less or equal to 8 or 3 compressed pyxels in case bpp is
pixels*... bytes?
And I will ask this **again**: does this mean we can halve pclk?
My guess would be no, since all other data transfers are not scaled by
wide bus.
+ * greater than 8.
+ *
+ * The back/font porch and pulse width are kept intact. They
+ * represent timing parameters rather than actual data
+ * transfer.
See FIXME above on dsi_adjust_pclk_for_compression()?
Thanks so much for finally putting some of this to paper.
- Marijn
+ *
+ * XXX: widebus is not supported by the driver (yet).
*/
h_total -= hdisplay;
hdisplay = DIV_ROUND_UP(msm_dsc_get_bytes_per_line(msm_host->dsc), 3);
--
2.39.2
--
With best wishes
Dmitry
