2023-01-26 16:38:19

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH 1/2] accep/ivpu: PM: remove broken ivpu_dbg() statements

From: Arnd Bergmann <[email protected]>

When CONFIG_PM is disabled, the driver fails to build:

drivers/accel/ivpu/ivpu_pm.c: In function 'ivpu_rpm_get':
drivers/accel/ivpu/ivpu_pm.c:240:84: error: 'struct dev_pm_info' has no member named 'usage_count'
240 | ivpu_dbg(vdev, RPM, "rpm_get count %d\n", atomic_read(&vdev->drm.dev->power.usage_count));
| ^
include/linux/dynamic_debug.h:223:29: note: in definition of macro '__dynamic_func_call_cls'
223 | func(&id, ##__VA_ARGS__); \
| ^~~~~~~~~~~
include/linux/dynamic_debug.h:249:9: note: in expansion of macro '_dynamic_func_call_cls'
249 | _dynamic_func_call_cls(_DPRINTK_CLASS_DFLT, fmt, func, ##__VA_ARGS__)
| ^~~~~~~~~~~~~~~~~~~~~~
include/linux/dynamic_debug.h:272:9: note: in expansion of macro '_dynamic_func_call'
272 | _dynamic_func_call(fmt, __dynamic_dev_dbg, \
| ^~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:155:9: note: in expansion of macro 'dynamic_dev_dbg'
155 | dynamic_dev_dbg(dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~~~~~~~~~
drivers/accel/ivpu/ivpu_drv.h:65:17: note: in expansion of macro 'dev_dbg'
65 | dev_dbg((vdev)->drm.dev, "[%s] " fmt, #type, ##args); \
| ^~~~~~~
drivers/accel/ivpu/ivpu_pm.c:240:9: note: in expansion of macro 'ivpu_dbg'
240 | ivpu_dbg(vdev, RPM, "rpm_get count %d\n", atomic_read(&vdev->drm.dev->power.usage_count));
| ^~~~~~~~

It would be possible to rework these statements to only conditionally print
the reference counter, or to make the driver depend on CONFIG_PM, but my
impression is that these are not actually needed at all if the driver generally
works, or they could be put back when required. Just remove all four of these
to make the driver build in all configurations.

Fixes: 852be13f3bd3 ("accel/ivpu: Add PM support")
Signed-off-by: Arnd Bergmann <[email protected]>
---
drivers/accel/ivpu/ivpu_pm.c | 10 ----------
1 file changed, 10 deletions(-)

diff --git a/drivers/accel/ivpu/ivpu_pm.c b/drivers/accel/ivpu/ivpu_pm.c
index 553bcbd787b3..a880f1dd857e 100644
--- a/drivers/accel/ivpu/ivpu_pm.c
+++ b/drivers/accel/ivpu/ivpu_pm.c
@@ -237,8 +237,6 @@ int ivpu_rpm_get(struct ivpu_device *vdev)
{
int ret;

- ivpu_dbg(vdev, RPM, "rpm_get count %d\n", atomic_read(&vdev->drm.dev->power.usage_count));
-
ret = pm_runtime_resume_and_get(vdev->drm.dev);
if (!drm_WARN_ON(&vdev->drm, ret < 0))
vdev->pm->suspend_reschedule_counter = PM_RESCHEDULE_LIMIT;
@@ -248,8 +246,6 @@ int ivpu_rpm_get(struct ivpu_device *vdev)

void ivpu_rpm_put(struct ivpu_device *vdev)
{
- ivpu_dbg(vdev, RPM, "rpm_put count %d\n", atomic_read(&vdev->drm.dev->power.usage_count));
-
pm_runtime_mark_last_busy(vdev->drm.dev);
pm_runtime_put_autosuspend(vdev->drm.dev);
}
@@ -314,16 +310,10 @@ void ivpu_pm_enable(struct ivpu_device *vdev)
pm_runtime_allow(dev);
pm_runtime_mark_last_busy(dev);
pm_runtime_put_autosuspend(dev);
-
- ivpu_dbg(vdev, RPM, "Enable RPM count %d\n", atomic_read(&dev->power.usage_count));
}

void ivpu_pm_disable(struct ivpu_device *vdev)
{
- struct device *dev = vdev->drm.dev;
-
- ivpu_dbg(vdev, RPM, "Disable RPM count %d\n", atomic_read(&dev->power.usage_count));
-
pm_runtime_get_noresume(vdev->drm.dev);
pm_runtime_forbid(vdev->drm.dev);
}
--
2.39.0



2023-01-26 16:38:30

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH 2/2] accel/ivpu: avoid duplciate assignment

From: Arnd Bergmann <[email protected]>

With extra warnings enabled, gcc warns about two assignments
of the same .mmap callback:

In file included from drivers/accel/ivpu/ivpu_drv.c:10:
include/drm/drm_accel.h:31:27: error: initialized field overwritten [-Werror=override-init]
31 | .mmap = drm_gem_mmap
| ^~~~~~~~~~~~
drivers/accel/ivpu/ivpu_drv.c:360:9: note: in expansion of macro 'DRM_ACCEL_FOPS'
360 | DRM_ACCEL_FOPS,
| ^~~~~~~~~~~~~~

Remove the unused local assignment.

Fixes: 20709aa9435b ("accel: Add .mmap to DRM_ACCEL_FOPS")
Signed-off-by: Arnd Bergmann <[email protected]>
---
drivers/accel/ivpu/ivpu_drv.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/drivers/accel/ivpu/ivpu_drv.c b/drivers/accel/ivpu/ivpu_drv.c
index 2bc2f1b90671..a29e8ee0dce6 100644
--- a/drivers/accel/ivpu/ivpu_drv.c
+++ b/drivers/accel/ivpu/ivpu_drv.c
@@ -356,7 +356,6 @@ int ivpu_shutdown(struct ivpu_device *vdev)

static const struct file_operations ivpu_fops = {
.owner = THIS_MODULE,
- .mmap = drm_gem_mmap,
DRM_ACCEL_FOPS,
};

--
2.39.0


2023-01-26 17:08:52

by Jeffrey Hugo

[permalink] [raw]
Subject: Re: [PATCH 1/2] accep/ivpu: PM: remove broken ivpu_dbg() statements

Subject has a typo: "accep" should be "accel"

On 1/26/2023 9:37 AM, Arnd Bergmann wrote:
> From: Arnd Bergmann <[email protected]>
>
> When CONFIG_PM is disabled, the driver fails to build:
>
> drivers/accel/ivpu/ivpu_pm.c: In function 'ivpu_rpm_get':
> drivers/accel/ivpu/ivpu_pm.c:240:84: error: 'struct dev_pm_info' has no member named 'usage_count'
> 240 | ivpu_dbg(vdev, RPM, "rpm_get count %d\n", atomic_read(&vdev->drm.dev->power.usage_count));
> | ^
> include/linux/dynamic_debug.h:223:29: note: in definition of macro '__dynamic_func_call_cls'
> 223 | func(&id, ##__VA_ARGS__); \
> | ^~~~~~~~~~~
> include/linux/dynamic_debug.h:249:9: note: in expansion of macro '_dynamic_func_call_cls'
> 249 | _dynamic_func_call_cls(_DPRINTK_CLASS_DFLT, fmt, func, ##__VA_ARGS__)
> | ^~~~~~~~~~~~~~~~~~~~~~
> include/linux/dynamic_debug.h:272:9: note: in expansion of macro '_dynamic_func_call'
> 272 | _dynamic_func_call(fmt, __dynamic_dev_dbg, \
> | ^~~~~~~~~~~~~~~~~~
> include/linux/dev_printk.h:155:9: note: in expansion of macro 'dynamic_dev_dbg'
> 155 | dynamic_dev_dbg(dev, dev_fmt(fmt), ##__VA_ARGS__)
> | ^~~~~~~~~~~~~~~
> drivers/accel/ivpu/ivpu_drv.h:65:17: note: in expansion of macro 'dev_dbg'
> 65 | dev_dbg((vdev)->drm.dev, "[%s] " fmt, #type, ##args); \
> | ^~~~~~~
> drivers/accel/ivpu/ivpu_pm.c:240:9: note: in expansion of macro 'ivpu_dbg'
> 240 | ivpu_dbg(vdev, RPM, "rpm_get count %d\n", atomic_read(&vdev->drm.dev->power.usage_count));
> | ^~~~~~~~
>
> It would be possible to rework these statements to only conditionally print
> the reference counter, or to make the driver depend on CONFIG_PM, but my
> impression is that these are not actually needed at all if the driver generally
> works, or they could be put back when required. Just remove all four of these
> to make the driver build in all configurations.
>
> Fixes: 852be13f3bd3 ("accel/ivpu: Add PM support")
> Signed-off-by: Arnd Bergmann <[email protected]>
> ---
> drivers/accel/ivpu/ivpu_pm.c | 10 ----------
> 1 file changed, 10 deletions(-)
>
> diff --git a/drivers/accel/ivpu/ivpu_pm.c b/drivers/accel/ivpu/ivpu_pm.c
> index 553bcbd787b3..a880f1dd857e 100644
> --- a/drivers/accel/ivpu/ivpu_pm.c
> +++ b/drivers/accel/ivpu/ivpu_pm.c
> @@ -237,8 +237,6 @@ int ivpu_rpm_get(struct ivpu_device *vdev)
> {
> int ret;
>
> - ivpu_dbg(vdev, RPM, "rpm_get count %d\n", atomic_read(&vdev->drm.dev->power.usage_count));
> -
> ret = pm_runtime_resume_and_get(vdev->drm.dev);
> if (!drm_WARN_ON(&vdev->drm, ret < 0))
> vdev->pm->suspend_reschedule_counter = PM_RESCHEDULE_LIMIT;
> @@ -248,8 +246,6 @@ int ivpu_rpm_get(struct ivpu_device *vdev)
>
> void ivpu_rpm_put(struct ivpu_device *vdev)
> {
> - ivpu_dbg(vdev, RPM, "rpm_put count %d\n", atomic_read(&vdev->drm.dev->power.usage_count));
> -
> pm_runtime_mark_last_busy(vdev->drm.dev);
> pm_runtime_put_autosuspend(vdev->drm.dev);
> }
> @@ -314,16 +310,10 @@ void ivpu_pm_enable(struct ivpu_device *vdev)
> pm_runtime_allow(dev);
> pm_runtime_mark_last_busy(dev);
> pm_runtime_put_autosuspend(dev);
> -
> - ivpu_dbg(vdev, RPM, "Enable RPM count %d\n", atomic_read(&dev->power.usage_count));
> }
>
> void ivpu_pm_disable(struct ivpu_device *vdev)
> {
> - struct device *dev = vdev->drm.dev;
> -
> - ivpu_dbg(vdev, RPM, "Disable RPM count %d\n", atomic_read(&dev->power.usage_count));
> -
> pm_runtime_get_noresume(vdev->drm.dev);
> pm_runtime_forbid(vdev->drm.dev);
> }


2023-01-26 17:11:35

by Jeffrey Hugo

[permalink] [raw]
Subject: Re: [PATCH 2/2] accel/ivpu: avoid duplciate assignment

duplciate -> duplicate

On 1/26/2023 9:37 AM, Arnd Bergmann wrote:
> From: Arnd Bergmann <[email protected]>
>
> With extra warnings enabled, gcc warns about two assignments
> of the same .mmap callback:
>
> In file included from drivers/accel/ivpu/ivpu_drv.c:10:
> include/drm/drm_accel.h:31:27: error: initialized field overwritten [-Werror=override-init]
> 31 | .mmap = drm_gem_mmap
> | ^~~~~~~~~~~~
> drivers/accel/ivpu/ivpu_drv.c:360:9: note: in expansion of macro 'DRM_ACCEL_FOPS'
> 360 | DRM_ACCEL_FOPS,
> | ^~~~~~~~~~~~~~
>
> Remove the unused local assignment.
>
> Fixes: 20709aa9435b ("accel: Add .mmap to DRM_ACCEL_FOPS")
> Signed-off-by: Arnd Bergmann <[email protected]>

Yep, two things in flight at the same time. Thanks for getting to the
cleanup before I did.

Reviewed-by: Jeffrey Hugo <[email protected]>



2023-01-27 09:16:25

by Stanislaw Gruszka

[permalink] [raw]
Subject: Re: [PATCH 1/2] accep/ivpu: PM: remove broken ivpu_dbg() statements

On Thu, Jan 26, 2023 at 05:37:54PM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann <[email protected]>
>
> When CONFIG_PM is disabled, the driver fails to build:
>
> drivers/accel/ivpu/ivpu_pm.c: In function 'ivpu_rpm_get':
> drivers/accel/ivpu/ivpu_pm.c:240:84: error: 'struct dev_pm_info' has no member named 'usage_count'
> 240 | ivpu_dbg(vdev, RPM, "rpm_get count %d\n", atomic_read(&vdev->drm.dev->power.usage_count));
> | ^
> include/linux/dynamic_debug.h:223:29: note: in definition of macro '__dynamic_func_call_cls'
> 223 | func(&id, ##__VA_ARGS__); \
> | ^~~~~~~~~~~
> include/linux/dynamic_debug.h:249:9: note: in expansion of macro '_dynamic_func_call_cls'
> 249 | _dynamic_func_call_cls(_DPRINTK_CLASS_DFLT, fmt, func, ##__VA_ARGS__)
> | ^~~~~~~~~~~~~~~~~~~~~~
> include/linux/dynamic_debug.h:272:9: note: in expansion of macro '_dynamic_func_call'
> 272 | _dynamic_func_call(fmt, __dynamic_dev_dbg, \
> | ^~~~~~~~~~~~~~~~~~
> include/linux/dev_printk.h:155:9: note: in expansion of macro 'dynamic_dev_dbg'
> 155 | dynamic_dev_dbg(dev, dev_fmt(fmt), ##__VA_ARGS__)
> | ^~~~~~~~~~~~~~~
> drivers/accel/ivpu/ivpu_drv.h:65:17: note: in expansion of macro 'dev_dbg'
> 65 | dev_dbg((vdev)->drm.dev, "[%s] " fmt, #type, ##args); \
> | ^~~~~~~
> drivers/accel/ivpu/ivpu_pm.c:240:9: note: in expansion of macro 'ivpu_dbg'
> 240 | ivpu_dbg(vdev, RPM, "rpm_get count %d\n", atomic_read(&vdev->drm.dev->power.usage_count));
> | ^~~~~~~~
>
> It would be possible to rework these statements to only conditionally print
> the reference counter, or to make the driver depend on CONFIG_PM, but my
> impression is that these are not actually needed at all if the driver generally
> works, or they could be put back when required. Just remove all four of these
> to make the driver build in all configurations.
>
> Fixes: 852be13f3bd3 ("accel/ivpu: Add PM support")
> Signed-off-by: Arnd Bergmann <[email protected]>

Reviewed-by: Stanislaw Gruszka <[email protected]>

2023-01-31 09:22:39

by Jacek Lawrynowicz

[permalink] [raw]
Subject: Re: [PATCH 1/2] accep/ivpu: PM: remove broken ivpu_dbg() statements

Applied to drm-misc-next.
Thanks.

On 26.01.2023 17:37, Arnd Bergmann wrote:
> From: Arnd Bergmann <[email protected]>
>
> When CONFIG_PM is disabled, the driver fails to build:
>
> drivers/accel/ivpu/ivpu_pm.c: In function 'ivpu_rpm_get':
> drivers/accel/ivpu/ivpu_pm.c:240:84: error: 'struct dev_pm_info' has no member named 'usage_count'
> 240 | ivpu_dbg(vdev, RPM, "rpm_get count %d\n", atomic_read(&vdev->drm.dev->power.usage_count));
> | ^
> include/linux/dynamic_debug.h:223:29: note: in definition of macro '__dynamic_func_call_cls'
> 223 | func(&id, ##__VA_ARGS__); \
> | ^~~~~~~~~~~
> include/linux/dynamic_debug.h:249:9: note: in expansion of macro '_dynamic_func_call_cls'
> 249 | _dynamic_func_call_cls(_DPRINTK_CLASS_DFLT, fmt, func, ##__VA_ARGS__)
> | ^~~~~~~~~~~~~~~~~~~~~~
> include/linux/dynamic_debug.h:272:9: note: in expansion of macro '_dynamic_func_call'
> 272 | _dynamic_func_call(fmt, __dynamic_dev_dbg, \
> | ^~~~~~~~~~~~~~~~~~
> include/linux/dev_printk.h:155:9: note: in expansion of macro 'dynamic_dev_dbg'
> 155 | dynamic_dev_dbg(dev, dev_fmt(fmt), ##__VA_ARGS__)
> | ^~~~~~~~~~~~~~~
> drivers/accel/ivpu/ivpu_drv.h:65:17: note: in expansion of macro 'dev_dbg'
> 65 | dev_dbg((vdev)->drm.dev, "[%s] " fmt, #type, ##args); \
> | ^~~~~~~
> drivers/accel/ivpu/ivpu_pm.c:240:9: note: in expansion of macro 'ivpu_dbg'
> 240 | ivpu_dbg(vdev, RPM, "rpm_get count %d\n", atomic_read(&vdev->drm.dev->power.usage_count));
> | ^~~~~~~~
>
> It would be possible to rework these statements to only conditionally print
> the reference counter, or to make the driver depend on CONFIG_PM, but my
> impression is that these are not actually needed at all if the driver generally
> works, or they could be put back when required. Just remove all four of these
> to make the driver build in all configurations.
>
> Fixes: 852be13f3bd3 ("accel/ivpu: Add PM support")
> Signed-off-by: Arnd Bergmann <[email protected]>
> ---
> drivers/accel/ivpu/ivpu_pm.c | 10 ----------
> 1 file changed, 10 deletions(-)
>
> diff --git a/drivers/accel/ivpu/ivpu_pm.c b/drivers/accel/ivpu/ivpu_pm.c
> index 553bcbd787b3..a880f1dd857e 100644
> --- a/drivers/accel/ivpu/ivpu_pm.c
> +++ b/drivers/accel/ivpu/ivpu_pm.c
> @@ -237,8 +237,6 @@ int ivpu_rpm_get(struct ivpu_device *vdev)
> {
> int ret;
>
> - ivpu_dbg(vdev, RPM, "rpm_get count %d\n", atomic_read(&vdev->drm.dev->power.usage_count));
> -
> ret = pm_runtime_resume_and_get(vdev->drm.dev);
> if (!drm_WARN_ON(&vdev->drm, ret < 0))
> vdev->pm->suspend_reschedule_counter = PM_RESCHEDULE_LIMIT;
> @@ -248,8 +246,6 @@ int ivpu_rpm_get(struct ivpu_device *vdev)
>
> void ivpu_rpm_put(struct ivpu_device *vdev)
> {
> - ivpu_dbg(vdev, RPM, "rpm_put count %d\n", atomic_read(&vdev->drm.dev->power.usage_count));
> -
> pm_runtime_mark_last_busy(vdev->drm.dev);
> pm_runtime_put_autosuspend(vdev->drm.dev);
> }
> @@ -314,16 +310,10 @@ void ivpu_pm_enable(struct ivpu_device *vdev)
> pm_runtime_allow(dev);
> pm_runtime_mark_last_busy(dev);
> pm_runtime_put_autosuspend(dev);
> -
> - ivpu_dbg(vdev, RPM, "Enable RPM count %d\n", atomic_read(&dev->power.usage_count));
> }
>
> void ivpu_pm_disable(struct ivpu_device *vdev)
> {
> - struct device *dev = vdev->drm.dev;
> -
> - ivpu_dbg(vdev, RPM, "Disable RPM count %d\n", atomic_read(&dev->power.usage_count));
> -
> pm_runtime_get_noresume(vdev->drm.dev);
> pm_runtime_forbid(vdev->drm.dev);
> }

2023-01-31 14:00:20

by Jacek Lawrynowicz

[permalink] [raw]
Subject: Re: [PATCH 2/2] accel/ivpu: avoid duplciate assignment

Applied to drm-misc-next. Thanks.

On 26.01.2023 17:37, Arnd Bergmann wrote:
> From: Arnd Bergmann <[email protected]>
>
> With extra warnings enabled, gcc warns about two assignments
> of the same .mmap callback:
>
> In file included from drivers/accel/ivpu/ivpu_drv.c:10:
> include/drm/drm_accel.h:31:27: error: initialized field overwritten [-Werror=override-init]
> 31 | .mmap = drm_gem_mmap
> | ^~~~~~~~~~~~
> drivers/accel/ivpu/ivpu_drv.c:360:9: note: in expansion of macro 'DRM_ACCEL_FOPS'
> 360 | DRM_ACCEL_FOPS,
> | ^~~~~~~~~~~~~~
>
> Remove the unused local assignment.
>
> Fixes: 20709aa9435b ("accel: Add .mmap to DRM_ACCEL_FOPS")
> Signed-off-by: Arnd Bergmann <[email protected]>
> ---
> drivers/accel/ivpu/ivpu_drv.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/accel/ivpu/ivpu_drv.c b/drivers/accel/ivpu/ivpu_drv.c
> index 2bc2f1b90671..a29e8ee0dce6 100644
> --- a/drivers/accel/ivpu/ivpu_drv.c
> +++ b/drivers/accel/ivpu/ivpu_drv.c
> @@ -356,7 +356,6 @@ int ivpu_shutdown(struct ivpu_device *vdev)
>
> static const struct file_operations ivpu_fops = {
> .owner = THIS_MODULE,
> - .mmap = drm_gem_mmap,
> DRM_ACCEL_FOPS,
> };
>

2023-02-01 10:38:27

by Thomas Zimmermann

[permalink] [raw]
Subject: Re: [PATCH 2/2] accel/ivpu: avoid duplciate assignment



Am 31.01.23 um 15:00 schrieb Jacek Lawrynowicz:
> Applied to drm-misc-next. Thanks.

I have cherry-picked the patch into drm-misc-next-fixes.

>
> On 26.01.2023 17:37, Arnd Bergmann wrote:
>> From: Arnd Bergmann <[email protected]>
>>
>> With extra warnings enabled, gcc warns about two assignments
>> of the same .mmap callback:
>>
>> In file included from drivers/accel/ivpu/ivpu_drv.c:10:
>> include/drm/drm_accel.h:31:27: error: initialized field overwritten [-Werror=override-init]
>> 31 | .mmap = drm_gem_mmap
>> | ^~~~~~~~~~~~
>> drivers/accel/ivpu/ivpu_drv.c:360:9: note: in expansion of macro 'DRM_ACCEL_FOPS'
>> 360 | DRM_ACCEL_FOPS,
>> | ^~~~~~~~~~~~~~
>>
>> Remove the unused local assignment.
>>
>> Fixes: 20709aa9435b ("accel: Add .mmap to DRM_ACCEL_FOPS")
>> Signed-off-by: Arnd Bergmann <[email protected]>
>> ---
>> drivers/accel/ivpu/ivpu_drv.c | 1 -
>> 1 file changed, 1 deletion(-)
>>
>> diff --git a/drivers/accel/ivpu/ivpu_drv.c b/drivers/accel/ivpu/ivpu_drv.c
>> index 2bc2f1b90671..a29e8ee0dce6 100644
>> --- a/drivers/accel/ivpu/ivpu_drv.c
>> +++ b/drivers/accel/ivpu/ivpu_drv.c
>> @@ -356,7 +356,6 @@ int ivpu_shutdown(struct ivpu_device *vdev)
>>
>> static const struct file_operations ivpu_fops = {
>> .owner = THIS_MODULE,
>> - .mmap = drm_gem_mmap,
>> DRM_ACCEL_FOPS,
>> };
>>

--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev


Attachments:
OpenPGP_signature (840.00 B)
OpenPGP digital signature