| Message ID | 20210723174239.1551352-2-matthew.d.roper@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Begin enabling Xe_HP SDV and DG2 platforms | expand |
On Fri, 2021-07-23 at 10:42 -0700, Matt Roper wrote: > During a rebase the parameters were partially renamed, but not > completely. Since the subsequent patches that start using this macro > haven't landed on an upstream tree yet this didn't cause a build > failure. > > Fixes: 086df54e20be ("drm/i915/xehpsdv: add initial XeHP SDV > definitions") > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > --- > drivers/gpu/drm/i915/i915_drv.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > b/drivers/gpu/drm/i915/i915_drv.h > index d118834a4ed9..d44d0050beec 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1562,8 +1562,8 @@ IS_SUBPLATFORM(const struct drm_i915_private > *i915, > (IS_ALDERLAKE_P(__i915) && \ > IS_GT_STEP(__i915, since, until)) > > -#define IS_XEHPSDV_GT_STEP(p, since, until) \ > - (IS_XEHPSDV(p) && IS_GT_STEP(__i915, since, until)) > +#define IS_XEHPSDV_GT_STEP(__i915, since, until) \ > + (IS_XEHPSDV(__i915) && IS_GT_STEP(__i915, since, until)) Is your comment saying that the first parameter of IS_XEHPSDV_GT_STEP(), p or __i915 must be the first parameter of both IS_XEHPSDV() and IS_GT_STEP()? The older code is a bug, correct? -caz > > /* > * DG2 hardware steppings are a bit unusual. The hardware design > was forked
On Tue, Jul 27, 2021 at 11:34:28AM -0700, Yokoyama, Caz wrote: > On Fri, 2021-07-23 at 10:42 -0700, Matt Roper wrote: > > During a rebase the parameters were partially renamed, but not > > completely. Since the subsequent patches that start using this macro > > haven't landed on an upstream tree yet this didn't cause a build > > failure. > > > > Fixes: 086df54e20be ("drm/i915/xehpsdv: add initial XeHP SDV > > definitions") > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > > --- > > drivers/gpu/drm/i915/i915_drv.h | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > > b/drivers/gpu/drm/i915/i915_drv.h > > index d118834a4ed9..d44d0050beec 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -1562,8 +1562,8 @@ IS_SUBPLATFORM(const struct drm_i915_private > > *i915, > > (IS_ALDERLAKE_P(__i915) && \ > > IS_GT_STEP(__i915, since, until)) > > > > -#define IS_XEHPSDV_GT_STEP(p, since, until) \ > > - (IS_XEHPSDV(p) && IS_GT_STEP(__i915, since, until)) > > +#define IS_XEHPSDV_GT_STEP(__i915, since, until) \ > > + (IS_XEHPSDV(__i915) && IS_GT_STEP(__i915, since, until)) > Is your comment saying that the first parameter > of IS_XEHPSDV_GT_STEP(), p or __i915 must be the first parameter of > both IS_XEHPSDV() and IS_GT_STEP()? The older code is a bug, correct? > -caz We can name the parameter anything we want, it just has to be used consistently throughout the macro. Defining the parameter as 'p' but then passing a different undefined name '__i915' into IS_GT_STEP won't work (but it will only start causing compile errors when we land workarounds and such that start using the macro). Matt > > > > > /* > > * DG2 hardware steppings are a bit unusual. The hardware design > > was forked
On Tue, 2021-07-27 at 11:38 -0700, Matt Roper wrote: > On Tue, Jul 27, 2021 at 11:34:28AM -0700, Yokoyama, Caz wrote: > > On Fri, 2021-07-23 at 10:42 -0700, Matt Roper wrote: > > > During a rebase the parameters were partially renamed, but not > > > completely. Since the subsequent patches that start using this > > > macro > > > haven't landed on an upstream tree yet this didn't cause a build > > > failure. > > > > > > Fixes: 086df54e20be ("drm/i915/xehpsdv: add initial XeHP SDV > > > definitions") > > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > > > --- > > > drivers/gpu/drm/i915/i915_drv.h | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > > > b/drivers/gpu/drm/i915/i915_drv.h > > > index d118834a4ed9..d44d0050beec 100644 > > > --- a/drivers/gpu/drm/i915/i915_drv.h > > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > > @@ -1562,8 +1562,8 @@ IS_SUBPLATFORM(const struct > > > drm_i915_private > > > *i915, > > > (IS_ALDERLAKE_P(__i915) && \ > > > IS_GT_STEP(__i915, since, until)) > > > > > > -#define IS_XEHPSDV_GT_STEP(p, since, until) \ > > > - (IS_XEHPSDV(p) && IS_GT_STEP(__i915, since, until)) > > > +#define IS_XEHPSDV_GT_STEP(__i915, since, until) \ > > > + (IS_XEHPSDV(__i915) && IS_GT_STEP(__i915, since, until)) > > Is your comment saying that the first parameter > > of IS_XEHPSDV_GT_STEP(), p or __i915 must be the first parameter of > > both IS_XEHPSDV() and IS_GT_STEP()? The older code is a bug, > > correct? > > -caz > > We can name the parameter anything we want, it just has to be used > consistently throughout the macro. Defining the parameter as 'p' but > then passing a different undefined name '__i915' into IS_GT_STEP > won't > work (but it will only start causing compile errors when we land > workarounds and such that start using the macro). Reviewed-by: Caz Yokoyama <caz.yokoyama@intel.com> -caz > > > Matt > > > > /* > > > * DG2 hardware steppings are a bit unusual. The hardware > > > design > > > was forked
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index d118834a4ed9..d44d0050beec 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1562,8 +1562,8 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915, (IS_ALDERLAKE_P(__i915) && \ IS_GT_STEP(__i915, since, until)) -#define IS_XEHPSDV_GT_STEP(p, since, until) \ - (IS_XEHPSDV(p) && IS_GT_STEP(__i915, since, until)) +#define IS_XEHPSDV_GT_STEP(__i915, since, until) \ + (IS_XEHPSDV(__i915) && IS_GT_STEP(__i915, since, until)) /* * DG2 hardware steppings are a bit unusual. The hardware design was forked
During a rebase the parameters were partially renamed, but not completely. Since the subsequent patches that start using this macro haven't landed on an upstream tree yet this didn't cause a build failure. Fixes: 086df54e20be ("drm/i915/xehpsdv: add initial XeHP SDV definitions") Signed-off-by: Matt Roper <matthew.d.roper@intel.com> --- drivers/gpu/drm/i915/i915_drv.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)