From: Hans de Goede <hdegoede@redhat.com> To: "Ville Syrjälä" <ville.syrjala@linux.intel.com> Cc: intel-gfx <intel-gfx@lists.freedesktop.org>, dri-devel@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH resend 2/2] drm/i915/display: Make vlv_find_free_pps() skip pipes which are in use for non DP purposes Date: Wed, 24 Mar 2021 15:10:59 +0100 [thread overview] Message-ID: <7d9bb155-5e07-161d-c699-581d89b9fb39@redhat.com> (raw) In-Reply-To: <YFtGjHEdkMfR3bLr@intel.com> Hi, On 3/24/21 3:02 PM, Ville Syrjälä wrote: > On Tue, Mar 23, 2021 at 11:39:09AM +0100, Hans de Goede wrote: >> Hi, >> >> On 3/2/21 3:51 PM, Ville Syrjälä wrote: >>> On Tue, Mar 02, 2021 at 01:00:40PM +0100, Hans de Goede wrote: >>>> As explained by a long comment block, on VLV intel_setup_outputs() >>>> sometimes thinks there might be an eDP panel connected while there is none. >>>> In this case intel_setup_outputs() will call intel_dp_init() to check. >>>> >>>> In this scenario vlv_find_free_pps() ends up selecting pipe A for the pps, >>>> even though this might be in use for non DP purposes. When this is the case >>>> then the assert_pipe() in vlv_force_pll_on() will fail when called from >>>> vlv_power_sequencer_kick(). >>> >>> The idea is that you *can* select a PPS from a pipe used for a non-DP >>> port since those don't care about the PPS stuff. So this doesn't seem >>> correct. >> >> They may not care about the PPS stuff, but as the WARN / backtrace >> shows if the DPLL_VCO_ENABLE bit is not already set for the pipe, while >> the pipe is "otherwise" in use then vlv_force_pll_on() becomes unhappy >> triggering the WARN.DPLL_VCO_ENABLE bit is not >> >>> a) I would like to see the VBT for this machine >> >> https://fedorapeople.org/~jwrdegoede/voyo-winpad-a15-vbt >> >>> b) I wonder if the DSI PLL is sufficient for getting the PPS going? >> >> I have no idea, I just noticed the WARN / backtrace and this seemed >> like a reasonably way to deal with it. With that said I'm fine with fixing >> this a different way. >> >>> c) If we do need the normal DPLL is there any harm to DSI in enabling it? >> >> I would assume this increases power-consumption and DSI panels are almost >> always used in battery powered devices. > > This is just used while probing the panel, so power consumption is > not a concern. Sorry I misinterpreted what you wrote, I interpreted it as have the DSI code enable it to avoid this problem. I see now that that is now what you meant. >> Also this would impact all BYT/CHT devices, possible triggering unwanted >> side-effects. Where as the proposed fix below is much more narrowly targeted >> at the problem. It might not be the most pretty fix but AFAICT it has a low >> risk of causing regressions. > > It rather significantly changes the logic of the workaround, potentially > causing us to not find a free PPS at all. Eg. if you were to boot with > a VLV with pipe A -> eDP B + eDP C inactive + pipe B -> VGA then your > change would cause us to not find the free pipe B PPS for probing eDP C, > and in the end we'd get a WARN and fall back to pipe A PPS which would > clobber the actually in use pipe A PPS. I would welcome, and will happily test, another fix for this. ATM we have a WARN triggering on actual hardware (and not just in a hypothetical example) and I would like to see that WARN fixed. If you can come up with a better fix I would be happy to test. Regards, Hans _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
WARNING: multiple messages have this Message-ID (diff)
From: Hans de Goede <hdegoede@redhat.com> To: "Ville Syrjälä" <ville.syrjala@linux.intel.com> Cc: intel-gfx <intel-gfx@lists.freedesktop.org>, dri-devel@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH resend 2/2] drm/i915/display: Make vlv_find_free_pps() skip pipes which are in use for non DP purposes Date: Wed, 24 Mar 2021 15:10:59 +0100 [thread overview] Message-ID: <7d9bb155-5e07-161d-c699-581d89b9fb39@redhat.com> (raw) In-Reply-To: <YFtGjHEdkMfR3bLr@intel.com> Hi, On 3/24/21 3:02 PM, Ville Syrjälä wrote: > On Tue, Mar 23, 2021 at 11:39:09AM +0100, Hans de Goede wrote: >> Hi, >> >> On 3/2/21 3:51 PM, Ville Syrjälä wrote: >>> On Tue, Mar 02, 2021 at 01:00:40PM +0100, Hans de Goede wrote: >>>> As explained by a long comment block, on VLV intel_setup_outputs() >>>> sometimes thinks there might be an eDP panel connected while there is none. >>>> In this case intel_setup_outputs() will call intel_dp_init() to check. >>>> >>>> In this scenario vlv_find_free_pps() ends up selecting pipe A for the pps, >>>> even though this might be in use for non DP purposes. When this is the case >>>> then the assert_pipe() in vlv_force_pll_on() will fail when called from >>>> vlv_power_sequencer_kick(). >>> >>> The idea is that you *can* select a PPS from a pipe used for a non-DP >>> port since those don't care about the PPS stuff. So this doesn't seem >>> correct. >> >> They may not care about the PPS stuff, but as the WARN / backtrace >> shows if the DPLL_VCO_ENABLE bit is not already set for the pipe, while >> the pipe is "otherwise" in use then vlv_force_pll_on() becomes unhappy >> triggering the WARN.DPLL_VCO_ENABLE bit is not >> >>> a) I would like to see the VBT for this machine >> >> https://fedorapeople.org/~jwrdegoede/voyo-winpad-a15-vbt >> >>> b) I wonder if the DSI PLL is sufficient for getting the PPS going? >> >> I have no idea, I just noticed the WARN / backtrace and this seemed >> like a reasonably way to deal with it. With that said I'm fine with fixing >> this a different way. >> >>> c) If we do need the normal DPLL is there any harm to DSI in enabling it? >> >> I would assume this increases power-consumption and DSI panels are almost >> always used in battery powered devices. > > This is just used while probing the panel, so power consumption is > not a concern. Sorry I misinterpreted what you wrote, I interpreted it as have the DSI code enable it to avoid this problem. I see now that that is now what you meant. >> Also this would impact all BYT/CHT devices, possible triggering unwanted >> side-effects. Where as the proposed fix below is much more narrowly targeted >> at the problem. It might not be the most pretty fix but AFAICT it has a low >> risk of causing regressions. > > It rather significantly changes the logic of the workaround, potentially > causing us to not find a free PPS at all. Eg. if you were to boot with > a VLV with pipe A -> eDP B + eDP C inactive + pipe B -> VGA then your > change would cause us to not find the free pipe B PPS for probing eDP C, > and in the end we'd get a WARN and fall back to pipe A PPS which would > clobber the actually in use pipe A PPS. I would welcome, and will happily test, another fix for this. ATM we have a WARN triggering on actual hardware (and not just in a hypothetical example) and I would like to see that WARN fixed. If you can come up with a better fix I would be happy to test. Regards, Hans _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2021-03-24 14:11 UTC|newest] Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-03-02 12:00 [PATCH resend 0/2] drm/i915/display: Make vlv_find_free_pps() skip pipes which are in use for non DP purposes Hans de Goede 2021-03-02 12:00 ` [Intel-gfx] " Hans de Goede 2021-03-02 12:00 ` [PATCH resend 1/2] drm/i915/display: Add a intel_pipe_is_enabled() helper Hans de Goede 2021-03-02 12:00 ` [Intel-gfx] " Hans de Goede 2021-03-02 12:46 ` Jani Nikula 2021-03-02 12:46 ` [Intel-gfx] " Jani Nikula 2021-03-02 12:00 ` [PATCH resend 2/2] drm/i915/display: Make vlv_find_free_pps() skip pipes which are in use for non DP purposes Hans de Goede 2021-03-02 12:00 ` [Intel-gfx] " Hans de Goede 2021-03-02 12:54 ` Jani Nikula 2021-03-02 12:54 ` [Intel-gfx] " Jani Nikula 2021-03-02 14:51 ` Ville Syrjälä 2021-03-02 14:51 ` [Intel-gfx] " Ville Syrjälä 2021-03-23 10:39 ` Hans de Goede 2021-03-23 10:39 ` Hans de Goede 2021-03-24 14:02 ` Ville Syrjälä 2021-03-24 14:02 ` Ville Syrjälä 2021-03-24 14:10 ` Hans de Goede [this message] 2021-03-24 14:10 ` Hans de Goede 2021-03-24 14:37 ` Ville Syrjälä 2021-03-24 14:37 ` Ville Syrjälä 2021-05-04 10:52 ` Ville Syrjälä 2021-05-04 10:52 ` Ville Syrjälä 2021-03-02 13:08 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for " Patchwork 2021-03-02 13:33 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=7d9bb155-5e07-161d-c699-581d89b9fb39@redhat.com \ --to=hdegoede@redhat.com \ --cc=dri-devel@lists.freedesktop.org \ --cc=intel-gfx@lists.freedesktop.org \ --cc=ville.syrjala@linux.intel.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.