[PATCH] drm/i915: disable set/get_tiling ioctl on gen12+

[PATCH] drm/i915: disable set/get_tiling ioctl on gen12+

[Daniel Vetter]

The cpu (de)tiler hw is gone, this stopped being useful. Plus it never
supported any of the fancy new tiling formats, which means userspace
also stopped using the magic side-channel this provides.

This would totally break a lot of the igts, but they're already broken
for the same reasons as userspace on gen12 would be.

Cc: Kenneth Graunke <kenneth@whitecape.org>
Cc: Jason Ekstrand <jason@jlekstrand.net>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_tiling.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_tiling.c b/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
index ca0c2f451742..2ee96f27a294 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
@@ -313,10 +313,15 @@ int
 i915_gem_set_tiling_ioctl(struct drm_device *dev, void *data,
 			  struct drm_file *file)
 {
+	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct drm_i915_gem_set_tiling *args = data;
 	struct drm_i915_gem_object *obj;
 	int err;
 
+	/* there is no cpu (de)tiling in the hw anymore on gen12+ */
+	if (INTEL_GEN(dev_priv) >= 12)
+		return -EOPNOTSUPP;
+
 	obj = i915_gem_object_lookup(file, args->handle);
 	if (!obj)
 		return -ENOENT;
@@ -402,6 +407,9 @@ i915_gem_get_tiling_ioctl(struct drm_device *dev, void *data,
 	struct drm_i915_gem_object *obj;
 	int err = -ENOENT;
 
+	if (INTEL_GEN(dev_priv) >= 12)
+		return -EOPNOTSUPP;
+
 	rcu_read_lock();
 	obj = i915_gem_object_lookup_rcu(file, args->handle);
 	if (obj) {
-- 
2.23.0.rc1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: disable set/get_tiling ioctl on gen12+

[Chris Wilson]

Quoting Daniel Vetter (2019-08-20 18:06:31)
> The cpu (de)tiler hw is gone, this stopped being useful. Plus it never
> supported any of the fancy new tiling formats, which means userspace
> also stopped using the magic side-channel this provides.
> 
> This would totally break a lot of the igts, but they're already broken
> for the same reasons as userspace on gen12 would be.
> 
> Cc: Kenneth Graunke <kenneth@whitecape.org>
> Cc: Jason Ekstrand <jason@jlekstrand.net>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/i915/gem/i915_gem_tiling.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_tiling.c b/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
> index ca0c2f451742..2ee96f27a294 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
> @@ -313,10 +313,15 @@ int
>  i915_gem_set_tiling_ioctl(struct drm_device *dev, void *data,
>                           struct drm_file *file)
>  {
> +       struct drm_i915_private *dev_priv = to_i915(dev);
>         struct drm_i915_gem_set_tiling *args = data;
>         struct drm_i915_gem_object *obj;
>         int err;
>  
> +       /* there is no cpu (de)tiling in the hw anymore on gen12+ */
> +       if (INTEL_GEN(dev_priv) >= 12)

This should be if (!NUM_FENCES(dev_priv)) if anything. You wouldn't even
need the comment talking about random features, and instead focus on why
not supporting the back channel is worth the maintenance cost of
divergence.

> +               return -EOPNOTSUPP;
> +
>         obj = i915_gem_object_lookup(file, args->handle);
>         if (!obj)
>                 return -ENOENT;
> @@ -402,6 +407,9 @@ i915_gem_get_tiling_ioctl(struct drm_device *dev, void *data,
>         struct drm_i915_gem_object *obj;
>         int err = -ENOENT;
>  
> +       if (INTEL_GEN(dev_priv) >= 12)
> +               return -EOPNOTSUPP;

Not required.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: disable set/get_tiling ioctl on gen12+

[Daniel Vetter]

On Tue, Aug 20, 2019 at 8:55 PM Chris Wilson <chris@chris-wilson.co.uk> wrote:
>
> Quoting Daniel Vetter (2019-08-20 18:06:31)
> > The cpu (de)tiler hw is gone, this stopped being useful. Plus it never
> > supported any of the fancy new tiling formats, which means userspace
> > also stopped using the magic side-channel this provides.
> >
> > This would totally break a lot of the igts, but they're already broken
> > for the same reasons as userspace on gen12 would be.
> >
> > Cc: Kenneth Graunke <kenneth@whitecape.org>
> > Cc: Jason Ekstrand <jason@jlekstrand.net>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > ---
> >  drivers/gpu/drm/i915/gem/i915_gem_tiling.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_tiling.c b/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
> > index ca0c2f451742..2ee96f27a294 100644
> > --- a/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
> > @@ -313,10 +313,15 @@ int
> >  i915_gem_set_tiling_ioctl(struct drm_device *dev, void *data,
> >                           struct drm_file *file)
> >  {
> > +       struct drm_i915_private *dev_priv = to_i915(dev);
> >         struct drm_i915_gem_set_tiling *args = data;
> >         struct drm_i915_gem_object *obj;
> >         int err;
> >
> > +       /* there is no cpu (de)tiling in the hw anymore on gen12+ */
> > +       if (INTEL_GEN(dev_priv) >= 12)
>
> This should be if (!NUM_FENCES(dev_priv)) if anything. You wouldn't even
> need the comment talking about random features, and instead focus on why
> not supporting the back channel is worth the maintenance cost of
> divergence.

Yeah that's much better.

> > +               return -EOPNOTSUPP;
> > +
> >         obj = i915_gem_object_lookup(file, args->handle);
> >         if (!obj)
> >                 return -ENOENT;
> > @@ -402,6 +407,9 @@ i915_gem_get_tiling_ioctl(struct drm_device *dev, void *data,
> >         struct drm_i915_gem_object *obj;
> >         int err = -ENOENT;
> >
> > +       if (INTEL_GEN(dev_priv) >= 12)
> > +               return -EOPNOTSUPP;
>
> Not required.

I figured since we have a lot of old libdrm that loved to do
get_tiling this would help in catching them. That's why I put it in.
Plus just some ocd, if the set is gone, kill the get one too. Otoh we
might stir something up.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.CHECKPATCH: warning for drm/i915: disable set/get_tiling ioctl on gen12+

[Patchwork]

== Series Details ==

Series: drm/i915: disable set/get_tiling ioctl on gen12+
URL   : https://patchwork.freedesktop.org/series/65495/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
7f41e4bbdc3a drm/i915: disable set/get_tiling ioctl on gen12+
-:48: WARNING:NO_AUTHOR_SIGN_OFF: Missing Signed-off-by: line by nominal patch author 'Daniel Vetter <daniel.vetter@ffwll.ch>'

total: 0 errors, 1 warnings, 0 checks, 24 lines checked

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: disable set/get_tiling ioctl on gen12+

[Chris Wilson]

Quoting Daniel Vetter (2019-08-20 20:06:19)
> On Tue, Aug 20, 2019 at 8:55 PM Chris Wilson <chris@chris-wilson.co.uk> wrote:
> >
> > Quoting Daniel Vetter (2019-08-20 18:06:31)
> > > The cpu (de)tiler hw is gone, this stopped being useful. Plus it never
> > > supported any of the fancy new tiling formats, which means userspace
> > > also stopped using the magic side-channel this provides.
> > >
> > > This would totally break a lot of the igts, but they're already broken
> > > for the same reasons as userspace on gen12 would be.
> > >
> > > Cc: Kenneth Graunke <kenneth@whitecape.org>
> > > Cc: Jason Ekstrand <jason@jlekstrand.net>
> > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/gem/i915_gem_tiling.c | 8 ++++++++
> > >  1 file changed, 8 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_tiling.c b/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
> > > index ca0c2f451742..2ee96f27a294 100644
> > > --- a/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
> > > +++ b/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
> > > @@ -313,10 +313,15 @@ int
> > >  i915_gem_set_tiling_ioctl(struct drm_device *dev, void *data,
> > >                           struct drm_file *file)
> > >  {
> > > +       struct drm_i915_private *dev_priv = to_i915(dev);
> > >         struct drm_i915_gem_set_tiling *args = data;
> > >         struct drm_i915_gem_object *obj;
> > >         int err;
> > >
> > > +       /* there is no cpu (de)tiling in the hw anymore on gen12+ */
> > > +       if (INTEL_GEN(dev_priv) >= 12)
> >
> > This should be if (!NUM_FENCES(dev_priv)) if anything. You wouldn't even
> > need the comment talking about random features, and instead focus on why
> > not supporting the back channel is worth the maintenance cost of
> > divergence.
> 
> Yeah that's much better.
> 
> > > +               return -EOPNOTSUPP;
> > > +
> > >         obj = i915_gem_object_lookup(file, args->handle);
> > >         if (!obj)
> > >                 return -ENOENT;
> > > @@ -402,6 +407,9 @@ i915_gem_get_tiling_ioctl(struct drm_device *dev, void *data,
> > >         struct drm_i915_gem_object *obj;
> > >         int err = -ENOENT;
> > >
> > > +       if (INTEL_GEN(dev_priv) >= 12)
> > > +               return -EOPNOTSUPP;
> >
> > Not required.
> 
> I figured since we have a lot of old libdrm that loved to do
> get_tiling this would help in catching them. That's why I put it in.
> Plus just some ocd, if the set is gone, kill the get one too. Otoh we
> might stir something up.

If we are using the num_fences as the feature test, they can query that
already via I915_GETPARAM. So this is just left as reporting what the
tiling mode is set on the object, which is then always none.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: failure for drm/i915: disable set/get_tiling ioctl on gen12+

[Patchwork]

== Series Details ==

Series: drm/i915: disable set/get_tiling ioctl on gen12+
URL   : https://patchwork.freedesktop.org/series/65495/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_6749 -> Patchwork_14109
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_14109 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_14109, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14109/

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_14109:

### IGT changes ###

#### Possible regressions ####

  * igt@i915_selftest@live_hugepages:
    - fi-kbl-8809g:       [PASS][1] -> [INCOMPLETE][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6749/fi-kbl-8809g/igt@i915_selftest@live_hugepages.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14109/fi-kbl-8809g/igt@i915_selftest@live_hugepages.html

  
#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * {igt@i915_selftest@live_gt_timelines}:
    - fi-kbl-7567u:       [PASS][3] -> [INCOMPLETE][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6749/fi-kbl-7567u/igt@i915_selftest@live_gt_timelines.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14109/fi-kbl-7567u/igt@i915_selftest@live_gt_timelines.html

  
Known issues
------------

  Here are the changes found in Patchwork_14109 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_create@basic-files:
    - fi-bxt-dsi:         [PASS][5] -> [INCOMPLETE][6] ([fdo#103927])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6749/fi-bxt-dsi/igt@gem_ctx_create@basic-files.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14109/fi-bxt-dsi/igt@gem_ctx_create@basic-files.html

  * igt@i915_module_load@reload:
    - fi-icl-u3:          [PASS][7] -> [INCOMPLETE][8] ([fdo#107713])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6749/fi-icl-u3/igt@i915_module_load@reload.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14109/fi-icl-u3/igt@i915_module_load@reload.html

  * igt@i915_selftest@live_active:
    - fi-byt-j1900:       [PASS][9] -> [INCOMPLETE][10] ([fdo#102657])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6749/fi-byt-j1900/igt@i915_selftest@live_active.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14109/fi-byt-j1900/igt@i915_selftest@live_active.html

  * igt@kms_chamelium@dp-crc-fast:
    - fi-icl-u2:          [PASS][11] -> [FAIL][12] ([fdo#109635 ] / [fdo#110387])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6749/fi-icl-u2/igt@kms_chamelium@dp-crc-fast.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14109/fi-icl-u2/igt@kms_chamelium@dp-crc-fast.html

  
#### Possible fixes ####

  * 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_6749/fi-blb-e6850/igt@gem_exec_suspend@basic-s3.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14109/fi-blb-e6850/igt@gem_exec_suspend@basic-s3.html

  * igt@gem_mmap_gtt@basic-copy:
    - fi-icl-u3:          [DMESG-WARN][15] ([fdo#107724]) -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6749/fi-icl-u3/igt@gem_mmap_gtt@basic-copy.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14109/fi-icl-u3/igt@gem_mmap_gtt@basic-copy.html

  
#### Warnings ####

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7567u:       [FAIL][17] ([fdo#109800]) -> [FAIL][18] ([fdo#111407])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6749/fi-kbl-7567u/igt@kms_chamelium@hdmi-hpd-fast.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14109/fi-kbl-7567u/igt@kms_chamelium@hdmi-hpd-fast.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#102657]: https://bugs.freedesktop.org/show_bug.cgi?id=102657
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [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#109635 ]: https://bugs.freedesktop.org/show_bug.cgi?id=109635 
  [fdo#109800]: https://bugs.freedesktop.org/show_bug.cgi?id=109800
  [fdo#110387]: https://bugs.freedesktop.org/show_bug.cgi?id=110387
  [fdo#111045]: https://bugs.freedesktop.org/show_bug.cgi?id=111045
  [fdo#111096]: https://bugs.freedesktop.org/show_bug.cgi?id=111096
  [fdo#111407]: https://bugs.freedesktop.org/show_bug.cgi?id=111407


Participating hosts (55 -> 46)
------------------------------

  Missing    (9): fi-kbl-soraka fi-cml-u2 fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-icl-y fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_6749 -> Patchwork_14109

  CI-20190529: 20190529
  CI_DRM_6749: 889140938c5c4bb9f2df652dd23d16fdcb86db72 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5146: 357dbe1869d88a2f08bcee4eebceff4ee9014424 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_14109: 7f41e4bbdc3a34bfd0659b883f061ce739741620 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

7f41e4bbdc3a drm/i915: disable set/get_tiling ioctl on gen12+

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14109/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] drm/i915: disable set/get_tiling ioctl on gen12+

[Daniel Vetter]

The cpu (de)tiler hw is gone, this stopped being useful. Plus it never
supported any of the fancy new tiling formats, which means userspace
also stopped using the magic side-channel this provides.

This would totally break a lot of the igts, but they're already broken
for the same reasons as userspace on gen12 would be.

v2: Look at ggtt->num_fences instead, that also avoids the need for a
comment (Chris). This also means that gen12 support really needs to
make sure num_fences is set to 0. There is a patch for that, but it
checks for HAS_MAPPABLE_APERTURE, which I'm not sure is the right
thing really. Adding relevant people.

Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Stuart Summers <stuart.summers@intel.com>
Cc: Matthew Auld <matthew.auld@intel.com>
Cc: Kenneth Graunke <kenneth@whitecape.org>
Cc: Jason Ekstrand <jason@jlekstrand.net>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_tiling.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_tiling.c b/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
index ca0c2f451742..e5d1ae8d4dba 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
@@ -313,10 +313,14 @@ int
 i915_gem_set_tiling_ioctl(struct drm_device *dev, void *data,
 			  struct drm_file *file)
 {
+	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct drm_i915_gem_set_tiling *args = data;
 	struct drm_i915_gem_object *obj;
 	int err;
 
+	if (!dev_priv->ggtt.num_fences)
+		return -EOPNOTSUPP;
+
 	obj = i915_gem_object_lookup(file, args->handle);
 	if (!obj)
 		return -ENOENT;
@@ -402,6 +406,9 @@ i915_gem_get_tiling_ioctl(struct drm_device *dev, void *data,
 	struct drm_i915_gem_object *obj;
 	int err = -ENOENT;
 
+	if (!dev_priv->ggtt.num_fences)
+		return -EOPNOTSUPP;
+
 	rcu_read_lock();
 	obj = i915_gem_object_lookup_rcu(file, args->handle);
 	if (obj) {
-- 
2.23.0.rc1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.CHECKPATCH: warning for drm/i915: disable set/get_tiling ioctl on gen12+ (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: disable set/get_tiling ioctl on gen12+ (rev2)
URL   : https://patchwork.freedesktop.org/series/65495/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
5281445913b8 drm/i915: disable set/get_tiling ioctl on gen12+
-:56: WARNING:NO_AUTHOR_SIGN_OFF: Missing Signed-off-by: line by nominal patch author 'Daniel Vetter <daniel.vetter@ffwll.ch>'

total: 0 errors, 1 warnings, 0 checks, 23 lines checked

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: disable set/get_tiling ioctl on gen12+

[Daniele Ceraolo Spurio]

On 8/20/19 12:54 PM, Daniel Vetter wrote:
> The cpu (de)tiler hw is gone, this stopped being useful. Plus it never
> supported any of the fancy new tiling formats, which means userspace
> also stopped using the magic side-channel this provides.
> 
> This would totally break a lot of the igts, but they're already broken
> for the same reasons as userspace on gen12 would be.
> 
> v2: Look at ggtt->num_fences instead, that also avoids the need for a
> comment (Chris). This also means that gen12 support really needs to
> make sure num_fences is set to 0. There is a patch for that, but it
> checks for HAS_MAPPABLE_APERTURE, which I'm not sure is the right
> thing really. Adding relevant people.
> 

We'd obviously need to make that setting for all gen12+, because TGL 
does have mappable aperture.

Apart from the tiling ioctl, the only place I see where we set tiling is 
intel_alloc_initial_plane_obj(), can the users of that object handle the 
lack of fences gracefully? When I wrote the num_fences=0 patch I was 
expecting display to be unavailable, so I didn't really look at that 
part of the code.

It'd also be nice to be more explicit with fencing since we seem to 
often call i915_vma_pin_iomap, which implicitly applies a fence if 
needed, on objects that can't be tiled or have had a fence assigned a 
few lines before. This is more a nice to have tough, possibly together 
with a split of the "mappable" and "fenceable" attributes of the vma.

Daniele

> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Stuart Summers <stuart.summers@intel.com>
> Cc: Matthew Auld <matthew.auld@intel.com>
> Cc: Kenneth Graunke <kenneth@whitecape.org>
> Cc: Jason Ekstrand <jason@jlekstrand.net>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>   drivers/gpu/drm/i915/gem/i915_gem_tiling.c | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_tiling.c b/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
> index ca0c2f451742..e5d1ae8d4dba 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
> @@ -313,10 +313,14 @@ int
>   i915_gem_set_tiling_ioctl(struct drm_device *dev, void *data,
>   			  struct drm_file *file)
>   {
> +	struct drm_i915_private *dev_priv = to_i915(dev);
>   	struct drm_i915_gem_set_tiling *args = data;
>   	struct drm_i915_gem_object *obj;
>   	int err;
>   
> +	if (!dev_priv->ggtt.num_fences)
> +		return -EOPNOTSUPP;
> +
>   	obj = i915_gem_object_lookup(file, args->handle);
>   	if (!obj)
>   		return -ENOENT;
> @@ -402,6 +406,9 @@ i915_gem_get_tiling_ioctl(struct drm_device *dev, void *data,
>   	struct drm_i915_gem_object *obj;
>   	int err = -ENOENT;
>   
> +	if (!dev_priv->ggtt.num_fences)
> +		return -EOPNOTSUPP;
> +
>   	rcu_read_lock();
>   	obj = i915_gem_object_lookup_rcu(file, args->handle);
>   	if (obj) {
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915: disable set/get_tiling ioctl on gen12+ (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: disable set/get_tiling ioctl on gen12+ (rev2)
URL   : https://patchwork.freedesktop.org/series/65495/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6750 -> Patchwork_14110
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/

Known issues
------------

  Here are the changes found in Patchwork_14110 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_reloc@basic-cpu-noreloc:
    - fi-icl-u3:          [PASS][1] -> [DMESG-WARN][2] ([fdo#107724])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/fi-icl-u3/igt@gem_exec_reloc@basic-cpu-noreloc.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/fi-icl-u3/igt@gem_exec_reloc@basic-cpu-noreloc.html

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-cml-u2:          [PASS][3] -> [DMESG-WARN][4] ([fdo#102505])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/fi-cml-u2/igt@kms_chamelium@common-hpd-after-suspend.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/fi-cml-u2/igt@kms_chamelium@common-hpd-after-suspend.html

  * igt@kms_chamelium@hdmi-crc-fast:
    - fi-kbl-7500u:       [PASS][5] -> [FAIL][6] ([fdo#109635 ])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/fi-kbl-7500u/igt@kms_chamelium@hdmi-crc-fast.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/fi-kbl-7500u/igt@kms_chamelium@hdmi-crc-fast.html

  
#### Possible fixes ####

  * igt@i915_selftest@live_reset:
    - fi-icl-u2:          [INCOMPLETE][7] ([fdo#107713]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/fi-icl-u2/igt@i915_selftest@live_reset.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/fi-icl-u2/igt@i915_selftest@live_reset.html

  
#### Warnings ####

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7500u:       [FAIL][9] ([fdo#111096]) -> [FAIL][10] ([fdo#111407])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html

  
  [fdo#102505]: https://bugs.freedesktop.org/show_bug.cgi?id=102505
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#109635 ]: https://bugs.freedesktop.org/show_bug.cgi?id=109635 
  [fdo#111096]: https://bugs.freedesktop.org/show_bug.cgi?id=111096
  [fdo#111407]: https://bugs.freedesktop.org/show_bug.cgi?id=111407


Participating hosts (50 -> 45)
------------------------------

  Additional (3): fi-skl-guc fi-apl-guc fi-pnv-d510 
  Missing    (8): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-icl-y fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_6750 -> Patchwork_14110

  CI-20190529: 20190529
  CI_DRM_6750: ba37f74dbdc1e78e70a5a2e5f4ce8d762d06eaa7 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5146: 357dbe1869d88a2f08bcee4eebceff4ee9014424 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_14110: 5281445913b8df5c789c9ec66dcfb0e0576d3881 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

5281445913b8 drm/i915: disable set/get_tiling ioctl on gen12+

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.IGT: failure for drm/i915: disable set/get_tiling ioctl on gen12+ (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: disable set/get_tiling ioctl on gen12+ (rev2)
URL   : https://patchwork.freedesktop.org/series/65495/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_6750_full -> Patchwork_14110_full
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_14110_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_14110_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_14110_full:

### IGT changes ###

#### Possible regressions ####

  * igt@i915_selftest@live_gtt:
    - shard-glk:          [PASS][1] -> [DMESG-WARN][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-glk1/igt@i915_selftest@live_gtt.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-glk5/igt@i915_selftest@live_gtt.html

  
Known issues
------------

  Here are the changes found in Patchwork_14110_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_shared@exec-single-timeline-bsd:
    - shard-iclb:         [PASS][3] -> [SKIP][4] ([fdo#110841])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb8/igt@gem_ctx_shared@exec-single-timeline-bsd.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-iclb1/igt@gem_ctx_shared@exec-single-timeline-bsd.html

  * igt@gem_exec_async@concurrent-writes-bsd:
    - shard-iclb:         [PASS][5] -> [SKIP][6] ([fdo#111325]) +4 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb3/igt@gem_exec_async@concurrent-writes-bsd.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-iclb4/igt@gem_exec_async@concurrent-writes-bsd.html

  * igt@gem_softpin@noreloc-s3:
    - shard-skl:          [PASS][7] -> [INCOMPLETE][8] ([fdo#104108])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-skl9/igt@gem_softpin@noreloc-s3.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-skl1/igt@gem_softpin@noreloc-s3.html

  * igt@kms_cursor_legacy@long-nonblocking-modeset-vs-cursor-atomic:
    - shard-skl:          [PASS][9] -> [DMESG-WARN][10] ([fdo#105541])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-skl4/igt@kms_cursor_legacy@long-nonblocking-modeset-vs-cursor-atomic.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-skl6/igt@kms_cursor_legacy@long-nonblocking-modeset-vs-cursor-atomic.html

  * igt@kms_plane_lowres@pipe-a-tiling-y:
    - shard-iclb:         [PASS][11] -> [FAIL][12] ([fdo#103166])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb3/igt@kms_plane_lowres@pipe-a-tiling-y.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-iclb5/igt@kms_plane_lowres@pipe-a-tiling-y.html

  * igt@kms_psr2_su@frontbuffer:
    - shard-iclb:         [PASS][13] -> [SKIP][14] ([fdo#109642] / [fdo#111068])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb2/igt@kms_psr2_su@frontbuffer.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-iclb7/igt@kms_psr2_su@frontbuffer.html

  * igt@kms_psr@psr2_cursor_mmap_cpu:
    - shard-iclb:         [PASS][15] -> [SKIP][16] ([fdo#109441]) +3 similar issues
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb2/igt@kms_psr@psr2_cursor_mmap_cpu.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-iclb8/igt@kms_psr@psr2_cursor_mmap_cpu.html

  * igt@kms_vblank@pipe-a-ts-continuation-suspend:
    - shard-apl:          [PASS][17] -> [DMESG-WARN][18] ([fdo#108566]) +2 similar issues
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-apl3/igt@kms_vblank@pipe-a-ts-continuation-suspend.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-apl7/igt@kms_vblank@pipe-a-ts-continuation-suspend.html

  * igt@perf_pmu@rc6:
    - shard-kbl:          [PASS][19] -> [SKIP][20] ([fdo#109271])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-kbl4/igt@perf_pmu@rc6.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-kbl7/igt@perf_pmu@rc6.html

  * igt@prime_busy@hang-bsd2:
    - shard-iclb:         [PASS][21] -> [SKIP][22] ([fdo#109276]) +24 similar issues
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb4/igt@prime_busy@hang-bsd2.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-iclb3/igt@prime_busy@hang-bsd2.html

  
#### Possible fixes ####

  * igt@gem_eio@reset-stress:
    - shard-skl:          [FAIL][23] ([fdo#109661]) -> [PASS][24]
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-skl5/igt@gem_eio@reset-stress.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-skl10/igt@gem_eio@reset-stress.html

  * igt@gem_exec_balancer@smoke:
    - shard-iclb:         [SKIP][25] ([fdo#110854]) -> [PASS][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb8/igt@gem_exec_balancer@smoke.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-iclb1/igt@gem_exec_balancer@smoke.html

  * igt@gem_exec_schedule@wide-bsd:
    - shard-iclb:         [SKIP][27] ([fdo#111325]) -> [PASS][28] +7 similar issues
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb2/igt@gem_exec_schedule@wide-bsd.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-iclb8/igt@gem_exec_schedule@wide-bsd.html

  * igt@i915_pm_rps@min-max-config-loaded:
    - shard-iclb:         [FAIL][29] ([fdo#111409]) -> [PASS][30]
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb7/igt@i915_pm_rps@min-max-config-loaded.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-iclb5/igt@i915_pm_rps@min-max-config-loaded.html

  * igt@i915_suspend@sysfs-reader:
    - shard-apl:          [DMESG-WARN][31] ([fdo#108566]) -> [PASS][32] +6 similar issues
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-apl7/igt@i915_suspend@sysfs-reader.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-apl4/igt@i915_suspend@sysfs-reader.html

  * igt@kms_cursor_crc@pipe-c-cursor-256x85-sliding:
    - shard-iclb:         [INCOMPLETE][33] ([fdo#107713]) -> [PASS][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb7/igt@kms_cursor_crc@pipe-c-cursor-256x85-sliding.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-iclb2/igt@kms_cursor_crc@pipe-c-cursor-256x85-sliding.html

  * igt@kms_cursor_legacy@pipe-c-single-move:
    - shard-apl:          [INCOMPLETE][35] ([fdo#103927]) -> [PASS][36] +2 similar issues
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-apl1/igt@kms_cursor_legacy@pipe-c-single-move.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-apl2/igt@kms_cursor_legacy@pipe-c-single-move.html

  * igt@kms_fbcon_fbt@psr-suspend:
    - shard-skl:          [INCOMPLETE][37] ([fdo#104108]) -> [PASS][38]
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-skl10/igt@kms_fbcon_fbt@psr-suspend.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-skl1/igt@kms_fbcon_fbt@psr-suspend.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-shrfb-plflip-blt:
    - shard-iclb:         [FAIL][39] ([fdo#103167]) -> [PASS][40] +3 similar issues
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb4/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-shrfb-plflip-blt.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-iclb2/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-shrfb-plflip-blt.html

  * igt@kms_psr@psr2_sprite_mmap_gtt:
    - shard-iclb:         [SKIP][41] ([fdo#109441]) -> [PASS][42] +3 similar issues
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb4/igt@kms_psr@psr2_sprite_mmap_gtt.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-iclb2/igt@kms_psr@psr2_sprite_mmap_gtt.html

  * igt@perf@blocking:
    - shard-skl:          [FAIL][43] ([fdo#110728]) -> [PASS][44]
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-skl10/igt@perf@blocking.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-skl7/igt@perf@blocking.html

  * igt@prime_vgem@fence-wait-bsd2:
    - shard-iclb:         [SKIP][45] ([fdo#109276]) -> [PASS][46] +16 similar issues
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb7/igt@prime_vgem@fence-wait-bsd2.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-iclb2/igt@prime_vgem@fence-wait-bsd2.html

  
#### Warnings ####

  * igt@gem_mocs_settings@mocs-reset-bsd2:
    - shard-iclb:         [FAIL][47] ([fdo#111330]) -> [SKIP][48] ([fdo#109276]) +2 similar issues
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb2/igt@gem_mocs_settings@mocs-reset-bsd2.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-iclb8/igt@gem_mocs_settings@mocs-reset-bsd2.html

  * igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-indfb-draw-blt:
    - shard-skl:          [FAIL][49] ([fdo#108040]) -> [FAIL][50] ([fdo#103167])
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-skl7/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-indfb-draw-blt.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-skl10/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-indfb-draw-blt.html

  
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#105541]: https://bugs.freedesktop.org/show_bug.cgi?id=105541
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#108040]: https://bugs.freedesktop.org/show_bug.cgi?id=108040
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#109661]: https://bugs.freedesktop.org/show_bug.cgi?id=109661
  [fdo#110728]: https://bugs.freedesktop.org/show_bug.cgi?id=110728
  [fdo#110841]: https://bugs.freedesktop.org/show_bug.cgi?id=110841
  [fdo#110854]: https://bugs.freedesktop.org/show_bug.cgi?id=110854
  [fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
  [fdo#111325]: https://bugs.freedesktop.org/show_bug.cgi?id=111325
  [fdo#111330]: https://bugs.freedesktop.org/show_bug.cgi?id=111330
  [fdo#111409]: https://bugs.freedesktop.org/show_bug.cgi?id=111409


Participating hosts (10 -> 10)
------------------------------

  No changes in participating hosts


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_6750 -> Patchwork_14110

  CI-20190529: 20190529
  CI_DRM_6750: ba37f74dbdc1e78e70a5a2e5f4ce8d762d06eaa7 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5146: 357dbe1869d88a2f08bcee4eebceff4ee9014424 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_14110: 5281445913b8df5c789c9ec66dcfb0e0576d3881 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: disable set/get_tiling ioctl on gen12+

[Ville Syrjälä]

On Tue, Aug 20, 2019 at 01:57:44PM -0700, Daniele Ceraolo Spurio wrote:
> 
> 
> On 8/20/19 12:54 PM, Daniel Vetter wrote:
> > The cpu (de)tiler hw is gone, this stopped being useful. Plus it never
> > supported any of the fancy new tiling formats, which means userspace
> > also stopped using the magic side-channel this provides.
> > 
> > This would totally break a lot of the igts, but they're already broken
> > for the same reasons as userspace on gen12 would be.
> > 
> > v2: Look at ggtt->num_fences instead, that also avoids the need for a
> > comment (Chris). This also means that gen12 support really needs to
> > make sure num_fences is set to 0. There is a patch for that, but it
> > checks for HAS_MAPPABLE_APERTURE, which I'm not sure is the right
> > thing really. Adding relevant people.
> > 
> 
> We'd obviously need to make that setting for all gen12+, because TGL 
> does have mappable aperture.
> 
> Apart from the tiling ioctl, the only place I see where we set tiling is 
> intel_alloc_initial_plane_obj(), can the users of that object handle the 
> lack of fences gracefully?

Gen4+ display engine has its own tiling controls and doesn't care about
fences. So we should be able to leave the obj tiling set to NONE.

Hmm. Actually I think we should reject tiled framebuffers in the BIOS
fb takeover because fbdev needs a linear view for the memory. No can
do without the fence.

> When I wrote the num_fences=0 patch I was 
> expecting display to be unavailable, so I didn't really look at that 
> part of the code.
> 
> It'd also be nice to be more explicit with fencing since we seem to 
> often call i915_vma_pin_iomap, which implicitly applies a fence if 
> needed, on objects that can't be tiled or have had a fence assigned a 
> few lines before. This is more a nice to have tough, possibly together 
> with a split of the "mappable" and "fenceable" attributes of the vma.
> 
> Daniele
> 
> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> > Cc: Stuart Summers <stuart.summers@intel.com>
> > Cc: Matthew Auld <matthew.auld@intel.com>
> > Cc: Kenneth Graunke <kenneth@whitecape.org>
> > Cc: Jason Ekstrand <jason@jlekstrand.net>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > ---
> >   drivers/gpu/drm/i915/gem/i915_gem_tiling.c | 7 +++++++
> >   1 file changed, 7 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_tiling.c b/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
> > index ca0c2f451742..e5d1ae8d4dba 100644
> > --- a/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
> > @@ -313,10 +313,14 @@ int
> >   i915_gem_set_tiling_ioctl(struct drm_device *dev, void *data,
> >   			  struct drm_file *file)
> >   {
> > +	struct drm_i915_private *dev_priv = to_i915(dev);
> >   	struct drm_i915_gem_set_tiling *args = data;
> >   	struct drm_i915_gem_object *obj;
> >   	int err;
> >   
> > +	if (!dev_priv->ggtt.num_fences)
> > +		return -EOPNOTSUPP;
> > +
> >   	obj = i915_gem_object_lookup(file, args->handle);
> >   	if (!obj)
> >   		return -ENOENT;
> > @@ -402,6 +406,9 @@ i915_gem_get_tiling_ioctl(struct drm_device *dev, void *data,
> >   	struct drm_i915_gem_object *obj;
> >   	int err = -ENOENT;
> >   
> > +	if (!dev_priv->ggtt.num_fences)
> > +		return -EOPNOTSUPP;
> > +
> >   	rcu_read_lock();
> >   	obj = i915_gem_object_lookup_rcu(file, args->handle);
> >   	if (obj) {
> > 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: disable set/get_tiling ioctl on gen12+

[Daniel Vetter]

On Wed, Aug 21, 2019 at 3:55 PM Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
>
> On Tue, Aug 20, 2019 at 01:57:44PM -0700, Daniele Ceraolo Spurio wrote:
> >
> >
> > On 8/20/19 12:54 PM, Daniel Vetter wrote:
> > > The cpu (de)tiler hw is gone, this stopped being useful. Plus it never
> > > supported any of the fancy new tiling formats, which means userspace
> > > also stopped using the magic side-channel this provides.
> > >
> > > This would totally break a lot of the igts, but they're already broken
> > > for the same reasons as userspace on gen12 would be.
> > >
> > > v2: Look at ggtt->num_fences instead, that also avoids the need for a
> > > comment (Chris). This also means that gen12 support really needs to
> > > make sure num_fences is set to 0. There is a patch for that, but it
> > > checks for HAS_MAPPABLE_APERTURE, which I'm not sure is the right
> > > thing really. Adding relevant people.
> > >
> >
> > We'd obviously need to make that setting for all gen12+, because TGL
> > does have mappable aperture.
> >
> > Apart from the tiling ioctl, the only place I see where we set tiling is
> > intel_alloc_initial_plane_obj(), can the users of that object handle the
> > lack of fences gracefully?
>
> Gen4+ display engine has its own tiling controls and doesn't care about
> fences. So we should be able to leave the obj tiling set to NONE.
>
> Hmm. Actually I think we should reject tiled framebuffers in the BIOS
> fb takeover because fbdev needs a linear view for the memory. No can
> do without the fence.

Yeah I think this is just more fallout from "no more fences in the hw".
-Daniel

>
> > When I wrote the num_fences=0 patch I was
> > expecting display to be unavailable, so I didn't really look at that
> > part of the code.
> >
> > It'd also be nice to be more explicit with fencing since we seem to
> > often call i915_vma_pin_iomap, which implicitly applies a fence if
> > needed, on objects that can't be tiled or have had a fence assigned a
> > few lines before. This is more a nice to have tough, possibly together
> > with a split of the "mappable" and "fenceable" attributes of the vma.
> >
> > Daniele
> >
> > > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> > > Cc: Stuart Summers <stuart.summers@intel.com>
> > > Cc: Matthew Auld <matthew.auld@intel.com>
> > > Cc: Kenneth Graunke <kenneth@whitecape.org>
> > > Cc: Jason Ekstrand <jason@jlekstrand.net>
> > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > > ---
> > >   drivers/gpu/drm/i915/gem/i915_gem_tiling.c | 7 +++++++
> > >   1 file changed, 7 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_tiling.c b/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
> > > index ca0c2f451742..e5d1ae8d4dba 100644
> > > --- a/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
> > > +++ b/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
> > > @@ -313,10 +313,14 @@ int
> > >   i915_gem_set_tiling_ioctl(struct drm_device *dev, void *data,
> > >                       struct drm_file *file)
> > >   {
> > > +   struct drm_i915_private *dev_priv = to_i915(dev);
> > >     struct drm_i915_gem_set_tiling *args = data;
> > >     struct drm_i915_gem_object *obj;
> > >     int err;
> > >
> > > +   if (!dev_priv->ggtt.num_fences)
> > > +           return -EOPNOTSUPP;
> > > +
> > >     obj = i915_gem_object_lookup(file, args->handle);
> > >     if (!obj)
> > >             return -ENOENT;
> > > @@ -402,6 +406,9 @@ i915_gem_get_tiling_ioctl(struct drm_device *dev, void *data,
> > >     struct drm_i915_gem_object *obj;
> > >     int err = -ENOENT;
> > >
> > > +   if (!dev_priv->ggtt.num_fences)
> > > +           return -EOPNOTSUPP;
> > > +
> > >     rcu_read_lock();
> > >     obj = i915_gem_object_lookup_rcu(file, args->handle);
> > >     if (obj) {
> > >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Ville Syrjälä
> Intel



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: disable set/get_tiling ioctl on gen12+

[Jason Ekstrand]

[-- Attachment #1.1: Type: text/plain, Size: 4837 bytes --]

Acked-by: Jason Ekstrand <jason@jlekstrand.net>

On Wed, Aug 21, 2019 at 10:20 AM Daniel Vetter <daniel.vetter@ffwll.ch>
wrote:

> On Wed, Aug 21, 2019 at 3:55 PM Ville Syrjälä
> <ville.syrjala@linux.intel.com> wrote:
> >
> > On Tue, Aug 20, 2019 at 01:57:44PM -0700, Daniele Ceraolo Spurio wrote:
> > >
> > >
> > > On 8/20/19 12:54 PM, Daniel Vetter wrote:
> > > > The cpu (de)tiler hw is gone, this stopped being useful. Plus it
> never
> > > > supported any of the fancy new tiling formats, which means userspace
> > > > also stopped using the magic side-channel this provides.
> > > >
> > > > This would totally break a lot of the igts, but they're already
> broken
> > > > for the same reasons as userspace on gen12 would be.
> > > >
> > > > v2: Look at ggtt->num_fences instead, that also avoids the need for a
> > > > comment (Chris). This also means that gen12 support really needs to
> > > > make sure num_fences is set to 0. There is a patch for that, but it
> > > > checks for HAS_MAPPABLE_APERTURE, which I'm not sure is the right
> > > > thing really. Adding relevant people.
> > > >
> > >
> > > We'd obviously need to make that setting for all gen12+, because TGL
> > > does have mappable aperture.
> > >
> > > Apart from the tiling ioctl, the only place I see where we set tiling
> is
> > > intel_alloc_initial_plane_obj(), can the users of that object handle
> the
> > > lack of fences gracefully?
> >
> > Gen4+ display engine has its own tiling controls and doesn't care about
> > fences. So we should be able to leave the obj tiling set to NONE.
> >
> > Hmm. Actually I think we should reject tiled framebuffers in the BIOS
> > fb takeover because fbdev needs a linear view for the memory. No can
> > do without the fence.
>
> Yeah I think this is just more fallout from "no more fences in the hw".
> -Daniel
>
> >
> > > When I wrote the num_fences=0 patch I was
> > > expecting display to be unavailable, so I didn't really look at that
> > > part of the code.
> > >
> > > It'd also be nice to be more explicit with fencing since we seem to
> > > often call i915_vma_pin_iomap, which implicitly applies a fence if
> > > needed, on objects that can't be tiled or have had a fence assigned a
> > > few lines before. This is more a nice to have tough, possibly together
> > > with a split of the "mappable" and "fenceable" attributes of the vma.
> > >
> > > Daniele
> > >
> > > > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> > > > Cc: Stuart Summers <stuart.summers@intel.com>
> > > > Cc: Matthew Auld <matthew.auld@intel.com>
> > > > Cc: Kenneth Graunke <kenneth@whitecape.org>
> > > > Cc: Jason Ekstrand <jason@jlekstrand.net>
> > > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > > > ---
> > > >   drivers/gpu/drm/i915/gem/i915_gem_tiling.c | 7 +++++++
> > > >   1 file changed, 7 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
> b/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
> > > > index ca0c2f451742..e5d1ae8d4dba 100644
> > > > --- a/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
> > > > +++ b/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
> > > > @@ -313,10 +313,14 @@ int
> > > >   i915_gem_set_tiling_ioctl(struct drm_device *dev, void *data,
> > > >                       struct drm_file *file)
> > > >   {
> > > > +   struct drm_i915_private *dev_priv = to_i915(dev);
> > > >     struct drm_i915_gem_set_tiling *args = data;
> > > >     struct drm_i915_gem_object *obj;
> > > >     int err;
> > > >
> > > > +   if (!dev_priv->ggtt.num_fences)
> > > > +           return -EOPNOTSUPP;
> > > > +
> > > >     obj = i915_gem_object_lookup(file, args->handle);
> > > >     if (!obj)
> > > >             return -ENOENT;
> > > > @@ -402,6 +406,9 @@ i915_gem_get_tiling_ioctl(struct drm_device
> *dev, void *data,
> > > >     struct drm_i915_gem_object *obj;
> > > >     int err = -ENOENT;
> > > >
> > > > +   if (!dev_priv->ggtt.num_fences)
> > > > +           return -EOPNOTSUPP;
> > > > +
> > > >     rcu_read_lock();
> > > >     obj = i915_gem_object_lookup_rcu(file, args->handle);
> > > >     if (obj) {
> > > >
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> > --
> > Ville Syrjälä
> > Intel
>
>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[-- Attachment #1.2: Type: text/html, Size: 7241 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: disable set/get_tiling ioctl on gen12+

[Souza, Jose]

Reviewed-by: José Roberto de Souza <jose.souza@intel.com>

Pushing this one in a few minutes.

On Thu, 2019-08-22 at 14:25 -0500, Jason Ekstrand wrote:
> Acked-by: Jason Ekstrand <jason@jlekstrand.net>
> 
> On Wed, Aug 21, 2019 at 10:20 AM Daniel Vetter <
> daniel.vetter@ffwll.ch> wrote:
> > On Wed, Aug 21, 2019 at 3:55 PM Ville Syrjälä
> > <ville.syrjala@linux.intel.com> wrote:
> > >
> > > On Tue, Aug 20, 2019 at 01:57:44PM -0700, Daniele Ceraolo Spurio
> > wrote:
> > > >
> > > >
> > > > On 8/20/19 12:54 PM, Daniel Vetter wrote:
> > > > > The cpu (de)tiler hw is gone, this stopped being useful. Plus
> > it never
> > > > > supported any of the fancy new tiling formats, which means
> > userspace
> > > > > also stopped using the magic side-channel this provides.
> > > > >
> > > > > This would totally break a lot of the igts, but they're
> > already broken
> > > > > for the same reasons as userspace on gen12 would be.
> > > > >
> > > > > v2: Look at ggtt->num_fences instead, that also avoids the
> > need for a
> > > > > comment (Chris). This also means that gen12 support really
> > needs to
> > > > > make sure num_fences is set to 0. There is a patch for that,
> > but it
> > > > > checks for HAS_MAPPABLE_APERTURE, which I'm not sure is the
> > right
> > > > > thing really. Adding relevant people.
> > > > >
> > > >
> > > > We'd obviously need to make that setting for all gen12+,
> > because TGL
> > > > does have mappable aperture.
> > > >
> > > > Apart from the tiling ioctl, the only place I see where we set
> > tiling is
> > > > intel_alloc_initial_plane_obj(), can the users of that object
> > handle the
> > > > lack of fences gracefully?
> > >
> > > Gen4+ display engine has its own tiling controls and doesn't care
> > about
> > > fences. So we should be able to leave the obj tiling set to NONE.
> > >
> > > Hmm. Actually I think we should reject tiled framebuffers in the
> > BIOS
> > > fb takeover because fbdev needs a linear view for the memory. No
> > can
> > > do without the fence.
> > 
> > Yeah I think this is just more fallout from "no more fences in the
> > hw".
> > -Daniel
> > 
> > >
> > > > When I wrote the num_fences=0 patch I was
> > > > expecting display to be unavailable, so I didn't really look at
> > that
> > > > part of the code.
> > > >
> > > > It'd also be nice to be more explicit with fencing since we
> > seem to
> > > > often call i915_vma_pin_iomap, which implicitly applies a fence
> > if
> > > > needed, on objects that can't be tiled or have had a fence
> > assigned a
> > > > few lines before. This is more a nice to have tough, possibly
> > together
> > > > with a split of the "mappable" and "fenceable" attributes of
> > the vma.
> > > >
> > > > Daniele
> > > >
> > > > > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> > > > > Cc: Stuart Summers <stuart.summers@intel.com>
> > > > > Cc: Matthew Auld <matthew.auld@intel.com>
> > > > > Cc: Kenneth Graunke <kenneth@whitecape.org>
> > > > > Cc: Jason Ekstrand <jason@jlekstrand.net>
> > > > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > > > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > > > > ---
> > > > >   drivers/gpu/drm/i915/gem/i915_gem_tiling.c | 7 +++++++
> > > > >   1 file changed, 7 insertions(+)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
> > b/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
> > > > > index ca0c2f451742..e5d1ae8d4dba 100644
> > > > > --- a/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
> > > > > +++ b/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
> > > > > @@ -313,10 +313,14 @@ int
> > > > >   i915_gem_set_tiling_ioctl(struct drm_device *dev, void
> > *data,
> > > > >                       struct drm_file *file)
> > > > >   {
> > > > > +   struct drm_i915_private *dev_priv = to_i915(dev);
> > > > >     struct drm_i915_gem_set_tiling *args = data;
> > > > >     struct drm_i915_gem_object *obj;
> > > > >     int err;
> > > > >
> > > > > +   if (!dev_priv->ggtt.num_fences)
> > > > > +           return -EOPNOTSUPP;
> > > > > +
> > > > >     obj = i915_gem_object_lookup(file, args->handle);
> > > > >     if (!obj)
> > > > >             return -ENOENT;
> > > > > @@ -402,6 +406,9 @@ i915_gem_get_tiling_ioctl(struct
> > drm_device *dev, void *data,
> > > > >     struct drm_i915_gem_object *obj;
> > > > >     int err = -ENOENT;
> > > > >
> > > > > +   if (!dev_priv->ggtt.num_fences)
> > > > > +           return -EOPNOTSUPP;
> > > > > +
> > > > >     rcu_read_lock();
> > > > >     obj = i915_gem_object_lookup_rcu(file, args->handle);
> > > > >     if (obj) {
> > > > >
> > > > _______________________________________________
> > > > Intel-gfx mailing list
> > > > Intel-gfx@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > >
> > > --
> > > Ville Syrjälä
> > > Intel
> > 
> > 
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: disable set/get_tiling ioctl on gen12+

[Chris Wilson]

Quoting Souza, Jose (2019-08-28 21:11:53)
> Reviewed-by: José Roberto de Souza <jose.souza@intel.com>

It's using a non-standard for i915 error code, and get_tiling is not
affected, it will always return LINEAR. You cannot set tiling as there
is no fence (according to the new abi).
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: disable set/get_tiling ioctl on gen12+

[Souza, Jose]

On Wed, 2019-08-28 at 21:13 +0100, Chris Wilson wrote:
> Quoting Souza, Jose (2019-08-28 21:11:53)
> > Reviewed-by: José Roberto de Souza <jose.souza@intel.com>
> 
> It's using a non-standard for i915 error code, and get_tiling is not

Huum should it use ENOTSUPP then?!

> affected, it will always return LINEAR. You cannot set tiling as 

Following this set_tiling() LINEAR should be allowed too.
I prefer the current approach of returning error.

> there
> is no fence (according to the new abi).
> -Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: disable set/get_tiling ioctl on gen12+

[Daniel Vetter]

On Wed, Aug 28, 2019 at 08:31:27PM +0000, Souza, Jose wrote:
> On Wed, 2019-08-28 at 21:13 +0100, Chris Wilson wrote:
> > Quoting Souza, Jose (2019-08-28 21:11:53)
> > > Reviewed-by: José Roberto de Souza <jose.souza@intel.com>
> > 
> > It's using a non-standard for i915 error code, and get_tiling is not
> 
> Huum should it use ENOTSUPP then?!

https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#recommended-ioctl-return-values

Per above "feature not supported" -> EOPNOTSUPP.

> > affected, it will always return LINEAR. You cannot set tiling as 
> 
> Following this set_tiling() LINEAR should be allowed too.
> I prefer the current approach of returning error.

I'm not seeing the value in keeping get_tiling supported. Either userspace
still uses the legacy backhannel and dri2, in which case it needs to be
fixed no matter what. Or it's using modifiers, in which case this is dead
code. Only other user I can think of is takeover for fastboot, but if you
get anything else than untiled it's also broken (we don't have an ioctl to
read out the modifiers, heck even all the planes, there's no getfb2).

So really not seeing the point in keeping that working.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for drm/i915: disable set/get_tiling ioctl on gen12+ (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: disable set/get_tiling ioctl on gen12+ (rev2)
URL   : https://patchwork.freedesktop.org/series/65495/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6750_full -> Patchwork_14110_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Known issues
------------

  Here are the changes found in Patchwork_14110_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_shared@exec-single-timeline-bsd:
    - shard-iclb:         [PASS][1] -> [SKIP][2] ([fdo#110841])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb8/igt@gem_ctx_shared@exec-single-timeline-bsd.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-iclb1/igt@gem_ctx_shared@exec-single-timeline-bsd.html

  * igt@gem_exec_async@concurrent-writes-bsd:
    - shard-iclb:         [PASS][3] -> [SKIP][4] ([fdo#111325]) +4 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb3/igt@gem_exec_async@concurrent-writes-bsd.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-iclb4/igt@gem_exec_async@concurrent-writes-bsd.html

  * igt@gem_softpin@noreloc-s3:
    - shard-skl:          [PASS][5] -> [INCOMPLETE][6] ([fdo#104108])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-skl9/igt@gem_softpin@noreloc-s3.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-skl1/igt@gem_softpin@noreloc-s3.html

  * igt@i915_selftest@live_gtt:
    - shard-glk:          [PASS][7] -> [DMESG-WARN][8] ([fdo#110633 ])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-glk1/igt@i915_selftest@live_gtt.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-glk5/igt@i915_selftest@live_gtt.html

  * igt@kms_cursor_legacy@long-nonblocking-modeset-vs-cursor-atomic:
    - shard-skl:          [PASS][9] -> [DMESG-WARN][10] ([fdo#105541])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-skl4/igt@kms_cursor_legacy@long-nonblocking-modeset-vs-cursor-atomic.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-skl6/igt@kms_cursor_legacy@long-nonblocking-modeset-vs-cursor-atomic.html

  * igt@kms_plane_lowres@pipe-a-tiling-y:
    - shard-iclb:         [PASS][11] -> [FAIL][12] ([fdo#103166])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb3/igt@kms_plane_lowres@pipe-a-tiling-y.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-iclb5/igt@kms_plane_lowres@pipe-a-tiling-y.html

  * igt@kms_psr2_su@frontbuffer:
    - shard-iclb:         [PASS][13] -> [SKIP][14] ([fdo#109642] / [fdo#111068])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb2/igt@kms_psr2_su@frontbuffer.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-iclb7/igt@kms_psr2_su@frontbuffer.html

  * igt@kms_psr@psr2_cursor_mmap_cpu:
    - shard-iclb:         [PASS][15] -> [SKIP][16] ([fdo#109441]) +3 similar issues
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb2/igt@kms_psr@psr2_cursor_mmap_cpu.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-iclb8/igt@kms_psr@psr2_cursor_mmap_cpu.html

  * igt@kms_vblank@pipe-a-ts-continuation-suspend:
    - shard-apl:          [PASS][17] -> [DMESG-WARN][18] ([fdo#108566]) +2 similar issues
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-apl3/igt@kms_vblank@pipe-a-ts-continuation-suspend.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-apl7/igt@kms_vblank@pipe-a-ts-continuation-suspend.html

  * igt@perf_pmu@rc6:
    - shard-kbl:          [PASS][19] -> [SKIP][20] ([fdo#109271])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-kbl4/igt@perf_pmu@rc6.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-kbl7/igt@perf_pmu@rc6.html

  * igt@prime_busy@hang-bsd2:
    - shard-iclb:         [PASS][21] -> [SKIP][22] ([fdo#109276]) +24 similar issues
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb4/igt@prime_busy@hang-bsd2.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-iclb3/igt@prime_busy@hang-bsd2.html

  
#### Possible fixes ####

  * igt@gem_eio@reset-stress:
    - shard-skl:          [FAIL][23] ([fdo#109661]) -> [PASS][24]
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-skl5/igt@gem_eio@reset-stress.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-skl10/igt@gem_eio@reset-stress.html

  * igt@gem_exec_balancer@smoke:
    - shard-iclb:         [SKIP][25] ([fdo#110854]) -> [PASS][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb8/igt@gem_exec_balancer@smoke.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-iclb1/igt@gem_exec_balancer@smoke.html

  * igt@gem_exec_schedule@wide-bsd:
    - shard-iclb:         [SKIP][27] ([fdo#111325]) -> [PASS][28] +7 similar issues
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb2/igt@gem_exec_schedule@wide-bsd.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-iclb8/igt@gem_exec_schedule@wide-bsd.html

  * igt@i915_pm_rps@min-max-config-loaded:
    - shard-iclb:         [FAIL][29] ([fdo#111409]) -> [PASS][30]
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb7/igt@i915_pm_rps@min-max-config-loaded.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-iclb5/igt@i915_pm_rps@min-max-config-loaded.html

  * igt@i915_suspend@sysfs-reader:
    - shard-apl:          [DMESG-WARN][31] ([fdo#108566]) -> [PASS][32] +6 similar issues
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-apl7/igt@i915_suspend@sysfs-reader.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-apl4/igt@i915_suspend@sysfs-reader.html

  * igt@kms_cursor_crc@pipe-c-cursor-256x85-sliding:
    - shard-iclb:         [INCOMPLETE][33] ([fdo#107713]) -> [PASS][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb7/igt@kms_cursor_crc@pipe-c-cursor-256x85-sliding.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-iclb2/igt@kms_cursor_crc@pipe-c-cursor-256x85-sliding.html

  * igt@kms_cursor_legacy@pipe-c-single-move:
    - shard-apl:          [INCOMPLETE][35] ([fdo#103927]) -> [PASS][36] +2 similar issues
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-apl1/igt@kms_cursor_legacy@pipe-c-single-move.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-apl2/igt@kms_cursor_legacy@pipe-c-single-move.html

  * igt@kms_fbcon_fbt@psr-suspend:
    - shard-skl:          [INCOMPLETE][37] ([fdo#104108]) -> [PASS][38]
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-skl10/igt@kms_fbcon_fbt@psr-suspend.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-skl1/igt@kms_fbcon_fbt@psr-suspend.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-shrfb-plflip-blt:
    - shard-iclb:         [FAIL][39] ([fdo#103167]) -> [PASS][40] +3 similar issues
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb4/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-shrfb-plflip-blt.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-iclb2/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-shrfb-plflip-blt.html

  * igt@kms_psr@psr2_sprite_mmap_gtt:
    - shard-iclb:         [SKIP][41] ([fdo#109441]) -> [PASS][42] +3 similar issues
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb4/igt@kms_psr@psr2_sprite_mmap_gtt.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-iclb2/igt@kms_psr@psr2_sprite_mmap_gtt.html

  * igt@perf@blocking:
    - shard-skl:          [FAIL][43] ([fdo#110728]) -> [PASS][44]
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-skl10/igt@perf@blocking.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-skl7/igt@perf@blocking.html

  * igt@prime_vgem@fence-wait-bsd2:
    - shard-iclb:         [SKIP][45] ([fdo#109276]) -> [PASS][46] +16 similar issues
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb7/igt@prime_vgem@fence-wait-bsd2.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-iclb2/igt@prime_vgem@fence-wait-bsd2.html

  
#### Warnings ####

  * igt@gem_mocs_settings@mocs-reset-bsd2:
    - shard-iclb:         [FAIL][47] ([fdo#111330]) -> [SKIP][48] ([fdo#109276]) +2 similar issues
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb2/igt@gem_mocs_settings@mocs-reset-bsd2.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-iclb8/igt@gem_mocs_settings@mocs-reset-bsd2.html

  * igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-indfb-draw-blt:
    - shard-skl:          [FAIL][49] ([fdo#108040]) -> [FAIL][50] ([fdo#103167])
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-skl7/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-indfb-draw-blt.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-skl10/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-indfb-draw-blt.html

  
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#105541]: https://bugs.freedesktop.org/show_bug.cgi?id=105541
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#108040]: https://bugs.freedesktop.org/show_bug.cgi?id=108040
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#109661]: https://bugs.freedesktop.org/show_bug.cgi?id=109661
  [fdo#110633 ]: https://bugs.freedesktop.org/show_bug.cgi?id=110633 
  [fdo#110728]: https://bugs.freedesktop.org/show_bug.cgi?id=110728
  [fdo#110841]: https://bugs.freedesktop.org/show_bug.cgi?id=110841
  [fdo#110854]: https://bugs.freedesktop.org/show_bug.cgi?id=110854
  [fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
  [fdo#111325]: https://bugs.freedesktop.org/show_bug.cgi?id=111325
  [fdo#111330]: https://bugs.freedesktop.org/show_bug.cgi?id=111330
  [fdo#111409]: https://bugs.freedesktop.org/show_bug.cgi?id=111409


Participating hosts (10 -> 10)
------------------------------

  No changes in participating hosts


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_6750 -> Patchwork_14110

  CI-20190529: 20190529
  CI_DRM_6750: ba37f74dbdc1e78e70a5a2e5f4ce8d762d06eaa7 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5146: 357dbe1869d88a2f08bcee4eebceff4ee9014424 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_14110: 5281445913b8df5c789c9ec66dcfb0e0576d3881 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: disable set/get_tiling ioctl on gen12+

[Souza, Jose]

On Thu, 2019-08-29 at 08:50 +0200, Daniel Vetter wrote:
> On Wed, Aug 28, 2019 at 08:31:27PM +0000, Souza, Jose wrote:
> > On Wed, 2019-08-28 at 21:13 +0100, Chris Wilson wrote:
> > > Quoting Souza, Jose (2019-08-28 21:11:53)
> > > > Reviewed-by: José Roberto de Souza <jose.souza@intel.com>
> > > 
> > > It's using a non-standard for i915 error code, and get_tiling is
> > > not
> > 
> > Huum should it use ENOTSUPP then?!
> 
> https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#recommended-ioctl-return-values
> 
> Per above "feature not supported" -> EOPNOTSUPP.

But like Chris said we are not using EOPNOTSUPP in i915,
i915_perf_open_ioctl() and other 2 perf ioctl uses ENOSUPP, should we
convert those to EOPNOTSUPP?

> 
> > > affected, it will always return LINEAR. You cannot set tiling as 
> > 
> > Following this set_tiling() LINEAR should be allowed too.
> > I prefer the current approach of returning error.
> 
> I'm not seeing the value in keeping get_tiling supported. Either
> userspace
> still uses the legacy backhannel and dri2, in which case it needs to
> be
> fixed no matter what. Or it's using modifiers, in which case this is
> dead
> code. Only other user I can think of is takeover for fastboot, but if
> you
> get anything else than untiled it's also broken (we don't have an
> ioctl to
> read out the modifiers, heck even all the planes, there's no getfb2).
> 
> So really not seeing the point in keeping that working.
> -Daniel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: disable set/get_tiling ioctl on gen12+

[Daniel Vetter]

On Tue, Sep 3, 2019 at 9:21 PM Souza, Jose <jose.souza@intel.com> wrote:
>
> On Thu, 2019-08-29 at 08:50 +0200, Daniel Vetter wrote:
> > On Wed, Aug 28, 2019 at 08:31:27PM +0000, Souza, Jose wrote:
> > > On Wed, 2019-08-28 at 21:13 +0100, Chris Wilson wrote:
> > > > Quoting Souza, Jose (2019-08-28 21:11:53)
> > > > > Reviewed-by: José Roberto de Souza <jose.souza@intel.com>
> > > >
> > > > It's using a non-standard for i915 error code, and get_tiling is
> > > > not
> > >
> > > Huum should it use ENOTSUPP then?!
> >
> > https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#recommended-ioctl-return-values
> >
> > Per above "feature not supported" -> EOPNOTSUPP.
>
> But like Chris said we are not using EOPNOTSUPP in i915,
> i915_perf_open_ioctl() and other 2 perf ioctl uses ENOSUPP, should we
> convert those to EOPNOTSUPP?

$ git grep EOPNOTSUP -- drivers/gpu/drm/ | wc -l
114
$ git grep ENOTSUP -- drivers/gpu/drm/ | wc -l
32

Plus i915_pmu.c also has a use of EOPNOTSUPP already.

Furthermore the header for EOPNOTSUP has a pretty clear comment:

/* Defined for the NFSv3 protocol */

Above the entore block of error codes containing ENOTSUPP.

So given all that, plus that we've decided to go with EOPNOTSUPP as
the drm-wide recommendation: EOPNOTSUPP it is.

If you disagree, I think there's a pretty substantial patch series for
you to type and fix the docs and most users plus explain why we should
use an nsf-specific error code (which isn't much worse than the
abuse/reinterpretation we currently do, but still I think it's a bit
more bending of errno code intentions).

Cheers, Daniel



>
> >
> > > > affected, it will always return LINEAR. You cannot set tiling as
> > >
> > > Following this set_tiling() LINEAR should be allowed too.
> > > I prefer the current approach of returning error.
> >
> > I'm not seeing the value in keeping get_tiling supported. Either
> > userspace
> > still uses the legacy backhannel and dri2, in which case it needs to
> > be
> > fixed no matter what. Or it's using modifiers, in which case this is
> > dead
> > code. Only other user I can think of is takeover for fastboot, but if
> > you
> > get anything else than untiled it's also broken (we don't have an
> > ioctl to
> > read out the modifiers, heck even all the planes, there's no getfb2).
> >
> > So really not seeing the point in keeping that working.
> > -Daniel



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: disable set/get_tiling ioctl on gen12+

[Daniel Vetter]

On Wed, Sep 4, 2019 at 4:29 PM Daniel Vetter <daniel@ffwll.ch> wrote:
>
> On Tue, Sep 3, 2019 at 9:21 PM Souza, Jose <jose.souza@intel.com> wrote:
> >
> > On Thu, 2019-08-29 at 08:50 +0200, Daniel Vetter wrote:
> > > On Wed, Aug 28, 2019 at 08:31:27PM +0000, Souza, Jose wrote:
> > > > On Wed, 2019-08-28 at 21:13 +0100, Chris Wilson wrote:
> > > > > Quoting Souza, Jose (2019-08-28 21:11:53)
> > > > > > Reviewed-by: José Roberto de Souza <jose.souza@intel.com>
> > > > >
> > > > > It's using a non-standard for i915 error code, and get_tiling is
> > > > > not
> > > >
> > > > Huum should it use ENOTSUPP then?!
> > >
> > > https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#recommended-ioctl-return-values
> > >
> > > Per above "feature not supported" -> EOPNOTSUPP.
> >
> > But like Chris said we are not using EOPNOTSUPP in i915,
> > i915_perf_open_ioctl() and other 2 perf ioctl uses ENOSUPP, should we
> > convert those to EOPNOTSUPP?
>
> $ git grep EOPNOTSUP -- drivers/gpu/drm/ | wc -l
> 114
> $ git grep ENOTSUP -- drivers/gpu/drm/ | wc -l
> 32

Note that most of the ENOTSUP is in drivers, for the drm core it's
even more clear:

$ git grep EOPNOTSUP -- drivers/gpu/drm/*c | wc -l
83
$ git grep ENOTSUP -- drivers/gpu/drm/*c | wc -l
5

Cheers, Daniel

> Plus i915_pmu.c also has a use of EOPNOTSUPP already.
>
> Furthermore the header for EOPNOTSUP has a pretty clear comment:
>
> /* Defined for the NFSv3 protocol */
>
> Above the entore block of error codes containing ENOTSUPP.
>
> So given all that, plus that we've decided to go with EOPNOTSUPP as
> the drm-wide recommendation: EOPNOTSUPP it is.
>
> If you disagree, I think there's a pretty substantial patch series for
> you to type and fix the docs and most users plus explain why we should
> use an nsf-specific error code (which isn't much worse than the
> abuse/reinterpretation we currently do, but still I think it's a bit
> more bending of errno code intentions).
>
> Cheers, Daniel
>
>
>
> >
> > >
> > > > > affected, it will always return LINEAR. You cannot set tiling as
> > > >
> > > > Following this set_tiling() LINEAR should be allowed too.
> > > > I prefer the current approach of returning error.
> > >
> > > I'm not seeing the value in keeping get_tiling supported. Either
> > > userspace
> > > still uses the legacy backhannel and dri2, in which case it needs to
> > > be
> > > fixed no matter what. Or it's using modifiers, in which case this is
> > > dead
> > > code. Only other user I can think of is takeover for fastboot, but if
> > > you
> > > get anything else than untiled it's also broken (we don't have an
> > > ioctl to
> > > read out the modifiers, heck even all the planes, there's no getfb2).
> > >
> > > So really not seeing the point in keeping that working.
> > > -Daniel
>
>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: disable set/get_tiling ioctl on gen12+

[Souza, Jose]

On Wed, 2019-09-04 at 16:31 +0200, Daniel Vetter wrote:
> On Wed, Sep 4, 2019 at 4:29 PM Daniel Vetter <daniel@ffwll.ch> wrote:
> > On Tue, Sep 3, 2019 at 9:21 PM Souza, Jose <jose.souza@intel.com>
> > wrote:
> > > On Thu, 2019-08-29 at 08:50 +0200, Daniel Vetter wrote:
> > > > On Wed, Aug 28, 2019 at 08:31:27PM +0000, Souza, Jose wrote:
> > > > > On Wed, 2019-08-28 at 21:13 +0100, Chris Wilson wrote:
> > > > > > Quoting Souza, Jose (2019-08-28 21:11:53)
> > > > > > > Reviewed-by: José Roberto de Souza <jose.souza@intel.com>
> > > > > > 
> > > > > > It's using a non-standard for i915 error code, and
> > > > > > get_tiling is
> > > > > > not
> > > > > 
> > > > > Huum should it use ENOTSUPP then?!
> > > > 
> > > > https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#recommended-ioctl-return-values
> > > > 
> > > > Per above "feature not supported" -> EOPNOTSUPP.
> > > 
> > > But like Chris said we are not using EOPNOTSUPP in i915,
> > > i915_perf_open_ioctl() and other 2 perf ioctl uses ENOSUPP,
> > > should we
> > > convert those to EOPNOTSUPP?
> > 
> > $ git grep EOPNOTSUP -- drivers/gpu/drm/ | wc -l
> > 114
> > $ git grep ENOTSUP -- drivers/gpu/drm/ | wc -l
> > 32
> 
> Note that most of the ENOTSUP is in drivers, for the drm core it's
> even more clear:
> 
> $ git grep EOPNOTSUP -- drivers/gpu/drm/*c | wc -l
> 83
> $ git grep ENOTSUP -- drivers/gpu/drm/*c | wc -l
> 5
> 
> Cheers, Daniel
> 
> > Plus i915_pmu.c also has a use of EOPNOTSUPP already.
> > 
> > Furthermore the header for EOPNOTSUP has a pretty clear comment:
> > 
> > /* Defined for the NFSv3 protocol */
> > 
> > Above the entore block of error codes containing ENOTSUPP.
> > 
> > So given all that, plus that we've decided to go with EOPNOTSUPP as
> > the drm-wide recommendation: EOPNOTSUPP it is.

Reviewed-by: José Roberto de Souza <jose.souza@intel.com>

> > 
> > If you disagree, I think there's a pretty substantial patch series
> > for
> > you to type and fix the docs and most users plus explain why we
> > should
> > use an nsf-specific error code (which isn't much worse than the
> > abuse/reinterpretation we currently do, but still I think it's a
> > bit
> > more bending of errno code intentions).
> > 
> > Cheers, Daniel
> > 
> > 
> > 
> > > > > > affected, it will always return LINEAR. You cannot set
> > > > > > tiling as
> > > > > 
> > > > > Following this set_tiling() LINEAR should be allowed too.
> > > > > I prefer the current approach of returning error.
> > > > 
> > > > I'm not seeing the value in keeping get_tiling supported.
> > > > Either
> > > > userspace
> > > > still uses the legacy backhannel and dri2, in which case it
> > > > needs to
> > > > be
> > > > fixed no matter what. Or it's using modifiers, in which case
> > > > this is
> > > > dead
> > > > code. Only other user I can think of is takeover for fastboot,
> > > > but if
> > > > you
> > > > get anything else than untiled it's also broken (we don't have
> > > > an
> > > > ioctl to
> > > > read out the modifiers, heck even all the planes, there's no
> > > > getfb2).
> > > > 
> > > > So really not seeing the point in keeping that working.
> > > > -Daniel
> > 
> > 
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> 
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✓ Fi.CI.IGT: success for drm/i915: disable set/get_tiling ioctl on gen12+ (rev2)

[Souza, Jose]

On Thu, 2019-08-29 at 14:18 +0000, Patchwork wrote:
> == Series Details ==
> 
> Series: drm/i915: disable set/get_tiling ioctl on gen12+ (rev2)
> URL   : https://patchwork.freedesktop.org/series/65495/
> State : success
> 
> == Summary ==
> 
> CI Bug Log - changes from CI_DRM_6750_full -> Patchwork_14110_full
> ====================================================
> 
> Summary
> -------
> 
>   **SUCCESS**
> 
>   No regressions found.

And pushed to dinq, thanks for the patch.

> 
>   
> 
> Known issues
> ------------
> 
>   Here are the changes found in Patchwork_14110_full that come from
> known issues:
> 
> ### IGT changes ###
> 
> #### Issues hit ####
> 
>   * igt@gem_ctx_shared@exec-single-timeline-bsd:
>     - shard-iclb:         [PASS][1] -> [SKIP][2] ([fdo#110841])
>    [1]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb8/igt@gem_ctx_shared@exec-single-timeline-bsd.html
>    [2]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-iclb1/igt@gem_ctx_shared@exec-single-timeline-bsd.html
> 
>   * igt@gem_exec_async@concurrent-writes-bsd:
>     - shard-iclb:         [PASS][3] -> [SKIP][4] ([fdo#111325]) +4
> similar issues
>    [3]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb3/igt@gem_exec_async@concurrent-writes-bsd.html
>    [4]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-iclb4/igt@gem_exec_async@concurrent-writes-bsd.html
> 
>   * igt@gem_softpin@noreloc-s3:
>     - shard-skl:          [PASS][5] -> [INCOMPLETE][6] ([fdo#104108])
>    [5]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-skl9/igt@gem_softpin@noreloc-s3.html
>    [6]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-skl1/igt@gem_softpin@noreloc-s3.html
> 
>   * igt@i915_selftest@live_gtt:
>     - shard-glk:          [PASS][7] -> [DMESG-WARN][8] ([fdo#110633
> ])
>    [7]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-glk1/igt@i915_selftest@live_gtt.html
>    [8]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-glk5/igt@i915_selftest@live_gtt.html
> 
>   * igt@kms_cursor_legacy@long-nonblocking-modeset-vs-cursor-atomic:
>     - shard-skl:          [PASS][9] -> [DMESG-WARN][10]
> ([fdo#105541])
>    [9]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-skl4/igt@kms_cursor_legacy@long-nonblocking-modeset-vs-cursor-atomic.html
>    [10]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-skl6/igt@kms_cursor_legacy@long-nonblocking-modeset-vs-cursor-atomic.html
> 
>   * igt@kms_plane_lowres@pipe-a-tiling-y:
>     - shard-iclb:         [PASS][11] -> [FAIL][12] ([fdo#103166])
>    [11]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb3/igt@kms_plane_lowres@pipe-a-tiling-y.html
>    [12]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-iclb5/igt@kms_plane_lowres@pipe-a-tiling-y.html
> 
>   * igt@kms_psr2_su@frontbuffer:
>     - shard-iclb:         [PASS][13] -> [SKIP][14] ([fdo#109642] /
> [fdo#111068])
>    [13]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb2/igt@kms_psr2_su@frontbuffer.html
>    [14]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-iclb7/igt@kms_psr2_su@frontbuffer.html
> 
>   * igt@kms_psr@psr2_cursor_mmap_cpu:
>     - shard-iclb:         [PASS][15] -> [SKIP][16] ([fdo#109441]) +3
> similar issues
>    [15]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb2/igt@kms_psr@psr2_cursor_mmap_cpu.html
>    [16]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-iclb8/igt@kms_psr@psr2_cursor_mmap_cpu.html
> 
>   * igt@kms_vblank@pipe-a-ts-continuation-suspend:
>     - shard-apl:          [PASS][17] -> [DMESG-WARN][18]
> ([fdo#108566]) +2 similar issues
>    [17]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-apl3/igt@kms_vblank@pipe-a-ts-continuation-suspend.html
>    [18]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-apl7/igt@kms_vblank@pipe-a-ts-continuation-suspend.html
> 
>   * igt@perf_pmu@rc6:
>     - shard-kbl:          [PASS][19] -> [SKIP][20] ([fdo#109271])
>    [19]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-kbl4/igt@perf_pmu@rc6.html
>    [20]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-kbl7/igt@perf_pmu@rc6.html
> 
>   * igt@prime_busy@hang-bsd2:
>     - shard-iclb:         [PASS][21] -> [SKIP][22] ([fdo#109276]) +24
> similar issues
>    [21]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb4/igt@prime_busy@hang-bsd2.html
>    [22]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-iclb3/igt@prime_busy@hang-bsd2.html
> 
>   
> #### Possible fixes ####
> 
>   * igt@gem_eio@reset-stress:
>     - shard-skl:          [FAIL][23] ([fdo#109661]) -> [PASS][24]
>    [23]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-skl5/igt@gem_eio@reset-stress.html
>    [24]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-skl10/igt@gem_eio@reset-stress.html
> 
>   * igt@gem_exec_balancer@smoke:
>     - shard-iclb:         [SKIP][25] ([fdo#110854]) -> [PASS][26]
>    [25]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb8/igt@gem_exec_balancer@smoke.html
>    [26]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-iclb1/igt@gem_exec_balancer@smoke.html
> 
>   * igt@gem_exec_schedule@wide-bsd:
>     - shard-iclb:         [SKIP][27] ([fdo#111325]) -> [PASS][28] +7
> similar issues
>    [27]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb2/igt@gem_exec_schedule@wide-bsd.html
>    [28]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-iclb8/igt@gem_exec_schedule@wide-bsd.html
> 
>   * igt@i915_pm_rps@min-max-config-loaded:
>     - shard-iclb:         [FAIL][29] ([fdo#111409]) -> [PASS][30]
>    [29]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb7/igt@i915_pm_rps@min-max-config-loaded.html
>    [30]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-iclb5/igt@i915_pm_rps@min-max-config-loaded.html
> 
>   * igt@i915_suspend@sysfs-reader:
>     - shard-apl:          [DMESG-WARN][31] ([fdo#108566]) ->
> [PASS][32] +6 similar issues
>    [31]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-apl7/igt@i915_suspend@sysfs-reader.html
>    [32]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-apl4/igt@i915_suspend@sysfs-reader.html
> 
>   * igt@kms_cursor_crc@pipe-c-cursor-256x85-sliding:
>     - shard-iclb:         [INCOMPLETE][33] ([fdo#107713]) ->
> [PASS][34]
>    [33]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb7/igt@kms_cursor_crc@pipe-c-cursor-256x85-sliding.html
>    [34]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-iclb2/igt@kms_cursor_crc@pipe-c-cursor-256x85-sliding.html
> 
>   * igt@kms_cursor_legacy@pipe-c-single-move:
>     - shard-apl:          [INCOMPLETE][35] ([fdo#103927]) ->
> [PASS][36] +2 similar issues
>    [35]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-apl1/igt@kms_cursor_legacy@pipe-c-single-move.html
>    [36]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-apl2/igt@kms_cursor_legacy@pipe-c-single-move.html
> 
>   * igt@kms_fbcon_fbt@psr-suspend:
>     - shard-skl:          [INCOMPLETE][37] ([fdo#104108]) ->
> [PASS][38]
>    [37]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-skl10/igt@kms_fbcon_fbt@psr-suspend.html
>    [38]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-skl1/igt@kms_fbcon_fbt@psr-suspend.html
> 
>   * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-shrfb-plflip-blt:
>     - shard-iclb:         [FAIL][39] ([fdo#103167]) -> [PASS][40] +3
> similar issues
>    [39]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb4/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-shrfb-plflip-blt.html
>    [40]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-iclb2/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-shrfb-plflip-blt.html
> 
>   * igt@kms_psr@psr2_sprite_mmap_gtt:
>     - shard-iclb:         [SKIP][41] ([fdo#109441]) -> [PASS][42] +3
> similar issues
>    [41]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb4/igt@kms_psr@psr2_sprite_mmap_gtt.html
>    [42]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-iclb2/igt@kms_psr@psr2_sprite_mmap_gtt.html
> 
>   * igt@perf@blocking:
>     - shard-skl:          [FAIL][43] ([fdo#110728]) -> [PASS][44]
>    [43]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-skl10/igt@perf@blocking.html
>    [44]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-skl7/igt@perf@blocking.html
> 
>   * igt@prime_vgem@fence-wait-bsd2:
>     - shard-iclb:         [SKIP][45] ([fdo#109276]) -> [PASS][46] +16
> similar issues
>    [45]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb7/igt@prime_vgem@fence-wait-bsd2.html
>    [46]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-iclb2/igt@prime_vgem@fence-wait-bsd2.html
> 
>   
> #### Warnings ####
> 
>   * igt@gem_mocs_settings@mocs-reset-bsd2:
>     - shard-iclb:         [FAIL][47] ([fdo#111330]) -> [SKIP][48]
> ([fdo#109276]) +2 similar issues
>    [47]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb2/igt@gem_mocs_settings@mocs-reset-bsd2.html
>    [48]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-iclb8/igt@gem_mocs_settings@mocs-reset-bsd2.html
> 
>   * igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-indfb-draw-blt:
>     - shard-skl:          [FAIL][49] ([fdo#108040]) -> [FAIL][50]
> ([fdo#103167])
>    [49]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-skl7/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-indfb-draw-blt.html
>    [50]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/shard-skl10/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-indfb-draw-blt.html
> 
>   
>   [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
>   [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
>   [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
>   [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
>   [fdo#105541]: https://bugs.freedesktop.org/show_bug.cgi?id=105541
>   [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
>   [fdo#108040]: https://bugs.freedesktop.org/show_bug.cgi?id=108040
>   [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
>   [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
>   [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
>   [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
>   [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
>   [fdo#109661]: https://bugs.freedesktop.org/show_bug.cgi?id=109661
>   [fdo#110633 ]: https://bugs.freedesktop.org/show_bug.cgi?id=110633 
>   [fdo#110728]: https://bugs.freedesktop.org/show_bug.cgi?id=110728
>   [fdo#110841]: https://bugs.freedesktop.org/show_bug.cgi?id=110841
>   [fdo#110854]: https://bugs.freedesktop.org/show_bug.cgi?id=110854
>   [fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
>   [fdo#111325]: https://bugs.freedesktop.org/show_bug.cgi?id=111325
>   [fdo#111330]: https://bugs.freedesktop.org/show_bug.cgi?id=111330
>   [fdo#111409]: https://bugs.freedesktop.org/show_bug.cgi?id=111409
> 
> 
> Participating hosts (10 -> 10)
> ------------------------------
> 
>   No changes in participating hosts
> 
> 
> Build changes
> -------------
> 
>   * CI: CI-20190529 -> None
>   * Linux: CI_DRM_6750 -> Patchwork_14110
> 
>   CI-20190529: 20190529
>   CI_DRM_6750: ba37f74dbdc1e78e70a5a2e5f4ce8d762d06eaa7 @
> git://anongit.freedesktop.org/gfx-ci/linux
>   IGT_5146: 357dbe1869d88a2f08bcee4eebceff4ee9014424 @
> git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
>   Patchwork_14110: 5281445913b8df5c789c9ec66dcfb0e0576d3881 @
> git://anongit.freedesktop.org/gfx-ci/linux
>   piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @
> git://anongit.freedesktop.org/piglit
> 
> == Logs ==
> 
> For more details see: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14110/
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: disable set/get_tiling ioctl on gen12+

[Daniele Ceraolo Spurio]

On 8/21/19 8:20 AM, Daniel Vetter wrote:
> On Wed, Aug 21, 2019 at 3:55 PM Ville Syrjälä
> <ville.syrjala@linux.intel.com> wrote:
>>
>> On Tue, Aug 20, 2019 at 01:57:44PM -0700, Daniele Ceraolo Spurio wrote:
>>>
>>>
>>> On 8/20/19 12:54 PM, Daniel Vetter wrote:
>>>> The cpu (de)tiler hw is gone, this stopped being useful. Plus it never
>>>> supported any of the fancy new tiling formats, which means userspace
>>>> also stopped using the magic side-channel this provides.
>>>>
>>>> This would totally break a lot of the igts, but they're already broken
>>>> for the same reasons as userspace on gen12 would be.
>>>>
>>>> v2: Look at ggtt->num_fences instead, that also avoids the need for a
>>>> comment (Chris). This also means that gen12 support really needs to
>>>> make sure num_fences is set to 0. There is a patch for that, but it
>>>> checks for HAS_MAPPABLE_APERTURE, which I'm not sure is the right
>>>> thing really. Adding relevant people.
>>>>
>>>
>>> We'd obviously need to make that setting for all gen12+, because TGL
>>> does have mappable aperture.
>>>
>>> Apart from the tiling ioctl, the only place I see where we set tiling is
>>> intel_alloc_initial_plane_obj(), can the users of that object handle the
>>> lack of fences gracefully?
>>
>> Gen4+ display engine has its own tiling controls and doesn't care about
>> fences. So we should be able to leave the obj tiling set to NONE.
>>
>> Hmm. Actually I think we should reject tiled framebuffers in the BIOS
>> fb takeover because fbdev needs a linear view for the memory. No can
>> do without the fence.
> 
> Yeah I think this is just more fallout from "no more fences in the hw".
> -Daniel
> 

Is anyone looking at implementing this (fence = 0 and reject tiled FBs) 
for TGL? I can pick it up if no one is.

Daniele

>>
>>> When I wrote the num_fences=0 patch I was
>>> expecting display to be unavailable, so I didn't really look at that
>>> part of the code.
>>>
>>> It'd also be nice to be more explicit with fencing since we seem to
>>> often call i915_vma_pin_iomap, which implicitly applies a fence if
>>> needed, on objects that can't be tiled or have had a fence assigned a
>>> few lines before. This is more a nice to have tough, possibly together
>>> with a split of the "mappable" and "fenceable" attributes of the vma.
>>>
>>> Daniele
>>>
>>>> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>>>> Cc: Stuart Summers <stuart.summers@intel.com>
>>>> Cc: Matthew Auld <matthew.auld@intel.com>
>>>> Cc: Kenneth Graunke <kenneth@whitecape.org>
>>>> Cc: Jason Ekstrand <jason@jlekstrand.net>
>>>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>>>> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>>>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
>>>> ---
>>>>    drivers/gpu/drm/i915/gem/i915_gem_tiling.c | 7 +++++++
>>>>    1 file changed, 7 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_tiling.c b/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
>>>> index ca0c2f451742..e5d1ae8d4dba 100644
>>>> --- a/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
>>>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
>>>> @@ -313,10 +313,14 @@ int
>>>>    i915_gem_set_tiling_ioctl(struct drm_device *dev, void *data,
>>>>                        struct drm_file *file)
>>>>    {
>>>> +   struct drm_i915_private *dev_priv = to_i915(dev);
>>>>      struct drm_i915_gem_set_tiling *args = data;
>>>>      struct drm_i915_gem_object *obj;
>>>>      int err;
>>>>
>>>> +   if (!dev_priv->ggtt.num_fences)
>>>> +           return -EOPNOTSUPP;
>>>> +
>>>>      obj = i915_gem_object_lookup(file, args->handle);
>>>>      if (!obj)
>>>>              return -ENOENT;
>>>> @@ -402,6 +406,9 @@ i915_gem_get_tiling_ioctl(struct drm_device *dev, void *data,
>>>>      struct drm_i915_gem_object *obj;
>>>>      int err = -ENOENT;
>>>>
>>>> +   if (!dev_priv->ggtt.num_fences)
>>>> +           return -EOPNOTSUPP;
>>>> +
>>>>      rcu_read_lock();
>>>>      obj = i915_gem_object_lookup_rcu(file, args->handle);
>>>>      if (obj) {
>>>>
>>> _______________________________________________
>>> Intel-gfx mailing list
>>> Intel-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>
>> --
>> Ville Syrjälä
>> Intel
> 
> 
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: disable set/get_tiling ioctl on gen12+

[Jason Ekstrand]

[-- Attachment #1.1: Type: text/plain, Size: 2330 bytes --]

While I'm generally a fan of this change, we've been talking on IRC a bit
today and, apparently, the X server hasn't actually had a release where
modifiers have been enabled by default so this is causing problems.  Adam &
Daniel, is there something that's preventing us from enabling it by
default?  This is kind-of important these days.

--Jason


On Tue, Aug 20, 2019 at 12:06 PM Daniel Vetter <daniel.vetter@ffwll.ch>
wrote:

> The cpu (de)tiler hw is gone, this stopped being useful. Plus it never
> supported any of the fancy new tiling formats, which means userspace
> also stopped using the magic side-channel this provides.
>
> This would totally break a lot of the igts, but they're already broken
> for the same reasons as userspace on gen12 would be.
>
> Cc: Kenneth Graunke <kenneth@whitecape.org>
> Cc: Jason Ekstrand <jason@jlekstrand.net>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/i915/gem/i915_gem_tiling.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
> b/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
> index ca0c2f451742..2ee96f27a294 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
> @@ -313,10 +313,15 @@ int
>  i915_gem_set_tiling_ioctl(struct drm_device *dev, void *data,
>                           struct drm_file *file)
>  {
> +       struct drm_i915_private *dev_priv = to_i915(dev);
>         struct drm_i915_gem_set_tiling *args = data;
>         struct drm_i915_gem_object *obj;
>         int err;
>
> +       /* there is no cpu (de)tiling in the hw anymore on gen12+ */
> +       if (INTEL_GEN(dev_priv) >= 12)
> +               return -EOPNOTSUPP;
> +
>         obj = i915_gem_object_lookup(file, args->handle);
>         if (!obj)
>                 return -ENOENT;
> @@ -402,6 +407,9 @@ i915_gem_get_tiling_ioctl(struct drm_device *dev, void
> *data,
>         struct drm_i915_gem_object *obj;
>         int err = -ENOENT;
>
> +       if (INTEL_GEN(dev_priv) >= 12)
> +               return -EOPNOTSUPP;
> +
>         rcu_read_lock();
>         obj = i915_gem_object_lookup_rcu(file, args->handle);
>         if (obj) {
> --
> 2.23.0.rc1
>
>

[-- Attachment #1.2: Type: text/html, Size: 3276 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: disable set/get_tiling ioctl on gen12+

[Jason Ekstrand]

[-- Attachment #1.1: Type: text/plain, Size: 2330 bytes --]

While I'm generally a fan of this change, we've been talking on IRC a bit
today and, apparently, the X server hasn't actually had a release where
modifiers have been enabled by default so this is causing problems.  Adam &
Daniel, is there something that's preventing us from enabling it by
default?  This is kind-of important these days.

--Jason


On Tue, Aug 20, 2019 at 12:06 PM Daniel Vetter <daniel.vetter@ffwll.ch>
wrote:

> The cpu (de)tiler hw is gone, this stopped being useful. Plus it never
> supported any of the fancy new tiling formats, which means userspace
> also stopped using the magic side-channel this provides.
>
> This would totally break a lot of the igts, but they're already broken
> for the same reasons as userspace on gen12 would be.
>
> Cc: Kenneth Graunke <kenneth@whitecape.org>
> Cc: Jason Ekstrand <jason@jlekstrand.net>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/i915/gem/i915_gem_tiling.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
> b/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
> index ca0c2f451742..2ee96f27a294 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
> @@ -313,10 +313,15 @@ int
>  i915_gem_set_tiling_ioctl(struct drm_device *dev, void *data,
>                           struct drm_file *file)
>  {
> +       struct drm_i915_private *dev_priv = to_i915(dev);
>         struct drm_i915_gem_set_tiling *args = data;
>         struct drm_i915_gem_object *obj;
>         int err;
>
> +       /* there is no cpu (de)tiling in the hw anymore on gen12+ */
> +       if (INTEL_GEN(dev_priv) >= 12)
> +               return -EOPNOTSUPP;
> +
>         obj = i915_gem_object_lookup(file, args->handle);
>         if (!obj)
>                 return -ENOENT;
> @@ -402,6 +407,9 @@ i915_gem_get_tiling_ioctl(struct drm_device *dev, void
> *data,
>         struct drm_i915_gem_object *obj;
>         int err = -ENOENT;
>
> +       if (INTEL_GEN(dev_priv) >= 12)
> +               return -EOPNOTSUPP;
> +
>         rcu_read_lock();
>         obj = i915_gem_object_lookup_rcu(file, args->handle);
>         if (obj) {
> --
> 2.23.0.rc1
>
>

[-- Attachment #1.2: Type: text/html, Size: 3276 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: disable set/get_tiling ioctl on gen12+

[Brian Welty]

On 8/28/2019 11:50 PM, Daniel Vetter wrote:
> On Wed, Aug 28, 2019 at 08:31:27PM +0000, Souza, Jose wrote:
>> On Wed, 2019-08-28 at 21:13 +0100, Chris Wilson wrote:
>>> Quoting Souza, Jose (2019-08-28 21:11:53)
>>>> Reviewed-by: José Roberto de Souza <jose.souza@intel.com>
>>>
>>> It's using a non-standard for i915 error code, and get_tiling is not
>>
>> Huum should it use ENOTSUPP then?!
> 
> https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#recommended-ioctl-return-values
> 
> Per above "feature not supported" -> EOPNOTSUPP.
> 
>>> affected, it will always return LINEAR. You cannot set tiling as 
>>
>> Following this set_tiling() LINEAR should be allowed too.
>> I prefer the current approach of returning error.
> 
> I'm not seeing the value in keeping get_tiling supported. Either userspace
> still uses the legacy backhannel and dri2, in which case it needs to be
> fixed no matter what. Or it's using modifiers, in which case this is dead
> code. Only other user I can think of is takeover for fastboot, but if you
> get anything else than untiled it's also broken (we don't have an ioctl to
> read out the modifiers, heck even all the planes, there's no getfb2).
> 
> So really not seeing the point in keeping that working.

Daniel,  I came across usage of GET_TILING in libdrm.
Is used in drm_intel_bo_gem_create_from_name() and drm_intel_bo_gem_create_from_prime().
Should these be updated so they don't fail when EOPNOTSUPP is returned on gen12+?
Maybe libdrm should just set tiling_mode to 0 on EOPNOTSUPP error instead of those calls failing?

-Brian

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: disable set/get_tiling ioctl on gen12+

[Brian Welty]

On 8/28/2019 11:50 PM, Daniel Vetter wrote:
> On Wed, Aug 28, 2019 at 08:31:27PM +0000, Souza, Jose wrote:
>> On Wed, 2019-08-28 at 21:13 +0100, Chris Wilson wrote:
>>> Quoting Souza, Jose (2019-08-28 21:11:53)
>>>> Reviewed-by: José Roberto de Souza <jose.souza@intel.com>
>>>
>>> It's using a non-standard for i915 error code, and get_tiling is not
>>
>> Huum should it use ENOTSUPP then?!
> 
> https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#recommended-ioctl-return-values
> 
> Per above "feature not supported" -> EOPNOTSUPP.
> 
>>> affected, it will always return LINEAR. You cannot set tiling as 
>>
>> Following this set_tiling() LINEAR should be allowed too.
>> I prefer the current approach of returning error.
> 
> I'm not seeing the value in keeping get_tiling supported. Either userspace
> still uses the legacy backhannel and dri2, in which case it needs to be
> fixed no matter what. Or it's using modifiers, in which case this is dead
> code. Only other user I can think of is takeover for fastboot, but if you
> get anything else than untiled it's also broken (we don't have an ioctl to
> read out the modifiers, heck even all the planes, there's no getfb2).
> 
> So really not seeing the point in keeping that working.

Daniel,  I came across usage of GET_TILING in libdrm.
Is used in drm_intel_bo_gem_create_from_name() and drm_intel_bo_gem_create_from_prime().
Should these be updated so they don't fail when EOPNOTSUPP is returned on gen12+?
Maybe libdrm should just set tiling_mode to 0 on EOPNOTSUPP error instead of those calls failing?

-Brian

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: disable set/get_tiling ioctl on gen12+

[Daniel Vetter]

On Fri, Nov 8, 2019 at 12:07 AM Brian Welty <brian.welty@intel.com> wrote:
>
>
>
> On 8/28/2019 11:50 PM, Daniel Vetter wrote:
> > On Wed, Aug 28, 2019 at 08:31:27PM +0000, Souza, Jose wrote:
> >> On Wed, 2019-08-28 at 21:13 +0100, Chris Wilson wrote:
> >>> Quoting Souza, Jose (2019-08-28 21:11:53)
> >>>> Reviewed-by: José Roberto de Souza <jose.souza@intel.com>
> >>>
> >>> It's using a non-standard for i915 error code, and get_tiling is not
> >>
> >> Huum should it use ENOTSUPP then?!
> >
> > https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#recommended-ioctl-return-values
> >
> > Per above "feature not supported" -> EOPNOTSUPP.
> >
> >>> affected, it will always return LINEAR. You cannot set tiling as
> >>
> >> Following this set_tiling() LINEAR should be allowed too.
> >> I prefer the current approach of returning error.
> >
> > I'm not seeing the value in keeping get_tiling supported. Either userspace
> > still uses the legacy backhannel and dri2, in which case it needs to be
> > fixed no matter what. Or it's using modifiers, in which case this is dead
> > code. Only other user I can think of is takeover for fastboot, but if you
> > get anything else than untiled it's also broken (we don't have an ioctl to
> > read out the modifiers, heck even all the planes, there's no getfb2).
> >
> > So really not seeing the point in keeping that working.
>
> Daniel,  I came across usage of GET_TILING in libdrm.
> Is used in drm_intel_bo_gem_create_from_name() and drm_intel_bo_gem_create_from_prime().
> Should these be updated so they don't fail when EOPNOTSUPP is returned on gen12+?
> Maybe libdrm should just set tiling_mode to 0 on EOPNOTSUPP error instead of those calls failing?

Yes, I guess that part of mesa hasn't seen much testing yet.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: disable set/get_tiling ioctl on gen12+

[Daniel Vetter]

On Fri, Nov 8, 2019 at 12:07 AM Brian Welty <brian.welty@intel.com> wrote:
>
>
>
> On 8/28/2019 11:50 PM, Daniel Vetter wrote:
> > On Wed, Aug 28, 2019 at 08:31:27PM +0000, Souza, Jose wrote:
> >> On Wed, 2019-08-28 at 21:13 +0100, Chris Wilson wrote:
> >>> Quoting Souza, Jose (2019-08-28 21:11:53)
> >>>> Reviewed-by: José Roberto de Souza <jose.souza@intel.com>
> >>>
> >>> It's using a non-standard for i915 error code, and get_tiling is not
> >>
> >> Huum should it use ENOTSUPP then?!
> >
> > https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#recommended-ioctl-return-values
> >
> > Per above "feature not supported" -> EOPNOTSUPP.
> >
> >>> affected, it will always return LINEAR. You cannot set tiling as
> >>
> >> Following this set_tiling() LINEAR should be allowed too.
> >> I prefer the current approach of returning error.
> >
> > I'm not seeing the value in keeping get_tiling supported. Either userspace
> > still uses the legacy backhannel and dri2, in which case it needs to be
> > fixed no matter what. Or it's using modifiers, in which case this is dead
> > code. Only other user I can think of is takeover for fastboot, but if you
> > get anything else than untiled it's also broken (we don't have an ioctl to
> > read out the modifiers, heck even all the planes, there's no getfb2).
> >
> > So really not seeing the point in keeping that working.
>
> Daniel,  I came across usage of GET_TILING in libdrm.
> Is used in drm_intel_bo_gem_create_from_name() and drm_intel_bo_gem_create_from_prime().
> Should these be updated so they don't fail when EOPNOTSUPP is returned on gen12+?
> Maybe libdrm should just set tiling_mode to 0 on EOPNOTSUPP error instead of those calls failing?

Yes, I guess that part of mesa hasn't seen much testing yet.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx