Re: [RFC 1/2] core: Add generic object registry implementation

[Andrzej Hajda <a.hajda@samsung.com>]

On 11/06/2014 10:48 AM, Thierry Reding wrote:
> On Wed, Nov 05, 2014 at 05:00:47PM +0100, Andrzej Hajda wrote:
>> On 11/05/2014 03:04 PM, Thierry Reding wrote:
>>> On Wed, Nov 05, 2014 at 01:36:24PM +0100, Andrzej Hajda wrote:
>>>> On 11/04/2014 05:29 PM, Thierry Reding wrote:
>>>>> From: Thierry Reding <treding@nvidia.com>
>>>>>
>>>>> Add a generic implementation of an object registry. This targets drivers
>>>>> and subsystems that provide auxiliary objects that other drivers need to
>>>>> look up. The goal is to put the difficult parts (keep object references,
>>>>> module usage count, ...) into core code so that individual subsystems do
>>>>> not have to deal with them.
>>>>>
>>>>> The intention is for subsystems to instantiate a struct registry and use
>>>>> a struct registry_record embedded into a subsystem-specific structure to
>>>>> provide a subsystem-specific API around that.
>>>>
>>>>
>>>> As I understand you want to use this registry for panels and bridges.
>>>> Could you explain the idea and describe example scenario when these
>>>> refcountings are useful. I guess it should be when panel attached to
>>>> drmdrv want to disappear.
>>>
>>> Correct. When a panel driver is unloaded it frees memory associated with
>>> the panel. The goal of this registry is for the panel object to stay
>>> around until all references are gone.
>>>
>>>> Real lifetime of panel is limited by probe/remove callbacks of panel
>>>> driver, do you want to prolong it behind these limits?
>>>
>>> Yes.
>>>
>>>> Do you want to have zombie panels, without hardware they abstract? For
>>>> what purpose?
>>>
>>> So that display drivers don't try to access objects that have been
>>> freed.
>>
>> Why do not just release panel references from drm_dev, I have
>> successfully implemented dsi panels this way, thanks to dsi bus specific
>> attach/detach callbacks and drm hotplug mechansim.
> 
> Like you say yourself, that's something that work only for DSI. Any
> other type of panel can't do this.

But it means that if we want to make panels safe we just need add
registration/deregistration notifications to panels, nothing more.


> 
>> My point is we do not need to make the whole tricky double refcounting,
> 
> There's no double refcounting. We have no refcounting at all at the
> moment.

For me registry_record.kref and try_module_get sounds like refcounting.

> 
>> with total redesign of panels, revoke, zombies, etc.... It is enough to
> 
> It's not a total redesign. It just makes it more mature and implements
> features that I think are useful (and needed) but that were left out for
> the sake of simplicity. Now it turns out that this is actually quite
> fragile and easy to get wrong.

And I try to convince you we can still keep simplicity and make it safe.

> 
>> have just hot plug/unplug callbacks. This is why I have proposed few
>> months ago interface_tracker framework. It can add hot(un)plug
>> capability in a generic way to any framework.
> 
> That's something that this object registry could easily implement as
> well. But instead of passing around void * and type IDs as in the
> interface tracker it could deal with real objects for proper type-
> safety.

It is not a problem to add type-safe helpers to interface tracker.

Regards
Andrzej

> 
> Thierry
> 
> 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>