On Mon, 28 Sep 2020, "Surendrakumar Upadhyay, TejaskumarX" <tejaskumarx.surendrakumar.upadhyay@xxxxxxxxx> wrote:
> ________________________________
> 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
Please fix your email quoting when interacting on the public lists.
>
> 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?
If you need to differentiate between the two platforms, IS_ELKHARTLAKE()
must mean only ELK and IS_JASPERLAKE() must mean only JSP.
It's non-negotiable. We've made the mistake before, we're not doing it
again.
There are 32 references to IS_ELKHARTLAKE(). It's slightly painful, but
the alternative is worse.
BR,
Jani.
>
>> #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
--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
