Re: [PATCH v3 3/8] drm/bridge/synopsys: dsi: add ability to check dsi-device attachment

[Heiko Stuebner <heiko@sntech.de>]

Hi Andrzej,

Am Montag, 13. August 2018, 12:23:21 CEST schrieb Andrzej Hajda:
> On 13.08.2018 10:44, Heiko Stuebner wrote:
> > Am Montag, 13. August 2018, 10:28:39 CEST schrieb Andrzej Hajda:
> >> On 09.07.2018 15:48, Heiko Stuebner wrote:
> >>> +bool dw_mipi_dsi_device_attached(struct dw_mipi_dsi *dsi)
> >>> +{
> >>> +	bool output;
> >>> +
> >>> +	mutex_lock(&dsi->panel_mutex);
> >>> +	output = !!dsi->panel_bridge;
> >>> +	mutex_unlock(&dsi->panel_mutex);
> >>> +
> >>> +	return output;
> >>> +}
> >>> +EXPORT_SYMBOL_GPL(dw_mipi_dsi_device_attached);
> >> The function does not make sense. After releasing panel_mutex device can
> >> be detached/(re-)attached multiple times. Ie it reports useless
> >> information. Of course in most cases it will work as expected, but for
> >> sure it is not bulletproof.
> > Ok. Can you suggest how we should check for the described case?
> > I.e. initially I tried to simply defer in dw_mipi_dsi_bridge_attach [0]
> > where you suggested to do that in probe.
> >
> > I moved the check to bind - see patch 5 - to fix the issue regarding
> > panel only probing after the dsi bus gets created, so this function
> > is a means to check if the panel has finished attaching, or to defer
> > binding - see dw_mipi_dsi_rockchip_bind in patch 5.
> >
> > So I'm somewhat out of ideas right now, how to do this right.
> 
> I am just after vacation, so please be kind if I write sth stupid :)
> 
> I would stick to the rule "do not expose functionality until you gather
> required resources".
> With this in mind I would try this way:
> 1. In bridge probe create mipi bus, but do not expose drm_bridge and do
> not call component_add - because we still do not have the sink
> (downstream panel or bridge).
> 2. In mipi_dsi_host_attach callback gather sink resource and then expose
> drm_bridge and the component (by calling component_add) - it will work
> with assumption the sink is registered/added before attaching to dsi bus

I think that is the general consensus in all kernel parts, like when you
request an irq, the driver should be able to handle it firing 
immediately, so similarly a dsi-sink should in a position to be directly
usable after it attached to the dsi host.


> [*].
> 3. Similar way it should be done in remove path.
> 
> This way in bind callback all resources should be there.

You're a genius ... That seems to work like a charm :-D .

v4 following shortly :-)


> [*]: This could be seen as sth against the rule "first resources, then
> exposition", but since panel/bridge framework does not provide
> notification about appearance of the objects, it works as a workaround
> for missing notification system.

Actually I'd say it follows that paradigm way better now, as the
component framework only tries to bring up the component hierarchy
after all resources are in place.

And now I even don't get any deferrals at all, where it was around 2
deferrals waiting for the panel before. So in terms of being deterministic
this feels like a plus :-)


Heiko


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel