Re: [PATCH v4 12/23] drm: zte: Provide ddc symlink in hdmi connector sysfs directory

2019-07-22 Thread Shawn Guo
On Thu, Jul 11, 2019 at 01:26:39PM +0200, Andrzej Pietrasiewicz wrote:
> Use the ddc pointer provided by the generic connector.
> 
> Signed-off-by: Andrzej Pietrasiewicz 

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

[Bug 110749] [Vega 11] [amdgpu retry page fault VM_L2_PROTECTION_FAULT_STATUS] System lock up during playing Steam version of Saints Row 3

2019-07-22 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=110749

--- Comment #5 from weden...@yandex.ru ---
Seems like another case of https://bugs.freedesktop.org/show_bug.cgi?id=105251

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

[Bug 105718] amdgpu reported fan speed looks too high (dual fan Sapphire Pulse Vega 56)

2019-07-22 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=105718

--- Comment #3 from weden...@yandex.ru ---
I have exactly the same issue with Sapphire Pulse Vega 56. It also reports some
unreasonably high value (something around 3000rpm) as max fans RPM.

I've found this thread
https://www.hwinfo.com/forum/Thread-Solved-SAPPHIRE-PULSE-Radeon%E2%84%A2-RX-Vega56-8G-HBM2-Wrong-FAN-RPM.
Apparently this issue was solved in windows with AMD driver update.

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

Re: [PATCH 1/3] mm/gup: introduce __put_user_pages()

2019-07-22 Thread Christoph Hellwig
On Mon, Jul 22, 2019 at 03:34:13PM -0700, john.hubb...@gmail.com wrote:
> +enum pup_flags_t {
> + PUP_FLAGS_CLEAN = 0,
> + PUP_FLAGS_DIRTY = 1,
> + PUP_FLAGS_LOCK  = 2,
> + PUP_FLAGS_DIRTY_LOCK= 3,
> +};

Well, the enum defeats the ease of just being able to pass a boolean
expression to the function, which would simplify a lot of the caller,
so if we need to support the !locked version I'd rather see that as
a separate helper.

But do we actually have callers where not using the _lock version is
not a bug?  set_page_dirty makes sense in the context of a file systems
that have a reference to the inode the page hangs off, but that is
(almost?) never the case for get_user_pages.


[PATCH] drm/mediatek: make imported PRIME buffers contiguous

2019-07-22 Thread Alexandre Courbot
This driver requires imported PRIME buffers to appear contiguously in
its IO address space. Make sure this is the case by setting the maximum
DMA segment size to a better value than the default 64K on the DMA
device, and use said DMA device when importing PRIME buffers.

Signed-off-by: Alexandre Courbot 
---
 drivers/gpu/drm/mediatek/mtk_drm_drv.c | 47 --
 drivers/gpu/drm/mediatek/mtk_drm_drv.h |  2 ++
 2 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c 
b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index 95fdbd0fbcac..4ad4770fab13 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -213,6 +213,7 @@ static int mtk_drm_kms_init(struct drm_device *drm)
struct mtk_drm_private *private = drm->dev_private;
struct platform_device *pdev;
struct device_node *np;
+   struct device *dma_dev;
int ret;
 
if (!iommu_present(_bus_type))
@@ -275,7 +276,27 @@ static int mtk_drm_kms_init(struct drm_device *drm)
goto err_component_unbind;
}
 
-   private->dma_dev = >dev;
+   dma_dev = >dev;
+   private->dma_dev = dma_dev;
+
+   /*
+* Configure the DMA segment size to make sure we get contiguous IOVA
+* when importing PRIME buffers.
+*/
+   if (!dma_dev->dma_parms) {
+   private->dma_parms_allocated = true;
+   dma_dev->dma_parms =
+   devm_kzalloc(drm->dev, sizeof(*dma_dev->dma_parms),
+GFP_KERNEL);
+   }
+   if (!dma_dev->dma_parms)
+   goto err_component_unbind;
+
+   ret = dma_set_max_seg_size(dma_dev, (unsigned int)DMA_BIT_MASK(32));
+   if (ret) {
+   dev_err(dma_dev, "Failed to set DMA segment size\n");
+   goto err_unset_dma_parms;
+   }
 
/*
 * We don't use the drm_irq_install() helpers provided by the DRM
@@ -285,13 +306,16 @@ static int mtk_drm_kms_init(struct drm_device *drm)
drm->irq_enabled = true;
ret = drm_vblank_init(drm, MAX_CRTC);
if (ret < 0)
-   goto err_component_unbind;
+   goto err_unset_dma_parms;
 
drm_kms_helper_poll_init(drm);
drm_mode_config_reset(drm);
 
return 0;
 
+err_unset_dma_parms:
+   if (private->dma_parms_allocated)
+   dma_dev->dma_parms = NULL;
 err_component_unbind:
component_unbind_all(drm->dev, drm);
 err_config_cleanup:
@@ -302,9 +326,14 @@ static int mtk_drm_kms_init(struct drm_device *drm)
 
 static void mtk_drm_kms_deinit(struct drm_device *drm)
 {
+   struct mtk_drm_private *private = drm->dev_private;
+
drm_kms_helper_poll_fini(drm);
drm_atomic_helper_shutdown(drm);
 
+   if (private->dma_parms_allocated)
+   private->dma_dev->dma_parms = NULL;
+
component_unbind_all(drm->dev, drm);
drm_mode_config_cleanup(drm);
 }
@@ -320,6 +349,18 @@ static const struct file_operations mtk_drm_fops = {
.compat_ioctl = drm_compat_ioctl,
 };
 
+/*
+ * We need to override this because the device used to import the memory is
+ * not dev->dev, as drm_gem_prime_import() expects.
+ */
+struct drm_gem_object *mtk_drm_gem_prime_import(struct drm_device *dev,
+   struct dma_buf *dma_buf)
+{
+   struct mtk_drm_private *private = dev->dev_private;
+
+   return drm_gem_prime_import_dev(dev, dma_buf, private->dma_dev);
+}
+
 static struct drm_driver mtk_drm_driver = {
.driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME |
   DRIVER_ATOMIC,
@@ -331,7 +372,7 @@ static struct drm_driver mtk_drm_driver = {
.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
.gem_prime_export = drm_gem_prime_export,
-   .gem_prime_import = drm_gem_prime_import,
+   .gem_prime_import = mtk_drm_gem_prime_import,
.gem_prime_get_sg_table = mtk_gem_prime_get_sg_table,
.gem_prime_import_sg_table = mtk_gem_prime_import_sg_table,
.gem_prime_mmap = mtk_drm_gem_mmap_buf,
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h 
b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
index 598ff3e70446..e03fea12ff59 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
@@ -51,6 +51,8 @@ struct mtk_drm_private {
} commit;
 
struct drm_atomic_state *suspend_state;
+
+   bool dma_parms_allocated;
 };
 
 extern struct platform_driver mtk_ddp_driver;
-- 
2.22.0.657.g960e92d24f-goog



[PATCH] drm/syncobj: extend syncobj query ability

2019-07-22 Thread Chunming Zhou
user space needs a flexiable query ability.
So that umd can get last signaled or submitted point.

Change-Id: I6512b430524ebabe715e602a2bf5abb0a7e780ea
Signed-off-by: Chunming Zhou 
Cc: Lionel Landwerlin 
Cc: Christian König 
---
 drivers/gpu/drm/drm_syncobj.c | 36 +--
 include/uapi/drm/drm.h|  3 ++-
 2 files changed, 20 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index 3d400905100b..f70dedf3ef4f 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -1197,9 +1197,6 @@ drm_syncobj_timeline_signal_ioctl(struct drm_device *dev, 
void *data,
if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
return -EOPNOTSUPP;
 
-   if (args->pad != 0)
-   return -EINVAL;
-
if (args->count_handles == 0)
return -EINVAL;
 
@@ -1268,9 +1265,6 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, void 
*data,
if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
return -EOPNOTSUPP;
 
-   if (args->pad != 0)
-   return -EINVAL;
-
if (args->count_handles == 0)
return -EINVAL;
 
@@ -1291,23 +1285,29 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, 
void *data,
if (chain) {
struct dma_fence *iter, *last_signaled = NULL;
 
-   dma_fence_chain_for_each(iter, fence) {
-   if (!iter)
-   break;
-   dma_fence_put(last_signaled);
-   last_signaled = dma_fence_get(iter);
-   if 
(!to_dma_fence_chain(last_signaled)->prev_seqno)
-   /* It is most likely that timeline has
-* unorder points. */
-   break;
+   if (args->flags &
+   DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED) {
+   point = fence->seqno;
+   } else {
+   dma_fence_chain_for_each(iter, fence) {
+   if (!iter)
+   break;
+   dma_fence_put(last_signaled);
+   last_signaled = dma_fence_get(iter);
+   if 
(!to_dma_fence_chain(last_signaled)->prev_seqno)
+   /* It is most likely that 
timeline has
+   * unorder points. */
+   break;
+   }
+   point = dma_fence_is_signaled(last_signaled) ?
+   last_signaled->seqno :
+   
to_dma_fence_chain(last_signaled)->prev_seqno;
}
-   point = dma_fence_is_signaled(last_signaled) ?
-   last_signaled->seqno :
-   to_dma_fence_chain(last_signaled)->prev_seqno;
dma_fence_put(last_signaled);
} else {
point = 0;
}
+   dma_fence_put(fence);
ret = copy_to_user([i], , sizeof(uint64_t));
ret = ret ? -EFAULT : 0;
if (ret)
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index 661d73f9a919..fd987ce24d9f 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -777,11 +777,12 @@ struct drm_syncobj_array {
__u32 pad;
 };
 
+#define DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED (1 << 0) /* last available 
point on timeline syncobj */
 struct drm_syncobj_timeline_array {
__u64 handles;
__u64 points;
__u32 count_handles;
-   __u32 pad;
+   __u32 flags;
 };
 
 
-- 
2.17.1

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

Re: [PATCH v6 4/5] dma-buf: heaps: Add CMA heap to dmabuf heaps

2019-07-22 Thread John Stultz
On Thu, Jul 18, 2019 at 3:08 AM Christoph Hellwig  wrote:
>
> This and the previous one seem very much duplicated boilerplate
> code.

So yes, there is some duplicate boilerplate between the system and cma
heaps particularly in the allocation function, where we allocate and
set up the helper buffer structure, allocate the pages, then create
and export the dmabuf. I took a pass at trying to minimize some of
this earlier, but those efforts ended up adding helper functions that
take a ton of arguments, and had some trouble properly handling error
paths without leaking memory, so I left it as is.

I'll try to take another pass myself but if you have specific
suggestions for improving here, I'd be happy to try them.

> Why can't just normal branches for allocating and freeing
> normal pages vs cma?  We even have an existing helper for that
> with dma_alloc_contiguous().

Apologies, I'm not sure I'm understanding your suggestion here.
dma_alloc_contiguous() does have some interesting optimizations
(avoiding allocating single page from cma), though its focus on
default area vs specific device area doesn't quite match up the usage
model for dma heaps.  Instead of allocating memory for a single
device, we want to be able to allow userland, for a given usage mode,
to be able to allocate a dmabuf from a specific heap of memory which
will satisfy the usage mode for that buffer pipeline (across multiple
devices).

Now, indeed, the system and cma heaps in this patchset are a bit
simple/trivial (though useful with my devices that require contiguous
buffers for the display driver), but more complex ION heaps have
traditionally stayed out of tree in vendor code, in many cases making
incompatible tweaks to the ION core dmabuf exporter logic. That's why
dmabuf heaps is trying to destage ION in a way that allows heaps to
implement their exporter logic themselves, so we can start pulling
those more complicated heaps out of their vendor hidey-holes and get
some proper upstream review.

But I suspect I just am confused as to what your suggesting. Maybe
could you expand a bit? Apologies for being a bit dense.

Thanks again for the input!
-john
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/3] net/xdp: convert put_page() to put_user_page*()

2019-07-22 Thread John Hubbard

On 7/22/19 5:25 PM, Ira Weiny wrote:

On Mon, Jul 22, 2019 at 03:34:15PM -0700, john.hubb...@gmail.com wrote:

From: John Hubbard 

For pages that were retained via get_user_pages*(), release those pages
via the new put_user_page*() routines, instead of via put_page() or
release_pages().

This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
("mm: introduce put_user_page*(), placeholder versions").

Cc: Björn Töpel 
Cc: Magnus Karlsson 
Cc: David S. Miller 
Cc: net...@vger.kernel.org
Signed-off-by: John Hubbard 
---
  net/xdp/xdp_umem.c | 9 +
  1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/net/xdp/xdp_umem.c b/net/xdp/xdp_umem.c
index 83de74ca729a..0325a17915de 100644
--- a/net/xdp/xdp_umem.c
+++ b/net/xdp/xdp_umem.c
@@ -166,14 +166,7 @@ void xdp_umem_clear_dev(struct xdp_umem *umem)
  
  static void xdp_umem_unpin_pages(struct xdp_umem *umem)

  {
-   unsigned int i;
-
-   for (i = 0; i < umem->npgs; i++) {
-   struct page *page = umem->pgs[i];
-
-   set_page_dirty_lock(page);
-   put_page(page);
-   }
+   put_user_pages_dirty_lock(umem->pgs, umem->npgs);


What is the difference between this and

__put_user_pages(umem->pgs, umem->npgs, PUP_FLAGS_DIRTY_LOCK);

?


No difference.



I'm a bit concerned with adding another form of the same interface.  We should
either have 1 call with flags (enum in this case) or multiple calls.  Given the
previous discussion lets move in the direction of having the enum but don't
introduce another caller of the "old" interface.


I disagree that this is a "problem". There is no maintenance pitfall here; there
are merely two ways to call the put_user_page*() API. Both are correct, and
neither one will get you into trouble.

Not only that, but there is ample precedent for this approach in other
kernel APIs.



So I think on this patch NAK from me.

I also don't like having a __* call in the exported interface but there is a
__get_user_pages_fast() call so I guess there is precedent.  :-/



I thought about this carefully, and looked at other APIs. And I noticed that
things like __get_user_pages*() are how it's often done:

* The leading underscores are often used for the more elaborate form of the
call (as oppposed to decorating the core function name with "_flags", for
example).

* There are often calls in which you can either call the simpler form, or the
form with flags and additional options, and yes, you'll get the same result.

Obviously, this stuff is all subject to a certain amount of opinion, but I
think I'm on really solid ground as far as precedent goes. So I'm pushing
back on the NAK... :)

thanks,
--
John Hubbard
NVIDIA



Re: [PATCH v6 2/5] dma-buf: heaps: Add heap helpers

2019-07-22 Thread John Stultz
On Thu, Jul 18, 2019 at 3:06 AM Christoph Hellwig  wrote:
>
> > +void INIT_HEAP_HELPER_BUFFER(struct heap_helper_buffer *buffer,
> > +  void (*free)(struct heap_helper_buffer *))
>
> Please use a lower case naming following the naming scheme for the
> rest of the file.

Yes! Apologies as this was a hold-over from when the initialization
function was an inline function in the style of
INIT_WORK/INIT_LIST_HEAD. No longer appropriate that its a function.
I'll change it.

> > +static void *dma_heap_map_kernel(struct heap_helper_buffer *buffer)
> > +{
> > + void *vaddr;
> > +
> > + vaddr = vmap(buffer->pages, buffer->pagecount, VM_MAP, PAGE_KERNEL);
> > + if (!vaddr)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + return vaddr;
> > +}
>
> Unless I'm misreading the patches this is used for the same pages that
> also might be dma mapped.  In this case you need to use
> flush_kernel_vmap_range and invalidate_kernel_vmap_range in the right
> places to ensure coherency between the vmap and device view.  Please
> also document the buffer ownership, as this really can get complicated.

Forgive me I wasn't familiar with those calls, but this seems like it
would apply to all dma-buf exporters if so, and I don't see any
similar flush_kernel_vmap_range calls there (both functions are
seemingly only used by xfs, md and bio).

We do have the 
dma_heap_dma_buf_begin_cpu_access()/dma_heap_dma_buf_end_cpu_access()
hooks (see more on these below) which sync the buffers for each
attachment (via dma_sync_sg_for_cpu/device), and are used around cpu
access to the buffers. Are you suggesting the
flush_kernel_vmap_range() call be added to those hooks or is the
dma_sync_sg_for_* calls sufficient there?

> > +static vm_fault_t dma_heap_vm_fault(struct vm_fault *vmf)
> > +{
> > + struct vm_area_struct *vma = vmf->vma;
> > + struct heap_helper_buffer *buffer = vma->vm_private_data;
> > +
> > + vmf->page = buffer->pages[vmf->pgoff];
> > + get_page(vmf->page);
> > +
> > + return 0;
> > +}
>
> Is there any exlusion between mmap / vmap and the device accessing
> the data?  Without that you are going to run into a lot of coherency
> problems.

This has actually been a concern of mine recently, but at the higher
dma-buf core level.  Conceptually, there is the
dma_buf_map_attachment() and dma_buf_unmap_attachment() calls drivers
use to map the buffer to an attached device, and there are the
dma_buf_begin_cpu_access() and dma_buf_end_cpu_access() calls which
are to be done when touching the cpu mapped pages.  These look like
serializing functions, but actually don't seem to have any enforcement
mechanism to exclude parallel access.

To me it seems like adding the exclusion between those operations
should be done at the dmabuf core level, and would actually be helpful
for optimizing some of the cache maintenance rules w/ dmabuf.  Does
this sound like something closer to what your suggesting, or am I
misunderstanding your point?

Again, I really appreciate the review and feedback!

Thanks so much!
-john
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

linux-next: manual merge of the drm-intel tree with the kspp-gustavo tree

2019-07-22 Thread Stephen Rothwell
Hi all,

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

  drivers/gpu/drm/i915/display/intel_dp.c

between commit:

  b6ac32eac063 ("drm/i915: Mark expected switch fall-throughs")

from the kspp-gustavo tree and commit:

  bc85328ff431 ("drm/i915: Move the TypeC port handling code to a separate 
file")
  4f36afb26cbe ("drm/i915: Sanitize the TypeC FIA lane configuration decoding")

from the drm-intel tree.

I fixed it up (bc85328ff431 moved the function updated by b6ac32eac063
and 4f36afb26cbe added an equivalt fixup) 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


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

Re: [PATCH v9 04/18] kunit: test: add kunit_stream a std::stream like logger

2019-07-22 Thread Brendan Higgins
On Mon, Jul 22, 2019 at 4:54 PM Stephen Boyd  wrote:
>
> Quoting Brendan Higgins (2019-07-22 15:30:49)
> > On Mon, Jul 22, 2019 at 1:03 PM Stephen Boyd  wrote:
> > >
> > >
> > > What's the calling context of the assertions and expectations? I still
> > > don't like the fact that string stream needs to allocate buffers and
> > > throw them into a list somewhere because the calling context matters
> > > there.
> >
> > The calling context is the same as before, which is anywhere.
>
> Ok. That's concerning then.

Yeah. Luis suggested just not supporting the IRQ context until later.
See my later comment.

> > > I'd prefer we just wrote directly to the console/log via printk
> > > instead. That way things are simple because we use the existing
> > > buffering path of printk, but maybe there's some benefit to the string
> > > stream that I don't see? Right now it looks like it builds a string and
> > > then dumps it to printk so I'm sort of lost what the benefit is over
> > > just writing directly with printk.
> >
> > It's just buffering it so the whole string gets printed uninterrupted.
> > If we were to print out piecemeal to printk, couldn't we have another
> > call to printk come in causing it to garble the KUnit message we are
> > in the middle of printing?
>
> Yes, printing piecemeal by calling printk many times could lead to
> interleaving of messages if something else comes in such as an interrupt
> printing something. Printk has some support to hold "records" but I'm
> not sure how that would work here because KERN_CONT talks about only
> being used early on in boot code. I haven't looked at printk in detail
> though so maybe I'm all wrong and KERN_CONT just works?

It says KERN_CONT is not SMP safe, and it isn't supposed to contain
newlines, so it doesn't sound like it does any buffering for you. I
looked at it a while ago and those comments agreed with my
understanding of the code, but I could be wrong.

> Can printk be called once with whatever is in the struct?

Unfortunately, no. That is part of what I was trying to illustrate
with this patch. Most of the messages are deterministic, but
hardcoding all the possible message types would lead to a massive
number of hard coded strings. However, even this would break down for
the mocking formatters. All the different ways a function can be
called are just too complex to encode into a finite set of hard coded
fmt strings.

> Otherwise if
> this is about making printk into a structured log then maybe printk
> isn't the proper solution anyway. Maybe a dev interface should be used
> instead that can handle starting and stopping tests (via ioctl) in
> addition to reading test results, records, etc. with read() and a
> clearing of the records. Then the seqfile API works naturally. All of

Ehhh...I wouldn't mind providing such an interface, but I would really
like to be able to provide the results without having to depend on a
userland doing something to get test results. That has always been a
pretty important goal for me.

> this is a bit premature, but it looks like you're going down the path of
> making something akin to ftrace that stores binary formatted
> assertion/expectation records in a lockless ring buffer that then
> formats those records when the user asks for them.

Like you said, I think it is a bit premature to go that far.

In anycase, I don't see a way to get rid of string_stream, without
significantly sacrificing usability.

> I can imagine someone wanting to write unit tests that check conditions
> from a simulated hardirq context via irq works (a driver mock
> framework?), so this doesn't seem far off.

Yep, I actually presented the first pieces of that in the RFC v1 that
I linked to you earlier in this discussion. I have a more fleshed out
example here:

https://kunit.googlesource.com/linux/+/e10484ad2f9fc7926412ec84739fe105981b4771/drivers/i2c/busses/i2c-aspeed-test.c

I actually already have some people at Google playing around with it.
So yeah, not far off at all! However, in these cases we are not
actually running in the IRQ context (despite the fact that we are
testing IRQ code) because we provide a fake IRQ chip, or some other
fake mechanism that triggers the IRQ. Still, I could see someone
wanting to do it in a non-fake-IRQ context.

Luis' suggestion was just to hold off on the IRQ safe stuff at the
outset, since that is going to require a lot more effort to review. I
know that's kind of the future coding argument again, but maybe the
answer will be to just restrict what features an IRQ user has access
to (maybe just really simple expectations, for example). I mean, we
will probably have to restrict what they are allowed to use anyway.

Luis, do you have any ideas?

Cheers


Re: [PATCH v2] drm/tegra: sor: Enable HDA interrupts at plugin

2019-07-22 Thread Dmitry Osipenko
22.07.2019 12:27, Viswanath L пишет:
> HDMI plugout calls runtime suspend, which clears interrupt registers
> and causes audio functionality to break on subsequent plugin; setting
> interrupt registers in sor_audio_prepare() solves the issue

Hello Viswanath,

A dot should be in the end of sentence, please. And should be better to
s/plugin/plug-in/ in the commit's message/title because 'plugin' sounds
as a noun.

Please don't version patch as v2 if v1 wasn't ever sent out.

You should add a stable tag here to get patch backported into stable
kernel versions:

Cc: 

> Signed-off-by: Viswanath L 

The kernel upstreaming rules require a full name. I'm pretty sure that L
isn't yours surname.

> ---
>  drivers/gpu/drm/tegra/sor.c | 18 +-
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c
> index 5be5a08..0470cfe 100644
> --- a/drivers/gpu/drm/tegra/sor.c
> +++ b/drivers/gpu/drm/tegra/sor.c
> @@ -2164,6 +2164,15 @@ static void tegra_sor_audio_prepare(struct tegra_sor 
> *sor)
>  
>   value = SOR_AUDIO_HDA_PRESENSE_ELDV | SOR_AUDIO_HDA_PRESENSE_PD;
>   tegra_sor_writel(sor, value, SOR_AUDIO_HDA_PRESENSE);
> +
> + /*
> +  * Enable and unmask the HDA codec SCRATCH0 register interrupt. This
> +  * is used for interoperability between the HDA codec driver and the
> +  * HDMI/DP driver.
> +  */
> + value = SOR_INT_CODEC_SCRATCH1 | SOR_INT_CODEC_SCRATCH0;
> + tegra_sor_writel(sor, value, SOR_INT_ENABLE);
> + tegra_sor_writel(sor, value, SOR_INT_MASK);
>  }
>  
>  static void tegra_sor_audio_unprepare(struct tegra_sor *sor)
> @@ -2913,15 +2922,6 @@ static int tegra_sor_init(struct host1x_client *client)
>   if (err < 0)
>   return err;
>  
> - /*
> -  * Enable and unmask the HDA codec SCRATCH0 register interrupt. This
> -  * is used for interoperability between the HDA codec driver and the
> -  * HDMI/DP driver.
> -  */
> - value = SOR_INT_CODEC_SCRATCH1 | SOR_INT_CODEC_SCRATCH0;
> - tegra_sor_writel(sor, value, SOR_INT_ENABLE);
> - tegra_sor_writel(sor, value, SOR_INT_MASK);
> -
>   return 0;
>  }
>  
> 



Re: [PATCH 3/3] net/xdp: convert put_page() to put_user_page*()

2019-07-22 Thread Ira Weiny
On Mon, Jul 22, 2019 at 03:34:15PM -0700, john.hubb...@gmail.com wrote:
> From: John Hubbard 
> 
> For pages that were retained via get_user_pages*(), release those pages
> via the new put_user_page*() routines, instead of via put_page() or
> release_pages().
> 
> This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
> ("mm: introduce put_user_page*(), placeholder versions").
> 
> Cc: Björn Töpel 
> Cc: Magnus Karlsson 
> Cc: David S. Miller 
> Cc: net...@vger.kernel.org
> Signed-off-by: John Hubbard 
> ---
>  net/xdp/xdp_umem.c | 9 +
>  1 file changed, 1 insertion(+), 8 deletions(-)
> 
> diff --git a/net/xdp/xdp_umem.c b/net/xdp/xdp_umem.c
> index 83de74ca729a..0325a17915de 100644
> --- a/net/xdp/xdp_umem.c
> +++ b/net/xdp/xdp_umem.c
> @@ -166,14 +166,7 @@ void xdp_umem_clear_dev(struct xdp_umem *umem)
>  
>  static void xdp_umem_unpin_pages(struct xdp_umem *umem)
>  {
> - unsigned int i;
> -
> - for (i = 0; i < umem->npgs; i++) {
> - struct page *page = umem->pgs[i];
> -
> - set_page_dirty_lock(page);
> - put_page(page);
> - }
> + put_user_pages_dirty_lock(umem->pgs, umem->npgs);

What is the difference between this and

__put_user_pages(umem->pgs, umem->npgs, PUP_FLAGS_DIRTY_LOCK);

?

I'm a bit concerned with adding another form of the same interface.  We should
either have 1 call with flags (enum in this case) or multiple calls.  Given the
previous discussion lets move in the direction of having the enum but don't
introduce another caller of the "old" interface.

So I think on this patch NAK from me.

I also don't like having a __* call in the exported interface but there is a
__get_user_pages_fast() call so I guess there is precedent.  :-/

Ira

>  
>   kfree(umem->pgs);
>   umem->pgs = NULL;
> -- 
> 2.22.0
> 


Re: [Intel-gfx] [PATCH] drm/i915: Mark expected switch fall-throughs

2019-07-22 Thread Rodrigo Vivi
Hi Gustavo,

could you please rebase on top of drm-tip and resend it please?

Thanks,
Rodrigo.

On Mon, Jul 22, 2019 at 01:12:44PM -0500, Gustavo A. R. Silva wrote:
> In preparation to enabling -Wimplicit-fallthrough, mark switch
> cases where we are expecting to fall through.
> 
> This patch fixes the following warnings:
> 
> drivers/gpu/drm/i915/gem/i915_gem_mman.c: In function ‘i915_gem_fault’:
> drivers/gpu/drm/i915/gem/i915_gem_mman.c:342:6: warning: this statement may 
> fall through [-Wimplicit-fallthrough=]
>if (!i915_terminally_wedged(i915))
>   ^
> drivers/gpu/drm/i915/gem/i915_gem_mman.c:345:2: note: here
>   case -EAGAIN:
>   ^~~~
> 
> drivers/gpu/drm/i915/gem/i915_gem_pages.c: In function ‘i915_gem_object_map’:
> ./include/linux/compiler.h:78:22: warning: this statement may fall through 
> [-Wimplicit-fallthrough=]
>  # define unlikely(x) __builtin_expect(!!(x), 0)
>   ^~
> ./include/asm-generic/bug.h:136:2: note: in expansion of macro ‘unlikely’
>   unlikely(__ret_warn_on); \
>   ^~~~
> drivers/gpu/drm/i915/i915_utils.h:49:25: note: in expansion of macro ‘WARN’
>  #define MISSING_CASE(x) WARN(1, "Missing case (%s == %ld)\n", \
>  ^~~~
> drivers/gpu/drm/i915/gem/i915_gem_pages.c:270:3: note: in expansion of macro 
> ‘MISSING_CASE’
>MISSING_CASE(type);
>^~~~
> drivers/gpu/drm/i915/gem/i915_gem_pages.c:272:2: note: here
>   case I915_MAP_WB:
>   ^~~~
> 
> drivers/gpu/drm/i915/i915_gpu_error.c: In function 
> ‘error_record_engine_registers’:
> ./include/linux/compiler.h:78:22: warning: this statement may fall through 
> [-Wimplicit-fallthrough=]
>  # define unlikely(x) __builtin_expect(!!(x), 0)
>   ^~
> ./include/asm-generic/bug.h:136:2: note: in expansion of macro ‘unlikely’
>   unlikely(__ret_warn_on); \
>   ^~~~
> drivers/gpu/drm/i915/i915_utils.h:49:25: note: in expansion of macro ‘WARN’
>  #define MISSING_CASE(x) WARN(1, "Missing case (%s == %ld)\n", \
>  ^~~~
> drivers/gpu/drm/i915/i915_gpu_error.c:1196:5: note: in expansion of macro 
> ‘MISSING_CASE’
>  MISSING_CASE(engine->id);
>  ^~~~
> drivers/gpu/drm/i915/i915_gpu_error.c:1197:4: note: here
> case RCS0:
> ^~~~
> 
> drivers/gpu/drm/i915/display/intel_dp.c: In function 
> ‘intel_dp_get_fia_supported_lane_count’:
> ./include/linux/compiler.h:78:22: warning: this statement may fall through 
> [-Wimplicit-fallthrough=]
>  # define unlikely(x) __builtin_expect(!!(x), 0)
>   ^~
> ./include/asm-generic/bug.h:136:2: note: in expansion of macro ‘unlikely’
>   unlikely(__ret_warn_on); \
>   ^~~~
> drivers/gpu/drm/i915/i915_utils.h:49:25: note: in expansion of macro ‘WARN’
>  #define MISSING_CASE(x) WARN(1, "Missing case (%s == %ld)\n", \
>  ^~~~
> drivers/gpu/drm/i915/display/intel_dp.c:233:3: note: in expansion of macro 
> ‘MISSING_CASE’
>MISSING_CASE(lane_info);
>^~~~
> drivers/gpu/drm/i915/display/intel_dp.c:234:2: note: here
>   case 1:
>   ^~~~
> 
> drivers/gpu/drm/i915/display/intel_display.c: In function 
> ‘check_digital_port_conflicts’:
>   CC [M]  drivers/gpu/drm/nouveau/nvkm/engine/disp/cursgv100.o
> drivers/gpu/drm/i915/display/intel_display.c:12043:7: warning: this statement 
> may fall through [-Wimplicit-fallthrough=]
> if (WARN_ON(!HAS_DDI(to_i915(dev
>^
> drivers/gpu/drm/i915/display/intel_display.c:12046:3: note: here
>case INTEL_OUTPUT_DP:
>^~~~
> 
> Also, notice that the Makefile is modified in order to stop
> ignoring fall-through warnings. The -Wimplicit-fallthrough
> option will be enabled globally in v5.3.
> 
> Warning level 3 was used: -Wimplicit-fallthrough=3
> 
> This patch is part of the ongoing efforts to enable
> -Wimplicit-fallthrough.
> 
> Signed-off-by: Gustavo A. R. Silva 
> ---
>  drivers/gpu/drm/i915/Makefile| 1 -
>  drivers/gpu/drm/i915/display/intel_display.c | 2 +-
>  drivers/gpu/drm/i915/display/intel_dp.c  | 1 +
>  drivers/gpu/drm/i915/gem/i915_gem_mman.c | 2 +-
>  drivers/gpu/drm/i915/gem/i915_gem_pages.c| 2 +-
>  drivers/gpu/drm/i915/i915_gpu_error.c| 1 +
>  6 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 91355c2ea8a5..8cace65f50ce 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -16,7 +16,6 @@ subdir-ccflags-y := -Wall -Wextra
>  subdir-ccflags-y += $(call cc-disable-warning, unused-parameter)
>  subdir-ccflags-y += $(call cc-disable-warning, type-limits)
>  subdir-ccflags-y += $(call cc-disable-warning, missing-field-initializers)
> -subdir-ccflags-y += $(call cc-disable-warning, implicit-fallthrough)
>  subdir-ccflags-y += $(call cc-disable-warning, unused-but-set-variable)
>  # clang warnings
>  subdir-ccflags-y += $(call 

Re: [PATCH v3 0/4] backlight: Expose brightness curve type through sysfs

2019-07-22 Thread Matthias Kaehlcke
On Tue, Jul 09, 2019 at 12:00:03PM -0700, Matthias Kaehlcke wrote:
> Backlight brightness curves can have different shapes. The two main
> types are linear and non-linear curves. The human eye doesn't
> perceive linearly increasing/decreasing brightness as linear (see
> also 88ba95bedb79 "backlight: pwm_bl: Compute brightness of LED
> linearly to human eye"), hence many backlights use non-linear (often
> logarithmic) brightness curves. The type of curve is currently opaque
> to userspace, so userspace often relies on more or less reliable
> heuristics (like the number of brightness levels) to decide whether
> to treat a backlight device as linear or non-linear.
> 
> Export the type of the brightness curve via a new sysfs attribute.
> 
> Matthias Kaehlcke (4):
>   MAINTAINERS: Add entry for stable backlight sysfs ABI documentation
>   backlight: Expose brightness curve type through sysfs
>   backlight: pwm_bl: Set scale type for CIE 1931 curves
>   backlight: pwm_bl: Set scale type for brightness curves specified in
> the DT
> 
>  .../ABI/testing/sysfs-class-backlight | 26 ++
>  MAINTAINERS   |  2 ++
>  drivers/video/backlight/backlight.c   | 19 ++
>  drivers/video/backlight/pwm_bl.c  | 35 ++-
>  include/linux/backlight.h |  8 +
>  5 files changed, 89 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/ABI/testing/sysfs-class-backlight

ping, any comments on v3?

Thanks

Matthias


Re: [PATCH v9 04/18] kunit: test: add kunit_stream a std::stream like logger

2019-07-22 Thread Stephen Boyd
Quoting Brendan Higgins (2019-07-22 15:30:49)
> On Mon, Jul 22, 2019 at 1:03 PM Stephen Boyd  wrote:
> >
> >
> > What's the calling context of the assertions and expectations? I still
> > don't like the fact that string stream needs to allocate buffers and
> > throw them into a list somewhere because the calling context matters
> > there.
> 
> The calling context is the same as before, which is anywhere.

Ok. That's concerning then.

> 
> > I'd prefer we just wrote directly to the console/log via printk
> > instead. That way things are simple because we use the existing
> > buffering path of printk, but maybe there's some benefit to the string
> > stream that I don't see? Right now it looks like it builds a string and
> > then dumps it to printk so I'm sort of lost what the benefit is over
> > just writing directly with printk.
> 
> It's just buffering it so the whole string gets printed uninterrupted.
> If we were to print out piecemeal to printk, couldn't we have another
> call to printk come in causing it to garble the KUnit message we are
> in the middle of printing?

Yes, printing piecemeal by calling printk many times could lead to
interleaving of messages if something else comes in such as an interrupt
printing something. Printk has some support to hold "records" but I'm
not sure how that would work here because KERN_CONT talks about only
being used early on in boot code. I haven't looked at printk in detail
though so maybe I'm all wrong and KERN_CONT just works?

Can printk be called once with whatever is in the struct? Otherwise if
this is about making printk into a structured log then maybe printk
isn't the proper solution anyway. Maybe a dev interface should be used
instead that can handle starting and stopping tests (via ioctl) in
addition to reading test results, records, etc. with read() and a
clearing of the records. Then the seqfile API works naturally. All of
this is a bit premature, but it looks like you're going down the path of
making something akin to ftrace that stores binary formatted
assertion/expectation records in a lockless ring buffer that then
formats those records when the user asks for them.

I can imagine someone wanting to write unit tests that check conditions
from a simulated hardirq context via irq works (a driver mock
framework?), so this doesn't seem far off.



Re: [RFC] Expanding drm_mode_modeinfo flags

2019-07-22 Thread Jeykumar Sankaran

On 2019-07-19 07:29, Sean Paul wrote:

On Fri, Jul 19, 2019 at 05:15:28PM +0300, Ville Syrjälä wrote:

On Fri, Jul 19, 2019 at 09:55:58AM -0400, Sean Paul wrote:
> On Fri, Jul 19, 2019 at 11:05:53AM +0200, Daniel Vetter wrote:
> > On Thu, Jul 18, 2019 at 11:18:42AM -0700, Jeykumar Sankaran wrote:
> > > On 2019-07-16 02:07, Daniel Vetter wrote:
> > > > On Thu, Jul 11, 2019 at 11:46:44AM -0700, Jeykumar Sankaran wrote:


/snip


> > > > >   drm: add mode flags in uapi for seamless mode switch
> > > >
> > > > I think the uapi is the trivial part here, the real deal is how
> > > > userspace
> > > > uses this. Can you pls post the patches for your compositor?
> > > >
> > > > Also note that we already allow userspace to tell the kernel whether
> > > > flickering is ok or not for a modeset. msm driver could use that to at
> > > > least tell userspace whether a modeset change is possible. So you can
> > > > already implement glitch-free modeset changes for at least video mode.
> > > > -Daniel
> > >
> > > I believe you are referring to the below tv property of the connector.
> > >
> > > /**
> > >  * @tv_flicker_reduction_property: Optional TV property to control the
> > >  * flicker reduction mode.
> > >  */
> > > struct drm_property *tv_flicker_reduction_property;
> >
> > Not even close :-)
> >
> > I mean the DRM_MODE_ATOMIC_ALLOW_MODESET flag for the atomic ioctl. This
> > is not a property of a mode, this is a property of a _transition_ between
> > configurations. Some transitions can be done flicker free, others can't.
>
> Agree that an atomic flag on a commit is the way to accomplish this. It's 
pretty
> similar to the psr transitions, where we want to reuse most of the atomic
> circuitry, but in a specialized way. We'd also have to be careful to only
> involve the drm objects which are seamless modeset aware (you could imagine
> a bridge chain where the bridges downstream of the first bridge don't care).
>
> >
> > There's then still the question of how to pick video vs command mode, but
> > imo better to start with implementing the transition behaviour correctly
> > first.
>
> Connector property? Possibly a terrible idea, but I wonder if we could [re]use
> the vrr properties for command mode. The docs state that the driver has the
> option of putting upper and lower bounds on the refresh rate.

Not really sure why this needs new props and whatnot. This is kinda 
what

the i915 "fastset" stuff already does:
1. userspace asks for something to be changed via atomic
2. driver calculates whether a modeset is actually required
3. atomic validates need_modeset() vs. DRM_MODE_ATOMIC_ALLOW_MODESET
4. if (need_modeset) heavyweight_commit() else lightweight_commit()

Ie. why should userspace really care about anything except the
"flickers are OK" vs. "flickers not wanted" thing?


Agree, I don't think the seamless modeset (ie: changing resolution 
without
flicker) needs a property. Just need to test the commit without 
ALLOW_MODESET

and commit it if the test passes.



Agreed that a TEST_ONLY commit without ALLOW_MODESET flag can be used to 
check
whether the modeset can be done seamless. But since there are no error 
code returns,
the client cannot distinguish the test_only commit failures from other 
invalid config failures.


Also, note that when the client sees two 1080p modes (vid/cmd) and it is 
interested only
to do *only* seamless switches, without any additional flag it cannot 
distinguish between
these two 1080p modes. The client has to invoke two test_only commits 
with these

modes to identify the seamless one. Is that a preferred approach?

Intel's "fastset" calculates the need for modeset internally based on 
the

configuration and chooses the best commit path. But the requirement here
is to expose the information up-front since the use case cannot afford
to fall back to the normal modeset when it has requested for a seamless 
one.




Also what's the benefit of using video mode if your panel supportes
command mode? Can you turn off the memory in the panel and actually
save power that way? And if there is a benefit can't the driver just
automagically switch between the two based on how often things are
getting updated? That would match how eDP PSR already works.


I'm guessing video mode might have some latency benefits over command 
mode?


Sean


Yes. Pretty much those are reasons we need to switch to video mode. But 
instead
of making the decision internal to the driver based on the frequency of 
frame updates,
we have proprietary use cases where the client has to trigger the switch 
explicitly.
So we are trying to find ways to represent the same resolution in both 
video/cmd modes.


Thanks and Regards,
Jeykumar S.





--
Ville Syrjälä
Intel


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

[Bug 204145] amdgpu video playback causes host to hard reset (checkstop) on POWER9 with RX 580

2019-07-22 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=204145

Michael Ellerman (mich...@ellerman.id.au) changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 CC||mich...@ellerman.id.au
 Resolution|--- |CODE_FIX

--- Comment #25 from Michael Ellerman (mich...@ellerman.id.au) ---
This is in my fixes branch which I'll send to Linus later this week:

https://git.kernel.org/powerpc/c/b4fc36e60f25cf22bf8b7b015a701015740c3743

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

[PATCH 3/3] net/xdp: convert put_page() to put_user_page*()

2019-07-22 Thread john . hubbard
From: John Hubbard 

For pages that were retained via get_user_pages*(), release those pages
via the new put_user_page*() routines, instead of via put_page() or
release_pages().

This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
("mm: introduce put_user_page*(), placeholder versions").

Cc: Björn Töpel 
Cc: Magnus Karlsson 
Cc: David S. Miller 
Cc: net...@vger.kernel.org
Signed-off-by: John Hubbard 
---
 net/xdp/xdp_umem.c | 9 +
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/net/xdp/xdp_umem.c b/net/xdp/xdp_umem.c
index 83de74ca729a..0325a17915de 100644
--- a/net/xdp/xdp_umem.c
+++ b/net/xdp/xdp_umem.c
@@ -166,14 +166,7 @@ void xdp_umem_clear_dev(struct xdp_umem *umem)
 
 static void xdp_umem_unpin_pages(struct xdp_umem *umem)
 {
-   unsigned int i;
-
-   for (i = 0; i < umem->npgs; i++) {
-   struct page *page = umem->pgs[i];
-
-   set_page_dirty_lock(page);
-   put_page(page);
-   }
+   put_user_pages_dirty_lock(umem->pgs, umem->npgs);
 
kfree(umem->pgs);
umem->pgs = NULL;
-- 
2.22.0



[PATCH 2/3] drivers/gpu/drm/via: convert put_page() to put_user_page*()

2019-07-22 Thread john . hubbard
From: John Hubbard 

For pages that were retained via get_user_pages*(), release those pages
via the new put_user_page*() routines, instead of via put_page() or
release_pages().

This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
("mm: introduce put_user_page*(), placeholder versions").

Also reverse the order of a comparison, in order to placate
checkpatch.pl.

Cc: David Airlie 
Cc: Daniel Vetter 
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: John Hubbard 
---
 drivers/gpu/drm/via/via_dmablit.c | 11 +++
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/via/via_dmablit.c 
b/drivers/gpu/drm/via/via_dmablit.c
index 062067438f1d..754f2bb97d61 100644
--- a/drivers/gpu/drm/via/via_dmablit.c
+++ b/drivers/gpu/drm/via/via_dmablit.c
@@ -171,7 +171,6 @@ via_map_blit_for_device(struct pci_dev *pdev,
 static void
 via_free_sg_info(struct pci_dev *pdev, drm_via_sg_info_t *vsg)
 {
-   struct page *page;
int i;
 
switch (vsg->state) {
@@ -186,13 +185,9 @@ via_free_sg_info(struct pci_dev *pdev, drm_via_sg_info_t 
*vsg)
kfree(vsg->desc_pages);
/* fall through */
case dr_via_pages_locked:
-   for (i = 0; i < vsg->num_pages; ++i) {
-   if (NULL != (page = vsg->pages[i])) {
-   if (!PageReserved(page) && (DMA_FROM_DEVICE == 
vsg->direction))
-   SetPageDirty(page);
-   put_page(page);
-   }
-   }
+   __put_user_pages(vsg->pages, vsg->num_pages,
+(vsg->direction == DMA_FROM_DEVICE) ?
+PUP_FLAGS_DIRTY : PUP_FLAGS_CLEAN);
/* fall through */
case dr_via_pages_alloc:
vfree(vsg->pages);
-- 
2.22.0



[PATCH 0/3] introduce __put_user_pages(), convert a few call sites

2019-07-22 Thread john . hubbard
From: John Hubbard 

As discussed in [1] just now, this adds a more capable variation of
put_user_pages() to the API set, and uses it to simplify both the
main implementation, and (especially) the call sites.

Thanks to Christoph for the simplifying ideas, and Matthew for (again)
recommending an enum in the API. Matthew, I seem to recall you asked
for enums before this, so I'm sorry it took until now for me to add
them. :)

The new __put_user_pages() takes an enum that handles the various
combinations of needing to call set_page_dirty() or
set_page_dirty_lock(), before calling put_user_page().

I'm using the same CC list as in [1], even though IB is no longer
included in the series. That's everyone can see what the end result
turns out to be.

Notes about the remaining patches to come:

There are about 50+ patches in my tree [2], and I'll be sending out the
remaining ones in a few more groups:

* The block/bio related changes (Jerome mostly wrote those, but I've
  had to move stuff around extensively, and add a little code)

* mm/ changes

* other subsystem patches

* an RFC that shows the current state of the tracking patch set. That
  can only be applied after all call sites are converted, but it's
  good to get an early look at it.

This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
("mm: introduce put_user_page*(), placeholder versions").

[1] https://lore.kernel.org/r/20190722093355.gb29...@lst.de
[2] https://github.com/johnhubbard/linux/tree/gup_dma_core

John Hubbard (3):
  mm/gup: introduce __put_user_pages()
  drivers/gpu/drm/via: convert put_page() to put_user_page*()
  net/xdp: convert put_page() to put_user_page*()

 drivers/gpu/drm/via/via_dmablit.c |  11 +--
 include/linux/mm.h|  58 +++-
 mm/gup.c  | 149 +++---
 net/xdp/xdp_umem.c|   9 +-
 4 files changed, 135 insertions(+), 92 deletions(-)

-- 
2.22.0



[PATCH 1/3] mm/gup: introduce __put_user_pages()

2019-07-22 Thread john . hubbard
From: John Hubbard 

Add a more capable variation of put_user_pages() to the
API set, and call it from the simple ones.

The new __put_user_pages() takes an enum that handles the various
combinations of needing to call set_page_dirty() or
set_page_dirty_lock(), before calling put_user_page().

Cc: Matthew Wilcox 
Cc: Jan Kara 
Cc: Christoph Hellwig 
Signed-off-by: John Hubbard 
---
 include/linux/mm.h |  58 ++-
 mm/gup.c   | 137 ++---
 2 files changed, 124 insertions(+), 71 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 0334ca97c584..7218585681b2 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1057,8 +1057,62 @@ static inline void put_user_page(struct page *page)
put_page(page);
 }
 
-void put_user_pages_dirty(struct page **pages, unsigned long npages);
-void put_user_pages_dirty_lock(struct page **pages, unsigned long npages);
+enum pup_flags_t {
+   PUP_FLAGS_CLEAN = 0,
+   PUP_FLAGS_DIRTY = 1,
+   PUP_FLAGS_LOCK  = 2,
+   PUP_FLAGS_DIRTY_LOCK= 3,
+};
+
+void __put_user_pages(struct page **pages, unsigned long npages,
+ enum pup_flags_t flags);
+
+/**
+ * put_user_pages_dirty() - release and dirty an array of gup-pinned pages
+ * @pages:  array of pages to be marked dirty and released.
+ * @npages: number of pages in the @pages array.
+ *
+ * "gup-pinned page" refers to a page that has had one of the get_user_pages()
+ * variants called on that page.
+ *
+ * For each page in the @pages array, make that page (or its head page, if a
+ * compound page) dirty, if it was previously listed as clean. Then, release
+ * the page using put_user_page().
+ *
+ * Please see the put_user_page() documentation for details.
+ *
+ * set_page_dirty(), which does not lock the page, is used here.
+ * Therefore, it is the caller's responsibility to ensure that this is
+ * safe. If not, then put_user_pages_dirty_lock() should be called instead.
+ *
+ */
+static inline void put_user_pages_dirty(struct page **pages,
+   unsigned long npages)
+{
+   __put_user_pages(pages, npages, PUP_FLAGS_DIRTY);
+}
+
+/**
+ * put_user_pages_dirty_lock() - release and dirty an array of gup-pinned pages
+ * @pages:  array of pages to be marked dirty and released.
+ * @npages: number of pages in the @pages array.
+ *
+ * For each page in the @pages array, make that page (or its head page, if a
+ * compound page) dirty, if it was previously listed as clean. Then, release
+ * the page using put_user_page().
+ *
+ * Please see the put_user_page() documentation for details.
+ *
+ * This is just like put_user_pages_dirty(), except that it invokes
+ * set_page_dirty_lock(), instead of set_page_dirty().
+ *
+ */
+static inline void put_user_pages_dirty_lock(struct page **pages,
+unsigned long npages)
+{
+   __put_user_pages(pages, npages, PUP_FLAGS_DIRTY_LOCK);
+}
+
 void put_user_pages(struct page **pages, unsigned long npages);
 
 #if defined(CONFIG_SPARSEMEM) && !defined(CONFIG_SPARSEMEM_VMEMMAP)
diff --git a/mm/gup.c b/mm/gup.c
index 98f13ab37bac..6831ef064d76 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -29,87 +29,86 @@ struct follow_page_context {
unsigned int page_mask;
 };
 
-typedef int (*set_dirty_func_t)(struct page *page);
-
-static void __put_user_pages_dirty(struct page **pages,
-  unsigned long npages,
-  set_dirty_func_t sdf)
-{
-   unsigned long index;
-
-   for (index = 0; index < npages; index++) {
-   struct page *page = compound_head(pages[index]);
-
-   /*
-* Checking PageDirty at this point may race with
-* clear_page_dirty_for_io(), but that's OK. Two key cases:
-*
-* 1) This code sees the page as already dirty, so it skips
-* the call to sdf(). That could happen because
-* clear_page_dirty_for_io() called page_mkclean(),
-* followed by set_page_dirty(). However, now the page is
-* going to get written back, which meets the original
-* intention of setting it dirty, so all is well:
-* clear_page_dirty_for_io() goes on to call
-* TestClearPageDirty(), and write the page back.
-*
-* 2) This code sees the page as clean, so it calls sdf().
-* The page stays dirty, despite being written back, so it
-* gets written back again in the next writeback cycle.
-* This is harmless.
-*/
-   if (!PageDirty(page))
-   sdf(page);
-
-   put_user_page(page);
-   }
-}
-
 /**
- * put_user_pages_dirty() - release and dirty an array of gup-pinned pages
+ * __put_user_pages() - 

Re: [PATCH v9 04/18] kunit: test: add kunit_stream a std::stream like logger

2019-07-22 Thread Brendan Higgins
On Mon, Jul 22, 2019 at 1:03 PM Stephen Boyd  wrote:
>
> Quoting Brendan Higgins (2019-07-18 17:08:34)
> > On Thu, Jul 18, 2019 at 12:22:33PM -0700, Brendan Higgins wrote:
> >
> > I started poking around with your suggestion while we are waiting. A
> > couple early observations:
> >
> > 1) It is actually easier to do than I previously thought and will probably
> >help with getting more of the planned TAP output stuff working.
> >
> >That being said, this is still a pretty substantial undertaking and
> >will likely take *at least* a week to implement and properly review.
> >Assuming everything goes extremely well (no unexpected issues on my
> >end, very responsive reviewers, etc).
> >
> > 2) It *will* eliminate the need for kunit_stream.
> >
> > 3) ...but, it *will not* eliminate the need for string_stream.
> >
> > Based on my early observations, I do think it is worth doing, but I
> > don't think it is worth trying to make it in this patchset (unless I
> > have already missed the window, or it is going to be open for a while):
>
> The merge window is over. Typically code needs to be settled a few weeks
> before it opens (i.e. around -rc4 or -rc5) for most maintainers to pick
> up patches for the next merge window.

Yeah, it closed on Sunday, right?

I thought we might be able to squeak in since it was *mostly* settled,
and Shuah sent me an email two weeks ago which I interpreted to mean
she was still willing to take it.

In any case, it doesn't matter now.

> > I do think it will make things much cleaner, but I don't think it will
> > achieve your desired goal of getting rid of an unstructured
> > {kunit|string}_stream style interface; it just adds a layer on top of it
> > that makes it harder to misuse.
>
> Ok.
>
> >
> > I attached a patch of what I have so far at the end of this email so you
> > can see what I am talking about. And of course, if you agree with my
> > assessment, so we can start working on it as a future patch.
> >
> > A couple things in regard to the patch I attached:
> >
> > 1) I wrote it pretty quickly so there are almost definitely mistakes.
> >You should consider it RFC. I did verify it compiles though.
> >
> > 2) Also, I did use kunit_stream in writing it: all occurences should be
> >pretty easy to replace with string_stream; nevertheless, the reason
> >for this is just to make it easier to play with the current APIs. I
> >wanted to have something working before I went through a big tedious
> >refactoring. So sorry if it causes any confusion.
> >
> > 3) I also based the patch on all the KUnit patches I have queued up
> >(includes things like mocking and such) since I want to see how this
> >serialization thing will work with mocks and matchers and things like
> >that.
>
> Great!
>
> >
> > From 53d475d3d56afcf92b452c6d347dbedfa1a17d34 Mon Sep 17 00:00:00 2001
> > From: Brendan Higgins 
> > Date: Thu, 18 Jul 2019 16:08:52 -0700
> > Subject: [PATCH v1] DO NOT MERGE: started playing around with the
> >  serialization api
> >
> > ---
> >  include/kunit/assert.h | 130 ++
> >  include/kunit/mock.h   |   4 +
> >  kunit/Makefile |   3 +-
> >  kunit/assert.c | 179 +
> >  kunit/mock.c   |   6 +-
> >  5 files changed, 318 insertions(+), 4 deletions(-)
> >  create mode 100644 include/kunit/assert.h
> >  create mode 100644 kunit/assert.c
> >
> > diff --git a/include/kunit/assert.h b/include/kunit/assert.h
> > new file mode 100644
> > index 0..e054fdff4642f
> > --- /dev/null
> > +++ b/include/kunit/assert.h
> > @@ -0,0 +1,130 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Assertion and expectation serialization API.
> > + *
> > + * Copyright (C) 2019, Google LLC.
> > + * Author: Brendan Higgins 
> > + */
> > +
> > +#ifndef _KUNIT_ASSERT_H
> > +#define _KUNIT_ASSERT_H
> > +
> > +#include 
> > +#include 
> > +
> > +enum kunit_assert_type {
> > +   KUNIT_ASSERTION,
> > +   KUNIT_EXPECTATION,
> > +};
> > +
> > +struct kunit_assert {
> > +   enum kunit_assert_type type;
> > +   const char *line;
> > +   const char *file;
> > +   struct va_format message;
> > +   void (*format)(struct kunit_assert *assert,
> > +  struct kunit_stream *stream);
>
> Would passing in the test help too?

Yeah, it would probably be good to put one in `struct kunit_assert`.

> > +};
> > +
> > +void kunit_base_assert_format(struct kunit_assert *assert,
> > + struct kunit_stream *stream);
> > +
> > +void kunit_assert_print_msg(struct kunit_assert *assert,
> > +   struct kunit_stream *stream);
> > +
> > +struct kunit_unary_assert {
> > +   struct kunit_assert assert;
> > +   const char *condition;
> > +   bool expected_true;
> > +};
> > +
> > +void kunit_unary_assert_format(struct kunit_assert *assert,
> > +  struct 

[PATCH] drm/exynos: fix missing decrement of retry counter

2019-07-22 Thread Colin King
From: Colin Ian King 

Currently the retry counter is not being decremented, leading to a
potential infinite spin if the scalar_reads don't change state.

Addresses-Coverity: ("Infinite loop")
Fixes: 280e54c9f614 ("drm/exynos: scaler: Reset hardware before starting the 
operation")
Signed-off-by: Colin Ian King 
---
 drivers/gpu/drm/exynos/exynos_drm_scaler.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_scaler.c 
b/drivers/gpu/drm/exynos/exynos_drm_scaler.c
index 9af096479e1c..b24ba948b725 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_scaler.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_scaler.c
@@ -94,12 +94,12 @@ static inline int scaler_reset(struct scaler_context 
*scaler)
scaler_write(SCALER_CFG_SOFT_RESET, SCALER_CFG);
do {
cpu_relax();
-   } while (retry > 1 &&
+   } while (--retry > 1 &&
 scaler_read(SCALER_CFG) & SCALER_CFG_SOFT_RESET);
do {
cpu_relax();
scaler_write(1, SCALER_INT_EN);
-   } while (retry > 0 && scaler_read(SCALER_INT_EN) != 1);
+   } while (--retry > 0 && scaler_read(SCALER_INT_EN) != 1);
 
return retry ? 0 : -EIO;
 }
-- 
2.20.1

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

Re: [PATCH] drm/i915: Fix up broken merge

2019-07-22 Thread Daniel Vetter
On Mon, Jul 22, 2019 at 11:41 PM Chris Wilson  wrote:
>
> Quoting Daniel Vetter (2019-07-22 22:37:59)
> > Maxime didn't really compile-test this :-/
> >
> > We need to re-apply
> >
> > commit e4fa8457b2197118538a1400b75c898f9faaf164
> > Author: Daniel Vetter 
> > Date:   Fri Jun 14 22:35:25 2019 +0200
> >
> > drm/prime: Align gem_prime_export with obj_funcs.export
> >
> > plus make sure i915_gem_dma_buf.c doesn't get zombie-resurrect. It
> > moved in
> >
> > commit 10be98a77c558f8cfb823cd2777171fbb35040f6
> > Author: Chris Wilson 
> > Date:   Tue May 28 10:29:49 2019 +0100
> >
> > drm/i915: Move more GEM objects under gem/
> >
> > v2: Remember the selftests (Chris).
> >
> > Fixes: 03b0f2ce735e ("Merge v5.3-rc1 into drm-misc-next")
> > Cc: Maxime Ripard 
> > Cc: Chris Wilson 
> > Signed-off-by: Daniel Vetter 
>
> Matches the same compile fix and git rm as I did locally,
> Reviewed-by: Chris Wilson 

And pushed with Dave's irc-ack. Thanks for helping along to get this rectified.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/i915: Fix up broken merge

2019-07-22 Thread Chris Wilson
Quoting Daniel Vetter (2019-07-22 22:37:59)
> Maxime didn't really compile-test this :-/
> 
> We need to re-apply
> 
> commit e4fa8457b2197118538a1400b75c898f9faaf164
> Author: Daniel Vetter 
> Date:   Fri Jun 14 22:35:25 2019 +0200
> 
> drm/prime: Align gem_prime_export with obj_funcs.export
> 
> plus make sure i915_gem_dma_buf.c doesn't get zombie-resurrect. It
> moved in
> 
> commit 10be98a77c558f8cfb823cd2777171fbb35040f6
> Author: Chris Wilson 
> Date:   Tue May 28 10:29:49 2019 +0100
> 
> drm/i915: Move more GEM objects under gem/
> 
> v2: Remember the selftests (Chris).
> 
> Fixes: 03b0f2ce735e ("Merge v5.3-rc1 into drm-misc-next")
> Cc: Maxime Ripard 
> Cc: Chris Wilson 
> Signed-off-by: Daniel Vetter 

Matches the same compile fix and git rm as I did locally,
Reviewed-by: Chris Wilson 
-Chris
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm/i915: Fix up broken merge

2019-07-22 Thread Daniel Vetter
Maxime didn't really compile-test this :-/

We need to re-apply

commit e4fa8457b2197118538a1400b75c898f9faaf164
Author: Daniel Vetter 
Date:   Fri Jun 14 22:35:25 2019 +0200

drm/prime: Align gem_prime_export with obj_funcs.export

plus make sure i915_gem_dma_buf.c doesn't get zombie-resurrect. It
moved in

commit 10be98a77c558f8cfb823cd2777171fbb35040f6
Author: Chris Wilson 
Date:   Tue May 28 10:29:49 2019 +0100

drm/i915: Move more GEM objects under gem/

v2: Remember the selftests (Chris).

Fixes: 03b0f2ce735e ("Merge v5.3-rc1 into drm-misc-next")
Cc: Maxime Ripard 
Cc: Chris Wilson 
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c|   5 +-
 .../drm/i915/gem/selftests/i915_gem_dmabuf.c  |   8 +-
 drivers/gpu/drm/i915/i915_gem_dmabuf.c| 336 ---
 .../gpu/drm/i915/selftests/i915_gem_dmabuf.c  | 404 --
 4 files changed, 6 insertions(+), 747 deletions(-)
 delete mode 100644 drivers/gpu/drm/i915/i915_gem_dmabuf.c
 delete mode 100644 drivers/gpu/drm/i915/selftests/i915_gem_dmabuf.c

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c 
b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
index cbf1701d3acc..570b20ad9e58 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
@@ -204,8 +204,7 @@ static const struct dma_buf_ops i915_dmabuf_ops =  {
.end_cpu_access = i915_gem_end_cpu_access,
 };
 
-struct dma_buf *i915_gem_prime_export(struct drm_device *dev,
- struct drm_gem_object *gem_obj, int flags)
+struct dma_buf *i915_gem_prime_export(struct drm_gem_object *gem_obj, int 
flags)
 {
struct drm_i915_gem_object *obj = to_intel_bo(gem_obj);
DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
@@ -222,7 +221,7 @@ struct dma_buf *i915_gem_prime_export(struct drm_device 
*dev,
return ERR_PTR(ret);
}
 
-   return drm_gem_dmabuf_export(dev, _info);
+   return drm_gem_dmabuf_export(gem_obj->dev, _info);
 }
 
 static int i915_gem_object_get_pages_dmabuf(struct drm_i915_gem_object *obj)
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c 
b/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c
index e3a64edef918..d85d1ce273ca 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c
@@ -20,7 +20,7 @@ static int igt_dmabuf_export(void *arg)
if (IS_ERR(obj))
return PTR_ERR(obj);
 
-   dmabuf = i915_gem_prime_export(>drm, >base, 0);
+   dmabuf = i915_gem_prime_export(>base, 0);
i915_gem_object_put(obj);
if (IS_ERR(dmabuf)) {
pr_err("i915_gem_prime_export failed with err=%d\n",
@@ -44,7 +44,7 @@ static int igt_dmabuf_import_self(void *arg)
if (IS_ERR(obj))
return PTR_ERR(obj);
 
-   dmabuf = i915_gem_prime_export(>drm, >base, 0);
+   dmabuf = i915_gem_prime_export(>base, 0);
if (IS_ERR(dmabuf)) {
pr_err("i915_gem_prime_export failed with err=%d\n",
   (int)PTR_ERR(dmabuf));
@@ -219,7 +219,7 @@ static int igt_dmabuf_export_vmap(void *arg)
if (IS_ERR(obj))
return PTR_ERR(obj);
 
-   dmabuf = i915_gem_prime_export(>drm, >base, 0);
+   dmabuf = i915_gem_prime_export(>base, 0);
if (IS_ERR(dmabuf)) {
pr_err("i915_gem_prime_export failed with err=%d\n",
   (int)PTR_ERR(dmabuf));
@@ -266,7 +266,7 @@ static int igt_dmabuf_export_kmap(void *arg)
if (IS_ERR(obj))
return PTR_ERR(obj);
 
-   dmabuf = i915_gem_prime_export(>drm, >base, 0);
+   dmabuf = i915_gem_prime_export(>base, 0);
i915_gem_object_put(obj);
if (IS_ERR(dmabuf)) {
err = PTR_ERR(dmabuf);
diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c 
b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
deleted file mode 100644
index 54ecab91b3a9..
--- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
+++ /dev/null
@@ -1,336 +0,0 @@
-/*
- * Copyright 2012 Red Hat Inc
- *
- * Permission is hereby granted, free of charge, to any person obtaining a
- * copy of this software and associated documentation files (the "Software"),
- * to deal in the Software without restriction, including without limitation
- * the rights to use, copy, modify, merge, publish, distribute, sublicense,
- * and/or sell copies of the Software, and to permit persons to whom the
- * Software is furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice (including the next
- * paragraph) shall be included in all copies or substantial portions of the
- * Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
- * THE 

Re: [Intel-gfx] [PATCH] drm/i915: Fix up broken merge

2019-07-22 Thread Chris Wilson
Quoting Daniel Vetter (2019-07-22 22:21:01)
> Maxime didn't really compile-test this :-/
> 
> We need to re-apply
> 
> commit e4fa8457b2197118538a1400b75c898f9faaf164
> Author: Daniel Vetter 
> Date:   Fri Jun 14 22:35:25 2019 +0200
> 
> drm/prime: Align gem_prime_export with obj_funcs.export
> 
> plus make sure i915_gem_dma_buf.c doesn't get zombie-resurrect. It
> moved in
> 
> commit 10be98a77c558f8cfb823cd2777171fbb35040f6
> Author: Chris Wilson 
> Date:   Tue May 28 10:29:49 2019 +0100
> 
> drm/i915: Move more GEM objects under gem/
> 
> Fixes: 03b0f2ce735e ("Merge v5.3-rc1 into drm-misc-next")
> Cc: Maxime Ripard 
> Signed-off-by: Daniel Vetter 
> ---
>  drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c |   5 +-
>  drivers/gpu/drm/i915/i915_gem_dmabuf.c | 336 -

4 little fixes for the change in iface for
gem/selftests/i915_gem_dmabuf.c (plus git rm
selftests/i915_gem_dmabuf.c)
-Chris
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v4 0/4] Add a generic driver for LED-based backlight

2019-07-22 Thread Jacek Anaszewski
On 7/22/19 9:06 AM, Lee Jones wrote:
> On Thu, 18 Jul 2019, Jacek Anaszewski wrote:
> 
>> On 7/17/19 4:15 PM, Jean-Jacques Hiblot wrote:
>>> This series aims to add a led-backlight driver, similar to pwm-backlight,
>>> but using a LED class device underneath.
>>>
>>> A few years ago (2015), Tomi Valkeinen posted a series implementing a
>>> backlight driver on top of a LED device:
>>> https://patchwork.kernel.org/patch/7293991/
>>> https://patchwork.kernel.org/patch/7294001/
>>> https://patchwork.kernel.org/patch/7293981/
>>>
>>> The discussion stopped because Tomi lacked the time to work on it.
>>>
>>> changes in v4:
>>> - fix dev_err() messages and commit logs following the advices of Pavel
>>> - cosmetic changes (indents, getting rid of  "? 1 : 0" in
>>>   led_match_led_node())
>>>
>>> changes in v3:
>>> - dt binding: don't limit the brightness range to 0-255. Use the range of
>>>   the underlying LEDs. as a side-effect, all LEDs must now have the same
>>>   range
>>> - driver: Adapt to dt binding update.
>>> - driver: rework probe() for clarity and remove the remaining goto.
>>>
>>> changes in v2:
>>> - handle more than one LED.
>>> - don't make the backlight device a child of the LED controller.
>>> - make brightness-levels and default-brightness-level optional
>>> - removed the option to use a GPIO enable.
>>> - removed the option to use a regulator. It should be handled by the LED
>>>   core
>>> - don't make any change to the LED core (not needed anymore)
>>>
>>> Jean-Jacques Hiblot (2):
>>>   leds: Add managed API to get a LED from a device driver
>>>   dt-bindings: backlight: Add led-backlight binding
>>>
>>> Tomi Valkeinen (2):
>>>   leds: Add of_led_get() and led_put()
>>>   backlight: add led-backlight driver
>>>
>>>  .../bindings/leds/backlight/led-backlight.txt |  28 ++
>>>  drivers/leds/led-class.c  |  92 ++
>>>  drivers/video/backlight/Kconfig   |   7 +
>>>  drivers/video/backlight/Makefile  |   1 +
>>>  drivers/video/backlight/led_bl.c  | 268 ++
>>>  include/linux/leds.h  |   6 +
>>>  6 files changed, 402 insertions(+)
>>>  create mode 100644 
>>> Documentation/devicetree/bindings/leds/backlight/led-backlight.txt
>>>  create mode 100644 drivers/video/backlight/led_bl.c
>>>
>>
>> For the whole set:
>>
>> Acked-by: Jacek Anaszewski 
>>
>> Lee - we need to create immutable branch for this set since there will
>> be some interfering changes in the LED core in this cycle.
>>
>> I can create the branch and send the pull request once we will
>> obtain the ack from Rob for DT bindings, unless you have other
>> preference.
> 
> We also require a review to be conducted by Daniel Thompson.
> 
> After which, an immutable branch sounds like a good idea.  I'd like to
> create this myself if you don't mind.

Sure, thanks.

-- 
Best regards,
Jacek Anaszewski


[PATCH] drm/i915: Fix up broken merge

2019-07-22 Thread Daniel Vetter
Maxime didn't really compile-test this :-/

We need to re-apply

commit e4fa8457b2197118538a1400b75c898f9faaf164
Author: Daniel Vetter 
Date:   Fri Jun 14 22:35:25 2019 +0200

drm/prime: Align gem_prime_export with obj_funcs.export

plus make sure i915_gem_dma_buf.c doesn't get zombie-resurrect. It
moved in

commit 10be98a77c558f8cfb823cd2777171fbb35040f6
Author: Chris Wilson 
Date:   Tue May 28 10:29:49 2019 +0100

drm/i915: Move more GEM objects under gem/

Fixes: 03b0f2ce735e ("Merge v5.3-rc1 into drm-misc-next")
Cc: Maxime Ripard 
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c |   5 +-
 drivers/gpu/drm/i915/i915_gem_dmabuf.c | 336 -
 2 files changed, 2 insertions(+), 339 deletions(-)
 delete mode 100644 drivers/gpu/drm/i915/i915_gem_dmabuf.c

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c 
b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
index cbf1701d3acc..570b20ad9e58 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
@@ -204,8 +204,7 @@ static const struct dma_buf_ops i915_dmabuf_ops =  {
.end_cpu_access = i915_gem_end_cpu_access,
 };
 
-struct dma_buf *i915_gem_prime_export(struct drm_device *dev,
- struct drm_gem_object *gem_obj, int flags)
+struct dma_buf *i915_gem_prime_export(struct drm_gem_object *gem_obj, int 
flags)
 {
struct drm_i915_gem_object *obj = to_intel_bo(gem_obj);
DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
@@ -222,7 +221,7 @@ struct dma_buf *i915_gem_prime_export(struct drm_device 
*dev,
return ERR_PTR(ret);
}
 
-   return drm_gem_dmabuf_export(dev, _info);
+   return drm_gem_dmabuf_export(gem_obj->dev, _info);
 }
 
 static int i915_gem_object_get_pages_dmabuf(struct drm_i915_gem_object *obj)
diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c 
b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
deleted file mode 100644
index 54ecab91b3a9..
--- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
+++ /dev/null
@@ -1,336 +0,0 @@
-/*
- * Copyright 2012 Red Hat Inc
- *
- * Permission is hereby granted, free of charge, to any person obtaining a
- * copy of this software and associated documentation files (the "Software"),
- * to deal in the Software without restriction, including without limitation
- * the rights to use, copy, modify, merge, publish, distribute, sublicense,
- * and/or sell copies of the Software, and to permit persons to whom the
- * Software is furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice (including the next
- * paragraph) shall be included in all copies or substantial portions of the
- * Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
- * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
- * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
- * DEALINGS IN THE SOFTWARE.
- *
- * Authors:
- * Dave Airlie 
- */
-
-#include 
-#include 
-
-
-#include "i915_drv.h"
-
-static struct drm_i915_gem_object *dma_buf_to_obj(struct dma_buf *buf)
-{
-   return to_intel_bo(buf->priv);
-}
-
-static struct sg_table *i915_gem_map_dma_buf(struct dma_buf_attachment 
*attachment,
-enum dma_data_direction dir)
-{
-   struct drm_i915_gem_object *obj = dma_buf_to_obj(attachment->dmabuf);
-   struct sg_table *st;
-   struct scatterlist *src, *dst;
-   int ret, i;
-
-   ret = i915_gem_object_pin_pages(obj);
-   if (ret)
-   goto err;
-
-   /* Copy sg so that we make an independent mapping */
-   st = kmalloc(sizeof(struct sg_table), GFP_KERNEL);
-   if (st == NULL) {
-   ret = -ENOMEM;
-   goto err_unpin_pages;
-   }
-
-   ret = sg_alloc_table(st, obj->mm.pages->nents, GFP_KERNEL);
-   if (ret)
-   goto err_free;
-
-   src = obj->mm.pages->sgl;
-   dst = st->sgl;
-   for (i = 0; i < obj->mm.pages->nents; i++) {
-   sg_set_page(dst, sg_page(src), src->length, 0);
-   dst = sg_next(dst);
-   src = sg_next(src);
-   }
-
-   if (!dma_map_sg(attachment->dev, st->sgl, st->nents, dir)) {
-   ret = -ENOMEM;
-   goto err_free_sg;
-   }
-
-   return st;
-
-err_free_sg:
-   sg_free_table(st);
-err_free:
-   kfree(st);
-err_unpin_pages:
-   i915_gem_object_unpin_pages(obj);
-err:
-   return ERR_PTR(ret);
-}
-
-static void i915_gem_unmap_dma_buf(struct dma_buf_attachment *attachment,
-  struct sg_table *sg,
-  

[Bug 105251] [Vega10] GPU lockup on boot: VMC page fault

2019-07-22 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=105251

--- Comment #78 from deltasquared  ---
Created attachment 144846
  --> https://bugs.freedesktop.org/attachment.cgi?id=144846=edit
vega_crasher after patch, colour shaded central output, on ryzen 2200G with
vega 8 graphics

Screenshot 2/2 of vega_crasher after patch. It seems to indeterministically
switch between shaded and black central regions - I can only assume this is
down to whether or not the offending index ends up out of bounds?

If it helps I can attempt more tests with an asserts-enabled build, though that
will take some more time, a resource I am out of for today. (Will have to look
at how to do that also - just a question of a debug build or another flag that
needs passing?)

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

Re: [PATCH] Enable backlight when trigger is activated

2019-07-22 Thread Pavel Machek
Hi!

> >> This looks fishy.
> >>
> >> Maybe you should use a default-state = "keep" instead? (and you'll have
> >> to support it in the LED driver).
> >>
> >> That'll give you proper "don't touch the LED if it was turned on" behavior,
> >> which is what you seem to want.
> > 
> > Actually no, that's not what I want. LED should go on if the display
> > is active, as soon as trigger is activated.
> > 
> > Unfortunately, I have see no good way to tell if the display is
> > active (and display is usually active when trigger is activated).
> 
> default-state DT property can be also set to "on"
> (see Documentation/devicetree/bindings/leds/common.txt).

Ok, thanks for the hint, that could work. (I thought we were using
default trigger to set the LED "on").

Now...this gives me option of 0% or 100% brightness, while best would
be 10% brightness but I guess we can live with that ;-).

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature


[Bug 105251] [Vega10] GPU lockup on boot: VMC page fault

2019-07-22 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=105251

--- Comment #77 from deltasquared  ---
Created attachment 144845
  --> https://bugs.freedesktop.org/attachment.cgi?id=144845=edit
vega_crasher after patch, black central output, on ryzen 2200G with vega 8
graphics

Screenshot time (1/2). It seems sometimes vega_crasher will render black - I
haven't thoroughly looked over the code so I'm not sure if this is the
adformentioned incorrect result (where an assert would have been hit).

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

Re: [PATCH v2] drm/bridge: dw-hdmi: Refuse DDC/CI transfers on the internal I2C controller

2019-07-22 Thread Matthias Kaehlcke
On Mon, Jul 22, 2019 at 04:24:26PM -0400, Sean Paul wrote:
> On Mon, Jul 22, 2019 at 11:19:45AM -0700, Matthias Kaehlcke wrote:
> > The DDC/CI protocol involves sending a multi-byte request to the
> > display via I2C, which is typically followed by a multi-byte
> > response. The internal I2C controller only allows single byte
> > reads/writes or reads of 8 sequential bytes, hence DDC/CI is not
> > supported when the internal I2C controller is used. The I2C
> 
> This is very likely a stupid question, but I didn't see an answer for it, so
> I'll just ask :)
> 
> If the controller supports xfers of 8 bytes and 1 bytes, could you just split
> up any of these transactions into len/8+len%8 transactions?

The controller interprets all transfers to be register accesses. It is
not possible to just send the sequence '0x0a 0x0b 0x0c' as three byte
transfers, the controller expects an address for each byte and
(supposedly) sends it over the wire, which typically isn't what you
want.

Also the 8-byte reads only seem to be supported in certain
configurations ("when the DWC_HDMI_TX_20 parameter is enabled").

> > transfers complete without errors, however the data in the response
> > is garbage. Abort transfers to/from slave address 0x37 (DDC) with
> > -EOPNOTSUPP, to make it evident that the communication is failing.
> > 
> > Signed-off-by: Matthias Kaehlcke 
> > ---
> > Changes in v2:
> > - changed DDC_I2C_ADDR to DDC_CI_ADDR
> > ---
> >  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 8 
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c 
> > b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > index 045b1b13fd0e..28933629f3c7 100644
> > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > @@ -35,6 +35,7 @@
> >  
> >  #include 
> >  
> > +#define DDC_CI_ADDR0x37
> >  #define DDC_SEGMENT_ADDR   0x30
> >  
> >  #define HDMI_EDID_LEN  512
> > @@ -322,6 +323,13 @@ static int dw_hdmi_i2c_xfer(struct i2c_adapter *adap,
> > u8 addr = msgs[0].addr;
> > int i, ret = 0;
> >  
> > +   if (addr == DDC_CI_ADDR)
> > +   /*
> > +* The internal I2C controller does not support the multi-byte
> > +* read and write operations needed for DDC/CI.
> > +*/
> > +   return -EOPNOTSUPP;
> > +
> > dev_dbg(hdmi->dev, "xfer: num: %d, addr: %#x\n", num, addr);
> >  
> > for (i = 0; i < num; i++) {
> 


[Bug 105251] [Vega10] GPU lockup on boot: VMC page fault

2019-07-22 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=105251

--- Comment #76 from deltasquared  ---
(In reply to deltasquared from comment #75)

> L CALLBACK:  type = 0x8251, severity = 0x826b, message = LLVM diagnostic
GL_CALLBACK rather on that first line. terminal copypaste fail.

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

Re: [PATCH v2] drm/bridge: dw-hdmi: Refuse DDC/CI transfers on the internal I2C controller

2019-07-22 Thread Doug Anderson
Hi,

On Mon, Jul 22, 2019 at 1:24 PM Sean Paul  wrote:
>
> On Mon, Jul 22, 2019 at 11:19:45AM -0700, Matthias Kaehlcke wrote:
> > The DDC/CI protocol involves sending a multi-byte request to the
> > display via I2C, which is typically followed by a multi-byte
> > response. The internal I2C controller only allows single byte
> > reads/writes or reads of 8 sequential bytes, hence DDC/CI is not
> > supported when the internal I2C controller is used. The I2C
>
> This is very likely a stupid question, but I didn't see an answer for it, so
> I'll just ask :)
>
> If the controller supports xfers of 8 bytes and 1 bytes, could you just split
> up any of these transactions into len/8+len%8 transactions?

It's not quite that easy, I think.  Specifically a 1-byte transfer
isn't really a 1-byte transfer.

It always sticks this on the wire for a 1-byte write:

Start
Slave address (7 bits) + write (1 bit)
(wait ack)
Register address
1 byte of data
wait for ack
Stop

...or for a 1-byte read:

Start
Slave address (7 bits) + write (1 bit)
(wait ack)
Register address
(wait ack)
Repeated Start (1 bit)
Slave address (7 bits) + read (1 bit)
(read 1 byte of data)
Ack
Stop

Putting more than one of those in a row is not the same thing as just
doing a whole bunch of reads or a whole bunch of writes with no "stop"
in between.

As far as I could find out about DDC/CI it's part of the spec to _not_
send the stop between the reads / writes.


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

[Bug 105251] [Vega10] GPU lockup on boot: VMC page fault

2019-07-22 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=105251

--- Comment #75 from deltasquared  ---
After compiling mesa-git on commit 0661c357c60 from the AUR pkgbuild, I can now
confirm my system seems to have become impervious to the above "vega_crasher"
program.

Output from said program after resizing and moving vega_crasher's window a bit,
in case it was important:

L CALLBACK:  type = 0x8251, severity = 0x826b, message = LLVM diagnostic
(remark): :0:0: 9 instructions in function
GL CALLBACK:  type = 0x8251, severity = 0x826b, message = Shader Stats: SGPRS:
16 VGPRS: 24 Code Size: 52 LDS: 0 Scratch: 0 Max Waves: 10 Spilled SGPRs: 0
Spilled VGPRs: 0 PrivMem VGPRs: 0
GL CALLBACK:  type = 0x8251, severity = 0x826b, message = LLVM diagnostic
(remark): :0:0: 12 instructions in function
GL CALLBACK:  type = 0x8251, severity = 0x826b, message = Shader Stats: SGPRS:
16 VGPRS: 8 Code Size: 92 LDS: 0 Scratch: 0 Max Waves: 10 Spilled SGPRs: 0
Spilled VGPRs: 0 PrivMem VGPRs: 0
GL CALLBACK:  type = 0x8251, severity = 0x826b, message = Shader Stats: SGPRS:
16 VGPRS: 24 Code Size: 44 LDS: 0 Scratch: 0 Max Waves: 10 Spilled SGPRs: 0
Spilled VGPRs: 0 PrivMem VGPRs: 0
GL CALLBACK:  type = 0x8251, severity = 0x826b, message = Shader Stats: SGPRS:
16 VGPRS: 8 Code Size: 80 LDS: 0 Scratch: 0 Max Waves: 10 Spilled SGPRs: 0
Spilled VGPRs: 0 PrivMem VGPRs: 0
GL CALLBACK:  type = 0x8251, severity = 0x826b, message = LLVM diagnostic
(remark): :0:0: 2 instructions in function
GL CALLBACK:  type = 0x8251, severity = 0x826b, message = LLVM diagnostic
(remark): :0:0: 3 instructions in function
GL CALLBACK:  type = 0x8251, severity = 0x826b, message = LLVM diagnostic
(remark): :0:0: 4 instructions in function
GL CALLBACK:  type = 0x8251, severity = 0x826b, message = Shader Stats: SGPRS:
16 VGPRS: 24 Code Size: 44 LDS: 0 Scratch: 0 Max Waves: 10 Spilled SGPRs: 0
Spilled VGPRs: 0 PrivMem VGPRs: 0
GL CALLBACK:  type = 0x8251, severity = 0x826b, message = Shader Stats: SGPRS:
16 VGPRS: 8 Code Size: 136 LDS: 0 Scratch: 0 Max Waves: 10 Spilled SGPRs: 0
Spilled VGPRs: 0 PrivMem VGPRs: 0
GL CALLBACK:  type = 0x8251, severity = 0x826b, message = LLVM diagnostic
(remark): :0:0: 16 instructions in function
GL CALLBACK:  type = 0x8251, severity = 0x826b, message = Shader Stats: SGPRS:
24 VGPRS: 24 Code Size: 92 LDS: 0 Scratch: 0 Max Waves: 10 Spilled SGPRs: 0
Spilled VGPRs: 0 PrivMem VGPRs: 0
GL CALLBACK:  type = 0x8251, severity = 0x826b, message = Shader Stats: SGPRS:
24 VGPRS: 24 Code Size: 88 LDS: 0 Scratch: 0 Max Waves: 10 Spilled SGPRs: 0
Spilled VGPRs: 0 PrivMem VGPRs: 0

Minetest will take longer to test as the pkgbuild doesn't enable asserts, and
also because of adformentioned $dayjob. I guess in that case I'd only know if I
saw garbled output; it was never very consistent when it occured but would
always be during the loading bar screen (but when it did happen some very
colourful blocky corruption would result).

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

Re: [PATCH v2] drm/bridge: dw-hdmi: Refuse DDC/CI transfers on the internal I2C controller

2019-07-22 Thread Matthias Kaehlcke
On Mon, Jul 22, 2019 at 01:12:40PM -0700, Doug Anderson wrote:
> Hi,
> 
> On Mon, Jul 22, 2019 at 11:19 AM Matthias Kaehlcke  wrote:
> >
> > The DDC/CI protocol involves sending a multi-byte request to the
> > display via I2C, which is typically followed by a multi-byte
> > response. The internal I2C controller only allows single byte
> > reads/writes or reads of 8 sequential bytes, hence DDC/CI is not
> > supported when the internal I2C controller is used. The I2C
> > transfers complete without errors, however the data in the response
> > is garbage. Abort transfers to/from slave address 0x37 (DDC) with
> > -EOPNOTSUPP, to make it evident that the communication is failing.
> >
> > Signed-off-by: Matthias Kaehlcke 
> > ---
> > Changes in v2:
> > - changed DDC_I2C_ADDR to DDC_CI_ADDR
> > ---
> >  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 8 
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c 
> > b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > index 045b1b13fd0e..28933629f3c7 100644
> > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > @@ -35,6 +35,7 @@
> >
> >  #include 
> >
> > +#define DDC_CI_ADDR0x37
> >  #define DDC_SEGMENT_ADDR   0x30
> >
> >  #define HDMI_EDID_LEN  512
> > @@ -322,6 +323,13 @@ static int dw_hdmi_i2c_xfer(struct i2c_adapter *adap,
> > u8 addr = msgs[0].addr;
> > int i, ret = 0;
> >
> > +   if (addr == DDC_CI_ADDR)
> > +   /*
> > +* The internal I2C controller does not support the 
> > multi-byte
> > +* read and write operations needed for DDC/CI.
> > +*/
> > +   return -EOPNOTSUPP;
> > +
> 
> In theory we could also solve this by detecting (in other parts of the
> function) the bad multi-byte read/write operations.  That would maybe
> be more generic (AKA it would more properly handle random userspace
> tools fiddling with i2c-dev) but add complexity to the code.

But how do you know it's an unsupported operation, and not the driver
knowing the wacky limitations doing something valid.

E.g. you get the sequence:

0x01 0x0a 0x0b 0x0c 0x0d

This could be interpreted as "send the above bytes to the slave" or
as "send 0x0a to address 0x01, 0x0b to 0x02, 0x0c to 0x03 and 0x0d to
0x04" (at least by this driver ;-) . The latter could be intended.

> ...possibly a better long-term solution is to just totally stop
> emulating a regular i2c adapter when the hardware just doesn't support
> that.  In theory we could remove the "drm_get_edid()" call in
> dw_hdmi_connector_get_modes() and replace it with a direct call to
> drm_do_get_edid() if we're using the built-in adapter.  Possibly
> "tda998x_drv.c" would be a good reference.  If we did that, we could
> remove all the weird / hacky i2c code in this driver.

yes, that would be another and probably better option than trying to
detect unsupported transaction.

> Since the bigger cleanup seems like a bit much to ask, I'm OK with
> this as a stopgap to at least prevent userspace tools from getting
> confused.  Thus:
> 
> Reviewed-by: Douglas Anderson 

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

[PATCH v3 0/4] drm/via: drop use of deprecated headers drmP.h and drm_os_linux.h

2019-07-22 Thread Sam Ravnborg
Added subject

On Mon, Jul 22, 2019 at 10:35:41PM +0200, Sam Ravnborg wrote:
> The first three patches prepare for the removal of drmP.h.
> The last patch remove use of drmP.h and replace with necessary
> include files to fix build.
> 
> Build tested with various configs and various architectures.
> 
> I had preferred that the via driver was replaced by the
> openchrome driver, but until we have it then we need
> to deal with the legacy via driver when removing old cruft
> in the drm subsystem.
> 
> v3:
> - Use static inline functions for the read/write operations (Emil)
> - Use dedicated *_mask_or() and *_mask_and() (Emil)
> - Replace DRM_WAIT_ON in same path that introduces VIA_WAIT_ON (Emil)
> - Collected r-b's
> - Changelog adjustments
> - Rebased on top of drm-misc-next
> 
> v2:
> - Add a copy of DRM_WAIT_ON to the via driver, keeping
>   the changes to this legacy driver to a minimum.
>   This also gives much more confidence that the
>   driver continues to work as there is no changes
>   in logic. Therefore dropped "RFT".
> - Added Cc: Michel Dänzer  to all
>   patches, as Michael have commented on the series.
> 
> Sam
> 
> Sam Ravnborg (4):
>   drm/via: drop use of DRM(READ|WRITE) macros
>   drm/via: copy DRM_WAIT_ON as VIA_WAIT_ON and use it
>   drm/via: make via_drv.h self-contained
>   drm/via: drop use of drmP.h
> 
>  drivers/gpu/drm/via/via_dma.c  | 43 +++-
>  drivers/gpu/drm/via/via_dmablit.c  | 41 ++-
>  drivers/gpu/drm/via/via_drv.c  |  7 +++-
>  drivers/gpu/drm/via/via_drv.h  | 83 
> +++---
>  drivers/gpu/drm/via/via_irq.c  | 54 +
>  drivers/gpu/drm/via/via_map.c  |  6 ++-
>  drivers/gpu/drm/via/via_mm.c   |  7 +++-
>  drivers/gpu/drm/via/via_verifier.c | 22 +-
>  drivers/gpu/drm/via/via_video.c|  5 ++-
>  9 files changed, 182 insertions(+), 86 deletions(-)
> 
> 
> ___
> 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 7/9] drm/tinydrm/mipi-dbi: Select DRM_KMS_HELPER

2019-07-22 Thread David Lechner

On 7/22/19 5:43 AM, Noralf Trønnes wrote:

mipi-dbi uses several KMS helper functions but that build dependency is
not expressed. Select DRM_KMS_HELPER to fix that.

Reported-by: kbuild test robot 
Signed-off-by: Noralf Trønnes 
---


Reviewed-by: David Lechner 



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

[PATCH v3 1/4] drm/via: drop use of DRM(READ|WRITE) macros

2019-07-22 Thread Sam Ravnborg
The DRM_READ, DRM_WRITE macros comes from the deprecated drm_os_linux.h
header file. Remove their use to remove this dependency.

Replace the use of the macros with static inline variants.

v3:
- Use static inline functions, rather than macros (Emil)
- Use dedicated mask variants for byte access (Emil)

Signed-off-by: Sam Ravnborg 
Cc: Kevin Brace 
Cc: Thomas Hellstrom 
Cc: "Gustavo A. R. Silva" 
Cc: Mike Marshall 
Cc: Ira Weiny 
Cc: Daniel Vetter 
Cc: Emil Velikov 
Cc: Michel Dänzer 
---
 drivers/gpu/drm/via/via_dma.c  | 34 +++---
 drivers/gpu/drm/via/via_dmablit.c  | 24 
 drivers/gpu/drm/via/via_drv.h  | 40 ++
 drivers/gpu/drm/via/via_irq.c  | 46 +++---
 drivers/gpu/drm/via/via_verifier.c | 12 
 5 files changed, 92 insertions(+), 64 deletions(-)

diff --git a/drivers/gpu/drm/via/via_dma.c b/drivers/gpu/drm/via/via_dma.c
index d17d8f245c1a..344a12b63967 100644
--- a/drivers/gpu/drm/via/via_dma.c
+++ b/drivers/gpu/drm/via/via_dma.c
@@ -430,14 +430,14 @@ static int via_hook_segment(drm_via_private_t *dev_priv,
diff = (uint32_t) (ptr - reader) - dev_priv->dma_diff;
count = 1000;
while (diff == 0 && count--) {
-   paused = (VIA_READ(0x41c) & 0x8000);
+   paused = (via_read(dev_priv, 0x41c) & 0x8000);
if (paused)
break;
reader = *(dev_priv->hw_addr_ptr);
diff = (uint32_t) (ptr - reader) - dev_priv->dma_diff;
}
 
-   paused = VIA_READ(0x41c) & 0x8000;
+   paused = via_read(dev_priv, 0x41c) & 0x8000;
 
if (paused && !no_pci_fire) {
reader = *(dev_priv->hw_addr_ptr);
@@ -454,10 +454,10 @@ static int via_hook_segment(drm_via_private_t *dev_priv,
 * doesn't make a difference.
 */
 
-   VIA_WRITE(VIA_REG_TRANSET, (HC_ParaType_PreCR << 16));
-   VIA_WRITE(VIA_REG_TRANSPACE, pause_addr_hi);
-   VIA_WRITE(VIA_REG_TRANSPACE, pause_addr_lo);
-   VIA_READ(VIA_REG_TRANSPACE);
+   via_write(dev_priv, VIA_REG_TRANSET, (HC_ParaType_PreCR 
<< 16));
+   via_write(dev_priv, VIA_REG_TRANSPACE, pause_addr_hi);
+   via_write(dev_priv, VIA_REG_TRANSPACE, pause_addr_lo);
+   via_read(dev_priv, VIA_REG_TRANSPACE);
}
}
return paused;
@@ -467,10 +467,10 @@ static int via_wait_idle(drm_via_private_t *dev_priv)
 {
int count = 1000;
 
-   while (!(VIA_READ(VIA_REG_STATUS) & VIA_VR_QUEUE_BUSY) && --count)
+   while (!(via_read(dev_priv, VIA_REG_STATUS) & VIA_VR_QUEUE_BUSY) && 
--count)
;
 
-   while (count && (VIA_READ(VIA_REG_STATUS) &
+   while (count && (via_read(dev_priv, VIA_REG_STATUS) &
   (VIA_CMD_RGTR_BUSY | VIA_2D_ENG_BUSY |
VIA_3D_ENG_BUSY)))
--count;
@@ -536,21 +536,21 @@ static void via_cmdbuf_start(drm_via_private_t *dev_priv)
via_flush_write_combine();
(void) *(volatile uint32_t *)dev_priv->last_pause_ptr;
 
-   VIA_WRITE(VIA_REG_TRANSET, (HC_ParaType_PreCR << 16));
-   VIA_WRITE(VIA_REG_TRANSPACE, command);
-   VIA_WRITE(VIA_REG_TRANSPACE, start_addr_lo);
-   VIA_WRITE(VIA_REG_TRANSPACE, end_addr_lo);
+   via_write(dev_priv, VIA_REG_TRANSET, (HC_ParaType_PreCR << 16));
+   via_write(dev_priv, VIA_REG_TRANSPACE, command);
+   via_write(dev_priv, VIA_REG_TRANSPACE, start_addr_lo);
+   via_write(dev_priv, VIA_REG_TRANSPACE, end_addr_lo);
 
-   VIA_WRITE(VIA_REG_TRANSPACE, pause_addr_hi);
-   VIA_WRITE(VIA_REG_TRANSPACE, pause_addr_lo);
+   via_write(dev_priv, VIA_REG_TRANSPACE, pause_addr_hi);
+   via_write(dev_priv, VIA_REG_TRANSPACE, pause_addr_lo);
wmb();
-   VIA_WRITE(VIA_REG_TRANSPACE, command | HC_HAGPCMNT_MASK);
-   VIA_READ(VIA_REG_TRANSPACE);
+   via_write(dev_priv, VIA_REG_TRANSPACE, command | HC_HAGPCMNT_MASK);
+   via_read(dev_priv, VIA_REG_TRANSPACE);
 
dev_priv->dma_diff = 0;
 
count = 1000;
-   while (!(VIA_READ(0x41c) & 0x8000) && count--);
+   while (!(via_read(dev_priv, 0x41c) & 0x8000) && count--);
 
reader = *(dev_priv->hw_addr_ptr);
ptr = ((volatile char *)dev_priv->last_pause_ptr - dev_priv->dma_ptr) +
diff --git a/drivers/gpu/drm/via/via_dmablit.c 
b/drivers/gpu/drm/via/via_dmablit.c
index 062067438f1d..e1557dd67083 100644
--- a/drivers/gpu/drm/via/via_dmablit.c
+++ b/drivers/gpu/drm/via/via_dmablit.c
@@ -214,16 +214,16 @@ via_fire_dmablit(struct drm_device *dev, 
drm_via_sg_info_t *vsg, int engine)
 {
drm_via_private_t *dev_priv = (drm_via_private_t *)dev->dev_private;
 
-   VIA_WRITE(VIA_PCI_DMA_MAR0 + engine*0x10, 0);
-   

[PATCH v3 3/4] drm/via: make via_drv.h self-contained

2019-07-22 Thread Sam Ravnborg
Added include of header files to make via_drv.h self-contained.

v3:
- Reworded changelog a little - to reflect that more than one
  header files are added

Signed-off-by: Sam Ravnborg 
Reviewed-by: Emil Velikov 
Cc: Kevin Brace 
Cc: Thomas Hellstrom 
Cc: "Gustavo A. R. Silva" 
Cc: Mike Marshall 
Cc: Ira Weiny 
Cc: Daniel Vetter 
Cc: Michel Dänzer 
---
 drivers/gpu/drm/via/via_drv.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/via/via_drv.h b/drivers/gpu/drm/via/via_drv.h
index 3eb92e81655e..88deb9d8b765 100644
--- a/drivers/gpu/drm/via/via_drv.h
+++ b/drivers/gpu/drm/via/via_drv.h
@@ -24,13 +24,16 @@
 #ifndef _VIA_DRV_H_
 #define _VIA_DRV_H_
 
+#include 
 #include 
 #include 
 #include 
 #include 
 
+#include 
 #include 
 #include 
+#include 
 
 #define DRIVER_AUTHOR  "Various"
 
-- 
2.20.1

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

Re: [PATCH v2 5/9] drm/tinydrm/mipi-dbi: Remove CMA helper dependency

2019-07-22 Thread David Lechner

On 7/22/19 5:43 AM, Noralf Trønnes wrote:

mipi-dbi depends on the CMA helper through it's use of
drm_fb_cma_get_gem_obj(). This is an unnecessary dependency to drag in for
drivers that only want to use the MIPI DBI interface part.
Avoid this by open coding the function.

Signed-off-by: Noralf Trønnes 
---


Reviewed-by: David Lechner 

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

[no subject]

2019-07-22 Thread Sam Ravnborg
The first three patches prepare for the removal of drmP.h.
The last patch remove use of drmP.h and replace with necessary
include files to fix build.

Build tested with various configs and various architectures.

I had preferred that the via driver was replaced by the
openchrome driver, but until we have it then we need
to deal with the legacy via driver when removing old cruft
in the drm subsystem.

v3:
- Use static inline functions for the read/write operations (Emil)
- Use dedicated *_mask_or() and *_mask_and() (Emil)
- Replace DRM_WAIT_ON in same path that introduces VIA_WAIT_ON (Emil)
- Collected r-b's
- Changelog adjustments
- Rebased on top of drm-misc-next

v2:
- Add a copy of DRM_WAIT_ON to the via driver, keeping
  the changes to this legacy driver to a minimum.
  This also gives much more confidence that the
  driver continues to work as there is no changes
  in logic. Therefore dropped "RFT".
- Added Cc: Michel Dänzer  to all
  patches, as Michael have commented on the series.

Sam

Sam Ravnborg (4):
  drm/via: drop use of DRM(READ|WRITE) macros
  drm/via: copy DRM_WAIT_ON as VIA_WAIT_ON and use it
  drm/via: make via_drv.h self-contained
  drm/via: drop use of drmP.h

 drivers/gpu/drm/via/via_dma.c  | 43 +++-
 drivers/gpu/drm/via/via_dmablit.c  | 41 ++-
 drivers/gpu/drm/via/via_drv.c  |  7 +++-
 drivers/gpu/drm/via/via_drv.h  | 83 +++---
 drivers/gpu/drm/via/via_irq.c  | 54 +
 drivers/gpu/drm/via/via_map.c  |  6 ++-
 drivers/gpu/drm/via/via_mm.c   |  7 +++-
 drivers/gpu/drm/via/via_verifier.c | 22 +-
 drivers/gpu/drm/via/via_video.c|  5 ++-
 9 files changed, 182 insertions(+), 86 deletions(-)


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

[PATCH v3 4/4] drm/via: drop use of drmP.h

2019-07-22 Thread Sam Ravnborg
Drop use of the deprecated drmP.h header.
While touching the files divide include files in blocks
and sort the files alphabetically.

v2:
- Replace all uses of DRM_WAIT_ON() with VIA_WAIT_ON()
  and thus avoiding to pull in drm_os_linux.h

v3:
- DRM_WAIT_ON replacement moved to earlier patch (Emil)

Signed-off-by: Sam Ravnborg 
Reviewed-by: Emil Velikov 
Cc: Kevin Brace 
Cc: Thomas Hellstrom 
Cc: "Gustavo A. R. Silva" 
Cc: Mike Marshall 
Cc: Ira Weiny 
Cc: Daniel Vetter 
Cc: Michel Dänzer 
---
 drivers/gpu/drm/via/via_dma.c  |  9 -
 drivers/gpu/drm/via/via_dmablit.c  | 13 -
 drivers/gpu/drm/via/via_drv.c  |  7 +--
 drivers/gpu/drm/via/via_irq.c  |  4 +++-
 drivers/gpu/drm/via/via_map.c  |  6 +-
 drivers/gpu/drm/via/via_mm.c   |  7 ++-
 drivers/gpu/drm/via/via_verifier.c | 10 +-
 drivers/gpu/drm/via/via_video.c|  3 ++-
 8 files changed, 42 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/via/via_dma.c b/drivers/gpu/drm/via/via_dma.c
index 344a12b63967..1208445e341d 100644
--- a/drivers/gpu/drm/via/via_dma.c
+++ b/drivers/gpu/drm/via/via_dma.c
@@ -34,8 +34,15 @@
  *Thomas Hellstrom.
  */
 
-#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
 #include 
+
 #include "via_drv.h"
 #include "via_3d_reg.h"
 
diff --git a/drivers/gpu/drm/via/via_dmablit.c 
b/drivers/gpu/drm/via/via_dmablit.c
index 2ec93d976a43..feaa538026a0 100644
--- a/drivers/gpu/drm/via/via_dmablit.c
+++ b/drivers/gpu/drm/via/via_dmablit.c
@@ -34,13 +34,16 @@
  * the same DMA mappings?
  */
 
-#include 
-#include 
-#include "via_drv.h"
-#include "via_dmablit.h"
-
 #include 
 #include 
+#include 
+
+#include 
+#include 
+#include 
+
+#include "via_dmablit.h"
+#include "via_drv.h"
 
 #define VIA_PGDN(x) (((unsigned long)(x)) & PAGE_MASK)
 #define VIA_PGOFF(x)   (((unsigned long)(x)) & ~PAGE_MASK)
diff --git a/drivers/gpu/drm/via/via_drv.c b/drivers/gpu/drm/via/via_drv.c
index af6a12d3c058..666a16de84f9 100644
--- a/drivers/gpu/drm/via/via_drv.c
+++ b/drivers/gpu/drm/via/via_drv.c
@@ -24,11 +24,14 @@
 
 #include 
 
-#include 
+#include 
+#include 
+#include 
+#include 
 #include 
+
 #include "via_drv.h"
 
-#include 
 
 static int via_driver_open(struct drm_device *dev, struct drm_file *file)
 {
diff --git a/drivers/gpu/drm/via/via_irq.c b/drivers/gpu/drm/via/via_irq.c
index 5ac26d203f44..07a8d6b95a68 100644
--- a/drivers/gpu/drm/via/via_irq.c
+++ b/drivers/gpu/drm/via/via_irq.c
@@ -35,8 +35,10 @@
  * The refresh rate is also calculated for video playback sync purposes.
  */
 
-#include 
+#include 
+#include 
 #include 
+
 #include "via_drv.h"
 
 #define VIA_REG_INTERRUPT   0x200
diff --git a/drivers/gpu/drm/via/via_map.c b/drivers/gpu/drm/via/via_map.c
index 2ad865870372..431c150df014 100644
--- a/drivers/gpu/drm/via/via_map.c
+++ b/drivers/gpu/drm/via/via_map.c
@@ -21,8 +21,12 @@
  * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
  * DEALINGS IN THE SOFTWARE.
  */
-#include 
+
+#include 
+#include 
+#include 
 #include 
+
 #include "via_drv.h"
 
 static int via_do_init_map(struct drm_device *dev, drm_via_init_t *init)
diff --git a/drivers/gpu/drm/via/via_mm.c b/drivers/gpu/drm/via/via_mm.c
index 4217d66a5cc6..45cc9e900260 100644
--- a/drivers/gpu/drm/via/via_mm.c
+++ b/drivers/gpu/drm/via/via_mm.c
@@ -25,8 +25,13 @@
  * Authors: Thomas Hellström 
  */
 
-#include 
+#include 
+
+#include 
+#include 
+#include 
 #include 
+
 #include "via_drv.h"
 
 #define VIA_MM_ALIGN_SHIFT 4
diff --git a/drivers/gpu/drm/via/via_verifier.c 
b/drivers/gpu/drm/via/via_verifier.c
index 002c883b0d4b..8d8135f424ef 100644
--- a/drivers/gpu/drm/via/via_verifier.c
+++ b/drivers/gpu/drm/via/via_verifier.c
@@ -28,13 +28,13 @@
  * be very slow.
  */
 
-#include "via_3d_reg.h"
-#include 
-#include 
+#include 
 #include 
-#include "via_verifier.h"
+#include 
+
+#include "via_3d_reg.h"
 #include "via_drv.h"
-#include 
+#include "via_verifier.h"
 
 typedef enum {
state_command,
diff --git a/drivers/gpu/drm/via/via_video.c b/drivers/gpu/drm/via/via_video.c
index cf53612c945e..53b1f58f99b4 100644
--- a/drivers/gpu/drm/via/via_video.c
+++ b/drivers/gpu/drm/via/via_video.c
@@ -25,8 +25,9 @@
  * Video and XvMC related functions.
  */
 
-#include 
+#include 
 #include 
+
 #include "via_drv.h"
 
 void via_init_futex(drm_via_private_t *dev_priv)
-- 
2.20.1

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

Re: [PATCH v2 6/9] drm/tinydrm/Kconfig: drivers: Select BACKLIGHT_CLASS_DEVICE

2019-07-22 Thread David Lechner

On 7/22/19 5:43 AM, Noralf Trønnes wrote:

The mipi_dbi helper is missing a dependency on DRM_KMS_HELPER and putting
that in revealed this problem:

drivers/video/fbdev/Kconfig:12:error: recursive dependency detected!
drivers/video/fbdev/Kconfig:12: symbol FB is selected by DRM_KMS_FB_HELPER
drivers/gpu/drm/Kconfig:75: symbol DRM_KMS_FB_HELPER depends on 
DRM_KMS_HELPER
drivers/gpu/drm/Kconfig:69: symbol DRM_KMS_HELPER is selected by 
TINYDRM_MIPI_DBI
drivers/gpu/drm/tinydrm/Kconfig:11: symbol TINYDRM_MIPI_DBI is selected by 
TINYDRM_HX8357D
drivers/gpu/drm/tinydrm/Kconfig:15: symbol TINYDRM_HX8357D depends on 
BACKLIGHT_CLASS_DEVICE
drivers/video/backlight/Kconfig:144:symbol BACKLIGHT_CLASS_DEVICE is 
selected by FB_BACKLIGHT
drivers/video/fbdev/Kconfig:187:symbol FB_BACKLIGHT depends on FB

A symbol that selects DRM_KMS_HELPER can not depend on
BACKLIGHT_CLASS_DEVICE. The reason for this is that DRM_KMS_FB_HELPER
selects FB instead of depending on it.

The tinydrm drivers have somehow gotten away with depending on
BACKLIGHT_CLASS_DEVICE because DRM_TINYDRM selects DRM_KMS_HELPER and the
drivers depend on that symbol.

An audit shows that all DRM drivers that select DRM_KMS_HELPER and use
BACKLIGHT_CLASS_DEVICE, selects it:
   DRM_TILCDC, DRM_GMA500, DRM_SHMOBILE, DRM_NOUVEAU, DRM_FSL_DCU,
   DRM_I915, DRM_RADEON, DRM_AMDGPU, DRM_PARADE_PS8622

Documentation/kbuild/kconfig-language.txt has a note regarding select:
1. 'select should be used with care since it doesn't visit dependencies.'
This is not a problem since BACKLIGHT_CLASS_DEVICE doesn't have any
dependencies.
2. 'In general use select only for non-visible symbols'
BACKLIGHT_CLASS_DEVICE is user visible.

The real solution to this would be to have DRM_KMS_FB_HELPER depend on the
user visible symbol FB. That is a can of worms I'm not willing to tackle.
I fear that such a change will result in me handling difficult fallouts
for the next weeks. So I'm following DRM suite here.

Signed-off-by: Noralf Trønnes 
---


Reviewed-by: David Lechner 



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

[PATCH v3 2/4] drm/via: copy DRM_WAIT_ON as VIA_WAIT_ON and use it

2019-07-22 Thread Sam Ravnborg
VIA_WAIT_ON() is a direct copy of DRM_WAIT_ON() from
drm_os_linux.h.
The copy is made so we can avoid the dependency on the legacy header.
A more involved approach had been to introduce wait_event_* but for this
legacy driver the simpler and more safe approach with a copy of the
macro was selected.
Added the relevant header files for the functions used in VIA_WAIT_ON.

v3:
- Updated users of DRM_WAIT_ON => VIA_WAIT_ON (Emil)
- Updated $subject (Emil)

Signed-off-by: Sam Ravnborg 
Cc: Kevin Brace 
Cc: Thomas Hellstrom 
Cc: "Gustavo A. R. Silva" 
Cc: Mike Marshall 
Cc: Ira Weiny 
Cc: Daniel Vetter 
Cc: Emil Velikov 
Cc: Michel Dänzer 
---
 drivers/gpu/drm/via/via_dmablit.c |  4 +--
 drivers/gpu/drm/via/via_drv.h | 42 ++-
 drivers/gpu/drm/via/via_irq.c |  4 +--
 drivers/gpu/drm/via/via_video.c   |  2 +-
 4 files changed, 46 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/via/via_dmablit.c 
b/drivers/gpu/drm/via/via_dmablit.c
index e1557dd67083..2ec93d976a43 100644
--- a/drivers/gpu/drm/via/via_dmablit.c
+++ b/drivers/gpu/drm/via/via_dmablit.c
@@ -436,7 +436,7 @@ via_dmablit_sync(struct drm_device *dev, uint32_t handle, 
int engine)
int ret = 0;
 
if (via_dmablit_active(blitq, engine, handle, )) {
-   DRM_WAIT_ON(ret, *queue, 3 * HZ,
+   VIA_WAIT_ON(ret, *queue, 3 * HZ,
!via_dmablit_active(blitq, engine, handle, NULL));
}
DRM_DEBUG("DMA blit sync handle 0x%x engine %d returned %d\n",
@@ -687,7 +687,7 @@ via_dmablit_grab_slot(drm_via_blitq_t *blitq, int engine)
while (blitq->num_free == 0) {
spin_unlock_irqrestore(>blit_lock, irqsave);
 
-   DRM_WAIT_ON(ret, blitq->busy_queue, HZ, blitq->num_free > 0);
+   VIA_WAIT_ON(ret, blitq->busy_queue, HZ, blitq->num_free > 0);
if (ret)
return (-EINTR == ret) ? -EAGAIN : ret;
 
diff --git a/drivers/gpu/drm/via/via_drv.h b/drivers/gpu/drm/via/via_drv.h
index 8c16c208ae83..3eb92e81655e 100644
--- a/drivers/gpu/drm/via/via_drv.h
+++ b/drivers/gpu/drm/via/via_drv.h
@@ -24,8 +24,13 @@
 #ifndef _VIA_DRV_H_
 #define _VIA_DRV_H_
 
-#include 
+#include 
+#include 
+#include 
+#include 
+
 #include 
+#include 
 
 #define DRIVER_AUTHOR  "Various"
 
@@ -148,6 +153,41 @@ static inline void via_write8_mask_and(struct 
drm_via_private *dev_priv,
writeb(val & mask, (void __iomem *)(dev_priv->mmio->handle + reg));
 }
 
+/*
+ * Poll in a loop waiting for 'contidition' to be true.
+ * Note: A direct replacement with wait_event_interruptible_timeout()
+ *   will not work unless driver is updated to emit wake_up()
+ *   in relevant places that can impact the 'condition'
+ *
+ * Returns:
+ *   ret keeps current value if 'condition' becomes true
+ *   ret = -BUSY if timeout happens
+ *   ret = -EINTR if a signal interrupted the waiting period
+ */
+#define VIA_WAIT_ON( ret, queue, timeout, condition )  \
+do {   \
+   DECLARE_WAITQUEUE(entry, current);  \
+   unsigned long end = jiffies + (timeout);\
+   add_wait_queue(&(queue), );   \
+   \
+   for (;;) {  \
+   __set_current_state(TASK_INTERRUPTIBLE);\
+   if (condition)  \
+   break;  \
+   if (time_after_eq(jiffies, end)) {  \
+   ret = -EBUSY;   \
+   break;  \
+   }   \
+   schedule_timeout((HZ/100 > 1) ? HZ/100 : 1);\
+   if (signal_pending(current)) {  \
+   ret = -EINTR;   \
+   break;  \
+   }   \
+   }   \
+   __set_current_state(TASK_RUNNING);  \
+   remove_wait_queue(&(queue), );\
+} while (0)
+
 extern const struct drm_ioctl_desc via_ioctls[];
 extern int via_max_ioctl;
 
diff --git a/drivers/gpu/drm/via/via_irq.c b/drivers/gpu/drm/via/via_irq.c
index 0ee0b767c3d7..5ac26d203f44 100644
--- a/drivers/gpu/drm/via/via_irq.c
+++ b/drivers/gpu/drm/via/via_irq.c
@@ -233,12 +233,12 @@ via_driver_irq_wait(struct drm_device *dev, unsigned int 
irq, int force_sequence
cur_irq = dev_priv->via_irqs + real_irq;
 
if (masks[real_irq][2] && !force_sequence) {
-   DRM_WAIT_ON(ret, cur_irq->irq_queue, 3 * HZ,
+   VIA_WAIT_ON(ret, cur_irq->irq_queue, 3 * HZ,

[PATCH] video: fbdev: pvr2fb: remove unnecessary comparison of unsigned integer with < 0

2019-07-22 Thread Gustavo A. R. Silva
There is no need to compare *var->xoffset* or *var->yoffset* with < 0
because such variables are of type unsigned, making it impossible to
hold a negative value.

Fix this by removing such comparisons.

Addresses-Coverity-ID: 1451964 ("Unsigned compared against 0")
Signed-off-by: Gustavo A. R. Silva 
---
 drivers/video/fbdev/pvr2fb.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/video/fbdev/pvr2fb.c b/drivers/video/fbdev/pvr2fb.c
index 7ff4b6b84282..0a3b2b7c7891 100644
--- a/drivers/video/fbdev/pvr2fb.c
+++ b/drivers/video/fbdev/pvr2fb.c
@@ -458,13 +458,11 @@ static int pvr2fb_check_var(struct fb_var_screeninfo 
*var, struct fb_info *info)
set_color_bitfields(var);
 
if (var->vmode & FB_VMODE_YWRAP) {
-   if (var->xoffset || var->yoffset < 0 ||
-   var->yoffset >= var->yres_virtual) {
+   if (var->xoffset || var->yoffset >= var->yres_virtual) {
var->xoffset = var->yoffset = 0;
} else {
if (var->xoffset > var->xres_virtual - var->xres ||
-   var->yoffset > var->yres_virtual - var->yres ||
-   var->xoffset < 0 || var->yoffset < 0)
+   var->yoffset > var->yres_virtual - var->yres)
var->xoffset = var->yoffset = 0;
}
} else {
-- 
2.22.0



Re: [PATCH v2 9/9] MAINTAINERS: Remove tinydrm entry

2019-07-22 Thread David Lechner

On 7/22/19 5:43 AM, Noralf Trønnes wrote:

tinydrm is just a collection of tiny drivers now.
Add T: drm-misc entry for tinydrm drivers that lacks it.

Cc: David Lechner 
Reviewed-by: Sam Ravnborg 
Signed-off-by: Noralf Trønnes 
---


Acked-by: David Lechner 


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

Re: [PATCH v2 8/9] drm/tinydrm: Move mipi-dbi

2019-07-22 Thread David Lechner

On 7/22/19 5:43 AM, Noralf Trønnes wrote:

This moves mipi-dbi to be a core helper with the name drm_mipi_dbi.

Fixup include's in drivers.
Move the docs entry and delete tinydrm.rst.
Delete the last tinydrm todo entry.

v2: Make DRM_MIPI_DBI tristate to enable it being built as a module.

Cc: Eric Anholt 
Cc: David Lechner 
Reviewed-by: Sam Ravnborg 
Signed-off-by: Noralf Trønnes 
---


Acked-by: David Lechner 


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

[Bug 110671] Regression: DP outputs out of sync on dual-DP tiled 5k screen

2019-07-22 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=110671

--- Comment #9 from Denys  ---
Broken in 5.3-rc1

Workaround to revert 5fc0cbfad4564856ee0f323d3f88a7cff19cc3f1 is still working.

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

Re: [PATCH v2 4/9] drm/tinydrm: Split struct mipi_dbi in two

2019-07-22 Thread David Lechner

On 7/22/19 5:43 AM, Noralf Trønnes wrote:

Split struct mipi_dbi into an interface part and a display pipeline part.
The interface part can be used by drivers that need to initialize the
controller, but that won't upload the framebuffer over this interface.

MIPI DBI supports 3 interface types:
- A. Motorola 6800 type parallel bus
- B. Intel 8080 type parallel bus
- C. SPI type with 3 options:

I've embedded the SPI type specifics in the mipi_dbi struct to avoid
adding unnecessary complexity. If more interface types will be supported
in the future, the type specifics might have to be split out.

Rename functions to match the new struct mipi_dbi_dev:
- drm_to_mipi_dbi() -> drm_to_mipi_dbi_dev().
- mipi_dbi_init*() -> mipi_dbi_dev_init*().

Cc: Eric Anholt 
Cc: David Lechner 
Reviewed-by: Sam Ravnborg 
Signed-off-by: Noralf Trønnes 
---


Acked-by: David Lechner 


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

Re: [PATCH] Enable backlight when trigger is activated

2019-07-22 Thread Jacek Anaszewski
Hi Pavel,

On 7/22/19 9:50 AM, Pavel Machek wrote:
> Hi!
> 
>>> Configuring backlight trigger from dts results in backlight off during
>>> boot. Machine looks dead upon boot, which is not good.
>>>
>>> Fix that by enabling LED on trigger activation.
> 
>>> +++ b/drivers/leds/trigger/ledtrig-backlight.c
>>> @@ -114,6 +114,8 @@ static int bl_trig_activate(struct led_classdev *led)
>>> n->old_status = UNBLANK;
>>> n->notifier.notifier_call = fb_notifier_callback;
>>>  
>>> +   led_set_brightness(led, LED_ON);
>>> +
>>
>> This looks fishy.
>>
>> Maybe you should use a default-state = "keep" instead? (and you'll have
>> to support it in the LED driver).
>>
>> That'll give you proper "don't touch the LED if it was turned on" behavior,
>> which is what you seem to want.
> 
> Actually no, that's not what I want. LED should go on if the display
> is active, as soon as trigger is activated.
> 
> Unfortunately, I have see no good way to tell if the display is
> active (and display is usually active when trigger is activated).

default-state DT property can be also set to "on"
(see Documentation/devicetree/bindings/leds/common.txt).

You could make use of LED_INIT_DEFAULT_TRIGGER flag and
parse DT property in the activate op. Similar approach has been
applied e.g. in ledtrig-pattern.c.

-- 
Best regards,
Jacek Anaszewski


Re: [PATCH v2] drm/bridge: dw-hdmi: Refuse DDC/CI transfers on the internal I2C controller

2019-07-22 Thread Sean Paul
On Mon, Jul 22, 2019 at 11:19:45AM -0700, Matthias Kaehlcke wrote:
> The DDC/CI protocol involves sending a multi-byte request to the
> display via I2C, which is typically followed by a multi-byte
> response. The internal I2C controller only allows single byte
> reads/writes or reads of 8 sequential bytes, hence DDC/CI is not
> supported when the internal I2C controller is used. The I2C

This is very likely a stupid question, but I didn't see an answer for it, so
I'll just ask :)

If the controller supports xfers of 8 bytes and 1 bytes, could you just split
up any of these transactions into len/8+len%8 transactions?

Sean

> transfers complete without errors, however the data in the response
> is garbage. Abort transfers to/from slave address 0x37 (DDC) with
> -EOPNOTSUPP, to make it evident that the communication is failing.
> 
> Signed-off-by: Matthias Kaehlcke 
> ---
> Changes in v2:
> - changed DDC_I2C_ADDR to DDC_CI_ADDR
> ---
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c 
> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index 045b1b13fd0e..28933629f3c7 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -35,6 +35,7 @@
>  
>  #include 
>  
> +#define DDC_CI_ADDR  0x37
>  #define DDC_SEGMENT_ADDR 0x30
>  
>  #define HDMI_EDID_LEN512
> @@ -322,6 +323,13 @@ static int dw_hdmi_i2c_xfer(struct i2c_adapter *adap,
>   u8 addr = msgs[0].addr;
>   int i, ret = 0;
>  
> + if (addr == DDC_CI_ADDR)
> + /*
> +  * The internal I2C controller does not support the multi-byte
> +  * read and write operations needed for DDC/CI.
> +  */
> + return -EOPNOTSUPP;
> +
>   dev_dbg(hdmi->dev, "xfer: num: %d, addr: %#x\n", num, addr);
>  
>   for (i = 0; i < num; i++) {
> -- 
> 2.22.0.657.g960e92d24f-goog
> 

-- 
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 v2 3/9] drm/tinydrm: Rename remaining variable mipi -> dbidev

2019-07-22 Thread David Lechner

On 7/22/19 5:43 AM, Noralf Trønnes wrote:

struct mipi_dbi is going to be split into an interface part and a display
pipeline part. The interface part can be used by drivers that need to
initialize the controller, but that won't upload the framebuffer over
this interface.

tinydrm uses the variable name 'mipi' but this is not a good name since
MIPI refers to a lot of standards. This patch changes the variable name
to 'dbidev' where it refers to the pipeline part of struct mipi_dbi.

Cc: Eric Anholt 
Cc: David Lechner 
Reviewed-by: Sam Ravnborg 
Signed-off-by: Noralf Trønnes 
---


Acked-by: David Lechner 

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

Re: [PATCH v2 2/9] drm/tinydrm: Rename variable mipi -> dbi

2019-07-22 Thread David Lechner

On 7/22/19 5:43 AM, Noralf Trønnes wrote:

struct mipi_dbi is going to be split into an interface part and a display
pipeline part. The interface part can be used by drivers that need to
initialize the controller, but that won't upload the framebuffer over
this interface.

tinydrm uses the variable name 'mipi' but this is not a good name since
MIPI refers to a lot of standards. This patch changes the variable name
to 'dbi' where it refers to the interface part of struct mipi_dbi.

Functions that use both future parts will have both variables temporarily
pointing to the same structure.

Cc: Eric Anholt 
Cc: David Lechner 
Reviewed-by: Sam Ravnborg 
Signed-off-by: Noralf Trønnes 
---


Acked-by: David Lechner 


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

[Bug 105251] [Vega10] GPU lockup on boot: VMC page fault

2019-07-22 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=105251

--- Comment #74 from deltasquared  ---
(In reply to Juan A. Suarez from comment #73)
> It could be good if people could report here if this improved with this MR.

I can utilise the mesa-git package in the arch user repository to compile from
latest sources. I will then test both vega_crasher and minetest with that
package installed to see what occurs. Stay tuned for updates, though it may
take a couple of days while I juggle $dayjob.

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

Re: [PATCH v2] drm/bridge: dw-hdmi: Refuse DDC/CI transfers on the internal I2C controller

2019-07-22 Thread Doug Anderson
Hi,

On Mon, Jul 22, 2019 at 11:19 AM Matthias Kaehlcke  wrote:
>
> The DDC/CI protocol involves sending a multi-byte request to the
> display via I2C, which is typically followed by a multi-byte
> response. The internal I2C controller only allows single byte
> reads/writes or reads of 8 sequential bytes, hence DDC/CI is not
> supported when the internal I2C controller is used. The I2C
> transfers complete without errors, however the data in the response
> is garbage. Abort transfers to/from slave address 0x37 (DDC) with
> -EOPNOTSUPP, to make it evident that the communication is failing.
>
> Signed-off-by: Matthias Kaehlcke 
> ---
> Changes in v2:
> - changed DDC_I2C_ADDR to DDC_CI_ADDR
> ---
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 8 
>  1 file changed, 8 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c 
> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index 045b1b13fd0e..28933629f3c7 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -35,6 +35,7 @@
>
>  #include 
>
> +#define DDC_CI_ADDR0x37
>  #define DDC_SEGMENT_ADDR   0x30
>
>  #define HDMI_EDID_LEN  512
> @@ -322,6 +323,13 @@ static int dw_hdmi_i2c_xfer(struct i2c_adapter *adap,
> u8 addr = msgs[0].addr;
> int i, ret = 0;
>
> +   if (addr == DDC_CI_ADDR)
> +   /*
> +* The internal I2C controller does not support the multi-byte
> +* read and write operations needed for DDC/CI.
> +*/
> +   return -EOPNOTSUPP;
> +

In theory we could also solve this by detecting (in other parts of the
function) the bad multi-byte read/write operations.  That would maybe
be more generic (AKA it would more properly handle random userspace
tools fiddling with i2c-dev) but add complexity to the code.

...possibly a better long-term solution is to just totally stop
emulating a regular i2c adapter when the hardware just doesn't support
that.  In theory we could remove the "drm_get_edid()" call in
dw_hdmi_connector_get_modes() and replace it with a direct call to
drm_do_get_edid() if we're using the built-in adapter.  Possibly
"tda998x_drv.c" would be a good reference.  If we did that, we could
remove all the weird / hacky i2c code in this driver.


Since the bigger cleanup seems like a bit much to ask, I'm OK with
this as a stopgap to at least prevent userspace tools from getting
confused.  Thus:

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

Re: [PATCH] drm/bridge: Improve the help text for DRM_ANALOGIX_ANX78XX

2019-07-22 Thread Sean Paul
On Mon, Jul 22, 2019 at 04:40:49PM -0300, Fabio Estevam wrote:
> Improve the help text for DRM_ANALOGIX_ANX78XX by adding the missing
> "power" word.
> 
> After this change the help text matches with the ANX7814
> product description from the Analogix website:
> 
> https://www.analogix.com/en/products/convertersbridges/anx7814
> 
> Signed-off-by: Fabio Estevam 

Thanks for your patch! I've applied it to drm-misc-next

Sean

> ---
>  drivers/gpu/drm/bridge/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> index ee777469293a..a6eec908c43e 100644
> --- a/drivers/gpu/drm/bridge/Kconfig
> +++ b/drivers/gpu/drm/bridge/Kconfig
> @@ -21,7 +21,7 @@ config DRM_ANALOGIX_ANX78XX
>   select DRM_KMS_HELPER
>   select REGMAP_I2C
>   ---help---
> -   ANX78XX is an ultra-low Full-HD SlimPort transmitter
> +   ANX78XX is an ultra-low power Full-HD SlimPort transmitter
> designed for portable devices. The ANX78XX transforms
> the HDMI output of an application processor to MyDP
> or DisplayPort.
> -- 
> 2.17.1
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
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] drm: silence variable 'conn' set but not used

2019-07-22 Thread Sean Paul
On Mon, Jul 22, 2019 at 03:14:46PM -0400, Qian Cai wrote:
> The "struct drm_connector" iteration cursor from
> "for_each_new_connector_in_state" is never used in atomic_remove_fb()
> which generates a compilation warning,
> 
> drivers/gpu/drm/drm_framebuffer.c: In function 'atomic_remove_fb':
> drivers/gpu/drm/drm_framebuffer.c:838:24: warning: variable 'conn' set
> but not used [-Wunused-but-set-variable]
> 
> Silence it by marking "conn" __maybe_unused.
> 
> Signed-off-by: Qian Cai 

Thanks for your patch! I've applied it to drm-misc-fixes

Sean

> ---
>  drivers/gpu/drm/drm_framebuffer.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_framebuffer.c 
> b/drivers/gpu/drm/drm_framebuffer.c
> index 0b72468e8131..57564318ceea 100644
> --- a/drivers/gpu/drm/drm_framebuffer.c
> +++ b/drivers/gpu/drm/drm_framebuffer.c
> @@ -835,7 +835,7 @@ static int atomic_remove_fb(struct drm_framebuffer *fb)
>   struct drm_device *dev = fb->dev;
>   struct drm_atomic_state *state;
>   struct drm_plane *plane;
> - struct drm_connector *conn;
> + struct drm_connector *conn __maybe_unused;
>   struct drm_connector_state *conn_state;
>   int i, ret;
>   unsigned plane_mask;
> -- 
> 1.8.3.1
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS


Re: [PATCH v9 04/18] kunit: test: add kunit_stream a std::stream like logger

2019-07-22 Thread Stephen Boyd
Quoting Brendan Higgins (2019-07-18 17:08:34)
> On Thu, Jul 18, 2019 at 12:22:33PM -0700, Brendan Higgins wrote:
>
> I started poking around with your suggestion while we are waiting. A
> couple early observations:
> 
> 1) It is actually easier to do than I previously thought and will probably
>help with getting more of the planned TAP output stuff working.
> 
>That being said, this is still a pretty substantial undertaking and
>will likely take *at least* a week to implement and properly review.
>Assuming everything goes extremely well (no unexpected issues on my
>end, very responsive reviewers, etc).
> 
> 2) It *will* eliminate the need for kunit_stream.
> 
> 3) ...but, it *will not* eliminate the need for string_stream.
> 
> Based on my early observations, I do think it is worth doing, but I
> don't think it is worth trying to make it in this patchset (unless I
> have already missed the window, or it is going to be open for a while):

The merge window is over. Typically code needs to be settled a few weeks
before it opens (i.e. around -rc4 or -rc5) for most maintainers to pick
up patches for the next merge window.

> I do think it will make things much cleaner, but I don't think it will
> achieve your desired goal of getting rid of an unstructured
> {kunit|string}_stream style interface; it just adds a layer on top of it
> that makes it harder to misuse.

Ok.

> 
> I attached a patch of what I have so far at the end of this email so you
> can see what I am talking about. And of course, if you agree with my
> assessment, so we can start working on it as a future patch.
> 
> A couple things in regard to the patch I attached:
> 
> 1) I wrote it pretty quickly so there are almost definitely mistakes.
>You should consider it RFC. I did verify it compiles though.
> 
> 2) Also, I did use kunit_stream in writing it: all occurences should be
>pretty easy to replace with string_stream; nevertheless, the reason
>for this is just to make it easier to play with the current APIs. I
>wanted to have something working before I went through a big tedious
>refactoring. So sorry if it causes any confusion.
> 
> 3) I also based the patch on all the KUnit patches I have queued up
>(includes things like mocking and such) since I want to see how this
>serialization thing will work with mocks and matchers and things like
>that.

Great!

> 
> From 53d475d3d56afcf92b452c6d347dbedfa1a17d34 Mon Sep 17 00:00:00 2001
> From: Brendan Higgins 
> Date: Thu, 18 Jul 2019 16:08:52 -0700
> Subject: [PATCH v1] DO NOT MERGE: started playing around with the
>  serialization api
> 
> ---
>  include/kunit/assert.h | 130 ++
>  include/kunit/mock.h   |   4 +
>  kunit/Makefile |   3 +-
>  kunit/assert.c | 179 +
>  kunit/mock.c   |   6 +-
>  5 files changed, 318 insertions(+), 4 deletions(-)
>  create mode 100644 include/kunit/assert.h
>  create mode 100644 kunit/assert.c
> 
> diff --git a/include/kunit/assert.h b/include/kunit/assert.h
> new file mode 100644
> index 0..e054fdff4642f
> --- /dev/null
> +++ b/include/kunit/assert.h
> @@ -0,0 +1,130 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Assertion and expectation serialization API.
> + *
> + * Copyright (C) 2019, Google LLC.
> + * Author: Brendan Higgins 
> + */
> +
> +#ifndef _KUNIT_ASSERT_H
> +#define _KUNIT_ASSERT_H
> +
> +#include 
> +#include 
> +
> +enum kunit_assert_type {
> +   KUNIT_ASSERTION,
> +   KUNIT_EXPECTATION,
> +};
> +
> +struct kunit_assert {
> +   enum kunit_assert_type type;
> +   const char *line;
> +   const char *file;
> +   struct va_format message;
> +   void (*format)(struct kunit_assert *assert,
> +  struct kunit_stream *stream);

Would passing in the test help too?

> +};
> +
> +void kunit_base_assert_format(struct kunit_assert *assert,
> + struct kunit_stream *stream);
> +
> +void kunit_assert_print_msg(struct kunit_assert *assert,
> +   struct kunit_stream *stream);
> +
> +struct kunit_unary_assert {
> +   struct kunit_assert assert;
> +   const char *condition;
> +   bool expected_true;
> +};
> +
> +void kunit_unary_assert_format(struct kunit_assert *assert,
> +  struct kunit_stream *stream);
> +
> +struct kunit_ptr_not_err_assert {
> +   struct kunit_assert assert;
> +   const char *text;
> +   void *value;
> +};
> +
> +void kunit_ptr_not_err_assert_format(struct kunit_assert *assert,
> +struct kunit_stream *stream);
> +
> +struct kunit_binary_assert {
> +   struct kunit_assert assert;
> +   const char *operation;
> +   const char *left_text;
> +   long long left_value;
> +   const char *right_text;
> +   long long right_value;
> +};
> +
> +void kunit_binary_assert_format(struct 

Re: Need 5.3-rc1 in drm-misc-next

2019-07-22 Thread Noralf Trønnes


Den 22.07.2019 21.44, skrev Maxime Ripard:
> Hi!
> 
> On Mon, Jul 22, 2019 at 01:20:35PM +0200, Noralf Trønnes wrote:
>> Hi drm-misc maintainers,
>>
>> I have this series:
>>
>> drm/tinydrm: Remove tinydrm.ko
>> https://patchwork.freedesktop.org/series/63811/
>>
>> That depends on this -rc1 commit:
>>
>> e6f3f7e4dc76 ("spi: Add spi_is_bpw_supported()")
>>
>> I would would be nice if it would show up in drm-misc-next soon.
> 
> I just did it
> 

Thanks Maxime, that was fast!

Noralf.

> Maxime
> 
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
> 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 10/11] drm/tinydrm/mipi-dbi: Add mipi_dbi_init_with_formats()

2019-07-22 Thread David Lechner

On 7/19/19 10:59 AM, Noralf Trønnes wrote:

The MIPI DBI standard support more pixel formats than what this helper
supports. Add an init function that lets the driver use different
format(s). This avoids open coding mipi_dbi_init() in st7586.

st7586 sets preferred_depth but this is not necessary since it only
supports one format.

v2: Forgot to remove the mipi->rotation assignment in st7586,
 mipi_dbi_init_with_formats() handles it.

Cc: David Lechner 
Acked-by: David Lechner 
Signed-off-by: Noralf Trønnes 
---
  drivers/gpu/drm/tinydrm/mipi-dbi.c | 91 +-
  drivers/gpu/drm/tinydrm/st7586.c   | 33 ++-
  include/drm/tinydrm/mipi-dbi.h |  5 ++
  3 files changed, 74 insertions(+), 55 deletions(-)



Tested whole series on st7586. Seems to be still working.

Just putting Tested-by here since this patch is the only
one that changed st7586 significantly.

Tested-by: David Lechner 

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

Re: [PATCH] drm/amdkfd: Fix missing break in switch statement

2019-07-22 Thread Gustavo A. R. Silva


On 7/22/19 2:45 PM, Alex Deucher wrote:

>>
>> By the way, Alex, I'm planning to add these fixes to my tree. I want
>> to send a pull-request to Linus for v5.3-rc2 this afternoon. We want
>> to have the -Wimplicit-fallthrough option globally enabled in v5.3,
>> and these are some of the last fall-through warnings remaining in
>> the kernel.
>>
>> Can I have your Ack or Signed-off-by for all these drm patches?
> 
> I didn't realize you were sending these yourself. I was going to
> include them in my upcoming -fixes pull.  Feel free to add my RB to
> all three.
> 

Awesome! :)

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

Re: [PATCH] drm/amdkfd: Fix missing break in switch statement

2019-07-22 Thread Alex Deucher
On Mon, Jul 22, 2019 at 3:19 PM Gustavo A. R. Silva
 wrote:
>
>
>
> On 7/22/19 2:10 PM, Alex Deucher wrote:
> > On Sun, Jul 21, 2019 at 6:12 PM Gustavo A. R. Silva
> >  wrote:
> >>
> >> Add missing break statement in order to prevent the code from falling
> >> through to case CHIP_NAVI10.
> >>
> >> This bug was found thanks to the ongoing efforts to enable
> >> -Wimplicit-fallthrough.
> >>
> >> Fixes: 14328aa58ce5 ("drm/amdkfd: Add navi10 support to amdkfd. (v3)")
> >> Cc: sta...@vger.kernel.org
> >> Signed-off-by: Gustavo A. R. Silva 
> >
> > Applied.  Thanks!
> >
>
> By the way, Alex, I'm planning to add these fixes to my tree. I want
> to send a pull-request to Linus for v5.3-rc2 this afternoon. We want
> to have the -Wimplicit-fallthrough option globally enabled in v5.3,
> and these are some of the last fall-through warnings remaining in
> the kernel.
>
> Can I have your Ack or Signed-off-by for all these drm patches?

I didn't realize you were sending these yourself. I was going to
include them in my upcoming -fixes pull.  Feel free to add my RB to
all three.

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

Re: Need 5.3-rc1 in drm-misc-next

2019-07-22 Thread Maxime Ripard
Hi!

On Mon, Jul 22, 2019 at 01:20:35PM +0200, Noralf Trønnes wrote:
> Hi drm-misc maintainers,
>
> I have this series:
>
> drm/tinydrm: Remove tinydrm.ko
> https://patchwork.freedesktop.org/series/63811/
>
> That depends on this -rc1 commit:
>
> e6f3f7e4dc76 ("spi: Add spi_is_bpw_supported()")
>
> I would would be nice if it would show up in drm-misc-next soon.

I just did it

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


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

[PATCH] drm/bridge: Improve the help text for DRM_ANALOGIX_ANX78XX

2019-07-22 Thread Fabio Estevam
Improve the help text for DRM_ANALOGIX_ANX78XX by adding the missing
"power" word.

After this change the help text matches with the ANX7814
product description from the Analogix website:

https://www.analogix.com/en/products/convertersbridges/anx7814

Signed-off-by: Fabio Estevam 
---
 drivers/gpu/drm/bridge/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index ee777469293a..a6eec908c43e 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -21,7 +21,7 @@ config DRM_ANALOGIX_ANX78XX
select DRM_KMS_HELPER
select REGMAP_I2C
---help---
- ANX78XX is an ultra-low Full-HD SlimPort transmitter
+ ANX78XX is an ultra-low power Full-HD SlimPort transmitter
  designed for portable devices. The ANX78XX transforms
  the HDMI output of an application processor to MyDP
  or DisplayPort.
-- 
2.17.1

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

Re: [PATCH] drm/msm: stop abusing dma_map/unmap for cache

2019-07-22 Thread Sean Paul
On Sun, Jun 30, 2019 at 05:47:22AM -0700, Rob Clark wrote:
> From: Rob Clark 
> 
> Recently splats like this started showing up:
> 
>WARNING: CPU: 4 PID: 251 at drivers/iommu/dma-iommu.c:451 
> __iommu_dma_unmap+0xb8/0xc0
>Modules linked in: ath10k_snoc ath10k_core fuse msm ath mac80211 uvcvideo 
> cfg80211 videobuf2_vmalloc videobuf2_memops vide
>CPU: 4 PID: 251 Comm: kworker/u16:4 Tainted: GW 
> 5.2.0-rc5-next-20190619+ #2317
>Hardware name: LENOVO 81JL/LNVNB161216, BIOS 9UCN23WW(V1.06) 10/25/2018
>Workqueue: msm msm_gem_free_work [msm]
>pstate: 80c5 (Nzcv daif +PAN +UAO)
>pc : __iommu_dma_unmap+0xb8/0xc0
>lr : __iommu_dma_unmap+0x54/0xc0
>sp : 119abce0
>x29: 119abce0 x28: 
>x27: 8001f9946648 x26: 8001ec271068
>x25:  x24: 8001ea3580a8
>x23: 8001f95ba010 x22: 80018e83ba88
>x21: 8001e548f000 x20: f000
>x19: 1000 x18: c1fe
>x17:  x16: 
>x15: 15b70068 x14: 0005
>x13: 0003142cc1be1768 x12: 0001
>x11: 8001f6de9100 x10: 0009
>x9 : 15b78000 x8 : 
>x7 : 0001 x6 : f000
>x5 : 0fff x4 : 1065dbc8
>x3 : 000d x2 : 1000
>x1 : f000 x0 : 
>Call trace:
> __iommu_dma_unmap+0xb8/0xc0
> iommu_dma_unmap_sg+0x98/0xb8
> put_pages+0x5c/0xf0 [msm]
> msm_gem_free_work+0x10c/0x150 [msm]
> process_one_work+0x1e0/0x330
> worker_thread+0x40/0x438
> kthread+0x12c/0x130
> ret_from_fork+0x10/0x18
>---[ end trace afc0dc5ab81a06bf ]---
> 
> Not quite sure what triggered that, but we really shouldn't be abusing
> dma_{map,unmap}_sg() for cache maint.
> 
> Signed-off-by: Rob Clark 
> Cc: Stephen Boyd 

Applied to -misc-fixes

Thanks,

Sean

> ---
>  drivers/gpu/drm/msm/msm_gem.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
> index d31d9f927887..3b84cbdcafa3 100644
> --- a/drivers/gpu/drm/msm/msm_gem.c
> +++ b/drivers/gpu/drm/msm/msm_gem.c
> @@ -108,7 +108,7 @@ static struct page **get_pages(struct drm_gem_object *obj)
>* because display controller, GPU, etc. are not coherent:
>*/
>   if (msm_obj->flags & (MSM_BO_WC|MSM_BO_UNCACHED))
> - dma_map_sg(dev->dev, msm_obj->sgt->sgl,
> + dma_sync_sg_for_device(dev->dev, msm_obj->sgt->sgl,
>   msm_obj->sgt->nents, DMA_BIDIRECTIONAL);
>   }
>  
> @@ -138,7 +138,7 @@ static void put_pages(struct drm_gem_object *obj)
>* GPU, etc. are not coherent:
>*/
>   if (msm_obj->flags & (MSM_BO_WC|MSM_BO_UNCACHED))
> - dma_unmap_sg(obj->dev->dev, msm_obj->sgt->sgl,
> + dma_sync_sg_for_cpu(obj->dev->dev, 
> msm_obj->sgt->sgl,
>msm_obj->sgt->nents,
>DMA_BIDIRECTIONAL);
>  
> -- 
> 2.20.1
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS


Re: [PATCH] drm/amdgpu/gfx10: Fix missing break in switch statement

2019-07-22 Thread Gustavo A. R. Silva


On 7/22/19 2:12 PM, Alex Deucher wrote:
> On Sun, Jul 21, 2019 at 6:39 PM Gustavo A. R. Silva
>  wrote:
>>
>> Add missing break statement in order to prevent the code from falling
>> through to case AMDGPU_IRQ_STATE_ENABLE.
>>
>> This bug was found thanks to the ongoing efforts to enable
>> -Wimplicit-fallthrough.
>>
>> Fixes: a644d85a5cd4 ("drm/amdgpu: add gfx v10 implementation (v10)")
>> Cc: sta...@vger.kernel.org
>> Signed-off-by: Gustavo A. R. Silva 
> 
> Applied.  Thanks!
> 

Awesome! Glad to help. :)

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

[PATCH] drm: silence variable 'conn' set but not used

2019-07-22 Thread Qian Cai
The "struct drm_connector" iteration cursor from
"for_each_new_connector_in_state" is never used in atomic_remove_fb()
which generates a compilation warning,

drivers/gpu/drm/drm_framebuffer.c: In function 'atomic_remove_fb':
drivers/gpu/drm/drm_framebuffer.c:838:24: warning: variable 'conn' set
but not used [-Wunused-but-set-variable]

Silence it by marking "conn" __maybe_unused.

Signed-off-by: Qian Cai 
---
 drivers/gpu/drm/drm_framebuffer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_framebuffer.c 
b/drivers/gpu/drm/drm_framebuffer.c
index 0b72468e8131..57564318ceea 100644
--- a/drivers/gpu/drm/drm_framebuffer.c
+++ b/drivers/gpu/drm/drm_framebuffer.c
@@ -835,7 +835,7 @@ static int atomic_remove_fb(struct drm_framebuffer *fb)
struct drm_device *dev = fb->dev;
struct drm_atomic_state *state;
struct drm_plane *plane;
-   struct drm_connector *conn;
+   struct drm_connector *conn __maybe_unused;
struct drm_connector_state *conn_state;
int i, ret;
unsigned plane_mask;
-- 
1.8.3.1



Re: [PATCH] drm/amdkfd/kfd_mqd_manager_v10: Avoid fall-through warning

2019-07-22 Thread Alex Deucher
Applied.  Thanks!

Alex

On Mon, Jul 22, 2019 at 2:14 PM Liu, Shaoyun  wrote:
>
> Reviewed-by:  shaoyunl 
>
> On 2019-07-22 1:47 p.m., Gustavo A. R. Silva wrote:
> > In preparation to enabling -Wimplicit-fallthrough, this patch silences
> > the following warning:
> >
> > drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_mqd_manager_v10.c: In function 
> > ‘mqd_manager_init_v10’:
> > ./include/linux/dynamic_debug.h:122:52: warning: this statement may fall 
> > through [-Wimplicit-fallthrough=]
> >   #define __dynamic_func_call(id, fmt, func, ...) do { \
> >  ^
> > ./include/linux/dynamic_debug.h:143:2: note: in expansion of macro 
> > ‘__dynamic_func_call’
> >__dynamic_func_call(__UNIQUE_ID(ddebug), fmt, func, ##__VA_ARGS__)
> >^~~
> > ./include/linux/dynamic_debug.h:153:2: note: in expansion of macro 
> > ‘_dynamic_func_call’
> >_dynamic_func_call(fmt, __dynamic_pr_debug,  \
> >^~
> > ./include/linux/printk.h:336:2: note: in expansion of macro 
> > ‘dynamic_pr_debug’
> >dynamic_pr_debug(fmt, ##__VA_ARGS__)
> >^~~~
> > drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_mqd_manager_v10.c:432:3: note: in 
> > expansion of macro ‘pr_debug’
> > pr_debug("%s@%i\n", __func__, __LINE__);
> > ^~~~
> > drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_mqd_manager_v10.c:433:2: note: here
> >case KFD_MQD_TYPE_COMPUTE:
> >^~~~
> >
> > by removing the call to pr_debug() in KFD_MQD_TYPE_CP:
> >
> > "The mqd init for CP and COMPUTE will have the same  routine." [1]
> >
> > This bug was found thanks to the ongoing efforts to enable
> > -Wimplicit-fallthrough.
> >
> > [1] 
> > https://lore.kernel.org/lkml/c735a1cc-a545-50fb-44e7-c0ad93ee8...@amd.com/
> >
> > Signed-off-by: Gustavo A. R. Silva 
> > ---
> >   drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v10.c | 1 -
> >   1 file changed, 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v10.c 
> > b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v10.c
> > index 4f8a6ffc5775..9cd3eb2d90bd 100644
> > --- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v10.c
> > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v10.c
> > @@ -429,7 +429,6 @@ struct mqd_manager *mqd_manager_init_v10(enum 
> > KFD_MQD_TYPE type,
> >
> >   switch (type) {
> >   case KFD_MQD_TYPE_CP:
> > - pr_debug("%s@%i\n", __func__, __LINE__);
> >   case KFD_MQD_TYPE_COMPUTE:
> >   pr_debug("%s@%i\n", __func__, __LINE__);
> >   mqd->allocate_mqd = allocate_mqd;
> ___
> 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 3/3] drm: drop DRM_AUTH from PRIME_TO/FROM_HANDLE ioctls

2019-07-22 Thread Koenig, Christian
Am 22.07.19 um 19:40 schrieb Emil Velikov:
> From: Emil Velikov 
>
> As mentioned by Christian, for drivers which support only primary nodes
> this changes the returned error from -EACCES into -EOPNOTSUPP/-ENOSYS.
>
> For others, this check in particular will be a noop. So let's remove it
> as suggested by Christian.
>
> Cc: Alex Deucher 
> Cc: Christian König 
> Cc: amd-...@lists.freedesktop.org
> Cc: Daniel Vetter 
> Signed-off-by: Emil Velikov 

Looks like I can't convince you that opening up the primary node like 
this is a bad idea.

Well at least we don't need to add any new flags, so feel free to add my 
Acked-by.

Christian.

> ---
>   drivers/gpu/drm/drm_ioctl.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index 09f7f8e33fa3..a397177af369 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -647,8 +647,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
>   
>   DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETRESOURCES, drm_mode_getresources, 0),
>   
> - DRM_IOCTL_DEF(DRM_IOCTL_PRIME_HANDLE_TO_FD, 
> drm_prime_handle_to_fd_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
> - DRM_IOCTL_DEF(DRM_IOCTL_PRIME_FD_TO_HANDLE, 
> drm_prime_fd_to_handle_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
> + DRM_IOCTL_DEF(DRM_IOCTL_PRIME_HANDLE_TO_FD, 
> drm_prime_handle_to_fd_ioctl, DRM_RENDER_ALLOW),
> + DRM_IOCTL_DEF(DRM_IOCTL_PRIME_FD_TO_HANDLE, 
> drm_prime_fd_to_handle_ioctl, DRM_RENDER_ALLOW),
>   
>   DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPLANERESOURCES, drm_mode_getplane_res, 
> 0),
>   DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETCRTC, drm_mode_getcrtc, 0),

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

Re: [PATCH] drm/amdgpu/gfx10: Fix missing break in switch statement

2019-07-22 Thread Alex Deucher
On Sun, Jul 21, 2019 at 6:39 PM Gustavo A. R. Silva
 wrote:
>
> Add missing break statement in order to prevent the code from falling
> through to case AMDGPU_IRQ_STATE_ENABLE.
>
> This bug was found thanks to the ongoing efforts to enable
> -Wimplicit-fallthrough.
>
> Fixes: a644d85a5cd4 ("drm/amdgpu: add gfx v10 implementation (v10)")
> Cc: sta...@vger.kernel.org
> Signed-off-by: Gustavo A. R. Silva 

Applied.  Thanks!

Alex

> ---
>  drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c 
> b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> index 1675d5837c3c..35e8e29139b1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> @@ -4611,6 +4611,7 @@ gfx_v10_0_set_gfx_eop_interrupt_state(struct 
> amdgpu_device *adev,
> cp_int_cntl = REG_SET_FIELD(cp_int_cntl, CP_INT_CNTL_RING0,
> TIME_STAMP_INT_ENABLE, 0);
> WREG32(cp_int_cntl_reg, cp_int_cntl);
> +   break;
> case AMDGPU_IRQ_STATE_ENABLE:
> cp_int_cntl = RREG32(cp_int_cntl_reg);
> cp_int_cntl = REG_SET_FIELD(cp_int_cntl, CP_INT_CNTL_RING0,
> --
> 2.22.0
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [v3 1/4] dt-bindngs: display: panel: Add BOE tv101wum-n16 panel bindings

2019-07-22 Thread Uwe Kleine-König
$Subject ~= s/bindngs/bindings/

Best regards
Uwe

-- 
Pengutronix e.K.   | Uwe Kleine-König|
Industrial Linux Solutions | http://www.pengutronix.de/  |
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] drivers/gpu/drm/via: convert put_page() to put_user_page*()

2019-07-22 Thread John Hubbard
On 7/22/19 12:07 PM, Matthew Wilcox wrote:
> On Mon, Jul 22, 2019 at 11:53:54AM -0700, John Hubbard wrote:
>> On 7/22/19 2:33 AM, Christoph Hellwig wrote:
>>> On Sun, Jul 21, 2019 at 09:30:10PM -0700, john.hubb...@gmail.com wrote:
for (i = 0; i < vsg->num_pages; ++i) {
if (NULL != (page = vsg->pages[i])) {
if (!PageReserved(page) && (DMA_FROM_DEVICE == 
 vsg->direction))
 -  SetPageDirty(page);
 -  put_page(page);
 +  put_user_pages_dirty(, 1);
 +  else
 +  put_user_page(page);
}
>>>
>>> Can't just pass a dirty argument to put_user_pages?  Also do we really
>>
>> Yes, and in fact that would help a lot more than the single page case,
>> which is really just cosmetic after all.
>>
>>> need a separate put_user_page for the single page case?
>>> put_user_pages_dirty?
>>
>> Not really. I'm still zeroing in on the ideal API for all these call sites,
>> and I agree that the approach below is cleaner.
> 
> so enum { CLEAN = 0, DIRTY = 1, LOCK = 2, DIRTY_LOCK = 3 };
> ?
> 

Sure. In fact, I just applied something similar to bio_release_pages()
locally, in order to reconcile Christoph's and Jerome's approaches 
(they each needed to add a bool arg), so I'm all about the enums in the
arg lists. :)

I'm going to post that one shortly, let's see how it goes over. heh.

thanks,
-- 
John Hubbard
NVIDIA


Re: [PATCH] drm/amdkfd: Fix missing break in switch statement

2019-07-22 Thread Alex Deucher
On Sun, Jul 21, 2019 at 6:12 PM Gustavo A. R. Silva
 wrote:
>
> Add missing break statement in order to prevent the code from falling
> through to case CHIP_NAVI10.
>
> This bug was found thanks to the ongoing efforts to enable
> -Wimplicit-fallthrough.
>
> Fixes: 14328aa58ce5 ("drm/amdkfd: Add navi10 support to amdkfd. (v3)")
> Cc: sta...@vger.kernel.org
> Signed-off-by: Gustavo A. R. Silva 

Applied.  Thanks!

Alex

> ---
>  drivers/gpu/drm/amd/amdkfd/kfd_crat.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c 
> b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
> index 792371442195..4e3fc284f6ac 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
> @@ -668,6 +668,7 @@ static int kfd_fill_gpu_cache_info(struct kfd_dev *kdev,
> case CHIP_RAVEN:
> pcache_info = raven_cache_info;
> num_of_cache_types = ARRAY_SIZE(raven_cache_info);
> +   break;
> case CHIP_NAVI10:
> pcache_info = navi10_cache_info;
> num_of_cache_types = ARRAY_SIZE(navi10_cache_info);
> --
> 2.22.0
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] drivers/gpu/drm/via: convert put_page() to put_user_page*()

2019-07-22 Thread Matthew Wilcox
On Mon, Jul 22, 2019 at 11:53:54AM -0700, John Hubbard wrote:
> On 7/22/19 2:33 AM, Christoph Hellwig wrote:
> > On Sun, Jul 21, 2019 at 09:30:10PM -0700, john.hubb...@gmail.com wrote:
> >>for (i = 0; i < vsg->num_pages; ++i) {
> >>if (NULL != (page = vsg->pages[i])) {
> >>if (!PageReserved(page) && (DMA_FROM_DEVICE == 
> >> vsg->direction))
> >> -  SetPageDirty(page);
> >> -  put_page(page);
> >> +  put_user_pages_dirty(, 1);
> >> +  else
> >> +  put_user_page(page);
> >>}
> > 
> > Can't just pass a dirty argument to put_user_pages?  Also do we really
> 
> Yes, and in fact that would help a lot more than the single page case,
> which is really just cosmetic after all.
> 
> > need a separate put_user_page for the single page case?
> > put_user_pages_dirty?
> 
> Not really. I'm still zeroing in on the ideal API for all these call sites,
> and I agree that the approach below is cleaner.

so enum { CLEAN = 0, DIRTY = 1, LOCK = 2, DIRTY_LOCK = 3 };
?


Re: [PATCH 3/3] gup: new put_user_page_dirty*() helpers

2019-07-22 Thread John Hubbard
On 7/21/19 9:30 PM, john.hubb...@gmail.com wrote:
> From: John Hubbard 
> 
> While converting call sites to use put_user_page*() [1], quite a few
> places ended up needing a single-page routine to put and dirty a
> page.
> 
> Provide put_user_page_dirty() and put_user_page_dirty_lock(),
> and use them in a few places: net/xdp, drm/via/, drivers/infiniband.
> 

Please disregard this one, I'm going to drop it, as per the
discussion in patch 1.

thanks,
-- 
John Hubbard
NVIDIA

> Cc: Jason Gunthorpe 
> Cc: Jan Kara 
> Signed-off-by: John Hubbard 
> ---
>  drivers/gpu/drm/via/via_dmablit.c|  2 +-
>  drivers/infiniband/core/umem.c   |  2 +-
>  drivers/infiniband/hw/usnic/usnic_uiom.c |  2 +-
>  include/linux/mm.h   | 10 ++
>  net/xdp/xdp_umem.c   |  2 +-
>  5 files changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/via/via_dmablit.c 
> b/drivers/gpu/drm/via/via_dmablit.c
> index 219827ae114f..d30b2d75599f 100644
> --- a/drivers/gpu/drm/via/via_dmablit.c
> +++ b/drivers/gpu/drm/via/via_dmablit.c
> @@ -189,7 +189,7 @@ via_free_sg_info(struct pci_dev *pdev, drm_via_sg_info_t 
> *vsg)
>   for (i = 0; i < vsg->num_pages; ++i) {
>   if (NULL != (page = vsg->pages[i])) {
>   if (!PageReserved(page) && (DMA_FROM_DEVICE == 
> vsg->direction))
> - put_user_pages_dirty(, 1);
> + put_user_page_dirty(page);
>   else
>   put_user_page(page);
>   }
> diff --git a/drivers/infiniband/core/umem.c b/drivers/infiniband/core/umem.c
> index 08da840ed7ee..a7337cc3ca20 100644
> --- a/drivers/infiniband/core/umem.c
> +++ b/drivers/infiniband/core/umem.c
> @@ -55,7 +55,7 @@ static void __ib_umem_release(struct ib_device *dev, struct 
> ib_umem *umem, int d
>   for_each_sg_page(umem->sg_head.sgl, _iter, umem->sg_nents, 0) {
>   page = sg_page_iter_page(_iter);
>   if (umem->writable && dirty)
> - put_user_pages_dirty_lock(, 1);
> + put_user_page_dirty_lock(page);
>   else
>   put_user_page(page);
>   }
> diff --git a/drivers/infiniband/hw/usnic/usnic_uiom.c 
> b/drivers/infiniband/hw/usnic/usnic_uiom.c
> index 0b0237d41613..d2ded624fb2a 100644
> --- a/drivers/infiniband/hw/usnic/usnic_uiom.c
> +++ b/drivers/infiniband/hw/usnic/usnic_uiom.c
> @@ -76,7 +76,7 @@ static void usnic_uiom_put_pages(struct list_head 
> *chunk_list, int dirty)
>   page = sg_page(sg);
>   pa = sg_phys(sg);
>   if (dirty)
> - put_user_pages_dirty_lock(, 1);
> + put_user_page_dirty_lock(page);
>   else
>   put_user_page(page);
>   usnic_dbg("pa: %pa\n", );
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 0334ca97c584..c0584c6d9d78 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -1061,6 +1061,16 @@ void put_user_pages_dirty(struct page **pages, 
> unsigned long npages);
>  void put_user_pages_dirty_lock(struct page **pages, unsigned long npages);
>  void put_user_pages(struct page **pages, unsigned long npages);
>  
> +static inline void put_user_page_dirty(struct page *page)
> +{
> + put_user_pages_dirty(, 1);
> +}
> +
> +static inline void put_user_page_dirty_lock(struct page *page)
> +{
> + put_user_pages_dirty_lock(, 1);
> +}
> +
>  #if defined(CONFIG_SPARSEMEM) && !defined(CONFIG_SPARSEMEM_VMEMMAP)
>  #define SECTION_IN_PAGE_FLAGS
>  #endif
> diff --git a/net/xdp/xdp_umem.c b/net/xdp/xdp_umem.c
> index 9cbbb96c2a32..1d122e52c6de 100644
> --- a/net/xdp/xdp_umem.c
> +++ b/net/xdp/xdp_umem.c
> @@ -171,7 +171,7 @@ static void xdp_umem_unpin_pages(struct xdp_umem *umem)
>   for (i = 0; i < umem->npgs; i++) {
>   struct page *page = umem->pgs[i];
>  
> - put_user_pages_dirty_lock(, 1);
> + put_user_page_dirty_lock(page);
>   }
>  
>   kfree(umem->pgs);
> 


Re: [PATCH 0/6] drm/tinydrm: Move mipi_dbi

2019-07-22 Thread Noralf Trønnes


Den 22.07.2019 20.06, skrev Eric Anholt:
> Noralf Trønnes  writes:
> 
>> This series ticks off the last tinydrm todo entry and moves out mipi_dbi
>> to be a core helper.
>>
>> It splits struct mipi_dbi into an interface part and a display pipeline
>> part (upload framebuffer over SPI). I also took the opportunity to
>> rename the ambiguous 'mipi' variable name to 'dbi'. This lines up with
>> the use of the 'dsi' variable name in the MIPI DSI helper.
>>
>> Note:
>> This depends on series: drm/tinydrm: Remove tinydrm.ko
>>
>> Series is also available here:
>> https://github.com/notro/linux/tree/move_mipi_dbi
> 
> Congratulations on making it to this stage.  This looks like a fine
> conclusion to tinydrm.
> 

Thanks, it took a while, but really nice to finally get here.

There are only two patches left to do now, one to remove the menuconfig
so the drivers are visible by default and people can start putting their
tiny drivers here, and secondly to change the folder name to 'tiny' as
Daniel wants it.

> Acked-by: Eric Anholt 
> 

Thanks, care to take a look at version 2 of the series? I had to add 3
patches to deal with a kconfig dependency that I had missed out on.

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

Re: [PATCH 1/3] drivers/gpu/drm/via: convert put_page() to put_user_page*()

2019-07-22 Thread John Hubbard
On 7/22/19 2:33 AM, Christoph Hellwig wrote:
> On Sun, Jul 21, 2019 at 09:30:10PM -0700, john.hubb...@gmail.com wrote:
>>  for (i = 0; i < vsg->num_pages; ++i) {
>>  if (NULL != (page = vsg->pages[i])) {
>>  if (!PageReserved(page) && (DMA_FROM_DEVICE == 
>> vsg->direction))
>> -SetPageDirty(page);
>> -put_page(page);
>> +put_user_pages_dirty(, 1);
>> +else
>> +put_user_page(page);
>>  }
> 
> Can't just pass a dirty argument to put_user_pages?  Also do we really

Yes, and in fact that would help a lot more than the single page case,
which is really just cosmetic after all.

> need a separate put_user_page for the single page case?
> put_user_pages_dirty?

Not really. I'm still zeroing in on the ideal API for all these call sites,
and I agree that the approach below is cleaner.

> 
> Also the PageReserved check looks bogus, as I can't see how a reserved
> page can end up here.  So IMHO the above snippled should really look
> something like this:
> 
>   put_user_pages(vsg->pages[i], vsg->num_pages,
>   vsg->direction == DMA_FROM_DEVICE);
> 
> in the end.
> 

Agreed.

thanks,
-- 
John Hubbard
NVIDIA


Re: [PATCH v3 6/7] dt-bindings: Add ANX6345 DP/eDP transmitter binding

2019-07-22 Thread Vasily Khoruzhick
On Mon, Jul 22, 2019 at 8:12 AM Torsten Duwe  wrote:
>
> The anx6345 is an ultra-low power DisplayPort/eDP transmitter designed
> for portable devices.
>
> Add a binding document for it.

I believe you'll have to convert it to yaml format.

>
> Signed-off-by: Icenowy Zheng 
> Signed-off-by: Vasily Khoruzhick 
> Reviewed-by: Rob Herring 
> Signed-off-by: Torsten Duwe 
> Reviewed-by: Laurent Pinchart 
> ---
>  .../devicetree/bindings/display/bridge/anx6345.txt | 57 
> ++
>  1 file changed, 57 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/display/bridge/anx6345.txt
>
> diff --git a/Documentation/devicetree/bindings/display/bridge/anx6345.txt 
> b/Documentation/devicetree/bindings/display/bridge/anx6345.txt
> new file mode 100644
> index ..0af092d101c5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/anx6345.txt
> @@ -0,0 +1,57 @@
> +Analogix ANX6345 eDP Transmitter
> +
> +
> +The ANX6345 is an ultra-low power Full-HD eDP transmitter designed for
> +portable devices.
> +
> +Required properties:
> +
> + - compatible  : "analogix,anx6345"
> + - reg : I2C address of the device
> + - reset-gpios : Which GPIO to use for reset (active low)
> + - dvdd12-supply   : Regulator for 1.2V digital core power.
> + - dvdd25-supply   : Regulator for 2.5V digital core power.
> + - Video port 0 for LVTTL input, using the DT bindings defined in [1].
> +
> +Optional properties:
> +
> + - Video port 1 for eDP output (panel or connector) using the DT bindings
> +   defined in [1].
> +
> +[1]: Documentation/devicetree/bindings/media/video-interfaces.txt
> +
> +Example:
> +
> +anx6345: anx6345@38 {
> +   compatible = "analogix,anx6345";
> +   reg = <0x38>;
> +   reset-gpios = < 3 24 GPIO_ACTIVE_LOW>; /* PD24 */
> +   dvdd25-supply = <_dldo2>;
> +   dvdd12-supply = <_fldo1>;
> +
> +   ports {
> +   #address-cells = <1>;
> +   #size-cells = <0>;
> +
> +   anx6345_in: port@0 {
> +   #address-cells = <1>;
> +   #size-cells = <0>;
> +   reg = <0>;
> +   anx6345_in_tcon0: endpoint@0 {
> +   reg = <0>;
> +   remote-endpoint = <_out_anx6345>;
> +   };
> +   };
> +
> +   anx6345_out: port@1 {
> +   #address-cells = <1>;
> +   #size-cells = <0>;
> +   reg = <1>;
> +
> +   anx6345_out_panel: endpoint@0 {
> +   reg = <0>;
> +   remote-endpoint = <_in_edp>;
> +   };
> +   };
> +   };
> +};
> --
> 2.16.4
>


Re: [PATCH v3 5/7] drm/bridge: Add Analogix anx6345 support

2019-07-22 Thread Vasily Khoruzhick
On Mon, Jul 22, 2019 at 8:11 AM Torsten Duwe  wrote:
>
> From: Icenowy Zheng 
>
> The ANX6345 is an ultra-low power DisplayPower/eDP transmitter designed
> for portable devices. This driver adds initial support for RGB to eDP
> mode, without HPD and interrupts.
>
> This is a configuration usually seen in eDP applications.
>
> Signed-off-by: Icenowy Zheng 
> Signed-off-by: Vasily Khoruzhick 
> Signed-off-by: Torsten Duwe 
> ---
>  drivers/gpu/drm/bridge/analogix/Kconfig|  12 +
>  drivers/gpu/drm/bridge/analogix/Makefile   |   1 +
>  drivers/gpu/drm/bridge/analogix/analogix-anx6345.c | 797 
> +
>  3 files changed, 810 insertions(+)
>  create mode 100644 drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
>
> --- a/drivers/gpu/drm/bridge/analogix/Kconfig
> +++ b/drivers/gpu/drm/bridge/analogix/Kconfig
> @@ -1,6 +1,18 @@
>  # SPDX-License-Identifier: GPL-2.0-only
> +config DRM_ANALOGIX_ANX6345
> +   tristate "Analogix ANX6345 bridge"
> +   select DRM_ANALOGIX_DP
> +   select DRM_KMS_HELPER
> +   select REGMAP_I2C
> +   help
> + ANX6345 is an ultra-low Full-HD DisplayPort/eDP
> + transmitter designed for portable devices. The
> + ANX6345 transforms the LVTTL RGB output of an
> + application processor to eDP or DisplayPort.
> +
>  config DRM_ANALOGIX_ANX78XX
> tristate "Analogix ANX78XX bridge"
> +   select DRM_ANALOGIX_DP
> select DRM_KMS_HELPER
> select REGMAP_I2C
> help
> --- a/drivers/gpu/drm/bridge/analogix/Makefile
> +++ b/drivers/gpu/drm/bridge/analogix/Makefile
> @@ -1,4 +1,5 @@
>  # SPDX-License-Identifier: GPL-2.0-only
>  analogix_dp-objs := analogix_dp_core.o analogix_dp_reg.o analogix-i2c-dptx.o
> +obj-$(CONFIG_DRM_ANALOGIX_ANX6345) += analogix-anx6345.o
>  obj-$(CONFIG_DRM_ANALOGIX_ANX78XX) += analogix-anx78xx.o
>  obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix_dp.o
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
> @@ -0,0 +1,793 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright(c) 2016, Analogix Semiconductor.
> + * Copyright(c) 2017, Icenowy Zheng 
> + *
> + * Based on anx7808 driver obtained from chromeos with copyright:
> + * Copyright(c) 2013, Google Inc.
> + */
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "analogix-i2c-dptx.h"
> +#include "analogix-i2c-txcommon.h"
> +
> +#define POLL_DELAY 5 /* us */
> +#define POLL_TIMEOUT   500 /* us */
> +
> +#define I2C_IDX_DPTX   0
> +#define I2C_IDX_TXCOM  1
> +
> +static const u8 anx6345_i2c_addresses[] = {
> +   [I2C_IDX_DPTX]  = ANALOGIX_I2C_DPTX,
> +   [I2C_IDX_TXCOM] = ANALOGIX_I2C_TXCOMMON,
> +};
> +#define I2C_NUM_ADDRESSES  ARRAY_SIZE(anx6345_i2c_addresses)
> +
> +struct anx6345 {
> +   struct drm_dp_aux aux;
> +   struct drm_bridge bridge;
> +   struct i2c_client *client;
> +   struct edid *edid;
> +   struct drm_connector connector;
> +   struct drm_dp_link link;
> +   struct drm_panel *panel;
> +   struct regulator *dvdd12;
> +   struct regulator *dvdd25;
> +   struct gpio_desc *gpiod_reset;
> +   struct mutex lock;  /* protect EDID access */
> +
> +   /* I2C Slave addresses of ANX6345 are mapped as DPTX and SYS */
> +   struct i2c_client *i2c_clients[I2C_NUM_ADDRESSES];
> +   struct regmap *map[I2C_NUM_ADDRESSES];
> +
> +   u16 chipid;
> +   u8 dpcd[DP_RECEIVER_CAP_SIZE];
> +
> +   bool powered;
> +};
> +
> +static inline struct anx6345 *connector_to_anx6345(struct drm_connector *c)
> +{
> +   return container_of(c, struct anx6345, connector);
> +}
> +
> +static inline struct anx6345 *bridge_to_anx6345(struct drm_bridge *bridge)
> +{
> +   return container_of(bridge, struct anx6345, bridge);
> +}
> +
> +static int anx6345_set_bits(struct regmap *map, u8 reg, u8 mask)
> +{
> +   return regmap_update_bits(map, reg, mask, mask);
> +}
> +
> +static int anx6345_clear_bits(struct regmap *map, u8 reg, u8 mask)
> +{
> +   return regmap_update_bits(map, reg, mask, 0);
> +}
> +
> +static ssize_t anx6345_aux_transfer(struct drm_dp_aux *aux,
> +   struct drm_dp_aux_msg *msg)
> +{
> +   struct anx6345 *anx6345 = container_of(aux, struct anx6345, aux);
> +
> +   return anx_dp_aux_transfer(anx6345->map[I2C_IDX_DPTX], msg);
> +}
> +
> +static int anx6345_dp_link_training(struct anx6345 *anx6345)
> +{
> +   unsigned int value;
> +   u8 dp_bw;
> +   int err;
> +
> +   err = anx6345_clear_bits(anx6345->map[I2C_IDX_TXCOM],
> +SP_POWERDOWN_CTRL_REG,
> +SP_TOTAL_PD);
> +   if (err)
> +   

Re: [PATCH v2 0/7] Add anx6345 DP/eDP bridge for Olimex Teres-I

2019-07-22 Thread Vasily Khoruzhick
On Mon, Jul 22, 2019 at 8:04 AM Torsten Duwe  wrote:
>
> ANX6345 LVTTL->eDP video bridge, driver with device tree bindings.
>
> Changes from v2:
>
> * use SPDX-IDs throughout
>
> * removed the panel output again, as it was not what Maxime had in mind.
>   At least the Teres-I does very well without. No connector spec, no 
> "quirks"[1],
>   just plain EDID at work.

You still need a panel to put backlight in there. Otherwise backlight
will stay on when display is turned off.

>
> * binding clarifications and cosmetic changes as suggested by Andrzej
>
> Changes from v1:
>
> * fixed up copyright information. Most code changes are only moves and thus
>   retain copyright and module ownership. Even the new analogix-anx6345.c 
> originates
>   from the old 1495-line analogix-anx78xx.c, with 306 insertions and 987 
> deletions
>   (ignoring the trivial anx78xx -> anx6345 replacements) 306 new vs. 508 
> old...
>
> * fixed all minor formatting issues brought up
>
> * merged previously separate new analogix_dp_i2c module into existing 
> analogix_dp
>
> * split additional defines into a preparatory patch
>
> * renamed the factored-out common functions anx_aux_* -> anx_dp_aux_*, because
>   anx_...aux_transfer was exported globally. Besides, it is now GPL-only 
> exported.
>
> * moved chip ID read into a separate function.
>
> * keep the chip powered after a successful probe.
>   (There's a good chance that this is the only display during boot!)
>
> * updated the binding document: LVTTL input is now required, only the output 
> side
>   description is optional.
>
>  Laurent: I have also looked into the drm_panel_bridge infrastructure,
>  but it's not that trivial to convert these drivers to it.
>
> Changes from the respective previous versions:
>
> * the reset polarity is corrected in DT and the driver;
>   things should be clearer now.
>
> * as requested, add a panel (the known innolux,n116bge) and connect
>   the ports.
>
> * renamed dvdd?? to *-supply to match the established scheme
>
> * trivial update to the #include list, to make it compile in 5.2
>
> --
> [1] I hesitate to associate information about a connected panel with that 
> term.
> But should it be neccessary for maximum power saving (e.g. pinebooks),
> it would be good to have something here, regardless of nomenclature.
>
> Torsten


Re: [PATCH] drm: fix out-of-bounds access with short VSDB blocks

2019-07-22 Thread Sean Paul
On Mon, Jul 22, 2019 at 02:38:34PM +, Simon Ser wrote:
> From: Simon Ser 
> 
> The VSDB parsing code contains a few len >= N checks, accessing db[N] on
> success. However if len == N, db[N] is out-of-bounds.
> 
> This commit changes the checks to test for len > N.

I'm not familiar with this at all, but is db[0] the header and db[1] to db[len]
the data block? In that case, it's not out of bounds.

I looked up the spec and it says:
 Length of following data block (in bytes) (02h)

Sean

> 
> Signed-off-by: Simon Ser 
> ---
>  drivers/gpu/drm/drm_edid.c | 24 
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 82a4ceed3fcf..13d632f14172 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -3569,7 +3569,7 @@ do_hdmi_vsdb_modes(struct drm_connector *connector, 
> const u8 *db, u8 len,
>   vic_len = db[8 + offset] >> 5;
>   hdmi_3d_len = db[8 + offset] & 0x1f;
> 
> - for (i = 0; i < vic_len && len >= (9 + offset + i); i++) {
> + for (i = 0; i < vic_len && len > (9 + offset + i); i++) {
>   u8 vic;
> 
>   vic = db[9 + offset + i];
> @@ -3971,11 +3971,11 @@ drm_parse_hdr_metadata_block(struct drm_connector 
> *connector, const u8 *db)
>   connector->hdr_sink_metadata.hdmi_type1.metadata_type =
>   hdr_metadata_type(db);
> 
> - if (len >= 4)
> + if (len > 4)
>   connector->hdr_sink_metadata.hdmi_type1.max_cll = db[4];
> - if (len >= 5)
> + if (len > 5)
>   connector->hdr_sink_metadata.hdmi_type1.max_fall = db[5];
> - if (len >= 6)
> + if (len > 6)
>   connector->hdr_sink_metadata.hdmi_type1.min_cll = db[6];
>  }
> 
> @@ -3984,19 +3984,19 @@ drm_parse_hdmi_vsdb_audio(struct drm_connector 
> *connector, const u8 *db)
>  {
>   u8 len = cea_db_payload_len(db);
> 
> - if (len >= 6 && (db[6] & (1 << 7)))
> + if (len > 6 && (db[6] & (1 << 7)))
>   connector->eld[DRM_ELD_SAD_COUNT_CONN_TYPE] |= 
> DRM_ELD_SUPPORTS_AI;
> - if (len >= 8) {
> + if (len > 8) {
>   connector->latency_present[0] = db[8] >> 7;
>   connector->latency_present[1] = (db[8] >> 6) & 1;
>   }
> - if (len >= 9)
> + if (len > 9)
>   connector->video_latency[0] = db[9];
> - if (len >= 10)
> + if (len > 10)
>   connector->audio_latency[0] = db[10];
> - if (len >= 11)
> + if (len > 11)
>   connector->video_latency[1] = db[11];
> - if (len >= 12)
> + if (len > 12)
>   connector->audio_latency[1] = db[12];
> 
>   DRM_DEBUG_KMS("HDMI: latency present %d %d, "
> @@ -4559,9 +4559,9 @@ drm_parse_hdmi_vsdb_video(struct drm_connector 
> *connector, const u8 *db)
>   struct drm_display_info *info = >display_info;
>   u8 len = cea_db_payload_len(db);
> 
> - if (len >= 6)
> + if (len > 6)
>   info->dvi_dual = db[6] & 1;
> - if (len >= 7)
> + if (len > 7)
>   info->max_tmds_clock = db[7] * 5000;
> 
>   DRM_DEBUG_KMS("HDMI: DVI dual %d, "
> --
> 2.22.0
> 
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
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 v2 1/4] drm/via: drop use of DRM(READ|WRITE) macros

2019-07-22 Thread Sam Ravnborg
Hi Email.

> > > IMHO a far better idea is to expand these macros as static inline 
> > > functions.
> > > The extra bonus here is that the pseudo-magical VIA_BASE will also 
> > > disappear.
> > >
> > > Since all the VIA_READ8 are used for masking, one might as well drop
> > > them in favour of via_mask().

Like this:
static inline u32 via_read(struct drm_via_private *dev_priv, u32 reg)
{
return readl((void __iomem *)(dev_priv->mmio->handle + reg));
}

static inline void via_write(struct drm_via_private *dev_priv, u32 reg,
 u32 val)
{
writel(val, (void __iomem *)(dev_priv->mmio->handle + reg));
}

static inline void via_write8(struct drm_via_private *dev_priv, u32 reg,
  u32 val)
{
writeb(val, (void __iomem *)(dev_priv->mmio->handle + reg));
}

static inline void via_write8_mask_or(struct drm_via_private *dev_priv,
  u32 reg, u32 mask)
{
u32 val;

val = readb((void __iomem *)(dev_priv->mmio->handle + reg));
writeb(val | mask, (void __iomem *)(dev_priv->mmio->handle + reg));
}

static inline void via_write8_mask_and(struct drm_via_private *dev_priv,
   u32 reg, u32 mask)
{
u32 val;

val = readb((void __iomem *)(dev_priv->mmio->handle + reg));
writeb(val & mask, (void __iomem *)(dev_priv->mmio->handle + reg));
}

Patches are almost ready, but if there is any quick feedback let me
know.

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

Re: [PATCH] drm/panel: simple: Doxygenize 'struct panel_desc'; rename a few functions

2019-07-22 Thread Doug Anderson
Hi,

On Sun, Jul 21, 2019 at 2:38 AM Sam Ravnborg  wrote:
>
> Hi Doug.
>
> On Wed, Jul 17, 2019 at 07:33:17PM +0200, Sam Ravnborg wrote:
> > Hi Doug.
> >
> > On Fri, Jul 12, 2019 at 09:33:33AM -0700, Douglas Anderson wrote:
> > > This attempts to address outstanding review feedback from commit
> > > b8a2948fa2b3 ("drm/panel: simple: Add ability to override typical
> > > timing").  Specifically:
> > >
> > > * It was requested that I document (in the structure definition) that
> > >   the device tree override had no effect if 'struct drm_display_mode'
> > >   was used in the panel description.  I have provided full Doxygen
> > >   comments for 'struct panel_desc' to accomplish that.
> > > * panel_simple_get_fixed_modes() was thought to be a confusing name,
> > >   so it has been renamed to panel_simple_get_display_modes().
> > > * panel_simple_parse_override_mode() was thought to be better named as
> > >   panel_simple_parse_panel_timing_node().
> > >
> > > Suggested-by: Sam Ravnborg 
> > > Signed-off-by: Douglas Anderson 
> >
> > Thanks.
> >
> > I updated the $subject to:
> > drm/panel: simple: document panel_desc; rename a few functions
> >
> > And pushed out to drm-misc-next.
>
> I see the following error printed at each boot:
>
> /panel: could not find node 'panel-timing'
>
> The error originates from this snip (from panel-simple.c):
>
>if (!of_get_display_timing(dev->of_node, "panel-timing", ))
> panel_simple_parse_panel_timing_node(dev, panel, );
>
> of_get_display_timing() returns -2 (-ENOENT).
>
> In the setup I use there is no panel-timing node as the timing specified
> in panel-simple.c is fine.
> This is the typical setup - and there should not in the normal case
> be printed error messages like this during boot.
>
> Will you please take a look at this.

Breadcrumbs: series has been posted to address this.  PTAL.

https://lkml.kernel.org/r/20190722182439.44844-1-diand...@chromium.org


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

[PATCH 4/4] video: amba-clcd: Spout an error if of_get_display_timing() gives an error

2019-07-22 Thread Douglas Anderson
In the patch ("video: of: display_timing: Don't yell if no timing node
is present") we'll stop spouting an error directly in
of_get_display_timing() if no node is present.  Presumably amba-clcd
should take charge of spouting its own error now.

NOTE: we'll print two errors if the node was present but there were
problems parsing the timing node (one in of_parse_display_timing() and
this new one).  Since this is a fatal error for the driver's probe
(and presumably someone will be debugging), this should be OK.

Signed-off-by: Douglas Anderson 
---

 drivers/video/fbdev/amba-clcd.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/amba-clcd.c b/drivers/video/fbdev/amba-clcd.c
index 89324e42a033..7de43be6ef2c 100644
--- a/drivers/video/fbdev/amba-clcd.c
+++ b/drivers/video/fbdev/amba-clcd.c
@@ -561,8 +561,10 @@ static int clcdfb_of_get_dpi_panel_mode(struct device_node 
*node,
struct videomode video;
 
err = of_get_display_timing(node, "panel-timing", );
-   if (err)
+   if (err) {
+   pr_err("%pOF: problems parsing panel-timing (%d)\n", node, err);
return err;
+   }
 
videomode_from_timing(, );
 
-- 
2.22.0.657.g960e92d24f-goog

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

[PATCH 1/4] video: of: display_timing: Add of_node_put() in of_get_display_timing()

2019-07-22 Thread Douglas Anderson
>From code inspection it can be seen that of_get_display_timing() is
lacking an of_node_put().  Add it.

Fixes: ffa3fd21de8a ("videomode: implement public of_get_display_timing()")
Signed-off-by: Douglas Anderson 
---

 drivers/video/of_display_timing.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/video/of_display_timing.c 
b/drivers/video/of_display_timing.c
index f5c1c469c0af..5eedae0799f0 100644
--- a/drivers/video/of_display_timing.c
+++ b/drivers/video/of_display_timing.c
@@ -119,6 +119,7 @@ int of_get_display_timing(const struct device_node *np, 
const char *name,
struct display_timing *dt)
 {
struct device_node *timing_np;
+   int ret;
 
if (!np)
return -EINVAL;
@@ -129,7 +130,11 @@ int of_get_display_timing(const struct device_node *np, 
const char *name,
return -ENOENT;
}
 
-   return of_parse_display_timing(timing_np, dt);
+   ret = of_parse_display_timing(timing_np, dt);
+
+   of_node_put(timing_np);
+
+   return ret;
 }
 EXPORT_SYMBOL_GPL(of_get_display_timing);
 
-- 
2.22.0.657.g960e92d24f-goog



[PATCH 2/4] video: of: display_timing: Don't yell if no timing node is present

2019-07-22 Thread Douglas Anderson
There may be cases (like in panel-simple.c) where we have a sane
fallback if no timings are specified in the device tree.  Let's get
rid of the unconditional pr_err().  We can add error messages in
individual drivers if it makes sense.

NOTE: we'll still print errors if the node is present but there are
problems parsing the timings.

Fixes: b8a2948fa2b3 ("drm/panel: simple: Add ability to override typical 
timing")
Reported-by: Sam Ravnborg 
Signed-off-by: Douglas Anderson 
---

 drivers/video/of_display_timing.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/video/of_display_timing.c 
b/drivers/video/of_display_timing.c
index 5eedae0799f0..abc9ada798ee 100644
--- a/drivers/video/of_display_timing.c
+++ b/drivers/video/of_display_timing.c
@@ -125,10 +125,8 @@ int of_get_display_timing(const struct device_node *np, 
const char *name,
return -EINVAL;
 
timing_np = of_get_child_by_name(np, name);
-   if (!timing_np) {
-   pr_err("%pOF: could not find node '%s'\n", np, name);
+   if (!timing_np)
return -ENOENT;
-   }
 
ret = of_parse_display_timing(timing_np, dt);
 
-- 
2.22.0.657.g960e92d24f-goog

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

[PATCH 0/4] video: of: display_timing: Adjust err printing of of_get_display_timing()

2019-07-22 Thread Douglas Anderson
As reported by Sam Ravnborg [1], after commit b8a2948fa2b3
("drm/panel: simple: Add ability to override typical timing") we now
see a pointless error message printed on every boot for many systems.
Let's fix that by adjusting who is responsible for printing error
messages when of_get_display_timing() is used.

Most certainly we can bikeshed the topic about whether this is the
right fix or we should instead add logic to panel_simple_probe() to
avoid calling of_get_display_timing() in the case where there is no
"panel-timing" sub-node.  If there is consensus that I should move the
fix to panel_simple_probe() I'm happy to spin this series.  In that
case we probably should _remove_ the extra prints that were already
present in the other two callers of of_get_display_timing().

While at it, fix a missing of_node_put() found by code inspection.

NOTE: amba-clcd and panel-lvds were only compile-tested.

[1] https://lkml.kernel.org/r/20190721093815.ga4...@ravnborg.org


Douglas Anderson (4):
  video: of: display_timing: Add of_node_put() in
of_get_display_timing()
  video: of: display_timing: Don't yell if no timing node is present
  drm: panel-lvds: Spout an error if of_get_display_timing() gives an
error
  video: amba-clcd: Spout an error if of_get_display_timing() gives an
error

 drivers/gpu/drm/panel/panel-lvds.c |  5 -
 drivers/video/fbdev/amba-clcd.c|  4 +++-
 drivers/video/of_display_timing.c  | 11 +++
 3 files changed, 14 insertions(+), 6 deletions(-)

-- 
2.22.0.657.g960e92d24f-goog

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

[PATCH 3/4] drm: panel-lvds: Spout an error if of_get_display_timing() gives an error

2019-07-22 Thread Douglas Anderson
In the patch ("video: of: display_timing: Don't yell if no timing node
is present") we'll stop spouting an error directly in
of_get_display_timing() if no node is present.  Presumably panel-lvds
should take charge of spouting its own error now.

NOTE: we'll print two errors if the node was present but there were
problems parsing the timing node (one in of_parse_display_timing() and
this new one).  Since this is a fatal error for the driver's probe
(and presumably someone will be debugging), this should be OK.

Signed-off-by: Douglas Anderson 
---

 drivers/gpu/drm/panel/panel-lvds.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/panel/panel-lvds.c 
b/drivers/gpu/drm/panel/panel-lvds.c
index 1ec57d0806a8..ad47cc95459e 100644
--- a/drivers/gpu/drm/panel/panel-lvds.c
+++ b/drivers/gpu/drm/panel/panel-lvds.c
@@ -147,8 +147,11 @@ static int panel_lvds_parse_dt(struct panel_lvds *lvds)
int ret;
 
ret = of_get_display_timing(np, "panel-timing", );
-   if (ret < 0)
+   if (ret < 0) {
+   dev_err(lvds->dev, "%pOF: problems parsing panel-timing (%d)\n",
+   np, ret);
return ret;
+   }
 
videomode_from_timing(, >video_mode);
 
-- 
2.22.0.657.g960e92d24f-goog

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

Re: [PATCH] drm/i915: Mark expected switch fall-throughs

2019-07-22 Thread Kees Cook
On Mon, Jul 22, 2019 at 01:12:44PM -0500, Gustavo A. R. Silva wrote:
> In preparation to enabling -Wimplicit-fallthrough, mark switch
> cases where we are expecting to fall through.
> 
> This patch fixes the following warnings:
> 
> drivers/gpu/drm/i915/gem/i915_gem_mman.c: In function ‘i915_gem_fault’:
> drivers/gpu/drm/i915/gem/i915_gem_mman.c:342:6: warning: this statement may 
> fall through [-Wimplicit-fallthrough=]
>if (!i915_terminally_wedged(i915))
>   ^
> drivers/gpu/drm/i915/gem/i915_gem_mman.c:345:2: note: here
>   case -EAGAIN:
>   ^~~~
> 
> drivers/gpu/drm/i915/gem/i915_gem_pages.c: In function ‘i915_gem_object_map’:
> ./include/linux/compiler.h:78:22: warning: this statement may fall through 
> [-Wimplicit-fallthrough=]
>  # define unlikely(x) __builtin_expect(!!(x), 0)
>   ^~
> ./include/asm-generic/bug.h:136:2: note: in expansion of macro ‘unlikely’
>   unlikely(__ret_warn_on); \
>   ^~~~
> drivers/gpu/drm/i915/i915_utils.h:49:25: note: in expansion of macro ‘WARN’
>  #define MISSING_CASE(x) WARN(1, "Missing case (%s == %ld)\n", \
>  ^~~~
> drivers/gpu/drm/i915/gem/i915_gem_pages.c:270:3: note: in expansion of macro 
> ‘MISSING_CASE’
>MISSING_CASE(type);
>^~~~
> drivers/gpu/drm/i915/gem/i915_gem_pages.c:272:2: note: here
>   case I915_MAP_WB:
>   ^~~~
> 
> drivers/gpu/drm/i915/i915_gpu_error.c: In function 
> ‘error_record_engine_registers’:
> ./include/linux/compiler.h:78:22: warning: this statement may fall through 
> [-Wimplicit-fallthrough=]
>  # define unlikely(x) __builtin_expect(!!(x), 0)
>   ^~
> ./include/asm-generic/bug.h:136:2: note: in expansion of macro ‘unlikely’
>   unlikely(__ret_warn_on); \
>   ^~~~
> drivers/gpu/drm/i915/i915_utils.h:49:25: note: in expansion of macro ‘WARN’
>  #define MISSING_CASE(x) WARN(1, "Missing case (%s == %ld)\n", \
>  ^~~~
> drivers/gpu/drm/i915/i915_gpu_error.c:1196:5: note: in expansion of macro 
> ‘MISSING_CASE’
>  MISSING_CASE(engine->id);
>  ^~~~
> drivers/gpu/drm/i915/i915_gpu_error.c:1197:4: note: here
> case RCS0:
> ^~~~
> 
> drivers/gpu/drm/i915/display/intel_dp.c: In function 
> ‘intel_dp_get_fia_supported_lane_count’:
> ./include/linux/compiler.h:78:22: warning: this statement may fall through 
> [-Wimplicit-fallthrough=]
>  # define unlikely(x) __builtin_expect(!!(x), 0)
>   ^~
> ./include/asm-generic/bug.h:136:2: note: in expansion of macro ‘unlikely’
>   unlikely(__ret_warn_on); \
>   ^~~~
> drivers/gpu/drm/i915/i915_utils.h:49:25: note: in expansion of macro ‘WARN’
>  #define MISSING_CASE(x) WARN(1, "Missing case (%s == %ld)\n", \
>  ^~~~
> drivers/gpu/drm/i915/display/intel_dp.c:233:3: note: in expansion of macro 
> ‘MISSING_CASE’
>MISSING_CASE(lane_info);
>^~~~
> drivers/gpu/drm/i915/display/intel_dp.c:234:2: note: here
>   case 1:
>   ^~~~
> 
> drivers/gpu/drm/i915/display/intel_display.c: In function 
> ‘check_digital_port_conflicts’:
>   CC [M]  drivers/gpu/drm/nouveau/nvkm/engine/disp/cursgv100.o
> drivers/gpu/drm/i915/display/intel_display.c:12043:7: warning: this statement 
> may fall through [-Wimplicit-fallthrough=]
> if (WARN_ON(!HAS_DDI(to_i915(dev
>^
> drivers/gpu/drm/i915/display/intel_display.c:12046:3: note: here
>case INTEL_OUTPUT_DP:
>^~~~
> 
> Also, notice that the Makefile is modified in order to stop
> ignoring fall-through warnings. The -Wimplicit-fallthrough
> option will be enabled globally in v5.3.
> 
> Warning level 3 was used: -Wimplicit-fallthrough=3
> 
> This patch is part of the ongoing efforts to enable
> -Wimplicit-fallthrough.
> 
> Signed-off-by: Gustavo A. R. Silva 

Excellent; I think these are literally the last remaining cases in the
kernel. :)

Reviewed-by: Kees Cook 

-Kees

> ---
>  drivers/gpu/drm/i915/Makefile| 1 -
>  drivers/gpu/drm/i915/display/intel_display.c | 2 +-
>  drivers/gpu/drm/i915/display/intel_dp.c  | 1 +
>  drivers/gpu/drm/i915/gem/i915_gem_mman.c | 2 +-
>  drivers/gpu/drm/i915/gem/i915_gem_pages.c| 2 +-
>  drivers/gpu/drm/i915/i915_gpu_error.c| 1 +
>  6 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 91355c2ea8a5..8cace65f50ce 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -16,7 +16,6 @@ subdir-ccflags-y := -Wall -Wextra
>  subdir-ccflags-y += $(call cc-disable-warning, unused-parameter)
>  subdir-ccflags-y += $(call cc-disable-warning, type-limits)
>  subdir-ccflags-y += $(call cc-disable-warning, missing-field-initializers)
> -subdir-ccflags-y += $(call cc-disable-warning, implicit-fallthrough)
>  subdir-ccflags-y += $(call cc-disable-warning, unused-but-set-variable)
>  # clang warnings
>  

Re: [Freedreno] [PATCH] drm/msm: correct NULL pointer dereference in context_init

2019-07-22 Thread Sean Paul
On Fri, Jun 28, 2019 at 05:57:26AM -0700, Rob Clark wrote:
> On Wed, Jun 26, 2019 at 7:05 PM Brian Masney  wrote:
> >
> > Correct attempted NULL pointer dereference in context_init() when
> > running without an IOMMU.
> >
> > Signed-off-by: Brian Masney 
> > Fixes: 295b22ae596c ("drm/msm: Pass the MMU domain index in struct 
> > msm_file_private")
> > ---
> > The no IOMMU case seems like functionality that we may want to keep
> > based on this comment:
> > https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/msm/adreno/a3xx_gpu.c#L523
> > Once I get the msm8974 interconnect driver done, I'm going to look into
> > what needs to be done to get the IOMMU working on the Nexus 5.
> >
> > Alternatively, for development purposes, maybe we could have a NOOP
> > IOMMU driver that would allow us to remove these NULL checks that are
> > sprinkled throughout the code. I haven't looked into this in detail.
> > Thoughts?
> 
> yeah, we probably want to keep !iommu support, it is at least useful
> for bringup of new (or old) devices.  But tends to bitrot a since it
> isn't a case that gets tested much once iommu is in place.  Perhaps
> there is a way to have a null iommu/aspace, although I'm not quite
> sure how that would work..
> 
> Anyways,
> 
> Reviewed-by: Rob Clark 
> 
> (I guess this can go in via drm-misc-fixes unless we get some more
> fixes to justify sending msm-fixes MR..)

Applied to drm-misc-fixes for 5.3

Sean

> 
> >
> >  drivers/gpu/drm/msm/msm_drv.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> > index 451bd4508793..83047cb2c735 100644
> > --- a/drivers/gpu/drm/msm/msm_drv.c
> > +++ b/drivers/gpu/drm/msm/msm_drv.c
> > @@ -619,7 +619,7 @@ static int context_init(struct drm_device *dev, struct 
> > drm_file *file)
> >
> > msm_submitqueue_init(dev, ctx);
> >
> > -   ctx->aspace = priv->gpu->aspace;
> > +   ctx->aspace = priv->gpu ? priv->gpu->aspace : NULL;
> > file->driver_priv = ctx;
> >
> > return 0;
> > --
> > 2.20.1
> >
> > ___
> > Freedreno mailing list
> > freedr...@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/freedreno

-- 
Sean Paul, Software Engineer, Google / Chromium OS


Re: drm/msm/dpu: Correct dpu encoder spinlock initialization

2019-07-22 Thread Sean Paul
On Mon, Jun 24, 2019 at 11:57:12AM +0530, Shubhashree Dhar wrote:
> dpu encoder spinlock should be initialized during dpu encoder
> init instead of dpu encoder setup which is part of commit.
> There are chances that vblank control uses the uninitialized
> spinlock if not initialized during encoder init.
> 
> Change-Id: I5a18b95fa47397c834a266b22abf33a517b03a4e
> Signed-off-by: Shubhashree Dhar 

Thanks for your patch.

I've resolved the conflict and tweaked the commit message a bit to reflect
current reality.

Applied to drm-misc-fixes for 5.3

Sean

> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c 
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index 5f085b5..22938c7 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -2195,8 +2195,6 @@ int dpu_encoder_setup(struct drm_device *dev, struct 
> drm_encoder *enc,
>   if (ret)
>   goto fail;
>  
> - spin_lock_init(_enc->enc_spinlock);
> -
>   atomic_set(_enc->frame_done_timeout, 0);
>   timer_setup(_enc->frame_done_timer,
>   dpu_encoder_frame_done_timeout, 0);
> @@ -2250,6 +2248,7 @@ struct drm_encoder *dpu_encoder_init(struct drm_device 
> *dev,
>  
>   drm_encoder_helper_add(_enc->base, _encoder_helper_funcs);
>  
> + spin_lock_init(_enc->enc_spinlock);
>   dpu_enc->enabled = false;
>  
>   return _enc->base;
> -- 
> 1.9.1
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS


[PATCH v2] drm/bridge: dw-hdmi: Refuse DDC/CI transfers on the internal I2C controller

2019-07-22 Thread Matthias Kaehlcke
The DDC/CI protocol involves sending a multi-byte request to the
display via I2C, which is typically followed by a multi-byte
response. The internal I2C controller only allows single byte
reads/writes or reads of 8 sequential bytes, hence DDC/CI is not
supported when the internal I2C controller is used. The I2C
transfers complete without errors, however the data in the response
is garbage. Abort transfers to/from slave address 0x37 (DDC) with
-EOPNOTSUPP, to make it evident that the communication is failing.

Signed-off-by: Matthias Kaehlcke 
---
Changes in v2:
- changed DDC_I2C_ADDR to DDC_CI_ADDR
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c 
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index 045b1b13fd0e..28933629f3c7 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -35,6 +35,7 @@
 
 #include 
 
+#define DDC_CI_ADDR0x37
 #define DDC_SEGMENT_ADDR   0x30
 
 #define HDMI_EDID_LEN  512
@@ -322,6 +323,13 @@ static int dw_hdmi_i2c_xfer(struct i2c_adapter *adap,
u8 addr = msgs[0].addr;
int i, ret = 0;
 
+   if (addr == DDC_CI_ADDR)
+   /*
+* The internal I2C controller does not support the multi-byte
+* read and write operations needed for DDC/CI.
+*/
+   return -EOPNOTSUPP;
+
dev_dbg(hdmi->dev, "xfer: num: %d, addr: %#x\n", num, addr);
 
for (i = 0; i < num; i++) {
-- 
2.22.0.657.g960e92d24f-goog

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

Re: [PATCH] drm/amdkfd/kfd_mqd_manager_v10: Avoid fall-through warning

2019-07-22 Thread Liu, Shaoyun
Reviewed-by:  shaoyunl 

On 2019-07-22 1:47 p.m., Gustavo A. R. Silva wrote:
> In preparation to enabling -Wimplicit-fallthrough, this patch silences
> the following warning:
>
> drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_mqd_manager_v10.c: In function 
> ‘mqd_manager_init_v10’:
> ./include/linux/dynamic_debug.h:122:52: warning: this statement may fall 
> through [-Wimplicit-fallthrough=]
>   #define __dynamic_func_call(id, fmt, func, ...) do { \
>  ^
> ./include/linux/dynamic_debug.h:143:2: note: in expansion of macro 
> ‘__dynamic_func_call’
>__dynamic_func_call(__UNIQUE_ID(ddebug), fmt, func, ##__VA_ARGS__)
>^~~
> ./include/linux/dynamic_debug.h:153:2: note: in expansion of macro 
> ‘_dynamic_func_call’
>_dynamic_func_call(fmt, __dynamic_pr_debug,  \
>^~
> ./include/linux/printk.h:336:2: note: in expansion of macro ‘dynamic_pr_debug’
>dynamic_pr_debug(fmt, ##__VA_ARGS__)
>^~~~
> drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_mqd_manager_v10.c:432:3: note: in 
> expansion of macro ‘pr_debug’
> pr_debug("%s@%i\n", __func__, __LINE__);
> ^~~~
> drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_mqd_manager_v10.c:433:2: note: here
>case KFD_MQD_TYPE_COMPUTE:
>^~~~
>
> by removing the call to pr_debug() in KFD_MQD_TYPE_CP:
>
> "The mqd init for CP and COMPUTE will have the same  routine." [1]
>
> This bug was found thanks to the ongoing efforts to enable
> -Wimplicit-fallthrough.
>
> [1] https://lore.kernel.org/lkml/c735a1cc-a545-50fb-44e7-c0ad93ee8...@amd.com/
>
> Signed-off-by: Gustavo A. R. Silva 
> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v10.c | 1 -
>   1 file changed, 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v10.c 
> b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v10.c
> index 4f8a6ffc5775..9cd3eb2d90bd 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v10.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v10.c
> @@ -429,7 +429,6 @@ struct mqd_manager *mqd_manager_init_v10(enum 
> KFD_MQD_TYPE type,
>   
>   switch (type) {
>   case KFD_MQD_TYPE_CP:
> - pr_debug("%s@%i\n", __func__, __LINE__);
>   case KFD_MQD_TYPE_COMPUTE:
>   pr_debug("%s@%i\n", __func__, __LINE__);
>   mqd->allocate_mqd = allocate_mqd;
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm/i915: Mark expected switch fall-throughs

2019-07-22 Thread Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, mark switch
cases where we are expecting to fall through.

This patch fixes the following warnings:

drivers/gpu/drm/i915/gem/i915_gem_mman.c: In function ‘i915_gem_fault’:
drivers/gpu/drm/i915/gem/i915_gem_mman.c:342:6: warning: this statement may 
fall through [-Wimplicit-fallthrough=]
   if (!i915_terminally_wedged(i915))
  ^
drivers/gpu/drm/i915/gem/i915_gem_mman.c:345:2: note: here
  case -EAGAIN:
  ^~~~

drivers/gpu/drm/i915/gem/i915_gem_pages.c: In function ‘i915_gem_object_map’:
./include/linux/compiler.h:78:22: warning: this statement may fall through 
[-Wimplicit-fallthrough=]
 # define unlikely(x) __builtin_expect(!!(x), 0)
  ^~
./include/asm-generic/bug.h:136:2: note: in expansion of macro ‘unlikely’
  unlikely(__ret_warn_on); \
  ^~~~
drivers/gpu/drm/i915/i915_utils.h:49:25: note: in expansion of macro ‘WARN’
 #define MISSING_CASE(x) WARN(1, "Missing case (%s == %ld)\n", \
 ^~~~
drivers/gpu/drm/i915/gem/i915_gem_pages.c:270:3: note: in expansion of macro 
‘MISSING_CASE’
   MISSING_CASE(type);
   ^~~~
drivers/gpu/drm/i915/gem/i915_gem_pages.c:272:2: note: here
  case I915_MAP_WB:
  ^~~~

drivers/gpu/drm/i915/i915_gpu_error.c: In function 
‘error_record_engine_registers’:
./include/linux/compiler.h:78:22: warning: this statement may fall through 
[-Wimplicit-fallthrough=]
 # define unlikely(x) __builtin_expect(!!(x), 0)
  ^~
./include/asm-generic/bug.h:136:2: note: in expansion of macro ‘unlikely’
  unlikely(__ret_warn_on); \
  ^~~~
drivers/gpu/drm/i915/i915_utils.h:49:25: note: in expansion of macro ‘WARN’
 #define MISSING_CASE(x) WARN(1, "Missing case (%s == %ld)\n", \
 ^~~~
drivers/gpu/drm/i915/i915_gpu_error.c:1196:5: note: in expansion of macro 
‘MISSING_CASE’
 MISSING_CASE(engine->id);
 ^~~~
drivers/gpu/drm/i915/i915_gpu_error.c:1197:4: note: here
case RCS0:
^~~~

drivers/gpu/drm/i915/display/intel_dp.c: In function 
‘intel_dp_get_fia_supported_lane_count’:
./include/linux/compiler.h:78:22: warning: this statement may fall through 
[-Wimplicit-fallthrough=]
 # define unlikely(x) __builtin_expect(!!(x), 0)
  ^~
./include/asm-generic/bug.h:136:2: note: in expansion of macro ‘unlikely’
  unlikely(__ret_warn_on); \
  ^~~~
drivers/gpu/drm/i915/i915_utils.h:49:25: note: in expansion of macro ‘WARN’
 #define MISSING_CASE(x) WARN(1, "Missing case (%s == %ld)\n", \
 ^~~~
drivers/gpu/drm/i915/display/intel_dp.c:233:3: note: in expansion of macro 
‘MISSING_CASE’
   MISSING_CASE(lane_info);
   ^~~~
drivers/gpu/drm/i915/display/intel_dp.c:234:2: note: here
  case 1:
  ^~~~

drivers/gpu/drm/i915/display/intel_display.c: In function 
‘check_digital_port_conflicts’:
  CC [M]  drivers/gpu/drm/nouveau/nvkm/engine/disp/cursgv100.o
drivers/gpu/drm/i915/display/intel_display.c:12043:7: warning: this statement 
may fall through [-Wimplicit-fallthrough=]
if (WARN_ON(!HAS_DDI(to_i915(dev
   ^
drivers/gpu/drm/i915/display/intel_display.c:12046:3: note: here
   case INTEL_OUTPUT_DP:
   ^~~~

Also, notice that the Makefile is modified in order to stop
ignoring fall-through warnings. The -Wimplicit-fallthrough
option will be enabled globally in v5.3.

Warning level 3 was used: -Wimplicit-fallthrough=3

This patch is part of the ongoing efforts to enable
-Wimplicit-fallthrough.

Signed-off-by: Gustavo A. R. Silva 
---
 drivers/gpu/drm/i915/Makefile| 1 -
 drivers/gpu/drm/i915/display/intel_display.c | 2 +-
 drivers/gpu/drm/i915/display/intel_dp.c  | 1 +
 drivers/gpu/drm/i915/gem/i915_gem_mman.c | 2 +-
 drivers/gpu/drm/i915/gem/i915_gem_pages.c| 2 +-
 drivers/gpu/drm/i915/i915_gpu_error.c| 1 +
 6 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 91355c2ea8a5..8cace65f50ce 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -16,7 +16,6 @@ subdir-ccflags-y := -Wall -Wextra
 subdir-ccflags-y += $(call cc-disable-warning, unused-parameter)
 subdir-ccflags-y += $(call cc-disable-warning, type-limits)
 subdir-ccflags-y += $(call cc-disable-warning, missing-field-initializers)
-subdir-ccflags-y += $(call cc-disable-warning, implicit-fallthrough)
 subdir-ccflags-y += $(call cc-disable-warning, unused-but-set-variable)
 # clang warnings
 subdir-ccflags-y += $(call cc-disable-warning, sign-compare)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c 
b/drivers/gpu/drm/i915/display/intel_display.c
index 8592a7d422de..30b97ded6fdd 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -12042,7 +12042,7 @@ static bool check_digital_port_conflicts(struct 
intel_atomic_state *state)

Re: [PATCH v9 04/18] kunit: test: add kunit_stream a std::stream like logger

2019-07-22 Thread Brendan Higgins
On Thu, Jul 18, 2019 at 5:08 PM Brendan Higgins
 wrote:
>
> On Thu, Jul 18, 2019 at 12:22:33PM -0700, Brendan Higgins wrote:
> > On Thu, Jul 18, 2019 at 10:50 AM Stephen Boyd  wrote:
> > >
> > > Quoting Brendan Higgins (2019-07-16 11:52:01)
> > > > On Tue, Jul 16, 2019 at 10:50 AM Stephen Boyd  wrote:
> [...]
> > > Do you have a link to those earlier patches?
> >
> > This is the first patchset:
> >
> > https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1788057.html
> >
> > In particular you can see the code for matching functions here:
> >
> > https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1788073.html
> >
> > And parameter matching code here:
> >
> > https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1788072.html
> >
> > https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1788086.html
> >
> > My apologies in advance, but the code at this early stage had not
> > adopted the kunit_* prefix and was still using the test_* and mock_*
> > prefix. (Hence, struct kunit_stream was known as struct test_stream).
> [...]
> > > The crux of my complaint is that the string stream API is too loosely
> > > defined to be usable. It allows tests to build up a string of
> > > unstructured information, but with certain calling constraints so we
> > > have to tread carefully. If there was more structure to the data that's
> > > being recorded then the test case runner could operate on the data
> > > without having to do string/stream operations, allocations, etc. This
> > > would make the assertion logic much more concrete and specific to kunit,
> > > instead of this small kunit wrapper that's been placed on top of string
> > > stream.
> >
> > Yeah, I can see the point of wanting something that provides more
> > structure than the raw `struct kunit_stream` interface. In fact, it is
> > something I had already started working on, when I had determined it
> > would be a large effort to capture all the variations. I was further
> > put off from the idea when I had been asked to convert the KUnit
> > intermediate format from what I was using to TAP, because, as it is,
> > the current data printed out by KUnit doesn't contain all the data I
> > would like to put in it in a way that best takes advantage of the TAP
> > specification. One problematic area in particular: TAP already
> > provides a way to present a lot of the data I would like to export,
> > but it involves JSON serialization which was an idea that some of the
> > other reviewers understandably weren't too keen on. TAP also wants to
> > report data some time after it is available, which is generally not a
> > good idea for test debug information; you want to make it available as
> > soon as you can or you risk crashing with the data still inside.
> >
> > Hence, I decided we could probably spend a good long while debating
> > how I present the information. So the idea of having a loose
> > definition seemed attractive to me in its own right since it would
> > likely conform to whatever we ended up deciding in the long run. Also,
> > all the better that it was what I already had and no one seemed to
> > mind too much.
> >
> > The only constant I expect is that `struct kunit` will likely need to
> > take an abstract object with a `commit` method, or a `format` method
> > or whatever so it could control when data was going to be printed out
> > to the user. We will probably also use a string builder in there
> > somewhere.
> >
> > > TL;DR: If we can get rid of the string stream API I'd view that as an
> > > improvement because building arbitrary strings in the kernel is complex,
> > > error prone and has calling context concerns.
> >
> > True. No argument there.
> >
> > > Is the intention that other code besides unit tests will use this string
> > > stream API to build up strings? Any targets in mind? This would be a
> > > good way to get the API merged upstream given that its 2019 and we
> > > haven't had such an API in the kernel so far.
> >
> > Someone, (was it you?) asked about code sharing with a string builder
> > thingy that was used for creating structured human readable files, but
> > that seemed like a pretty massive undertaking.
> >
> > Aside from that, no. I would kind of prefered that nobody used it for
> > anything else because I the issues you described.
> >
> > Nevertheless, I think the debate over the usefulness of the
> > string_stream and kunit_stream are separate topics. Even if we made
> > kunit_stream more structured, I am pretty sure I would want to use
> > string_stream or some variation for constructing the message.
> >
> > > An "object oriented" (strong quotes!) approach where kunit_fail_msg is
> > > the innermost struct in some assertion specific structure might work
> > > nicely and allow the test runner to call a generic 'format' function to
> > > print out the message based on the type of assertion/expectation it is.
> > > It probably would mean less code size too because the strings that are
> > > 

Re: [PATCH 0/6] drm/tinydrm: Move mipi_dbi

2019-07-22 Thread Eric Anholt
Noralf Trønnes  writes:

> This series ticks off the last tinydrm todo entry and moves out mipi_dbi
> to be a core helper.
>
> It splits struct mipi_dbi into an interface part and a display pipeline
> part (upload framebuffer over SPI). I also took the opportunity to
> rename the ambiguous 'mipi' variable name to 'dbi'. This lines up with
> the use of the 'dsi' variable name in the MIPI DSI helper.
>
> Note:
> This depends on series: drm/tinydrm: Remove tinydrm.ko
>
> Series is also available here:
> https://github.com/notro/linux/tree/move_mipi_dbi

Congratulations on making it to this stage.  This looks like a fine
conclusion to tinydrm.

Acked-by: Eric Anholt 


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

Re: [PATCH 05/10] dt-bindings: display: Add max-res property for mxsfb

2019-07-22 Thread Rob Herring
On Wed, Jun 26, 2019 at 04:32:13PM +0300, Robert Chiras wrote:
> Add new optional property 'max-res', to limit the maximum supported
> resolution by the MXSFB_DRM driver.

Bindings are for h/w description, not driver config.

> 
> Signed-off-by: Robert Chiras 
> ---
>  Documentation/devicetree/bindings/display/mxsfb.txt | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/mxsfb.txt 
> b/Documentation/devicetree/bindings/display/mxsfb.txt
> index 472e1ea..55e22ed 100644
> --- a/Documentation/devicetree/bindings/display/mxsfb.txt
> +++ b/Documentation/devicetree/bindings/display/mxsfb.txt
> @@ -17,6 +17,12 @@ Required properties:
>  Required sub-nodes:
>- port: The connection to an encoder chip.
>  
> +Optional properties:
> +- max-res:   an array with a maximum of two integers, representing the
> + maximum supported resolution, in the form of
> + , ; if one of the item is <0>, the default
> + driver-defined maximum resolution for that axis is used

I suppose what you are after is bandwidth limits? IIRC, there's already 
some bindings expressing such limits. Also, wouldn't you need to account 
for bpp and using the 2nd plane (IIRC that there is one).

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

  1   2   3   >