| Message ID | 20230823170740.1180212-32-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:29AM -0700, Lucas De Marchi wrote: > LNL's south display uses the same table as MTP. Check for LNL's fake PCH > to make it consistent with the other checks. > > The VBT table doesn't contain the VBT -> spec mapping for LNL. Like in > other cases, uses the same as the previous platform. > > Bspec: 68971, 20124 > Cc: Anusha Srivatsa <anusha.srivatsa@intel.com> > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> > --- > drivers/gpu/drm/i915/display/intel_bios.c | 2 +- > drivers/gpu/drm/i915/display/intel_gmbus.c | 5 ++++- > 2 files changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c > index 097c1f23d3ae..3772b91e155c 100644 > --- a/drivers/gpu/drm/i915/display/intel_bios.c > +++ b/drivers/gpu/drm/i915/display/intel_bios.c > @@ -2195,7 +2195,7 @@ static u8 map_ddc_pin(struct drm_i915_private *i915, u8 vbt_pin) > const u8 *ddc_pin_map; > int i, n_entries; > > - if (HAS_PCH_MTP(i915) || IS_ALDERLAKE_P(i915)) { > + if (HAS_PCH_MTP(i915) || IS_ALDERLAKE_P(i915) || IS_LUNARLAKE(i915)) { The LUNARLAKE here vs PCH_LNL below seems inconsistent. Either way, we should probably put the newer platform first in the condition. Aside from those Reviewed-by: Matt Roper <matthew.d.roper@intel.com> > ddc_pin_map = adlp_ddc_pin_map; > n_entries = ARRAY_SIZE(adlp_ddc_pin_map); > } else if (IS_ALDERLAKE_S(i915)) { > diff --git a/drivers/gpu/drm/i915/display/intel_gmbus.c b/drivers/gpu/drm/i915/display/intel_gmbus.c > index e95ddb580ef6..801fabbccf7e 100644 > --- a/drivers/gpu/drm/i915/display/intel_gmbus.c > +++ b/drivers/gpu/drm/i915/display/intel_gmbus.c > @@ -155,7 +155,10 @@ static const struct gmbus_pin *get_gmbus_pin(struct drm_i915_private *i915, > const struct gmbus_pin *pins; > size_t size; > > - if (INTEL_PCH_TYPE(i915) >= PCH_DG2) { > + if (INTEL_PCH_TYPE(i915) >= PCH_LNL) { > + pins = gmbus_pins_mtp; > + size = ARRAY_SIZE(gmbus_pins_mtp); > + } else if (INTEL_PCH_TYPE(i915) >= PCH_DG2) { > pins = gmbus_pins_dg2; > size = ARRAY_SIZE(gmbus_pins_dg2); > } else if (INTEL_PCH_TYPE(i915) >= PCH_DG1) { > -- > 2.40.1 >
On Wed, Aug 23, 2023 at 01:49:21PM -0700, Matt Roper wrote: >On Wed, Aug 23, 2023 at 10:07:29AM -0700, Lucas De Marchi wrote: >> LNL's south display uses the same table as MTP. Check for LNL's fake PCH >> to make it consistent with the other checks. >> >> The VBT table doesn't contain the VBT -> spec mapping for LNL. Like in >> other cases, uses the same as the previous platform. >> >> Bspec: 68971, 20124 >> Cc: Anusha Srivatsa <anusha.srivatsa@intel.com> >> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> >> --- >> drivers/gpu/drm/i915/display/intel_bios.c | 2 +- >> drivers/gpu/drm/i915/display/intel_gmbus.c | 5 ++++- >> 2 files changed, 5 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c >> index 097c1f23d3ae..3772b91e155c 100644 >> --- a/drivers/gpu/drm/i915/display/intel_bios.c >> +++ b/drivers/gpu/drm/i915/display/intel_bios.c >> @@ -2195,7 +2195,7 @@ static u8 map_ddc_pin(struct drm_i915_private *i915, u8 vbt_pin) >> const u8 *ddc_pin_map; >> int i, n_entries; >> >> - if (HAS_PCH_MTP(i915) || IS_ALDERLAKE_P(i915)) { >> + if (HAS_PCH_MTP(i915) || IS_ALDERLAKE_P(i915) || IS_LUNARLAKE(i915)) { > >The LUNARLAKE here vs PCH_LNL below seems inconsistent. Either way, we >should probably put the newer platform first in the condition. > >Aside from those If we drop IS_LUNARLAKE, then we need to check for something else here. What about doing this? if (INTEL_PCH_TYPE(i915) >= PCH_LNL || HAS_PCH_MTP(i915) || IS_ALDERLAKE_P(i915)) { ? > > Reviewed-by: Matt Roper <matthew.d.roper@intel.com> > >> ddc_pin_map = adlp_ddc_pin_map; >> n_entries = ARRAY_SIZE(adlp_ddc_pin_map); >> } else if (IS_ALDERLAKE_S(i915)) { >> diff --git a/drivers/gpu/drm/i915/display/intel_gmbus.c b/drivers/gpu/drm/i915/display/intel_gmbus.c >> index e95ddb580ef6..801fabbccf7e 100644 >> --- a/drivers/gpu/drm/i915/display/intel_gmbus.c >> +++ b/drivers/gpu/drm/i915/display/intel_gmbus.c >> @@ -155,7 +155,10 @@ static const struct gmbus_pin *get_gmbus_pin(struct drm_i915_private *i915, >> const struct gmbus_pin *pins; >> size_t size; >> >> - if (INTEL_PCH_TYPE(i915) >= PCH_DG2) { >> + if (INTEL_PCH_TYPE(i915) >= PCH_LNL) { >> + pins = gmbus_pins_mtp; >> + size = ARRAY_SIZE(gmbus_pins_mtp); >> + } else if (INTEL_PCH_TYPE(i915) >= PCH_DG2) { >> pins = gmbus_pins_dg2; >> size = ARRAY_SIZE(gmbus_pins_dg2); >> } else if (INTEL_PCH_TYPE(i915) >= PCH_DG1) { >> -- >> 2.40.1 >> > >-- >Matt Roper >Graphics Software Engineer >Linux GPU Platform Enablement >Intel Corporation
On Thu, Aug 24, 2023 at 09:25:54PM -0700, Lucas De Marchi wrote: > On Wed, Aug 23, 2023 at 01:49:21PM -0700, Matt Roper wrote: > > On Wed, Aug 23, 2023 at 10:07:29AM -0700, Lucas De Marchi wrote: > > > LNL's south display uses the same table as MTP. Check for LNL's fake PCH > > > to make it consistent with the other checks. > > > > > > The VBT table doesn't contain the VBT -> spec mapping for LNL. Like in > > > other cases, uses the same as the previous platform. > > > > > > Bspec: 68971, 20124 > > > Cc: Anusha Srivatsa <anusha.srivatsa@intel.com> > > > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> > > > --- > > > drivers/gpu/drm/i915/display/intel_bios.c | 2 +- > > > drivers/gpu/drm/i915/display/intel_gmbus.c | 5 ++++- > > > 2 files changed, 5 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c > > > index 097c1f23d3ae..3772b91e155c 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_bios.c > > > +++ b/drivers/gpu/drm/i915/display/intel_bios.c > > > @@ -2195,7 +2195,7 @@ static u8 map_ddc_pin(struct drm_i915_private *i915, u8 vbt_pin) > > > const u8 *ddc_pin_map; > > > int i, n_entries; > > > > > > - if (HAS_PCH_MTP(i915) || IS_ALDERLAKE_P(i915)) { > > > + if (HAS_PCH_MTP(i915) || IS_ALDERLAKE_P(i915) || IS_LUNARLAKE(i915)) { > > > > The LUNARLAKE here vs PCH_LNL below seems inconsistent. Either way, we > > should probably put the newer platform first in the condition. > > > > Aside from those > > If we drop IS_LUNARLAKE, then we need to check for something else here. > What about doing this? > > > if (INTEL_PCH_TYPE(i915) >= PCH_LNL || HAS_PCH_MTP(i915) || IS_ALDERLAKE_P(i915)) { > > ? Yeah, that's fine in the short term. Although I wonder if moving PCH_LNL before the discrete GPUs might simplify the various conditions that need to match on SDE behavior since it's probably closer to the MTL SDE than to the discrete SDEs? I haven't looked through all the conditions to see which order is simplest overall. Longer term I think we need to replace the whole intel_pch enum with an intel_sde enum or something since this stuff generally isn't related to PCH anymore, and we should be looking at different things to determine the exact version of the SDE logic. - For MTL, both NDE and SDE live on the same die (SOC); PCH isn't involved. - For LNL, NDE lives on the compute die and SDE lives on the SOC die (as does the PICA, so the PICA ID can be used to fingerprint a specific version). Matt > > > > > Reviewed-by: Matt Roper <matthew.d.roper@intel.com> > > > > > ddc_pin_map = adlp_ddc_pin_map; > > > n_entries = ARRAY_SIZE(adlp_ddc_pin_map); > > > } else if (IS_ALDERLAKE_S(i915)) { > > > diff --git a/drivers/gpu/drm/i915/display/intel_gmbus.c b/drivers/gpu/drm/i915/display/intel_gmbus.c > > > index e95ddb580ef6..801fabbccf7e 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_gmbus.c > > > +++ b/drivers/gpu/drm/i915/display/intel_gmbus.c > > > @@ -155,7 +155,10 @@ static const struct gmbus_pin *get_gmbus_pin(struct drm_i915_private *i915, > > > const struct gmbus_pin *pins; > > > size_t size; > > > > > > - if (INTEL_PCH_TYPE(i915) >= PCH_DG2) { > > > + if (INTEL_PCH_TYPE(i915) >= PCH_LNL) { > > > + pins = gmbus_pins_mtp; > > > + size = ARRAY_SIZE(gmbus_pins_mtp); > > > + } else if (INTEL_PCH_TYPE(i915) >= PCH_DG2) { > > > pins = gmbus_pins_dg2; > > > size = ARRAY_SIZE(gmbus_pins_dg2); > > > } else if (INTEL_PCH_TYPE(i915) >= PCH_DG1) { > > > -- > > > 2.40.1 > > > > > > > -- > > Matt Roper > > Graphics Software Engineer > > Linux GPU Platform Enablement > > Intel Corporation
On Fri, Aug 25, 2023 at 02:55:33PM -0700, Matt Roper wrote: >On Thu, Aug 24, 2023 at 09:25:54PM -0700, Lucas De Marchi wrote: >> On Wed, Aug 23, 2023 at 01:49:21PM -0700, Matt Roper wrote: >> > On Wed, Aug 23, 2023 at 10:07:29AM -0700, Lucas De Marchi wrote: >> > > LNL's south display uses the same table as MTP. Check for LNL's fake PCH >> > > to make it consistent with the other checks. >> > > >> > > The VBT table doesn't contain the VBT -> spec mapping for LNL. Like in >> > > other cases, uses the same as the previous platform. >> > > >> > > Bspec: 68971, 20124 >> > > Cc: Anusha Srivatsa <anusha.srivatsa@intel.com> >> > > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> >> > > --- >> > > drivers/gpu/drm/i915/display/intel_bios.c | 2 +- >> > > drivers/gpu/drm/i915/display/intel_gmbus.c | 5 ++++- >> > > 2 files changed, 5 insertions(+), 2 deletions(-) >> > > >> > > diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c >> > > index 097c1f23d3ae..3772b91e155c 100644 >> > > --- a/drivers/gpu/drm/i915/display/intel_bios.c >> > > +++ b/drivers/gpu/drm/i915/display/intel_bios.c >> > > @@ -2195,7 +2195,7 @@ static u8 map_ddc_pin(struct drm_i915_private *i915, u8 vbt_pin) >> > > const u8 *ddc_pin_map; >> > > int i, n_entries; >> > > >> > > - if (HAS_PCH_MTP(i915) || IS_ALDERLAKE_P(i915)) { >> > > + if (HAS_PCH_MTP(i915) || IS_ALDERLAKE_P(i915) || IS_LUNARLAKE(i915)) { >> > >> > The LUNARLAKE here vs PCH_LNL below seems inconsistent. Either way, we >> > should probably put the newer platform first in the condition. >> > >> > Aside from those >> >> If we drop IS_LUNARLAKE, then we need to check for something else here. >> What about doing this? >> >> >> if (INTEL_PCH_TYPE(i915) >= PCH_LNL || HAS_PCH_MTP(i915) || IS_ALDERLAKE_P(i915)) { >> >> ? > >Yeah, that's fine in the short term. Although I wonder if moving >PCH_LNL before the discrete GPUs might simplify the various conditions >that need to match on SDE behavior since it's probably closer to the MTL >SDE than to the discrete SDEs? I haven't looked through all the >conditions to see which order is simplest overall. > >Longer term I think we need to replace the whole intel_pch enum with an >intel_sde enum or something since this stuff generally isn't related to >PCH anymore, and we should be looking at different things to determine >the exact version of the SDE logic. > > - For MTL, both NDE and SDE live on the same die (SOC); PCH isn't > involved. > - For LNL, NDE lives on the compute die and SDE lives on the SOC die > (as does the PICA, so the PICA ID can be used to fingerprint a > specific version). Yeah, agreed. Lucas De Marchi > > >Matt > >> >> > >> > Reviewed-by: Matt Roper <matthew.d.roper@intel.com> >> > >> > > ddc_pin_map = adlp_ddc_pin_map; >> > > n_entries = ARRAY_SIZE(adlp_ddc_pin_map); >> > > } else if (IS_ALDERLAKE_S(i915)) { >> > > diff --git a/drivers/gpu/drm/i915/display/intel_gmbus.c b/drivers/gpu/drm/i915/display/intel_gmbus.c >> > > index e95ddb580ef6..801fabbccf7e 100644 >> > > --- a/drivers/gpu/drm/i915/display/intel_gmbus.c >> > > +++ b/drivers/gpu/drm/i915/display/intel_gmbus.c >> > > @@ -155,7 +155,10 @@ static const struct gmbus_pin *get_gmbus_pin(struct drm_i915_private *i915, >> > > const struct gmbus_pin *pins; >> > > size_t size; >> > > >> > > - if (INTEL_PCH_TYPE(i915) >= PCH_DG2) { >> > > + if (INTEL_PCH_TYPE(i915) >= PCH_LNL) { >> > > + pins = gmbus_pins_mtp; >> > > + size = ARRAY_SIZE(gmbus_pins_mtp); >> > > + } else if (INTEL_PCH_TYPE(i915) >= PCH_DG2) { >> > > pins = gmbus_pins_dg2; >> > > size = ARRAY_SIZE(gmbus_pins_dg2); >> > > } else if (INTEL_PCH_TYPE(i915) >= PCH_DG1) { >> > > -- >> > > 2.40.1 >> > > >> > >> > -- >> > Matt Roper >> > Graphics Software Engineer >> > Linux GPU Platform Enablement >> > Intel Corporation > >-- >Matt Roper >Graphics Software Engineer >Linux GPU Platform Enablement >Intel Corporation
diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c index 097c1f23d3ae..3772b91e155c 100644 --- a/drivers/gpu/drm/i915/display/intel_bios.c +++ b/drivers/gpu/drm/i915/display/intel_bios.c @@ -2195,7 +2195,7 @@ static u8 map_ddc_pin(struct drm_i915_private *i915, u8 vbt_pin) const u8 *ddc_pin_map; int i, n_entries; - if (HAS_PCH_MTP(i915) || IS_ALDERLAKE_P(i915)) { + if (HAS_PCH_MTP(i915) || IS_ALDERLAKE_P(i915) || IS_LUNARLAKE(i915)) { ddc_pin_map = adlp_ddc_pin_map; n_entries = ARRAY_SIZE(adlp_ddc_pin_map); } else if (IS_ALDERLAKE_S(i915)) { diff --git a/drivers/gpu/drm/i915/display/intel_gmbus.c b/drivers/gpu/drm/i915/display/intel_gmbus.c index e95ddb580ef6..801fabbccf7e 100644 --- a/drivers/gpu/drm/i915/display/intel_gmbus.c +++ b/drivers/gpu/drm/i915/display/intel_gmbus.c @@ -155,7 +155,10 @@ static const struct gmbus_pin *get_gmbus_pin(struct drm_i915_private *i915, const struct gmbus_pin *pins; size_t size; - if (INTEL_PCH_TYPE(i915) >= PCH_DG2) { + if (INTEL_PCH_TYPE(i915) >= PCH_LNL) { + pins = gmbus_pins_mtp; + size = ARRAY_SIZE(gmbus_pins_mtp); + } else if (INTEL_PCH_TYPE(i915) >= PCH_DG2) { pins = gmbus_pins_dg2; size = ARRAY_SIZE(gmbus_pins_dg2); } else if (INTEL_PCH_TYPE(i915) >= PCH_DG1) {
LNL's south display uses the same table as MTP. Check for LNL's fake PCH to make it consistent with the other checks. The VBT table doesn't contain the VBT -> spec mapping for LNL. Like in other cases, uses the same as the previous platform. Bspec: 68971, 20124 Cc: Anusha Srivatsa <anusha.srivatsa@intel.com> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> --- drivers/gpu/drm/i915/display/intel_bios.c | 2 +- drivers/gpu/drm/i915/display/intel_gmbus.c | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-)