[PATCH] drm/i915/mtl: Add fake PCH for Meteor Lake
Kalvala, Haridhar
haridhar.kalvala at intel.com
Tue Dec 19 18:57:25 UTC 2023
On 12/19/2023 10:39 PM, Matt Roper wrote:
> On Tue, Dec 19, 2023 at 02:58:00PM +0530, Haridhar Kalvala wrote:
>> Correct the implementation trying to detect MTL PCH with
>> the MTL fake PCH id.
>>
>> On MTL, both the North Display (NDE) and South Display (SDE) functionality
>> reside on the same die (the SoC die in this case), unlike many past
>> platforms where the SDE was on a separate PCH die. The code is (badly)
>> structured today in a way that assumes the SDE is always on the PCH for
>> modern platforms, so on platforms where we don't actually need to identify
>> the PCH to figure out how the SDE behaves (i.e., all DG1/2 GPUs as well as
>> MTL and LNL),we've been assigning a "fake PCH" as a quickhack that allows
>> us to avoid restructuring a bunch of the code.we've been assigning a
>> "fake PCH" as a quick hack that allows us to avoid restructuring a bunch
>> of the code.
>>
>> Removed unused macros of LNL amd MTL as well.
> In the future, make sure you generate your patches with "-v<#>" and add
> a changelog to the commit message so it's clear what's changed since the
> previous revision.
sure. With commit title changed, this will take new series so started as
initial patches
taking previous comments into account. I will add version to next patchset.
>> Signed-off-by: Haridhar Kalvala <haridhar.kalvala at intel.com>
>> ---
>> drivers/gpu/drm/i915/display/intel_backlight.c | 2 +-
>> drivers/gpu/drm/i915/display/intel_bios.c | 3 +--
>> drivers/gpu/drm/i915/display/intel_cdclk.c | 2 +-
>> drivers/gpu/drm/i915/display/intel_display_irq.c | 2 +-
>> drivers/gpu/drm/i915/display/intel_gmbus.c | 5 +----
>> drivers/gpu/drm/i915/display/intel_hotplug_irq.c | 6 ++----
>> drivers/gpu/drm/i915/display/intel_pps.c | 2 +-
>> drivers/gpu/drm/i915/soc/intel_pch.c | 16 ++++++++--------
>> drivers/gpu/drm/i915/soc/intel_pch.h | 6 +-----
>> 9 files changed, 17 insertions(+), 27 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_backlight.c b/drivers/gpu/drm/i915/display/intel_backlight.c
>> index 612d4cd9dacb..696ae59874a9 100644
>> --- a/drivers/gpu/drm/i915/display/intel_backlight.c
>> +++ b/drivers/gpu/drm/i915/display/intel_backlight.c
>> @@ -1465,7 +1465,7 @@ static bool cnp_backlight_controller_is_valid(struct drm_i915_private *i915, int
>>
>> if (controller == 1 &&
>> INTEL_PCH_TYPE(i915) >= PCH_ICP &&
>> - INTEL_PCH_TYPE(i915) < PCH_MTP)
>> + INTEL_PCH_TYPE(i915) <= PCH_ADP)
>> return intel_de_read(i915, SOUTH_CHICKEN1) & ICP_SECOND_PPS_IO_SELECT;
>>
>> return true;
>> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
>> index aa169b0055e9..0e61e424802e 100644
>> --- a/drivers/gpu/drm/i915/display/intel_bios.c
>> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
>> @@ -2204,8 +2204,7 @@ static u8 map_ddc_pin(struct drm_i915_private *i915, u8 vbt_pin)
>> if (IS_DGFX(i915))
>> return vbt_pin;
>>
>> - if (INTEL_PCH_TYPE(i915) >= PCH_LNL || HAS_PCH_MTP(i915) ||
>> - IS_ALDERLAKE_P(i915)) {
>> + if (INTEL_PCH_TYPE(i915) >= PCH_MTL || IS_ALDERLAKE_P(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_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
>> index c985ebb6831a..2e6e55d3e885 100644
>> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
>> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
>> @@ -3469,7 +3469,7 @@ u32 intel_read_rawclk(struct drm_i915_private *dev_priv)
>>
>> if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1)
>> freq = dg1_rawclk(dev_priv);
>> - else if (INTEL_PCH_TYPE(dev_priv) >= PCH_MTP)
>> + else if (INTEL_PCH_TYPE(dev_priv) >= PCH_MTL)
> Since PCH_MTL > PCH_DG1, this condition can never be reached. You need
> to re-order the conditions here so that they go from highest to lowest:
>
> if ( >= MTL )
> else if ( >= DG1 )
> else if ( >= CNP )
> ...
>
> Once you do this, it looks like it will also solve a pre-existing LNL
> bug that was causing LNL to (incorrectly) take the DG1 path instead of
> the MTL path. Bspec 68969 confirms that LNL should be inheriting MTL's
> behavior, not DG1's.
Thank you. Will reorder.
>> /*
>> * MTL always uses a 38.4 MHz rawclk. The bspec tells us
>> * "RAWCLK_FREQ defaults to the values for 38.4 and does
>> diff --git a/drivers/gpu/drm/i915/display/intel_display_irq.c b/drivers/gpu/drm/i915/display/intel_display_irq.c
>> index a7d8f3fc98de..e318e24d1efd 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_irq.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display_irq.c
>> @@ -986,7 +986,7 @@ static void gen8_read_and_ack_pch_irqs(struct drm_i915_private *i915, u32 *pch_i
>> * their flags both in the PICA and SDE IIR.
>> */
>> if (*pch_iir & SDE_PICAINTERRUPT) {
>> - drm_WARN_ON(&i915->drm, INTEL_PCH_TYPE(i915) < PCH_MTP);
>> + drm_WARN_ON(&i915->drm, INTEL_PCH_TYPE(i915) <= PCH_ADP);
> I think you can keep this one "< PCH_MTL." It's a bug if we ever see a
> PICA interrupt on DG1/DG2 since neither of those platforms had a PICA
> either.
>
>
> Matt
Sure.
Thanks & regards,
Haridhar Kalvala
>>
>> pica_ier = intel_de_rmw(i915, PICAINTERRUPT_IER, ~0, 0);
>> *pica_iir = intel_de_read(i915, PICAINTERRUPT_IIR);
>> diff --git a/drivers/gpu/drm/i915/display/intel_gmbus.c b/drivers/gpu/drm/i915/display/intel_gmbus.c
>> index 40d7b6f3f489..854566ba5414 100644
>> --- a/drivers/gpu/drm/i915/display/intel_gmbus.c
>> +++ b/drivers/gpu/drm/i915/display/intel_gmbus.c
>> @@ -155,7 +155,7 @@ 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_LNL) {
>> + if (INTEL_PCH_TYPE(i915) >= PCH_MTL) {
>> pins = gmbus_pins_mtp;
>> size = ARRAY_SIZE(gmbus_pins_mtp);
>> } else if (INTEL_PCH_TYPE(i915) >= PCH_DG2) {
>> @@ -164,9 +164,6 @@ static const struct gmbus_pin *get_gmbus_pin(struct drm_i915_private *i915,
>> } else if (INTEL_PCH_TYPE(i915) >= PCH_DG1) {
>> pins = gmbus_pins_dg1;
>> size = ARRAY_SIZE(gmbus_pins_dg1);
>> - } else if (INTEL_PCH_TYPE(i915) >= PCH_MTP) {
>> - pins = gmbus_pins_mtp;
>> - size = ARRAY_SIZE(gmbus_pins_mtp);
>> } else if (INTEL_PCH_TYPE(i915) >= PCH_ICP) {
>> pins = gmbus_pins_icp;
>> size = ARRAY_SIZE(gmbus_pins_icp);
>> diff --git a/drivers/gpu/drm/i915/display/intel_hotplug_irq.c b/drivers/gpu/drm/i915/display/intel_hotplug_irq.c
>> index 04f62f27ad74..76076509f771 100644
>> --- a/drivers/gpu/drm/i915/display/intel_hotplug_irq.c
>> +++ b/drivers/gpu/drm/i915/display/intel_hotplug_irq.c
>> @@ -163,12 +163,10 @@ static void intel_hpd_init_pins(struct drm_i915_private *dev_priv)
>> (!HAS_PCH_SPLIT(dev_priv) || HAS_PCH_NOP(dev_priv)))
>> return;
>>
>> - if (INTEL_PCH_TYPE(dev_priv) >= PCH_LNL)
>> + if (INTEL_PCH_TYPE(dev_priv) >= PCH_MTL)
>> hpd->pch_hpd = hpd_mtp;
>> else if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1)
>> hpd->pch_hpd = hpd_sde_dg1;
>> - else if (INTEL_PCH_TYPE(dev_priv) >= PCH_MTP)
>> - hpd->pch_hpd = hpd_mtp;
>> else if (INTEL_PCH_TYPE(dev_priv) >= PCH_ICP)
>> hpd->pch_hpd = hpd_icp;
>> else if (HAS_PCH_CNP(dev_priv) || HAS_PCH_SPT(dev_priv))
>> @@ -1139,7 +1137,7 @@ static void xelpdp_hpd_irq_setup(struct drm_i915_private *i915)
>>
>> if (INTEL_PCH_TYPE(i915) >= PCH_LNL)
>> xe2lpd_sde_hpd_irq_setup(i915);
>> - else if (INTEL_PCH_TYPE(i915) >= PCH_MTP)
>> + else if (INTEL_PCH_TYPE(i915) >= PCH_MTL)
>> mtp_hpd_irq_setup(i915);
>> }
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_pps.c b/drivers/gpu/drm/i915/display/intel_pps.c
>> index a8fa3a20990e..2d65a538f83e 100644
>> --- a/drivers/gpu/drm/i915/display/intel_pps.c
>> +++ b/drivers/gpu/drm/i915/display/intel_pps.c
>> @@ -366,7 +366,7 @@ static bool intel_pps_is_valid(struct intel_dp *intel_dp)
>>
>> if (intel_dp->pps.pps_idx == 1 &&
>> INTEL_PCH_TYPE(i915) >= PCH_ICP &&
>> - INTEL_PCH_TYPE(i915) < PCH_MTP)
>> + INTEL_PCH_TYPE(i915) <= PCH_ADP)
>> return intel_de_read(i915, SOUTH_CHICKEN1) & ICP_SECOND_PPS_IO_SELECT;
>>
>> return true;
>> diff --git a/drivers/gpu/drm/i915/soc/intel_pch.c b/drivers/gpu/drm/i915/soc/intel_pch.c
>> index 240beafb38ed..3cad6dac06b0 100644
>> --- a/drivers/gpu/drm/i915/soc/intel_pch.c
>> +++ b/drivers/gpu/drm/i915/soc/intel_pch.c
>> @@ -140,11 +140,6 @@ intel_pch_type(const struct drm_i915_private *dev_priv, unsigned short id)
>> drm_WARN_ON(&dev_priv->drm, !IS_ALDERLAKE_S(dev_priv) &&
>> !IS_ALDERLAKE_P(dev_priv));
>> return PCH_ADP;
>> - case INTEL_PCH_MTP_DEVICE_ID_TYPE:
>> - case INTEL_PCH_MTP2_DEVICE_ID_TYPE:
>> - drm_dbg_kms(&dev_priv->drm, "Found Meteor Lake PCH\n");
>> - drm_WARN_ON(&dev_priv->drm, !IS_METEORLAKE(dev_priv));
>> - return PCH_MTP;
>> default:
>> return PCH_NONE;
>> }
>> @@ -173,9 +168,7 @@ intel_virt_detect_pch(const struct drm_i915_private *dev_priv,
>> * make an educated guess as to which PCH is really there.
>> */
>>
>> - if (IS_METEORLAKE(dev_priv))
>> - id = INTEL_PCH_MTP_DEVICE_ID_TYPE;
>> - else if (IS_ALDERLAKE_S(dev_priv) || IS_ALDERLAKE_P(dev_priv))
>> + if (IS_ALDERLAKE_S(dev_priv) || IS_ALDERLAKE_P(dev_priv))
>> id = INTEL_PCH_ADP_DEVICE_ID_TYPE;
>> else if (IS_TIGERLAKE(dev_priv) || IS_ROCKETLAKE(dev_priv))
>> id = INTEL_PCH_TGP_DEVICE_ID_TYPE;
>> @@ -225,6 +218,13 @@ void intel_detect_pch(struct drm_i915_private *dev_priv)
>> if (DISPLAY_VER(dev_priv) >= 20) {
>> dev_priv->pch_type = PCH_LNL;
>> return;
>> + } else if (IS_METEORLAKE(dev_priv)) {
>> + /*
>> + * Both north display and south display are on the SoC die.
>> + * The real PCH is uninvolved in display.
>> + */
>> + dev_priv->pch_type = PCH_MTL;
>> + return;
>> } else if (IS_DG2(dev_priv)) {
>> dev_priv->pch_type = PCH_DG2;
>> return;
>> diff --git a/drivers/gpu/drm/i915/soc/intel_pch.h b/drivers/gpu/drm/i915/soc/intel_pch.h
>> index 1b03ea60a7a8..89e89ede265d 100644
>> --- a/drivers/gpu/drm/i915/soc/intel_pch.h
>> +++ b/drivers/gpu/drm/i915/soc/intel_pch.h
>> @@ -25,11 +25,11 @@ enum intel_pch {
>> PCH_ICP, /* Ice Lake/Jasper Lake PCH */
>> PCH_TGP, /* Tiger Lake/Mule Creek Canyon PCH */
>> PCH_ADP, /* Alder Lake PCH */
>> - PCH_MTP, /* Meteor Lake PCH */
>>
>> /* Fake PCHs, functionality handled on the same PCI dev */
>> PCH_DG1 = 1024,
>> PCH_DG2,
>> + PCH_MTL,
>> PCH_LNL,
>> };
>>
>> @@ -59,16 +59,12 @@ enum intel_pch {
>> #define INTEL_PCH_ADP2_DEVICE_ID_TYPE 0x5180
>> #define INTEL_PCH_ADP3_DEVICE_ID_TYPE 0x7A00
>> #define INTEL_PCH_ADP4_DEVICE_ID_TYPE 0x5480
>> -#define INTEL_PCH_MTP_DEVICE_ID_TYPE 0x7E00
>> -#define INTEL_PCH_MTP2_DEVICE_ID_TYPE 0xAE00
>> #define INTEL_PCH_P2X_DEVICE_ID_TYPE 0x7100
>> #define INTEL_PCH_P3X_DEVICE_ID_TYPE 0x7000
>> #define INTEL_PCH_QEMU_DEVICE_ID_TYPE 0x2900 /* qemu q35 has 2918 */
>>
>> #define INTEL_PCH_TYPE(dev_priv) ((dev_priv)->pch_type)
>> #define INTEL_PCH_ID(dev_priv) ((dev_priv)->pch_id)
>> -#define HAS_PCH_LNL(dev_priv) (INTEL_PCH_TYPE(dev_priv) == PCH_LNL)
>> -#define HAS_PCH_MTP(dev_priv) (INTEL_PCH_TYPE(dev_priv) == PCH_MTP)
>> #define HAS_PCH_DG2(dev_priv) (INTEL_PCH_TYPE(dev_priv) == PCH_DG2)
>> #define HAS_PCH_ADP(dev_priv) (INTEL_PCH_TYPE(dev_priv) == PCH_ADP)
>> #define HAS_PCH_DG1(dev_priv) (INTEL_PCH_TYPE(dev_priv) == PCH_DG1)
>> --
>> 2.25.1
>>
--
Regards,
Haridhar Kalvala
More information about the Intel-gfx
mailing list