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.
Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
---
drivers/video/fbdev/simplefb.c | 83 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 83 insertions(+)
diff --git a/drivers/video/fbdev/simplefb.c b/drivers/video/fbdev/simplefb.c
index be95fcddce4c8ca794826b805cd7dad2985bd637..ca73e079fd13550ddc779e84db80f7f9b743d074 100644
--- a/drivers/video/fbdev/simplefb.c
+++ b/drivers/video/fbdev/simplefb.c
@@ -27,6 +27,7 @@
#include <linux/parser.h>
#include <linux/pm_domain.h>
#include <linux/regulator/consumer.h>
+#include <linux/interconnect.h>
static const struct fb_fix_screeninfo simplefb_fix = {
.id = "simple",
@@ -89,6 +90,10 @@ struct simplefb_par {
u32 regulator_count;
struct regulator **regulators;
#endif
+#if defined CONFIG_OF && defined CONFIG_INTERCONNECT
+ unsigned int icc_count;
+ struct icc_path **icc_paths;
+#endif
};
static void simplefb_clocks_destroy(struct simplefb_par *par);
@@ -525,6 +530,80 @@ static int simplefb_attach_genpds(struct simplefb_par *par,
}
#endif
+#if defined CONFIG_OF && defined CONFIG_PM_GENERIC_DOMAINS
+/*
+ * Generic interconnect path handling code.
+ */
+static void simplefb_detach_icc(void *res)
+{
+ struct simplefb_par *par = res;
+ int i;
+
+ for (i = par->icc_count - 1; i >= 0; i--) {
+ if (!IS_ERR_OR_NULL(par->icc_paths[i]))
+ icc_put(par->icc_paths[i]);
+ }
+}
+
+static int simplefb_attach_icc(struct simplefb_par *par,
+ struct platform_device *pdev)
+{
+ struct device *dev = &pdev->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) {
+ dev_err(dev, "invalid interconnects value\n");
+ return -EINVAL;
+ }
+ par->icc_count = count / 2;
+
+ par->icc_paths = devm_kcalloc(dev, par->icc_count,
+ sizeof(*par->icc_paths),
+ GFP_KERNEL);
+ if (!par->icc_paths)
+ return -ENOMEM;
+
+ for (i = 0; i < par->icc_count; i++) {
+ par->icc_paths[i] = of_icc_get_by_index(dev, i);
+ if (IS_ERR_OR_NULL(par->icc_paths[i])) {
+ ret = PTR_ERR(par->icc_paths[i]);
+ if (ret == -EPROBE_DEFER)
+ goto err;
+ dev_err(dev, "failed to get interconnect path %u: %d\n", i, ret);
+ continue;
+ }
+
+ ret = icc_set_bw(par->icc_paths[i], 0, UINT_MAX);
+ if (ret) {
+ dev_err(dev, "failed to set interconnect bandwidth %u: %d\n", i, ret);
+ continue;
+ }
+ }
+
+ return devm_add_action_or_reset(dev, simplefb_detach_icc, par);
+
+err:
+ while (i) {
+ --i;
+ if (!IS_ERR_OR_NULL(par->icc_paths[i]))
+ icc_put(par->icc_paths[i]);
+ }
+ return ret;
+}
+#else
+static int simplefb_attach_icc(struct simplefb_par *par,
+ struct platform_device *pdev)
+{
+ return 0;
+}
+#endif
+
static int simplefb_probe(struct platform_device *pdev)
{
int ret;
@@ -615,6 +694,10 @@ static int simplefb_probe(struct platform_device *pdev)
if (ret < 0)
goto error_regulators;
+ ret = simplefb_attach_icc(par, pdev);
+ if (ret < 0)
+ goto error_regulators;
+
simplefb_clocks_enable(par, pdev);
simplefb_regulators_enable(par, pdev);
--
2.50.0Hi Luca,
kernel test robot noticed the following build errors:
[auto build test ERROR on 19272b37aa4f83ca52bdf9c16d5d81bdd1354494]
url: https://github.com/intel-lab-lkp/linux/commits/Luca-Weiss/dt-bindings-display-simple-framebuffer-Add-interconnects-property/20250620-183302
base: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494
patch link: https://lore.kernel.org/r/20250620-simple-drm-fb-icc-v1-3-d92142e8f74f%40fairphone.com
patch subject: [PATCH 3/3] fbdev/simplefb: Add support for interconnect paths
config: sparc-randconfig-r063-20250622 (https://download.01.org/0day-ci/archive/20250622/202506221019.ooLo1xBw-lkp@intel.com/config)
compiler: sparc64-linux-gcc (GCC) 10.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250622/202506221019.ooLo1xBw-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202506221019.ooLo1xBw-lkp@intel.com/
All errors (new ones prefixed by >>):
drivers/video/fbdev/simplefb.c: In function 'simplefb_detach_icc':
>> drivers/video/fbdev/simplefb.c:542:14: error: 'struct simplefb_par' has no member named 'icc_count'
542 | for (i = par->icc_count - 1; i >= 0; i--) {
| ^~
>> drivers/video/fbdev/simplefb.c:543:26: error: 'struct simplefb_par' has no member named 'icc_paths'
543 | if (!IS_ERR_OR_NULL(par->icc_paths[i]))
| ^~
drivers/video/fbdev/simplefb.c:544:15: error: 'struct simplefb_par' has no member named 'icc_paths'
544 | icc_put(par->icc_paths[i]);
| ^~
drivers/video/fbdev/simplefb.c: In function 'simplefb_attach_icc':
drivers/video/fbdev/simplefb.c:564:5: error: 'struct simplefb_par' has no member named 'icc_count'
564 | par->icc_count = count / 2;
| ^~
drivers/video/fbdev/simplefb.c:566:5: error: 'struct simplefb_par' has no member named 'icc_paths'
566 | par->icc_paths = devm_kcalloc(dev, par->icc_count,
| ^~
drivers/video/fbdev/simplefb.c:566:40: error: 'struct simplefb_par' has no member named 'icc_count'
566 | par->icc_paths = devm_kcalloc(dev, par->icc_count,
| ^~
drivers/video/fbdev/simplefb.c:567:22: error: 'struct simplefb_par' has no member named 'icc_paths'
567 | sizeof(*par->icc_paths),
| ^~
drivers/video/fbdev/simplefb.c:569:10: error: 'struct simplefb_par' has no member named 'icc_paths'
569 | if (!par->icc_paths)
| ^~
drivers/video/fbdev/simplefb.c:572:21: error: 'struct simplefb_par' has no member named 'icc_count'
572 | for (i = 0; i < par->icc_count; i++) {
| ^~
drivers/video/fbdev/simplefb.c:573:6: error: 'struct simplefb_par' has no member named 'icc_paths'
573 | par->icc_paths[i] = of_icc_get_by_index(dev, i);
| ^~
drivers/video/fbdev/simplefb.c:574:25: error: 'struct simplefb_par' has no member named 'icc_paths'
574 | if (IS_ERR_OR_NULL(par->icc_paths[i])) {
| ^~
drivers/video/fbdev/simplefb.c:575:21: error: 'struct simplefb_par' has no member named 'icc_paths'
575 | ret = PTR_ERR(par->icc_paths[i]);
| ^~
drivers/video/fbdev/simplefb.c:582:23: error: 'struct simplefb_par' has no member named 'icc_paths'
582 | ret = icc_set_bw(par->icc_paths[i], 0, UINT_MAX);
| ^~
drivers/video/fbdev/simplefb.c:594:26: error: 'struct simplefb_par' has no member named 'icc_paths'
594 | if (!IS_ERR_OR_NULL(par->icc_paths[i]))
| ^~
drivers/video/fbdev/simplefb.c:595:15: error: 'struct simplefb_par' has no member named 'icc_paths'
595 | icc_put(par->icc_paths[i]);
| ^~
vim +542 drivers/video/fbdev/simplefb.c
532
533 #if defined CONFIG_OF && defined CONFIG_PM_GENERIC_DOMAINS
534 /*
535 * Generic interconnect path handling code.
536 */
537 static void simplefb_detach_icc(void *res)
538 {
539 struct simplefb_par *par = res;
540 int i;
541
> 542 for (i = par->icc_count - 1; i >= 0; i--) {
> 543 if (!IS_ERR_OR_NULL(par->icc_paths[i]))
544 icc_put(par->icc_paths[i]);
545 }
546 }
547
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Hi
Am 20.06.25 um 12:31 schrieb Luca Weiss:
> 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.
>
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---
> drivers/video/fbdev/simplefb.c | 83 ++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 83 insertions(+)
>
> diff --git a/drivers/video/fbdev/simplefb.c b/drivers/video/fbdev/simplefb.c
> index be95fcddce4c8ca794826b805cd7dad2985bd637..ca73e079fd13550ddc779e84db80f7f9b743d074 100644
> --- a/drivers/video/fbdev/simplefb.c
> +++ b/drivers/video/fbdev/simplefb.c
> @@ -27,6 +27,7 @@
> #include <linux/parser.h>
> #include <linux/pm_domain.h>
> #include <linux/regulator/consumer.h>
> +#include <linux/interconnect.h>
With alphabetical sorting:
Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
Best regards
Thomas
>
> static const struct fb_fix_screeninfo simplefb_fix = {
> .id = "simple",
> @@ -89,6 +90,10 @@ struct simplefb_par {
> u32 regulator_count;
> struct regulator **regulators;
> #endif
> +#if defined CONFIG_OF && defined CONFIG_INTERCONNECT
> + unsigned int icc_count;
> + struct icc_path **icc_paths;
> +#endif
> };
>
> static void simplefb_clocks_destroy(struct simplefb_par *par);
> @@ -525,6 +530,80 @@ static int simplefb_attach_genpds(struct simplefb_par *par,
> }
> #endif
>
> +#if defined CONFIG_OF && defined CONFIG_PM_GENERIC_DOMAINS
> +/*
> + * Generic interconnect path handling code.
> + */
> +static void simplefb_detach_icc(void *res)
> +{
> + struct simplefb_par *par = res;
> + int i;
> +
> + for (i = par->icc_count - 1; i >= 0; i--) {
> + if (!IS_ERR_OR_NULL(par->icc_paths[i]))
> + icc_put(par->icc_paths[i]);
> + }
> +}
> +
> +static int simplefb_attach_icc(struct simplefb_par *par,
> + struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->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) {
> + dev_err(dev, "invalid interconnects value\n");
> + return -EINVAL;
> + }
> + par->icc_count = count / 2;
> +
> + par->icc_paths = devm_kcalloc(dev, par->icc_count,
> + sizeof(*par->icc_paths),
> + GFP_KERNEL);
> + if (!par->icc_paths)
> + return -ENOMEM;
> +
> + for (i = 0; i < par->icc_count; i++) {
> + par->icc_paths[i] = of_icc_get_by_index(dev, i);
> + if (IS_ERR_OR_NULL(par->icc_paths[i])) {
> + ret = PTR_ERR(par->icc_paths[i]);
> + if (ret == -EPROBE_DEFER)
> + goto err;
> + dev_err(dev, "failed to get interconnect path %u: %d\n", i, ret);
> + continue;
> + }
> +
> + ret = icc_set_bw(par->icc_paths[i], 0, UINT_MAX);
> + if (ret) {
> + dev_err(dev, "failed to set interconnect bandwidth %u: %d\n", i, ret);
> + continue;
> + }
> + }
> +
> + return devm_add_action_or_reset(dev, simplefb_detach_icc, par);
> +
> +err:
> + while (i) {
> + --i;
> + if (!IS_ERR_OR_NULL(par->icc_paths[i]))
> + icc_put(par->icc_paths[i]);
> + }
> + return ret;
> +}
> +#else
> +static int simplefb_attach_icc(struct simplefb_par *par,
> + struct platform_device *pdev)
> +{
> + return 0;
> +}
> +#endif
> +
> static int simplefb_probe(struct platform_device *pdev)
> {
> int ret;
> @@ -615,6 +694,10 @@ static int simplefb_probe(struct platform_device *pdev)
> if (ret < 0)
> goto error_regulators;
>
> + ret = simplefb_attach_icc(par, pdev);
> + if (ret < 0)
> + goto error_regulators;
> +
> simplefb_clocks_enable(par, pdev);
> simplefb_regulators_enable(par, pdev);
>
>
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
Hi Thomas,
On Fri Jun 20, 2025 at 1:02 PM CEST, Thomas Zimmermann wrote:
> Hi
>
> Am 20.06.25 um 12:31 schrieb Luca Weiss:
>> 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.
>>
>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
>> ---
>> drivers/video/fbdev/simplefb.c | 83 ++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 83 insertions(+)
>>
>> diff --git a/drivers/video/fbdev/simplefb.c b/drivers/video/fbdev/simplefb.c
>> index be95fcddce4c8ca794826b805cd7dad2985bd637..ca73e079fd13550ddc779e84db80f7f9b743d074 100644
>> --- a/drivers/video/fbdev/simplefb.c
>> +++ b/drivers/video/fbdev/simplefb.c
>> @@ -27,6 +27,7 @@
>> #include <linux/parser.h>
>> #include <linux/pm_domain.h>
>> #include <linux/regulator/consumer.h>
>> +#include <linux/interconnect.h>
>
> With alphabetical sorting:
>
> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
Thanks for the reviews!
For both simpledrm.c and simplefb.c, the includes are not strictly
alphabetically sorted (1 mis-sort in simpledrm, 3 in simplefb), shall I
just try and slot it into the best fitting place, or make them sorted in
my patch? Or I can add a separate commit for each driver before to sort
them.
Let me know!
Regards
Luca
>
> Best regards
> Thomas
>
>
>>
>> static const struct fb_fix_screeninfo simplefb_fix = {
>> .id = "simple",
>> @@ -89,6 +90,10 @@ struct simplefb_par {
>> u32 regulator_count;
>> struct regulator **regulators;
>> #endif
>> +#if defined CONFIG_OF && defined CONFIG_INTERCONNECT
>> + unsigned int icc_count;
>> + struct icc_path **icc_paths;
>> +#endif
>> };
>>
>> static void simplefb_clocks_destroy(struct simplefb_par *par);
>> @@ -525,6 +530,80 @@ static int simplefb_attach_genpds(struct simplefb_par *par,
>> }
>> #endif
>>
>> +#if defined CONFIG_OF && defined CONFIG_PM_GENERIC_DOMAINS
>> +/*
>> + * Generic interconnect path handling code.
>> + */
>> +static void simplefb_detach_icc(void *res)
>> +{
>> + struct simplefb_par *par = res;
>> + int i;
>> +
>> + for (i = par->icc_count - 1; i >= 0; i--) {
>> + if (!IS_ERR_OR_NULL(par->icc_paths[i]))
>> + icc_put(par->icc_paths[i]);
>> + }
>> +}
>> +
>> +static int simplefb_attach_icc(struct simplefb_par *par,
>> + struct platform_device *pdev)
>> +{
>> + struct device *dev = &pdev->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) {
>> + dev_err(dev, "invalid interconnects value\n");
>> + return -EINVAL;
>> + }
>> + par->icc_count = count / 2;
>> +
>> + par->icc_paths = devm_kcalloc(dev, par->icc_count,
>> + sizeof(*par->icc_paths),
>> + GFP_KERNEL);
>> + if (!par->icc_paths)
>> + return -ENOMEM;
>> +
>> + for (i = 0; i < par->icc_count; i++) {
>> + par->icc_paths[i] = of_icc_get_by_index(dev, i);
>> + if (IS_ERR_OR_NULL(par->icc_paths[i])) {
>> + ret = PTR_ERR(par->icc_paths[i]);
>> + if (ret == -EPROBE_DEFER)
>> + goto err;
>> + dev_err(dev, "failed to get interconnect path %u: %d\n", i, ret);
>> + continue;
>> + }
>> +
>> + ret = icc_set_bw(par->icc_paths[i], 0, UINT_MAX);
>> + if (ret) {
>> + dev_err(dev, "failed to set interconnect bandwidth %u: %d\n", i, ret);
>> + continue;
>> + }
>> + }
>> +
>> + return devm_add_action_or_reset(dev, simplefb_detach_icc, par);
>> +
>> +err:
>> + while (i) {
>> + --i;
>> + if (!IS_ERR_OR_NULL(par->icc_paths[i]))
>> + icc_put(par->icc_paths[i]);
>> + }
>> + return ret;
>> +}
>> +#else
>> +static int simplefb_attach_icc(struct simplefb_par *par,
>> + struct platform_device *pdev)
>> +{
>> + return 0;
>> +}
>> +#endif
>> +
>> static int simplefb_probe(struct platform_device *pdev)
>> {
>> int ret;
>> @@ -615,6 +694,10 @@ static int simplefb_probe(struct platform_device *pdev)
>> if (ret < 0)
>> goto error_regulators;
>>
>> + ret = simplefb_attach_icc(par, pdev);
>> + if (ret < 0)
>> + goto error_regulators;
>> +
>> simplefb_clocks_enable(par, pdev);
>> simplefb_regulators_enable(par, pdev);
>>
>>
Hi
Am 20.06.25 um 13:07 schrieb Luca Weiss:
> Hi Thomas,
>
> On Fri Jun 20, 2025 at 1:02 PM CEST, Thomas Zimmermann wrote:
>> Hi
>>
>> Am 20.06.25 um 12:31 schrieb Luca Weiss:
>>> 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.
>>>
>>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
>>> ---
>>> drivers/video/fbdev/simplefb.c | 83 ++++++++++++++++++++++++++++++++++++++++++
>>> 1 file changed, 83 insertions(+)
>>>
>>> diff --git a/drivers/video/fbdev/simplefb.c b/drivers/video/fbdev/simplefb.c
>>> index be95fcddce4c8ca794826b805cd7dad2985bd637..ca73e079fd13550ddc779e84db80f7f9b743d074 100644
>>> --- a/drivers/video/fbdev/simplefb.c
>>> +++ b/drivers/video/fbdev/simplefb.c
>>> @@ -27,6 +27,7 @@
>>> #include <linux/parser.h>
>>> #include <linux/pm_domain.h>
>>> #include <linux/regulator/consumer.h>
>>> +#include <linux/interconnect.h>
>> With alphabetical sorting:
>>
>> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
> Thanks for the reviews!
>
> For both simpledrm.c and simplefb.c, the includes are not strictly
> alphabetically sorted (1 mis-sort in simpledrm, 3 in simplefb), shall I
> just try and slot it into the best fitting place, or make them sorted in
> my patch? Or I can add a separate commit for each driver before to sort
> them.
>
> Let me know!
Best is to try to fit it into the <linux/*> block. In simpledrm, it's
probably my mistake. Don't bother with sending an extra cleanup if you
don't want to.
Best regards
Thomas
>
> Regards
> Luca
>
>
>> Best regards
>> Thomas
>>
>>
>>>
>>> static const struct fb_fix_screeninfo simplefb_fix = {
>>> .id = "simple",
>>> @@ -89,6 +90,10 @@ struct simplefb_par {
>>> u32 regulator_count;
>>> struct regulator **regulators;
>>> #endif
>>> +#if defined CONFIG_OF && defined CONFIG_INTERCONNECT
>>> + unsigned int icc_count;
>>> + struct icc_path **icc_paths;
>>> +#endif
>>> };
>>>
>>> static void simplefb_clocks_destroy(struct simplefb_par *par);
>>> @@ -525,6 +530,80 @@ static int simplefb_attach_genpds(struct simplefb_par *par,
>>> }
>>> #endif
>>>
>>> +#if defined CONFIG_OF && defined CONFIG_PM_GENERIC_DOMAINS
>>> +/*
>>> + * Generic interconnect path handling code.
>>> + */
>>> +static void simplefb_detach_icc(void *res)
>>> +{
>>> + struct simplefb_par *par = res;
>>> + int i;
>>> +
>>> + for (i = par->icc_count - 1; i >= 0; i--) {
>>> + if (!IS_ERR_OR_NULL(par->icc_paths[i]))
>>> + icc_put(par->icc_paths[i]);
>>> + }
>>> +}
>>> +
>>> +static int simplefb_attach_icc(struct simplefb_par *par,
>>> + struct platform_device *pdev)
>>> +{
>>> + struct device *dev = &pdev->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) {
>>> + dev_err(dev, "invalid interconnects value\n");
>>> + return -EINVAL;
>>> + }
>>> + par->icc_count = count / 2;
>>> +
>>> + par->icc_paths = devm_kcalloc(dev, par->icc_count,
>>> + sizeof(*par->icc_paths),
>>> + GFP_KERNEL);
>>> + if (!par->icc_paths)
>>> + return -ENOMEM;
>>> +
>>> + for (i = 0; i < par->icc_count; i++) {
>>> + par->icc_paths[i] = of_icc_get_by_index(dev, i);
>>> + if (IS_ERR_OR_NULL(par->icc_paths[i])) {
>>> + ret = PTR_ERR(par->icc_paths[i]);
>>> + if (ret == -EPROBE_DEFER)
>>> + goto err;
>>> + dev_err(dev, "failed to get interconnect path %u: %d\n", i, ret);
>>> + continue;
>>> + }
>>> +
>>> + ret = icc_set_bw(par->icc_paths[i], 0, UINT_MAX);
>>> + if (ret) {
>>> + dev_err(dev, "failed to set interconnect bandwidth %u: %d\n", i, ret);
>>> + continue;
>>> + }
>>> + }
>>> +
>>> + return devm_add_action_or_reset(dev, simplefb_detach_icc, par);
>>> +
>>> +err:
>>> + while (i) {
>>> + --i;
>>> + if (!IS_ERR_OR_NULL(par->icc_paths[i]))
>>> + icc_put(par->icc_paths[i]);
>>> + }
>>> + return ret;
>>> +}
>>> +#else
>>> +static int simplefb_attach_icc(struct simplefb_par *par,
>>> + struct platform_device *pdev)
>>> +{
>>> + return 0;
>>> +}
>>> +#endif
>>> +
>>> static int simplefb_probe(struct platform_device *pdev)
>>> {
>>> int ret;
>>> @@ -615,6 +694,10 @@ static int simplefb_probe(struct platform_device *pdev)
>>> if (ret < 0)
>>> goto error_regulators;
>>>
>>> + ret = simplefb_attach_icc(par, pdev);
>>> + if (ret < 0)
>>> + goto error_regulators;
>>> +
>>> simplefb_clocks_enable(par, pdev);
>>> simplefb_regulators_enable(par, pdev);
>>>
>>>
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
On Fri Jun 20, 2025 at 1:28 PM CEST, Thomas Zimmermann wrote: > Hi > > Am 20.06.25 um 13:07 schrieb Luca Weiss: >> Hi Thomas, >> >> On Fri Jun 20, 2025 at 1:02 PM CEST, Thomas Zimmermann wrote: >>> Hi >>> >>> Am 20.06.25 um 12:31 schrieb Luca Weiss: >>>> 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. >>>> >>>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com> >>>> --- >>>> drivers/video/fbdev/simplefb.c | 83 ++++++++++++++++++++++++++++++++++++++++++ >>>> 1 file changed, 83 insertions(+) >>>> >>>> diff --git a/drivers/video/fbdev/simplefb.c b/drivers/video/fbdev/simplefb.c >>>> index be95fcddce4c8ca794826b805cd7dad2985bd637..ca73e079fd13550ddc779e84db80f7f9b743d074 100644 >>>> --- a/drivers/video/fbdev/simplefb.c >>>> +++ b/drivers/video/fbdev/simplefb.c >>>> @@ -27,6 +27,7 @@ >>>> #include <linux/parser.h> >>>> #include <linux/pm_domain.h> >>>> #include <linux/regulator/consumer.h> >>>> +#include <linux/interconnect.h> >>> With alphabetical sorting: >>> >>> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de> >> Thanks for the reviews! >> >> For both simpledrm.c and simplefb.c, the includes are not strictly >> alphabetically sorted (1 mis-sort in simpledrm, 3 in simplefb), shall I >> just try and slot it into the best fitting place, or make them sorted in >> my patch? Or I can add a separate commit for each driver before to sort >> them. >> >> Let me know! > > Best is to try to fit it into the <linux/*> block. In simpledrm, it's > probably my mistake. Don't bother with sending an extra cleanup if you > don't want to. I was mostly asking whether this diff is okay as part of my patch (for just adding <linux/interconnect.h>) diff --git a/drivers/video/fbdev/simplefb.c b/drivers/video/fbdev/simplefb.c index be95fcddce4c..f2efa4b51401 100644 --- a/drivers/video/fbdev/simplefb.c +++ b/drivers/video/fbdev/simplefb.c @@ -13,18 +13,19 @@ */ #include <linux/aperture.h> +#include <linux/clk.h> #include <linux/errno.h> #include <linux/fb.h> +#include <linux/interconnect.h> #include <linux/io.h> #include <linux/module.h> -#include <linux/platform_data/simplefb.h> -#include <linux/platform_device.h> -#include <linux/clk.h> #include <linux/of.h> #include <linux/of_address.h> #include <linux/of_clk.h> #include <linux/of_platform.h> #include <linux/parser.h> +#include <linux/platform_data/simplefb.h> +#include <linux/platform_device.h> #include <linux/pm_domain.h> #include <linux/regulator/consumer.h> Or if you want this churn to be a separate commit. Either way is fine with me, just trying to figure out the preferences of this subsystem :) Regards Luca
Hi Am 20.06.25 um 14:07 schrieb Luca Weiss: > On Fri Jun 20, 2025 at 1:28 PM CEST, Thomas Zimmermann wrote: >> Hi >> >> Am 20.06.25 um 13:07 schrieb Luca Weiss: >>> Hi Thomas, >>> >>> On Fri Jun 20, 2025 at 1:02 PM CEST, Thomas Zimmermann wrote: >>>> Hi >>>> >>>> Am 20.06.25 um 12:31 schrieb Luca Weiss: >>>>> 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. >>>>> >>>>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com> >>>>> --- >>>>> drivers/video/fbdev/simplefb.c | 83 ++++++++++++++++++++++++++++++++++++++++++ >>>>> 1 file changed, 83 insertions(+) >>>>> >>>>> diff --git a/drivers/video/fbdev/simplefb.c b/drivers/video/fbdev/simplefb.c >>>>> index be95fcddce4c8ca794826b805cd7dad2985bd637..ca73e079fd13550ddc779e84db80f7f9b743d074 100644 >>>>> --- a/drivers/video/fbdev/simplefb.c >>>>> +++ b/drivers/video/fbdev/simplefb.c >>>>> @@ -27,6 +27,7 @@ >>>>> #include <linux/parser.h> >>>>> #include <linux/pm_domain.h> >>>>> #include <linux/regulator/consumer.h> >>>>> +#include <linux/interconnect.h> >>>> With alphabetical sorting: >>>> >>>> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de> >>> Thanks for the reviews! >>> >>> For both simpledrm.c and simplefb.c, the includes are not strictly >>> alphabetically sorted (1 mis-sort in simpledrm, 3 in simplefb), shall I >>> just try and slot it into the best fitting place, or make them sorted in >>> my patch? Or I can add a separate commit for each driver before to sort >>> them. >>> >>> Let me know! >> Best is to try to fit it into the <linux/*> block. In simpledrm, it's >> probably my mistake. Don't bother with sending an extra cleanup if you >> don't want to. > I was mostly asking whether this diff is okay as part of my patch (for > just adding <linux/interconnect.h>) > > diff --git a/drivers/video/fbdev/simplefb.c b/drivers/video/fbdev/simplefb.c > index be95fcddce4c..f2efa4b51401 100644 > --- a/drivers/video/fbdev/simplefb.c > +++ b/drivers/video/fbdev/simplefb.c > @@ -13,18 +13,19 @@ > */ > > #include <linux/aperture.h> > +#include <linux/clk.h> > #include <linux/errno.h> > #include <linux/fb.h> > +#include <linux/interconnect.h> > #include <linux/io.h> > #include <linux/module.h> > -#include <linux/platform_data/simplefb.h> > -#include <linux/platform_device.h> > -#include <linux/clk.h> > #include <linux/of.h> > #include <linux/of_address.h> > #include <linux/of_clk.h> > #include <linux/of_platform.h> > #include <linux/parser.h> > +#include <linux/platform_data/simplefb.h> > +#include <linux/platform_device.h> > #include <linux/pm_domain.h> > #include <linux/regulator/consumer.h> > > > Or if you want this churn to be a separate commit. Either way is fine > with me, just trying to figure out the preferences of this subsystem :) If you want to resort the entries, please do so in a separate patch. Best regards Thomas > > Regards > Luca > -- -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Frankenstrasse 146, 90461 Nuernberg, Germany GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman HRB 36809 (AG Nuernberg)
On Fri Jun 20, 2025 at 2:36 PM CEST, Thomas Zimmermann wrote: > Hi > > Am 20.06.25 um 14:07 schrieb Luca Weiss: >> On Fri Jun 20, 2025 at 1:28 PM CEST, Thomas Zimmermann wrote: >>> Hi >>> >>> Am 20.06.25 um 13:07 schrieb Luca Weiss: >>>> Hi Thomas, >>>> >>>> On Fri Jun 20, 2025 at 1:02 PM CEST, Thomas Zimmermann wrote: >>>>> Hi >>>>> >>>>> Am 20.06.25 um 12:31 schrieb Luca Weiss: >>>>>> 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. >>>>>> >>>>>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com> >>>>>> --- >>>>>> drivers/video/fbdev/simplefb.c | 83 ++++++++++++++++++++++++++++++++++++++++++ >>>>>> 1 file changed, 83 insertions(+) >>>>>> >>>>>> diff --git a/drivers/video/fbdev/simplefb.c b/drivers/video/fbdev/simplefb.c >>>>>> index be95fcddce4c8ca794826b805cd7dad2985bd637..ca73e079fd13550ddc779e84db80f7f9b743d074 100644 >>>>>> --- a/drivers/video/fbdev/simplefb.c >>>>>> +++ b/drivers/video/fbdev/simplefb.c >>>>>> @@ -27,6 +27,7 @@ >>>>>> #include <linux/parser.h> >>>>>> #include <linux/pm_domain.h> >>>>>> #include <linux/regulator/consumer.h> >>>>>> +#include <linux/interconnect.h> >>>>> With alphabetical sorting: >>>>> >>>>> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de> >>>> Thanks for the reviews! >>>> >>>> For both simpledrm.c and simplefb.c, the includes are not strictly >>>> alphabetically sorted (1 mis-sort in simpledrm, 3 in simplefb), shall I >>>> just try and slot it into the best fitting place, or make them sorted in >>>> my patch? Or I can add a separate commit for each driver before to sort >>>> them. >>>> >>>> Let me know! >>> Best is to try to fit it into the <linux/*> block. In simpledrm, it's >>> probably my mistake. Don't bother with sending an extra cleanup if you >>> don't want to. >> I was mostly asking whether this diff is okay as part of my patch (for >> just adding <linux/interconnect.h>) >> >> diff --git a/drivers/video/fbdev/simplefb.c b/drivers/video/fbdev/simplefb.c >> index be95fcddce4c..f2efa4b51401 100644 >> --- a/drivers/video/fbdev/simplefb.c >> +++ b/drivers/video/fbdev/simplefb.c >> @@ -13,18 +13,19 @@ >> */ >> >> #include <linux/aperture.h> >> +#include <linux/clk.h> >> #include <linux/errno.h> >> #include <linux/fb.h> >> +#include <linux/interconnect.h> >> #include <linux/io.h> >> #include <linux/module.h> >> -#include <linux/platform_data/simplefb.h> >> -#include <linux/platform_device.h> >> -#include <linux/clk.h> >> #include <linux/of.h> >> #include <linux/of_address.h> >> #include <linux/of_clk.h> >> #include <linux/of_platform.h> >> #include <linux/parser.h> >> +#include <linux/platform_data/simplefb.h> >> +#include <linux/platform_device.h> >> #include <linux/pm_domain.h> >> #include <linux/regulator/consumer.h> >> >> >> Or if you want this churn to be a separate commit. Either way is fine >> with me, just trying to figure out the preferences of this subsystem :) > > If you want to resort the entries, please do so in a separate patch. Ack, will do in v2! Regards Luca > > Best regards > Thomas > >> >> Regards >> Luca >>
© 2016 - 2025 Red Hat, Inc.