| Message ID | 20191215211223.1451499-1-martin.blumenstingl@googlemail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | drm: lima: devfreq and cooling device support | expand |
Thanks for adding this. As the license, it's up to you, I think it's OK for now. For the code, I think you may need some lock to protect the time records as there are two kernel threads gp/pp will try to mark GPU busy and several interrupts try to mark GPU idle. Regards, Qiang On Mon, Dec 16, 2019 at 5:12 AM Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote: > > This is my attempt at adding devfreq (and cooling device) support to > the lima driver. > I didn't have much time to do in-depth testing. However, I'm sending > this out early because there are many SoCs with Mali-400/450 GPU so > I want to avoid duplicating the work with somebody else. > > The code is derived from panfrost_devfreq.c which is why I kept the > Collabora copyright in lima_devfreq.c. Please let me know if I should > drop this or how I can make it more clear that I "borrowed" the code > from panfrost. > > I am seeking comments in two general areas: > - regarding the integration into the existing lima code > - for the actual devfreq code (I had to adapt the panfrost code > slightly, because lima uses a bus and a GPU/core clock) > > My own TODO list includes "more" testing on various Amlogic SoCs. > So far I have tested this on Meson8b and Meson8m2 (which both have a > GPU OPP table defined). However, I still need to test this on a GXL > board (which is currently missing the GPU OPP table). > > > Martin Blumenstingl (1): > drm/lima: Add optional devfreq support > > drivers/gpu/drm/lima/Kconfig | 1 + > drivers/gpu/drm/lima/Makefile | 3 +- > drivers/gpu/drm/lima/lima_devfreq.c | 162 ++++++++++++++++++++++++++++ > drivers/gpu/drm/lima/lima_devfreq.h | 15 +++ > drivers/gpu/drm/lima/lima_device.c | 4 + > drivers/gpu/drm/lima/lima_device.h | 11 ++ > drivers/gpu/drm/lima/lima_drv.c | 14 ++- > drivers/gpu/drm/lima/lima_sched.c | 7 ++ > drivers/gpu/drm/lima/lima_sched.h | 3 + > 9 files changed, 217 insertions(+), 3 deletions(-) > create mode 100644 drivers/gpu/drm/lima/lima_devfreq.c > create mode 100644 drivers/gpu/drm/lima/lima_devfreq.h > > -- > 2.24.1 >
On Mon, Dec 16, 2019 at 5:12 AM Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote: > > This is my attempt at adding devfreq (and cooling device) support to > the lima driver. > I didn't have much time to do in-depth testing. However, I'm sending > this out early because there are many SoCs with Mali-400/450 GPU so > I want to avoid duplicating the work with somebody else. > > The code is derived from panfrost_devfreq.c which is why I kept the > Collabora copyright in lima_devfreq.c. Please let me know if I should > drop this or how I can make it more clear that I "borrowed" the code > from panfrost. I think it's more common to have separate copyright notices. First you have yours, then a second paragraph stating the code is derived from foo, and then attach the copyright statements for foo. ChenYu > I am seeking comments in two general areas: > - regarding the integration into the existing lima code > - for the actual devfreq code (I had to adapt the panfrost code > slightly, because lima uses a bus and a GPU/core clock) > > My own TODO list includes "more" testing on various Amlogic SoCs. > So far I have tested this on Meson8b and Meson8m2 (which both have a > GPU OPP table defined). However, I still need to test this on a GXL > board (which is currently missing the GPU OPP table). > > > Martin Blumenstingl (1): > drm/lima: Add optional devfreq support > > drivers/gpu/drm/lima/Kconfig | 1 + > drivers/gpu/drm/lima/Makefile | 3 +- > drivers/gpu/drm/lima/lima_devfreq.c | 162 ++++++++++++++++++++++++++++ > drivers/gpu/drm/lima/lima_devfreq.h | 15 +++ > drivers/gpu/drm/lima/lima_device.c | 4 + > drivers/gpu/drm/lima/lima_device.h | 11 ++ > drivers/gpu/drm/lima/lima_drv.c | 14 ++- > drivers/gpu/drm/lima/lima_sched.c | 7 ++ > drivers/gpu/drm/lima/lima_sched.h | 3 + > 9 files changed, 217 insertions(+), 3 deletions(-) > create mode 100644 drivers/gpu/drm/lima/lima_devfreq.c > create mode 100644 drivers/gpu/drm/lima/lima_devfreq.h > > -- > 2.24.1 >
If so much code is being duplicated over, I'm wondering if it makes sense for us to move some of the common devfreq code to core DRM helpers? On Sun, Dec 15, 2019 at 10:12:22PM +0100, Martin Blumenstingl wrote: > This is my attempt at adding devfreq (and cooling device) support to > the lima driver. > I didn't have much time to do in-depth testing. However, I'm sending > this out early because there are many SoCs with Mali-400/450 GPU so > I want to avoid duplicating the work with somebody else. > > The code is derived from panfrost_devfreq.c which is why I kept the > Collabora copyright in lima_devfreq.c. Please let me know if I should > drop this or how I can make it more clear that I "borrowed" the code > from panfrost. > > I am seeking comments in two general areas: > - regarding the integration into the existing lima code > - for the actual devfreq code (I had to adapt the panfrost code > slightly, because lima uses a bus and a GPU/core clock) > > My own TODO list includes "more" testing on various Amlogic SoCs. > So far I have tested this on Meson8b and Meson8m2 (which both have a > GPU OPP table defined). However, I still need to test this on a GXL > board (which is currently missing the GPU OPP table). > > > Martin Blumenstingl (1): > drm/lima: Add optional devfreq support > > drivers/gpu/drm/lima/Kconfig | 1 + > drivers/gpu/drm/lima/Makefile | 3 +- > drivers/gpu/drm/lima/lima_devfreq.c | 162 ++++++++++++++++++++++++++++ > drivers/gpu/drm/lima/lima_devfreq.h | 15 +++ > drivers/gpu/drm/lima/lima_device.c | 4 + > drivers/gpu/drm/lima/lima_device.h | 11 ++ > drivers/gpu/drm/lima/lima_drv.c | 14 ++- > drivers/gpu/drm/lima/lima_sched.c | 7 ++ > drivers/gpu/drm/lima/lima_sched.h | 3 + > 9 files changed, 217 insertions(+), 3 deletions(-) > create mode 100644 drivers/gpu/drm/lima/lima_devfreq.c > create mode 100644 drivers/gpu/drm/lima/lima_devfreq.h > > -- > 2.24.1 >
On Mon, Dec 16, 2019 at 3:51 AM Qiang Yu <yuq825@gmail.com> wrote: [...] > For the code, I think you may need some lock to protect the time records as > there are two kernel threads gp/pp will try to mark GPU busy and several > interrupts try to mark GPU idle. good catch, thank you for this! I assume the reason is that the panfrost GPUs are using a "unified" architecture, while the ones supported by lima don't I'll add locking so I don't run into trouble. Thank you! Martin
On Mon, Dec 16, 2019 at 4:03 AM Chen-Yu Tsai <wens@csie.org> wrote: > > On Mon, Dec 16, 2019 at 5:12 AM Martin Blumenstingl > <martin.blumenstingl@googlemail.com> wrote: > > > > This is my attempt at adding devfreq (and cooling device) support to > > the lima driver. > > I didn't have much time to do in-depth testing. However, I'm sending > > this out early because there are many SoCs with Mali-400/450 GPU so > > I want to avoid duplicating the work with somebody else. > > > > The code is derived from panfrost_devfreq.c which is why I kept the > > Collabora copyright in lima_devfreq.c. Please let me know if I should > > drop this or how I can make it more clear that I "borrowed" the code > > from panfrost. > > I think it's more common to have separate copyright notices. First you > have yours, then a second paragraph stating the code is derived from > foo, and then attach the copyright statements for foo. thank you for the hint! I found other examples that do it like this, so I'll update it. Martin
Hi Alyssa, On Mon, Dec 16, 2019 at 4:48 PM Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com> wrote: > > If so much code is being duplicated over, I'm wondering if it makes > sense for us to move some of the common devfreq code to core DRM > helpers? if you have any recommendation where to put it then please let me know (I am not familiar with the DRM subsystem at all) my initial idea was that the devfreq logic needs the same information that the scheduler needs (whether we're submitting something to be executed, there was a timeout, ...). however, looking at drivers/gpu/drm/scheduler/ this seems pretty stand-alone so I'm not sure it should go there also the Mali-4x0 GPUs have some "PMU" which *may* be used instead of polling the statistics internally so this is where I realize that with my current knowledge I don't know enough about lima, panfrost, DRM or the devfreq subsystem to get a good idea where to put the code. Martin
于 2019年12月24日 GMT+08:00 下午7:28:41, Martin Blumenstingl <martin.blumenstingl@googlemail.com> 写到: >Hi Alyssa, > >On Mon, Dec 16, 2019 at 4:48 PM Alyssa Rosenzweig ><alyssa.rosenzweig@collabora.com> wrote: >> >> If so much code is being duplicated over, I'm wondering if it makes >> sense for us to move some of the common devfreq code to core DRM >> helpers? >if you have any recommendation where to put it then please let me know >(I am not familiar with the DRM subsystem at all) > >my initial idea was that the devfreq logic needs the same information >that the scheduler needs (whether we're submitting something to be >executed, there was a timeout, ...). >however, looking at drivers/gpu/drm/scheduler/ this seems pretty >stand-alone so I'm not sure it should go there >also the Mali-4x0 GPUs have some "PMU" which *may* be used instead of It's optional. We cannot promise its existance on a given hardware, and I heard that at least on Allwinner H5 Mali PMU is broken. >polling the statistics internally >so this is where I realize that with my current knowledge I don't know >enough about lima, panfrost, DRM or the devfreq subsystem to get a >good idea where to put the code. > > >Martin >_______________________________________________ >lima mailing list >lima@lists.freedesktop.org >https://lists.freedesktop.org/mailman/listinfo/lima