Hi John, On Monday 16 Jan 2017 12:14:48 John Stultz wrote: > On Mon, Jan 16, 2017 at 8:03 AM, Laurent Pinchart wrote: > > On Tuesday 03 Jan 2017 11:41:42 John Stultz wrote: > >> 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. Using the logic > >> in __adv7511_power_on/off() which syncs and dirtys the cache > >> avoids this issue. > >> > >> Thus this patch changes the EDID probing logic so that we > >> re-use the __adv7511_power_on/off() calls. > > > > regcache_sync() is quite costly as it will write a bunch of registers. > > Wouldn't it be more efficient to only write the registers that are needed > > for EDID access ? > > So yes, you've mentioned this concern before, and I did spend some > time to narrow which lost-register state (0x43 > - ADV7511_REG_EDID_I2C_ADDR) was causing the trouble with i2c > trasnfer errors I was seeing: > https://lkml.org/lkml/2016/11/22/677 > > However, I didn't get much feedback on that, and it seems (to me at > least) concerning that we are losing the underlying state of a > register in the cache, so just syncing that one register back to the > hardware might solve the issue I was seeing, but I worry what other > registers might also be out of sync. > > The comment above the regmap_sync in adv7511_power_on after all states: > "Most of the registers are reset during power down or when HPD is low."
You're right that most registers will be out of sync. > So it seems like if we're setting the power down (and setting HPD in > cases where Archit had a patch to add HPD pulsing to the > adv7511_get_modes path), it seems reasonable to do the same > regmap_sync()? It would be if we had to keep the device powered up, but we're powering it down right after reading the EDID. I don't think there's a need to reconfigure it completely, only setting the registers needed to read the EDID should be enough. > But, I'm not really picky here, and I'm very open to other approaches > (including something like the patch in the link above) if you have > suggestions/preferences. I just want it to work reliably on my > hardware. :) > > And just so I can better understand it, can you explain some about the > impact of your efficiency concerns? I'm not too picky either :-) If we can't find a reliable way to read the EDID by just configuring the registers we need, we could go for a full reconfiguration. However, restoring the value of all cached registers will result in lots of I2C writes, which are time-consuming operations. EDID read would be sped up if we could avoid that. -- Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel