<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1"> <style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style> </head> <body dir="ltr"> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> </div> <div> <div id="appendonsend"></div> <div style="font-family:Calibri,Arial,Helvetica,sans-serif; font-size:12pt; color:rgb(0,0,0)"> </div> <hr tabindex="-1" style="display:inline-block; width:98%"> <div id="divRplyFwdMsg" dir="ltr">From: Jani Nikula <jani.nikula@linux.intel.com> Sent: Monday, September 28, 2020 7:07 PM To: Surendrakumar Upadhyay, TejaskumarX <tejaskumarx.surendrakumar.upadhyay@intel.com>; Vivi, Rodrigo <rodrigo.vivi@intel.com>; airlied@linux.ie <airlied@linux.ie>; daniel@ffwll.ch <daniel@ffwll.ch>; intel-gfx@lists.freedesktop.org <intel-gfx@lists.freedesktop.org>; dri-devel@lists.freedesktop.org <dri-devel@lists.freedesktop.org>; linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org>; Ausmus, James <james.ausmus@intel.com>; Roper, Matthew D <matthew.d.roper@intel.com>; Souza, Jose <jose.souza@intel.com>; ville.syrjala@linux.intel.com <ville.syrjala@linux.intel.com>; De Marchi, Lucas <lucas.demarchi@intel.com>; Pandey, Hariom <hariom.pandey@intel.com> Subject: Re: [PATCH 1/2] drm/i915/jsl: Split EHL/JSL platform info and PCI ids <div> </div> </div> <div class="BodyFragment"> <div class="PlainText">On Mon, 28 Sep 2020, Tejas Upadhyay <tejaskumarx.surendrakumar.upadhyay@intel.com> 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@intel.com> > --- > 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 <span style="color: rgb(0, 0, 0); font-family: "Segoe UI", "Segoe UI Web (West European)", "Segoe UI", -apple-system, BlinkMacSystemFont, Roboto, "Helvetica Neue", sans-serif; background-color: rgb(255, 255, 255); display: inline !important">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 </div> </div> </div> </body> </html>