On Tue, Dec 17, 2013 at 07:11:21PM +0200, Jani Nikula wrote: > On Tue, 17 Dec 2013, Thierry Reding <thierry.reding@xxxxxxxxx> wrote: [...] > > + case DP_AUX_NATIVE_REPLY_DEFER: > > + /* > > + * We could check for I2C bitrate capabilities and if > > + * available adjust this interval. We could also be > > + * more careful with DP-to-legacy adapters where a > > + * long legacy cable may forc very low I2C bit rates. > ^^^^ force Fixed. > > + * For now just defer for long enough to hopefully be > > + * safe for all use-cases. > > + */ > > + usleep_range(500, 600); > > I've banged my head against the wall a bit with this (the original i915 > comment similar to the above was mine) and I'd be hesitant to switch to > this as-is in i915. > > First, I'd like to retain the DRM_DEBUG_KMS messages all around, as > otherwise issues here will be hard to debug. It's a pain to have to ask > to patch the kernel just get the debug info. At least we've had a fair > share of DP aux issues in our driver. Okay, I'll add those back in. I'll assume the same goes for DRM_ERROR messages. > Second, I fear one size does not fit all wrt the delays. What would you > say about making all the DP_AUX_NATIVE_REPLY_DEFER and > DP_AUX_I2C_REPLY_DEFER checks do callbacks if defined, and fallback to > the default delays otherwise? I think there'll be some more churn in the > error handling, particularly for the more exotic sinks, and IMO having > them per driver would make development and bug fixing faster. I'm not > saying you need to do this now, it can be a follow-up; just asking what > you'd think of it. Sure, we should be able to easily do that. One possible alternative to this would be to handle in in the drivers by handling DEFER explicitly and then returning -EBUSY. -EBUSY will cause native AUX transactions to be retried and I just noticed that radeon does the same for I2C-over-AUX transactions, so I'll add that special case there as well. The benefit of doing that would be that the helper stays relatively simple. On the downside it may not be as explicit as a driver-specific callback. Thanks, Thierry
Attachment:
pgpg8RUDVwImi.pgp
Description: PGP signature
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
