| Message ID | 08d50e5eec760273a3b3699ea98732f5ec66ad25.1551303673.git.jsarha@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/bridge: sii902x: HDMI-audio support and some fixes | expand |
Hi Jyri, Thank you for the patch. On Wed, Feb 27, 2019 at 11:54:20PM +0200, Jyri Sarha wrote: > Set output mode to HDMI or DVI according to EDID HDMI signature. > > Signed-off-by: Jyri Sarha <jsarha@ti.com> > Reviewed-by: Andrzej Hajda <a.hajda@samsung.com> > --- > drivers/gpu/drm/bridge/sii902x.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c > index 1afa000141d5..0e21fa419d27 100644 > --- a/drivers/gpu/drm/bridge/sii902x.c > +++ b/drivers/gpu/drm/bridge/sii902x.c > @@ -181,11 +181,15 @@ static int sii902x_get_modes(struct drm_connector *connector) > struct sii902x *sii902x = connector_to_sii902x(connector); > u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24; > struct edid *edid; > + u8 output_mode = SII902X_SYS_CTRL_OUTPUT_DVI; I'd move this one line up, but that's certainly a matter of taste :-) > int num = 0, ret; > > edid = drm_get_edid(connector, sii902x->i2cmux->adapter[0]); > drm_connector_update_edid_property(connector, edid); > if (edid) { > + if (drm_detect_hdmi_monitor(edid)) > + output_mode = SII902X_SYS_CTRL_OUTPUT_HDMI; > + > num = drm_add_edid_modes(connector, edid); > kfree(edid); > } > @@ -195,6 +199,11 @@ static int sii902x_get_modes(struct drm_connector *connector) > if (ret) > return ret; > > + ret = regmap_update_bits(sii902x->regmap, SII902X_SYS_CTRL_DATA, > + SII902X_SYS_CTRL_OUTPUT_MODE, output_mode); > + if (ret) > + return ret; > + Is this the right place to update the register, shouldn't this be done in sii902x_bridge_enable() instead ? I could foresee cases where the chip could be powered down between get_modes() and enable(), losing its internal state. > return num; > } >
On 04/03/2019 14:52, Laurent Pinchart wrote: > Hi Jyri, > > Thank you for the patch. > On Wed, Feb 27, 2019 at 11:54:20PM +0200, Jyri Sarha wrote: >> Set output mode to HDMI or DVI according to EDID HDMI signature. >> >> Signed-off-by: Jyri Sarha <jsarha@ti.com> >> Reviewed-by: Andrzej Hajda <a.hajda@samsung.com> >> --- >> drivers/gpu/drm/bridge/sii902x.c | 9 +++++++++ >> 1 file changed, 9 insertions(+) >> >> diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c >> index 1afa000141d5..0e21fa419d27 100644 >> --- a/drivers/gpu/drm/bridge/sii902x.c >> +++ b/drivers/gpu/drm/bridge/sii902x.c >> @@ -181,11 +181,15 @@ static int sii902x_get_modes(struct drm_connector *connector) >> struct sii902x *sii902x = connector_to_sii902x(connector); >> u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24; >> struct edid *edid; >> + u8 output_mode = SII902X_SYS_CTRL_OUTPUT_DVI; > > I'd move this one line up, but that's certainly a matter of taste :-) I usually sort the local variables by length too. I wonder why I did not do it this time... I'll fix it :). >> int num = 0, ret; >> >> edid = drm_get_edid(connector, sii902x->i2cmux->adapter[0]); >> drm_connector_update_edid_property(connector, edid); >> if (edid) { >> + if (drm_detect_hdmi_monitor(edid)) >> + output_mode = SII902X_SYS_CTRL_OUTPUT_HDMI; >> + >> num = drm_add_edid_modes(connector, edid); >> kfree(edid); >> } >> @@ -195,6 +199,11 @@ static int sii902x_get_modes(struct drm_connector *connector) >> if (ret) >> return ret; >> >> + ret = regmap_update_bits(sii902x->regmap, SII902X_SYS_CTRL_DATA, >> + SII902X_SYS_CTRL_OUTPUT_MODE, output_mode); >> + if (ret) >> + return ret; >> + > > Is this the right place to update the register, shouldn't this be done > in sii902x_bridge_enable() instead ? I could foresee cases where the > chip could be powered down between get_modes() and enable(), losing its > internal state. > I have a spec (unfortunately I can not share it) that describes the sequence of handling a hot plug event on sii9022. There it is said that the HDMI mode, if HDMI signature is found, should be set at the same time while releasing the DCC access by setting SII902X_SYS_CTRL_DATA bits #1 and #2 to zero. However, I did not dare to change sii902x_i2c_bypass_deselect() function, so I set the HDMI mode right after the DCC pass trough mode is disabled. Having it there is logical too, since the HDMI/DVI-mode should not change without a hot plug event. In practice sii9022 does not appear to be too picky when the bit is set. >> return num; >> } >> >
diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c index 1afa000141d5..0e21fa419d27 100644 --- a/drivers/gpu/drm/bridge/sii902x.c +++ b/drivers/gpu/drm/bridge/sii902x.c @@ -181,11 +181,15 @@ static int sii902x_get_modes(struct drm_connector *connector) struct sii902x *sii902x = connector_to_sii902x(connector); u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24; struct edid *edid; + u8 output_mode = SII902X_SYS_CTRL_OUTPUT_DVI; int num = 0, ret; edid = drm_get_edid(connector, sii902x->i2cmux->adapter[0]); drm_connector_update_edid_property(connector, edid); if (edid) { + if (drm_detect_hdmi_monitor(edid)) + output_mode = SII902X_SYS_CTRL_OUTPUT_HDMI; + num = drm_add_edid_modes(connector, edid); kfree(edid); } @@ -195,6 +199,11 @@ static int sii902x_get_modes(struct drm_connector *connector) if (ret) return ret; + ret = regmap_update_bits(sii902x->regmap, SII902X_SYS_CTRL_DATA, + SII902X_SYS_CTRL_OUTPUT_MODE, output_mode); + if (ret) + return ret; + return num; }