Re: [PATCH V2 00/11] Subject: Remove duplicated kmap code

2020-05-03 Thread Ira Weiny
On Mon, May 04, 2020 at 02:35:09AM +0100, Al Viro wrote:
> On Sun, May 03, 2020 at 06:09:01PM -0700, ira.we...@intel.com wrote:
> > From: Ira Weiny 
> > 
> > The kmap infrastructure has been copied almost verbatim to every 
> > architecture.
> > This series consolidates obvious duplicated code by defining core functions
> > which call into the architectures only when needed.
> > 
> > Some of the k[un]map_atomic() implementations have some similarities but the
> > similarities were not sufficient to warrant further changes.
> > 
> > In addition we remove a duplicate implementation of kmap() in DRM.
> > 
> > Testing was done by 0day to cover all the architectures I can't readily
> > build/test.
> 
> OK...  Looking through my old notes on kmap unification (this winter, never
> went anywhere),
> 
> * arch/mips/mm/cache.c ought to use linux/highmem.h, not asm/highmem.h
> I suspect that your series doesn't build on some configs there.  Hadn't
> verified that, though.

Yes patch 6 makes the change because kmap_atomic() was no longer declared in
asm/highmem.h.  I'm pretty sure 0-day caught that ...  but I seem to remember
noticing some oddness in that file and I did go through it by hand.

> 
> * kmap_atomic_to_page() is dead, but not quite gone - csky and nds32 brought
> the damn thing back (nds32 - only an extern).  It needs killin'...

Easy enough. Added as a follow on patch.

> 
> * parisc is (arguably) abusing kunmap()/kunmap_atomic() for cache flushing.
> Replace the bulk of its highmem.h with
> #define ARCH_HAS_FLUSH_ON_KUNMAP
> #define arch_before_kunmap flush_kernel_dcache_page_addr
> and have default kunmap()/kunmap_atomic() do
> #ifdef ARCH_HAS_FLUSH_ON_KUNMAP
>   arch_before_kunmap(page_address(page));
> #endif
> and
> #ifdef ARCH_HAS_FLUSH_ON_KUNMAP
>   arch_before_kunmap(addr);
> #endif
> resp.  Kills ARCH_HAS_KMAP along with ifdefs on it, makes parisc use somewhat
> less hacky.

Agreed.  Done in a follow on patch.

> 
> I'd suggest checking various configs on mips - that's likely to cause 
> headache.
> Said that, my analysis of include chains back then is pretty much worthless
> by now - I really hate the amount of indirect include chains leading to that
> sucker on some, but not all configs ;-/  IIRC, the proof that everything
> using kmap*/kunmap* would pull linux/highmem.h regardless of config took 
> several
> hours of digging, ran for several pages and had been hopelessly brittle.
> arch/mips/mm/cache.c was the only exception caught by it, but these days
> there might be more.

Grepping for 'asm/highmem.h' and investigations don't reveal any issues...  But
you do have me worried.  That said 0-day has been crunching on multiple
versions of this series without issues such as this (save the mips issue
above).

I have to say it would be nice if the relation between linux/highmem.h and
asm/highmem.h was more straightforward.

Ira

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


linux-next: manual merge of the devicetree tree with the drm tree

2020-05-03 Thread Stephen Rothwell
Hi all,

Today's linux-next merge of the devicetree tree got a conflict in:

  Documentation/devicetree/bindings/display/panel/panel-common.yaml

between commit:

  92e513fb0798 ("dt-bindings: display: grammar fixes in panel/")

from the drm tree and commit:

  3d21a4609335 ("dt-bindings: Remove cases of 'allOf' containing a '$ref'")

from the devicetree tree.

I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging.  You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.

-- 
Cheers,
Stephen Rothwell

diff --cc Documentation/devicetree/bindings/display/panel/panel-common.yaml
index 17b8367f12dd,db3d270a33c6..
--- a/Documentation/devicetree/bindings/display/panel/panel-common.yaml
+++ b/Documentation/devicetree/bindings/display/panel/panel-common.yaml
@@@ -63,11 -61,10 +61,10 @@@ properties
  
display-timings:
  description:
 -  Some display panels supports several resolutions with different timing.
 +  Some display panels support several resolutions with different timings.
The display-timings bindings supports specifying several timings and
 -  optional specify which is the native mode.
 +  optionally specifying which is the native mode.
- allOf:
-   - $ref: display-timings.yaml#
+ $ref: display-timings.yaml#
  
# Connectivity
port:


pgpU0VAuamGA9.pgp
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH V2 01/11] arch/kmap: Remove BUG_ON()

2020-05-03 Thread ira . weiny
From: Ira Weiny 

Replace the use of BUG_ON(in_interrupt()) in the kmap() and kunmap()
in favor of might_sleep().

Besides the benefits of might_sleep(), this normalizes the
implementations such that they can be made generic in subsequent
patches.

Reviewed-by: Dan Williams 
Reviewed-by: Christoph Hellwig 
Signed-off-by: Ira Weiny 
---
 arch/arc/include/asm/highmem.h| 2 +-
 arch/arc/mm/highmem.c | 2 +-
 arch/arm/mm/highmem.c | 2 +-
 arch/csky/mm/highmem.c| 2 +-
 arch/microblaze/include/asm/highmem.h | 2 +-
 arch/mips/mm/highmem.c| 2 +-
 arch/nds32/mm/highmem.c   | 2 +-
 arch/powerpc/include/asm/highmem.h| 2 +-
 arch/sparc/include/asm/highmem.h  | 4 ++--
 arch/x86/mm/highmem_32.c  | 3 +--
 arch/xtensa/include/asm/highmem.h | 4 ++--
 11 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/arch/arc/include/asm/highmem.h b/arch/arc/include/asm/highmem.h
index 1af00accb37f..042e92921c4c 100644
--- a/arch/arc/include/asm/highmem.h
+++ b/arch/arc/include/asm/highmem.h
@@ -45,7 +45,7 @@ static inline void flush_cache_kmaps(void)
 
 static inline void kunmap(struct page *page)
 {
-   BUG_ON(in_interrupt());
+   might_sleep();
if (!PageHighMem(page))
return;
kunmap_high(page);
diff --git a/arch/arc/mm/highmem.c b/arch/arc/mm/highmem.c
index fc8849e4f72e..39ef7b9a3aa9 100644
--- a/arch/arc/mm/highmem.c
+++ b/arch/arc/mm/highmem.c
@@ -51,7 +51,7 @@ static pte_t * fixmap_page_table;
 
 void *kmap(struct page *page)
 {
-   BUG_ON(in_interrupt());
+   might_sleep();
if (!PageHighMem(page))
return page_address(page);
 
diff --git a/arch/arm/mm/highmem.c b/arch/arm/mm/highmem.c
index a76f8ace9ce6..cc6eb79ef20c 100644
--- a/arch/arm/mm/highmem.c
+++ b/arch/arm/mm/highmem.c
@@ -42,7 +42,7 @@ EXPORT_SYMBOL(kmap);
 
 void kunmap(struct page *page)
 {
-   BUG_ON(in_interrupt());
+   might_sleep();
if (!PageHighMem(page))
return;
kunmap_high(page);
diff --git a/arch/csky/mm/highmem.c b/arch/csky/mm/highmem.c
index 813129145f3d..690d678649d1 100644
--- a/arch/csky/mm/highmem.c
+++ b/arch/csky/mm/highmem.c
@@ -29,7 +29,7 @@ EXPORT_SYMBOL(kmap);
 
 void kunmap(struct page *page)
 {
-   BUG_ON(in_interrupt());
+   might_sleep();
if (!PageHighMem(page))
return;
kunmap_high(page);
diff --git a/arch/microblaze/include/asm/highmem.h 
b/arch/microblaze/include/asm/highmem.h
index 332c78e15198..99ced7278b5c 100644
--- a/arch/microblaze/include/asm/highmem.h
+++ b/arch/microblaze/include/asm/highmem.h
@@ -66,7 +66,7 @@ static inline void *kmap(struct page *page)
 
 static inline void kunmap(struct page *page)
 {
-   BUG_ON(in_interrupt());
+   might_sleep();
if (!PageHighMem(page))
return;
kunmap_high(page);
diff --git a/arch/mips/mm/highmem.c b/arch/mips/mm/highmem.c
index d08e6d7d533b..edd889f6cede 100644
--- a/arch/mips/mm/highmem.c
+++ b/arch/mips/mm/highmem.c
@@ -28,7 +28,7 @@ EXPORT_SYMBOL(kmap);
 
 void kunmap(struct page *page)
 {
-   BUG_ON(in_interrupt());
+   might_sleep();
if (!PageHighMem(page))
return;
kunmap_high(page);
diff --git a/arch/nds32/mm/highmem.c b/arch/nds32/mm/highmem.c
index 022779af6148..4c7c28e994ea 100644
--- a/arch/nds32/mm/highmem.c
+++ b/arch/nds32/mm/highmem.c
@@ -24,7 +24,7 @@ EXPORT_SYMBOL(kmap);
 
 void kunmap(struct page *page)
 {
-   BUG_ON(in_interrupt());
+   might_sleep();
if (!PageHighMem(page))
return;
kunmap_high(page);
diff --git a/arch/powerpc/include/asm/highmem.h 
b/arch/powerpc/include/asm/highmem.h
index a4b65b186ec6..529512f6d65a 100644
--- a/arch/powerpc/include/asm/highmem.h
+++ b/arch/powerpc/include/asm/highmem.h
@@ -74,7 +74,7 @@ static inline void *kmap(struct page *page)
 
 static inline void kunmap(struct page *page)
 {
-   BUG_ON(in_interrupt());
+   might_sleep();
if (!PageHighMem(page))
return;
kunmap_high(page);
diff --git a/arch/sparc/include/asm/highmem.h b/arch/sparc/include/asm/highmem.h
index 18d776925c45..7dd2d4b3f980 100644
--- a/arch/sparc/include/asm/highmem.h
+++ b/arch/sparc/include/asm/highmem.h
@@ -55,7 +55,7 @@ void kunmap_high(struct page *page);
 
 static inline void *kmap(struct page *page)
 {
-   BUG_ON(in_interrupt());
+   might_sleep();
if (!PageHighMem(page))
return page_address(page);
return kmap_high(page);
@@ -63,7 +63,7 @@ static inline void *kmap(struct page *page)
 
 static inline void kunmap(struct page *page)
 {
-   BUG_ON(in_interrupt());
+   might_sleep();
if (!PageHighMem(page))
return;
kunmap_high(page);
diff --git a/arch/x86/mm/highmem_32.c b/arch/x86/mm/highmem_32.c
index 0a1898b8552e..8af66382672b 100644
--- a/arch/x86/mm/highmem_32.c
+++ 

[PATCH V2 05/11] {x86,powerpc,microblaze}/kmap: Move preempt disable

2020-05-03 Thread ira . weiny
From: Ira Weiny 

During this kmap() conversion series we must maintain bisect-ability.
To do this, kmap_atomic_prot() in x86, powerpc, and microblaze need to
remain functional.

Create a temporary inline version of kmap_atomic_prot within these
architectures so we can rework their kmap_atomic() calls and then lift
kmap_atomic_prot() to the core.

Signed-off-by: Ira Weiny 

---
Changes from V1:
New patch
---
 arch/microblaze/include/asm/highmem.h | 11 ++-
 arch/microblaze/mm/highmem.c  | 10 ++
 arch/powerpc/include/asm/highmem.h| 11 ++-
 arch/powerpc/mm/highmem.c |  9 ++---
 arch/x86/include/asm/highmem.h| 11 ++-
 arch/x86/mm/highmem_32.c  | 10 ++
 6 files changed, 36 insertions(+), 26 deletions(-)

diff --git a/arch/microblaze/include/asm/highmem.h 
b/arch/microblaze/include/asm/highmem.h
index 0c94046f2d58..ec9954b091e1 100644
--- a/arch/microblaze/include/asm/highmem.h
+++ b/arch/microblaze/include/asm/highmem.h
@@ -51,7 +51,16 @@ extern pte_t *pkmap_page_table;
 #define PKMAP_NR(virt)  ((virt - PKMAP_BASE) >> PAGE_SHIFT)
 #define PKMAP_ADDR(nr)  (PKMAP_BASE + ((nr) << PAGE_SHIFT))
 
-extern void *kmap_atomic_prot(struct page *page, pgprot_t prot);
+extern void *kmap_atomic_high_prot(struct page *page, pgprot_t prot);
+void *kmap_atomic_prot(struct page *page, pgprot_t prot)
+{
+   preempt_disable();
+   pagefault_disable();
+   if (!PageHighMem(page))
+   return page_address(page);
+
+   return kmap_atomic_high_prot(page, prot);
+}
 extern void __kunmap_atomic(void *kvaddr);
 
 static inline void *kmap_atomic(struct page *page)
diff --git a/arch/microblaze/mm/highmem.c b/arch/microblaze/mm/highmem.c
index d7569f77fa15..0e3efaa8a004 100644
--- a/arch/microblaze/mm/highmem.c
+++ b/arch/microblaze/mm/highmem.c
@@ -32,18 +32,12 @@
  */
 #include 
 
-void *kmap_atomic_prot(struct page *page, pgprot_t prot)
+void *kmap_atomic_high_prot(struct page *page, pgprot_t prot)
 {
 
unsigned long vaddr;
int idx, type;
 
-   preempt_disable();
-   pagefault_disable();
-   if (!PageHighMem(page))
-   return page_address(page);
-
-
type = kmap_atomic_idx_push();
idx = type + KM_TYPE_NR*smp_processor_id();
vaddr = __fix_to_virt(FIX_KMAP_BEGIN + idx);
@@ -55,7 +49,7 @@ void *kmap_atomic_prot(struct page *page, pgprot_t prot)
 
return (void *) vaddr;
 }
-EXPORT_SYMBOL(kmap_atomic_prot);
+EXPORT_SYMBOL(kmap_atomic_high_prot);
 
 void __kunmap_atomic(void *kvaddr)
 {
diff --git a/arch/powerpc/include/asm/highmem.h 
b/arch/powerpc/include/asm/highmem.h
index ba3371977d49..d049806a8354 100644
--- a/arch/powerpc/include/asm/highmem.h
+++ b/arch/powerpc/include/asm/highmem.h
@@ -59,7 +59,16 @@ extern pte_t *pkmap_page_table;
 #define PKMAP_NR(virt)  ((virt-PKMAP_BASE) >> PAGE_SHIFT)
 #define PKMAP_ADDR(nr)  (PKMAP_BASE + ((nr) << PAGE_SHIFT))
 
-extern void *kmap_atomic_prot(struct page *page, pgprot_t prot);
+extern void *kmap_atomic_high_prot(struct page *page, pgprot_t prot);
+static inline void *kmap_atomic_prot(struct page *page, pgprot_t prot)
+{
+   preempt_disable();
+   pagefault_disable();
+   if (!PageHighMem(page))
+   return page_address(page);
+
+   return kmap_atomic_high_prot(page, prot);
+}
 extern void __kunmap_atomic(void *kvaddr);
 
 static inline void *kmap_atomic(struct page *page)
diff --git a/arch/powerpc/mm/highmem.c b/arch/powerpc/mm/highmem.c
index 320c1672b2ae..f075cef6d663 100644
--- a/arch/powerpc/mm/highmem.c
+++ b/arch/powerpc/mm/highmem.c
@@ -30,16 +30,11 @@
  * be used in IRQ contexts, so in some (very limited) cases we need
  * it.
  */
-void *kmap_atomic_prot(struct page *page, pgprot_t prot)
+void *kmap_atomic_high_prot(struct page *page, pgprot_t prot)
 {
unsigned long vaddr;
int idx, type;
 
-   preempt_disable();
-   pagefault_disable();
-   if (!PageHighMem(page))
-   return page_address(page);
-
type = kmap_atomic_idx_push();
idx = type + KM_TYPE_NR*smp_processor_id();
vaddr = __fix_to_virt(FIX_KMAP_BEGIN + idx);
@@ -49,7 +44,7 @@ void *kmap_atomic_prot(struct page *page, pgprot_t prot)
 
return (void*) vaddr;
 }
-EXPORT_SYMBOL(kmap_atomic_prot);
+EXPORT_SYMBOL(kmap_atomic_high_prot);
 
 void __kunmap_atomic(void *kvaddr)
 {
diff --git a/arch/x86/include/asm/highmem.h b/arch/x86/include/asm/highmem.h
index 90b96594d6c5..61f47fef40e5 100644
--- a/arch/x86/include/asm/highmem.h
+++ b/arch/x86/include/asm/highmem.h
@@ -58,7 +58,16 @@ extern unsigned long highstart_pfn, highend_pfn;
 #define PKMAP_NR(virt)  ((virt-PKMAP_BASE) >> PAGE_SHIFT)
 #define PKMAP_ADDR(nr)  (PKMAP_BASE + ((nr) << PAGE_SHIFT))
 
-void *kmap_atomic_prot(struct page *page, pgprot_t prot);
+extern void *kmap_atomic_high_prot(struct page *page, pgprot_t prot);
+static inline void *kmap_atomic_prot(struct page *page, pgprot_t prot)

[PATCH V2 06/11] arch/kmap_atomic: Consolidate duplicate code

2020-05-03 Thread ira . weiny
From: Ira Weiny 

Every arch has the same code to ensure atomic operations and a check for
!HIGHMEM page.

Remove the duplicate code by defining a core kmap_atomic() which only
calls the arch specific kmap_atomic_high() when the page is high memory.

Signed-off-by: Ira Weiny 

---
Changes from V1:
Adjust to preserve bisect-ability
Remove unneeded kmap_atomic_high declarations
---
 arch/arc/include/asm/highmem.h|  1 -
 arch/arc/mm/highmem.c |  9 ++---
 arch/arm/include/asm/highmem.h|  1 -
 arch/arm/mm/highmem.c |  9 ++---
 arch/csky/include/asm/highmem.h   |  1 -
 arch/csky/mm/highmem.c|  9 ++---
 arch/microblaze/include/asm/highmem.h |  4 ++--
 arch/mips/include/asm/highmem.h   |  1 -
 arch/mips/mm/cache.c  |  2 +-
 arch/mips/mm/highmem.c| 18 ++
 arch/nds32/include/asm/highmem.h  |  1 -
 arch/nds32/mm/highmem.c   |  9 ++---
 arch/powerpc/include/asm/highmem.h|  4 ++--
 arch/powerpc/mm/highmem.c |  6 --
 arch/sparc/include/asm/highmem.h  |  1 -
 arch/sparc/mm/highmem.c   |  9 ++---
 arch/x86/include/asm/highmem.h|  5 -
 arch/x86/mm/highmem_32.c  | 14 --
 arch/xtensa/include/asm/highmem.h |  1 -
 arch/xtensa/mm/highmem.c  |  9 ++---
 include/linux/highmem.h   | 23 +++
 21 files changed, 46 insertions(+), 91 deletions(-)

diff --git a/arch/arc/include/asm/highmem.h b/arch/arc/include/asm/highmem.h
index 8387a5596a91..db425cd38545 100644
--- a/arch/arc/include/asm/highmem.h
+++ b/arch/arc/include/asm/highmem.h
@@ -30,7 +30,6 @@
 
 #include 
 
-extern void *kmap_atomic(struct page *page);
 extern void __kunmap_atomic(void *kvaddr);
 
 extern void kmap_init(void);
diff --git a/arch/arc/mm/highmem.c b/arch/arc/mm/highmem.c
index 4db13a6b9f3b..0964b011c29f 100644
--- a/arch/arc/mm/highmem.c
+++ b/arch/arc/mm/highmem.c
@@ -49,16 +49,11 @@
 extern pte_t * pkmap_page_table;
 static pte_t * fixmap_page_table;
 
-void *kmap_atomic(struct page *page)
+void *kmap_atomic_high(struct page *page)
 {
int idx, cpu_idx;
unsigned long vaddr;
 
-   preempt_disable();
-   pagefault_disable();
-   if (!PageHighMem(page))
-   return page_address(page);
-
cpu_idx = kmap_atomic_idx_push();
idx = cpu_idx + KM_TYPE_NR * smp_processor_id();
vaddr = FIXMAP_ADDR(idx);
@@ -68,7 +63,7 @@ void *kmap_atomic(struct page *page)
 
return (void *)vaddr;
 }
-EXPORT_SYMBOL(kmap_atomic);
+EXPORT_SYMBOL(kmap_atomic_high);
 
 void __kunmap_atomic(void *kv)
 {
diff --git a/arch/arm/include/asm/highmem.h b/arch/arm/include/asm/highmem.h
index 736f65283e7b..8c80bfe18a34 100644
--- a/arch/arm/include/asm/highmem.h
+++ b/arch/arm/include/asm/highmem.h
@@ -60,7 +60,6 @@ static inline void *kmap_high_get(struct page *page)
  * when CONFIG_HIGHMEM is not set.
  */
 #ifdef CONFIG_HIGHMEM
-extern void *kmap_atomic(struct page *page);
 extern void __kunmap_atomic(void *kvaddr);
 extern void *kmap_atomic_pfn(unsigned long pfn);
 #endif
diff --git a/arch/arm/mm/highmem.c b/arch/arm/mm/highmem.c
index c700b32350ee..075fdc235091 100644
--- a/arch/arm/mm/highmem.c
+++ b/arch/arm/mm/highmem.c
@@ -31,18 +31,13 @@ static inline pte_t get_fixmap_pte(unsigned long vaddr)
return *ptep;
 }
 
-void *kmap_atomic(struct page *page)
+void *kmap_atomic_high(struct page *page)
 {
unsigned int idx;
unsigned long vaddr;
void *kmap;
int type;
 
-   preempt_disable();
-   pagefault_disable();
-   if (!PageHighMem(page))
-   return page_address(page);
-
 #ifdef CONFIG_DEBUG_HIGHMEM
/*
 * There is no cache coherency issue when non VIVT, so force the
@@ -76,7 +71,7 @@ void *kmap_atomic(struct page *page)
 
return (void *)vaddr;
 }
-EXPORT_SYMBOL(kmap_atomic);
+EXPORT_SYMBOL(kmap_atomic_high);
 
 void __kunmap_atomic(void *kvaddr)
 {
diff --git a/arch/csky/include/asm/highmem.h b/arch/csky/include/asm/highmem.h
index be11c5b67122..8ceee12f9bc1 100644
--- a/arch/csky/include/asm/highmem.h
+++ b/arch/csky/include/asm/highmem.h
@@ -32,7 +32,6 @@ extern pte_t *pkmap_page_table;
 
 #define ARCH_HAS_KMAP_FLUSH_TLB
 extern void kmap_flush_tlb(unsigned long addr);
-extern void *kmap_atomic(struct page *page);
 extern void __kunmap_atomic(void *kvaddr);
 extern void *kmap_atomic_pfn(unsigned long pfn);
 extern struct page *kmap_atomic_to_page(void *ptr);
diff --git a/arch/csky/mm/highmem.c b/arch/csky/mm/highmem.c
index e9952211264b..63d74b47eee6 100644
--- a/arch/csky/mm/highmem.c
+++ b/arch/csky/mm/highmem.c
@@ -21,16 +21,11 @@ EXPORT_SYMBOL(kmap_flush_tlb);
 
 EXPORT_SYMBOL(kmap);
 
-void *kmap_atomic(struct page *page)
+void *kmap_atomic_high(struct page *page)
 {
unsigned long vaddr;
int idx, type;
 
-   preempt_disable();

[PATCH V2 00/11] Subject: Remove duplicated kmap code

2020-05-03 Thread ira . weiny
From: Ira Weiny 

The kmap infrastructure has been copied almost verbatim to every architecture.
This series consolidates obvious duplicated code by defining core functions
which call into the architectures only when needed.

Some of the k[un]map_atomic() implementations have some similarities but the
similarities were not sufficient to warrant further changes.

In addition we remove a duplicate implementation of kmap() in DRM.

Testing was done by 0day to cover all the architectures I can't readily
build/test.

---
Changes from V1:
Fix bisect-ability
Update commit message and fix line lengths
Remove unneded kunmap_atomic_high() declarations
Remove unneded kmap_atomic_high() declarations
collect reviews
rebase to 5.7-rc4

Changes from V0:
Define kmap_flush_tlb() and make kmap() truely arch independent.
Redefine the k[un]map_atomic_* code to call into the architectures for
high mem pages
Ensure all architectures define kmap_prot, use it appropriately, and
define kmap_atomic_prot()
Remove drm implementation of kmap_atomic()

Ira Weiny (11):
  arch/kmap: Remove BUG_ON()
  arch/xtensa: Move kmap build bug out of the way
  arch/kmap: Remove redundant arch specific kmaps
  arch/kunmap: Remove duplicate kunmap implementations
  {x86,powerpc,microblaze}/kmap: Move preempt disable
  arch/kmap_atomic: Consolidate duplicate code
  arch/kunmap_atomic: Consolidate duplicate code
  arch/kmap: Ensure kmap_prot visibility
  arch/kmap: Don't hard code kmap_prot values
  arch/kmap: Define kmap_atomic_prot() for all arch's
  drm: Remove drm specific kmap_atomic code

 arch/arc/include/asm/highmem.h| 15 ---
 arch/arc/mm/highmem.c | 28 +++-
 arch/arm/include/asm/highmem.h|  7 ---
 arch/arm/mm/highmem.c | 35 +++
 arch/csky/include/asm/highmem.h   |  9 +---
 arch/csky/mm/highmem.c| 43 +--
 arch/microblaze/include/asm/highmem.h | 28 +---
 arch/microblaze/mm/highmem.c  | 16 ++-
 arch/microblaze/mm/init.c |  3 --
 arch/mips/include/asm/highmem.h   |  9 +---
 arch/mips/mm/cache.c  |  6 +--
 arch/mips/mm/highmem.c| 49 -
 arch/nds32/include/asm/highmem.h  |  7 ---
 arch/nds32/mm/highmem.c   | 39 +++--
 arch/parisc/include/asm/cacheflush.h  |  4 +-
 arch/powerpc/include/asm/highmem.h| 29 +
 arch/powerpc/mm/highmem.c | 21 ++---
 arch/powerpc/mm/mem.c |  3 --
 arch/sparc/include/asm/highmem.h  | 22 --
 arch/sparc/mm/highmem.c   | 18 +++-
 arch/x86/include/asm/highmem.h|  9 
 arch/x86/mm/highmem_32.c  | 50 ++---
 arch/xtensa/include/asm/highmem.h | 27 
 arch/xtensa/mm/highmem.c  | 22 --
 drivers/gpu/drm/ttm/ttm_bo_util.c | 56 ++--
 drivers/gpu/drm/vmwgfx/vmwgfx_blit.c  | 16 +++
 include/drm/ttm/ttm_bo_api.h  |  4 --
 include/linux/highmem.h   | 62 +--
 28 files changed, 140 insertions(+), 497 deletions(-)

-- 
2.25.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH V2 07/11] arch/kunmap_atomic: Consolidate duplicate code

2020-05-03 Thread ira . weiny
From: Ira Weiny 

Every single architecture (including !CONFIG_HIGHMEM) calls...

pagefault_enable();
preempt_enable();

... before returning from __kunmap_atomic().  Lift this code into the
kunmap_atomic() macro.

While we are at it rename __kunmap_atomic() to kunmap_atomic_high() to
be consistent.

Signed-off-by: Ira Weiny 

---
Changes from V1:
Adjust to preserve bisect-ability
Remove uneeded kunmap_atomic_high() declarations
---
 arch/arc/include/asm/highmem.h|  2 --
 arch/arc/mm/highmem.c |  7 ++-
 arch/arm/include/asm/highmem.h|  1 -
 arch/arm/mm/highmem.c |  6 ++
 arch/csky/include/asm/highmem.h   |  1 -
 arch/csky/mm/highmem.c|  9 +++--
 arch/microblaze/include/asm/highmem.h |  1 -
 arch/microblaze/mm/highmem.c  |  6 ++
 arch/mips/include/asm/highmem.h   |  1 -
 arch/mips/mm/cache.c  |  4 ++--
 arch/mips/mm/highmem.c|  6 ++
 arch/nds32/include/asm/highmem.h  |  1 -
 arch/nds32/mm/highmem.c   |  6 ++
 arch/parisc/include/asm/cacheflush.h  |  4 +---
 arch/powerpc/include/asm/highmem.h|  1 -
 arch/powerpc/mm/highmem.c |  6 ++
 arch/sparc/include/asm/highmem.h  |  2 --
 arch/sparc/mm/highmem.c   |  6 ++
 arch/x86/include/asm/highmem.h|  1 -
 arch/x86/mm/highmem_32.c  |  7 ++-
 arch/xtensa/include/asm/highmem.h |  2 --
 arch/xtensa/mm/highmem.c  |  7 ++-
 include/linux/highmem.h   | 11 +++
 23 files changed, 31 insertions(+), 67 deletions(-)

diff --git a/arch/arc/include/asm/highmem.h b/arch/arc/include/asm/highmem.h
index db425cd38545..70900a73bfc8 100644
--- a/arch/arc/include/asm/highmem.h
+++ b/arch/arc/include/asm/highmem.h
@@ -30,8 +30,6 @@
 
 #include 
 
-extern void __kunmap_atomic(void *kvaddr);
-
 extern void kmap_init(void);
 
 static inline void flush_cache_kmaps(void)
diff --git a/arch/arc/mm/highmem.c b/arch/arc/mm/highmem.c
index 0964b011c29f..5d3eab4ac0b0 100644
--- a/arch/arc/mm/highmem.c
+++ b/arch/arc/mm/highmem.c
@@ -65,7 +65,7 @@ void *kmap_atomic_high(struct page *page)
 }
 EXPORT_SYMBOL(kmap_atomic_high);
 
-void __kunmap_atomic(void *kv)
+void kunmap_atomic_high(void *kv)
 {
unsigned long kvaddr = (unsigned long)kv;
 
@@ -87,11 +87,8 @@ void __kunmap_atomic(void *kv)
 
kmap_atomic_idx_pop();
}
-
-   pagefault_enable();
-   preempt_enable();
 }
-EXPORT_SYMBOL(__kunmap_atomic);
+EXPORT_SYMBOL(kunmap_atomic_high);
 
 static noinline pte_t * __init alloc_kmap_pgtable(unsigned long kvaddr)
 {
diff --git a/arch/arm/include/asm/highmem.h b/arch/arm/include/asm/highmem.h
index 8c80bfe18a34..b0d4bd8dc3c1 100644
--- a/arch/arm/include/asm/highmem.h
+++ b/arch/arm/include/asm/highmem.h
@@ -60,7 +60,6 @@ static inline void *kmap_high_get(struct page *page)
  * when CONFIG_HIGHMEM is not set.
  */
 #ifdef CONFIG_HIGHMEM
-extern void __kunmap_atomic(void *kvaddr);
 extern void *kmap_atomic_pfn(unsigned long pfn);
 #endif
 
diff --git a/arch/arm/mm/highmem.c b/arch/arm/mm/highmem.c
index 075fdc235091..ac8394655a6e 100644
--- a/arch/arm/mm/highmem.c
+++ b/arch/arm/mm/highmem.c
@@ -73,7 +73,7 @@ void *kmap_atomic_high(struct page *page)
 }
 EXPORT_SYMBOL(kmap_atomic_high);
 
-void __kunmap_atomic(void *kvaddr)
+void kunmap_atomic_high(void *kvaddr)
 {
unsigned long vaddr = (unsigned long) kvaddr & PAGE_MASK;
int idx, type;
@@ -95,10 +95,8 @@ void __kunmap_atomic(void *kvaddr)
/* this address was obtained through kmap_high_get() */
kunmap_high(pte_page(pkmap_page_table[PKMAP_NR(vaddr)]));
}
-   pagefault_enable();
-   preempt_enable();
 }
-EXPORT_SYMBOL(__kunmap_atomic);
+EXPORT_SYMBOL(kunmap_atomic_high);
 
 void *kmap_atomic_pfn(unsigned long pfn)
 {
diff --git a/arch/csky/include/asm/highmem.h b/arch/csky/include/asm/highmem.h
index 8ceee12f9bc1..263fbddcd0a3 100644
--- a/arch/csky/include/asm/highmem.h
+++ b/arch/csky/include/asm/highmem.h
@@ -32,7 +32,6 @@ extern pte_t *pkmap_page_table;
 
 #define ARCH_HAS_KMAP_FLUSH_TLB
 extern void kmap_flush_tlb(unsigned long addr);
-extern void __kunmap_atomic(void *kvaddr);
 extern void *kmap_atomic_pfn(unsigned long pfn);
 extern struct page *kmap_atomic_to_page(void *ptr);
 
diff --git a/arch/csky/mm/highmem.c b/arch/csky/mm/highmem.c
index 63d74b47eee6..0aafbbbe651c 100644
--- a/arch/csky/mm/highmem.c
+++ b/arch/csky/mm/highmem.c
@@ -39,13 +39,13 @@ void *kmap_atomic_high(struct page *page)
 }
 EXPORT_SYMBOL(kmap_atomic_high);
 
-void __kunmap_atomic(void *kvaddr)
+void kunmap_atomic_high(void *kvaddr)
 {
unsigned long vaddr = (unsigned long) kvaddr & PAGE_MASK;
int idx;
 
if (vaddr < FIXADDR_START)
-   goto out;
+   return;
 
 #ifdef CONFIG_DEBUG_HIGHMEM
idx = KM_TYPE_NR*smp_processor_id() + 

[PATCH V2 04/11] arch/kunmap: Remove duplicate kunmap implementations

2020-05-03 Thread ira . weiny
From: Ira Weiny 

All architectures do exactly the same thing for kunmap(); remove all the
duplicate definitions and lift the call to the core.

This also has the benefit of changing kmap_unmap() on a number of
architectures to be an inline call rather than an actual function.

Reviewed-by: Christoph Hellwig 
Signed-off-by: Ira Weiny 
---
 arch/arc/include/asm/highmem.h| 10 --
 arch/arm/include/asm/highmem.h|  3 ---
 arch/arm/mm/highmem.c |  9 -
 arch/csky/include/asm/highmem.h   |  3 ---
 arch/csky/mm/highmem.c|  9 -
 arch/microblaze/include/asm/highmem.h |  9 -
 arch/mips/include/asm/highmem.h   |  3 ---
 arch/mips/mm/highmem.c|  9 -
 arch/nds32/include/asm/highmem.h  |  3 ---
 arch/nds32/mm/highmem.c   | 10 --
 arch/powerpc/include/asm/highmem.h|  9 -
 arch/sparc/include/asm/highmem.h  | 10 --
 arch/x86/include/asm/highmem.h|  4 
 arch/x86/mm/highmem_32.c  |  9 -
 arch/xtensa/include/asm/highmem.h | 10 --
 include/linux/highmem.h   |  9 +
 16 files changed, 9 insertions(+), 110 deletions(-)

diff --git a/arch/arc/include/asm/highmem.h b/arch/arc/include/asm/highmem.h
index 96eb67c86961..8387a5596a91 100644
--- a/arch/arc/include/asm/highmem.h
+++ b/arch/arc/include/asm/highmem.h
@@ -32,7 +32,6 @@
 
 extern void *kmap_atomic(struct page *page);
 extern void __kunmap_atomic(void *kvaddr);
-extern void kunmap_high(struct page *page);
 
 extern void kmap_init(void);
 
@@ -41,15 +40,6 @@ static inline void flush_cache_kmaps(void)
flush_cache_all();
 }
 
-static inline void kunmap(struct page *page)
-{
-   might_sleep();
-   if (!PageHighMem(page))
-   return;
-   kunmap_high(page);
-}
-
-
 #endif
 
 #endif
diff --git a/arch/arm/include/asm/highmem.h b/arch/arm/include/asm/highmem.h
index c917522541de..736f65283e7b 100644
--- a/arch/arm/include/asm/highmem.h
+++ b/arch/arm/include/asm/highmem.h
@@ -20,8 +20,6 @@
 
 extern pte_t *pkmap_page_table;
 
-extern void kunmap_high(struct page *page);
-
 /*
  * The reason for kmap_high_get() is to ensure that the currently kmap'd
  * page usage count does not decrease to zero while we're using its
@@ -62,7 +60,6 @@ static inline void *kmap_high_get(struct page *page)
  * when CONFIG_HIGHMEM is not set.
  */
 #ifdef CONFIG_HIGHMEM
-extern void kunmap(struct page *page);
 extern void *kmap_atomic(struct page *page);
 extern void __kunmap_atomic(void *kvaddr);
 extern void *kmap_atomic_pfn(unsigned long pfn);
diff --git a/arch/arm/mm/highmem.c b/arch/arm/mm/highmem.c
index e8ba37c36590..c700b32350ee 100644
--- a/arch/arm/mm/highmem.c
+++ b/arch/arm/mm/highmem.c
@@ -31,15 +31,6 @@ static inline pte_t get_fixmap_pte(unsigned long vaddr)
return *ptep;
 }
 
-void kunmap(struct page *page)
-{
-   might_sleep();
-   if (!PageHighMem(page))
-   return;
-   kunmap_high(page);
-}
-EXPORT_SYMBOL(kunmap);
-
 void *kmap_atomic(struct page *page)
 {
unsigned int idx;
diff --git a/arch/csky/include/asm/highmem.h b/arch/csky/include/asm/highmem.h
index 9d0516e38110..be11c5b67122 100644
--- a/arch/csky/include/asm/highmem.h
+++ b/arch/csky/include/asm/highmem.h
@@ -30,11 +30,8 @@ extern pte_t *pkmap_page_table;
 #define PKMAP_NR(virt)  ((virt-PKMAP_BASE) >> PAGE_SHIFT)
 #define PKMAP_ADDR(nr)  (PKMAP_BASE + ((nr) << PAGE_SHIFT))
 
-extern void kunmap_high(struct page *page);
-
 #define ARCH_HAS_KMAP_FLUSH_TLB
 extern void kmap_flush_tlb(unsigned long addr);
-extern void kunmap(struct page *page);
 extern void *kmap_atomic(struct page *page);
 extern void __kunmap_atomic(void *kvaddr);
 extern void *kmap_atomic_pfn(unsigned long pfn);
diff --git a/arch/csky/mm/highmem.c b/arch/csky/mm/highmem.c
index 4a3c273bc8b9..e9952211264b 100644
--- a/arch/csky/mm/highmem.c
+++ b/arch/csky/mm/highmem.c
@@ -21,15 +21,6 @@ EXPORT_SYMBOL(kmap_flush_tlb);
 
 EXPORT_SYMBOL(kmap);
 
-void kunmap(struct page *page)
-{
-   might_sleep();
-   if (!PageHighMem(page))
-   return;
-   kunmap_high(page);
-}
-EXPORT_SYMBOL(kunmap);
-
 void *kmap_atomic(struct page *page)
 {
unsigned long vaddr;
diff --git a/arch/microblaze/include/asm/highmem.h 
b/arch/microblaze/include/asm/highmem.h
index 8c5bfd228bd8..0c94046f2d58 100644
--- a/arch/microblaze/include/asm/highmem.h
+++ b/arch/microblaze/include/asm/highmem.h
@@ -51,18 +51,9 @@ extern pte_t *pkmap_page_table;
 #define PKMAP_NR(virt)  ((virt - PKMAP_BASE) >> PAGE_SHIFT)
 #define PKMAP_ADDR(nr)  (PKMAP_BASE + ((nr) << PAGE_SHIFT))
 
-extern void kunmap_high(struct page *page);
 extern void *kmap_atomic_prot(struct page *page, pgprot_t prot);
 extern void __kunmap_atomic(void *kvaddr);
 
-static inline void kunmap(struct page *page)
-{
-   might_sleep();
-   if (!PageHighMem(page))
-   return;
-   kunmap_high(page);
-}

[PATCH V2 10/11] arch/kmap: Define kmap_atomic_prot() for all arch's

2020-05-03 Thread ira . weiny
From: Ira Weiny 

To support kmap_atomic_prot(), all architectures need to support
protections passed to their kmap_atomic_high() function.  Pass
protections into kmap_atomic_high() and change the name to
kmap_atomic_high_prot() to match.

Then define kmap_atomic_prot() as a core function which calls
kmap_atomic_high_prot() when needed.

Finally, redefine kmap_atomic() as a wrapper of kmap_atomic_prot() with
the default kmap_prot exported by the architectures.

Signed-off-by: Ira Weiny 

---
Changes from V1:
Adjust for bisect-ability
Adjust for removing kunmap_atomic_high
Remove kmap_atomic_high_prot declarations
---
 arch/arc/mm/highmem.c |  6 +++---
 arch/arm/mm/highmem.c |  6 +++---
 arch/csky/mm/highmem.c|  6 +++---
 arch/microblaze/include/asm/highmem.h | 16 
 arch/mips/mm/highmem.c|  6 +++---
 arch/nds32/mm/highmem.c   |  6 +++---
 arch/powerpc/include/asm/highmem.h| 17 -
 arch/sparc/mm/highmem.c   |  6 +++---
 arch/x86/include/asm/highmem.h| 14 --
 arch/xtensa/mm/highmem.c  |  6 +++---
 include/linux/highmem.h   |  7 ---
 11 files changed, 25 insertions(+), 71 deletions(-)

diff --git a/arch/arc/mm/highmem.c b/arch/arc/mm/highmem.c
index 5d3eab4ac0b0..479b0d72d3cf 100644
--- a/arch/arc/mm/highmem.c
+++ b/arch/arc/mm/highmem.c
@@ -49,7 +49,7 @@
 extern pte_t * pkmap_page_table;
 static pte_t * fixmap_page_table;
 
-void *kmap_atomic_high(struct page *page)
+void *kmap_atomic_high_prot(struct page *page, pgprot_t prot)
 {
int idx, cpu_idx;
unsigned long vaddr;
@@ -59,11 +59,11 @@ void *kmap_atomic_high(struct page *page)
vaddr = FIXMAP_ADDR(idx);
 
set_pte_at(_mm, vaddr, fixmap_page_table + idx,
-  mk_pte(page, kmap_prot));
+  mk_pte(page, prot));
 
return (void *)vaddr;
 }
-EXPORT_SYMBOL(kmap_atomic_high);
+EXPORT_SYMBOL(kmap_atomic_high_prot);
 
 void kunmap_atomic_high(void *kv)
 {
diff --git a/arch/arm/mm/highmem.c b/arch/arm/mm/highmem.c
index ac8394655a6e..e013f6b81328 100644
--- a/arch/arm/mm/highmem.c
+++ b/arch/arm/mm/highmem.c
@@ -31,7 +31,7 @@ static inline pte_t get_fixmap_pte(unsigned long vaddr)
return *ptep;
 }
 
-void *kmap_atomic_high(struct page *page)
+void *kmap_atomic_high_prot(struct page *page, pgprot_t prot)
 {
unsigned int idx;
unsigned long vaddr;
@@ -67,11 +67,11 @@ void *kmap_atomic_high(struct page *page)
 * in place, so the contained TLB flush ensures the TLB is updated
 * with the new mapping.
 */
-   set_fixmap_pte(idx, mk_pte(page, kmap_prot));
+   set_fixmap_pte(idx, mk_pte(page, prot));
 
return (void *)vaddr;
 }
-EXPORT_SYMBOL(kmap_atomic_high);
+EXPORT_SYMBOL(kmap_atomic_high_prot);
 
 void kunmap_atomic_high(void *kvaddr)
 {
diff --git a/arch/csky/mm/highmem.c b/arch/csky/mm/highmem.c
index f4311669b5bb..3ae5c8cd7619 100644
--- a/arch/csky/mm/highmem.c
+++ b/arch/csky/mm/highmem.c
@@ -21,7 +21,7 @@ EXPORT_SYMBOL(kmap_flush_tlb);
 
 EXPORT_SYMBOL(kmap);
 
-void *kmap_atomic_high(struct page *page)
+void *kmap_atomic_high_prot(struct page *page, pgprot_t prot)
 {
unsigned long vaddr;
int idx, type;
@@ -32,12 +32,12 @@ void *kmap_atomic_high(struct page *page)
 #ifdef CONFIG_DEBUG_HIGHMEM
BUG_ON(!pte_none(*(kmap_pte - idx)));
 #endif
-   set_pte(kmap_pte-idx, mk_pte(page, kmap_prot));
+   set_pte(kmap_pte-idx, mk_pte(page, prot));
flush_tlb_one((unsigned long)vaddr);
 
return (void *)vaddr;
 }
-EXPORT_SYMBOL(kmap_atomic_high);
+EXPORT_SYMBOL(kmap_atomic_high_prot);
 
 void kunmap_atomic_high(void *kvaddr)
 {
diff --git a/arch/microblaze/include/asm/highmem.h 
b/arch/microblaze/include/asm/highmem.h
index 033ac5b5c2da..d7c55cfd27bd 100644
--- a/arch/microblaze/include/asm/highmem.h
+++ b/arch/microblaze/include/asm/highmem.h
@@ -51,22 +51,6 @@ extern pte_t *pkmap_page_table;
 #define PKMAP_NR(virt)  ((virt - PKMAP_BASE) >> PAGE_SHIFT)
 #define PKMAP_ADDR(nr)  (PKMAP_BASE + ((nr) << PAGE_SHIFT))
 
-extern void *kmap_atomic_high_prot(struct page *page, pgprot_t prot);
-void *kmap_atomic_prot(struct page *page, pgprot_t prot)
-{
-   preempt_disable();
-   pagefault_disable();
-   if (!PageHighMem(page))
-   return page_address(page);
-
-   return kmap_atomic_high_prot(page, prot);
-}
-
-static inline void *kmap_atomic_high(struct page *page)
-{
-   return kmap_atomic_high_prot(page, kmap_prot);
-}
-
 #define flush_cache_kmaps(){ flush_icache(); flush_dcache(); }
 
 #endif /* __KERNEL__ */
diff --git a/arch/mips/mm/highmem.c b/arch/mips/mm/highmem.c
index 87023bd1a33c..37e244cdb14e 100644
--- a/arch/mips/mm/highmem.c
+++ b/arch/mips/mm/highmem.c
@@ -18,7 +18,7 @@ void kmap_flush_tlb(unsigned long addr)
 }
 EXPORT_SYMBOL(kmap_flush_tlb);
 
-void 

[PATCH V2 03/11] arch/kmap: Remove redundant arch specific kmaps

2020-05-03 Thread ira . weiny
From: Ira Weiny 

The kmap code for all the architectures is almost 100% identical.

Lift the common code to the core.  Use ARCH_HAS_KMAP_FLUSH_TLB to
indicate if an arch defines kmap_flush_tlb() and call if if needed.

This also has the benefit of changing kmap() on a number of
architectures to be an inline call rather than an actual function.

Reviewed-by: Christoph Hellwig 
Signed-off-by: Ira Weiny 
---
 arch/arc/include/asm/highmem.h|  2 --
 arch/arc/mm/highmem.c | 10 --
 arch/arm/include/asm/highmem.h|  2 --
 arch/arm/mm/highmem.c |  9 -
 arch/csky/include/asm/highmem.h   |  4 ++--
 arch/csky/mm/highmem.c| 14 --
 arch/microblaze/include/asm/highmem.h |  9 -
 arch/mips/include/asm/highmem.h   |  4 ++--
 arch/mips/mm/highmem.c| 14 +++---
 arch/nds32/include/asm/highmem.h  |  2 --
 arch/nds32/mm/highmem.c   | 12 
 arch/powerpc/include/asm/highmem.h|  9 -
 arch/sparc/include/asm/highmem.h  |  9 -
 arch/x86/include/asm/highmem.h|  2 --
 arch/x86/mm/highmem_32.c  |  9 -
 arch/xtensa/include/asm/highmem.h |  9 -
 include/linux/highmem.h   | 18 ++
 17 files changed, 29 insertions(+), 109 deletions(-)

diff --git a/arch/arc/include/asm/highmem.h b/arch/arc/include/asm/highmem.h
index 042e92921c4c..96eb67c86961 100644
--- a/arch/arc/include/asm/highmem.h
+++ b/arch/arc/include/asm/highmem.h
@@ -30,8 +30,6 @@
 
 #include 
 
-extern void *kmap(struct page *page);
-extern void *kmap_high(struct page *page);
 extern void *kmap_atomic(struct page *page);
 extern void __kunmap_atomic(void *kvaddr);
 extern void kunmap_high(struct page *page);
diff --git a/arch/arc/mm/highmem.c b/arch/arc/mm/highmem.c
index 39ef7b9a3aa9..4db13a6b9f3b 100644
--- a/arch/arc/mm/highmem.c
+++ b/arch/arc/mm/highmem.c
@@ -49,16 +49,6 @@
 extern pte_t * pkmap_page_table;
 static pte_t * fixmap_page_table;
 
-void *kmap(struct page *page)
-{
-   might_sleep();
-   if (!PageHighMem(page))
-   return page_address(page);
-
-   return kmap_high(page);
-}
-EXPORT_SYMBOL(kmap);
-
 void *kmap_atomic(struct page *page)
 {
int idx, cpu_idx;
diff --git a/arch/arm/include/asm/highmem.h b/arch/arm/include/asm/highmem.h
index eb4e4207cd3c..c917522541de 100644
--- a/arch/arm/include/asm/highmem.h
+++ b/arch/arm/include/asm/highmem.h
@@ -20,7 +20,6 @@
 
 extern pte_t *pkmap_page_table;
 
-extern void *kmap_high(struct page *page);
 extern void kunmap_high(struct page *page);
 
 /*
@@ -63,7 +62,6 @@ static inline void *kmap_high_get(struct page *page)
  * when CONFIG_HIGHMEM is not set.
  */
 #ifdef CONFIG_HIGHMEM
-extern void *kmap(struct page *page);
 extern void kunmap(struct page *page);
 extern void *kmap_atomic(struct page *page);
 extern void __kunmap_atomic(void *kvaddr);
diff --git a/arch/arm/mm/highmem.c b/arch/arm/mm/highmem.c
index cc6eb79ef20c..e8ba37c36590 100644
--- a/arch/arm/mm/highmem.c
+++ b/arch/arm/mm/highmem.c
@@ -31,15 +31,6 @@ static inline pte_t get_fixmap_pte(unsigned long vaddr)
return *ptep;
 }
 
-void *kmap(struct page *page)
-{
-   might_sleep();
-   if (!PageHighMem(page))
-   return page_address(page);
-   return kmap_high(page);
-}
-EXPORT_SYMBOL(kmap);
-
 void kunmap(struct page *page)
 {
might_sleep();
diff --git a/arch/csky/include/asm/highmem.h b/arch/csky/include/asm/highmem.h
index a345a2f2c22e..9d0516e38110 100644
--- a/arch/csky/include/asm/highmem.h
+++ b/arch/csky/include/asm/highmem.h
@@ -30,10 +30,10 @@ extern pte_t *pkmap_page_table;
 #define PKMAP_NR(virt)  ((virt-PKMAP_BASE) >> PAGE_SHIFT)
 #define PKMAP_ADDR(nr)  (PKMAP_BASE + ((nr) << PAGE_SHIFT))
 
-extern void *kmap_high(struct page *page);
 extern void kunmap_high(struct page *page);
 
-extern void *kmap(struct page *page);
+#define ARCH_HAS_KMAP_FLUSH_TLB
+extern void kmap_flush_tlb(unsigned long addr);
 extern void kunmap(struct page *page);
 extern void *kmap_atomic(struct page *page);
 extern void __kunmap_atomic(void *kvaddr);
diff --git a/arch/csky/mm/highmem.c b/arch/csky/mm/highmem.c
index 690d678649d1..4a3c273bc8b9 100644
--- a/arch/csky/mm/highmem.c
+++ b/arch/csky/mm/highmem.c
@@ -13,18 +13,12 @@ static pte_t *kmap_pte;
 
 unsigned long highstart_pfn, highend_pfn;
 
-void *kmap(struct page *page)
+void kmap_flush_tlb(unsigned long addr)
 {
-   void *addr;
-
-   might_sleep();
-   if (!PageHighMem(page))
-   return page_address(page);
-   addr = kmap_high(page);
-   flush_tlb_one((unsigned long)addr);
-
-   return addr;
+   flush_tlb_one(addr);
 }
+EXPORT_SYMBOL(kmap_flush_tlb);
+
 EXPORT_SYMBOL(kmap);
 
 void kunmap(struct page *page)
diff --git a/arch/microblaze/include/asm/highmem.h 
b/arch/microblaze/include/asm/highmem.h
index 99ced7278b5c..8c5bfd228bd8 100644
--- 

[PATCH V2 09/11] arch/kmap: Don't hard code kmap_prot values

2020-05-03 Thread ira . weiny
From: Ira Weiny 

To support kmap_atomic_prot() on all architectures each arch must
support protections passed in to them.

Change csky, mips, nds32 and xtensa to use their global constant
kmap_prot rather than a hard coded value which was equal.

Reviewed-by: Christoph Hellwig 
Signed-off-by: Ira Weiny 

---
changes from V1:
Mention that kmap_prot is a constant in commit message
---
 arch/csky/mm/highmem.c   | 2 +-
 arch/mips/mm/highmem.c   | 2 +-
 arch/nds32/mm/highmem.c  | 2 +-
 arch/xtensa/mm/highmem.c | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/csky/mm/highmem.c b/arch/csky/mm/highmem.c
index 0aafbbbe651c..f4311669b5bb 100644
--- a/arch/csky/mm/highmem.c
+++ b/arch/csky/mm/highmem.c
@@ -32,7 +32,7 @@ void *kmap_atomic_high(struct page *page)
 #ifdef CONFIG_DEBUG_HIGHMEM
BUG_ON(!pte_none(*(kmap_pte - idx)));
 #endif
-   set_pte(kmap_pte-idx, mk_pte(page, PAGE_KERNEL));
+   set_pte(kmap_pte-idx, mk_pte(page, kmap_prot));
flush_tlb_one((unsigned long)vaddr);
 
return (void *)vaddr;
diff --git a/arch/mips/mm/highmem.c b/arch/mips/mm/highmem.c
index 155fbb107b35..87023bd1a33c 100644
--- a/arch/mips/mm/highmem.c
+++ b/arch/mips/mm/highmem.c
@@ -29,7 +29,7 @@ void *kmap_atomic_high(struct page *page)
 #ifdef CONFIG_DEBUG_HIGHMEM
BUG_ON(!pte_none(*(kmap_pte - idx)));
 #endif
-   set_pte(kmap_pte-idx, mk_pte(page, PAGE_KERNEL));
+   set_pte(kmap_pte-idx, mk_pte(page, kmap_prot));
local_flush_tlb_one((unsigned long)vaddr);
 
return (void*) vaddr;
diff --git a/arch/nds32/mm/highmem.c b/arch/nds32/mm/highmem.c
index f6e6915c0d31..809f8c830f06 100644
--- a/arch/nds32/mm/highmem.c
+++ b/arch/nds32/mm/highmem.c
@@ -21,7 +21,7 @@ void *kmap_atomic_high(struct page *page)
 
idx = type + KM_TYPE_NR * smp_processor_id();
vaddr = __fix_to_virt(FIX_KMAP_BEGIN + idx);
-   pte = (page_to_pfn(page) << PAGE_SHIFT) | (PAGE_KERNEL);
+   pte = (page_to_pfn(page) << PAGE_SHIFT) | (kmap_prot);
ptep = pte_offset_kernel(pmd_off_k(vaddr), vaddr);
set_pte(ptep, pte);
 
diff --git a/arch/xtensa/mm/highmem.c b/arch/xtensa/mm/highmem.c
index 4de323e43682..50168b09510a 100644
--- a/arch/xtensa/mm/highmem.c
+++ b/arch/xtensa/mm/highmem.c
@@ -48,7 +48,7 @@ void *kmap_atomic_high(struct page *page)
 #ifdef CONFIG_DEBUG_HIGHMEM
BUG_ON(!pte_none(*(kmap_pte + idx)));
 #endif
-   set_pte(kmap_pte + idx, mk_pte(page, PAGE_KERNEL_EXEC));
+   set_pte(kmap_pte + idx, mk_pte(page, kmap_prot));
 
return (void *)vaddr;
 }
-- 
2.25.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH V2 11/11] drm: Remove drm specific kmap_atomic code

2020-05-03 Thread ira . weiny
From: Ira Weiny 

kmap_atomic_prot() is now exported by all architectures.  Use this
function rather than open coding a driver specific kmap_atomic.

Reviewed-by: Christian König 
Reviewed-by: Christoph Hellwig 
Signed-off-by: Ira Weiny 
---
 drivers/gpu/drm/ttm/ttm_bo_util.c| 56 ++--
 drivers/gpu/drm/vmwgfx/vmwgfx_blit.c | 16 
 include/drm/ttm/ttm_bo_api.h |  4 --
 3 files changed, 12 insertions(+), 64 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c 
b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 52d2b71f1588..f09b096ba4fd 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -257,54 +257,6 @@ static int ttm_copy_io_page(void *dst, void *src, unsigned 
long page)
return 0;
 }
 
-#ifdef CONFIG_X86
-#define __ttm_kmap_atomic_prot(__page, __prot) kmap_atomic_prot(__page, __prot)
-#define __ttm_kunmap_atomic(__addr) kunmap_atomic(__addr)
-#else
-#define __ttm_kmap_atomic_prot(__page, __prot) vmap(&__page, 1, 0,  __prot)
-#define __ttm_kunmap_atomic(__addr) vunmap(__addr)
-#endif
-
-
-/**
- * ttm_kmap_atomic_prot - Efficient kernel map of a single page with
- * specified page protection.
- *
- * @page: The page to map.
- * @prot: The page protection.
- *
- * This function maps a TTM page using the kmap_atomic api if available,
- * otherwise falls back to vmap. The user must make sure that the
- * specified page does not have an aliased mapping with a different caching
- * policy unless the architecture explicitly allows it. Also mapping and
- * unmapping using this api must be correctly nested. Unmapping should
- * occur in the reverse order of mapping.
- */
-void *ttm_kmap_atomic_prot(struct page *page, pgprot_t prot)
-{
-   if (pgprot_val(prot) == pgprot_val(PAGE_KERNEL))
-   return kmap_atomic(page);
-   else
-   return __ttm_kmap_atomic_prot(page, prot);
-}
-EXPORT_SYMBOL(ttm_kmap_atomic_prot);
-
-/**
- * ttm_kunmap_atomic_prot - Unmap a page that was mapped using
- * ttm_kmap_atomic_prot.
- *
- * @addr: The virtual address from the map.
- * @prot: The page protection.
- */
-void ttm_kunmap_atomic_prot(void *addr, pgprot_t prot)
-{
-   if (pgprot_val(prot) == pgprot_val(PAGE_KERNEL))
-   kunmap_atomic(addr);
-   else
-   __ttm_kunmap_atomic(addr);
-}
-EXPORT_SYMBOL(ttm_kunmap_atomic_prot);
-
 static int ttm_copy_io_ttm_page(struct ttm_tt *ttm, void *src,
unsigned long page,
pgprot_t prot)
@@ -316,13 +268,13 @@ static int ttm_copy_io_ttm_page(struct ttm_tt *ttm, void 
*src,
return -ENOMEM;
 
src = (void *)((unsigned long)src + (page << PAGE_SHIFT));
-   dst = ttm_kmap_atomic_prot(d, prot);
+   dst = kmap_atomic_prot(d, prot);
if (!dst)
return -ENOMEM;
 
memcpy_fromio(dst, src, PAGE_SIZE);
 
-   ttm_kunmap_atomic_prot(dst, prot);
+   kunmap_atomic(dst);
 
return 0;
 }
@@ -338,13 +290,13 @@ static int ttm_copy_ttm_io_page(struct ttm_tt *ttm, void 
*dst,
return -ENOMEM;
 
dst = (void *)((unsigned long)dst + (page << PAGE_SHIFT));
-   src = ttm_kmap_atomic_prot(s, prot);
+   src = kmap_atomic_prot(s, prot);
if (!src)
return -ENOMEM;
 
memcpy_toio(dst, src, PAGE_SIZE);
 
-   ttm_kunmap_atomic_prot(src, prot);
+   kunmap_atomic(src);
 
return 0;
 }
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_blit.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_blit.c
index bb46ca0c458f..94d456a1d1a9 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_blit.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_blit.c
@@ -374,12 +374,12 @@ static int vmw_bo_cpu_blit_line(struct 
vmw_bo_blit_line_data *d,
copy_size = min_t(u32, copy_size, PAGE_SIZE - src_page_offset);
 
if (unmap_src) {
-   ttm_kunmap_atomic_prot(d->src_addr, d->src_prot);
+   kunmap_atomic(d->src_addr);
d->src_addr = NULL;
}
 
if (unmap_dst) {
-   ttm_kunmap_atomic_prot(d->dst_addr, d->dst_prot);
+   kunmap_atomic(d->dst_addr);
d->dst_addr = NULL;
}
 
@@ -388,8 +388,8 @@ static int vmw_bo_cpu_blit_line(struct 
vmw_bo_blit_line_data *d,
return -EINVAL;
 
d->dst_addr =
-   ttm_kmap_atomic_prot(d->dst_pages[dst_page],
-d->dst_prot);
+   kmap_atomic_prot(d->dst_pages[dst_page],
+d->dst_prot);
if (!d->dst_addr)
return -ENOMEM;
 
@@ -401,8 +401,8 @@ static int vmw_bo_cpu_blit_line(struct 
vmw_bo_blit_line_data *d,
return -EINVAL;
 
   

[PATCH V2 02/11] arch/xtensa: Move kmap build bug out of the way

2020-05-03 Thread ira . weiny
From: Ira Weiny 

Move the kmap() build bug to kmap_init() to facilitate patches to lift
kmap() to the core.

Reviewed-by: Christoph Hellwig 
Signed-off-by: Ira Weiny 

---
Changes from V1:
combine code onto 1 line.
---
 arch/xtensa/include/asm/highmem.h | 5 -
 arch/xtensa/mm/highmem.c  | 4 
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/arch/xtensa/include/asm/highmem.h 
b/arch/xtensa/include/asm/highmem.h
index 413848cc1e56..a9587c85be85 100644
--- a/arch/xtensa/include/asm/highmem.h
+++ b/arch/xtensa/include/asm/highmem.h
@@ -68,11 +68,6 @@ void kunmap_high(struct page *page);
 
 static inline void *kmap(struct page *page)
 {
-   /* Check if this memory layout is broken because PKMAP overlaps
-* page table.
-*/
-   BUILD_BUG_ON(PKMAP_BASE <
-TLBTEMP_BASE_1 + TLBTEMP_SIZE);
might_sleep();
if (!PageHighMem(page))
return page_address(page);
diff --git a/arch/xtensa/mm/highmem.c b/arch/xtensa/mm/highmem.c
index 184ceadccc1a..da734a2ed641 100644
--- a/arch/xtensa/mm/highmem.c
+++ b/arch/xtensa/mm/highmem.c
@@ -88,6 +88,10 @@ void __init kmap_init(void)
 {
unsigned long kmap_vstart;
 
+   /* Check if this memory layout is broken because PKMAP overlaps
+* page table.
+*/
+   BUILD_BUG_ON(PKMAP_BASE < TLBTEMP_BASE_1 + TLBTEMP_SIZE);
/* cache the first kmap pte */
kmap_vstart = __fix_to_virt(FIX_KMAP_BEGIN);
kmap_pte = kmap_get_fixmap_pte(kmap_vstart);
-- 
2.25.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH V2 08/11] arch/kmap: Ensure kmap_prot visibility

2020-05-03 Thread ira . weiny
From: Ira Weiny 

We want to support kmap_atomic_prot() on all architectures and it makes
sense to define kmap_atomic() to use the default kmap_prot.

So we ensure all arch's have a globally available kmap_prot either as a
define or exported symbol.

Signed-off-by: Ira Weiny 
---
 arch/microblaze/include/asm/highmem.h | 2 +-
 arch/microblaze/mm/init.c | 3 ---
 arch/powerpc/include/asm/highmem.h| 2 +-
 arch/powerpc/mm/mem.c | 3 ---
 arch/sparc/mm/highmem.c   | 1 +
 5 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/arch/microblaze/include/asm/highmem.h 
b/arch/microblaze/include/asm/highmem.h
index 1b8a3c5102bd..033ac5b5c2da 100644
--- a/arch/microblaze/include/asm/highmem.h
+++ b/arch/microblaze/include/asm/highmem.h
@@ -25,8 +25,8 @@
 #include 
 #include 
 
+#define kmap_prot  PAGE_KERNEL
 extern pte_t *kmap_pte;
-extern pgprot_t kmap_prot;
 extern pte_t *pkmap_page_table;
 
 /*
diff --git a/arch/microblaze/mm/init.c b/arch/microblaze/mm/init.c
index 1ffbfa96b9b8..a467686c13af 100644
--- a/arch/microblaze/mm/init.c
+++ b/arch/microblaze/mm/init.c
@@ -49,8 +49,6 @@ unsigned long lowmem_size;
 #ifdef CONFIG_HIGHMEM
 pte_t *kmap_pte;
 EXPORT_SYMBOL(kmap_pte);
-pgprot_t kmap_prot;
-EXPORT_SYMBOL(kmap_prot);
 
 static inline pte_t *virt_to_kpte(unsigned long vaddr)
 {
@@ -68,7 +66,6 @@ static void __init highmem_init(void)
pkmap_page_table = virt_to_kpte(PKMAP_BASE);
 
kmap_pte = virt_to_kpte(__fix_to_virt(FIX_KMAP_BEGIN));
-   kmap_prot = PAGE_KERNEL;
 }
 
 static void highmem_setup(void)
diff --git a/arch/powerpc/include/asm/highmem.h 
b/arch/powerpc/include/asm/highmem.h
index 373a470df205..ee5de974c5ef 100644
--- a/arch/powerpc/include/asm/highmem.h
+++ b/arch/powerpc/include/asm/highmem.h
@@ -29,8 +29,8 @@
 #include 
 #include 
 
+#define kmap_prot  PAGE_KERNEL
 extern pte_t *kmap_pte;
-extern pgprot_t kmap_prot;
 extern pte_t *pkmap_page_table;
 
 /*
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index 041ed7cfd341..3f642b058731 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -64,8 +64,6 @@ bool init_mem_is_free;
 #ifdef CONFIG_HIGHMEM
 pte_t *kmap_pte;
 EXPORT_SYMBOL(kmap_pte);
-pgprot_t kmap_prot;
-EXPORT_SYMBOL(kmap_prot);
 #endif
 
 pgprot_t phys_mem_access_prot(struct file *file, unsigned long pfn,
@@ -245,7 +243,6 @@ void __init paging_init(void)
pkmap_page_table = virt_to_kpte(PKMAP_BASE);
 
kmap_pte = virt_to_kpte(__fix_to_virt(FIX_KMAP_BEGIN));
-   kmap_prot = PAGE_KERNEL;
 #endif /* CONFIG_HIGHMEM */
 
printk(KERN_DEBUG "Top of RAM: 0x%llx, Total RAM: 0x%llx\n",
diff --git a/arch/sparc/mm/highmem.c b/arch/sparc/mm/highmem.c
index 469786bc430f..9f06d75e88e1 100644
--- a/arch/sparc/mm/highmem.c
+++ b/arch/sparc/mm/highmem.c
@@ -33,6 +33,7 @@
 #include 
 
 pgprot_t kmap_prot;
+EXPORT_SYMBOL(kmap_prot);
 
 static pte_t *kmap_pte;
 
-- 
2.25.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] drm/msm: Fix undefined "rd_full" link error

2020-05-03 Thread Stephen Rothwell
Hi all,

On Thu, 30 Apr 2020 12:24:27 -0700 Bjorn Andersson  
wrote:
>
> rd_full should be defined outside the CONFIG_DEBUG_FS region, in order
> to be able to link the msm driver even when CONFIG_DEBUG_FS is disabled.
> 
> Fixes: e515af8d4a6f ("drm/msm: devcoredump should dump MSM_SUBMIT_BO_DUMP 
> buffers")
> Reported-by: Stephen Rothwell 
> Signed-off-by: Bjorn Andersson 
> ---
>  drivers/gpu/drm/msm/msm_rd.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/msm_rd.c b/drivers/gpu/drm/msm/msm_rd.c
> index 732f65df5c4f..fea30e7aa9e8 100644
> --- a/drivers/gpu/drm/msm/msm_rd.c
> +++ b/drivers/gpu/drm/msm/msm_rd.c
> @@ -29,8 +29,6 @@
>   * or shader programs (if not emitted inline in cmdstream).
>   */
>  
> -#ifdef CONFIG_DEBUG_FS
> -
>  #include 
>  #include 
>  #include 
> @@ -47,6 +45,8 @@ bool rd_full = false;
>  MODULE_PARM_DESC(rd_full, "If true, $debugfs/.../rd will snapshot all buffer 
> contents");
>  module_param_named(rd_full, rd_full, bool, 0600);
>  
> +#ifdef CONFIG_DEBUG_FS
> +
>  enum rd_sect_type {
>   RD_NONE,
>   RD_TEST,   /* ascii text */
> -- 
> 2.24.0
> 

Added to my fixes tree form today.  I will remove it when it is merged
upstream through someone else's tree.

-- 
Cheers,
Stephen Rothwell


pgp2Q1aqAWgJM.pgp
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] drm/i915: check to see if SIMD registers are available before using SIMD

2020-05-03 Thread Chris Wilson
Quoting Jason A. Donenfeld (2020-04-30 23:10:16)
> Sometimes it's not okay to use SIMD registers, the conditions for which
> have changed subtly from kernel release to kernel release. Usually the
> pattern is to check for may_use_simd() and then fallback to using
> something slower in the unlikely case SIMD registers aren't available.
> So, this patch fixes up i915's accelerated memcpy routines to fallback
> to boring memcpy if may_use_simd() is false.
> 
> Cc: sta...@vger.kernel.org

The same argument as on the previous submission is that we return to the
caller if we can't use movntqda as their fallback path should be faster
than uncached memcpy.
-Chris
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] drm/i915: check to see if SIMD registers are available before using SIMD

2020-05-03 Thread Chris Wilson
Quoting Christoph Hellwig (2020-05-01 19:07:31)
> On Thu, Apr 30, 2020 at 04:10:16PM -0600, Jason A. Donenfeld wrote:
> > Sometimes it's not okay to use SIMD registers, the conditions for which
> > have changed subtly from kernel release to kernel release. Usually the
> > pattern is to check for may_use_simd() and then fallback to using
> > something slower in the unlikely case SIMD registers aren't available.
> > So, this patch fixes up i915's accelerated memcpy routines to fallback
> > to boring memcpy if may_use_simd() is false.
> 
> Err, why does i915 implements its own uncached memcpy instead of relying
> on core functionality to start with?

What is this core functionality that provides movntqda?
-Chris
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] video/fbdev/matroxfb: Remove dead code

2020-05-03 Thread Sam Ravnborg
Hi Souptick

On Sat, May 02, 2020 at 03:28:11AM +0530, Souptick Joarder wrote:
> These are dead code since 3.15. If there is no plan to use it further
> it can be removed forever.
Could you explain why you conclude this is dead code sine 3.15 -
and maybe point to the commit that made it dead.
I failed to look it up.
And I would assume I did not have to look it up, but that you have
provided enough background to evaluate the patch.

Sometimes dead code are kept becasue it documents something etc.
So it is not always a simple removal.

Sam

> 
> Signed-off-by: Souptick Joarder 
> ---
>  drivers/video/fbdev/matrox/matroxfb_DAC1064.c | 11 ---
>  1 file changed, 11 deletions(-)
> 
> diff --git a/drivers/video/fbdev/matrox/matroxfb_DAC1064.c 
> b/drivers/video/fbdev/matrox/matroxfb_DAC1064.c
> index 765e805..9c2a2c0 100644
> --- a/drivers/video/fbdev/matrox/matroxfb_DAC1064.c
> +++ b/drivers/video/fbdev/matrox/matroxfb_DAC1064.c
> @@ -603,9 +603,6 @@ static void MGA1064_ramdac_init(struct matrox_fb_info 
> *minfo)
>  /* BIOS environ */
>  static int x7AF4 = 0x10; /* flags, maybe 0x10 = SDRAM, 0x00 = SGRAM??? */
>   /* G100 wants 0x10, G200 SGRAM does not care... 
> */
> -#if 0
> -static int def50 = 0;/* reg50, & 0x0F, & 0x3000 (only 0x, 
> 0x1000, 0x2000 (0x3000 disallowed and treated as 0) */
> -#endif
>  
>  static void MGAG100_progPixClock(const struct matrox_fb_info *minfo, int 
> flags,
>int m, int n, int p)
> @@ -843,9 +840,6 @@ static int MGAG100_preinit(struct matrox_fb_info *minfo)
>   struct matrox_hw_state *hw = >hw;
>  
>  u_int32_t reg50;
> -#if 0
> - u_int32_t q;
> -#endif
>  
>   DBG(__func__)
>  
> @@ -927,11 +921,6 @@ static int MGAG100_preinit(struct matrox_fb_info *minfo)
>   mga_writeb(minfo->video.vbase, 0x, 0xAA);
>   mga_writeb(minfo->video.vbase, 0x0800, 0x55);
>   mga_writeb(minfo->video.vbase, 0x4000, 0x55);
> -#if 0
> - if (mga_readb(minfo->video.vbase, 0x) != 0xAA) {
> - hw->MXoptionReg &= ~0x1000;
> - }
> -#endif
>   hw->MXoptionReg |= 0x00078020;
>   } else if (minfo->devflags.accelerator == FB_ACCEL_MATROX_MGAG200) {
>   pci_read_config_dword(minfo->pcidev, PCI_OPTION2_REG, );
> -- 
> 1.9.1
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 16/17] drm/mgag200: Convert to simple KMS helper

2020-05-03 Thread Sam Ravnborg
Hi Thomas.

Introducing drm_simple_display_pipe to hold some of the data,
and then a later patch to change over would have helped the readability
I think.

Anyway - deleted unused code belongs to another patch.
Other than tthose details I did not spot anything, but then I did not
read this patch as carefully as some of the others.

With the removed code migrated to another aptch, and the missing code
explained the patch is:
Acked-by: Sam Ravnborg 

On Wed, Apr 29, 2020 at 04:32:37PM +0200, Thomas Zimmermann wrote:
> The mgag200 supports a single pipeline with only a primary plane. It can
> be converted to simple KMS helpers. This also adds support for atomic
> modesetting. Wayland compositors, which use pageflip ioctls, can now be
> used with mgag200.
> 
> Signed-off-by: Thomas Zimmermann 
> ---
>  drivers/gpu/drm/mgag200/mgag200_drv.c  |   2 +-
>  drivers/gpu/drm/mgag200/mgag200_drv.h  |   4 +-
>  drivers/gpu/drm/mgag200/mgag200_mode.c | 396 +++--
>  3 files changed, 171 insertions(+), 231 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c 
> b/drivers/gpu/drm/mgag200/mgag200_drv.c
> index 3298b7ef18b03..b1272165621ed 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_drv.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c
> @@ -140,7 +140,7 @@ int mgag200_driver_dumb_create(struct drm_file *file,
>  }
>  
>  static struct drm_driver driver = {
> - .driver_features = DRIVER_GEM | DRIVER_MODESET,
> + .driver_features = DRIVER_ATOMIC | DRIVER_GEM | DRIVER_MODESET,
>   .fops = _driver_fops,
>   .name = DRIVER_NAME,
>   .desc = DRIVER_DESC,
> diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h 
> b/drivers/gpu/drm/mgag200/mgag200_drv.h
> index b10da90e0f35a..2e407508714c8 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_drv.h
> +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
> @@ -19,6 +19,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include "mgag200_reg.h"
>  
> @@ -107,6 +108,7 @@
>  
>  #define to_mga_crtc(x) container_of(x, struct mga_crtc, base)
>  #define to_mga_connector(x) container_of(x, struct mga_connector, base)
> +#define to_mga_device(x) (dev->dev_private)
Upclassing please.

>  
>  struct mga_crtc {
>   struct drm_crtc base;
> @@ -176,7 +178,7 @@ struct mga_device {
>   u32 unique_rev_id;
>  
>   struct mga_connector connector;
> - struct drm_encoder encoder;
> + struct drm_simple_display_pipe display_pipe;
>  };
>  
>  static inline enum mga_type
> diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c 
> b/drivers/gpu/drm/mgag200/mgag200_mode.c
> index 884fc668a6dae..d9b4055e38982 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_mode.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
> @@ -11,10 +11,13 @@
>  #include 
>  #include 
>  
> +#include 
> +#include 
>  #include 
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  
> @@ -30,13 +33,18 @@ static void mga_crtc_load_lut(struct drm_crtc *crtc)
>  {
>   struct drm_device *dev = crtc->dev;
>   struct mga_device *mdev = dev->dev_private;
> - struct drm_framebuffer *fb = crtc->primary->fb;
> + struct drm_framebuffer *fb;
>   u16 *r_ptr, *g_ptr, *b_ptr;
>   int i;
>  
>   if (!crtc->enabled)
>   return;
>  
> + if (!mdev->display_pipe.plane.state)
> + return;
> +
> + fb = mdev->display_pipe.plane.state->fb;
> +
>   r_ptr = crtc->gamma_store;
>   g_ptr = r_ptr + crtc->gamma_size;
>   b_ptr = g_ptr + crtc->gamma_size;
> @@ -845,56 +853,6 @@ static void mgag200_set_startadd(struct mga_device *mdev,
>   WREG_ECRT(0x00, crtcext0);
>  }
>  
> -static int mga_crtc_do_set_base(struct mga_device *mdev,
> - const struct drm_framebuffer *fb,
> - const struct drm_framebuffer *old_fb)
> -{
> - struct drm_gem_vram_object *gbo;
> - int ret;
> - s64 gpu_addr;
> -
> - if (old_fb) {
> - gbo = drm_gem_vram_of_gem(old_fb->obj[0]);
> - drm_gem_vram_unpin(gbo);
> - }
> -
> - gbo = drm_gem_vram_of_gem(fb->obj[0]);
> -
> - ret = drm_gem_vram_pin(gbo, DRM_GEM_VRAM_PL_FLAG_VRAM);
> - if (ret)
> - return ret;
> - gpu_addr = drm_gem_vram_offset(gbo);
> - if (gpu_addr < 0) {
> - ret = (int)gpu_addr;
> - goto err_drm_gem_vram_unpin;
> - }
> -
> - mgag200_set_startadd(mdev, (unsigned long)gpu_addr);
> -
> - return 0;
> -
> -err_drm_gem_vram_unpin:
> - drm_gem_vram_unpin(gbo);
> - return ret;
> -}
> -
> -static int mga_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
> -   struct drm_framebuffer *old_fb)
> -{
> - struct drm_device *dev = crtc->dev;
> - struct mga_device *mdev = dev->dev_private;
> - struct drm_framebuffer *fb = crtc->primary->fb;
> - unsigned int count;
> -
> - while (RREG8(0x1fda) & 0x08) { }
> - while (!(RREG8(0x1fda) & 0x08)) { }
> -
> - 

Re: [PATCH 14/17] drm/mgag200: Move register initialization into separate function

2020-05-03 Thread Sam Ravnborg
On Wed, Apr 29, 2020 at 04:32:35PM +0200, Thomas Zimmermann wrote:
> Registers are initialized with constants. This is now done in
> mgag200_init_regs(), mgag200_set_dac_regs() and mgag200_set_pci_regs().
> Later patches should move these calls from mode setting to device
> initialization.
> 
> Signed-off-by: Thomas Zimmermann 
Acked-by: Sam Ravnborg 
> ---
>  drivers/gpu/drm/mgag200/mgag200_mode.c | 262 ++---
>  1 file changed, 148 insertions(+), 114 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c 
> b/drivers/gpu/drm/mgag200/mgag200_mode.c
> index a04404c5aa769..ee1cbe5decd71 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_mode.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
> @@ -919,6 +919,153 @@ static int mga_crtc_mode_set_base(struct drm_crtc 
> *crtc, int x, int y,
>   return mga_crtc_do_set_base(mdev, fb, old_fb);
>  }
>  
> +static void mgag200_set_pci_regs(struct mga_device *mdev)
> +{
> + uint32_t option = 0, option2 = 0;
> + struct drm_device *dev = mdev->dev;
> +
> + switch (mdev->type) {
> + case G200_SE_A:
> + case G200_SE_B:
> + if (mdev->has_sdram)
> + option = 0x40049120;
> + else
> + option = 0x4004d120;
> + option2 = 0x8000;
> + break;
> + case G200_WB:
> + case G200_EW3:
> + option = 0x41049120;
> + option2 = 0xb000;
> + break;
> + case G200_EV:
> + option = 0x0120;
> + option2 = 0xb000;
> + break;
> + case G200_EH:
> + case G200_EH3:
> + option = 0x0120;
> + option2 = 0xb000;
> + break;
> + case G200_ER:
> + break;
> + }
> +
> + if (option)
> + pci_write_config_dword(dev->pdev, PCI_MGA_OPTION, option);
> +
> + if (option2)
> + pci_write_config_dword(dev->pdev, PCI_MGA_OPTION2, option2);
> +}
> +
> +static void mgag200_set_dac_regs(struct mga_device *mdev)
> +{
> + size_t i;
> + uint8_t dacvalue[] = {
> + /* 0x00: */0,0,0,0,0,0, 0x00,0,
> + /* 0x08: */0,0,0,0,0,0,0,0,
> + /* 0x10: */0,0,0,0,0,0,0,0,
> + /* 0x18: */ 0x00,0, 0xC9, 0xFF, 0xBF, 0x20, 0x1F, 0x20,
> + /* 0x20: */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> + /* 0x28: */ 0x00, 0x00, 0x00, 0x00,0,0,0, 0x40,
> + /* 0x30: */ 0x00, 0xB0, 0x00, 0xC2, 0x34, 0x14, 0x02, 0x83,
> + /* 0x38: */ 0x00, 0x93, 0x00, 0x77, 0x00, 0x00, 0x00, 0x3A,
> + /* 0x40: */0,0,0,0,0,0,0,0,
> + /* 0x48: */0,0,0,0,0,0,0,0
> + };
> +
> + switch (mdev->type) {
> + case G200_SE_A:
> + case G200_SE_B:
> + dacvalue[MGA1064_VREF_CTL] = 0x03;
> + dacvalue[MGA1064_PIX_CLK_CTL] = MGA1064_PIX_CLK_CTL_SEL_PLL;
> + dacvalue[MGA1064_MISC_CTL] = MGA1064_MISC_CTL_DAC_EN |
> +  MGA1064_MISC_CTL_VGA8 |
> +  MGA1064_MISC_CTL_DAC_RAM_CS;
> + break;
> + case G200_WB:
> + case G200_EW3:
> + dacvalue[MGA1064_VREF_CTL] = 0x07;
> + break;
> + case G200_EV:
> + dacvalue[MGA1064_PIX_CLK_CTL] = MGA1064_PIX_CLK_CTL_SEL_PLL;
> + dacvalue[MGA1064_MISC_CTL] = MGA1064_MISC_CTL_VGA8 |
> +  MGA1064_MISC_CTL_DAC_RAM_CS;
> + break;
> + case G200_EH:
> + case G200_EH3:
> + dacvalue[MGA1064_MISC_CTL] = MGA1064_MISC_CTL_VGA8 |
> +  MGA1064_MISC_CTL_DAC_RAM_CS;
> + break;
> + case G200_ER:
> + break;
> + }
> +
> + for (i = 0; i < ARRAY_SIZE(dacvalue); i++) {
> + if ((i <= 0x17) ||
> + (i == 0x1b) ||
> + (i == 0x1c) ||
> + ((i >= 0x1f) && (i <= 0x29)) ||
> + ((i >= 0x30) && (i <= 0x37)))
> + continue;
> + if (IS_G200_SE(mdev) &&
> + ((i == 0x2c) || (i == 0x2d) || (i == 0x2e)))
> + continue;
> + if ((mdev->type == G200_EV ||
> + mdev->type == G200_WB ||
> + mdev->type == G200_EH ||
> + mdev->type == G200_EW3 ||
> + mdev->type == G200_EH3) &&
> + (i >= 0x44) && (i <= 0x4e))
> + continue;
> +
> + WREG_DAC(i, dacvalue[i]);
> + }
> +
> + if (mdev->type == G200_ER)
> + WREG_DAC(0x90, 0);
> +}
> +
> +static void mgag200_init_regs(struct mga_device *mdev)
> +{
> + uint8_t 

Re: [PATCH 13/17] drm/mgag200: Move hiprilvl setting into separate functions

2020-05-03 Thread Sam Ravnborg
On Wed, Apr 29, 2020 at 04:32:34PM +0200, Thomas Zimmermann wrote:
> The hiprivlvl settings are now updated in mgag200_g200se_set_hiprilvl()
> and mgag200_g200ev_set_hiprilvl().
> 
> Signed-off-by: Thomas Zimmermann 
Acked-by: Sam Ravnborg 
> ---
>  drivers/gpu/drm/mgag200/mgag200_mode.c | 98 ++
>  1 file changed, 54 insertions(+), 44 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c 
> b/drivers/gpu/drm/mgag200/mgag200_mode.c
> index 6b88c306ff4d7..a04404c5aa769 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_mode.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
> @@ -1118,6 +1118,56 @@ static void mgag200_g200er_reset_tagfifo(struct 
> mga_device *mdev)
>   WREG_SEQ(0x01, seq1);
>  }
>  
> +static void mgag200_g200se_set_hiprilvl(struct mga_device *mdev,
> + const struct drm_display_mode *mode,
> + const struct drm_framebuffer *fb)
> +{
> + unsigned int hiprilvl;
> + uint8_t crtcext6;
> +
> + if  (mdev->unique_rev_id >= 0x04) {
> + hiprilvl = 0;
> + } else if (mdev->unique_rev_id >= 0x02) {
> + unsigned int bpp;
> + unsigned long mb;
> +
> + if (fb->format->cpp[0] * 8 > 16)
> + bpp = 32;
> + else if (fb->format->cpp[0] * 8 > 8)
> + bpp = 16;
> + else
> + bpp = 8;
> +
> + mb = (mode->clock * bpp) / 1000;
> + if (mb > 3100)
> + hiprilvl = 0;
> + else if (mb > 2600)
> + hiprilvl = 1;
> + else if (mb > 1900)
> + hiprilvl = 2;
> + else if (mb > 1160)
> + hiprilvl = 3;
> + else if (mb > 440)
> + hiprilvl = 4;
> + else
> + hiprilvl = 5;
> +
> + } else if (mdev->unique_rev_id >= 0x01) {
> + hiprilvl = 3;
> + } else {
> + hiprilvl = 4;
> + }
> +
> + crtcext6 = hiprilvl; /* implicitly sets maxhipri to 0 */
> +
> + WREG_ECRT(0x06, crtcext6);
> +}
> +
> +static void mgag200_g200ev_set_hiprilvl(struct mga_device *mdev)
> +{
> + WREG_ECRT(0x06, 0x00);
> +}
> +
>  static int mga_crtc_mode_set(struct drm_crtc *crtc,
>   struct drm_display_mode *mode,
>   struct drm_display_mode *adjusted_mode,
> @@ -1236,10 +1286,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
>   if (mdev->type == G200_EW3)
>   WREG_ECRT(0x34, 0x5);
>  
> - if (mdev->type == G200_EV) {
> - WREG_ECRT(6, 0);
> - }
> -
>   misc = RREG8(MGA_MISC_IN);
>   misc |= MGAREG_MISC_IOADSEL |
>   MGAREG_MISC_RAMMAPEN |
> @@ -1255,47 +1301,11 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
>   if (mdev->type == G200_ER)
>   mgag200_g200er_reset_tagfifo(mdev);
>  
> + if (IS_G200_SE(mdev))
> + mgag200_g200se_set_hiprilvl(mdev, mode, fb);
> + else if (mdev->type == G200_EV)
> + mgag200_g200ev_set_hiprilvl(mdev);
>  
> - if (IS_G200_SE(mdev)) {
> - if  (mdev->unique_rev_id >= 0x04) {
> - WREG8(MGAREG_CRTCEXT_INDEX, 0x06);
> - WREG8(MGAREG_CRTCEXT_DATA, 0);
> - } else if (mdev->unique_rev_id >= 0x02) {
> - u8 hi_pri_lvl;
> - u32 bpp;
> - u32 mb;
> -
> - if (fb->format->cpp[0] * 8 > 16)
> - bpp = 32;
> - else if (fb->format->cpp[0] * 8 > 8)
> - bpp = 16;
> - else
> - bpp = 8;
> -
> - mb = (mode->clock * bpp) / 1000;
> - if (mb > 3100)
> - hi_pri_lvl = 0;
> - else if (mb > 2600)
> - hi_pri_lvl = 1;
> - else if (mb > 1900)
> - hi_pri_lvl = 2;
> - else if (mb > 1160)
> - hi_pri_lvl = 3;
> - else if (mb > 440)
> - hi_pri_lvl = 4;
> - else
> - hi_pri_lvl = 5;
> -
> - WREG8(MGAREG_CRTCEXT_INDEX, 0x06);
> - WREG8(MGAREG_CRTCEXT_DATA, hi_pri_lvl);
> - } else {
> - WREG8(MGAREG_CRTCEXT_INDEX, 0x06);
> - if (mdev->unique_rev_id >= 0x01)
> - WREG8(MGAREG_CRTCEXT_DATA, 0x03);
> - else
> - WREG8(MGAREG_CRTCEXT_DATA, 0x04);
> - }
> - }
>   return 0;
>  }
>  
> -- 
> 2.26.0
> 
> ___
> dri-devel mailing list

Re: [PATCH 12/17] drm/mgag200: Move TAGFIFO reset into separate function

2020-05-03 Thread Sam Ravnborg
Hi Thomas.

One nit about a bit name below.
Acked-by: Sam Ravnborg 

On Wed, Apr 29, 2020 at 04:32:33PM +0200, Thomas Zimmermann wrote:
> 5
> 
> The TAGFIFO state is now reset in mgag200_g200er_reset_tagfifo().
> 
> Signed-off-by: Thomas Zimmermann 
> ---
>  drivers/gpu/drm/mgag200/mgag200_drv.h  |  6 
>  drivers/gpu/drm/mgag200/mgag200_mode.c | 45 +-
>  2 files changed, 35 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h 
> b/drivers/gpu/drm/mgag200/mgag200_drv.h
> index 9b957d9fc7e04..b10da90e0f35a 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_drv.h
> +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
> @@ -49,6 +49,12 @@
>   WREG8(ATTR_DATA, v);\
>   } while (0) \
>  
> +#define RREG_SEQ(reg, v) \
> + do {\
> + WREG8(MGAREG_SEQ_INDEX, reg);   \
> + v = RREG8(MGAREG_SEQ_DATA); \
> + } while (0) \
> +
>  #define WREG_SEQ(reg, v) \
>   do {\
>   WREG8(MGAREG_SEQ_INDEX, reg);   \
> diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c 
> b/drivers/gpu/drm/mgag200/mgag200_mode.c
> index 73f7135cbb3d8..6b88c306ff4d7 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_mode.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
> @@ -1091,6 +1091,33 @@ static void mgag200_set_format_regs(struct mga_device 
> *mdev,
>   WREG_ECRT(3, crtcext3);
>  }
>  
> +static void mgag200_g200er_reset_tagfifo(struct mga_device *mdev)
> +{
> + static uint32_t RESET_FLAG = 0x0020; /* undocumented magic value */
> + u8 seq1;
> + u32 memctl;
> +
> + /* screen off */
> + RREG_SEQ(0x01, seq1);
> + seq1 |= 0x20;
This looks like this:
#defineM_SEQ1_SCROFF0x20


> + WREG_SEQ(0x01, seq1);
> +
> + memctl = RREG32(MGAREG_MEMCTL);
> +
> + memctl |= RESET_FLAG;
> + WREG32(MGAREG_MEMCTL, memctl);
> +
> + udelay(1000);
> +
> + memctl &= ~RESET_FLAG;
> + WREG32(MGAREG_MEMCTL, memctl);
> +
> + /* screen on */
> + RREG_SEQ(0x01, seq1);
> + seq1 &= ~0x20;
> + WREG_SEQ(0x01, seq1);
Here seq1 is read again, the old code used the old value.
I think new code is better.

> +}
> +
>  static int mga_crtc_mode_set(struct drm_crtc *crtc,
>   struct drm_display_mode *mode,
>   struct drm_display_mode *adjusted_mode,
> @@ -1225,22 +1252,8 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
>  
>   mgag200_set_mode_regs(mdev, mode);
>  
> - /* reset tagfifo */
> - if (mdev->type == G200_ER) {
> - u32 mem_ctl = RREG32(MGAREG_MEMCTL);
> - u8 seq1;
> -
> - /* screen off */
> - WREG8(MGAREG_SEQ_INDEX, 0x01);
> - seq1 = RREG8(MGAREG_SEQ_DATA) | 0x20;
> - WREG8(MGAREG_SEQ_DATA, seq1);
> -
> - WREG32(MGAREG_MEMCTL, mem_ctl | 0x0020);
> - udelay(1000);
> - WREG32(MGAREG_MEMCTL, mem_ctl & ~0x0020);
> -
> - WREG8(MGAREG_SEQ_DATA, seq1 & ~0x20);
> - }
> + if (mdev->type == G200_ER)
> + mgag200_g200er_reset_tagfifo(mdev);
>  
>  
>   if (IS_G200_SE(mdev)) {
> -- 
> 2.26.0
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-dev
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 10/17] drm/mgag200: Set pitch in a separate helper function

2020-05-03 Thread Sam Ravnborg
Hi Thomas.

On Wed, Apr 29, 2020 at 04:32:31PM +0200, Thomas Zimmermann wrote:
> The framebuffer's pitch is now set in mgag200_set_offset().
> 
> Signed-off-by: Thomas Zimmermann 

I failed to follow this code.

> ---
>  drivers/gpu/drm/mgag200/mgag200_mode.c | 41 +-
>  1 file changed, 27 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c 
> b/drivers/gpu/drm/mgag200/mgag200_mode.c
> index 92dee31f16c42..eb83e471d72fc 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_mode.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
> @@ -1003,6 +1003,32 @@ static void mgag200_set_mode_regs(struct mga_device 
> *mdev,
>   mga_crtc_set_plls(mdev, mode->clock);
>  }
>  
> +static void mgag200_set_offset(struct mga_device *mdev,
> +const struct drm_framebuffer *fb)
> +{
> + unsigned int offset;
> + uint8_t crtc13, crtcext0;
> + u8 bppshift;
> +
> + bppshift = mdev->bpp_shifts[fb->format->cpp[0] - 1];
Hmm, use of cpp is deprecated. But is continue to be used all over.

> +
> + offset = fb->pitches[0] / fb->format->cpp[0];
> + if (fb->format->cpp[0] * 8 == 24)
> + offset = (offset * 3) >> (4 - bppshift);
> + else
> + offset = offset >> (4 - bppshift);
> +
> + RREG_ECRT(0, crtcext0);
> +
> + crtc13 = offset & 0xff;
> +
> + crtcext0 &= ~GENMASK(5, 4);
> + crtcext0 |= (offset & GENMASK(9, 8)) >> 4;
Lot's of hardcoded numbers.
Could the reg file include these so you had more readable defined names?

> +
> + WREG_CRT(0x13, crtc13);
> + WREG_ECRT(0x00, crtcext0);
> +}
> +
>  static int mga_crtc_mode_set(struct drm_crtc *crtc,
>   struct drm_display_mode *mode,
>   struct drm_display_mode *adjusted_mode,
> @@ -1011,7 +1037,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
>   struct drm_device *dev = crtc->dev;
>   struct mga_device *mdev = dev->dev_private;
>   const struct drm_framebuffer *fb = crtc->primary->fb;
> - int pitch;
>   int option = 0, option2 = 0;
>   int i;
>   unsigned char misc = 0;
> @@ -1122,12 +1147,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
>   WREG_SEQ(3, 0);
>   WREG_SEQ(4, 0xe);
>  
> - pitch = fb->pitches[0] / fb->format->cpp[0];
> - if (fb->format->cpp[0] * 8 == 24)
> - pitch = (pitch * 3) >> (4 - bppshift);
> - else
> - pitch = pitch >> (4 - bppshift);
> -
>   WREG_GFX(0, 0);
>   WREG_GFX(1, 0);
>   WREG_GFX(2, 0);
> @@ -1144,20 +1163,15 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
>   WREG_CRT(13, 0);
>   WREG_CRT(14, 0);
>   WREG_CRT(15, 0);
> - WREG_CRT(19, pitch & 0xFF);
> -
> - ext_vga[0] = 0;
>  
>   /* TODO interlace */
>  
> - ext_vga[0] |= (pitch & 0x300) >> 4;
>   if (fb->format->cpp[0] * 8 == 24)
>   ext_vga[3] = (((1 << bppshift) * 3) - 1) | 0x80;
>   else
>   ext_vga[3] = ((1 << bppshift) - 1) | 0x80;
>   ext_vga[4] = 0;
>  
> - WREG_ECRT(0, ext_vga[0]);
>   WREG_ECRT(3, ext_vga[3]);
>   WREG_ECRT(4, ext_vga[4]);
>  
> @@ -1171,8 +1185,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
>   WREG_ECRT(6, 0);
>   }
>  
> - WREG_ECRT(0, ext_vga[0]);
> -
>   misc = RREG8(MGA_MISC_IN);
>   misc |= MGAREG_MISC_IOADSEL |
>   MGAREG_MISC_RAMMAPEN |
> @@ -1180,6 +1192,7 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
>   WREG8(MGA_MISC_OUT, misc);
>  
>   mga_crtc_do_set_base(mdev, fb, old_fb);
> + mgag200_set_offset(mdev, fb);
>  
>   mgag200_set_mode_regs(mdev, mode);

Sam
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 09/17] drm/mgag200: Update mode registers after plane registers

2020-05-03 Thread Sam Ravnborg
On Wed, Apr 29, 2020 at 04:32:30PM +0200, Thomas Zimmermann wrote:
> Setting the plane registers first and the mode registers afterwards
> reproduces the sequence used by atomic helpers. Done in preparation
> of switching to simple KMS helpers.
> 
> Signed-off-by: Thomas Zimmermann 
Acked-by: Sam Ravnborg 
> ---
>  drivers/gpu/drm/mgag200/mgag200_mode.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c 
> b/drivers/gpu/drm/mgag200/mgag200_mode.c
> index b5bb02e9f05d6..92dee31f16c42 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_mode.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
> @@ -1146,8 +1146,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
>   WREG_CRT(15, 0);
>   WREG_CRT(19, pitch & 0xFF);
>  
> - mgag200_set_mode_regs(mdev, mode);
> -
>   ext_vga[0] = 0;
>  
>   /* TODO interlace */
> @@ -1183,6 +1181,8 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
>  
>   mga_crtc_do_set_base(mdev, fb, old_fb);
>  
> + mgag200_set_mode_regs(mdev, mode);
> +
>   /* reset tagfifo */
>   if (mdev->type == G200_ER) {
>   u32 mem_ctl = RREG32(MGAREG_MEMCTL);
> -- 
> 2.26.0
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 08/17] drm/mgag200: Split MISC register update into PLL selection, SYNC and I/O

2020-05-03 Thread Sam Ravnborg
Hi Thomas.

On Wed, Apr 29, 2020 at 04:32:29PM +0200, Thomas Zimmermann wrote:
> Set different fields in MISC in their rsp location in the code. This
> patch also fixes a bug in the original code where the mode's SYNC flags
> were never written into the MISC register.
> 
> Signed-off-by: Thomas Zimmermann 
> ---
>  drivers/gpu/drm/mgag200/mgag200_mode.c | 37 ++
>  drivers/gpu/drm/mgag200/mgag200_reg.h  |  5 +++-
>  2 files changed, 30 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c 
> b/drivers/gpu/drm/mgag200/mgag200_mode.c
> index 749ba6e420ac7..b5bb02e9f05d6 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_mode.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
> @@ -704,6 +704,8 @@ static int mga_g200er_set_plls(struct mga_device *mdev, 
> long clock)
>  
>  static int mga_crtc_set_plls(struct mga_device *mdev, long clock)
>  {
> + uint8_t misc;

General comment.
uint8_t and friends are for uapi stuff.
kernel internal prefer u8 and friends.

Would be good to clean this up in the drivire, maybe as a follow-up
patch after is becomes atomic.


> +
>   switch(mdev->type) {
>   case G200_SE_A:
>   case G200_SE_B:
> @@ -724,6 +726,12 @@ static int mga_crtc_set_plls(struct mga_device *mdev, 
> long clock)
>   return mga_g200er_set_plls(mdev, clock);
>   break;
>   }
> +
> + misc = RREG8(MGA_MISC_IN);
> + misc &= ~GENMASK(3, 2);
The code would be easier to read if we had a 
#define MGAREG_MISC_CLK_SEL_MASKGENMASK(3, 2)

So the above became:
misc &= ~MGAREG_MISC_CLK_SEL_MASK;

Then it is more clear that the CLK_SEL bits are clared and then
MGA_MSK is set.

> + misc |= MGAREG_MISC_CLK_SEL_MGA_MSK;
> + WREG8(MGA_MISC_OUT, misc);
> +
>   return 0;
>  }
>  
> @@ -916,7 +924,7 @@ static void mgag200_set_mode_regs(struct mga_device *mdev,
>  {
>   unsigned int hdisplay, hsyncstart, hsyncend, htotal;
>   unsigned int vdisplay, vsyncstart, vsyncend, vtotal;
> - uint8_t misc = 0;
> + uint8_t misc;
>   uint8_t crtcext1, crtcext2, crtcext5;
>  
>   hdisplay = mode->hdisplay / 8 - 1;
> @@ -933,10 +941,17 @@ static void mgag200_set_mode_regs(struct mga_device 
> *mdev,
>   vsyncend = mode->vsync_end - 1;
>   vtotal = mode->vtotal - 2;
>  
> + misc = RREG8(MGA_MISC_IN);
> +
>   if (mode->flags & DRM_MODE_FLAG_NHSYNC)
> - misc |= 0x40;
> + misc |= MGAREG_MISC_HSYNCPOL;
> + else
> + misc &= ~MGAREG_MISC_HSYNCPOL;
> +
So the code just assumes DRM_MODE_FLAG_PHSYNC if
DRM_MODE_FLAG_NHSYNC is not set.
I think that is fine, but it also indicate how hoorible the
flags definitions are in mode->flags


>   if (mode->flags & DRM_MODE_FLAG_NVSYNC)
> - misc |= 0x80;
> + misc |= MGAREG_MISC_VSYNCPOL;
> + else
> + misc &= ~MGAREG_MISC_VSYNCPOL;
And this code was touched in previous patch, but I gess it is better
to update it here.

>  
>   crtcext1 = (((htotal - 4) & 0x100) >> 8) |
>  ((hdisplay & 0x100) >> 7) |
> @@ -982,6 +997,10 @@ static void mgag200_set_mode_regs(struct mga_device 
> *mdev,
>   WREG_ECRT(0x01, crtcext1);
>   WREG_ECRT(0x02, crtcext2);
>   WREG_ECRT(0x05, crtcext5);
> +
> + WREG8(MGA_MISC_OUT, misc);
> +
> + mga_crtc_set_plls(mdev, mode->clock);
>  }
>  
>  static int mga_crtc_mode_set(struct drm_crtc *crtc,
> @@ -1140,12 +1159,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
>   ext_vga[3] = ((1 << bppshift) - 1) | 0x80;
>   ext_vga[4] = 0;
>  
> - /* Set pixel clocks */
> - misc = 0x2d;
> - WREG8(MGA_MISC_OUT, misc);
> -
> - mga_crtc_set_plls(mdev, mode->clock);
> -
>   WREG_ECRT(0, ext_vga[0]);
>   WREG_ECRT(3, ext_vga[3]);
>   WREG_ECRT(4, ext_vga[4]);
> @@ -1161,9 +1174,11 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
>   }
>  
>   WREG_ECRT(0, ext_vga[0]);
> - /* Enable mga pixel clock */
> - misc = 0x2d;
>  
> + misc = RREG8(MGA_MISC_IN);
> + misc |= MGAREG_MISC_IOADSEL |
> + MGAREG_MISC_RAMMAPEN |
> + MGAREG_MISC_HIGH_PG_SEL;
>   WREG8(MGA_MISC_OUT, misc);

I am left puzzeled why there is several writes to MGA_MISC_OUT.
The driver needs to read back and then write again.

Would it be simpler to have one function that only took care of
misc register update?

>  
>   mga_crtc_do_set_base(mdev, fb, old_fb);
> diff --git a/drivers/gpu/drm/mgag200/mgag200_reg.h 
> b/drivers/gpu/drm/mgag200/mgag200_reg.h
> index c096a9d6bcbc1..89e12c55153cf 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_reg.h
> +++ b/drivers/gpu/drm/mgag200/mgag200_reg.h
> @@ -16,10 +16,11 @@
>   *   MGA1064SG Mystique register file
>   */
>  
> -
>  #ifndef _MGA_REG_H_
>  #define _MGA_REG_H_
>  
> +#include 
> +
>  #define  MGAREG_DWGCTL   0x1c00
>  #define  MGAREG_MACCESS  0x1c04
>  /* the following is 

[Bug 207383] [Regression] 5.7-rc: amdgpu/polaris11 gpf: amdgpu_atomic_commit_tail

2020-05-03 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=207383

--- Comment #10 from Duncan (1i5t5.dun...@cox.net) ---
(In reply to Duncan from comment #9)
> I'm not there yet but it's starting to look like a possibly dud bisect:
> everything showing good so far

Good but not ideal news!

I did get an apparent graphics crash at the bisect-point above, but it didn't
dump anything in the log this time and behavior was a bit different than usual
for this bug -- audio continued playing longer and I was able to confirm SRQ-E
termination via audio and cpu-fan, and SRQ-S sync via sata-activity LED.

So I'm not sure it's the same bug, or maybe a different one; I'm bisecting
pre-rc1 after all so others aren't unlikely.

So I'm rebooted to the same bisect step to try again, with any luck to get that
gpf dump in the log confirming it's the same bug this time.

If it *is* the same bug, it looks like I avoided a dud bisect after all, just
happened to be all good until almost the very end, I'm only a few steps away
from pinning it down, and it's almost certainly one of the commits listed in
comment #9. =:^)

> Meanwhile, user-side I've gotten vulkan/mesa/etc updates recently.  I'm
> considering checking out linus-master/HEAD again, doing a pull, and seeing
> if by chance either the last week's kernel updates or the user-side updates
> have eliminated the problem.

Been there, done that, still had the bug, with gpf-log-dump confirmation.  Back
to the bisect.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [RFC PATCH 4/4] arm64: dts: imx8mm: Add GPU nodes for 2D and 3D core using Etnaviv

2020-05-03 Thread Adam Ford
On Thu, Apr 30, 2020 at 7:46 AM Schrempf Frieder
 wrote:
>
> From: Frieder Schrempf 
>
> According to the documents, the i.MX8M-Mini features a GC320 and a
> GCNanoUltra GPU core. Etnaviv detects them as:
>
> etnaviv-gpu 3800.gpu: model: GC600, revision: 4653
> etnaviv-gpu 38008000.gpu: model: GC520, revision: 5341
>
> This seems to work fine more or less without any changes to the HWDB,
> which still might be needed in the future to correct some features,
> etc.
>
> Signed-off-by: Frieder Schrempf 
> ---
Since not everyone uses the 3D or 2D, would it make sense to mark them
as disabled by default and let people who need the 3D and 2D enable
them at their respective board files?

adam

> 2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 07/10] drm/format-helper: Add drm_fb_swab()

2020-05-03 Thread Noralf Trønnes


Den 03.05.2020 12.29, skrev Sam Ravnborg:
> Hi Noralf
> 
> On Wed, Apr 29, 2020 at 02:48:27PM +0200, Noralf Trønnes wrote:
>> This replaces drm_fb_swab16() with drm_fb_swab() supporting 16 and 32-bit.
>> Also make pixel line caching optional.
>>
>> Signed-off-by: Noralf Trønnes 
> A couple of nits, with these considered:
> Reviewed-by: Sam Ravnborg 
> 
>> ---
>>  drivers/gpu/drm/drm_format_helper.c | 61 +++--
>>  drivers/gpu/drm/drm_mipi_dbi.c  |  2 +-
>>  include/drm/drm_format_helper.h |  4 +-
>>  3 files changed, 44 insertions(+), 23 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_format_helper.c 
>> b/drivers/gpu/drm/drm_format_helper.c
>> index 0897cb9aeaff..5c147c49668c 100644
>> --- a/drivers/gpu/drm/drm_format_helper.c
>> +++ b/drivers/gpu/drm/drm_format_helper.c
>> @@ -79,39 +79,60 @@ void drm_fb_memcpy_dstclip(void __iomem *dst, void 
>> *vaddr,
>>  EXPORT_SYMBOL(drm_fb_memcpy_dstclip);
>>  
>>  /**
>> - * drm_fb_swab16 - Swap bytes into clip buffer
>> - * @dst: RGB565 destination buffer
>> - * @vaddr: RGB565 source buffer
>> + * drm_fb_swab - Swap bytes into clip buffer
>> + * @dst: Destination buffer
>> + * @src: Source buffer
>>   * @fb: DRM framebuffer
>>   * @clip: Clip rectangle area to copy
>> + * @cached: Source buffer is mapped cached (eg. not write-combined)
>> + *
>> + * If @cached is false a temporary buffer is used to cache one pixel line 
>> at a
>> + * time to speed up slow uncached reads.
>> + *
>> + * This function does not apply clipping on dst, i.e. the destination
>> + * is a small buffer containing the clip rect only.
>>   */
>> -void drm_fb_swab16(u16 *dst, void *vaddr, struct drm_framebuffer *fb,
>> -   struct drm_rect *clip)
>> +void drm_fb_swab(void *dst, void *src, struct drm_framebuffer *fb,
>> + struct drm_rect *clip, bool cached)
>>  {
>> -size_t len = (clip->x2 - clip->x1) * sizeof(u16);
>> +u8 cpp = fb->format->cpp[0];
> Use of format->cpp is deprecated, should be char_per_block according to
> the comment in drm_fourcc.h

I ducked this because if I had to do it properly I would have to look at
block width/height as well and yes ensure that num_planes is 1. That
would leave me with writing a helper function for something I don't
really understand :-)

static inline bool
drm_format_info_is_WHAT_TO_CALL_THIS(const struct drm_format_info *info)
{
return info->num_planes == 1 &&
   drm_format_info_block_width(info, 0) == 1 &&
   drm_format_info_block_height(info, 0) == 1;
}

Or I could ofc just spell out the full assert inside this function:

info->num_planes == 1 &&
drm_format_info_block_width(info, 0) == 1 &&
drm_format_info_block_height(info, 0) == 1 &&
info->char_per_block[0] == 2 &&
info->char_per_block[0] == 4

That way I don't have to know what I'm actually checking.
I'm using drm_fb_swab() for RGB formats, but it can be used for any
format that meets the above criteria.

And maybe I should check .hsub and .vsub as well, I don't know.

cpp was such a nice simple concept :-) So I'll keep it unless someone
knowledgeable shines some light on this.

> 
>> +size_t len = drm_rect_width(clip) * cpp;
>> +u16 *src16, *dst16 = dst;
>> +u32 *src32, *dst32 = dst;
>>  unsigned int x, y;
>> -u16 *src, *buf;
>> +void *buf = NULL;
>>  
>> -/*
>> - * The cma memory is write-combined so reads are uncached.
>> - * Speed up by fetching one line at a time.
>> - */
>> -buf = kmalloc(len, GFP_KERNEL);
>> -if (!buf)
>> +if (WARN_ON_ONCE(cpp == 1))
>>  return;
> Or cpp != 2 && cpp != 4?

Indeed, I agree.

Noralf.

>>  
>> +if (!cached)
>> +buf = kmalloc(len, GFP_KERNEL);
>> +
>> +src += clip_offset(clip, fb->pitches[0], cpp);
> Good that drm_rect_width() and clip_offset() are used,
> replacing open-coded variants.
> 
>> +
>>  for (y = clip->y1; y < clip->y2; y++) {
>> -src = vaddr + (y * fb->pitches[0]);
>> -src += clip->x1;
>> -memcpy(buf, src, len);
>> -src = buf;
>> -for (x = clip->x1; x < clip->x2; x++)
>> -*dst++ = swab16(*src++);
>> +if (buf) {
>> +memcpy(buf, src, len);
>> +src16 = buf;
>> +src32 = buf;
>> +} else {
>> +src16 = src;
>> +src32 = src;
>> +}
>> +
>> +for (x = clip->x1; x < clip->x2; x++) {
>> +if (cpp == 4)
>> +*dst32++ = swab32(*src32++);
>> +else
>> +*dst16++ = swab16(*src16++);
>> +}
>> +
>> +src += fb->pitches[0];
>>  }
>>  
>>  kfree(buf);
>>  }
>> -EXPORT_SYMBOL(drm_fb_swab16);
>> +EXPORT_SYMBOL(drm_fb_swab);
>>  
>>  static void drm_fb_xrgb_to_rgb565_line(u16 *dbuf, u32 *sbuf,
>>   

Re: [PATCH v7 01/12] dt-bindings: add img, pvrsgx.yaml for Imagination GPUs

2020-05-03 Thread H. Nikolaus Schaller
Hi Paul,

> Am 26.04.2020 um 15:11 schrieb Paul Cercueil :
> 
> Hi Nikolaus,
> 
> Le ven. 24 avril 2020 à 22:34, H. Nikolaus Schaller  a 
> écrit :
>> The Imagination PVR/SGX GPU is part of several SoC from
>> multiple vendors, e.g. TI OMAP, Ingenic JZ4780, Intel Poulsbo,
>> Allwinner A83 and others.
>> With this binding, we describe how the SGX processor is
>> interfaced to the SoC (registers and interrupt).
>> The interface also consists of clocks, reset, power but
>> information from data sheets is vague and some SoC integrators
>> (TI) deciced to use a PRCM wrapper (ti,sysc) which does
>> all clock, reset and power-management through registers
>> outside of the sgx register block.
>> Therefore all these properties are optional.
>> Tested by make dt_binding_check
>> Signed-off-by: H. Nikolaus Schaller 
>> ---
>> .../devicetree/bindings/gpu/img,pvrsgx.yaml   | 150 ++
>> 1 file changed, 150 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/gpu/img,pvrsgx.yaml
>> diff --git a/Documentation/devicetree/bindings/gpu/img,pvrsgx.yaml 
>> b/Documentation/devicetree/bindings/gpu/img,pvrsgx.yaml
>> new file mode 100644
>> index ..33a9c4c6e784
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/gpu/img,pvrsgx.yaml
>> @@ -0,0 +1,150 @@
>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/gpu/img,pvrsgx.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Imagination PVR/SGX GPU
>> +
>> +maintainers:
>> +  - H. Nikolaus Schaller 
>> +
>> +description: |+
>> +  This binding describes the Imagination SGX5 series of 3D accelerators 
>> which
>> +  are found in several different SoC like TI OMAP, Sitara, Ingenic JZ4780,
>> +  Allwinner A83, and Intel Poulsbo and CedarView and more.
>> +
>> +  For an extensive list see: 
>> https://en.wikipedia.org/wiki/PowerVR#Implementations
>> +
>> +  The SGX node is usually a child node of some DT node belonging to the SoC
>> +  which handles clocks, reset and general address space mapping of the SGX
>> +  register area. If not, an optional clock can be specified here.
>> +
>> +properties:
>> +  $nodename:
>> +pattern: '^gpu@[a-f0-9]+$'
>> +  compatible:
>> +oneOf:
>> +  - description: SGX530-121 based SoC
>> +items:
>> +  - enum:
>> +- ti,omap3-sgx530-121 # BeagleBoard A/B/C, OpenPandora 600MHz 
>> and similar
>> +  - const: img,sgx530-121
>> +  - const: img,sgx530
>> +
>> +  - description: SGX530-125 based SoC
>> +items:
>> +  - enum:
>> +- ti,am3352-sgx530-125 # BeagleBone Black
>> +- ti,am3517-sgx530-125
>> +- ti,am4-sgx530-125
>> +- ti,omap3-sgx530-125 # BeagleBoard XM, GTA04, OpenPandora 1GHz 
>> and similar
>> +- ti,ti81xx-sgx530-125
>> +  - const: ti,omap3-sgx530-125
>> +  - const: img,sgx530-125
>> +  - const: img,sgx530
>> +
>> +  - description: SGX535-116 based SoC
>> +items:
>> +  - const: intel,poulsbo-gma500-sgx535 # Atom Z5xx
>> +  - const: img,sgx535-116
>> +  - const: img,sgx535
>> +
>> +  - description: SGX540-116 based SoC
>> +items:
>> +  - const: intel,medfield-gma-sgx540 # Atom Z24xx
>> +  - const: img,sgx540-116
>> +  - const: img,sgx540
>> +
>> +  - description: SGX540-120 based SoC
>> +items:
>> +  - enum:
>> +- samsung,s5pv210-sgx540-120
>> +- ti,omap4-sgx540-120 # Pandaboard, Pandaboard ES and similar
>> +  - const: img,sgx540-120
>> +  - const: img,sgx540
>> +
>> +  - description: SGX540-130 based SoC
>> +items:
>> +  - enum:
>> +- ingenic,jz4780-sgx540-130 # CI20
>> +  - const: img,sgx540-130
>> +  - const: img,sgx540
>> +
>> +  - description: SGX544-112 based SoC
>> +items:
>> +  - const: ti,omap4470-sgx544-112
>> +  - const: img,sgx544-112
>> +  - const: img,sgx544
>> +
>> +  - description: SGX544-115 based SoC
>> +items:
>> +  - enum:
>> +- allwinner,sun8i-a31-sgx544-115
>> +- allwinner,sun8i-a31s-sgx544-115
>> +- allwinner,sun8i-a83t-sgx544-115 # Banana-Pi-M3 (Allwinner 
>> A83T) and similar
>> +  - const: img,sgx544-115
>> +  - const: img,sgx544
>> +
>> +  - description: SGX544-116 based SoC
>> +items:
>> +  - enum:
>> +- ti,dra7-sgx544-116 # DRA7
>> +- ti,omap5-sgx544-116 # OMAP5 UEVM, Pyra Handheld and similar
>> +  - const: img,sgx544-116
>> +  - const: img,sgx544
>> +
>> +  - description: SGX545 based SoC
>> +items:
>> +  - const: intel,cedarview-gma3600-sgx545 # Atom N2600, D2500
>> +  - const: img,sgx545-116
>> +  - const: img,sgx545
>> +
>> +  reg:
>> +

Re: [PATCH v7 01/12] dt-bindings: add img, pvrsgx.yaml for Imagination GPUs

2020-05-03 Thread Paul Cercueil

Hi Nikolaus,

Le sam. 2 mai 2020 à 22:26, H. Nikolaus Schaller  a 
écrit :

Hi Paul,


 Am 26.04.2020 um 15:11 schrieb Paul Cercueil :

 Hi Nikolaus,

 Le ven. 24 avril 2020 à 22:34, H. Nikolaus Schaller 
 a écrit :

 The Imagination PVR/SGX GPU is part of several SoC from
 multiple vendors, e.g. TI OMAP, Ingenic JZ4780, Intel Poulsbo,
 Allwinner A83 and others.
 With this binding, we describe how the SGX processor is
 interfaced to the SoC (registers and interrupt).
 The interface also consists of clocks, reset, power but
 information from data sheets is vague and some SoC integrators
 (TI) deciced to use a PRCM wrapper (ti,sysc) which does
 all clock, reset and power-management through registers
 outside of the sgx register block.
 Therefore all these properties are optional.
 Tested by make dt_binding_check
 Signed-off-by: H. Nikolaus Schaller 
 ---
 .../devicetree/bindings/gpu/img,pvrsgx.yaml   | 150 
++

 1 file changed, 150 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/gpu/img,pvrsgx.yaml
 diff --git a/Documentation/devicetree/bindings/gpu/img,pvrsgx.yaml 
b/Documentation/devicetree/bindings/gpu/img,pvrsgx.yaml

 new file mode 100644
 index ..33a9c4c6e784
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/gpu/img,pvrsgx.yaml
 @@ -0,0 +1,150 @@
 +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
 +%YAML 1.2
 +---
 +$id: http://devicetree.org/schemas/gpu/img,pvrsgx.yaml#
 +$schema: http://devicetree.org/meta-schemas/core.yaml#
 +
 +title: Imagination PVR/SGX GPU
 +
 +maintainers:
 +  - H. Nikolaus Schaller 
 +
 +description: |+
 +  This binding describes the Imagination SGX5 series of 3D 
accelerators which
 +  are found in several different SoC like TI OMAP, Sitara, 
Ingenic JZ4780,

 +  Allwinner A83, and Intel Poulsbo and CedarView and more.
 +
 +  For an extensive list see: 
https://en.wikipedia.org/wiki/PowerVR#Implementations

 +
 +  The SGX node is usually a child node of some DT node belonging 
to the SoC
 +  which handles clocks, reset and general address space mapping 
of the SGX

 +  register area. If not, an optional clock can be specified here.
 +
 +properties:
 +  $nodename:
 +pattern: '^gpu@[a-f0-9]+$'
 +  compatible:
 +oneOf:
 +  - description: SGX530-121 based SoC
 +items:
 +  - enum:
 +- ti,omap3-sgx530-121 # BeagleBoard A/B/C, 
OpenPandora 600MHz and similar

 +  - const: img,sgx530-121
 +  - const: img,sgx530
 +
 +  - description: SGX530-125 based SoC
 +items:
 +  - enum:
 +- ti,am3352-sgx530-125 # BeagleBone Black
 +- ti,am3517-sgx530-125
 +- ti,am4-sgx530-125
 +- ti,omap3-sgx530-125 # BeagleBoard XM, GTA04, 
OpenPandora 1GHz and similar

 +- ti,ti81xx-sgx530-125
 +  - const: ti,omap3-sgx530-125
 +  - const: img,sgx530-125
 +  - const: img,sgx530
 +
 +  - description: SGX535-116 based SoC
 +items:
 +  - const: intel,poulsbo-gma500-sgx535 # Atom Z5xx
 +  - const: img,sgx535-116
 +  - const: img,sgx535
 +
 +  - description: SGX540-116 based SoC
 +items:
 +  - const: intel,medfield-gma-sgx540 # Atom Z24xx
 +  - const: img,sgx540-116
 +  - const: img,sgx540
 +
 +  - description: SGX540-120 based SoC
 +items:
 +  - enum:
 +- samsung,s5pv210-sgx540-120
 +- ti,omap4-sgx540-120 # Pandaboard, Pandaboard ES and 
similar

 +  - const: img,sgx540-120
 +  - const: img,sgx540
 +
 +  - description: SGX540-130 based SoC
 +items:
 +  - enum:
 +- ingenic,jz4780-sgx540-130 # CI20
 +  - const: img,sgx540-130
 +  - const: img,sgx540
 +
 +  - description: SGX544-112 based SoC
 +items:
 +  - const: ti,omap4470-sgx544-112
 +  - const: img,sgx544-112
 +  - const: img,sgx544
 +
 +  - description: SGX544-115 based SoC
 +items:
 +  - enum:
 +- allwinner,sun8i-a31-sgx544-115
 +- allwinner,sun8i-a31s-sgx544-115
 +- allwinner,sun8i-a83t-sgx544-115 # Banana-Pi-M3 
(Allwinner A83T) and similar

 +  - const: img,sgx544-115
 +  - const: img,sgx544
 +
 +  - description: SGX544-116 based SoC
 +items:
 +  - enum:
 +- ti,dra7-sgx544-116 # DRA7
 +- ti,omap5-sgx544-116 # OMAP5 UEVM, Pyra Handheld and 
similar

 +  - const: img,sgx544-116
 +  - const: img,sgx544
 +
 +  - description: SGX545 based SoC
 +items:
 +  - const: intel,cedarview-gma3600-sgx545 # Atom N2600, 
D2500

 +  - const: img,sgx545-116
 +  - const: img,sgx545
 +
 +  reg:
 +maxItems: 1
 +
 +  interrupts:
 +maxItems: 1
 +
 +  interrupt-names:
 +maxItems: 1
 +items:
 +  - const: sgx
 +
 +  clocks:
 +maxItems: 4
 +
 +  clock-names:
 +

Re: [PATCH 2/2] drm/panfrost: add devfreq regulator support

2020-05-03 Thread Clément Péron
Hi Steven,

On Tue, 14 Apr 2020 at 15:10, Steven Price  wrote:
>
> Hi Clément,
>
> On 13/04/2020 18:28, Clément Péron wrote:
> > Hi Steven,
> >



> Getting a backtrace from the two occurrences, I see one added from:
>
>(debugfs_create_dir) from [] (create_regulator+0xe0/0x220)
>(create_regulator) from [] (_regulator_get+0x168/0x204)
>(_regulator_get) from [] (regulator_bulk_get+0x64/0xf4)
>(regulator_bulk_get) from []
> (devm_regulator_bulk_get+0x40/0x74)
>(devm_regulator_bulk_get) from []
> (panfrost_device_init+0x1b4/0x48c [panfrost])
>(panfrost_device_init [panfrost]) from []
> (panfrost_probe+0x94/0x184 [panfrost])
>(panfrost_probe [panfrost]) from []
> (platform_drv_probe+0x48/0x94)
>
> And the other:
>
>(debugfs_create_dir) from [] (create_regulator+0xe0/0x220)
>(create_regulator) from [] (_regulator_get+0x168/0x204)
>(_regulator_get) from [] (dev_pm_opp_set_regulators+0x6c/0x184)
>(dev_pm_opp_set_regulators) from []
> (panfrost_devfreq_init+0x38/0x1ac [panfrost])
>(panfrost_devfreq_init [panfrost]) from []
> (panfrost_probe+0xc8/0x184 [panfrost])
>(panfrost_probe [panfrost]) from []
> (platform_drv_probe+0x48/0x94)
>
> Both are created at /regulator/vdd_gpu

I'm preparing a new version with some clean from lima devfreq.
My working branch :
https://github.com/clementperon/linux/commits/panfrost_devfreq

Two strange things I observe:
 - After 30sec the regulator is released by OPP ???
[   33.757627] vdd-gpu: disabling
Introduce the regulator support in this commit:
https://github.com/clementperon/linux/commit/be310c37b82010e293b7f129ccdcb711a2abb2ce

 - The Cooling map is not probe correctly :
[2.545756] panfrost 180.gpu: [drm:panfrost_devfreq_init
[panfrost]] Failed to register cooling device
Introduce in this commit :
https://github.com/clementperon/linux/commit/0252c38fd55ad78366ac4b1714e285c88db34557

Do you have an hint about what I'm missing ?

Thanks for your help,
Clement
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 1/2] drivers: drm: panel: Add TM5P5 NT35596 panel driver

2020-05-03 Thread Konrad Dybcio
Hi,

Thanks for your review. I'll send a v2 soon, however we need to solve the
compat string issue first.

How should I document tm5p5? I think it's rather some kind of a model no.
along with the nt35596 IC than a vendor name.. Or should we call it
something like "unknown,tm5p5-nt35596", perhaps "asus,z00t-tm5p5-n35596"
[1]?

[1] Z00T is the model number of the smartphone that uses this panel

Konrad

On Sat, May 2, 2020, 09:05 Sam Ravnborg  wrote:

> Hi Konrad.
>
> On Fri, May 01, 2020 at 10:48:22PM +0200, Konrad Dybcio wrote:
> > This adds support for TMP5P5 NT35596 1080x1920 video
> > mode panel that can be found on some Asus Zenfone 2
> > Laser (Z00T) devices.
>
> Very well-writen driver. Only a few small things in the following.
>
> Sam
>
> >
> > Signed-off-by: Konrad Dybcio 
> > ---
> >  drivers/gpu/drm/panel/Kconfig   |   9 +
> >  drivers/gpu/drm/panel/Makefile  |   1 +
> >  drivers/gpu/drm/panel/panel-tm5p5-nt35596.c | 366 
> >  3 files changed, 376 insertions(+)
> >  create mode 100644 drivers/gpu/drm/panel/panel-tm5p5-nt35596.c
> >
> > diff --git a/drivers/gpu/drm/panel/Kconfig
> b/drivers/gpu/drm/panel/Kconfig
> > index a1723c1b5fbf8..6ff892334ac4b 100644
> > --- a/drivers/gpu/drm/panel/Kconfig
> > +++ b/drivers/gpu/drm/panel/Kconfig
> > @@ -401,6 +401,15 @@ config DRM_PANEL_SONY_ACX565AKM
> > Say Y here if you want to enable support for the Sony ACX565AKM
> > 800x600 3.5" panel (found on the Nokia N900).
> >
> > +config DRM_PANEL_TM5P5_NT35596
> > + tristate "TM5P5 NT35596 panel"
> > + depends on GPIOLIB && OF
> > + depends on DRM_MIPI_DSI
> > + help
> > +   Say Y here if you want to enable support for the TMP5P5
> > +   NT35596 1080x1920 video mode panel as found in some Asus
> > +   Zenfone 2 Laser Z00T devices.
> > +
> >  config DRM_PANEL_TPO_TD028TTEC1
> >   tristate "Toppoly (TPO) TD028TTEC1 panel driver"
> >   depends on OF && SPI
> > diff --git a/drivers/gpu/drm/panel/Makefile
> b/drivers/gpu/drm/panel/Makefile
> > index 96a883cd66305..4fc7e00b18502 100644
> > --- a/drivers/gpu/drm/panel/Makefile
> > +++ b/drivers/gpu/drm/panel/Makefile
> > @@ -42,6 +42,7 @@ obj-$(CONFIG_DRM_PANEL_SITRONIX_ST7701) +=
> panel-sitronix-st7701.o
> >  obj-$(CONFIG_DRM_PANEL_SITRONIX_ST7789V) += panel-sitronix-st7789v.o
> >  obj-$(CONFIG_DRM_PANEL_SONY_ACX424AKP) += panel-sony-acx424akp.o
> >  obj-$(CONFIG_DRM_PANEL_SONY_ACX565AKM) += panel-sony-acx565akm.o
> > +obj-$(CONFIG_DRM_PANEL_TM5P5_NT35596) += panel-tm5p5-nt35596.o
> >  obj-$(CONFIG_DRM_PANEL_TPO_TD028TTEC1) += panel-tpo-td028ttec1.o
> >  obj-$(CONFIG_DRM_PANEL_TPO_TD043MTEA1) += panel-tpo-td043mtea1.o
> >  obj-$(CONFIG_DRM_PANEL_TPO_TPG110) += panel-tpo-tpg110.o
> > diff --git a/drivers/gpu/drm/panel/panel-tm5p5-nt35596.c
> b/drivers/gpu/drm/panel/panel-tm5p5-nt35596.c
> > new file mode 100644
> > index 0..c361ab76812b8
> > --- /dev/null
> > +++ b/drivers/gpu/drm/panel/panel-tm5p5-nt35596.c
> > @@ -0,0 +1,366 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include 
> > +#include 
> > +#include 
> > +
> > +struct tm5p5_nt35596 {
> > + struct drm_panel panel;
> > + struct mipi_dsi_device *dsi;
> > + struct regulator_bulk_data supplies[2];
> > + struct gpio_desc *reset_gpio;
> > + bool prepared;
> > +};
> > +
> > +static inline struct tm5p5_nt35596 *to_tm5p5_nt35596(struct drm_panel
> *panel)
> > +{
> > + return container_of(panel, struct tm5p5_nt35596, panel);
> > +}
> > +
> > +#define dsi_generic_write_seq(dsi, seq...) do {
>   \
> > + static const u8 d[] = { seq };  \
> > + int ret;\
> > + ret = mipi_dsi_generic_write(dsi, d, ARRAY_SIZE(d));\
> > + if (ret < 0)\
> > + return ret; \
> > + } while (0)
> > +
> > +#define dsi_dcs_write_seq(dsi, seq...) do {  \
> > + static const u8 d[] = { seq };  \
> > + int ret;\
> > + ret = mipi_dsi_dcs_write_buffer(dsi, d, ARRAY_SIZE(d)); \
> > + if (ret < 0)\
> > + return ret; \
> > + } while (0)
> > +
> > +static void tm5p5_nt35596_reset(struct tm5p5_nt35596 *ctx)
> > +{
> > + gpiod_set_value_cansleep(ctx->reset_gpio, 1);
> > + usleep_range(1000, 2000);
> > + gpiod_set_value_cansleep(ctx->reset_gpio, 0);
> > + usleep_range(1000, 2000);
> > + gpiod_set_value_cansleep(ctx->reset_gpio, 1);
> > + usleep_range(15000, 16000);
> > +}
> > +
> > +static int tm5p5_nt35596_on(struct 

Re: [PATCH 1/2] drivers: drm: panel: Add TM5P5 NT35596 panel driver

2020-05-03 Thread Konrad Dybcio
Disclaimer: sorry if you see it twice, I got a response saying something
was wrong when I first tried to send this.

Hi,

Thanks for your review. I'll send a v2 soon, however we need to solve the
compat string issue first.

How should I document tm5p5? I think it's rather some kind of a model no.
along with the nt35596 IC than a vendor name.. Or should we call it
something like "unknown,tm5p5-nt35596", perhaps "asus,z00t-tm5p5-n35596"
[1]?

[1] Z00T is the model number of the smartphone that uses this panel

Konrad
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] drm/meson: viu: fix setting the OSD burst length in VIU_OSD1_FIFO_CTRL_STAT

2020-05-03 Thread Martin Blumenstingl
Hi Neil,

On Tue, Apr 28, 2020 at 10:38 AM Neil Armstrong  wrote:
[...]
> > @@ -444,9 +437,9 @@ void meson_viu_init(struct meson_drm *priv)
> >   VIU_OSD_FIFO_LIMITS(2);  /* fifo_lim: 2*16=32 */
> >
> >   if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A))
> > - reg |= meson_viu_osd_burst_length_reg(32);
> > + reg |= VIU_OSD_BURST_LENGTH_32;
> >   else
> > - reg |= meson_viu_osd_burst_length_reg(64);
> > + reg |= VIU_OSD_BURST_LENGTH_64;
> >
> >   writel_relaxed(reg, priv->io_base + _REG(VIU_OSD1_FIFO_CTRL_STAT));
> >   writel_relaxed(reg, priv->io_base + _REG(VIU_OSD2_FIFO_CTRL_STAT));
> >
>
> Thanks,
> Will run some tests !
Does this fix/improve anything for you?
On the 32-bit SoCs kmscube is not smooth (neither on the CVBS nor on
the HDMI output) with a burst length of 24 (which was the old
"accidentally used" value).


Martin
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v4 4/6] drm/msm: dsi: Use OPP API to set clk/perf state

2020-05-03 Thread Rajendra Nayak
On SDM845 DSI needs to express a perforamnce state
requirement on a power domain depending on the clock rates.
Use OPP table from DT to register with OPP framework and use
dev_pm_opp_set_rate() to set the clk/perf state.

Signed-off-by: Rajendra Nayak 
Cc: Rob Clark 
Cc: Sean Paul 
Cc: dri-devel@lists.freedesktop.org
---
 drivers/gpu/drm/msm/dsi/dsi.h  |  2 ++
 drivers/gpu/drm/msm/dsi/dsi_cfg.c  |  4 +--
 drivers/gpu/drm/msm/dsi/dsi_host.c | 58 ++
 3 files changed, 62 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi.h b/drivers/gpu/drm/msm/dsi/dsi.h
index 4de771d..ba7583c 100644
--- a/drivers/gpu/drm/msm/dsi/dsi.h
+++ b/drivers/gpu/drm/msm/dsi/dsi.h
@@ -180,10 +180,12 @@ int msm_dsi_runtime_suspend(struct device *dev);
 int msm_dsi_runtime_resume(struct device *dev);
 int dsi_link_clk_set_rate_6g(struct msm_dsi_host *msm_host);
 int dsi_link_clk_set_rate_v2(struct msm_dsi_host *msm_host);
+int dsi_link_clk_set_rate_6g_v2(struct msm_dsi_host *msm_host);
 int dsi_link_clk_enable_6g(struct msm_dsi_host *msm_host);
 int dsi_link_clk_enable_v2(struct msm_dsi_host *msm_host);
 void dsi_link_clk_disable_6g(struct msm_dsi_host *msm_host);
 void dsi_link_clk_disable_v2(struct msm_dsi_host *msm_host);
+void dsi_link_clk_disable_6g_v2(struct msm_dsi_host *msm_host);
 int dsi_tx_buf_alloc_6g(struct msm_dsi_host *msm_host, int size);
 int dsi_tx_buf_alloc_v2(struct msm_dsi_host *msm_host, int size);
 void *dsi_tx_buf_get_6g(struct msm_dsi_host *msm_host);
diff --git a/drivers/gpu/drm/msm/dsi/dsi_cfg.c 
b/drivers/gpu/drm/msm/dsi/dsi_cfg.c
index 813d69d..773c4fe 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_cfg.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_cfg.c
@@ -210,9 +210,9 @@ static const struct msm_dsi_host_cfg_ops 
msm_dsi_6g_host_ops = {
 };
 
 static const struct msm_dsi_host_cfg_ops msm_dsi_6g_v2_host_ops = {
-   .link_clk_set_rate = dsi_link_clk_set_rate_6g,
+   .link_clk_set_rate = dsi_link_clk_set_rate_6g_v2,
.link_clk_enable = dsi_link_clk_enable_6g,
-   .link_clk_disable = dsi_link_clk_disable_6g,
+   .link_clk_disable = dsi_link_clk_disable_6g_v2,
.clk_init_ver = dsi_clk_init_6g_v2,
.tx_buf_alloc = dsi_tx_buf_alloc_6g,
.tx_buf_get = dsi_tx_buf_get_6g,
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c 
b/drivers/gpu/drm/msm/dsi/dsi_host.c
index 11ae5b8..d5f3dcd 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -14,6 +14,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -111,6 +112,9 @@ struct msm_dsi_host {
struct clk *pixel_clk_src;
struct clk *byte_intf_clk;
 
+   struct opp_table *opp_table;
+   bool has_opp_table;
+
u32 byte_clk_rate;
u32 pixel_clk_rate;
u32 esc_clk_rate;
@@ -537,6 +541,38 @@ int dsi_link_clk_set_rate_6g(struct msm_dsi_host *msm_host)
return 0;
 }
 
+int dsi_link_clk_set_rate_6g_v2(struct msm_dsi_host *msm_host)
+{
+   int ret;
+   struct device *dev = _host->pdev->dev;
+
+   DBG("Set clk rates: pclk=%d, byteclk=%d",
+   msm_host->mode->clock, msm_host->byte_clk_rate);
+
+   ret = dev_pm_opp_set_rate(dev, msm_host->byte_clk_rate);
+   if (ret) {
+   pr_err("%s: dev_pm_opp_set_rate failed %d\n", __func__, ret);
+   return ret;
+   }
+
+   ret = clk_set_rate(msm_host->pixel_clk, msm_host->pixel_clk_rate);
+   if (ret) {
+   pr_err("%s: Failed to set rate pixel clk, %d\n", __func__, ret);
+   return ret;
+   }
+
+   if (msm_host->byte_intf_clk) {
+   ret = clk_set_rate(msm_host->byte_intf_clk,
+  msm_host->byte_clk_rate / 2);
+   if (ret) {
+   pr_err("%s: Failed to set rate byte intf clk, %d\n",
+  __func__, ret);
+   return ret;
+   }
+   }
+
+   return 0;
+}
 
 int dsi_link_clk_enable_6g(struct msm_dsi_host *msm_host)
 {
@@ -665,6 +701,13 @@ void dsi_link_clk_disable_6g(struct msm_dsi_host *msm_host)
clk_disable_unprepare(msm_host->byte_clk);
 }
 
+void dsi_link_clk_disable_6g_v2(struct msm_dsi_host *msm_host)
+{
+   /* Drop the performance state vote */
+   dev_pm_opp_set_rate(_host->pdev->dev, 0);
+   dsi_link_clk_disable_6g(msm_host);
+}
+
 void dsi_link_clk_disable_v2(struct msm_dsi_host *msm_host)
 {
clk_disable_unprepare(msm_host->pixel_clk);
@@ -1879,6 +1922,18 @@ int msm_dsi_host_init(struct msm_dsi *msm_dsi)
goto fail;
}
 
+   msm_host->opp_table = dev_pm_opp_set_clkname(>dev, "byte");
+   if (IS_ERR(msm_host->opp_table))
+   return PTR_ERR(msm_host->opp_table);
+   /* OPP table is optional */
+   ret = dev_pm_opp_of_add_table(>dev);
+   if (!ret) {
+   msm_host->has_opp_table = true;
+   } else if (ret != -ENODEV) 

[PATCH v4 3/6] drm/msm/dpu: Use OPP API to set clk/perf state

2020-05-03 Thread Rajendra Nayak
On some qualcomm platforms DPU needs to express a performance state
requirement on a power domain depending on the clock rates.
Use OPP table from DT to register with OPP framework and use
dev_pm_opp_set_rate() to set the clk/perf state.

Signed-off-by: Rajendra Nayak 
Reviewed-by: Rob Clark 
Reviewed-by: Matthias Kaehlcke 
Cc: Rob Clark 
Cc: Sean Paul 
Cc: dri-devel@lists.freedesktop.org
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c |  3 ++-
 drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c   | 33 ---
 drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.h   |  1 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c   | 25 +++-
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h   |  4 
 5 files changed, 30 insertions(+), 36 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 11f2beb..fe5717df 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
@@ -7,6 +7,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -239,7 +240,7 @@ static int _dpu_core_perf_set_core_clk_rate(struct dpu_kms 
*kms, u64 rate)
rate = core_clk->max_rate;
 
core_clk->rate = rate;
-   return msm_dss_clk_set_rate(core_clk, 1);
+   return dev_pm_opp_set_rate(>pdev->dev, core_clk->rate);
 }
 
 static u64 _dpu_core_perf_get_core_clk_rate(struct dpu_kms *kms)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c
index 078afc5..b9a4bf8 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c
@@ -51,39 +51,6 @@ int msm_dss_get_clk(struct device *dev, struct dss_clk 
*clk_arry, int num_clk)
return rc;
 }
 
-int msm_dss_clk_set_rate(struct dss_clk *clk_arry, int num_clk)
-{
-   int i, rc = 0;
-
-   for (i = 0; i < num_clk; i++) {
-   if (clk_arry[i].clk) {
-   if (clk_arry[i].type != DSS_CLK_AHB) {
-   DEV_DBG("%pS->%s: '%s' rate %ld\n",
-   __builtin_return_address(0), __func__,
-   clk_arry[i].clk_name,
-   clk_arry[i].rate);
-   rc = clk_set_rate(clk_arry[i].clk,
-   clk_arry[i].rate);
-   if (rc) {
-   DEV_ERR("%pS->%s: %s failed. rc=%d\n",
-   __builtin_return_address(0),
-   __func__,
-   clk_arry[i].clk_name, rc);
-   break;
-   }
-   }
-   } else {
-   DEV_ERR("%pS->%s: '%s' is not available\n",
-   __builtin_return_address(0), __func__,
-   clk_arry[i].clk_name);
-   rc = -EPERM;
-   break;
-   }
-   }
-
-   return rc;
-}
-
 int msm_dss_enable_clk(struct dss_clk *clk_arry, int num_clk, int enable)
 {
int i, rc = 0;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.h 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.h
index e6b5c77..ca25c90 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.h
@@ -33,7 +33,6 @@ struct dss_module_power {
 
 int msm_dss_get_clk(struct device *dev, struct dss_clk *clk_arry, int num_clk);
 void msm_dss_put_clk(struct dss_clk *clk_arry, int num_clk);
-int msm_dss_clk_set_rate(struct dss_clk *clk_arry, int num_clk);
 int msm_dss_enable_clk(struct dss_clk *clk_arry, int num_clk, int enable);
 int msm_dss_parse_clock(struct platform_device *pdev,
struct dss_module_power *mp);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index ce19f1d..f16d715 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -10,6 +10,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -1033,11 +1034,23 @@ static int dpu_bind(struct device *dev, struct device 
*master, void *data)
if (!dpu_kms)
return -ENOMEM;
 
+   dpu_kms->opp_table = dev_pm_opp_set_clkname(dev, "core");
+   if (IS_ERR(dpu_kms->opp_table))
+   return PTR_ERR(dpu_kms->opp_table);
+   /* OPP table is optional */
+   ret = dev_pm_opp_of_add_table(dev);
+   if (!ret) {
+   dpu_kms->has_opp_table = true;
+   } else if (ret != -ENODEV) {
+   dev_err(dev, "invalid OPP table in device tree\n");
+   return ret;
+   }
+
mp = _kms->mp;
ret = msm_dss_parse_clock(pdev, mp);
if (ret) {

Re: [PATCH 06/10] drm/client: Add a way to set modeset, properties and rotation

2020-05-03 Thread Noralf Trønnes


Den 03.05.2020 10.47, skrev Sam Ravnborg:
> Hi Noralf.
> 
> Some comments in the following - partly because I still do not fully
> grasp modeset etc.

I still don't fully understand it either. I have wandered inside the
atomic machinery countless times to track what happens to this or that
state/property and 2 weeks later the insight is gone. It just doesn't
stick. What makes it difficult for me I believe is that I have never
looked much at the userspace side, how this is all used.

> 
>   Sam
> 
> On Wed, Apr 29, 2020 at 02:48:26PM +0200, Noralf Trønnes wrote:
>> This adds functions for clients that need more control over the
>> configuration than what's setup by drm_client_modeset_probe().
>> Connector, fb and display mode can be set using drm_client_modeset_set().
>> Plane rotation can be set using drm_client_modeset_set_rotation() and
>> other properties using drm_client_modeset_set_property(). Property
>> setting is only implemented for atomic drivers.
>>
>> Signed-off-by: Noralf Trønnes 
>> ---
>>  drivers/gpu/drm/drm_client_modeset.c | 139 +++
>>  include/drm/drm_client.h |  38 +++-
>>  2 files changed, 176 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/drm_client_modeset.c 
>> b/drivers/gpu/drm/drm_client_modeset.c
>> index 177158ff2a40..1eef6869cae1 100644
>> --- a/drivers/gpu/drm/drm_client_modeset.c
>> +++ b/drivers/gpu/drm/drm_client_modeset.c
>> @@ -83,6 +83,10 @@ static void drm_client_modeset_release(struct 
>> drm_client_dev *client)
>>  }
>>  modeset->num_connectors = 0;
>>  }
>> +
>> +kfree(client->properties);
>> +client->properties = NULL;
>> +client->num_properties = 0;
> 
> I failed to see why this code is in drm_client_modeset_release()
> and not in drm_client_modeset_free().
> In other words - why do we need to free properties in 
> drm_client_modeset_probe()
> which is the only other user of drm_client_modeset_release().

This is legacy behaviour that was moved over from drm_fb_helper. It
cleared the modeset before probing for a new useable modeset. So the
same applies when the client sets the modeset itself, we need to clear
out the previous modeset.

If drm_client was written from scratch, the state/modeset would not be
stored in drm_client_dev, but stored by the client.

With the current situation a client can't check a configuration while
retaining a working config/state. It looses the working state setting up
one for testing.

This is not a problem for my usecase, but it can be for a future
usecase. Since it works for me I decided to keep it as-is instead of
trying to fix this without knowing what the future usecase might be.

> 
>>  }
>>  
>>  void drm_client_modeset_free(struct drm_client_dev *client)
>> @@ -879,6 +883,132 @@ int drm_client_modeset_probe(struct drm_client_dev 
>> *client, unsigned int width,
>>  }
>>  EXPORT_SYMBOL(drm_client_modeset_probe);
>>  
>> +/**
>> + * drm_client_modeset_set() - Set modeset configuration
>> + * @client: DRM client
>> + * @connector: Connector
>> + * @mode: Display mode
>> + * @fb: Framebuffer
>> + *
>> + * This function releases any current modeset info, including properties, 
>> and
>> + * sets the new modeset in the client's modeset array.
>> + *
>> + * Returns:
>> + * Zero on success or negative error code on failure.
>> + */
>> +int drm_client_modeset_set(struct drm_client_dev *client, struct 
>> drm_connector *connector,
>> +   struct drm_display_mode *mode, struct 
>> drm_framebuffer *fb)
>> +{
>> +struct drm_mode_set *modeset;
>> +int ret = -ENOENT;
>> +
> Need to check if atomic is supported?

Not needed here because drm_client_modeset_commit() supports
legacy/non-atomic.

> Or maybe I just do not get all this atomic stuff yet..
> 
>> +mutex_lock(>modeset_mutex);
>> +
>> +drm_client_modeset_release(client);
> If the check below fails - is it then correct to release modeset?
>> +
>> +if (!connector || !mode || !fb) {
>> +ret = 0;
> Error out, it is not a success if we cannot do anything?

Ah, I haven't documented this. This actually clears the modesets. If
this is commited, it results in all outputs being turned off.
Maybe I should do a drm_client_modeset_clear() instead.

> 
>> +goto unlock;
>> +}
>> +
>> +drm_client_for_each_modeset(modeset, client) {
>> +if (!connector_has_possible_crtc(connector, modeset->crtc))
>> +continue;
>> +
>> +modeset->mode = drm_mode_duplicate(client->dev, mode);
>> +if (!modeset->mode) {
>> +ret = -ENOMEM;
>> +break;
>> +}
>> +
>> +drm_mode_set_crtcinfo(modeset->mode, CRTC_INTERLACE_HALVE_V);
>> +
>> +drm_connector_get(connector);
>> +modeset->connectors[modeset->num_connectors++] = connector;
>> +
>> +modeset->fb = fb;
>> +ret = 0;
>> +break;
>> 

Re: [PATCH 07/10] drm/format-helper: Add drm_fb_swab()

2020-05-03 Thread Sam Ravnborg
Hi Noralf

On Wed, Apr 29, 2020 at 02:48:27PM +0200, Noralf Trønnes wrote:
> This replaces drm_fb_swab16() with drm_fb_swab() supporting 16 and 32-bit.
> Also make pixel line caching optional.
> 
> Signed-off-by: Noralf Trønnes 
A couple of nits, with these considered:
Reviewed-by: Sam Ravnborg 

> ---
>  drivers/gpu/drm/drm_format_helper.c | 61 +++--
>  drivers/gpu/drm/drm_mipi_dbi.c  |  2 +-
>  include/drm/drm_format_helper.h |  4 +-
>  3 files changed, 44 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_format_helper.c 
> b/drivers/gpu/drm/drm_format_helper.c
> index 0897cb9aeaff..5c147c49668c 100644
> --- a/drivers/gpu/drm/drm_format_helper.c
> +++ b/drivers/gpu/drm/drm_format_helper.c
> @@ -79,39 +79,60 @@ void drm_fb_memcpy_dstclip(void __iomem *dst, void *vaddr,
>  EXPORT_SYMBOL(drm_fb_memcpy_dstclip);
>  
>  /**
> - * drm_fb_swab16 - Swap bytes into clip buffer
> - * @dst: RGB565 destination buffer
> - * @vaddr: RGB565 source buffer
> + * drm_fb_swab - Swap bytes into clip buffer
> + * @dst: Destination buffer
> + * @src: Source buffer
>   * @fb: DRM framebuffer
>   * @clip: Clip rectangle area to copy
> + * @cached: Source buffer is mapped cached (eg. not write-combined)
> + *
> + * If @cached is false a temporary buffer is used to cache one pixel line at 
> a
> + * time to speed up slow uncached reads.
> + *
> + * This function does not apply clipping on dst, i.e. the destination
> + * is a small buffer containing the clip rect only.
>   */
> -void drm_fb_swab16(u16 *dst, void *vaddr, struct drm_framebuffer *fb,
> -struct drm_rect *clip)
> +void drm_fb_swab(void *dst, void *src, struct drm_framebuffer *fb,
> +  struct drm_rect *clip, bool cached)
>  {
> - size_t len = (clip->x2 - clip->x1) * sizeof(u16);
> + u8 cpp = fb->format->cpp[0];
Use of format->cpp is deprecated, should be char_per_block according to
the comment in drm_fourcc.h

> + size_t len = drm_rect_width(clip) * cpp;
> + u16 *src16, *dst16 = dst;
> + u32 *src32, *dst32 = dst;
>   unsigned int x, y;
> - u16 *src, *buf;
> + void *buf = NULL;
>  
> - /*
> -  * The cma memory is write-combined so reads are uncached.
> -  * Speed up by fetching one line at a time.
> -  */
> - buf = kmalloc(len, GFP_KERNEL);
> - if (!buf)
> + if (WARN_ON_ONCE(cpp == 1))
>   return;
Or cpp != 2 && cpp != 4?
>  
> + if (!cached)
> + buf = kmalloc(len, GFP_KERNEL);
> +
> + src += clip_offset(clip, fb->pitches[0], cpp);
Good that drm_rect_width() and clip_offset() are used,
replacing open-coded variants.

> +
>   for (y = clip->y1; y < clip->y2; y++) {
> - src = vaddr + (y * fb->pitches[0]);
> - src += clip->x1;
> - memcpy(buf, src, len);
> - src = buf;
> - for (x = clip->x1; x < clip->x2; x++)
> - *dst++ = swab16(*src++);
> + if (buf) {
> + memcpy(buf, src, len);
> + src16 = buf;
> + src32 = buf;
> + } else {
> + src16 = src;
> + src32 = src;
> + }
> +
> + for (x = clip->x1; x < clip->x2; x++) {
> + if (cpp == 4)
> + *dst32++ = swab32(*src32++);
> + else
> + *dst16++ = swab16(*src16++);
> + }
> +
> + src += fb->pitches[0];
>   }
>  
>   kfree(buf);
>  }
> -EXPORT_SYMBOL(drm_fb_swab16);
> +EXPORT_SYMBOL(drm_fb_swab);
>  
>  static void drm_fb_xrgb_to_rgb565_line(u16 *dbuf, u32 *sbuf,
>  unsigned int pixels,
> diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c
> index 16bff1be4b8a..bfefbcb69287 100644
> --- a/drivers/gpu/drm/drm_mipi_dbi.c
> +++ b/drivers/gpu/drm/drm_mipi_dbi.c
> @@ -217,7 +217,7 @@ int mipi_dbi_buf_copy(void *dst, struct drm_framebuffer 
> *fb,
>   switch (fb->format->format) {
>   case DRM_FORMAT_RGB565:
>   if (swap)
> - drm_fb_swab16(dst, src, fb, clip);
> + drm_fb_swab(dst, src, fb, clip, !import_attach);
>   else
>   drm_fb_memcpy(dst, src, fb, clip);
>   break;
> diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h
> index ac220aa1a245..5f9e37032468 100644
> --- a/include/drm/drm_format_helper.h
> +++ b/include/drm/drm_format_helper.h
> @@ -14,8 +14,8 @@ void drm_fb_memcpy(void *dst, void *vaddr, struct 
> drm_framebuffer *fb,
>  void drm_fb_memcpy_dstclip(void __iomem *dst, void *vaddr,
>  struct drm_framebuffer *fb,
>  struct drm_rect *clip);
> -void drm_fb_swab16(u16 *dst, void *vaddr, struct drm_framebuffer *fb,
> -struct drm_rect *clip);
> +void 

Re: [PATCH 05/10] drm/client: Add drm_client_modeset_check()

2020-05-03 Thread Noralf Trønnes


Den 03.05.2020 10.03, skrev Sam Ravnborg:
> Hi Noralf.
> 
> On Wed, Apr 29, 2020 at 02:48:25PM +0200, Noralf Trønnes wrote:
>> Add a way for client to check the configuration before comitting.
>>
>> Signed-off-by: Noralf Trønnes 
> Two small ntis. With these addressed:
> Reviewed-by: Sam Ravnborg 
>> ---
>>  drivers/gpu/drm/drm_client_modeset.c | 35 
>>  include/drm/drm_client.h |  1 +
>>  2 files changed, 32 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_client_modeset.c 
>> b/drivers/gpu/drm/drm_client_modeset.c
>> index 7443114bd713..177158ff2a40 100644
>> --- a/drivers/gpu/drm/drm_client_modeset.c
>> +++ b/drivers/gpu/drm/drm_client_modeset.c
>> @@ -966,7 +966,7 @@ bool drm_client_rotation(struct drm_mode_set *modeset, 
>> unsigned int *rotation)
>>  }
>>  EXPORT_SYMBOL(drm_client_rotation);
>>  
>> -static int drm_client_modeset_commit_atomic(struct drm_client_dev *client, 
>> bool active)
>> +static int drm_client_modeset_commit_atomic(struct drm_client_dev *client, 
>> bool active, bool check)
>>  {
>>  struct drm_device *dev = client->dev;
>>  struct drm_plane *plane;
>> @@ -1033,7 +1033,10 @@ static int drm_client_modeset_commit_atomic(struct 
>> drm_client_dev *client, bool
>>  }
>>  }
>>  
>> -ret = drm_atomic_commit(state);
>> +if (check)
>> +ret = drm_atomic_check_only(state);
>> +else
>> +ret = drm_atomic_commit(state);
>>  
>>  out_state:
>>  if (ret == -EDEADLK)
>> @@ -1094,6 +1097,30 @@ static int drm_client_modeset_commit_legacy(struct 
>> drm_client_dev *client)
>>  return ret;
>>  }
>>  
>> +/**
>> + * drm_client_modeset_check() - Check CRTC configuration
> This part of the comment does not match the description below.
> 
> 
>> + * @client: DRM client
>> + *
>> + * Check modeset configuration.
>> + *
>> + * Returns:
>> + * Zero on success or negative error code on failure.
>> + */
>> +int drm_client_modeset_check(struct drm_client_dev *client)
>> +{
>> +int ret;
>> +
>> +if (!drm_drv_uses_atomic_modeset(client->dev))
>> +return 0;
> If client does not use atomic the check should fail - no?

Returning an error here would result in the client not working with
non-atomic drivers which AFAIK doesn't have a way to check the state
before hand. The client have to commit the state/configuration to find
out if it is good.

Noralf.

> 
>> +
>> +mutex_lock(>modeset_mutex);
>> +ret = drm_client_modeset_commit_atomic(client, true, true);
>> +mutex_unlock(>modeset_mutex);
>> +
>> +return ret;
>> +}
>> +EXPORT_SYMBOL(drm_client_modeset_check);
>> +
>>  /**
>>   * drm_client_modeset_commit_locked() - Force commit CRTC configuration
>>   * @client: DRM client
>> @@ -1112,7 +1139,7 @@ int drm_client_modeset_commit_locked(struct 
>> drm_client_dev *client)
>>  
>>  mutex_lock(>modeset_mutex);
>>  if (drm_drv_uses_atomic_modeset(dev))
>> -ret = drm_client_modeset_commit_atomic(client, true);
>> +ret = drm_client_modeset_commit_atomic(client, true, false);
>>  else
>>  ret = drm_client_modeset_commit_legacy(client);
>>  mutex_unlock(>modeset_mutex);
>> @@ -1188,7 +1215,7 @@ int drm_client_modeset_dpms(struct drm_client_dev 
>> *client, int mode)
>>  
>>  mutex_lock(>modeset_mutex);
>>  if (drm_drv_uses_atomic_modeset(dev))
>> -ret = drm_client_modeset_commit_atomic(client, mode == 
>> DRM_MODE_DPMS_ON);
>> +ret = drm_client_modeset_commit_atomic(client, mode == 
>> DRM_MODE_DPMS_ON, false);
>>  else
>>  drm_client_modeset_dpms_legacy(client, mode);
>>  mutex_unlock(>modeset_mutex);
>> diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h
>> index 6ef5364d6dfb..b6ffa4863e45 100644
>> --- a/include/drm/drm_client.h
>> +++ b/include/drm/drm_client.h
>> @@ -164,6 +164,7 @@ int drm_client_modeset_create(struct drm_client_dev 
>> *client);
>>  void drm_client_modeset_free(struct drm_client_dev *client);
>>  int drm_client_modeset_probe(struct drm_client_dev *client, unsigned int 
>> width, unsigned int height);
>>  bool drm_client_rotation(struct drm_mode_set *modeset, unsigned int 
>> *rotation);
>> +int drm_client_modeset_check(struct drm_client_dev *client);
>>  int drm_client_modeset_commit_locked(struct drm_client_dev *client);
>>  int drm_client_modeset_commit(struct drm_client_dev *client);
>>  int drm_client_modeset_dpms(struct drm_client_dev *client, int mode);
>> -- 
>> 2.23.0
>>
>> ___
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 04/10] drm/client: Add drm_client_framebuffer_flush()

2020-05-03 Thread Noralf Trønnes


Den 03.05.2020 09.48, skrev Sam Ravnborg:
> Hi Noralf.
> 
> On Wed, Apr 29, 2020 at 02:48:24PM +0200, Noralf Trønnes wrote:
>> Some drivers need explicit flushing of buffer changes, add a function
>> that does that.
> I trust you on this.

All drivers in tiny/ and at least udl need to be told to flush changes.
For userspace this happens either by calling DRM_IOCTL_MODE_DIRTYFB or
doing a page/buffer flip DRM_IOCTL_MODE_PAGE_FLIP or do a
DRM_IOCTL_MODE_ATOMIC (can contain damage report using plane property
FB_DAMAGE_CLIPS). For drivers that use drm_gem_fb_create_with_dirty()
and the drm_damage_helper (all of them now I think) this will result in
an atomic commit. The driver can use drm_atomic_helper_damage_merged()
to get the combined damage rectangle.

Noralf.

> 
>>
>> Signed-off-by: Noralf Trønnes 
> Some bikeshedding below. Either way:
> Reviewed-by: Sam Ravnborg 
> 
>> ---
>>  drivers/gpu/drm/drm_client.c | 31 +++
>>  include/drm/drm_client.h |  1 +
>>  2 files changed, 32 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
>> index cb5ee9f1ffaa..8dbc2ecdcaea 100644
>> --- a/drivers/gpu/drm/drm_client.c
>> +++ b/drivers/gpu/drm/drm_client.c
>> @@ -483,6 +483,37 @@ void drm_client_framebuffer_delete(struct 
>> drm_client_buffer *buffer)
>>  }
>>  EXPORT_SYMBOL(drm_client_framebuffer_delete);
>>  
>> +/**
>> + * drm_client_framebuffer_flush - Manually flush client framebuffer
>> + * @buffer: DRM client buffer (can be NULL)
>> + * @rect: Damage rectangle (if NULL flushes all)
>> + *
>> + * This calls _framebuffer_funcs->dirty (if present) to flush buffer 
>> changes
>> + * for drivers that need it.
>> + *
>> + * Returns:
>> + * Zero on success or negative error code on failure.
>> + */
> 
> Alternative proposal - that I think is simpler.
> But both variants works for me.
>> +int drm_client_framebuffer_flush(struct drm_client_buffer *buffer, struct 
>> drm_rect *rect)
>> +{
>   struct drm_framebuffer_funcs *funcs;
>   struct drm_clip_rect clip;
>> +
>> +if (!buffer || !buffer->fb || !buffer->fb->funcs->dirty)
>> +return 0;
>   funcs = buffer->fb->funcs;
>> +
>> +if (rect) {
>> +clip.x1 = rect->x1;
>> +clip.y1 = rect->y1;
>> +clip.x2 = rect->x2;
>> +clip.y2 = rect->y2;
>   return funcs->dirty(buffer->fb, buffer->client->file,
>   0, 0, , 1);
>> +} else {
>   return funcs->dirty(buffer->fb, buffer->client->file,
>   0, 0, NULL, 0);
>   }
> 
>> +
>> +return buffer->fb->funcs->dirty(buffer->fb, buffer->client->file,
>> +0, 0, clip, clip ? 1 : 0);
>> +}
>> +EXPORT_SYMBOL(drm_client_framebuffer_flush);
>> +
>>  #ifdef CONFIG_DEBUG_FS
>>  static int drm_client_debugfs_internal_clients(struct seq_file *m, void 
>> *data)
>>  {
>> diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h
>> index bbb5689fa9a8..6ef5364d6dfb 100644
>> --- a/include/drm/drm_client.h
>> +++ b/include/drm/drm_client.h
>> @@ -156,6 +156,7 @@ struct drm_client_buffer {
>>  struct drm_client_buffer *
>>  drm_client_framebuffer_create(struct drm_client_dev *client, u32 width, u32 
>> height, u32 format);
>>  void drm_client_framebuffer_delete(struct drm_client_buffer *buffer);
>> +int drm_client_framebuffer_flush(struct drm_client_buffer *buffer, struct 
>> drm_rect *rect);
>>  void *drm_client_buffer_vmap(struct drm_client_buffer *buffer);
>>  void drm_client_buffer_vunmap(struct drm_client_buffer *buffer);
>>  
>> -- 
>> 2.23.0
>>
>> ___
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 06/10] drm/client: Add a way to set modeset, properties and rotation

2020-05-03 Thread Sam Ravnborg
Hi Noralf.

Some comments in the following - partly because I still do not fully
grasp modeset etc.

Sam

On Wed, Apr 29, 2020 at 02:48:26PM +0200, Noralf Trønnes wrote:
> This adds functions for clients that need more control over the
> configuration than what's setup by drm_client_modeset_probe().
> Connector, fb and display mode can be set using drm_client_modeset_set().
> Plane rotation can be set using drm_client_modeset_set_rotation() and
> other properties using drm_client_modeset_set_property(). Property
> setting is only implemented for atomic drivers.
> 
> Signed-off-by: Noralf Trønnes 
> ---
>  drivers/gpu/drm/drm_client_modeset.c | 139 +++
>  include/drm/drm_client.h |  38 +++-
>  2 files changed, 176 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_client_modeset.c 
> b/drivers/gpu/drm/drm_client_modeset.c
> index 177158ff2a40..1eef6869cae1 100644
> --- a/drivers/gpu/drm/drm_client_modeset.c
> +++ b/drivers/gpu/drm/drm_client_modeset.c
> @@ -83,6 +83,10 @@ static void drm_client_modeset_release(struct 
> drm_client_dev *client)
>   }
>   modeset->num_connectors = 0;
>   }
> +
> + kfree(client->properties);
> + client->properties = NULL;
> + client->num_properties = 0;

I failed to see why this code is in drm_client_modeset_release()
and not in drm_client_modeset_free().
In other words - why do we need to free properties in drm_client_modeset_probe()
which is the only other user of drm_client_modeset_release().

>  }
>  
>  void drm_client_modeset_free(struct drm_client_dev *client)
> @@ -879,6 +883,132 @@ int drm_client_modeset_probe(struct drm_client_dev 
> *client, unsigned int width,
>  }
>  EXPORT_SYMBOL(drm_client_modeset_probe);
>  
> +/**
> + * drm_client_modeset_set() - Set modeset configuration
> + * @client: DRM client
> + * @connector: Connector
> + * @mode: Display mode
> + * @fb: Framebuffer
> + *
> + * This function releases any current modeset info, including properties, and
> + * sets the new modeset in the client's modeset array.
> + *
> + * Returns:
> + * Zero on success or negative error code on failure.
> + */
> +int drm_client_modeset_set(struct drm_client_dev *client, struct 
> drm_connector *connector,
> +struct drm_display_mode *mode, struct 
> drm_framebuffer *fb)
> +{
> + struct drm_mode_set *modeset;
> + int ret = -ENOENT;
> +
Need to check if atomic is supported?
Or maybe I just do not get all this atomic stuff yet..

> + mutex_lock(>modeset_mutex);
> +
> + drm_client_modeset_release(client);
If the check below fails - is it then correct to release modeset?
> +
> + if (!connector || !mode || !fb) {
> + ret = 0;
Error out, it is not a success if we cannot do anything?

> + goto unlock;
> + }
> +
> + drm_client_for_each_modeset(modeset, client) {
> + if (!connector_has_possible_crtc(connector, modeset->crtc))
> + continue;
> +
> + modeset->mode = drm_mode_duplicate(client->dev, mode);
> + if (!modeset->mode) {
> + ret = -ENOMEM;
> + break;
> + }
> +
> + drm_mode_set_crtcinfo(modeset->mode, CRTC_INTERLACE_HALVE_V);
> +
> + drm_connector_get(connector);
> + modeset->connectors[modeset->num_connectors++] = connector;
> +
> + modeset->fb = fb;
> + ret = 0;
> + break;
> + }
> +unlock:
> + mutex_unlock(>modeset_mutex);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL(drm_client_modeset_set);
> +
> +/**
> + * drm_client_modeset_set_property() - Set a property on the current 
> configuration
> + * @client: DRM client
> + * @obj: DRM Mode Object
> + * @prop: DRM Property
> + * @value: Property value
> + *
> + * Note: Currently only implemented for atomic drivers.
Are there any reason to in the future support legacy (non-atomic)
drivers.
If not then reword - as the above reads like it is on a TODO list to
support legacy drivers.

> + *
> + * Returns:
> + * Zero on success or negative error code on failure.
> + */
> +int drm_client_modeset_set_property(struct drm_client_dev *client, struct 
> drm_mode_object *obj,
> + struct drm_property *prop, u64 value)
> +{
> + struct drm_client_property *properties;
> + int ret = 0;
> +
> + if (!prop)
> + return -EINVAL;
> +
Need to check if atomic is supported?
Or maybe I just do not get all this atomic stuff yet..

> + mutex_lock(>modeset_mutex);
> +
> + properties = krealloc(client->properties,
> +   (client->num_properties + 1) * 
> sizeof(*properties), GFP_KERNEL);
> + if (!properties) {
> + ret = -ENOMEM;
> + goto unlock;
> + }
> +
> + properties[client->num_properties].obj = obj;
> + properties[client->num_properties].prop = prop;
The 

Re: [PATCH 05/10] drm/client: Add drm_client_modeset_check()

2020-05-03 Thread Sam Ravnborg
Hi Noralf.

On Wed, Apr 29, 2020 at 02:48:25PM +0200, Noralf Trønnes wrote:
> Add a way for client to check the configuration before comitting.
> 
> Signed-off-by: Noralf Trønnes 
Two small ntis. With these addressed:
Reviewed-by: Sam Ravnborg 
> ---
>  drivers/gpu/drm/drm_client_modeset.c | 35 
>  include/drm/drm_client.h |  1 +
>  2 files changed, 32 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_client_modeset.c 
> b/drivers/gpu/drm/drm_client_modeset.c
> index 7443114bd713..177158ff2a40 100644
> --- a/drivers/gpu/drm/drm_client_modeset.c
> +++ b/drivers/gpu/drm/drm_client_modeset.c
> @@ -966,7 +966,7 @@ bool drm_client_rotation(struct drm_mode_set *modeset, 
> unsigned int *rotation)
>  }
>  EXPORT_SYMBOL(drm_client_rotation);
>  
> -static int drm_client_modeset_commit_atomic(struct drm_client_dev *client, 
> bool active)
> +static int drm_client_modeset_commit_atomic(struct drm_client_dev *client, 
> bool active, bool check)
>  {
>   struct drm_device *dev = client->dev;
>   struct drm_plane *plane;
> @@ -1033,7 +1033,10 @@ static int drm_client_modeset_commit_atomic(struct 
> drm_client_dev *client, bool
>   }
>   }
>  
> - ret = drm_atomic_commit(state);
> + if (check)
> + ret = drm_atomic_check_only(state);
> + else
> + ret = drm_atomic_commit(state);
>  
>  out_state:
>   if (ret == -EDEADLK)
> @@ -1094,6 +1097,30 @@ static int drm_client_modeset_commit_legacy(struct 
> drm_client_dev *client)
>   return ret;
>  }
>  
> +/**
> + * drm_client_modeset_check() - Check CRTC configuration
This part of the comment does not match the description below.


> + * @client: DRM client
> + *
> + * Check modeset configuration.
> + *
> + * Returns:
> + * Zero on success or negative error code on failure.
> + */
> +int drm_client_modeset_check(struct drm_client_dev *client)
> +{
> + int ret;
> +
> + if (!drm_drv_uses_atomic_modeset(client->dev))
> + return 0;
If client does not use atomic the check should fail - no?

> +
> + mutex_lock(>modeset_mutex);
> + ret = drm_client_modeset_commit_atomic(client, true, true);
> + mutex_unlock(>modeset_mutex);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL(drm_client_modeset_check);
> +
>  /**
>   * drm_client_modeset_commit_locked() - Force commit CRTC configuration
>   * @client: DRM client
> @@ -1112,7 +1139,7 @@ int drm_client_modeset_commit_locked(struct 
> drm_client_dev *client)
>  
>   mutex_lock(>modeset_mutex);
>   if (drm_drv_uses_atomic_modeset(dev))
> - ret = drm_client_modeset_commit_atomic(client, true);
> + ret = drm_client_modeset_commit_atomic(client, true, false);
>   else
>   ret = drm_client_modeset_commit_legacy(client);
>   mutex_unlock(>modeset_mutex);
> @@ -1188,7 +1215,7 @@ int drm_client_modeset_dpms(struct drm_client_dev 
> *client, int mode)
>  
>   mutex_lock(>modeset_mutex);
>   if (drm_drv_uses_atomic_modeset(dev))
> - ret = drm_client_modeset_commit_atomic(client, mode == 
> DRM_MODE_DPMS_ON);
> + ret = drm_client_modeset_commit_atomic(client, mode == 
> DRM_MODE_DPMS_ON, false);
>   else
>   drm_client_modeset_dpms_legacy(client, mode);
>   mutex_unlock(>modeset_mutex);
> diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h
> index 6ef5364d6dfb..b6ffa4863e45 100644
> --- a/include/drm/drm_client.h
> +++ b/include/drm/drm_client.h
> @@ -164,6 +164,7 @@ int drm_client_modeset_create(struct drm_client_dev 
> *client);
>  void drm_client_modeset_free(struct drm_client_dev *client);
>  int drm_client_modeset_probe(struct drm_client_dev *client, unsigned int 
> width, unsigned int height);
>  bool drm_client_rotation(struct drm_mode_set *modeset, unsigned int 
> *rotation);
> +int drm_client_modeset_check(struct drm_client_dev *client);
>  int drm_client_modeset_commit_locked(struct drm_client_dev *client);
>  int drm_client_modeset_commit(struct drm_client_dev *client);
>  int drm_client_modeset_dpms(struct drm_client_dev *client, int mode);
> -- 
> 2.23.0
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 04/10] drm/client: Add drm_client_framebuffer_flush()

2020-05-03 Thread Sam Ravnborg
Hi Noralf.

On Wed, Apr 29, 2020 at 02:48:24PM +0200, Noralf Trønnes wrote:
> Some drivers need explicit flushing of buffer changes, add a function
> that does that.
I trust you on this.

> 
> Signed-off-by: Noralf Trønnes 
Some bikeshedding below. Either way:
Reviewed-by: Sam Ravnborg 

> ---
>  drivers/gpu/drm/drm_client.c | 31 +++
>  include/drm/drm_client.h |  1 +
>  2 files changed, 32 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
> index cb5ee9f1ffaa..8dbc2ecdcaea 100644
> --- a/drivers/gpu/drm/drm_client.c
> +++ b/drivers/gpu/drm/drm_client.c
> @@ -483,6 +483,37 @@ void drm_client_framebuffer_delete(struct 
> drm_client_buffer *buffer)
>  }
>  EXPORT_SYMBOL(drm_client_framebuffer_delete);
>  
> +/**
> + * drm_client_framebuffer_flush - Manually flush client framebuffer
> + * @buffer: DRM client buffer (can be NULL)
> + * @rect: Damage rectangle (if NULL flushes all)
> + *
> + * This calls _framebuffer_funcs->dirty (if present) to flush buffer 
> changes
> + * for drivers that need it.
> + *
> + * Returns:
> + * Zero on success or negative error code on failure.
> + */

Alternative proposal - that I think is simpler.
But both variants works for me.
> +int drm_client_framebuffer_flush(struct drm_client_buffer *buffer, struct 
> drm_rect *rect)
> +{
struct drm_framebuffer_funcs *funcs;
struct drm_clip_rect clip;
> +
> + if (!buffer || !buffer->fb || !buffer->fb->funcs->dirty)
> + return 0;
funcs = buffer->fb->funcs;
> +
> + if (rect) {
> + clip.x1 = rect->x1;
> + clip.y1 = rect->y1;
> + clip.x2 = rect->x2;
> + clip.y2 = rect->y2;
return funcs->dirty(buffer->fb, buffer->client->file,
0, 0, , 1);
> + } else {
return funcs->dirty(buffer->fb, buffer->client->file,
0, 0, NULL, 0);
}

> +
> + return buffer->fb->funcs->dirty(buffer->fb, buffer->client->file,
> + 0, 0, clip, clip ? 1 : 0);
> +}
> +EXPORT_SYMBOL(drm_client_framebuffer_flush);
> +
>  #ifdef CONFIG_DEBUG_FS
>  static int drm_client_debugfs_internal_clients(struct seq_file *m, void 
> *data)
>  {
> diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h
> index bbb5689fa9a8..6ef5364d6dfb 100644
> --- a/include/drm/drm_client.h
> +++ b/include/drm/drm_client.h
> @@ -156,6 +156,7 @@ struct drm_client_buffer {
>  struct drm_client_buffer *
>  drm_client_framebuffer_create(struct drm_client_dev *client, u32 width, u32 
> height, u32 format);
>  void drm_client_framebuffer_delete(struct drm_client_buffer *buffer);
> +int drm_client_framebuffer_flush(struct drm_client_buffer *buffer, struct 
> drm_rect *rect);
>  void *drm_client_buffer_vmap(struct drm_client_buffer *buffer);
>  void drm_client_buffer_vunmap(struct drm_client_buffer *buffer);
>  
> -- 
> 2.23.0
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 01/10] backlight: Add backlight_device_get_by_name()

2020-05-03 Thread Sam Ravnborg
Hi Noralf.

On Wed, Apr 29, 2020 at 02:48:21PM +0200, Noralf Trønnes wrote:
> Add a way to lookup a backlight device based on its name.
> Will be used by a USB display gadget getting the name from configfs.
> 
> Cc: Lee Jones 
> Cc: Daniel Thompson 
> Cc: Jingoo Han 
> Signed-off-by: Noralf Trønnes 

Simple and well-documented.
Reviewed-by: Sam Ravnborg 
> ---
>  drivers/video/backlight/backlight.c | 21 +
>  include/linux/backlight.h   |  1 +
>  2 files changed, 22 insertions(+)
> 
> diff --git a/drivers/video/backlight/backlight.c 
> b/drivers/video/backlight/backlight.c
> index cac3e35d7630..92d80aa0c0ef 100644
> --- a/drivers/video/backlight/backlight.c
> +++ b/drivers/video/backlight/backlight.c
> @@ -432,6 +432,27 @@ struct backlight_device 
> *backlight_device_get_by_type(enum backlight_type type)
>  }
>  EXPORT_SYMBOL(backlight_device_get_by_type);
>  
> +/**
> + * backlight_device_get_by_name - Get backlight device by name
> + * @name: Device name
> + *
> + * This function looks up a backlight device by its name. It obtains a 
> reference
> + * on the backlight device and it is the caller's responsibility to drop the
> + * reference by calling backlight_put().
> + *
> + * Returns:
> + * A pointer to the backlight device if found, otherwise NULL.
> + */
> +struct backlight_device *backlight_device_get_by_name(const char *name)
> +{
> + struct device *dev;
> +
> + dev = class_find_device_by_name(backlight_class, name);
> +
> + return dev ? to_backlight_device(dev) : NULL;
> +}
> +EXPORT_SYMBOL(backlight_device_get_by_name);
> +
>  /**
>   * backlight_device_unregister - unregisters a backlight device object.
>   * @bd: the backlight device object to be unregistered and freed.
> diff --git a/include/linux/backlight.h b/include/linux/backlight.h
> index c7d6b2e8c3b5..56e4580d4f55 100644
> --- a/include/linux/backlight.h
> +++ b/include/linux/backlight.h
> @@ -190,6 +190,7 @@ extern void backlight_force_update(struct 
> backlight_device *bd,
>  extern int backlight_register_notifier(struct notifier_block *nb);
>  extern int backlight_unregister_notifier(struct notifier_block *nb);
>  extern struct backlight_device *backlight_device_get_by_type(enum 
> backlight_type type);
> +struct backlight_device *backlight_device_get_by_name(const char *name);
>  extern int backlight_device_set_brightness(struct backlight_device *bd, 
> unsigned long brightness);
>  
>  #define to_backlight_device(obj) container_of(obj, struct backlight_device, 
> dev)
> -- 
> 2.23.0
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel