On Wed, 2021-04-14 at 18:49 +0200, Thierry Reding wrote: > On Fri, Feb 19, 2021 at 04:52:59PM -0500, Lyude Paul wrote: > > As pointed out by the documentation for drm_dp_aux_register(), > > drm_dp_aux_init() should be used in situations where the AUX channel for a > > display driver can potentially be registered before it's respective DRM > > driver. This is the case with Tegra, since the DP aux channel exists as a > > platform device instead of being a grandchild of the DRM device. > > > > Since we're about to add a backpointer to a DP AUX channel's respective > > DRM > > device, let's fix this so that we don't potentially allow userspace to use > > the AUX channel before we've associated it with it's DRM connector. > > > > Signed-off-by: Lyude Paul <lyude@xxxxxxxxxx> > > --- > > drivers/gpu/drm/tegra/dpaux.c | 11 ++++++----- > > 1 file changed, 6 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/gpu/drm/tegra/dpaux.c b/drivers/gpu/drm/tegra/dpaux.c > > index 105fb9cdbb3b..ea56c6ec25e4 100644 > > --- a/drivers/gpu/drm/tegra/dpaux.c > > +++ b/drivers/gpu/drm/tegra/dpaux.c > > @@ -534,9 +534,7 @@ static int tegra_dpaux_probe(struct platform_device > > *pdev) > > dpaux->aux.transfer = tegra_dpaux_transfer; > > dpaux->aux.dev = &pdev->dev; > > > > - err = drm_dp_aux_register(&dpaux->aux); > > - if (err < 0) > > - return err; > > + drm_dp_aux_init(&dpaux->aux); > > I just noticed that this change causes an error on some setups that I > haven't seen before. The problem is that the SOR driver tries to grab a > reference to the I2C device to make sure it doesn't go away while it has > a pointer to it. > > However, since now the I2C adapter hasn't been registered yet, I get > this: > > [ 15.013969] kobject: '(null)' (000000005c903e43): is not > initialized, yet kobject_get() is being called. > > I recall that you wanted to make this change so that a backpointer to > the DRM device could be added (I think that's patch 15 of the series), > but I didn't see that patch get merged, so it's a bit difficult to try > and fix this up. I'm pretty sure I already merged the tegra change in drm-misc-next, so if it's causing issues you probably should send out a revert for now and I can r-b it so we can figure out a better solution for this in the mean time > Has the situation changed? Do we no longer need the backpointer? If we > still want it, what's the plan for merging the change? Should I work > under the assumption that patch will make it in sometime and try to fix > this on top of that? yes we do still need the backpointer - I'm just still working on getting reviews for some of the other parts of this series, and have been on PTO/busy with a couple of other things. > > I'm thinking that perhaps we can move the I2C adapter registration into > drm_dp_aux_init() since that's independent of the DRM device. Yeah this makes sense for me - I can try to make this change on the next respin of this series. What kind of setup were you able to reproduce issues on this with btw? > It would > also make a bit more sense from the Tegra driver's point of view where > all devices would be created during the ->probe() path, and only during > the ->init() path would the connection between DRM device and DRM DP AUX > device be established. > > Thierry -- Cheers, Lyude Paul (she/her) Software Engineer at Red Hat
