Hi Javier,
On Fri Jun 27, 2025 at 9:51 AM CEST, Javier Martinez Canillas wrote:
Luca Weiss [off-list ref] writes:
quoted
Some devices might require keeping an interconnect path alive so that
the framebuffer continues working. Add support for that by setting the
bandwidth requirements appropriately for all provided interconnect
paths.
Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Luca Weiss <redacted>
---
drivers/gpu/drm/sysfb/simpledrm.c | 83 +++++++++++++++++++++++++++++++++++++++
1 file changed, 83 insertions(+)
diff --git a/drivers/gpu/drm/sysfb/simpledrm.c b/drivers/gpu/drm/sysfb/simpledrm.c
index 349219330314e3421a6bb26ad5cf39a679a5cb7a..47d213e20cab1dd1e19528674a95edea00f4bb30 100644
--- a/drivers/gpu/drm/sysfb/simpledrm.c
+++ b/drivers/gpu/drm/sysfb/simpledrm.c
@@ -2,6 +2,7 @@
#include <linux/aperture.h>
#include <linux/clk.h>
+#include <linux/interconnect.h>
#include <linux/minmax.h>
#include <linux/of_address.h>
#include <linux/of_clk.h>
@@ -225,6 +226,10 @@ struct simpledrm_device {
struct device **pwr_dom_devs;
struct device_link **pwr_dom_links;
#endif
Can you add a /* interconnects */ comment here? Similarly how there is one
for clocks, regulators, power domains, etc.
Sure!
quoted
+#if defined CONFIG_OF && defined CONFIG_INTERCONNECT
+ unsigned int icc_count;
+ struct icc_path **icc_paths;
+#endif
...
quoted
+static int simpledrm_device_attach_icc(struct simpledrm_device *sdev)
+{
+ struct device *dev = sdev->sysfb.dev.dev;
+ int ret, count, i;
+
+ count = of_count_phandle_with_args(dev->of_node, "interconnects",
+ "#interconnect-cells");
+ if (count < 0)
+ return 0;
+
+ /* An interconnect path consists of two elements */
+ if (count % 2) {
+ drm_err(&sdev->sysfb.dev,
+ "invalid interconnects value\n");
+ return -EINVAL;
+ }
+ sdev->icc_count = count / 2;
+
+ sdev->icc_paths = devm_kcalloc(dev, sdev->icc_count,
+ sizeof(*sdev->icc_paths),
+ GFP_KERNEL);
+ if (!sdev->icc_paths)
+ return -ENOMEM;
+
+ for (i = 0; i < sdev->icc_count; i++) {
+ sdev->icc_paths[i] = of_icc_get_by_index(dev, i);
+ if (IS_ERR_OR_NULL(sdev->icc_paths[i])) {
+ ret = PTR_ERR(sdev->icc_paths[i]);
+ if (ret == -EPROBE_DEFER)
+ goto err;
+ drm_err(&sdev->sysfb.dev, "failed to get interconnect path %u: %d\n",
+ i, ret);
You could use dev_err_probe() instead that already handles the -EPROBE_DEFER
case and also will get this message in the /sys/kernel/debug/devices_deferred
debugfs entry, as the reason why the probe deferral happened.
Not quite sure how to implement dev_err_probe, but I think this should
be quite okay?
if (IS_ERR_OR_NULL(sdev->icc_paths[i])) {
ret = dev_err_probe(dev, PTR_ERR(sdev->icc_paths[i]),
"failed to get interconnect path %u\n", i);
if (ret == -EPROBE_DEFER)
goto err;
continue;
}
That would still keep the current behavior for defer vs permanent error
while printing when necessary and having it for devices_deferred for the
defer case.
Not sure what the difference between drm_err and dev_err are, but I
trust you on that.
Let me know.
Regards
Luca
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>