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
