Quoting Bjorn Andersson (2023-01-12 05:52:24) > Software normally uses the SW_COLLAPSE bit to collapse a GDSC, but in > some scenarios it's beneficial to let the hardware perform this without > software intervention. > > This is done by configuring the GDSC in "hardware control" state, in > which case the SW_COLLAPSE bit is ignored and some hardware signal is > relies upon instead. > > The GDSCs are modelled as power-domains in Linux and as such it's > reasonable to assume that the device drivers intend for the hardware > block to be accessible when their power domain is active. > > But in the current implementation, any GDSC that is marked to support > hardware control, gets hardware control unconditionally while the > client driver requests it to be active. It's therefor conceivable that > the hardware collapses a GDSC while Linux is accessing resources > depending on it. Why would software want the GDSC to be enabled and accessing resources while the hardware signals that it isn't required? It sounds like hardware control isn't complete? > > There are ongoing discussions about how to properly expose this control Any link? When we implemented hardware clk gating years ago the design was to have software override hardware control when the clk was enabled in software and let the hardware control go into effect when the clk was disabled in software. Hopefully with power domains this could be implemented in a better way by connecting hardware mode to some performance state so that enabling the power domain goes to software mode and then transitioning to a performance state switches to hardware control mode. > to the client drivers, but until conclusion in that discussion is > reached, the safer option would be to keep the GDSC in software control > mode. > > Signed-off-by: Bjorn Andersson <quic_bjorande@xxxxxxxxxxx> > --- > drivers/clk/qcom/gdsc.c | 48 ++++++----------------------------------- > 1 file changed, 7 insertions(+), 41 deletions(-) > > diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c > index 9e4d6ce891aa..6d3b36a52a48 100644 > --- a/drivers/clk/qcom/gdsc.c > +++ b/drivers/clk/qcom/gdsc.c > @@ -439,6 +398,13 @@ static int gdsc_init(struct gdsc *sc) > on = true; > } > > + /* Disable HW trigger mode until propertly supported */ > + if (sc->flags & HW_CTRL) { > + ret = gdsc_hwctrl(sc, false); > + if (ret < 0) > + return ret; > + } > + Is it a problem for all hardware controlled gdscs? Or just some of them? Should we backport this to stable kernels? I seem to recall that hardware mode was required for some drivers like camera and video? Are they going to keep working if we simply knock out the hardware control mode here?