On Tue, Sep 29, 2015 at 02:49:20PM +0200, Lukas Wunner wrote:
Hi Rafael,
On Mon, Sep 28, 2015 at 04:45:35PM -0700, Rafael Antognolli wrote:
This is useful to determine which connector owns this AUX channel.
WTF? I posted a patch in August which does exactly that: http://lists.freedesktop.org/archives/dri-devel/2015-August/088172.html
Can also be pulled in from this git repo: https://github.com/l1k/linux/commit/b78b38d53fc0fc4fa0f6acf699b0fcad56ec1fe6
My patch has the advantage that it updates all the drivers which use drm_dp_aux to fill that attribute. Yours only updates i915.
Daniel Vetter criticized storing a drm_connector pointer in drm_dp_aux, quote:
"That will also clear up the confusion with drm_dp_aux, adding a drm_connector there feels wrong since not every dp_aux line has a connector (e.g. for dp mst). If we can lift this relation out into drivers (where this is known) that seems cleaner."
So now Intel itself does precisely what Daniel criticized? Confusing!
Source: http://lists.freedesktop.org/archives/dri-devel/2015-August/089108.html
Critism is still valid, and thinking about this again a cleaner solution would be to just have a correct parent/child relationship in the device hirarchy. I.e. add a struct device *parent to the aux channel structure which should point to the right connector.
Thanks for pointing out that I missed properly delayering this. -Daniel
Best regards,
Lukas
Signed-off-by: Rafael Antognolli rafael.antognolli@intel.com
drivers/gpu/drm/i915/intel_dp.c | 1 + include/drm/drm_dp_helper.h | 1 + 2 files changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 77f7330..f90439d 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -1079,6 +1079,7 @@ intel_dp_aux_init(struct intel_dp *intel_dp, struct intel_connector *connector)
intel_dp->aux.name = name; intel_dp->aux.dev = dev->dev;
intel_dp->aux.connector = connector->base.kdev; intel_dp->aux.transfer = intel_dp_aux_transfer;
DRM_DEBUG_KMS("registering %s bus for %s\n", name,
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index 9ec4716..e009b5d 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -702,6 +702,7 @@ struct drm_dp_aux { const char *name; struct i2c_adapter ddc; struct device *dev;
- struct device *connector; struct mutex hw_mutex; ssize_t (*transfer)(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg);
-- 2.4.3
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel