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

From: Andrzej Hajda
Date: Wed Nov 05 2014 - 11:00:59 EST


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@xxxxxxxxxx>
>>>
>>> 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.

My point is we do not need to make the whole tricky double refcounting,
with total redesign of panels, revoke, zombies, etc.... It is enough to
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.

Regards
Andrzej


>
>> What do you want to do with panel ops? Do they need support both life
>> states?
>
> That's a slightly separate issue for which David Herrmann had a solution
> in mind. As I understand it, the idea would be for these objects to gain
> revoke support. The goal is that once the underlying object disappears,
> calling any of the operations involved would fail (with something like a
> -ENODEV). It's a little more complicated than that because the device
> could disappear right in the middle of such an operation, but that's
> precisely what revoke should allow us to do. Readding David for
> visibility.
>
>> Anyway implementation currently seems to be broken,
>
> DRM panels are currently completely broken. If you remove the driver for
> a panel the display driver that uses this panel will simply crash the
> next time it tries to do anything with the panel. This type of registry
> is the first step in trying to fix this.
>
>> you try to refcount
>> objects which are usually embedded in driver priv data, which disappears
>> during remove callback of the driver.
>
> A second step in fixing this will be to convert drivers to no longer
> free the panel but rather drop their reference to the panel that they've
> registered. This will make sure that memory doesn't get freed until all
> references are gone.
>
> Note that this means that drivers will also need to be converted not to
> use devm_* helpers since that conflicts with the reference counted life-
> times of these record objects.
>
> Thierry
>
>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/