drm/i915: Allow set context SSEU on platforms after gen 11
diff mbox series

Message ID 20190918173121.183132-1-stuart.summers@intel.com
State New
Headers show
Series
  • drm/i915: Allow set context SSEU on platforms after gen 11
Related show

Commit Message

Summers, Stuart Sept. 18, 2019, 5:31 p.m. UTC
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110559

Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Signed-off-by: Stuart Summers <stuart.summers@intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_context.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Summers, Stuart Sept. 18, 2019, 8:28 p.m. UTC | #1
On Wed, 2019-09-18 at 19:31 +0000, Patchwork wrote:
> == Series Details ==
> 
> Series: drm/i915: Allow set context SSEU on platforms after gen 11
> URL   : https://patchwork.freedesktop.org/series/66870/
> State : success
> 
> == Summary ==
> 
> CI Bug Log - changes from CI_DRM_6917 -> Patchwork_14447
> ====================================================
> 
> Summary
> -------
> 
>   **SUCCESS**
> 
>   No regressions found.
> 
>   External URL: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14447/
> 
> Known issues
> ------------
> 
>   Here are the changes found in Patchwork_14447 that come from known
> issues:
> 
> ### IGT changes ###
> 
> #### Issues hit ####
> 
>   * igt@debugfs_test@read_all_entries:
>     - fi-icl-u3:          [PASS][1] -> [DMESG-WARN][2] ([fdo#107724])
> +1 similar issue
>    [1]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6917/fi-icl-u3/igt@debugfs_test@read_all_entries.html
>    [2]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14447/fi-icl-u3/igt@debugfs_test@read_all_entries.html
> 
>   * igt@i915_module_load@reload:
>     - fi-icl-u3:          [PASS][3] -> [DMESG-WARN][4] ([fdo#107724]
> / [fdo#111214])
>    [3]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6917/fi-icl-u3/igt@i915_module_load@reload.html
>    [4]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14447/fi-icl-u3/igt@i915_module_load@reload.html
> 
>   * igt@i915_selftest@live_hangcheck:
>     - fi-icl-u3:          [PASS][5] -> [INCOMPLETE][6] ([fdo#107713]
> / [fdo#108569])
>    [5]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6917/fi-icl-u3/igt@i915_selftest@live_hangcheck.html
>    [6]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14447/fi-icl-u3/igt@i915_selftest@live_hangcheck.html
> 
>   * igt@kms_chamelium@dp-edid-read:
>     - fi-icl-u2:          [PASS][7] -> [FAIL][8] ([fdo#109483] /
> [fdo#109635 ])
>    [7]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6917/fi-icl-u2/igt@kms_chamelium@dp-edid-read.html
>    [8]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14447/fi-icl-u2/igt@kms_chamelium@dp-edid-read.html
> 
>   * igt@kms_frontbuffer_tracking@basic:
>     - fi-icl-u2:          [PASS][9] -> [FAIL][10] ([fdo#103167])
>    [9]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6917/fi-icl-u2/igt@kms_frontbuffer_tracking@basic.html
>    [10]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14447/fi-icl-u2/igt@kms_frontbuffer_tracking@basic.html
> 
>   
> #### Possible fixes ####
> 
>   * igt@gem_ctx_create@basic-files:
>     - {fi-tgl-u}:         [INCOMPLETE][11] ([fdo#111735]) ->
> [PASS][12]
>    [11]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6917/fi-tgl-u/igt@gem_ctx_create@basic-files.html
>    [12]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14447/fi-tgl-u/igt@gem_ctx_create@basic-files.html
> 
>   * igt@gem_exec_suspend@basic-s3:
>     - fi-blb-e6850:       [INCOMPLETE][13] ([fdo#107718]) ->
> [PASS][14]
>    [13]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6917/fi-blb-e6850/igt@gem_exec_suspend@basic-s3.html
>    [14]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14447/fi-blb-e6850/igt@gem_exec_suspend@basic-s3.html
> 
>   * igt@gem_mmap_gtt@basic-write-no-prefault:
>     - fi-icl-u3:          [DMESG-WARN][15] ([fdo#107724]) ->
> [PASS][16] +1 similar issue
>    [15]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6917/fi-icl-u3/igt@gem_mmap_gtt@basic-write-no-prefault.html
>    [16]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14447/fi-icl-u3/igt@gem_mmap_gtt@basic-write-no-prefault.html
> 
>   
>   {name}: This element is suppressed. This means it is ignored when
> computing
>           the status of the difference (SUCCESS, WARNING, or
> FAILURE).
> 
>   [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
>   [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
>   [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
>   [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
>   [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
>   [fdo#109483]: https://bugs.freedesktop.org/show_bug.cgi?id=109483
>   [fdo#109635 ]: https://bugs.freedesktop.org/show_bug.cgi?id=109635 
>   [fdo#111214]: https://bugs.freedesktop.org/show_bug.cgi?id=111214
>   [fdo#111735]: https://bugs.freedesktop.org/show_bug.cgi?id=111735
> 
> 
> Participating hosts (53 -> 47)
> ------------------------------
> 
>   Additional (1): fi-bxt-dsi 
>   Missing    (7): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan 
> fi-icl-y fi-byt-clapper fi-bdw-samus 

Hi Martin,

Is there an easy way to tell if these missing machines are a result of
changes made to the patches here vs something else in the test
infrastructure?

Thanks,
Stuart

> 
> 
> Build changes
> -------------
> 
>   * CI: CI-20190529 -> None
>   * Linux: CI_DRM_6917 -> Patchwork_14447
> 
>   CI-20190529: 20190529
>   CI_DRM_6917: 7ca2b123ae999133223b882e3190955897df8b03 @
> git://anongit.freedesktop.org/gfx-ci/linux
>   IGT_5191: 63e30122cadaf2798ae2bd44a56cee81a27fbc40 @
> git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
>   Patchwork_14447: 807439de93492cd02c3b104effc28780aea605b8 @
> git://anongit.freedesktop.org/gfx-ci/linux
> 
> 
> == Linux commits ==
> 
> 807439de9349 drm/i915: Allow set context SSEU on platforms after gen
> 11
> 
> == Logs ==
> 
> For more details see: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14447/index.html
Daniele Ceraolo Spurio Sept. 18, 2019, 8:39 p.m. UTC | #2
On 9/18/19 10:31 AM, Stuart Summers wrote:
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110559
> 

What's the planned usage here? TGL HW only supports slice-level 
power-gating and with only 1 slice on TGL we don't really have a choice 
of what to program, do we?

Daniele

> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> Signed-off-by: Stuart Summers <stuart.summers@intel.com>
> ---
>   drivers/gpu/drm/i915/gem/i915_gem_context.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> index f1c0e5d958f3..39af4c81b29a 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> @@ -1310,7 +1310,7 @@ static int set_sseu(struct i915_gem_context *ctx,
>   	if (args->size < sizeof(user_sseu))
>   		return -EINVAL;
>   
> -	if (!IS_GEN(i915, 11))
> +	if (INTEL_GEN(i915) < 11)
>   		return -ENODEV;
>   
>   	if (copy_from_user(&user_sseu, u64_to_user_ptr(args->value),
>
Tvrtko Ursulin Sept. 19, 2019, 7 a.m. UTC | #3
On 18/09/2019 18:31, Stuart Summers wrote:
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110559

Unless there was some discussion I missed we can't just turn it on to 
work around a SKIP in IGT. Feature was deliberately limited to Icelake 
and even there just to a sub-set of possible configurations.

Regards,

Tvrtko

> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> Signed-off-by: Stuart Summers <stuart.summers@intel.com>
> ---
>   drivers/gpu/drm/i915/gem/i915_gem_context.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> index f1c0e5d958f3..39af4c81b29a 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> @@ -1310,7 +1310,7 @@ static int set_sseu(struct i915_gem_context *ctx,
>   	if (args->size < sizeof(user_sseu))
>   		return -EINVAL;
>   
> -	if (!IS_GEN(i915, 11))
> +	if (INTEL_GEN(i915) < 11)
>   		return -ENODEV;
>   
>   	if (copy_from_user(&user_sseu, u64_to_user_ptr(args->value),
>
Martin Peres Sept. 19, 2019, 8:01 a.m. UTC | #4
On 18/09/2019 23:28, Summers, Stuart wrote:
> On Wed, 2019-09-18 at 19:31 +0000, Patchwork wrote:
>> == Series Details ==
>>
>> Series: drm/i915: Allow set context SSEU on platforms after gen 11
>> URL   : https://patchwork.freedesktop.org/series/66870/
>> State : success
>>
>> == Summary ==
>>
>> CI Bug Log - changes from CI_DRM_6917 -> Patchwork_14447
>> ====================================================
>>
>> Summary
>> -------
>>
>>   **SUCCESS**
>>
>>   No regressions found.
>>
>>   External URL: 
>> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14447/
>>
>> Known issues
>> ------------
>>
>>   Here are the changes found in Patchwork_14447 that come from known
>> issues:
>>
>> ### IGT changes ###
>>
>> #### Issues hit ####
>>
>>   * igt@debugfs_test@read_all_entries:
>>     - fi-icl-u3:          [PASS][1] -> [DMESG-WARN][2] ([fdo#107724])
>> +1 similar issue
>>    [1]: 
>> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6917/fi-icl-u3/igt@debugfs_test@read_all_entries.html
>>    [2]: 
>> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14447/fi-icl-u3/igt@debugfs_test@read_all_entries.html
>>
>>   * igt@i915_module_load@reload:
>>     - fi-icl-u3:          [PASS][3] -> [DMESG-WARN][4] ([fdo#107724]
>> / [fdo#111214])
>>    [3]: 
>> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6917/fi-icl-u3/igt@i915_module_load@reload.html
>>    [4]: 
>> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14447/fi-icl-u3/igt@i915_module_load@reload.html
>>
>>   * igt@i915_selftest@live_hangcheck:
>>     - fi-icl-u3:          [PASS][5] -> [INCOMPLETE][6] ([fdo#107713]
>> / [fdo#108569])
>>    [5]: 
>> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6917/fi-icl-u3/igt@i915_selftest@live_hangcheck.html
>>    [6]: 
>> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14447/fi-icl-u3/igt@i915_selftest@live_hangcheck.html
>>
>>   * igt@kms_chamelium@dp-edid-read:
>>     - fi-icl-u2:          [PASS][7] -> [FAIL][8] ([fdo#109483] /
>> [fdo#109635 ])
>>    [7]: 
>> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6917/fi-icl-u2/igt@kms_chamelium@dp-edid-read.html
>>    [8]: 
>> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14447/fi-icl-u2/igt@kms_chamelium@dp-edid-read.html
>>
>>   * igt@kms_frontbuffer_tracking@basic:
>>     - fi-icl-u2:          [PASS][9] -> [FAIL][10] ([fdo#103167])
>>    [9]: 
>> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6917/fi-icl-u2/igt@kms_frontbuffer_tracking@basic.html
>>    [10]: 
>> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14447/fi-icl-u2/igt@kms_frontbuffer_tracking@basic.html
>>
>>   
>> #### Possible fixes ####
>>
>>   * igt@gem_ctx_create@basic-files:
>>     - {fi-tgl-u}:         [INCOMPLETE][11] ([fdo#111735]) ->
>> [PASS][12]
>>    [11]: 
>> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6917/fi-tgl-u/igt@gem_ctx_create@basic-files.html
>>    [12]: 
>> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14447/fi-tgl-u/igt@gem_ctx_create@basic-files.html
>>
>>   * igt@gem_exec_suspend@basic-s3:
>>     - fi-blb-e6850:       [INCOMPLETE][13] ([fdo#107718]) ->
>> [PASS][14]
>>    [13]: 
>> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6917/fi-blb-e6850/igt@gem_exec_suspend@basic-s3.html
>>    [14]: 
>> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14447/fi-blb-e6850/igt@gem_exec_suspend@basic-s3.html
>>
>>   * igt@gem_mmap_gtt@basic-write-no-prefault:
>>     - fi-icl-u3:          [DMESG-WARN][15] ([fdo#107724]) ->
>> [PASS][16] +1 similar issue
>>    [15]: 
>> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6917/fi-icl-u3/igt@gem_mmap_gtt@basic-write-no-prefault.html
>>    [16]: 
>> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14447/fi-icl-u3/igt@gem_mmap_gtt@basic-write-no-prefault.html
>>
>>   
>>   {name}: This element is suppressed. This means it is ignored when
>> computing
>>           the status of the difference (SUCCESS, WARNING, or
>> FAILURE).
>>
>>   [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
>>   [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
>>   [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
>>   [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
>>   [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
>>   [fdo#109483]: https://bugs.freedesktop.org/show_bug.cgi?id=109483
>>   [fdo#109635 ]: https://bugs.freedesktop.org/show_bug.cgi?id=109635 
>>   [fdo#111214]: https://bugs.freedesktop.org/show_bug.cgi?id=111214
>>   [fdo#111735]: https://bugs.freedesktop.org/show_bug.cgi?id=111735
>>
>>
>> Participating hosts (53 -> 47)
>> ------------------------------
>>
>>   Additional (1): fi-bxt-dsi 
>>   Missing    (7): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan 
>> fi-icl-y fi-byt-clapper fi-bdw-samus 
> 
> Hi Martin,
> 
> Is there an easy way to tell if these missing machines are a result of
> changes made to the patches here vs something else in the test
> infrastructure?

Thanks for asking!

Sometimes, some machines fail to boot for reasons unrelated to the patch
series. We introduced the ci@boot test to be able to tell them apart,
but this is WIP (https://bugs.freedesktop.org/show_bug.cgi?id=111208)...

In the mean time, the best you can do is look at the result grid... Sorry!

Martin

> 
> Thanks,
> Stuart
> 
>>
>>
>> Build changes
>> -------------
>>
>>   * CI: CI-20190529 -> None
>>   * Linux: CI_DRM_6917 -> Patchwork_14447
>>
>>   CI-20190529: 20190529
>>   CI_DRM_6917: 7ca2b123ae999133223b882e3190955897df8b03 @
>> git://anongit.freedesktop.org/gfx-ci/linux
>>   IGT_5191: 63e30122cadaf2798ae2bd44a56cee81a27fbc40 @
>> git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
>>   Patchwork_14447: 807439de93492cd02c3b104effc28780aea605b8 @
>> git://anongit.freedesktop.org/gfx-ci/linux
>>
>>
>> == Linux commits ==
>>
>> 807439de9349 drm/i915: Allow set context SSEU on platforms after gen
>> 11
>>
>> == Logs ==
>>
>> For more details see: 
>> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14447/index.html
Summers, Stuart Sept. 20, 2019, 9:09 p.m. UTC | #5
On Thu, 2019-09-19 at 08:00 +0100, Tvrtko Ursulin wrote:
> On 18/09/2019 18:31, Stuart Summers wrote:
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110559
> 
> Unless there was some discussion I missed we can't just turn it on
> to 
> work around a SKIP in IGT. Feature was deliberately limited to
> Icelake 
> and even there just to a sub-set of possible configurations.

No conversation was missed, or at least none I was a part of. Is there
a reason we don't allow this on future platforms?

We do claim powergate support on TGL, so I assumed it would be good to
take this path on that platform. Maybe I'm misunderstanding something
here though.

Thanks,
Stuart

> 
> Regards,
> 
> Tvrtko
> 
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> > Signed-off-by: Stuart Summers <stuart.summers@intel.com>
> > ---
> >   drivers/gpu/drm/i915/gem/i915_gem_context.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > index f1c0e5d958f3..39af4c81b29a 100644
> > --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > @@ -1310,7 +1310,7 @@ static int set_sseu(struct i915_gem_context
> > *ctx,
> >   	if (args->size < sizeof(user_sseu))
> >   		return -EINVAL;
> >   
> > -	if (!IS_GEN(i915, 11))
> > +	if (INTEL_GEN(i915) < 11)
> >   		return -ENODEV;
> >   
> >   	if (copy_from_user(&user_sseu, u64_to_user_ptr(args->value),
> >
Summers, Stuart Sept. 20, 2019, 9:10 p.m. UTC | #6
On Wed, 2019-09-18 at 13:39 -0700, Daniele Ceraolo Spurio wrote:
> 
> On 9/18/19 10:31 AM, Stuart Summers wrote:
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110559
> > 
> 
> What's the planned usage here? TGL HW only supports slice-level 
> power-gating and with only 1 slice on TGL we don't really have a
> choice 
> of what to program, do we?

Well, we do claim powergate support on TGL, so I assumed we'd want to
enable this path. Maybe I'm missing something though. Had a similar
response to Tvrtko.

Thanks,
Stuart

> 
> Daniele
> 
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> > Signed-off-by: Stuart Summers <stuart.summers@intel.com>
> > ---
> >   drivers/gpu/drm/i915/gem/i915_gem_context.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > index f1c0e5d958f3..39af4c81b29a 100644
> > --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > @@ -1310,7 +1310,7 @@ static int set_sseu(struct i915_gem_context
> > *ctx,
> >   	if (args->size < sizeof(user_sseu))
> >   		return -EINVAL;
> >   
> > -	if (!IS_GEN(i915, 11))
> > +	if (INTEL_GEN(i915) < 11)
> >   		return -ENODEV;
> >   
> >   	if (copy_from_user(&user_sseu, u64_to_user_ptr(args->value),
> >
Chris Wilson Sept. 20, 2019, 9:29 p.m. UTC | #7
Quoting Summers, Stuart (2019-09-20 22:09:46)
> On Thu, 2019-09-19 at 08:00 +0100, Tvrtko Ursulin wrote:
> > On 18/09/2019 18:31, Stuart Summers wrote:
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110559
> > 
> > Unless there was some discussion I missed we can't just turn it on
> > to 
> > work around a SKIP in IGT. Feature was deliberately limited to
> > Icelake 
> > and even there just to a sub-set of possible configurations.
> 
> No conversation was missed, or at least none I was a part of. Is there
> a reason we don't allow this on future platforms?
> 
> We do claim powergate support on TGL, so I assumed it would be good to
> take this path on that platform. Maybe I'm misunderstanding something
> here though.

The current interface is purely to work around a silicon issue on icl.
It was not developed into a fully reconfigurable sseu interface mostly
due to a lack of demonstrable need and a lack of appreciation of the
tradeoffs between different system users (along with the claim that this
is all meant to be handled by instructions in the command stream...).
Another team did try to autoadjust sseu but also did not produce the
rationale nor attempt to integrate with the existing code.
-Chris
Summers, Stuart Sept. 20, 2019, 9:36 p.m. UTC | #8
On Fri, 2019-09-20 at 22:29 +0100, Chris Wilson wrote:
> Quoting Summers, Stuart (2019-09-20 22:09:46)
> > On Thu, 2019-09-19 at 08:00 +0100, Tvrtko Ursulin wrote:
> > > On 18/09/2019 18:31, Stuart Summers wrote:
> > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110559
> > > 
> > > Unless there was some discussion I missed we can't just turn it
> > > on
> > > to 
> > > work around a SKIP in IGT. Feature was deliberately limited to
> > > Icelake 
> > > and even there just to a sub-set of possible configurations.
> > 
> > No conversation was missed, or at least none I was a part of. Is
> > there
> > a reason we don't allow this on future platforms?
> > 
> > We do claim powergate support on TGL, so I assumed it would be good
> > to
> > take this path on that platform. Maybe I'm misunderstanding
> > something
> > here though.
> 
> The current interface is purely to work around a silicon issue on
> icl.
> It was not developed into a fully reconfigurable sseu interface
> mostly
> due to a lack of demonstrable need and a lack of appreciation of the
> tradeoffs between different system users (along with the claim that
> this
> is all meant to be handled by instructions in the command stream...).
> Another team did try to autoadjust sseu but also did not produce the
> rationale nor attempt to integrate with the existing code.

Ok that makes sense then and thanks for the explanations here. Please
consider my patch dropped :)

-Stuart

> -Chris
Tvrtko Ursulin Sept. 23, 2019, 10:34 a.m. UTC | #9
Replying with some more information for benefit of archives.

On 20/09/2019 22:29, Chris Wilson wrote:
> Quoting Summers, Stuart (2019-09-20 22:09:46)
>> On Thu, 2019-09-19 at 08:00 +0100, Tvrtko Ursulin wrote:
>>> On 18/09/2019 18:31, Stuart Summers wrote:
>>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110559
>>>
>>> Unless there was some discussion I missed we can't just turn it on
>>> to
>>> work around a SKIP in IGT. Feature was deliberately limited to
>>> Icelake
>>> and even there just to a sub-set of possible configurations.
>>
>> No conversation was missed, or at least none I was a part of. Is there
>> a reason we don't allow this on future platforms?
>>
>> We do claim powergate support on TGL, so I assumed it would be good to
>> take this path on that platform. Maybe I'm misunderstanding something
>> here though.
> 
> The current interface is purely to work around a silicon issue on icl.
> It was not developed into a fully reconfigurable sseu interface mostly
> due to a lack of demonstrable need and a lack of appreciation of the
> tradeoffs between different system users (along with the claim that this
> is all meant to be handled by instructions in the command stream...).

For the record and from my memory, the demonstrated need pre-gen11 was 
from the media transcoding side where it benefited performance for some 
workload types. What wasn't clear was the best strategy of striking a 
balance between increased cost of context switching (between contexts 
with different sseu configs) and performance benefit of going with 
reduced sseu. Again from memory, the best option looked to be a hybrid 
solution of not re-configuring SSEU on every context switch, but done 
periodically from an external entity, based on workload types input. As 
such it did not align completely with the per-context controls.

> Another team did try to autoadjust sseu but also did not produce the
> rationale nor attempt to integrate with the existing code.

I forgot about this one, wonder what happened there. This was also for 
pre-gen11 but with demonstrated performance-per-Watt benefit for some 3d 
workloads on Android. This was known as "Dynamic SSEU".

Regards,

Tvrtko

Patch
diff mbox series

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index f1c0e5d958f3..39af4c81b29a 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -1310,7 +1310,7 @@  static int set_sseu(struct i915_gem_context *ctx,
 	if (args->size < sizeof(user_sseu))
 		return -EINVAL;
 
-	if (!IS_GEN(i915, 11))
+	if (INTEL_GEN(i915) < 11)
 		return -ENODEV;
 
 	if (copy_from_user(&user_sseu, u64_to_user_ptr(args->value),