Re: [Freedreno] [PATCH] drm/msm/dp: Fix warnings reported by kbot in DP driver

2021-03-04 Thread Stephen Boyd
Maybe subject could be "Ignore debugfs failures, fix indentation, fix
logical error"?

Quoting Abhinav Kumar (2021-03-04 17:31:52)
> Fix the warnings reported by kbot across MSM DP driver.

Which warnings? Can you include them? Or at least list the three things
that are being fixed in this patch?

> 
> Reported-by: kernel test robot 
> Reported-by: Dan Carpenter 
> Signed-off-by: Abhinav Kumar 
> ---
>  drivers/gpu/drm/msm/dp/dp_debug.c | 33 +
>  drivers/gpu/drm/msm/dp/dp_hpd.c   |  4 ++--
>  drivers/gpu/drm/msm/dp/dp_power.c |  2 +-
>  3 files changed, 8 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/dp/dp_debug.c 
> b/drivers/gpu/drm/msm/dp/dp_debug.c
> index 84670bc..2f6247e 100644
> --- a/drivers/gpu/drm/msm/dp/dp_debug.c
> +++ b/drivers/gpu/drm/msm/dp/dp_debug.c
> @@ -226,7 +226,7 @@ static int dp_test_data_show(struct seq_file *m, void 
> *data)
> debug->link->test_video.test_h_width);
> seq_printf(m, "vdisplay: %d\n",
> 
> debug->link->test_video.test_v_height);
> -   seq_printf(m, "bpc: %u\n",
> +   seq_printf(m, "bpc: %u\n",
> dp_link_bit_depth_to_bpc(bpc));
> } else
> seq_puts(m, "0");

Indentation.

> @@ -368,44 +368,21 @@ static int dp_debug_init(struct dp_debug *dp_debug, 
> struct drm_minor *minor)
> int rc = 0;
> struct dp_debug_private *debug = container_of(dp_debug,
> struct dp_debug_private, dp_debug);
> -   struct dentry *file;
> -   struct dentry *test_active;
> -   struct dentry *test_data, *test_type;
>  
> -   file = debugfs_create_file("dp_debug", 0444, minor->debugfs_root,
> +   debugfs_create_file("dp_debug", 0444, minor->debugfs_root,
> debug, &dp_debug_fops);
> -   if (IS_ERR_OR_NULL(file)) {
> -   rc = PTR_ERR(file);
> -   DRM_ERROR("[%s] debugfs create file failed, rc=%d\n",
> - DEBUG_NAME, rc);
> -   }
>  
> -   test_active = debugfs_create_file("msm_dp_test_active", 0444,
> +   debugfs_create_file("msm_dp_test_active", 0444,
> minor->debugfs_root,
> debug, &test_active_fops);
> -   if (IS_ERR_OR_NULL(test_active)) {
> -   rc = PTR_ERR(test_active);
> -   DRM_ERROR("[%s] debugfs test_active failed, rc=%d\n",
> - DEBUG_NAME, rc);
> -   }
>  
> -   test_data = debugfs_create_file("msm_dp_test_data", 0444,
> +   debugfs_create_file("msm_dp_test_data", 0444,
> minor->debugfs_root,
> debug, &dp_test_data_fops);
> -   if (IS_ERR_OR_NULL(test_data)) {
> -   rc = PTR_ERR(test_data);
> -   DRM_ERROR("[%s] debugfs test_data failed, rc=%d\n",
> - DEBUG_NAME, rc);
> -   }
>  
> -   test_type = debugfs_create_file("msm_dp_test_type", 0444,
> +   debugfs_create_file("msm_dp_test_type", 0444,
> minor->debugfs_root,
> debug, &dp_test_type_fops);
> -   if (IS_ERR_OR_NULL(test_type)) {
> -   rc = PTR_ERR(test_type);
> -   DRM_ERROR("[%s] debugfs test_type failed, rc=%d\n",
> - DEBUG_NAME, rc);
> -   }

Debugfs failures.

>  
> debug->root = minor->debugfs_root;
>  
> diff --git a/drivers/gpu/drm/msm/dp/dp_hpd.c b/drivers/gpu/drm/msm/dp/dp_hpd.c
> index 5b8fe3202..e1c90fa 100644
> --- a/drivers/gpu/drm/msm/dp/dp_hpd.c
> +++ b/drivers/gpu/drm/msm/dp/dp_hpd.c
> @@ -34,8 +34,8 @@ int dp_hpd_connect(struct dp_usbpd *dp_usbpd, bool hpd)
>  
> dp_usbpd->hpd_high = hpd;
>  
> -   if (!hpd_priv->dp_cb && !hpd_priv->dp_cb->configure
> -   && !hpd_priv->dp_cb->disconnect) {
> +   if (!hpd_priv->dp_cb || !hpd_priv->dp_cb->configure
> +   || !hpd_priv->dp_cb->disconnect) {

Logical error.

> pr_err("hpd dp_cb not initialized\n");
> return -EINVAL;
> }
> diff --git a/drivers/gpu/drm/msm/dp/dp_power.c 
> b/drivers/gpu/drm/msm/dp/dp_power.c
> index 9c4ea00..3961ba4 100644
> --- a/drivers/gpu/drm/msm/dp/dp_power.c
> +++ b/drivers/gpu/drm/msm/dp/dp_power.c
> @@ -269,7 +269,7 @@ int dp_power_clk_enable(struct dp_power *dp_power,
> DRM_ERROR("failed to '%s' clks for: %s. err=%d\n",
> enable ? "enable" : "disable",
> dp_parser_pm_name(pm_type), rc);
> -   return rc;
> +   return rc;

Indentation.

> }
>  
> if (pm_type == DP_CORE_PM)
___
Freedreno mailing l

[Freedreno] [PATCH] drm/msm/dp: Fix warnings reported by kbot in DP driver

2021-03-04 Thread Abhinav Kumar
Fix the warnings reported by kbot across MSM DP driver.

Reported-by: kernel test robot 
Reported-by: Dan Carpenter 
Signed-off-by: Abhinav Kumar 
---
 drivers/gpu/drm/msm/dp/dp_debug.c | 33 +
 drivers/gpu/drm/msm/dp/dp_hpd.c   |  4 ++--
 drivers/gpu/drm/msm/dp/dp_power.c |  2 +-
 3 files changed, 8 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_debug.c 
b/drivers/gpu/drm/msm/dp/dp_debug.c
index 84670bc..2f6247e 100644
--- a/drivers/gpu/drm/msm/dp/dp_debug.c
+++ b/drivers/gpu/drm/msm/dp/dp_debug.c
@@ -226,7 +226,7 @@ static int dp_test_data_show(struct seq_file *m, void *data)
debug->link->test_video.test_h_width);
seq_printf(m, "vdisplay: %d\n",
debug->link->test_video.test_v_height);
-   seq_printf(m, "bpc: %u\n",
+   seq_printf(m, "bpc: %u\n",
dp_link_bit_depth_to_bpc(bpc));
} else
seq_puts(m, "0");
@@ -368,44 +368,21 @@ static int dp_debug_init(struct dp_debug *dp_debug, 
struct drm_minor *minor)
int rc = 0;
struct dp_debug_private *debug = container_of(dp_debug,
struct dp_debug_private, dp_debug);
-   struct dentry *file;
-   struct dentry *test_active;
-   struct dentry *test_data, *test_type;
 
-   file = debugfs_create_file("dp_debug", 0444, minor->debugfs_root,
+   debugfs_create_file("dp_debug", 0444, minor->debugfs_root,
debug, &dp_debug_fops);
-   if (IS_ERR_OR_NULL(file)) {
-   rc = PTR_ERR(file);
-   DRM_ERROR("[%s] debugfs create file failed, rc=%d\n",
- DEBUG_NAME, rc);
-   }
 
-   test_active = debugfs_create_file("msm_dp_test_active", 0444,
+   debugfs_create_file("msm_dp_test_active", 0444,
minor->debugfs_root,
debug, &test_active_fops);
-   if (IS_ERR_OR_NULL(test_active)) {
-   rc = PTR_ERR(test_active);
-   DRM_ERROR("[%s] debugfs test_active failed, rc=%d\n",
- DEBUG_NAME, rc);
-   }
 
-   test_data = debugfs_create_file("msm_dp_test_data", 0444,
+   debugfs_create_file("msm_dp_test_data", 0444,
minor->debugfs_root,
debug, &dp_test_data_fops);
-   if (IS_ERR_OR_NULL(test_data)) {
-   rc = PTR_ERR(test_data);
-   DRM_ERROR("[%s] debugfs test_data failed, rc=%d\n",
- DEBUG_NAME, rc);
-   }
 
-   test_type = debugfs_create_file("msm_dp_test_type", 0444,
+   debugfs_create_file("msm_dp_test_type", 0444,
minor->debugfs_root,
debug, &dp_test_type_fops);
-   if (IS_ERR_OR_NULL(test_type)) {
-   rc = PTR_ERR(test_type);
-   DRM_ERROR("[%s] debugfs test_type failed, rc=%d\n",
- DEBUG_NAME, rc);
-   }
 
debug->root = minor->debugfs_root;
 
diff --git a/drivers/gpu/drm/msm/dp/dp_hpd.c b/drivers/gpu/drm/msm/dp/dp_hpd.c
index 5b8fe3202..e1c90fa 100644
--- a/drivers/gpu/drm/msm/dp/dp_hpd.c
+++ b/drivers/gpu/drm/msm/dp/dp_hpd.c
@@ -34,8 +34,8 @@ int dp_hpd_connect(struct dp_usbpd *dp_usbpd, bool hpd)
 
dp_usbpd->hpd_high = hpd;
 
-   if (!hpd_priv->dp_cb && !hpd_priv->dp_cb->configure
-   && !hpd_priv->dp_cb->disconnect) {
+   if (!hpd_priv->dp_cb || !hpd_priv->dp_cb->configure
+   || !hpd_priv->dp_cb->disconnect) {
pr_err("hpd dp_cb not initialized\n");
return -EINVAL;
}
diff --git a/drivers/gpu/drm/msm/dp/dp_power.c 
b/drivers/gpu/drm/msm/dp/dp_power.c
index 9c4ea00..3961ba4 100644
--- a/drivers/gpu/drm/msm/dp/dp_power.c
+++ b/drivers/gpu/drm/msm/dp/dp_power.c
@@ -269,7 +269,7 @@ int dp_power_clk_enable(struct dp_power *dp_power,
DRM_ERROR("failed to '%s' clks for: %s. err=%d\n",
enable ? "enable" : "disable",
dp_parser_pm_name(pm_type), rc);
-   return rc;
+   return rc;
}
 
if (pm_type == DP_CORE_PM)
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


Re: [Freedreno] [PATCH 16/17] iommu: remove DOMAIN_ATTR_IO_PGTABLE_CFG

2021-03-04 Thread Rob Clark
On Thu, Mar 4, 2021 at 7:48 AM Robin Murphy  wrote:
>
> On 2021-03-01 08:42, Christoph Hellwig wrote:
> > Signed-off-by: Christoph Hellwig 
>
> Moreso than the previous patch, where the feature is at least relatively
> generic (note that there's a bunch of in-flight development around
> DOMAIN_ATTR_NESTING), I'm really not convinced that it's beneficial to
> bloat the generic iommu_ops structure with private driver-specific
> interfaces. The attribute interface is a great compromise for these
> kinds of things, and you can easily add type-checked wrappers around it
> for external callers (maybe even make the actual attributes internal
> between the IOMMU core and drivers) if that's your concern.

I suppose if this is *just* for the GPU we could move it into adreno_smmu_priv..

But one thing I'm not sure about is whether
IO_PGTABLE_QUIRK_ARM_OUTER_WBWA is something that other devices
*should* be using as well, but just haven't gotten around to yet.

BR,
-R

> Robin.
>
> > ---
> >   drivers/gpu/drm/msm/adreno/adreno_gpu.c |  2 +-
> >   drivers/iommu/arm/arm-smmu/arm-smmu.c   | 40 +++--
> >   drivers/iommu/iommu.c   |  9 ++
> >   include/linux/iommu.h   |  9 +-
> >   4 files changed, 29 insertions(+), 31 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c 
> > b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> > index 0f184c3dd9d9ec..78d98ab2ee3a68 100644
> > --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> > +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> > @@ -191,7 +191,7 @@ void adreno_set_llc_attributes(struct iommu_domain 
> > *iommu)
> >   struct io_pgtable_domain_attr pgtbl_cfg;
> >
> >   pgtbl_cfg.quirks = IO_PGTABLE_QUIRK_ARM_OUTER_WBWA;
> > - iommu_domain_set_attr(iommu, DOMAIN_ATTR_IO_PGTABLE_CFG, &pgtbl_cfg);
> > + iommu_domain_set_pgtable_attr(iommu, &pgtbl_cfg);
> >   }
> >
> >   struct msm_gem_address_space *
> > diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c 
> > b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> > index 2e17d990d04481..2858999c86dfd1 100644
> > --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> > +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> > @@ -1515,40 +1515,22 @@ static int arm_smmu_domain_enable_nesting(struct 
> > iommu_domain *domain)
> >   return ret;
> >   }
> >
> > -static int arm_smmu_domain_set_attr(struct iommu_domain *domain,
> > - enum iommu_attr attr, void *data)
> > +static int arm_smmu_domain_set_pgtable_attr(struct iommu_domain *domain,
> > + struct io_pgtable_domain_attr *pgtbl_cfg)
> >   {
> > - int ret = 0;
> >   struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
> > + int ret = -EPERM;
> >
> > - mutex_lock(&smmu_domain->init_mutex);
> > -
> > - switch(domain->type) {
> > - case IOMMU_DOMAIN_UNMANAGED:
> > - switch (attr) {
> > - case DOMAIN_ATTR_IO_PGTABLE_CFG: {
> > - struct io_pgtable_domain_attr *pgtbl_cfg = data;
> > -
> > - if (smmu_domain->smmu) {
> > - ret = -EPERM;
> > - goto out_unlock;
> > - }
> > + if (domain->type != IOMMU_DOMAIN_UNMANAGED)
> > + return -EINVAL;
> >
> > - smmu_domain->pgtbl_cfg = *pgtbl_cfg;
> > - break;
> > - }
> > - default:
> > - ret = -ENODEV;
> > - }
> > - break;
> > - case IOMMU_DOMAIN_DMA:
> > - ret = -ENODEV;
> > - break;
> > - default:
> > - ret = -EINVAL;
> > + mutex_lock(&smmu_domain->init_mutex);
> > + if (!smmu_domain->smmu) {
> > + smmu_domain->pgtbl_cfg = *pgtbl_cfg;
> > + ret = 0;
> >   }
> > -out_unlock:
> >   mutex_unlock(&smmu_domain->init_mutex);
> > +
> >   return ret;
> >   }
> >
> > @@ -1609,7 +1591,7 @@ static struct iommu_ops arm_smmu_ops = {
> >   .device_group   = arm_smmu_device_group,
> >   .dma_use_flush_queue= arm_smmu_dma_use_flush_queue,
> >   .dma_enable_flush_queue = arm_smmu_dma_enable_flush_queue,
> > - .domain_set_attr= arm_smmu_domain_set_attr,
> > + .domain_set_pgtable_attr = arm_smmu_domain_set_pgtable_attr,
> >   .domain_enable_nesting  = arm_smmu_domain_enable_nesting,
> >   .of_xlate   = arm_smmu_of_xlate,
> >   .get_resv_regions   = arm_smmu_get_resv_regions,
> > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> > index 2e9e058501a953..8490aefd4b41f8 100644
> > --- a/drivers/iommu/iommu.c
> > +++ b/drivers/iommu/iommu.c
> > @@ -2693,6 +2693,15 @@ int iommu_domain_enable_nesting(struct iommu_domain 
> > *domain)
> >   }
> >   EXPORT_SYMBOL_GPL(iommu_domain_enable_nesting);
> >
> > +int iommu_domain_set_pgtable_attr(struct iommu_domain *domain,
> > + struct io_pgtable_domain_attr *pgtbl_

Re: [Freedreno] [PATCH] devfreq: Register devfreq as a cooling device

2021-03-04 Thread Lukasz Luba




On 3/4/21 4:53 PM, Daniel Lezcano wrote:


Hi Lukasz,

thanks for commenting this patch,

On 04/03/2021 14:47, Lukasz Luba wrote:

Hi Daniel,

On 3/4/21 12:50 PM, Daniel Lezcano wrote:

Currently the default behavior is to manually having the devfreq
backend to register themselves as a devfreq cooling device.

There are no so many and actually it makes more sense to register the
devfreq device when adding it.

Consequently, every devfreq becomes a cooling device like cpufreq is.

Having a devfreq being registered as a cooling device can not mitigate
a thermal zone if it is not bound to this one. Thus, the current
configurations are not impacted by this change.


There are also different type of devices, which register into devfreq
framework like NoC buses, UFS/eMMC, jpeg and video accelerators, ISP,
etc.
In some platforms there are plenty of those devices and they all would
occupy memory due to private freq_table in devfreq_cooling, function:
devfreq_cooling_gen_tables().

IIRC in OdroidXU4 there are ~20 devfreq devs for NoC buses.


I'm curious, do you have a pointer to such kernels having all those
devfreq ?


Sure, it's mainline code, you can build it with exynos_defconfig.
You can check the DT files to find them arch/arm/boot/dts/exynos*.
(this particular OdroidXU4 is Exynos5422, but it grabs some generic dt
files).

Here is the mainline kernel content of /sys/class/devfreq/
--
sys/class/devfreq /
10c2.memory-controller  soc:bus-g2d  soc:bus-mfc
1180.gpusoc:bus-g2d-acp  soc:bus-mscl
soc:bus-disp1   soc:bus-gen  soc:bus-noc
soc:bus-disp1-fimd  soc:bus-gscl-scaler  soc:bus-peri
soc:bus-fsys-apbsoc:bus-jpeg soc:bus-wcore
soc:bus-fsys2   soc:bus-jpeg-apb
--

IIRC some Odroid kernel maintained by Hardkernel had more devices
in this dir.


Here is how these bus devices print themselves during boot:
--
[8.674840] exynos-bus: new bus device registered: soc:bus-wcore ( 
88700 KHz ~ 532000 KHz)
[8.686412] exynos-bus: new bus device registered: soc:bus-noc ( 
66600 KHz ~ 111000 KHz)
[8.696080] exynos-bus: new bus device registered: soc:bus-fsys-apb 
(111000 KHz ~ 222000 KHz)
[8.706590] exynos-bus: new bus device registered: soc:bus-fsys2 ( 
75000 KHz ~ 20 KHz)
[8.717661] exynos-bus: new bus device registered: soc:bus-mfc ( 
83250 KHz ~ 333000 KHz)
[8.728139] exynos-bus: new bus device registered: soc:bus-gen ( 
88700 KHz ~ 266000 KHz)
[8.737551] exynos-bus: new bus device registered: soc:bus-peri ( 
66600 KHz ~  66600 KHz)
[8.748625] exynos-bus: new bus device registered: soc:bus-g2d ( 
83250 KHz ~ 333000 KHz)
[8.759427] exynos-bus: new bus device registered: soc:bus-g2d-acp ( 
66500 KHz ~ 266000 KHz)
[8.770366] exynos-bus: new bus device registered: soc:bus-jpeg ( 
75000 KHz ~ 30 KHz)
[8.781135] exynos-bus: new bus device registered: soc:bus-jpeg-apb ( 
83250 KHz ~ 166500 KHz)
[8.791366] exynos-bus: new bus device registered: soc:bus-disp1-fimd 
(12 KHz ~ 20 KHz)
[8.802418] exynos-bus: new bus device registered: soc:bus-disp1 
(12 KHz ~ 30 KHz)
[8.813050] exynos-bus: new bus device registered: 
soc:bus-gscl-scaler (15 KHz ~ 30 KHz)
[8.825308] exynos-bus: new bus device registered: soc:bus-mscl ( 
84000 KHz ~ 666000 KHz)


--





It's true that they will not affect thermal zones, but unnecessarily,
they all will show up in the /sys/class/thermal/ as
thermal-devfreq-X


IMO the devfreq shouldn't be tight with devfreq cooling thermal.


The energy model is tied with a cooling device initialization.

So if we want to do power limitation, the energy model must be
initialized with the device, thus the cooling device also.

That is the reason why I'm ending up with this change. Chanwoo
suggestion makes sense and that will allow to move the initialization to
devfreq which is more sane but it does not solve the initial problem
with the energy model.


Make sense, the 'is_cooling_device' does the job.

Regards,
Lukasz
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


Re: [Freedreno] [PATCH] devfreq: Register devfreq as a cooling device

2021-03-04 Thread Daniel Lezcano
On 04/03/2021 16:06, Chanwoo Choi wrote:
> Hi Daniel,
> 
> As Lukasz's comment, actually some devfreq devices like memory bus
> might not affect the thermal critically. In the mainline,
> there are four types devfreq as following:
> 1. GPU
> 2. UFS Storage
> 3. DMC (Memory Controller)
> 4. Memory bus like AMBA AXI
> 
> I think that you can specify this devfreq device will be used
> for cooling device by editing the devfreq_dev_profile structure.

Thanks for the suggestion, it makes sense.

I will do the change following your example below.

  -- Daniel

> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index bf3047896e41..77966a17d03f 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -935,6 +935,13 @@ struct devfreq *devfreq_add_device(struct device *dev,
> 
>     mutex_unlock(&devfreq_list_lock);
> 
> +   if (devfreq->profile->is_cooling_device) {
> +   devfreq->cdev = devfreq_cooling_em_register(devfreq, NULL);
> +   if (IS_ERR(devfreq->cdev))
> +   dev_info(dev,
> +   "Failed to register devfreq cooling
> device\n");
> +   }
> +
>     return devfreq;
> 
>  err_init:
> diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
> index 26ea0850be9b..26dc69f1047b 100644
> --- a/include/linux/devfreq.h
> +++ b/include/linux/devfreq.h
> @@ -103,6 +103,7 @@ struct devfreq_dev_profile {
>     unsigned long initial_freq;
>     unsigned int polling_ms;
>     enum devfreq_timer timer;
> +   bool is_cooling_device;
> 
>     int (*target)(struct device *dev, unsigned long *freq, u32 flags);
>     int (*get_dev_status)(struct device *dev,
> 



-- 
 Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


Re: [Freedreno] [PATCH] devfreq: Register devfreq as a cooling device

2021-03-04 Thread Daniel Lezcano

Hi Lukasz,

thanks for commenting this patch,

On 04/03/2021 14:47, Lukasz Luba wrote:
> Hi Daniel,
> 
> On 3/4/21 12:50 PM, Daniel Lezcano wrote:
>> Currently the default behavior is to manually having the devfreq
>> backend to register themselves as a devfreq cooling device.
>>
>> There are no so many and actually it makes more sense to register the
>> devfreq device when adding it.
>>
>> Consequently, every devfreq becomes a cooling device like cpufreq is.
>>
>> Having a devfreq being registered as a cooling device can not mitigate
>> a thermal zone if it is not bound to this one. Thus, the current
>> configurations are not impacted by this change.
> 
> There are also different type of devices, which register into devfreq
> framework like NoC buses, UFS/eMMC, jpeg and video accelerators, ISP,
> etc.
> In some platforms there are plenty of those devices and they all would
> occupy memory due to private freq_table in devfreq_cooling, function:
> devfreq_cooling_gen_tables().
> 
> IIRC in OdroidXU4 there are ~20 devfreq devs for NoC buses.

I'm curious, do you have a pointer to such kernels having all those
devfreq ?

> It's true that they will not affect thermal zones, but unnecessarily,
> they all will show up in the /sys/class/thermal/ as
> thermal-devfreq-X
>
>
> IMO the devfreq shouldn't be tight with devfreq cooling thermal.

The energy model is tied with a cooling device initialization.

So if we want to do power limitation, the energy model must be
initialized with the device, thus the cooling device also.

That is the reason why I'm ending up with this change. Chanwoo
suggestion makes sense and that will allow to move the initialization to
devfreq which is more sane but it does not solve the initial problem
with the energy model.




-- 
 Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


Re: [Freedreno] [PATCH 16/17] iommu: remove DOMAIN_ATTR_IO_PGTABLE_CFG

2021-03-04 Thread Robin Murphy

On 2021-03-01 08:42, Christoph Hellwig wrote:

Signed-off-by: Christoph Hellwig 


Moreso than the previous patch, where the feature is at least relatively 
generic (note that there's a bunch of in-flight development around 
DOMAIN_ATTR_NESTING), I'm really not convinced that it's beneficial to 
bloat the generic iommu_ops structure with private driver-specific 
interfaces. The attribute interface is a great compromise for these 
kinds of things, and you can easily add type-checked wrappers around it 
for external callers (maybe even make the actual attributes internal 
between the IOMMU core and drivers) if that's your concern.


Robin.


---
  drivers/gpu/drm/msm/adreno/adreno_gpu.c |  2 +-
  drivers/iommu/arm/arm-smmu/arm-smmu.c   | 40 +++--
  drivers/iommu/iommu.c   |  9 ++
  include/linux/iommu.h   |  9 +-
  4 files changed, 29 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c 
b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index 0f184c3dd9d9ec..78d98ab2ee3a68 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -191,7 +191,7 @@ void adreno_set_llc_attributes(struct iommu_domain *iommu)
struct io_pgtable_domain_attr pgtbl_cfg;
  
  	pgtbl_cfg.quirks = IO_PGTABLE_QUIRK_ARM_OUTER_WBWA;

-   iommu_domain_set_attr(iommu, DOMAIN_ATTR_IO_PGTABLE_CFG, &pgtbl_cfg);
+   iommu_domain_set_pgtable_attr(iommu, &pgtbl_cfg);
  }
  
  struct msm_gem_address_space *

diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c 
b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index 2e17d990d04481..2858999c86dfd1 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -1515,40 +1515,22 @@ static int arm_smmu_domain_enable_nesting(struct 
iommu_domain *domain)
return ret;
  }
  
-static int arm_smmu_domain_set_attr(struct iommu_domain *domain,

-   enum iommu_attr attr, void *data)
+static int arm_smmu_domain_set_pgtable_attr(struct iommu_domain *domain,
+   struct io_pgtable_domain_attr *pgtbl_cfg)
  {
-   int ret = 0;
struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
+   int ret = -EPERM;
  
-	mutex_lock(&smmu_domain->init_mutex);

-
-   switch(domain->type) {
-   case IOMMU_DOMAIN_UNMANAGED:
-   switch (attr) {
-   case DOMAIN_ATTR_IO_PGTABLE_CFG: {
-   struct io_pgtable_domain_attr *pgtbl_cfg = data;
-
-   if (smmu_domain->smmu) {
-   ret = -EPERM;
-   goto out_unlock;
-   }
+   if (domain->type != IOMMU_DOMAIN_UNMANAGED)
+   return -EINVAL;
  
-			smmu_domain->pgtbl_cfg = *pgtbl_cfg;

-   break;
-   }
-   default:
-   ret = -ENODEV;
-   }
-   break;
-   case IOMMU_DOMAIN_DMA:
-   ret = -ENODEV;
-   break;
-   default:
-   ret = -EINVAL;
+   mutex_lock(&smmu_domain->init_mutex);
+   if (!smmu_domain->smmu) {
+   smmu_domain->pgtbl_cfg = *pgtbl_cfg;
+   ret = 0;
}
-out_unlock:
mutex_unlock(&smmu_domain->init_mutex);
+
return ret;
  }
  
@@ -1609,7 +1591,7 @@ static struct iommu_ops arm_smmu_ops = {

.device_group   = arm_smmu_device_group,
.dma_use_flush_queue= arm_smmu_dma_use_flush_queue,
.dma_enable_flush_queue = arm_smmu_dma_enable_flush_queue,
-   .domain_set_attr= arm_smmu_domain_set_attr,
+   .domain_set_pgtable_attr = arm_smmu_domain_set_pgtable_attr,
.domain_enable_nesting  = arm_smmu_domain_enable_nesting,
.of_xlate   = arm_smmu_of_xlate,
.get_resv_regions   = arm_smmu_get_resv_regions,
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 2e9e058501a953..8490aefd4b41f8 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -2693,6 +2693,15 @@ int iommu_domain_enable_nesting(struct iommu_domain 
*domain)
  }
  EXPORT_SYMBOL_GPL(iommu_domain_enable_nesting);
  
+int iommu_domain_set_pgtable_attr(struct iommu_domain *domain,

+   struct io_pgtable_domain_attr *pgtbl_cfg)
+{
+   if (!domain->ops->domain_set_pgtable_attr)
+   return -EINVAL;
+   return domain->ops->domain_set_pgtable_attr(domain, pgtbl_cfg);
+}
+EXPORT_SYMBOL_GPL(iommu_domain_set_pgtable_attr);
+
  void iommu_get_resv_regions(struct device *dev, struct list_head *list)
  {
const struct iommu_ops *ops = dev->bus->iommu_ops;
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index aed88aa3bd3edf..39d3ed4d2700ac 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -40,6 +40,7 @@ struct iommu_domain;
  struct notifier_block;
  struct iommu_sva;
  struct iommu_fault_event;
+stru

Re: [Freedreno] [PATCH] devfreq: Register devfreq as a cooling device

2021-03-04 Thread Lukasz Luba

Hi Daniel,

On 3/4/21 12:50 PM, Daniel Lezcano wrote:

Currently the default behavior is to manually having the devfreq
backend to register themselves as a devfreq cooling device.

There are no so many and actually it makes more sense to register the
devfreq device when adding it.

Consequently, every devfreq becomes a cooling device like cpufreq is.

Having a devfreq being registered as a cooling device can not mitigate
a thermal zone if it is not bound to this one. Thus, the current
configurations are not impacted by this change.


There are also different type of devices, which register into devfreq
framework like NoC buses, UFS/eMMC, jpeg and video accelerators, ISP,
etc.
In some platforms there are plenty of those devices and they all would
occupy memory due to private freq_table in devfreq_cooling, function:
devfreq_cooling_gen_tables().

IIRC in OdroidXU4 there are ~20 devfreq devs for NoC buses.

It's true that they will not affect thermal zones, but unnecessarily,
they all will show up in the /sys/class/thermal/ as
thermal-devfreq-X.

IMO the devfreq shouldn't be tight with devfreq cooling thermal.

CpuFreq is different because it handles only CPUs. Here we have
many different devices.

Regards,
Lukasz
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


[Freedreno] [PATCH] devfreq: Register devfreq as a cooling device

2021-03-04 Thread Daniel Lezcano
Currently the default behavior is to manually having the devfreq
backend to register themselves as a devfreq cooling device.

There are no so many and actually it makes more sense to register the
devfreq device when adding it.

Consequently, every devfreq becomes a cooling device like cpufreq is.

Having a devfreq being registered as a cooling device can not mitigate
a thermal zone if it is not bound to this one. Thus, the current
configurations are not impacted by this change.

Signed-off-by: Daniel Lezcano 
---
 drivers/devfreq/devfreq.c   |  8 
 drivers/gpu/drm/lima/lima_devfreq.c | 13 -
 drivers/gpu/drm/lima/lima_devfreq.h |  2 --
 drivers/gpu/drm/msm/msm_gpu.c   | 11 ---
 drivers/gpu/drm/msm/msm_gpu.h   |  2 --
 drivers/gpu/drm/panfrost/panfrost_devfreq.c | 13 -
 include/linux/devfreq.h |  3 +++
 7 files changed, 11 insertions(+), 41 deletions(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index b6d63f02d293..19149b31b000 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -11,6 +11,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -26,6 +27,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include "governor.h"
 
@@ -935,6 +937,10 @@ struct devfreq *devfreq_add_device(struct device *dev,
 
mutex_unlock(&devfreq_list_lock);
 
+   devfreq->cdev = devfreq_cooling_em_register(devfreq, NULL);
+   if (IS_ERR(devfreq->cdev))
+   dev_info(dev, "Failed to register devfreq cooling device\n");
+
return devfreq;
 
 err_init:
@@ -960,6 +966,8 @@ int devfreq_remove_device(struct devfreq *devfreq)
if (!devfreq)
return -EINVAL;
 
+   thermal_cooling_device_unregister(devfreq->cdev);
+
if (devfreq->governor) {
devfreq->governor->event_handler(devfreq,
 DEVFREQ_GOV_STOP, NULL);
diff --git a/drivers/gpu/drm/lima/lima_devfreq.c 
b/drivers/gpu/drm/lima/lima_devfreq.c
index 5686ad4aaf7c..a696eff1642c 100644
--- a/drivers/gpu/drm/lima/lima_devfreq.c
+++ b/drivers/gpu/drm/lima/lima_devfreq.c
@@ -7,7 +7,6 @@
  */
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -90,11 +89,6 @@ void lima_devfreq_fini(struct lima_device *ldev)
 {
struct lima_devfreq *devfreq = &ldev->devfreq;
 
-   if (devfreq->cooling) {
-   devfreq_cooling_unregister(devfreq->cooling);
-   devfreq->cooling = NULL;
-   }
-
if (devfreq->devfreq) {
devm_devfreq_remove_device(ldev->dev, devfreq->devfreq);
devfreq->devfreq = NULL;
@@ -110,7 +104,6 @@ void lima_devfreq_fini(struct lima_device *ldev)
 
 int lima_devfreq_init(struct lima_device *ldev)
 {
-   struct thermal_cooling_device *cooling;
struct device *dev = ldev->dev;
struct opp_table *opp_table;
struct devfreq *devfreq;
@@ -173,12 +166,6 @@ int lima_devfreq_init(struct lima_device *ldev)
 
ldevfreq->devfreq = devfreq;
 
-   cooling = of_devfreq_cooling_register(dev->of_node, devfreq);
-   if (IS_ERR(cooling))
-   dev_info(dev, "Failed to register cooling device\n");
-   else
-   ldevfreq->cooling = cooling;
-
return 0;
 
 err_fini:
diff --git a/drivers/gpu/drm/lima/lima_devfreq.h 
b/drivers/gpu/drm/lima/lima_devfreq.h
index 2d9b3008ce77..c43a2069e5d3 100644
--- a/drivers/gpu/drm/lima/lima_devfreq.h
+++ b/drivers/gpu/drm/lima/lima_devfreq.h
@@ -9,7 +9,6 @@
 
 struct devfreq;
 struct opp_table;
-struct thermal_cooling_device;
 
 struct lima_device;
 
@@ -17,7 +16,6 @@ struct lima_devfreq {
struct devfreq *devfreq;
struct opp_table *clkname_opp_table;
struct opp_table *regulators_opp_table;
-   struct thermal_cooling_device *cooling;
 
ktime_t busy_time;
ktime_t idle_time;
diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
index ab7c167b0623..d7f80ebfe9df 100644
--- a/drivers/gpu/drm/msm/msm_gpu.c
+++ b/drivers/gpu/drm/msm/msm_gpu.c
@@ -14,7 +14,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 
@@ -112,14 +111,6 @@ static void msm_devfreq_init(struct msm_gpu *gpu)
}
 
devfreq_suspend_device(gpu->devfreq.devfreq);
-
-   gpu->cooling = of_devfreq_cooling_register(gpu->pdev->dev.of_node,
-   gpu->devfreq.devfreq);
-   if (IS_ERR(gpu->cooling)) {
-   DRM_DEV_ERROR(&gpu->pdev->dev,
-   "Couldn't register GPU cooling device\n");
-   gpu->cooling = NULL;
-   }
 }
 
 static int enable_pwrrail(struct msm_gpu *gpu)
@@ -1056,6 +1047,4 @@ void msm_gpu_cleanup(struct msm_gpu *gpu)
if (gpu->worker) {
kthread_destroy_worker(gpu->worker);
}
-
-   devfreq_cooling_unregister(gpu->cooling)

Re: [Freedreno] [PATCH 14/17] iommu: remove DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE

2021-03-04 Thread Robin Murphy

On 2021-03-01 08:42, Christoph Hellwig wrote:

Use explicit methods for setting and querying the information instead.


Now that everyone's using iommu-dma, is there any point in bouncing this 
through the drivers at all? Seems like it would make more sense for the 
x86 drivers to reflect their private options back to iommu_dma_strict 
(and allow Intel's caching mode to override it as well), then have 
iommu_dma_init_domain just test !iommu_dma_strict && 
domain->ops->flush_iotlb_all.


Robin.


Also remove the now unused iommu_domain_get_attr functionality.

Signed-off-by: Christoph Hellwig 
---
  drivers/iommu/amd/iommu.c   | 23 ++---
  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 47 ++---
  drivers/iommu/arm/arm-smmu/arm-smmu.c   | 56 +
  drivers/iommu/dma-iommu.c   |  8 ++-
  drivers/iommu/intel/iommu.c | 27 ++
  drivers/iommu/iommu.c   | 19 +++
  include/linux/iommu.h   | 17 ++-
  7 files changed, 51 insertions(+), 146 deletions(-)

diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index a69a8b573e40d0..37a8e51db17656 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -1771,24 +1771,11 @@ static struct iommu_group 
*amd_iommu_device_group(struct device *dev)
return acpihid_device_group(dev);
  }
  
-static int amd_iommu_domain_get_attr(struct iommu_domain *domain,

-   enum iommu_attr attr, void *data)
+static bool amd_iommu_dma_use_flush_queue(struct iommu_domain *domain)
  {
-   switch (domain->type) {
-   case IOMMU_DOMAIN_UNMANAGED:
-   return -ENODEV;
-   case IOMMU_DOMAIN_DMA:
-   switch (attr) {
-   case DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE:
-   *(int *)data = !amd_iommu_unmap_flush;
-   return 0;
-   default:
-   return -ENODEV;
-   }
-   break;
-   default:
-   return -EINVAL;
-   }
+   if (domain->type != IOMMU_DOMAIN_DMA)
+   return false;
+   return !amd_iommu_unmap_flush;
  }
  
  /*

@@ -2257,7 +2244,7 @@ const struct iommu_ops amd_iommu_ops = {
.release_device = amd_iommu_release_device,
.probe_finalize = amd_iommu_probe_finalize,
.device_group = amd_iommu_device_group,
-   .domain_get_attr = amd_iommu_domain_get_attr,
+   .dma_use_flush_queue = amd_iommu_dma_use_flush_queue,
.get_resv_regions = amd_iommu_get_resv_regions,
.put_resv_regions = generic_iommu_put_resv_regions,
.is_attach_deferred = amd_iommu_is_attach_deferred,
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c 
b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 8594b4a8304375..bf96172e8c1f71 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2449,33 +2449,21 @@ static struct iommu_group *arm_smmu_device_group(struct 
device *dev)
return group;
  }
  
-static int arm_smmu_domain_get_attr(struct iommu_domain *domain,

-   enum iommu_attr attr, void *data)
+static bool arm_smmu_dma_use_flush_queue(struct iommu_domain *domain)
  {
struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
  
-	switch (domain->type) {

-   case IOMMU_DOMAIN_UNMANAGED:
-   switch (attr) {
-   case DOMAIN_ATTR_NESTING:
-   *(int *)data = (smmu_domain->stage == 
ARM_SMMU_DOMAIN_NESTED);
-   return 0;
-   default:
-   return -ENODEV;
-   }
-   break;
-   case IOMMU_DOMAIN_DMA:
-   switch (attr) {
-   case DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE:
-   *(int *)data = smmu_domain->non_strict;
-   return 0;
-   default:
-   return -ENODEV;
-   }
-   break;
-   default:
-   return -EINVAL;
-   }
+   if (domain->type != IOMMU_DOMAIN_DMA)
+   return false;
+   return smmu_domain->non_strict;
+}
+
+
+static void arm_smmu_dma_enable_flush_queue(struct iommu_domain *domain)
+{
+   if (domain->type != IOMMU_DOMAIN_DMA)
+   return;
+   to_smmu_domain(domain)->non_strict = true;
  }
  
  static int arm_smmu_domain_set_attr(struct iommu_domain *domain,

@@ -2505,13 +2493,7 @@ static int arm_smmu_domain_set_attr(struct iommu_domain 
*domain,
}
break;
case IOMMU_DOMAIN_DMA:
-   switch(attr) {
-   case DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE:
-   smmu_domain->non_strict = *(int *)data;
-   break;
-   default:
-   ret = -EN

Re: [Freedreno] [PATCH] devfreq: Register devfreq as a cooling device

2021-03-04 Thread Chanwoo Choi

Hi Daniel,

As Lukasz's comment, actually some devfreq devices like memory bus
might not affect the thermal critically. In the mainline,
there are four types devfreq as following:
1. GPU
2. UFS Storage
3. DMC (Memory Controller)
4. Memory bus like AMBA AXI

I think that you can specify this devfreq device will be used
for cooling device by editing the devfreq_dev_profile structure.

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index bf3047896e41..77966a17d03f 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -935,6 +935,13 @@ struct devfreq *devfreq_add_device(struct device *dev,

mutex_unlock(&devfreq_list_lock);

+   if (devfreq->profile->is_cooling_device) {
+   devfreq->cdev = devfreq_cooling_em_register(devfreq, NULL);
+   if (IS_ERR(devfreq->cdev))
+   dev_info(dev,
+   "Failed to register devfreq cooling 
device\n");

+   }
+
return devfreq;

 err_init:
diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
index 26ea0850be9b..26dc69f1047b 100644
--- a/include/linux/devfreq.h
+++ b/include/linux/devfreq.h
@@ -103,6 +103,7 @@ struct devfreq_dev_profile {
unsigned long initial_freq;
unsigned int polling_ms;
enum devfreq_timer timer;
+   bool is_cooling_device;

int (*target)(struct device *dev, unsigned long *freq, u32 flags);
int (*get_dev_status)(struct device *dev,


Thanks
Chanwoo Choi

On 21. 3. 4. 오후 9:50, Daniel Lezcano wrote:

Currently the default behavior is to manually having the devfreq
backend to register themselves as a devfreq cooling device.

There are no so many and actually it makes more sense to register the
devfreq device when adding it.

Consequently, every devfreq becomes a cooling device like cpufreq is.

Having a devfreq being registered as a cooling device can not mitigate
a thermal zone if it is not bound to this one. Thus, the current
configurations are not impacted by this change.

Signed-off-by: Daniel Lezcano 
---
  drivers/devfreq/devfreq.c   |  8 
  drivers/gpu/drm/lima/lima_devfreq.c | 13 -
  drivers/gpu/drm/lima/lima_devfreq.h |  2 --
  drivers/gpu/drm/msm/msm_gpu.c   | 11 ---
  drivers/gpu/drm/msm/msm_gpu.h   |  2 --
  drivers/gpu/drm/panfrost/panfrost_devfreq.c | 13 -
  include/linux/devfreq.h |  3 +++
  7 files changed, 11 insertions(+), 41 deletions(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index b6d63f02d293..19149b31b000 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -11,6 +11,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
  #include 
@@ -26,6 +27,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include "governor.h"
  
@@ -935,6 +937,10 @@ struct devfreq *devfreq_add_device(struct device *dev,
  
  	mutex_unlock(&devfreq_list_lock);
  
+	devfreq->cdev = devfreq_cooling_em_register(devfreq, NULL);

+   if (IS_ERR(devfreq->cdev))
+   dev_info(dev, "Failed to register devfreq cooling device\n");
+
return devfreq;
  
  err_init:

@@ -960,6 +966,8 @@ int devfreq_remove_device(struct devfreq *devfreq)
if (!devfreq)
return -EINVAL;
  
+	thermal_cooling_device_unregister(devfreq->cdev);

+
if (devfreq->governor) {
devfreq->governor->event_handler(devfreq,
 DEVFREQ_GOV_STOP, NULL);
diff --git a/drivers/gpu/drm/lima/lima_devfreq.c 
b/drivers/gpu/drm/lima/lima_devfreq.c
index 5686ad4aaf7c..a696eff1642c 100644
--- a/drivers/gpu/drm/lima/lima_devfreq.c
+++ b/drivers/gpu/drm/lima/lima_devfreq.c
@@ -7,7 +7,6 @@
   */
  #include 
  #include 
-#include 
  #include 
  #include 
  #include 
@@ -90,11 +89,6 @@ void lima_devfreq_fini(struct lima_device *ldev)
  {
struct lima_devfreq *devfreq = &ldev->devfreq;
  
-	if (devfreq->cooling) {

-   devfreq_cooling_unregister(devfreq->cooling);
-   devfreq->cooling = NULL;
-   }
-
if (devfreq->devfreq) {
devm_devfreq_remove_device(ldev->dev, devfreq->devfreq);
devfreq->devfreq = NULL;
@@ -110,7 +104,6 @@ void lima_devfreq_fini(struct lima_device *ldev)
  
  int lima_devfreq_init(struct lima_device *ldev)

  {
-   struct thermal_cooling_device *cooling;
struct device *dev = ldev->dev;
struct opp_table *opp_table;
struct devfreq *devfreq;
@@ -173,12 +166,6 @@ int lima_devfreq_init(struct lima_device *ldev)
  
  	ldevfreq->devfreq = devfreq;
  
-	cooling = of_devfreq_cooling_register(dev->of_node, devfreq);

-   if (IS_ERR(cooling))
-   dev_info(dev, "Failed to register cooling device\n");
-   else
-   ldevfreq->cooling = cooling;
-
return 0;
  
  err_fini:

diff --git a/driver

Re: [Freedreno] cleanup unused or almost unused IOMMU APIs and the FSL PAMU driver

2021-03-04 Thread Joerg Roedel
On Mon, Mar 01, 2021 at 09:42:40AM +0100, Christoph Hellwig wrote:
> Diffstat:
>  arch/powerpc/include/asm/fsl_pamu_stash.h   |   12 
>  drivers/gpu/drm/msm/adreno/adreno_gpu.c |2 
>  drivers/iommu/amd/iommu.c   |   23 
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c |   85 ---
>  drivers/iommu/arm/arm-smmu/arm-smmu.c   |  122 +---
>  drivers/iommu/dma-iommu.c   |8 
>  drivers/iommu/fsl_pamu.c|  264 --
>  drivers/iommu/fsl_pamu.h|   10 
>  drivers/iommu/fsl_pamu_domain.c |  694 
> ++--
>  drivers/iommu/fsl_pamu_domain.h |   46 -
>  drivers/iommu/intel/iommu.c |   55 --
>  drivers/iommu/iommu.c   |   75 ---
>  drivers/soc/fsl/qbman/qman_portal.c |   56 --
>  drivers/vfio/vfio_iommu_type1.c |   31 -
>  drivers/vhost/vdpa.c|   10 
>  include/linux/iommu.h   |   81 ---
>  16 files changed, 214 insertions(+), 1360 deletions(-)

Nice cleanup, thanks. The fsl_pamu driver and interface has always been
a little bit of an alien compared to other IOMMU drivers. I am inclined
to merge this after -rc3 is out, given some reviews. Can you also please
add changelogs to the last three patches?

Thanks,

Joerg
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


[Freedreno] [v1] drm/msm/disp/dpu1: fix warning reported by kernel bot in dpu driver

2021-03-04 Thread Kalyan Thota
Fix a warning, pointing to an early deference of a variable before
check. This bug was introduced in the following commit.

commit 4259ff7ae509
("drm/msm/dpu: add support for pcc color block in dpu driver")

Reported-by: kernel test robot 
Reported-by: Dan Carpenter 
Signed-off-by: Kalyan Thota 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dspp.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dspp.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dspp.c
index a7a2453..0f9974c 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dspp.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dspp.c
@@ -26,10 +26,16 @@ static void dpu_setup_dspp_pcc(struct dpu_hw_dspp *ctx,
struct dpu_hw_pcc_cfg *cfg)
 {
 
-   u32 base = ctx->cap->sblk->pcc.base;
+   u32 base;
 
-   if (!ctx || !base) {
-   DRM_ERROR("invalid ctx %pK pcc base 0x%x\n", ctx, base);
+   if (!ctx) {
+   DRM_ERROR("invalid dspp ctx %pK\n", ctx);
+   return;
+   }
+
+   base = ctx->cap->sblk->pcc.base;
+   if (!base) {
+   DRM_ERROR("invalid pcc base 0x%x\n", base);
return;
}
 
-- 
2.7.4

___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno