From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751631AbdAPXg0 (ORCPT ); Mon, 16 Jan 2017 18:36:26 -0500 Received: from galahad.ideasonboard.com ([185.26.127.97]:52760 "EHLO galahad.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750884AbdAPXgX (ORCPT ); Mon, 16 Jan 2017 18:36:23 -0500 From: Laurent Pinchart To: John Stultz Cc: lkml , David Airlie , Archit Taneja , Wolfram Sang , Lars-Peter Clausen , dri-devel@lists.freedesktop.org Subject: Re: [RFC][PATCH] drm/bridge: adv7511: Re-write the i2c address as it may have been lost Date: Tue, 17 Jan 2017 01:36:37 +0200 Message-ID: <5310723.io2gjIHX6R@avalon> User-Agent: KMail/4.14.10 (Linux/4.8.6-gentoo; KDE/4.14.24; x86_64; ; ) In-Reply-To: <1484608611-10956-1-git-send-email-john.stultz@linaro.org> References: <1668382.5d9jBvLc1l@avalon> <1484608611-10956-1-git-send-email-john.stultz@linaro.org> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > Cc: Archit Taneja > Cc: Wolfram Sang > Cc: Lars-Peter Clausen > Cc: Laurent Pinchart > Cc: dri-devel@lists.freedesktop.org > Signed-off-by: John Stultz > --- > 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