On Fri, 13 Dec 2024, Dnyaneshwar Bhadane <[email protected]> wrote:
> Separate MTL-U platform PCI ids in one define macro.
>
> Add the MTL U/ARL U as subplatform member in MTL platform description
> structure to use display.platform.<platform> from intel_display
> structure instead of IS_<PLATFORM>() in display code path.
>
> Signed-off-by: Dnyaneshwar Bhadane <[email protected]>
> ---
>  .../drm/i915/display/intel_display_device.c   | 21 +++++++++++++++++++
>  .../drm/i915/display/intel_display_device.h   |  2 ++
>  include/drm/intel/pciids.h                    |  5 ++++-
>  3 files changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_device.c 
> b/drivers/gpu/drm/i915/display/intel_display_device.c
> index 68cb7f9b9ef3..5dc689a8b1ae 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_device.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_device.c
> @@ -1357,6 +1357,16 @@ static const struct intel_display_device_info 
> xe2_hpd_display = {
>               BIT(PORT_TC1) | BIT(PORT_TC2) | BIT(PORT_TC3) | BIT(PORT_TC4),
>  };
>  
> +static const u16 arl_u_ids[] = {
> +     INTEL_ARL_U_IDS(ID),
> +     0
> +};
> +
> +static const u16 mtl_u_ids[] = {
> +     INTEL_MTL_U_IDS(ID),
> +     0
> +};

We don't have arrowlake platform definition. They're all just
meteorlakes. Do you actually need the mtl-u/arl-u distinction, or do you
just need mtl+arl vs. mtl-u+arl-u distinction?

I.e. could we just have 

static const u16 mtl_u_ids[] = {
        INTEL_MTL_U_IDS(ID),
        INTEL_ARL_U_IDS(ID),
        0
};

And call them all mtl-u?

> +
>  /*
>   * Do not initialize the .info member of the platform desc for GMD ID based
>   * platforms. Their display will be probed automatically based on the IP 
> version
> @@ -1364,6 +1374,17 @@ static const struct intel_display_device_info 
> xe2_hpd_display = {
>   */
>  static const struct platform_desc mtl_desc = {
>       PLATFORM(meteorlake),
> +     .subplatforms = (const struct subplatform_desc[]) {
> +             {
> +                     SUBPLATFORM(meteorlake, u),
> +                     .pciidlist = mtl_u_ids,
> +             },
> +             {
> +                     SUBPLATFORM(arrowlake, u),
> +                     .pciidlist = arl_u_ids,

You're defining subplatfroms for meteorlake. All the platform parameters
for SUBPLATFORM() *must* match the PLATFORM() above.

> +             },
> +             {},
> +     }
>  };
>  
>  static const struct platform_desc lnl_desc = {
> diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h 
> b/drivers/gpu/drm/i915/display/intel_display_device.h
> index 9a333d9e6601..87a614e2dfab 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_device.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_device.h
> @@ -96,6 +96,8 @@ struct pci_dev;
>       func(dg2_g12) \
>       /* Display ver 14 (based on GMD ID) */ \
>       func(meteorlake) \
> +     func(meteorlake_u) \
> +     func(arrowlake_u) \

The naming needs to be <platform>_<subplatform>. We don't have arrowlake
platform, so we can't have arrowlake_u.

Either we can just put all mtl+arl u's together in meteorlake_u, or we
define arl-u as meteorlake_arrowlake_u with meteorlake being the
platform and arrowlake_u the subplatform.

>       /* Display ver 20 (based on GMD ID) */ \
>       func(lunarlake) \
>       /* Display ver 14.1 (based on GMD ID) */ \
> diff --git a/include/drm/intel/pciids.h b/include/drm/intel/pciids.h
> index c6518b0992cf..f29034ccb36c 100644
> --- a/include/drm/intel/pciids.h
> +++ b/include/drm/intel/pciids.h
> @@ -811,9 +811,12 @@
>       INTEL_ARL_S_IDS(MACRO__, ## __VA_ARGS__)
>  
>  /* MTL */
> +#define INTEL_MTL_U_IDS(MACRO__, ...) \
> +     MACRO__(0x7D45, ## __VA_ARGS__)
> +
>  #define INTEL_MTL_IDS(MACRO__, ...) \
>       MACRO__(0x7D40, ## __VA_ARGS__), \
> -     MACRO__(0x7D45, ## __VA_ARGS__), \
> +     INTEL_MTL_U_IDS(MACRO__, ## __VA_ARGS__), \
>       MACRO__(0x7D55, ## __VA_ARGS__), \
>       MACRO__(0x7D60, ## __VA_ARGS__), \
>       MACRO__(0x7DD5, ## __VA_ARGS__)

-- 
Jani Nikula, Intel

Reply via email to