Re: [PATCH 1/2] drm/i915/jsl: Split EHL/JSL platform info and PCI ids

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 






From: Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx>
Sent: Monday, September 28, 2020 7:07 PM
To: Surendrakumar Upadhyay, TejaskumarX <tejaskumarx.surendrakumar.upadhyay@xxxxxxxxx>; Vivi, Rodrigo <rodrigo.vivi@xxxxxxxxx>; airlied@xxxxxxxx <airlied@xxxxxxxx>; daniel@xxxxxxxx <daniel@xxxxxxxx>; intel-gfx@xxxxxxxxxxxxxxxxxxxxx <intel-gfx@xxxxxxxxxxxxxxxxxxxxx>; dri-devel@xxxxxxxxxxxxxxxxxxxxx <dri-devel@xxxxxxxxxxxxxxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx <linux-kernel@xxxxxxxxxxxxxxx>; Ausmus, James <james.ausmus@xxxxxxxxx>; Roper, Matthew D <matthew.d.roper@xxxxxxxxx>; Souza, Jose <jose.souza@xxxxxxxxx>; ville.syrjala@xxxxxxxxxxxxxxx <ville.syrjala@xxxxxxxxxxxxxxx>; De Marchi, Lucas <lucas.demarchi@xxxxxxxxx>; Pandey, Hariom <hariom.pandey@xxxxxxxxx>
Subject: Re: [PATCH 1/2] drm/i915/jsl: Split EHL/JSL platform info and PCI ids
 
On Mon, 28 Sep 2020, Tejas Upadhyay <tejaskumarx.surendrakumar.upadhyay@xxxxxxxxx> wrote:
> Split the basic platform definition, macros, and PCI IDs to
> differentiate between EHL and JSL platforms.
>
> Signed-off-by: Tejas Upadhyay <tejaskumarx.surendrakumar.upadhyay@xxxxxxxxx>
> ---
>  drivers/gpu/drm/i915/i915_drv.h          | 4 +++-
>  drivers/gpu/drm/i915/i915_pci.c          | 9 +++++++++
>  drivers/gpu/drm/i915/intel_device_info.c | 1 +
>  drivers/gpu/drm/i915/intel_device_info.h | 1 +
>  include/drm/i915_pciids.h                | 9 ++++++---
>  5 files changed, 20 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 72a9449b674e..4f20acebb038 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1417,7 +1417,9 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
>  #define IS_COMETLAKE(dev_priv)       IS_PLATFORM(dev_priv, INTEL_COMETLAKE)
>  #define IS_CANNONLAKE(dev_priv)      IS_PLATFORM(dev_priv, INTEL_CANNONLAKE)
>  #define IS_ICELAKE(dev_priv) IS_PLATFORM(dev_priv, INTEL_ICELAKE)
> -#define IS_ELKHARTLAKE(dev_priv)     IS_PLATFORM(dev_priv, INTEL_ELKHARTLAKE)
> +#define IS_ELKHARTLAKE(dev_priv) (IS_PLATFORM(dev_priv, INTEL_ELKHARTLAKE) || \
> +                             IS_PLATFORM(dev_priv, INTEL_JASPERLAKE))
> +#define IS_JASPERLAKE(dev_priv)      IS_PLATFORM(dev_priv, INTEL_JASPERLAKE)

I think we've learned from history that we want the platform checks to
be independent. I.e. if you need to split ELK and JSP, you need to make
IS_ELKHARTLAKE() match *only* ELK, and you need to replace every current
IS_ELKHARTLAKE() check with IS_ELKHARTLAKE() || IS_JASPERLAKE().

We've been here before, and we've thought before that we can get by with
the minimal change. It's just postponing the inevitable and generates
confusion.

BR,
Jani.

Tejas :  Replacing IS_ELKHARTLAKE() || IS_JASPERLAKE() everywhere will make lot of changes at each place. To avoid huge change and to differentiate between platforms we have taken this way. Do you think we still change it everywhere? Do you have example where it can harm this change?

>  #define IS_TIGERLAKE(dev_priv)       IS_PLATFORM(dev_priv, INTEL_TIGERLAKE)
>  #define IS_ROCKETLAKE(dev_priv)      IS_PLATFORM(dev_priv, INTEL_ROCKETLAKE)
>  #define IS_DG1(dev_priv)        IS_PLATFORM(dev_priv, INTEL_DG1)
> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> index 366ddfc8df6b..8690b69fcf33 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -846,6 +846,14 @@ static const struct intel_device_info ehl_info = {
>        .ppgtt_size = 36,
>  };

> +static const struct intel_device_info jsl_info = {
> +     GEN11_FEATURES,
> +     PLATFORM(INTEL_JASPERLAKE),
> +     .require_force_probe = 1,
> +     .platform_engine_mask = BIT(RCS0) | BIT(BCS0) | BIT(VCS0) | BIT(VECS0),
> +     .ppgtt_size = 36,
> +};
> +
>  #define GEN12_FEATURES \
>        GEN11_FEATURES, \
>        GEN(12), \
> @@ -985,6 +993,7 @@ static const struct pci_device_id pciidlist[] = {
>        INTEL_CNL_IDS(&cnl_info),
>        INTEL_ICL_11_IDS(&icl_info),
>        INTEL_EHL_IDS(&ehl_info),
> +     INTEL_JSL_IDS(&jsl_info),
>        INTEL_TGL_12_IDS(&tgl_info),
>        INTEL_RKL_IDS(&rkl_info),
>        {0, 0, 0}
> diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
> index adc836f15fde..e67cec8fa2aa 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.c
> +++ b/drivers/gpu/drm/i915/intel_device_info.c
> @@ -62,6 +62,7 @@ static const char * const platform_names[] = {
>        PLATFORM_NAME(CANNONLAKE),
>        PLATFORM_NAME(ICELAKE),
>        PLATFORM_NAME(ELKHARTLAKE),
> +     PLATFORM_NAME(JASPERLAKE),
>        PLATFORM_NAME(TIGERLAKE),
>        PLATFORM_NAME(ROCKETLAKE),
>        PLATFORM_NAME(DG1),
> diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
> index 6a3d607218aa..d92fa041c700 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.h
> +++ b/drivers/gpu/drm/i915/intel_device_info.h
> @@ -79,6 +79,7 @@ enum intel_platform {
>        /* gen11 */
>        INTEL_ICELAKE,
>        INTEL_ELKHARTLAKE,
> +     INTEL_JASPERLAKE,
>        /* gen12 */
>        INTEL_TIGERLAKE,
>        INTEL_ROCKETLAKE,
> diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h
> index 7eeecb07c9a1..1b5e09cfa11e 100644
> --- a/include/drm/i915_pciids.h
> +++ b/include/drm/i915_pciids.h
> @@ -579,15 +579,18 @@
>        INTEL_VGA_DEVICE(0x8A51, info), \
>        INTEL_VGA_DEVICE(0x8A5D, info)

> -/* EHL/JSL */
> +/* EHL */
>  #define INTEL_EHL_IDS(info) \
>        INTEL_VGA_DEVICE(0x4500, info), \
>        INTEL_VGA_DEVICE(0x4571, info), \
>        INTEL_VGA_DEVICE(0x4551, info), \
>        INTEL_VGA_DEVICE(0x4541, info), \
> -     INTEL_VGA_DEVICE(0x4E71, info), \
>        INTEL_VGA_DEVICE(0x4557, info), \
> -     INTEL_VGA_DEVICE(0x4555, info), \
> +     INTEL_VGA_DEVICE(0x4555, info)
> +
> +/* JSL */
> +#define INTEL_JSL_IDS(info) \
> +     INTEL_VGA_DEVICE(0x4E71, info), \
>        INTEL_VGA_DEVICE(0x4E61, info), \
>        INTEL_VGA_DEVICE(0x4E57, info), \
>        INTEL_VGA_DEVICE(0x4E55, info), \

--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux