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@xxxxxx>
>> Reviewed-by: Andrzej Hajda <a.hajda@xxxxxxxxxxx>
>> ---
>> 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;
>> }
>>
>
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
