On Mon Sep 21 09:15:33 2026 +0200, Lucas Sinn wrote:
> hdmirx_get_detected_timings() divides the TMDS clock by
> hdmirx_dev->color_depth, but nothing on its own path ever sets it. The
> only producer is hdmirx_get_colordepth(), reached solely from
> hdmirx_format_change() at the tail of hdmirx_wait_signal_lock().
>
> VIDIOC_QUERY_DV_TIMINGS does not come through there. It only refuses to
> proceed while signal_not_lock() is true, and that reads the lock state
> straight out of the hardware, so it can already be false while the work
> ending in hdmirx_format_change() has yet to run. Query the timings in
> that window, on the first lock after a boot, and color_depth is still
> zero from devm_kzalloc().
>
> The division does not fault. do_div() is plain C on 64 bit and arm64
> compiles it to udiv, which returns zero for a zero divisor. What comes
> out is a pixelclock of zero, and a stitched output frame.
>
> Read the colordepth where it is used instead.
>
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Lucas Sinn <[email protected]>
> Signed-off-by: Sascha Hauer <[email protected]>
> Fixes: 7b59b132ad43 ("media: platform: synopsys: Add support for HDMI input
> driver")
> Cc: [email protected]
> Reviewed-by: Dmitry Osipenko <[email protected]>
> Signed-off-by: Hans Verkuil <[email protected]>
Patch committed.
Thanks,
Hans Verkuil
.../media/platform/synopsys/hdmirx/snps_hdmirx.c | 71 +++++++++++-----------
1 file changed, 36 insertions(+), 35 deletions(-)
---
diff --git a/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
b/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
index 7ec25ab235b6..6810bfc39da7 100644
--- a/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
+++ b/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
@@ -165,7 +165,6 @@ struct snps_hdmirx_dev {
int num_clks;
u32 edid_blocks_written;
u32 cur_fmt_fourcc;
- u32 color_depth;
spinlock_t rst_lock; /* to lock register access */
u8 edid[EDID_NUM_BLOCKS_MAX * EDID_BLOCK_SIZE];
};
@@ -389,6 +388,38 @@ static void hdmirx_toggle_polarity(struct snps_hdmirx_dev
*hdmirx_dev)
VPROC_HSYNC_POL_OVR_EN, 0);
}
+static u32 hdmirx_get_colordepth(struct snps_hdmirx_dev *hdmirx_dev)
+{
+ struct v4l2_device *v4l2_dev = &hdmirx_dev->v4l2_dev;
+ u32 val, color_depth_reg, color_depth;
+
+ val = hdmirx_readl(hdmirx_dev, DMA_STATUS11);
+ color_depth_reg = (val & HDMIRX_COLOR_DEPTH_MASK) >> 3;
+
+ switch (color_depth_reg) {
+ case 0x4:
+ color_depth = 24;
+ break;
+ case 0x5:
+ color_depth = 30;
+ break;
+ case 0x6:
+ color_depth = 36;
+ break;
+ case 0x7:
+ color_depth = 48;
+ break;
+ default:
+ color_depth = 24;
+ break;
+ }
+
+ v4l2_dbg(1, debug, v4l2_dev, "%s: color_depth: %d, reg_val:%d\n",
+ __func__, color_depth, color_depth_reg);
+
+ return color_depth;
+}
+
/*
* When querying DV timings during preview, if the DMA's timing is stable,
* we retrieve the timings directly from the DMA. However, if the current
@@ -402,7 +433,7 @@ static int hdmirx_get_detected_timings(struct
snps_hdmirx_dev *hdmirx_dev,
struct v4l2_bt_timings *bt = &timings->bt;
u32 val, tmdsqpclk_freq, pix_clk;
unsigned int num_retries = 0;
- u32 field_type, deframer_st;
+ u32 field_type, deframer_st, color_depth;
u64 tmp_data, tmds_clk;
bool is_dvi_mode;
int ret;
@@ -423,10 +454,11 @@ retry:
deframer_st = hdmirx_readl(hdmirx_dev, DEFRAMER_STATUS);
is_dvi_mode = !(deframer_st & OPMODE_STS_MASK);
+ color_depth = hdmirx_get_colordepth(hdmirx_dev);
tmdsqpclk_freq = hdmirx_readl(hdmirx_dev, CMU_TMDSQPCLK_FREQ);
tmds_clk = tmdsqpclk_freq * 4 * 1000;
tmp_data = tmds_clk * 24;
- do_div(tmp_data, hdmirx_dev->color_depth);
+ do_div(tmp_data, color_depth);
pix_clk = tmp_data;
bt->pixelclock = pix_clk;
@@ -438,7 +470,7 @@ retry:
v4l2_dbg(2, debug, v4l2_dev, "tmds_clk:%llu, pix_clk:%d\n", tmds_clk,
pix_clk);
v4l2_dbg(1, debug, v4l2_dev, "interlace:%d, fmt:%d, color:%d,
mode:%s\n",
bt->interlaced, hdmirx_dev->pix_fmt,
- hdmirx_dev->color_depth,
+ color_depth,
is_dvi_mode ? "dvi" : "hdmi");
v4l2_dbg(2, debug, v4l2_dev, "deframer_st:%#x\n", deframer_st);
@@ -997,36 +1029,6 @@ static void hdmirx_controller_init(struct snps_hdmirx_dev
*hdmirx_dev)
VS_REMAPFILTER_EN_QST | VS_FILTER_ORDER_QST(0x3));
}
-static void hdmirx_get_colordepth(struct snps_hdmirx_dev *hdmirx_dev)
-{
- struct v4l2_device *v4l2_dev = &hdmirx_dev->v4l2_dev;
- u32 val, color_depth_reg;
-
- val = hdmirx_readl(hdmirx_dev, DMA_STATUS11);
- color_depth_reg = (val & HDMIRX_COLOR_DEPTH_MASK) >> 3;
-
- switch (color_depth_reg) {
- case 0x4:
- hdmirx_dev->color_depth = 24;
- break;
- case 0x5:
- hdmirx_dev->color_depth = 30;
- break;
- case 0x6:
- hdmirx_dev->color_depth = 36;
- break;
- case 0x7:
- hdmirx_dev->color_depth = 48;
- break;
- default:
- hdmirx_dev->color_depth = 24;
- break;
- }
-
- v4l2_dbg(1, debug, v4l2_dev, "%s: color_depth: %d, reg_val:%d\n",
- __func__, hdmirx_dev->color_depth, color_depth_reg);
-}
-
static void hdmirx_get_pix_fmt(struct snps_hdmirx_dev *hdmirx_dev)
{
struct v4l2_device *v4l2_dev = &hdmirx_dev->v4l2_dev;
@@ -1137,7 +1139,6 @@ static void hdmirx_format_change(struct snps_hdmirx_dev
*hdmirx_dev)
};
hdmirx_get_pix_fmt(hdmirx_dev);
- hdmirx_get_colordepth(hdmirx_dev);
hdmirx_get_avi_infoframe(hdmirx_dev);
v4l2_dbg(1, debug, v4l2_dev, "%s: queue res_chg_event\n", __func__);
_______________________________________________
linuxtv-commits mailing list -- [email protected]
To unsubscribe send an email to [email protected]
