Message ID | m3sg1uq6xu.fsf@t19.piap.pl |
---|---|
State | Superseded |
Headers | show |
Series | TDA1997x: enable EDID support | expand |
Tim, Tim Harvey <tharvey@gateworks.com> writes: >> I'm looking at the previous version of this driver from Gateworks and it >> contains: >> >> /* Configure EDID >> * >> * EDID_ENABLE bits: >> * 7 - nack_off >> * 6 - edid_only >> * 1 - edid_b_en >> * 0 - edid_a_en >> */ >> reg = io_read(REG_EDID_ENABLE); >> if (!tda1997x->internal_edid) >> reg &= ~0x83; /* EDID Nack ON */ >> else >> reg |= 0x83; /* EDID Nack OFF */ >> io_write(REG_EDID_ENABLE, reg); > Not sure where the source above is from (this was all so long ago) but That's your gateworks_fslc_3.14_1.0.x_ga branch (3.14 is the kernel and 1.0.x_ga is IIRC some Freescale versioning) :-) > my guess is that 'internal_edid' meant an EDID had been provided via > software and the else case meant there was no EDID available. > There is no support on that chip for an external EEPROM. Right. I guess the else meant it was available and &= ~83 meant no EDID... Anyway one could simply drop a 24c02 or a similar chip directly to SDA/SCL HDMI lines, that's what the monitor makers had been doing for a long time. Then, I guess, you would need the internal_edid = 0 (otherwise the TDA chip would be fighting the EEPROM on the SDA line). Not that I know of any such design using this driver, I guess we can safely skip this part.
Hi Krzysztof, On 08/06/2021 06:54, Krzysztof Hałasa wrote: > Tim, > > Tim Harvey <tharvey@gateworks.com> writes: > >>> I'm looking at the previous version of this driver from Gateworks and it >>> contains: >>> >>> /* Configure EDID >>> * >>> * EDID_ENABLE bits: >>> * 7 - nack_off >>> * 6 - edid_only >>> * 1 - edid_b_en >>> * 0 - edid_a_en >>> */ >>> reg = io_read(REG_EDID_ENABLE); >>> if (!tda1997x->internal_edid) >>> reg &= ~0x83; /* EDID Nack ON */ >>> else >>> reg |= 0x83; /* EDID Nack OFF */ >>> io_write(REG_EDID_ENABLE, reg); > >> Not sure where the source above is from (this was all so long ago) but > > That's your gateworks_fslc_3.14_1.0.x_ga branch (3.14 is the kernel and > 1.0.x_ga is IIRC some Freescale versioning) :-) > >> my guess is that 'internal_edid' meant an EDID had been provided via >> software and the else case meant there was no EDID available. >> There is no support on that chip for an external EEPROM. > > Right. I guess the else meant it was available and &= ~83 meant no > EDID... Anyway one could simply drop a 24c02 or a similar chip directly > to SDA/SCL HDMI lines, that's what the monitor makers had been doing for > a long time. Then, I guess, you would need the internal_edid = 0 > (otherwise the TDA chip would be fighting the EEPROM on the SDA line). > Not that I know of any such design using this driver, I guess we can > safely skip this part. > OK, I think the history is clear. Can you post a v2 with a Fixes tag and comment a bit on why this was not caught before? Regards, Hans
On 08/06/2021 10:45, Krzysztof Hałasa wrote: > Hans Verkuil <hverkuil@xs4all.nl> writes: > >> OK, I think the history is clear. Can you post a v2 with a Fixes tag and >> comment a bit on why this was not caught before? > > Sure, will do. That "Fixes" tag... since it's from the beginning (the > Gateworks' branch was never a part of the official tree), do I still > need it? It would have to point to the initial submission of this > driver. > Yes, that's fine. It's been broken since the beginning, so the Fixes tag will indicate that. Regards, Hans
--- a/drivers/media/i2c/tda1997x.c +++ b/drivers/media/i2c/tda1997x.c @@ -2233,6 +2233,7 @@ static int tda1997x_core_init(struct v4l2_subdev *sd) /* get initial HDMI status */ state->hdmi_status = io_read(sd, REG_HDMI_FLAGS); + io_write(sd, REG_EDID_ENABLE, EDID_ENABLE_A_EN | EDID_ENABLE_B_EN); return 0; }
Without this patch, the TDA19971 chip's EDID is inactive. Signed-off-by: Krzysztof Halasa <khalasa@piap.pl>