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

From: mripard@xxxxxxxxxx
Date: Mon Dec 11 2023 - 04:17:20 EST


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