[Intel-gfx] [PATCH 04/22] drm/i915/tgl: Add hpd interrupt handling

Lucas De Marchi lucas.demarchi at intel.com
Fri Jul 19 17:28:39 UTC 2019


On Fri, Jul 19, 2019 at 08:14:40PM +0300, Ville Syrjälä wrote:
>On Fri, Jul 19, 2019 at 08:08:47PM +0300, Ville Syrjälä wrote:
>> On Fri, Jul 19, 2019 at 09:45:16AM -0700, Lucas De Marchi wrote:
>> > On Fri, Jul 19, 2019 at 04:47:45PM +0300, Ville Syrjälä wrote:
>> > >On Fri, Jul 12, 2019 at 06:09:22PM -0700, Lucas De Marchi wrote:
>> > >> Add hotdplug detection for all ports on TGP. icp_hpd_detection_setup()
>> > >> is refactored to be shared with TGP.
>> > >>
>> > >> While we increase the number of pins, add a BUILD_BUG_ON() to avoid
>> > >> going over the number of bits allowed.
>> > >>
>> > >> Cc: Jose Souza <jose.souza at intel.com>
>> > >> Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
>> > >> Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
>> > >> ---
>> > >>  drivers/gpu/drm/i915/display/intel_hotplug.c |   6 +
>> > >>  drivers/gpu/drm/i915/i915_drv.h              |   4 +
>> > >>  drivers/gpu/drm/i915/i915_irq.c              | 128 +++++++++++++++++--
>> > >>  drivers/gpu/drm/i915/i915_reg.h              |  28 +++-
>> > >>  4 files changed, 154 insertions(+), 12 deletions(-)
>> > >>
>> > >> diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c b/drivers/gpu/drm/i915/display/intel_hotplug.c
>> > >> index ea3de4acc850..a7833f45dc4d 100644
>> > >> --- a/drivers/gpu/drm/i915/display/intel_hotplug.c
>> > >> +++ b/drivers/gpu/drm/i915/display/intel_hotplug.c
>> > >> @@ -104,6 +104,12 @@ enum hpd_pin intel_hpd_pin_default(struct drm_i915_private *dev_priv,
>> > >>  		if (IS_CNL_WITH_PORT_F(dev_priv))
>> > >>  			return HPD_PORT_E;
>> > >>  		return HPD_PORT_F;
>> > >> +	case PORT_G:
>> > >> +		return HPD_PORT_G;
>> > >> +	case PORT_H:
>> > >> +		return HPD_PORT_H;
>> > >> +	case PORT_I:
>> > >> +		return HPD_PORT_I;
>> > >>  	default:
>> > >>  		MISSING_CASE(port);
>> > >>  		return HPD_NONE;
>> > >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> > >> index cf7e92ca72e9..069337f11872 100644
>> > >> --- a/drivers/gpu/drm/i915/i915_drv.h
>> > >> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> > >> @@ -153,6 +153,10 @@ enum hpd_pin {
>> > >>  	HPD_PORT_D,
>> > >>  	HPD_PORT_E,
>> > >>  	HPD_PORT_F,
>> > >> +	HPD_PORT_G,
>> > >> +	HPD_PORT_H,
>> > >> +	HPD_PORT_I,
>> > >> +
>> > >>  	HPD_NUM_PINS
>> > >>  };
>> > >>
>> > >> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>> > >> index 7c5ba5cbea34..a7a90674db89 100644
>> > >> --- a/drivers/gpu/drm/i915/i915_irq.c
>> > >> +++ b/drivers/gpu/drm/i915/i915_irq.c
>> > >> @@ -148,6 +148,18 @@ static const u32 hpd_mcc[HPD_NUM_PINS] = {
>> > >>  	[HPD_PORT_C] = SDE_TC1_HOTPLUG_ICP
>> > >>  };
>> > >>
>> > >> +static const u32 hpd_tgp[HPD_NUM_PINS] = {
>> > >> +	[HPD_PORT_A] = SDE_DDIA_HOTPLUG_ICP,
>> > >> +	[HPD_PORT_B] = SDE_DDIB_HOTPLUG_ICP,
>> > >> +	[HPD_PORT_C] = SDE_DDIC_HOTPLUG_TGP,
>> > >> +	[HPD_PORT_D] = SDE_TC1_HOTPLUG_ICP,
>> > >> +	[HPD_PORT_E] = SDE_TC2_HOTPLUG_ICP,
>> > >> +	[HPD_PORT_F] = SDE_TC3_HOTPLUG_ICP,
>> > >> +	[HPD_PORT_G] = SDE_TC4_HOTPLUG_ICP,
>> > >> +	[HPD_PORT_H] = SDE_TC5_HOTPLUG_TGP,
>> > >> +	[HPD_PORT_I] = SDE_TC6_HOTPLUG_TGP,
>> > >> +};
>> > >> +
>> > >>  static void gen3_irq_reset(struct intel_uncore *uncore, i915_reg_t imr,
>> > >>  			   i915_reg_t iir, i915_reg_t ier)
>> > >>  {
>> > >> @@ -1706,6 +1718,40 @@ static bool icp_tc_port_hotplug_long_detect(enum hpd_pin pin, u32 val)
>> > >>  	}
>> > >>  }
>> > >>
>> > >> +static bool tgp_ddi_port_hotplug_long_detect(enum hpd_pin pin, u32 val)
>> > >> +{
>> > >> +	switch (pin) {
>> > >> +	case HPD_PORT_A:
>> > >> +		return val & ICP_DDIA_HPD_LONG_DETECT;
>> > >> +	case HPD_PORT_B:
>> > >> +		return val & ICP_DDIB_HPD_LONG_DETECT;
>> > >> +	case HPD_PORT_C:
>> > >> +		return val & TGP_DDIC_HPD_LONG_DETECT;
>> > >> +	default:
>> > >> +		return false;
>> > >> +	}
>> > >> +}
>> > >> +
>> > >> +static bool tgp_tc_port_hotplug_long_detect(enum hpd_pin pin, u32 val)
>> > >> +{
>> > >> +	switch (pin) {
>> > >> +	case HPD_PORT_D:
>> > >> +		return val & ICP_TC_HPD_LONG_DETECT(PORT_TC1);
>> > >> +	case HPD_PORT_E:
>> > >> +		return val & ICP_TC_HPD_LONG_DETECT(PORT_TC2);
>> > >> +	case HPD_PORT_F:
>> > >> +		return val & ICP_TC_HPD_LONG_DETECT(PORT_TC3);
>> > >> +	case HPD_PORT_G:
>> > >> +		return val & ICP_TC_HPD_LONG_DETECT(PORT_TC4);
>> > >> +	case HPD_PORT_H:
>> > >> +		return val & ICP_TC_HPD_LONG_DETECT(PORT_TC5);
>> > >> +	case HPD_PORT_I:
>> > >> +		return val & ICP_TC_HPD_LONG_DETECT(PORT_TC6);
>> > >> +	default:
>> > >> +		return false;
>> > >> +	}
>> > >> +}
>> > >> +
>> > >>  static bool spt_port_hotplug2_long_detect(enum hpd_pin pin, u32 val)
>> > >>  {
>> > >>  	switch (pin) {
>> > >> @@ -1785,6 +1831,8 @@ static void intel_get_hpd_pins(struct drm_i915_private *dev_priv,
>> > >>  {
>> > >>  	enum hpd_pin pin;
>> > >>
>> > >> +	BUILD_BUG_ON(sizeof(int) * 8 < HPD_NUM_PINS);
>> > >
>> > >BUILD_BUG_ON(HPD_NUM_PINS > BITS_PER_TYPE(*pin_mask));
>> > >would be a clearer way to express that.
>> >
>> > For the BITS_PER_TYPE, ok. But for the swapped order, checkpatch doesn't
>> > agree:
>> >
>> > 8b77abf61be2 (HEAD) drm/i915/tgl: Add hpd interrupt handling
>> > -:117: WARNING:CONSTANT_COMPARISON: Comparisons should place the constant on the right side of the test
>> > #117: FILE: drivers/gpu/drm/i915/i915_irq.c:1852:
>> > +       BUILD_BUG_ON(HPD_NUM_PINS > BITS_PER_TYPE(*pin_mask));
>> >
>> > Note: Initially I did with the same order you suggested and had to swap
>> > it while cleaning up the checkpatch warnings.
>>
>> Checkpatch is stupid. HPD_NUM_PINS is clearly the "variable" we're trying
>> to check here, not the constant we're checking against.
>
>Well, I guess you can argue either way. But this is the way my brain
>wants to read this :)

my brain read it that way, too. Then after reading the warning I
thought: humnn... there's a constant written in stone and one that
depends on the size of that type: maybe I'm wrong and checkpatch is
right. So I changed the order.

Since we both agree prefer HPD_NUM_PINS as a variable, I will
ignore checkpatch here and swap it back.

Lucas De Marchi

>
>-- 
>Ville Syrjälä
>Intel


More information about the Intel-gfx mailing list