Hi, On Mon, Jul 22, 2019 at 11:19 AM Matthias Kaehlcke <mka@xxxxxxxxxxxx> wrote: > > The DDC/CI protocol involves sending a multi-byte request to the > display via I2C, which is typically followed by a multi-byte > response. The internal I2C controller only allows single byte > reads/writes or reads of 8 sequential bytes, hence DDC/CI is not > supported when the internal I2C controller is used. The I2C > transfers complete without errors, however the data in the response > is garbage. Abort transfers to/from slave address 0x37 (DDC) with > -EOPNOTSUPP, to make it evident that the communication is failing. > > Signed-off-by: Matthias Kaehlcke <mka@xxxxxxxxxxxx> > --- > Changes in v2: > - changed DDC_I2C_ADDR to DDC_CI_ADDR > --- > drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > index 045b1b13fd0e..28933629f3c7 100644 > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > @@ -35,6 +35,7 @@ > > #include <media/cec-notifier.h> > > +#define DDC_CI_ADDR 0x37 > #define DDC_SEGMENT_ADDR 0x30 > > #define HDMI_EDID_LEN 512 > @@ -322,6 +323,13 @@ static int dw_hdmi_i2c_xfer(struct i2c_adapter *adap, > u8 addr = msgs[0].addr; > int i, ret = 0; > > + if (addr == DDC_CI_ADDR) > + /* > + * The internal I2C controller does not support the multi-byte > + * read and write operations needed for DDC/CI. > + */ > + return -EOPNOTSUPP; > + In theory we could also solve this by detecting (in other parts of the function) the bad multi-byte read/write operations. That would maybe be more generic (AKA it would more properly handle random userspace tools fiddling with i2c-dev) but add complexity to the code. ...possibly a better long-term solution is to just totally stop emulating a regular i2c adapter when the hardware just doesn't support that. In theory we could remove the "drm_get_edid()" call in dw_hdmi_connector_get_modes() and replace it with a direct call to drm_do_get_edid() if we're using the built-in adapter. Possibly "tda998x_drv.c" would be a good reference. If we did that, we could remove all the weird / hacky i2c code in this driver. Since the bigger cleanup seems like a bit much to ask, I'm OK with this as a stopgap to at least prevent userspace tools from getting confused. Thus: Reviewed-by: Douglas Anderson <dianders@xxxxxxxxxxxx> _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
