Re: [Freedreno] [PATCH v18 1/5] iommu/arm-smmu: Add pm_runtime/sleep ops

2018-11-28 Thread Thor Thayer

On 11/27/18 4:11 AM, Vivek Gautam wrote:

From: Sricharan R 

The smmu needs to be functional only when the respective
master's using it are active. The device_link feature
helps to track such functional dependencies, so that the
iommu gets powered when the master device enables itself
using pm_runtime. So by adapting the smmu driver for
runtime pm, above said dependency can be addressed.

This patch adds the pm runtime/sleep callbacks to the
driver and also the functions to parse the smmu clocks
from DT and enable them in resume/suspend.
We pull all the information about clocks from device tree.

Also, while we enable the runtime pm add a pm sleep suspend
callback that pushes devices to low power state by turning
the clocks off in a system sleep.
Also add corresponding clock enable path in resume callback.

Signed-off-by: Sricharan R 
Signed-off-by: Archit Taneja 
[Thor: Rework to get clocks from device tree]
Signed-off-by: Thor Thayer 
[vivek: rework for clock and pm ops]
Signed-off-by: Vivek Gautam 
Reviewed-by: Tomasz Figa 
Tested-by: Srinivas Kandagatla 
Reviewed-by: Robin Murphy 
---
  drivers/iommu/arm-smmu.c | 100 +--
  1 file changed, 97 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 5a28ae892504..e47c840fc6a8 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -44,10 +44,12 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
 


Thanks! Tested the device tree clock portions on Intel SOCFPGA Stratix10 
DevKit.


Tested-by: Thor Thayer 
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


Re: [Freedreno] [RESEND PATCH v17 5/5] iommu/arm-smmu: Add support for qcom, smmu-v2 variant

2018-11-26 Thread Thor Thayer

Hi Vivek,

On 11/26/18 4:55 AM, Vivek Gautam wrote:


On 11/24/2018 12:04 AM, Will Deacon wrote:

On Fri, Nov 23, 2018 at 03:06:29PM +0530, Vivek Gautam wrote:

On Fri, Nov 23, 2018 at 2:52 PM Tomasz Figa  wrote:

On Fri, Nov 23, 2018 at 6:13 PM Vivek Gautam
 wrote:
On Wed, Nov 21, 2018 at 11:09 PM Will Deacon  
wrote:

On Fri, Nov 16, 2018 at 04:54:30PM +0530, Vivek Gautam wrote:
@@ -2026,6 +2027,17 @@ ARM_SMMU_MATCH_DATA(arm_mmu401, 
ARM_SMMU_V1_64K, GENERIC_SMMU);

  ARM_SMMU_MATCH_DATA(arm_mmu500, ARM_SMMU_V2, ARM_MMU500);
  ARM_SMMU_MATCH_DATA(cavium_smmuv2, ARM_SMMU_V2, CAVIUM_SMMUV2);

+static const char * const qcom_smmuv2_clks[] = {
+ "bus", "iface",
+};
+
+static const struct arm_smmu_match_data qcom_smmuv2 = {
+ .version = ARM_SMMU_V2,
+ .model = QCOM_SMMUV2,
+ .clks = qcom_smmuv2_clks,
+ .num_clks = ARRAY_SIZE(qcom_smmuv2_clks),
+};
These seems redundant if we go down the route proposed by Thor, 
where we
just pull all of the clocks out of the device-tree. In which case, 
why

do we need this match_data at all?

Which is better? Driver relying solely on the device tree to tell
which all clocks
are required to be enabled,
or, the driver deciding itself based on the platform's match data,
that it should
have X, Y, & Z clocks that should be supplied from the device tree.

The former would simplify the driver, but would also make it
impossible to spot mistakes in DT, which would ultimately surface out
as very hard to debug bugs (likely complete system lockups).

Thanks.
Yea, this is how I understand things presently. Relying on device tree
puts the things out of driver's control.

But it also has the undesirable effect of having to update the driver
code whenever we want to add support for a new SMMU implementation. If
we do this all in the DT, as Thor is trying to do, then older kernels
will work well with new hardware.


Hi Will,
Am I unable to understand the intentions here for Thor's clock-fetch
design change?

I'm having trouble parsing your question, sorry. Please work with Thor
so that we have a single way to get the clock information. My preference
is to take it from the firmware, for the reason I stated above.

Hi Will,

Sure, thanks. I will work with Thor to get this going.

Hi Thor,
Does it sound okay to you to squash your patch [1] into my patch [2] with
your 'Signed-off-by' tag?
I will update the commit log to include the information about getting
clock details from device tree.

[1] https://patchwork.kernel.org/patch/10628725/
[2] https://patchwork.kernel.org/patch/10686061/



Yes, that would be great and easier to understand than my patch on top 
of yours.


Additionally, can you remove the "Error:" as Will requested as part of 
the squash?


Thank you!

Thor


Best regards
Vivek


Will





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