Re: [v3 3/6] drm/vs: Register DRM device

[mripard@xxxxxxxxxx]

Hi,

On Mon, Dec 11, 2023 at 05:00:04PM +0800, Keith Zhao wrote:
> >> +static int vs_drm_device_init_clocks(struct vs_drm_device *priv)
> >> +{
> >> +	struct drm_device *dev = &priv->base;
> >> +	struct platform_device *pdev = to_platform_device(dev->dev);
> >> +	struct device_node *of_node = pdev->dev.of_node;
> >> +	struct clk *clock;
> >> +	unsigned int i;
> >> +	int ret;
> >> +
> >> +	if (dev_get_platdata(&pdev->dev) || !of_node)
> >> +		return 0;
> >> +
> >> +	priv->nrsts = ARRAY_SIZE(priv->rst_vout);
> >> +	for (int i = 0; i < priv->nrsts; ++i)
> >> +		priv->rst_vout[i].id = vout_resets[i];
> >> +	ret = devm_reset_control_bulk_get_shared(dev->dev, priv->nrsts,
> >> +						 priv->rst_vout);
> > 
> > I would request resets and clocks in _probe().
> 
> > 
> > If component_bind_all() returns -EPROBE_DEFER because of a still
> > missing DSI panel backlight or similar, this doesn't have to be done
> > multiple times.
> I got what you mean. component_bind_all should be done multiple times
>  to prevent the dsi panel driver from lagging load.

No. component_bind_all only needs to be called once.

> in my drm subsystem , there are 2 pipeline 
> 
>           +------------------------------+
>           |                              |
>           |                              |
>   +----+  |   +-------------------+      |   +-------+   +------+   +------+
>   |    +----->+  dc controller 0  +--->----->+HDMICtl| ->+ PHY  +-->+PANEL0+
>   |AXI |  |   +-------------------+      |   +-------+   +------+   +------+
>   |    |  |                              |
>   |    |  |                              |
>   |    |  |                              |
>   |    |  |                              |
>   |APB |  |   +-------------------+         +---------+    +------+  +-------+
>   |    +----->+  dc controller 1  +--->---->+ dsiTx   +--->+DPHY  +->+ PANEL1+
>   |    |  |   +-------------------+         +---------+    +------+  +-------+
>   +----+  |                              |
>           +------------------------------+
> 
> 
> component_bind_all will bind the hdmi encoder and dsi encoder .
> binding the hdmi encoder will always return ok .
> 
> binging the dsi encoder has a question :
> I used the panel-raspberrypi-touchscreen.c as panel driver , 
> this driver is a i2c device and it use a i2c command to read reg ID
> if read success , it will do drm_panel_add. 
> 
> if I disconnect the panel ,it will not do drm_panel_add.
> dsiTx will fail to find panel , The consequence is that the inputbridge cannot be created , 
> also outputbridge cannot be created.
> for encoder bind , it will fail to find the input bridge of dsi.
> Under this premise, although returning -EPROBE_DEFER allows bind to be executed multiple times, 
> the final result is that the entire bind fails.
> 
> returning -EPROBE_DEFER can solve panel driver from lagging load , 
> but for no panel case , it will destory all pipeline (include hdmi and dsi).

Yes, that's expected.

> I did two things:
> late_initcall_sync(vs_drm_init); to make sure the panel drive has been probed;
> dsi encoder bind always return ok to make sure hdmi pipeline ok at lease.
> component_bind_all do once . 

You should have a look at
https://www.kernel.org/doc/html/latest/gpu/drm-kms-helpers.html#special-care-with-mipi-dsi-bridges

Maxime

Attachment: signature.asc
Description: PGP signature