From: Jani Nikula <jani.nikula@linux.intel.com>
To: Ville Syrjala <ville.syrjala@linux.intel.com>,
intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH v3 05/22] drm/i915: Introduce intel_hpd_detection()
Date: Tue, 28 Feb 2023 14:40:20 +0200 [thread overview]
Message-ID: <87k002ynd7.fsf@intel.com> (raw)
In-Reply-To: <20230221230227.6244-6-ville.syrjala@linux.intel.com>
On Wed, 22 Feb 2023, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Add a mechanism by which we can toggle the HPD sense for
> individual encoders.
>
> This will be used during eDP probing to figure out if
> anything is actually connected. The normal intel_hpd_irq_setup()
> thing doesn't work since we only do that after probing the
> outputs, and we only enable HPD sense for encoders that were
> successfully probed.
>
> The other idea that crossed my minds was to just turn on
> HPD sense for all pins before output probing and let hpd_irq_setup()
> clean it up afterwards. But that doesn't work for BXT/GLK where
> the HPD invert information comes from the VBT child device.
> So looks like this really needs to be per-encoder.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_irq.c | 132 ++++++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/i915_irq.h | 2 +
> 2 files changed, 134 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index ecfa6dad145a..c5058d834f99 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -2893,8 +2893,17 @@ static void ibx_hpd_detection_setup(struct drm_i915_private *dev_priv)
> ibx_hotplug_mask(dev_priv, HPD_PORT_D),
> intel_hpd_hotplug_enables(dev_priv, ibx_hotplug_enables));
> }
>
> +static void ibx_hpd_detection(struct intel_encoder *encoder, bool enable)
Just quickly skimming through the series...
...and nitpicking, *_detection() should really be a verb.
It isn't obvious to me what intel_hpd_detection() does without looking
at the implementation.
BR,
Jani.
> +{
> + struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> +
> + intel_uncore_rmw(&i915->uncore, PCH_PORT_HOTPLUG,
> + ibx_hotplug_mask(i915, encoder->hpd_pin),
> + enable ? ibx_hotplug_enables(encoder) : 0);
> +}
> +
> static void ibx_hpd_irq_setup(struct drm_i915_private *dev_priv)
> {
> u32 hotplug_irqs, enabled_irqs;
>
> @@ -2963,8 +2972,17 @@ static void icp_ddi_hpd_detection_setup(struct drm_i915_private *dev_priv)
> icp_ddi_hotplug_mask(dev_priv, HPD_PORT_D),
> intel_hpd_hotplug_enables(dev_priv, icp_ddi_hotplug_enables));
> }
>
> +static void icp_ddi_hpd_detection(struct intel_encoder *encoder, bool enable)
> +{
> + struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> +
> + intel_uncore_rmw(&i915->uncore, SHOTPLUG_CTL_DDI,
> + icp_ddi_hotplug_mask(i915, encoder->hpd_pin),
> + enable ? icp_ddi_hotplug_enables(encoder) : 0);
> +}
> +
> static void icp_tc_hpd_detection_setup(struct drm_i915_private *dev_priv)
> {
> intel_uncore_rmw(&dev_priv->uncore, SHOTPLUG_CTL_TC,
> icp_tc_hotplug_mask(dev_priv, HPD_PORT_TC1) |
> @@ -2975,8 +2993,23 @@ static void icp_tc_hpd_detection_setup(struct drm_i915_private *dev_priv)
> icp_tc_hotplug_mask(dev_priv, HPD_PORT_TC6),
> intel_hpd_hotplug_enables(dev_priv, icp_tc_hotplug_enables));
> }
>
> +static void icp_tc_hpd_detection(struct intel_encoder *encoder, bool enable)
> +{
> + struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> +
> + intel_uncore_rmw(&i915->uncore, SHOTPLUG_CTL_TC,
> + icp_tc_hotplug_mask(i915, encoder->hpd_pin),
> + enable ? icp_tc_hotplug_enables(encoder) : 0);
> +}
> +
> +static void icp_hpd_detection(struct intel_encoder *encoder, bool enable)
> +{
> + icp_ddi_hpd_detection(encoder, enable);
> + icp_tc_hpd_detection(encoder, enable);
> +}
> +
> static void icp_hpd_irq_setup(struct drm_i915_private *dev_priv)
> {
> u32 hotplug_irqs, enabled_irqs;
>
> @@ -3025,8 +3058,16 @@ static void dg1_hpd_invert(struct drm_i915_private *i915)
> INVERT_DDID_HPD);
> intel_uncore_rmw(&i915->uncore, SOUTH_CHICKEN1, 0, val);
> }
>
> +static void dg1_hpd_detection(struct intel_encoder *encoder, bool enable)
> +{
> + struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> +
> + dg1_hpd_invert(i915);
> + icp_hpd_detection(encoder, enable);
> +}
> +
> static void dg1_hpd_irq_setup(struct drm_i915_private *dev_priv)
> {
> dg1_hpd_invert(dev_priv);
> icp_hpd_irq_setup(dev_priv);
> @@ -3043,8 +3084,17 @@ static void gen11_tc_hpd_detection_setup(struct drm_i915_private *dev_priv)
> gen11_hotplug_mask(dev_priv, HPD_PORT_TC6),
> intel_hpd_hotplug_enables(dev_priv, gen11_hotplug_enables));
> }
>
> +static void gen11_tc_hpd_detection(struct intel_encoder *encoder, bool enable)
> +{
> + struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> +
> + intel_uncore_rmw(&i915->uncore, GEN11_TC_HOTPLUG_CTL,
> + gen11_hotplug_mask(i915, encoder->hpd_pin),
> + enable ? gen11_hotplug_enables(encoder) : 0);
> +}
> +
> static void gen11_tbt_hpd_detection_setup(struct drm_i915_private *dev_priv)
> {
> intel_uncore_rmw(&dev_priv->uncore, GEN11_TBT_HOTPLUG_CTL,
> gen11_hotplug_mask(dev_priv, HPD_PORT_TC1) |
> @@ -3055,8 +3105,28 @@ static void gen11_tbt_hpd_detection_setup(struct drm_i915_private *dev_priv)
> gen11_hotplug_mask(dev_priv, HPD_PORT_TC6),
> intel_hpd_hotplug_enables(dev_priv, gen11_hotplug_enables));
> }
>
> +static void gen11_tbt_hpd_detection(struct intel_encoder *encoder, bool enable)
> +{
> + struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> +
> + intel_uncore_rmw(&i915->uncore, GEN11_TBT_HOTPLUG_CTL,
> + gen11_hotplug_mask(i915, encoder->hpd_pin),
> + enable ? gen11_hotplug_enables(encoder) : 0);
> +}
> +
> +static void gen11_hpd_detection(struct intel_encoder *encoder, bool enable)
> +{
> + struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> +
> + gen11_tc_hpd_detection(encoder, enable);
> + gen11_tbt_hpd_detection(encoder, enable);
> +
> + if (INTEL_PCH_TYPE(i915) >= PCH_ICP)
> + icp_hpd_detection(encoder, enable);
> +}
> +
> static void gen11_hpd_irq_setup(struct drm_i915_private *dev_priv)
> {
> u32 hotplug_irqs, enabled_irqs;
>
> @@ -3140,8 +3210,28 @@ static void spt_hpd_detection_setup(struct drm_i915_private *dev_priv)
> spt_hotplug2_mask(dev_priv, HPD_PORT_E),
> intel_hpd_hotplug_enables(dev_priv, spt_hotplug2_enables));
> }
>
> +static void spt_hpd_detection(struct intel_encoder *encoder, bool enable)
> +{
> + struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> +
> + /* Display WA #1179 WaHardHangonHotPlug: cnp */
> + if (HAS_PCH_CNP(i915)) {
> + intel_uncore_rmw(&i915->uncore, SOUTH_CHICKEN1,
> + CHASSIS_CLK_REQ_DURATION_MASK,
> + CHASSIS_CLK_REQ_DURATION(0xf));
> + }
> +
> + intel_uncore_rmw(&i915->uncore, PCH_PORT_HOTPLUG,
> + spt_hotplug_mask(i915, encoder->hpd_pin),
> + enable ? spt_hotplug_enables(encoder) : 0);
> +
> + intel_uncore_rmw(&i915->uncore, PCH_PORT_HOTPLUG2,
> + spt_hotplug2_mask(i915, encoder->hpd_pin),
> + enable ? spt_hotplug2_enables(encoder) : 0);
> +}
> +
> static void spt_hpd_irq_setup(struct drm_i915_private *dev_priv)
> {
> u32 hotplug_irqs, enabled_irqs;
>
> @@ -3189,8 +3279,19 @@ static void ilk_hpd_detection_setup(struct drm_i915_private *dev_priv)
> ilk_hotplug_mask(dev_priv, HPD_PORT_A),
> intel_hpd_hotplug_enables(dev_priv, ilk_hotplug_enables));
> }
>
> +static void ilk_hpd_detection(struct intel_encoder *encoder, bool enable)
> +{
> + struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> +
> + intel_uncore_rmw(&i915->uncore, DIGITAL_PORT_HOTPLUG_CNTRL,
> + ilk_hotplug_mask(i915, encoder->hpd_pin),
> + enable ? ilk_hotplug_enables(encoder) : 0);
> +
> + ibx_hpd_detection(encoder, enable);
> +}
> +
> static void ilk_hpd_irq_setup(struct drm_i915_private *dev_priv)
> {
> u32 hotplug_irqs, enabled_irqs;
>
> @@ -3254,8 +3355,17 @@ static void bxt_hpd_detection_setup(struct drm_i915_private *dev_priv)
> bxt_hotplug_mask(dev_priv, HPD_PORT_C),
> intel_hpd_hotplug_enables(dev_priv, bxt_hotplug_enables));
> }
>
> +static void bxt_hpd_detection(struct intel_encoder *encoder, bool enable)
> +{
> + struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> +
> + intel_uncore_rmw(&i915->uncore, PCH_PORT_HOTPLUG,
> + bxt_hotplug_mask(i915, encoder->hpd_pin),
> + enable ? bxt_hotplug_enables(encoder) : 0);
> +}
> +
> static void bxt_hpd_irq_setup(struct drm_i915_private *dev_priv)
> {
> u32 hotplug_irqs, enabled_irqs;
>
> @@ -3885,8 +3995,18 @@ static void i965_irq_postinstall(struct drm_i915_private *dev_priv)
>
> i915_enable_asle_pipestat(dev_priv);
> }
>
> +static void i915_hpd_detection(struct intel_encoder *encoder, bool enable)
> +{
> + struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> + u32 hotplug_en = hpd_mask_i915[encoder->hpd_pin];
> +
> + /* HPD sense and interrupt enable are one and the same */
> + i915_hotplug_interrupt_update(i915, hotplug_en,
> + enable ? hotplug_en : 0);
> +}
> +
> static void i915_hpd_irq_setup(struct drm_i915_private *dev_priv)
> {
> u32 hotplug_en;
>
> @@ -3970,14 +4090,18 @@ static irqreturn_t i965_irq_handler(int irq, void *arg)
> return ret;
> }
>
> struct intel_hotplug_funcs {
> + /* Enable HPD sense and interrupts for all present encoders */
> void (*hpd_irq_setup)(struct drm_i915_private *i915);
> + /* Enable/disable HPD sense for a single encoder */
> + void (*hpd_detection)(struct intel_encoder *encoder, bool enable);
> };
>
> #define HPD_FUNCS(platform) \
> static const struct intel_hotplug_funcs platform##_hpd_funcs = { \
> .hpd_irq_setup = platform##_hpd_irq_setup, \
> + .hpd_detection = platform##_hpd_detection, \
> }
>
> HPD_FUNCS(i915);
> HPD_FUNCS(dg1);
> @@ -3987,8 +4111,16 @@ HPD_FUNCS(icp);
> HPD_FUNCS(spt);
> HPD_FUNCS(ilk);
> #undef HPD_FUNCS
>
> +void intel_hpd_detection(struct intel_encoder *encoder, bool enable)
> +{
> + struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> +
> + if (i915->display.funcs.hotplug)
> + i915->display.funcs.hotplug->hpd_detection(encoder, enable);
> +}
> +
> void intel_hpd_irq_setup(struct drm_i915_private *i915)
> {
> if (i915->display_irqs_enabled && i915->display.funcs.hotplug)
> i915->display.funcs.hotplug->hpd_irq_setup(i915);
> diff --git a/drivers/gpu/drm/i915/i915_irq.h b/drivers/gpu/drm/i915/i915_irq.h
> index 03ee4c8b1ed3..a5eb85ed7537 100644
> --- a/drivers/gpu/drm/i915/i915_irq.h
> +++ b/drivers/gpu/drm/i915/i915_irq.h
> @@ -16,8 +16,9 @@ struct drm_crtc;
> struct drm_device;
> struct drm_display_mode;
> struct drm_i915_private;
> struct intel_crtc;
> +struct intel_encoder;
> struct intel_uncore;
>
> void intel_irq_init(struct drm_i915_private *dev_priv);
> void intel_irq_fini(struct drm_i915_private *dev_priv);
> @@ -36,8 +37,9 @@ i915_disable_pipestat(struct drm_i915_private *dev_priv, enum pipe pipe,
>
> void valleyview_enable_display_irqs(struct drm_i915_private *dev_priv);
> void valleyview_disable_display_irqs(struct drm_i915_private *dev_priv);
>
> +void intel_hpd_detection(struct intel_encoder *encoder, bool enable);
> void intel_hpd_irq_setup(struct drm_i915_private *i915);
> void i915_hotplug_interrupt_update(struct drm_i915_private *dev_priv,
> u32 mask,
> u32 bits);
--
Jani Nikula, Intel Open Source Graphics Center
next prev parent reply other threads:[~2023-02-28 12:40 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-21 23:02 [Intel-gfx] [PATCH v3 00/22] drm/i915: Init DDI ports in VBT order Ville Syrjala
2023-02-21 23:02 ` [Intel-gfx] [PATCH v3 01/22] drm/i915: Populate dig_port->connected() before connector init Ville Syrjala
2023-02-28 17:58 ` Jani Nikula
2023-02-21 23:02 ` [Intel-gfx] [PATCH v3 02/22] drm/i915: Fix SKL DDI A digital port .connected() Ville Syrjala
2023-02-28 18:18 ` Jani Nikula
2023-02-21 23:02 ` [Intel-gfx] [PATCH v3 03/22] drm/i915: Get rid of the gm45 HPD live state nonsense Ville Syrjala
2023-02-28 18:19 ` Jani Nikula
2023-02-21 23:02 ` [Intel-gfx] [PATCH v3 04/22] drm/i915: Introduce <platoform>_hotplug_mask() Ville Syrjala
2023-02-28 18:36 ` Jani Nikula
2023-02-28 19:52 ` Ville Syrjälä
2023-02-21 23:02 ` [Intel-gfx] [PATCH v3 05/22] drm/i915: Introduce intel_hpd_detection() Ville Syrjala
2023-02-28 12:40 ` Jani Nikula [this message]
2023-02-28 12:50 ` Ville Syrjälä
2023-02-21 23:02 ` [Intel-gfx] [PATCH v3 06/22] drm/i915: Check HPD live state during eDP probe Ville Syrjala
2023-02-21 23:02 ` [Intel-gfx] [PATCH v3 07/22] drm/i915: Sanitize child devices later Ville Syrjala
2023-02-21 23:02 ` [Intel-gfx] [PATCH v3 08/22] drm/i915: Split map_aux_ch() into per-platform arrays Ville Syrjala
2023-02-21 23:02 ` [Intel-gfx] [PATCH v3 09/22] drm/i915: Flip VBT DDC pin maps around Ville Syrjala
2023-02-21 23:02 ` [Intel-gfx] [PATCH v3 10/22] drm/i915: Nuke intel_bios_is_port_dp_dual_mode() Ville Syrjala
2023-02-21 23:02 ` [Intel-gfx] [PATCH v3 11/22] drm/i915: Remove bogus DDI-F from hsw/bdw output init Ville Syrjala
2023-02-21 23:02 ` [Intel-gfx] [PATCH v3 12/22] drm/i915: Introduce device info port_mask Ville Syrjala
2023-02-21 23:02 ` [Intel-gfx] [PATCH v3 13/22] drm/i915: Assert that device info bitmasks have enough bits Ville Syrjala
2023-02-21 23:02 ` [Intel-gfx] [PATCH v3 14/22] drm/i915: Assert that the port being initialized is valid Ville Syrjala
2023-02-21 23:02 ` [Intel-gfx] [PATCH v3 15/22] drm/i915: Beef up SDVO/HDMI port checks Ville Syrjala
2023-02-21 23:02 ` [Intel-gfx] [PATCH v3 16/22] drm/i915: Init DDI outputs based on port_mask on skl+ Ville Syrjala
2023-02-21 23:02 ` [Intel-gfx] [PATCH v3 17/22] drm/i915: Try to initialize DDI/ICL+ DSI ports for every VBT child device Ville Syrjala
2023-02-21 23:02 ` [Intel-gfx] [PATCH v3 18/22] drm/i915: Convert HSW/BDW to use VBT driven DDI probe Ville Syrjala
2023-02-21 23:02 ` [Intel-gfx] [PATCH v3 19/22] drm/i915: Remove DDC pin sanitation Ville Syrjala
2023-02-21 23:02 ` [Intel-gfx] [PATCH v3 20/22] drm/i915: Remove AUX CH sanitation Ville Syrjala
2023-02-21 23:02 ` [Intel-gfx] [PATCH v3 21/22] drm/i915: Initialize dig_port->aux_ch to NONE to be sure Ville Syrjala
2023-02-21 23:02 ` [Intel-gfx] [PATCH v3 22/22] drm/i915: Only populate aux_ch is really needed Ville Syrjala
2023-02-21 23:35 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Init DDI ports in VBT order (rev3) Patchwork
2023-02-22 3:04 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2023-02-22 5:11 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
2023-02-22 15:50 ` [Intel-gfx] [PATCH v3 00/22] drm/i915: Init DDI ports in VBT order Ville Syrjälä
2023-05-26 4:48 ` Kai-Heng Feng
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=87k002ynd7.fsf@intel.com \
--to=jani.nikula@linux.intel.com \
--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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox