Use the pm_ptr() macro to handle the .suspend / .resume / .reset_resume
callbacks.
This macro allows the suspend and resume functions to be automatically
dropped by the compiler when CONFIG_PM is disabled, without having
to use #ifdef guards.
This has the advantage of always compiling these functions in,
independently of any Kconfig option. Thanks to that, bugs and other
regressions are subsequently easier to catch. It also allows to drop the
__maybe_unused tags.
Signed-off-by: Paul Cercueil <[email protected]>
---
Cc: Hans de Goede <[email protected]>
---
drivers/gpu/drm/tiny/gm12u320.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/tiny/gm12u320.c b/drivers/gpu/drm/tiny/gm12u320.c
index 7441d992a5d7..0a901201142e 100644
--- a/drivers/gpu/drm/tiny/gm12u320.c
+++ b/drivers/gpu/drm/tiny/gm12u320.c
@@ -4,6 +4,7 @@
*/
#include <linux/module.h>
+#include <linux/pm.h>
#include <linux/usb.h>
#include <drm/drm_atomic_helper.h>
@@ -718,15 +719,15 @@ static void gm12u320_usb_disconnect(struct usb_interface *interface)
drm_atomic_helper_shutdown(dev);
}
-static __maybe_unused int gm12u320_suspend(struct usb_interface *interface,
- pm_message_t message)
+static int gm12u320_suspend(struct usb_interface *interface,
+ pm_message_t message)
{
struct drm_device *dev = usb_get_intfdata(interface);
return drm_mode_config_helper_suspend(dev);
}
-static __maybe_unused int gm12u320_resume(struct usb_interface *interface)
+static int gm12u320_resume(struct usb_interface *interface)
{
struct drm_device *dev = usb_get_intfdata(interface);
struct gm12u320_device *gm12u320 = to_gm12u320(dev);
@@ -747,11 +748,9 @@ static struct usb_driver gm12u320_usb_driver = {
.probe = gm12u320_usb_probe,
.disconnect = gm12u320_usb_disconnect,
.id_table = id_table,
-#ifdef CONFIG_PM
- .suspend = gm12u320_suspend,
- .resume = gm12u320_resume,
- .reset_resume = gm12u320_resume,
-#endif
+ .suspend = pm_ptr(gm12u320_suspend),
+ .resume = pm_ptr(gm12u320_resume),
+ .reset_resume = pm_ptr(gm12u320_resume),
};
module_usb_driver(gm12u320_usb_driver);
--
2.35.1
Instead of defining two versions of intel_sysfs_rc6_init(), one for each
value of CONFIG_PM, add a check on !IS_ENABLED(CONFIG_PM) early in the
function. This will allow the compiler to automatically drop the dead
code when CONFIG_PM is disabled, without having to use #ifdef guards.
This has the advantage of always compiling these functions in,
independently of any Kconfig option. Thanks to that, bugs and other
regressions are subsequently easier to catch.
Signed-off-by: Paul Cercueil <[email protected]>
---
Cc: Jani Nikula <[email protected]>
Cc: Joonas Lahtinen <[email protected]>
Cc: Rodrigo Vivi <[email protected]>
Cc: Tvrtko Ursulin <[email protected]>
Cc: [email protected]
---
drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
index 2b5f05b31187..decf892a4508 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
@@ -164,7 +164,6 @@ sysfs_gt_attribute_r_func(struct kobject *kobj, struct attribute *attr,
NULL); \
INTEL_GT_ATTR_RO(_name)
-#ifdef CONFIG_PM
static u32 get_residency(struct intel_gt *gt, i915_reg_t reg)
{
intel_wakeref_t wakeref;
@@ -300,7 +299,7 @@ static void intel_sysfs_rc6_init(struct intel_gt *gt, struct kobject *kobj)
{
int ret;
- if (!HAS_RC6(gt->i915))
+ if (!IS_ENABLED(CONFIG_PM) || !HAS_RC6(gt->i915))
return;
ret = __intel_gt_sysfs_create_group(kobj, rc6_attr_group);
@@ -329,11 +328,6 @@ static void intel_sysfs_rc6_init(struct intel_gt *gt, struct kobject *kobj)
gt->info.id, ERR_PTR(ret));
}
}
-#else
-static void intel_sysfs_rc6_init(struct intel_gt *gt, struct kobject *kobj)
-{
-}
-#endif /* CONFIG_PM */
static u32 __act_freq_mhz_show(struct intel_gt *gt)
{
--
2.35.1
Hi,
On 11/7/22 18:55, Paul Cercueil wrote:
> Use the pm_ptr() macro to handle the .suspend / .resume / .reset_resume
> callbacks.
>
> This macro allows the suspend and resume functions to be automatically
> dropped by the compiler when CONFIG_PM is disabled, without having
> to use #ifdef guards.
>
> This has the advantage of always compiling these functions in,
> independently of any Kconfig option. Thanks to that, bugs and other
> regressions are subsequently easier to catch. It also allows to drop the
> __maybe_unused tags.
>
> Signed-off-by: Paul Cercueil <[email protected]>
> ---
> Cc: Hans de Goede <[email protected]>
Thanks, patch looks good to me:
Reviewed-by: Hans de Goede <[email protected]>
Regards,
Hans
> ---
> drivers/gpu/drm/tiny/gm12u320.c | 15 +++++++--------
> 1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/tiny/gm12u320.c b/drivers/gpu/drm/tiny/gm12u320.c
> index 7441d992a5d7..0a901201142e 100644
> --- a/drivers/gpu/drm/tiny/gm12u320.c
> +++ b/drivers/gpu/drm/tiny/gm12u320.c
> @@ -4,6 +4,7 @@
> */
>
> #include <linux/module.h>
> +#include <linux/pm.h>
> #include <linux/usb.h>
>
> #include <drm/drm_atomic_helper.h>
> @@ -718,15 +719,15 @@ static void gm12u320_usb_disconnect(struct usb_interface *interface)
> drm_atomic_helper_shutdown(dev);
> }
>
> -static __maybe_unused int gm12u320_suspend(struct usb_interface *interface,
> - pm_message_t message)
> +static int gm12u320_suspend(struct usb_interface *interface,
> + pm_message_t message)
> {
> struct drm_device *dev = usb_get_intfdata(interface);
>
> return drm_mode_config_helper_suspend(dev);
> }
>
> -static __maybe_unused int gm12u320_resume(struct usb_interface *interface)
> +static int gm12u320_resume(struct usb_interface *interface)
> {
> struct drm_device *dev = usb_get_intfdata(interface);
> struct gm12u320_device *gm12u320 = to_gm12u320(dev);
> @@ -747,11 +748,9 @@ static struct usb_driver gm12u320_usb_driver = {
> .probe = gm12u320_usb_probe,
> .disconnect = gm12u320_usb_disconnect,
> .id_table = id_table,
> -#ifdef CONFIG_PM
> - .suspend = gm12u320_suspend,
> - .resume = gm12u320_resume,
> - .reset_resume = gm12u320_resume,
> -#endif
> + .suspend = pm_ptr(gm12u320_suspend),
> + .resume = pm_ptr(gm12u320_resume),
> + .reset_resume = pm_ptr(gm12u320_resume),
> };
>
> module_usb_driver(gm12u320_usb_driver);
On Mon, Nov 07, 2022 at 05:55:10PM +0000, Paul Cercueil wrote:
> Instead of defining two versions of intel_sysfs_rc6_init(), one for each
> value of CONFIG_PM, add a check on !IS_ENABLED(CONFIG_PM) early in the
> function. This will allow the compiler to automatically drop the dead
> code when CONFIG_PM is disabled, without having to use #ifdef guards.
Making the RC6 to depend on the CONFIG_PM is probably an historical
mistake and probably the right solution is simply to remove that
dependency.
>
> This has the advantage of always compiling these functions in,
> independently of any Kconfig option. Thanks to that, bugs and other
> regressions are subsequently easier to catch.
>
> Signed-off-by: Paul Cercueil <[email protected]>
> ---
> Cc: Jani Nikula <[email protected]>
> Cc: Joonas Lahtinen <[email protected]>
> Cc: Rodrigo Vivi <[email protected]>
> Cc: Tvrtko Ursulin <[email protected]>
> Cc: [email protected]
> ---
> drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c | 8 +-------
> 1 file changed, 1 insertion(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
> index 2b5f05b31187..decf892a4508 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
> @@ -164,7 +164,6 @@ sysfs_gt_attribute_r_func(struct kobject *kobj, struct attribute *attr,
> NULL); \
> INTEL_GT_ATTR_RO(_name)
>
> -#ifdef CONFIG_PM
> static u32 get_residency(struct intel_gt *gt, i915_reg_t reg)
> {
> intel_wakeref_t wakeref;
> @@ -300,7 +299,7 @@ static void intel_sysfs_rc6_init(struct intel_gt *gt, struct kobject *kobj)
> {
> int ret;
>
> - if (!HAS_RC6(gt->i915))
> + if (!IS_ENABLED(CONFIG_PM) || !HAS_RC6(gt->i915))
> return;
>
> ret = __intel_gt_sysfs_create_group(kobj, rc6_attr_group);
> @@ -329,11 +328,6 @@ static void intel_sysfs_rc6_init(struct intel_gt *gt, struct kobject *kobj)
> gt->info.id, ERR_PTR(ret));
> }
> }
> -#else
> -static void intel_sysfs_rc6_init(struct intel_gt *gt, struct kobject *kobj)
> -{
> -}
> -#endif /* CONFIG_PM */
>
> static u32 __act_freq_mhz_show(struct intel_gt *gt)
> {
> --
> 2.35.1
>
Hi Rodrigo,
Le lun. 7 nov. 2022 ? 15:31:49 -0500, Rodrigo Vivi
<[email protected]> a ?crit :
> On Mon, Nov 07, 2022 at 05:55:10PM +0000, Paul Cercueil wrote:
>> Instead of defining two versions of intel_sysfs_rc6_init(), one for
>> each
>> value of CONFIG_PM, add a check on !IS_ENABLED(CONFIG_PM) early in
>> the
>> function. This will allow the compiler to automatically drop the
>> dead
>> code when CONFIG_PM is disabled, without having to use #ifdef
>> guards.
>
> Making the RC6 to depend on the CONFIG_PM is probably an historical
> mistake and probably the right solution is simply to remove that
> dependency.
I don't know anything about i915, so the best I can do is update the
code without changing the functionality.
Then someone with a better understanding can send a patch to remove the
check on CONFIG_PM.
Cheers,
-Paul
>>
>> This has the advantage of always compiling these functions in,
>> independently of any Kconfig option. Thanks to that, bugs and other
>> regressions are subsequently easier to catch.
>>
>> Signed-off-by: Paul Cercueil <[email protected]>
>> ---
>> Cc: Jani Nikula <[email protected]>
>> Cc: Joonas Lahtinen <[email protected]>
>> Cc: Rodrigo Vivi <[email protected]>
>> Cc: Tvrtko Ursulin <[email protected]>
>> Cc: [email protected]
>> ---
>> drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c | 8 +-------
>> 1 file changed, 1 insertion(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
>> b/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
>> index 2b5f05b31187..decf892a4508 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
>> @@ -164,7 +164,6 @@ sysfs_gt_attribute_r_func(struct kobject *kobj,
>> struct attribute *attr,
>> NULL); \
>> INTEL_GT_ATTR_RO(_name)
>>
>> -#ifdef CONFIG_PM
>> static u32 get_residency(struct intel_gt *gt, i915_reg_t reg)
>> {
>> intel_wakeref_t wakeref;
>> @@ -300,7 +299,7 @@ static void intel_sysfs_rc6_init(struct
>> intel_gt *gt, struct kobject *kobj)
>> {
>> int ret;
>>
>> - if (!HAS_RC6(gt->i915))
>> + if (!IS_ENABLED(CONFIG_PM) || !HAS_RC6(gt->i915))
>> return;
>>
>> ret = __intel_gt_sysfs_create_group(kobj, rc6_attr_group);
>> @@ -329,11 +328,6 @@ static void intel_sysfs_rc6_init(struct
>> intel_gt *gt, struct kobject *kobj)
>> gt->info.id, ERR_PTR(ret));
>> }
>> }
>> -#else
>> -static void intel_sysfs_rc6_init(struct intel_gt *gt, struct
>> kobject *kobj)
>> -{
>> -}
>> -#endif /* CONFIG_PM */
>>
>> static u32 __act_freq_mhz_show(struct intel_gt *gt)
>> {
>> --
>> 2.35.1
>>