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.sriva...@intel.com>
> > > Signed-off-by: Lucas De Marchi <lucas.demar...@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.ro...@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
