| Message ID | 20240625202652.315936-1-gustavo.sousa@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | drm/i915: Skip programming FIA link enable bits for MTL+ | expand |
On 6/26/2024 01:56, Gustavo Sousa wrote: > Starting with Xe_LPDP, support for Type-C connections is provided by > PICA and programming PORT_TX_DFLEXDPMLE1(*) registers is not applicable > anymore. Those registers don't even exist in recent display IPs. As > such, skip programming them. > > Bspec: 65750, 65448 I guess, we can add the Bspec page (49190) of the last platform which contained the reg DFLEXDPMLE, so as to have a better diff view for someone reviewing. > Signed-off-by: Gustavo Sousa<gustavo.sousa@intel.com> With above comments, LGTM and Reviewed-by: Shekhar Chauhan <shekhar.chauhan@intel.com> > --- > drivers/gpu/drm/i915/display/intel_tc.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c > index 9887967b2ca5..6f2ee7dbc43b 100644 > --- a/drivers/gpu/drm/i915/display/intel_tc.c > +++ b/drivers/gpu/drm/i915/display/intel_tc.c > @@ -393,6 +393,9 @@ void intel_tc_port_set_fia_lane_count(struct intel_digital_port *dig_port, > bool lane_reversal = dig_port->saved_port_bits & DDI_BUF_PORT_REVERSAL; > u32 val; > > + if (DISPLAY_VER(i915) >= 14) > + return; > + > drm_WARN_ON(&i915->drm, > lane_reversal && tc->mode != TC_PORT_LEGACY); >
Had formatting issues with the first review, sending it again for clarity. On 6/26/2024 01:56, Gustavo Sousa wrote: > Starting with Xe_LPDP, support for Type-C connections is provided by > PICA and programming PORT_TX_DFLEXDPMLE1(*) registers is not applicable > anymore. Those registers don't even exist in recent display IPs. As > such, skip programming them. > > Bspec: 65750, 65448 We can add the Bspec (49190) of the last platform which had this reg DFLEXDPMLE in the Display Sequence, so as to have a better reference point for a reviewer. > Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com> With that, Reviewed-by: Shekhar Chauhan <shekhar.chauhan@intel.com> > --- > drivers/gpu/drm/i915/display/intel_tc.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c > index 9887967b2ca5..6f2ee7dbc43b 100644 > --- a/drivers/gpu/drm/i915/display/intel_tc.c > +++ b/drivers/gpu/drm/i915/display/intel_tc.c > @@ -393,6 +393,9 @@ void intel_tc_port_set_fia_lane_count(struct intel_digital_port *dig_port, > bool lane_reversal = dig_port->saved_port_bits & DDI_BUF_PORT_REVERSAL; > u32 val; > > + if (DISPLAY_VER(i915) >= 14) > + return; > + > drm_WARN_ON(&i915->drm, > lane_reversal && tc->mode != TC_PORT_LEGACY); >
On Tue, Jun 25, 2024 at 05:26:52PM -0300, Gustavo Sousa wrote: > Starting with Xe_LPDP, support for Type-C connections is provided by > PICA and programming PORT_TX_DFLEXDPMLE1(*) registers is not applicable > anymore. Those registers don't even exist in recent display IPs. As > such, skip programming them. > > Bspec: 65750, 65448 > Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com> MTL still has a FIA mux and the DP-alt pin configuration is read out from that, but programming DPMLE1 accordingly doesn't seem to be required indeed (the register still exists but programming it doesn't make a difference based on my test): Reviewed-by: Imre Deak <imre.deak@intel.com> > --- > drivers/gpu/drm/i915/display/intel_tc.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c > index 9887967b2ca5..6f2ee7dbc43b 100644 > --- a/drivers/gpu/drm/i915/display/intel_tc.c > +++ b/drivers/gpu/drm/i915/display/intel_tc.c > @@ -393,6 +393,9 @@ void intel_tc_port_set_fia_lane_count(struct intel_digital_port *dig_port, > bool lane_reversal = dig_port->saved_port_bits & DDI_BUF_PORT_REVERSAL; > u32 val; > > + if (DISPLAY_VER(i915) >= 14) > + return; > + > drm_WARN_ON(&i915->drm, > lane_reversal && tc->mode != TC_PORT_LEGACY); > > -- > 2.45.2 >
Quoting Imre Deak (2024-06-26 13:24:24-03:00) >On Tue, Jun 25, 2024 at 05:26:52PM -0300, Gustavo Sousa wrote: >> Starting with Xe_LPDP, support for Type-C connections is provided by >> PICA and programming PORT_TX_DFLEXDPMLE1(*) registers is not applicable >> anymore. Those registers don't even exist in recent display IPs. As >> such, skip programming them. >> >> Bspec: 65750, 65448 >> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com> > >MTL still has a FIA mux and the DP-alt pin configuration is read out Yep. Maybe I could rephrase the commit message like below? Starting with Xe_LPDP, although FIA is still used to readout Type-C pin assignment, part of Type-C support is moved to PICA and programming PORT_TX_DFLEXDPMLE1(*) registers is not applicable anymore. >from that, but programming DPMLE1 accordingly doesn't seem to be >required indeed (the register still exists but programming it doesn't >make a difference based on my test): Well, yes, one of the base offsets (0x16f8c0) does exist on MTL, but it maps to a completely different register (according to the register database). > >Reviewed-by: Imre Deak <imre.deak@intel.com> Thanks! Gustavo Sousa > >> --- >> drivers/gpu/drm/i915/display/intel_tc.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c >> index 9887967b2ca5..6f2ee7dbc43b 100644 >> --- a/drivers/gpu/drm/i915/display/intel_tc.c >> +++ b/drivers/gpu/drm/i915/display/intel_tc.c >> @@ -393,6 +393,9 @@ void intel_tc_port_set_fia_lane_count(struct intel_digital_port *dig_port, >> bool lane_reversal = dig_port->saved_port_bits & DDI_BUF_PORT_REVERSAL; >> u32 val; >> >> + if (DISPLAY_VER(i915) >= 14) >> + return; >> + >> drm_WARN_ON(&i915->drm, >> lane_reversal && tc->mode != TC_PORT_LEGACY); >> >> -- >> 2.45.2 >>
On Tue, Jun 25, 2024 at 05:26:52PM -0300, Gustavo Sousa wrote: > Starting with Xe_LPDP, support for Type-C connections is provided by Drive-by nitpick: In human-readable text like comments and commit messages, the official name of the IP is "Xe_LPD+." We only need to replace the "+" with a "P" in code identifiers where a symbol wouldn't be legal. Matt > PICA and programming PORT_TX_DFLEXDPMLE1(*) registers is not applicable > anymore. Those registers don't even exist in recent display IPs. As > such, skip programming them. > > Bspec: 65750, 65448 > Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com> > --- > drivers/gpu/drm/i915/display/intel_tc.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c > index 9887967b2ca5..6f2ee7dbc43b 100644 > --- a/drivers/gpu/drm/i915/display/intel_tc.c > +++ b/drivers/gpu/drm/i915/display/intel_tc.c > @@ -393,6 +393,9 @@ void intel_tc_port_set_fia_lane_count(struct intel_digital_port *dig_port, > bool lane_reversal = dig_port->saved_port_bits & DDI_BUF_PORT_REVERSAL; > u32 val; > > + if (DISPLAY_VER(i915) >= 14) > + return; > + > drm_WARN_ON(&i915->drm, > lane_reversal && tc->mode != TC_PORT_LEGACY); > > -- > 2.45.2 >
Quoting Matt Roper (2024-06-26 14:57:58-03:00) >On Tue, Jun 25, 2024 at 05:26:52PM -0300, Gustavo Sousa wrote: >> Starting with Xe_LPDP, support for Type-C connections is provided by > >Drive-by nitpick: In human-readable text like comments and commit >messages, the official name of the IP is "Xe_LPD+." We only need to >replace the "+" with a "P" in code identifiers where a symbol wouldn't >be legal. Yeah... I was unsure between the two forms and was afraid that the "+" in "Xe_LPD+" could somehow be misinterpreted as "and higher", so I ended up going with the other one. I'll update to use the official name while applying. Thanks! -- Gustavo Sousa > > >Matt > >> PICA and programming PORT_TX_DFLEXDPMLE1(*) registers is not applicable >> anymore. Those registers don't even exist in recent display IPs. As >> such, skip programming them. >> >> Bspec: 65750, 65448 >> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com> >> --- >> drivers/gpu/drm/i915/display/intel_tc.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c >> index 9887967b2ca5..6f2ee7dbc43b 100644 >> --- a/drivers/gpu/drm/i915/display/intel_tc.c >> +++ b/drivers/gpu/drm/i915/display/intel_tc.c >> @@ -393,6 +393,9 @@ void intel_tc_port_set_fia_lane_count(struct intel_digital_port *dig_port, >> bool lane_reversal = dig_port->saved_port_bits & DDI_BUF_PORT_REVERSAL; >> u32 val; >> >> + if (DISPLAY_VER(i915) >= 14) >> + return; >> + >> drm_WARN_ON(&i915->drm, >> lane_reversal && tc->mode != TC_PORT_LEGACY); >> >> -- >> 2.45.2 >> > >-- >Matt Roper >Graphics Software Engineer >Linux GPU Platform Enablement >Intel Corporation
On Wed, Jun 26, 2024 at 02:31:49PM -0300, Gustavo Sousa wrote: > Quoting Imre Deak (2024-06-26 13:24:24-03:00) > >On Tue, Jun 25, 2024 at 05:26:52PM -0300, Gustavo Sousa wrote: > >> Starting with Xe_LPDP, support for Type-C connections is provided by > >> PICA and programming PORT_TX_DFLEXDPMLE1(*) registers is not applicable > >> anymore. Those registers don't even exist in recent display IPs. As > >> such, skip programming them. > >> > >> Bspec: 65750, 65448 > >> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com> > > > >MTL still has a FIA mux and the DP-alt pin configuration is read out > > Yep. Maybe I could rephrase the commit message like below? > > Starting with Xe_LPDP, although FIA is still used to readout Type-C > pin assignment, part of Type-C support is moved to PICA and > programming PORT_TX_DFLEXDPMLE1(*) registers is not applicable > anymore. Ok, maybe worth mentioning how things changed. > >from that, but programming DPMLE1 accordingly doesn't seem to be > >required indeed (the register still exists but programming it doesn't > >make a difference based on my test): > > Well, yes, one of the base offsets (0x16f8c0) does exist on MTL, but it > maps to a completely different register (according to the register > database). 0x16f8c0 is in the third FIA instance, which afaics wouldn't be used on MTL/ARL with the max 4 TC ports on those. I still assume that the registers in the first two FIA instances exist the same way on MTL as on earlier platforms, just the DPMLE1 value is not used there. > >Reviewed-by: Imre Deak <imre.deak@intel.com> > > Thanks! > > Gustavo Sousa > > > > >> --- > >> drivers/gpu/drm/i915/display/intel_tc.c | 3 +++ > >> 1 file changed, 3 insertions(+) > >> > >> diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c > >> index 9887967b2ca5..6f2ee7dbc43b 100644 > >> --- a/drivers/gpu/drm/i915/display/intel_tc.c > >> +++ b/drivers/gpu/drm/i915/display/intel_tc.c > >> @@ -393,6 +393,9 @@ void intel_tc_port_set_fia_lane_count(struct intel_digital_port *dig_port, > >> bool lane_reversal = dig_port->saved_port_bits & DDI_BUF_PORT_REVERSAL; > >> u32 val; > >> > >> + if (DISPLAY_VER(i915) >= 14) > >> + return; > >> + > >> drm_WARN_ON(&i915->drm, > >> lane_reversal && tc->mode != TC_PORT_LEGACY); > >> > >> -- > >> 2.45.2 > >>
diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c index 9887967b2ca5..6f2ee7dbc43b 100644 --- a/drivers/gpu/drm/i915/display/intel_tc.c +++ b/drivers/gpu/drm/i915/display/intel_tc.c @@ -393,6 +393,9 @@ void intel_tc_port_set_fia_lane_count(struct intel_digital_port *dig_port, bool lane_reversal = dig_port->saved_port_bits & DDI_BUF_PORT_REVERSAL; u32 val; + if (DISPLAY_VER(i915) >= 14) + return; + drm_WARN_ON(&i915->drm, lane_reversal && tc->mode != TC_PORT_LEGACY);
Starting with Xe_LPDP, support for Type-C connections is provided by PICA and programming PORT_TX_DFLEXDPMLE1(*) registers is not applicable anymore. Those registers don't even exist in recent display IPs. As such, skip programming them. Bspec: 65750, 65448 Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com> --- drivers/gpu/drm/i915/display/intel_tc.c | 3 +++ 1 file changed, 3 insertions(+)