Re: [Freedreno] [PATCH] of/device: add blacklist for iommu dma_ops

2018-12-03 Thread Vivek Gautam
On Mon, Dec 3, 2018 at 7:56 PM Rob Clark  wrote:
>
> On Mon, Dec 3, 2018 at 7:45 AM Robin Murphy  wrote:
> >
> > Hi Rob,
> >
> > On 01/12/2018 16:53, Rob Clark wrote:
> > > This solves a problem we see with drm/msm, caused by getting
> > > iommu_dma_ops while we attach our own domain and manage it directly at
> > > the iommu API level:
> > >
> > >[0038] user address but active_mm is swapper
> > >Internal error: Oops: 9605 [#1] PREEMPT SMP
> > >Modules linked in:
> > >CPU: 7 PID: 70 Comm: kworker/7:1 Tainted: GW 4.19.3 #90
> > >Hardware name: xxx (DT)
> > >Workqueue: events deferred_probe_work_func
> > >pstate: 80c9 (Nzcv daif +PAN +UAO)
> > >pc : iommu_dma_map_sg+0x7c/0x2c8
> > >lr : iommu_dma_map_sg+0x40/0x2c8
> > >sp : ff80095eb4f0
> > >x29: ff80095eb4f0 x28: 
> > >x27: ffc0f9431578 x26: 
> > >x25:  x24: 0003
> > >x23: 0001 x22: ffc0fa9ac010
> > >x21:  x20: ffc0fab40980
> > >x19: ffc0fab40980 x18: 0003
> > >x17: 01c4 x16: 0007
> > >x15: 000e x14: 
> > >x13:  x12: 0028
> > >x11: 0101010101010101 x10: 7f7f7f7f7f7f7f7f
> > >x9 :  x8 : ffc0fab409a0
> > >x7 :  x6 : 0002
> > >x5 : 0001 x4 : 
> > >x3 : 0001 x2 : 0002
> > >x1 : ffc0f9431578 x0 : 
> > >Process kworker/7:1 (pid: 70, stack limit = 0x17d08ffb)
> > >Call trace:
> > > iommu_dma_map_sg+0x7c/0x2c8
> > > __iommu_map_sg_attrs+0x70/0x84
> > > get_pages+0x170/0x1e8
> > > msm_gem_get_iova+0x8c/0x128
> > > _msm_gem_kernel_new+0x6c/0xc8
> > > msm_gem_kernel_new+0x4c/0x58
> > > dsi_tx_buf_alloc_6g+0x4c/0x8c
> > > msm_dsi_host_modeset_init+0xc8/0x108
> > > msm_dsi_modeset_init+0x54/0x18c
> > > _dpu_kms_drm_obj_init+0x430/0x474
> > > dpu_kms_hw_init+0x5f8/0x6b4
> > > msm_drm_bind+0x360/0x6c8
> > > try_to_bring_up_master.part.7+0x28/0x70
> > > component_master_add_with_match+0xe8/0x124
> > > msm_pdev_probe+0x294/0x2b4
> > > platform_drv_probe+0x58/0xa4
> > > really_probe+0x150/0x294
> > > driver_probe_device+0xac/0xe8
> > > __device_attach_driver+0xa4/0xb4
> > > bus_for_each_drv+0x98/0xc8
> > > __device_attach+0xac/0x12c
> > > device_initial_probe+0x24/0x30
> > > bus_probe_device+0x38/0x98
> > > deferred_probe_work_func+0x78/0xa4
> > > process_one_work+0x24c/0x3dc
> > > worker_thread+0x280/0x360
> > > kthread+0x134/0x13c
> > > ret_from_fork+0x10/0x18
> > >Code: d284 91000725 6b17039f 5400048a (f9401f40)
> > >---[ end trace f22dda57f3648e2c ]---
> > >Kernel panic - not syncing: Fatal exception
> > >SMP: stopping secondary CPUs
> > >Kernel Offset: disabled
> > >CPU features: 0x0,22802a18
> > >Memory Limit: none
> > >
> > > The problem is that when drm/msm does it's own iommu_attach_device(),
> > > now the domain returned by iommu_get_domain_for_dev() is drm/msm's
> > > domain, and it doesn't have domain->iova_cookie.
> >
> > Does this crash still happen with 4.20-rc? Because as of 6af588fed391 it
> > really shouldn't.
> >
> > > We kind of avoided this problem prior to sdm845/dpu because the iommu
> > > was attached to the mdp node in dt, which is a child of the toplevel
> > > mdss node (which corresponds to the dev passed in dma_map_sg()).  But
> > > with sdm845, now the iommu is attached at the mdss level so we hit the
> > > iommu_dma_ops in dma_map_sg().
> > >
> > > But auto allocating/attaching a domain before the driver is probed was
> > > already a blocking problem for enabling per-context pagetables for the
> > > GPU.  This problem is also now solved with this patch.
> >
> > s/solved/worked around/
> >
> > If you want a guarantee of actually getting a specific hardware context
> > allocated for a given domain, there needs to be code in the IOMMU driver
> > to understand and honour that. Implicitly depending on whatever happens
> > to fall out of current driver behaviour on current systems is not a real
> > solution.
> >
> > > Fixes: 97890ba9289c dma-mapping: detect and configure IOMMU in 
> > > of_dma_configure
> >
> > That's rather misleading, since the crash described above depends on at
> > least two other major changes which came long after that commit.
> >
> > It's not that I don't understand exactly what you want here - just that
> > this commit message isn't a coherent justification for that ;)
> >
> > > Tested-by: Douglas Anderson 
> > > Signed-off-by: Rob Clark 
> > > ---
> > > This is an alternative/replacement for [1].  What it lacks in elegance
> > > it makes up for in practicality ;-)
> > >
> > > [1] https://patchwork.freedesktop.org/patch

[Freedreno] [PATCH v19 3/5] iommu/arm-smmu: Add the device_link between masters and smmu

2018-12-03 Thread Vivek Gautam
From: Sricharan R 

Finally add the device link between the master device and
smmu, so that the smmu gets runtime enabled/disabled only when the
master needs it. This is done from add_device callback which gets
called once when the master is added to the smmu.

Signed-off-by: Sricharan R 
Signed-off-by: Vivek Gautam 
Reviewed-by: Tomasz Figa 
Tested-by: Srinivas Kandagatla 
Reviewed-by: Robin Murphy 
---

Changes since v18:
 None.

 drivers/iommu/arm-smmu.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 1917d214c4d9..b6b11642b3a9 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -1500,6 +1500,9 @@ static int arm_smmu_add_device(struct device *dev)
 
iommu_device_link(&smmu->iommu, dev);
 
+   device_link_add(dev, smmu->dev,
+   DL_FLAG_PM_RUNTIME | DL_FLAG_AUTOREMOVE_SUPPLIER);
+
return 0;
 
 out_cfg_free:
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

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


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

2018-12-03 Thread Vivek Gautam
qcom,smmu-v2 is an arm,smmu-v2 implementation with specific
clock and power requirements.
On msm8996, multiple cores, viz. mdss, video, etc. use this
smmu. On sdm845, this smmu is used with gpu.
Add bindings for the same.

Signed-off-by: Vivek Gautam 
Reviewed-by: Rob Herring 
Reviewed-by: Tomasz Figa 
Tested-by: Srinivas Kandagatla 
Reviewed-by: Robin Murphy 
---

Changes since v18:
 None.

 drivers/iommu/arm-smmu.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index b6b11642b3a9..ba18d89d4732 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -120,6 +120,7 @@ enum arm_smmu_implementation {
GENERIC_SMMU,
ARM_MMU500,
CAVIUM_SMMUV2,
+   QCOM_SMMUV2,
 };
 
 struct arm_smmu_s2cr {
@@ -2030,6 +2031,7 @@ ARM_SMMU_MATCH_DATA(smmu_generic_v2, ARM_SMMU_V2, 
GENERIC_SMMU);
 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);
+ARM_SMMU_MATCH_DATA(qcom_smmuv2, ARM_SMMU_V2, QCOM_SMMUV2);
 
 static const struct of_device_id arm_smmu_of_match[] = {
{ .compatible = "arm,smmu-v1", .data = &smmu_generic_v1 },
@@ -2038,6 +2040,7 @@ static const struct of_device_id arm_smmu_of_match[] = {
{ .compatible = "arm,mmu-401", .data = &arm_mmu401 },
{ .compatible = "arm,mmu-500", .data = &arm_mmu500 },
{ .compatible = "cavium,smmu-v2", .data = &cavium_smmuv2 },
+   { .compatible = "qcom,smmu-v2", .data = &qcom_smmuv2 },
{ },
 };
 MODULE_DEVICE_TABLE(of, arm_smmu_of_match);
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

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


[Freedreno] [PATCH v19 4/5] dt-bindings: arm-smmu: Add bindings for qcom, smmu-v2

2018-12-03 Thread Vivek Gautam
Add bindings doc for Qcom's smmu-v2 implementation.

Signed-off-by: Vivek Gautam 
Reviewed-by: Tomasz Figa 
Tested-by: Srinivas Kandagatla 
Reviewed-by: Rob Herring 
Reviewed-by: Robin Murphy 
---

Changes since v18:
 None.

 .../devicetree/bindings/iommu/arm,smmu.txt | 39 ++
 1 file changed, 39 insertions(+)

diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt 
b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
index 8a6ffce12af5..a6504b37cc21 100644
--- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
+++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
@@ -17,10 +17,16 @@ conditions.
 "arm,mmu-401"
 "arm,mmu-500"
 "cavium,smmu-v2"
+"qcom,smmu-v2"
 
   depending on the particular implementation and/or the
   version of the architecture implemented.
 
+  Qcom SoCs must contain, as below, SoC-specific compatibles
+  along with "qcom,smmu-v2":
+  "qcom,msm8996-smmu-v2", "qcom,smmu-v2",
+  "qcom,sdm845-smmu-v2", "qcom,smmu-v2".
+
 - reg   : Base address and size of the SMMU.
 
 - #global-interrupts : The number of global interrupts exposed by the
@@ -71,6 +77,22 @@ conditions.
   or using stream matching with #iommu-cells = <2>, and
   may be ignored if present in such cases.
 
+- clock-names:List of the names of clocks input to the device. The
+  required list depends on particular implementation and
+  is as follows:
+  - for "qcom,smmu-v2":
+- "bus": clock required for downstream bus access and
+ for the smmu ptw,
+- "iface": clock required to access smmu's registers
+   through the TCU's programming interface.
+  - unspecified for other implementations.
+
+- clocks: Specifiers for all clocks listed in the clock-names property,
+  as per generic clock bindings.
+
+- power-domains:  Specifiers for power domains required to be powered on for
+  the SMMU to operate, as per generic power domain bindings.
+
 ** Deprecated properties:
 
 - mmu-masters (deprecated in favour of the generic "iommus" binding) :
@@ -137,3 +159,20 @@ conditions.
 iommu-map = <0 &smmu3 0 0x400>;
 ...
 };
+
+   /* Qcom's arm,smmu-v2 implementation */
+   smmu4: iommu@d0 {
+   compatible = "qcom,msm8996-smmu-v2", "qcom,smmu-v2";
+   reg = <0xd0 0x1>;
+
+   #global-interrupts = <1>;
+   interrupts = ,
+,
+;
+   #iommu-cells = <1>;
+   power-domains = <&mmcc MDSS_GDSC>;
+
+   clocks = <&mmcc SMMU_MDP_AXI_CLK>,
+<&mmcc SMMU_MDP_AHB_CLK>;
+   clock-names = "bus", "iface";
+   };
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

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


[Freedreno] [PATCH v19 2/5] iommu/arm-smmu: Invoke pm_runtime across the driver

2018-12-03 Thread Vivek Gautam
From: Sricharan R 

Enable pm-runtime on devices that implement a pm domain. Then,
add pm runtime hooks to several iommu_ops to power cycle the
smmu device for explicit TLB invalidation requests, and
register space accesses, etc.
We need these hooks when the smmu, linked to its master through
device links, has to be powered-up without the master device
being in context.

Signed-off-by: Sricharan R 
[vivek: Cleanup pm runtime calls]
Signed-off-by: Vivek Gautam 
Reviewed-by: Tomasz Figa 
Tested-by: Srinivas Kandagatla 
Reviewed-by: Robin Murphy 
---

Changes since v18:
 None.

 drivers/iommu/arm-smmu.c | 108 ++-
 1 file changed, 98 insertions(+), 10 deletions(-)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 602b67d4f2d6..1917d214c4d9 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -270,6 +270,20 @@ static struct arm_smmu_option_prop arm_smmu_options[] = {
{ 0, NULL},
 };
 
+static inline int arm_smmu_rpm_get(struct arm_smmu_device *smmu)
+{
+   if (pm_runtime_enabled(smmu->dev))
+   return pm_runtime_get_sync(smmu->dev);
+
+   return 0;
+}
+
+static inline void arm_smmu_rpm_put(struct arm_smmu_device *smmu)
+{
+   if (pm_runtime_enabled(smmu->dev))
+   pm_runtime_put(smmu->dev);
+}
+
 static struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom)
 {
return container_of(dom, struct arm_smmu_domain, domain);
@@ -929,11 +943,15 @@ static void arm_smmu_destroy_domain_context(struct 
iommu_domain *domain)
struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
struct arm_smmu_device *smmu = smmu_domain->smmu;
struct arm_smmu_cfg *cfg = &smmu_domain->cfg;
-   int irq;
+   int ret, irq;
 
if (!smmu || domain->type == IOMMU_DOMAIN_IDENTITY)
return;
 
+   ret = arm_smmu_rpm_get(smmu);
+   if (ret < 0)
+   return;
+
/*
 * Disable the context bank and free the page tables before freeing
 * it.
@@ -948,6 +966,8 @@ static void arm_smmu_destroy_domain_context(struct 
iommu_domain *domain)
 
free_io_pgtable_ops(smmu_domain->pgtbl_ops);
__arm_smmu_free_bitmap(smmu->context_map, cfg->cbndx);
+
+   arm_smmu_rpm_put(smmu);
 }
 
 static struct iommu_domain *arm_smmu_domain_alloc(unsigned type)
@@ -1229,10 +1249,15 @@ static int arm_smmu_attach_dev(struct iommu_domain 
*domain, struct device *dev)
return -ENODEV;
 
smmu = fwspec_smmu(fwspec);
+
+   ret = arm_smmu_rpm_get(smmu);
+   if (ret < 0)
+   return ret;
+
/* Ensure that the domain is finalised */
ret = arm_smmu_init_domain_context(domain, smmu);
if (ret < 0)
-   return ret;
+   goto rpm_put;
 
/*
 * Sanity check the domain. We don't support domains across
@@ -1242,49 +1267,74 @@ static int arm_smmu_attach_dev(struct iommu_domain 
*domain, struct device *dev)
dev_err(dev,
"cannot attach to SMMU %s whilst already attached to 
domain on SMMU %s\n",
dev_name(smmu_domain->smmu->dev), dev_name(smmu->dev));
-   return -EINVAL;
+   ret = -EINVAL;
+   goto rpm_put;
}
 
/* Looks ok, so add the device to the domain */
-   return arm_smmu_domain_add_master(smmu_domain, fwspec);
+   ret = arm_smmu_domain_add_master(smmu_domain, fwspec);
+
+rpm_put:
+   arm_smmu_rpm_put(smmu);
+   return ret;
 }
 
 static int arm_smmu_map(struct iommu_domain *domain, unsigned long iova,
phys_addr_t paddr, size_t size, int prot)
 {
struct io_pgtable_ops *ops = to_smmu_domain(domain)->pgtbl_ops;
+   struct arm_smmu_device *smmu = to_smmu_domain(domain)->smmu;
+   int ret;
 
if (!ops)
return -ENODEV;
 
-   return ops->map(ops, iova, paddr, size, prot);
+   arm_smmu_rpm_get(smmu);
+   ret = ops->map(ops, iova, paddr, size, prot);
+   arm_smmu_rpm_put(smmu);
+
+   return ret;
 }
 
 static size_t arm_smmu_unmap(struct iommu_domain *domain, unsigned long iova,
 size_t size)
 {
struct io_pgtable_ops *ops = to_smmu_domain(domain)->pgtbl_ops;
+   struct arm_smmu_device *smmu = to_smmu_domain(domain)->smmu;
+   size_t ret;
 
if (!ops)
return 0;
 
-   return ops->unmap(ops, iova, size);
+   arm_smmu_rpm_get(smmu);
+   ret = ops->unmap(ops, iova, size);
+   arm_smmu_rpm_put(smmu);
+
+   return ret;
 }
 
 static void arm_smmu_flush_iotlb_all(struct iommu_domain *domain)
 {
struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
+   struct arm_smmu_device *smmu = smmu_domain->smmu;
 
-   if (smmu_domain->tlb_ops)
+   if (smmu_domain->tlb_ops) {
+   arm_smmu_rpm_get(smmu);
smmu_domain-

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

2018-12-03 Thread Vivek Gautam
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 the corresponding bulk clock handling for all
the clocks needed by smmu.

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.
Add corresponding clock enable path in resume callback as well.

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 
Tested-by: Thor Thayer 
---

Changes since v18:
 - Replaced the entire clock bulk data filling and handling with
   devm_clk_bulk_get_all().

 drivers/iommu/arm-smmu.c | 58 +---
 1 file changed, 55 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 5a28ae892504..602b67d4f2d6 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -48,6 +48,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -206,6 +207,8 @@ struct arm_smmu_device {
u32 num_global_irqs;
u32 num_context_irqs;
unsigned int*irqs;
+   struct clk_bulk_data*clks;
+   int num_clks;
 
u32 cavium_id_base; /* Specific to Cavium */
 
@@ -1947,7 +1950,7 @@ struct arm_smmu_match_data {
 };
 
 #define ARM_SMMU_MATCH_DATA(name, ver, imp)\
-static struct arm_smmu_match_data name = { .version = ver, .model = imp }
+static const struct arm_smmu_match_data name = { .version = ver, .model = imp }
 
 ARM_SMMU_MATCH_DATA(smmu_generic_v1, ARM_SMMU_V1, GENERIC_SMMU);
 ARM_SMMU_MATCH_DATA(smmu_generic_v2, ARM_SMMU_V2, GENERIC_SMMU);
@@ -2150,6 +2153,17 @@ static int arm_smmu_device_probe(struct platform_device 
*pdev)
smmu->irqs[i] = irq;
}
 
+   err = devm_clk_bulk_get_all(dev, &smmu->clks);
+   if (err < 0) {
+   dev_err(dev, "failed to get clocks %d\n", err);
+   return err;
+   }
+   smmu->num_clks = err;
+
+   err = clk_bulk_prepare_enable(smmu->num_clks, smmu->clks);
+   if (err)
+   return err;
+
err = arm_smmu_device_cfg_probe(smmu);
if (err)
return err;
@@ -2236,6 +2250,9 @@ static int arm_smmu_device_remove(struct platform_device 
*pdev)
 
/* Turn the thing off */
writel(sCR0_CLIENTPD, ARM_SMMU_GR0_NS(smmu) + ARM_SMMU_GR0_sCR0);
+
+   clk_bulk_disable_unprepare(smmu->num_clks, smmu->clks);
+
return 0;
 }
 
@@ -2244,15 +2261,50 @@ static void arm_smmu_device_shutdown(struct 
platform_device *pdev)
arm_smmu_device_remove(pdev);
 }
 
-static int __maybe_unused arm_smmu_pm_resume(struct device *dev)
+static int __maybe_unused arm_smmu_runtime_resume(struct device *dev)
 {
struct arm_smmu_device *smmu = dev_get_drvdata(dev);
+   int ret;
+
+   ret = clk_bulk_enable(smmu->num_clks, smmu->clks);
+   if (ret)
+   return ret;
 
arm_smmu_device_reset(smmu);
+
return 0;
 }
 
-static SIMPLE_DEV_PM_OPS(arm_smmu_pm_ops, NULL, arm_smmu_pm_resume);
+static int __maybe_unused arm_smmu_runtime_suspend(struct device *dev)
+{
+   struct arm_smmu_device *smmu = dev_get_drvdata(dev);
+
+   clk_bulk_disable(smmu->num_clks, smmu->clks);
+
+   return 0;
+}
+
+static int __maybe_unused arm_smmu_pm_resume(struct device *dev)
+{
+   if (pm_runtime_suspended(dev))
+   return 0;
+
+   return arm_smmu_runtime_resume(dev);
+}
+
+static int __maybe_unused arm_smmu_pm_suspend(struct device *dev)
+{
+   if (pm_runtime_suspended(dev))
+   return 0;
+
+   return arm_smmu_runtime_suspend(dev);
+}
+
+static const struct dev_pm_ops arm_smmu_pm_ops = {
+   SET_SYSTEM_SLEEP_PM_OPS(arm_smmu_pm_suspend, arm_smmu_pm_resume)
+   SET_RUNTIME_PM_OPS(arm_smmu_runtime_suspend,
+  arm_smmu_runtime_resume, NULL)
+};
 
 static struct platform_driver arm_smmu_driver = {
.driver = {
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

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


[Freedreno] [PATCH v19 0/5] iommu/arm-smmu: Add runtime pm/sleep support

2018-12-03 Thread Vivek Gautam
Changes since v18:
 - Addressing Stephen's comment [5]:
 Replaced the entire clock bulk data filling and handling with
 devm_clk_bulk_get_all().

Changes since v17:
 - Addressing Will's comment to embed Thor's change [2] for pulling
   clocks information from device tree. This is done by squashing
   Thor's change [2] in v17's 1/5 patch [3].
 - Another minor change is addition of runtime pm hooks to
   arm_smmu_iova_to_phys_hard().

Previous version of this patch series is @ [1].
Also refer to [4] for change logs for previous versions.

[1] https://lore.kernel.org/patchwork/cover/1017699/
[2] https://lore.kernel.org/patchwork/patch/996143/
[3] https://lore.kernel.org/patchwork/patch/1013167/
[4] https://lore.kernel.org/patchwork/cover/979429/
[5] https://lore.kernel.org/patchwork/patch/1017700/

Sricharan R (3):
  iommu/arm-smmu: Add pm_runtime/sleep ops
  iommu/arm-smmu: Invoke pm_runtime during probe, add/remove device
  iommu/arm-smmu: Add the device_link between masters and smmu

Vivek Gautam (2):
  dt-bindings: arm-smmu: Add bindings for qcom,smmu-v2
  iommu/arm-smmu: Add support for qcom,smmu-v2 variant

 .../devicetree/bindings/iommu/arm,smmu.txt |  39 +
 drivers/iommu/arm-smmu.c   | 170 +++--
 2 files changed, 197 insertions(+), 12 deletions(-)

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

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


Re: [Freedreno] [PATCH v2] drm/msm/dpu: add display port support in DPU

2018-12-03 Thread Jeykumar Sankaran

On 2018-12-03 06:47, Sean Paul wrote:

On Tue, Nov 27, 2018 at 02:28:30PM -0800, Jeykumar Sankaran wrote:

Add display port support in DPU by creating hooks
for DP encoder enumeration and encoder mode
initialization.

This change is based on the SDM845 Display port
driver changes[1].

changes in v2:
- rebase on [2] (Sean Paul)
- remove unwanted error checks and
  switch cases (Jordan Crouse)

[1] https://lwn.net/Articles/768265/
[2] https://lkml.org/lkml/2018/11/17/87

Signed-off-by: Jeykumar Sankaran 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c |  8 ++---
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 47

+

 2 files changed, 45 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c

b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c

index d3f4501..1f6b4b1 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -2015,7 +2015,7 @@ static int dpu_encoder_setup_display(struct

dpu_encoder_virt *dpu_enc,

 {
int ret = 0;
int i = 0;
-   enum dpu_intf_type intf_type;
+   enum dpu_intf_type intf_type = INTF_NONE;


dpu_intf_type seems unnecessary, you could just use the 
DRM_MODE_ENCODER_*

value
directly?


struct dpu_enc_phys_init_params phys_params;

if (!dpu_enc || !dpu_kms) {
@@ -2038,9 +2038,9 @@ static int dpu_encoder_setup_display(struct

dpu_encoder_virt *dpu_enc,

case DRM_MODE_ENCODER_DSI:
intf_type = INTF_DSI;
break;
-   default:
-   DPU_ERROR_ENC(dpu_enc, "unsupported display interface

type\n");

-   return -EINVAL;
+   case DRM_MODE_ENCODER_TMDS:
+   intf_type = INTF_DP;
+   break;
}

WARN_ON(disp_info->num_of_h_tiles < 1);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c

b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c

index 985c855..7d931ae 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -473,6 +473,32 @@ static void _dpu_kms_initialize_dsi(struct

drm_device *dev,

}
 }

+static void _dpu_kms_initialize_displayport(struct drm_device *dev,
+   struct msm_drm_private *priv,
+   struct dpu_kms *dpu_kms)
+{
+   struct drm_encoder *encoder = NULL;
+   int rc;
+
+   if (!priv->dp)
+   return;
+
+   encoder = dpu_encoder_init(dev, DRM_MODE_ENCODER_TMDS);
+   if (IS_ERR(encoder)) {
+   DPU_ERROR("encoder init failed for dsi display\n");
+   return;
+   }
+
+   rc = msm_dp_modeset_init(priv->dp, dev, encoder);
+   if (rc) {
+   DPU_ERROR("modeset_init failed for DP, rc = %d\n", rc);
+   drm_encoder_cleanup(encoder);
+   return;
+   }
+
+   priv->encoders[priv->num_encoders++] = encoder;


No need to keep track of drm resources at the driver level, the core 
will

do
this for you. So can you please add a patch preceding this one to 
remove

the
priv->encoders/crtc/planes/connectors arrays?

priv arrays for tracking drm components and priv->num_** counters are 
introduced

by mdp4/5. DPU just adapted the implementation.

De-coupling DPU from these arrays is much easier than fixing them in 
mdp4/5. I see

mdp5 is using it to track the max resources and also in ./msm_fbdev.c.

Thanks and Regards,
Jeykumar S.

+}
+
 /**
  * _dpu_kms_setup_displays - create encoders, bridges and connectors
  *   for underlying displays
@@ -487,6 +513,8 @@ static void _dpu_kms_setup_displays(struct

drm_device *dev,

Why are these functions voids? Seems like there are plenty of places 
for

them to
fail :)

Let's add a patch to the beginning of this series to properly handle
failures in
setup_displays and initialize_dsi


 {
_dpu_kms_initialize_dsi(dev, priv, dpu_kms);

+   _dpu_kms_initialize_displayport(dev, priv, dpu_kms);
+
/**
 * Extend this function to initialize other
 * types of displays
@@ -723,13 +751,20 @@ static void _dpu_kms_set_encoder_mode(struct

msm_kms *kms,

info.capabilities = cmd_mode ? MSM_DISPLAY_CAP_CMD_MODE :
MSM_DISPLAY_CAP_VID_MODE;

-   /* TODO: No support for DSI swap */
-   for (i = 0; i < ARRAY_SIZE(priv->dsi); i++) {
-   if (priv->dsi[i]) {
-   info.h_tile_instance[info.num_of_h_tiles] = i;
-   info.num_of_h_tiles++;
+   switch (info.intf_type) {
+   case DRM_MODE_ENCODER_DSI:
+   /* TODO: No support for DSI swap */
+   for (i = 0; i < ARRAY_SIZE(priv->dsi); i++) {
+   if (priv->dsi[i]) {
+   info.h_tile_instance[info.num_of_h_tiles]

= i;

+   info.num_of_h_tiles++;
+   }
   

Re: [Freedreno] [PATCH v5 1/3] arm64: dts: qcom: sdm845: Add dpu to sdm845 dts file

2018-12-03 Thread Jeykumar Sankaran

On 2018-12-03 16:57, Doug Anderson wrote:

Hi,

On Mon, Dec 3, 2018 at 2:27 PM Jeykumar Sankaran 


wrote:

+   dsi0: dsi@ae94000 {
+   compatible = "qcom,mdss-dsi-ctrl";
+   reg = <0xae94000 0x400>;
+   reg-names = "dsi_ctrl";
+
+   interrupt-parent = <&mdss>;
+   interrupts = <4 IRQ_TYPE_LEVEL_HIGH>;
+
+   clocks = <&dispcc 
DISP_CC_MDSS_BYTE0_CLK>,

+<&dispcc
DISP_CC_MDSS_BYTE0_INTF_CLK>,
+<&dispcc 
DISP_CC_MDSS_PCLK0_CLK>,
+<&dispcc 
DISP_CC_MDSS_ESC0_CLK>,
+<&dispcc 
DISP_CC_MDSS_AHB_CLK>,
+<&dispcc 
DISP_CC_MDSS_AXI_CLK>;

+   clock-names = "byte",
+ "byte_intf",
+ "pixel",
+ "core",
+ "iface",
+ "bus";
+
+   phys = <&dsi0_phy>;
+   phy-names = "dsi0";


I'm pretty sure that this should just be "dsi" and the one below in
dsi1 should also be called "dsi".  +Jordan should confirm.


Makes sense. I can fix that!



+   dsi1: dsi@ae96000 {
+   compatible = "qcom,mdss-dsi-ctrl";
+   reg = <0xae96000 0x400>;
+   reg-names = "dsi_ctrl";
+
+   interrupt-parent = <&mdss>;
+   interrupts = <5 IRQ_TYPE_LEVEL_HIGH>;
+
+   clocks = <&dispcc 
DISP_CC_MDSS_BYTE1_CLK>,

+<&dispcc
DISP_CC_MDSS_BYTE1_INTF_CLK>,
+<&dispcc 
DISP_CC_MDSS_PCLK1_CLK>,
+<&dispcc 
DISP_CC_MDSS_ESC1_CLK>,
+<&dispcc 
DISP_CC_MDSS_AHB_CLK>,
+<&dispcc 
DISP_CC_MDSS_AXI_CLK>;

+   clock-names = "byte",
+ "byte_intf",
+ "pixel",
+ "core",
+ "iface",
+ "bus";
+
+   phys = <&dsi1_phy>;
+   phy-names = "dsi1";
+
+   status = "disabled";


This "disabled" is causing me problems.  I don't actually need "dsi1"
but if I don't enable "dsi1" then my display doesn't come up.  :(  I
ran out of time to debug but I wonder if this is this the standard
thing where DRM needs to wait for all the components to probe until it
can finish?  If nobody on this list just knows I'll dig tomorrow and
confirm that my memory isn't faulty and see what we've done about this
in the past.


https://patchwork.kernel.org/patch/10467895/

Can you try out with this change (reviewed but not merged yet). It 
validates

the nodes before adding to the DSI list.



One last note: it's pretty weird that you sent out only 1/3 and not
2/3 and 3/3.  If you're not ready to send out MTP stuff yet then you
should send out v6 as just a singleton patch.
Yes. I was trying to separate this one out as an independent change. 
Sandeep

is working on the comments on removing the pinctrl nodes and updated
mtp nodes. He should be posting 2/3 and 3/3 in the next couple of days.

Thanks,
Jeykumar S.




-Doug


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


Re: [Freedreno] [PATCH v5 1/3] arm64: dts: qcom: sdm845: Add dpu to sdm845 dts file

2018-12-03 Thread Doug Anderson
Hi,

On Mon, Dec 3, 2018 at 2:27 PM Jeykumar Sankaran  wrote:
> +   dsi0: dsi@ae94000 {
> +   compatible = "qcom,mdss-dsi-ctrl";
> +   reg = <0xae94000 0x400>;
> +   reg-names = "dsi_ctrl";
> +
> +   interrupt-parent = <&mdss>;
> +   interrupts = <4 IRQ_TYPE_LEVEL_HIGH>;
> +
> +   clocks = <&dispcc DISP_CC_MDSS_BYTE0_CLK>,
> +<&dispcc 
> DISP_CC_MDSS_BYTE0_INTF_CLK>,
> +<&dispcc DISP_CC_MDSS_PCLK0_CLK>,
> +<&dispcc DISP_CC_MDSS_ESC0_CLK>,
> +<&dispcc DISP_CC_MDSS_AHB_CLK>,
> +<&dispcc DISP_CC_MDSS_AXI_CLK>;
> +   clock-names = "byte",
> + "byte_intf",
> + "pixel",
> + "core",
> + "iface",
> + "bus";
> +
> +   phys = <&dsi0_phy>;
> +   phy-names = "dsi0";

I'm pretty sure that this should just be "dsi" and the one below in
dsi1 should also be called "dsi".  +Jordan should confirm.


> +   dsi1: dsi@ae96000 {
> +   compatible = "qcom,mdss-dsi-ctrl";
> +   reg = <0xae96000 0x400>;
> +   reg-names = "dsi_ctrl";
> +
> +   interrupt-parent = <&mdss>;
> +   interrupts = <5 IRQ_TYPE_LEVEL_HIGH>;
> +
> +   clocks = <&dispcc DISP_CC_MDSS_BYTE1_CLK>,
> +<&dispcc 
> DISP_CC_MDSS_BYTE1_INTF_CLK>,
> +<&dispcc DISP_CC_MDSS_PCLK1_CLK>,
> +<&dispcc DISP_CC_MDSS_ESC1_CLK>,
> +<&dispcc DISP_CC_MDSS_AHB_CLK>,
> +<&dispcc DISP_CC_MDSS_AXI_CLK>;
> +   clock-names = "byte",
> + "byte_intf",
> + "pixel",
> + "core",
> + "iface",
> + "bus";
> +
> +   phys = <&dsi1_phy>;
> +   phy-names = "dsi1";
> +
> +   status = "disabled";

This "disabled" is causing me problems.  I don't actually need "dsi1"
but if I don't enable "dsi1" then my display doesn't come up.  :(  I
ran out of time to debug but I wonder if this is this the standard
thing where DRM needs to wait for all the components to probe until it
can finish?  If nobody on this list just knows I'll dig tomorrow and
confirm that my memory isn't faulty and see what we've done about this
in the past.



One last note: it's pretty weird that you sent out only 1/3 and not
2/3 and 3/3.  If you're not ready to send out MTP stuff yet then you
should send out v6 as just a singleton patch.


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


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

2018-12-03 Thread Stephen Boyd
Quoting Vivek Gautam (2018-12-02 22:43:38)
> On Fri, Nov 30, 2018 at 11:45 PM Will Deacon  wrote:
> >
> > On Thu, Nov 29, 2018 at 08:25:20PM +0530, Vivek Gautam wrote:
> > > clk_bulk_get_all() seems like going only the OF way.
> > > Is there another way here to have something common between ACPI
> > > and OF, and then do the clk_bulk_get?
> >
> > I'd say just go with clk_bulk_get_all() and if somebody really wants to
> > mess with the SMMU clocks on a system booted via ACPI, then it's their
> > problem to solve. My understanding is that the design of IORT makes this
> > next to impossible to solve anyway, because a static table is used and
> > therefore we're unable to run whatever ASL methods need to be invoked to
> > mess with the clocks.
> 
> Sure then. I will respin this patch-series.
> 

Right. The idea is to add non-OF support to clk_bulk_get_all() if/when
we get the requirement. Sounds like we can keep waiting a little longer
for that to happen.

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


Re: [Freedreno] [PATCH v3 4/4] ARM: dts: imx5: add gpu nodes

2018-12-03 Thread Fabio Estevam
On Mon, Dec 3, 2018 at 8:24 PM Jonathan marek  wrote:

> > I can't find the dt-bindings for these compatible entries. Have you
> > documented them?
> >
>
> It is the same as qcom,adreno which is documented here:
>
> Documentation/devicetree/bindings/display/msm/gpu.txt
>
> I guess I should add amd,imageon there.

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


[Freedreno] [PATCH] gpu/drm: remove DEFINE_DPU_DEBUGFS_SEQ_FOPS()

2018-12-03 Thread Yangtao Li
We already have the DEFINE_SHOW_ATTRIBUTE.There is no need to define
such a macro separately,so remove DEFINE_DPU_DEBUGFS_SEQ_FOPS.
Also use DEFINE_SHOW_ATTRIBUTE to simplify some code.

Signed-off-by: Yangtao Li 
---
 drivers/gpu/drm/armada/armada_debugfs.c  | 21 
 drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c | 14 +---
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 33 +++---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c  | 17 ++---
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c  | 36 
 drivers/gpu/host1x/debug.c   | 34 --
 6 files changed, 29 insertions(+), 126 deletions(-)

diff --git a/drivers/gpu/drm/armada/armada_debugfs.c 
b/drivers/gpu/drm/armada/armada_debugfs.c
index 6758c3a83de2..64dccb7c8be2 100644
--- a/drivers/gpu/drm/armada/armada_debugfs.c
+++ b/drivers/gpu/drm/armada/armada_debugfs.c
@@ -28,7 +28,7 @@ static int armada_debugfs_gem_linear_show(struct seq_file *m, 
void *data)
return 0;
 }
 
-static int armada_debugfs_reg_show(struct seq_file *m, void *data)
+static int reg_show(struct seq_file *m, void *data)
 {
struct drm_device *dev = m->private;
struct armada_private *priv = dev->dev_private;
@@ -50,18 +50,7 @@ static int armada_debugfs_reg_show(struct seq_file *m, void 
*data)
return 0;
 }
 
-static int armada_debugfs_reg_r_open(struct inode *inode, struct file *file)
-{
-   return single_open(file, armada_debugfs_reg_show, inode->i_private);
-}
-
-static const struct file_operations fops_reg_r = {
-   .owner = THIS_MODULE,
-   .open = armada_debugfs_reg_r_open,
-   .read = seq_read,
-   .llseek = seq_lseek,
-   .release = single_release,
-};
+DEFINE_SHOW_ATTRIBUTE(reg);
 
 static int armada_debugfs_write(struct file *file, const char __user *ptr,
size_t len, loff_t *off)
@@ -119,12 +108,14 @@ int armada_drm_debugfs_init(struct drm_minor *minor)
return ret;
 
de = debugfs_create_file("reg", S_IFREG | S_IRUSR,
-minor->debugfs_root, minor->dev, &fops_reg_r);
+minor->debugfs_root, minor->dev,
+®_fops);
if (!de)
return -ENOMEM;
 
de = debugfs_create_file("reg_wr", S_IFREG | S_IWUSR,
-minor->debugfs_root, minor->dev, &fops_reg_w);
+minor->debugfs_root, minor->dev,
+&fops_reg_w);
if (!de)
return -ENOMEM;
 
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c
index 879c13fe74e0..7607aac9449c 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c
@@ -299,18 +299,6 @@ static void dpu_disable_all_irqs(struct dpu_kms *dpu_kms)
 }
 
 #ifdef CONFIG_DEBUG_FS
-#define DEFINE_DPU_DEBUGFS_SEQ_FOPS(__prefix)  \
-static int __prefix ## _open(struct inode *inode, struct file *file)   \
-{  \
-   return single_open(file, __prefix ## _show, inode->i_private);  \
-}  \
-static const struct file_operations __prefix ## _fops = {  \
-   .owner = THIS_MODULE,   \
-   .open = __prefix ## _open,  \
-   .release = single_release,  \
-   .read = seq_read,   \
-   .llseek = seq_lseek,\
-}
 
 static int dpu_debugfs_core_irq_show(struct seq_file *s, void *v)
 {
@@ -341,7 +329,7 @@ static int dpu_debugfs_core_irq_show(struct seq_file *s, 
void *v)
return 0;
 }
 
-DEFINE_DPU_DEBUGFS_SEQ_FOPS(dpu_debugfs_core_irq);
+DEFINE_SHOW_ATTRIBUTE(dpu_debugfs_core_irq);
 
 int dpu_debugfs_core_irq_init(struct dpu_kms *dpu_kms,
struct dentry *parent)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index d4530d60767b..e705bad980ee 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -1309,7 +1309,7 @@ int dpu_crtc_vblank(struct drm_crtc *crtc, bool en)
 }
 
 #ifdef CONFIG_DEBUG_FS
-static int _dpu_debugfs_status_show(struct seq_file *s, void *data)
+static int status_show(struct seq_file *s, void *data)
 {
struct dpu_crtc *dpu_crtc;
struct dpu_plane_state *pstate = NULL;
@@ -1428,24 +1428,6 @@ static int _dpu_debugfs_status_show(struct seq_file *s, 
void *data)
return 0;
 }
 
-static int _dpu_debugfs_status_open(struct inode *inode, struct file *file)
-{
-   return single_open(file, _dpu_debugfs_status_show, inode->i_private);
-}
-
-#define DEFINE_DPU_DEBUGFS_SEQ_FOPS(__prefix) 

Re: [Freedreno] [PATCH v3 4/4] ARM: dts: imx5: add gpu nodes

2018-12-03 Thread Fabio Estevam
Hi Jonathan,

Thanks for working on this. Really nice to see GPU support for mx51/mx53!

On Mon, Dec 3, 2018 at 7:21 PM Jonathan Marek  wrote:

Please add a commit log.

> Signed-off-by: Jonathan Marek 

> ---
>  arch/arm/boot/dts/imx51.dtsi | 17 +
>  arch/arm/boot/dts/imx53.dtsi | 17 +
>  2 files changed, 34 insertions(+)
>
> diff --git a/arch/arm/boot/dts/imx51.dtsi b/arch/arm/boot/dts/imx51.dtsi
> index 67d462715..e9a7bbce9 100644
> --- a/arch/arm/boot/dts/imx51.dtsi
> +++ b/arch/arm/boot/dts/imx51.dtsi
> @@ -628,5 +628,22 @@
> clock-names = "ipg", "ahb";
> };
> };
> +
> +   gpu: gpu@3000 {

We put the peripheral nodes in address order, so this one should go
prior to the IPU node.

> +   compatible = "amd,imageon-200.1", "amd,imageon";

I can't find the dt-bindings for these compatible entries. Have you
documented them?

> +   reg = <0x3000 0x2>;
> +   reg-names = "kgsl_3d0_reg_memory";
> +   interrupts = <12>;
> +   interrupt-names = "kgsl_3d0_irq";
> +   clocks = <&clks IMX5_CLK_GPU3D_GATE>, <&clks 
> IMX5_CLK_GARB_GATE>;
> +   clock-names = "core_clk", "mem_iface_clk";
> +
> +   qcom,gpu-pwrlevels {
> +   compatible = "qcom,gpu-pwrlevels";
> +   qcom,gpu-pwrlevel@0 {

You could drop this @0
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


[Freedreno] [PATCH v3 01/10] drm/msm/dpu: Remove dpu_dbg

2018-12-03 Thread Jordan Crouse
The functions in dpu_dbg.c aren't used. The two main dump functions
fail after a lookup from dpu_dbg_base.reg_base_list which turns out
to never be populated and once those are removed the rest of the
file doesn't make any sense.

v3: No changes
v2: Moved some unrelated changes to another patch

Reviewed-by: Sean Paul 
Signed-off-by: Jordan Crouse 
---
 drivers/gpu/drm/msm/Makefile  |3 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.c   | 2393 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.h   |  103 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c   |4 +-
 .../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c  |1 -
 .../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c  |1 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c|1 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c   |1 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c |1 -
 .../gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c   |1 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c   |1 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c|3 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_vbif.c   |1 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c   |   20 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h   |1 -
 15 files changed, 4 insertions(+), 2531 deletions(-)
 delete mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.c
 delete mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.h

diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
index 7d02ef3655b5..b3c45130a5e2 100644
--- a/drivers/gpu/drm/msm/Makefile
+++ b/drivers/gpu/drm/msm/Makefile
@@ -91,8 +91,7 @@ msm-y := \
msm_ringbuffer.o \
msm_submitqueue.o
 
-msm-$(CONFIG_DEBUG_FS) += adreno/a5xx_debugfs.o \
- disp/dpu1/dpu_dbg.o
+msm-$(CONFIG_DEBUG_FS) += adreno/a5xx_debugfs.o
 
 msm-$(CONFIG_DRM_FBDEV_EMULATION) += msm_fbdev.o
 msm-$(CONFIG_COMMON_CLK) += disp/mdp4/mdp4_lvds_pll.o
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.c
deleted file mode 100644
index a85078123119..
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.c
+++ /dev/null
@@ -1,2393 +0,0 @@
-/* Copyright (c) 2009-2018, The Linux Foundation. All rights reserved.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 and
- * only version 2 as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- */
-
-#define pr_fmt(fmt)"[drm:%s:%d] " fmt, __func__, __LINE__
-
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-
-#include "dpu_dbg.h"
-#include "disp/dpu1/dpu_hw_catalog.h"
-
-
-#define DEFAULT_DBGBUS_DPU DPU_DBG_DUMP_IN_MEM
-#define DEFAULT_DBGBUS_VBIFRT  DPU_DBG_DUMP_IN_MEM
-#define REG_BASE_NAME_LEN  80
-
-#define DBGBUS_FLAGS_DSPP  BIT(0)
-#define DBGBUS_DSPP_STATUS 0x34C
-
-#define DBGBUS_NAME_DPU"dpu"
-#define DBGBUS_NAME_VBIF_RT"vbif_rt"
-
-/* offsets from dpu top address for the debug buses */
-#define DBGBUS_SSPP0   0x188
-#define DBGBUS_AXI_INTF0x194
-#define DBGBUS_SSPP1   0x298
-#define DBGBUS_DSPP0x348
-#define DBGBUS_PERIPH  0x418
-
-#define TEST_MASK(id, tp)  ((id << 4) | (tp << 1) | BIT(0))
-
-/* following offsets are with respect to MDP VBIF base for DBG BUS access */
-#define MMSS_VBIF_CLKON0x4
-#define MMSS_VBIF_TEST_BUS_OUT_CTRL0x210
-#define MMSS_VBIF_TEST_BUS_OUT 0x230
-
-/* Vbif error info */
-#define MMSS_VBIF_PND_ERR  0x190
-#define MMSS_VBIF_SRC_ERR  0x194
-#define MMSS_VBIF_XIN_HALT_CTRL1   0x204
-#define MMSS_VBIF_ERR_INFO 0X1a0
-#define MMSS_VBIF_ERR_INFO_1   0x1a4
-#define MMSS_VBIF_CLIENT_NUM   14
-
-/**
- * struct dpu_dbg_reg_base - register region base.
- * may sub-ranges: sub-ranges are used for dumping
- * or may not have sub-ranges: dumping is base -> max_offset
- * @reg_base_head: head of this node
- * @name: register base name
- * @base: base pointer
- * @off: cached offset of region for manual register dumping
- * @cnt: cached range of region for manual register dumping
- * @max_offset: length of region
- * @buf: buffer used for manual register dumping
- * @buf_len:  buffer length used for manual register dumping
- * @cb: callback for external dump function, null if not defined
- * @cb_ptr: private pointer to callback function
- */
-struct dpu_dbg_reg_base {
-   struct list_head reg_base_head;
-   char name[REG_BASE_NAME_LEN];
-   void __iomem *base;
-   size_t off;
-   size_t cnt;
-   size_t max_offset;
-   char *buf;
-   size_t buf_len;
-   void (*cb)(void *ptr);
-   void *cb_ptr;
-};
-
-s

[Freedreno] [PATCH v3 10/10] drm/msm/dpu: Clean up dpu_media_info.h static inline functions

2018-12-03 Thread Jordan Crouse
Do some cleanup in the static inline functions defined in
dpu_media_info.h by cleaning up gotos and unneeded local
variables.

v3: Added spaces between operators per Seal Paul and Sam Ravnborg

Reviewed-by: Sean Paul 
Signed-off-by: Jordan Crouse 
---
 .../gpu/drm/msm/disp/dpu1/msm_media_info.h| 164 ++
 1 file changed, 57 insertions(+), 107 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/msm_media_info.h 
b/drivers/gpu/drm/msm/disp/dpu1/msm_media_info.h
index 75470ee5b18f..9fc9dbde8a27 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/msm_media_info.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/msm_media_info.h
@@ -822,36 +822,30 @@ enum color_fmts {
  */
 static unsigned int VENUS_Y_STRIDE(int color_fmt, int width)
 {
-   unsigned int alignment, stride = 0;
+   unsigned int stride = 0;
 
if (!width)
-   goto invalid_input;
+   return 0;
 
switch (color_fmt) {
case COLOR_FMT_NV21:
case COLOR_FMT_NV12:
case COLOR_FMT_NV12_MVTB:
case COLOR_FMT_NV12_UBWC:
-   alignment = 128;
-   stride = MSM_MEDIA_ALIGN(width, alignment);
+   stride = MSM_MEDIA_ALIGN(width, 128);
break;
case COLOR_FMT_NV12_BPP10_UBWC:
-   alignment = 256;
stride = MSM_MEDIA_ALIGN(width, 192);
-   stride = MSM_MEDIA_ALIGN(stride * 4/3, alignment);
+   stride = MSM_MEDIA_ALIGN(stride * 4 / 3, 256);
break;
case COLOR_FMT_P010_UBWC:
-   alignment = 256;
-   stride = MSM_MEDIA_ALIGN(width * 2, alignment);
+   stride = MSM_MEDIA_ALIGN(width * 2, 256);
break;
case COLOR_FMT_P010:
-   alignment = 128;
-   stride = MSM_MEDIA_ALIGN(width*2, alignment);
-   break;
-   default:
+   stride = MSM_MEDIA_ALIGN(width * 2, 128);
break;
}
-invalid_input:
+
return stride;
 }
 
@@ -864,36 +858,30 @@ static unsigned int VENUS_Y_STRIDE(int color_fmt, int 
width)
  */
 static unsigned int VENUS_UV_STRIDE(int color_fmt, int width)
 {
-   unsigned int alignment, stride = 0;
+   unsigned int stride = 0;
 
if (!width)
-   goto invalid_input;
+   return 0;
 
switch (color_fmt) {
case COLOR_FMT_NV21:
case COLOR_FMT_NV12:
case COLOR_FMT_NV12_MVTB:
case COLOR_FMT_NV12_UBWC:
-   alignment = 128;
-   stride = MSM_MEDIA_ALIGN(width, alignment);
+   stride = MSM_MEDIA_ALIGN(width, 128);
break;
case COLOR_FMT_NV12_BPP10_UBWC:
-   alignment = 256;
stride = MSM_MEDIA_ALIGN(width, 192);
-   stride = MSM_MEDIA_ALIGN(stride * 4/3, alignment);
+   stride = MSM_MEDIA_ALIGN(stride * 4 / 3, 256);
break;
case COLOR_FMT_P010_UBWC:
-   alignment = 256;
-   stride = MSM_MEDIA_ALIGN(width * 2, alignment);
+   stride = MSM_MEDIA_ALIGN(width * 2, 256);
break;
case COLOR_FMT_P010:
-   alignment = 128;
-   stride = MSM_MEDIA_ALIGN(width*2, alignment);
-   break;
-   default:
+   stride = MSM_MEDIA_ALIGN(width * 2, 128);
break;
}
-invalid_input:
+
return stride;
 }
 
@@ -906,10 +894,10 @@ static unsigned int VENUS_UV_STRIDE(int color_fmt, int 
width)
  */
 static unsigned int VENUS_Y_SCANLINES(int color_fmt, int height)
 {
-   unsigned int alignment, sclines = 0;
+   unsigned int sclines = 0;
 
if (!height)
-   goto invalid_input;
+   return 0;
 
switch (color_fmt) {
case COLOR_FMT_NV21:
@@ -917,17 +905,14 @@ static unsigned int VENUS_Y_SCANLINES(int color_fmt, int 
height)
case COLOR_FMT_NV12_MVTB:
case COLOR_FMT_NV12_UBWC:
case COLOR_FMT_P010:
-   alignment = 32;
+   sclines = MSM_MEDIA_ALIGN(height, 32);
break;
case COLOR_FMT_NV12_BPP10_UBWC:
case COLOR_FMT_P010_UBWC:
-   alignment = 16;
+   sclines = MSM_MEDIA_ALIGN(height, 16);
break;
-   default:
-   return 0;
}
-   sclines = MSM_MEDIA_ALIGN(height, alignment);
-invalid_input:
+
return sclines;
 }
 
@@ -940,10 +925,10 @@ static unsigned int VENUS_Y_SCANLINES(int color_fmt, int 
height)
  */
 static unsigned int VENUS_UV_SCANLINES(int color_fmt, int height)
 {
-   unsigned int alignment, sclines = 0;
+   unsigned int sclines = 0;
 
if (!height)
-   goto invalid_input;
+   return 0;
 
switch (color_fmt) {
case COLOR_FMT_NV21:
@@ -952,18 +937,13 @@ static unsigned int VENUS_UV_SCANLINES(int color_fmt, int 
height)
case COLOR_FMT_NV12_BPP10_UBWC:
   

[Freedreno] [PATCH v3 06/10] drm/msm: Make irq_postinstall optional

2018-12-03 Thread Jordan Crouse
Allow the KMS operation 'irq_postinstall' to be optional
so that the target display drivers don't need to define
a dummy function if they don't need one.

v3: No changes

Reviewed-by: Sean Paul 
Signed-off-by: Jordan Crouse 
---
 drivers/gpu/drm/msm/msm_drv.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 9c9f7ff6960b..d8af97407b3c 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -724,7 +724,11 @@ static int msm_irq_postinstall(struct drm_device *dev)
struct msm_drm_private *priv = dev->dev_private;
struct msm_kms *kms = priv->kms;
BUG_ON(!kms);
-   return kms->funcs->irq_postinstall(kms);
+
+   if (kms->funcs->irq_postinstall)
+   return kms->funcs->irq_postinstall(kms);
+
+   return 0;
 }
 
 static void msm_irq_uninstall(struct drm_device *dev)
-- 
2.18.0

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


[Freedreno] [PATCH v3 04/10] drm/msm/dpu: Remove unused functions

2018-12-03 Thread Jordan Crouse
Remove some unused container_of() helper functions.

v3: No changes
v2: Retained still used helper functions in the name of readability

Reviewed-by: Sean Paul 
Signed-off-by: Jordan Crouse 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h | 10 --
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.h | 10 --
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h | 10 --
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h  | 10 --
 4 files changed, 40 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h
index 3b77df460dea..a2b0dbc23058 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h
@@ -91,16 +91,6 @@ struct dpu_hw_intf {
struct dpu_hw_intf_ops ops;
 };
 
-/**
- * to_dpu_hw_intf - convert base object dpu_hw_base to container
- * @hw: Pointer to base hardware block
- * return: Pointer to hardware block container
- */
-static inline struct dpu_hw_intf *to_dpu_hw_intf(struct dpu_hw_blk *hw)
-{
-   return container_of(hw, struct dpu_hw_intf, base);
-}
-
 /**
  * dpu_hw_intf_init(): Initializes the intf driver for the passed
  * interface idx.
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.h 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.h
index 3caccd7d6a3e..0e02e43cee14 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.h
@@ -104,16 +104,6 @@ struct dpu_hw_pingpong {
struct dpu_hw_pingpong_ops ops;
 };
 
-/**
- * dpu_hw_pingpong - convert base object dpu_hw_base to container
- * @hw: Pointer to base hardware block
- * return: Pointer to hardware block container
- */
-static inline struct dpu_hw_pingpong *to_dpu_hw_pingpong(struct dpu_hw_blk *hw)
-{
-   return container_of(hw, struct dpu_hw_pingpong, base);
-}
-
 /**
  * dpu_hw_pingpong_init - initializes the pingpong driver for the passed
  * pingpong idx.
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
index 4d81e5f5ce1b..119b4e1c16be 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
@@ -391,16 +391,6 @@ struct dpu_hw_pipe {
struct dpu_hw_sspp_ops ops;
 };
 
-/**
- * dpu_hw_pipe - convert base object dpu_hw_base to container
- * @hw: Pointer to base hardware block
- * return: Pointer to hardware block container
- */
-static inline struct dpu_hw_pipe *to_dpu_hw_pipe(struct dpu_hw_blk *hw)
-{
-   return container_of(hw, struct dpu_hw_pipe, base);
-}
-
 /**
  * dpu_hw_sspp_init - initializes the sspp hw driver object.
  * Should be called once before accessing every pipe.
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h
index 192e338f20bb..aa21fd834398 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h
@@ -160,16 +160,6 @@ struct dpu_hw_mdp {
struct dpu_hw_mdp_ops ops;
 };
 
-/**
- * to_dpu_hw_mdp - convert base object dpu_hw_base to container
- * @hw: Pointer to base hardware block
- * return: Pointer to hardware block container
- */
-static inline struct dpu_hw_mdp *to_dpu_hw_mdp(struct dpu_hw_blk *hw)
-{
-   return container_of(hw, struct dpu_hw_mdp, base);
-}
-
 /**
  * dpu_hw_mdptop_init - initializes the top driver for the passed idx
  * @idx:  Interface index for which driver object is required
-- 
2.18.0

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


[Freedreno] [PATCH v3 00/10] drm/msm/dpu: cleanups

2018-12-03 Thread Jordan Crouse
This is a rebase of 

https://patchwork.freedesktop.org/series/51214/

On top of

https://gitlab.freedesktop.org/seanpaul/dpu-staging/tags/for_jcrouse

Which should be up to date for the latest and greatest of the DPU. This should
be mostly unchanged since the last revision with the exception that I dropped
("drm/msm/dpu: Use DEFINE_SHOW_ATTRIBUTE") in favor of

https://patchwork.freedesktop.org/patch/265159/

Which does the same thing but is a little bit more tree-wide in its efforts.

Jordan Crouse (10):
  drm/msm/dpu: Remove dpu_dbg
  drm/msm/dpu: Remove dpu_crtc_get_mixer_height
  drm/msm/dpu: Remove dpu_crtc_is_enabled()
  drm/msm/dpu: Remove unused functions
  drm/msm/dpu: Cleanup callers of dpu_hw_blk_init
  drm/msm: Make irq_postinstall optional
  drm/msm/dpu: Remove dpu_irq and unused functions
  drm/msm/dpu: Cleanup the debugfs functions
  drm/msm/dpu: Further cleanups for static inline functions
  drm/msm/dpu: Clean up dpu_media_info.h static inline functions

 drivers/gpu/drm/msm/Makefile  |4 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c  |   45 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h  |   16 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c |  122 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h |7 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c  |   45 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h  |   32 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.c   | 2393 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.h   |  103 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c   |   35 +-
 .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h  |2 +-
 .../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c  |   12 +-
 .../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c  |1 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.c|   10 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.h|2 +-
 .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h|9 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c|   18 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c   |   18 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h   |   10 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c |   24 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.h |5 -
 .../gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c   |   18 +-
 .../gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.h   |   10 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c   |   21 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h   |   10 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c|   20 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h|   10 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_vbif.c   |1 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_irq.c   |   66 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_irq.h   |   59 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c   |  154 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h   |8 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c  |8 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c |  108 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c  |   24 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h  |   15 +-
 .../gpu/drm/msm/disp/dpu1/msm_media_info.h|  164 +-
 drivers/gpu/drm/msm/msm_drv.c |6 +-
 38 files changed, 221 insertions(+), 3394 deletions(-)
 delete mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.c
 delete mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.h
 delete mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_irq.c
 delete mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_irq.h

-- 
2.18.0

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


[Freedreno] [PATCH v3 05/10] drm/msm/dpu: Cleanup callers of dpu_hw_blk_init

2018-12-03 Thread Jordan Crouse
Outside of superfluous parameter checks the dpu_hw_blk_init()
doesn't have any failure paths. Switch it over to be a void
function and we can remove error handling paths in all the functions
that call it. While we're in those functions remove unneeded
initialization for a static variable.

v3: No changes
v2: Removed a cleanup intended for a different patch

Reviewed-by: Sean Paul 
Signed-off-by: Jordan Crouse 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.c  | 10 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.h  |  2 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c  | 17 ++---
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 17 ++---
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c   | 17 ++---
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c | 17 ++---
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 17 ++---
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c  | 17 ++---
 8 files changed, 14 insertions(+), 100 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.c
index 58d29e43faef..92f1c4241b9a 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.c
@@ -30,16 +30,10 @@ static LIST_HEAD(dpu_hw_blk_list);
  * @type: hw block type - enum dpu_hw_blk_type
  * @id: instance id of the hw block
  * @ops: Pointer to block operations
- * return: 0 if success; error code otherwise
  */
-int dpu_hw_blk_init(struct dpu_hw_blk *hw_blk, u32 type, int id,
+void dpu_hw_blk_init(struct dpu_hw_blk *hw_blk, u32 type, int id,
struct dpu_hw_blk_ops *ops)
 {
-   if (!hw_blk) {
-   pr_err("invalid parameters\n");
-   return -EINVAL;
-   }
-
INIT_LIST_HEAD(&hw_blk->list);
hw_blk->type = type;
hw_blk->id = id;
@@ -51,8 +45,6 @@ int dpu_hw_blk_init(struct dpu_hw_blk *hw_blk, u32 type, int 
id,
mutex_lock(&dpu_hw_blk_lock);
list_add(&hw_blk->list, &dpu_hw_blk_list);
mutex_unlock(&dpu_hw_blk_lock);
-
-   return 0;
 }
 
 /**
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.h 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.h
index 0f4ca8af1ec5..1934c2f7e8fa 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.h
@@ -44,7 +44,7 @@ struct dpu_hw_blk {
struct dpu_hw_blk_ops ops;
 };
 
-int dpu_hw_blk_init(struct dpu_hw_blk *hw_blk, u32 type, int id,
+void dpu_hw_blk_init(struct dpu_hw_blk *hw_blk, u32 type, int id,
struct dpu_hw_blk_ops *ops);
 void dpu_hw_blk_destroy(struct dpu_hw_blk *hw_blk);
 
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
index 4aab04335c6d..1068b4b7940f 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
@@ -483,10 +483,7 @@ static void _setup_ctl_ops(struct dpu_hw_ctl_ops *ops,
ops->get_bitmask_intf = dpu_hw_ctl_get_bitmask_intf;
 };
 
-static struct dpu_hw_blk_ops dpu_hw_ops = {
-   .start = NULL,
-   .stop = NULL,
-};
+static struct dpu_hw_blk_ops dpu_hw_ops;
 
 struct dpu_hw_ctl *dpu_hw_ctl_init(enum dpu_ctl idx,
void __iomem *addr,
@@ -494,7 +491,6 @@ struct dpu_hw_ctl *dpu_hw_ctl_init(enum dpu_ctl idx,
 {
struct dpu_hw_ctl *c;
struct dpu_ctl_cfg *cfg;
-   int rc;
 
c = kzalloc(sizeof(*c), GFP_KERNEL);
if (!c)
@@ -513,18 +509,9 @@ struct dpu_hw_ctl *dpu_hw_ctl_init(enum dpu_ctl idx,
c->mixer_count = m->mixer_count;
c->mixer_hw_caps = m->mixer;
 
-   rc = dpu_hw_blk_init(&c->base, DPU_HW_BLK_CTL, idx, &dpu_hw_ops);
-   if (rc) {
-   DPU_ERROR("failed to init hw blk %d\n", rc);
-   goto blk_init_error;
-   }
+   dpu_hw_blk_init(&c->base, DPU_HW_BLK_CTL, idx, &dpu_hw_ops);
 
return c;
-
-blk_init_error:
-   kzfree(c);
-
-   return ERR_PTR(rc);
 }
 
 void dpu_hw_ctl_destroy(struct dpu_hw_ctl *ctx)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
index 695d27a730e8..f6a83daa385b 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
@@ -264,10 +264,7 @@ static void _setup_intf_ops(struct dpu_hw_intf_ops *ops,
ops->get_line_count = dpu_hw_intf_get_line_count;
 }
 
-static struct dpu_hw_blk_ops dpu_hw_ops = {
-   .start = NULL,
-   .stop = NULL,
-};
+static struct dpu_hw_blk_ops dpu_hw_ops;
 
 struct dpu_hw_intf *dpu_hw_intf_init(enum dpu_intf idx,
void __iomem *addr,
@@ -275,7 +272,6 @@ struct dpu_hw_intf *dpu_hw_intf_init(enum dpu_intf idx,
 {
struct dpu_hw_intf *c;
struct dpu_intf_cfg *cfg;
-   int rc;
 
c = kzalloc(sizeof(*c), GFP_KERNEL);
if (!c)
@@ -296,18 +292,9 @@ struct dpu_hw_intf *dpu_hw_intf_init(enum dpu_intf idx,
c-

[Freedreno] [PATCH v3 09/10] drm/msm/dpu: Further cleanups for static inline functions

2018-12-03 Thread Jordan Crouse
Remove more static inline functions that are lightly used and/or
very simple and easy to build into the calling functions.

v3: Fix a nit from Sean Paul
v2: Removed another unused function from dpu_hw_lm.c and add back
dpu_crtc_get_client_type() since there was a question regarding
its usefulness.

Reviewed-by: Sean Paul 
Signed-off-by: Jordan Crouse 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c   | 12 +++-
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h   | 10 --
 .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h   |  2 +-
 .../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c   | 11 ++-
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h |  9 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c  |  6 --
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.h  |  5 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c|  3 ++-
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c|  8 +---
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c  | 18 --
 10 files changed, 13 insertions(+), 71 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 306e3df6fd63..f9a7479b8f1c 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -46,12 +46,6 @@
 #define LEFT_MIXER 0
 #define RIGHT_MIXER 1
 
-static inline int _dpu_crtc_get_mixer_width(struct dpu_crtc_state *cstate,
-   struct drm_display_mode *mode)
-{
-   return mode->hdisplay / cstate->num_mixers;
-}
-
 static struct dpu_kms *_dpu_crtc_get_kms(struct drm_crtc *crtc)
 {
struct msm_drm_private *priv = crtc->dev->dev_private;
@@ -497,7 +491,7 @@ static void _dpu_crtc_setup_lm_bounds(struct drm_crtc *crtc,
 {
struct dpu_crtc_state *cstate = to_dpu_crtc_state(state);
struct drm_display_mode *adj_mode = &state->adjusted_mode;
-   u32 crtc_split_width = _dpu_crtc_get_mixer_width(cstate, adj_mode);
+   u32 crtc_split_width = adj_mode->hdisplay / cstate->num_mixers;
int i;
 
for (i = 0; i < cstate->num_mixers; i++) {
@@ -940,7 +934,7 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
 
memset(pipe_staged, 0, sizeof(pipe_staged));
 
-   mixer_width = _dpu_crtc_get_mixer_width(cstate, mode);
+   mixer_width = mode->hdisplay / cstate->num_mixers;
 
_dpu_crtc_setup_lm_bounds(crtc, state);
 
@@ -1181,7 +1175,7 @@ static int _dpu_debugfs_status_show(struct seq_file *s, 
void *data)
cstate = to_dpu_crtc_state(crtc->state);
 
mode = &crtc->state->adjusted_mode;
-   out_width = _dpu_crtc_get_mixer_width(cstate, mode);
+   out_width = mode->hdisplay / cstate->num_mixers;
 
seq_printf(s, "crtc:%d width:%d height:%d\n", crtc->base.id,
mode->hdisplay, mode->vdisplay);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
index 94f5cea4e0d2..dbfb38a1986c 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
@@ -226,16 +226,6 @@ struct dpu_crtc_state {
 #define to_dpu_crtc_state(x) \
container_of(x, struct dpu_crtc_state, base)
 
-/**
- * dpu_crtc_state_is_stereo - Is crtc virtualized with two mixers?
- * @cstate: Pointer to dpu crtc state
- * @Return: true - has two mixers, false - has one mixer
- */
-static inline bool dpu_crtc_state_is_stereo(struct dpu_crtc_state *cstate)
-{
-   return cstate->num_mixers == CRTC_DUAL_MIXERS;
-}
-
 /**
  * dpu_crtc_frame_pending - retun the number of pending frames
  * @crtc: Pointer to drm crtc object
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
index 3a67bb9f9d9d..44e6f8b68e70 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
@@ -350,7 +350,7 @@ static inline enum dpu_3d_blend_mode 
dpu_encoder_helper_get_3d_blend_mode(
dpu_cstate = to_dpu_crtc_state(phys_enc->parent->crtc->state);
 
if (phys_enc->split_role == ENC_ROLE_SOLO &&
-   dpu_crtc_state_is_stereo(dpu_cstate))
+   dpu_cstate->num_mixers == CRTC_DUAL_MIXERS)
return BLEND_3D_H_ROW_INT;
 
return BLEND_3D_NONE;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
index b37a0992e326..99ab5ca9bed3 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
@@ -44,14 +44,7 @@
 
 #define DPU_ENC_WR_PTR_START_TIMEOUT_US 2
 
-static inline int _dpu_encoder_phys_cmd_get_idle_timeout(
-   struct dpu_encoder_phys_cmd *cmd_enc)
-{
-   return KICKOFF_TIMEOUT_MS;
-}
-
-static inline bool dpu_encoder_phys_cmd_is_master(
-   struct dpu_encoder_phys *phys_enc)
+static bool dpu_encoder_phys_cmd_is_master(struct dpu_encoder_phys *phys_enc)
 {

[Freedreno] [PATCH v3 03/10] drm/msm/dpu: Remove dpu_crtc_is_enabled()

2018-12-03 Thread Jordan Crouse
The static inline function dpu_crtc_enabled() is only called once
and the function that calls it in turn is only called once and
the return value can be easily checked in the calling functions
so collapse everything down.

v3: No changes

Reviewed-by: Sean Paul 
Signed-off-by: Jordan Crouse 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 17 ++---
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h  |  9 -
 2 files changed, 6 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
index bffc51e496e7..e8a87f4b8e0e 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
@@ -57,18 +57,13 @@ static struct dpu_kms *_dpu_crtc_get_kms(struct drm_crtc 
*crtc)
return to_dpu_kms(priv->kms);
 }
 
-static bool _dpu_core_perf_crtc_is_power_on(struct drm_crtc *crtc)
-{
-   return dpu_crtc_is_enabled(crtc);
-}
-
 static bool _dpu_core_video_mode_intf_connected(struct drm_crtc *crtc)
 {
struct drm_crtc *tmp_crtc;
 
drm_for_each_crtc(tmp_crtc, crtc->dev) {
if ((dpu_crtc_get_intf_mode(tmp_crtc) == INTF_MODE_VIDEO) &&
-   _dpu_core_perf_crtc_is_power_on(tmp_crtc)) {
+   tmp_crtc->enabled) {
DPU_DEBUG("video interface connected crtc:%d\n",
tmp_crtc->base.id);
return true;
@@ -164,7 +159,7 @@ int dpu_core_perf_crtc_check(struct drm_crtc *crtc,
curr_client_type = dpu_crtc_get_client_type(crtc);
 
drm_for_each_crtc(tmp_crtc, crtc->dev) {
-   if (_dpu_core_perf_crtc_is_power_on(tmp_crtc) &&
+   if (tmp_crtc->enabled &&
(dpu_crtc_get_client_type(tmp_crtc) ==
curr_client_type) &&
(tmp_crtc != crtc)) {
@@ -223,7 +218,7 @@ static int _dpu_core_perf_crtc_update_bus(struct dpu_kms 
*kms,
int ret = 0;
 
drm_for_each_crtc(tmp_crtc, crtc->dev) {
-   if (_dpu_core_perf_crtc_is_power_on(tmp_crtc) &&
+   if (tmp_crtc->enabled &&
curr_client_type ==
dpu_crtc_get_client_type(tmp_crtc)) {
dpu_cstate = to_dpu_crtc_state(tmp_crtc->state);
@@ -280,7 +275,7 @@ void dpu_core_perf_crtc_release_bw(struct drm_crtc *crtc)
 */
if (dpu_crtc_get_intf_mode(crtc) == INTF_MODE_CMD)
drm_for_each_crtc(tmp_crtc, crtc->dev) {
-   if (_dpu_core_perf_crtc_is_power_on(tmp_crtc) &&
+   if (tmp_crtc->enabled &&
dpu_crtc_get_intf_mode(tmp_crtc) ==
INTF_MODE_VIDEO)
return;
@@ -315,7 +310,7 @@ static u64 _dpu_core_perf_get_core_clk_rate(struct dpu_kms 
*kms)
struct dpu_crtc_state *dpu_cstate;
 
drm_for_each_crtc(crtc, kms->dev) {
-   if (_dpu_core_perf_crtc_is_power_on(crtc)) {
+   if (crtc->enabled) {
dpu_cstate = to_dpu_crtc_state(crtc->state);
clk_rate = max(dpu_cstate->new_perf.core_clk_rate,
clk_rate);
@@ -366,7 +361,7 @@ int dpu_core_perf_crtc_update(struct drm_crtc *crtc,
old = &dpu_crtc->cur_perf;
new = &dpu_cstate->new_perf;
 
-   if (_dpu_core_perf_crtc_is_power_on(crtc) && !stop_req) {
+   if (crtc->enabled && !stop_req) {
for (i = 0; i < DPU_CORE_PERF_DATA_BUS_ID_MAX; i++) {
/*
 * cases for bus bandwidth update.
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
index b84dc5730a0e..94f5cea4e0d2 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
@@ -309,13 +309,4 @@ static inline enum dpu_crtc_client_type 
dpu_crtc_get_client_type(
return crtc && crtc->state ? RT_CLIENT : NRT_CLIENT;
 }
 
-/**
- * dpu_crtc_is_enabled - check if dpu crtc is enabled or not
- * @crtc: Pointer to crtc
- */
-static inline bool dpu_crtc_is_enabled(struct drm_crtc *crtc)
-{
-   return crtc ? crtc->enabled : false;
-}
-
 #endif /* _DPU_CRTC_H_ */
-- 
2.18.0

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


[Freedreno] [PATCH v3 02/10] drm/msm/dpu: Remove dpu_crtc_get_mixer_height

2018-12-03 Thread Jordan Crouse
dpu_crtc_get_mixer_height() is only used once and the value it
returns can be easily derived from the calling function.

v3: No changes

Reviewed-by: Sean Paul 
Signed-off-by: Jordan Crouse 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c |  3 +--
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h | 13 -
 2 files changed, 1 insertion(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 949517b25431..39a0a6854b6c 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -495,7 +495,6 @@ static void _dpu_crtc_setup_mixers(struct drm_crtc *crtc)
 static void _dpu_crtc_setup_lm_bounds(struct drm_crtc *crtc,
struct drm_crtc_state *state)
 {
-   struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
struct dpu_crtc_state *cstate = to_dpu_crtc_state(state);
struct drm_display_mode *adj_mode = &state->adjusted_mode;
u32 crtc_split_width = _dpu_crtc_get_mixer_width(cstate, adj_mode);
@@ -506,7 +505,7 @@ static void _dpu_crtc_setup_lm_bounds(struct drm_crtc *crtc,
r->x1 = crtc_split_width * i;
r->y1 = 0;
r->x2 = r->x1 + crtc_split_width;
-   r->y2 = dpu_crtc_get_mixer_height(dpu_crtc, cstate, adj_mode);
+   r->y2 = adj_mode->vdisplay;
 
trace_dpu_crtc_setup_lm_bounds(DRMID(crtc), i, r);
}
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
index fc7123573891..b84dc5730a0e 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
@@ -236,19 +236,6 @@ static inline bool dpu_crtc_state_is_stereo(struct 
dpu_crtc_state *cstate)
return cstate->num_mixers == CRTC_DUAL_MIXERS;
 }
 
-/**
- * dpu_crtc_get_mixer_height - get the mixer height
- * Mixer height will be same as panel height
- */
-static inline int dpu_crtc_get_mixer_height(struct dpu_crtc *dpu_crtc,
-   struct dpu_crtc_state *cstate, struct drm_display_mode *mode)
-{
-   if (!dpu_crtc || !cstate || !mode)
-   return 0;
-
-   return mode->vdisplay;
-}
-
 /**
  * dpu_crtc_frame_pending - retun the number of pending frames
  * @crtc: Pointer to drm crtc object
-- 
2.18.0

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


[Freedreno] [PATCH v3 08/10] drm/msm/dpu: Cleanup the debugfs functions

2018-12-03 Thread Jordan Crouse
Do some debugfs cleanups from across the DPU driver. The DRM
destroy functions will do a recursive delete on the entire
debugfs node so there is no need to store dentry pointers for
the debugfs files that are persistent for the life of the
driver. This also means that the destroy functions can go
away too.

Also, use standard API functions where applicable instead of
using hand written code.

v3: No changes
v2: Add more code; most of the dpu debugfs files should be
addressed now.

Reviewed-by: Sean Paul 
Signed-off-by: Jordan Crouse 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c  |  30 +
 drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h  |   9 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 105 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h |   7 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c  |  30 +
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c   |  31 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c   | 104 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h   |   6 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c  |   3 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c |  90 +++
 drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c  |  24 ++--
 drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h  |  15 +--
 12 files changed, 93 insertions(+), 361 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c
index 9d5a8d217bc6..e45c69044935 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c
@@ -319,10 +319,8 @@ static int dpu_debugfs_core_irq_show(struct seq_file *s, 
void *v)
unsigned long irq_flags;
int i, irq_count, enable_count, cb_count;
 
-   if (!irq_obj || !irq_obj->enable_counts || !irq_obj->irq_cb_tbl) {
-   DPU_ERROR("invalid parameters\n");
+   if (WARN_ON(!irq_obj->enable_counts || !irq_obj->irq_cb_tbl))
return 0;
-   }
 
for (i = 0; i < irq_obj->total_irqs; i++) {
spin_lock_irqsave(&irq_obj->cb_lock, irq_flags);
@@ -343,31 +341,11 @@ static int dpu_debugfs_core_irq_show(struct seq_file *s, 
void *v)
 
 DEFINE_DPU_DEBUGFS_SEQ_FOPS(dpu_debugfs_core_irq);
 
-int dpu_debugfs_core_irq_init(struct dpu_kms *dpu_kms,
-   struct dentry *parent)
-{
-   dpu_kms->irq_obj.debugfs_file = debugfs_create_file("core_irq", 0600,
-   parent, &dpu_kms->irq_obj,
-   &dpu_debugfs_core_irq_fops);
-
-   return 0;
-}
-
-void dpu_debugfs_core_irq_destroy(struct dpu_kms *dpu_kms)
-{
-   debugfs_remove(dpu_kms->irq_obj.debugfs_file);
-   dpu_kms->irq_obj.debugfs_file = NULL;
-}
-
-#else
-int dpu_debugfs_core_irq_init(struct dpu_kms *dpu_kms,
+void dpu_debugfs_core_irq_init(struct dpu_kms *dpu_kms,
struct dentry *parent)
 {
-   return 0;
-}
-
-void dpu_debugfs_core_irq_destroy(struct dpu_kms *dpu_kms)
-{
+   debugfs_create_file("core_irq", 0600, parent, &dpu_kms->irq_obj,
+   &dpu_debugfs_core_irq_fops);
 }
 #endif
 
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h
index 884f77fa3eb6..e9015a2b23fe 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h
@@ -132,15 +132,8 @@ int dpu_core_irq_unregister_callback(
  * dpu_debugfs_core_irq_init - register core irq debugfs
  * @dpu_kms: pointer to kms
  * @parent: debugfs directory root
- * @Return: 0 on success
  */
-int dpu_debugfs_core_irq_init(struct dpu_kms *dpu_kms,
+void dpu_debugfs_core_irq_init(struct dpu_kms *dpu_kms,
struct dentry *parent);
 
-/**
- * dpu_debugfs_core_irq_destroy - deregister core irq debugfs
- * @dpu_kms: pointer to kms
- */
-void dpu_debugfs_core_irq_destroy(struct dpu_kms *dpu_kms);
-
 #endif /* __DPU_CORE_IRQ_H__ */
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
index e8a87f4b8e0e..9f20f397f77d 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
@@ -24,8 +24,6 @@
 #include "dpu_crtc.h"
 #include "dpu_core_perf.h"
 
-#define DPU_PERF_MODE_STRING_SIZE  128
-
 /**
  * enum dpu_perf_mode - performance tuning mode
  * @DPU_PERF_MODE_NORMAL: performance controlled by user mode client
@@ -451,24 +449,14 @@ static ssize_t _dpu_core_perf_mode_write(struct file 
*file,
struct dpu_core_perf *perf = file->private_data;
struct dpu_perf_cfg *cfg = &perf->catalog->perf;
u32 perf_mode = 0;
-   char buf[10];
-
-   if (!perf)
-   return -ENODEV;
-
-   if (count >= sizeof(buf))
-   return -EFAULT;
-
-   if (copy_from_user(buf, user_buf, count))
-   return -EFAULT;
-
-   buf[count] = 0; /* end of string */
+   int ret;
 
-   if (kstrtouint(buf, 0, &perf_mode))
-   return -EFAULT;
+  

[Freedreno] [PATCH v3 07/10] drm/msm/dpu: Remove dpu_irq and unused functions

2018-12-03 Thread Jordan Crouse
dpu_irq.c does some unneeded checks and passes control
to dpu_core_irq.c  The simple functions can be defined
in the same file where we use them and the files and
their associated hangers on can be deleted.

Additionally the postinstall hook isn't used even
in dpu_core_irq.c so zap that entire path.

v3: No changes

Reviewed-by: Sean Paul 
Signed-off-by: Jordan Crouse 
---
 drivers/gpu/drm/msm/Makefile |  1 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c | 15 +
 drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h |  7 ---
 drivers/gpu/drm/msm/disp/dpu1/dpu_irq.c  | 66 
 drivers/gpu/drm/msm/disp/dpu1/dpu_irq.h  | 59 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c  | 22 ++-
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h  |  1 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c |  5 ++
 8 files changed, 28 insertions(+), 148 deletions(-)
 delete mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_irq.c
 delete mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_irq.h

diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
index b3c45130a5e2..ae70fca6ee3b 100644
--- a/drivers/gpu/drm/msm/Makefile
+++ b/drivers/gpu/drm/msm/Makefile
@@ -68,7 +68,6 @@ msm-y := \
disp/dpu1/dpu_hw_util.o \
disp/dpu1/dpu_hw_vbif.o \
disp/dpu1/dpu_io_util.o \
-   disp/dpu1/dpu_irq.o \
disp/dpu1/dpu_kms.o \
disp/dpu1/dpu_mdss.o \
disp/dpu1/dpu_plane.o \
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c
index 879c13fe74e0..9d5a8d217bc6 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c
@@ -376,10 +376,7 @@ void dpu_core_irq_preinstall(struct dpu_kms *dpu_kms)
struct msm_drm_private *priv;
int i;
 
-   if (!dpu_kms) {
-   DPU_ERROR("invalid dpu_kms\n");
-   return;
-   } else if (!dpu_kms->dev) {
+   if (!dpu_kms->dev) {
DPU_ERROR("invalid drm device\n");
return;
} else if (!dpu_kms->dev->dev_private) {
@@ -410,20 +407,12 @@ void dpu_core_irq_preinstall(struct dpu_kms *dpu_kms)
}
 }
 
-int dpu_core_irq_postinstall(struct dpu_kms *dpu_kms)
-{
-   return 0;
-}
-
 void dpu_core_irq_uninstall(struct dpu_kms *dpu_kms)
 {
struct msm_drm_private *priv;
int i;
 
-   if (!dpu_kms) {
-   DPU_ERROR("invalid dpu_kms\n");
-   return;
-   } else if (!dpu_kms->dev) {
+   if (!dpu_kms->dev) {
DPU_ERROR("invalid drm device\n");
return;
} else if (!dpu_kms->dev->dev_private) {
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h
index 5e98bba46af5..884f77fa3eb6 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h
@@ -23,13 +23,6 @@
  */
 void dpu_core_irq_preinstall(struct dpu_kms *dpu_kms);
 
-/**
- * dpu_core_irq_postinstall - perform post-installation of core IRQ handler
- * @dpu_kms:   DPU handle
- * @return:0 if success; error code otherwise
- */
-int dpu_core_irq_postinstall(struct dpu_kms *dpu_kms);
-
 /**
  * dpu_core_irq_uninstall - uninstall core IRQ handler
  * @dpu_kms:   DPU handle
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_irq.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_irq.c
deleted file mode 100644
index d5e6ce0140cf..
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_irq.c
+++ /dev/null
@@ -1,66 +0,0 @@
-/* Copyright (c) 2015-2018, The Linux Foundation. All rights reserved.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 and
- * only version 2 as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- */
-
-#define pr_fmt(fmt)"[drm:%s:%d] " fmt, __func__, __LINE__
-
-#include 
-#include 
-#include 
-
-#include "dpu_irq.h"
-#include "dpu_core_irq.h"
-
-irqreturn_t dpu_irq(struct msm_kms *kms)
-{
-   struct dpu_kms *dpu_kms = to_dpu_kms(kms);
-
-   return dpu_core_irq(dpu_kms);
-}
-
-void dpu_irq_preinstall(struct msm_kms *kms)
-{
-   struct dpu_kms *dpu_kms = to_dpu_kms(kms);
-
-   if (!dpu_kms->dev || !dpu_kms->dev->dev) {
-   pr_err("invalid device handles\n");
-   return;
-   }
-
-   dpu_core_irq_preinstall(dpu_kms);
-}
-
-int dpu_irq_postinstall(struct msm_kms *kms)
-{
-   struct dpu_kms *dpu_kms = to_dpu_kms(kms);
-   int rc;
-
-   if (!kms) {
-   DPU_ERROR("invalid parameters\n");
-   return -EINVAL;
-   }
-
-   rc = dpu_core_irq_postinstall(dpu_kms);
-
-  

[Freedreno] [PATCH v5 1/3] arm64: dts: qcom: sdm845: Add dpu to sdm845 dts file

2018-12-03 Thread Jeykumar Sankaran
DPU is short for the Display Processing Unit. It is the display
controller on Qualcomm SDM845 chips.

This change adds MDSS and DSI nodes to enable display on the
target device.

Changes in v2:
 - Beefed up commit message
 - Use SoC specific compatibles for mdss and dpu (Rob H)
 - Use assigned-clocks to set initial clock frequency(Rob H)
Changes in v3:
 - added IOMMU node
 - Fix device naming (remove _phys)
 - Use correct IRQ_TYPE in interrupt specifiers
Changes in v4:
 - move mdss node to preserve the unit address sort order
 - remove _clk suffix from dsi clocks
 (both the comments are from Doug Anderson)
Changes in v5:
- Keep the device status "disabled" by default (Bjorn Andersson)
- Use MDSS_GDSC macro (Jordan)
- Fix phy-names (Jordan)
- List reg ranges in numerical order (Jordan)

Signed-off-by: Jeykumar Sankaran 
Signed-off-by: Sean Paul 
---
 arch/arm64/boot/dts/qcom/sdm845.dtsi | 203 +++
 1 file changed, 203 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi 
b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index 1419b00..a16f1fe 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -1256,6 +1256,209 @@
};
};
 
+   mdss: mdss@ae0 {
+   compatible = "qcom,sdm845-mdss";
+   reg = <0xae0 0x1000>;
+   reg-names = "mdss";
+
+   power-domains = <&dispcc MDSS_GDSC>;
+
+   clocks = <&gcc GCC_DISP_AHB_CLK>,
+<&gcc GCC_DISP_AXI_CLK>,
+<&dispcc DISP_CC_MDSS_MDP_CLK>;
+   clock-names = "iface", "bus", "core";
+
+   assigned-clocks = <&dispcc DISP_CC_MDSS_MDP_CLK>;
+   assigned-clock-rates = <3>;
+
+   interrupts = ;
+   interrupt-controller;
+   #interrupt-cells = <1>;
+
+   iommus = <&apps_smmu 0x880 0x8>,
+<&apps_smmu 0xc80 0x8>;
+
+   status = "disabled";
+
+   #address-cells = <1>;
+   #size-cells = <1>;
+   ranges;
+
+   mdss_mdp: mdp@ae01000 {
+   compatible = "qcom,sdm845-dpu";
+   reg = <0x0ae01000 0x8f000>,
+ <0x0aeb 0x2008>;
+   reg-names = "mdp", "vbif";
+
+   clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>,
+<&dispcc DISP_CC_MDSS_AXI_CLK>,
+<&dispcc DISP_CC_MDSS_MDP_CLK>,
+<&dispcc DISP_CC_MDSS_VSYNC_CLK>;
+   clock-names = "iface", "bus", "core", "vsync";
+
+   assigned-clocks = <&dispcc 
DISP_CC_MDSS_MDP_CLK>,
+ <&dispcc 
DISP_CC_MDSS_VSYNC_CLK>;
+   assigned-clock-rates = <3>,
+  <1920>;
+
+   interrupt-parent = <&mdss>;
+   interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
+
+   status = "disabled";
+
+   ports {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   port@0 {
+   reg = <0>;
+   dpu_intf1_out: endpoint {
+   remote-endpoint = 
<&dsi0_in>;
+   };
+   };
+
+   port@1 {
+   reg = <1>;
+   dpu_intf2_out: endpoint {
+   remote-endpoint = 
<&dsi1_in>;
+   };
+   };
+   };
+   };
+
+   dsi0: dsi@ae94000 {
+   compatible = "qcom,mdss-dsi-ctrl";
+   reg = <0xae94000 0x400>;
+   reg-names = "dsi_ctrl";
+
+   interrupt-parent = <&mdss>;
+   interrupts = <4 IRQ_TYPE_LEVEL_HIGH>;
+
+   clocks = <&dispcc DISP_CC_MDSS_BYTE0_CLK>,
+ 

Re: [Freedreno] [PATCH v3 4/4] ARM: dts: imx5: add gpu nodes

2018-12-03 Thread Jonathan marek

On 12/03/2018 05:10 PM, Jordan Crouse wrote:

On Mon, Dec 03, 2018 at 04:18:16PM -0500, Jonathan Marek wrote:

Signed-off-by: Jonathan Marek 
---
  arch/arm/boot/dts/imx51.dtsi | 17 +
  arch/arm/boot/dts/imx53.dtsi | 17 +
  2 files changed, 34 insertions(+)

diff --git a/arch/arm/boot/dts/imx51.dtsi b/arch/arm/boot/dts/imx51.dtsi
index 67d462715..e9a7bbce9 100644
--- a/arch/arm/boot/dts/imx51.dtsi
+++ b/arch/arm/boot/dts/imx51.dtsi
@@ -628,5 +628,22 @@
clock-names = "ipg", "ahb";
};
};
+
+   gpu: gpu@3000 {
+   compatible = "amd,imageon-200.1", "amd,imageon";
+   reg = <0x3000 0x2>;
+   reg-names = "kgsl_3d0_reg_memory";
+   interrupts = <12>;
+   interrupt-names = "kgsl_3d0_irq";
+   clocks = <&clks IMX5_CLK_GPU3D_GATE>, <&clks 
IMX5_CLK_GARB_GATE>;
+   clock-names = "core_clk", "mem_iface_clk";
+
+   qcom,gpu-pwrlevels {
+   compatible = "qcom,gpu-pwrlevels";
+   qcom,gpu-pwrlevel@0 {
+   qcom,gpu-freq = <16625>;
+   };
+   };


There shouldn't be any incremental cost in the source code to use OPP; it should
just work. And then this won't give us a further reason to keep the legacy
code around when we decide to dump any pretense of downstream "compatibility".



I will switch to OPP.


+   };
};
  };
diff --git a/arch/arm/boot/dts/imx53.dtsi b/arch/arm/boot/dts/imx53.dtsi
index 207eb557c..586d45586 100644
--- a/arch/arm/boot/dts/imx53.dtsi
+++ b/arch/arm/boot/dts/imx53.dtsi
@@ -838,5 +838,22 @@
reg = <0xf800 0x2>;
clocks = <&clks IMX5_CLK_OCRAM>;
};
+
+   gpu: gpu@3000 {
+   compatible = "amd,imageon-200.0", "amd,imageon";
+   reg = <0x3000 0x2>;
+   reg-names = "kgsl_3d0_reg_memory";
+   interrupts = <12>;
+   interrupt-names = "kgsl_3d0_irq";
+   clocks = <&clks IMX5_CLK_GPU3D_GATE>, <&clks 
IMX5_CLK_GARB_GATE>;
+   clock-names = "core_clk", "mem_iface_clk";
+
+   qcom,gpu-pwrlevels {
+   compatible = "qcom,gpu-pwrlevels";
+   qcom,gpu-pwrlevel@0 {
+   qcom,gpu-freq = <2>;
+   };


Same.


+   };



+   };
};
  };
--
2.17.1



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


Re: [Freedreno] [PATCH v3 4/4] ARM: dts: imx5: add gpu nodes

2018-12-03 Thread Jonathan marek

On 12/03/2018 05:16 PM, Fabio Estevam wrote:

Hi Jonathan,

Thanks for working on this. Really nice to see GPU support for mx51/mx53!

On Mon, Dec 3, 2018 at 7:21 PM Jonathan Marek  wrote:

Please add a commit log.


Signed-off-by: Jonathan Marek 



---
  arch/arm/boot/dts/imx51.dtsi | 17 +
  arch/arm/boot/dts/imx53.dtsi | 17 +
  2 files changed, 34 insertions(+)

diff --git a/arch/arm/boot/dts/imx51.dtsi b/arch/arm/boot/dts/imx51.dtsi
index 67d462715..e9a7bbce9 100644
--- a/arch/arm/boot/dts/imx51.dtsi
+++ b/arch/arm/boot/dts/imx51.dtsi
@@ -628,5 +628,22 @@
 clock-names = "ipg", "ahb";
 };
 };
+
+   gpu: gpu@3000 {


We put the peripheral nodes in address order, so this one should go
prior to the IPU node.


+   compatible = "amd,imageon-200.1", "amd,imageon";


I can't find the dt-bindings for these compatible entries. Have you
documented them?



It is the same as qcom,adreno which is documented here:

Documentation/devicetree/bindings/display/msm/gpu.txt

I guess I should add amd,imageon there.


+   reg = <0x3000 0x2>;
+   reg-names = "kgsl_3d0_reg_memory";
+   interrupts = <12>;
+   interrupt-names = "kgsl_3d0_irq";
+   clocks = <&clks IMX5_CLK_GPU3D_GATE>, <&clks 
IMX5_CLK_GARB_GATE>;
+   clock-names = "core_clk", "mem_iface_clk";
+
+   qcom,gpu-pwrlevels {
+   compatible = "qcom,gpu-pwrlevels";
+   qcom,gpu-pwrlevel@0 {


You could drop this @0


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


Re: [Freedreno] [PATCH v3 4/4] ARM: dts: imx5: add gpu nodes

2018-12-03 Thread Jordan Crouse
On Mon, Dec 03, 2018 at 04:18:16PM -0500, Jonathan Marek wrote:
> Signed-off-by: Jonathan Marek 
> ---
>  arch/arm/boot/dts/imx51.dtsi | 17 +
>  arch/arm/boot/dts/imx53.dtsi | 17 +
>  2 files changed, 34 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/imx51.dtsi b/arch/arm/boot/dts/imx51.dtsi
> index 67d462715..e9a7bbce9 100644
> --- a/arch/arm/boot/dts/imx51.dtsi
> +++ b/arch/arm/boot/dts/imx51.dtsi
> @@ -628,5 +628,22 @@
>   clock-names = "ipg", "ahb";
>   };
>   };
> +
> + gpu: gpu@3000 {
> + compatible = "amd,imageon-200.1", "amd,imageon";
> + reg = <0x3000 0x2>;
> + reg-names = "kgsl_3d0_reg_memory";
> + interrupts = <12>;
> + interrupt-names = "kgsl_3d0_irq";
> + clocks = <&clks IMX5_CLK_GPU3D_GATE>, <&clks 
> IMX5_CLK_GARB_GATE>;
> + clock-names = "core_clk", "mem_iface_clk";
> +
> + qcom,gpu-pwrlevels {
> + compatible = "qcom,gpu-pwrlevels";
> + qcom,gpu-pwrlevel@0 {
> + qcom,gpu-freq = <16625>;
> + };
> + };

There shouldn't be any incremental cost in the source code to use OPP; it should
just work. And then this won't give us a further reason to keep the legacy
code around when we decide to dump any pretense of downstream "compatibility".

> + };
>   };
>  };
> diff --git a/arch/arm/boot/dts/imx53.dtsi b/arch/arm/boot/dts/imx53.dtsi
> index 207eb557c..586d45586 100644
> --- a/arch/arm/boot/dts/imx53.dtsi
> +++ b/arch/arm/boot/dts/imx53.dtsi
> @@ -838,5 +838,22 @@
>   reg = <0xf800 0x2>;
>   clocks = <&clks IMX5_CLK_OCRAM>;
>   };
> +
> + gpu: gpu@3000 {
> + compatible = "amd,imageon-200.0", "amd,imageon";
> + reg = <0x3000 0x2>;
> + reg-names = "kgsl_3d0_reg_memory";
> + interrupts = <12>;
> + interrupt-names = "kgsl_3d0_irq";
> + clocks = <&clks IMX5_CLK_GPU3D_GATE>, <&clks 
> IMX5_CLK_GARB_GATE>;
> + clock-names = "core_clk", "mem_iface_clk";
> +
> + qcom,gpu-pwrlevels {
> + compatible = "qcom,gpu-pwrlevels";
> + qcom,gpu-pwrlevel@0 {
> + qcom,gpu-freq = <2>;
> + };

Same.

> + };

> + };
>   };
>  };
> -- 
> 2.17.1

-- 
The Qualcomm Innovation Center, Inc. is a member of 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 1/4] drm/edid: Pass connector to AVI inforframe functions

2018-12-03 Thread Ville Syrjälä
On Thu, Nov 29, 2018 at 09:46:16AM +0100, Andrzej Hajda wrote:
> Quite late, hopefully not too late.
> 
> 
> On 21.11.2018 12:51, Ville Syrjälä wrote:
> > On Wed, Nov 21, 2018 at 01:40:43PM +0200, Jani Nikula wrote:
> >>
> >>>   return;
> >>> diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c 
> >>> b/drivers/gpu/drm/bridge/sil-sii8620.c
> >>> index a6e8f4591e63..0cc293a6ac24 100644
> >>> --- a/drivers/gpu/drm/bridge/sil-sii8620.c
> >>> +++ b/drivers/gpu/drm/bridge/sil-sii8620.c
> >>> @@ -1104,8 +1104,7 @@ static void sii8620_set_infoframes(struct sii8620 
> >>> *ctx,
> >>>   int ret;
> >>>  
> >>>   ret = drm_hdmi_avi_infoframe_from_display_mode(&frm.avi,
> >>> -mode,
> >>> -true);
> >>> +NULL, mode);
> >>>   if (ctx->use_packed_pixel)
> >>>   frm.avi.colorspace = HDMI_COLORSPACE_YUV422;
> >>>  
> >>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c 
> >>> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> >>> index 64c3cf027518..88b720b63126 100644
> >>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> >>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> >>> @@ -1344,7 +1344,8 @@ static void hdmi_config_AVI(struct dw_hdmi *hdmi, 
> >>> struct drm_display_mode *mode)
> >>>   u8 val;
> >>>  
> >>>   /* Initialise info frame from DRM mode */
> >>> - drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
> >>> + drm_hdmi_avi_infoframe_from_display_mode(&frame,
> >>> +  &hdmi->connector, mode);
> >>>  
> >>>   if (hdmi_bus_fmt_is_yuv444(hdmi->hdmi_data.enc_out_bus_format))
> >>>   frame.colorspace = HDMI_COLORSPACE_YUV444;
> >>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> >>> index b506e3622b08..501ac05ba7da 100644
> >>> --- a/drivers/gpu/drm/drm_edid.c
> >>> +++ b/drivers/gpu/drm/drm_edid.c
> >>> @@ -4830,19 +4830,32 @@ void drm_set_preferred_mode(struct drm_connector 
> >>> *connector,
> >>>  }
> >>>  EXPORT_SYMBOL(drm_set_preferred_mode);
> >>>  
> >>> +static bool is_hdmi2_sink(struct drm_connector *connector)
> >> You're usually known for adding const all around, why not const pointer
> >> here and in all the other drm_* functions that call this?
> > My current approach is to constify states/fbs/etc. but not so much
> > crtcs/connectors/etc. Too much const can sometimes get in the way
> > of things requiring that you remove the const later. But I guess
> > in this case the const shouldn't really get in the way of anything
> > because these are pretty much supposed to be pure functions.
> >
> >>> +{
> >>> + /*
> >>> +  * FIXME: sil-sii8620 doesn't have a connector around when
> >>> +  * we need one, so we have to be prepared for a NULL connector.
> >>> +  */
> >>> + if (!connector)
> >>> + return false;
> >> This actually changes the is_hdmi2_sink value for sil-sii8620.
> > Hmm. No idea why they would have set that to true when everyone else is
> > passing false. 
> 
> 
> Because false does not work :) More precisely MHLv3 (used in Sii8620)
> uses CTA-861-F standard for infoframes, which is specific to HDMI2.0.
> 
> Unfortunately I have no access to MHL specs, but my experiments and
> vendor drivers strongly suggests it is done this way.
> 
> This is important in case of 4K modes which are handled differently by
> HDMI 1.4 and HDMI2.0.

HDMI 2.0 handles 4k just like 1.4 handled it when you use one of
the 4k modes defined in 1.4. Only if you use features beyond 1.4 do we
switch over to the HDMI 2.0 specific signalling.

> 
> The pipeline looks like (in parenthesis HDMI version on the stream):
> 
> exynos_hdmi --(1.4)--> SII8620 --(2.0)--> MHL_dongle --(1.4)--> TV
> 
> 
> > I guess I can change this to true to not change it. IIRC
> > that was the only driver that didn't have a connector around.
> >
> > That said, I was actually thinking of removing this hdmi2 vs. not
> > stuff from drm_hdmi_avi_infoframe_from_display_mode(). We added it
> > "just in case", but we already have a similar issue with earlier
> > cea-861 revisions and haven't seen any bugs where an older monitor
> > would get confused by a VIC not yet defined when the monitor was
> > produced.
> >
> Are you sure this is a good idea? As I said earlier 4K modes are present
> in HDMI 1.4 and 2.0 but they are handled differently, so this is not
> only matter of unknown VIC in AVIF.
> 
> 
> Regards
> 
> Andrzej

-- 
Ville Syrjälä
Intel
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


Re: [Freedreno] [PATCH v3 14/24] drm/msm: dpu: Grab the modeset locks in frame_event

2018-12-03 Thread Daniel Vetter
On Fri, Nov 30, 2018 at 05:00:02PM -0500, Sean Paul wrote:
> From: Sean Paul 
> 
> This patch wraps dpu_core_perf_crtc_release_bw() with modeset locks
> since it digs into the state objects.
> 
> Changes in v2:
> - None
> Changes in v3:
> - Use those nifty new DRM_MODESET_LOCK_ALL_* helpers (Daniel)
> 
> Cc: Daniel Vetter 
> Cc: Jeykumar Sankaran 
> Signed-off-by: Sean Paul 
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 15 ++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c 
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index 74ef384d9cd6a..03ddd281a354f 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -306,6 +306,19 @@ static void dpu_crtc_vblank_cb(void *data)
>   trace_dpu_crtc_vblank_cb(DRMID(crtc));
>  }
>  
> +static void dpu_crtc_release_bw_unlocked(struct drm_crtc *crtc)
> +{
> + int ret = 0;
> + struct drm_modeset_acquire_ctx ctx;
> +
> + DRM_MODESET_LOCK_ALL_BEGIN(crtc->dev, ctx, 0, ret);
> + dpu_core_perf_crtc_release_bw(crtc);
> + DRM_MODESET_LOCK_ALL_END(ctx, ret);

So pretty, and correct even :-)

Acked-by: Daniel Vetter 
> + if (ret)
> + DRM_ERROR("Failed to acquire modeset locks to release bw, %d\n",
> +   ret);
> +}
> +
>  static void dpu_crtc_frame_event_work(struct kthread_work *work)
>  {
>   struct dpu_crtc_frame_event *fevent = container_of(work,
> @@ -335,7 +348,7 @@ static void dpu_crtc_frame_event_work(struct kthread_work 
> *work)
>   /* release bandwidth and other resources */
>   trace_dpu_crtc_frame_event_done(DRMID(crtc),
>   fevent->event);
> - dpu_core_perf_crtc_release_bw(crtc);
> + dpu_crtc_release_bw_unlocked(crtc);
>   } else {
>   trace_dpu_crtc_frame_event_more_pending(DRMID(crtc),
>   fevent->event);
> -- 
> Sean Paul, Software Engineer, Google / Chromium OS
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


Re: [Freedreno] [PATCH 1/4] drm/edid: Pass connector to AVI inforframe functions

2018-12-03 Thread Ville Syrjälä
On Thu, Nov 29, 2018 at 10:08:07AM +0100, Andrzej Hajda wrote:
> On 21.11.2018 19:19, Laurent Pinchart wrote:
> > Hi Ville,
> >
> > Thank you for the patch.
> >
> > On Tuesday, 20 November 2018 18:13:42 EET Ville Syrjala wrote:
> >> From: Ville Syrjälä 
> >>
> >> Make life easier for drivers by simply passing the connector
> >> to drm_hdmi_avi_infoframe_from_display_mode() and
> >> drm_hdmi_avi_infoframe_quant_range(). That way drivers don't
> >> need to worry about is_hdmi2_sink mess.
> > While this is good for display controller drivers, the change isn't great 
> > for 
> > bridge drivers. Down the road we're looking at moving connector support out 
> > of 
> > the bridge drivers. Adding an additional dependency to connectors in the 
> > bridges will make that more difficult. Ideally bridges should retrieve the 
> > information from their sink, regardless of whether it is a connector or 
> > another bridge.
> 
> 
> I agree with it, and case of sii8620 shows that there are cases where
> bridge has no direct access to the connector.

It's just a matter of plumbing it through.

> 
> On the other side,  since you are passing connector to
> drm_hdmi_avi_infoframe_from_display_mode(), you could drop mode
> parameter and rename the function to
> drm_hdmi_avi_infoframe_from_connector() then, unless mode passed and
> mode set on the connector differs?

Connectors don't have a mode.

> 
> 
> Regards
> 
> Andrzej
> 
> 
> >
> > Please see below for an additional comment.
> >
> >> Cc: Alex Deucher 
> >> Cc: "Christian König" 
> >> Cc: "David (ChunMing) Zhou" 
> >> Cc: Archit Taneja 
> >> Cc: Andrzej Hajda 
> >> Cc: Laurent Pinchart 
> >> Cc: Inki Dae 
> >> Cc: Joonyoung Shim 
> >> Cc: Seung-Woo Kim 
> >> Cc: Kyungmin Park 
> >> Cc: Russell King 
> >> Cc: CK Hu 
> >> Cc: Philipp Zabel 
> >> Cc: Rob Clark 
> >> Cc: Ben Skeggs 
> >> Cc: Tomi Valkeinen 
> >> Cc: Sandy Huang 
> >> Cc: "Heiko Stübner" 
> >> Cc: Benjamin Gaignard 
> >> Cc: Vincent Abriou 
> >> Cc: Thierry Reding 
> >> Cc: Eric Anholt 
> >> Cc: Shawn Guo 
> >> Cc: Ilia Mirkin 
> >> Cc: amd-...@lists.freedesktop.org
> >> Cc: linux-arm-...@vger.kernel.org
> >> Cc: freedreno@lists.freedesktop.org
> >> Cc: nouv...@lists.freedesktop.org
> >> Cc: linux-te...@vger.kernel.org
> >> Signed-off-by: Ville Syrjälä 
> >> ---
> >>  drivers/gpu/drm/amd/amdgpu/dce_v10_0.c|  2 +-
> >>  drivers/gpu/drm/amd/amdgpu/dce_v11_0.c|  2 +-
> >>  drivers/gpu/drm/amd/amdgpu/dce_v6_0.c |  3 ++-
> >>  drivers/gpu/drm/amd/amdgpu/dce_v8_0.c |  2 +-
> >>  drivers/gpu/drm/bridge/analogix-anx78xx.c |  5 ++--
> >>  drivers/gpu/drm/bridge/sii902x.c  |  3 ++-
> >>  drivers/gpu/drm/bridge/sil-sii8620.c  |  3 +--
> >>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c |  3 ++-
> >>  drivers/gpu/drm/drm_edid.c| 33 ++-
> >>  drivers/gpu/drm/exynos/exynos_hdmi.c  |  3 ++-
> >>  drivers/gpu/drm/i2c/tda998x_drv.c |  3 ++-
> >>  drivers/gpu/drm/i915/intel_hdmi.c | 14 +-
> >>  drivers/gpu/drm/i915/intel_lspcon.c   | 15 ++-
> >>  drivers/gpu/drm/i915/intel_sdvo.c | 10 ---
> >>  drivers/gpu/drm/mediatek/mtk_hdmi.c   |  3 ++-
> >>  drivers/gpu/drm/msm/hdmi/hdmi_bridge.c|  3 ++-
> >>  drivers/gpu/drm/nouveau/dispnv50/disp.c   |  7 +++--
> >>  drivers/gpu/drm/omapdrm/omap_encoder.c|  5 ++--
> >>  drivers/gpu/drm/radeon/radeon_audio.c |  2 +-
> >>  drivers/gpu/drm/rockchip/inno_hdmi.c  |  4 ++-
> >>  drivers/gpu/drm/sti/sti_hdmi.c|  3 ++-
> >>  drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c|  3 ++-
> >>  drivers/gpu/drm/tegra/hdmi.c  |  3 ++-
> >>  drivers/gpu/drm/tegra/sor.c   |  3 ++-
> >>  drivers/gpu/drm/vc4/vc4_hdmi.c| 11 +---
> >>  drivers/gpu/drm/zte/zx_hdmi.c |  4 ++-
> >>  include/drm/drm_edid.h|  8 +++---
> >>  27 files changed, 94 insertions(+), 66 deletions(-)
> > For dw-hdmi and omapdrm,
> >
> > Reviewed-by: Laurent Pinchart 
> >

-- 
Ville Syrjälä
Intel
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


[Freedreno] [PATCH v3 3/4] drm/msm: implement a2xx mmu

2018-12-03 Thread Jonathan Marek
A2XX has its own very simple MMU.

Added a msm_use_mmu() function because we can't rely on iommu_present to
decide to use MMU or not.

Signed-off-by: Jonathan Marek 
---
v3: rebased on msm-next-staging and moved is_a2xx initialization earlier

 drivers/gpu/drm/msm/Makefile   |   3 +-
 drivers/gpu/drm/msm/adreno/a2xx_gpu.c  |  50 -
 drivers/gpu/drm/msm/adreno/adreno_device.c |   3 +
 drivers/gpu/drm/msm/adreno/adreno_gpu.c|   3 +
 drivers/gpu/drm/msm/msm_drv.c  |  11 +-
 drivers/gpu/drm/msm/msm_drv.h  |   8 ++
 drivers/gpu/drm/msm/msm_gem.c  |   4 +-
 drivers/gpu/drm/msm/msm_gem_vma.c  |  23 
 drivers/gpu/drm/msm/msm_gpu.c  |  31 --
 drivers/gpu/drm/msm/msm_gpummu.c   | 122 +
 drivers/gpu/drm/msm/msm_mmu.h  |   3 +
 11 files changed, 241 insertions(+), 20 deletions(-)
 create mode 100644 drivers/gpu/drm/msm/msm_gpummu.c

diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
index 61e76f87a..1b26c4105 100644
--- a/drivers/gpu/drm/msm/Makefile
+++ b/drivers/gpu/drm/msm/Makefile
@@ -93,7 +93,8 @@ msm-y := \
msm_rd.o \
msm_ringbuffer.o \
msm_submitqueue.o \
-   msm_gpu_tracepoints.o
+   msm_gpu_tracepoints.o \
+   msm_gpummu.o
 
 msm-$(CONFIG_DEBUG_FS) += adreno/a5xx_debugfs.o \
  disp/dpu1/dpu_dbg.o
diff --git a/drivers/gpu/drm/msm/adreno/a2xx_gpu.c 
b/drivers/gpu/drm/msm/adreno/a2xx_gpu.c
index 5eddcf14e..1f83bc18d 100644
--- a/drivers/gpu/drm/msm/adreno/a2xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a2xx_gpu.c
@@ -2,6 +2,8 @@
 /* Copyright (c) 2018 The Linux Foundation. All rights reserved. */
 
 #include "a2xx_gpu.h"
+#include "msm_gem.h"
+#include "msm_mmu.h"
 
 extern bool hang_debug;
 
@@ -58,9 +60,12 @@ static bool a2xx_me_init(struct msm_gpu *gpu)
 static int a2xx_hw_init(struct msm_gpu *gpu)
 {
struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
+   dma_addr_t pt_base, tran_error;
uint32_t *ptr, len;
int i, ret;
 
+   msm_gpummu_params(gpu->aspace->mmu, &pt_base, &tran_error);
+
DBG("%s", gpu->name);
 
/* halt ME to avoid ucode upload issues on a20x */
@@ -80,9 +85,34 @@ static int a2xx_hw_init(struct msm_gpu *gpu)
/* note: kgsl uses 0x for a20x */
gpu_write(gpu, REG_A2XX_RBBM_CNTL, 0x4442);
 
-   gpu_write(gpu, REG_A2XX_MH_MMU_CONFIG, 0);
-   gpu_write(gpu, REG_A2XX_MH_MMU_MPU_BASE, 0);
+   /* MPU: physical range */
+   gpu_write(gpu, REG_A2XX_MH_MMU_MPU_BASE, 0x);
gpu_write(gpu, REG_A2XX_MH_MMU_MPU_END, 0xf000);
+
+   gpu_write(gpu, REG_A2XX_MH_MMU_CONFIG, A2XX_MH_MMU_CONFIG_MMU_ENABLE |
+   A2XX_MH_MMU_CONFIG_RB_W_CLNT_BEHAVIOR(BEH_TRAN_RNG) |
+   A2XX_MH_MMU_CONFIG_CP_W_CLNT_BEHAVIOR(BEH_TRAN_RNG) |
+   A2XX_MH_MMU_CONFIG_CP_R0_CLNT_BEHAVIOR(BEH_TRAN_RNG) |
+   A2XX_MH_MMU_CONFIG_CP_R1_CLNT_BEHAVIOR(BEH_TRAN_RNG) |
+   A2XX_MH_MMU_CONFIG_CP_R2_CLNT_BEHAVIOR(BEH_TRAN_RNG) |
+   A2XX_MH_MMU_CONFIG_CP_R3_CLNT_BEHAVIOR(BEH_TRAN_RNG) |
+   A2XX_MH_MMU_CONFIG_CP_R4_CLNT_BEHAVIOR(BEH_TRAN_RNG) |
+   A2XX_MH_MMU_CONFIG_VGT_R0_CLNT_BEHAVIOR(BEH_TRAN_RNG) |
+   A2XX_MH_MMU_CONFIG_VGT_R1_CLNT_BEHAVIOR(BEH_TRAN_RNG) |
+   A2XX_MH_MMU_CONFIG_TC_R_CLNT_BEHAVIOR(BEH_TRAN_RNG) |
+   A2XX_MH_MMU_CONFIG_PA_W_CLNT_BEHAVIOR(BEH_TRAN_RNG));
+
+   /* same as parameters in adreno_gpu */
+   gpu_write(gpu, REG_A2XX_MH_MMU_VA_RANGE, SZ_16M |
+   A2XX_MH_MMU_VA_RANGE_NUM_64KB_REGIONS(0xfff));
+
+   gpu_write(gpu, REG_A2XX_MH_MMU_PT_BASE, pt_base);
+   gpu_write(gpu, REG_A2XX_MH_MMU_TRAN_ERROR, tran_error);
+
+   gpu_write(gpu, REG_A2XX_MH_MMU_INVALIDATE,
+   A2XX_MH_MMU_INVALIDATE_INVALIDATE_ALL |
+   A2XX_MH_MMU_INVALIDATE_INVALIDATE_TC);
+
gpu_write(gpu, REG_A2XX_MH_ARBITER_CONFIG,
A2XX_MH_ARBITER_CONFIG_SAME_PAGE_LIMIT(16) |
A2XX_MH_ARBITER_CONFIG_L1_ARB_ENABLE |
@@ -109,9 +139,21 @@ static int a2xx_hw_init(struct msm_gpu *gpu)
/* note: gsl doesn't set this */
gpu_write(gpu, REG_A2XX_RBBM_DEBUG, 0x0008);
 
-   gpu_write(gpu, REG_A2XX_RBBM_INT_CNTL, 0);
-   gpu_write(gpu, REG_AXXX_CP_INT_CNTL, 0x8000); /* RB INT */
+   gpu_write(gpu, REG_A2XX_RBBM_INT_CNTL,
+   A2XX_RBBM_INT_CNTL_RDERR_INT_MASK);
+   gpu_write(gpu, REG_AXXX_CP_INT_CNTL,
+   AXXX_CP_INT_CNTL_T0_PACKET_IN_IB_MASK |
+   AXXX_CP_INT_CNTL_OPCODE_ERROR_MASK |
+   AXXX_CP_INT_CNTL_PROTECTED_MODE_ERROR_MASK |
+   AXXX_CP_INT_CNTL_RESERVED_BIT_ERROR_MASK |
+   AXXX_CP_INT_CNTL_IB_ERROR_MASK |
+   AXXX_CP_INT_CNTL_IB1_INT_MASK |
+   AXXX_CP_INT_CNTL_RB_INT_MASK);
gpu_write(gpu, REG

[Freedreno] [PATCH v3 1/4] drm/msm/mdp4: add lcdc-align-lsb flag to control lane alignment

2018-12-03 Thread Jonathan Marek
This allows controlling which of the 8 lanes are used for 6 bit color.

Signed-off-by: Jonathan Marek 
---
v3: removed empty line and added documentation

 .../devicetree/bindings/display/msm/mdp4.txt  |  2 ++
 .../gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c | 21 ---
 2 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/msm/mdp4.txt 
b/Documentation/devicetree/bindings/display/msm/mdp4.txt
index 3c341a15c..b07eeb38f 100644
--- a/Documentation/devicetree/bindings/display/msm/mdp4.txt
+++ b/Documentation/devicetree/bindings/display/msm/mdp4.txt
@@ -38,6 +38,8 @@ Required properties:
 Optional properties:
 - clock-names: the following clocks are optional:
   * "lut_clk"
+- qcom,lcdc-align-lsb: Boolean value indicating that LSB alignment should be
+  used for LCDC. This is only valid for 18bpp panels.
 
 Example:
 
diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c 
b/drivers/gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c
index 9e08c2efa..c9e34501a 100644
--- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c
+++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c
@@ -377,20 +377,25 @@ static void mdp4_lcdc_encoder_enable(struct drm_encoder 
*encoder)
unsigned long pc = mdp4_lcdc_encoder->pixclock;
struct mdp4_kms *mdp4_kms = get_kms(encoder);
struct drm_panel *panel;
+   uint32_t config;
int i, ret;
 
if (WARN_ON(mdp4_lcdc_encoder->enabled))
return;
 
/* TODO: hard-coded for 18bpp: */
-   mdp4_crtc_set_config(encoder->crtc,
-   MDP4_DMA_CONFIG_R_BPC(BPC6) |
-   MDP4_DMA_CONFIG_G_BPC(BPC6) |
-   MDP4_DMA_CONFIG_B_BPC(BPC6) |
-   MDP4_DMA_CONFIG_PACK_ALIGN_MSB |
-   MDP4_DMA_CONFIG_PACK(0x21) |
-   MDP4_DMA_CONFIG_DEFLKR_EN |
-   MDP4_DMA_CONFIG_DITHER_EN);
+   config =
+   MDP4_DMA_CONFIG_R_BPC(BPC6) |
+   MDP4_DMA_CONFIG_G_BPC(BPC6) |
+   MDP4_DMA_CONFIG_B_BPC(BPC6) |
+   MDP4_DMA_CONFIG_PACK(0x21) |
+   MDP4_DMA_CONFIG_DEFLKR_EN |
+   MDP4_DMA_CONFIG_DITHER_EN;
+
+   if (!of_property_read_bool(dev->dev->of_node, "qcom,lcdc-align-lsb"))
+   config |= MDP4_DMA_CONFIG_PACK_ALIGN_MSB;
+
+   mdp4_crtc_set_config(encoder->crtc, config);
mdp4_crtc_set_intf(encoder->crtc, INTF_LCDC_DTV, 0);
 
bs_set(mdp4_lcdc_encoder, 1);
-- 
2.17.1

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


[Freedreno] [PATCH v3 4/4] ARM: dts: imx5: add gpu nodes

2018-12-03 Thread Jonathan Marek
Signed-off-by: Jonathan Marek 
---
 arch/arm/boot/dts/imx51.dtsi | 17 +
 arch/arm/boot/dts/imx53.dtsi | 17 +
 2 files changed, 34 insertions(+)

diff --git a/arch/arm/boot/dts/imx51.dtsi b/arch/arm/boot/dts/imx51.dtsi
index 67d462715..e9a7bbce9 100644
--- a/arch/arm/boot/dts/imx51.dtsi
+++ b/arch/arm/boot/dts/imx51.dtsi
@@ -628,5 +628,22 @@
clock-names = "ipg", "ahb";
};
};
+
+   gpu: gpu@3000 {
+   compatible = "amd,imageon-200.1", "amd,imageon";
+   reg = <0x3000 0x2>;
+   reg-names = "kgsl_3d0_reg_memory";
+   interrupts = <12>;
+   interrupt-names = "kgsl_3d0_irq";
+   clocks = <&clks IMX5_CLK_GPU3D_GATE>, <&clks 
IMX5_CLK_GARB_GATE>;
+   clock-names = "core_clk", "mem_iface_clk";
+
+   qcom,gpu-pwrlevels {
+   compatible = "qcom,gpu-pwrlevels";
+   qcom,gpu-pwrlevel@0 {
+   qcom,gpu-freq = <16625>;
+   };
+   };
+   };
};
 };
diff --git a/arch/arm/boot/dts/imx53.dtsi b/arch/arm/boot/dts/imx53.dtsi
index 207eb557c..586d45586 100644
--- a/arch/arm/boot/dts/imx53.dtsi
+++ b/arch/arm/boot/dts/imx53.dtsi
@@ -838,5 +838,22 @@
reg = <0xf800 0x2>;
clocks = <&clks IMX5_CLK_OCRAM>;
};
+
+   gpu: gpu@3000 {
+   compatible = "amd,imageon-200.0", "amd,imageon";
+   reg = <0x3000 0x2>;
+   reg-names = "kgsl_3d0_reg_memory";
+   interrupts = <12>;
+   interrupt-names = "kgsl_3d0_irq";
+   clocks = <&clks IMX5_CLK_GPU3D_GATE>, <&clks 
IMX5_CLK_GARB_GATE>;
+   clock-names = "core_clk", "mem_iface_clk";
+
+   qcom,gpu-pwrlevels {
+   compatible = "qcom,gpu-pwrlevels";
+   qcom,gpu-pwrlevel@0 {
+   qcom,gpu-freq = <2>;
+   };
+   };
+   };
};
 };
-- 
2.17.1

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


[Freedreno] [PATCH v3 2/4] drm/msm: add headless gpu device for imx5

2018-12-03 Thread Jonathan Marek
This patch allows using drm/msm without qcom display hardware. It adds a
amd,imageon compatible, which is used instead of qcom,adreno, but does
not require a top level msm node.

Signed-off-by: Jonathan Marek 
---
v3: reworked to work with only a amd,imageon node

 drivers/gpu/drm/msm/Kconfig|  4 +--
 drivers/gpu/drm/msm/adreno/adreno_device.c | 35 --
 drivers/gpu/drm/msm/msm_debugfs.c  |  2 +-
 drivers/gpu/drm/msm/msm_drv.c  | 21 +++--
 4 files changed, 46 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
index 843a9d40c..cf549f1ed 100644
--- a/drivers/gpu/drm/msm/Kconfig
+++ b/drivers/gpu/drm/msm/Kconfig
@@ -2,7 +2,7 @@
 config DRM_MSM
tristate "MSM DRM"
depends on DRM
-   depends on ARCH_QCOM || (ARM && COMPILE_TEST)
+   depends on ARCH_QCOM || SOC_IMX5 || (ARM && COMPILE_TEST)
depends on OF && COMMON_CLK
depends on MMU
select QCOM_MDT_LOADER if ARCH_QCOM
@@ -11,7 +11,7 @@ config DRM_MSM
select DRM_PANEL
select SHMEM
select TMPFS
-   select QCOM_SCM
+   select QCOM_SCM if ARCH_QCOM
select WANT_DEV_COREDUMP
select SND_SOC_HDMI_CODEC if SND_SOC
select SYNC_FILE
diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c 
b/drivers/gpu/drm/msm/adreno/adreno_device.c
index adc442f73..99e363c3d 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_device.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
@@ -271,7 +271,8 @@ static int find_chipid(struct device *dev, struct 
adreno_rev *rev)
if (ret == 0) {
unsigned int r, patch;
 
-   if (sscanf(compat, "qcom,adreno-%u.%u", &r, &patch) == 2) {
+   if (sscanf(compat, "qcom,adreno-%u.%u", &r, &patch) == 2 ||
+   sscanf(compat, "amd,imageon-%u.%u", &r, &patch) == 2) {
rev->core = r / 100;
r %= 100;
rev->major = r / 10;
@@ -356,9 +357,37 @@ static const struct component_ops a3xx_ops = {
.unbind = adreno_unbind,
 };
 
+static void adreno_device_register_headless(void)
+{
+   /* on imx5, we don't have a top-level mdp/dpu node
+* this creates a dummy node for the driver for that case
+*/
+   struct platform_device_info dummy_info = {
+   .parent = NULL,
+   .name = "msm",
+   .id = -1,
+   .res = NULL,
+   .num_res = 0,
+   .data = NULL,
+   .size_data = 0,
+   .dma_mask = ~0,
+   };
+   platform_device_register_full(&dummy_info);
+}
+
 static int adreno_probe(struct platform_device *pdev)
 {
-   return component_add(&pdev->dev, &a3xx_ops);
+
+   int ret;
+
+   ret = component_add(&pdev->dev, &a3xx_ops);
+   if (ret)
+   return ret;
+
+   if (of_device_is_compatible(pdev->dev.of_node, "amd,imageon"))
+   adreno_device_register_headless();
+
+   return 0;
 }
 
 static int adreno_remove(struct platform_device *pdev)
@@ -370,6 +399,8 @@ static int adreno_remove(struct platform_device *pdev)
 static const struct of_device_id dt_match[] = {
{ .compatible = "qcom,adreno" },
{ .compatible = "qcom,adreno-3xx" },
+   /* for compatibility with imx5 gpu: */
+   { .compatible = "amd,imageon" },
/* for backwards compat w/ downstream kgsl DT files: */
{ .compatible = "qcom,kgsl-3d0" },
{}
diff --git a/drivers/gpu/drm/msm/msm_debugfs.c 
b/drivers/gpu/drm/msm/msm_debugfs.c
index 9a7cf9fe2..fb423d309 100644
--- a/drivers/gpu/drm/msm/msm_debugfs.c
+++ b/drivers/gpu/drm/msm/msm_debugfs.c
@@ -242,7 +242,7 @@ int msm_debugfs_init(struct drm_minor *minor)
debugfs_create_file("gpu", S_IRUSR, minor->debugfs_root,
dev, &msm_gpu_fops);
 
-   if (priv->kms->funcs->debugfs_init) {
+   if (priv->kms && priv->kms->funcs->debugfs_init) {
ret = priv->kms->funcs->debugfs_init(priv->kms, minor);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 81bfac744..7842518a9 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -510,17 +510,13 @@ static int msm_drm_init(struct device *dev, struct 
drm_driver *drv)
priv->kms = kms;
break;
default:
-   kms = ERR_PTR(-ENODEV);
+   /* valid only for the dummy headless case, where of_node=NULL */
+   WARN_ON(dev->of_node);
+   kms = NULL;
break;
}
 
if (IS_ERR(kms)) {
-   /*
-* NOTE: once we have GPU support, having no kms should not
-* be considered fatal.. ideally we would still support gpu
-* and (for example) use dmabuf/prime to share buffers 

Re: [Freedreno] [PATCH v2 5/5] drm/msm: subclass work object for vblank events

2018-12-03 Thread Jeykumar Sankaran

On 2018-12-03 06:21, Sean Paul wrote:

On Fri, Nov 30, 2018 at 04:21:15PM -0800, Jeykumar Sankaran wrote:

On 2018-11-30 12:07, Sean Paul wrote:
> On Fri, Nov 30, 2018 at 11:45:55AM -0800, Jeykumar Sankaran wrote:
> > On 2018-11-29 14:15, Sean Paul wrote:
> > > On Tue, Nov 20, 2018 at 02:04:14PM -0800, Jeykumar Sankaran wrote:
> > > > On 2018-11-07 07:55, Sean Paul wrote:
> > > > > On Tue, Nov 06, 2018 at 02:36:30PM -0800, Jeykumar Sankaran

wrote:

> > > > > > msm maintains a separate structure to define vblank
> > > > > > work definitions and a list to track events submitted
> > > > > > to the workqueue. We can avoid this redundant list
> > > > > > and its protection mechanism, if we subclass the
> > > > > > work object to encapsulate vblank event parameters.
> > > > > >
> > > > > > changes in v2:
> > > > > >   - subclass optimization on system wq (Sean Paul)
> > > > >
> > > > > I wouldn't do it like this, tbh. One problem is that you've

lost

> your
> > > > > flush() on
> > > > > unbind, so there's no way to know if you have workers in the

wild

> > > > > waiting
> > > > > to
> > > > > enable/disable vblank.
> > > > >
> > > > > Another issues is that AFAICT, we don't need a queue of
> > > > > enables/disables,
> > > > > but
> > > > > rather just the last requested state (ie: should we be on or

off).

> So
> > > > > things
> > > > > don't need to be this complicated (and we're possibly

thrashing

> vblank
> > > > > on/off
> > > > > for no reason).
> > > > >
> > > > > I'm still of the mind that you should just make this

synchronous

> and
> > > be
> > > > > done
> > > > > with the threads (especially since we're still
> uncovering/introducing
> > > > > races!).
> > > > >
> > > > While scoping out the effort to make vblank events synchronous,

I

> > > > found
> > > > that the spinlock locking order of vblank request sequence and
> vblank
> > > > callback
> > > > sequences are the opposite.
> > > >
> > > > In DPU, drm_vblank_enable acquires vblank_time_lock before
> registering
> > > > the crtc to encoder which happens after acquiring

encoder_spinlock.

> > > > But
> > > > the vblank_callback acquires encoder_spinlock before accessing

the

> > > > registered
> > > > crtc and calling into drm_vblank_handler which tries to acquire
> > > > vblank_time_lock.
> > > > Acquiring both vblank_time_lock and encoder_spinlock in the same
> > > > thread
> > > > is leading to deadlock.
> > >
> > > Hmm, I'm not sure I follow. Are you seeing issues where irq

overlaps

> > > with
> > > enable/disable? I hacked in sync vblank enable/disable quickly to

see

> if
> > > I
> > > could
> > > reproduce what you're seeing, but things seemed well behaved.
> > >
> >
> > The race is between drm_vblank_get/put and vblank_handler contexts.
> >
> > When made synchronous:
> >
> > while calling drm_vblank_get, the callstack looks like below:
> > drm_vblank_get -> drm_vblank_enable (acquires vblank_time_lock) ->
> > __enable_vblank -> dpu_crtc_vblank ->
> > dpu_encoder_toggle_vblank_for_crtc
> > (tries to acquire enc_spinlock)
> >
> > In vblank handler, the call stack will be:
> > dpu_encoder_phys_vid_vblank_irq -> dpu_encoder_vblank_callback
> > (acquires
> > enc_spinlock) -> dpu_crtc_vblank_callback -> drm_handle_vblank
> > (tries to
> > acquire vblank_time_lock)
>
> Hmm, I'm not sure how this can happen. We acquire and release the
> enc_spinlock
> before enabling the irq, yes we will hold on to the vbl_time_lock, but
> we
> shouldn't be trying to reacquire an encoder's spinlock after we've
> enabled
> it.
In the synchronous approach dpu_encoder_toggle_vblank_for_crtc(which
acquires the enc_spinlock) will be called while we
are holding the vbl_time_lock.

> I don't know how that can deadlock, since we should never be running
> enable and
> the handler concurrently.
>
I agree that vblank_irq handler should not be running before the 
enable

sequence. But
don't you expect the handler to be running while calling the

vblank_disable

sequence?


This is an entirely different problem though. It's also one that is 
easier

to
fix. I think we could probably grab the enc_spinlock in disable and 
clear

the
crtc pointer.

we do hold enc_spinlock in dpu_encoder_assign_crtc (drm/msm: dpu: Remove 
vblank_callback from encoder)

where we clear the crtc pointer.

What I'm getting at is that there's no fundamental reason why we need 
to

have
async vblank enable/disable.

Sean

There is really no *need* to have them async. But I believe the reason 
why they
are implemented this way is to avoid deadlock between the below two 
paths.


Restating the above findings:
vblank_handlers and vblank enable/disable can run concurrently. The 
first trying to acquire
vbl_time_lock holding enc_spinlock. Other trying to acquire enc_spinlock 
holding

vbl_time_lock.

Thanks,
Jeykumar S.


vbl disable will try to acquire the locks in the opposite order to 
that

of

irq_handler and the
same issue is bound to happen.

With your patch, you should be able to sim

[Freedreno] [PATCH] drm/msm: dpu: Allocate proper amount for dpu_crtc_state

2018-12-03 Thread Sean Paul
From: Sean Paul 

Since dpu_crtc subclasses crtc_state, we need a custom .reset hook in
order to allocate the right amount of memory to accommodate the
additional struct members in dpu_crtc_state. So bring it [partially]
back.

Relevant KASAN splat:
[   10.82] 
==
[   10.344288] BUG: KASAN: slab-out-of-bounds in kmemdup+0x50/0x80
[   10.350390] Read of size 736 at addr ffc0d9f06080 by task frecon/394

[   10.358861] CPU: 6 PID: 394 Comm: frecon Tainted: GW 4.19.4 
#121
[   10.366476] Hardware name: Google Cheza (rev2) (DT)
[   10.371514] Call trace:
[   10.374087]  dump_backtrace+0x0/0x194
[   10.377878]  show_stack+0x20/0x28
[   10.381330]  dump_stack+0xa0/0xc8
[   10.384783]  print_address_description+0x78/0x2e0
[   10.389639]  kasan_report+0x290/0x2d0
[   10.393428]  check_memory_region+0x20/0x14c
[   10.397740]  __asan_loadN+0x14/0x1c
[   10.401345]  kmemdup+0x50/0x80
[   10.404524]  dpu_crtc_duplicate_state+0x58/0xa0
[   10.409228]  drm_atomic_get_crtc_state+0xac/0x178
[   10.414095]  __drm_atomic_helper_set_config+0x54/0x4a4
[   10.419393]  drm_atomic_helper_set_config+0x60/0xb4
[   10.424435]  drm_mode_setcrtc+0x720/0x760
[   10.428570]  drm_ioctl_kernel+0xd8/0x13c
[   10.432617]  drm_ioctl+0x380/0x4f4
[   10.436150]  drm_compat_ioctl+0x54/0x13c
[   10.440219]  __arm64_compat_sys_ioctl+0x1d8/0xef4
[   10.445086]  el0_svc_common+0xd8/0x138
[   10.448961]  el0_svc_compat_handler+0x58/0x68
[   10.453463]  el0_svc_compat+0x8/0x18

[   10.458712] Allocated by task 56:
[   10.462148]  kasan_kmalloc.part.4+0x48/0xf4
[   10.466465]  kasan_kmalloc+0x8c/0xa0
[   10.470165]  kmem_cache_alloc_trace+0x25c/0x27c
[   10.474848]  drm_atomic_helper_crtc_reset+0x68/0x98
[   10.479877]  drm_mode_config_reset+0xc4/0x19c
[   10.484383]  msm_drm_bind+0x814/0x8dc
[   10.488169]  try_to_bring_up_master.part.7+0x48/0xac
[   10.493282]  component_master_add_with_match+0x158/0x198
[   10.498758]  msm_pdev_probe+0x328/0x348
[   10.502736]  platform_drv_probe+0x74/0xc8
[   10.506877]  really_probe+0x1ac/0x35c
[   10.510659]  driver_probe_device+0xd4/0x118
[   10.514975]  __device_attach_driver+0xc8/0xf4
[   10.519477]  bus_for_each_drv+0xb4/0xe4
[   10.523439]  __device_attach+0xd0/0x158
[   10.527394]  device_initial_probe+0x24/0x30
[   10.531715]  bus_probe_device+0x50/0xe4
[   10.535681]  deferred_probe_work_func+0xac/0xdc
[   10.540376]  process_one_work+0x3f0/0x6d4
[   10.544521]  worker_thread+0x3f4/0x520
[   10.548399]  kthread+0x1b4/0x1c8
[   10.551740]  ret_from_fork+0x10/0x18

[   10.556986] Freed by task 0:
[   10.559967] (stack is not available)

[   10.565216] The buggy address belongs to the object at ffc0d9f06080
which belongs to the cache kmalloc-1024 of size 1024
[   10.578268] The buggy address is located 0 bytes inside of
1024-byte region [ffc0d9f06080, ffc0d9f06480)
[   10.590248] The buggy address belongs to the page:
[   10.595195] page:ffbf0367c000 count:1 mapcount:0 
mapping:ffc0de40f680 index:0x0 compound_mapcount: 0
[   10.605321] flags: 0x40008100(slab|head)
[   10.610100] raw: 40008100 ffbf0369fa08 ffbf0367f008 
ffc0de40f680
[   10.618077] raw:  00150015 0001 

[   10.626049] page dumped because: kasan: bad access detected

[   10.633341] Memory state around the buggy address:
[   10.638282]  ffc0d9f06180: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
00
[   10.645710]  ffc0d9f06200: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
00
[   10.653139] >ffc0d9f06280: 00 00 00 00 00 00 00 fc fc fc fc fc fc fc fc 
fc
[   10.660571] ^
[   10.665774]  ffc0d9f06300: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc 
fc
[   10.673210]  ffc0d9f06380: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc 
fc
[   10.680639] 
==

Fixes: a6ba45afda41 ("drm/msm/dpu: Replace dpu_crtc_reset by atomic helper")
Cc: Sean Paul 
Cc: Bruce Wang 
Cc: Rob Clark 
Signed-off-by: Sean Paul 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 14 +-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 1e3e57817b72b..d27ea2a95f85b 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -819,6 +819,18 @@ static void _dpu_crtc_vblank_enable_no_lock(
}
 }
 
+static void dpu_crtc_reset(struct drm_crtc *crtc)
+{
+   struct dpu_crtc_state *cstate;
+
+   if (crtc->state)
+   dpu_crtc_destroy_state(crtc, crtc->state);
+
+   crtc->state = kzalloc(sizeof(*cstate), GFP_KERNEL);
+   if (crtc->state)
+   crtc->state->crtc = crtc;
+}
+
 /**
  * dpu_crtc_duplicate_state - state duplicate hook
  * @crtc: Pointer to drm crtc 

Re: [Freedreno] [PATCH v3 14/24] drm/msm: dpu: Grab the modeset locks in frame_event

2018-12-03 Thread Jeykumar Sankaran

On 2018-11-30 14:00, Sean Paul wrote:

From: Sean Paul 

This patch wraps dpu_core_perf_crtc_release_bw() with modeset locks
since it digs into the state objects.

Changes in v2:
- None
Changes in v3:
- Use those nifty new DRM_MODESET_LOCK_ALL_* helpers (Daniel)

Cc: Daniel Vetter 
Cc: Jeykumar Sankaran 
Signed-off-by: Sean Paul 
---

I see Daniel's comments are addressed. So ..
Reviewed-by: Jeykumar Sankaran 


 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 15 ++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 74ef384d9cd6a..03ddd281a354f 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -306,6 +306,19 @@ static void dpu_crtc_vblank_cb(void *data)
trace_dpu_crtc_vblank_cb(DRMID(crtc));
 }

+static void dpu_crtc_release_bw_unlocked(struct drm_crtc *crtc)
+{
+   int ret = 0;
+   struct drm_modeset_acquire_ctx ctx;
+
+   DRM_MODESET_LOCK_ALL_BEGIN(crtc->dev, ctx, 0, ret);
+   dpu_core_perf_crtc_release_bw(crtc);
+   DRM_MODESET_LOCK_ALL_END(ctx, ret);
+   if (ret)
+   DRM_ERROR("Failed to acquire modeset locks to release bw,
%d\n",
+ ret);
+}
+
 static void dpu_crtc_frame_event_work(struct kthread_work *work)
 {
struct dpu_crtc_frame_event *fevent = container_of(work,
@@ -335,7 +348,7 @@ static void dpu_crtc_frame_event_work(struct
kthread_work *work)
/* release bandwidth and other resources */
trace_dpu_crtc_frame_event_done(DRMID(crtc),
fevent->event);
-   dpu_core_perf_crtc_release_bw(crtc);
+   dpu_crtc_release_bw_unlocked(crtc);
} else {

trace_dpu_crtc_frame_event_more_pending(DRMID(crtc),

fevent->event);


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


Re: [Freedreno] [PATCH v2 12/24] drm/msm: dpu: Move crtc runtime resume to encoder

2018-12-03 Thread Jeykumar Sankaran

On 2018-11-16 10:42, Sean Paul wrote:

From: Sean Paul 

The crtc runtime resume doesn't actually operate on the crtc, but 
rather

its encoders. The problem with this is that we need to inspect the crtc
state to get the currently connected encoders. Since runtime resume
isn't guaranteed to be called while holding the modeset locks (although
it sometimes is), this presents a race condition.

Now that we have ->enabled on the virtual encoders, and a lock to
protect it, just call resume on each encoder and only restore the ones
that are enabled.

Changes in v2:
- None

Cc: Jeykumar Sankaran 
Signed-off-by: Sean Paul 
---


Reviewed-by: Jeykumar Sankaran 


 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c| 24 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h|  6 --
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 24 +
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h |  4 ++--
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c |  6 +++---
 5 files changed, 15 insertions(+), 49 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index d8f58caf2772..9be24907f8c1 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -841,30 +841,6 @@ static struct drm_crtc_state
*dpu_crtc_duplicate_state(struct drm_crtc *crtc)
return &cstate->base;
 }

-void dpu_crtc_runtime_resume(struct drm_crtc *crtc)
-{
-   struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
-   struct drm_encoder *encoder;
-
-   mutex_lock(&dpu_crtc->crtc_lock);
-
-   if (!dpu_crtc->enabled)
-   goto end;
-
-   trace_dpu_crtc_runtime_resume(DRMID(crtc));
-
-   /* restore encoder; crtc will be programmed during commit */
-   drm_for_each_encoder(encoder, crtc->dev) {
-   if (encoder->crtc != crtc)
-   continue;
-
-   dpu_encoder_virt_restore(encoder);
-   }
-
-end:
-   mutex_unlock(&dpu_crtc->crtc_lock);
-}
-
 static void dpu_crtc_disable(struct drm_crtc *crtc)
 {
struct dpu_crtc *dpu_crtc;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
index 1dca91d1210f..93d21a61a040 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
@@ -329,10 +329,4 @@ static inline bool dpu_crtc_is_enabled(struct
drm_crtc *crtc)
return crtc ? crtc->enabled : false;
 }

-/**
- * dpu_crtc_runtime_resume - called by the top-level on 
pm_runtime_resume

- * @crtc: CRTC to resume
- */
-void dpu_crtc_runtime_resume(struct drm_crtc *crtc);
-
 #endif /* _DPU_CRTC_H_ */
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 3daa86220d47..d89ac520f7e6 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -1089,28 +1089,24 @@ static void 
_dpu_encoder_virt_enable_helper(struct

drm_encoder *drm_enc)
_dpu_encoder_update_vsync_source(dpu_enc, &dpu_enc->disp_info);
 }

-void dpu_encoder_virt_restore(struct drm_encoder *drm_enc)
+void dpu_encoder_virt_runtime_resume(struct drm_encoder *drm_enc)
 {
-   struct dpu_encoder_virt *dpu_enc = NULL;
-   int i;
-
-   if (!drm_enc) {
-   DPU_ERROR("invalid encoder\n");
-   return;
-   }
-   dpu_enc = to_dpu_encoder_virt(drm_enc);
+   struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(drm_enc);

-   for (i = 0; i < dpu_enc->num_phys_encs; i++) {
-   struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i];
+   mutex_lock(&dpu_enc->enc_lock);

-   if (phys && (phys != dpu_enc->cur_master) &&
phys->ops.restore)
-   phys->ops.restore(phys);
-   }
+   if (!dpu_enc->enabled)
+   goto out;

+   if (dpu_enc->cur_slave && dpu_enc->cur_slave->ops.restore)
+   dpu_enc->cur_slave->ops.restore(dpu_enc->cur_slave);
if (dpu_enc->cur_master && dpu_enc->cur_master->ops.restore)
dpu_enc->cur_master->ops.restore(dpu_enc->cur_master);

_dpu_encoder_virt_enable_helper(drm_enc);
+
+out:
+   mutex_unlock(&dpu_enc->enc_lock);
 }

 static void dpu_encoder_virt_enable(struct drm_encoder *drm_enc)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
index 9dbf38f446d9..aa4f135218fa 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
@@ -126,10 +126,10 @@ int dpu_encoder_wait_for_event(struct drm_encoder
*drm_encoder,
 enum dpu_intf_mode dpu_encoder_get_intf_mode(struct drm_encoder
*encoder);

 /**
- * dpu_encoder_virt_restore - restore the encoder configs
+ * dpu_encoder_virt_runtime_resume - pm runtime resume the encoder
configs
  * @encoder:   encoder pointer
  */
-void dpu_encoder_virt_restore(struct drm_encoder *encoder);
+void dpu_encoder_virt_runtime_re

Re: [Freedreno] [PATCH v2 11/24] drm/msm: dpu: Add ->enabled to dpu_encoder_virt

2018-12-03 Thread Jeykumar Sankaran

On 2018-11-16 10:42, Sean Paul wrote:

From: Sean Paul 

Add a bool to dpu_encoder_virt to track whether the encoder is enabled
or not. Repurpose the enc_lock mutex to ensure that it is consistent
with the hw state.

Changes in v2:
- None

Cc: Jeykumar Sankaran 
Signed-off-by: Sean Paul 
---

Reviewed-by: Jeykumar Sankaran 


 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 27 +
 1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 10a0676d1dcf..3daa86220d47 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -132,6 +132,7 @@ enum dpu_enc_rc_states {
  * @base:  drm_encoder base class for registration with DRM
  * @enc_spinlock:  Virtual-Encoder-Wide Spin Lock for IRQ purposes
  * @bus_scaling_client:Client handle to the bus scaling interface
+ * @enabled:   True if the encoder is active, protected by
enc_lock
  * @num_phys_encs: Actual number of physical encoders contained.
  * @phys_encs: Container of physical encoders managed.
  * @cur_master:Pointer to the current master in this
mode. Optimization
@@ -148,8 +149,8 @@ enum dpu_enc_rc_states {
  * all CTL paths
  * @crtc_kickoff_cb_data:  Opaque user data given to crtc_kickoff_cb
  * @debugfs_root:  Debug file system root file node
- * @enc_lock:  Lock around physical encoder
create/destroy and
-   access.
+ * @enc_lock:  Lock around physical encoder
+ * create/destroy/enable/disable
  * @frame_busy_mask:   Bitmask tracking which phys_enc we are
still
  * busy processing current command.
  * Bit0 = phys_encs[0] etc.
@@ -175,6 +176,8 @@ struct dpu_encoder_virt {
spinlock_t enc_spinlock;
uint32_t bus_scaling_client;

+   bool enabled;
+
unsigned int num_phys_encs;
struct dpu_encoder_phys *phys_encs[MAX_PHYS_ENCODERS_PER_VIRTUAL];
struct dpu_encoder_phys *cur_master;
@@ -1121,6 +1124,8 @@ static void dpu_encoder_virt_enable(struct
drm_encoder *drm_enc)
return;
}
dpu_enc = to_dpu_encoder_virt(drm_enc);
+
+   mutex_lock(&dpu_enc->enc_lock);
cur_mode = &dpu_enc->base.crtc->state->adjusted_mode;

trace_dpu_enc_enable(DRMID(drm_enc), cur_mode->hdisplay,
@@ -1137,10 +1142,15 @@ static void dpu_encoder_virt_enable(struct
drm_encoder *drm_enc)
if (ret) {
DPU_ERROR_ENC(dpu_enc, "dpu resource control failed:
%d\n",
ret);
-   return;
+   goto out;
}

_dpu_encoder_virt_enable_helper(drm_enc);
+
+   dpu_enc->enabled = true;
+
+out:
+   mutex_unlock(&dpu_enc->enc_lock);
 }

 static void dpu_encoder_virt_disable(struct drm_encoder *drm_enc)
@@ -1162,11 +1172,14 @@ static void dpu_encoder_virt_disable(struct
drm_encoder *drm_enc)
return;
}

-   mode = &drm_enc->crtc->state->adjusted_mode;
-
dpu_enc = to_dpu_encoder_virt(drm_enc);
DPU_DEBUG_ENC(dpu_enc, "\n");

+   mutex_lock(&dpu_enc->enc_lock);
+   dpu_enc->enabled = false;
+
+   mode = &drm_enc->crtc->state->adjusted_mode;
+
priv = drm_enc->dev->dev_private;
dpu_kms = to_dpu_kms(priv->kms);

@@ -1200,6 +1213,8 @@ static void dpu_encoder_virt_disable(struct
drm_encoder *drm_enc)
DPU_DEBUG_ENC(dpu_enc, "encoder disabled\n");

dpu_rm_release(&dpu_kms->rm, drm_enc);
+
+   mutex_unlock(&dpu_enc->enc_lock);
 }

 static enum dpu_intf dpu_encoder_get_intf(struct dpu_mdss_cfg 
*catalog,

@@ -2233,6 +2248,8 @@ struct drm_encoder *dpu_encoder_init(struct
drm_device *dev,

drm_encoder_helper_add(&dpu_enc->base, &dpu_encoder_helper_funcs);

+   dpu_enc->enabled = false;
+
return &dpu_enc->base;
 }


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


Re: [Freedreno] [PATCH v2 04/24] drm/msm: dpu: Don't use power_event for vbif_init_memtypes

2018-12-03 Thread Jeykumar Sankaran

On 2018-11-16 10:42, Sean Paul wrote:

From: Sean Paul 

power_events are only used for pm_runtime, and that's all handled in
dpu_kms. So just call vbif_init_memtypes at the correct times.

Changes in v2:
- Removed obsolete comment (Jeykumar)

Cc: Jeykumar Sankaran 
Signed-off-by: Sean Paul 
---


Reviewed-by: Jeykumar Sankaran 


 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 24 +++-
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h |  1 -
 2 files changed, 3 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 23094d108e81..ab8574ab8327 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -651,10 +651,6 @@ static void _dpu_kms_hw_destroy(struct dpu_kms
*dpu_kms)
dpu_hw_intr_destroy(dpu_kms->hw_intr);
dpu_kms->hw_intr = NULL;

-   if (dpu_kms->power_event)
-   dpu_power_handle_unregister_event(
-   &dpu_kms->phandle, dpu_kms->power_event);
-
/* safe to call these more than once during shutdown */
_dpu_debugfs_destroy(dpu_kms);
_dpu_kms_mmu_destroy(dpu_kms);
@@ -832,16 +828,6 @@ u64 dpu_kms_get_clk_rate(struct dpu_kms *dpu_kms,
char *clock_name)
return clk_get_rate(clk->clk);
 }

-static void dpu_kms_handle_power_event(u32 event_type, void *usr)
-{
-   struct dpu_kms *dpu_kms = usr;
-
-   if (!dpu_kms)
-   return;
-
-   dpu_vbif_init_memtypes(dpu_kms);
-}
-
 static int dpu_kms_hw_init(struct msm_kms *kms)
 {
struct dpu_kms *dpu_kms;
@@ -1012,13 +998,7 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
 */
dev->mode_config.allow_fb_modifiers = true;

-   /*
-* Handle (re)initializations during power enable
-*/
-   dpu_kms_handle_power_event(DPU_POWER_EVENT_ENABLE, dpu_kms);
-   dpu_kms->power_event = dpu_power_handle_register_event(
-   &dpu_kms->phandle, DPU_POWER_EVENT_ENABLE,
-   dpu_kms_handle_power_event, dpu_kms, "kms");
+   dpu_vbif_init_memtypes(dpu_kms);

pm_runtime_put_sync(&dpu_kms->pdev->dev);

@@ -1172,6 +1152,8 @@ static int __maybe_unused 
dpu_runtime_resume(struct

device *dev)
return rc;
}

+   dpu_vbif_init_memtypes(dpu_kms);
+
rc = dpu_power_resource_enable(&dpu_kms->phandle, true);
if (rc)
DPU_ERROR("resource enable failed: %d\n", rc);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
index f2c78deb0854..5f08be187c86 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
@@ -114,7 +114,6 @@ struct dpu_kms {
struct dpu_mdss_cfg *catalog;

struct dpu_power_handle phandle;
-   struct dpu_power_event *power_event;

/* directory entry for debugfs */
struct dentry *debugfs_root;


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


Re: [Freedreno] [PATCH v2 15/24] drm/msm: dpu: Stop using encoder->crtc pointer

2018-12-03 Thread Jeykumar Sankaran

On 2018-11-26 13:53, Sean Paul wrote:

On Mon, Nov 19, 2018 at 12:03:53PM -0800, Jeykumar Sankaran wrote:

On 2018-11-16 13:14, Sean Paul wrote:
> On Fri, Nov 16, 2018 at 12:05:09PM -0800, Jeykumar Sankaran wrote:
> > On 2018-11-16 10:42, Sean Paul wrote:
> > > From: Sean Paul 
> > >
> > > It's for legacy drivers, for atomic drivers

crtc->state->encoder_mask

> > > should be used to map encoder to crtc.
> > >
> > > Changes in v2:
> > > - None
> > >
> > > Signed-off-by: Sean Paul 
> > > ---
> > >  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 46
> 
> > >  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c  | 19 +++---
> > >  2 files changed, 29 insertions(+), 36 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > > b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > > index 156f4c77ca44..a008a87a8113 100644
> > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > > @@ -284,9 +284,9 @@ enum dpu_intf_mode

dpu_crtc_get_intf_mode(struct

> > > drm_crtc *crtc)
> > >  return INTF_MODE_NONE;
> > >  }
> > >
> > > -drm_for_each_encoder(encoder, crtc->dev)
> > > -if (encoder->crtc == crtc)
> > > -return dpu_encoder_get_intf_mode(encoder);
> > > +/* TODO: Returns the first INTF_MODE, could there be

multiple

> > > values? */
> > > +drm_for_each_encoder_mask(encoder, crtc->dev,
> > > crtc->state->encoder_mask)
> > > +return dpu_encoder_get_intf_mode(encoder);
> > >
> > >  return INTF_MODE_NONE;
> > >  }
> > > @@ -551,13 +551,9 @@ static void dpu_crtc_atomic_begin(struct

drm_crtc

> > > *crtc,
> > >  spin_unlock_irqrestore(&dev->event_lock, flags);
> > >  }
> > >
> > > -list_for_each_entry(encoder,

&dev->mode_config.encoder_list, head)

> > > {
> > > -if (encoder->crtc != crtc)
> > > -continue;
> > > -
> > > -/* encoder will trigger pending mask now */
> > > +/* encoder will trigger pending mask now */
> > > +drm_for_each_encoder_mask(encoder, crtc->dev,
> > > crtc->state->encoder_mask)
> > >  dpu_encoder_trigger_kickoff_pending(encoder);
> > > -}
> > >
> > >  /*
> > >   * If no mixers have been allocated in

dpu_crtc_atomic_check(),

> > > @@ -704,7 +700,6 @@ static int

_dpu_crtc_wait_for_frame_done(struct

> > > drm_crtc *crtc)
> > >  void dpu_crtc_commit_kickoff(struct drm_crtc *crtc)
> > >  {
> > >  struct drm_encoder *encoder;
> > > -struct drm_device *dev = crtc->dev;
> > >  struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
> > >  struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc);
> > >  struct dpu_crtc_state *cstate =

to_dpu_crtc_state(crtc->state);

> > > @@ -720,16 +715,13 @@ void dpu_crtc_commit_kickoff(struct drm_crtc
> > > *crtc)
> > >
> > >  DPU_ATRACE_BEGIN("crtc_commit");
> > >
> > > -list_for_each_entry(encoder,

&dev->mode_config.encoder_list, head)

> > > {
> > > +/*
> > > + * Encoder will flush/start now, unless it has a tx

pending. If

> > > so, it
> > > + * may delay and flush at an irq event (e.g. ppdone)
> > > + */
> > > +drm_for_each_encoder_mask(encoder, crtc->dev,
> > > +  crtc->state->encoder_mask) {
> > >  struct dpu_encoder_kickoff_params params = { 0 };
> > > -
> > > -if (encoder->crtc != crtc)
> > > -continue;
> > > -
> > > -/*
> > > - * Encoder will flush/start now, unless it has a

tx

> > > pending.
> > > - * If so, it may delay and flush at an irq event

(e.g.

> > > ppdone)
> > > - */
> > >  dpu_encoder_prepare_for_kickoff(encoder, ¶ms);
> > >  }
> > >
> > > @@ -754,12 +746,8 @@ void dpu_crtc_commit_kickoff(struct drm_crtc
> *crtc)
> > >
> > >  dpu_vbif_clear_errors(dpu_kms);
> > >
> > > -list_for_each_entry(encoder,

&dev->mode_config.encoder_list, head)

> > > {
> > > -if (encoder->crtc != crtc)
> > > -continue;
> > > -
> > > +drm_for_each_encoder_mask(encoder, crtc->dev,
> > > crtc->state->encoder_mask)
> > >  dpu_encoder_kickoff(encoder);
> > > -}
> > We wont be holding the modeset locks here (and in crtc_atomic_begin)
> > in
> the
> > display thread. Is
> > it safe to iterate over encoder_mask?
>
> Hmm, I'm not sure I follow. AFAICT, there are 2 callsites for
> dpu_crtc_commit_kickoff():
>
> 1- dpu_kms_encoder_enable() which is called via the
> encoder->funcs->enable
> hook
> 2- dpu_kms_commit() which is called in the
> mode_config->funcs->atomic_commit_tail

atomic_commit_tail will be called from the WQ. Do we hold
the modeset locks in the WQ? I believe userspace thread will dr

[Freedreno] [PATCH] drm/msm/a6xx: Remove unwanted regulator code

2018-12-03 Thread Jordan Crouse
The GMU code currently has some misguided code to try to work around
a hardware quirk that requires the power domains on the GPU be
collapsed in a certain order. Future changes will do this the
right way so get rid of the unused and unwanted regulator
code.

Signed-off-by: Jordan Crouse 
---
 drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 4 
 drivers/gpu/drm/msm/adreno/a6xx_gmu.h | 2 --
 2 files changed, 6 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c 
b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
index c58e953fefa3..4a989d4e65f9 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
@@ -671,9 +671,6 @@ int a6xx_gmu_reset(struct a6xx_gpu *a6xx_gpu)
gmu_poll_timeout(gmu, REG_A6XX_RSCC_TCS3_DRV0_STATUS, val,
(val & 1), 100, 1000);
 
-   /* Force off the GX GSDC */
-   regulator_force_disable(gmu->gx);
-
/* Disable the resources */
clk_bulk_disable_unprepare(gmu->nr_clocks, gmu->clocks);
pm_runtime_put_sync(gmu->dev);
@@ -1214,7 +1211,6 @@ int a6xx_gmu_probe(struct a6xx_gpu *a6xx_gpu, struct 
device_node *node)
gmu->idle_level = GMU_IDLE_STATE_ACTIVE;
 
pm_runtime_enable(gmu->dev);
-   gmu->gx = devm_regulator_get(gmu->dev, "vdd");
 
/* Get the list of clocks */
ret = a6xx_gmu_clocks_probe(gmu);
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h 
b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
index c721d9165d8e..8081083cd062 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
@@ -52,8 +52,6 @@ struct a6xx_gmu {
int hfi_irq;
int gmu_irq;
 
-   struct regulator *gx;
-
struct iommu_domain *domain;
u64 uncached_iova_base;
 
-- 
2.18.0

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


[Freedreno] [PATCH] drm/msm/a6xx: Add a name for the crashdumper buffer

2018-12-03 Thread Jordan Crouse
Add a buffer object name for the a6xx crashdumper so it can be
seen with the changes introduced by 7799a98edd
("drm/msm: Add a name field for gem objects").

Signed-off-by: Jordan Crouse 
---
 drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c 
b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c
index 716595b664dd..e686331fa089 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c
@@ -116,7 +116,10 @@ static int a6xx_crashdumper_init(struct msm_gpu *gpu,
SZ_1M, MSM_BO_UNCACHED, gpu->aspace,
&dumper->bo, &dumper->iova);
 
-   return IS_ERR(dumper->ptr) ? PTR_ERR(dumper->ptr) : 0;
+   if (!IS_ERR(dumper->ptr))
+   msm_gem_object_set_name(dumper->bo, "crashdump");
+
+   return PTR_ERR_OR_ZERO(dumper->ptr);
 }
 
 static int a6xx_crashdumper_run(struct msm_gpu *gpu,
-- 
2.18.0

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


[Freedreno] [PATCH] drm/msm/a6xx: Use new kernel API free function for gpu state

2018-12-03 Thread Jordan Crouse
dadb36b7ec42 ("drm/msm: Add a common function to free kernel buffer objects")
missed freeing the crashdumper state for a6xx.

Signed-off-by: Jordan Crouse 
---
 drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c | 11 +--
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c 
b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c
index df6308e7ea67..716595b664dd 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c
@@ -149,15 +149,6 @@ static int a6xx_crashdumper_run(struct msm_gpu *gpu,
return ret;
 }
 
-static void a6xx_crashdumper_free(struct msm_gpu *gpu,
-   struct a6xx_crashdumper *dumper)
-{
-   msm_gem_unpin_iova(dumper->bo, gpu->aspace);
-   msm_gem_put_vaddr(dumper->bo);
-
-   drm_gem_object_unreference(dumper->bo);
-}
-
 /* read a value from the GX debug bus */
 static int debugbus_read(struct msm_gpu *gpu, u32 block, u32 offset,
u32 *data)
@@ -900,7 +891,7 @@ struct msm_gpu_state *a6xx_gpu_state_get(struct msm_gpu 
*gpu)
a6xx_get_clusters(gpu, a6xx_state, &dumper);
a6xx_get_dbgahb_clusters(gpu, a6xx_state, &dumper);
 
-   a6xx_crashdumper_free(gpu, &dumper);
+   msm_gem_kernel_put(dumper.bo, gpu->aspace, true);
}
 
a6xx_get_debugbus(gpu, a6xx_state);
-- 
2.18.0

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


Re: [Freedreno] [PATCH v2] drm/msm/dpu: add display port support in DPU

2018-12-03 Thread Sean Paul
On Tue, Nov 27, 2018 at 02:28:30PM -0800, Jeykumar Sankaran wrote:
> Add display port support in DPU by creating hooks
> for DP encoder enumeration and encoder mode
> initialization.
> 
> This change is based on the SDM845 Display port
> driver changes[1].
> 
> changes in v2:
>   - rebase on [2] (Sean Paul)
>   - remove unwanted error checks and
> switch cases (Jordan Crouse)
> 
> [1] https://lwn.net/Articles/768265/
> [2] https://lkml.org/lkml/2018/11/17/87
> 
> Signed-off-by: Jeykumar Sankaran 
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c |  8 ++---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 47 
> +
>  2 files changed, 45 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c 
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index d3f4501..1f6b4b1 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -2015,7 +2015,7 @@ static int dpu_encoder_setup_display(struct 
> dpu_encoder_virt *dpu_enc,
>  {
>   int ret = 0;
>   int i = 0;
> - enum dpu_intf_type intf_type;
> + enum dpu_intf_type intf_type = INTF_NONE;

dpu_intf_type seems unnecessary, you could just use the DRM_MODE_ENCODER_* value
directly?

>   struct dpu_enc_phys_init_params phys_params;
>  
>   if (!dpu_enc || !dpu_kms) {
> @@ -2038,9 +2038,9 @@ static int dpu_encoder_setup_display(struct 
> dpu_encoder_virt *dpu_enc,
>   case DRM_MODE_ENCODER_DSI:
>   intf_type = INTF_DSI;
>   break;
> - default:
> - DPU_ERROR_ENC(dpu_enc, "unsupported display interface type\n");
> - return -EINVAL;
> + case DRM_MODE_ENCODER_TMDS:
> + intf_type = INTF_DP;
> + break;
>   }
>  
>   WARN_ON(disp_info->num_of_h_tiles < 1);
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c 
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index 985c855..7d931ae 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -473,6 +473,32 @@ static void _dpu_kms_initialize_dsi(struct drm_device 
> *dev,
>   }
>  }
>  
> +static void _dpu_kms_initialize_displayport(struct drm_device *dev,
> + struct msm_drm_private *priv,
> + struct dpu_kms *dpu_kms)
> +{
> + struct drm_encoder *encoder = NULL;
> + int rc;
> +
> + if (!priv->dp)
> + return;
> +
> + encoder = dpu_encoder_init(dev, DRM_MODE_ENCODER_TMDS);
> + if (IS_ERR(encoder)) {
> + DPU_ERROR("encoder init failed for dsi display\n");
> + return;
> + }
> +
> + rc = msm_dp_modeset_init(priv->dp, dev, encoder);
> + if (rc) {
> + DPU_ERROR("modeset_init failed for DP, rc = %d\n", rc);
> + drm_encoder_cleanup(encoder);
> + return;
> + }
> +
> + priv->encoders[priv->num_encoders++] = encoder;

No need to keep track of drm resources at the driver level, the core will do
this for you. So can you please add a patch preceding this one to remove the
priv->encoders/crtc/planes/connectors arrays?

> +}
> +
>  /**
>   * _dpu_kms_setup_displays - create encoders, bridges and connectors
>   *   for underlying displays
> @@ -487,6 +513,8 @@ static void _dpu_kms_setup_displays(struct drm_device 
> *dev,

Why are these functions voids? Seems like there are plenty of places for them to
fail :)

Let's add a patch to the beginning of this series to properly handle failures in
setup_displays and initialize_dsi

>  {
>   _dpu_kms_initialize_dsi(dev, priv, dpu_kms);
>  
> + _dpu_kms_initialize_displayport(dev, priv, dpu_kms);
> +
>   /**
>* Extend this function to initialize other
>* types of displays
> @@ -723,13 +751,20 @@ static void _dpu_kms_set_encoder_mode(struct msm_kms 
> *kms,
>   info.capabilities = cmd_mode ? MSM_DISPLAY_CAP_CMD_MODE :
>   MSM_DISPLAY_CAP_VID_MODE;
>  
> - /* TODO: No support for DSI swap */
> - for (i = 0; i < ARRAY_SIZE(priv->dsi); i++) {
> - if (priv->dsi[i]) {
> - info.h_tile_instance[info.num_of_h_tiles] = i;
> - info.num_of_h_tiles++;
> + switch (info.intf_type) {
> + case DRM_MODE_ENCODER_DSI:
> + /* TODO: No support for DSI swap */
> + for (i = 0; i < ARRAY_SIZE(priv->dsi); i++) {
> + if (priv->dsi[i]) {
> + info.h_tile_instance[info.num_of_h_tiles] = i;
> + info.num_of_h_tiles++;
> + }
>   }
> - }
> + break;
> + case DRM_MODE_ENCODER_TMDS:
> + info.num_of_h_tiles = 1;
> + break;
> + };
>  
>   rc = dpu_encoder_setup(encoder->dev, encoder, &info);
>   if (rc)
> -- 
> The Qualcomm Innovatio

Re: [Freedreno] [PATCH] of/device: add blacklist for iommu dma_ops

2018-12-03 Thread Rob Clark
On Mon, Dec 3, 2018 at 7:45 AM Robin Murphy  wrote:
>
> Hi Rob,
>
> On 01/12/2018 16:53, Rob Clark wrote:
> > This solves a problem we see with drm/msm, caused by getting
> > iommu_dma_ops while we attach our own domain and manage it directly at
> > the iommu API level:
> >
> >[0038] user address but active_mm is swapper
> >Internal error: Oops: 9605 [#1] PREEMPT SMP
> >Modules linked in:
> >CPU: 7 PID: 70 Comm: kworker/7:1 Tainted: GW 4.19.3 #90
> >Hardware name: xxx (DT)
> >Workqueue: events deferred_probe_work_func
> >pstate: 80c9 (Nzcv daif +PAN +UAO)
> >pc : iommu_dma_map_sg+0x7c/0x2c8
> >lr : iommu_dma_map_sg+0x40/0x2c8
> >sp : ff80095eb4f0
> >x29: ff80095eb4f0 x28: 
> >x27: ffc0f9431578 x26: 
> >x25:  x24: 0003
> >x23: 0001 x22: ffc0fa9ac010
> >x21:  x20: ffc0fab40980
> >x19: ffc0fab40980 x18: 0003
> >x17: 01c4 x16: 0007
> >x15: 000e x14: 
> >x13:  x12: 0028
> >x11: 0101010101010101 x10: 7f7f7f7f7f7f7f7f
> >x9 :  x8 : ffc0fab409a0
> >x7 :  x6 : 0002
> >x5 : 0001 x4 : 
> >x3 : 0001 x2 : 0002
> >x1 : ffc0f9431578 x0 : 
> >Process kworker/7:1 (pid: 70, stack limit = 0x17d08ffb)
> >Call trace:
> > iommu_dma_map_sg+0x7c/0x2c8
> > __iommu_map_sg_attrs+0x70/0x84
> > get_pages+0x170/0x1e8
> > msm_gem_get_iova+0x8c/0x128
> > _msm_gem_kernel_new+0x6c/0xc8
> > msm_gem_kernel_new+0x4c/0x58
> > dsi_tx_buf_alloc_6g+0x4c/0x8c
> > msm_dsi_host_modeset_init+0xc8/0x108
> > msm_dsi_modeset_init+0x54/0x18c
> > _dpu_kms_drm_obj_init+0x430/0x474
> > dpu_kms_hw_init+0x5f8/0x6b4
> > msm_drm_bind+0x360/0x6c8
> > try_to_bring_up_master.part.7+0x28/0x70
> > component_master_add_with_match+0xe8/0x124
> > msm_pdev_probe+0x294/0x2b4
> > platform_drv_probe+0x58/0xa4
> > really_probe+0x150/0x294
> > driver_probe_device+0xac/0xe8
> > __device_attach_driver+0xa4/0xb4
> > bus_for_each_drv+0x98/0xc8
> > __device_attach+0xac/0x12c
> > device_initial_probe+0x24/0x30
> > bus_probe_device+0x38/0x98
> > deferred_probe_work_func+0x78/0xa4
> > process_one_work+0x24c/0x3dc
> > worker_thread+0x280/0x360
> > kthread+0x134/0x13c
> > ret_from_fork+0x10/0x18
> >Code: d284 91000725 6b17039f 5400048a (f9401f40)
> >---[ end trace f22dda57f3648e2c ]---
> >Kernel panic - not syncing: Fatal exception
> >SMP: stopping secondary CPUs
> >Kernel Offset: disabled
> >CPU features: 0x0,22802a18
> >Memory Limit: none
> >
> > The problem is that when drm/msm does it's own iommu_attach_device(),
> > now the domain returned by iommu_get_domain_for_dev() is drm/msm's
> > domain, and it doesn't have domain->iova_cookie.
>
> Does this crash still happen with 4.20-rc? Because as of 6af588fed391 it
> really shouldn't.
>
> > We kind of avoided this problem prior to sdm845/dpu because the iommu
> > was attached to the mdp node in dt, which is a child of the toplevel
> > mdss node (which corresponds to the dev passed in dma_map_sg()).  But
> > with sdm845, now the iommu is attached at the mdss level so we hit the
> > iommu_dma_ops in dma_map_sg().
> >
> > But auto allocating/attaching a domain before the driver is probed was
> > already a blocking problem for enabling per-context pagetables for the
> > GPU.  This problem is also now solved with this patch.
>
> s/solved/worked around/
>
> If you want a guarantee of actually getting a specific hardware context
> allocated for a given domain, there needs to be code in the IOMMU driver
> to understand and honour that. Implicitly depending on whatever happens
> to fall out of current driver behaviour on current systems is not a real
> solution.
>
> > Fixes: 97890ba9289c dma-mapping: detect and configure IOMMU in 
> > of_dma_configure
>
> That's rather misleading, since the crash described above depends on at
> least two other major changes which came long after that commit.
>
> It's not that I don't understand exactly what you want here - just that
> this commit message isn't a coherent justification for that ;)
>
> > Tested-by: Douglas Anderson 
> > Signed-off-by: Rob Clark 
> > ---
> > This is an alternative/replacement for [1].  What it lacks in elegance
> > it makes up for in practicality ;-)
> >
> > [1] https://patchwork.freedesktop.org/patch/264930/
> >
> >   drivers/of/device.c | 22 ++
> >   1 file changed, 22 insertions(+)
> >
> > diff --git a/drivers/of/device.c b/drivers/of/device.c
> > index 5957cd4fa262..15ffee00fb22 100644
> > --- a/drivers/of/device.c
> > +++ b/drivers/of/device.c
> > 

Re: [Freedreno] [PATCH v2 5/5] drm/msm: subclass work object for vblank events

2018-12-03 Thread Sean Paul
On Fri, Nov 30, 2018 at 04:21:15PM -0800, Jeykumar Sankaran wrote:
> On 2018-11-30 12:07, Sean Paul wrote:
> > On Fri, Nov 30, 2018 at 11:45:55AM -0800, Jeykumar Sankaran wrote:
> > > On 2018-11-29 14:15, Sean Paul wrote:
> > > > On Tue, Nov 20, 2018 at 02:04:14PM -0800, Jeykumar Sankaran wrote:
> > > > > On 2018-11-07 07:55, Sean Paul wrote:
> > > > > > On Tue, Nov 06, 2018 at 02:36:30PM -0800, Jeykumar Sankaran wrote:
> > > > > > > msm maintains a separate structure to define vblank
> > > > > > > work definitions and a list to track events submitted
> > > > > > > to the workqueue. We can avoid this redundant list
> > > > > > > and its protection mechanism, if we subclass the
> > > > > > > work object to encapsulate vblank event parameters.
> > > > > > >
> > > > > > > changes in v2:
> > > > > > >   - subclass optimization on system wq (Sean Paul)
> > > > > >
> > > > > > I wouldn't do it like this, tbh. One problem is that you've lost
> > your
> > > > > > flush() on
> > > > > > unbind, so there's no way to know if you have workers in the wild
> > > > > > waiting
> > > > > > to
> > > > > > enable/disable vblank.
> > > > > >
> > > > > > Another issues is that AFAICT, we don't need a queue of
> > > > > > enables/disables,
> > > > > > but
> > > > > > rather just the last requested state (ie: should we be on or off).
> > So
> > > > > > things
> > > > > > don't need to be this complicated (and we're possibly thrashing
> > vblank
> > > > > > on/off
> > > > > > for no reason).
> > > > > >
> > > > > > I'm still of the mind that you should just make this synchronous
> > and
> > > > be
> > > > > > done
> > > > > > with the threads (especially since we're still
> > uncovering/introducing
> > > > > > races!).
> > > > > >
> > > > > While scoping out the effort to make vblank events synchronous, I
> > > > > found
> > > > > that the spinlock locking order of vblank request sequence and
> > vblank
> > > > > callback
> > > > > sequences are the opposite.
> > > > >
> > > > > In DPU, drm_vblank_enable acquires vblank_time_lock before
> > registering
> > > > > the crtc to encoder which happens after acquiring encoder_spinlock.
> > > > > But
> > > > > the vblank_callback acquires encoder_spinlock before accessing the
> > > > > registered
> > > > > crtc and calling into drm_vblank_handler which tries to acquire
> > > > > vblank_time_lock.
> > > > > Acquiring both vblank_time_lock and encoder_spinlock in the same
> > > > > thread
> > > > > is leading to deadlock.
> > > >
> > > > Hmm, I'm not sure I follow. Are you seeing issues where irq overlaps
> > > > with
> > > > enable/disable? I hacked in sync vblank enable/disable quickly to see
> > if
> > > > I
> > > > could
> > > > reproduce what you're seeing, but things seemed well behaved.
> > > >
> > > 
> > > The race is between drm_vblank_get/put and vblank_handler contexts.
> > > 
> > > When made synchronous:
> > > 
> > > while calling drm_vblank_get, the callstack looks like below:
> > > drm_vblank_get -> drm_vblank_enable (acquires vblank_time_lock) ->
> > > __enable_vblank -> dpu_crtc_vblank ->
> > > dpu_encoder_toggle_vblank_for_crtc
> > > (tries to acquire enc_spinlock)
> > > 
> > > In vblank handler, the call stack will be:
> > > dpu_encoder_phys_vid_vblank_irq -> dpu_encoder_vblank_callback
> > > (acquires
> > > enc_spinlock) -> dpu_crtc_vblank_callback -> drm_handle_vblank
> > > (tries to
> > > acquire vblank_time_lock)
> > 
> > Hmm, I'm not sure how this can happen. We acquire and release the
> > enc_spinlock
> > before enabling the irq, yes we will hold on to the vbl_time_lock, but
> > we
> > shouldn't be trying to reacquire an encoder's spinlock after we've
> > enabled
> > it.
> In the synchronous approach dpu_encoder_toggle_vblank_for_crtc(which
> acquires the enc_spinlock) will be called while we
> are holding the vbl_time_lock.
> 
> > I don't know how that can deadlock, since we should never be running
> > enable and
> > the handler concurrently.
> > 
> I agree that vblank_irq handler should not be running before the enable
> sequence. But
> don't you expect the handler to be running while calling the vblank_disable
> sequence?

This is an entirely different problem though. It's also one that is easier to
fix. I think we could probably grab the enc_spinlock in disable and clear the
crtc pointer.

What I'm getting at is that there's no fundamental reason why we need to have
async vblank enable/disable.

Sean

> vbl disable will try to acquire the locks in the opposite order to that of
> irq_handler and the
> same issue is bound to happen.
> 
> With your patch, you should be able to simulate this deadlock if you can
> inject a delay
> by adding a pr_err log in vblank_ctrl_queue_work
> 
> Thanks,
> Jeykumar S.
> 
> > The only thing I can think of is that the vblank interrupts are firing
> > after
> > vblank has been disabled? In that case, it seems like we should properly
> > flush
> > them.
> > 
> > Sean
> > 
> > 
> > > 
> > > 
> > > > I do see that there is a chance to

Re: [Freedreno] [PATCH] of/device: add blacklist for iommu dma_ops

2018-12-03 Thread Rob Clark
On Mon, Dec 3, 2018 at 7:45 AM Robin Murphy  wrote:
>
> Hi Rob,
>
> On 01/12/2018 16:53, Rob Clark wrote:
> > This solves a problem we see with drm/msm, caused by getting
> > iommu_dma_ops while we attach our own domain and manage it directly at
> > the iommu API level:
> >
> >[0038] user address but active_mm is swapper
> >Internal error: Oops: 9605 [#1] PREEMPT SMP
> >Modules linked in:
> >CPU: 7 PID: 70 Comm: kworker/7:1 Tainted: GW 4.19.3 #90
> >Hardware name: xxx (DT)
> >Workqueue: events deferred_probe_work_func
> >pstate: 80c9 (Nzcv daif +PAN +UAO)
> >pc : iommu_dma_map_sg+0x7c/0x2c8
> >lr : iommu_dma_map_sg+0x40/0x2c8
> >sp : ff80095eb4f0
> >x29: ff80095eb4f0 x28: 
> >x27: ffc0f9431578 x26: 
> >x25:  x24: 0003
> >x23: 0001 x22: ffc0fa9ac010
> >x21:  x20: ffc0fab40980
> >x19: ffc0fab40980 x18: 0003
> >x17: 01c4 x16: 0007
> >x15: 000e x14: 
> >x13:  x12: 0028
> >x11: 0101010101010101 x10: 7f7f7f7f7f7f7f7f
> >x9 :  x8 : ffc0fab409a0
> >x7 :  x6 : 0002
> >x5 : 0001 x4 : 
> >x3 : 0001 x2 : 0002
> >x1 : ffc0f9431578 x0 : 
> >Process kworker/7:1 (pid: 70, stack limit = 0x17d08ffb)
> >Call trace:
> > iommu_dma_map_sg+0x7c/0x2c8
> > __iommu_map_sg_attrs+0x70/0x84
> > get_pages+0x170/0x1e8
> > msm_gem_get_iova+0x8c/0x128
> > _msm_gem_kernel_new+0x6c/0xc8
> > msm_gem_kernel_new+0x4c/0x58
> > dsi_tx_buf_alloc_6g+0x4c/0x8c
> > msm_dsi_host_modeset_init+0xc8/0x108
> > msm_dsi_modeset_init+0x54/0x18c
> > _dpu_kms_drm_obj_init+0x430/0x474
> > dpu_kms_hw_init+0x5f8/0x6b4
> > msm_drm_bind+0x360/0x6c8
> > try_to_bring_up_master.part.7+0x28/0x70
> > component_master_add_with_match+0xe8/0x124
> > msm_pdev_probe+0x294/0x2b4
> > platform_drv_probe+0x58/0xa4
> > really_probe+0x150/0x294
> > driver_probe_device+0xac/0xe8
> > __device_attach_driver+0xa4/0xb4
> > bus_for_each_drv+0x98/0xc8
> > __device_attach+0xac/0x12c
> > device_initial_probe+0x24/0x30
> > bus_probe_device+0x38/0x98
> > deferred_probe_work_func+0x78/0xa4
> > process_one_work+0x24c/0x3dc
> > worker_thread+0x280/0x360
> > kthread+0x134/0x13c
> > ret_from_fork+0x10/0x18
> >Code: d284 91000725 6b17039f 5400048a (f9401f40)
> >---[ end trace f22dda57f3648e2c ]---
> >Kernel panic - not syncing: Fatal exception
> >SMP: stopping secondary CPUs
> >Kernel Offset: disabled
> >CPU features: 0x0,22802a18
> >Memory Limit: none
> >
> > The problem is that when drm/msm does it's own iommu_attach_device(),
> > now the domain returned by iommu_get_domain_for_dev() is drm/msm's
> > domain, and it doesn't have domain->iova_cookie.
>
> Does this crash still happen with 4.20-rc? Because as of 6af588fed391 it
> really shouldn't.

for this hw, I'm still on 4.19, although that does look like it would
avoid the issue.

> > We kind of avoided this problem prior to sdm845/dpu because the iommu
> > was attached to the mdp node in dt, which is a child of the toplevel
> > mdss node (which corresponds to the dev passed in dma_map_sg()).  But
> > with sdm845, now the iommu is attached at the mdss level so we hit the
> > iommu_dma_ops in dma_map_sg().
> >
> > But auto allocating/attaching a domain before the driver is probed was
> > already a blocking problem for enabling per-context pagetables for the
> > GPU.  This problem is also now solved with this patch.
>
> s/solved/worked around/
>
> If you want a guarantee of actually getting a specific hardware context
> allocated for a given domain, there needs to be code in the IOMMU driver
> to understand and honour that. Implicitly depending on whatever happens
> to fall out of current driver behaviour on current systems is not a real
> solution.

ok, fair.. but I'll settle for "works" in the absence of better options..

At some level, it would be nice to be able to optionally specify a
context-bank in the iommu bindings.  But not sure how to make that fit
w/ cb allocated per domain.  And I assume I'm the only one who has
this problem?

> > Fixes: 97890ba9289c dma-mapping: detect and configure IOMMU in 
> > of_dma_configure
>
> That's rather misleading, since the crash described above depends on at
> least two other major changes which came long after that commit.

Fair, when I realized it was the difference in where the iommu
attaches between dpu1 (sdm845) and everything coming before, I should
have removed the tag.

BR,
-R

> It's not that I don't understand exactly what you want here - just that
> this commit message isn't a coheren

Re: [Freedreno] [PATCH] of/device: add blacklist for iommu dma_ops

2018-12-03 Thread Robin Murphy

Hi Rob,

On 01/12/2018 16:53, Rob Clark wrote:

This solves a problem we see with drm/msm, caused by getting
iommu_dma_ops while we attach our own domain and manage it directly at
the iommu API level:

   [0038] user address but active_mm is swapper
   Internal error: Oops: 9605 [#1] PREEMPT SMP
   Modules linked in:
   CPU: 7 PID: 70 Comm: kworker/7:1 Tainted: GW 4.19.3 #90
   Hardware name: xxx (DT)
   Workqueue: events deferred_probe_work_func
   pstate: 80c9 (Nzcv daif +PAN +UAO)
   pc : iommu_dma_map_sg+0x7c/0x2c8
   lr : iommu_dma_map_sg+0x40/0x2c8
   sp : ff80095eb4f0
   x29: ff80095eb4f0 x28: 
   x27: ffc0f9431578 x26: 
   x25:  x24: 0003
   x23: 0001 x22: ffc0fa9ac010
   x21:  x20: ffc0fab40980
   x19: ffc0fab40980 x18: 0003
   x17: 01c4 x16: 0007
   x15: 000e x14: 
   x13:  x12: 0028
   x11: 0101010101010101 x10: 7f7f7f7f7f7f7f7f
   x9 :  x8 : ffc0fab409a0
   x7 :  x6 : 0002
   x5 : 0001 x4 : 
   x3 : 0001 x2 : 0002
   x1 : ffc0f9431578 x0 : 
   Process kworker/7:1 (pid: 70, stack limit = 0x17d08ffb)
   Call trace:
iommu_dma_map_sg+0x7c/0x2c8
__iommu_map_sg_attrs+0x70/0x84
get_pages+0x170/0x1e8
msm_gem_get_iova+0x8c/0x128
_msm_gem_kernel_new+0x6c/0xc8
msm_gem_kernel_new+0x4c/0x58
dsi_tx_buf_alloc_6g+0x4c/0x8c
msm_dsi_host_modeset_init+0xc8/0x108
msm_dsi_modeset_init+0x54/0x18c
_dpu_kms_drm_obj_init+0x430/0x474
dpu_kms_hw_init+0x5f8/0x6b4
msm_drm_bind+0x360/0x6c8
try_to_bring_up_master.part.7+0x28/0x70
component_master_add_with_match+0xe8/0x124
msm_pdev_probe+0x294/0x2b4
platform_drv_probe+0x58/0xa4
really_probe+0x150/0x294
driver_probe_device+0xac/0xe8
__device_attach_driver+0xa4/0xb4
bus_for_each_drv+0x98/0xc8
__device_attach+0xac/0x12c
device_initial_probe+0x24/0x30
bus_probe_device+0x38/0x98
deferred_probe_work_func+0x78/0xa4
process_one_work+0x24c/0x3dc
worker_thread+0x280/0x360
kthread+0x134/0x13c
ret_from_fork+0x10/0x18
   Code: d284 91000725 6b17039f 5400048a (f9401f40)
   ---[ end trace f22dda57f3648e2c ]---
   Kernel panic - not syncing: Fatal exception
   SMP: stopping secondary CPUs
   Kernel Offset: disabled
   CPU features: 0x0,22802a18
   Memory Limit: none

The problem is that when drm/msm does it's own iommu_attach_device(),
now the domain returned by iommu_get_domain_for_dev() is drm/msm's
domain, and it doesn't have domain->iova_cookie.


Does this crash still happen with 4.20-rc? Because as of 6af588fed391 it 
really shouldn't.



We kind of avoided this problem prior to sdm845/dpu because the iommu
was attached to the mdp node in dt, which is a child of the toplevel
mdss node (which corresponds to the dev passed in dma_map_sg()).  But
with sdm845, now the iommu is attached at the mdss level so we hit the
iommu_dma_ops in dma_map_sg().

But auto allocating/attaching a domain before the driver is probed was
already a blocking problem for enabling per-context pagetables for the
GPU.  This problem is also now solved with this patch.


s/solved/worked around/

If you want a guarantee of actually getting a specific hardware context 
allocated for a given domain, there needs to be code in the IOMMU driver 
to understand and honour that. Implicitly depending on whatever happens 
to fall out of current driver behaviour on current systems is not a real 
solution.



Fixes: 97890ba9289c dma-mapping: detect and configure IOMMU in of_dma_configure


That's rather misleading, since the crash described above depends on at 
least two other major changes which came long after that commit.


It's not that I don't understand exactly what you want here - just that 
this commit message isn't a coherent justification for that ;)



Tested-by: Douglas Anderson 
Signed-off-by: Rob Clark 
---
This is an alternative/replacement for [1].  What it lacks in elegance
it makes up for in practicality ;-)

[1] https://patchwork.freedesktop.org/patch/264930/

  drivers/of/device.c | 22 ++
  1 file changed, 22 insertions(+)

diff --git a/drivers/of/device.c b/drivers/of/device.c
index 5957cd4fa262..15ffee00fb22 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -72,6 +72,14 @@ int of_device_add(struct platform_device *ofdev)
return device_add(&ofdev->dev);
  }
  
+static const struct of_device_id iommu_blacklist[] = {

+   { .compatible = "qcom,mdp4" },
+   { .compatible = "qcom,mdss" },
+   { .compatible = "qcom,sdm845-mdss" },
+   { .compatible = "qcom,adreno" },
+   {}
+};
+
  /**
   * of_dma_configure - Setup DMA configuration
   * @dev:  Device to apply DMA configurati

Re: [Freedreno] [PATCH] of/device: add blacklist for iommu dma_ops

2018-12-03 Thread Marek Szyprowski
Hi Tomasz,

On 2018-12-03 01:10, Tomasz Figa wrote:
> On Sat, Dec 1, 2018 at 8:54 AM Rob Clark  wrote:
>> This solves a problem we see with drm/msm, caused by getting
>> iommu_dma_ops while we attach our own domain and manage it directly at
>> the iommu API level:
>>
>>   [0038] user address but active_mm is swapper
>>   Internal error: Oops: 9605 [#1] PREEMPT SMP
>>   Modules linked in:
>>   CPU: 7 PID: 70 Comm: kworker/7:1 Tainted: GW 4.19.3 #90
>>   Hardware name: xxx (DT)
>>   Workqueue: events deferred_probe_work_func
>>   pstate: 80c9 (Nzcv daif +PAN +UAO)
>>   pc : iommu_dma_map_sg+0x7c/0x2c8
>>   lr : iommu_dma_map_sg+0x40/0x2c8
>>   sp : ff80095eb4f0
>>   x29: ff80095eb4f0 x28: 
>>   x27: ffc0f9431578 x26: 
>>   x25:  x24: 0003
>>   x23: 0001 x22: ffc0fa9ac010
>>   x21:  x20: ffc0fab40980
>>   x19: ffc0fab40980 x18: 0003
>>   x17: 01c4 x16: 0007
>>   x15: 000e x14: 
>>   x13:  x12: 0028
>>   x11: 0101010101010101 x10: 7f7f7f7f7f7f7f7f
>>   x9 :  x8 : ffc0fab409a0
>>   x7 :  x6 : 0002
>>   x5 : 0001 x4 : 
>>   x3 : 0001 x2 : 0002
>>   x1 : ffc0f9431578 x0 : 
>>   Process kworker/7:1 (pid: 70, stack limit = 0x17d08ffb)
>>   Call trace:
>>iommu_dma_map_sg+0x7c/0x2c8
>>__iommu_map_sg_attrs+0x70/0x84
>>get_pages+0x170/0x1e8
>>msm_gem_get_iova+0x8c/0x128
>>_msm_gem_kernel_new+0x6c/0xc8
>>msm_gem_kernel_new+0x4c/0x58
>>dsi_tx_buf_alloc_6g+0x4c/0x8c
>>msm_dsi_host_modeset_init+0xc8/0x108
>>msm_dsi_modeset_init+0x54/0x18c
>>_dpu_kms_drm_obj_init+0x430/0x474
>>dpu_kms_hw_init+0x5f8/0x6b4
>>msm_drm_bind+0x360/0x6c8
>>try_to_bring_up_master.part.7+0x28/0x70
>>component_master_add_with_match+0xe8/0x124
>>msm_pdev_probe+0x294/0x2b4
>>platform_drv_probe+0x58/0xa4
>>really_probe+0x150/0x294
>>driver_probe_device+0xac/0xe8
>>__device_attach_driver+0xa4/0xb4
>>bus_for_each_drv+0x98/0xc8
>>__device_attach+0xac/0x12c
>>device_initial_probe+0x24/0x30
>>bus_probe_device+0x38/0x98
>>deferred_probe_work_func+0x78/0xa4
>>process_one_work+0x24c/0x3dc
>>worker_thread+0x280/0x360
>>kthread+0x134/0x13c
>>ret_from_fork+0x10/0x18
>>   Code: d284 91000725 6b17039f 5400048a (f9401f40)
>>   ---[ end trace f22dda57f3648e2c ]---
>>   Kernel panic - not syncing: Fatal exception
>>   SMP: stopping secondary CPUs
>>   Kernel Offset: disabled
>>   CPU features: 0x0,22802a18
>>   Memory Limit: none
>>
>> The problem is that when drm/msm does it's own iommu_attach_device(),
>> now the domain returned by iommu_get_domain_for_dev() is drm/msm's
>> domain, and it doesn't have domain->iova_cookie.
>>
>> We kind of avoided this problem prior to sdm845/dpu because the iommu
>> was attached to the mdp node in dt, which is a child of the toplevel
>> mdss node (which corresponds to the dev passed in dma_map_sg()).  But
>> with sdm845, now the iommu is attached at the mdss level so we hit the
>> iommu_dma_ops in dma_map_sg().
>>
>> But auto allocating/attaching a domain before the driver is probed was
>> already a blocking problem for enabling per-context pagetables for the
>> GPU.  This problem is also now solved with this patch.
>>
>> Fixes: 97890ba9289c dma-mapping: detect and configure IOMMU in 
>> of_dma_configure
>> Tested-by: Douglas Anderson 
>> Signed-off-by: Rob Clark 
>> ---
>> This is an alternative/replacement for [1].  What it lacks in elegance
>> it makes up for in practicality ;-)
>>
>> [1] https://patchwork.freedesktop.org/patch/264930/
>>
>>  drivers/of/device.c | 22 ++
>>  1 file changed, 22 insertions(+)
>>
>> diff --git a/drivers/of/device.c b/drivers/of/device.c
>> index 5957cd4fa262..15ffee00fb22 100644
>> --- a/drivers/of/device.c
>> +++ b/drivers/of/device.c
>> @@ -72,6 +72,14 @@ int of_device_add(struct platform_device *ofdev)
>> return device_add(&ofdev->dev);
>>  }
>>
>> +static const struct of_device_id iommu_blacklist[] = {
>> +   { .compatible = "qcom,mdp4" },
>> +   { .compatible = "qcom,mdss" },
>> +   { .compatible = "qcom,sdm845-mdss" },
>> +   { .compatible = "qcom,adreno" },
>> +   {}
>> +};
>> +
>>  /**
>>   * of_dma_configure - Setup DMA configuration
>>   * @dev:   Device to apply DMA configuration
>> @@ -164,6 +172,20 @@ int of_dma_configure(struct device *dev, struct 
>> device_node *np, bool force_dma)
>> dev_dbg(dev, "device is%sbehind an iommu\n",
>> iommu ? " " : " not ");
>>
>> +   /*
>> +* There is at least one case where the driver wants to directly
>> +* manage the IOMMU, but if we end up with iommu dma_ops, that
>> +* int