[Intel-gfx] [Intel-xe] [PATCH 31/42] drm/i915/lnl: Add gmbus/ddc support

[Lucas De Marchi]

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 at intel.com>
>> > > Signed-off-by: Lucas De Marchi <lucas.demarchi at 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 at 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