RE: [Intel-gfx] [PATCH] drm/vgem: Use 256B aligned pitch
> -Original Message- > From: Daniel Vetter > Sent: 30 June 2021 17:52 > To: Surendrakumar Upadhyay, TejaskumarX > > Cc: dri-devel@lists.freedesktop.org; intel-...@lists.freedesktop.org; > ch...@chris-wilson.co.uk > Subject: Re: [Intel-gfx] [PATCH] drm/vgem: Use 256B aligned pitch > > On Wed, Jun 30, 2021 at 05:32:15PM +0530, Tejas Upadhyay wrote: > > Having different alignment requirement by different drivers, 256B > > aligned should work for all drm drivers. > > What. > > Like yes vgem abuses dumb_create, but it's not a kms driver. Pitch is > meaningless, and that's why we align it minimally to 1 byte (bpp = bits per > pixel here). > > Maybe start with explaining what you're trying to do here. > -Daniel > > Igt tool tests which are trying to exercise tests through VGEM are getting failure (if not 64B aligned) on Intel platforms in creating framebuffer as they need them to be 64B aligned. Then 64B alignment is not A requirement for all drm drivers. Thanks, Tejas > > Signed-off-by: Tejas Upadhyay > > > > --- > > drivers/gpu/drm/vgem/vgem_drv.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/vgem/vgem_drv.c > > b/drivers/gpu/drm/vgem/vgem_drv.c index bf38a7e319d1..1da6df5e256a > > 100644 > > --- a/drivers/gpu/drm/vgem/vgem_drv.c > > +++ b/drivers/gpu/drm/vgem/vgem_drv.c > > @@ -215,7 +215,7 @@ static int vgem_gem_dumb_create(struct drm_file > *file, struct drm_device *dev, > > struct drm_gem_object *gem_object; > > u64 pitch, size; > > > > - pitch = args->width * DIV_ROUND_UP(args->bpp, 8); > > + pitch = ALIGN(args->width * DIV_ROUND_UP(args->bpp, 8), 256); > > size = args->height * pitch; > > if (size == 0) > > return -EINVAL; > > -- > > 2.31.1 > > > > ___ > > Intel-gfx mailing list > > intel-...@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
RE: [PATCH v4] drm/i915/gen9bc: Handle TGP PCH during suspend/resume
> -Original Message-
> From: Lyude Paul
> Sent: 18 February 2021 02:49
> To: Deak, Imre
> Cc: intel-...@lists.freedesktop.org; Surendrakumar Upadhyay, TejaskumarX
> ; Roper, Matthew D
> ; Jani Nikula ;
> Joonas Lahtinen ; Vivi, Rodrigo
> ; David Airlie ; Daniel Vetter
> ; open list:DRM DRIVERS de...@lists.freedesktop.org>; open list
> Subject: Re: [PATCH v4] drm/i915/gen9bc: Handle TGP PCH during
> suspend/resume
>
> On Wed, 2021-02-17 at 23:18 +0200, Imre Deak wrote:
> > On Wed, Feb 17, 2021 at 01:00:16PM -0500, Lyude Paul wrote:
> > > From: Tejas Upadhyay
>
> > >
> > > For Legacy S3 suspend/resume GEN9 BC needs to enable and setup TGP
> > > PCH.
> > >
> > > v2:
> > > * Move Wa_14010685332 into it's own function - vsyrjala
> > > * Add TODO comment about figuring out if we can move this workaround
> > > - imre
> > > v3:
> > > * Rename cnp_irq_post_reset() to cnp_display_clock_wa()
> > > * Add TODO item mentioning we need to clarify which platforms this
> > > workaround applies to
> > > * Just use ibx_irq_reset() in gen8_irq_reset(). This code should be
> > > functionally equivalent on gen9 bc to the code v2 added
> > > * Drop icp_hpd_irq_setup() call in spt_hpd_irq_setup(), this looks
> > > to be
> > > more or less identical to spt_hpd_irq_setup() minus additionally
> > > enabling
> > > one port. Will update i915 to use icp_hpd_irq_setup() for ICP in a
> > > separate patch.
> > > v4:
> > > * Revert Wa_14010685332 system list in comments to how it was before
> > > * Add back HAS_PCH_SPLIT() check before calling ibx_irq_reset()
> > >
> > > Cc: Matt Roper
> > > Signed-off-by: Tejas Upadhyay
> > >
> > > Signed-off-by: Lyude Paul
> >
> > Thanks, looks ok to me:
> > Reviewed-by: Imre Deak
> >
> > nit: cnp_display_clock_gating_wa() would be an even better name, could
> > be renamed while applying.
>
> Sure thing. JFYI - I'm going to hold off on pushing this patch until I've got
> confirmation from the OEMs this is for that these patches still fix their
> issues
> (since I unfortunately don't have any access to this hardware).
I can follow up with OEM to test or I can get it tested in my LAB, as I have
RKL RVP (CML CPU) + TGP PCH with me.
>
> >
> > > ---
> > > drivers/gpu/drm/i915/i915_irq.c | 49
> > > +
> > > 1 file changed, 32 insertions(+), 17 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_irq.c
> > > b/drivers/gpu/drm/i915/i915_irq.c index 98145a7f28a4..9b56a8f81e1a
> > > 100644
> > > --- a/drivers/gpu/drm/i915/i915_irq.c
> > > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > > @@ -3040,6 +3040,24 @@ static void valleyview_irq_reset(struct
> > > drm_i915_private *dev_priv)
> > > spin_unlock_irq(_priv->irq_lock);
> > > }
> > >
> > > +static void cnp_display_clock_wa(struct drm_i915_private *dev_priv)
> > > +{
> > > + struct intel_uncore *uncore = _priv->uncore;
> > > +
> > > + /*
> > > + * Wa_14010685332:cnp/cmp,tgp,adp
> > > + * TODO: Clarify which platforms this applies to
> > > + * TODO: Figure out if this workaround can be applied in the
> > > +s0ix
> > > suspend/resume handlers as
> > > + * on earlier platforms and whether the workaround is also
> > > +needed
> > > for runtime suspend/resume
> > > + */
> > > + if (INTEL_PCH_TYPE(dev_priv) == PCH_CNP ||
> > > + (INTEL_PCH_TYPE(dev_priv) >= PCH_TGP &&
> > > +INTEL_PCH_TYPE(dev_priv)
> > > < PCH_DG1)) {
> > > + intel_uncore_rmw(uncore, SOUTH_CHICKEN1,
> > > SBCLK_RUN_REFCLK_DIS,
> > > + SBCLK_RUN_REFCLK_DIS);
> > > + intel_uncore_rmw(uncore, SOUTH_CHICKEN1,
> > > SBCLK_RUN_REFCLK_DIS, 0);
> > > + }
> > > +}
> > > +
> > > static void gen8_irq_reset(struct drm_i915_private *dev_priv)
> > > {
> > > struct intel_uncore *uncore = _priv->uncore; @@ -3063,6
> > > +3081,8 @@ static void gen8_irq_reset(struct drm_i915_private
> > > *dev_priv)
> > >
> > > if (HAS_PCH_SPLIT(dev_priv))
> > > ibx_irq_reset(dev_priv);
> > > +
> > > + cnp_display_clock_wa(dev_priv);
> >
RE: [PATCH v2] drm/i915/edp/jsl: Update vswing table for HBR and HBR2
-Original Message-
From: Ville Syrjälä
Sent: 29 September 2020 18:23
To: Surendrakumar Upadhyay, TejaskumarX
Cc: intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Ausmus,
James ; Roper, Matthew D ;
Souza, Jose ; De Marchi, Lucas
; Pandey, Hariom
Subject: Re: [PATCH v2] drm/i915/edp/jsl: Update vswing table for HBR and HBR2
On Tue, Sep 29, 2020 at 05:41:27PM +0530, Tejas Upadhyay wrote:
> JSL has update in vswing table for eDP
>
> BSpec: 21257
>
> Changes since V1 :
> - IS_ELKHARTLAKE and IS_JASPERLAKE is replaced with
> HAS_PCH_MCC(EHL) and HAS_PCH_JSP(JSL) respectively
What do vswing values have to do with the PCH type?
Tejas : There is difference in voltage swing values for EHL and JSL. Till now
we were not differentiating between EHL and JSL as both were
based on ICL. Now as per compliance test we have found change in JSL eDP vswing
values which does not apply to EHL which leads to differentiate
between EHL and JSL. Thus differentiator between JSL and EHL is PCH i.e
HAS_PCH_MCC(EHL) and HAS_PCH_JSP(JSL). There is no direct relation of PCH with
vswing.
> - Reverted EHL/JSL PCI ids split change
>
> Signed-off-by: Tejas Upadhyay
>
> ---
> drivers/gpu/drm/i915/display/intel_ddi.c | 67
> ++--
> 1 file changed, 64 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 4d06178cd76c..e6e93d01d0ce 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -582,6 +582,34 @@ static const struct cnl_ddi_buf_trans
> ehl_combo_phy_ddi_translations_dp[] = {
> { 0x6, 0x7F, 0x3F, 0x00, 0x00 },/* 900 900 0.0 */
> };
>
> +static const struct cnl_ddi_buf_trans
> jsl_combo_phy_ddi_translations_edp_hbr[] = {
> + /* NT mV Trans mV db*/
> + { 0x8, 0x7F, 0x3F, 0x00, 0x00 },/* 200 200 0.0 */
> + { 0x8, 0x7F, 0x38, 0x00, 0x07 },/* 200 250 1.9 */
> + { 0x1, 0x7F, 0x33, 0x00, 0x0C },/* 200 300 3.5 */
> + { 0xA, 0x35, 0x36, 0x00, 0x09 },/* 200 350 4.9 */
> + { 0x8, 0x7F, 0x3F, 0x00, 0x00 },/* 250 250 0.0 */
> + { 0x1, 0x7F, 0x38, 0x00, 0x07 },/* 250 300 1.6 */
> + { 0xA, 0x35, 0x35, 0x00, 0x0A },/* 250 350 2.9 */
> + { 0x1, 0x7F, 0x3F, 0x00, 0x00 },/* 300 300 0.0 */
> + { 0xA, 0x35, 0x38, 0x00, 0x07 },/* 300 350 1.3 */
> + { 0xA, 0x35, 0x3F, 0x00, 0x00 },/* 350 350 0.0 */
> +};
> +
> +static const struct cnl_ddi_buf_trans
> jsl_combo_phy_ddi_translations_edp_hbr2[] = {
> + /* NT mV Trans mV db*/
> + { 0x8, 0x7F, 0x3F, 0x00, 0x00 },/* 200 200 0.0 */
> + { 0x8, 0x7F, 0x3F, 0x00, 0x00 },/* 200 250 1.9 */
> + { 0x1, 0x7F, 0x3D, 0x00, 0x02 },/* 200 300 3.5 */
> + { 0xA, 0x35, 0x38, 0x00, 0x07 },/* 200 350 4.9 */
> + { 0x8, 0x7F, 0x3F, 0x00, 0x00 },/* 250 250 0.0 */
> + { 0x1, 0x7F, 0x3F, 0x00, 0x00 },/* 250 300 1.6 */
> + { 0xA, 0x35, 0x3A, 0x00, 0x05 },/* 250 350 2.9 */
> + { 0x1, 0x7F, 0x3F, 0x00, 0x00 },/* 300 300 0.0 */
> + { 0xA, 0x35, 0x38, 0x00, 0x07 },/* 300 350 1.3 */
> + { 0xA, 0x35, 0x3F, 0x00, 0x00 },/* 350 350 0.0 */
> +};
> +
> struct icl_mg_phy_ddi_buf_trans {
> u32 cri_txdeemph_override_11_6;
> u32 cri_txdeemph_override_5_0;
> @@ -1069,7 +1097,6 @@ icl_get_mg_buf_trans(struct intel_encoder *encoder, int
> type, int rate,
> *n_entries = ARRAY_SIZE(icl_mg_phy_ddi_translations_rbr_hbr);
> return icl_mg_phy_ddi_translations_rbr_hbr;
> }
> -
> static const struct cnl_ddi_buf_trans *
> ehl_get_combo_buf_trans(struct intel_encoder *encoder, int type, int rate,
> int *n_entries)
> @@ -1098,6 +1125,34 @@ ehl_get_combo_buf_trans(struct intel_encoder *encoder,
> int type, int rate,
> }
> }
>
> +static const struct cnl_ddi_buf_trans *
> +jsl_get_combo_buf_trans(struct intel_encoder *encoder, int type, int rate,
> + int *n_entries)
> +{
> + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> +
> + switch (type) {
> + case INTEL_OUTPUT_HDMI:
> + *n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_hdmi);
> + return icl_combo_phy_ddi_translations_hdmi;
> + case
Re: [PATCH 1/2] drm/i915/jsl: Split EHL/JSL platform info and PCI ids
From: Matt Roper Sent: Monday, September 28, 2020 10:54 PM To: Jani Nikula Cc: Surendrakumar Upadhyay, TejaskumarX ; Vivi, Rodrigo ; airl...@linux.ie ; dan...@ffwll.ch ; intel-...@lists.freedesktop.org ; dri-devel@lists.freedesktop.org ; linux-ker...@vger.kernel.org ; Ausmus, James ; Souza, Jose ; ville.syrj...@linux.intel.com ; De Marchi, Lucas ; Pandey, Hariom Subject: Re: [PATCH 1/2] drm/i915/jsl: Split EHL/JSL platform info and PCI ids On Mon, Sep 28, 2020 at 08:14:02PM +0300, Jani Nikula wrote: > On Mon, 28 Sep 2020, "Surendrakumar Upadhyay, TejaskumarX" > wrote: > > > > From: Jani Nikula > > Sent: Monday, September 28, 2020 7:07 PM > > To: Surendrakumar Upadhyay, TejaskumarX > > ; Vivi, Rodrigo > > ; airl...@linux.ie ; > > dan...@ffwll.ch ; intel-...@lists.freedesktop.org > > ; dri-devel@lists.freedesktop.org > > ; linux-ker...@vger.kernel.org > > ; Ausmus, James ; > > Roper, Matthew D ; Souza, Jose > > ; ville.syrj...@linux.intel.com > > ; De Marchi, Lucas > > ; Pandey, Hariom > > 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 > > wrote: > >> Split the basic platform definition, macros, and PCI IDs to > >> differentiate between EHL and JSL platforms. > >> > >> Signed-off-by: Tejas Upadhyay > >> > >> --- > >> 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. Why are we adding IS_JASPERLAKE at all? EHL/JSL are documented as the same graphics IP, but are paired with different PCHs in the final SoCs, which is what causes the minor differences in programming. My understanding is that the voltage programming differences are ultimately due to that difference in PCH so we should just use HAS_PCH_MCC (EHL) and HAS_PCH_JSP (JSL) to distinguish which type of programming is needed rather than using a platform test. Matt Thanks for pointing out this Matt. I can change accordingly and send V2. Tejas > > > BR, > Jani. > > > &g
Re: [Intel-gfx] [PATCH 2/2] drm/i915/edp/jsl: Update vswing table for HBR and HBR2
From: Jani Nikula
Sent: Monday, September 28, 2020 7:13 PM
To: Surendrakumar Upadhyay, TejaskumarX
; Vivi, Rodrigo
; airl...@linux.ie ; dan...@ffwll.ch
; intel-...@lists.freedesktop.org
; dri-devel@lists.freedesktop.org
; linux-ker...@vger.kernel.org
; Ausmus, James ; Roper,
Matthew D ; Souza, Jose ;
ville.syrj...@linux.intel.com ; De Marchi, Lucas
; Pandey, Hariom
Subject: Re: [Intel-gfx] [PATCH 2/2] drm/i915/edp/jsl: Update vswing table for
HBR and HBR2
On Mon, 28 Sep 2020, Tejas Upadhyay
wrote:
> JSL has update in vswing table for eDP
I've thought the TLA for Jasper Lake is JSP, not JSL. At least we have
PCH_JSP for Jasper Lake PCH.
>
> BSpec: 21257
> Signed-off-by: Tejas Upadhyay
> ---
> drivers/gpu/drm/i915/display/intel_ddi.c | 67 ++--
> 1 file changed, 64 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 4d06178cd76c..fa08463bcf2e 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -582,6 +582,34 @@ static const struct cnl_ddi_buf_trans
> ehl_combo_phy_ddi_translations_dp[] = {
>{ 0x6, 0x7F, 0x3F, 0x00, 0x00 },/* 900 900 0.0 */
> };
>
> +static const struct cnl_ddi_buf_trans
> jsl_combo_phy_ddi_translations_edp_hbr[] = {
> + /* NT mV Trans mV db*/
> + { 0x8, 0x7F, 0x3F, 0x00, 0x00 },/* 200 200 0.0 */
> + { 0x8, 0x7F, 0x38, 0x00, 0x07 },/* 200 250 1.9 */
> + { 0x1, 0x7F, 0x33, 0x00, 0x0C },/* 200 300 3.5 */
> + { 0xA, 0x35, 0x36, 0x00, 0x09 },/* 200 350 4.9 */
> + { 0x8, 0x7F, 0x3F, 0x00, 0x00 },/* 250 250 0.0 */
> + { 0x1, 0x7F, 0x38, 0x00, 0x07 },/* 250 300 1.6 */
> + { 0xA, 0x35, 0x35, 0x00, 0x0A },/* 250 350 2.9 */
> + { 0x1, 0x7F, 0x3F, 0x00, 0x00 },/* 300 300 0.0 */
> + { 0xA, 0x35, 0x38, 0x00, 0x07 },/* 300 350 1.3 */
> + { 0xA, 0x35, 0x3F, 0x00, 0x00 },/* 350 350 0.0 */
> +};
> +
> +static const struct cnl_ddi_buf_trans
> jsl_combo_phy_ddi_translations_edp_hbr2[] = {
> + /* NT mV Trans mV db*/
> + { 0x8, 0x7F, 0x3F, 0x00, 0x00 },/* 200 200 0.0 */
> + { 0x8, 0x7F, 0x3F, 0x00, 0x00 },/* 200 250 1.9 */
> + { 0x1, 0x7F, 0x3D, 0x00, 0x02 },/* 200 300 3.5 */
> + { 0xA, 0x35, 0x38, 0x00, 0x07 },/* 200 350 4.9 */
> + { 0x8, 0x7F, 0x3F, 0x00, 0x00 },/* 250 250 0.0 */
> + { 0x1, 0x7F, 0x3F, 0x00, 0x00 },/* 250 300 1.6 */
> + { 0xA, 0x35, 0x3A, 0x00, 0x05 },/* 250 350 2.9 */
> + { 0x1, 0x7F, 0x3F, 0x00, 0x00 },/* 300 300 0.0 */
> + { 0xA, 0x35, 0x38, 0x00, 0x07 },/* 300 350 1.3 */
> + { 0xA, 0x35, 0x3F, 0x00, 0x00 },/* 350 350 0.0 */
> +};
> +
> struct icl_mg_phy_ddi_buf_trans {
>u32 cri_txdeemph_override_11_6;
>u32 cri_txdeemph_override_5_0;
> @@ -1069,7 +1097,6 @@ icl_get_mg_buf_trans(struct intel_encoder *encoder, int
> type, int rate,
>*n_entries = ARRAY_SIZE(icl_mg_phy_ddi_translations_rbr_hbr);
>return icl_mg_phy_ddi_translations_rbr_hbr;
> }
> -
> static const struct cnl_ddi_buf_trans *
> ehl_get_combo_buf_trans(struct intel_encoder *encoder, int type, int rate,
>int *n_entries)
> @@ -1098,6 +1125,34 @@ ehl_get_combo_buf_trans(struct intel_encoder *encoder,
> int type, int rate,
>}
> }
>
> +static const struct cnl_ddi_buf_trans *
> +jsl_get_combo_buf_trans(struct intel_encoder *encoder, int type, int rate,
> + int *n_entries)
> +{
> + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> +
> + switch (type) {
> + case INTEL_OUTPUT_HDMI:
> + *n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_hdmi);
> + return icl_combo_phy_ddi_translations_hdmi;
> + case INTEL_OUTPUT_EDP:
> + if (dev_priv->vbt.edp.low_vswing) {
> + if (rate > 27) {
> + *n_entries =
> ARRAY_SIZE(jsl_combo_phy_ddi_translations_edp_hbr2);
> + return jsl_combo_phy_ddi_translations_edp_hbr2;
> + } else {
> + *n_entries =
> ARRAY_SIZE(jsl_combo_phy_ddi_translations_edp_hbr);
> +
Re: [PATCH 1/2] drm/i915/jsl: Split EHL/JSL platform info and PCI ids
From: Jani Nikula
Sent: Monday, September 28, 2020 7:07 PM
To: Surendrakumar Upadhyay, TejaskumarX
; Vivi, Rodrigo
; airl...@linux.ie ; dan...@ffwll.ch
; intel-...@lists.freedesktop.org
; dri-devel@lists.freedesktop.org
; linux-ker...@vger.kernel.org
; Ausmus, James ; Roper,
Matthew D ; Souza, Jose ;
ville.syrj...@linux.intel.com ; De Marchi, Lucas
; Pandey, Hariom
Subject: Re: [PATCH 1/2] drm/i915/jsl: Split EHL/JSL platform info and PCI ids
On Mon, 28 Sep 2020, Tejas Upadhyay
wrote:
> Split the basic platform definition, macros, and PCI IDs to
> differentiate between EHL and JSL platforms.
>
> Signed-off-by: Tejas Upadhyay
> ---
> 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(_info),
>INTEL_ICL_11_IDS(_info),
>INTEL_EHL_IDS(_info),
> + INTEL_JSL_IDS(_info),
>INTEL_TGL_12_IDS(_info),
>INTEL_RKL_IDS(_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
