Hi John,
Thank you for the patch.
On Monday 16 Jan 2017 15:16:51 John Stultz wrote:
> Laurent: Would something like the following be preferred? Seems
> to work as well for me..
That looks good to me. Feel free to still de-duplicate the power on/off code
if you want (but of course without adding the regcache_sync to the common
power on function this time).
Please see below for an additional comment.
> I've found that by just turning the chip on and off via the
> POWER_DOWN register, I end up getting i2c_transfer errors on
> HiKey.
>
> Investigating further, it seems some of the register state in
> the regmap cache is somehow getting lost, likely as the device
> registers were reset during power off.
>
> Thus this patch simply re-writes the i2c address to the
> ADV7511_REG_EDID_I2C_ADDR register to ensure its properly set
> before we try to read the EDID data.
>
> Cc: David Airlie <airlied@xxxxxxxx>
> Cc: Archit Taneja <architt@xxxxxxxxxxxxxx>
> Cc: Wolfram Sang <wsa+renesas@xxxxxxxxxxxxxxxxxxxx>
> Cc: Lars-Peter Clausen <lars@xxxxxxxxxx>
> Cc: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx
> Signed-off-by: John Stultz <john.stultz@xxxxxxxxxx>
> ---
> drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index 405e460..32c59cb
> 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> @@ -567,6 +567,8 @@ static int adv7511_get_modes(struct adv7511 *adv7511,
>
> /* Reading the EDID only works if the device is powered */
> if (!adv7511->powered) {
> + unsigned int edid_i2c_addr = (adv7511->i2c_main->addr << 1) +
4;
> +
> regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER,
> ADV7511_POWER_POWER_DOWN, 0);
> if (adv7511->i2c_main->irq) {
> @@ -576,6 +578,9 @@ static int adv7511_get_modes(struct adv7511 *adv7511,
> ADV7511_INT1_DDC_ERROR);
> }
> adv7511->current_edid_segment = -1;
> +
> + /* Reset the EDID_I2C_ADDR register as it may have been
cleared */
> + regmap_write(adv7511->regmap, ADV7511_REG_EDID_I2C_ADDR,
edid_i2c_addr);
As powering the device off called regcache_mark_dirty(), this will perform an
I2C write and cache the value. If we then try to read the EDID a second time
without going through a full power on/off sequence, I believe that regmap will
skip the write the second time, as the cache will contain the same value and
won't be marked as dirty. Should we call regcache_mark_dirty() when powering
the device down further down this function ?
> }
>
> edid = drm_do_get_edid(connector, adv7511_get_edid_block, adv7511);
--
Regards,
Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
