From 8d246e293228881b557798416cb064d644cdb10e Mon Sep 17 00:00:00 2001 From: Krzysztof Hałasa Date: Fri, 30 Jul 2021 08:59:19 +0200 Subject: media: TDA1997x: fix tda1997x_remove() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TDA1997x driver tried to hold two values in a single variable: device's "client data" pointer was first set to "sd" in v4l2_i2c_subdev_init(), then it was overwritten explicitly using dev_set_drvdata() with "state". This caused tda1997x_remove() to fail badly. Signed-off-by: Krzysztof Hałasa Signed-off-by: Hans Verkuil Signed-off-by: Mauro Carvalho Chehab --- drivers/media/i2c/tda1997x.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/media/i2c/tda1997x.c') diff --git a/drivers/media/i2c/tda1997x.c b/drivers/media/i2c/tda1997x.c index 6070aaf0b32e..1e2a263be933 100644 --- a/drivers/media/i2c/tda1997x.c +++ b/drivers/media/i2c/tda1997x.c @@ -2450,7 +2450,8 @@ static const struct media_entity_operations tda1997x_media_ops = { static int tda1997x_pcm_startup(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) { - struct tda1997x_state *state = snd_soc_dai_get_drvdata(dai); + struct v4l2_subdev *sd = snd_soc_dai_get_drvdata(dai); + struct tda1997x_state *state = to_state(sd); struct snd_soc_component *component = dai->component; struct snd_pcm_runtime *rtd = substream->runtime; int rate, err; @@ -2759,7 +2760,6 @@ static int tda1997x_probe(struct i2c_client *client, dev_err(&client->dev, "register audio codec failed\n"); goto err_free_media; } - dev_set_drvdata(&state->client->dev, state); v4l_info(state->client, "registered audio codec\n"); } -- cgit v1.2.3 From 48d219f9cc667bc6fbc3e3af0b1bfd75db94fce4 Mon Sep 17 00:00:00 2001 From: Tom Rix Date: Thu, 12 Aug 2021 19:00:43 +0200 Subject: media: TDA1997x: handle short reads of hdmi info frame. Static analysis reports this representative problem tda1997x.c:1939: warning: 7th function call argument is an uninitialized value The 7th argument is buffer[0], which is set in the earlier call to io_readn(). When io_readn() call to io_read() fails with the first read, buffer[0] is not set and 0 is returned and stored in len. The later call to hdmi_infoframe_unpack()'s size parameter is the static size of buffer, always 40, so a short read is not caught in hdmi_infoframe_unpacks()'s checking. The variable len should be used instead. Zero initialize buffer to 0 so it is in a known start state. Fixes: 9ac0038db9a7 ("media: i2c: Add TDA1997x HDMI receiver driver") Signed-off-by: Tom Rix Reviewed-by: Tim Harvey Signed-off-by: Hans Verkuil Signed-off-by: Mauro Carvalho Chehab --- drivers/media/i2c/tda1997x.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'drivers/media/i2c/tda1997x.c') diff --git a/drivers/media/i2c/tda1997x.c b/drivers/media/i2c/tda1997x.c index 1e2a263be933..0b995424cb34 100644 --- a/drivers/media/i2c/tda1997x.c +++ b/drivers/media/i2c/tda1997x.c @@ -1248,13 +1248,13 @@ tda1997x_parse_infoframe(struct tda1997x_state *state, u16 addr) { struct v4l2_subdev *sd = &state->sd; union hdmi_infoframe frame; - u8 buffer[40]; + u8 buffer[40] = { 0 }; u8 reg; int len, err; /* read data */ len = io_readn(sd, addr, sizeof(buffer), buffer); - err = hdmi_infoframe_unpack(&frame, buffer, sizeof(buffer)); + err = hdmi_infoframe_unpack(&frame, buffer, len); if (err) { v4l_err(state->client, "failed parsing %d byte infoframe: 0x%04x/0x%02x\n", @@ -1928,13 +1928,13 @@ static int tda1997x_log_infoframe(struct v4l2_subdev *sd, int addr) { struct tda1997x_state *state = to_state(sd); union hdmi_infoframe frame; - u8 buffer[40]; + u8 buffer[40] = { 0 }; int len, err; /* read data */ len = io_readn(sd, addr, sizeof(buffer), buffer); v4l2_dbg(1, debug, sd, "infoframe: addr=%d len=%d\n", addr, len); - err = hdmi_infoframe_unpack(&frame, buffer, sizeof(buffer)); + err = hdmi_infoframe_unpack(&frame, buffer, len); if (err) { v4l_err(state->client, "failed parsing %d byte infoframe: 0x%04x/0x%02x\n", -- cgit v1.2.3 From d64a7709a81c298761aadc9690c110ee3fcc675a Mon Sep 17 00:00:00 2001 From: Krzysztof Hałasa Date: Wed, 6 Oct 2021 06:30:19 +0100 Subject: media: TDA1997x: replace video detection routine MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The TDA1997x (HDMI receiver) driver currently uses a specific video format detection scheme. The frame (or field in interlaced mode), line and HSync pulse durations are compared to those of known, standard video modes. If a match is found, the mode is assumed to be detected, otherwise -ERANGE is returned (then possibly ignored). This means that: - another mode with similar timings will be detected incorrectly (e.g. 2x faster clock and lines twice as long) - non-standard modes will not work. This patch replaces this scheme with a direct read of geometry registers. This way all modes recognized by the chip are supported. In interlaced modes, the code assumes the V sync signal has the same duration for both fields. While this may be not necessarily true, I can't see any way to get the "other" V sync width. This is most probably harmless. All tests have been performed on Gateworks' Ventana GW54xx board, with a TDA19971 chip. Signed-off-by: Krzysztof Hałasa Tested-by: Tim Harvey Signed-off-by: Hans Verkuil Signed-off-by: Mauro Carvalho Chehab --- drivers/media/i2c/tda1997x.c | 119 +++++++++++++++++++++----------------- drivers/media/i2c/tda1997x_regs.h | 3 + 2 files changed, 70 insertions(+), 52 deletions(-) (limited to 'drivers/media/i2c/tda1997x.c') diff --git a/drivers/media/i2c/tda1997x.c b/drivers/media/i2c/tda1997x.c index 0b995424cb34..8fafce26d62f 100644 --- a/drivers/media/i2c/tda1997x.c +++ b/drivers/media/i2c/tda1997x.c @@ -1092,67 +1092,82 @@ tda1997x_detect_std(struct tda1997x_state *state, struct v4l2_dv_timings *timings) { struct v4l2_subdev *sd = &state->sd; - u32 vper; - u16 hper; - u16 hsper; - int i; /* * Read the FMT registers - * REG_V_PER: Period of a frame (or two fields) in MCLK(27MHz) cycles - * REG_H_PER: Period of a line in MCLK(27MHz) cycles - * REG_HS_WIDTH: Period of horiz sync pulse in MCLK(27MHz) cycles + * REG_V_PER: Period of a frame (or field) in MCLK (27MHz) cycles + * REG_H_PER: Period of a line in MCLK (27MHz) cycles + * REG_HS_WIDTH: Period of horiz sync pulse in MCLK (27MHz) cycles */ - vper = io_read24(sd, REG_V_PER) & MASK_VPER; - hper = io_read16(sd, REG_H_PER) & MASK_HPER; - hsper = io_read16(sd, REG_HS_WIDTH) & MASK_HSWIDTH; - v4l2_dbg(1, debug, sd, "Signal Timings: %u/%u/%u\n", vper, hper, hsper); + u32 vper, vsync_pos; + u16 hper, hsync_pos, hsper, interlaced; + u16 htot, hact, hfront, hsync, hback; + u16 vtot, vact, vfront1, vfront2, vsync, vback1, vback2; if (!state->input_detect[0] && !state->input_detect[1]) return -ENOLINK; - for (i = 0; v4l2_dv_timings_presets[i].bt.width; i++) { - const struct v4l2_bt_timings *bt; - u32 lines, width, _hper, _hsper; - u32 vmin, vmax, hmin, hmax, hsmin, hsmax; - bool vmatch, hmatch, hsmatch; - - bt = &v4l2_dv_timings_presets[i].bt; - width = V4L2_DV_BT_FRAME_WIDTH(bt); - lines = V4L2_DV_BT_FRAME_HEIGHT(bt); - _hper = (u32)bt->pixelclock / width; - if (bt->interlaced) - lines /= 2; - /* vper +/- 0.7% */ - vmin = ((27000000 / 1000) * 993) / _hper * lines; - vmax = ((27000000 / 1000) * 1007) / _hper * lines; - /* hper +/- 1.0% */ - hmin = ((27000000 / 100) * 99) / _hper; - hmax = ((27000000 / 100) * 101) / _hper; - /* hsper +/- 2 (take care to avoid 32bit overflow) */ - _hsper = 27000 * bt->hsync / ((u32)bt->pixelclock/1000); - hsmin = _hsper - 2; - hsmax = _hsper + 2; - - /* vmatch matches the framerate */ - vmatch = ((vper <= vmax) && (vper >= vmin)) ? 1 : 0; - /* hmatch matches the width */ - hmatch = ((hper <= hmax) && (hper >= hmin)) ? 1 : 0; - /* hsmatch matches the hswidth */ - hsmatch = ((hsper <= hsmax) && (hsper >= hsmin)) ? 1 : 0; - if (hmatch && vmatch && hsmatch) { - v4l2_print_dv_timings(sd->name, "Detected format: ", - &v4l2_dv_timings_presets[i], - false); - if (timings) - *timings = v4l2_dv_timings_presets[i]; - return 0; - } - } + vper = io_read24(sd, REG_V_PER); + hper = io_read16(sd, REG_H_PER); + hsper = io_read16(sd, REG_HS_WIDTH); + vsync_pos = vper & MASK_VPER_SYNC_POS; + hsync_pos = hper & MASK_HPER_SYNC_POS; + interlaced = hsper & MASK_HSWIDTH_INTERLACED; + vper &= MASK_VPER; + hper &= MASK_HPER; + hsper &= MASK_HSWIDTH; + v4l2_dbg(1, debug, sd, "Signal Timings: %u/%u/%u\n", vper, hper, hsper); - v4l_err(state->client, "no resolution match for timings: %d/%d/%d\n", - vper, hper, hsper); - return -ERANGE; + htot = io_read16(sd, REG_FMT_H_TOT); + hact = io_read16(sd, REG_FMT_H_ACT); + hfront = io_read16(sd, REG_FMT_H_FRONT); + hsync = io_read16(sd, REG_FMT_H_SYNC); + hback = io_read16(sd, REG_FMT_H_BACK); + + vtot = io_read16(sd, REG_FMT_V_TOT); + vact = io_read16(sd, REG_FMT_V_ACT); + vfront1 = io_read(sd, REG_FMT_V_FRONT_F1); + vfront2 = io_read(sd, REG_FMT_V_FRONT_F2); + vsync = io_read(sd, REG_FMT_V_SYNC); + vback1 = io_read(sd, REG_FMT_V_BACK_F1); + vback2 = io_read(sd, REG_FMT_V_BACK_F2); + + v4l2_dbg(1, debug, sd, "Geometry: H %u %u %u %u %u Sync%c V %u %u %u %u %u %u %u Sync%c\n", + htot, hact, hfront, hsync, hback, hsync_pos ? '+' : '-', + vtot, vact, vfront1, vfront2, vsync, vback1, vback2, vsync_pos ? '+' : '-'); + + if (!timings) + return 0; + + timings->type = V4L2_DV_BT_656_1120; + timings->bt.width = hact; + timings->bt.hfrontporch = hfront; + timings->bt.hsync = hsync; + timings->bt.hbackporch = hback; + timings->bt.height = vact; + timings->bt.vfrontporch = vfront1; + timings->bt.vsync = vsync; + timings->bt.vbackporch = vback1; + timings->bt.interlaced = interlaced ? V4L2_DV_INTERLACED : V4L2_DV_PROGRESSIVE; + timings->bt.polarities = vsync_pos ? V4L2_DV_VSYNC_POS_POL : 0; + timings->bt.polarities |= hsync_pos ? V4L2_DV_HSYNC_POS_POL : 0; + + timings->bt.pixelclock = (u64)htot * vtot * 27000000; + if (interlaced) { + timings->bt.il_vfrontporch = vfront2; + timings->bt.il_vsync = timings->bt.vsync; + timings->bt.il_vbackporch = vback2; + do_div(timings->bt.pixelclock, vper * 2 /* full frame */); + } else { + timings->bt.il_vfrontporch = 0; + timings->bt.il_vsync = 0; + timings->bt.il_vbackporch = 0; + do_div(timings->bt.pixelclock, vper); + } + v4l2_find_dv_timings_cap(timings, &tda1997x_dv_timings_cap, + (u32)timings->bt.pixelclock / 500, NULL, NULL); + v4l2_print_dv_timings(sd->name, "Detected format: ", timings, false); + return 0; } /* some sort of errata workaround for chip revision 0 (N1) */ diff --git a/drivers/media/i2c/tda1997x_regs.h b/drivers/media/i2c/tda1997x_regs.h index d9b3daada07d..115371ba33f0 100644 --- a/drivers/media/i2c/tda1997x_regs.h +++ b/drivers/media/i2c/tda1997x_regs.h @@ -117,9 +117,12 @@ #define REG_CURPAGE_00H 0xFF #define MASK_VPER 0x3fffff +#define MASK_VPER_SYNC_POS 0x800000 #define MASK_VHREF 0x3fff #define MASK_HPER 0x0fff +#define MASK_HPER_SYNC_POS 0x8000 #define MASK_HSWIDTH 0x03ff +#define MASK_HSWIDTH_INTERLACED 0x8000 /* HPD Detection */ #define DETECT_UTIL BIT(7) /* utility of HDMI level */ -- cgit v1.2.3