| Message ID | 20230823170740.1180212-28-lucas.demarchi@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Enable Lunar Lake display | expand |
On Wed, Aug 23, 2023 at 10:07:25AM -0700, Lucas De Marchi wrote: > From: Luca Coelho <luciano.coelho@intel.com> > > Starting from display version 20, we need to read the pin assignment > from the IOM TCSS_DDI_STATUS register instead of reading it from the > FIA. > > We use the pin assignment to decide the maximum lane count. So, to > support this change, add a new lnl_tc_port_get_max_lane_count() function > that reads from the TCSS_DDI_STATUS register and decides the maximum > lane count based on that. > > BSpec: 69594 > Cc: Mika Kahola <mika.kahola@intel.com> > Signed-off-by: Luca Coelho <luciano.coelho@intel.com> > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> > --- > drivers/gpu/drm/i915/display/intel_tc.c | 28 +++++++++++++++++++++++++ > drivers/gpu/drm/i915/i915_reg.h | 1 + > 2 files changed, 29 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c > index 3c94bbcb5497..37b0f8529b4f 100644 > --- a/drivers/gpu/drm/i915/display/intel_tc.c > +++ b/drivers/gpu/drm/i915/display/intel_tc.c > @@ -290,6 +290,31 @@ u32 intel_tc_port_get_pin_assignment_mask(struct intel_digital_port *dig_port) > DP_PIN_ASSIGNMENT_SHIFT(tc->phy_fia_idx); > } > > +static int lnl_tc_port_get_max_lane_count(struct intel_digital_port *dig_port) > +{ > + struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev); > + enum tc_port tc_port = intel_port_to_tc(i915, dig_port->base.port); > + intel_wakeref_t wakeref; > + u32 val, pin_assignment; > + > + with_intel_display_power(i915, POWER_DOMAIN_DISPLAY_CORE, wakeref) Do we need this? I don't think POWER_DOMAIN_DISPLAY_CORE has been tied to any power wells since VLV/CHV. Hmm, it looks like we actually grab it (and even assert it) in a bunch of places on modern platforms that don't make sense to me since it isn't tied to anything. I guess leaving this here doesn't hurt anything, although we might want to go back and take another look at this in the future. Reviewed-by: Matt Roper <matthew.d.roper@intel.com> > + val = intel_de_read(i915, TCSS_DDI_STATUS(tc_port)); > + > + pin_assignment = > + REG_FIELD_GET(TCSS_DDI_STATUS_PIN_ASSIGNMENT_MASK, val); > + > + switch (pin_assignment) { > + default: > + MISSING_CASE(pin_assignment); > + fallthrough; > + case DP_PIN_ASSIGNMENT_D: > + return 2; > + case DP_PIN_ASSIGNMENT_C: > + case DP_PIN_ASSIGNMENT_E: > + return 4; > + } > +} > + > static int mtl_tc_port_get_max_lane_count(struct intel_digital_port *dig_port) > { > struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev); > @@ -348,6 +373,9 @@ int intel_tc_port_max_lane_count(struct intel_digital_port *dig_port) > > assert_tc_cold_blocked(tc); > > + if (DISPLAY_VER(i915) >= 20) > + return lnl_tc_port_get_max_lane_count(dig_port); > + > if (DISPLAY_VER(i915) >= 14) > return mtl_tc_port_get_max_lane_count(dig_port); > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index e31a985b02d5..fa85530afac3 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -6628,6 +6628,7 @@ enum skl_power_gate { > #define TCSS_DDI_STATUS(tc) _MMIO(_PICK_EVEN(tc, \ > _TCSS_DDI_STATUS_1, \ > _TCSS_DDI_STATUS_2)) > +#define TCSS_DDI_STATUS_PIN_ASSIGNMENT_MASK REG_GENMASK(28, 25) > #define TCSS_DDI_STATUS_READY REG_BIT(2) > #define TCSS_DDI_STATUS_HPD_LIVE_STATUS_TBT REG_BIT(1) > #define TCSS_DDI_STATUS_HPD_LIVE_STATUS_ALT REG_BIT(0) > -- > 2.40.1 >
On Wed, 2023-08-23 at 13:28 -0700, Matt Roper wrote: > On Wed, Aug 23, 2023 at 10:07:25AM -0700, Lucas De Marchi wrote: > > From: Luca Coelho <luciano.coelho@intel.com> > > > > Starting from display version 20, we need to read the pin assignment > > from the IOM TCSS_DDI_STATUS register instead of reading it from the > > FIA. > > > > We use the pin assignment to decide the maximum lane count. So, to > > support this change, add a new lnl_tc_port_get_max_lane_count() function > > that reads from the TCSS_DDI_STATUS register and decides the maximum > > lane count based on that. > > > > BSpec: 69594 > > Cc: Mika Kahola <mika.kahola@intel.com> > > Signed-off-by: Luca Coelho <luciano.coelho@intel.com> > > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_tc.c | 28 +++++++++++++++++++++++++ > > drivers/gpu/drm/i915/i915_reg.h | 1 + > > 2 files changed, 29 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c > > index 3c94bbcb5497..37b0f8529b4f 100644 > > --- a/drivers/gpu/drm/i915/display/intel_tc.c > > +++ b/drivers/gpu/drm/i915/display/intel_tc.c > > @@ -290,6 +290,31 @@ u32 intel_tc_port_get_pin_assignment_mask(struct intel_digital_port *dig_port) > > DP_PIN_ASSIGNMENT_SHIFT(tc->phy_fia_idx); > > } > > > > +static int lnl_tc_port_get_max_lane_count(struct intel_digital_port *dig_port) > > +{ > > + struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev); > > + enum tc_port tc_port = intel_port_to_tc(i915, dig_port->base.port); > > + intel_wakeref_t wakeref; > > + u32 val, pin_assignment; > > + > > + with_intel_display_power(i915, POWER_DOMAIN_DISPLAY_CORE, wakeref) > > Do we need this? I don't think POWER_DOMAIN_DISPLAY_CORE has been tied > to any power wells since VLV/CHV. > > Hmm, it looks like we actually grab it (and even assert it) in a bunch of > places on modern platforms that don't make sense to me since it isn't > tied to anything. > > I guess leaving this here doesn't hurt anything, although we might want > to go back and take another look at this in the future. > > Reviewed-by: Matt Roper <matthew.d.roper@intel.com> Thanks, Matt! You have a good point, but as you said, maybe this should be revisited in all occurrences and changed in one go. I just kept it consistent with other usage. -- Cheers, Luca.
On Wed, 2023-08-23 at 10:07 -0700, Lucas De Marchi wrote: > From: Luca Coelho <luciano.coelho@intel.com> > > Starting from display version 20, we need to read the pin assignment > from the IOM TCSS_DDI_STATUS register instead of reading it from the > FIA. > > We use the pin assignment to decide the maximum lane count. So, to > support this change, add a new lnl_tc_port_get_max_lane_count() function > that reads from the TCSS_DDI_STATUS register and decides the maximum > lane count based on that. > > BSpec: 69594 > Cc: Mika Kahola <mika.kahola@intel.com> > Signed-off-by: Luca Coelho <luciano.coelho@intel.com> > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> > --- Lucas, do you want me to send this together with my patchset with the preparation for this? In a way, the 4 patches I sent out can be applied independently of LNL- related changes, so maybe I could resend just those 4 patches and you base your entire series on top of my patches after they get applied? Then this patch, which is really related to LNL could be part of your series... Let me know what you prefer. -- Cheers, Luca.
On Thu, Aug 24, 2023 at 11:34:22AM +0000, Coelho, Luciano wrote: >On Wed, 2023-08-23 at 10:07 -0700, Lucas De Marchi wrote: >> From: Luca Coelho <luciano.coelho@intel.com> >> >> Starting from display version 20, we need to read the pin assignment >> from the IOM TCSS_DDI_STATUS register instead of reading it from the >> FIA. >> >> We use the pin assignment to decide the maximum lane count. So, to >> support this change, add a new lnl_tc_port_get_max_lane_count() function >> that reads from the TCSS_DDI_STATUS register and decides the maximum >> lane count based on that. >> >> BSpec: 69594 >> Cc: Mika Kahola <mika.kahola@intel.com> >> Signed-off-by: Luca Coelho <luciano.coelho@intel.com> >> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> >> --- > >Lucas, do you want me to send this together with my patchset with the >preparation for this? > >In a way, the 4 patches I sent out can be applied independently of LNL- >related changes, so maybe I could resend just those 4 patches and you >base your entire series on top of my patches after they get applied? >Then this patch, which is really related to LNL could be part of your >series... Please get the first 4 patches applied. I can keep this one in this series. Pasting from the cover letter to make clear we are on the same page: 3. Patches 7 through 10 can also be ignored: they are not applied yet, but being reviewed in other patch series by its author[2]. [2] https://patchwork.freedesktop.org/series/120980/ The only reason I added them here is that since this series will take some time to be applied, I figured it would be better not to create unnecessary conflicts. thanks Lucas De Marchi > >Let me know what you prefer. > >-- >Cheers, >Luca.
diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c index 3c94bbcb5497..37b0f8529b4f 100644 --- a/drivers/gpu/drm/i915/display/intel_tc.c +++ b/drivers/gpu/drm/i915/display/intel_tc.c @@ -290,6 +290,31 @@ u32 intel_tc_port_get_pin_assignment_mask(struct intel_digital_port *dig_port) DP_PIN_ASSIGNMENT_SHIFT(tc->phy_fia_idx); } +static int lnl_tc_port_get_max_lane_count(struct intel_digital_port *dig_port) +{ + struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev); + enum tc_port tc_port = intel_port_to_tc(i915, dig_port->base.port); + intel_wakeref_t wakeref; + u32 val, pin_assignment; + + with_intel_display_power(i915, POWER_DOMAIN_DISPLAY_CORE, wakeref) + val = intel_de_read(i915, TCSS_DDI_STATUS(tc_port)); + + pin_assignment = + REG_FIELD_GET(TCSS_DDI_STATUS_PIN_ASSIGNMENT_MASK, val); + + switch (pin_assignment) { + default: + MISSING_CASE(pin_assignment); + fallthrough; + case DP_PIN_ASSIGNMENT_D: + return 2; + case DP_PIN_ASSIGNMENT_C: + case DP_PIN_ASSIGNMENT_E: + return 4; + } +} + static int mtl_tc_port_get_max_lane_count(struct intel_digital_port *dig_port) { struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev); @@ -348,6 +373,9 @@ int intel_tc_port_max_lane_count(struct intel_digital_port *dig_port) assert_tc_cold_blocked(tc); + if (DISPLAY_VER(i915) >= 20) + return lnl_tc_port_get_max_lane_count(dig_port); + if (DISPLAY_VER(i915) >= 14) return mtl_tc_port_get_max_lane_count(dig_port); diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index e31a985b02d5..fa85530afac3 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -6628,6 +6628,7 @@ enum skl_power_gate { #define TCSS_DDI_STATUS(tc) _MMIO(_PICK_EVEN(tc, \ _TCSS_DDI_STATUS_1, \ _TCSS_DDI_STATUS_2)) +#define TCSS_DDI_STATUS_PIN_ASSIGNMENT_MASK REG_GENMASK(28, 25) #define TCSS_DDI_STATUS_READY REG_BIT(2) #define TCSS_DDI_STATUS_HPD_LIVE_STATUS_TBT REG_BIT(1) #define TCSS_DDI_STATUS_HPD_LIVE_STATUS_ALT REG_BIT(0)