Skip to contentSkip to conversation

drm/panfrost: make devfreq optional again

PATCH Superseded Neil Armstrong 2 participants 4 messages May 31, 2019 – Jun 5, 2019

Subsystems (4)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 drm/panfrost: make devfreq optional again

Devfreq runtime usage was made mandatory, thus making panfrost fail to probe on Amlogic S912 SoCs missin the "operating-points-v2" property. Make it optional again, leaving PM_DEVFREQ is selected by default.

Fixes f3617b449d ("drm/panfrost: Select devfreq")
---
 drivers/gpu/drm/panfrost/panfrost_devfreq.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)
drivers/gpu/drm/panfrost/panfrost_devfreq.c +12 -1
index 29fcffdf2d57..dc75f99ad53b 100644
@@ -140,7 +140,9 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev)
 		return 0;
 
 	ret = dev_pm_opp_of_add_table(&pfdev->pdev->dev);
-	if (ret)
+	if (ret == -ENODEV) /* Optional, continue without devfreq */
+		return 0;
+	else
 		return ret;
 
 	panfrost_devfreq_reset(pfdev);
@@ -170,6 +172,9 @@ void panfrost_devfreq_resume(struct panfrost_device *pfdev)
 {
 	int i;
 
+	if (!pfdev->devfreq.devfreq)
+		return;
+
 	panfrost_devfreq_reset(pfdev);
 	for (i = 0; i < NUM_JOB_SLOTS; i++)
 		pfdev->devfreq.slot[i].busy = false;
@@ -179,6 +184,9 @@ void panfrost_devfreq_resume(struct panfrost_device *pfdev)
 
 void panfrost_devfreq_suspend(struct panfrost_device *pfdev)
 {
+	if (!pfdev->devfreq.devfreq)
+		return;
+
 	devfreq_suspend_device(pfdev->devfreq.devfreq);
 }
 
@@ -188,6 +196,9 @@ static void panfrost_devfreq_update_utilization(struct panfrost_device *pfdev, i
 	ktime_t now;
 	ktime_t last;
 
+	if (!pfdev->devfreq.devfreq)
+		return;
+
 	now = ktime_get();
 	last = pfdev->devfreq.slot[slot].time_last_update;
 
Signature · 6 lines
2.21.0

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
#1 lore raw
Quoted 46 lines · Neil Armstrong

Devfreq runtime usage was made mandatory, thus making panfrost fail to probe on Amlogic S912 SoCs missin the "operating-points-v2" property. Make it optional again, leaving PM_DEVFREQ is selected by default.

Fixes f3617b449d ("drm/panfrost: Select devfreq")
---
 drivers/gpu/drm/panfrost/panfrost_devfreq.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)
drivers/gpu/drm/panfrost/panfrost_devfreq.c +9 -1
index 29fcffdf2d57..dc75f99ad53b 100644
@@ -140,7 +140,9 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev)
 		return 0;
 
 	ret = dev_pm_opp_of_add_table(&pfdev->pdev->dev);
-	if (ret)
+	if (ret == -ENODEV) /* Optional, continue without devfreq */
+		return 0;
+	else
 		return ret;
 
 	panfrost_devfreq_reset(pfdev);
@@ -170,6 +172,9 @@ void panfrost_devfreq_resume(struct panfrost_device *pfdev)
 {
 	int i;
 
+	if (!pfdev->devfreq.devfreq)
+		return;
+
 	panfrost_devfreq_reset(pfdev);
 	for (i = 0; i < NUM_JOB_SLOTS; i++)
 		pfdev->devfreq.slot[i].busy = false;
@@ -179,6 +184,9 @@ void panfrost_devfreq_resume(struct panfrost_device *pfdev)
 
 void panfrost_devfreq_suspend(struct panfrost_device *pfdev)
 {
+	if (!pfdev->devfreq.devfreq)
+		return;
+
 	devfreq_suspend_device(pfdev->devfreq.devfreq);
 }
 

drivers/gpu/drm/panfrost/panfrost_devfreq.c static void panfrost_devfreq_update_utilization(struct panfrost_device *pfdev, i · line 204

Quoted 9 lines
drivers/gpu/drm/panfrost/panfrost_devfreq.c +3 -0
@@ -188,6 +196,9 @@ static void panfrost_devfreq_update_utilization(struct panfrost_device *pfdev, i
 	ktime_t now;
 	ktime_t last;
 
+	if (!pfdev->devfreq.devfreq)
+		return;
+
 	now = ktime_get();
 	last = pfdev->devfreq.slot[slot].time_last_update;
 

This patch should also fix RK3288 platforms, which at the moment are broken as well by my patch.

I will test and confirm next week.

Thanks for the fix,
Ezequiel

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

#2 lore raw
In reply to Neil Armstrong
Quoted 15 lines · Neil Armstrong

Devfreq runtime usage was made mandatory, thus making panfrost fail to probe on Amlogic S912 SoCs missin the "operating-points-v2" property. Make it optional again, leaving PM_DEVFREQ is selected by default.

Fixes f3617b449d ("drm/panfrost: Select devfreq")
---
 drivers/gpu/drm/panfrost/panfrost_devfreq.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)
drivers/gpu/drm/panfrost/panfrost_devfreq.c +0 -0
index 29fcffdf2d57..dc75f99ad53b 100644

drivers/gpu/drm/panfrost/panfrost_devfreq.c int panfrost_devfreq_init(struct panfrost_device *pfdev) · line 147

Quoted 9 lines
drivers/gpu/drm/panfrost/panfrost_devfreq.c +3 -1
@@ -140,7 +140,9 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev)
 		return 0;
 
 	ret = dev_pm_opp_of_add_table(&pfdev->pdev->dev);
-	if (ret)
+	if (ret == -ENODEV) /* Optional, continue without devfreq */
+		return 0;
+	else
 		return ret;
 

This looks incorrect, should be:

else if (ret)
return ret;

Otherwise, if ret == 0, we are bailing out.

Thanks,
Eze

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

#3 lore raw
Quoted 28 lines · Ezequiel Garcia
Quoted 15 lines · Neil Armstrong

Devfreq runtime usage was made mandatory, thus making panfrost fail to probe on Amlogic S912 SoCs missin the "operating-points-v2" property. Make it optional again, leaving PM_DEVFREQ is selected by default.

Fixes f3617b449d ("drm/panfrost: Select devfreq")
---
 drivers/gpu/drm/panfrost/panfrost_devfreq.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)
drivers/gpu/drm/panfrost/panfrost_devfreq.c +0 -0
index 29fcffdf2d57..dc75f99ad53b 100644

drivers/gpu/drm/panfrost/panfrost_devfreq.c int panfrost_devfreq_init(struct panfrost_device *pfdev) · line 147

Quoted 9 lines
drivers/gpu/drm/panfrost/panfrost_devfreq.c +3 -1
@@ -140,7 +140,9 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev)
 		return 0;
 
 	ret = dev_pm_opp_of_add_table(&pfdev->pdev->dev);
-	if (ret)
+	if (ret == -ENODEV) /* Optional, continue without devfreq */
+		return 0;
+	else
 		return ret;
 

This looks incorrect, should be:

Quoted 4 lines

else if (ret)
return ret;

Otherwise, if ret == 0, we are bailing out.

Indeed, thanks for spotting this...

Will resend.

Neil

Quoted 4 lines

Thanks,
Eze

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel