From: "Ville Syrjälä" <ville.syrjala@linux.intel.com> To: Hans de Goede <hdegoede@redhat.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 16:02:52 +0200 [thread overview] Message-ID: <YFtGjHEdkMfR3bLr@intel.com> (raw) In-Reply-To: <d620fd9d-1685-3b2a-7c3b-a5d5fa6daddc@redhat.com> 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. > > > 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. > > 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. -- Ville Syrjälä Intel _______________________________________________ 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: "Ville Syrjälä" <ville.syrjala@linux.intel.com> To: Hans de Goede <hdegoede@redhat.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 16:02:52 +0200 [thread overview] Message-ID: <YFtGjHEdkMfR3bLr@intel.com> (raw) In-Reply-To: <d620fd9d-1685-3b2a-7c3b-a5d5fa6daddc@redhat.com> 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. > > > 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. > > 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. -- Ville Syrjälä Intel _______________________________________________ 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:02 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ä [this message] 2021-03-24 14:02 ` Ville Syrjälä 2021-03-24 14:10 ` Hans de Goede 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=YFtGjHEdkMfR3bLr@intel.com \ --to=ville.syrjala@linux.intel.com \ --cc=dri-devel@lists.freedesktop.org \ --cc=hdegoede@redhat.com \ --cc=intel-gfx@lists.freedesktop.org \ /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.