All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marijn Suijten <marijn.suijten@somainline.org>
To: phone-devel@vger.kernel.org, Rob Clark <robdclark@gmail.com>,
	Dmitry Baryshkov <dmitry.baryshkov@linaro.org>,
	Vinod Koul <vkoul@kernel.org>
Cc: ~postmarketos/upstreaming@lists.sr.ht,
	AngeloGioacchino Del Regno 
	<angelogioacchino.delregno@somainline.org>,
	Konrad Dybcio <konrad.dybcio@somainline.org>,
	Martin Botka <martin.botka@somainline.org>,
	Jami Kettunen <jami.kettunen@somainline.org>,
	Marijn Suijten <marijn.suijten@somainline.org>,
	David Airlie <airlied@linux.ie>, Daniel Vetter <daniel@ffwll.ch>,
	Abhinav Kumar <quic_abhinavk@quicinc.com>,
	Sean Paul <sean@poorly.run>,
	Thomas Zimmermann <tzimmermann@suse.de>,
	Javier Martinez Canillas <javierm@redhat.com>,
	Alex Deucher <alexander.deucher@amd.com>,
	Douglas Anderson <dianders@chromium.org>,
	Vladimir Lypak <vladimir.lypak@gmail.com>,
	dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
	linux-arm-msm@vger.kernel.org, freedreno@lists.freedesktop.org,
	Marek Vasut <marex@denx.de>
Subject: [PATCH 3/5] drm/msm/dsi: Account for DSC's bits_per_pixel having 4 fractional bits
Date: Sat,  1 Oct 2022 21:08:05 +0200	[thread overview]
Message-ID: <20221001190807.358691-4-marijn.suijten@somainline.org> (raw)
In-Reply-To: <20221001190807.358691-1-marijn.suijten@somainline.org>

drm_dsc_config's bits_per_pixel field holds a fractional value with 4
bits, which all panel drivers should adhere to for
drm_dsc_pps_payload_pack() to generate a valid payload.  All code in the
DSI driver here seems to assume that this field doesn't contain any
fractional bits, hence resulting in the wrong values being computed.
Since none of the calculations leave any room for fractional bits or
seem to indicate any possible area of support, disallow such values
altogether.

Fixes: b9080324d6ca ("drm/msm/dsi: add support for dsc data")
Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
---
 drivers/gpu/drm/msm/dsi/dsi_host.c | 34 +++++++++++++++++++++++-------
 1 file changed, 26 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
index cb6f2fa11f58..42a5c9776f52 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -847,6 +847,11 @@ static void dsi_update_dsc_timing(struct msm_dsi_host *msm_host, bool is_cmd_mod
 	u32 pkt_per_line;
 	u32 bytes_in_slice;
 	u32 eol_byte_num;
+	int bpp = dsc->bits_per_pixel >> 4;
+
+	if (dsc->bits_per_pixel & 0xf)
+		/* dsi_populate_dsc_params() already caught this case */
+		pr_err("DSI does not support fractional bits_per_pixel\n");
 
 	/* first calculate dsc parameters and then program
 	 * compress mode registers
@@ -860,7 +865,7 @@ static void dsi_update_dsc_timing(struct msm_dsi_host *msm_host, bool is_cmd_mod
 	if (slice_per_intf > dsc->slice_count)
 		dsc->slice_count = 1;
 
-	bytes_in_slice = DIV_ROUND_UP(dsc->slice_width * dsc->bits_per_pixel, 8);
+	bytes_in_slice = DIV_ROUND_UP(dsc->slice_width * bpp, 8);
 
 	dsc->slice_chunk_size = bytes_in_slice;
 
@@ -913,6 +918,7 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi)
 	u32 va_end = va_start + mode->vdisplay;
 	u32 hdisplay = mode->hdisplay;
 	u32 wc;
+	int ret;
 
 	DBG("");
 
@@ -948,7 +954,9 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi)
 		/* we do the calculations for dsc parameters here so that
 		 * panel can use these parameters
 		 */
-		dsi_populate_dsc_params(dsc);
+		ret = dsi_populate_dsc_params(dsc);
+		if (ret)
+			return;
 
 		/* Divide the display by 3 but keep back/font porch and
 		 * pulse width same
@@ -1229,6 +1237,10 @@ static int dsi_cmd_dma_add(struct msm_dsi_host *msm_host,
 	if (packet.size < len)
 		memset(data + packet.size, 0xff, len - packet.size);
 
+	if (msg->type == MIPI_DSI_PICTURE_PARAMETER_SET)
+		print_hex_dump(KERN_DEBUG, "ALL:", DUMP_PREFIX_NONE,
+				16, 1, data, len, false);
+
 	if (cfg_hnd->ops->tx_buf_put)
 		cfg_hnd->ops->tx_buf_put(msm_host);
 
@@ -1786,6 +1798,12 @@ static int dsi_populate_dsc_params(struct drm_dsc_config *dsc)
 	int data;
 	int final_value, final_scale;
 	int i;
+	int bpp = dsc->bits_per_pixel >> 4;
+
+	if (dsc->bits_per_pixel & 0xf) {
+		pr_err("DSI does not support fractional bits_per_pixel\n");
+		return -EINVAL;
+	}
 
 	dsc->rc_model_size = 8192;
 	dsc->first_line_bpg_offset = 12;
@@ -1807,7 +1825,7 @@ static int dsi_populate_dsc_params(struct drm_dsc_config *dsc)
 	}
 
 	dsc->initial_offset = 6144; /* Not bpp 12 */
-	if (dsc->bits_per_pixel != 8)
+	if (bpp != 8)
 		dsc->initial_offset = 2048;	/* bpp = 12 */
 
 	mux_words_size = 48;		/* bpc == 8/10 */
@@ -1830,16 +1848,16 @@ static int dsi_populate_dsc_params(struct drm_dsc_config *dsc)
 	 * params are calculated
 	 */
 	groups_per_line = DIV_ROUND_UP(dsc->slice_width, 3);
-	dsc->slice_chunk_size = dsc->slice_width * dsc->bits_per_pixel / 8;
-	if ((dsc->slice_width * dsc->bits_per_pixel) % 8)
+	dsc->slice_chunk_size = dsc->slice_width * bpp / 8;
+	if ((dsc->slice_width * bpp) % 8)
 		dsc->slice_chunk_size++;
 
 	/* rbs-min */
 	min_rate_buffer_size =  dsc->rc_model_size - dsc->initial_offset +
-				dsc->initial_xmit_delay * dsc->bits_per_pixel +
+				dsc->initial_xmit_delay * bpp +
 				groups_per_line * dsc->first_line_bpg_offset;
 
-	hrd_delay = DIV_ROUND_UP(min_rate_buffer_size, dsc->bits_per_pixel);
+	hrd_delay = DIV_ROUND_UP(min_rate_buffer_size, bpp);
 
 	dsc->initial_dec_delay = hrd_delay - dsc->initial_xmit_delay;
 
@@ -1862,7 +1880,7 @@ static int dsi_populate_dsc_params(struct drm_dsc_config *dsc)
 	data = 2048 * (dsc->rc_model_size - dsc->initial_offset + num_extra_mux_bits);
 	dsc->slice_bpg_offset = DIV_ROUND_UP(data, groups_total);
 
-	target_bpp_x16 = dsc->bits_per_pixel * 16;
+	target_bpp_x16 = bpp * 16;
 
 	data = (dsc->initial_xmit_delay * target_bpp_x16) / 16;
 	final_value =  dsc->rc_model_size - data + num_extra_mux_bits;
-- 
2.37.3


WARNING: multiple messages have this Message-ID (diff)
From: Marijn Suijten <marijn.suijten@somainline.org>
To: phone-devel@vger.kernel.org, Rob Clark <robdclark@gmail.com>,
	Dmitry Baryshkov <dmitry.baryshkov@linaro.org>,
	Vinod Koul <vkoul@kernel.org>
Cc: Marek Vasut <marex@denx.de>,
	freedreno@lists.freedesktop.org,
	Douglas Anderson <dianders@chromium.org>,
	Thomas Zimmermann <tzimmermann@suse.de>,
	Jami Kettunen <jami.kettunen@somainline.org>,
	Vladimir Lypak <vladimir.lypak@gmail.com>,
	linux-arm-msm@vger.kernel.org,
	Konrad Dybcio <konrad.dybcio@somainline.org>,
	Abhinav Kumar <quic_abhinavk@quicinc.com>,
	dri-devel@lists.freedesktop.org,
	Javier Martinez Canillas <javierm@redhat.com>,
	David Airlie <airlied@linux.ie>,
	Martin Botka <martin.botka@somainline.org>,
	~postmarketos/upstreaming@lists.sr.ht,
	AngeloGioacchino Del Regno
	<angelogioacchino.delregno@somainline.org>,
	Alex Deucher <alexander.deucher@amd.com>,
	Marijn Suijten <marijn.suijten@somainline.org>,
	Sean Paul <sean@poorly.run>,
	linux-kernel@vger.kernel.org
Subject: [PATCH 3/5] drm/msm/dsi: Account for DSC's bits_per_pixel having 4 fractional bits
Date: Sat,  1 Oct 2022 21:08:05 +0200	[thread overview]
Message-ID: <20221001190807.358691-4-marijn.suijten@somainline.org> (raw)
In-Reply-To: <20221001190807.358691-1-marijn.suijten@somainline.org>

drm_dsc_config's bits_per_pixel field holds a fractional value with 4
bits, which all panel drivers should adhere to for
drm_dsc_pps_payload_pack() to generate a valid payload.  All code in the
DSI driver here seems to assume that this field doesn't contain any
fractional bits, hence resulting in the wrong values being computed.
Since none of the calculations leave any room for fractional bits or
seem to indicate any possible area of support, disallow such values
altogether.

Fixes: b9080324d6ca ("drm/msm/dsi: add support for dsc data")
Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
---
 drivers/gpu/drm/msm/dsi/dsi_host.c | 34 +++++++++++++++++++++++-------
 1 file changed, 26 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
index cb6f2fa11f58..42a5c9776f52 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -847,6 +847,11 @@ static void dsi_update_dsc_timing(struct msm_dsi_host *msm_host, bool is_cmd_mod
 	u32 pkt_per_line;
 	u32 bytes_in_slice;
 	u32 eol_byte_num;
+	int bpp = dsc->bits_per_pixel >> 4;
+
+	if (dsc->bits_per_pixel & 0xf)
+		/* dsi_populate_dsc_params() already caught this case */
+		pr_err("DSI does not support fractional bits_per_pixel\n");
 
 	/* first calculate dsc parameters and then program
 	 * compress mode registers
@@ -860,7 +865,7 @@ static void dsi_update_dsc_timing(struct msm_dsi_host *msm_host, bool is_cmd_mod
 	if (slice_per_intf > dsc->slice_count)
 		dsc->slice_count = 1;
 
-	bytes_in_slice = DIV_ROUND_UP(dsc->slice_width * dsc->bits_per_pixel, 8);
+	bytes_in_slice = DIV_ROUND_UP(dsc->slice_width * bpp, 8);
 
 	dsc->slice_chunk_size = bytes_in_slice;
 
@@ -913,6 +918,7 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi)
 	u32 va_end = va_start + mode->vdisplay;
 	u32 hdisplay = mode->hdisplay;
 	u32 wc;
+	int ret;
 
 	DBG("");
 
@@ -948,7 +954,9 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi)
 		/* we do the calculations for dsc parameters here so that
 		 * panel can use these parameters
 		 */
-		dsi_populate_dsc_params(dsc);
+		ret = dsi_populate_dsc_params(dsc);
+		if (ret)
+			return;
 
 		/* Divide the display by 3 but keep back/font porch and
 		 * pulse width same
@@ -1229,6 +1237,10 @@ static int dsi_cmd_dma_add(struct msm_dsi_host *msm_host,
 	if (packet.size < len)
 		memset(data + packet.size, 0xff, len - packet.size);
 
+	if (msg->type == MIPI_DSI_PICTURE_PARAMETER_SET)
+		print_hex_dump(KERN_DEBUG, "ALL:", DUMP_PREFIX_NONE,
+				16, 1, data, len, false);
+
 	if (cfg_hnd->ops->tx_buf_put)
 		cfg_hnd->ops->tx_buf_put(msm_host);
 
@@ -1786,6 +1798,12 @@ static int dsi_populate_dsc_params(struct drm_dsc_config *dsc)
 	int data;
 	int final_value, final_scale;
 	int i;
+	int bpp = dsc->bits_per_pixel >> 4;
+
+	if (dsc->bits_per_pixel & 0xf) {
+		pr_err("DSI does not support fractional bits_per_pixel\n");
+		return -EINVAL;
+	}
 
 	dsc->rc_model_size = 8192;
 	dsc->first_line_bpg_offset = 12;
@@ -1807,7 +1825,7 @@ static int dsi_populate_dsc_params(struct drm_dsc_config *dsc)
 	}
 
 	dsc->initial_offset = 6144; /* Not bpp 12 */
-	if (dsc->bits_per_pixel != 8)
+	if (bpp != 8)
 		dsc->initial_offset = 2048;	/* bpp = 12 */
 
 	mux_words_size = 48;		/* bpc == 8/10 */
@@ -1830,16 +1848,16 @@ static int dsi_populate_dsc_params(struct drm_dsc_config *dsc)
 	 * params are calculated
 	 */
 	groups_per_line = DIV_ROUND_UP(dsc->slice_width, 3);
-	dsc->slice_chunk_size = dsc->slice_width * dsc->bits_per_pixel / 8;
-	if ((dsc->slice_width * dsc->bits_per_pixel) % 8)
+	dsc->slice_chunk_size = dsc->slice_width * bpp / 8;
+	if ((dsc->slice_width * bpp) % 8)
 		dsc->slice_chunk_size++;
 
 	/* rbs-min */
 	min_rate_buffer_size =  dsc->rc_model_size - dsc->initial_offset +
-				dsc->initial_xmit_delay * dsc->bits_per_pixel +
+				dsc->initial_xmit_delay * bpp +
 				groups_per_line * dsc->first_line_bpg_offset;
 
-	hrd_delay = DIV_ROUND_UP(min_rate_buffer_size, dsc->bits_per_pixel);
+	hrd_delay = DIV_ROUND_UP(min_rate_buffer_size, bpp);
 
 	dsc->initial_dec_delay = hrd_delay - dsc->initial_xmit_delay;
 
@@ -1862,7 +1880,7 @@ static int dsi_populate_dsc_params(struct drm_dsc_config *dsc)
 	data = 2048 * (dsc->rc_model_size - dsc->initial_offset + num_extra_mux_bits);
 	dsc->slice_bpg_offset = DIV_ROUND_UP(data, groups_total);
 
-	target_bpp_x16 = dsc->bits_per_pixel * 16;
+	target_bpp_x16 = bpp * 16;
 
 	data = (dsc->initial_xmit_delay * target_bpp_x16) / 16;
 	final_value =  dsc->rc_model_size - data + num_extra_mux_bits;
-- 
2.37.3


  parent reply	other threads:[~2022-10-01 19:08 UTC|newest]

Thread overview: 65+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-01 19:08 [PATCH 0/5] drm: Fix math issues in MSM DSC implementation Marijn Suijten
2022-10-01 19:08 ` Marijn Suijten
2022-10-01 19:08 ` [PATCH 1/5] drm/msm/dsi: Remove useless math in DSC calculation Marijn Suijten
2022-10-01 19:08   ` Marijn Suijten
2022-10-01 20:19   ` Konrad Dybcio
2022-10-01 20:19     ` Konrad Dybcio
2022-10-04  0:26   ` Bjorn Andersson
2022-10-04  0:26     ` Bjorn Andersson
2022-10-04 14:33   ` [Freedreno] " Abhinav Kumar
2022-10-04 14:33     ` Abhinav Kumar
2022-10-04 22:23     ` Marijn Suijten
2022-10-04 22:23       ` Marijn Suijten
2022-10-01 19:08 ` [PATCH 2/5] drm/msm/dsi: Remove repeated calculation of slice_per_intf Marijn Suijten
2022-10-01 19:08   ` Marijn Suijten
2022-10-01 20:22   ` Konrad Dybcio
2022-10-01 20:22     ` Konrad Dybcio
2022-10-04  0:30   ` Bjorn Andersson
2022-10-04  0:30     ` Bjorn Andersson
2022-10-04 14:41   ` Abhinav Kumar
2022-10-04 14:41     ` Abhinav Kumar
2022-10-01 19:08 ` Marijn Suijten [this message]
2022-10-01 19:08   ` [PATCH 3/5] drm/msm/dsi: Account for DSC's bits_per_pixel having 4 fractional bits Marijn Suijten
2022-10-01 20:28   ` Konrad Dybcio
2022-10-01 20:28     ` Konrad Dybcio
2022-10-01 20:37   ` Marijn Suijten
2022-10-01 20:37     ` Marijn Suijten
2022-10-04 14:45   ` Dmitry Baryshkov
2022-10-04 14:45     ` Dmitry Baryshkov
2022-10-04 22:35     ` Marijn Suijten
2022-10-04 22:35       ` Marijn Suijten
2022-10-04 22:40       ` Dmitry Baryshkov
2022-10-04 22:56         ` Marijn Suijten
2022-10-04 22:56           ` Marijn Suijten
2022-10-01 19:08 ` [PATCH 4/5] drm/msm/dpu1: " Marijn Suijten
2022-10-01 19:08   ` Marijn Suijten
2022-10-04 14:35   ` Dmitry Baryshkov
2022-10-04 14:35     ` Dmitry Baryshkov
2022-10-04 17:03   ` Abhinav Kumar
2022-10-04 17:03     ` Abhinav Kumar
2022-10-04 22:11     ` Marijn Suijten
2022-10-04 22:11       ` Marijn Suijten
2022-10-05 14:19       ` Abhinav Kumar
2022-10-05 18:45         ` Marijn Suijten
2022-10-05 18:45           ` Marijn Suijten
2022-10-01 19:08 ` [PATCH 5/5] drm/dsc: Prevent negative BPG offsets from shadowing adjacent bitfields Marijn Suijten
2022-10-01 19:08   ` Marijn Suijten
2022-10-01 20:23   ` Marijn Suijten
2022-10-01 20:23     ` Marijn Suijten
2022-10-04 14:41     ` Dmitry Baryshkov
2022-10-04 21:48       ` Marijn Suijten
2022-10-04 21:48         ` Marijn Suijten
2022-10-04 20:22   ` Abhinav Kumar
2022-10-04 20:22     ` Abhinav Kumar
2022-10-04 21:57     ` Marijn Suijten
2022-10-04 21:57       ` Marijn Suijten
2022-10-04 22:31       ` Abhinav Kumar
2022-10-04 22:39         ` Marijn Suijten
2022-10-04 22:39           ` Marijn Suijten
2022-10-05 15:33           ` Abhinav Kumar
2022-10-05 18:29             ` Marijn Suijten
2022-10-05 18:29               ` Marijn Suijten
2022-10-04  4:42 ` [PATCH 0/5] drm: Fix math issues in MSM DSC implementation Vinod Koul
2022-10-04  4:42   ` Vinod Koul
2022-10-04  9:51   ` Marijn Suijten
2022-10-04  9:51     ` Marijn Suijten

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=20221001190807.358691-4-marijn.suijten@somainline.org \
    --to=marijn.suijten@somainline.org \
    --cc=airlied@linux.ie \
    --cc=alexander.deucher@amd.com \
    --cc=angelogioacchino.delregno@somainline.org \
    --cc=daniel@ffwll.ch \
    --cc=dianders@chromium.org \
    --cc=dmitry.baryshkov@linaro.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=freedreno@lists.freedesktop.org \
    --cc=jami.kettunen@somainline.org \
    --cc=javierm@redhat.com \
    --cc=konrad.dybcio@somainline.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marex@denx.de \
    --cc=martin.botka@somainline.org \
    --cc=phone-devel@vger.kernel.org \
    --cc=quic_abhinavk@quicinc.com \
    --cc=robdclark@gmail.com \
    --cc=sean@poorly.run \
    --cc=tzimmermann@suse.de \
    --cc=vkoul@kernel.org \
    --cc=vladimir.lypak@gmail.com \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.