Nouveau - Apr 2021 - [PATCH v3 03/20] drm/dp: Move i2c init to drm_dp_aux_init, add __must_check and fini

OK - talked with Ville a bit on this and did some of my own research, I
actually think that moving i2c to drm_dp_aux_init() is the right decision for
the time being. The reasoning behind this being that as shown by my previous
work of fixing drivers that call drm_dp_aux_register() too early - it seems
like there's already been drivers that have been working just fine with
setting up the i2c device before DRM registration. 

In the future, it'd probably be better if we can split up i2c_add_adapter()
into an init and register function - but we'll have to talk with the i2c
maintainers to see if this is acceptable w/ them

On Thu, 2021-04-22 at 13:18 -0400, Lyude Paul wrote:
> On Tue, 2021-04-20 at 02:16 +0300, Ville Syrj?l? wrote:
> > 
> > The init vs. register split is intentional. Registering the thing
> > and allowing userspace access to it before the rest of the driver
> > is ready isn't particularly great. For a while now we've tried
to
> > move towards an architecture where the driver is fully initialzied
> > before anything gets exposed to userspace.
> 
> Yeah-thank you for pointing this out. Thierry - do you think there's an
> alternate solution we could go with in Tegra to fix the get_device() issue
> that wouldn't require us trying to expose the i2c adapter early?
> 
> > 
> 
-- 
Cheers,
 Lyude Paul (she/her)
 Software Engineer at Red Hat

On Thu, 2021-04-22 at 18:33 -0400, Lyude Paul wrote:
> OK - talked with Ville a bit on this and did some of my own research, I
> actually think that moving i2c to drm_dp_aux_init() is the right decision
> for
> the time being. The reasoning behind this being that as shown by my
previous
> work of fixing drivers that call drm_dp_aux_register() too early - it seems
> like there's already been drivers that have been working just fine with
> setting up the i2c device before DRM registration. 
> 
> In the future, it'd probably be better if we can split up
i2c_add_adapter()
> into an init and register function - but we'll have to talk with the
i2c
> maintainers to see if this is acceptable w/ them
Actually - I think adding the ability to refcount dp aux adapters might be a
better solution so I'm going to try that!
> 
> On Thu, 2021-04-22 at 13:18 -0400, Lyude Paul wrote:
> > On Tue, 2021-04-20 at 02:16 +0300, Ville Syrj?l? wrote:
> > > 
> > > The init vs. register split is intentional. Registering the thing
> > > and allowing userspace access to it before the rest of the driver
> > > is ready isn't particularly great. For a while now we've
tried to
> > > move towards an architecture where the driver is fully
initialzied
> > > before anything gets exposed to userspace.
> > 
> > Yeah-thank you for pointing this out. Thierry - do you think
there's an
> > alternate solution we could go with in Tegra to fix the get_device()
issue
> > that wouldn't require us trying to expose the i2c adapter early?
> > 
> > > 
> > 
> 
-- 
Cheers,
 Lyude Paul (she/her)
 Software Engineer at Red Hat

On Thu, Apr 22, 2021 at 06:33:44PM -0400, Lyude Paul
wrote:
> OK - talked with Ville a bit on this and did some of my own research, I
> actually think that moving i2c to drm_dp_aux_init() is the right decision
for
> the time being. The reasoning behind this being that as shown by my
previous
> work of fixing drivers that call drm_dp_aux_register() too early - it seems
> like there's already been drivers that have been working just fine with
> setting up the i2c device before DRM registration. 
> 
> In the future, it'd probably be better if we can split up
i2c_add_adapter()
> into an init and register function - but we'll have to talk with the
i2c
> maintainers to see if this is acceptable w/ them
Yeah, that sounds like a better long-term solution. We could leave
i2c_add_adapter() in place, since it's already half-way split up into
some initialization code and i2c_register_adapter(), so it shouldn't be
all that difficult to split out an i2c_init_adapter() so that outside
users can do the split setup.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL:
<https://lists.freedesktop.org/archives/nouveau/attachments/20210423/4bbd0088/attachment-0001.sig>