Hi,

Am Montag, 20. August 2018, 12:34:37 CEST schrieb Andrzej Hajda:

On 14.08.2018 12:26, Heiko Stuebner wrote:

quoted

With the regular means of adding the dsi-component in probe it creates
a race condition with the panel probing, as the panel device only gets
created after the dsi-bus got created.

When the panel-driver is build as a module it currently fails hard as the
panel cannot be probed directly:

dw_mipi_dsi_bind()
  __dw_mipi_dsi_probe()
    creates dsi bus
    creates panel device
    triggers panel module load
    panel not probed (module not loaded or panel probe slow)
  drm_bridge_attach
    fails with -EINVAL due to empty panel_bridge

Additionally the panel probing can run concurrently with dsi bringup
making it possible that the panel can already be found but dsi-attach
hasn't finished running.

To solve that cleanly we may want to only create the component after
the panel has finished probing, by calling component_add from the
host-attach dsi callback.

As that is specific to glue drivers, add a new struct for host_ops
so that glue drivers can tell the bridge to call specific functions
after the common host-attach and before the common host-detach run.

Sometimes I have an impression that core/glue driver architecture with
callbacks to glue drivers is quite complicated, and smells mid-layer
mistake :), I wonder if simple bunch of helpers with some base object
wouldn't be better, but this is subject for other discussion.

quoted

Suggested-by: Andrzej Hajda <redacted>
Signed-off-by: Heiko Stuebner <heiko@sntech.de>

quoted

@@ -300,6 +301,12 @@ static int dw_mipi_dsi_host_attach(struct mipi_dsi_host *host,
 
 	drm_bridge_add(&dsi->bridge);
 
+	if (pdata->host_ops && pdata->host_ops->attach) {
+		ret = pdata->host_ops->attach(pdata->priv_data, device);
+		if (ret < 0)
+			return ret;

It could be replaced by:
    return pdata->host_ops->attach(pdata->priv_data, device);

But no strong feelings. With or without the change:

I'll keep it the way it is then ;-) .

My rationale is that the "return 0" below is the last line of this
function and returns the success.

The specialized attach is only called in a fraction of instances, so
having a return on success in there, might be surprising if later on
additional code gets added between the attach call and the return below
and might be missed to modify in that case, possibly causing breakage.

quoted

+	}
+
 	return 0;
 }

Heiko


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