| Message ID | 20200729102539.134731-2-ayaz.siddiqui@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/gt: Initialize reserved and unspecified MOCS indices | expand |
> -----Original Message----- > From: Siddiqui, Ayaz A <ayaz.siddiqui@intel.com> > Sent: Wednesday, July 29, 2020 3:56 PM > To: intel-gfx@lists.freedesktop.org > Cc: Siddiqui, Ayaz A <ayaz.siddiqui@intel.com>; Chris Wilson <chris@chris- > wilson.co.uk>; De Marchi, Lucas <lucas.demarchi@intel.com>; Lis, Tomasz > <tomasz.lis@intel.com>; Roper, Matthew D <matthew.d.roper@intel.com>; > Joonas Lahtinen <joonas.lahtinen@linux.intel.com>; Francisco Jerez > <currojerez@riseup.net>; Mathew, Alwin <alwin.mathew@intel.com>; Mcguire, > Russell W <russell.w.mcguire@intel.com>; Spruit, Neil R > <neil.r.spruit@intel.com>; Zhou, Cheng <cheng.zhou@intel.com>; Benemelis, > Mike G <mike.g.benemelis@intel.com> > Subject: [PATCH v4 1/1] drm/i915/gt: Initialize reserved and unspecified MOCS > indices > > In order to avoid functional breakage of mis-programmed applications that have > grown to depend on unused MOCS entries, we are programming those entries to > be equal to fully cached ("L3 + LLC") entry. > > These reserved and unspecified entries should not be used as they may be > changed to less performant variants with better coherency in the future if more > entries are needed. > > V2: As suggested by Lucas De Marchi to utilise __init_mocs_table for > programming default value, setting I915_MOCS_PTE index of tgl_mocs_table > with desired value. > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Lucas De Marchi <lucas.demarchi@intel.com> > Cc: Tomasz Lis <tomasz.lis@intel.com> > Cc: Matt Roper <matthew.d.roper@intel.com> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > Cc: Francisco Jerez <currojerez@riseup.net> > Cc: Mathew Alwin <alwin.mathew@intel.com> > Cc: Mcguire Russell W <russell.w.mcguire@intel.com> > Cc: Spruit Neil R <neil.r.spruit@intel.com> > Cc: Zhou Cheng <cheng.zhou@intel.com> > Cc: Benemelis Mike G <mike.g.benemelis@intel.com> > > Signed-off-by: Ayaz A Siddiqui <ayaz.siddiqui@intel.com> > Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com> > --- > drivers/gpu/drm/i915/gt/intel_mocs.c | 15 +++++++++++---- > 1 file changed, 11 insertions(+), 4 deletions(-) > I'm getting a false failure with this patch , which I tested locally and its passing with this patch. I think that this failure is blocking merge of this patch. Can Someone please let me know how to proceed in this case for merging? Regards -Ayaz > diff --git a/drivers/gpu/drm/i915/gt/intel_mocs.c > b/drivers/gpu/drm/i915/gt/intel_mocs.c > index 632e08a4592b..f5dde723f612 100644 > --- a/drivers/gpu/drm/i915/gt/intel_mocs.c > +++ b/drivers/gpu/drm/i915/gt/intel_mocs.c > @@ -234,11 +234,17 @@ static const struct drm_i915_mocs_entry > broxton_mocs_table[] = { > L3_1_UC) > > static const struct drm_i915_mocs_entry tgl_mocs_table[] = { > - /* Base - Error (Reserved for Non-Use) */ > - MOCS_ENTRY(0, 0x0, 0x0), > - /* Base - Reserved */ > - MOCS_ENTRY(1, 0x0, 0x0), > > + /* NOTE: > + * Reserved and unspecified MOCS indices have been set to (L3 + LCC). > + * These reserved entries should never be used, they may be changed > + * to low performant variants with better coherency in the future if > + * more entries are needed. We are programming index > I915_MOCS_PTE(1) > + * only, __init_mocs_table() take care to program unused index with > + * this entry. > + */ > + MOCS_ENTRY(1, LE_3_WB | LE_TC_1_LLC | LE_LRUM(3), > + L3_3_WB), > GEN11_MOCS_ENTRIES, > > /* Implicitly enable L1 - HDC:L1 + L3 + LLC */ @@ -265,6 +271,7 @@ > static const struct drm_i915_mocs_entry tgl_mocs_table[] = { > MOCS_ENTRY(61, > LE_1_UC | LE_TC_1_LLC, > L3_3_WB), > + > }; > > static const struct drm_i915_mocs_entry icl_mocs_table[] = { > -- > 2.26.2
Quoting Ayaz A Siddiqui (2020-07-29 13:25:39) > In order to avoid functional breakage of mis-programmed applications that > have grown to depend on unused MOCS entries, we are programming > those entries to be equal to fully cached ("L3 + LLC") entry. > > These reserved and unspecified entries should not be used as they may be > changed to less performant variants with better coherency in the future > if more entries are needed. > > V2: As suggested by Lucas De Marchi to utilise __init_mocs_table for > programming default value, setting I915_MOCS_PTE index of tgl_mocs_table > with desired value. > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Lucas De Marchi <lucas.demarchi@intel.com> > Cc: Tomasz Lis <tomasz.lis@intel.com> > Cc: Matt Roper <matthew.d.roper@intel.com> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > Cc: Francisco Jerez <currojerez@riseup.net> > Cc: Mathew Alwin <alwin.mathew@intel.com> > Cc: Mcguire Russell W <russell.w.mcguire@intel.com> > Cc: Spruit Neil R <neil.r.spruit@intel.com> > Cc: Zhou Cheng <cheng.zhou@intel.com> > Cc: Benemelis Mike G <mike.g.benemelis@intel.com> > > Signed-off-by: Ayaz A Siddiqui <ayaz.siddiqui@intel.com> > Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com> Acked-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Regards, Joonas > --- > drivers/gpu/drm/i915/gt/intel_mocs.c | 15 +++++++++++---- > 1 file changed, 11 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_mocs.c b/drivers/gpu/drm/i915/gt/intel_mocs.c > index 632e08a4592b..f5dde723f612 100644 > --- a/drivers/gpu/drm/i915/gt/intel_mocs.c > +++ b/drivers/gpu/drm/i915/gt/intel_mocs.c > @@ -234,11 +234,17 @@ static const struct drm_i915_mocs_entry broxton_mocs_table[] = { > L3_1_UC) > > static const struct drm_i915_mocs_entry tgl_mocs_table[] = { > - /* Base - Error (Reserved for Non-Use) */ > - MOCS_ENTRY(0, 0x0, 0x0), > - /* Base - Reserved */ > - MOCS_ENTRY(1, 0x0, 0x0), > > + /* NOTE: > + * Reserved and unspecified MOCS indices have been set to (L3 + LCC). > + * These reserved entries should never be used, they may be changed > + * to low performant variants with better coherency in the future if > + * more entries are needed. We are programming index I915_MOCS_PTE(1) > + * only, __init_mocs_table() take care to program unused index with > + * this entry. > + */ > + MOCS_ENTRY(1, LE_3_WB | LE_TC_1_LLC | LE_LRUM(3), > + L3_3_WB), > GEN11_MOCS_ENTRIES, > > /* Implicitly enable L1 - HDC:L1 + L3 + LLC */ > @@ -265,6 +271,7 @@ static const struct drm_i915_mocs_entry tgl_mocs_table[] = { > MOCS_ENTRY(61, > LE_1_UC | LE_TC_1_LLC, > L3_3_WB), > + > }; > > static const struct drm_i915_mocs_entry icl_mocs_table[] = { > -- > 2.26.2 >
On Fri, Jul 31, 2020 at 12:23:57AM -0700, Siddiqui, Ayaz A wrote: > > >> -----Original Message----- >> From: Siddiqui, Ayaz A <ayaz.siddiqui@intel.com> >> Sent: Wednesday, July 29, 2020 3:56 PM >> To: intel-gfx@lists.freedesktop.org >> Cc: Siddiqui, Ayaz A <ayaz.siddiqui@intel.com>; Chris Wilson <chris@chris- >> wilson.co.uk>; De Marchi, Lucas <lucas.demarchi@intel.com>; Lis, Tomasz >> <tomasz.lis@intel.com>; Roper, Matthew D <matthew.d.roper@intel.com>; >> Joonas Lahtinen <joonas.lahtinen@linux.intel.com>; Francisco Jerez >> <currojerez@riseup.net>; Mathew, Alwin <alwin.mathew@intel.com>; Mcguire, >> Russell W <russell.w.mcguire@intel.com>; Spruit, Neil R >> <neil.r.spruit@intel.com>; Zhou, Cheng <cheng.zhou@intel.com>; Benemelis, >> Mike G <mike.g.benemelis@intel.com> >> Subject: [PATCH v4 1/1] drm/i915/gt: Initialize reserved and unspecified MOCS >> indices >> >> In order to avoid functional breakage of mis-programmed applications that have >> grown to depend on unused MOCS entries, we are programming those entries to >> be equal to fully cached ("L3 + LLC") entry. >> >> These reserved and unspecified entries should not be used as they may be >> changed to less performant variants with better coherency in the future if more >> entries are needed. >> >> V2: As suggested by Lucas De Marchi to utilise __init_mocs_table for >> programming default value, setting I915_MOCS_PTE index of tgl_mocs_table >> with desired value. >> >> Cc: Chris Wilson <chris@chris-wilson.co.uk> >> Cc: Lucas De Marchi <lucas.demarchi@intel.com> >> Cc: Tomasz Lis <tomasz.lis@intel.com> >> Cc: Matt Roper <matthew.d.roper@intel.com> >> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> >> Cc: Francisco Jerez <currojerez@riseup.net> >> Cc: Mathew Alwin <alwin.mathew@intel.com> >> Cc: Mcguire Russell W <russell.w.mcguire@intel.com> >> Cc: Spruit Neil R <neil.r.spruit@intel.com> >> Cc: Zhou Cheng <cheng.zhou@intel.com> >> Cc: Benemelis Mike G <mike.g.benemelis@intel.com> >> >> Signed-off-by: Ayaz A Siddiqui <ayaz.siddiqui@intel.com> >> Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com> >> --- >> drivers/gpu/drm/i915/gt/intel_mocs.c | 15 +++++++++++---- >> 1 file changed, 11 insertions(+), 4 deletions(-) >> > >I'm getting a false failure with this patch , which I tested locally and its passing >with this patch. I think that this failure is blocking merge of this patch. >Can Someone please let me know how to proceed in this case for merging? If it's a false failure, you can go ahead and merge it. Better to reply to the CI email how is it unrelated. If you are not sure, you can re-trigger CI by going to the patchwork page and asking it to test revision 5 again: https://patchwork.freedesktop.org/series/78012/ Lucas De Marchi > >Regards >-Ayaz > >> diff --git a/drivers/gpu/drm/i915/gt/intel_mocs.c >> b/drivers/gpu/drm/i915/gt/intel_mocs.c >> index 632e08a4592b..f5dde723f612 100644 >> --- a/drivers/gpu/drm/i915/gt/intel_mocs.c >> +++ b/drivers/gpu/drm/i915/gt/intel_mocs.c >> @@ -234,11 +234,17 @@ static const struct drm_i915_mocs_entry >> broxton_mocs_table[] = { >> L3_1_UC) >> >> static const struct drm_i915_mocs_entry tgl_mocs_table[] = { >> - /* Base - Error (Reserved for Non-Use) */ >> - MOCS_ENTRY(0, 0x0, 0x0), >> - /* Base - Reserved */ >> - MOCS_ENTRY(1, 0x0, 0x0), >> >> + /* NOTE: >> + * Reserved and unspecified MOCS indices have been set to (L3 + LCC). >> + * These reserved entries should never be used, they may be changed >> + * to low performant variants with better coherency in the future if >> + * more entries are needed. We are programming index >> I915_MOCS_PTE(1) >> + * only, __init_mocs_table() take care to program unused index with >> + * this entry. >> + */ >> + MOCS_ENTRY(1, LE_3_WB | LE_TC_1_LLC | LE_LRUM(3), >> + L3_3_WB), >> GEN11_MOCS_ENTRIES, >> >> /* Implicitly enable L1 - HDC:L1 + L3 + LLC */ @@ -265,6 +271,7 @@ >> static const struct drm_i915_mocs_entry tgl_mocs_table[] = { >> MOCS_ENTRY(61, >> LE_1_UC | LE_TC_1_LLC, >> L3_3_WB), >> + >> }; >> >> static const struct drm_i915_mocs_entry icl_mocs_table[] = { >> -- >> 2.26.2 >
Quoting Lucas De Marchi (2020-10-12 17:55:03) > On Fri, Jul 31, 2020 at 12:23:57AM -0700, Siddiqui, Ayaz A wrote: > > > > > >> -----Original Message----- > >> From: Siddiqui, Ayaz A <ayaz.siddiqui@intel.com> > >> Sent: Wednesday, July 29, 2020 3:56 PM > >> To: intel-gfx@lists.freedesktop.org > >> Cc: Siddiqui, Ayaz A <ayaz.siddiqui@intel.com>; Chris Wilson <chris@chris- > >> wilson.co.uk>; De Marchi, Lucas <lucas.demarchi@intel.com>; Lis, Tomasz > >> <tomasz.lis@intel.com>; Roper, Matthew D <matthew.d.roper@intel.com>; > >> Joonas Lahtinen <joonas.lahtinen@linux.intel.com>; Francisco Jerez > >> <currojerez@riseup.net>; Mathew, Alwin <alwin.mathew@intel.com>; Mcguire, > >> Russell W <russell.w.mcguire@intel.com>; Spruit, Neil R > >> <neil.r.spruit@intel.com>; Zhou, Cheng <cheng.zhou@intel.com>; Benemelis, > >> Mike G <mike.g.benemelis@intel.com> > >> Subject: [PATCH v4 1/1] drm/i915/gt: Initialize reserved and unspecified MOCS > >> indices > >> > >> In order to avoid functional breakage of mis-programmed applications that have > >> grown to depend on unused MOCS entries, we are programming those entries to > >> be equal to fully cached ("L3 + LLC") entry. > >> > >> These reserved and unspecified entries should not be used as they may be > >> changed to less performant variants with better coherency in the future if more > >> entries are needed. > >> > >> V2: As suggested by Lucas De Marchi to utilise __init_mocs_table for > >> programming default value, setting I915_MOCS_PTE index of tgl_mocs_table > >> with desired value. > >> > >> Cc: Chris Wilson <chris@chris-wilson.co.uk> > >> Cc: Lucas De Marchi <lucas.demarchi@intel.com> > >> Cc: Tomasz Lis <tomasz.lis@intel.com> > >> Cc: Matt Roper <matthew.d.roper@intel.com> > >> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > >> Cc: Francisco Jerez <currojerez@riseup.net> > >> Cc: Mathew Alwin <alwin.mathew@intel.com> > >> Cc: Mcguire Russell W <russell.w.mcguire@intel.com> > >> Cc: Spruit Neil R <neil.r.spruit@intel.com> > >> Cc: Zhou Cheng <cheng.zhou@intel.com> > >> Cc: Benemelis Mike G <mike.g.benemelis@intel.com> > >> > >> Signed-off-by: Ayaz A Siddiqui <ayaz.siddiqui@intel.com> > >> Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com> > >> --- > >> drivers/gpu/drm/i915/gt/intel_mocs.c | 15 +++++++++++---- > >> 1 file changed, 11 insertions(+), 4 deletions(-) > >> > > > >I'm getting a false failure with this patch , which I tested locally and its passing > >with this patch. I think that this failure is blocking merge of this patch. > >Can Someone please let me know how to proceed in this case for merging? > > If it's a false failure, you can go ahead and merge it. Better to reply > to the CI email how is it unrelated. I would say the kms_frontbuffer_tracking is genuine fail as that is very cache sensitive. And I've just pushed this to upstream, oops, so now the problem is a little more urgent. -Chris
diff --git a/drivers/gpu/drm/i915/gt/intel_mocs.c b/drivers/gpu/drm/i915/gt/intel_mocs.c index 632e08a4592b..f5dde723f612 100644 --- a/drivers/gpu/drm/i915/gt/intel_mocs.c +++ b/drivers/gpu/drm/i915/gt/intel_mocs.c @@ -234,11 +234,17 @@ static const struct drm_i915_mocs_entry broxton_mocs_table[] = { L3_1_UC) static const struct drm_i915_mocs_entry tgl_mocs_table[] = { - /* Base - Error (Reserved for Non-Use) */ - MOCS_ENTRY(0, 0x0, 0x0), - /* Base - Reserved */ - MOCS_ENTRY(1, 0x0, 0x0), + /* NOTE: + * Reserved and unspecified MOCS indices have been set to (L3 + LCC). + * These reserved entries should never be used, they may be changed + * to low performant variants with better coherency in the future if + * more entries are needed. We are programming index I915_MOCS_PTE(1) + * only, __init_mocs_table() take care to program unused index with + * this entry. + */ + MOCS_ENTRY(1, LE_3_WB | LE_TC_1_LLC | LE_LRUM(3), + L3_3_WB), GEN11_MOCS_ENTRIES, /* Implicitly enable L1 - HDC:L1 + L3 + LLC */ @@ -265,6 +271,7 @@ static const struct drm_i915_mocs_entry tgl_mocs_table[] = { MOCS_ENTRY(61, LE_1_UC | LE_TC_1_LLC, L3_3_WB), + }; static const struct drm_i915_mocs_entry icl_mocs_table[] = {
In order to avoid functional breakage of mis-programmed applications that have grown to depend on unused MOCS entries, we are programming those entries to be equal to fully cached ("L3 + LLC") entry. These reserved and unspecified entries should not be used as they may be changed to less performant variants with better coherency in the future if more entries are needed. V2: As suggested by Lucas De Marchi to utilise __init_mocs_table for programming default value, setting I915_MOCS_PTE index of tgl_mocs_table with desired value. Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Lucas De Marchi <lucas.demarchi@intel.com> Cc: Tomasz Lis <tomasz.lis@intel.com> Cc: Matt Roper <matthew.d.roper@intel.com> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: Francisco Jerez <currojerez@riseup.net> Cc: Mathew Alwin <alwin.mathew@intel.com> Cc: Mcguire Russell W <russell.w.mcguire@intel.com> Cc: Spruit Neil R <neil.r.spruit@intel.com> Cc: Zhou Cheng <cheng.zhou@intel.com> Cc: Benemelis Mike G <mike.g.benemelis@intel.com> Signed-off-by: Ayaz A Siddiqui <ayaz.siddiqui@intel.com> Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com> --- drivers/gpu/drm/i915/gt/intel_mocs.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-)