Re: [PATCH] drm/omap: dsi: Fix PM for display blank with paired dss_pll calls

[Tony Lindgren <tony@atomide.com>]

* Tomi Valkeinen <tomi.valkeinen@ti.com> [190206 16:09]:
> On 06/02/2019 18:00, Tony Lindgren wrote:
> 
> > OK I'll give it a try. Based on a quick glance, we need to still
> > check for enabled regulator to avoid unpaired calls.
> > 
> >>  static int dsi_dump_dsi_clocks(struct seq_file *s, void *p)
> >> @@ -4108,6 +4094,10 @@ static int dsi_display_init_dsi(struct dsi_data *dsi)
> >>  
> >>  	DSSDBG("PLL OK\n");
> >>  
> >> +	// XXX enable the regulator for the lanes
> >> +	regulator_enable(dsi->vdds_dsi_reg);
> >> +	dsi->vdds_dsi_enabled = true;
> >> +
> > 
> > So the above should only be done if !dsi->vdds_dsi_enabled?

Actually the conditional handling is only needed above. And
the regulator_enable needs error handling added that causes
some renumbering of the exit path.

> >>  	r = dsi_cio_init(dsi);
> >>  	if (r)
> >>  		goto err2;
> >> @@ -4136,6 +4126,10 @@ static int dsi_display_init_dsi(struct dsi_data *dsi)
> >>  err3:
> >>  	dsi_cio_uninit(dsi);
> >>  err2:
> >> +	// XXX disable the regulator for the lanes
> >> +	regulator_disable(dsi->vdds_dsi_reg);
> >> +	dsi->vdds_dsi_enabled = false;
> >> +
> > 
> > And here only if dsi->vdds_dsi_enabled?

On errors here we should just shut it down.

> >> @@ -4158,7 +4152,12 @@ static void dsi_display_uninit_dsi(struct dsi_data *dsi, bool disconnect_lanes,
> >>  
> >>  	dss_select_dsi_clk_source(dsi->dss, dsi->module_id, DSS_CLK_SRC_FCK);
> >>  	dsi_cio_uninit(dsi);
> >> -	dsi_pll_uninit(dsi, disconnect_lanes);
> >> +	dss_pll_disable(&dsi->pll);
> >> +
> >> +	if (disconnect_lanes) {
> >> +		regulator_disable(dsi->vdds_dsi_reg);
> >> +		dsi->vdds_dsi_enabled = false;
> >> +	}
> >>  }
> > 
> > Since they would be paired with the conditional handling
> > here?
> 
> Hmm, yes, I think you're right. And there's already one in dsi_remove(),
> which handles the final disable at unload time.

OK. Looks good to me otherwise.

So I guess we should fix. Do you want me to post it all
as a single patch after some testing?

It seems that we'll be breaking things one way or another
trying to do it in two patches :)

Regards,

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