Hi Rafael, On Thu, Dec 03, 2015 at 02:54:02PM -0800, Rafael Antognolli wrote: > So far, the i915 driver and some other drivers set it to the drm_device, > which doesn't allow one to know which DP a given aux channel is related > to. Changing this to be the drm_connector provides proper nesting, still > allowing one to get the drm_device from it. Some drivers already set it > to the drm_connector. > > This also removes the need to add a sysfs link for the i2c device under > the connector, as it will already be there. > > v2: Two minor nits, I think this should be "v9" instead of "v2" because this was newly added since v8, and the subject should be prefixed "drm/i915" (or "drm/i915/dp" or whatever) instead of "drm/dp" because this only touches i915 and not drm core. > - As a side effect, drm_dp_aux_unregister() must be called before > intel_connector_unregister(), as both the aux.dev and the i2c adapter > dev are children of the drm_connector device now. Calling > drm_dp_aux_unregister() before prevents them from being destroyed > twice. I haven't verified what Ville wrote (that there are WARNs because of this ordering issue), but if this is true then we need to make sure all other drivers get the order right, otherwise they'd spew WARNs as well once this gets merged. I've checked that for every driver and the only one affected is radeon. A fix is now in my GitHub repo, feel free to include the commit in your series if you want to. I haven't been able to actually test it though as I don't have a radeon card: https://github.com/l1k/linux/commit/485e197a7cac88e24348150526988149428feeaa > > Signed-off-by: Rafael Antognolli <rafael.antognolli@xxxxxxxxx> > --- > drivers/gpu/drm/i915/intel_dp.c | 22 ++++------------------ > 1 file changed, 4 insertions(+), 18 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index f2bfca0..515893c 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -1162,14 +1162,12 @@ static void intel_aux_reg_init(struct intel_dp *intel_dp) > static void > intel_dp_aux_fini(struct intel_dp *intel_dp) > { > - drm_dp_aux_unregister(&intel_dp->aux); > kfree(intel_dp->aux.name); > } Hm, instead of moving the single call of drm_dp_aux_unregister() to intel_dp_connector_unregister() I think it would be more clear to call intel_dp_aux_fini() from intel_dp_connector_unregister() and remove its invocation from intel_dp_encoder_destroy(). Ville introduced intel_dp_aux_fini() very recently with a121f4e5fae5, he should probably have a say on how to solve this. Kind regards, Lukas > > static int > intel_dp_aux_init(struct intel_dp *intel_dp, struct intel_connector *connector) > { > - struct drm_device *dev = intel_dp_to_dev(intel_dp); > struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); > enum port port = intel_dig_port->port; > int ret; > @@ -1180,7 +1178,7 @@ intel_dp_aux_init(struct intel_dp *intel_dp, struct intel_connector *connector) > if (!intel_dp->aux.name) > return -ENOMEM; > > - intel_dp->aux.dev = dev->dev; > + intel_dp->aux.dev = connector->base.kdev; > intel_dp->aux.transfer = intel_dp_aux_transfer; > > DRM_DEBUG_KMS("registering %s bus for %s\n", > @@ -1195,27 +1193,15 @@ intel_dp_aux_init(struct intel_dp *intel_dp, struct intel_connector *connector) > return ret; > } > > - ret = sysfs_create_link(&connector->base.kdev->kobj, > - &intel_dp->aux.ddc.dev.kobj, > - intel_dp->aux.ddc.dev.kobj.name); > - if (ret < 0) { > - DRM_ERROR("sysfs_create_link() for %s failed (%d)\n", > - intel_dp->aux.name, ret); > - intel_dp_aux_fini(intel_dp); > - return ret; > - } > - > return 0; > } > > static void > intel_dp_connector_unregister(struct intel_connector *intel_connector) > { > - struct intel_dp *intel_dp = intel_attached_dp(&intel_connector->base); > - > - if (!intel_connector->mst_port) > - sysfs_remove_link(&intel_connector->base.kdev->kobj, > - intel_dp->aux.ddc.dev.kobj.name); > + struct intel_dp *intel_dp = > + enc_to_intel_dp(&intel_connector->encoder->base); > + drm_dp_aux_unregister(&intel_dp->aux); > intel_connector_unregister(intel_connector); > } > > -- > 2.4.3 > > _______________________________________________ > dri-devel mailing list > dri-devel@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/dri-devel _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
