We've discussed a bit how to get the gem/gt team better integrated and collaborate more with the wider community and agreed to the following:
- all gem/gt patches are reviewed on dri-devel for now. That's overkill, but in the past there was definitely too little of that.
- i915-gem folks are encouraged to cross review core patches from other teams
- big features (especially uapi changes) need to be discussed in an rfc patch that documents the interface and big picture design, before we get lost in the details of the code
- Also a rough TODO (can be refined as we go ofc) to get gem/gt back on track, like we've e.g. done with DAL/DC to get that in shape.
Cc: Jani Nikula jani.nikula@linux.intel.com Cc: Joonas Lahtinen joonas.lahtinen@linux.intel.com Cc: Rodrigo Vivi rodrigo.vivi@intel.com Cc: Jason Ekstrand jason@jlekstrand.net Cc: Dave Airlie airlied@redhat.com Signed-off-by: Daniel Vetter daniel.vetter@intel.com --- drivers/gpu/drm/i915/TODO.txt | 36 +++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 drivers/gpu/drm/i915/TODO.txt
diff --git a/drivers/gpu/drm/i915/TODO.txt b/drivers/gpu/drm/i915/TODO.txt new file mode 100644 index 000000000000..d2e5bbb6339d --- /dev/null +++ b/drivers/gpu/drm/i915/TODO.txt @@ -0,0 +1,36 @@ +gem/gt TODO items +----------------- + +- For discrete memory manager, merge enough dg1 to be able to refactor it to + TTM. Then land pci ids (just in case that turns up an uapi problem). TTM has + improved a lot the past 2 years, there's no reason anymore not to use it. + +- Come up with a plan what to do with drm/scheduler and how to get there. + +- There's a lot of complexity added past few years to make relocations faster. + That doesn't make sense given hw and gpu apis moved away from this model years + ago: + 1. Land a modern pre-bound uapi like VM_BIND + 2. Any complexity added in this area past few years which can't be justified + with VM_BIND using userspace should be removed. Looking at amdgpu dma_resv on + the bo and vm, plus some lru locks is all that needed. No complex rcu, + refcounts, caching, ... on everything. + This is the matching task on the vm side compared to ttm/dma_resv on the + backing storage side. + +- i915_sw_fence seems to be the main structure for the i915-gem dma_fence model. + How-to-dma_fence is core and drivers really shouldn't build their own world + here, treating everything else as a fixed platform. i915_sw_fence concepts + should be moved to dma_fence, drm/scheduler or atomic commit helpers. Or + removed if dri-devel consensus is that it's not a good idea. Once that's done + maybe even remove it if there's nothing left. + +Smaller things: +- i915_utils.h needs to be moved to the right places. + +- dma_fence_work should be in drivers/dma-buf + +- i915_mm.c should be moved to the right places. Some of the helpers also look a + bit fishy: + + https://lore.kernel.org/linux-mm/20210301083320.943079-1-hch@lst.de/
Motivated by the pre-review process for i915 gem/gt features, but probably useful in general for complex stuff.
Cc: Jani Nikula jani.nikula@linux.intel.com Cc: Joonas Lahtinen joonas.lahtinen@linux.intel.com Cc: Rodrigo Vivi rodrigo.vivi@intel.com Cc: Jason Ekstrand jason@jlekstrand.net Cc: Dave Airlie airlied@redhat.com Signed-off-by: Daniel Vetter daniel.vetter@intel.com --- Documentation/gpu/index.rst | 1 + Documentation/gpu/rfc.rst | 16 ++++++++++++++++ 2 files changed, 17 insertions(+) create mode 100644 Documentation/gpu/rfc.rst
diff --git a/Documentation/gpu/index.rst b/Documentation/gpu/index.rst index c9a51e3bfb5a..df58cb826d68 100644 --- a/Documentation/gpu/index.rst +++ b/Documentation/gpu/index.rst @@ -16,6 +16,7 @@ Linux GPU Driver Developer's Guide vga-switcheroo vgaarbiter todo + rfc
.. only:: subproject and html
diff --git a/Documentation/gpu/rfc.rst b/Documentation/gpu/rfc.rst new file mode 100644 index 000000000000..9d0ff2921af8 --- /dev/null +++ b/Documentation/gpu/rfc.rst @@ -0,0 +1,16 @@ +=============== +GPU RFC Section +=============== + +For complex work, especially new uapi, it is often good to nail the high level +design issues before getting lost in the code details. This section is meant to +host such documentation: + +* Each RFC should be a section in this file, explaining the goal and main design + considerations. + +* For uapi structures add a file to this directory with and then pull the + kerneldoc in like with real uapi headers. + +* Once the code has landed move all the documentation to the right places in + the main core, helper or driver sections.
On Tue, Mar 23, 2021 at 09:44:53AM +0100, Daniel Vetter wrote:
Motivated by the pre-review process for i915 gem/gt features, but probably useful in general for complex stuff.
Cc: Jani Nikula jani.nikula@linux.intel.com Cc: Joonas Lahtinen joonas.lahtinen@linux.intel.com Cc: Rodrigo Vivi rodrigo.vivi@intel.com Cc: Jason Ekstrand jason@jlekstrand.net Cc: Dave Airlie airlied@redhat.com Signed-off-by: Daniel Vetter daniel.vetter@intel.com
Documentation/gpu/index.rst | 1 + Documentation/gpu/rfc.rst | 16 ++++++++++++++++ 2 files changed, 17 insertions(+) create mode 100644 Documentation/gpu/rfc.rst
diff --git a/Documentation/gpu/index.rst b/Documentation/gpu/index.rst index c9a51e3bfb5a..df58cb826d68 100644 --- a/Documentation/gpu/index.rst +++ b/Documentation/gpu/index.rst @@ -16,6 +16,7 @@ Linux GPU Driver Developer's Guide vga-switcheroo vgaarbiter todo
- rfc
I understand the motivation here so I didn't commented earlier, but looking now, I'm wondering that this section will polute the official doc...
.. only:: subproject and html
diff --git a/Documentation/gpu/rfc.rst b/Documentation/gpu/rfc.rst new file mode 100644 index 000000000000..9d0ff2921af8 --- /dev/null +++ b/Documentation/gpu/rfc.rst @@ -0,0 +1,16 @@ +=============== +GPU RFC Section +===============
+For complex work, especially new uapi, it is often good to nail the high level +design issues before getting lost in the code details. This section is meant to +host such documentation:
+* Each RFC should be a section in this file, explaining the goal and main design
- considerations.
+* For uapi structures add a file to this directory with and then pull the
- kerneldoc in like with real uapi headers.
+* Once the code has landed move all the documentation to the right places in
- the main core, helper or driver sections.
-- 2.31.0
On Tue, Mar 23, 2021 at 08:37:07AM -0400, Rodrigo Vivi wrote:
On Tue, Mar 23, 2021 at 09:44:53AM +0100, Daniel Vetter wrote:
Motivated by the pre-review process for i915 gem/gt features, but probably useful in general for complex stuff.
Cc: Jani Nikula jani.nikula@linux.intel.com Cc: Joonas Lahtinen joonas.lahtinen@linux.intel.com Cc: Rodrigo Vivi rodrigo.vivi@intel.com Cc: Jason Ekstrand jason@jlekstrand.net Cc: Dave Airlie airlied@redhat.com Signed-off-by: Daniel Vetter daniel.vetter@intel.com
Documentation/gpu/index.rst | 1 + Documentation/gpu/rfc.rst | 16 ++++++++++++++++ 2 files changed, 17 insertions(+) create mode 100644 Documentation/gpu/rfc.rst
diff --git a/Documentation/gpu/index.rst b/Documentation/gpu/index.rst index c9a51e3bfb5a..df58cb826d68 100644 --- a/Documentation/gpu/index.rst +++ b/Documentation/gpu/index.rst @@ -16,6 +16,7 @@ Linux GPU Driver Developer's Guide vga-switcheroo vgaarbiter todo
- rfc
I understand the motivation here so I didn't commented earlier, but looking now, I'm wondering that this section will polute the official doc...
We already have this problem between documentation meant for kernel driver developers and documentation meant for userspace developers around uapi and all that. "who is the audience here" is very ill-defined for our current set of docs in Documentation/gpu :-(
So I agree with you, but I also don't think this will make things worse. -Daniel
.. only:: subproject and html
diff --git a/Documentation/gpu/rfc.rst b/Documentation/gpu/rfc.rst new file mode 100644 index 000000000000..9d0ff2921af8 --- /dev/null +++ b/Documentation/gpu/rfc.rst @@ -0,0 +1,16 @@ +=============== +GPU RFC Section +===============
+For complex work, especially new uapi, it is often good to nail the high level +design issues before getting lost in the code details. This section is meant to +host such documentation:
+* Each RFC should be a section in this file, explaining the goal and main design
- considerations.
+* For uapi structures add a file to this directory with and then pull the
- kerneldoc in like with real uapi headers.
+* Once the code has landed move all the documentation to the right places in
- the main core, helper or driver sections.
-- 2.31.0
On Tue, Mar 23, 2021 at 02:10:17PM +0100, Daniel Vetter wrote:
On Tue, Mar 23, 2021 at 08:37:07AM -0400, Rodrigo Vivi wrote:
On Tue, Mar 23, 2021 at 09:44:53AM +0100, Daniel Vetter wrote:
Motivated by the pre-review process for i915 gem/gt features, but probably useful in general for complex stuff.
Cc: Jani Nikula jani.nikula@linux.intel.com Cc: Joonas Lahtinen joonas.lahtinen@linux.intel.com Cc: Rodrigo Vivi rodrigo.vivi@intel.com Cc: Jason Ekstrand jason@jlekstrand.net Cc: Dave Airlie airlied@redhat.com Signed-off-by: Daniel Vetter daniel.vetter@intel.com
Documentation/gpu/index.rst | 1 + Documentation/gpu/rfc.rst | 16 ++++++++++++++++ 2 files changed, 17 insertions(+) create mode 100644 Documentation/gpu/rfc.rst
diff --git a/Documentation/gpu/index.rst b/Documentation/gpu/index.rst index c9a51e3bfb5a..df58cb826d68 100644 --- a/Documentation/gpu/index.rst +++ b/Documentation/gpu/index.rst @@ -16,6 +16,7 @@ Linux GPU Driver Developer's Guide vga-switcheroo vgaarbiter todo
- rfc
I understand the motivation here so I didn't commented earlier, but looking now, I'm wondering that this section will polute the official doc...
We already have this problem between documentation meant for kernel driver developers and documentation meant for userspace developers around uapi and all that. "who is the audience here" is very ill-defined for our current set of docs in Documentation/gpu :-(
you have a point...
another thing that I don't like is the overhead, but otoh it forces us to write more docs...
with all the pros and cons understood, let's move on...
Acked-by: Rodrigo Vivi rodrigo.vivi@intel.com
So I agree with you, but I also don't think this will make things worse. -Daniel
.. only:: subproject and html
diff --git a/Documentation/gpu/rfc.rst b/Documentation/gpu/rfc.rst new file mode 100644 index 000000000000..9d0ff2921af8 --- /dev/null +++ b/Documentation/gpu/rfc.rst @@ -0,0 +1,16 @@ +=============== +GPU RFC Section +===============
+For complex work, especially new uapi, it is often good to nail the high level +design issues before getting lost in the code details. This section is meant to +host such documentation:
+* Each RFC should be a section in this file, explaining the goal and main design
- considerations.
+* For uapi structures add a file to this directory with and then pull the
- kerneldoc in like with real uapi headers.
+* Once the code has landed move all the documentation to the right places in
- the main core, helper or driver sections.
-- 2.31.0
-- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Side note: I feel like we could have better lines of communication between kernel devs and user-space devs. The new uAPI requirements seem to be a high barrier to entry for kernel devs. However sometimes user-space devs might be interested in helping out with the user-space part…
Maybe it would be good to CC e.g. wayland-devel for new RFCs, so that user-space devs can jump in if they're interested. And also provide feedback, since new uAPI is hard to spot in the sea of messages in dri-devel.
On Tue, Mar 23, 2021 at 12:01 PM Simon Ser contact@emersion.fr wrote:
Side note: I feel like we could have better lines of communication between kernel devs and user-space devs. The new uAPI requirements seem to be a high barrier to entry for kernel devs. However sometimes user-space devs might be interested in helping out with the user-space part…
Maybe it would be good to CC e.g. wayland-devel for new RFCs, so that user-space devs can jump in if they're interested. And also provide feedback, since new uAPI is hard to spot in the sea of messages in dri-devel.
That's a good suggestion. CCing wayland-dev or mesa-dev, as appropriate, with such docs patches sounds like a good idea. I'm not sure where we would put that in here but it would be good to call out.
On Tue, Mar 23, 2021 at 6:55 PM Jason Ekstrand jason@jlekstrand.net wrote:
On Tue, Mar 23, 2021 at 12:01 PM Simon Ser contact@emersion.fr wrote:
Side note: I feel like we could have better lines of communication between kernel devs and user-space devs. The new uAPI requirements seem to be a high barrier to entry for kernel devs. However sometimes user-space devs might be interested in helping out with the user-space part…
Maybe it would be good to CC e.g. wayland-devel for new RFCs, so that user-space devs can jump in if they're interested. And also provide feedback, since new uAPI is hard to spot in the sea of messages in dri-devel.
That's a good suggestion. CCing wayland-dev or mesa-dev, as appropriate, with such docs patches sounds like a good idea. I'm not sure where we would put that in here but it would be good to call out.
I'll add a suggestion to that extend, it's a good one. -Daniel
On Tue, 23 Mar 2021 at 18:45, Daniel Vetter daniel.vetter@ffwll.ch wrote:
Motivated by the pre-review process for i915 gem/gt features, but probably useful in general for complex stuff.
Cc: Jani Nikula jani.nikula@linux.intel.com Cc: Joonas Lahtinen joonas.lahtinen@linux.intel.com Cc: Rodrigo Vivi rodrigo.vivi@intel.com Cc: Jason Ekstrand jason@jlekstrand.net Cc: Dave Airlie airlied@redhat.com Signed-off-by: Daniel Vetter daniel.vetter@intel.com
Let's give it a try, I do like the future ideas of doing things with gitlab issues as well.
Acked-by: Dave Airlie airlied@redhat.com
Documentation/gpu/index.rst | 1 + Documentation/gpu/rfc.rst | 16 ++++++++++++++++ 2 files changed, 17 insertions(+) create mode 100644 Documentation/gpu/rfc.rst
diff --git a/Documentation/gpu/index.rst b/Documentation/gpu/index.rst index c9a51e3bfb5a..df58cb826d68 100644 --- a/Documentation/gpu/index.rst +++ b/Documentation/gpu/index.rst @@ -16,6 +16,7 @@ Linux GPU Driver Developer's Guide vga-switcheroo vgaarbiter todo
- rfc
.. only:: subproject and html
diff --git a/Documentation/gpu/rfc.rst b/Documentation/gpu/rfc.rst new file mode 100644 index 000000000000..9d0ff2921af8 --- /dev/null +++ b/Documentation/gpu/rfc.rst @@ -0,0 +1,16 @@ +=============== +GPU RFC Section +===============
+For complex work, especially new uapi, it is often good to nail the high level +design issues before getting lost in the code details. This section is meant to +host such documentation:
+* Each RFC should be a section in this file, explaining the goal and main design
- considerations.
+* For uapi structures add a file to this directory with and then pull the
- kerneldoc in like with real uapi headers.
+* Once the code has landed move all the documentation to the right places in
- the main core, helper or driver sections.
-- 2.31.0
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, 23 Mar 2021, Daniel Vetter daniel.vetter@ffwll.ch wrote:
We've discussed a bit how to get the gem/gt team better integrated and collaborate more with the wider community and agreed to the following:
all gem/gt patches are reviewed on dri-devel for now. That's overkill, but in the past there was definitely too little of that.
i915-gem folks are encouraged to cross review core patches from other teams
big features (especially uapi changes) need to be discussed in an rfc patch that documents the interface and big picture design, before we get lost in the details of the code
Also a rough TODO (can be refined as we go ofc) to get gem/gt back on track, like we've e.g. done with DAL/DC to get that in shape.
I personally think there should be a lower bar for discussing and editing the TODO items than via patches on the mailing list. Granted, the TODO file enforces the discussion happens at a large enough audience, but for at least some of the items I'd suggest filing gitlab issues [1], with todo label, and tracking there.
BR, Jani.
[1] https://gitlab.freedesktop.org/drm/intel/-/issues
Cc: Jani Nikula jani.nikula@linux.intel.com Cc: Joonas Lahtinen joonas.lahtinen@linux.intel.com Cc: Rodrigo Vivi rodrigo.vivi@intel.com Cc: Jason Ekstrand jason@jlekstrand.net Cc: Dave Airlie airlied@redhat.com Signed-off-by: Daniel Vetter daniel.vetter@intel.com
drivers/gpu/drm/i915/TODO.txt | 36 +++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 drivers/gpu/drm/i915/TODO.txt
diff --git a/drivers/gpu/drm/i915/TODO.txt b/drivers/gpu/drm/i915/TODO.txt new file mode 100644 index 000000000000..d2e5bbb6339d --- /dev/null +++ b/drivers/gpu/drm/i915/TODO.txt @@ -0,0 +1,36 @@ +gem/gt TODO items +-----------------
+- For discrete memory manager, merge enough dg1 to be able to refactor it to
- TTM. Then land pci ids (just in case that turns up an uapi problem). TTM has
- improved a lot the past 2 years, there's no reason anymore not to use it.
+- Come up with a plan what to do with drm/scheduler and how to get there.
+- There's a lot of complexity added past few years to make relocations faster.
- That doesn't make sense given hw and gpu apis moved away from this model years
- ago:
- Land a modern pre-bound uapi like VM_BIND
- Any complexity added in this area past few years which can't be justified
- with VM_BIND using userspace should be removed. Looking at amdgpu dma_resv on
- the bo and vm, plus some lru locks is all that needed. No complex rcu,
- refcounts, caching, ... on everything.
- This is the matching task on the vm side compared to ttm/dma_resv on the
- backing storage side.
+- i915_sw_fence seems to be the main structure for the i915-gem dma_fence model.
- How-to-dma_fence is core and drivers really shouldn't build their own world
- here, treating everything else as a fixed platform. i915_sw_fence concepts
- should be moved to dma_fence, drm/scheduler or atomic commit helpers. Or
- removed if dri-devel consensus is that it's not a good idea. Once that's done
- maybe even remove it if there's nothing left.
+Smaller things: +- i915_utils.h needs to be moved to the right places.
+- dma_fence_work should be in drivers/dma-buf
+- i915_mm.c should be moved to the right places. Some of the helpers also look a
On Tue, Mar 23, 2021 at 12:13:11PM +0200, Jani Nikula wrote:
On Tue, 23 Mar 2021, Daniel Vetter daniel.vetter@ffwll.ch wrote:
We've discussed a bit how to get the gem/gt team better integrated and collaborate more with the wider community and agreed to the following:
all gem/gt patches are reviewed on dri-devel for now. That's overkill, but in the past there was definitely too little of that.
i915-gem folks are encouraged to cross review core patches from other teams
big features (especially uapi changes) need to be discussed in an rfc patch that documents the interface and big picture design, before we get lost in the details of the code
Also a rough TODO (can be refined as we go ofc) to get gem/gt back on track, like we've e.g. done with DAL/DC to get that in shape.
I personally think there should be a lower bar for discussing and editing the TODO items than via patches on the mailing list. Granted, the TODO file enforces the discussion happens at a large enough audience, but for at least some of the items I'd suggest filing gitlab issues [1], with todo label, and tracking there.
In general yes, and I'd go even further: it's up to each team/contributor how they track review feedback and further work, whether that's gitlab or some notes or just in their heads.
This is a different situation here, and the "changes require big audience" is a feature, not a bug. But it is a very exceptional situation, I think this is only the 2nd time we're using a formal TODO for a gpu driver. If we ignore gma500 in staging, which for me only showed that the separate staging tree doesn't work so well for complex drivers like we have. -Daniel
BR, Jani.
[1] https://gitlab.freedesktop.org/drm/intel/-/issues
Cc: Jani Nikula jani.nikula@linux.intel.com Cc: Joonas Lahtinen joonas.lahtinen@linux.intel.com Cc: Rodrigo Vivi rodrigo.vivi@intel.com Cc: Jason Ekstrand jason@jlekstrand.net Cc: Dave Airlie airlied@redhat.com Signed-off-by: Daniel Vetter daniel.vetter@intel.com
drivers/gpu/drm/i915/TODO.txt | 36 +++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 drivers/gpu/drm/i915/TODO.txt
diff --git a/drivers/gpu/drm/i915/TODO.txt b/drivers/gpu/drm/i915/TODO.txt new file mode 100644 index 000000000000..d2e5bbb6339d --- /dev/null +++ b/drivers/gpu/drm/i915/TODO.txt @@ -0,0 +1,36 @@ +gem/gt TODO items +-----------------
+- For discrete memory manager, merge enough dg1 to be able to refactor it to
- TTM. Then land pci ids (just in case that turns up an uapi problem). TTM has
- improved a lot the past 2 years, there's no reason anymore not to use it.
+- Come up with a plan what to do with drm/scheduler and how to get there.
+- There's a lot of complexity added past few years to make relocations faster.
- That doesn't make sense given hw and gpu apis moved away from this model years
- ago:
- Land a modern pre-bound uapi like VM_BIND
- Any complexity added in this area past few years which can't be justified
- with VM_BIND using userspace should be removed. Looking at amdgpu dma_resv on
- the bo and vm, plus some lru locks is all that needed. No complex rcu,
- refcounts, caching, ... on everything.
- This is the matching task on the vm side compared to ttm/dma_resv on the
- backing storage side.
+- i915_sw_fence seems to be the main structure for the i915-gem dma_fence model.
- How-to-dma_fence is core and drivers really shouldn't build their own world
- here, treating everything else as a fixed platform. i915_sw_fence concepts
- should be moved to dma_fence, drm/scheduler or atomic commit helpers. Or
- removed if dri-devel consensus is that it's not a good idea. Once that's done
- maybe even remove it if there's nothing left.
+Smaller things: +- i915_utils.h needs to be moved to the right places.
+- dma_fence_work should be in drivers/dma-buf
+- i915_mm.c should be moved to the right places. Some of the helpers also look a
-- Jani Nikula, Intel Open Source Graphics Center
On Tue, Mar 23, 2021 at 12:57:39PM +0100, Daniel Vetter wrote:
On Tue, Mar 23, 2021 at 12:13:11PM +0200, Jani Nikula wrote:
On Tue, 23 Mar 2021, Daniel Vetter daniel.vetter@ffwll.ch wrote:
We've discussed a bit how to get the gem/gt team better integrated and collaborate more with the wider community and agreed to the following:
all gem/gt patches are reviewed on dri-devel for now. That's overkill, but in the past there was definitely too little of that.
i915-gem folks are encouraged to cross review core patches from other teams
big features (especially uapi changes) need to be discussed in an rfc patch that documents the interface and big picture design, before we get lost in the details of the code
Also a rough TODO (can be refined as we go ofc) to get gem/gt back on track, like we've e.g. done with DAL/DC to get that in shape.
I personally think there should be a lower bar for discussing and editing the TODO items than via patches on the mailing list. Granted, the TODO file enforces the discussion happens at a large enough audience, but for at least some of the items I'd suggest filing gitlab issues [1], with todo label, and tracking there.
I also don't like the todo list in files and I agree that gitlab issues section should be better...
In general yes, and I'd go even further: it's up to each team/contributor how they track review feedback and further work, whether that's gitlab or some notes or just in their heads.
This is a different situation here, and the "changes require big audience" is a feature, not a bug. But it is a very exceptional situation, I think this is only the 2nd time we're using a formal TODO for a gpu driver. If we ignore gma500 in staging, which for me only showed that the separate staging tree doesn't work so well for complex drivers like we have.
... but I understand the motivation, so
Acked-by: Rodrigo Vivi rodrigo.vivi@intel.com
However... what about:
1. moving the smaller items to gitlab at least? 2. having both, all the entries in the todo file have gitlab issue associated and the number-id is also here in the todo file?
-Daniel
BR, Jani.
[1] https://gitlab.freedesktop.org/drm/intel/-/issues
Cc: Jani Nikula jani.nikula@linux.intel.com Cc: Joonas Lahtinen joonas.lahtinen@linux.intel.com Cc: Rodrigo Vivi rodrigo.vivi@intel.com Cc: Jason Ekstrand jason@jlekstrand.net Cc: Dave Airlie airlied@redhat.com Signed-off-by: Daniel Vetter daniel.vetter@intel.com
drivers/gpu/drm/i915/TODO.txt | 36 +++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 drivers/gpu/drm/i915/TODO.txt
diff --git a/drivers/gpu/drm/i915/TODO.txt b/drivers/gpu/drm/i915/TODO.txt new file mode 100644 index 000000000000..d2e5bbb6339d --- /dev/null +++ b/drivers/gpu/drm/i915/TODO.txt @@ -0,0 +1,36 @@ +gem/gt TODO items +-----------------
+- For discrete memory manager, merge enough dg1 to be able to refactor it to
- TTM. Then land pci ids (just in case that turns up an uapi problem). TTM has
- improved a lot the past 2 years, there's no reason anymore not to use it.
+- Come up with a plan what to do with drm/scheduler and how to get there.
+- There's a lot of complexity added past few years to make relocations faster.
- That doesn't make sense given hw and gpu apis moved away from this model years
- ago:
- Land a modern pre-bound uapi like VM_BIND
- Any complexity added in this area past few years which can't be justified
- with VM_BIND using userspace should be removed. Looking at amdgpu dma_resv on
- the bo and vm, plus some lru locks is all that needed. No complex rcu,
- refcounts, caching, ... on everything.
- This is the matching task on the vm side compared to ttm/dma_resv on the
- backing storage side.
+- i915_sw_fence seems to be the main structure for the i915-gem dma_fence model.
- How-to-dma_fence is core and drivers really shouldn't build their own world
- here, treating everything else as a fixed platform. i915_sw_fence concepts
- should be moved to dma_fence, drm/scheduler or atomic commit helpers. Or
- removed if dri-devel consensus is that it's not a good idea. Once that's done
- maybe even remove it if there's nothing left.
+Smaller things: +- i915_utils.h needs to be moved to the right places.
+- dma_fence_work should be in drivers/dma-buf
+- i915_mm.c should be moved to the right places. Some of the helpers also look a
-- Jani Nikula, Intel Open Source Graphics Center
-- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
On Tue, Mar 23, 2021 at 08:34:55AM -0400, Rodrigo Vivi wrote:
On Tue, Mar 23, 2021 at 12:57:39PM +0100, Daniel Vetter wrote:
On Tue, Mar 23, 2021 at 12:13:11PM +0200, Jani Nikula wrote:
On Tue, 23 Mar 2021, Daniel Vetter daniel.vetter@ffwll.ch wrote:
We've discussed a bit how to get the gem/gt team better integrated and collaborate more with the wider community and agreed to the following:
all gem/gt patches are reviewed on dri-devel for now. That's overkill, but in the past there was definitely too little of that.
i915-gem folks are encouraged to cross review core patches from other teams
big features (especially uapi changes) need to be discussed in an rfc patch that documents the interface and big picture design, before we get lost in the details of the code
Also a rough TODO (can be refined as we go ofc) to get gem/gt back on track, like we've e.g. done with DAL/DC to get that in shape.
I personally think there should be a lower bar for discussing and editing the TODO items than via patches on the mailing list. Granted, the TODO file enforces the discussion happens at a large enough audience, but for at least some of the items I'd suggest filing gitlab issues [1], with todo label, and tracking there.
I also don't like the todo list in files and I agree that gitlab issues section should be better...
In general yes, and I'd go even further: it's up to each team/contributor how they track review feedback and further work, whether that's gitlab or some notes or just in their heads.
This is a different situation here, and the "changes require big audience" is a feature, not a bug. But it is a very exceptional situation, I think this is only the 2nd time we're using a formal TODO for a gpu driver. If we ignore gma500 in staging, which for me only showed that the separate staging tree doesn't work so well for complex drivers like we have.
... but I understand the motivation, so
Acked-by: Rodrigo Vivi rodrigo.vivi@intel.com
However... what about:
- moving the smaller items to gitlab at least?
- having both, all the entries in the todo file have gitlab issue
associated and the number-id is also here in the todo file?
Yeah that sounds reasonable. tbh we haven't started any of the intel-internal planning on most of these (ttm and scheduler are started), so none of these tracking things exist yet at all ... -Daniel
-Daniel
BR, Jani.
[1] https://gitlab.freedesktop.org/drm/intel/-/issues
Cc: Jani Nikula jani.nikula@linux.intel.com Cc: Joonas Lahtinen joonas.lahtinen@linux.intel.com Cc: Rodrigo Vivi rodrigo.vivi@intel.com Cc: Jason Ekstrand jason@jlekstrand.net Cc: Dave Airlie airlied@redhat.com Signed-off-by: Daniel Vetter daniel.vetter@intel.com
drivers/gpu/drm/i915/TODO.txt | 36 +++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 drivers/gpu/drm/i915/TODO.txt
diff --git a/drivers/gpu/drm/i915/TODO.txt b/drivers/gpu/drm/i915/TODO.txt new file mode 100644 index 000000000000..d2e5bbb6339d --- /dev/null +++ b/drivers/gpu/drm/i915/TODO.txt @@ -0,0 +1,36 @@ +gem/gt TODO items +-----------------
+- For discrete memory manager, merge enough dg1 to be able to refactor it to
- TTM. Then land pci ids (just in case that turns up an uapi problem). TTM has
- improved a lot the past 2 years, there's no reason anymore not to use it.
+- Come up with a plan what to do with drm/scheduler and how to get there.
+- There's a lot of complexity added past few years to make relocations faster.
- That doesn't make sense given hw and gpu apis moved away from this model years
- ago:
- Land a modern pre-bound uapi like VM_BIND
- Any complexity added in this area past few years which can't be justified
- with VM_BIND using userspace should be removed. Looking at amdgpu dma_resv on
- the bo and vm, plus some lru locks is all that needed. No complex rcu,
- refcounts, caching, ... on everything.
- This is the matching task on the vm side compared to ttm/dma_resv on the
- backing storage side.
+- i915_sw_fence seems to be the main structure for the i915-gem dma_fence model.
- How-to-dma_fence is core and drivers really shouldn't build their own world
- here, treating everything else as a fixed platform. i915_sw_fence concepts
- should be moved to dma_fence, drm/scheduler or atomic commit helpers. Or
- removed if dri-devel consensus is that it's not a good idea. Once that's done
- maybe even remove it if there's nothing left.
+Smaller things: +- i915_utils.h needs to be moved to the right places.
+- dma_fence_work should be in drivers/dma-buf
+- i915_mm.c should be moved to the right places. Some of the helpers also look a
-- Jani Nikula, Intel Open Source Graphics Center
-- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
On Tue, Mar 23, 2021 at 8:18 AM Daniel Vetter daniel@ffwll.ch wrote:
On Tue, Mar 23, 2021 at 08:34:55AM -0400, Rodrigo Vivi wrote:
On Tue, Mar 23, 2021 at 12:57:39PM +0100, Daniel Vetter wrote:
On Tue, Mar 23, 2021 at 12:13:11PM +0200, Jani Nikula wrote:
On Tue, 23 Mar 2021, Daniel Vetter daniel.vetter@ffwll.ch wrote:
We've discussed a bit how to get the gem/gt team better integrated and collaborate more with the wider community and agreed to the following:
all gem/gt patches are reviewed on dri-devel for now. That's overkill, but in the past there was definitely too little of that.
i915-gem folks are encouraged to cross review core patches from other teams
big features (especially uapi changes) need to be discussed in an rfc patch that documents the interface and big picture design, before we get lost in the details of the code
Also a rough TODO (can be refined as we go ofc) to get gem/gt back on track, like we've e.g. done with DAL/DC to get that in shape.
I personally think there should be a lower bar for discussing and editing the TODO items than via patches on the mailing list. Granted, the TODO file enforces the discussion happens at a large enough audience, but for at least some of the items I'd suggest filing gitlab issues [1], with todo label, and tracking there.
I also don't like the todo list in files and I agree that gitlab issues section should be better...
In general yes, and I'd go even further: it's up to each team/contributor how they track review feedback and further work, whether that's gitlab or some notes or just in their heads.
This is a different situation here, and the "changes require big audience" is a feature, not a bug. But it is a very exceptional situation, I think this is only the 2nd time we're using a formal TODO for a gpu driver. If we ignore gma500 in staging, which for me only showed that the separate staging tree doesn't work so well for complex drivers like we have.
... but I understand the motivation, so
Acked-by: Rodrigo Vivi rodrigo.vivi@intel.com
However... what about:
- moving the smaller items to gitlab at least?
- having both, all the entries in the todo file have gitlab issue
associated and the number-id is also here in the todo file?
Yeah that sounds reasonable. tbh we haven't started any of the intel-internal planning on most of these (ttm and scheduler are started), so none of these tracking things exist yet at all ...
I'm a fan of this. GitLab issues provide a good place to organize the chatter on any particular ToDo item. I'd also rather see people chattering about this stuff on public GitLab than JIRA, when possible. The last patch in the series closing out a ToDo can be a patch to this file to remove the bullet point.
--Jason
-Daniel
-Daniel
BR, Jani.
[1] https://gitlab.freedesktop.org/drm/intel/-/issues
Cc: Jani Nikula jani.nikula@linux.intel.com Cc: Joonas Lahtinen joonas.lahtinen@linux.intel.com Cc: Rodrigo Vivi rodrigo.vivi@intel.com Cc: Jason Ekstrand jason@jlekstrand.net Cc: Dave Airlie airlied@redhat.com Signed-off-by: Daniel Vetter daniel.vetter@intel.com
drivers/gpu/drm/i915/TODO.txt | 36 +++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 drivers/gpu/drm/i915/TODO.txt
diff --git a/drivers/gpu/drm/i915/TODO.txt b/drivers/gpu/drm/i915/TODO.txt new file mode 100644 index 000000000000..d2e5bbb6339d --- /dev/null +++ b/drivers/gpu/drm/i915/TODO.txt @@ -0,0 +1,36 @@ +gem/gt TODO items +-----------------
+- For discrete memory manager, merge enough dg1 to be able to refactor it to
- TTM. Then land pci ids (just in case that turns up an uapi problem). TTM has
- improved a lot the past 2 years, there's no reason anymore not to use it.
+- Come up with a plan what to do with drm/scheduler and how to get there.
+- There's a lot of complexity added past few years to make relocations faster.
- That doesn't make sense given hw and gpu apis moved away from this model years
- ago:
- Land a modern pre-bound uapi like VM_BIND
- Any complexity added in this area past few years which can't be justified
- with VM_BIND using userspace should be removed. Looking at amdgpu dma_resv on
- the bo and vm, plus some lru locks is all that needed. No complex rcu,
- refcounts, caching, ... on everything.
- This is the matching task on the vm side compared to ttm/dma_resv on the
- backing storage side.
+- i915_sw_fence seems to be the main structure for the i915-gem dma_fence model.
- How-to-dma_fence is core and drivers really shouldn't build their own world
- here, treating everything else as a fixed platform. i915_sw_fence concepts
- should be moved to dma_fence, drm/scheduler or atomic commit helpers. Or
- removed if dri-devel consensus is that it's not a good idea. Once that's done
- maybe even remove it if there's nothing left.
+Smaller things: +- i915_utils.h needs to be moved to the right places.
+- dma_fence_work should be in drivers/dma-buf
+- i915_mm.c should be moved to the right places. Some of the helpers also look a
-- Jani Nikula, Intel Open Source Graphics Center
-- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
-- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
On Tue, Mar 23, 2021 at 09:44:52AM +0100, Daniel Vetter wrote:
We've discussed a bit how to get the gem/gt team better integrated and collaborate more with the wider community and agreed to the following:
all gem/gt patches are reviewed on dri-devel for now. That's overkill, but in the past there was definitely too little of that.
i915-gem folks are encouraged to cross review core patches from other teams
big features (especially uapi changes) need to be discussed in an rfc patch that documents the interface and big picture design, before we get lost in the details of the code
Also a rough TODO (can be refined as we go ofc) to get gem/gt back on track, like we've e.g. done with DAL/DC to get that in shape.
Cc: Jani Nikula jani.nikula@linux.intel.com Cc: Joonas Lahtinen joonas.lahtinen@linux.intel.com Cc: Rodrigo Vivi rodrigo.vivi@intel.com Cc: Jason Ekstrand jason@jlekstrand.net Cc: Dave Airlie airlied@redhat.com Signed-off-by: Daniel Vetter daniel.vetter@intel.com
drivers/gpu/drm/i915/TODO.txt | 36 +++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 drivers/gpu/drm/i915/TODO.txt
diff --git a/drivers/gpu/drm/i915/TODO.txt b/drivers/gpu/drm/i915/TODO.txt new file mode 100644 index 000000000000..d2e5bbb6339d --- /dev/null +++ b/drivers/gpu/drm/i915/TODO.txt @@ -0,0 +1,36 @@ +gem/gt TODO items +-----------------
+- For discrete memory manager, merge enough dg1 to be able to refactor it to
- TTM. Then land pci ids (just in case that turns up an uapi problem). TTM has
- improved a lot the past 2 years, there's no reason anymore not to use it.
+- Come up with a plan what to do with drm/scheduler and how to get there.
+- There's a lot of complexity added past few years to make relocations faster.
- That doesn't make sense given hw and gpu apis moved away from this model years
- ago:
- Land a modern pre-bound uapi like VM_BIND
- Any complexity added in this area past few years which can't be justified
- with VM_BIND using userspace should be removed. Looking at amdgpu dma_resv on
- the bo and vm, plus some lru locks is all that needed. No complex rcu,
- refcounts, caching, ... on everything.
- This is the matching task on the vm side compared to ttm/dma_resv on the
- backing storage side.
+- i915_sw_fence seems to be the main structure for the i915-gem dma_fence model.
- How-to-dma_fence is core and drivers really shouldn't build their own world
- here, treating everything else as a fixed platform. i915_sw_fence concepts
- should be moved to dma_fence, drm/scheduler or atomic commit helpers. Or
- removed if dri-devel consensus is that it's not a good idea. Once that's done
- maybe even remove it if there's nothing left.
+Smaller things: +- i915_utils.h needs to be moved to the right places.
+- dma_fence_work should be in drivers/dma-buf
+- i915_mm.c should be moved to the right places. Some of the helpers also look a
Jani just pointed me at another small helper that we should look at:
- tasklet helpers in i915_gem.h also look a bit misplaced and should probably be moved to tasklet headers.
Spotted through https://lore.kernel.org/lkml/20210323092221.awq7g5b2muzypjw3@flow/
I'll add that one in v2.
There is also the i915 ww locking context helpers from Maarten in there, but right now those truly are i915 specific. Plus as part of the ttm conversion they're already on the list of things we have to move. So I think redundant to add the entire file. -Daniel
On Tue, 23 Mar 2021 at 23:17, Daniel Vetter daniel@ffwll.ch wrote:
On Tue, Mar 23, 2021 at 09:44:52AM +0100, Daniel Vetter wrote:
We've discussed a bit how to get the gem/gt team better integrated and collaborate more with the wider community and agreed to the following:
all gem/gt patches are reviewed on dri-devel for now. That's overkill, but in the past there was definitely too little of that.
i915-gem folks are encouraged to cross review core patches from other teams
big features (especially uapi changes) need to be discussed in an rfc patch that documents the interface and big picture design, before we get lost in the details of the code
Also a rough TODO (can be refined as we go ofc) to get gem/gt back on track, like we've e.g. done with DAL/DC to get that in shape.
I think we mentioned in the past about having better annotations for dma_fence critical sections,
Otherwise I think this is a great list to get us out of the woods and seeing how to move forward again.
Dave.
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
Daniel Vetter -
Dave Airlie -
Jani Nikula -
Jason Ekstrand -
Rodrigo Vivi -
Simon Ser