On Wed, Jun 27, 2018 at 2:48 PM, Doug Anderson <dianders@xxxxxxxxxxxx> wrote: > Hi, > > On Tue, Jun 26, 2018 at 10:27 AM, Rob Clark <robdclark@xxxxxxxxx> wrote: >> On Tue, Jun 26, 2018 at 11:55 AM, Doug Anderson <dianders@xxxxxxxxxxxx> wrote: >>> Hi, >>> >>> On Mon, Jun 25, 2018 at 9:45 PM, Sandeep Panda <spanda@xxxxxxxxxxxxxx> wrote: >>>> From: Abhinav Kumar <abhinavk@xxxxxxxxxxxxxx> >>>> >>>> Setting the DSI PLL src in probe doesn't provide the clock >>>> driver sufficient time to reclaim unused clock resources >>>> from coreboot resulting in warnings from clock driver. >>>> >>>> Move the DSI PLL src setting to modeset_init() so that the >>>> clock driver can claim unused display clock resources before >>>> the display driver requests for them again. >>> >>> IMHO this is a bad design. Sean and Stephen can feel free to override >>> me, but I think the clock driver should be improved to handle this >>> case and not require the clock to get disabled before Linux enables >>> it. >>> >> >> I experimented with it a while back[1] (in this case w/ lk lighting up >> the display). In that case I needed both the clk and gdsc code to >> realize that clks/gdsc's were on at boot, and fixup the refcnt of the >> clks (and parent clocks and so on). And then when probed the display >> driver would check if clocks were enabled to decide to readback the >> state from the hw. (Maybe you can short-circuit some of that if you >> only care about DSI panels with a single fixed resolution, but as soon >> as external displays come into the picture you can't assume so much >> about the hw state.) >> >> BR, >> -R >> >> [1] https://github.com/freedreno/kernel-msm/commits/display-handover > > It seems like something like that is the right real solution here, but > I guess someone needs to step up and work on it. > yeah, sadly I haven't found time to revisit it (kinda been short on time to work on display/kernel side of things.. it would be helpful if qcom would stop coming out with new gpu's so often :-P) But I am pretty sure if we want a general solution that can also deal w/ splash screen on hot-pluggable display, and not just the simpler case of fixed resolution dsi panels, it is going to require cooperation between clk/gdsc and display driver. > ...in general I'm wary of any patch that will not work when > "clk_ignore_unused" is passed as a command line parameter. It's > useful to be able to use this command line parameter for debugging > sometimes and it would be unfortunate if doing so spewed a bunch of > extra warnings. for "full distro" config where all the display drivers are built as modules and prior to real display driver loading you have kernel inheriting the display via efifb or simplefb, I think we need to be able to flag certain clocks and power domains (and on other platforms, perhaps regulators) as "if the bootloader left this on, keep it that way".. I guess for simple-fb we could perhaps solve it by attaching power domains and clks to the simple-fb node, but that doesn't really work for efifb.. BR, -R > > > On a side node, it appears that even without ${SUBJECT} patch the > warnings seem to have gone away with the latest stack of patches I've > been testing. The warnings don't even come back with > "clk_ignore_unused". I haven't personally dug into why, but I figured > I'd mention it. > > > -Doug -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html