| Message ID | 1484897930-1275-18-git-send-email-a.hajda@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 01/20/2017 01:08 PM, Andrzej Hajda wrote: > MHL3 requires that after reading EDID from the sink source should ask > peer for features. To make both protocols happy the patch splits the code > accordingly. I was wondering about the EDID code in the driver, what does it exactly do with the EDID? We don't seem to be populating connector modes with it. I saw a func called sii8620_set_upstream_edid(), but didn't understand what it does. Archit > > Signed-off-by: Andrzej Hajda <a.hajda@samsung.com> > --- > drivers/gpu/drm/bridge/sil-sii8620.c | 31 +++++++++++++++++++++++++++---- > 1 file changed, 27 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c b/drivers/gpu/drm/bridge/sil-sii8620.c > index 6ef1a4d..d0e6dc3 100644 > --- a/drivers/gpu/drm/bridge/sil-sii8620.c > +++ b/drivers/gpu/drm/bridge/sil-sii8620.c > @@ -482,6 +482,13 @@ static void sii8620_sink_detected(struct sii8620 *ctx, int ret) > > dev_info(dev, "detected sink(type: %s): %s\n", > sink_str[ctx->sink_type], sink_name); > +} > + > +static void sii8620_edid_read(struct sii8620 *ctx, int ret) > +{ > + if (ret < 0) > + return; > + > sii8620_set_upstream_edid(ctx); > sii8620_enable_hpd(ctx); > } > @@ -787,12 +794,12 @@ static void sii8620_fetch_edid(struct sii8620 *ctx) > edid = new_edid; > } > } > - > - if (fetched + FETCH_SIZE == edid_len) > - sii8620_write(ctx, REG_INTR3, int3); > } > > - sii8620_write(ctx, REG_LM_DDC, lm_ddc); > + sii8620_write_seq(ctx, > + REG_INTR3_MASK, BIT_DDC_CMD_DONE, > + REG_LM_DDC, lm_ddc > + ); > > end: > kfree(ctx->edid); > @@ -1706,6 +1713,21 @@ static void sii8620_irq_block(struct sii8620 *ctx) > sii8620_write(ctx, REG_EMSCINTR, stat); > } > > +static void sii8620_irq_ddc(struct sii8620 *ctx) > +{ > + u8 stat = sii8620_readb(ctx, REG_INTR3); > + > + if (stat & BIT_DDC_CMD_DONE) { > + sii8620_write(ctx, REG_INTR3_MASK, 0); > + if (sii8620_is_mhl3(ctx)) > + sii8620_mt_set_int(ctx, MHL_INT_REG(RCHANGE), > + MHL_INT_RC_FEAT_REQ); > + else > + sii8620_edid_read(ctx, 0); > + } > + sii8620_write(ctx, REG_INTR3, stat); > +} > + > /* endian agnostic, non-volatile version of test_bit */ > static bool sii8620_test_bit(unsigned int nr, const u8 *addr) > { > @@ -1726,6 +1748,7 @@ static irqreturn_t sii8620_irq_thread(int irq, void *data) > { BIT_FAST_INTR_STAT_MERR, sii8620_irq_merr }, > { BIT_FAST_INTR_STAT_BLOCK, sii8620_irq_block }, > { BIT_FAST_INTR_STAT_EDID, sii8620_irq_edid }, > + { BIT_FAST_INTR_STAT_DDC, sii8620_irq_ddc }, > { BIT_FAST_INTR_STAT_SCDT, sii8620_irq_scdt }, > { BIT_FAST_INTR_STAT_INFR, sii8620_irq_infr }, > }; >
On 24.01.2017 11:31, Archit Taneja wrote: > > On 01/20/2017 01:08 PM, Andrzej Hajda wrote: >> MHL3 requires that after reading EDID from the sink source should ask >> peer for features. To make both protocols happy the patch splits the code >> accordingly. > I was wondering about the EDID code in the driver, what does it exactly > do with the EDID? We don't seem to be populating connector modes with it. > I saw a func called sii8620_set_upstream_edid(), but didn't understand > what it does. MHL reads EDID from TV (or MHL dongle) using MHL protocol and publish it upstream (for example to exynos_hdmi on tm2 devices) via i2c bus. With some modifications SiI8620 could act as connector and do mode population itself, it would require modification of exynos_hdmi, and the main drawback is that modes unsupported by exynos_hdmi would not be filtered out (as mode filtering is done in connector). Regards Andrzej
On 01/24/2017 05:09 PM, Andrzej Hajda wrote: > On 24.01.2017 11:31, Archit Taneja wrote: >> >> On 01/20/2017 01:08 PM, Andrzej Hajda wrote: >>> MHL3 requires that after reading EDID from the sink source should ask >>> peer for features. To make both protocols happy the patch splits the code >>> accordingly. >> I was wondering about the EDID code in the driver, what does it exactly >> do with the EDID? We don't seem to be populating connector modes with it. >> I saw a func called sii8620_set_upstream_edid(), but didn't understand >> what it does. > > MHL reads EDID from TV (or MHL dongle) using MHL protocol and publish it > upstream (for example to exynos_hdmi on tm2 devices) via i2c bus. Oh okay. So, the MHL writes it into some sort of flash, which exynos_hdmi can access via an i2c master? > With some modifications SiI8620 could act as connector and do mode > population itself, it would require modification of exynos_hdmi, and the > main drawback is that modes unsupported by exynos_hdmi would not be > filtered out (as mode filtering is done in connector). Yeah, I guess if exynos_hdmi as a drm_encoder wouldn't be able to drop modes. The best it could do is return an error in mode_fixup. Anyway, since there seems to be only one kms user for the sil8620 driver, I think we can leave it for later. Btw, the patchset looks fine to me. It'd be great if you could have slightly more descriptive commit messages for some of the patches. Thanks, Archit
diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c b/drivers/gpu/drm/bridge/sil-sii8620.c index 6ef1a4d..d0e6dc3 100644 --- a/drivers/gpu/drm/bridge/sil-sii8620.c +++ b/drivers/gpu/drm/bridge/sil-sii8620.c @@ -482,6 +482,13 @@ static void sii8620_sink_detected(struct sii8620 *ctx, int ret) dev_info(dev, "detected sink(type: %s): %s\n", sink_str[ctx->sink_type], sink_name); +} + +static void sii8620_edid_read(struct sii8620 *ctx, int ret) +{ + if (ret < 0) + return; + sii8620_set_upstream_edid(ctx); sii8620_enable_hpd(ctx); } @@ -787,12 +794,12 @@ static void sii8620_fetch_edid(struct sii8620 *ctx) edid = new_edid; } } - - if (fetched + FETCH_SIZE == edid_len) - sii8620_write(ctx, REG_INTR3, int3); } - sii8620_write(ctx, REG_LM_DDC, lm_ddc); + sii8620_write_seq(ctx, + REG_INTR3_MASK, BIT_DDC_CMD_DONE, + REG_LM_DDC, lm_ddc + ); end: kfree(ctx->edid); @@ -1706,6 +1713,21 @@ static void sii8620_irq_block(struct sii8620 *ctx) sii8620_write(ctx, REG_EMSCINTR, stat); } +static void sii8620_irq_ddc(struct sii8620 *ctx) +{ + u8 stat = sii8620_readb(ctx, REG_INTR3); + + if (stat & BIT_DDC_CMD_DONE) { + sii8620_write(ctx, REG_INTR3_MASK, 0); + if (sii8620_is_mhl3(ctx)) + sii8620_mt_set_int(ctx, MHL_INT_REG(RCHANGE), + MHL_INT_RC_FEAT_REQ); + else + sii8620_edid_read(ctx, 0); + } + sii8620_write(ctx, REG_INTR3, stat); +} + /* endian agnostic, non-volatile version of test_bit */ static bool sii8620_test_bit(unsigned int nr, const u8 *addr) { @@ -1726,6 +1748,7 @@ static irqreturn_t sii8620_irq_thread(int irq, void *data) { BIT_FAST_INTR_STAT_MERR, sii8620_irq_merr }, { BIT_FAST_INTR_STAT_BLOCK, sii8620_irq_block }, { BIT_FAST_INTR_STAT_EDID, sii8620_irq_edid }, + { BIT_FAST_INTR_STAT_DDC, sii8620_irq_ddc }, { BIT_FAST_INTR_STAT_SCDT, sii8620_irq_scdt }, { BIT_FAST_INTR_STAT_INFR, sii8620_irq_infr }, };
MHL3 requires that after reading EDID from the sink source should ask peer for features. To make both protocols happy the patch splits the code accordingly. Signed-off-by: Andrzej Hajda <a.hajda@samsung.com> --- drivers/gpu/drm/bridge/sil-sii8620.c | 31 +++++++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-)