Re: [PATCH 2/2] drm/udl: Use GEM vmap/mmap function from SHMEM helpers
Hi
Am 14.05.20 um 14:44 schrieb Daniel Vetter:
> On Wed, May 13, 2020 at 05:03:12PM +0200, Thomas Zimmermann wrote:
>> The udl driver contains an implementation of GEM vmap and mmap
>> operations that is identical to the common SHMEM helper; except
>> that udl's code does not support writecombine mappings.
>>
>> Convert udl to regular SHMEM helper functions. There's no reason
>> to have udl behave differently from all other SHMEM drivers.
>>
>> Signed-off-by: Thomas Zimmermann
>
> Since I'm not sure how patch 1 works in this series I think keeping the
> udl_driver_gem_create_object but just let it set the map_cached = true; is
> all we need?
Yes. It tested this localy and it worked. The create function can simply
go into udl_drv.c. I'll send out a patch later.
Best regards
Thomas
> -Daniel
>
>> ---
>> drivers/gpu/drm/udl/Makefile | 2 +-
>> drivers/gpu/drm/udl/udl_drv.c | 3 -
>> drivers/gpu/drm/udl/udl_drv.h | 3 -
>> drivers/gpu/drm/udl/udl_gem.c | 106 --
>> 4 files changed, 1 insertion(+), 113 deletions(-)
>> delete mode 100644 drivers/gpu/drm/udl/udl_gem.c
>>
>> diff --git a/drivers/gpu/drm/udl/Makefile b/drivers/gpu/drm/udl/Makefile
>> index b50179bb4de06..24d61f61d7db2 100644
>> --- a/drivers/gpu/drm/udl/Makefile
>> +++ b/drivers/gpu/drm/udl/Makefile
>> @@ -1,4 +1,4 @@
>> # SPDX-License-Identifier: GPL-2.0-only
>> -udl-y := udl_drv.o udl_modeset.o udl_connector.o udl_main.o udl_transfer.o
>> udl_gem.o
>> +udl-y := udl_drv.o udl_modeset.o udl_connector.o udl_main.o udl_transfer.o
>>
>> obj-$(CONFIG_DRM_UDL) := udl.o
>> diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c
>> index d1aa50fd6d65a..cf5b679bf58bb 100644
>> --- a/drivers/gpu/drm/udl/udl_drv.c
>> +++ b/drivers/gpu/drm/udl/udl_drv.c
>> @@ -37,9 +37,6 @@ DEFINE_DRM_GEM_FOPS(udl_driver_fops);
>> static struct drm_driver driver = {
>> .driver_features = DRIVER_ATOMIC | DRIVER_GEM | DRIVER_MODESET,
>>
>> -/* gem hooks */
>> -.gem_create_object = udl_driver_gem_create_object,
>> -
>> .fops = &udl_driver_fops,
>> DRM_GEM_SHMEM_DRIVER_OPS,
>>
>> diff --git a/drivers/gpu/drm/udl/udl_drv.h b/drivers/gpu/drm/udl/udl_drv.h
>> index 2642f94a63fc8..b1461f30780bc 100644
>> --- a/drivers/gpu/drm/udl/udl_drv.h
>> +++ b/drivers/gpu/drm/udl/udl_drv.h
>> @@ -81,9 +81,6 @@ int udl_render_hline(struct drm_device *dev, int log_bpp,
>> struct urb **urb_ptr,
>> const char *front, char **urb_buf_ptr,
>> u32 byte_offset, u32 device_byte_offset, u32 byte_width);
>>
>> -struct drm_gem_object *udl_driver_gem_create_object(struct drm_device *dev,
>> -size_t size);
>> -
>> int udl_drop_usb(struct drm_device *dev);
>>
>> #define CMD_WRITE_RAW8 "\xAF\x60" /**< 8 bit raw write command. */
>> diff --git a/drivers/gpu/drm/udl/udl_gem.c b/drivers/gpu/drm/udl/udl_gem.c
>> deleted file mode 100644
>> index b6e26f98aa0af..0
>> --- a/drivers/gpu/drm/udl/udl_gem.c
>> +++ /dev/null
>> @@ -1,106 +0,0 @@
>> -// SPDX-License-Identifier: GPL-2.0-only
>> -/*
>> - * Copyright (C) 2012 Red Hat
>> - */
>> -
>> -#include
>> -#include
>> -
>> -#include
>> -#include
>> -#include
>> -#include
>> -
>> -#include "udl_drv.h"
>> -
>> -/*
>> - * GEM object funcs
>> - */
>> -
>> -static int udl_gem_object_mmap(struct drm_gem_object *obj,
>> - struct vm_area_struct *vma)
>> -{
>> -int ret;
>> -
>> -ret = drm_gem_shmem_mmap(obj, vma);
>> -if (ret)
>> -return ret;
>> -
>> -vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
>> -if (obj->import_attach)
>> -vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot);
>> -vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
>> -
>> -return 0;
>> -}
>> -
>> -static void *udl_gem_object_vmap(struct drm_gem_object *obj)
>> -{
>> -struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj);
>> -int ret;
>> -
>> -ret = mutex_lock_interruptible(&shmem->vmap_lock);
>> -if (ret)
>> -return ERR_PTR(ret);
>> -
>> -if (shmem->vmap_use_count++ > 0)
>> -goto out;
>> -
>> -ret = drm_gem_shmem_get_pages(shmem);
>> -if (ret)
>> -goto err_zero_use;
>> -
>> -if (obj->import_attach)
>> -shmem->vaddr = dma_buf_vmap(obj->import_attach->dmabuf);
>> -else
>> -shmem->vaddr = vmap(shmem->pages, obj->size >> PAGE_SHIFT,
>> -VM_MAP, PAGE_KERNEL);
>> -
>> -if (!shmem->vaddr) {
>> -DRM_DEBUG_KMS("Failed to vmap pages\n");
>> -ret = -ENOMEM;
>> -goto err_put_pages;
>> -}
>> -
>> -out:
>> -mutex_unlock(&shmem->vmap_lock);
>> -return shmem->vaddr;
>> -
>> -err_put_pages:
>> -drm_gem_shmem_put_pages(shmem);
>> -err_zero_use:
>> -shmem->vmap_use_count = 0;
>> -mutex_unlock(&shmem->vmap_lock);
>> -return E
Re: [PATCH 1/2] drm/shmem: Use cached mappings by default
Hi Am 18.05.20 um 16:40 schrieb Daniel Vetter: > On Mon, May 18, 2020 at 12:11:32PM +0200, Gerd Hoffmann wrote: >> On Mon, May 18, 2020 at 10:50:15AM +0200, Thomas Zimmermann wrote: >>> Hi Gerd >>> >>> Am 18.05.20 um 10:23 schrieb Gerd Hoffmann: >> $ git grep drm_gem_shmem_mmap >> >> We also need correct access from userspace, otherwise the gpu is going to >> be sad. > > I've been thinking about this, and I think it means that we can never > have cached mappings anywhere. Even if shmem supports it internally for > most drivers, as soon as the page are exported, the importer could > expect uncached memory. The importer should not expect anything but call dma-buf ops so the exporter has a chance to handle this correctly. >>> >>> I have the following case in mind: Suppose the exporter maps cached >>> pages and the importer expects uncached pages for DMA. There is >>> map_dma_buf/unmap_dma_buf, which can implement a cache flush for the >>> cached pages. Is it guaranteed that the importer calls this around each >>> DMA operation? >> >> I think the importer is supposed to do that, but I wouldn't surprised if >> there are cases in tree where this isn't implemented correctly ... > > Yup, this is very much a case of "supposed to" but "in practice, many > actually dont". The reason is that setting up mappings is expensive, so > best avoid. Thanks to both of you for answering the question. > > We filled that gap a few years after dma-buf landed with the > begin/end_cpu_access hooks, which allow the exporter to do cache flushing > (using something like dma_sync_sg_for_device/cpu) and for this to all work > properly. We even added ioctl so that the mmap on the dma-buf works > correctly. > > But most importers still ignore this, so it's all fail :-/ But in theory > the pieces to make cached mappings work over dma-buf, even for importers > that need uncached, are all there. > > Cheers, Daniel > -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer signature.asc Description: OpenPGP digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
linux-next: manual merge of the tip tree with the drm-misc tree
Hi all,
Today's linux-next merge of the tip tree got a conflict in:
drivers/gpu/drm/drm_dp_mst_topology.c
between commit:
a4292e52106b ("drm: Match drm_dp_send_clear_payload_id_table definition to
declaration")
from the drm-misc tree and commit:
53965dbe5396 ("drm: Make drm_dp_mst_dsc_aux_for_port() safe for old
compilers")
from the tip tree.
I fixed it up (I used the former) 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
pgpkKrSD3PeXw.pgp
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2] drm/etnaviv: fix perfmon domain interation
The GC860 has one GPU device which has a 2d and 3d core. In this case
we want to expose perfmon information for both cores.
The driver has one array which contains all possible perfmon domains
with some meta data - doms_meta. Here we can see that for the GC860
two elements of that array are relevant:
doms_3d: is at index 0 in the doms_meta array with 8 perfmon domains
doms_2d: is at index 1 in the doms_meta array with 1 perfmon domain
The userspace driver wants to get a list of all perfmon domains and
their perfmon signals. This is done by iterating over all domains and
their signals. If the userspace driver wants to access the domain with
id 8 the kernel driver fails and returns invalid data from doms_3d with
and invalid offset.
This results in:
Unable to handle kernel paging request at virtual address
On such a device it is not possible to use the userspace driver at all.
The fix for this off-by-one error is quite simple.
Reported-by: Paul Cercueil
Tested-by: Paul Cercueil
Fixes: ed1dd899baa3 ("drm/etnaviv: rework perfmon query infrastructure")
Cc: sta...@vger.kernel.org
Signed-off-by: Christian Gmeiner
---
drivers/gpu/drm/etnaviv/etnaviv_perfmon.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
index e6795bafcbb9..75f9db8f7bec 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
@@ -453,7 +453,7 @@ static const struct etnaviv_pm_domain *pm_domain(const
struct etnaviv_gpu *gpu,
if (!(gpu->identity.features & meta->feature))
continue;
- if (meta->nr_domains < (index - offset)) {
+ if (index - offset >= meta->nr_domains) {
offset += meta->nr_domains;
continue;
}
--
2.26.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] tee: convert convert get_user_pages() --> pin_user_pages()
This code was using get_user_pages*(), in a "Case 2" scenario
(DMA/RDMA), using the categorization from [1]. That means that it's
time to convert the get_user_pages*() + put_page() calls to
pin_user_pages*() + unpin_user_pages() calls.
There is some helpful background in [2]: basically, this is a small
part of fixing a long-standing disconnect between pinning pages, and
file systems' use of those pages.
[1] Documentation/core-api/pin_user_pages.rst
[2] "Explicit pinning of user-space pages":
https://lwn.net/Articles/807108/
Cc: Jens Wiklander
Cc: Sumit Semwal
Cc: tee-...@lists.linaro.org
Cc: linux-me...@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: linaro-mm-...@lists.linaro.org
Signed-off-by: John Hubbard
---
Note that I have only compile-tested this patch, although that does
also include cross-compiling for a few other arches.
thanks,
John Hubbard
NVIDIA
drivers/tee/tee_shm.c | 12 +++-
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c
index bd679b72bd05..7dffc42d8d5a 100644
--- a/drivers/tee/tee_shm.c
+++ b/drivers/tee/tee_shm.c
@@ -31,16 +31,13 @@ static void tee_shm_release(struct tee_shm *shm)
poolm->ops->free(poolm, shm);
} else if (shm->flags & TEE_SHM_REGISTER) {
- size_t n;
int rc = teedev->desc->ops->shm_unregister(shm->ctx, shm);
if (rc)
dev_err(teedev->dev.parent,
"unregister shm %p failed: %d", shm, rc);
- for (n = 0; n < shm->num_pages; n++)
- put_page(shm->pages[n]);
-
+ unpin_user_pages(shm->pages, shm->num_pages);
kfree(shm->pages);
}
@@ -226,7 +223,7 @@ struct tee_shm *tee_shm_register(struct tee_context *ctx,
unsigned long addr,
goto err;
}
- rc = get_user_pages_fast(start, num_pages, FOLL_WRITE, shm->pages);
+ rc = pin_user_pages_fast(start, num_pages, FOLL_WRITE, shm->pages);
if (rc > 0)
shm->num_pages = rc;
if (rc != num_pages) {
@@ -271,16 +268,13 @@ struct tee_shm *tee_shm_register(struct tee_context *ctx,
unsigned long addr,
return shm;
err:
if (shm) {
- size_t n;
-
if (shm->id >= 0) {
mutex_lock(&teedev->mutex);
idr_remove(&teedev->idr, shm->id);
mutex_unlock(&teedev->mutex);
}
if (shm->pages) {
- for (n = 0; n < shm->num_pages; n++)
- put_page(shm->pages[n]);
+ unpin_user_pages(shm->pages, shm->num_pages);
kfree(shm->pages);
}
}
--
2.26.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] Lenovo X13 Yoga OLED panel brightness fix. Add another panel that needs the edid quirk to the list so that brightness control works correctly. Fixes issue seen on Lenovo X13 Yoga with OLED pan
Co-developed-by: jendr...@lenovo.com
Signed-off-by: Mark Pearson
---
drivers/gpu/drm/drm_dp_helper.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index c6fbe6e6bc9d..41f0e797ce8c 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -1313,6 +1313,7 @@ static const struct edid_quirk edid_quirk_list[] = {
{ MFG(0x06, 0xaf), PROD_ID(0xeb, 0x41),
BIT(DP_QUIRK_FORCE_DPCD_BACKLIGHT) },
{ MFG(0x4d, 0x10), PROD_ID(0xc7, 0x14),
BIT(DP_QUIRK_FORCE_DPCD_BACKLIGHT) },
{ MFG(0x4d, 0x10), PROD_ID(0xe6, 0x14),
BIT(DP_QUIRK_FORCE_DPCD_BACKLIGHT) },
+ { MFG(0x4c, 0x83), PROD_ID(0x47, 0x41),
BIT(DP_QUIRK_FORCE_DPCD_BACKLIGHT) },
};
#undef MFG
--
2.26.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/4] mm/gup: introduce pin_user_pages_fast_only()
This is the FOLL_PIN equivalent of __get_user_pages_fast(),
except with a more descriptive name, and gup_flags instead of
a boolean "write" in the argument list.
Signed-off-by: John Hubbard
---
include/linux/mm.h | 2 ++
mm/gup.c | 36
2 files changed, 38 insertions(+)
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 84b601cab699..98be7289d7e9 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1820,6 +1820,8 @@ extern int mprotect_fixup(struct vm_area_struct *vma,
*/
int __get_user_pages_fast(unsigned long start, int nr_pages, int write,
struct page **pages);
+int pin_user_pages_fast_only(unsigned long start, int nr_pages,
+unsigned int gup_flags, struct page **pages);
/*
* per-process(per-mm_struct) statistics.
*/
diff --git a/mm/gup.c b/mm/gup.c
index bb3e2c4288c3..4413f0f94b68 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -2858,6 +2858,42 @@ int pin_user_pages_fast(unsigned long start, int
nr_pages,
}
EXPORT_SYMBOL_GPL(pin_user_pages_fast);
+/*
+ * This is the FOLL_PIN equivalent of __get_user_pages_fast(). Behavior is the
+ * same, except that this one sets FOLL_PIN instead of FOLL_GET.
+ *
+ * The API rules are the same, too: no negative values may be returned.
+ */
+int pin_user_pages_fast_only(unsigned long start, int nr_pages,
+unsigned int gup_flags, struct page **pages)
+{
+ int nr_pinned;
+
+ /*
+* FOLL_GET and FOLL_PIN are mutually exclusive. Note that the API
+* rules require returning 0, rather than -errno:
+*/
+ if (WARN_ON_ONCE(gup_flags & FOLL_GET))
+ return 0;
+ /*
+* FOLL_FAST_ONLY is required in order to match the API description of
+* this routine: no fall back to regular ("slow") GUP.
+*/
+ gup_flags |= (FOLL_PIN | FOLL_FAST_ONLY);
+ nr_pinned = internal_get_user_pages_fast(start, nr_pages, gup_flags,
+pages);
+ /*
+* This routine is not allowed to return negative values. However,
+* internal_get_user_pages_fast() *can* return -errno. Therefore,
+* correct for that here:
+*/
+ if (nr_pinned < 0)
+ nr_pinned = 0;
+
+ return nr_pinned;
+}
+EXPORT_SYMBOL_GPL(pin_user_pages_fast_only);
+
/**
* pin_user_pages_remote() - pin pages of a remote process (task != current)
*
--
2.26.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/4] mm/gup: move __get_user_pages_fast() down a few lines in gup.c
This is in order to avoid a forward declaration of
internal_get_user_pages_fast(), in the next patch.
This is code movement only--all generated code should
be identical.
Signed-off-by: John Hubbard
---
mm/gup.c | 112 +++
1 file changed, 56 insertions(+), 56 deletions(-)
diff --git a/mm/gup.c b/mm/gup.c
index 50cd9323efff..4502846d57f9 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -2666,62 +2666,6 @@ static bool gup_fast_permitted(unsigned long start,
unsigned long end)
}
#endif
-/*
- * Like get_user_pages_fast() except it's IRQ-safe in that it won't fall back
to
- * the regular GUP.
- * Note a difference with get_user_pages_fast: this always returns the
- * number of pages pinned, 0 if no pages were pinned.
- *
- * If the architecture does not support this function, simply return with no
- * pages pinned.
- */
-int __get_user_pages_fast(unsigned long start, int nr_pages, int write,
- struct page **pages)
-{
- unsigned long len, end;
- unsigned long flags;
- int nr_pinned = 0;
- /*
-* Internally (within mm/gup.c), gup fast variants must set FOLL_GET,
-* because gup fast is always a "pin with a +1 page refcount" request.
-*/
- unsigned int gup_flags = FOLL_GET;
-
- if (write)
- gup_flags |= FOLL_WRITE;
-
- start = untagged_addr(start) & PAGE_MASK;
- len = (unsigned long) nr_pages << PAGE_SHIFT;
- end = start + len;
-
- if (end <= start)
- return 0;
- if (unlikely(!access_ok((void __user *)start, len)))
- return 0;
-
- /*
-* Disable interrupts. We use the nested form as we can already have
-* interrupts disabled by get_futex_key.
-*
-* With interrupts disabled, we block page table pages from being
-* freed from under us. See struct mmu_table_batch comments in
-* include/asm-generic/tlb.h for more details.
-*
-* We do not adopt an rcu_read_lock(.) here as we also want to
-* block IPIs that come from THPs splitting.
-*/
-
- if (IS_ENABLED(CONFIG_HAVE_FAST_GUP) &&
- gup_fast_permitted(start, end)) {
- local_irq_save(flags);
- gup_pgd_range(start, end, gup_flags, pages, &nr_pinned);
- local_irq_restore(flags);
- }
-
- return nr_pinned;
-}
-EXPORT_SYMBOL_GPL(__get_user_pages_fast);
-
static int __gup_longterm_unlocked(unsigned long start, int nr_pages,
unsigned int gup_flags, struct page **pages)
{
@@ -2794,6 +2738,62 @@ static int internal_get_user_pages_fast(unsigned long
start, int nr_pages,
return ret;
}
+/*
+ * Like get_user_pages_fast() except it's IRQ-safe in that it won't fall back
to
+ * the regular GUP.
+ * Note a difference with get_user_pages_fast: this always returns the
+ * number of pages pinned, 0 if no pages were pinned.
+ *
+ * If the architecture does not support this function, simply return with no
+ * pages pinned.
+ */
+int __get_user_pages_fast(unsigned long start, int nr_pages, int write,
+ struct page **pages)
+{
+ unsigned long len, end;
+ unsigned long flags;
+ int nr_pinned = 0;
+ /*
+* Internally (within mm/gup.c), gup fast variants must set FOLL_GET,
+* because gup fast is always a "pin with a +1 page refcount" request.
+*/
+ unsigned int gup_flags = FOLL_GET;
+
+ if (write)
+ gup_flags |= FOLL_WRITE;
+
+ start = untagged_addr(start) & PAGE_MASK;
+ len = (unsigned long) nr_pages << PAGE_SHIFT;
+ end = start + len;
+
+ if (end <= start)
+ return 0;
+ if (unlikely(!access_ok((void __user *)start, len)))
+ return 0;
+
+ /*
+* Disable interrupts. We use the nested form as we can already have
+* interrupts disabled by get_futex_key.
+*
+* With interrupts disabled, we block page table pages from being
+* freed from under us. See struct mmu_table_batch comments in
+* include/asm-generic/tlb.h for more details.
+*
+* We do not adopt an rcu_read_lock(.) here as we also want to
+* block IPIs that come from THPs splitting.
+*/
+
+ if (IS_ENABLED(CONFIG_HAVE_FAST_GUP) &&
+ gup_fast_permitted(start, end)) {
+ local_irq_save(flags);
+ gup_pgd_range(start, end, gup_flags, pages, &nr_pinned);
+ local_irq_restore(flags);
+ }
+
+ return nr_pinned;
+}
+EXPORT_SYMBOL_GPL(__get_user_pages_fast);
+
/**
* get_user_pages_fast() - pin user pages in memory
* @start: starting user address
--
2.26.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/4] mm/gup, drm/i915: refactor gup_fast, convert to pin_user_pages()
This needs to go through Andrew's -mm tree, due to adding a new gup.c routine. However, I would really love to have some testing from the drm/i915 folks, because I haven't been able to run-time test that part of it. Otherwise, though, the series has passed my basic run time testing: some LTP tests, some xfs and etx4 non-destructive xfstests, and an assortment of other smaller ones: vm selftests, io_uring_register, a few more. But that's only on one particular machine. Also, cross-compile tests for half a dozen arches all pass. Details: In order to convert the drm/i915 driver from get_user_pages() to pin_user_pages(), a FOLL_PIN equivalent of __get_user_pages_fast() was required. That led to refactoring __get_user_pages_fast(), with the following goals: 1) As above: provide a pin_user_pages*() routine for drm/i915 to call, in place of __get_user_pages_fast(), 2) Get rid of the gup.c duplicate code for walking page tables with interrupts disabled. This duplicate code is a minor maintenance problem anyway. 3) Make it easy for an upcoming patch from Souptick, which aims to convert __get_user_pages_fast() to use a gup_flags argument, instead of a bool writeable arg. Also, if this series looks good, we can ask Souptick to change the name as well, to whatever the consensus is. My initial recommendation is: get_user_pages_fast_only(), to match the new pin_user_pages_only(). John Hubbard (4): mm/gup: move __get_user_pages_fast() down a few lines in gup.c mm/gup: refactor and de-duplicate gup_fast() code mm/gup: introduce pin_user_pages_fast_only() drm/i915: convert get_user_pages() --> pin_user_pages() drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 22 +-- include/linux/mm.h | 3 + mm/gup.c| 150 3 files changed, 107 insertions(+), 68 deletions(-) base-commit: 642b151f45dd54809ea00ecd3976a56c1ec9b53d -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/4] drm/i915: convert get_user_pages() --> pin_user_pages()
This code was using get_user_pages*(), in a "Case 2" scenario
(DMA/RDMA), using the categorization from [1]. That means that it's
time to convert the get_user_pages*() + put_page() calls to
pin_user_pages*() + unpin_user_pages() calls.
There is some helpful background in [2]: basically, this is a small
part of fixing a long-standing disconnect between pinning pages, and
file systems' use of those pages.
[1] Documentation/core-api/pin_user_pages.rst
[2] "Explicit pinning of user-space pages":
https://lwn.net/Articles/807108/
Signed-off-by: John Hubbard
---
drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 22 -
1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
index 7ffd7afeb7a5..b55ac7563189 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
@@ -471,7 +471,7 @@ __i915_gem_userptr_get_pages_worker(struct work_struct
*_work)
down_read(&mm->mmap_sem);
locked = 1;
}
- ret = get_user_pages_remote
+ ret = pin_user_pages_remote
(work->task, mm,
obj->userptr.ptr + pinned * PAGE_SIZE,
npages - pinned,
@@ -507,7 +507,7 @@ __i915_gem_userptr_get_pages_worker(struct work_struct
*_work)
}
mutex_unlock(&obj->mm.lock);
- release_pages(pvec, pinned);
+ unpin_user_pages(pvec, pinned);
kvfree(pvec);
i915_gem_object_put(obj);
@@ -564,6 +564,7 @@ static int i915_gem_userptr_get_pages(struct
drm_i915_gem_object *obj)
struct sg_table *pages;
bool active;
int pinned;
+ unsigned int gup_flags = 0;
/* If userspace should engineer that these pages are replaced in
* the vma between us binding this page into the GTT and completion
@@ -598,11 +599,14 @@ static int i915_gem_userptr_get_pages(struct
drm_i915_gem_object *obj)
GFP_KERNEL |
__GFP_NORETRY |
__GFP_NOWARN);
- if (pvec) /* defer to worker if malloc fails */
- pinned = __get_user_pages_fast(obj->userptr.ptr,
- num_pages,
-
!i915_gem_object_is_readonly(obj),
- pvec);
+ /* defer to worker if malloc fails */
+ if (pvec) {
+ if (!i915_gem_object_is_readonly(obj))
+ gup_flags |= FOLL_WRITE;
+ pinned = pin_user_pages_fast_only(obj->userptr.ptr,
+ num_pages, gup_flags,
+ pvec);
+ }
}
active = false;
@@ -620,7 +624,7 @@ static int i915_gem_userptr_get_pages(struct
drm_i915_gem_object *obj)
__i915_gem_userptr_set_active(obj, true);
if (IS_ERR(pages))
- release_pages(pvec, pinned);
+ unpin_user_pages(pvec, pinned);
kvfree(pvec);
return PTR_ERR_OR_ZERO(pages);
@@ -675,7 +679,7 @@ i915_gem_userptr_put_pages(struct drm_i915_gem_object *obj,
}
mark_page_accessed(page);
- put_page(page);
+ unpin_user_page(page);
}
obj->mm.dirty = false;
--
2.26.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/4] mm/gup: refactor and de-duplicate gup_fast() code
There were two nearly identical sets of code for gup_fast()
style of walking the page tables with interrupts disabled.
This has lead to the usual maintenance problems that arise from
having duplicated code.
There is already a core internal routine in gup.c for gup_fast(),
so just enhance it very slightly: allow skipping the fall-back
to "slow" (regular) get_user_pages(), via the new FOLL_FAST_ONLY
flag. Then, just call internal_get_user_pages_fast() from
__get_user_pages_fast(), and adjust the API to match pre-existing
API behavior.
There is a change in behavior from this refactoring: the nested
form of interrupt disabling is used in all gup_fast() variants
now. That's because there is only one place that interrupt disabling
for page walking is done, and so the safer form is required. This
should, if anything, eliminate possible (rare) bugs, because the
non-nested form of enabling interrupts was fragile at best.
Signed-off-by: John Hubbard
---
include/linux/mm.h | 1 +
mm/gup.c | 60 ++
2 files changed, 29 insertions(+), 32 deletions(-)
diff --git a/include/linux/mm.h b/include/linux/mm.h
index a5594ac9ebe3..84b601cab699 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2782,6 +2782,7 @@ struct page *follow_page(struct vm_area_struct *vma,
unsigned long address,
#define FOLL_LONGTERM 0x1 /* mapping lifetime is indefinite: see below */
#define FOLL_SPLIT_PMD 0x2 /* split huge pmd before returning */
#define FOLL_PIN 0x4 /* pages must be released via unpin_user_page */
+#define FOLL_FAST_ONLY 0x8 /* gup_fast: prevent fall-back to slow gup */
/*
* FOLL_PIN and FOLL_LONGTERM may be used in various combinations with each
diff --git a/mm/gup.c b/mm/gup.c
index 4502846d57f9..bb3e2c4288c3 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -2694,6 +2694,7 @@ static int internal_get_user_pages_fast(unsigned long
start, int nr_pages,
struct page **pages)
{
unsigned long addr, len, end;
+ unsigned long flags;
int nr_pinned = 0, ret = 0;
if (WARN_ON_ONCE(gup_flags & ~(FOLL_WRITE | FOLL_LONGTERM |
@@ -2710,15 +2711,26 @@ static int internal_get_user_pages_fast(unsigned long
start, int nr_pages,
if (unlikely(!access_ok((void __user *)start, len)))
return -EFAULT;
+ /*
+* Disable interrupts. The nested form is used, in order to allow full,
+* general purpose use of this routine.
+*
+* With interrupts disabled, we block page table pages from being
+* freed from under us. See struct mmu_table_batch comments in
+* include/asm-generic/tlb.h for more details.
+*
+* We do not adopt an rcu_read_lock(.) here as we also want to
+* block IPIs that come from THPs splitting.
+*/
if (IS_ENABLED(CONFIG_HAVE_FAST_GUP) &&
gup_fast_permitted(start, end)) {
- local_irq_disable();
+ local_irq_save(flags);
gup_pgd_range(addr, end, gup_flags, pages, &nr_pinned);
- local_irq_enable();
+ local_irq_restore(flags);
ret = nr_pinned;
}
- if (nr_pinned < nr_pages) {
+ if (nr_pinned < nr_pages && !(gup_flags & FOLL_FAST_ONLY)) {
/* Try to get the remaining pages with get_user_pages */
start += nr_pinned << PAGE_SHIFT;
pages += nr_pinned;
@@ -2750,45 +2762,29 @@ static int internal_get_user_pages_fast(unsigned long
start, int nr_pages,
int __get_user_pages_fast(unsigned long start, int nr_pages, int write,
struct page **pages)
{
- unsigned long len, end;
- unsigned long flags;
- int nr_pinned = 0;
+ int nr_pinned;
/*
* Internally (within mm/gup.c), gup fast variants must set FOLL_GET,
* because gup fast is always a "pin with a +1 page refcount" request.
+*
+* FOLL_FAST_ONLY is required in order to match the API description of
+* this routine: no fall back to regular ("slow") GUP.
*/
- unsigned int gup_flags = FOLL_GET;
+ unsigned int gup_flags = FOLL_GET | FOLL_FAST_ONLY;
if (write)
gup_flags |= FOLL_WRITE;
- start = untagged_addr(start) & PAGE_MASK;
- len = (unsigned long) nr_pages << PAGE_SHIFT;
- end = start + len;
-
- if (end <= start)
- return 0;
- if (unlikely(!access_ok((void __user *)start, len)))
- return 0;
-
+ nr_pinned = internal_get_user_pages_fast(start, nr_pages, gup_flags,
+pages);
/*
-* Disable interrupts. We use the nested form as we can already have
-* interrupts disabled by get_futex_key.
-*
-* With interrupts disabled, we block page table pages from being
-* freed from under
Re: [PATCH V3 07/15] arch/kunmap_atomic: Consolidate duplicate code
On Sun, May 17, 2020 at 09:29:32PM -0700, Guenter Roeck wrote:
> On Sun, May 17, 2020 at 08:49:39PM -0700, Ira Weiny wrote:
> > On Sat, May 16, 2020 at 03:33:06PM -0700, Guenter Roeck wrote:
> > > On Thu, May 07, 2020 at 07:59:55AM -0700, ira.we...@intel.com wrote:
> > > > 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.
> > > >
> > > > Reviewed-by: Christoph Hellwig
> > > > Signed-off-by: Ira Weiny
> > >
> > > This patch results in:
> > >
> > > Starting init: /bin/sh exists but couldn't execute it (error -14)
> > >
> > > when trying to boot microblazeel:petalogix-ml605 in qemu.
> >
> > Thanks for the report. I'm not readily seeing the issue.
> >
> > Do you have a kernel config? Specifically is CONFIG_HIGHMEM set?
> >
> See below. Yes, CONFIG_HIGHMEM is set.
>
> The scripts used to build and boot the image are at:
>
> https://github.com/groeck/linux-build-test/tree/master/rootfs/microblazeel
Despite finding the obvious error earlier today I've still been trying to get
this to work.
I had to make some slight modifications to use the 0-day cross compile build
and my local qemu build. But those were pretty minor modifications. I'm
running on x86_64 host.
With those slight mods to the scripts I get the following error even without my
patch set on 5.7-rc4. I have 1 cpu pegged at 100% while it is running... Is
there anything I can do to get more debug output? Perhaps I just need to let
it run longer?
Thanks,
Ira
16:46:54 > ../linux-build-test/rootfs/microblazeel/run-qemu-microblazeel.sh
Build reference: v5.7-rc4-2-g7c2411d7fb6a
Building microblaze:petalogix-s3adsp1800:qemu_microblazeel_defconfig ...
running failed (silent)
qemu log:
qemu-system-microblazeel: terminating on signal 15 from pid 3277686 (/bin/bash)
Building microblaze:petalogix-ml605:qemu_microblazeel_ml605_defconfig ...
running failed (silent)
qemu log:
qemu-system-microblazeel: terminating on signal 15 from pid 3277686 (/bin/bash)
16:47:23 > git di
diff --git a/rootfs/microblazeel/run-qemu-microblazeel.sh
b/rootfs/microblazeel/run-qemu-microblazeel.sh
index 68d4de39ab50..0d6a4f85308f 100755
--- a/rootfs/microblazeel/run-qemu-microblazeel.sh
+++ b/rootfs/microblazeel/run-qemu-microblazeel.sh
@@ -3,7 +3,8 @@
dir=$(cd $(dirname $0); pwd)
. ${dir}/../scripts/common.sh
-QEMU=${QEMU:-${QEMU_BIN}/qemu-system-microblazeel}
+#QEMU=${QEMU:-${QEMU_BIN}/qemu-system-microblazeel}
+QEMU=/home/iweiny/dev/qemu/microblazeel-softmmu/qemu-system-microblazeel
PREFIX=microblazeel-linux-
ARCH=microblaze
PATH_MICROBLAZE=/opt/kernel/microblazeel/gcc-4.9.1/usr/bin
diff --git a/rootfs/scripts/common.sh b/rootfs/scripts/common.sh
index 8fa6a9be2b2f..c4550a27beaa 100644
--- a/rootfs/scripts/common.sh
+++ b/rootfs/scripts/common.sh
@@ -1,5 +1,9 @@
#!/bin/bash
+# Set up make.cross
+export COMPILER_INSTALL_PATH=$HOME/0day
+export GCC_VERSION=6.5.0
+
# Set the following variable to true to skip DC395/AM53C97 build tests
__skip_dc395=0
@@ -569,7 +573,7 @@ doclean()
then
git clean -x -d -f -q
else
- make ARCH=${ARCH} mrproper >/dev/null 2>&1
+ make.cross ARCH=${ARCH} mrproper >/dev/null 2>&1
fi
}
@@ -669,7 +673,7 @@ __setup_config()
cp ${__progdir}/${defconfig} arch/${arch}/configs
fi
-if ! make ARCH=${ARCH} CROSS_COMPILE=${PREFIX} ${defconfig} >/dev/null
2>&1 /dev/null 2>&1 /dev/null
2>&1 /dev/null 2>&1 /dev/null 2>${logfile}
+make.cross -j${maxload} ARCH=${ARCH} ${EXTRAS} /dev/null
2>${logfile}
rv=$?
if [ ${rv} -ne 0 ]
then
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] Lenovo X13 Yoga OLED panel brightness fix
ok-actually, I'm not sure dim will actually allow me to push this because this
never hit dri-devel as a properly formatted patch (because of the --
-- you put at the top of the branch confusing things)
Can you try again without adding any comments, and ideally see if lenovo can
make it so you can just use git send-email? (unfortunately you're really not
going to be able to work around this if you'll need to be submitting patches
more regularly in the future I'm afraid)
On Mon, 2020-05-18 at 18:43 -0400, Lyude Paul wrote:
> Yeah, git send-email is kinda :(, but unfortunately the entire kernel uses
> this
> workflow. Hopefully freedesktop's gitlab efforts will change this someday...
>
> also - in the future, if you don't want comments to appear in the patch when
> they're applied put them below the ---, e.g. the one before the Reviewed-by:
> tag.
>
> Anyway-I'll go ahead and push this, thanks for keeping this list up to date!
>
> On Mon, 2020-05-18 at 00:06 +, Mark Pearson wrote:
> > Hi,
> >
> > Patch to fix an issue controlling the brightness of the OLED panel on the
> > Lenovo X13 Yoga
> > Please let me know any feedback or questions.
> > Note - apologies if this message has shown up before - I had some mail
> > client
> > issues.
> >
> > Mark Pearson
> > -
> >
> > Add another panel that needs the edid quirk to the list so that brightness
> > control works correctly. Fixes issue seen on Lenovo X13 Yoga with OLED panel
> >
> > Co-developed-by: jendr...@lenovo.com
> > Signed-off-by: jendr...@lenovo.com
> > Signed-off-by: Mark Pearson
> > Reviewed-by: Lyude Paul
> > ---
> > drivers/gpu/drm/drm_dp_helper.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/gpu/drm/drm_dp_helper.c
> > b/drivers/gpu/drm/drm_dp_helper.c
> > index c6fbe6e6bc9d..41f0e797ce8c 100644
> > --- a/drivers/gpu/drm/drm_dp_helper.c
> > +++ b/drivers/gpu/drm/drm_dp_helper.c
> > @@ -1313,6 +1313,7 @@ static const struct edid_quirk edid_quirk_list[] = {
> >{ MFG(0x06, 0xaf), PROD_ID(0xeb, 0x41),
> > BIT(DP_QUIRK_FORCE_DPCD_BACKLIGHT) },
> >{ MFG(0x4d, 0x10), PROD_ID(0xc7, 0x14),
> > BIT(DP_QUIRK_FORCE_DPCD_BACKLIGHT) },
> >{ MFG(0x4d, 0x10), PROD_ID(0xe6, 0x14),
> > BIT(DP_QUIRK_FORCE_DPCD_BACKLIGHT) },
> > + { MFG(0x4c, 0x83), PROD_ID(0x47, 0x41),
> > BIT(DP_QUIRK_FORCE_DPCD_BACKLIGHT) },
> > };
> >
> > #undef MFG
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] Lenovo X13 Yoga OLED panel brightness fix
Yeah, git send-email is kinda :(, but unfortunately the entire kernel uses this
workflow. Hopefully freedesktop's gitlab efforts will change this someday...
also - in the future, if you don't want comments to appear in the patch when
they're applied put them below the ---, e.g. the one before the Reviewed-by:
tag.
Anyway-I'll go ahead and push this, thanks for keeping this list up to date!
On Mon, 2020-05-18 at 00:06 +, Mark Pearson wrote:
> Hi,
>
> Patch to fix an issue controlling the brightness of the OLED panel on the
> Lenovo X13 Yoga
> Please let me know any feedback or questions.
> Note - apologies if this message has shown up before - I had some mail client
> issues.
>
> Mark Pearson
> -
>
> Add another panel that needs the edid quirk to the list so that brightness
> control works correctly. Fixes issue seen on Lenovo X13 Yoga with OLED panel
>
> Co-developed-by: jendr...@lenovo.com
> Signed-off-by: jendr...@lenovo.com
> Signed-off-by: Mark Pearson
> Reviewed-by: Lyude Paul
> ---
> drivers/gpu/drm/drm_dp_helper.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index c6fbe6e6bc9d..41f0e797ce8c 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -1313,6 +1313,7 @@ static const struct edid_quirk edid_quirk_list[] = {
>{ MFG(0x06, 0xaf), PROD_ID(0xeb, 0x41),
> BIT(DP_QUIRK_FORCE_DPCD_BACKLIGHT) },
>{ MFG(0x4d, 0x10), PROD_ID(0xc7, 0x14),
> BIT(DP_QUIRK_FORCE_DPCD_BACKLIGHT) },
>{ MFG(0x4d, 0x10), PROD_ID(0xe6, 0x14),
> BIT(DP_QUIRK_FORCE_DPCD_BACKLIGHT) },
> + { MFG(0x4c, 0x83), PROD_ID(0x47, 0x41),
> BIT(DP_QUIRK_FORCE_DPCD_BACKLIGHT) },
> };
>
> #undef MFG
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH AUTOSEL 5.6 33/50] drm/amdgpu: bump version for invalidate L2 before SDMA IBs
Hi Sasha, I disagree with this. Bumping the driver version will have implications on other new features, because it's like an ABI barrier exposing new functionality. Marek On Thu, May 7, 2020 at 10:28 AM Sasha Levin wrote: > From: Marek Olšák > > [ Upstream commit 9017a4897a20658f010bebea825262963c10afa6 ] > > This fixes GPU hangs due to cache coherency issues. > Bump the driver version. Split out from the original patch. > > Signed-off-by: Marek Olšák > Reviewed-by: Christian König > Tested-by: Pierre-Eric Pelloux-Prayer > Signed-off-by: Alex Deucher > Signed-off-by: Sasha Levin > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > index 42f4febe24c6d..8d45a2b662aeb 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > @@ -85,9 +85,10 @@ > * - 3.34.0 - Non-DC can flip correctly between buffers with different > pitches > * - 3.35.0 - Add drm_amdgpu_info_device::tcc_disabled_mask > * - 3.36.0 - Allow reading more status registers on si/cik > + * - 3.37.0 - L2 is invalidated before SDMA IBs, needed for correctness > */ > #define KMS_DRIVER_MAJOR 3 > -#define KMS_DRIVER_MINOR 36 > +#define KMS_DRIVER_MINOR 37 > #define KMS_DRIVER_PATCHLEVEL 0 > > int amdgpu_vram_limit = 0; > -- > 2.20.1 > > ___ > amd-gfx mailing list > amd-...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/i915/gvt: Use ARRAY_SIZE for vgpu_types
Quoting Aishwarya Ramakrishnan (2020-05-18 16:03:36) > Prefer ARRAY_SIZE instead of using sizeof > > Fixes coccicheck warning: Use ARRAY_SIZE > > Signed-off-by: Aishwarya Ramakrishnan Reviewed-by: Chris Wilson -Chris ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] dt-bindings: arm-smmu: Add sc7180 compatible string
Hi, On Mon, May 18, 2020 at 7:39 AM Will Deacon wrote: > > On Fri, May 15, 2020 at 12:05:39PM -0700, Doug Anderson wrote: > > On Fri, May 1, 2020 at 3:30 AM Sharat Masetty > > wrote: > > > > > > This patch simply adds a new compatible string for SC7180 platform. > > > > > > Signed-off-by: Sharat Masetty > > > --- > > > Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml > > > b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml > > > index 6515dbe..986098b 100644 > > > --- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml > > > +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml > > > @@ -28,6 +28,7 @@ properties: > > >- enum: > > >- qcom,msm8996-smmu-v2 > > >- qcom,msm8998-smmu-v2 > > > + - qcom,sc7180-smmu-v2 > > >- qcom,sdm845-smmu-v2 > > >- const: qcom,smmu-v2 > > > > Is anything blocking this patch from landing now? > > I thought updates to the bindings usually went via Rob and the device-tree > tree, but neither of those are on cc. > > Perhaps resend with that fixed? Ah, I guess I wasn't familiar with how things worked for this file, or maybe things have changed recently? I'm used to most bindings going through the same tree as the drivers that use them. Usually if things are at all complicated maintainers wait for an Ack from Rob (so he should have been CCed for sure) and then land. In this case it actually looks like Bjorn landed it in the Qualcomm and I just didn't realize it. That seems like it should be fine since it's in the middle of a clause that's all Qualcomm and the change shouldn't be controversial in any way. :-) Thanks! -Doug ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 15/16] backlight: make of_find_backlight_by_node() static
On Mon, May 18, 2020 at 08:12:27PM +0200, Sam Ravnborg wrote: > On Mon, May 18, 2020 at 05:56:48PM +0100, Daniel Thompson wrote: > > On Sun, May 17, 2020 at 09:01:38PM +0200, Sam Ravnborg wrote: > > > There are no external users of of_find_backlight_by_node(). > > > Make it static so we keep it that way. > > > > > > v2: > > > - drop EXPORT of of_find_backlight_by_node > > > > > > Signed-off-by: Sam Ravnborg > > > Cc: Lee Jones > > > Cc: Daniel Thompson > > > Cc: Jingoo Han > > > > Assuming the 0day-ci comments are because some of the patches have > > already been sucked up in a different tree then: > Correct. For now only drm-misc-next have no users of > of_find_backlight_by_node() which is why the other trees failed. > > > > Reviewed-by: Daniel Thompson > Thanks for all your reviews! > I will shortly (within a few days) address the comments and send out a v3. > > Is is correct that I assume you or Lee or Jingoo will apply the patches > to a backlight tree somewhere when they are ready? > If you have a tree you use for backlight patches I can base v3 on that, > given that I get a link and have access to pull from it. Absent holidays and the like, Lee usually does that actual patch hoovering. Daniel. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [REPOST PATCH v2] drm/bridge: ti-sn65dsi86: Implement lane reordering + polarity
Hi Douglas.
On Mon, May 18, 2020 at 11:47:17AM -0700, Douglas Anderson wrote:
> The ti-sn65dsi86 MIPI DSI to eDP bridge chip supports arbitrary
> remapping of eDP lanes and also polarity inversion. Both of these
> features have been described in the device tree bindings for the
> device since the beginning but were never implemented in the driver.
> Implement both of them.
>
> Part of this change also allows you to (via the same device tree
> bindings) specify to use fewer than the max number of DP lanes that
> the panel reports. This could be useful if your display supports more
> lanes but only a few are hooked up on your board.
>
> Signed-off-by: Douglas Anderson
> Reviewed-by: Stephen Boyd
> Reviewed-by: Rob Clark
> ---
> Re-posting patch v2, patch #1. I added tags and put Sam in the "To"
> list. Patch #2 was dropped since it was squashed elsewhere. This now
> applies to the top of drm-misc-next.
>
> Changes in v2:
> - Use SN_MAX_DP_LANES instead of 4 in one place.
> - Comment that we aren't doing full validation of dts params.
> - Check dp_lanes <= SN_MAX_DP_LANES to avoid buffer overrun.
> - Add missing of_node_put()
Applied to drm-misc-next.
Sam
>
> drivers/gpu/drm/bridge/ti-sn65dsi86.c | 82 ++-
> 1 file changed, 68 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> index 1855fb9f09f2..2240e9973178 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> @@ -50,8 +50,12 @@
> #define SN_CHA_VERTICAL_BACK_PORCH_REG 0x36
> #define SN_CHA_HORIZONTAL_FRONT_PORCH_REG0x38
> #define SN_CHA_VERTICAL_FRONT_PORCH_REG 0x3A
> +#define SN_LN_ASSIGN_REG 0x59
> +#define LN_ASSIGN_WIDTH 2
> #define SN_ENH_FRAME_REG 0x5A
> #define VSTREAM_ENABLE BIT(3)
> +#define LN_POLRS_OFFSET 4
> +#define LN_POLRS_MASK 0xf0
> #define SN_DATA_FORMAT_REG 0x5B
> #define BPP_18_RGB BIT(0)
> #define SN_HPD_DISABLE_REG 0x5C
> @@ -98,6 +102,7 @@
>
> #define SN_REGULATOR_SUPPLY_NUM 4
>
> +#define SN_MAX_DP_LANES 4
> #define SN_NUM_GPIOS 4
> #define SN_GPIO_PHYSICAL_OFFSET 1
>
> @@ -116,6 +121,8 @@
> * @enable_gpio: The GPIO we toggle to enable the bridge.
> * @supplies: Data for bulk enabling/disabling our regulators.
> * @dp_lanes: Count of dp_lanes we're using.
> + * @ln_assign:Value to program to the LN_ASSIGN register.
> + * @ln_polr: Value for the 4-bit LN_POLRS field of SN_ENH_FRAME_REG.
> *
> * @gchip:If we expose our GPIOs, this is used.
> * @gchip_output: A cache of whether we've set GPIOs to output. This
> @@ -141,6 +148,8 @@ struct ti_sn_bridge {
> struct gpio_desc*enable_gpio;
> struct regulator_bulk_data supplies[SN_REGULATOR_SUPPLY_NUM];
> int dp_lanes;
> + u8 ln_assign;
> + u8 ln_polrs;
>
> struct gpio_chipgchip;
> DECLARE_BITMAP(gchip_output, SN_NUM_GPIOS);
> @@ -708,26 +717,20 @@ static void ti_sn_bridge_enable(struct drm_bridge
> *bridge)
> int dp_rate_idx;
> unsigned int val;
> int ret = -EINVAL;
> + int max_dp_lanes;
>
> - /*
> - * Run with the maximum number of lanes that the DP sink supports.
> - *
> - * Depending use cases, we might want to revisit this later because:
> - * - It's plausible that someone may have run fewer lines to the
> - * sink than the sink actually supports, assuming that the lines
> - * will just be driven at a higher rate.
> - * - The DP spec seems to indicate that it's more important to minimize
> - * the number of lanes than the link rate.
> - *
> - * If we do revisit, it would be important to measure the power impact.
> - */
> - pdata->dp_lanes = ti_sn_get_max_lanes(pdata);
> + max_dp_lanes = ti_sn_get_max_lanes(pdata);
> + pdata->dp_lanes = min(pdata->dp_lanes, max_dp_lanes);
>
> /* DSI_A lane config */
> - val = CHA_DSI_LANES(4 - pdata->dsi->lanes);
> + val = CHA_DSI_LANES(SN_MAX_DP_LANES - pdata->dsi->lanes);
> regmap_update_bits(pdata->regmap, SN_DSI_LANES_REG,
> CHA_DSI_LANES_MASK, val);
>
> + regmap_write(pdata->regmap, SN_LN_ASSIGN_REG, pdata->ln_assign);
> + regmap_update_bits(pdata->regmap, SN_ENH_FRAME_REG, LN_POLRS_MASK,
> +pdata->ln_polrs << LN_POLRS_OFFSET);
> +
> /* set dsi clk frequency value */
> ti_sn_bridge_set_dsi_rate(pdata);
>
> @@ -1089,6 +1092,55 @@ static int ti_sn_setup_gpio_controller(struct
[PATCH] arch/{mips, sparc, microblaze, powerpc}: Don't enable pagefault/preempt twice
From: Ira Weiny
The kunmap_atomic clean up failed to remove one set of pagefault/preempt
enables when vaddr is not in the fixmap.
Fixes: bee2128a09e6 ("arch/kunmap_atomic: consolidate duplicate code")
Signed-off-by: Ira Weiny
---
arch/microblaze/mm/highmem.c | 5 +
arch/mips/mm/highmem.c | 5 +
arch/powerpc/mm/highmem.c| 5 +
arch/sparc/mm/highmem.c | 5 +
4 files changed, 4 insertions(+), 16 deletions(-)
diff --git a/arch/microblaze/mm/highmem.c b/arch/microblaze/mm/highmem.c
index ee8a422b2b76..92e0890416c9 100644
--- a/arch/microblaze/mm/highmem.c
+++ b/arch/microblaze/mm/highmem.c
@@ -57,11 +57,8 @@ void kunmap_atomic_high(void *kvaddr)
int type;
unsigned int idx;
- if (vaddr < __fix_to_virt(FIX_KMAP_END)) {
- pagefault_enable();
- preempt_enable();
+ if (vaddr < __fix_to_virt(FIX_KMAP_END))
return;
- }
type = kmap_atomic_idx();
diff --git a/arch/mips/mm/highmem.c b/arch/mips/mm/highmem.c
index 37e244cdb14e..8e8726992720 100644
--- a/arch/mips/mm/highmem.c
+++ b/arch/mips/mm/highmem.c
@@ -41,11 +41,8 @@ void kunmap_atomic_high(void *kvaddr)
unsigned long vaddr = (unsigned long) kvaddr & PAGE_MASK;
int type __maybe_unused;
- if (vaddr < FIXADDR_START) { // FIXME
- pagefault_enable();
- preempt_enable();
+ if (vaddr < FIXADDR_START)
return;
- }
type = kmap_atomic_idx();
#ifdef CONFIG_DEBUG_HIGHMEM
diff --git a/arch/powerpc/mm/highmem.c b/arch/powerpc/mm/highmem.c
index 35071c2913f1..624b4438aff9 100644
--- a/arch/powerpc/mm/highmem.c
+++ b/arch/powerpc/mm/highmem.c
@@ -44,11 +44,8 @@ void kunmap_atomic_high(void *kvaddr)
{
unsigned long vaddr = (unsigned long) kvaddr & PAGE_MASK;
- if (vaddr < __fix_to_virt(FIX_KMAP_END)) {
- pagefault_enable();
- preempt_enable();
+ if (vaddr < __fix_to_virt(FIX_KMAP_END))
return;
- }
if (IS_ENABLED(CONFIG_DEBUG_HIGHMEM)) {
int type = kmap_atomic_idx();
diff --git a/arch/sparc/mm/highmem.c b/arch/sparc/mm/highmem.c
index d237d902f9c3..6ff6e2a9f9b3 100644
--- a/arch/sparc/mm/highmem.c
+++ b/arch/sparc/mm/highmem.c
@@ -86,11 +86,8 @@ void kunmap_atomic_high(void *kvaddr)
unsigned long vaddr = (unsigned long) kvaddr & PAGE_MASK;
int type;
- if (vaddr < FIXADDR_START) { // FIXME
- pagefault_enable();
- preempt_enable();
+ if (vaddr < FIXADDR_START)
return;
- }
type = kmap_atomic_idx();
--
2.25.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[REPOST PATCH v2] drm/bridge: ti-sn65dsi86: Implement lane reordering + polarity
The ti-sn65dsi86 MIPI DSI to eDP bridge chip supports arbitrary
remapping of eDP lanes and also polarity inversion. Both of these
features have been described in the device tree bindings for the
device since the beginning but were never implemented in the driver.
Implement both of them.
Part of this change also allows you to (via the same device tree
bindings) specify to use fewer than the max number of DP lanes that
the panel reports. This could be useful if your display supports more
lanes but only a few are hooked up on your board.
Signed-off-by: Douglas Anderson
Reviewed-by: Stephen Boyd
Reviewed-by: Rob Clark
---
Re-posting patch v2, patch #1. I added tags and put Sam in the "To"
list. Patch #2 was dropped since it was squashed elsewhere. This now
applies to the top of drm-misc-next.
Changes in v2:
- Use SN_MAX_DP_LANES instead of 4 in one place.
- Comment that we aren't doing full validation of dts params.
- Check dp_lanes <= SN_MAX_DP_LANES to avoid buffer overrun.
- Add missing of_node_put()
drivers/gpu/drm/bridge/ti-sn65dsi86.c | 82 ++-
1 file changed, 68 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index 1855fb9f09f2..2240e9973178 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -50,8 +50,12 @@
#define SN_CHA_VERTICAL_BACK_PORCH_REG 0x36
#define SN_CHA_HORIZONTAL_FRONT_PORCH_REG 0x38
#define SN_CHA_VERTICAL_FRONT_PORCH_REG0x3A
+#define SN_LN_ASSIGN_REG 0x59
+#define LN_ASSIGN_WIDTH 2
#define SN_ENH_FRAME_REG 0x5A
#define VSTREAM_ENABLEBIT(3)
+#define LN_POLRS_OFFSET 4
+#define LN_POLRS_MASK 0xf0
#define SN_DATA_FORMAT_REG 0x5B
#define BPP_18_RGBBIT(0)
#define SN_HPD_DISABLE_REG 0x5C
@@ -98,6 +102,7 @@
#define SN_REGULATOR_SUPPLY_NUM4
+#define SN_MAX_DP_LANES4
#define SN_NUM_GPIOS 4
#define SN_GPIO_PHYSICAL_OFFSET1
@@ -116,6 +121,8 @@
* @enable_gpio: The GPIO we toggle to enable the bridge.
* @supplies: Data for bulk enabling/disabling our regulators.
* @dp_lanes: Count of dp_lanes we're using.
+ * @ln_assign:Value to program to the LN_ASSIGN register.
+ * @ln_polr: Value for the 4-bit LN_POLRS field of SN_ENH_FRAME_REG.
*
* @gchip:If we expose our GPIOs, this is used.
* @gchip_output: A cache of whether we've set GPIOs to output. This
@@ -141,6 +148,8 @@ struct ti_sn_bridge {
struct gpio_desc*enable_gpio;
struct regulator_bulk_data supplies[SN_REGULATOR_SUPPLY_NUM];
int dp_lanes;
+ u8 ln_assign;
+ u8 ln_polrs;
struct gpio_chipgchip;
DECLARE_BITMAP(gchip_output, SN_NUM_GPIOS);
@@ -708,26 +717,20 @@ static void ti_sn_bridge_enable(struct drm_bridge *bridge)
int dp_rate_idx;
unsigned int val;
int ret = -EINVAL;
+ int max_dp_lanes;
- /*
-* Run with the maximum number of lanes that the DP sink supports.
-*
-* Depending use cases, we might want to revisit this later because:
-* - It's plausible that someone may have run fewer lines to the
-* sink than the sink actually supports, assuming that the lines
-* will just be driven at a higher rate.
-* - The DP spec seems to indicate that it's more important to minimize
-* the number of lanes than the link rate.
-*
-* If we do revisit, it would be important to measure the power impact.
-*/
- pdata->dp_lanes = ti_sn_get_max_lanes(pdata);
+ max_dp_lanes = ti_sn_get_max_lanes(pdata);
+ pdata->dp_lanes = min(pdata->dp_lanes, max_dp_lanes);
/* DSI_A lane config */
- val = CHA_DSI_LANES(4 - pdata->dsi->lanes);
+ val = CHA_DSI_LANES(SN_MAX_DP_LANES - pdata->dsi->lanes);
regmap_update_bits(pdata->regmap, SN_DSI_LANES_REG,
CHA_DSI_LANES_MASK, val);
+ regmap_write(pdata->regmap, SN_LN_ASSIGN_REG, pdata->ln_assign);
+ regmap_update_bits(pdata->regmap, SN_ENH_FRAME_REG, LN_POLRS_MASK,
+ pdata->ln_polrs << LN_POLRS_OFFSET);
+
/* set dsi clk frequency value */
ti_sn_bridge_set_dsi_rate(pdata);
@@ -1089,6 +1092,55 @@ static int ti_sn_setup_gpio_controller(struct
ti_sn_bridge *pdata)
return ret;
}
+static void ti_sn_bridge_parse_lanes(struct ti_sn_bridge *pdata,
+struct device_node *np)
+{
+ u32 lane_assignments[SN_MAX_DP_LANES] = { 0, 1, 2, 3 };
+ u32
Re: [PATCH v2 1/2] drm/bridge: ti-sn65dsi86: Implement lane reordering + polarity
Hi Douglas. On Mon, May 18, 2020 at 11:22:22AM -0700, Doug Anderson wrote: > Sam, > > On Fri, May 15, 2020 at 2:43 PM Rob Clark wrote: > > > > On Wed, May 6, 2020 at 2:03 PM Douglas Anderson > > wrote: > > > > > > The ti-sn65dsi86 MIPI DSI to eDP bridge chip supports arbitrary > > > remapping of eDP lanes and also polarity inversion. Both of these > > > features have been described in the device tree bindings for the > > > device since the beginning but were never implemented in the driver. > > > Implement both of them. > > > > > > Part of this change also allows you to (via the same device tree > > > bindings) specify to use fewer than the max number of DP lanes that > > > the panel reports. This could be useful if your display supports more > > > lanes but only a few are hooked up on your board. > > > > > > Signed-off-by: Douglas Anderson > > > Reviewed-by: Stephen Boyd > > > > Reviewed-by: Rob Clark > > I guess get_maintainer is somehow not tagging you and I haven't got it > through my thick skull to CC you each time. If you're willing, I > think this patch is ready too. Happy to re-post it with you in the To > list if it helps. I have long lost the original patch, so shall I apply then please re-send. Sam ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/bridge: ti-sn65dsi86: Fix off-by-one error in clock choice
Hi Douglas.
On Mon, May 18, 2020 at 11:23:44AM -0700, Doug Anderson wrote:
> Sam,
>
> On Fri, May 15, 2020 at 2:49 PM Rob Clark wrote:
> >
> > On Mon, May 4, 2020 at 9:32 PM Douglas Anderson
> > wrote:
> > >
> > > If the rate in our table is _equal_ to the rate we want then it's OK
> > > to pick it. It doesn't need to be greater than the one we want.
> > >
> > > Fixes: a095f15c00e2 ("drm/bridge: add support for sn65dsi86 bridge
> > > driver")
> > > Signed-off-by: Douglas Anderson
> >
> > Reviewed-by: Rob Clark
>
> ...and I think this is the last of the patches I stupidly didn't CC
> you on that's ready to go.
I'm not a bridge maintainer so mostly I am just overstepping
what I in reality have authority to do.
But so far no loud complains...
Sam
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 0/6] drm: Prepare to use a GPIO on ti-sn65dsi86 for Hot Plug Detect
Hi Douglas, On Mon, May 18, 2020 at 11:05:58AM -0700, Doug Anderson wrote: > Sam, > > On Mon, May 18, 2020 at 10:59 AM Sam Ravnborg wrote: > > > > Hi Douglas. > > > > > > Given the previous feedback from Linus W, Stephen, and Laurent I > > > > expect things are good enough to land now, but it'd be good to get > > > > confirmation (I removed some of the previous tags just to get > > > > confirmation). If we can get review tags early next week maybe it'll > > > > still be in time to land for 5.8? > > > > > > I think all the others have reviews now. Is there anything blocking > > > them from getting applied? > > Applied, including the small fix pointed out by Linus. > > Thanks! Ugh, I just realized what the problem was. I posted a v6 > with this fix but insanely somehow didn't CC you (!!!). It was here: > > https://lore.kernel.org/r/20200513215902.261547-1-diand...@chromium.org > > I'm super sorry about that and thanks for fixing the nit. That was > the only difference between v5 and v6. I just checked what you pushed > and it looks great, thank you. No worries as long as what hits drm-misc-next is OK, which it was in this case. Sam ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] dt-bindings: n133hse-ea1: Add Innolux N133HSE panel
On Sat, May 09, 2020 at 01:18:33PM +0200, s...@48.io wrote: > From: Sean Cross > > The Innolux N133HSE panel is a 13.3" 1920x1080 panel that contains an > integrated backlight, and connects via eDP. > > It is used in the Kosagi Novena. > > Signed-off-by: Sean Cross > Signed-off-by: Richard Marko > Cc: Shawn Guo > Cc: Fabio Estevam > Cc: Thierry Reding > To: dri-devel@lists.freedesktop.org > --- > .../devicetree/bindings/display/panel/panel-simple.yaml | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git > a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml > b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml > index 393ffc6acbba..756bf6a48e8d 100644 > --- a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml > +++ b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml > @@ -139,6 +139,8 @@ properties: >- innolux,g121i1-l01 > # Innolux Corporation 12.1" G121X1-L03 XGA (1024x768) TFT LCD panel >- innolux,g121x1-l03 > +# Innolux Corporation 13.3" FHD (1920x1080) TFT LCD panel > + - innolux,n133hse-ea1 > # Innolux Corporation 11.6" WXGA (1366x768) TFT LCD panel >- innolux,n116bge Alpha-numeric order please. > # InnoLux 15.6" WXGA TFT LCD panel > -- > 2.25.1 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/bridge: ti-sn65dsi86: Fix off-by-one error in clock choice
Sam,
On Fri, May 15, 2020 at 2:49 PM Rob Clark wrote:
>
> On Mon, May 4, 2020 at 9:32 PM Douglas Anderson wrote:
> >
> > If the rate in our table is _equal_ to the rate we want then it's OK
> > to pick it. It doesn't need to be greater than the one we want.
> >
> > Fixes: a095f15c00e2 ("drm/bridge: add support for sn65dsi86 bridge driver")
> > Signed-off-by: Douglas Anderson
>
> Reviewed-by: Rob Clark
...and I think this is the last of the patches I stupidly didn't CC
you on that's ready to go.
-Doug
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/2] drm/bridge: ti-sn65dsi86: Implement lane reordering + polarity
Sam, On Fri, May 15, 2020 at 2:43 PM Rob Clark wrote: > > On Wed, May 6, 2020 at 2:03 PM Douglas Anderson wrote: > > > > The ti-sn65dsi86 MIPI DSI to eDP bridge chip supports arbitrary > > remapping of eDP lanes and also polarity inversion. Both of these > > features have been described in the device tree bindings for the > > device since the beginning but were never implemented in the driver. > > Implement both of them. > > > > Part of this change also allows you to (via the same device tree > > bindings) specify to use fewer than the max number of DP lanes that > > the panel reports. This could be useful if your display supports more > > lanes but only a few are hooked up on your board. > > > > Signed-off-by: Douglas Anderson > > Reviewed-by: Stephen Boyd > > Reviewed-by: Rob Clark I guess get_maintainer is somehow not tagging you and I haven't got it through my thick skull to CC you each time. If you're willing, I think this patch is ready too. Happy to re-post it with you in the To list if it helps. -Doug ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH V3 10/15] arch/kmap: Define kmap_atomic_prot() for all arch's
On Sun, May 17, 2020 at 10:37:22AM -0700, Guenter Roeck wrote: > Hi, > > On Thu, May 07, 2020 at 07:59:58AM -0700, ira.we...@intel.com wrote: > > 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. > > > > Reviewed-by: Christoph Hellwig > > Signed-off-by: Ira Weiny > > This patch causes a variety of crashes whem booting powerpc images in qemu. PowerPC has the same issue as microblaze and sparc. I'm preping a patch with all three fixed which fixes the kunmap_atomic clean up patch... Sorry for not seeing this last night... Hopefully this can explain all the problems. It is clearly a bug. Ira > > There are lots of warnings such as: > > WARNING: CPU: 0 PID: 0 at lib/locking-selftest.c:743 > irqsafe1_hard_spin_12+0x50/0xb0 > Modules linked in: > CPU: 0 PID: 0 Comm: swapper Tainted: GW > 5.7.0-rc5-next-20200515 #1 > NIP: c0660c7c LR: c0660c44 CTR: c0660c2c > REGS: c1223e68 TRAP: 0700 Tainted: GW > (5.7.0-rc5-next-20200515) > MSR: 00021000 CR: 28000224 XER: 2000 > > GPR00: c0669c78 c1223f20 c113d560 c0660c44 0001 c1223ea8 0001 > GPR08: 0001 fffc 88000222 > GPR16: c000 c1125084 > GPR24: c1125084 c123 c1879538 fffc 0001 c1011afc c123 > NIP [c0660c7c] irqsafe1_hard_spin_12+0x50/0xb0 > LR [c0660c44] irqsafe1_hard_spin_12+0x18/0xb0 > Call Trace: > [c1223f20] [c188] megasas_mgmt_info+0xee4/0x1008 (unreliable) > [c1223f40] [c0669c78] dotest+0x38/0x550 > [c1223f70] [c066aa4c] locking_selftest+0x8bc/0x1d54 > [c1223fa0] [c10e0bc8] start_kernel+0x3ec/0x510 > [c1223ff0] [c3a0] set_ivor+0x118/0x154 > Instruction dump: > 8142 38e80001 3d4a0001 2c08 9142 90e20488 40820008 91020470 > 8129 5529031e 7d290034 5529d97e <0f09> 3fe0c11c 3bff3964 3bff00ac > irq event stamp: 588 > hardirqs last enabled at (587): [] vprintk_emit+0x1b4/0x33c > hardirqs last disabled at (588): [] irqsafe1_hard_spin_12+0x18/0xb0 > softirqs last enabled at (0): [<>] 0x0 > softirqs last disabled at (0): [<>] 0x0 > ---[ end trace b18fe9e172f99d03 ]--- > > This is followed by: > > BUG: sleeping function called from invalid context at lib/mpi/mpi-pow.c:245 > in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 14, name: > cryptomgr_test > INFO: lockdep is turned off. > CPU: 0 PID: 14 Comm: cryptomgr_test Tainted: GW > 5.7.0-rc5-next-20200515 #1 > Call Trace: > [ce221b58] [c008755c] ___might_sleep+0x280/0x2a8 (unreliable) > [ce221b78] [c06bc524] mpi_powm+0x634/0xc50 > [ce221c38] [c05eafdc] rsa_dec+0x88/0x134 > [ce221c78] [c05f3b40] test_akcipher_one+0x678/0x804 > [ce221dc8] [c05f3d7c] alg_test_akcipher+0xb0/0x130 > [ce221df8] [c05ee674] alg_test.part.0+0xb4/0x458 > [ce221ed8] [c05ed2b0] cryptomgr_test+0x30/0x50 > [ce221ef8] [c007cd74] kthread+0x134/0x170 > [ce221f38] [c001433c] ret_from_kernel_thread+0x14/0x1c > Kernel panic - not syncing: Aiee, killing interrupt handler! > CPU: 0 PID: 14 Comm: cryptomgr_test Tainted: GW > 5.7.0-rc5-next-20200515 #1 > Call Trace: > [ce221e08] [c00530fc] panic+0x148/0x34c (unreliable) > [ce221e68] [c0056460] do_exit+0xac0/0xb40 > [ce221eb8] [c00f5be8] find_kallsyms_symbol_value+0x0/0x128 > [ce221ed8] [c05ed2d0] crypto_alg_put+0x0/0x70 > [ce221ef8] [c007cd74] kthread+0x134/0x170 > [ce221f38] [c001433c] ret_from_kernel_thread+0x14/0x1c > > Bisect log is attached. The patch can not easily be reverted since > it results in compile errors. > > Note that similar failures are seen with sparc32 images. Those bisect > to a different patch, but reverting that patch doesn't fix the problem. > The failure pattern (warnings followed by a crash in cryptomgr_test) > is the same. > > Guenter > > --- > # bad: [bdecf38f228bcca73b31ada98b5b7ba1215eb9c9] Add linux-next specific > files for 20200515 > # good: [2ef96a5bb12be62ef75b5828c0aab838ebb29cb8] Linux 5.7-rc5 > git bisect start 'HEAD' 'v5.7-rc5' > # good: [3674d7aa7a8e61d993886c2fb7c896c5ef85e988] Merge remote-tracking > branch 'crypto/master' > git bisect good 3674d7aa7a8e61d993886c2fb7c896c5ef85e988 > # good: [87f6f21783522e6d62127cf33ae5e95f50874beb] Merge remote-tracking > branch 'spi/for-next' > git bisect good 87f6f21783522e6d62127cf33ae5e95f50874beb > # good: [5c428e8277d5d97c85126387d4e00aa5adde4400] Merge remote-tracking > branch 'staging/staging-next' > git bisect good 5c428e8277d5d97c85126387d4e00aa5adde4400 > # good: [f68de67ed934e7b
Re: [PATCH v5 0/6] drm: Prepare to use a GPIO on ti-sn65dsi86 for Hot Plug Detect
Sam, On Mon, May 18, 2020 at 10:59 AM Sam Ravnborg wrote: > > Hi Douglas. > > > > Given the previous feedback from Linus W, Stephen, and Laurent I > > > expect things are good enough to land now, but it'd be good to get > > > confirmation (I removed some of the previous tags just to get > > > confirmation). If we can get review tags early next week maybe it'll > > > still be in time to land for 5.8? > > > > I think all the others have reviews now. Is there anything blocking > > them from getting applied? > Applied, including the small fix pointed out by Linus. Thanks! Ugh, I just realized what the problem was. I posted a v6 with this fix but insanely somehow didn't CC you (!!!). It was here: https://lore.kernel.org/r/20200513215902.261547-1-diand...@chromium.org I'm super sorry about that and thanks for fixing the nit. That was the only difference between v5 and v6. I just checked what you pushed and it looks great, thank you. -Doug ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 15/16] backlight: make of_find_backlight_by_node() static
On Mon, May 18, 2020 at 05:56:48PM +0100, Daniel Thompson wrote: > On Sun, May 17, 2020 at 09:01:38PM +0200, Sam Ravnborg wrote: > > There are no external users of of_find_backlight_by_node(). > > Make it static so we keep it that way. > > > > v2: > > - drop EXPORT of of_find_backlight_by_node > > > > Signed-off-by: Sam Ravnborg > > Cc: Lee Jones > > Cc: Daniel Thompson > > Cc: Jingoo Han > > Assuming the 0day-ci comments are because some of the patches have > already been sucked up in a different tree then: Correct. For now only drm-misc-next have no users of of_find_backlight_by_node() which is why the other trees failed. > Reviewed-by: Daniel Thompson Thanks for all your reviews! I will shortly (within a few days) address the comments and send out a v3. Is is correct that I assume you or Lee or Jingoo will apply the patches to a backlight tree somewhere when they are ready? If you have a tree you use for backlight patches I can base v3 on that, given that I get a link and have access to pull from it. Sam ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH V3 07/15] arch/kunmap_atomic: Consolidate duplicate code
On Sun, May 17, 2020 at 09:29:32PM -0700, Guenter Roeck wrote:
> On Sun, May 17, 2020 at 08:49:39PM -0700, Ira Weiny wrote:
> > On Sat, May 16, 2020 at 03:33:06PM -0700, Guenter Roeck wrote:
> > > On Thu, May 07, 2020 at 07:59:55AM -0700, ira.we...@intel.com wrote:
> > > > 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.
> > > >
> > > > Reviewed-by: Christoph Hellwig
> > > > Signed-off-by: Ira Weiny
> > >
> > > This patch results in:
> > >
> > > Starting init: /bin/sh exists but couldn't execute it (error -14)
> > >
> > > when trying to boot microblazeel:petalogix-ml605 in qemu.
> >
> > Thanks for the report. I'm not readily seeing the issue.
> >
> > Do you have a kernel config? Specifically is CONFIG_HIGHMEM set?
> >
> See below. Yes, CONFIG_HIGHMEM is set.
>
> The scripts used to build and boot the image are at:
>
> https://github.com/groeck/linux-build-test/tree/master/rootfs/microblazeel
>
> Hope this helps,
Thank you ...
Could you try the following patch?
commit 82c284b2bb74ca195dfcd35b70a175f010b9fd46 (HEAD -> lm-kmap17)
Author: Ira Weiny
Date: Mon May 18 11:01:10 2020 -0700
microblaze/kmap: Don't enable pagefault/preempt twice
The kunmap_atomic clean up failed to remove the pagefault/preempt
enables on this path.
Fixes: bee2128a09e6 ("arch/kunmap_atomic: consolidate duplicate code")
Signed-off-by: Ira Weiny
diff --git a/arch/microblaze/mm/highmem.c b/arch/microblaze/mm/highmem.c
index ee8a422b2b76..92e0890416c9 100644
--- a/arch/microblaze/mm/highmem.c
+++ b/arch/microblaze/mm/highmem.c
@@ -57,11 +57,8 @@ void kunmap_atomic_high(void *kvaddr)
int type;
unsigned int idx;
- if (vaddr < __fix_to_virt(FIX_KMAP_END)) {
- pagefault_enable();
- preempt_enable();
+ if (vaddr < __fix_to_virt(FIX_KMAP_END))
return;
- }
type = kmap_atomic_idx();
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/exynos: Remove dev_err() on platform_get_irq() failure
On Mon, 2020-05-18 at 22:57 +0530, Tamseel Shams wrote:
> platform_get_irq() will call dev_err() itself on failure,
> so there is no need for the driver to also do this.
> This is detected by coccinelle.
trivia:
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
[]
> @@ -1810,7 +1810,6 @@ static int exynos_dsi_probe(struct platform_device
> *pdev)
>
> dsi->irq = platform_get_irq(pdev, 0);
> if (dsi->irq < 0) {
> - dev_err(dev, "failed to request dsi irq resource\n");
> return dsi->irq;
> }
You could also remove the now atypical braces
if (dsi->irq < 0)
return dsi->irq;
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_rotator.c
> b/drivers/gpu/drm/exynos/exynos_drm_rotator.c
[]
> @@ -294,7 +294,6 @@ static int rotator_probe(struct platform_device *pdev)
>
> irq = platform_get_irq(pdev, 0);
> if (irq < 0) {
> - dev_err(dev, "failed to get irq\n");
> return irq;
> }
etc...
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/bridge: ti-sn65dsi86: Clear old error bits before AUX transfers
On Fri, May 08, 2020 at 04:33:29PM -0700, Douglas Anderson wrote:
> The AUX channel transfer error bits in the status register are latched
> and need to be cleared. Clear them before doing our transfer so we
> don't see old bits and get confused.
>
> Without this patch having a single failure would mean that all future
> transfers would look like they failed.
>
> Fixes: b814ec6d4535 ("drm/bridge: ti-sn65dsi86: Implement AUX channel")
> Signed-off-by: Douglas Anderson
Applied.
Sam
> ---
>
> drivers/gpu/drm/bridge/ti-sn65dsi86.c | 6 ++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> index 6ad688b320ae..d865cc2565bc 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> @@ -827,6 +827,12 @@ static ssize_t ti_sn_aux_transfer(struct drm_dp_aux *aux,
>buf[i]);
> }
>
> + /* Clear old status bits before start so we don't get confused */
> + regmap_write(pdata->regmap, SN_AUX_CMD_STATUS_REG,
> + AUX_IRQ_STATUS_NAT_I2C_FAIL |
> + AUX_IRQ_STATUS_AUX_RPLY_TOUT |
> + AUX_IRQ_STATUS_AUX_SHORT);
> +
> regmap_write(pdata->regmap, SN_AUX_CMD_REG, request_val | AUX_CMD_SEND);
>
> ret = regmap_read_poll_timeout(pdata->regmap, SN_AUX_CMD_REG, val,
> --
> 2.26.2.645.ge9eca65c58-goog
>
> ___
> 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 v5 0/6] drm: Prepare to use a GPIO on ti-sn65dsi86 for Hot Plug Detect
Hi Douglas. > > Given the previous feedback from Linus W, Stephen, and Laurent I > > expect things are good enough to land now, but it'd be good to get > > confirmation (I removed some of the previous tags just to get > > confirmation). If we can get review tags early next week maybe it'll > > still be in time to land for 5.8? > > I think all the others have reviews now. Is there anything blocking > them from getting applied? Applied, including the small fix pointed out by Linus. Sam ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 06/16] backlight: improve backlight_device documentation
On Mon, May 18, 2020 at 08:03:21PM +0300, Jani Nikula wrote:
> On Mon, 18 May 2020, Daniel Thompson wrote:
> > On Sun, May 17, 2020 at 09:01:29PM +0200, Sam Ravnborg wrote:
> >> Improve the documentation for backlight_device and
> >> adapt it to kernel-doc style.
> >>
> >> Signed-off-by: Sam Ravnborg
> >> Cc: Lee Jones
> >> Cc: Daniel Thompson
> >> Cc: Jingoo Han
> >> ---
> >> include/linux/backlight.h | 81 ---
> >> 1 file changed, 58 insertions(+), 23 deletions(-)
> >>
> >> diff --git a/include/linux/backlight.h b/include/linux/backlight.h
> >> index 7f9cef299d6e..e2d72936bf05 100644
> >> --- a/include/linux/backlight.h
> >> +++ b/include/linux/backlight.h
> >> @@ -14,21 +14,6 @@
> >> #include
> >> #include
> >>
> >> -/* Notes on locking:
> >> - *
> >> - * backlight_device->ops_lock is an internal backlight lock protecting the
> >> - * ops pointer and no code outside the core should need to touch it.
> >> - *
> >> - * Access to update_status() is serialised by the update_lock mutex since
> >> - * most drivers seem to need this and historically get it wrong.
> >> - *
> >> - * Most drivers don't need locking on their get_brightness() method.
> >> - * If yours does, you need to implement it in the driver. You can use the
> >> - * update_lock mutex if appropriate.
> >> - *
> >> - * Any other use of the locks below is probably wrong.
> >> - */
> >> -
> >> enum backlight_update_reason {
> >>BACKLIGHT_UPDATE_HOTKEY,
> >>BACKLIGHT_UPDATE_SYSFS,
> >> @@ -221,30 +206,80 @@ struct backlight_properties {
> >>enum backlight_scale scale;
> >> };
> >>
> >> +/**
> >> + * struct backlight_device - backlight device data
> >> + *
> >> + * This structure holds all data required by a backlight device.
> >> + */
> >> struct backlight_device {
> >> - /* Backlight properties */
> >> + /**
> >> + * @props:
> >> + *
> >
> > As last patch. Why no brief descriptions?
>
> There are no "brief descriptions" in kernel-doc struct member inline
> markup.
>
> It would be possible to shorten this to:
>
> /**
>* @props: Backlight properties
>*/
Sorry. Loose phrasing on my part... basically the question is why
deviate from the form proposed in "Writing kernel-doc comments"?
Daniel.
> >
> >> + * Backlight properties
> >> + */
> >>struct backlight_properties props;
> >>
> >> - /* Serialise access to update_status method */
> >> + /**
> >> + * @update_lock:
> >> + *
> >> + * update_lock is an internal backlight lock that serialise access
> >> + * to the update_status() method. The iupdate_lock mutex shall not be
> >> used
> >> + * by backlight drivers.
> >
> > In addition to the typo this directly contradicts the advice in the
> > original "Notes on locking".
> >
> > A change this dramatic needs to be fully explaining in the patch
> > description.
> >
> >
> > Daniel.
> >
> >
> >> + */
> >>struct mutex update_lock;
> >>
> >> - /* This protects the 'ops' field. If 'ops' is NULL, the driver that
> >> - registered this device has been unloaded, and if class_get_devdata()
> >> - points to something in the body of that driver, it is also invalid.
> >> */
> >> + /**
> >> + * @ops_lock:
> >> + *
> >> + * ops_lock is an internal backlight lock that protects the ops pointer
> >> + * and is used around all accesses to ops and when the operations are
> >> + * invoked. The mutex shall not be used by backlight drivers.
> >> + */
> >>struct mutex ops_lock;
> >> +
> >> + /**
> >> + * @ops:
> >> + *
> >> + * Pointer to the backlight operations. If ops is NULL, the driver that
> >> + * registered this device has been unloaded, and if class_get_devdata()
> >> + * points to something in the body of that driver, it is also invalid.
> >> + */
> >>const struct backlight_ops *ops;
> >>
> >> - /* The framebuffer notifier block */
> >> + /**
> >> + * @fb_notif:
> >> + *
> >> + * The framebuffer notifier block
> >> + */
> >>struct notifier_block fb_notif;
> >>
> >> - /* list entry of all registered backlight devices */
> >> + /**
> >> + * @entry:
> >> + *
> >> + * List entry of all registered backlight devices
> >> + */
> >>struct list_head entry;
> >>
> >> + /**
> >> + * @dev:
> >> + *
> >> + * Parent device.
> >> + */
> >>struct device dev;
> >>
> >> - /* Multiple framebuffers may share one backlight device */
> >> + /**
> >> + * @fb_bl_on:
> >> + *
> >> + * Multiple fbdev's may share one backlight device. The fb_bl_on
> >> + * records the state of the individual fbdev.
> >> + */
> >>bool fb_bl_on[FB_MAX];
> >>
> >> + /**
> >> + * @use_count:
> >> + *
> >> + * The number of uses of fb_bl_on.
> >> + */
> >>int use_count;
> >> };
> >>
> >> --
> >> 2.25.1
> >>
>
> --
> Jani Nikula, Intel Open Source Graphics Center
___
dri-devel mailing list
dri-devel@lists.freedeskto
Re: [PATCH v4 7/7] drm/mediatek: mtk_dsi: Create connector for bridges
Hi Enric/Chun-Kuang. > > > > My point is: when do you attach panel to a connector? > > In this patch, > > > > ret = drm_bridge_attach(&dsi->encoder, &dsi->bridge, NULL, > > DRM_BRIDGE_ATTACH_NO_CONNECTOR); > > > > it would call into mtk_dsi_bridge_attach() with > > DRM_BRIDGE_ATTACH_NO_CONNECTOR, and call into panel_bridge_attach() > > with DRM_BRIDGE_ATTACH_NO_CONNECTOR. > > My understanding is that the DRM_BRIDGE_ATTACH_NO_CONNECTOR flag is to > ease transition between the old and the new model. The drivers that > support the new model shall set that flag. Yes, right now we have fous on migrating all bridge drivers to the new model and next step is to make the transition for the display drivers one by one. Display drivers that uses the old model rely on the bridge driver to create the connector, whereas display drivers using the new model will create the connector themself. Display drivers following the new model will pass DRM_BRIDGE_ATTACH_NO_CONNECTOR to tell the bridge drive that no connector shall be created by the bridge driver. For this driver where only the new model is needed there is no reason to try to support both models. So the display driver shall always create the connector, and never ask the bridge driver to do it (always pass DRM_BRIDGE_ATTACH_NO_CONNECTOR). I hope this confirm and clarifies it. Sam ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 7/7] drm/mediatek: mtk_dsi: Create connector for bridges
Hi Chun-Kuang,
Missatge de Chun-Kuang Hu del dia ds., 16 de
maig 2020 a les 12:11:
>
> Hi, Enric:
>
> Enric Balletbo i Serra 於 2020年5月15日 週五
> 上午1:35寫道：
> >
> > Hi again,
> >
> > On 14/5/20 19:12, Enric Balletbo i Serra wrote:
> > > Hi Chun-Kuang,
> > >
> > > On 14/5/20 18:44, Chun-Kuang Hu wrote:
> > >> Hi, Enric:
> > >>
> > >> Enric Balletbo i Serra 於 2020年5月14日 週四
> > >> 下午11:42寫道：
> > >>>
> > >>> Hi Chun-Kuang,
> > >>>
> > >>> On 14/5/20 16:28, Chun-Kuang Hu wrote:
> > Hi, Enric:
> >
> > Enric Balletbo Serra 於 2020年5月14日 週四 上午12:41寫道：
> > >
> > > Hi Chun-Kuang,
> > >
> > > Missatge de Enric Balletbo i Serra del
> > > dia dv., 1 de maig 2020 a les 17:25:
> > >>
> > >> Use the drm_bridge_connector helper to create a connector for
> > >> pipelines
> > >> that use drm_bridge. This allows splitting connector operations
> > >> across
> > >> multiple bridges when necessary, instead of having the last bridge in
> > >> the chain creating the connector and handling all connector
> > >> operations
> > >> internally.
> > >>
> > >> Signed-off-by: Enric Balletbo i Serra
> > >> Acked-by: Sam Ravnborg
> > >
> > > A gentle ping on this, I think that this one is the only one that
> > > still needs a review in the series.
> >
> > This is what I reply in patch v3:
> >
> > >>>
> > >>> Sorry for missing this.
> > >>>
> > I think the panel is wrapped into next_bridge here,
> >
> > >>>
> > >>> Yes, you can have for example:
> > >>>
> > >>> 1. drm_bridge (mtk_dsi) -> drm_bridge (ps8640 - dsi-to-edp) ->
> > >>> drm_panel_bridge
> > >>> (edp panel)
> > >>>
> > >>> or a
> > >>>
> > >>> 2. drm_bridge (mtk_dsi)-> drm_panel_bridge (dsi panel)
> > >>>
> > >>> The _first_ one is my use case
> > >>>
> > if (panel) {
> > >>>
> > >>> This handles the second case, where you attach a dsi panel.
> > >>>
> > dsi->next_bridge = devm_drm_panel_bridge_add(dev, panel);
> >
> > so the next_bridge is a panel_bridge, in its attach function
> > panel_bridge_attach(),
> > according to the flag DRM_BRIDGE_ATTACH_NO_CONNECTOR, if not exist,
> > it would create connector and attach connector to panel.
> >
> > I'm not sure this flag would exist or not, but for both case, it's
> > strange.
> > If exist, you create connector in this patch but no where to attach
> > connector to panel.
> > >>>
> > >>> Yes, in fact, this is transitional patch needed, as once I converted
> > >>> mtk_dpi,
> > >>> mtk_dsi and mtk_hdmi to the new drm_bridge API the
> > >>> drm_bridge_connector_init()
> > >>> will be done in mtk_drm_drv. We will need to call
> > >>> drm_bridge_connector_init for
> > >>> dpi and dsi pipes and remove that call from mtk_dsi and mtk_dpi
> > >>> drivers. The
> > >>> graphic controller driver should create connectors and CRTCs, as
> > >>> example you can
> > >>> take a look at drivers/gpu/drm/omapdrm/omap_drv.c
> > >>>
> > >>
> > >> I have such question because I've reviewed omap's driver. In omap's
> > >> driver, after it call drm_bridge_connector_init(), it does this:
> > >>
> > >> if (pipe->output->panel) {
> > >> ret = drm_panel_attach(pipe->output->panel,
> > >> pipe->connector);
> > >> if (ret < 0)
> > >> return ret;
> > >> }
> > >>
> > >> In this patch, you does not do this.
> > >>
> > >
> > > I see, so yes, I am probably missing call drm_panel_attach in case there
> > > is a
> > > direct panel attached. Thanks for pointing it.
> > >
> > > I'll send a new version adding the drm_panel_attach call.
> > >
> >
> > Wait, shouldn't panel be attached on the call of mtk_dsi_bridge_attach as
> > next_bridge points to a bridge or a panel?
> >
> > static int mtk_dsi_bridge_attach(struct drm_bridge *bridge,
> > enum drm_bridge_attach_flags flags)
> > {
> > struct mtk_dsi *dsi = bridge_to_dsi(bridge);
> >
> > /* Attach the panel or bridge to the dsi bridge */
> > return drm_bridge_attach(bridge->encoder, dsi->next_bridge,
> > &dsi->bridge, flags);
> > }
> >
> > Or I am continuing misunderstanding all this?
> >
>
> My point is: when do you attach panel to a connector?
> In this patch,
>
> ret = drm_bridge_attach(&dsi->encoder, &dsi->bridge, NULL,
> DRM_BRIDGE_ATTACH_NO_CONNECTOR);
>
> it would call into mtk_dsi_bridge_attach() with
> DRM_BRIDGE_ATTACH_NO_CONNECTOR, and call into panel_bridge_attach()
> with DRM_BRIDGE_ATTACH_NO_CONNECTOR.
My understanding is that the DRM_BRIDGE_ATTACH_NO_CONNECTOR flag is to
ease transition between the old and the new model. The drivers that
support the new model shall set that flag.
> If you does not pass DRM_BRIDGE_ATTACH_NO_CONNECTOR into
> panel_bridge_attach(), it would create a connector and attach panel to
> that connector.
Yes, and create a connector an attach the panel is the o
[PATCH][next] drm/i915: fix incorrect return of an error status
From: Colin Ian King
Currently when a call to intel_atomic_get_dbuf_state fails the
error value being returns is a potentially uninitialized value
in variable ret. Fix this by returning the error value of
new_dbuf_state.
Addresses-Coverity: ("Uninitialized scalar value")
Fixes: 3cf43cdc63fb ("drm/i915: Introduce proper dbuf state")
Signed-off-by: Colin Ian King
---
drivers/gpu/drm/i915/intel_pm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index a21e36ed1a77..762a5184fc69 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5917,7 +5917,7 @@ skl_ddb_add_affected_pipes(struct intel_atomic_state
*state)
new_dbuf_state = intel_atomic_get_dbuf_state(state);
if (IS_ERR(new_dbuf_state))
- return ret;
+ return PTR_ERR(new_dbuf_state);
old_dbuf_state = intel_atomic_get_old_dbuf_state(state);
--
2.25.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 06/16] backlight: improve backlight_device documentation
On Mon, 18 May 2020, Daniel Thompson wrote:
> On Sun, May 17, 2020 at 09:01:29PM +0200, Sam Ravnborg wrote:
>> Improve the documentation for backlight_device and
>> adapt it to kernel-doc style.
>>
>> Signed-off-by: Sam Ravnborg
>> Cc: Lee Jones
>> Cc: Daniel Thompson
>> Cc: Jingoo Han
>> ---
>> include/linux/backlight.h | 81 ---
>> 1 file changed, 58 insertions(+), 23 deletions(-)
>>
>> diff --git a/include/linux/backlight.h b/include/linux/backlight.h
>> index 7f9cef299d6e..e2d72936bf05 100644
>> --- a/include/linux/backlight.h
>> +++ b/include/linux/backlight.h
>> @@ -14,21 +14,6 @@
>> #include
>> #include
>>
>> -/* Notes on locking:
>> - *
>> - * backlight_device->ops_lock is an internal backlight lock protecting the
>> - * ops pointer and no code outside the core should need to touch it.
>> - *
>> - * Access to update_status() is serialised by the update_lock mutex since
>> - * most drivers seem to need this and historically get it wrong.
>> - *
>> - * Most drivers don't need locking on their get_brightness() method.
>> - * If yours does, you need to implement it in the driver. You can use the
>> - * update_lock mutex if appropriate.
>> - *
>> - * Any other use of the locks below is probably wrong.
>> - */
>> -
>> enum backlight_update_reason {
>> BACKLIGHT_UPDATE_HOTKEY,
>> BACKLIGHT_UPDATE_SYSFS,
>> @@ -221,30 +206,80 @@ struct backlight_properties {
>> enum backlight_scale scale;
>> };
>>
>> +/**
>> + * struct backlight_device - backlight device data
>> + *
>> + * This structure holds all data required by a backlight device.
>> + */
>> struct backlight_device {
>> -/* Backlight properties */
>> +/**
>> + * @props:
>> + *
>
> As last patch. Why no brief descriptions?
There are no "brief descriptions" in kernel-doc struct member inline
markup.
It would be possible to shorten this to:
/**
* @props: Backlight properties
*/
BR,
Jani.
>
>
>> + * Backlight properties
>> + */
>> struct backlight_properties props;
>>
>> -/* Serialise access to update_status method */
>> +/**
>> + * @update_lock:
>> + *
>> + * update_lock is an internal backlight lock that serialise access
>> + * to the update_status() method. The iupdate_lock mutex shall not be
>> used
>> + * by backlight drivers.
>
> In addition to the typo this directly contradicts the advice in the
> original "Notes on locking".
>
> A change this dramatic needs to be fully explaining in the patch
> description.
>
>
> Daniel.
>
>
>> + */
>> struct mutex update_lock;
>>
>> -/* This protects the 'ops' field. If 'ops' is NULL, the driver that
>> - registered this device has been unloaded, and if class_get_devdata()
>> - points to something in the body of that driver, it is also invalid.
>> */
>> +/**
>> + * @ops_lock:
>> + *
>> + * ops_lock is an internal backlight lock that protects the ops pointer
>> + * and is used around all accesses to ops and when the operations are
>> + * invoked. The mutex shall not be used by backlight drivers.
>> + */
>> struct mutex ops_lock;
>> +
>> +/**
>> + * @ops:
>> + *
>> + * Pointer to the backlight operations. If ops is NULL, the driver that
>> + * registered this device has been unloaded, and if class_get_devdata()
>> + * points to something in the body of that driver, it is also invalid.
>> + */
>> const struct backlight_ops *ops;
>>
>> -/* The framebuffer notifier block */
>> +/**
>> + * @fb_notif:
>> + *
>> + * The framebuffer notifier block
>> + */
>> struct notifier_block fb_notif;
>>
>> -/* list entry of all registered backlight devices */
>> +/**
>> + * @entry:
>> + *
>> + * List entry of all registered backlight devices
>> + */
>> struct list_head entry;
>>
>> +/**
>> + * @dev:
>> + *
>> + * Parent device.
>> + */
>> struct device dev;
>>
>> -/* Multiple framebuffers may share one backlight device */
>> +/**
>> + * @fb_bl_on:
>> + *
>> + * Multiple fbdev's may share one backlight device. The fb_bl_on
>> + * records the state of the individual fbdev.
>> + */
>> bool fb_bl_on[FB_MAX];
>>
>> +/**
>> + * @use_count:
>> + *
>> + * The number of uses of fb_bl_on.
>> + */
>> int use_count;
>> };
>>
>> --
>> 2.25.1
>>
--
Jani Nikula, Intel Open Source Graphics Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 16/16] backlight: use backlight_is_blank() in all backlight drivers
On Sun, May 17, 2020 at 09:01:39PM +0200, Sam Ravnborg wrote:
> Replaces the open-coded checks of the state etc.,
> with the backlight_is_blank() helper.
> This increases readability of the code and align
> the functionality across the drivers.
>
> v2:
> - Fixed so changelog is readable
>
> Signed-off-by: Sam Ravnborg
> Cc: Lee Jones
> Cc: Daniel Thompson
> Cc: Jingoo Han
> Cc: Michael Hennerich
> Cc: Support Opensource
> Cc: Thierry Reding
> Cc: "Uwe Kleine-König"
> Cc: Andy Gross
> Cc: Bjorn Andersson
> Cc: linux-...@vger.kernel.org
> Cc: linux-arm-...@vger.kernel.org
> Cc: patc...@opensource.cirrus.com
Reviewed-by: Daniel Thompson
> ---
> drivers/video/backlight/88pm860x_bl.c| 8 +---
> drivers/video/backlight/adp5520_bl.c | 5 +
> drivers/video/backlight/adp8860_bl.c | 5 +
> drivers/video/backlight/adp8870_bl.c | 5 +
> drivers/video/backlight/as3711_bl.c | 8 +++-
> drivers/video/backlight/bd6107.c | 4 +---
> drivers/video/backlight/corgi_lcd.c | 5 +
> drivers/video/backlight/cr_bllcd.c | 22 +++---
> drivers/video/backlight/da903x_bl.c | 8 +---
> drivers/video/backlight/ep93xx_bl.c | 3 +--
> drivers/video/backlight/gpio_backlight.c | 4 +---
> drivers/video/backlight/hp680_bl.c | 4 +---
> drivers/video/backlight/jornada720_bl.c | 2 +-
> drivers/video/backlight/kb3886_bl.c | 4 +---
> drivers/video/backlight/led_bl.c | 4 +---
> drivers/video/backlight/lm3533_bl.c | 4 +---
> drivers/video/backlight/locomolcd.c | 4 +---
> drivers/video/backlight/lv5207lp.c | 4 +---
> drivers/video/backlight/max8925_bl.c | 8 +---
> drivers/video/backlight/pwm_bl.c | 4 +---
> drivers/video/backlight/qcom-wled.c | 4 +---
> drivers/video/backlight/tps65217_bl.c| 4 +---
> drivers/video/backlight/wm831x_bl.c | 8 +---
> 23 files changed, 31 insertions(+), 100 deletions(-)
>
> diff --git a/drivers/video/backlight/88pm860x_bl.c
> b/drivers/video/backlight/88pm860x_bl.c
> index 20d96a5ac384..162c83ab0f5a 100644
> --- a/drivers/video/backlight/88pm860x_bl.c
> +++ b/drivers/video/backlight/88pm860x_bl.c
> @@ -123,13 +123,7 @@ static int pm860x_backlight_update_status(struct
> backlight_device *bl)
> {
> int brightness = bl->props.brightness;
>
> - if (bl->props.power != FB_BLANK_UNBLANK)
> - brightness = 0;
> -
> - if (bl->props.fb_blank != FB_BLANK_UNBLANK)
> - brightness = 0;
> -
> - if (bl->props.state & BL_CORE_SUSPENDED)
> + if (backlight_is_blank(bl))
> brightness = 0;
>
> return pm860x_backlight_set(bl, brightness);
> diff --git a/drivers/video/backlight/adp5520_bl.c
> b/drivers/video/backlight/adp5520_bl.c
> index 0f63f76723a5..d817b0d95c9d 100644
> --- a/drivers/video/backlight/adp5520_bl.c
> +++ b/drivers/video/backlight/adp5520_bl.c
> @@ -67,10 +67,7 @@ static int adp5520_bl_update_status(struct
> backlight_device *bl)
> {
> int brightness = bl->props.brightness;
>
> - if (bl->props.power != FB_BLANK_UNBLANK)
> - brightness = 0;
> -
> - if (bl->props.fb_blank != FB_BLANK_UNBLANK)
> + if (backlight_is_blank(bl))
> brightness = 0;
>
> return adp5520_bl_set(bl, brightness);
> diff --git a/drivers/video/backlight/adp8860_bl.c
> b/drivers/video/backlight/adp8860_bl.c
> index 19968104fc47..a0ce2a3701fa 100644
> --- a/drivers/video/backlight/adp8860_bl.c
> +++ b/drivers/video/backlight/adp8860_bl.c
> @@ -363,10 +363,7 @@ static int adp8860_bl_update_status(struct
> backlight_device *bl)
> {
> int brightness = bl->props.brightness;
>
> - if (bl->props.power != FB_BLANK_UNBLANK)
> - brightness = 0;
> -
> - if (bl->props.fb_blank != FB_BLANK_UNBLANK)
> + if (backlight_is_blank(bl))
> brightness = 0;
>
> return adp8860_bl_set(bl, brightness);
> diff --git a/drivers/video/backlight/adp8870_bl.c
> b/drivers/video/backlight/adp8870_bl.c
> index 4c0032010cfe..ae4269fdb189 100644
> --- a/drivers/video/backlight/adp8870_bl.c
> +++ b/drivers/video/backlight/adp8870_bl.c
> @@ -401,10 +401,7 @@ static int adp8870_bl_update_status(struct
> backlight_device *bl)
> {
> int brightness = bl->props.brightness;
>
> - if (bl->props.power != FB_BLANK_UNBLANK)
> - brightness = 0;
> -
> - if (bl->props.fb_blank != FB_BLANK_UNBLANK)
> + if (backlight_is_blank(bl))
> brightness = 0;
>
> return adp8870_bl_set(bl, brightness);
> diff --git a/drivers/video/backlight/as3711_bl.c
> b/drivers/video/backlight/as3711_bl.c
> index 33f0f0f2e8b3..7fa76008c7bf 100644
> --- a/drivers/video/backlight/as3711_bl.c
> +++ b/drivers/video/backlight/as3711_bl.c
> @@ -107,13 +107,11 @@ static int as3711_bl_update_status(struct
> backlight_device *bl)
> int brightness = bl->props.brightness;
> int ret =
Re: [PATCH v2 15/16] backlight: make of_find_backlight_by_node() static
On Sun, May 17, 2020 at 09:01:38PM +0200, Sam Ravnborg wrote:
> There are no external users of of_find_backlight_by_node().
> Make it static so we keep it that way.
>
> v2:
> - drop EXPORT of of_find_backlight_by_node
>
> Signed-off-by: Sam Ravnborg
> Cc: Lee Jones
> Cc: Daniel Thompson
> Cc: Jingoo Han
Assuming the 0day-ci comments are because some of the patches have
already been sucked up in a different tree then:
Reviewed-by: Daniel Thompson
Daniel.
> ---
> drivers/video/backlight/backlight.c | 23 +--
> include/linux/backlight.h | 10 --
> 2 files changed, 9 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/video/backlight/backlight.c
> b/drivers/video/backlight/backlight.c
> index 7f3eecaf8317..3fa29330a820 100644
> --- a/drivers/video/backlight/backlight.c
> +++ b/drivers/video/backlight/backlight.c
> @@ -627,19 +627,9 @@ static int of_parent_match(struct device *dev, const
> void *data)
> return dev->parent && dev->parent->of_node == data;
> }
>
> -/**
> - * of_find_backlight_by_node() - find backlight device by device-tree node
> - * @node: device-tree node of the backlight device
> - *
> - * Returns a pointer to the backlight device corresponding to the given DT
> - * node or NULL if no such backlight device exists or if the device hasn't
> - * been probed yet.
> - *
> - * This function obtains a reference on the backlight device and it is the
> - * caller's responsibility to drop the reference by calling put_device() on
> - * the backlight device's .dev field.
> - */
> -struct backlight_device *of_find_backlight_by_node(struct device_node *node)
> +/* Find backlight device by device-tree node */
> +static struct backlight_device *
> +of_find_backlight_by_node(struct device_node *node)
> {
> struct device *dev;
>
> @@ -647,7 +637,12 @@ struct backlight_device
> *of_find_backlight_by_node(struct device_node *node)
>
> return dev ? to_backlight_device(dev) : NULL;
> }
> -EXPORT_SYMBOL(of_find_backlight_by_node);
> +#else
> +static struct backlight_device *
> +of_find_backlight_by_node(struct device_node *node)
> +{
> + return NULL;
> +}
> #endif
>
> static struct backlight_device *of_find_backlight(struct device *dev)
> diff --git a/include/linux/backlight.h b/include/linux/backlight.h
> index d92e523650ec..1e29ab43f4f6 100644
> --- a/include/linux/backlight.h
> +++ b/include/linux/backlight.h
> @@ -473,16 +473,6 @@ static inline void * bl_get_data(struct backlight_device
> *bl_dev)
> return dev_get_drvdata(&bl_dev->dev);
> }
>
> -#ifdef CONFIG_OF
> -struct backlight_device *of_find_backlight_by_node(struct device_node *node);
> -#else
> -static inline struct backlight_device *
> -of_find_backlight_by_node(struct device_node *node)
> -{
> - return NULL;
> -}
> -#endif
> -
> #if IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
> struct backlight_device *devm_of_find_backlight(struct device *dev);
> #else
> --
> 2.25.1
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 14/16] backlight: drop backlight_put()
On Sun, May 17, 2020 at 09:01:37PM +0200, Sam Ravnborg wrote:
> There are no external users of backlight_put().
> Drop it and open code the two users in backlight.c.
>
> Signed-off-by: Sam Ravnborg
> Cc: Lee Jones
> Cc: Daniel Thompson
> Cc: Jingoo Han
Reviewed-by: Daniel Thompson
> ---
> drivers/video/backlight/backlight.c | 7 +--
> include/linux/backlight.h | 10 --
> 2 files changed, 5 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/video/backlight/backlight.c
> b/drivers/video/backlight/backlight.c
> index e23b09d03a65..7f3eecaf8317 100644
> --- a/drivers/video/backlight/backlight.c
> +++ b/drivers/video/backlight/backlight.c
> @@ -679,7 +679,10 @@ static struct backlight_device *of_find_backlight(struct
> device *dev)
>
> static void devm_backlight_release(void *data)
> {
> - backlight_put(data);
> + struct backlight_device *bd = data;
> +
> + if (bd)
> + put_device(&bd->dev);
> }
>
> /**
> @@ -707,7 +710,7 @@ struct backlight_device *devm_of_find_backlight(struct
> device *dev)
> return bd;
> ret = devm_add_action(dev, devm_backlight_release, bd);
> if (ret) {
> - backlight_put(bd);
> + put_device(&bd->dev);
> return ERR_PTR(ret);
> }
> return bd;
> diff --git a/include/linux/backlight.h b/include/linux/backlight.h
> index 99e7cdace2be..d92e523650ec 100644
> --- a/include/linux/backlight.h
> +++ b/include/linux/backlight.h
> @@ -415,16 +415,6 @@ static inline int backlight_disable(struct
> backlight_device *bd)
> return backlight_update_status(bd);
> }
>
> -/**
> - * backlight_put - Drop backlight reference
> - * @bd: the backlight device to put
> - */
> -static inline void backlight_put(struct backlight_device *bd)
> -{
> - if (bd)
> - put_device(&bd->dev);
> -}
> -
> /**
> * backlight_is_blank - Return true if display is expected to be blank
> * @bd: the backlight device
> --
> 2.25.1
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 13/16] backlight: make of_find_backlight static
On Sun, May 17, 2020 at 09:01:36PM +0200, Sam Ravnborg wrote:
> There are no external users of of_find_backlight,
> as they have all changed to use the managed version.
> Make of_find_backlight static to prevent new external users.
>
> Signed-off-by: Sam Ravnborg
> Cc: Lee Jones
> Cc: Daniel Thompson
> Cc: Jingoo Han
In principle I'm OK with this... just a couple of tiny nitpicks in the
new documentation.
Daniel.
> ---
> drivers/video/backlight/backlight.c | 31 ++---
> include/linux/backlight.h | 6 --
> 2 files changed, 11 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/video/backlight/backlight.c
> b/drivers/video/backlight/backlight.c
> index 2212f0e3570e..e23b09d03a65 100644
> --- a/drivers/video/backlight/backlight.c
> +++ b/drivers/video/backlight/backlight.c
> @@ -650,22 +650,7 @@ struct backlight_device
> *of_find_backlight_by_node(struct device_node *node)
> EXPORT_SYMBOL(of_find_backlight_by_node);
> #endif
>
> -/**
> - * of_find_backlight - Get backlight device
> - * @dev: Device
> - *
> - * This function looks for a property named 'backlight' on the DT node
> - * connected to @dev and looks up the backlight device.
> - *
> - * Call backlight_put() to drop the reference on the backlight device.
> - *
> - * Returns:
> - * A pointer to the backlight device if found.
> - * Error pointer -EPROBE_DEFER if the DT property is set, but no backlight
> - * device is found.
> - * NULL if there's no backlight property.
> - */
> -struct backlight_device *of_find_backlight(struct device *dev)
> +static struct backlight_device *of_find_backlight(struct device *dev)
> {
> struct backlight_device *bd = NULL;
> struct device_node *np;
> @@ -691,7 +676,6 @@ struct backlight_device *of_find_backlight(struct device
> *dev)
>
> return bd;
> }
> -EXPORT_SYMBOL(of_find_backlight);
>
> static void devm_backlight_release(void *data)
> {
> @@ -702,9 +686,16 @@ static void devm_backlight_release(void *data)
> * devm_of_find_backlight - find backlight for a device
> * @dev: the device
> *
> - * Device managed version of of_find_backlight().
> - * The reference on the backlight device is automatically
> - * dropped on driver detach.
> + * This function looks for a property named 'backlight' on the DT node
> + * connected to @dev and looks up the backlight device.
Should this be a full paragraph?
> + * The lookup is device managed so the reference to the backlight device
> + * is automatically dropped on driver detach.
> + *
> + * Returns:
Wasn't this upper case everywhere else?
> + * A pointer to the backlight device if found.
> + * Error pointer -EPROBE_DEFER if the DT property is set, but no backlight
> + * device is found.
> + * NULL if there's no backlight property.
> */
> struct backlight_device *devm_of_find_backlight(struct device *dev)
> {
> diff --git a/include/linux/backlight.h b/include/linux/backlight.h
> index 308aec67fa4f..99e7cdace2be 100644
> --- a/include/linux/backlight.h
> +++ b/include/linux/backlight.h
> @@ -494,14 +494,8 @@ of_find_backlight_by_node(struct device_node *node)
> #endif
>
> #if IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
> -struct backlight_device *of_find_backlight(struct device *dev);
> struct backlight_device *devm_of_find_backlight(struct device *dev);
> #else
> -static inline struct backlight_device *of_find_backlight(struct device *dev)
> -{
> - return NULL;
> -}
> -
> static inline struct backlight_device *
> devm_of_find_backlight(struct device *dev)
> {
> --
> 2.25.1
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 12/16] backlight: wire up kernel-doc documentation
On Sun, May 17, 2020 at 09:01:35PM +0200, Sam Ravnborg wrote: > Include backlight so the documentation is now generated > with make htmldocs and friends. > > Signed-off-by: Sam Ravnborg > Cc: Maarten Lankhorst > Cc: Maxime Ripard > Cc: Thomas Zimmermann > Cc: David Airlie > Cc: Daniel Vetter > Cc: Jonathan Corbet Reviewed-by: Daniel Thompson > --- > Documentation/gpu/backlight.rst | 12 > Documentation/gpu/index.rst | 1 + > 2 files changed, 13 insertions(+) > create mode 100644 Documentation/gpu/backlight.rst > > diff --git a/Documentation/gpu/backlight.rst b/Documentation/gpu/backlight.rst > new file mode 100644 > index ..9ebfc9d0aced > --- /dev/null > +++ b/Documentation/gpu/backlight.rst > @@ -0,0 +1,12 @@ > += > +Backlight support > += > + > +.. kernel-doc:: drivers/video/backlight/backlight.c > + :doc: overview > + > +.. kernel-doc:: include/linux/backlight.h > + :internal: > + > +.. kernel-doc:: drivers/video/backlight/backlight.c > + :export: > diff --git a/Documentation/gpu/index.rst b/Documentation/gpu/index.rst > index 1fcf8e851e15..c9a51e3bfb5a 100644 > --- a/Documentation/gpu/index.rst > +++ b/Documentation/gpu/index.rst > @@ -12,6 +12,7 @@ Linux GPU Driver Developer's Guide > drm-uapi > drm-client > drivers > + backlight > vga-switcheroo > vgaarbiter > todo > -- > 2.25.1 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 11/16] backlight: add overview and update existing doc
On Sun, May 17, 2020 at 09:01:34PM +0200, Sam Ravnborg wrote:
> Add overview chapter to backlight.c.
> Update existing kernel-doc to follow a more consistent
> style and drop kernel-doc for deprecated functions.
>
> Signed-off-by: Sam Ravnborg
> Cc: Lee Jones
> Cc: Daniel Thompson
> Cc: Jingoo Han
> ---
> drivers/video/backlight/backlight.c | 131 +++-
> 1 file changed, 90 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/video/backlight/backlight.c
> b/drivers/video/backlight/backlight.c
> index 17f04cff50ab..2212f0e3570e 100644
> --- a/drivers/video/backlight/backlight.c
> +++ b/drivers/video/backlight/backlight.c
> @@ -22,6 +22,45 @@
> #include
> #endif
>
> +/**
> + * DOC: overview
> + *
> + * The backlight core supports implementing backlight drivers.
> + *
> + * backlight is controlled from userspace via firmware, a platform
> + * specific way or via sysfs. The backlight core provide support
> + * for all three types of backlight control.
This paragraph seems very difficult for a reader to absorb (isn't
"controlled from userspace via firmware" more like the backlight
subsytem notifying the userspace when something else controls the
backlight behind the userspace's back).
Maybe just drop the paragraph? Notifications are covered further down
anyway.
> + *
> + * A backlight driver registers a driver using
> + * devm_backlight_device_register(). The properties of the backlight
> + * driver such as type and max_brightness must be specified.
> + * When the core detect changes in for example brightness or power state
> + * the update_status() operation is called. The backlight driver shall
> + * implement this operation and use it to adjust backlight.
> + *
> + * Several sysfs attributes are provided by the backlight core::
> + *
> + * - brightness R/W, set the requested brightness level
> + * - actual_brighness RO, the brightness level used by the HW
> + * - max_brightness RO, the maximum brightness level supported
I've not come across this markup before. Do all these extra
spaces create readable output when formatted?
> + *
> + * See Documentation/ABI/stable/sysfs-class-backlight for the full list.
> + *
> + * The driver shall implement the get_brightness() operation if
> + * the HW do not support all the levels that can be specified in
> + * brightness, thus providing user-space access to the actual level
> + * via the actual_brightness attribute.
Again... this doesn't look like the formatted output will get a
paragraph break here.
> + * When the backlight changes this is reported to user-space using
> + * an uevent connected to the actual_brightness attribute.
> + * When brightness is set by platform specific means, for example
> + * a hot-key to adjust backlight, the driver must notify the backlight
> + * core that brighness has changed using backlight_force_update().
> + *
> + * The backlight driver core receives notifications from fbdev and
> + * if the event is FB_EVENT_BLANK the value of blank, from the FBIOBLANK
> + * ioclt, is passed to the driver via the update_status() operation.
> + */
> +
> static struct list_head backlight_dev_list;
> static struct mutex backlight_dev_list_mutex;
> static struct blocking_notifier_head backlight_notifier;
> @@ -40,9 +79,17 @@ static const char *const backlight_scale_types[] = {
>
> #if defined(CONFIG_FB) || (defined(CONFIG_FB_MODULE) && \
> defined(CONFIG_BACKLIGHT_CLASS_DEVICE_MODULE))
> -/* This callback gets called when something important happens inside a
> - * framebuffer driver. We're looking if that important event is blanking,
> - * and if it is and necessary, we're switching backlight power as well ...
> +/*
> + * fb_notifier_callback
> + *
> + * This callback gets called when something important happens inside a
> + * framebuffer driver. The backlight core only care about FB_BLANK_UNBLANK
s/care/cares/
> + * which is reported to the driver using backlight_update_status()
> + * as a state change.
> + *
> + * There may be several fbdev's connected to the backlight device,
> + * in which case they are kept track of. A state change is only reported
> + * if there is a change in backligt for the specified fbdev.
Typo.
> */
> static int fb_notifier_callback(struct notifier_block *self,
> unsigned long event, void *data)
> @@ -318,12 +365,16 @@ static struct attribute *bl_device_attrs[] = {
> ATTRIBUTE_GROUPS(bl_device);
>
> /**
> - * backlight_force_update - tell the backlight subsystem that hardware state
> - * has changed
> + * backlight_force_update - force an update due to a hardware change
> * @bd: the backlight device to update
> + * @reason: the method used for the backlight update
> *
> * Updates the internal state of the backlight in response to a hardware
> event,
> - * and generate a uevent to notify userspace
> + * and generate an uevent to notify userspace.
s/generate/generates/
> + * A backli
Re: [PATCH v6 00/16] drm/i915: Add support for HDCP 1.4 over MST connectors
On 2020-05-18 at 10:32:09 -0400, Sean Paul wrote: > On Fri, May 15, 2020 at 10:48 AM Ramalingam C wrote: > > > > On 2020-04-29 at 15:54:46 -0400, Sean Paul wrote: > > > From: Sean Paul > > > > > > Changes in v6: > > > -Rebased on -tip > > > -Disabled HDCP over MST on GEN12 > > > -Addressed Lyude's review comments in the QUERY_STREAM_ENCRYPTION_STATUS > > > patch > > > > Sean, > > > > What is the test setup you have used? > > > > Hi Ram, > Thanks for the feedback. To be completely honest it's really > frustrating that I'm just now getting questions and review feedback > (which I've been begging for on IRC) on this review that could have > been addressed ~5 months ago. It's super disruptive to have to keep > switching back to this after a long hiatus and many i915 refactors > complicating my rebases. Hi Sean, As a developer I really feel bad for the delay happened in review. I couldn't spend required time for understanding MST part hence I couldn't review. Just for this series now I have started preparing myself on these topics, hence started reviewing the series. If you are still interested to work on this, I can commit for regular reviews. Thanks, Ram. > > If no one wants this patchset, that's fine, please just let me know so > I don't waste any more time. If Intel is interested, could we please > stop the review trickle and lay out exactly what needs to be done to > get this landed? > > Sean > > > > I am afraid our CI dont have the coverage for MST capability yet to provide > > the functional status of the code. > > > > -Ram. > > > > > > Sean Paul (16): > > > drm/i915: Fix sha_text population code > > > drm/i915: Clear the repeater bit on HDCP disable > > > drm/i915: WARN if HDCP signalling is enabled upon disable > > > drm/i915: Intercept Aksv writes in the aux hooks > > > drm/i915: Use the cpu_transcoder in intel_hdcp to toggle HDCP > > > signalling > > > drm/i915: Factor out hdcp->value assignments > > > drm/i915: Protect workers against disappearing connectors > > > drm/i915: Don't fully disable HDCP on a port if multiple pipes are > > > using it > > > drm/i915: Support DP MST in enc_to_dig_port() function > > > drm/i915: Use ddi_update_pipe in intel_dp_mst > > > drm/i915: Factor out HDCP shim functions from dp for use by dp_mst > > > drm/i915: Plumb port through hdcp init > > > drm/i915: Add connector to hdcp_shim->check_link() > > > drm/mst: Add support for QUERY_STREAM_ENCRYPTION_STATUS MST sideband > > > message > > > drm/i915: Print HDCP version info for all connectors > > > drm/i915: Add HDCP 1.4 support for MST connectors > > > > > > drivers/gpu/drm/drm_dp_mst_topology.c | 142 > > > drivers/gpu/drm/i915/Makefile | 1 + > > > drivers/gpu/drm/i915/display/intel_ddi.c | 29 +- > > > drivers/gpu/drm/i915/display/intel_ddi.h | 2 + > > > .../drm/i915/display/intel_display_debugfs.c | 21 +- > > > .../drm/i915/display/intel_display_types.h| 30 +- > > > drivers/gpu/drm/i915/display/intel_dp.c | 654 +-- > > > drivers/gpu/drm/i915/display/intel_dp.h | 9 + > > > drivers/gpu/drm/i915/display/intel_dp_hdcp.c | 743 ++ > > > drivers/gpu/drm/i915/display/intel_dp_mst.c | 19 + > > > drivers/gpu/drm/i915/display/intel_hdcp.c | 217 +++-- > > > drivers/gpu/drm/i915/display/intel_hdcp.h | 2 +- > > > drivers/gpu/drm/i915/display/intel_hdmi.c | 25 +- > > > .../drm/selftests/test-drm_dp_mst_helper.c| 17 + > > > include/drm/drm_dp_helper.h | 3 + > > > include/drm/drm_dp_mst_helper.h | 44 ++ > > > include/drm/drm_hdcp.h| 3 + > > > 17 files changed, 1235 insertions(+), 726 deletions(-) > > > create mode 100644 drivers/gpu/drm/i915/display/intel_dp_hdcp.c > > > > > > -- > > > Sean Paul, Software Engineer, Google / Chromium OS > > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/4] drm: Help unconfuse gcc, avoid accidental impossible unsigned comparisons
Quoting Emil Velikov (2020-05-18 17:23:21) > On Sat, 16 May 2020 at 22:23, Chris Wilson wrote: > > > > drivers/gpu/drm/drm_client_modeset.c: In function > > ‘drm_client_firmware_config’: > > ./include/linux/bits.h:26:28: warning: comparison of unsigned expression < > > 0 is always false [-Wtype-limits] > >__builtin_constant_p((l) > (h)), (l) > (h), 0))) > > > Hmm this seems like a hack around a macro bug. I'll send a fix for the > macro in a few minutes. > With that you don't have to tinker to make it unsigned, although const > will be appreciated. Streams crossed. I added Daniel's suggestion for a WARN_ON to make it clear that we should never pass connectors_count==0 to drm_client_firmware_config -Chris ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Freedreno] [PATCH 5/6] drm: msm: a6xx: use dev_pm_opp_set_bw to set DDR bandwidth
On Mon, May 18, 2020 at 7:23 AM Jordan Crouse wrote:
>
> On Thu, May 14, 2020 at 04:24:18PM +0530, Sharat Masetty wrote:
> > This patches replaces the previously used static DDR vote and uses
> > dev_pm_opp_set_bw() to scale GPU->DDR bandwidth along with scaling
> > GPU frequency.
> >
> > Signed-off-by: Sharat Masetty
> > ---
> > drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 6 +-
> > 1 file changed, 1 insertion(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> > b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> > index 2d8124b..79433d3 100644
> > --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> > @@ -141,11 +141,7 @@ void a6xx_gmu_set_freq(struct msm_gpu *gpu, struct
> > dev_pm_opp *opp)
> >
> > gmu->freq = gmu->gpu_freqs[perf_index];
> >
> > - /*
> > - * Eventually we will want to scale the path vote with the frequency
> > but
> > - * for now leave it at max so that the performance is nominal.
> > - */
> > - icc_set_bw(gpu->icc_path, 0, MBps_to_icc(7216));
> > + dev_pm_opp_set_bw(&gpu->pdev->dev, opp);
> > }
>
> This adds an implicit requirement that all targets need bandwidth settings
> defined in the OPP or they won't get a bus vote at all. I would prefer that
> there be an default escape valve but if not you'll need to add
> bandwidth values for the sdm845 OPP that target doesn't regress.
>
it looks like we could maybe do something like:
ret = dev_pm_opp_set_bw(...);
if (ret) {
dev_warn_once(dev, "no bandwidth settings");
icc_set_bw(...);
}
?
BR,
-R
> Jordan
>
> > unsigned long a6xx_gmu_get_freq(struct msm_gpu *gpu)
> > --
> > 2.7.4
> >
>
> --
> The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project
> ___
> Freedreno mailing list
> freedr...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/freedreno
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/4] drm: Help unconfuse gcc, avoid accidental impossible unsigned comparisons
On Sat, 16 May 2020 at 22:23, Chris Wilson wrote: > > drivers/gpu/drm/drm_client_modeset.c: In function > ‘drm_client_firmware_config’: > ./include/linux/bits.h:26:28: warning: comparison of unsigned expression < 0 > is always false [-Wtype-limits] >__builtin_constant_p((l) > (h)), (l) > (h), 0))) > Hmm this seems like a hack around a macro bug. I'll send a fix for the macro in a few minutes. With that you don't have to tinker to make it unsigned, although const will be appreciated. -Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 10/16] backlight: drop extern from prototypes
On Sun, May 17, 2020 at 09:01:33PM +0200, Sam Ravnborg wrote: > No need to put "extern" in front of prototypes. > While touching the prototypes adjust indent to follow > the kernel style. > > Signed-off-by: Sam Ravnborg > Cc: Lee Jones > Cc: Daniel Thompson > Cc: Jingoo Han Reviewed-by: Daniel Thompson > --- > include/linux/backlight.h | 35 +++ > 1 file changed, 19 insertions(+), 16 deletions(-) > > diff --git a/include/linux/backlight.h b/include/linux/backlight.h > index eae7a5e66248..308aec67fa4f 100644 > --- a/include/linux/backlight.h > +++ b/include/linux/backlight.h > @@ -444,22 +444,25 @@ static inline bool backlight_is_blank(struct > backlight_device *bd) > bd->props.state & (BL_CORE_SUSPENDED | BL_CORE_FBBLANK); > } > > -extern struct backlight_device *backlight_device_register(const char *name, > - struct device *dev, void *devdata, const struct backlight_ops *ops, > - const struct backlight_properties *props); > -extern struct backlight_device *devm_backlight_device_register( > - struct device *dev, const char *name, struct device *parent, > - void *devdata, const struct backlight_ops *ops, > - const struct backlight_properties *props); > -extern void backlight_device_unregister(struct backlight_device *bd); > -extern void devm_backlight_device_unregister(struct device *dev, > - struct backlight_device *bd); > -extern void backlight_force_update(struct backlight_device *bd, > -enum backlight_update_reason reason); > -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); > -extern int backlight_device_set_brightness(struct backlight_device *bd, > unsigned long brightness); > +struct backlight_device * > +backlight_device_register(const char *name, struct device *dev, void > *devdata, > + const struct backlight_ops *ops, > + const struct backlight_properties *props); > +struct backlight_device * > +devm_backlight_device_register(struct device *dev, const char *name, > +struct device *parent, void *devdata, > +const struct backlight_ops *ops, > +const struct backlight_properties *props); > +void backlight_device_unregister(struct backlight_device *bd); > +void devm_backlight_device_unregister(struct device *dev, > + struct backlight_device *bd); > +void backlight_force_update(struct backlight_device *bd, > + enum backlight_update_reason reason); > +int backlight_register_notifier(struct notifier_block *nb); > +int backlight_unregister_notifier(struct notifier_block *nb); > +struct backlight_device *backlight_device_get_by_type(enum backlight_type > type); > +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.25.1 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 09/16] backlight: remove the unused backlight_bl driver
On Sun, May 17, 2020 at 09:01:32PM +0200, Sam Ravnborg wrote:
> The driver required initialization using struct generic_bl_info.
> As there are no more references to this struct there is no users left.
> So it is safe to delete the driver.
>
> Signed-off-by: Sam Ravnborg
> Cc: Lee Jones
> Cc: Daniel Thompson
> Cc: Jingoo Han
Reviewed-by: Daniel Thompson
> ---
> drivers/video/backlight/Kconfig | 8 --
> drivers/video/backlight/Makefile | 1 -
> drivers/video/backlight/generic_bl.c | 110 ---
> include/linux/backlight.h| 9 ---
> 4 files changed, 128 deletions(-)
> delete mode 100644 drivers/video/backlight/generic_bl.c
>
> diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
> index 7d22d7377606..14abfeee8868 100644
> --- a/drivers/video/backlight/Kconfig
> +++ b/drivers/video/backlight/Kconfig
> @@ -173,14 +173,6 @@ config BACKLIGHT_EP93XX
> To compile this driver as a module, choose M here: the module will
> be called ep93xx_bl.
>
> -config BACKLIGHT_GENERIC
> - tristate "Generic (aka Sharp Corgi) Backlight Driver"
> - default y
> - help
> - Say y to enable the generic platform backlight driver previously
> - known as the Corgi backlight driver. If you have a Sharp Zaurus
> - SL-C7xx, SL-Cxx00 or SL-6000x say y.
> -
> config BACKLIGHT_IPAQ_MICRO
> tristate "iPAQ microcontroller backlight driver"
> depends on MFD_IPAQ_MICRO
> diff --git a/drivers/video/backlight/Makefile
> b/drivers/video/backlight/Makefile
> index 0c1a1524627a..9b998cfdc56d 100644
> --- a/drivers/video/backlight/Makefile
> +++ b/drivers/video/backlight/Makefile
> @@ -31,7 +31,6 @@ obj-$(CONFIG_BACKLIGHT_CLASS_DEVICE)+= backlight.o
> obj-$(CONFIG_BACKLIGHT_DA903X) += da903x_bl.o
> obj-$(CONFIG_BACKLIGHT_DA9052) += da9052_bl.o
> obj-$(CONFIG_BACKLIGHT_EP93XX) += ep93xx_bl.o
> -obj-$(CONFIG_BACKLIGHT_GENERIC) += generic_bl.o
> obj-$(CONFIG_BACKLIGHT_GPIO) += gpio_backlight.o
> obj-$(CONFIG_BACKLIGHT_HP680)+= hp680_bl.o
> obj-$(CONFIG_BACKLIGHT_HP700)+= jornada720_bl.o
> diff --git a/drivers/video/backlight/generic_bl.c
> b/drivers/video/backlight/generic_bl.c
> deleted file mode 100644
> index 8fe63dbc8590..
> --- a/drivers/video/backlight/generic_bl.c
> +++ /dev/null
> @@ -1,110 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0-only
> -/*
> - * Generic Backlight Driver
> - *
> - * Copyright (c) 2004-2008 Richard Purdie
> - */
> -
> -#include
> -#include
> -#include
> -#include
> -#include
> -#include
> -#include
> -
> -static int genericbl_intensity;
> -static struct backlight_device *generic_backlight_device;
> -static struct generic_bl_info *bl_machinfo;
> -
> -static int genericbl_send_intensity(struct backlight_device *bd)
> -{
> - int intensity = bd->props.brightness;
> -
> - if (bd->props.power != FB_BLANK_UNBLANK)
> - intensity = 0;
> - if (bd->props.state & BL_CORE_FBBLANK)
> - intensity = 0;
> - if (bd->props.state & BL_CORE_SUSPENDED)
> - intensity = 0;
> -
> - bl_machinfo->set_bl_intensity(intensity);
> -
> - genericbl_intensity = intensity;
> -
> - if (bl_machinfo->kick_battery)
> - bl_machinfo->kick_battery();
> -
> - return 0;
> -}
> -
> -static int genericbl_get_intensity(struct backlight_device *bd)
> -{
> - return genericbl_intensity;
> -}
> -
> -static const struct backlight_ops genericbl_ops = {
> - .options = BL_CORE_SUSPENDRESUME,
> - .get_brightness = genericbl_get_intensity,
> - .update_status = genericbl_send_intensity,
> -};
> -
> -static int genericbl_probe(struct platform_device *pdev)
> -{
> - struct backlight_properties props;
> - struct generic_bl_info *machinfo = dev_get_platdata(&pdev->dev);
> - const char *name = "generic-bl";
> - struct backlight_device *bd;
> -
> - bl_machinfo = machinfo;
> - if (!machinfo->limit_mask)
> - machinfo->limit_mask = -1;
> -
> - if (machinfo->name)
> - name = machinfo->name;
> -
> - memset(&props, 0, sizeof(struct backlight_properties));
> - props.type = BACKLIGHT_RAW;
> - props.max_brightness = machinfo->max_intensity;
> - bd = devm_backlight_device_register(&pdev->dev, name, &pdev->dev,
> - NULL, &genericbl_ops, &props);
> - if (IS_ERR(bd))
> - return PTR_ERR(bd);
> -
> - platform_set_drvdata(pdev, bd);
> -
> - bd->props.power = FB_BLANK_UNBLANK;
> - bd->props.brightness = machinfo->default_intensity;
> - backlight_update_status(bd);
> -
> - generic_backlight_device = bd;
> -
> - dev_info(&pdev->dev, "Generic Backlight Driver Initialized.\n");
> - return 0;
> -}
> -
> -static int genericbl_remove(struct platform_device *pdev)
> -{
> - struct backlight_device
Re: [PATCH v2 08/16] backlight: document enums in backlight.h
On Sun, May 17, 2020 at 09:01:31PM +0200, Sam Ravnborg wrote:
> Add kernel-doc documentation for the backlight enums
>
> Signed-off-by: Sam Ravnborg
> Cc: Lee Jones
> Cc: Daniel Thompson
> Cc: Jingoo Han
Other than the usual comment about the form in the kernel-doc howto:
Reviewed-by: Daniel Thompson
> ---
> include/linux/backlight.h | 84 +++
> 1 file changed, 84 insertions(+)
>
> diff --git a/include/linux/backlight.h b/include/linux/backlight.h
> index 98349a2984dc..b779c29142fd 100644
> --- a/include/linux/backlight.h
> +++ b/include/linux/backlight.h
> @@ -14,26 +14,110 @@
> #include
> #include
>
> +/**
> + * enum backlight_update_reason - what method was used to update backlight
> + *
> + * A driver indicates the method (reason) used for updating the backlight
> + * when calling backlight_force_update().
> + */
> enum backlight_update_reason {
> + /**
> + * @BACKLIGHT_UPDATE_HOTKEY:
> + *
> + * The backlight was updated using a hot-key.
> + */
> BACKLIGHT_UPDATE_HOTKEY,
> +
> + /**
> + * @BACKLIGHT_UPDATE_SYSFS:
> + *
> + * The backlight was updated using sysfs.
> + */
> BACKLIGHT_UPDATE_SYSFS,
> };
>
> +/**
> + * enum backlight_type - the type of backlight control
> + *
> + * The type of interface used to control the backlight.
> + */
> enum backlight_type {
> + /**
> + * @BACKLIGHT_RAW:
> + *
> + * The backlight is controlled using hardware registers directly.
> + */
> BACKLIGHT_RAW = 1,
> +
> + /**
> + * @BACKLIGHT_PLATFORM:
> + *
> + * The backlight is controller using a platform-specific interface.
> + */
> BACKLIGHT_PLATFORM,
> +
> + /**
> + * @BACKLIGHT_FIRMWARE:
> + *
> + * The backlight is controlled using a standard firmware interface.
> + */
> BACKLIGHT_FIRMWARE,
> +
> + /**
> + * @BACKLIGHT_TYPE_MAX: Number of entries.
> + */
> BACKLIGHT_TYPE_MAX,
> };
>
> +/**
> + * enum backlight_notification - the type of notification
> + *
> + * The notifications that is used for notification sent to the receiver
> + * that registered notifications using backlight_register_notifier().
> + */
> enum backlight_notification {
> + /**
> + * @BACKLIGHT_REGISTERED:
> + *
> + * The backlight device is registered.
> + */
> BACKLIGHT_REGISTERED,
> +
> + /**
> + * @BACKLIGHT_UNREGISTERED:
> + *
> + * The backlight revice is unregistered.
> + */
> BACKLIGHT_UNREGISTERED,
> };
>
> +/** enum backlight_scale - the type of scale used for brightness values
> + *
> + * The type of scale used for brightness values.
> + */
> enum backlight_scale {
> + /**
> + * @BACKLIGHT_SCALE_UNKNOWN:
> + *
> + * The scale is unknown.
> + */
> BACKLIGHT_SCALE_UNKNOWN = 0,
> +
> + /**
> + * @BACKLIGHT_SCALE_LINEAR:
> + *
> + * The scale is linear, so the brigness increase is the same
> + * when increasing with the same steps (more or less).
> + */
> BACKLIGHT_SCALE_LINEAR,
> +
> + /**
> + * @BACKLIGHT_SCALE_NON_LINEAR:
> + *
> + * The scale is not linear. This is often used when the brightness
> + * values tries to adjust to the relative perception of the eye
> + * demanding a non-linear scale.
> + */
> BACKLIGHT_SCALE_NON_LINEAR,
> };
>
> --
> 2.25.1
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 07/16] backlight: document inline functions in backlight.h
On Sun, May 17, 2020 at 09:01:30PM +0200, Sam Ravnborg wrote:
> Add documentation for the inline functions in backlight.h
>
> Signed-off-by: Sam Ravnborg
> Cc: Lee Jones
> Cc: Daniel Thompson
> Cc: Jingoo Han
> ---
> include/linux/backlight.h | 16
> 1 file changed, 16 insertions(+)
>
> diff --git a/include/linux/backlight.h b/include/linux/backlight.h
> index e2d72936bf05..98349a2984dc 100644
> --- a/include/linux/backlight.h
> +++ b/include/linux/backlight.h
> @@ -283,6 +283,10 @@ struct backlight_device {
> int use_count;
> };
>
> +/**
> + * backlight_update_status - force an update of the backligt device status
Typo.
Other than that,
Reviewed-by: Daniel Thompson
Daniel.
> + * @bd: the backlight device
> + */
> static inline int backlight_update_status(struct backlight_device *bd)
> {
> int ret = -ENOENT;
> @@ -375,6 +379,18 @@ extern int backlight_device_set_brightness(struct
> backlight_device *bd, unsigned
>
> #define to_backlight_device(obj) container_of(obj, struct backlight_device,
> dev)
>
> +/**
> + * bl_get_data - access devdata
> + * @bl_dev: pointer to backlight device
> + *
> + * When a backlight device is registered the driver has the possibility
> + * to supply a void * devdata. bl_get_data() return a pointer to the
> + * devdata.
> + *
> + * RETURNS:
> + *
> + * pointer to devdata stored while registering the backlight device.
> + */
> static inline void * bl_get_data(struct backlight_device *bl_dev)
> {
> return dev_get_drvdata(&bl_dev->dev);
> --
> 2.25.1
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 06/16] backlight: improve backlight_device documentation
On Sun, May 17, 2020 at 09:01:29PM +0200, Sam Ravnborg wrote:
> Improve the documentation for backlight_device and
> adapt it to kernel-doc style.
>
> Signed-off-by: Sam Ravnborg
> Cc: Lee Jones
> Cc: Daniel Thompson
> Cc: Jingoo Han
> ---
> include/linux/backlight.h | 81 ---
> 1 file changed, 58 insertions(+), 23 deletions(-)
>
> diff --git a/include/linux/backlight.h b/include/linux/backlight.h
> index 7f9cef299d6e..e2d72936bf05 100644
> --- a/include/linux/backlight.h
> +++ b/include/linux/backlight.h
> @@ -14,21 +14,6 @@
> #include
> #include
>
> -/* Notes on locking:
> - *
> - * backlight_device->ops_lock is an internal backlight lock protecting the
> - * ops pointer and no code outside the core should need to touch it.
> - *
> - * Access to update_status() is serialised by the update_lock mutex since
> - * most drivers seem to need this and historically get it wrong.
> - *
> - * Most drivers don't need locking on their get_brightness() method.
> - * If yours does, you need to implement it in the driver. You can use the
> - * update_lock mutex if appropriate.
> - *
> - * Any other use of the locks below is probably wrong.
> - */
> -
> enum backlight_update_reason {
> BACKLIGHT_UPDATE_HOTKEY,
> BACKLIGHT_UPDATE_SYSFS,
> @@ -221,30 +206,80 @@ struct backlight_properties {
> enum backlight_scale scale;
> };
>
> +/**
> + * struct backlight_device - backlight device data
> + *
> + * This structure holds all data required by a backlight device.
> + */
> struct backlight_device {
> - /* Backlight properties */
> + /**
> + * @props:
> + *
As last patch. Why no brief descriptions?
> + * Backlight properties
> + */
> struct backlight_properties props;
>
> - /* Serialise access to update_status method */
> + /**
> + * @update_lock:
> + *
> + * update_lock is an internal backlight lock that serialise access
> + * to the update_status() method. The iupdate_lock mutex shall not be
> used
> + * by backlight drivers.
In addition to the typo this directly contradicts the advice in the
original "Notes on locking".
A change this dramatic needs to be fully explaining in the patch
description.
Daniel.
> + */
> struct mutex update_lock;
>
> - /* This protects the 'ops' field. If 'ops' is NULL, the driver that
> -registered this device has been unloaded, and if class_get_devdata()
> -points to something in the body of that driver, it is also invalid.
> */
> + /**
> + * @ops_lock:
> + *
> + * ops_lock is an internal backlight lock that protects the ops pointer
> + * and is used around all accesses to ops and when the operations are
> + * invoked. The mutex shall not be used by backlight drivers.
> + */
> struct mutex ops_lock;
> +
> + /**
> + * @ops:
> + *
> + * Pointer to the backlight operations. If ops is NULL, the driver that
> + * registered this device has been unloaded, and if class_get_devdata()
> + * points to something in the body of that driver, it is also invalid.
> + */
> const struct backlight_ops *ops;
>
> - /* The framebuffer notifier block */
> + /**
> + * @fb_notif:
> + *
> + * The framebuffer notifier block
> + */
> struct notifier_block fb_notif;
>
> - /* list entry of all registered backlight devices */
> + /**
> + * @entry:
> + *
> + * List entry of all registered backlight devices
> + */
> struct list_head entry;
>
> + /**
> + * @dev:
> + *
> + * Parent device.
> + */
> struct device dev;
>
> - /* Multiple framebuffers may share one backlight device */
> + /**
> + * @fb_bl_on:
> + *
> + * Multiple fbdev's may share one backlight device. The fb_bl_on
> + * records the state of the individual fbdev.
> + */
> bool fb_bl_on[FB_MAX];
>
> + /**
> + * @use_count:
> + *
> + * The number of uses of fb_bl_on.
> + */
> int use_count;
> };
>
> --
> 2.25.1
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 05/16] backlight: improve backlight_properties documentation
On Sun, May 17, 2020 at 09:01:28PM +0200, Sam Ravnborg wrote:
> Improve the documentation for backlight_properties and
> adapt it to kernel-doc style.
>
> Signed-off-by: Sam Ravnborg
> Cc: Lee Jones
> Cc: Daniel Thompson
> Cc: Jingoo Han
Overall looks good but enough nits that I felt compelled to comment!
> ---
> include/linux/backlight.h | 101 +-
> 1 file changed, 90 insertions(+), 11 deletions(-)
>
> diff --git a/include/linux/backlight.h b/include/linux/backlight.h
> index 519dc61ce7e4..7f9cef299d6e 100644
> --- a/include/linux/backlight.h
> +++ b/include/linux/backlight.h
> @@ -118,28 +118,107 @@ struct backlight_ops {
> int (*check_fb)(struct backlight_device *bd, struct fb_info *info);
> };
>
> -/* This structure defines all the properties of a backlight */
> +/**
> + * struct backlight_properties - backlight properties
> + *
> + * This structure defines all the properties of a backlight.
> + */
> struct backlight_properties {
> - /* Current User requested brightness (0 - max_brightness) */
> + /**
> + * @brightness:
> + *
> + * The current requested brightness by the user.
This applies throughout this file (and perhaps I overlooked it in the
previous patc too) but having line breaks after @brightness: differs
from the canonical description of a kerneldoc command in:
https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#in-line-member-documentation-comments
Also: s/requested brightness/brightness requested/
> + * The backlight core makes sure the range is (0 - max_brightness)
I know this is a copy of the original text but I'd prefer the range not
to use the subtract operator ;-). Maybe 0..max_brightness like the
ranges below?
> + * when the brightness is set via the sysfs attribute:
> + * /sys/class/backlight//brightness.
> + *
> + * This value can be set in the backlight_properties passed
> + * to devm_backlight_device_register() to set a default brightness
> + * value.
> + */
> int brightness;
> - /* Maximal value for brightness (read-only) */
> +
> + /**
> + * @max_brightness:
> + *
> + * The maximum brightness value.
> + *
> + * This value must be set in the backlight_properties passed
> + * to devm_backlight_device_register().
> + *
> + * This property must not be modified by a driver except
> + * before registering the backlight device as explained above.
Perhaps combine these (rather than "as explained above"):
This value must be set in the backlight_properties passed to
devm_backlight_device_register() and shall not be modified by the
driver after registration.
> + */
> int max_brightness;
> - /* Current FB Power mode (0: full on, 1..3: power saving
> -modes; 4: full off), see FB_BLANK_XXX */
> +
> + /**
> + * @power:
> + *
> + * The current power mode. User space configure the power mode using
s/configure/can configure/
> + * the sysfs attribute: /sys/class/backlight//bl_power
> + * When the power property is updated update_status() is called.
> + *
> + * The possible values are: (0: full on, 1..3: power saving
> + * modes; 4: full off), see FB_BLANK_XXX.
> + *
> + * When the backlight device is enabled @power is set
> + * to FB_BLANK_UNBLANK. When the backlight device is disabled
> + * @power is set to FB_BLANK_POWERDOWN.
> + */
> int power;
> - /* FB Blanking active? (values as for power) */
> - /* Due to be removed, please use (state & BL_CORE_FBBLANK) */
> +
> + /**
> + * @fb_blank:
> + *
> + * When the FBIOBLANK ioctl is called fb_blank is set to the
> + * blank parameter and the update_status() operation is called.
> + *
> + * When the backlight device is enabled @fb_blank is set
> + * to FB_BLANK_UNBLANK. When the backlight device is disabled
> + * @fb_blank is set to FB_BLANK_POWERDOWN.
> + *
> + * This property must not be modified by a driver.
> + * The backlight driver shall never read this variable,
> + * as the necessary info is avaialble via the state.
I'd rather be told what to do that what not to do!
Maybe.
Backlight drivers should avoid using this property. It has been
replaced by state & BL_CORE_FBLANK (although most drivers should
use backlight_is_blank() as the preferred means to get the blank
state.
Daniel.
> + *
> + * fb_blank is deprecated and will be removed.
> + */
> int fb_blank;
> - /* Backlight type */
> +
> + /**
> + * @type:
> + *
> + * The type of backlight supported.
> + * The backlight type allows userspace to make appropriate
> + * policy desicions based on the backlight type.
> + *
> + * This value must be set in the backlight_properties
> + * passed to devm_backlight_device_register().
> + */
>
Re: [PATCH] drm/mediatek: Delete not used of_device_get_match_data
Hi, Matthias:
於 2020年5月18日 週一 下午7:23寫道：
>
> From: Matthias Brugger
>
> The driver will be loaded by via a platform device. So we
> will need to get the device_node from the parent device.
> Depending on this we will set the driver data.
> As all this is done later already, just delete the call to
> of_device_get_match_data.
>
Reviewed-by: Chun-Kuang Hu
> Signed-off-by: Matthias Brugger
> ---
> drivers/gpu/drm/mediatek/mtk_drm_drv.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> index e2bb0d19ef99..63ec92ba0e92 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> @@ -447,7 +447,6 @@ static int mtk_drm_probe(struct platform_device *pdev)
> if (!private)
> return -ENOMEM;
>
> - private->data = of_device_get_match_data(dev);
> private->mmsys_dev = dev->parent;
> if (!private->mmsys_dev) {
> dev_err(dev, "Failed to get MMSYS device\n");
> --
> 2.26.2
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: print the current->comm alongside the pid
On Mon, May 18, 2020 at 09:33:15AM +0100, Emil Velikov wrote:
> From: Emil Velikov
>
> The question of "what process is this pid" keeps on popping up, so lets
> print the process name alongside the pid.
>
> Cc: Mauro Rossi
> Cc: Bob Beckett
> Cc: Pekka Paalanen
> Signed-off-by: Emil Velikov
> ---
> drivers/gpu/drm/drm_file.c | 7 ---
> drivers/gpu/drm/drm_ioc32.c | 4 ++--
> drivers/gpu/drm/drm_ioctl.c | 11 ++-
> 3 files changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
> index 7194e67e78bd..9b79bfc60ad7 100644
> --- a/drivers/gpu/drm/drm_file.c
> +++ b/drivers/gpu/drm/drm_file.c
> @@ -253,8 +253,8 @@ void drm_file_free(struct drm_file *file)
>
> dev = file->minor->dev;
>
> - DRM_DEBUG("pid = %d, device = 0x%lx, open_count = %d\n",
> - task_pid_nr(current),
> + DRM_DEBUG("comm=\"%s\", pid=%d, dev=0x%lx, open_count=%d\n",
> + current->comm, task_pid_nr(current),
> (long)old_encode_dev(file->minor->kdev->devt),
> atomic_read(&dev->open_count));
>
> @@ -345,7 +345,8 @@ static int drm_open_helper(struct file *filp, struct
> drm_minor *minor)
> if (dev->switch_power_state != DRM_SWITCH_POWER_ON &&
> dev->switch_power_state != DRM_SWITCH_POWER_DYNAMIC_OFF)
> return -EINVAL;
>
> - DRM_DEBUG("pid = %d, minor = %d\n", task_pid_nr(current), minor->index);
> + DRM_DEBUG("comm=\"%s\", pid=%d, minor=%d\n", current->comm,
> + task_pid_nr(current), minor->index);
>
> priv = drm_file_alloc(minor);
> if (IS_ERR(priv))
> diff --git a/drivers/gpu/drm/drm_ioc32.c b/drivers/gpu/drm/drm_ioc32.c
> index 22c7fd7196c8..ff5d40036e21 100644
> --- a/drivers/gpu/drm/drm_ioc32.c
> +++ b/drivers/gpu/drm/drm_ioc32.c
> @@ -985,8 +985,8 @@ long drm_compat_ioctl(struct file *filp, unsigned int
> cmd, unsigned long arg)
> if (!fn)
> return drm_ioctl(filp, cmd, arg);
>
> - DRM_DEBUG("pid=%d, dev=0x%lx, auth=%d, %s\n",
> - task_pid_nr(current),
> + DRM_DEBUG("comm=\"%s\", pid=%d, dev=0x%lx, auth=%d, %s\n",
> + current->comm, task_pid_nr(current),
> (long)old_encode_dev(file_priv->minor->kdev->devt),
> file_priv->authenticated,
> drm_compat_ioctls[nr].name);
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index 73e31dd4e442..b03c34fb4467 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -852,8 +852,8 @@ long drm_ioctl(struct file *filp,
> out_size = 0;
> ksize = max(max(in_size, out_size), drv_size);
>
> - DRM_DEBUG("pid=%d, dev=0x%lx, auth=%d, %s\n",
> - task_pid_nr(current),
> + DRM_DEBUG("comm=\"%s\" pid=%d, dev=0x%lx, auth=%d, %s\n",
> + current->comm, task_pid_nr(current),
> (long)old_encode_dev(file_priv->minor->kdev->devt),
> file_priv->authenticated, ioctl->name);
>
> @@ -890,15 +890,16 @@ long drm_ioctl(struct file *filp,
>
>err_i1:
> if (!ioctl)
> - DRM_DEBUG("invalid ioctl: pid=%d, dev=0x%lx, auth=%d,
> cmd=0x%02x, nr=0x%02x\n",
> - task_pid_nr(current),
> + DRM_DEBUG("invalid ioctl: comm=\"%s\", pid=%d, dev=0x%lx,
> auth=%d, cmd=0x%02x, nr=0x%02x\n",
> + current->comm, task_pid_nr(current),
> (long)old_encode_dev(file_priv->minor->kdev->devt),
> file_priv->authenticated, cmd, nr);
>
> if (kdata != stack_kdata)
> kfree(kdata);
> if (retcode)
> - DRM_DEBUG("pid=%d, ret = %d\n", task_pid_nr(current), retcode);
> + DRM_DEBUG("comm=\"%s\", pid=%d, ret = %d\n", current->comm,
Nit: Since you adjust them all, also delete the spaces around the = here
in ret = %d. With that
Reviewed-by: Daniel Vetter
> + task_pid_nr(current), retcode);
> return retcode;
> }
> EXPORT_SYMBOL(drm_ioctl);
> --
> 2.25.1
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 04/16] backlight: improve backlight_ops documentation
On Sun, May 17, 2020 at 09:01:27PM +0200, Sam Ravnborg wrote:
> Improve the documentation for backlight_ops and
> adapt it to kernel-doc style.
>
> Signed-off-by: Sam Ravnborg
> Cc: Lee Jones
> Cc: Daniel Thompson
> Cc: Jingoo Han
Reviewed-by: Daniel Thompson
> ---
> include/linux/backlight.h | 60 +++
> 1 file changed, 54 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/backlight.h b/include/linux/backlight.h
> index a0a083b35c47..519dc61ce7e4 100644
> --- a/include/linux/backlight.h
> +++ b/include/linux/backlight.h
> @@ -55,19 +55,67 @@ enum backlight_scale {
> struct backlight_device;
> struct fb_info;
>
> +/**
> + * struct backlight_ops - backlight operations
> + *
> + * The backlight operations are specifed when the backlight device is
> registered.
> + */
> struct backlight_ops {
> + /**
> + * @options:
> + *
> + * The options parameter is used to adjust the behaviour of the core.
> + * Set BL_CORE_SUSPENDRESUME to get the update_status() operation called
> + * upon suspend and resume.
> + */
> unsigned int options;
>
> #define BL_CORE_SUSPENDRESUME(1 << 0)
>
> - /* Notify the backlight driver some property has changed */
> + /**
> + * @update_status:
> + *
> + * Notify the backlight driver some property has changed.
> + * The update_status operation is protected by the update_lock.
> + *
> + * The backlight driver is expected to use backlight_is_blank()
> + * to check if the display is blanked and set brightness accordingly.
> + * update_status() is called when any of the properties has changed.
> + *
> + * RETURNS:
> + *
> + * 0 on sucees, negative error code if any failure occured.
> + */
> int (*update_status)(struct backlight_device *);
> - /* Return the current backlight brightness (accounting for power,
> -fb_blank etc.) */
> +
> + /**
> + * @get_brightness:
> + *
> + * Return the current backlight brightness.
> + * The driver may implement this as a readback from the HW.
> + * This operation is optional and if not present then the current
> brightness
> + * property value is used.
> + *
> + * RETURNS:
> + *
> + * A brightness value which is 0 or a positive numer.
> + * On failure a negative error code is returned.
> + */
> int (*get_brightness)(struct backlight_device *);
> - /* Check if given framebuffer device is the one bound to this backlight;
> -return 0 if not, !=0 if it is. If NULL, backlight always matches the
> fb. */
> - int (*check_fb)(struct backlight_device *, struct fb_info *);
> +
> + /**
> + * @check_fb:
> + *
> + * Check if given framebuffer device is the one bound to this backlight.
> + * This operation is optional and if not implemented it is assumed that
> the
> + * fbdev is always the one bound to the backlight.
> + *
> + * RETURNS:
> + *
> + * If info is NULL or the info matches the fbdev bound to the backlight
> return true.
> + * If info does not match the fbdev bound to the backlight return false.
> + */
> + int (*check_fb)(struct backlight_device *bd, struct fb_info *info);
> };
>
> /* This structure defines all the properties of a backlight */
> --
> 2.25.1
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/4] drm: Match drm_dp_send_clear_payload_id_table definition to declaration
On Sat, May 16, 2020 at 10:23:30PM +0100, Chris Wilson wrote:
> drivers/gpu/drm/drm_dp_mst_topology.c:2898:6: warning: symbol
> 'drm_dp_send_clear_payload_id_table' was not declared. Should it be static?
Compiler seems a bit confused, because there is a static declaration a bit
further up. So should have complained about inconsistent decls, but then
my understanding of C is rather shoddy.
> drivers/gpu/drm/drm_dp_mst_topology.c:5451:37: warning: missing braces around
> initializer
> drivers/gpu/drm/drm_dp_mst_topology.c:5451:37: warning: missing braces around
> initializer
>
> Signed-off-by: Chris Wilson
Reviewed-by: Daniel Vetter
> ---
> drivers/gpu/drm/drm_dp_mst_topology.c | 7 ---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 70455e304a26..1bdf3cfeeebb 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -2895,8 +2895,9 @@ static int drm_dp_send_link_address(struct
> drm_dp_mst_topology_mgr *mgr,
> return ret < 0 ? ret : changed;
> }
>
> -void drm_dp_send_clear_payload_id_table(struct drm_dp_mst_topology_mgr *mgr,
> - struct drm_dp_mst_branch *mstb)
> +static void
> +drm_dp_send_clear_payload_id_table(struct drm_dp_mst_topology_mgr *mgr,
> +struct drm_dp_mst_branch *mstb)
> {
> struct drm_dp_sideband_msg_tx *txmsg;
> int ret;
> @@ -5448,7 +5449,7 @@ struct drm_dp_aux *drm_dp_mst_dsc_aux_for_port(struct
> drm_dp_mst_port *port)
> {
> struct drm_dp_mst_port *immediate_upstream_port;
> struct drm_dp_mst_port *fec_port;
> - struct drm_dp_desc desc = { 0 };
> + struct drm_dp_desc desc = {};
> u8 endpoint_fec;
> u8 endpoint_dsc;
>
> --
> 2.20.1
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 03/16] backlight: add backlight_is_blank()
On Sun, May 17, 2020 at 09:01:26PM +0200, Sam Ravnborg wrote:
> The backlight support has two properties that express the state:
> - power
> - state
>
> It is un-documented and easy to get wrong.
> Add backlight_is_blank() helper to make it simpler for drivers
> to get the check of the state correct.
>
> A lot of drivers also includes checks for fb_blank.
> This check is redundant when the state is checked
> and thus not needed in this helper function.
> But added anyway to avoid introducing subtle bug
> due to the creative use in some drivers.
>
> Rolling out this helper to all relevant backlight drivers
> will eliminate almost all accesses to fb_blank.
>
> v2:
> - Added fb_blank condition (Daniel)
>
> Signed-off-by: Sam Ravnborg
> Cc: Daniel Vetter
> Cc: Lee Jones
> Cc: Daniel Thompson
> Cc: Jingoo Han
Reviewed-by: Daniel Thompson
> ---
> include/linux/backlight.h | 19 +++
> 1 file changed, 19 insertions(+)
>
> diff --git a/include/linux/backlight.h b/include/linux/backlight.h
> index c7d6b2e8c3b5..a0a083b35c47 100644
> --- a/include/linux/backlight.h
> +++ b/include/linux/backlight.h
> @@ -175,6 +175,25 @@ static inline void backlight_put(struct backlight_device
> *bd)
> put_device(&bd->dev);
> }
>
> +/**
> + * backlight_is_blank - Return true if display is expected to be blank
> + * @bd: the backlight device
> + *
> + * Display is expected to be blank if any of these is true::
> + *
> + * 1) if power in not UNBLANK
> + * 2) if fb_blank is not UNBLANK
> + * 3) if state indicate BLANK or SUSPENDED
> + *
> + * Returns true if display is expected to be blank, false otherwise.
> + */
> +static inline bool backlight_is_blank(struct backlight_device *bd)
> +{
> + return bd->props.power != FB_BLANK_UNBLANK ||
> +bd->props.fb_blank != FB_BLANK_UNBLANK ||
> +bd->props.state & (BL_CORE_SUSPENDED | BL_CORE_FBBLANK);
> +}
> +
> extern struct backlight_device *backlight_device_register(const char *name,
> struct device *dev, void *devdata, const struct backlight_ops *ops,
> const struct backlight_properties *props);
> --
> 2.25.1
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 02/16] backlight: refactor fb_notifier_callback()
On Sun, May 17, 2020 at 09:01:25PM +0200, Sam Ravnborg wrote:
> Increase readability of fb_notifier_callback() by removing
> a few indent levels.
> No functional change.
>
> Signed-off-by: Sam Ravnborg
> Cc: Lee Jones
> Cc: Daniel Thompson
> Cc: Jingoo Han
Reviewed-by: Daniel Thompson
> ---
> drivers/video/backlight/backlight.c | 43 +++--
> 1 file changed, 22 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/video/backlight/backlight.c
> b/drivers/video/backlight/backlight.c
> index cac3e35d7630..17f04cff50ab 100644
> --- a/drivers/video/backlight/backlight.c
> +++ b/drivers/video/backlight/backlight.c
> @@ -58,28 +58,29 @@ static int fb_notifier_callback(struct notifier_block
> *self,
>
> bd = container_of(self, struct backlight_device, fb_notif);
> mutex_lock(&bd->ops_lock);
> - if (bd->ops)
> - if (!bd->ops->check_fb ||
> - bd->ops->check_fb(bd, evdata->info)) {
> - fb_blank = *(int *)evdata->data;
> - if (fb_blank == FB_BLANK_UNBLANK &&
> - !bd->fb_bl_on[node]) {
> - bd->fb_bl_on[node] = true;
> - if (!bd->use_count++) {
> - bd->props.state &= ~BL_CORE_FBBLANK;
> - bd->props.fb_blank = FB_BLANK_UNBLANK;
> - backlight_update_status(bd);
> - }
> - } else if (fb_blank != FB_BLANK_UNBLANK &&
> -bd->fb_bl_on[node]) {
> - bd->fb_bl_on[node] = false;
> - if (!(--bd->use_count)) {
> - bd->props.state |= BL_CORE_FBBLANK;
> - bd->props.fb_blank = fb_blank;
> - backlight_update_status(bd);
> - }
> - }
> +
> + if (!bd->ops)
> + goto out;
> + if (bd->ops->check_fb && !bd->ops->check_fb(bd, evdata->info))
> + goto out;
> +
> + fb_blank = *(int *)evdata->data;
> + if (fb_blank == FB_BLANK_UNBLANK && !bd->fb_bl_on[node]) {
> + bd->fb_bl_on[node] = true;
> + if (!bd->use_count++) {
> + bd->props.state &= ~BL_CORE_FBBLANK;
> + bd->props.fb_blank = FB_BLANK_UNBLANK;
> + backlight_update_status(bd);
> + }
> + } else if (fb_blank != FB_BLANK_UNBLANK && bd->fb_bl_on[node]) {
> + bd->fb_bl_on[node] = false;
> + if (!(--bd->use_count)) {
> + bd->props.state |= BL_CORE_FBBLANK;
> + bd->props.fb_blank = fb_blank;
> + backlight_update_status(bd);
> }
> + }
> +out:
> mutex_unlock(&bd->ops_lock);
> return 0;
> }
> --
> 2.25.1
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/4] drm: Help unconfuse gcc, avoid accidental impossible unsigned comparisons
Quoting Daniel Vetter (2020-05-18 15:47:44) > On Sat, May 16, 2020 at 10:23:28PM +0100, Chris Wilson wrote: > > drivers/gpu/drm/drm_client_modeset.c: In function > > ‘drm_client_firmware_config’: > > ./include/linux/bits.h:26:28: warning: comparison of unsigned expression < > > 0 is always false [-Wtype-limits] > >__builtin_constant_p((l) > (h)), (l) > (h), 0))) > > > > Signed-off-by: Chris Wilson > > Took me a while to spot where this goes boom, kinda wonder whether we > should have an > > if (WARN_ON(!connector_count)) > return -EINVAL; Atm, drm_client_firmware_config() is called only if connector_count!=0. But if count==0, we would hit goto retry indefinitely, seems like that would be worth a WARN. -Chris ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 0/6] drm: Prepare to use a GPIO on ti-sn65dsi86 for Hot Plug Detect
Sam,
On Sat, May 9, 2020 at 3:48 PM Doug Anderson wrote:
>
> Hi,
>
> On Sat, May 9, 2020 at 1:15 PM Sam Ravnborg wrote:
> >
> > Hi Douglas.
> >
> > On Thu, May 07, 2020 at 02:34:54PM -0700, Douglas Anderson wrote:
> > >
> > > As talked about in commit c2bfc223882d ("drm/bridge: ti-sn65dsi86:
> > > Remove the mystery delay"), the normal HPD pin on ti-sn65dsi86 is
> > > kinda useless, at least for embedded DisplayPort (eDP). However,
> > > despite the fact that the actual HPD pin on the bridge is mostly
> > > useless for eDP, the concept of HPD for eDP still makes sense. It
> > > allows us to optimize out a hardcoded delay that many panels need if
> > > HPD isn't hooked up. Panel timing diagrams show HPD as one of the
> > > events to measure timing from and we have to assume the worst case if
> > > we can't actually read HPD.
> > >
> > > One way to use HPD for eDP without using the mostly useless HPD pin on
> > > ti-sn65dsi86 is to route the panel's HPD somewhere else in the system,
> > > like to a GPIO. This works great because eDP panels aren't physically
> > > hotplugged. That means the debouncing logic that caused us problems
> > > wasn't really needed and a raw GPIO works great.
> > >
> > > As per the above, a smart board designer would realize the value of
> > > HPD and choose to route it to a GPIO somewhere on the board to avoid
> > > the silly sn65dsi86 debouncing. While said "smart designer" could
> > > theoretically route HPD anywhere on the board, a really smart designer
> > > would realize that there are several GPIOs on the bridge itself that
> > > are nearly useless for anything but this purpose and route HPD to one
> > > of those.
> > >
> > > This series of patches is intended to allow the scenario described
> > > above.
> > >
> > > This patch has been tested on a board that is not yet mainline. On
> > > the hardware I have:
> > > - Panel spec says HPD could take up to 200 ms to come up, so without
> > > HPD hooked up we need to delay 200 ms.
> > > - On my board the panel is powered by the same rail as the
> > > touchscreen. By chance of probe order the touchscreen comes up
> > > first. This means by the time we check HPD in ti_sn_bridge_enable()
> > > it's already up. Thus we can use the panel on 200 ms earlier.
> > > - If I measure HPD on this pane it comes up ~56 ms after the panel is
> > > powered. This means I can save 144 ms of delay.
> > >
> > > Side effects (though not main goals) of this series are:
> > > - ti-sn65dsi86 GPIOs are now exported in Linux.
> > > - ti-sn65dsi86 bindings are converted to yaml.
> > > - Common panel bindings now have "hpd-gpios" listed.
> > > - The simple-panel driver in Linux can delay in prepare based on
> > > "hpd-gpios"
> > > - ti-sn65dsi86 bindings (and current user) now specifies "no-hpd"
> > > if HPD isn't hooked up.
> > >
> > > Changes in v5:
> > > - Use of_xlate so that numbers in dts start at 1, not 0.
> > > - Squash
> > > https://lore.kernel.org/r/20200506140208.v2.2.I0a2bca02b09c1fcb6b09479b489736d600b3e57f@changeid/
> > >
> > > Changes in v4:
> > > - Don't include gpio.h
> > > - Use gpiochip_get_data() instead of container_of() to get data.
> > > - GPIOF_DIR_XXX => GPIO_LINE_DIRECTION_XXX
> > > - Use Linus W's favorite syntax to read a bit from a bitfield.
> > > - Define and use SN_GPIO_MUX_MASK.
> > > - Add a comment about why we use a bitmap for gchip_output.
> > > - Tacked on "or is otherwise unusable." to description.
> > >
> > > Changes in v3:
> > > - Becaue => Because
> > > - Add a kernel-doc to our pdata to clarify double-duty of gchip_output.
> > > - More comments about how powering off affects us (get_dir, dir_input).
> > > - Cleanup tail of ti_sn_setup_gpio_controller() to avoid one "return".
> > > - Use a bitmap rather than rolling my own.
> > > - Remind how gpio_get_optional() works in the commit message.
> > > - useful implement => useful to implement
> > >
> > > Changes in v2:
> > > - ("Export...GPIOs") is 1/2 of replacement for ("Allow...bridge GPIOs")
> > > - ("dt-bindings: display: Add hpd-gpios to panel-common...") new for v2
> > > - ("simple...hpd-gpios") is 1/2 of replacement for ("Allow...bridge
> > > GPIOs")
> > > - specification => specifier.
> > > - power up => power.
> > > - Added back missing suspend-gpios.
> > > - data-lanes and lane-polarities are are the right place now.
> > > - endpoints don't need to be patternProperties.
> > > - Specified more details for data-lanes and lane-polarities.
> > > - Added old example back in, fixing bugs in it.
> > > - Example i2c bus is just called "i2c", not "i2c1" now.
> > > - ("dt-bindings: drm/bridge: ti-sn65dsi86: Document no-hpd") new for v2.
> > > - ("arm64: dts: sdm845: Add "no-hpd" to sn65dsi86 on cheza") new for v2.
> > >
> > > Douglas Anderson (6):
> > > drm/bridge: ti-sn65dsi86: Export bridge GPIOs to Linux
> > > dt-bindings: display: Add hpd-gpios to panel-common bindings
> > > drm/panel-simple: Support hpd-gpios for delaying prepare()
>
Re: [PATCH 3/4] drm: Describe dp_rev for drm_dp_set_phy_test_pattern
On Sat, May 16, 2020 at 10:23:29PM +0100, Chris Wilson wrote: > drivers/gpu/drm/drm_dp_helper.c:1598: warning: Function parameter or member > 'dp_rev' not described in 'drm_dp_set_phy_test_pattern' > > Signed-off-by: Chris Wilson Reviewed-by: Daniel Vetter > --- > drivers/gpu/drm/drm_dp_helper.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c > index 43e57632b00a..532b07118542 100644 > --- a/drivers/gpu/drm/drm_dp_helper.c > +++ b/drivers/gpu/drm/drm_dp_helper.c > @@ -1590,6 +1590,7 @@ EXPORT_SYMBOL(drm_dp_get_phy_test_pattern); > * drm_dp_set_phy_test_pattern() - set the pattern to the sink. > * @aux: DisplayPort AUX channel > * @data: DP phy compliance test parameters. > + * @dp_rev: DP revision to use for compliance testing > * > * Returns 0 on success or a negative error code on failure. > */ > -- > 2.20.1 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/4] drm: Help unconfuse gcc, avoid accidental impossible unsigned comparisons
On Sat, May 16, 2020 at 10:23:28PM +0100, Chris Wilson wrote:
> drivers/gpu/drm/drm_client_modeset.c: In function
> ‘drm_client_firmware_config’:
> ./include/linux/bits.h:26:28: warning: comparison of unsigned expression < 0
> is always false [-Wtype-limits]
>__builtin_constant_p((l) > (h)), (l) > (h), 0)))
>
> Signed-off-by: Chris Wilson
Took me a while to spot where this goes boom, kinda wonder whether we
should have an
if (WARN_ON(!connector_count))
return -EINVAL;
somewhere in here. Just for belts&suspenders approach.
Anyway this looks good.
Reviewed-by: Daniel Vetter
> ---
> drivers/gpu/drm/drm_client_modeset.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_client_modeset.c
> b/drivers/gpu/drm/drm_client_modeset.c
> index 7443114bd713..6e9530df0737 100644
> --- a/drivers/gpu/drm/drm_client_modeset.c
> +++ b/drivers/gpu/drm/drm_client_modeset.c
> @@ -563,7 +563,7 @@ static bool drm_client_firmware_config(struct
> drm_client_dev *client,
> struct drm_client_offset *offsets,
> bool *enabled, int width, int height)
> {
> - unsigned int count = min_t(unsigned int, connector_count,
> BITS_PER_LONG);
> + const int count = min_t(unsigned int, connector_count, BITS_PER_LONG);
> unsigned long conn_configured, conn_seq, mask;
> struct drm_device *dev = client->dev;
> int i, j;
> --
> 2.20.1
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 7/9] drm/msm/a6xx: update pdc/rscc GMU registers for A640/A650
On Thu, Apr 23, 2020 at 05:09:19PM -0400, Jonathan Marek wrote:
> Update the gmu_pdc registers for A640 and A650.
>
> Some of the RSCC registers on A650 are in a separate region.
>
> Note this also changes the address of these registers:
>
> RSCC_TCS1_DRV0_STATUS
> RSCC_TCS2_DRV0_STATUS
> RSCC_TCS3_DRV0_STATUS
>
> Based on the values in msm-4.14 and msm-4.19 kernels.
>
> v3: replaced adreno_is_a650 around ->rscc with checks for "rscc" resource
This is one of the most frustrating bits about the different GMU flavors. There
is simply no way to make this code elegant.
Reviewed-by: Jordan Crouse
> Signed-off-by: Jonathan Marek
> ---
> drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 90 ++-
> drivers/gpu/drm/msm/adreno/a6xx_gmu.h | 10 +++
> drivers/gpu/drm/msm/adreno/a6xx_gmu.xml.h | 38 +-
> 3 files changed, 85 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> index 988575fc8e71..c6fce994194d 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> @@ -421,7 +421,7 @@ static int a6xx_rpmh_start(struct a6xx_gmu *gmu)
> return ret;
> }
>
> - ret = gmu_poll_timeout(gmu, REG_A6XX_RSCC_SEQ_BUSY_DRV0, val,
> + ret = gmu_poll_timeout_rscc(gmu, REG_A6XX_RSCC_SEQ_BUSY_DRV0, val,
> !val, 100, 1);
>
> if (ret) {
> @@ -447,7 +447,7 @@ static void a6xx_rpmh_stop(struct a6xx_gmu *gmu)
>
> gmu_write(gmu, REG_A6XX_GMU_RSCC_CONTROL_REQ, 1);
>
> - ret = gmu_poll_timeout(gmu, REG_A6XX_GPU_RSCC_RSC_STATUS0_DRV0,
> + ret = gmu_poll_timeout_rscc(gmu, REG_A6XX_GPU_RSCC_RSC_STATUS0_DRV0,
> val, val & (1 << 16), 100, 1);
> if (ret)
> DRM_DEV_ERROR(gmu->dev, "Unable to power off the GPU RSC\n");
> @@ -470,32 +470,48 @@ static void a6xx_gmu_rpmh_init(struct a6xx_gmu *gmu)
> struct platform_device *pdev = to_platform_device(gmu->dev);
> void __iomem *pdcptr = a6xx_gmu_get_mmio(pdev, "gmu_pdc");
> void __iomem *seqptr = a6xx_gmu_get_mmio(pdev, "gmu_pdc_seq");
> + uint32_t pdc_address_offset;
>
> if (!pdcptr || !seqptr)
> goto err;
>
> + if (adreno_is_a618(adreno_gpu) || adreno_is_a640(adreno_gpu))
> + pdc_address_offset = 0x30090;
> + else if (adreno_is_a650(adreno_gpu))
> + pdc_address_offset = 0x300a0;
> + else
> + pdc_address_offset = 0x30080;
> +
> /* Disable SDE clock gating */
> - gmu_write(gmu, REG_A6XX_GPU_RSCC_RSC_STATUS0_DRV0, BIT(24));
> + gmu_write_rscc(gmu, REG_A6XX_GPU_RSCC_RSC_STATUS0_DRV0, BIT(24));
>
> /* Setup RSC PDC handshake for sleep and wakeup */
> - gmu_write(gmu, REG_A6XX_RSCC_PDC_SLAVE_ID_DRV0, 1);
> - gmu_write(gmu, REG_A6XX_RSCC_HIDDEN_TCS_CMD0_DATA, 0);
> - gmu_write(gmu, REG_A6XX_RSCC_HIDDEN_TCS_CMD0_ADDR, 0);
> - gmu_write(gmu, REG_A6XX_RSCC_HIDDEN_TCS_CMD0_DATA + 2, 0);
> - gmu_write(gmu, REG_A6XX_RSCC_HIDDEN_TCS_CMD0_ADDR + 2, 0);
> - gmu_write(gmu, REG_A6XX_RSCC_HIDDEN_TCS_CMD0_DATA + 4, 0x8000);
> - gmu_write(gmu, REG_A6XX_RSCC_HIDDEN_TCS_CMD0_ADDR + 4, 0);
> - gmu_write(gmu, REG_A6XX_RSCC_OVERRIDE_START_ADDR, 0);
> - gmu_write(gmu, REG_A6XX_RSCC_PDC_SEQ_START_ADDR, 0x4520);
> - gmu_write(gmu, REG_A6XX_RSCC_PDC_MATCH_VALUE_LO, 0x4510);
> - gmu_write(gmu, REG_A6XX_RSCC_PDC_MATCH_VALUE_HI, 0x4514);
> + gmu_write_rscc(gmu, REG_A6XX_RSCC_PDC_SLAVE_ID_DRV0, 1);
> + gmu_write_rscc(gmu, REG_A6XX_RSCC_HIDDEN_TCS_CMD0_DATA, 0);
> + gmu_write_rscc(gmu, REG_A6XX_RSCC_HIDDEN_TCS_CMD0_ADDR, 0);
> + gmu_write_rscc(gmu, REG_A6XX_RSCC_HIDDEN_TCS_CMD0_DATA + 2, 0);
> + gmu_write_rscc(gmu, REG_A6XX_RSCC_HIDDEN_TCS_CMD0_ADDR + 2, 0);
> + gmu_write_rscc(gmu, REG_A6XX_RSCC_HIDDEN_TCS_CMD0_DATA + 4, 0x8000);
> + gmu_write_rscc(gmu, REG_A6XX_RSCC_HIDDEN_TCS_CMD0_ADDR + 4, 0);
> + gmu_write_rscc(gmu, REG_A6XX_RSCC_OVERRIDE_START_ADDR, 0);
> + gmu_write_rscc(gmu, REG_A6XX_RSCC_PDC_SEQ_START_ADDR, 0x4520);
> + gmu_write_rscc(gmu, REG_A6XX_RSCC_PDC_MATCH_VALUE_LO, 0x4510);
> + gmu_write_rscc(gmu, REG_A6XX_RSCC_PDC_MATCH_VALUE_HI, 0x4514);
>
> /* Load RSC sequencer uCode for sleep and wakeup */
> - gmu_write(gmu, REG_A6XX_RSCC_SEQ_MEM_0_DRV0, 0xa7a506a0);
> - gmu_write(gmu, REG_A6XX_RSCC_SEQ_MEM_0_DRV0 + 1, 0xa1e6a6e7);
> - gmu_write(gmu, REG_A6XX_RSCC_SEQ_MEM_0_DRV0 + 2, 0xa2e081e1);
> - gmu_write(gmu, REG_A6XX_RSCC_SEQ_MEM_0_DRV0 + 3, 0xe9a982e2);
> - gmu_write(gmu, REG_A6XX_RSCC_SEQ_MEM_0_DRV0 + 4, 0x0020e8a8);
> + if (adreno_is_a650(adreno_gpu)) {
> + gmu_write_rscc(gmu, REG_A6XX_RSCC_SEQ_MEM_0_DRV0, 0xeaaae5a0);
> + gmu_write_rscc(gmu, REG_A6XX_RSCC_SEQ_MEM_0_DRV0 + 1,
> 0xe1a1ebab);
> + gmu_write_rscc(gmu, REG_A6XX_RSCC_SEQ_MEM_0_DRV0 + 2,
> 0xa2e0a581);
> + gmu_wr
Re: [PATCH v3 9/9] drm/msm/a6xx: update a6xx_hw_init for A640 and A650
On Thu, Apr 23, 2020 at 05:09:21PM -0400, Jonathan Marek wrote:
> Adreno 640 and 650 GPUs need some registers set differently.
As before, make sure you send the XML updates up so the database stays current.
Reviewed-by: Jordan Crouse
> Signed-off-by: Jonathan Marek
> ---
> drivers/gpu/drm/msm/adreno/a6xx.xml.h | 14 +++
> drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 56 ++-
> 2 files changed, 61 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx.xml.h
> b/drivers/gpu/drm/msm/adreno/a6xx.xml.h
> index ed78fee2a262..47840b73cdda 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx.xml.h
> +++ b/drivers/gpu/drm/msm/adreno/a6xx.xml.h
> @@ -1047,6 +1047,8 @@ enum a6xx_tex_type {
>
> #define REG_A6XX_CP_MISC_CNTL
> 0x0840
>
> +#define REG_A6XX_CP_APRIV_CNTL
> 0x0844
> +
> #define REG_A6XX_CP_ROQ_THRESHOLDS_1 0x08c1
>
> #define REG_A6XX_CP_ROQ_THRESHOLDS_2 0x08c2
> @@ -1764,6 +1766,8 @@ static inline uint32_t
> A6XX_CP_PROTECT_REG_MASK_LEN(uint32_t val)
>
> #define REG_A6XX_RBBM_VBIF_CLIENT_QOS_CNTL 0x0010
>
> +#define REG_A6XX_RBBM_GBIF_CLIENT_QOS_CNTL 0x0011
> +
> #define REG_A6XX_RBBM_INTERFACE_HANG_INT_CNTL
> 0x001f
>
> #define REG_A6XX_RBBM_INT_CLEAR_CMD 0x0037
> @@ -2418,6 +2422,16 @@ static inline uint32_t
> A6XX_UCHE_CLIENT_PF_PERFSEL(uint32_t val)
>
> #define REG_A6XX_TPL1_NC_MODE_CNTL 0xb604
>
> +#define REG_A6XX_TPL1_BICUBIC_WEIGHTS_TABLE_0
> 0xb608
> +
> +#define REG_A6XX_TPL1_BICUBIC_WEIGHTS_TABLE_1
> 0xb609
> +
> +#define REG_A6XX_TPL1_BICUBIC_WEIGHTS_TABLE_2
> 0xb60a
> +
> +#define REG_A6XX_TPL1_BICUBIC_WEIGHTS_TABLE_3
> 0xb60b
> +
> +#define REG_A6XX_TPL1_BICUBIC_WEIGHTS_TABLE_4
> 0xb60c
> +
> #define REG_A6XX_TPL1_PERFCTR_TP_SEL_0
> 0xb610
>
> #define REG_A6XX_TPL1_PERFCTR_TP_SEL_1
> 0xb611
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> index a860d4970e10..e1eb34fa3a99 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> @@ -414,7 +414,17 @@ static int a6xx_hw_init(struct msm_gpu *gpu)
> a6xx_set_hwcg(gpu, true);
>
> /* VBIF/GBIF start*/
> - gpu_write(gpu, REG_A6XX_RBBM_VBIF_CLIENT_QOS_CNTL, 0x3);
> + if (adreno_is_a640(adreno_gpu) || adreno_is_a650(adreno_gpu)) {
> + gpu_write(gpu, REG_A6XX_GBIF_QSB_SIDE0, 0x00071620);
> + gpu_write(gpu, REG_A6XX_GBIF_QSB_SIDE1, 0x00071620);
> + gpu_write(gpu, REG_A6XX_GBIF_QSB_SIDE2, 0x00071620);
> + gpu_write(gpu, REG_A6XX_GBIF_QSB_SIDE3, 0x00071620);
> + gpu_write(gpu, REG_A6XX_GBIF_QSB_SIDE3, 0x00071620);
> + gpu_write(gpu, REG_A6XX_RBBM_GBIF_CLIENT_QOS_CNTL, 0x3);
> + } else {
> + gpu_write(gpu, REG_A6XX_RBBM_VBIF_CLIENT_QOS_CNTL, 0x3);
> + }
> +
> if (adreno_is_a630(adreno_gpu))
> gpu_write(gpu, REG_A6XX_VBIF_GATE_OFF_WRREQ_EN, 0x0009);
>
> @@ -429,25 +439,35 @@ static int a6xx_hw_init(struct msm_gpu *gpu)
> gpu_write(gpu, REG_A6XX_UCHE_WRITE_THRU_BASE_LO, 0xf000);
> gpu_write(gpu, REG_A6XX_UCHE_WRITE_THRU_BASE_HI, 0x0001);
>
> - /* Set the GMEM VA range [0x10:0x10 + gpu->gmem - 1] */
> - gpu_write64(gpu, REG_A6XX_UCHE_GMEM_RANGE_MIN_LO,
> - REG_A6XX_UCHE_GMEM_RANGE_MIN_HI, 0x0010);
> + if (!adreno_is_a650(adreno_gpu)) {
> + /* Set the GMEM VA range [0x10:0x10 + gpu->gmem - 1] */
> + gpu_write64(gpu, REG_A6XX_UCHE_GMEM_RANGE_MIN_LO,
> + REG_A6XX_UCHE_GMEM_RANGE_MIN_HI, 0x0010);
>
> - gpu_write64(gpu, REG_A6XX_UCHE_GMEM_RANGE_MAX_LO,
> - REG_A6XX_UCHE_GMEM_RANGE_MAX_HI,
> - 0x0010 + adreno_gpu->gmem - 1);
> + gpu_write64(gpu, REG_A6XX_UCHE_GMEM_RANGE_MAX_LO,
> + REG_A6XX_UCHE_GMEM_RANGE_MAX_HI,
> + 0x0010 + adreno_gpu->gmem - 1);
> + }
>
> gpu_write(gpu, REG_A6XX_UCHE_FILTER_CNTL, 0x804);
> gpu_write(gpu, REG_A6XX_UCHE_CACHE_WAYS, 0x4);
>
> - gpu_write(gpu, REG_A6XX_CP_ROQ_THRESHOLDS_2, 0x01c0);
> + if (adreno_is_a640(adreno_gpu) || adreno_is_a650(adreno_gpu))
> + gpu_write(gpu, REG_A6XX_CP_ROQ_THRESHOLDS_2, 0x02000140);
> + else
> + gpu_write(gpu, REG_A6XX_CP_ROQ_THRESHOLDS_2, 0x01c0);
> gpu_write(gpu, REG_A6XX_CP_ROQ_THRESHOLDS_1, 0x8040362c);
>
> /* Setting the mem pool size */
>
Re: [PATCH v3 6/9] drm/msm/a6xx: A640/A650 GMU firmware path
On Thu, Apr 23, 2020 at 05:09:18PM -0400, Jonathan Marek wrote:
> Newer GPUs have different GMU firmware path.
>
> v3: updated a6xx_gmu_fw_load based on feedback, including gmu_write_bulk,
> and removed extra whitespace change
I'm not sure if you sent the XML updates to Rob yet, but don't forget to do that
so we don't accidentally lose the registers if we do a refresh.
Reviewed-by: Jordan Crouse
> Signed-off-by: Jonathan Marek
> ---
> drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 138 +++---
> drivers/gpu/drm/msm/adreno/a6xx_gmu.h | 10 ++
> drivers/gpu/drm/msm/adreno/a6xx_gmu.xml.h | 6 +
> 3 files changed, 138 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> index b22a69e2f4b0..988575fc8e71 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> @@ -571,6 +571,8 @@ static void a6xx_gmu_power_config(struct a6xx_gmu *gmu)
> {
> /* Disable GMU WB/RB buffer */
> gmu_write(gmu, REG_A6XX_GMU_SYS_BUS_CONFIG, 0x1);
> + gmu_write(gmu, REG_A6XX_GMU_ICACHE_CONFIG, 0x1);
> + gmu_write(gmu, REG_A6XX_GMU_DCACHE_CONFIG, 0x1);
>
> gmu_write(gmu, REG_A6XX_GMU_PWR_COL_INTER_FRAME_CTRL, 0x9c40400);
>
> @@ -600,14 +602,95 @@ static void a6xx_gmu_power_config(struct a6xx_gmu *gmu)
> A6XX_GMU_RPMH_CTRL_GFX_VOTE_ENABLE);
> }
>
> +struct block_header {
> + u32 addr;
> + u32 size;
> + u32 type;
> + u32 value;
> + u32 data[];
> +};
> +
> +/* this should be a general kernel helper */
> +static int in_range(u32 addr, u32 start, u32 size)
> +{
> + return addr >= start && addr < start + size;
> +}
> +
> +static bool fw_block_mem(struct a6xx_gmu_bo *bo, const struct block_header
> *blk)
> +{
> + if (!in_range(blk->addr, bo->iova, bo->size))
> + return false;
> +
> + memcpy(bo->virt + blk->addr - bo->iova, blk->data, blk->size);
> + return true;
> +}
> +
> +static int a6xx_gmu_fw_load(struct a6xx_gmu *gmu)
> +{
> + struct a6xx_gpu *a6xx_gpu = container_of(gmu, struct a6xx_gpu, gmu);
> + struct adreno_gpu *adreno_gpu = &a6xx_gpu->base;
> + const struct firmware *fw_image = adreno_gpu->fw[ADRENO_FW_GMU];
> + const struct block_header *blk;
> + u32 reg_offset;
> +
> + u32 itcm_base = 0x;
> + u32 dtcm_base = 0x0004;
> +
> + if (adreno_is_a650(adreno_gpu))
> + dtcm_base = 0x10004000;
> +
> + if (gmu->legacy) {
> + /* Sanity check the size of the firmware that was loaded */
> + if (fw_image->size > 0x8000) {
> + DRM_DEV_ERROR(gmu->dev,
> + "GMU firmware is bigger than the available
> region\n");
> + return -EINVAL;
> + }
> +
> + gmu_write_bulk(gmu, REG_A6XX_GMU_CM3_ITCM_START,
> +(u32*) fw_image->data, fw_image->size);
> + return 0;
> + }
> +
> +
> + for (blk = (const struct block_header *) fw_image->data;
> + (const u8*) blk < fw_image->data + fw_image->size;
> + blk = (const struct block_header *) &blk->data[blk->size >> 2]) {
> + if (blk->size == 0)
> + continue;
> +
> + if (in_range(blk->addr, itcm_base, SZ_16K)) {
> + reg_offset = (blk->addr - itcm_base) >> 2;
> + gmu_write_bulk(gmu,
> + REG_A6XX_GMU_CM3_ITCM_START + reg_offset,
> + blk->data, blk->size);
> + } else if (in_range(blk->addr, dtcm_base, SZ_16K)) {
> + reg_offset = (blk->addr - dtcm_base) >> 2;
> + gmu_write_bulk(gmu,
> + REG_A6XX_GMU_CM3_DTCM_START + reg_offset,
> + blk->data, blk->size);
> + } else if (!fw_block_mem(&gmu->icache, blk) &&
> +!fw_block_mem(&gmu->dcache, blk) &&
> +!fw_block_mem(&gmu->dummy, blk)) {
> + DRM_DEV_ERROR(gmu->dev,
> + "failed to match fw block (addr=%.8x size=%d
> data[0]=%.8x)\n",
> + blk->addr, blk->size, blk->data[0]);
> + }
> + }
> +
> + return 0;
> +}
> +
> static int a6xx_gmu_fw_start(struct a6xx_gmu *gmu, unsigned int state)
> {
> static bool rpmh_init;
> struct a6xx_gpu *a6xx_gpu = container_of(gmu, struct a6xx_gpu, gmu);
> struct adreno_gpu *adreno_gpu = &a6xx_gpu->base;
> - int i, ret;
> + int ret;
> u32 chipid;
> - u32 *image;
> +
> + if (adreno_is_a650(adreno_gpu))
> + gmu_write(gmu, REG_A6XX_GPU_GMU_CX_GMU_CX_FAL_INTF, 1);
>
> if (state == GMU_WARM_BOOT) {
> ret = a6xx_rpmh_start(gmu);
> @@ -618,13 +701,6 @@ static int a6xx_gmu_fw_start(struct a6xx_gmu *gmu,
> unsigned int
Re: [Freedreno] [PATCH v3 1/9] drm/msm: add msm_gem_get_and_pin_iova_range
On Thu, Apr 23, 2020 at 05:09:13PM -0400, Jonathan Marek wrote:
> This function allows pinning iova to a specific page range (for a6xx GMU).
Reviewed-by: Jordan Crouse
> Signed-off-by: Jonathan Marek
> ---
> drivers/gpu/drm/msm/msm_drv.h | 6 +-
> drivers/gpu/drm/msm/msm_gem.c | 28 +---
> drivers/gpu/drm/msm/msm_gem_vma.c | 6 --
> 3 files changed, 30 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
> index 194d900a460e..966fd9068c94 100644
> --- a/drivers/gpu/drm/msm/msm_drv.h
> +++ b/drivers/gpu/drm/msm/msm_drv.h
> @@ -236,7 +236,8 @@ int msm_crtc_enable_vblank(struct drm_crtc *crtc);
> void msm_crtc_disable_vblank(struct drm_crtc *crtc);
>
> int msm_gem_init_vma(struct msm_gem_address_space *aspace,
> - struct msm_gem_vma *vma, int npages);
> + struct msm_gem_vma *vma, int npages,
> + u64 range_start, u64 range_end);
> void msm_gem_purge_vma(struct msm_gem_address_space *aspace,
> struct msm_gem_vma *vma);
> void msm_gem_unmap_vma(struct msm_gem_address_space *aspace,
> @@ -276,6 +277,9 @@ vm_fault_t msm_gem_fault(struct vm_fault *vmf);
> uint64_t msm_gem_mmap_offset(struct drm_gem_object *obj);
> int msm_gem_get_iova(struct drm_gem_object *obj,
> struct msm_gem_address_space *aspace, uint64_t *iova);
> +int msm_gem_get_and_pin_iova_range(struct drm_gem_object *obj,
> + struct msm_gem_address_space *aspace, uint64_t *iova,
> + u64 range_start, u64 range_end);
> int msm_gem_get_and_pin_iova(struct drm_gem_object *obj,
> struct msm_gem_address_space *aspace, uint64_t *iova);
> uint64_t msm_gem_iova(struct drm_gem_object *obj,
> diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
> index 5a6a79fbc9d6..d8f56a34c117 100644
> --- a/drivers/gpu/drm/msm/msm_gem.c
> +++ b/drivers/gpu/drm/msm/msm_gem.c
> @@ -389,7 +389,8 @@ put_iova(struct drm_gem_object *obj)
> }
>
> static int msm_gem_get_iova_locked(struct drm_gem_object *obj,
> - struct msm_gem_address_space *aspace, uint64_t *iova)
> + struct msm_gem_address_space *aspace, uint64_t *iova,
> + u64 range_start, u64 range_end)
> {
> struct msm_gem_object *msm_obj = to_msm_bo(obj);
> struct msm_gem_vma *vma;
> @@ -404,7 +405,8 @@ static int msm_gem_get_iova_locked(struct drm_gem_object
> *obj,
> if (IS_ERR(vma))
> return PTR_ERR(vma);
>
> - ret = msm_gem_init_vma(aspace, vma, obj->size >> PAGE_SHIFT);
> + ret = msm_gem_init_vma(aspace, vma, obj->size >> PAGE_SHIFT,
> + range_start, range_end);
> if (ret) {
> del_vma(vma);
> return ret;
> @@ -443,9 +445,13 @@ static int msm_gem_pin_iova(struct drm_gem_object *obj,
> msm_obj->sgt, obj->size >> PAGE_SHIFT);
> }
>
> -/* get iova and pin it. Should have a matching put */
> -int msm_gem_get_and_pin_iova(struct drm_gem_object *obj,
> - struct msm_gem_address_space *aspace, uint64_t *iova)
> +/*
> + * get iova and pin it. Should have a matching put
> + * limits iova to specified range (in pages)
> + */
> +int msm_gem_get_and_pin_iova_range(struct drm_gem_object *obj,
> + struct msm_gem_address_space *aspace, uint64_t *iova,
> + u64 range_start, u64 range_end)
> {
> struct msm_gem_object *msm_obj = to_msm_bo(obj);
> u64 local;
> @@ -453,7 +459,8 @@ int msm_gem_get_and_pin_iova(struct drm_gem_object *obj,
>
> mutex_lock(&msm_obj->lock);
>
> - ret = msm_gem_get_iova_locked(obj, aspace, &local);
> + ret = msm_gem_get_iova_locked(obj, aspace, &local,
> + range_start, range_end);
>
> if (!ret)
> ret = msm_gem_pin_iova(obj, aspace);
> @@ -465,6 +472,13 @@ int msm_gem_get_and_pin_iova(struct drm_gem_object *obj,
> return ret;
> }
>
> +/* get iova and pin it. Should have a matching put */
> +int msm_gem_get_and_pin_iova(struct drm_gem_object *obj,
> + struct msm_gem_address_space *aspace, uint64_t *iova)
> +{
> + return msm_gem_get_and_pin_iova_range(obj, aspace, iova, 0, U64_MAX);
> +}
> +
> /*
> * Get an iova but don't pin it. Doesn't need a put because iovas are
> currently
> * valid for the life of the object
> @@ -476,7 +490,7 @@ int msm_gem_get_iova(struct drm_gem_object *obj,
> int ret;
>
> mutex_lock(&msm_obj->lock);
> - ret = msm_gem_get_iova_locked(obj, aspace, iova);
> + ret = msm_gem_get_iova_locked(obj, aspace, iova, 0, U64_MAX);
> mutex_unlock(&msm_obj->lock);
>
> return ret;
> diff --git a/drivers/gpu/drm/msm/msm_gem_vma.c
> b/drivers/gpu/drm/msm/msm_gem_vma.c
> index 1af5354bcd46..407b7ab82818 100644
> --- a/drivers/gpu/drm/msm/msm_gem_vma.c
> +++ b/drivers/gpu/drm/msm/msm_gem_vma.c
> @@ -103
Re: [PATCH 1/4] drm: Include internal header for managed function declarations
On Sat, May 16, 2020 at 10:23:27PM +0100, Chris Wilson wrote:
> drivers/gpu/drm/drm_managed.c:61:6: warning: symbol 'drm_managed_release' was
> not declared. Should it be static?
> CC drivers/gpu/drm/drm_managed.o
> drivers/gpu/drm/drm_managed.c:61:6: warning: no previous prototype for
> ‘drm_managed_release’ [-Wmissing-prototypes]
> void drm_managed_release(struct drm_device *dev)
Drat! Thanks for fixing.
Reviewed-by: Daniel Vetter
>
> Fixes: c6603c740e0e ("drm: add managed resources tied to drm_device")
> Signed-off-by: Chris Wilson
> Cc: Sam Ravnborg
> Cc: Daniel Vetter
> ---
> drivers/gpu/drm/drm_managed.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_managed.c b/drivers/gpu/drm/drm_managed.c
> index 9cebfe370a65..1e1356560c2e 100644
> --- a/drivers/gpu/drm/drm_managed.c
> +++ b/drivers/gpu/drm/drm_managed.c
> @@ -14,6 +14,8 @@
> #include
> #include
>
> +#include "drm_internal.h"
> +
> /**
> * DOC: managed resources
> *
> --
> 2.20.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm/shmem: Use cached mappings by default
On Mon, May 18, 2020 at 12:11:32PM +0200, Gerd Hoffmann wrote: > On Mon, May 18, 2020 at 10:50:15AM +0200, Thomas Zimmermann wrote: > > Hi Gerd > > > > Am 18.05.20 um 10:23 schrieb Gerd Hoffmann: > > >>> $ git grep drm_gem_shmem_mmap > > >>> > > >>> We also need correct access from userspace, otherwise the gpu is going > > >>> to > > >>> be sad. > > >> > > >> I've been thinking about this, and I think it means that we can never > > >> have cached mappings anywhere. Even if shmem supports it internally for > > >> most drivers, as soon as the page are exported, the importer could > > >> expect uncached memory. > > > > > > The importer should not expect anything but call dma-buf ops so the > > > exporter has a chance to handle this correctly. > > > > I have the following case in mind: Suppose the exporter maps cached > > pages and the importer expects uncached pages for DMA. There is > > map_dma_buf/unmap_dma_buf, which can implement a cache flush for the > > cached pages. Is it guaranteed that the importer calls this around each > > DMA operation? > > I think the importer is supposed to do that, but I wouldn't surprised if > there are cases in tree where this isn't implemented correctly ... Yup, this is very much a case of "supposed to" but "in practice, many actually dont". The reason is that setting up mappings is expensive, so best avoid. We filled that gap a few years after dma-buf landed with the begin/end_cpu_access hooks, which allow the exporter to do cache flushing (using something like dma_sync_sg_for_device/cpu) and for this to all work properly. We even added ioctl so that the mmap on the dma-buf works correctly. But most importers still ignore this, so it's all fail :-/ But in theory the pieces to make cached mappings work over dma-buf, even for importers that need uncached, are all there. Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm/msm/a4xx: add adreno a405 support
On Sat, May 09, 2020 at 08:38:45PM +0800, Shawn Guo wrote:
> It adds support for adreno a405 found on MSM8939. The adreno_is_a430()
> check in adreno_submit() needs an extension to cover a405. The
> downstream driver suggests it should cover the whole a4xx generation.
> That's why it gets changed to adreno_is_a4xx(), while a420 is not
> tested though.
This looks good to me and if it boots then that's the best test of all.
Reviewed-by: Jordan Crouse
> Signed-off-by: Shawn Guo
> ---
> drivers/gpu/drm/msm/adreno/a4xx_gpu.c | 29 +-
> drivers/gpu/drm/msm/adreno/adreno_device.c | 11
> drivers/gpu/drm/msm/adreno/adreno_gpu.c| 2 +-
> drivers/gpu/drm/msm/adreno/adreno_gpu.h| 5
> 4 files changed, 34 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
> b/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
> index 253d8d85daad..70de59751188 100644
> --- a/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
> @@ -66,19 +66,22 @@ static void a4xx_enable_hwcg(struct msm_gpu *gpu)
> }
> }
>
> - for (i = 0; i < 4; i++) {
> - gpu_write(gpu, REG_A4XX_RBBM_CLOCK_CTL_MARB_CCU(i),
> - 0x0922);
> - }
> + /* No CCU for A405 */
> + if (!adreno_is_a405(adreno_gpu)) {
> + for (i = 0; i < 4; i++) {
> + gpu_write(gpu, REG_A4XX_RBBM_CLOCK_CTL_MARB_CCU(i),
> + 0x0922);
> + }
>
> - for (i = 0; i < 4; i++) {
> - gpu_write(gpu, REG_A4XX_RBBM_CLOCK_HYST_RB_MARB_CCU(i),
> - 0x);
> - }
> + for (i = 0; i < 4; i++) {
> + gpu_write(gpu, REG_A4XX_RBBM_CLOCK_HYST_RB_MARB_CCU(i),
> + 0x);
> + }
>
> - for (i = 0; i < 4; i++) {
> - gpu_write(gpu, REG_A4XX_RBBM_CLOCK_DELAY_RB_MARB_CCU_L1(i),
> - 0x0001);
> + for (i = 0; i < 4; i++) {
> + gpu_write(gpu,
> REG_A4XX_RBBM_CLOCK_DELAY_RB_MARB_CCU_L1(i),
> + 0x0001);
> + }
> }
>
> gpu_write(gpu, REG_A4XX_RBBM_CLOCK_MODE_GPC, 0x0222);
> @@ -137,7 +140,9 @@ static int a4xx_hw_init(struct msm_gpu *gpu)
> uint32_t *ptr, len;
> int i, ret;
>
> - if (adreno_is_a420(adreno_gpu)) {
> + if (adreno_is_a405(adreno_gpu)) {
> + gpu_write(gpu, REG_A4XX_VBIF_ROUND_ROBIN_QOS_ARB, 0x0003);
> + } else if (adreno_is_a420(adreno_gpu)) {
> gpu_write(gpu, REG_A4XX_VBIF_ABIT_SORT, 0x0001001F);
> gpu_write(gpu, REG_A4XX_VBIF_ABIT_SORT_CONF, 0x00A4);
> gpu_write(gpu, REG_A4XX_VBIF_GATE_OFF_WRREQ_EN, 0x0001);
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c
> b/drivers/gpu/drm/msm/adreno/adreno_device.c
> index cb3a6e597d76..b69757383965 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_device.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
> @@ -92,6 +92,17 @@ static const struct adreno_info gpulist[] = {
> .gmem = SZ_1M,
> .inactive_period = DRM_MSM_INACTIVE_PERIOD,
> .init = a3xx_gpu_init,
> + }, {
> + .rev = ADRENO_REV(4, 0, 5, ANY_ID),
> + .revn = 405,
> + .name = "A405",
> + .fw = {
> + [ADRENO_FW_PM4] = "a420_pm4.fw",
> + [ADRENO_FW_PFP] = "a420_pfp.fw",
> + },
> + .gmem = SZ_256K,
> + .inactive_period = DRM_MSM_INACTIVE_PERIOD,
> + .init = a4xx_gpu_init,
> }, {
> .rev = ADRENO_REV(4, 2, 0, ANY_ID),
> .revn = 420,
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> index 1d5c43c22269..3ddbf507941c 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> @@ -459,7 +459,7 @@ void adreno_submit(struct msm_gpu *gpu, struct
> msm_gem_submit *submit,
> break;
> /* fall-thru */
> case MSM_SUBMIT_CMD_BUF:
> - OUT_PKT3(ring, adreno_is_a430(adreno_gpu) ?
> + OUT_PKT3(ring, adreno_is_a4xx(adreno_gpu) ?
> CP_INDIRECT_BUFFER_PFE :
> CP_INDIRECT_BUFFER_PFD, 2);
> OUT_RING(ring, lower_32_bits(submit->cmd[i].iova));
> OUT_RING(ring, submit->cmd[i].size);
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> index 9ff4e550e7bd..35f744834ea9 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> @@ -202,6 +202,11 @@ static inline bool adreno_is_a4xx(struct adreno_gpu *gpu)
>
Re: [PATCH 2/2] dt-bindings: arm-smmu: Add sc7180 compatible string
On Fri, May 15, 2020 at 12:05:39PM -0700, Doug Anderson wrote: > On Fri, May 1, 2020 at 3:30 AM Sharat Masetty wrote: > > > > This patch simply adds a new compatible string for SC7180 platform. > > > > Signed-off-by: Sharat Masetty > > --- > > Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml > > b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml > > index 6515dbe..986098b 100644 > > --- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml > > +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml > > @@ -28,6 +28,7 @@ properties: > >- enum: > >- qcom,msm8996-smmu-v2 > >- qcom,msm8998-smmu-v2 > > + - qcom,sc7180-smmu-v2 > >- qcom,sdm845-smmu-v2 > >- const: qcom,smmu-v2 > > Is anything blocking this patch from landing now? I thought updates to the bindings usually went via Rob and the device-tree tree, but neither of those are on cc. Perhaps resend with that fixed? Will ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm/msm/a4xx: add a405_registers for a405 device
On Sat, May 09, 2020 at 08:38:46PM +0800, Shawn Guo wrote:
> A405 device has a different set of registers than a4xx_registers. It
> has no VMIDMT or XPU registers, and VBIF registers are different. Let's
> add a405_registers for a405 device.
>
> As adreno_is_a405() works only after adreno_gpu_init() gets called, the
> assignments get moved down after adreno_gpu_init().
Reviewed-by: Jordan Crouse
> Signed-off-by: Shawn Guo
> ---
> drivers/gpu/drm/msm/adreno/a4xx_gpu.c | 53 +--
> 1 file changed, 50 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
> b/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
> index 70de59751188..9e244982974e 100644
> --- a/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
> @@ -445,6 +445,52 @@ static const unsigned int a4xx_registers[] = {
> ~0 /* sentinel */
> };
>
> +static const unsigned int a405_registers[] = {
> + /* RBBM */
> + 0x, 0x0002, 0x0004, 0x0021, 0x0023, 0x0024, 0x0026, 0x0026,
> + 0x0028, 0x002B, 0x002E, 0x0034, 0x0037, 0x0044, 0x0047, 0x0066,
> + 0x0068, 0x0095, 0x009C, 0x0170, 0x0174, 0x01AF,
> + /* CP */
> + 0x0200, 0x0233, 0x0240, 0x0250, 0x04C0, 0x04DD, 0x0500, 0x050B,
> + 0x0578, 0x058F,
> + /* VSC */
> + 0x0C00, 0x0C03, 0x0C08, 0x0C41, 0x0C50, 0x0C51,
> + /* GRAS */
> + 0x0C80, 0x0C81, 0x0C88, 0x0C8F,
> + /* RB */
> + 0x0CC0, 0x0CC0, 0x0CC4, 0x0CD2,
> + /* PC */
> + 0x0D00, 0x0D0C, 0x0D10, 0x0D17, 0x0D20, 0x0D23,
> + /* VFD */
> + 0x0E40, 0x0E4A,
> + /* VPC */
> + 0x0E60, 0x0E61, 0x0E63, 0x0E68,
> + /* UCHE */
> + 0x0E80, 0x0E84, 0x0E88, 0x0E95,
> + /* GRAS CTX 0 */
> + 0x2000, 0x2004, 0x2008, 0x2067, 0x2070, 0x2078, 0x207B, 0x216E,
> + /* PC CTX 0 */
> + 0x21C0, 0x21C6, 0x21D0, 0x21D0, 0x21D9, 0x21D9, 0x21E5, 0x21E7,
> + /* VFD CTX 0 */
> + 0x2200, 0x2204, 0x2208, 0x22A9,
> + /* GRAS CTX 1 */
> + 0x2400, 0x2404, 0x2408, 0x2467, 0x2470, 0x2478, 0x247B, 0x256E,
> + /* PC CTX 1 */
> + 0x25C0, 0x25C6, 0x25D0, 0x25D0, 0x25D9, 0x25D9, 0x25E5, 0x25E7,
> + /* VFD CTX 1 */
> + 0x2600, 0x2604, 0x2608, 0x26A9,
> + /* VBIF version 0x2005*/
> + 0x3000, 0x3007, 0x302C, 0x302C, 0x3030, 0x3030, 0x3034, 0x3036,
> + 0x3038, 0x3038, 0x303C, 0x303D, 0x3040, 0x3040, 0x3049, 0x3049,
> + 0x3058, 0x3058, 0x305B, 0x3061, 0x3064, 0x3068, 0x306C, 0x306D,
> + 0x3080, 0x3088, 0x308B, 0x308C, 0x3090, 0x3094, 0x3098, 0x3098,
> + 0x309C, 0x309C, 0x30C0, 0x30C0, 0x30C8, 0x30C8, 0x30D0, 0x30D0,
> + 0x30D8, 0x30D8, 0x30E0, 0x30E0, 0x3100, 0x3100, 0x3108, 0x3108,
> + 0x3110, 0x3110, 0x3118, 0x3118, 0x3120, 0x3120, 0x3124, 0x3125,
> + 0x3129, 0x3129, 0x340C, 0x340C, 0x3410, 0x3410,
> + ~0 /* sentinel */
> +};
> +
> static struct msm_gpu_state *a4xx_gpu_state_get(struct msm_gpu *gpu)
> {
> struct msm_gpu_state *state = kzalloc(sizeof(*state), GFP_KERNEL);
> @@ -568,13 +614,14 @@ struct msm_gpu *a4xx_gpu_init(struct drm_device *dev)
> gpu->perfcntrs = NULL;
> gpu->num_perfcntrs = 0;
>
> - adreno_gpu->registers = a4xx_registers;
> - adreno_gpu->reg_offsets = a4xx_register_offsets;
> -
> ret = adreno_gpu_init(dev, pdev, adreno_gpu, &funcs, 1);
> if (ret)
> goto fail;
>
> + adreno_gpu->registers = adreno_is_a405(adreno_gpu) ? a405_registers :
> + a4xx_registers;
> + adreno_gpu->reg_offsets = a4xx_register_offsets;
> +
> /* if needed, allocate gmem: */
> if (adreno_is_a4xx(adreno_gpu)) {
> ret = adreno_gpu_ocmem_init(dev->dev, adreno_gpu,
> --
> 2.17.1
>
--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v6 00/16] drm/i915: Add support for HDCP 1.4 over MST connectors
On Fri, May 15, 2020 at 10:48 AM Ramalingam C wrote: > > On 2020-04-29 at 15:54:46 -0400, Sean Paul wrote: > > From: Sean Paul > > > > Changes in v6: > > -Rebased on -tip > > -Disabled HDCP over MST on GEN12 > > -Addressed Lyude's review comments in the QUERY_STREAM_ENCRYPTION_STATUS > > patch > > Sean, > > What is the test setup you have used? > Hi Ram, Thanks for the feedback. To be completely honest it's really frustrating that I'm just now getting questions and review feedback (which I've been begging for on IRC) on this review that could have been addressed ~5 months ago. It's super disruptive to have to keep switching back to this after a long hiatus and many i915 refactors complicating my rebases. If no one wants this patchset, that's fine, please just let me know so I don't waste any more time. If Intel is interested, could we please stop the review trickle and lay out exactly what needs to be done to get this landed? Sean > I am afraid our CI dont have the coverage for MST capability yet to provide > the functional status of the code. > > -Ram. > > > > Sean Paul (16): > > drm/i915: Fix sha_text population code > > drm/i915: Clear the repeater bit on HDCP disable > > drm/i915: WARN if HDCP signalling is enabled upon disable > > drm/i915: Intercept Aksv writes in the aux hooks > > drm/i915: Use the cpu_transcoder in intel_hdcp to toggle HDCP > > signalling > > drm/i915: Factor out hdcp->value assignments > > drm/i915: Protect workers against disappearing connectors > > drm/i915: Don't fully disable HDCP on a port if multiple pipes are > > using it > > drm/i915: Support DP MST in enc_to_dig_port() function > > drm/i915: Use ddi_update_pipe in intel_dp_mst > > drm/i915: Factor out HDCP shim functions from dp for use by dp_mst > > drm/i915: Plumb port through hdcp init > > drm/i915: Add connector to hdcp_shim->check_link() > > drm/mst: Add support for QUERY_STREAM_ENCRYPTION_STATUS MST sideband > > message > > drm/i915: Print HDCP version info for all connectors > > drm/i915: Add HDCP 1.4 support for MST connectors > > > > drivers/gpu/drm/drm_dp_mst_topology.c | 142 > > drivers/gpu/drm/i915/Makefile | 1 + > > drivers/gpu/drm/i915/display/intel_ddi.c | 29 +- > > drivers/gpu/drm/i915/display/intel_ddi.h | 2 + > > .../drm/i915/display/intel_display_debugfs.c | 21 +- > > .../drm/i915/display/intel_display_types.h| 30 +- > > drivers/gpu/drm/i915/display/intel_dp.c | 654 +-- > > drivers/gpu/drm/i915/display/intel_dp.h | 9 + > > drivers/gpu/drm/i915/display/intel_dp_hdcp.c | 743 ++ > > drivers/gpu/drm/i915/display/intel_dp_mst.c | 19 + > > drivers/gpu/drm/i915/display/intel_hdcp.c | 217 +++-- > > drivers/gpu/drm/i915/display/intel_hdcp.h | 2 +- > > drivers/gpu/drm/i915/display/intel_hdmi.c | 25 +- > > .../drm/selftests/test-drm_dp_mst_helper.c| 17 + > > include/drm/drm_dp_helper.h | 3 + > > include/drm/drm_dp_mst_helper.h | 44 ++ > > include/drm/drm_hdcp.h| 3 + > > 17 files changed, 1235 insertions(+), 726 deletions(-) > > create mode 100644 drivers/gpu/drm/i915/display/intel_dp_hdcp.c > > > > -- > > Sean Paul, Software Engineer, Google / Chromium OS > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 5/6] drm: msm: a6xx: use dev_pm_opp_set_bw to set DDR bandwidth
On Thu, May 14, 2020 at 04:24:18PM +0530, Sharat Masetty wrote: > This patches replaces the previously used static DDR vote and uses > dev_pm_opp_set_bw() to scale GPU->DDR bandwidth along with scaling > GPU frequency. > > Signed-off-by: Sharat Masetty > --- > drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 6 +- > 1 file changed, 1 insertion(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > index 2d8124b..79433d3 100644 > --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > @@ -141,11 +141,7 @@ void a6xx_gmu_set_freq(struct msm_gpu *gpu, struct > dev_pm_opp *opp) > > gmu->freq = gmu->gpu_freqs[perf_index]; > > - /* > - * Eventually we will want to scale the path vote with the frequency but > - * for now leave it at max so that the performance is nominal. > - */ > - icc_set_bw(gpu->icc_path, 0, MBps_to_icc(7216)); > + dev_pm_opp_set_bw(&gpu->pdev->dev, opp); > } This adds an implicit requirement that all targets need bandwidth settings defined in the OPP or they won't get a bus vote at all. I would prefer that there be an default escape valve but if not you'll need to add bandwidth values for the sdm845 OPP that target doesn't regress. Jordan > unsigned long a6xx_gmu_get_freq(struct msm_gpu *gpu) > -- > 2.7.4 > -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/2] drm: add docs for standard CRTC properties
This patch adds docs for the ACTIVE and MODE_ID CRTC properties. Signed-off-by: Simon Ser Cc: Daniel Vetter Cc: Ville Syrjala Cc: Pekka Paalanen Cc: Michel Dänzer --- Let me know if you think of other things to add. Should we refer to ALLOW_MODESET in the MODE_ID docs? Documentation/gpu/drm-kms.rst | 5 + drivers/gpu/drm/drm_crtc.c| 16 2 files changed, 21 insertions(+) diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst index 397314d08f77..4289b7205696 100644 --- a/Documentation/gpu/drm-kms.rst +++ b/Documentation/gpu/drm-kms.rst @@ -460,6 +460,11 @@ HDMI Specific Connector Properties .. kernel-doc:: drivers/gpu/drm/drm_connector.c :doc: HDMI connector properties +Standard CRTC Properties + +.. kernel-doc:: drivers/gpu/drm/drm_crtc.c + :doc: standard CRTC properties + Plane Composition Properties diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 4936e1080e41..c28e662c1e93 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -204,6 +204,22 @@ struct dma_fence *drm_crtc_create_fence(struct drm_crtc *crtc) return fence; } +/** + * DOC: standard CRTC properties + * + * DRM CRTCs have a few standardized properties: + * + * ACTIVE: + * Atomic property for setting the power state of the CRTC. When set to 1 the + * CRTC will actively display content. When set to 0 the CRTC will be powered + * off. There is no expectation that user-space will reset CRTC resources like + * the mode and planes when setting ACTIVE to 0. + * MODE_ID: + * Atomic property for setting the CRTC display timings. The value is the ID + * of a blob containing the DRM mode info. To disable the CRTC, user-space + * must set this property to 0. + */ + /** * drm_crtc_init_with_planes - Initialise a new CRTC object with *specified primary and cursor planes. -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] drm: DPMS is no longer the only mutable connector prop
There are a bunch of other writable connector properties now.
Signed-off-by: Simon Ser
Cc: Daniel Vetter
Cc: Ville Syrjala
Cc: Pekka Paalanen
Cc: Michel Dänzer
---
drivers/gpu/drm/drm_connector.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index b1099e1251a2..f2b20fd66319 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -948,8 +948,7 @@ static const struct drm_prop_enum_list dp_colorspaces[] = {
* connector is linked to. Drivers should never set this property directly,
* it is handled by the DRM core by calling the &drm_connector_funcs.dpms
* callback. For atomic drivers the remapping to the "ACTIVE" property is
- * implemented in the DRM core. This is the only standard connector
- * property that userspace can change.
+ * implemented in the DRM core.
*
* Note that this property cannot be set through the MODE_ATOMIC ioctl,
* userspace must use "ACTIVE" on the CRTC instead.
--
2.26.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 2/2] powerpc/configs: replace deprecated riva/nvidia with nouveau
On Mai 18 2020, Michael Ellerman wrote: > The old drivers may be crufty but they presumably have been tested by > people and at least somewhat work. I can confirm that the nvidia fbdev driver is working perfectly fine. > I gave it a quick spin on a G5 I have access to and dmesg has a bunch of > errors in it (see below). I can't actually tell if the display is > working because the machine is remote, and I can't go and check it at > the moment because the office is closed. The nouveau driver is completely borked. [2.299204] nouveau :f0:10.0: NVIDIA NV34 (034100a2) [2.363100] nouveau :f0:10.0: bios: version 04.34.20.19.00 [2.363273] nouveau :f0:10.0: bios: OOB 1 0962 0962 [2.363323] nouveau :f0:10.0: bios: OOB 1 0966 0966 [2.363332] nouveau :f0:10.0: bios: OOB 1 0963 0963 [2.363341] nouveau :f0:10.0: bios: OOB 1 0964 0964 [2.363387] nouveau :f0:10.0: bios: OOB 1 096a 096a [2.363396] nouveau :f0:10.0: bios: OOB 1 0967 0967 [2.363405] nouveau :f0:10.0: bios: OOB 1 0968 0968 [2.363453] nouveau :f0:10.0: bios: OOB 1 096e 096e [2.363462] nouveau :f0:10.0: bios: OOB 1 096b 096b [2.363471] nouveau :f0:10.0: bios: OOB 1 096c 096c [2.363516] nouveau :f0:10.0: bios: OOB 1 0972 0972 [2.363526] nouveau :f0:10.0: bios: OOB 1 096f 096f [2.363534] nouveau :f0:10.0: bios: OOB 1 0970 0970 [2.363580] nouveau :f0:10.0: bios: OOB 1 0976 0976 [2.363589] nouveau :f0:10.0: bios: OOB 1 0973 0973 [2.363597] nouveau :f0:10.0: bios: OOB 1 0974 0974 [2.363643] nouveau :f0:10.0: bios: OOB 1 097a 097a [2.363652] nouveau :f0:10.0: bios: OOB 1 0977 0977 [2.363661] nouveau :f0:10.0: bios: OOB 1 0978 0978 [2.363709] nouveau :f0:10.0: bios: OOB 1 097e 097e [2.363718] nouveau :f0:10.0: bios: OOB 1 097b 097b [2.363727] nouveau :f0:10.0: bios: OOB 1 097c 097c [2.363772] nouveau :f0:10.0: bios: OOB 1 0982 0982 [2.363781] nouveau :f0:10.0: bios: OOB 1 097f 097f [2.363790] nouveau :f0:10.0: bios: OOB 1 0980 0980 [2.363836] nouveau :f0:10.0: bios: OOB 1 0986 0986 [2.363845] nouveau :f0:10.0: bios: OOB 1 0983 0983 [2.363854] nouveau :f0:10.0: bios: OOB 1 0984 0984 [2.363900] nouveau :f0:10.0: bios: OOB 1 098a 098a [2.363909] nouveau :f0:10.0: bios: OOB 1 0987 0987 [2.363918] nouveau :f0:10.0: bios: OOB 1 0988 0988 [2.363965] nouveau :f0:10.0: bios: OOB 1 098e 098e [2.363974] nouveau :f0:10.0: bios: OOB 1 098b 098b [2.363983] nouveau :f0:10.0: bios: OOB 1 098c 098c [2.364029] nouveau :f0:10.0: bios: OOB 1 0992 0992 [2.364038] nouveau :f0:10.0: bios: OOB 1 098f 098f [2.364047] nouveau :f0:10.0: bios: OOB 1 0990 0990 [2.364383] nouveau :f0:10.0: gpio: GPU is missing power, check its power cables. Boot with nouveau.config=NvPowerChecks=0 to disable. [2.364402] nouveau :f0:10.0: gpio: init failed, -22 [2.364431] nouveau :f0:10.0: init failed with -22 [2.364438] nouveau: DRM-master::0080: init failed with -22 [2.364450] nouveau :f0:10.0: DRM-master: Device allocation failed: -22 [2.365268] nouveau: probe of :f0:10.0 failed with error -22 Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 "And now for something completely different." ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 2/2] powerpc/configs: replace deprecated riva/nvidia with nouveau
On Mon, 18 May 2020 at 13:48, Bartlomiej Zolnierkiewicz wrote: > > > On 5/18/20 1:19 PM, Emil Velikov wrote: > > Hi Michael, > > > > On Mon, 18 May 2020 at 08:30, Michael Ellerman wrote: > >> > >> Emil Velikov writes: > >>> As mentioned in earlier commit, the riva and nvidia fbdev drivers have > >>> seen no love over the years, are short on features and overall below par > >>> > >>> Users are encouraged to switch to the nouveau drm driver instead. > >>> > >>> v2: Split configs to separate patch, enable nouveau (Bartlomiej) > >>> > >>> Cc: Antonino Daplas > >>> Cc: Bartlomiej Zolnierkiewicz > >>> Cc: linux-fb...@vger.kernel.org > >>> Cc: dri-devel@lists.freedesktop.org > >>> Cc: Michael Ellerman > >>> Cc: Benjamin Herrenschmidt > >>> Cc: Paul Mackerras > >>> Cc: linuxppc-...@lists.ozlabs.org > >>> Signed-off-by: Emil Velikov > >>> Acked-by: Daniel Vetter (v1) > >>> --- > >>> Hi all unless, there are objections I would prefer to merge this via > >>> the drm tree. > >> > >> Have you tested that the resulting kernels work on the relevant > >> hardware? > >> > > Sadly, no I haven't. I'm updating the defconfigs as requested by the > > fbdev maintainer. > > I've just noticed that v1 (patch #1/1) & v2 (patch #1/2) lack > Cc: to powerpc Maintainers so they cannot see the context of > changes in this patch. > Haven't see any instances of fbdev/drm patches being Cc to arch/ developers. As such I made sure the commit message illustrates 1/2 as clearly as possible. > Also you've proposed v1 yourself and it has already contained > modifications to defconfigs (removal of setting the config > options for the old drivers) in addition to marking the old > drivers as BROKEN. > > It now turns out that v1 has also never been tested. :( > > Please don't submit untested patches without marking them > as such. > Does x86 box with nv34 GPU count as testing? I suspect not :-( It was been using nouveau ever since I bought it 15+ years ago. Will be more careful in the future. -Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 2/2] powerpc/configs: replace deprecated riva/nvidia with nouveau
On 5/18/20 1:19 PM, Emil Velikov wrote: > Hi Michael, > > On Mon, 18 May 2020 at 08:30, Michael Ellerman wrote: >> >> Emil Velikov writes: >>> As mentioned in earlier commit, the riva and nvidia fbdev drivers have >>> seen no love over the years, are short on features and overall below par >>> >>> Users are encouraged to switch to the nouveau drm driver instead. >>> >>> v2: Split configs to separate patch, enable nouveau (Bartlomiej) >>> >>> Cc: Antonino Daplas >>> Cc: Bartlomiej Zolnierkiewicz >>> Cc: linux-fb...@vger.kernel.org >>> Cc: dri-devel@lists.freedesktop.org >>> Cc: Michael Ellerman >>> Cc: Benjamin Herrenschmidt >>> Cc: Paul Mackerras >>> Cc: linuxppc-...@lists.ozlabs.org >>> Signed-off-by: Emil Velikov >>> Acked-by: Daniel Vetter (v1) >>> --- >>> Hi all unless, there are objections I would prefer to merge this via >>> the drm tree. >> >> Have you tested that the resulting kernels work on the relevant >> hardware? >> > Sadly, no I haven't. I'm updating the defconfigs as requested by the > fbdev maintainer. I've just noticed that v1 (patch #1/1) & v2 (patch #1/2) lack Cc: to powerpc Maintainers so they cannot see the context of changes in this patch. Also you've proposed v1 yourself and it has already contained modifications to defconfigs (removal of setting the config options for the old drivers) in addition to marking the old drivers as BROKEN. It now turns out that v1 has also never been tested. :( Please don't submit untested patches without marking them as such. >> The old drivers may be crufty but they presumably have been tested by >> people and at least somewhat work. >> >> So I'd be inclined to leave the defconfigs alone until someone can test >> that the new driver works at all. @Michael: Fully agreed. I would also like you to review/ack patch #1/2: https://lore.kernel.org/dri-devel/20200517220524.4036334-1-emil.l.veli...@gmail.com/ Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics > Works for me. > >> I gave it a quick spin on a G5 I have access to and dmesg has a bunch of >> errors in it (see below). I can't actually tell if the display is >> working because the machine is remote, and I can't go and check it at >> the moment because the office is closed. >> > >>From what I can see, there seems to be three bits: > - attempted out-of-bound attempts to read the vbios > Genuine concern or noise? Likely using the bios from open firmware, > check any of the other options - see NvBios in [1] > - cannot figure out the timer input frequency > No idea > - the TV1 EDID is empty > Is there an actual TV connected to the device, check with another cable > > Regardless of the patches, reporting [2] the above would be a nice move. > > Thanks > Emil > [1] > https://protect2.fireeye.com/url?k=d6cf7004-8b548c67-d6cefb4b-0cc47a31cdbc-7c3b251c170ed483&q=1&u=https%3A%2F%2Fnouveau.freedesktop.org%2Fwiki%2FKernelModuleParameters%2F > [2] https://gitlab.freedesktop.org/xorg/driver/xf86-video-nouveau/-/issues ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 2/2] powerpc/configs: replace deprecated riva/nvidia with nouveau
On Mon, 2020-05-18 at 12:19 +0100, Emil Velikov wrote: > > - attempted out-of-bound attempts to read the vbios So on these things, the actual ROM doesn't contain what you want, but the device-tree has a property "NVDA,BMP" that contains some kind of mini-BIOS (around 2.4KB) which should contain the necessary tables the driver is looking for. I think nouveau has code to find these in nvkm/subdev/bios/shadowof.c, so at least that should have been working, but maybe some debugging/instrumentation would be useful there. > Genuine concern or noise? Likely using the bios from open firmware, > check any of the other options - see NvBios in [1] > - cannot figure out the timer input frequency > No idea > - the TV1 EDID is empty > Is there an actual TV connected to the device, check with another cable Probaby not. > Regardless of the patches, reporting [2] the above would be a nice move. > > Thanks > Emil > [1] https://nouveau.freedesktop.org/wiki/KernelModuleParameters/ > [2] https://gitlab.freedesktop.org/xorg/driver/xf86-video-nouveau/-/issues ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 2/2] powerpc/configs: replace deprecated riva/nvidia with nouveau
On Mon, 2020-05-18 at 12:00 +0100, Emil Velikov wrote: > I believe you reported issues due to different page size for the CPU/GPU. > Have you tried nouveau recently, there has been a handful of patches > on the topic since your report. > > Alternatively, it would make sense you rebase, cleanup and merge your patch. That was a problem for the G5s. There were other issues for more ancient machines with older nVidia GPUs. Additionally a lot of those Apple machines don't have a BIOS ROM to get the various tables from. At this stage unfortunately I don't have access to most of that HW to test with anymore. I do have one G5 I might be able to dig out of my basement this week to try out. In any case, digging out that patch should be useful as powerpc64 is still 64k pages :) Cheers, Ben. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/etnaviv: Fix a leak in submit_pin_objects()
If the mapping address is wrong then we have to release the reference to
it before returning -EINVAL.
Fixes: 00ddc0b2 ("drm/etnaviv: implement softpin")
Signed-off-by: Dan Carpenter
---
>From static analysis. Untested.
drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
index 3b0afa156d92..54def341c1db 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
@@ -238,8 +238,10 @@ static int submit_pin_objects(struct etnaviv_gem_submit
*submit)
}
if ((submit->flags & ETNA_SUBMIT_SOFTPIN) &&
-submit->bos[i].va != mapping->iova)
+submit->bos[i].va != mapping->iova) {
+ etnaviv_gem_mapping_unreference(mapping);
return -EINVAL;
+ }
atomic_inc(&etnaviv_obj->gpu_active);
--
2.26.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/mediatek: Delete not used of_device_get_match_data
From: Matthias Brugger
The driver will be loaded by via a platform device. So we
will need to get the device_node from the parent device.
Depending on this we will set the driver data.
As all this is done later already, just delete the call to
of_device_get_match_data.
Signed-off-by: Matthias Brugger
---
drivers/gpu/drm/mediatek/mtk_drm_drv.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index e2bb0d19ef99..63ec92ba0e92 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -447,7 +447,6 @@ static int mtk_drm_probe(struct platform_device *pdev)
if (!private)
return -ENOMEM;
- private->data = of_device_get_match_data(dev);
private->mmsys_dev = dev->parent;
if (!private->mmsys_dev) {
dev_err(dev, "Failed to get MMSYS device\n");
--
2.26.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 2/2] powerpc/configs: replace deprecated riva/nvidia with nouveau
Hi Michael, On Mon, 18 May 2020 at 08:30, Michael Ellerman wrote: > > Emil Velikov writes: > > As mentioned in earlier commit, the riva and nvidia fbdev drivers have > > seen no love over the years, are short on features and overall below par > > > > Users are encouraged to switch to the nouveau drm driver instead. > > > > v2: Split configs to separate patch, enable nouveau (Bartlomiej) > > > > Cc: Antonino Daplas > > Cc: Bartlomiej Zolnierkiewicz > > Cc: linux-fb...@vger.kernel.org > > Cc: dri-devel@lists.freedesktop.org > > Cc: Michael Ellerman > > Cc: Benjamin Herrenschmidt > > Cc: Paul Mackerras > > Cc: linuxppc-...@lists.ozlabs.org > > Signed-off-by: Emil Velikov > > Acked-by: Daniel Vetter (v1) > > --- > > Hi all unless, there are objections I would prefer to merge this via > > the drm tree. > > Have you tested that the resulting kernels work on the relevant > hardware? > Sadly, no I haven't. I'm updating the defconfigs as requested by the fbdev maintainer. > The old drivers may be crufty but they presumably have been tested by > people and at least somewhat work. > > So I'd be inclined to leave the defconfigs alone until someone can test > that the new driver works at all. > Works for me. > I gave it a quick spin on a G5 I have access to and dmesg has a bunch of > errors in it (see below). I can't actually tell if the display is > working because the machine is remote, and I can't go and check it at > the moment because the office is closed. > >From what I can see, there seems to be three bits: - attempted out-of-bound attempts to read the vbios Genuine concern or noise? Likely using the bios from open firmware, check any of the other options - see NvBios in [1] - cannot figure out the timer input frequency No idea - the TV1 EDID is empty Is there an actual TV connected to the device, check with another cable Regardless of the patches, reporting [2] the above would be a nice move. Thanks Emil [1] https://nouveau.freedesktop.org/wiki/KernelModuleParameters/ [2] https://gitlab.freedesktop.org/xorg/driver/xf86-video-nouveau/-/issues ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 1/4] dma-buf: add support for virtio exported objects
Hello David, On Fri, 15 May 2020 at 19:33, Daniel Vetter wrote: > > On Fri, May 15, 2020 at 02:07:06PM +0900, David Stevens wrote: > > On Thu, May 14, 2020 at 9:30 PM Daniel Vetter wrote: > > > On Thu, May 14, 2020 at 05:19:40PM +0900, David Stevens wrote: > > > > Sorry for the duplicate reply, didn't notice this until now. > > > > > > > > > Just storing > > > > > the uuid should be doable (assuming this doesn't change during the > > > > > lifetime of the buffer), so no need for a callback. > > > > > > > > Directly storing the uuid doesn't work that well because of > > > > synchronization issues. The uuid needs to be shared between multiple > > > > virtio devices with independent command streams, so to prevent races > > > > between importing and exporting, the exporting driver can't share the > > > > uuid with other drivers until it knows that the device has finished > > > > registering the uuid. That requires a round trip to and then back from > > > > the device. Using a callback allows the latency from that round trip > > > > registration to be hidden. > > > > > > Uh, that means you actually do something and there's locking involved. > > > Makes stuff more complicated, invariant attributes are a lot easier > > > generally. Registering that uuid just always doesn't work, and blocking > > > when you're exporting? > > > > Registering the id at creation and blocking in gem export is feasible, > > but it doesn't work well for systems with a centralized buffer > > allocator that doesn't support batch allocations (e.g. gralloc). In > > such a system, the round trip latency would almost certainly be > > included in the buffer allocation time. At least on the system I'm > > working on, I suspect that would add 10s of milliseconds of startup > > latency to video pipelines (although I haven't benchmarked the > > difference). Doing the blocking as late as possible means most or all > > of the latency can be hidden behind other pipeline setup work. > > > > In terms of complexity, I think the synchronization would be basically > > the same in either approach, just in different locations. All it would > > do is alleviate the need for a callback to fetch the UUID. > I think I agree with Daniel there - this seems best suited for code within virtio. > Hm ok. I guess if we go with the older patch, where this all is a lot more > just code in virtio, doing an extra function to allocate the uuid sounds > fine. Then synchronization is entirely up to the virtio subsystem and not > a dma-buf problem (and hence not mine). You can use dma_resv_lock or so, > but no need to. But with callbacks potentially going both ways things > always get a bit interesting wrt locking - this is what makes peer2peer > dma-buf so painful right now. Hence I'd like to avoid that if needed, at > least at the dma-buf level. virtio code I don't mind what you do there :-) > > Cheers, Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch Best, Sumit. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] drm/arm: Kconfig annotate drivers as COMPILE_TEST
On Sun, May 17, 2020 at 08:36:53PM +0100, Emil Velikov wrote: > Add the COMPILE_TEST conditional, so that people can at least build test > the drivers. > > Cc: Liviu Dudau Acked-by: Liviu Dudau > Cc: Brian Starkey > Cc: Mali DP Maintainers > Cc: dri-devel@lists.freedesktop.org > Signed-off-by: Emil Velikov > --- > Please merge through the ARM tree. We're not using the ARM tree anymore, we push patches through drm-misc. I'm happy if you want to push the whole series or I can do the 2 patches that I've been Cc-ed on. Best regards, Liviu > > Aside: would make sense to have the tree drivers in separate folders > alongside each other. > > With lima and panfrost in-tree, there's little point in the extra > "display" for komeda. Might as well remove that one. > --- > drivers/gpu/drm/arm/Kconfig | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/arm/Kconfig b/drivers/gpu/drm/arm/Kconfig > index a204103b3efb..3a9e966e0e78 100644 > --- a/drivers/gpu/drm/arm/Kconfig > +++ b/drivers/gpu/drm/arm/Kconfig > @@ -3,7 +3,7 @@ menu "ARM devices" > > config DRM_HDLCD > tristate "ARM HDLCD" > - depends on DRM && OF && (ARM || ARM64) > + depends on DRM && OF && (ARM || ARM64 || COMPILE_TEST) > depends on COMMON_CLK > select DRM_KMS_HELPER > select DRM_KMS_CMA_HELPER > @@ -24,7 +24,7 @@ config DRM_HDLCD_SHOW_UNDERRUN > > config DRM_MALI_DISPLAY > tristate "ARM Mali Display Processor" > - depends on DRM && OF && (ARM || ARM64) > + depends on DRM && OF && (ARM || ARM64 || COMPILE_TEST) > depends on COMMON_CLK > select DRM_KMS_HELPER > select DRM_KMS_CMA_HELPER > -- > 2.25.1 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- | I would like to | | fix the world, | | but they're not | | giving me the | \ source code! / --- ¯\_(ツ)_/¯ ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 2/2] powerpc/configs: replace deprecated riva/nvidia with nouveau
Hi Benjamin, On Mon, 18 May 2020 at 01:45, Benjamin Herrenschmidt wrote: > > On Sun, 2020-05-17 at 23:05 +0100, Emil Velikov wrote: > > As mentioned in earlier commit, the riva and nvidia fbdev drivers > > have > > seen no love over the years, are short on features and overall below > > par > > > > Users are encouraged to switch to the nouveau drm driver instead. > > > > v2: Split configs to separate patch, enable nouveau (Bartlomiej) > > Back when I still had these things to play with (years ago) nouveau > didn't work properly on these ancient machines. > I believe you reported issues due to different page size for the CPU/GPU. Have you tried nouveau recently, there has been a handful of patches on the topic since your report. Alternatively, it would make sense you rebase, cleanup and merge your patch. -Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 11/12] gpu/drm: Ingenic: Add support for the IPU
Hi Paul,
Disclaimer: I don't know much about the hardware :-P
On Sun, 17 May 2020 at 00:31, Paul Cercueil wrote:
>
> Add support for the Image Processing Unit (IPU) found in all Ingenic
> SoCs.
>
Since the IPU is present on all devices supported, does it make sense
to have it as separate module?
Didn't check closely although I suspect doing that will remove the
need for the component patch.
> --- a/drivers/gpu/drm/ingenic/ingenic-drm.c
> +++ b/drivers/gpu/drm/ingenic/ingenic-drm.c
> @@ -50,7 +50,7 @@ struct jz_soc_info {
>
> struct ingenic_drm {
> struct drm_device drm;
> - struct drm_plane f0, f1;
> + struct drm_plane f0, f1, *ipu_plane;
> struct drm_crtc crtc;
> struct drm_encoder encoder;
>
> @@ -186,13 +186,16 @@ static void ingenic_drm_crtc_update_timings(struct
> ingenic_drm *priv,
> regmap_update_bits(priv->map, JZ_REG_LCD_CTRL,
>JZ_LCD_CTRL_OFUP | JZ_LCD_CTRL_BURST_16,
>JZ_LCD_CTRL_OFUP | JZ_LCD_CTRL_BURST_16);
> +
> + regmap_write(priv->map, JZ_REG_LCD_IPUR, JZ_LCD_IPUR_IPUREN |
> +(ht * vpe / 3) << JZ_LCD_IPUR_IPUR_LSB);
This hunk also indicates that it may be better to merge the IPU within
the existing driver.
-Emil
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/debugfs: fix plain echo to connector "force" attribute
On Mon, 18 May 2020 at 10:22, Jani Nikula wrote: > > On Sun, 17 May 2020, Emil Velikov wrote: > > On Thu, 17 Aug 2017 at 12:34, Jani Nikula > > wrote: > >> > >> On Thu, 17 Aug 2017, Michael Tretter wrote: > >> > Using plain echo to set the "force" connector attribute fails with > >> > -EINVAL, because echo appends a newline to the output. > >> > > >> > Replace strcmp with sysfs_streq to also accept strings that end with a > >> > newline. > >> > > >> > v2: use sysfs_streq instead of stripping trailing whitespace > >> > > >> > Signed-off-by: Michael Tretter > >> > >> Reviewed-by: Jani Nikula > >> > > Seems like this fell through the cracks. Pushed to drm-misc-next. > > From 2017? How'd you find it? :o > > My bad, thanks. > A colleague mentioned about their experiences with \n, so I remembered seeing something. >From a quick grep, we have have handful of other cases mostly in amdgpu and radeon. Will see it I can prep a patch later on today. -Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 01/16] video: amba-clcd: use devm_of_find_backlight
Hi Linus. On Mon, May 18, 2020 at 10:10:12AM +0200, Linus Walleij wrote: > On Sun, May 17, 2020 at 9:01 PM Sam Ravnborg wrote: > > > Look up backlight device using devm_of_find_backlight(). > > This simplifies the code and prevents us from hardcoding > > the node name in the driver. > > > > v2: > > - Added Cc: Peter Ujfalusi > > > > Signed-off-by: Sam Ravnborg > > Cc: Russell King > > Cc: Sam Ravnborg > > Cc: Bartlomiej Zolnierkiewicz > > Cc: Peter Ujfalusi > > Cc: Daniel Vetter > > Cc: Linus Walleij > > Cc: Jani Nikula > > Cc: Douglas Anderson > > Acked-by: Linus Walleij Thanks. I went ahead and applied this now, so we could kill the last user of of_find_backlight_by_node(). I hope we can make of_find_backlight_by_node() static after the merge window so no new users appears. Sam ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 04/12] gpu/drm: ingenic: Fix bogus crtc_atomic_check callback
Hi Sam,
Le dim. 17 mai 2020 à 8:17, Sam Ravnborg a écrit :
On Sat, May 16, 2020 at 11:50:49PM +0200, Paul Cercueil wrote:
The code was comparing the SoC's maximum height with the mode's
width,
and vice-versa. D'oh.
Cc: sta...@vger.kernel.org # v5.6
Fixes: a7c909b7c037 ("gpu/drm: ingenic: Check for display size in
CRTC atomic check")
Signed-off-by: Paul Cercueil
Looks correct.
Acked-by: Sam Ravnborg
Pushed to drm-misc-fixes, thanks for the review.
-Paul
---
Notes:
This patch was previously sent standalone.
I marked it as superseded in patchwork.
Nothing has been changed here.
drivers/gpu/drm/ingenic/ingenic-drm.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/ingenic/ingenic-drm.c
b/drivers/gpu/drm/ingenic/ingenic-drm.c
index 632d72177123..0c472382a08b 100644
--- a/drivers/gpu/drm/ingenic/ingenic-drm.c
+++ b/drivers/gpu/drm/ingenic/ingenic-drm.c
@@ -330,8 +330,8 @@ static int ingenic_drm_crtc_atomic_check(struct
drm_crtc *crtc,
if (!drm_atomic_crtc_needs_modeset(state))
return 0;
- if (state->mode.hdisplay > priv->soc_info->max_height ||
- state->mode.vdisplay > priv->soc_info->max_width)
+ if (state->mode.hdisplay > priv->soc_info->max_width ||
+ state->mode.vdisplay > priv->soc_info->max_height)
return -EINVAL;
rate = clk_round_rate(priv->pix_clk,
--
2.26.2
___
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 V3 10/15] arch/kmap: Define kmap_atomic_prot() for all arch's
Hi, On Thu, May 07, 2020 at 07:59:58AM -0700, ira.we...@intel.com wrote: > 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. > > Reviewed-by: Christoph Hellwig > Signed-off-by: Ira Weiny This patch causes a variety of crashes whem booting powerpc images in qemu. There are lots of warnings such as: WARNING: CPU: 0 PID: 0 at lib/locking-selftest.c:743 irqsafe1_hard_spin_12+0x50/0xb0 Modules linked in: CPU: 0 PID: 0 Comm: swapper Tainted: GW 5.7.0-rc5-next-20200515 #1 NIP: c0660c7c LR: c0660c44 CTR: c0660c2c REGS: c1223e68 TRAP: 0700 Tainted: GW (5.7.0-rc5-next-20200515) MSR: 00021000 CR: 28000224 XER: 2000 GPR00: c0669c78 c1223f20 c113d560 c0660c44 0001 c1223ea8 0001 GPR08: 0001 fffc 88000222 GPR16: c000 c1125084 GPR24: c1125084 c123 c1879538 fffc 0001 c1011afc c123 NIP [c0660c7c] irqsafe1_hard_spin_12+0x50/0xb0 LR [c0660c44] irqsafe1_hard_spin_12+0x18/0xb0 Call Trace: [c1223f20] [c188] megasas_mgmt_info+0xee4/0x1008 (unreliable) [c1223f40] [c0669c78] dotest+0x38/0x550 [c1223f70] [c066aa4c] locking_selftest+0x8bc/0x1d54 [c1223fa0] [c10e0bc8] start_kernel+0x3ec/0x510 [c1223ff0] [c3a0] set_ivor+0x118/0x154 Instruction dump: 8142 38e80001 3d4a0001 2c08 9142 90e20488 40820008 91020470 8129 5529031e 7d290034 5529d97e <0f09> 3fe0c11c 3bff3964 3bff00ac irq event stamp: 588 hardirqs last enabled at (587): [] vprintk_emit+0x1b4/0x33c hardirqs last disabled at (588): [] irqsafe1_hard_spin_12+0x18/0xb0 softirqs last enabled at (0): [<>] 0x0 softirqs last disabled at (0): [<>] 0x0 ---[ end trace b18fe9e172f99d03 ]--- This is followed by: BUG: sleeping function called from invalid context at lib/mpi/mpi-pow.c:245 in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 14, name: cryptomgr_test INFO: lockdep is turned off. CPU: 0 PID: 14 Comm: cryptomgr_test Tainted: GW 5.7.0-rc5-next-20200515 #1 Call Trace: [ce221b58] [c008755c] ___might_sleep+0x280/0x2a8 (unreliable) [ce221b78] [c06bc524] mpi_powm+0x634/0xc50 [ce221c38] [c05eafdc] rsa_dec+0x88/0x134 [ce221c78] [c05f3b40] test_akcipher_one+0x678/0x804 [ce221dc8] [c05f3d7c] alg_test_akcipher+0xb0/0x130 [ce221df8] [c05ee674] alg_test.part.0+0xb4/0x458 [ce221ed8] [c05ed2b0] cryptomgr_test+0x30/0x50 [ce221ef8] [c007cd74] kthread+0x134/0x170 [ce221f38] [c001433c] ret_from_kernel_thread+0x14/0x1c Kernel panic - not syncing: Aiee, killing interrupt handler! CPU: 0 PID: 14 Comm: cryptomgr_test Tainted: GW 5.7.0-rc5-next-20200515 #1 Call Trace: [ce221e08] [c00530fc] panic+0x148/0x34c (unreliable) [ce221e68] [c0056460] do_exit+0xac0/0xb40 [ce221eb8] [c00f5be8] find_kallsyms_symbol_value+0x0/0x128 [ce221ed8] [c05ed2d0] crypto_alg_put+0x0/0x70 [ce221ef8] [c007cd74] kthread+0x134/0x170 [ce221f38] [c001433c] ret_from_kernel_thread+0x14/0x1c Bisect log is attached. The patch can not easily be reverted since it results in compile errors. Note that similar failures are seen with sparc32 images. Those bisect to a different patch, but reverting that patch doesn't fix the problem. The failure pattern (warnings followed by a crash in cryptomgr_test) is the same. Guenter --- # bad: [bdecf38f228bcca73b31ada98b5b7ba1215eb9c9] Add linux-next specific files for 20200515 # good: [2ef96a5bb12be62ef75b5828c0aab838ebb29cb8] Linux 5.7-rc5 git bisect start 'HEAD' 'v5.7-rc5' # good: [3674d7aa7a8e61d993886c2fb7c896c5ef85e988] Merge remote-tracking branch 'crypto/master' git bisect good 3674d7aa7a8e61d993886c2fb7c896c5ef85e988 # good: [87f6f21783522e6d62127cf33ae5e95f50874beb] Merge remote-tracking branch 'spi/for-next' git bisect good 87f6f21783522e6d62127cf33ae5e95f50874beb # good: [5c428e8277d5d97c85126387d4e00aa5adde4400] Merge remote-tracking branch 'staging/staging-next' git bisect good 5c428e8277d5d97c85126387d4e00aa5adde4400 # good: [f68de67ed934e7bdef4799fdc86f33f14982] Merge remote-tracking branch 'hyperv/hyperv-next' git bisect good f68de67ed934e7bdef4799fdc86f33f14982 # bad: [54acd2dc52b069da59639eea0d0c92726f32fb01] mm/memblock: fix a typo in comment "implict"->"implicit" git bisect bad 54acd2dc52b069da59639eea0d0c92726f32fb01 # good: [784a17aa58a529b84f7cc50f351ed4acf3bd11f3] mm: remove the pgprot argument to __vmalloc git bisect good 784a17aa58a529b84f7cc50f351ed4acf3bd11f3 # good: [6cd8137ff37e9a37aee2d2a8889c8beb8eab192f] khugepaged: replace the usage of sys
Re:Re: Re:Re: [PATCH v2] drm/arm: fixes pixel clock enabled with wrong format
发件人：Liviu Dudau
发送日期：2020-05-15 22:41:49
收件人：Bernard
抄送人：Brian Starkey ,David Airlie
,Daniel Vetter
,dri-devel@lists.freedesktop.org,linux-ker...@vger.kernel.org,opensource.ker...@vivo.com
主题：Re: Re:Re: [PATCH v2] drm/arm: fixes pixel clock enabled with wrong
format>Hi Bernard,
>
>On Fri, May 08, 2020 at 04:47:17PM +0800, Bernard wrote:
>> From: "赵军奎"
>> Date: 2020-04-24 19:37:36
>> To: Liviu Dudau
>> Cc: Brian Starkey ,David Airlie
>> ,Daniel Vetter
>> ,dri-devel@lists.freedesktop.org,linux-ker...@vger.kernel.org,opensource.ker...@vivo.com
>> Subject: Re:Re: [PATCH v2] drm/arm: fixes pixel clock enabled with wrong
>> format
>>
>>
>>
>>
>> From: Liviu Dudau
>> Date: 2020-04-24 19:09:50
>> To: Bernard Zhao
>> Cc: Brian Starkey ,David Airlie
>> ,Daniel Vetter
>> ,dri-devel@lists.freedesktop.org,linux-ker...@vger.kernel.org,opensource.ker...@vivo.com
>> Subject: Re: [PATCH v2] drm/arm: fixes pixel clock enabled with wrong
>> format>Hi Bernand,
>> >
>> >On Thu, Apr 23, 2020 at 11:35:51PM -0700, Bernard Zhao wrote:
>> >> The pixel clock is still enabled when the format is wrong.
>> >> no error branch handle, and also some register is not set
>> >> in this case, e.g: HDLCD_REG__SELECT. Maybe we
>> >> should disable this clock and throw an warn message when
>> >> this happened.
>> >> With this change, the code maybe a bit more readable.
>> >>
>> >> Signed-off-by: Bernard Zhao
>> >>
>> >> Changes since V1:
>> >> *add format error handle, if format is not correct, throw
>> >> an warning message and disable this clock.
>> >>
>> >> Link for V1:
>> >> *https://lore.kernel.org/patchwork/patch/1228501/
>> >> ---
>> >> drivers/gpu/drm/arm/hdlcd_crtc.c | 13 +
>> >> 1 file changed, 9 insertions(+), 4 deletions(-)
>> >>
>> >> diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c
>> >> b/drivers/gpu/drm/arm/hdlcd_crtc.c
>> >> index af67fefed38d..f3945dee2b7d 100644
>> >> --- a/drivers/gpu/drm/arm/hdlcd_crtc.c
>> >> +++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
>> >> @@ -96,7 +96,7 @@ static int hdlcd_set_pxl_fmt(struct drm_crtc *crtc)
>> >> }
>> >>
>> >> if (WARN_ON(!format))
>> >> - return 0;
>> >> + return -EINVAL;
>> >
>> >That is the right fix!
>> >
>> >>
>> >> /* HDLCD uses 'bytes per pixel', zero means 1 byte */
>> >> btpp = (format->bits_per_pixel + 7) / 8;
>> >> @@ -125,7 +125,7 @@ static int hdlcd_set_pxl_fmt(struct drm_crtc *crtc)
>> >> return 0;
>> >> }
>> >>
>> >> -static void hdlcd_crtc_mode_set_nofb(struct drm_crtc *crtc)
>> >> +static int hdlcd_crtc_mode_set_nofb(struct drm_crtc *crtc)
>> >
>> >But this is not. We don't need to propagate the error further, just
>> >
>> >> {
>> >> struct hdlcd_drm_private *hdlcd = crtc_to_hdlcd_priv(crtc);
>> >> struct drm_display_mode *m = &crtc->state->adjusted_mode;
>> >> @@ -162,9 +162,10 @@ static void hdlcd_crtc_mode_set_nofb(struct drm_crtc
>> >> *crtc)
>> >>
>> >> err = hdlcd_set_pxl_fmt(crtc);
>> >> if (err)
>> >> - return;
>> >
>>
>> My previous understanding was that when such an exception occurred, it was
>> caught
>> in the atomic_enable interface, and then disable pixel clock. I am not sure
>> is this ok or
>> i have to do more register clean operaction.
>>
>> >... return here so that we don't call clk_set_rate();
>> And for this part, i am a little confused :
>> 1 clk_set_rate must be set even if format is wrong?
>> 2 The original code logic shows that If format is not correct, we will not
>> set registers
>> HDLCD_REG_PIXEL_FORMAT & HDLCD_REG__SELECT, will this bring in
>> any problems?
>> 3 if 1 the rate must set & 2 registers above doesn`t matter, then maybe
>> there is no
>> need to disable pixel clock.
>> Am i misunderstanding
>
>You are right, the pixel format check should not happen inside
>hdlcd_crtc_atomic_enable()
>hook, it should be moved into a separate function hdlcd_crtc_atomic_check()
>and that needs
>to be hooked into .atomic_check() for hdlcd_crtc_helper_funcs().
>
>If you want to have another go I'll be happy to review and Ack your patch.
>
>Best regards,
>Liviu
>
Hi
Sure, i will check this and re-subbmit one patch.
Regards,
Bernard
>>
>> Regards,
>> Bernard
>>
>> >> + return err;
>> >>
>> >> clk_set_rate(hdlcd->clk, m->crtc_clock * 1000);
>> >> + return 0;
>> >> }
>> >>
>> >> static void hdlcd_crtc_atomic_enable(struct drm_crtc *crtc,
>> >> @@ -173,7 +174,11 @@ static void hdlcd_crtc_atomic_enable(struct drm_crtc
>> >> *crtc,
>> >> struct hdlcd_drm_private *hdlcd = crtc_to_hdlcd_priv(crtc);
>> >>
>> >> clk_prepare_enable(hdlcd->clk);
>> >> - hdlcd_crtc_mode_set_nofb(crtc);
>> >> + if (hdlcd_crtc_mode_set_nofb(crtc)) {
>> >> + DRM_DEBUG_KMS("Invalid format, pixel clock enable failed!\n");
>> >> + clk_disable_unprepare(hdlcd->clk);
>> >> + return;
>> >> + }
>> >> hdlcd_write(hdlcd, HDLCD_REG_COMMAND, 1);
>> >> drm_crtc_vblank_on(crtc);
>> >> }
>> >> --
>> >> 2.26.2
>> >>
>> >
>> >--
>> >
[PATCH] drm/mm: Fix an error handling path in '__igt_once()'
The label of the last gotos must be switched for the error handling code to
work as expected.
Fixes: 83bc4ec37210 ("drm/mm: Add a search-by-address variant to only inspect a
single hole")
Signed-off-by: Christophe JAILLET
---
drivers/gpu/drm/selftests/test-drm_mm.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/selftests/test-drm_mm.c
b/drivers/gpu/drm/selftests/test-drm_mm.c
index 9aabe82dcd3a..af38c4fa4db5 100644
--- a/drivers/gpu/drm/selftests/test-drm_mm.c
+++ b/drivers/gpu/drm/selftests/test-drm_mm.c
@@ -1864,14 +1864,14 @@ static int __igt_once(unsigned int mode)
pr_err("Unexpectedly inserted the node into the wrong hole:
node.start=%llx\n",
node.start);
err = -EINVAL;
- goto err_node;
+ goto err_hi;
}
err = drm_mm_insert_node_generic(&mm, &node, 2, 0, 0, mode);
if (err) {
pr_err("Could not insert the node into the available hole!\n");
err = -EINVAL;
- goto err_hi;
+ goto err_node;
}
err_node:
--
2.25.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 05/12] gpu/drm: Ingenic: Fix opaque pointer casted to wrong type
Hi Sam,
Le dim. 17 mai 2020 à 8:21, Sam Ravnborg a écrit :
On Sat, May 16, 2020 at 11:50:50PM +0200, Paul Cercueil wrote:
The opaque pointer passed to the IRQ handler is a pointer to the
drm_device, not a pointer to our ingenic_drm structure.
It still worked, because our ingenic_drm structure contains the
drm_device as its first field, so the pointer received had the same
value, but this was not semantically correct.
Cc: sta...@vger.kernel.org # v5.3
Fixes: 90b86fcc47b4 ("DRM: Add KMS driver for the Ingenic JZ47xx
SoCs")
Signed-off-by: Paul Cercueil
Acked-by: Sam Ravnborg
Pushed to drm-misc-fixes, thanks for the review.
-Paul
---
drivers/gpu/drm/ingenic/ingenic-drm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/ingenic/ingenic-drm.c
b/drivers/gpu/drm/ingenic/ingenic-drm.c
index 0c472382a08b..97244462599b 100644
--- a/drivers/gpu/drm/ingenic/ingenic-drm.c
+++ b/drivers/gpu/drm/ingenic/ingenic-drm.c
@@ -476,7 +476,7 @@ static int
ingenic_drm_encoder_atomic_check(struct drm_encoder *encoder,
static irqreturn_t ingenic_drm_irq_handler(int irq, void *arg)
{
- struct ingenic_drm *priv = arg;
+ struct ingenic_drm *priv = drm_device_get_priv(arg);
unsigned int state;
regmap_read(priv->map, JZ_REG_LCD_STATE, &state);
--
2.26.2
___
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 v2 2/2] powerpc/configs: replace deprecated riva/nvidia with nouveau
Emil Velikov writes: > As mentioned in earlier commit, the riva and nvidia fbdev drivers have > seen no love over the years, are short on features and overall below par > > Users are encouraged to switch to the nouveau drm driver instead. > > v2: Split configs to separate patch, enable nouveau (Bartlomiej) > > Cc: Antonino Daplas > Cc: Bartlomiej Zolnierkiewicz > Cc: linux-fb...@vger.kernel.org > Cc: dri-devel@lists.freedesktop.org > Cc: Michael Ellerman > Cc: Benjamin Herrenschmidt > Cc: Paul Mackerras > Cc: linuxppc-...@lists.ozlabs.org > Signed-off-by: Emil Velikov > Acked-by: Daniel Vetter (v1) > --- > Hi all unless, there are objections I would prefer to merge this via > the drm tree. Have you tested that the resulting kernels work on the relevant hardware? The old drivers may be crufty but they presumably have been tested by people and at least somewhat work. So I'd be inclined to leave the defconfigs alone until someone can test that the new driver works at all. I gave it a quick spin on a G5 I have access to and dmesg has a bunch of errors in it (see below). I can't actually tell if the display is working because the machine is remote, and I can't go and check it at the moment because the office is closed. cheers [ 128.563251] nouveau :f0:10.0: NVIDIA NV34 (034900b1) [ 128.565720] nouveau :f0:10.0: bios: version 04.34.20.18.00 [ 128.570994] nouveau :f0:10.0: bios: OOB 1 0cdd 0cdd [ 128.571702] nouveau :f0:10.0: bios: OOB 1 0ce1 0ce1 [ 128.571723] nouveau :f0:10.0: bios: OOB 1 0cde 0cde [ 128.571731] nouveau :f0:10.0: bios: OOB 1 0cdf 0cdf [ 128.572438] nouveau :f0:10.0: bios: OOB 1 0ce5 0ce5 [ 128.572459] nouveau :f0:10.0: bios: OOB 1 0ce2 0ce2 [ 128.572468] nouveau :f0:10.0: bios: OOB 1 0ce3 0ce3 [ 128.579187] nouveau :f0:10.0: bios: OOB 1 0ce9 0ce9 [ 128.579210] nouveau :f0:10.0: bios: OOB 1 0ce6 0ce6 [ 128.579219] nouveau :f0:10.0: bios: OOB 1 0ce7 0ce7 [ 128.580230] nouveau :f0:10.0: bios: OOB 1 0ced 0ced [ 128.580252] nouveau :f0:10.0: bios: OOB 1 0cea 0cea [ 128.580261] nouveau :f0:10.0: bios: OOB 1 0ceb 0ceb [ 128.581543] nouveau :f0:10.0: bios: OOB 1 0cf1 0cf1 [ 128.581567] nouveau :f0:10.0: bios: OOB 1 0cee 0cee [ 128.581576] nouveau :f0:10.0: bios: OOB 1 0cef 0cef [ 128.582456] nouveau :f0:10.0: bios: OOB 1 0cf5 0cf5 [ 128.582479] nouveau :f0:10.0: bios: OOB 1 0cf2 0cf2 [ 128.582487] nouveau :f0:10.0: bios: OOB 1 0cf3 0cf3 [ 128.583314] nouveau :f0:10.0: bios: OOB 1 0cf9 0cf9 [ 128.583336] nouveau :f0:10.0: bios: OOB 1 0cf6 0cf6 [ 128.583344] nouveau :f0:10.0: bios: OOB 1 0cf7 0cf7 [ 128.584186] nouveau :f0:10.0: bios: OOB 1 0cfd 0cfd [ 128.584208] nouveau :f0:10.0: bios: OOB 1 0cfa 0cfa [ 128.584217] nouveau :f0:10.0: bios: OOB 1 0cfb 0cfb [ 128.585635] nouveau :f0:10.0: bios: OOB 1 0d01 0d01 [ 128.585658] nouveau :f0:10.0: bios: OOB 1 0cfe 0cfe [ 128.585666] nouveau :f0:10.0: bios: OOB 1 0cff 0cff [ 128.589405] nouveau :f0:10.0: bios: OOB 1 0d05 0d05 [ 128.589428] nouveau :f0:10.0: bios: OOB 1 0d02 0d02 [ 128.589436] nouveau :f0:10.0: bios: OOB 1 0d03 0d03 [ 128.590316] nouveau :f0:10.0: bios: OOB 1 0d09 0d09 [ 128.590338] nouveau :f0:10.0: bios: OOB 1 0d06 0d06 [ 128.590346] nouveau :f0:10.0: bios: OOB 1 0d07 0d07 [ 128.591173] nouveau :f0:10.0: bios: OOB 1 0d0d 0d0d [ 128.591196] nouveau :f0:10.0: bios: OOB 1 0d0a 0d0a [ 128.591205] nouveau :f0:10.0: bios: OOB 1 0d0b 0d0b [ 128.593513] nouveau :f0:10.0: tmr: unknown input clock freq [ 128.595886] nouveau :f0:10.0: fb: 64 MiB DDR1 [ 128.609034] [TTM] Zone kernel: Available graphics memory: 500010 KiB [ 128.609058] [TTM] Initializing pool allocator [ 128.609106] nouveau :f0:10.0: DRM: VRAM: 63 MiB [ 128.609115] nouveau :f0:10.0: DRM: GART: 128 MiB [ 128.609134] nouveau :f0:10.0: DRM: BMP version 5.38 [ 128.609144] nouveau :f0:10.0: DRM: DCB version 2.2 [ 128.609155] nouveau :f0:10.0: DRM: DCB outp 00: 01000122 0004 [ 128.609168] nouveau :f0:10.0: DRM: DCB outp 01: 02010200 11b088b8 [ 128.609177] nouveau :f0:10.0: DRM: DCB outp 02: 02010201 11b00703 [ 128.609222] nouveau :f0:10.0: DRM: Loading NV17 power sequencing microcode [ 128.610947] nouveau :f0:10.0: DRM: MM: using M2MF for buffer copies [ 128.613371] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013). [ 128.615065] nouveau :f0:10.0: DRM: Setting dpms mode 3 on TV encoder (output 2) [ 128.707050] nouveau :f0:10.0: TV-1: EDID is invalid: [ 128.707075] [00] ZERO 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Re: [PATCH 02/12] dt-bindings: display: Add ingenic,ipu.yaml
Hi Sam,
Le dim. 17 mai 2020 à 8:17, Sam Ravnborg a écrit :
Hi Paul.
On Sat, May 16, 2020 at 11:50:47PM +0200, Paul Cercueil wrote:
Add documentation of the Device Tree bindings for the Image
Processing
Unit (IPU) found in most Ingenic SoCs.
Signed-off-by: Paul Cercueil
For me it fails like this:
Oops. I missed the 'const:' in the item list. Will fix when I send a
V2, and verify it this time.
Cheers,
-Paul
/Documentation/devicetree/bindings/display/ingenic,ipu.yaml:
ignoring, error in schema: properties: compatible: oneOf: 1: items
warning: no schema found in file:
Documentation/devicetree/bindings/display/ingenic,ipu.yaml
make[2]: *** [Documentation/devicetree/bindings/Makefile:42:
Documentation/devicetree/bindings/processed-schema.yaml] Error 255
make[2]: *** Waiting for unfinished jobs
Documentation/devicetree/bindings/display/ingenic,ipu.yaml:
properties:compatible:oneOf:1:items: ['ingenic,jz4770-ipu',
'ingenic,jz4760-ipu'] is not valid under any of the given schemas
(Possible causes of the failure):
Documentation/devicetree/bindings/display/ingenic,ipu.yaml:
properties:compatible:oneOf:1:items: ['ingenic,jz4770-ipu',
'ingenic,jz4760-ipu'] is not of type 'object', 'boolean'
Documentation/devicetree/bindings/display/ingenic,ipu.yaml:
properties:compatible:oneOf:1:items:0: 'ingenic,jz4770-ipu' is not of
type 'object', 'boolean'
Documentation/devicetree/bindings/display/ingenic,ipu.yaml:
properties:compatible:oneOf:1:items:1: 'ingenic,jz4760-ipu' is not of
type 'object', 'boolean'
Sam
---
.../bindings/display/ingenic,ipu.yaml | 65
+++
1 file changed, 65 insertions(+)
create mode 100644
Documentation/devicetree/bindings/display/ingenic,ipu.yaml
diff --git
a/Documentation/devicetree/bindings/display/ingenic,ipu.yaml
b/Documentation/devicetree/bindings/display/ingenic,ipu.yaml
new file mode 100644
index ..22fe02ca866d
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/ingenic,ipu.yaml
@@ -0,0 +1,65 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/ingenic,ipu.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Ingenic SoCs Image Processing Unit (IPU) devicetree bindings
+
+maintainers:
+ - Paul Cercueil
+
+properties:
+ compatible:
+oneOf:
+ - enum:
+- ingenic,jz4725b-ipu
+- ingenic,jz4760-ipu
+ - items:
+- ingenic,jz4770-ipu
+- ingenic,jz4760-ipu
+
+ reg:
+maxItems: 1
+
+ interrupts:
+maxItems: 1
+
+ clocks:
+maxItems: 1
+
+ clock-names:
+const: ipu
+
+patternProperties:
+ "^ports?$":
+description: OF graph bindings (specified in
bindings/graph.txt).
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - clocks
+ - clock-names
+
+additionalProperties: false
+
+examples:
+ - |
+#include
+ipu@1308 {
+ compatible = "ingenic,jz4770-ipu", "ingenic,jz4760-ipu";
+ reg = <0x1308 0x800>;
+
+ interrupt-parent = <&intc>;
+ interrupts = <29>;
+
+ clocks = <&cgu JZ4770_CLK_IPU>;
+ clock-names = "ipu";
+
+ port {
+ipu_ep: endpoint {
+ remote-endpoint = <&lcdc_ep>;
+};
+ };
+};
--
2.26.2
___
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 v10 0/2] Panel rotation patches
12.05.2020 23:59, Sean Paul пишет: > On Thu, Apr 16, 2020 at 7:03 PM Dmitry Osipenko wrote: >> >> 15.04.2020 00:32, dbasehore . пишет: >>> On Tue, Apr 14, 2020 at 2:18 PM Dmitry Osipenko wrote: 14.04.2020 22:32, dbasehore . пишет: > Hi Dmitry, sorry for the late reply. > > On Sun, Mar 8, 2020 at 12:25 PM Dmitry Osipenko wrote: >> >> 06.03.2020 03:21, Derek Basehore пишет: >>> This adds the plumbing for reading panel rotation from the devicetree >>> and sets up adding a panel property for the panel orientation on >>> Mediatek SoCs when a rotation is present. >> >> Hello Derek and everyone, >> >> I'm looking at adding display rotation support to NVIDIA Tegra DRM >> driver because some devices have display panel physically mounted >> upside-down, and thus, display controller's scan-out needs to be rotated >> by 180° in this case. >> >> Derek, yours panel-rotation patches add support for assigning panel's >> orientation to the connector, but then only primary display plane >> receives rotation value in [1], while rotation needs to be applied to >> all available overlay/cursor planes and this should happen in other >> places than [1] as well. > > This is intended. We don't correct the output in the kernel. We > instead rely on notifying userspace that the panel is rotated, then we > handle it there. > >> >> [1] drm_client_modeset_commit_atomic() >> >> Please also note that in a case of the scan-out rotation, plane's >> coordinates need to be changed in accordance to the display's rotation. >> >> I looked briefly through the DRM code and my understanding that the DRM >> core currently doesn't support use-case where scan-out needs to rotated >> based on a panel's orientation, correct? Is it the use-case you're >> working on for the Mediatek driver? > > Yes, we rely on userspace to rotate the output. The major reason for > this is because there may not be a "free" hardware rotation that can > be applied to the overlay. Sean Paul and others also preferred that > userspace control what is output to the screen instead of the kernel > taking care of it. This code just adds the drm property to the panel. > Could you please explain what that userspace is? >>> >>> This was added for Chrome OS, which uses its own graphics stack, >>> Ozone, instead of Xorg. >>> >> >> Thank you very much for the clarification. >> >> It's probably not a big problem for something monolithic and customized >> like ChromeOS to issue a software update in order to take into account >> all specifics of a particular device, but this doesn't work nicely for a >> generic software, like a usual Linux distro. >> AFAIK, things like Xorg modesetting don't support that orientation property. >> >> In my case it's not only the display panel which is upside-down, but >> also the touchscreen. Hence both display output and touchscreen input >> need to be rotated at once, otherwise you'll end up with either display >> or input being upside-down. >> >> The 180° rotation should be free on NVIDIA Tegra. There are no known >> limitations for the planes and BSP kernel video driver handles the >> plane's coordinates/framebuffer rotation within the driver. >> >> The kernel's input subsystem allows us to transparently (for userspace) >> remap the touchscreen input (by specifying generic touchscreen >> device-tree properties), while this is not the case for the DRM subsystem. >> >> @Thierry, @Sean, @Daniel, could you please help me to understand how a >> coordinated display / input rotation could be implemented, making the >> rotation transparent to the user (i.e. avoiding xorg.conf hacking and >> etc)? It should be nice if display's output could be flipped within the >> DRM driver, hiding this fact from userspace. > > I think the right thing to do is to fix userspace to respect this > property, since that has the most communal benefit. Hello Sean, This will be ideal, but it's difficult to achieve in a loosely controlled userspace environment. > However(!!) if you don't want to do that, how about inspecting the > info->panel_orientation value after drm_panel_attach in tegra driver > and then adjusting rotation values in the driver. Of course, you > wouldn't want to expose the panel orientation property since you don't > want userspaces to be double-rotating on you, but it's optional so > you'd be fine. Thank you very much for the suggestion, I'll be trying it out soon. >> >> Will it be okay if we'll add a transparent-rotation support specifically >> to the Tegra DRM driver? For example if device-tree contains >> nvidia,display-flip-y property, then the Tegra DRM driver will take care >> of rotating coordinates/framebuffer of the display planes. > > I don't think this is necessary, but it also wouldn't really be > appropriate to put software attributes into devi
