[PATCH 2/4] drm/tegra: dsi: Clear enable register if powered by bootloader

[Diogo Ivo]

On Fri, Sep 30, 2022 at 01:11:10PM +0200, Thierry Reding wrote:
> On Thu, Sep 29, 2022 at 06:05:00PM +0100, Diogo Ivo wrote:
> > +
> >  	err = tegra_dsi_prepare(dsi);
> >  	if (err < 0) {
> >  		dev_err(dsi->dev, "failed to prepare: %d\n", err);
> > @@ -1573,6 +1600,8 @@ static int tegra_dsi_probe(struct platform_device *pdev)
> >  
> >  	dsi->output.connector.polled = DRM_CONNECTOR_POLL_HPD;
> >  
> > +	/* Check if the DSI module was left on by bootloader. */
> > +	dsi->enabled = of_property_read_bool(pdev->dev.of_node, "nvidia,boot-on");
> 
> The isn't a documented property. But before you go and add this, are
> there no alternative ways to detect that the DSI controller is active?
> Could we not read one of the registers to find out?

Hello, thank you for your feedback.

You are correct, it is possible to simply read a register to obtain
this information, and this property is not needed.

> DRM/KMS has built-in mechanisms to read back hardware state on boot, so
> I wonder if we can hook that up. It'd make the most sense if all sub-
> drivers did this, because then we could eventually inherit the
> bootloader configuration and transition to the kernel display driver
> seamlessly, but doing this in DSI first may help prepare for that more
> extended use-case.

I have only recently started digging in the DRM/KMS subsystem, could
you point out what those mechanisms are? That end goal seems like
something worth pursuing.

> A slightly simpler alternative would be to add the reset code to the
> encoder's or connector's ->reset() implementation. This is called at the
> right time (i.e. when the mode configuration is first reset), so you can
> run the workaround from tegra_dsi_encoder_enable() there. That's better
> than having this guarded by the dsi->enabled flag so that it is run only
> once.
> 
> Thierry

Regarding the placement of the workaround, I placed it in encoder_enable()
since my attempts of placing it in other functions (such as the connector's
->reset() method) resulted in a kernel hang, and I have no solution for this.
I'm assuming this is due to some part of the DSI hardware not being fully
initialized, but I haven't been able to confirm this.

Best regards,

Diogo