Re: [PATCH 0/3] Propagate DP-over-Type-C hotplug events from Type-C subsys to drm-drivers
Hi, On 04-03-19 16:17, Heikki Krogerus wrote: Hi Hans, On Thu, Feb 28, 2019 at 05:54:21PM +0100, Hans de Goede wrote: Hi Heikki, On 28-02-19 15:47, Heikki Krogerus wrote: Hi Hans, On Thu, Feb 28, 2019 at 12:24:25PM +0100, Hans de Goede wrote: Hi, On 28-02-19 10:15, Heikki Krogerus wrote: I've been thinking about this... Do we actually need to link the correct drm_connector to the Type-C connector? Perhaps we can make this work by just linking the GFX device to the Type-C connector. What use is it to the kms driver if it gets an event there is a DP hotplug with x lanes and orientation foo, if we are not telling it on which DP port it is ? kms devices already have multiple DP ports and more then one could be hooked-up to type-c connectors. I was looking at this series. You walk trough every DP port in the system when the DP alt mode driver broadcasts the event, but maybe that's different. Never mind. Right, my "simple / naive" solution simply tells the kms driver to check all DP ports for connection state changes, similar to how running "xrandr" under Xorg causes the kms driver to re-check the connection status of all ports. Actually running xrandr under Xorg after plugging in the cable, is how I did my initial DP over Type-C testing on the GPD win. But once we start passing extra-info, I believe the kms driver needs to know to which connector that info belongs. Well, I don't think we can deny the GPU driver (in this case) the information that we have and that is relevant to it, just because it seems difficult to deliver that information to the right location. Right, but this does not require a tight-coupling. My original proposal can do this if we pass a data struct with an identifier for the DP port for which the event is to the notifier. I suggest using a string for identifier, something like: ":00:02.0/DP-1" this event struct could then also contain all the info we want to pass. I do agree that we should not tie the objects (device entries) representing these components in kernel together, but I assume that we agree now that the connection between the two - the USB Type-C connector and the DisplayPort - must be described somewhere, either in firmware or build-in? So I guess we are talking implementation details here, right? Right. If that is the case... That string identifier you proposed would basically provide the details about the connection, so if we know those details, we might as well use "normal" ways to describe the connection and just extract them in runtime in the function that our DP alt mode driver calls. I think the connection has to be defined in i915 on CHT in any case. Interesting, I think the connection should be described in the fwnode for the fusb302 device for the CHT/GPD win case. Specifically I think this fits well as a property of the dp altmode. OK, you are correct. I was stupidly still thinking about the driver loading order, but the order does not matter. The DP alt mode driver should definitely not need to pass anything else to the notifier other than handle to itself (actually, handle straight to the port device would be better) as an identifier. The notifier function needs to be the one that determines the actual connection using that handle. Even if the target DP is described using a string like you propose, then that string has to come from somewhere, most likely from a device property. The notifier function can just as well extract it, we don't need to pass it separately. Here's my suggestion for function prototype: int drm_typec_dp_notification(struct device *typec_port_dev, struct typec_displayport_data *data); How about instead of the port_dev we pass in the altmode object and we have a method to get the fwnode for the altmode? Then the drm_typec_dp_notification() function can get info from that fwnode to implement the connection finding you describe below: We can pass the altmode object, np, but let's not decide which fwnode we'll ultimately use. I'm still leaning towards the connector node. So that function finds the connection between typec_port_dev and the correct DP in runtime, possibly by letting i915 (or what ever GPU driver) to do that. Once the function is done, it decrements any ref counts that it incremented before returning. That struct typec_displayport_data has all the information we have - the current pin assignment from the Configure VDO, HPD IRQ from the last Status Update, etc. - so it needs to be passed as payload to the notifier. Ack. So I believe that this discussion ties into the discussion from the: "[PATCH 0/2] platform/x86: intel_cht_int33fe: Start using software nodes" Mail thread. As discussed there I agree that adding a usb_connector child fwnode to the fwnode for the fusb302 to describe things like sink- and source-pdos is a good idea. Our last few mails were discussing describing supported alt-modes on the connector by adding altmode child-nodes to
Re: [PATCH 0/3] drm/vboxvideo: Move the vboxvideo driver out of staging
On Mon, Mar 04, 2019 at 05:47:21PM +0100, Hans de Goede wrote: > Hi All, > > This patch-series addresses the 2 TODO / FIXME items recently reported > by Daniel and after that moves the vboxvideo driver out of staging. > > Note this applies on top of drm-misc-next + this patch: > https://patchwork.kernel.org/patch/10824279/ > > Currently that patch is not yet in drm-misc, I can push it myself before > pushing the rest of this series (after review). > > Greg, the intent is for this series to be merged upstream through > drm-misc, may we have your Acked-by for that please? > For all 3: Acked-by: Greg Kroah-Hartman ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 2/2] drm/lima: driver for ARM Mali4xx GPUs
On Tue, Mar 5, 2019 at 1:20 AM Rob Herring wrote:
>
> On Sat, Mar 2, 2019 at 12:23 PM Rob Clark wrote:
> >
> > On Fri, Mar 1, 2019 at 9:32 PM Qiang Yu wrote:
> > >
> > > On Thu, Feb 28, 2019 at 5:41 AM Rob Herring wrote:
> > > >
> > > > On Wed, Feb 27, 2019 at 7:42 AM Qiang Yu wrote:
> > > > > diff --git a/drivers/gpu/drm/lima/lima_drv.c
> > > > > b/drivers/gpu/drm/lima/lima_drv.c> > > new file mode 100644
> > > > > index ..e93bce16ee10
> > > > > --- /dev/null
> > > > > +++ b/drivers/gpu/drm/lima/lima_drv.c
> > > > > @@ -0,0 +1,353 @@
> > > > > +// SPDX-License-Identifier: GPL-2.0 OR MIT
> > > > > +/* Copyright 2017-2018 Qiang Yu */
> > > > > +
> > > > > +#include
> > > > > +#include
> > > > > +#include
> > > > > +#include
> > > > > +#include
> > > > > +
> > > > > +#include "lima_drv.h"
> > > > > +#include "lima_gem.h"
> > > > > +#include "lima_gem_prime.h"
> > > > > +#include "lima_vm.h"
> > > > > +
> > > > > +int lima_sched_timeout_ms = 0;
> > > > > +int lima_sched_max_tasks = 32;
> > > > > +
> > > > > +MODULE_PARM_DESC(sched_timeout_ms, "task run timeout in ms (0 = no
> > > > > timeout (default))");
> > > > > +module_param_named(sched_timeout_ms, lima_sched_timeout_ms, int,
> > > > > 0444);
> > > > > +
> > > > > +MODULE_PARM_DESC(sched_max_tasks, "max queued task num in a context
> > > > > (default 32)");
> > > > > +module_param_named(sched_max_tasks, lima_sched_max_tasks, int, 0444);
> > > > > +
> > > > > +static int lima_ioctl_info(struct drm_device *dev, void *data,
> > > > > struct drm_file *file)
> > > > > +{
> > > >
> > > > For panfrost, we generalized this to "get param" like other drivers.
> > > > Looks like you can only add 7 more items.
> > > >
> > > > What about GPU revisions?
> > >
> > > Currently I don't know there's any programming difference between GPUs
> > > with different revision. Would be appreciate if anyone can tell me before
> > > some hard reverse engineering effort.
>
> What does the vendor kernel driver have? I haven't checked utgard, but
> there's no shortage of quirks in the midgard/bifrost driver. I'd
> imagine utgard to be similar.
Vendor kernel driver will export the version. I've added it in the following
version of the patch.
>
> > Probably a safe bet there are some revisions that need userspace
> > workarounds.. and given that kernel to userspace uabi is something we
> > end up having to live with for a long time, better to expose more
> > information to userspace just in case.
>
> Right.
>
> More importantly than the 1 example I gave, design the ABI to be
> extendable beyond 7 more u32 values. It is quite easy to support 2^32
> params.
>
OK, I've changed to this way in latter version of this patch.
Regards,
Qiang
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [EARLY RFC][PATCH] dma-buf: Add dma-buf heaps framework
On Mon, Mar 4, 2019 at 6:53 AM Andrew F. Davis wrote:
> On 3/1/19 6:06 AM, Brian Starkey wrote:
> > On Mon, Feb 25, 2019 at 08:36:04AM -0600, Andrew F. Davis wrote:
> >> +static long dma_heap_ioctl(struct file *filp, unsigned int cmd, unsigned
> >> long arg)
> >> +{
> >> +switch (cmd) {
> >> +case DMA_HEAP_IOC_ALLOC:
> >> +{
> >> +struct dma_heap_allocation_data heap_allocation;
> >> +struct dma_heap *heap = filp->private_data;
> >> +int fd;
> >> +
> >> +if (copy_from_user(_allocation, (void __user *)arg,
> >> _IOC_SIZE(cmd)))
> >> +return -EFAULT;
> >
> > Am I right in thinking "cmd" comes from userspace? It might be a good
> > idea to not trust _IOC_SIZE(cmd) and use our own. I'm looking at
> > Documentation/ioctl/botching-up-ioctls.txt and drm_ioctl()
> >
>
> Yes cmd is from userspace, but we are in a switch-case that has already
> matched cmd == DMA_HEAP_IOC_ALLOC which is sized correctly.
>
Well, even so, I went through and made this cleanup over the weekend,
as sizeof(heap_allocation) is probably more straight forward.
The current patchset against v5.0 (with hikey960 patches), which
includes the flags and other suggested changes is here:
https://git.linaro.org/people/john.stultz/android-dev.git/log/?h=dev/dma-buf-heap
W/ userland support here:
https://android-review.googlesource.com/c/device/linaro/hikey/+/909436
I'm hoping to send this out for a real RFC in the next few days. So
Andrew, if you can check it out and make sure it suits you ok I'd
appreciate it!
> >> +heap->num_of_buffers = 0;
> >> +heap->num_of_alloc_bytes = 0;
> >> +heap->alloc_bytes_wm = 0;
> >> +spin_lock_init(>stat_lock);
> >> +heap_root = debugfs_create_dir(heap->name, dma_heap_debug_root);
> >> +debugfs_create_u64("num_of_buffers", 0444, heap_root,
> >> >num_of_buffers);
> >> +debugfs_create_u64("num_of_alloc_bytes", 0444, heap_root,
> >> >num_of_alloc_bytes);
> >> +debugfs_create_u64("alloc_bytes_wm", 0444, heap_root,
> >> >alloc_bytes_wm);
> >
> > I can see it being useful to be able to reset this one.
> >
>
> Agree, looks like these will be pulled into the heaps themselves in the
> next rev John is working on, so shouldn't matter here.
Yea. Sort of half-way done on this. I yanked the stats, but haven't
re-added them back to the heaps yet.
> What we are moving to is a better (I think) ownership model. 'DMA-heaps'
> only tracks 'heaps', 'heaps' track their 'buffers'. In the above we have
> 'DMA-heaps' tracking info on 'buffers', bypassing the 'heaps' layer, so
> in the next rev will be the 'DMA-heaps' core asks the 'heaps' to report
> back stats.
Yea. This matches my plan.
> >> +/*
> >> + * mappings of this buffer should be un-cached, otherwise dmabuf heaps
> >> will
> >> + * need cache maintenance using DMA_BUF_SYNC_* when the buffer is mapped
> >> for dma
> >> + */
> >> +#define DMA_HEAP_FLAG_COHERENT 1
> >
> > I'm not really clear on the intention of this flag, and I do think
> > that "COHERENT" is a confusing/overloaded term.
> >
>
> It is, I wanted to use the term as used by the other DMA frameworks, but
> I don't really like it either.
>
> > To me, if the buffer is actually coherent with DMA masters, then
> > there's no need for any cache maintenance - which is the opposite of
> > what the comment says.
> >
>
> Buffers themselves can't be (non)coherent, they are just memory, masters
> on a bus can have coherent interactions depending on which masters are
> talking and how the buffer in question is managed. So really the term
> isn't right almost anywhere as it only applies from the perspective of
> the local core (running Linux) and only if simply not locally caching
> the buffer is enough to make it "coherent". But that is a rant best
> saved for another time.
>
> For us lets drop that flag, if you want to allocate from a
> non-locally-cached buffer then it can be its own heap that only provides
> that type of allocation. Or even the same heap exporter providing both
> types, just registers itself twice with the DMA-heaps core, once for an
> interface that gives out cached buffers and one for uncached.
So I've not removed this yet. My only concern is that if its a
reasonable common attribute for heaps to implement, we probably should
keep it, rather then pushing for new heaps for coherent/non-coherent.
This comes from my experience creating the CLOCK_REALTIME_ALARM,
CLOCK_BOOTTIME_ALARM clockids, then later realizing ALARM should have
been just a attribute flag on the REALTIME/BOOTTIME clockids. I'd
rather not rework the heaps to have system and system-coherent and
cma and cma-coherent, if its a general thing.
That said, I did find the flag's meaning confusing myself initially,
so maybe holding off on it for now (if we don't have a clear user) is
a good idea?
thanks
-john
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
[Bug 198123] Console is the wrong color at boot with radeon 6670
https://bugzilla.kernel.org/show_bug.cgi?id=198123 crocket (crockabisc...@gmail.com) changed: What|Removed |Added CC||crockabisc...@gmail.com --- Comment #51 from crocket (crockabisc...@gmail.com) --- https://bugzilla.kernel.org/show_bug.cgi?id=202739 might be a fix. -- 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
[Bug 88501] AMD/ATI RS690M Console blanks to white
https://bugzilla.kernel.org/show_bug.cgi?id=88501 crocket (crockabisc...@gmail.com) changed: What|Removed |Added CC||crockabisc...@gmail.com --- Comment #11 from crocket (crockabisc...@gmail.com) --- https://bugzilla.kernel.org/show_bug.cgi?id=202739 might be a fix. -- 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
[Bug 60623] White Screen on boot with radeon.dpm=1
https://bugzilla.kernel.org/show_bug.cgi?id=60623 crocket (crockabisc...@gmail.com) changed: What|Removed |Added CC||crockabisc...@gmail.com --- Comment #2 from crocket (crockabisc...@gmail.com) --- https://bugzilla.kernel.org/show_bug.cgi?id=202739 might be a fix. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/vgem: fix use-after-free when drm_gem_handle_create() fails
On 02/26, Eric Biggers wrote:
> From: Eric Biggers
>
> If drm_gem_handle_create() fails in vgem_gem_create(), then the
> drm_vgem_gem_object is freed twice: once when the reference is dropped
> by drm_gem_object_put_unlocked(), and again by __vgem_gem_destroy().
>
> This was hit by syzkaller using fault injection.
>
> Fix it by skipping the second free.
>
> Reported-by: syzbot+e73f2fb5ed5a5df36...@syzkaller.appspotmail.com
> Fixes: af33a9190d02 ("drm/vgem: Enable dmabuf import interfaces")
> Reviewed-by: Chris Wilson
> Cc: Laura Abbott
> Cc: Daniel Vetter
> Cc: sta...@vger.kernel.org
> Signed-off-by: Eric Biggers
> ---
> drivers/gpu/drm/vgem/vgem_drv.c | 6 +-
> 1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c
> index 5930facd6d2d8..11a8f99ba18c5 100644
> --- a/drivers/gpu/drm/vgem/vgem_drv.c
> +++ b/drivers/gpu/drm/vgem/vgem_drv.c
> @@ -191,13 +191,9 @@ static struct drm_gem_object *vgem_gem_create(struct
> drm_device *dev,
> ret = drm_gem_handle_create(file, >base, handle);
> drm_gem_object_put_unlocked(>base);
> if (ret)
> - goto err;
> + return ERR_PTR(ret);
>
> return >base;
> -
> -err:
> - __vgem_gem_destroy(obj);
> - return ERR_PTR(ret);
> }
>
> static int vgem_gem_dumb_create(struct drm_file *file, struct drm_device
> *dev,
> --
> 2.21.0.rc2.261.ga7da99ff1b-goog
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Applied to drm-misc-fixes.
Thanks
--
Rodrigo Siqueira
https://siqueira.tech
Graduate Student
Department of Computer Science
University of São Paulo
signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/vkms: fix use-after-free when drm_gem_handle_create() fails
On 02/28, Dmitry Vyukov wrote:
> On Thu, Feb 28, 2019 at 12:12 AM Rodrigo Siqueira
> wrote:
> >
> > On 02/26, Eric Biggers wrote:
> > > From: Eric Biggers
> > >
> > > If drm_gem_handle_create() fails in vkms_gem_create(), then the
> > > vkms_gem_object is freed twice: once when the reference is dropped by
> > > drm_gem_object_put_unlocked(), and again by the extra calls to
> > > drm_gem_object_release() and kfree().
> > >
> > > Fix it by skipping the second release and free.
> > >
> > > This bug was originally found in the vgem driver by syzkaller using
> > > fault injection, but I noticed it's also present in the vkms driver.
> > >
> > > Fixes: 559e50fd34d1 ("drm/vkms: Add dumb operations")
> > > Cc: Rodrigo Siqueira
> > > Cc: Haneen Mohammed
> > > Cc: Daniel Vetter
> > > Cc: Chris Wilson
> > > Cc: sta...@vger.kernel.org
> > > Signed-off-by: Eric Biggers
> > > ---
> > > drivers/gpu/drm/vkms/vkms_gem.c | 5 +
> > > 1 file changed, 1 insertion(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/vkms/vkms_gem.c
> > > b/drivers/gpu/drm/vkms/vkms_gem.c
> > > index 138b0bb325cf9..69048e73377dc 100644
> > > --- a/drivers/gpu/drm/vkms/vkms_gem.c
> > > +++ b/drivers/gpu/drm/vkms/vkms_gem.c
> > > @@ -111,11 +111,8 @@ struct drm_gem_object *vkms_gem_create(struct
> > > drm_device *dev,
> > >
> > > ret = drm_gem_handle_create(file, >gem, handle);
> > > drm_gem_object_put_unlocked(>gem);
> > > - if (ret) {
> > > - drm_gem_object_release(>gem);
> > > - kfree(obj);
> > > + if (ret)
> > > return ERR_PTR(ret);
> > > - }
> > >
> > > return >gem;
> > > }
> > > --
> > > 2.21.0.rc2.261.ga7da99ff1b-goog
> > >
> >
> > Hi,
> >
> > Thanks for your patch! :)
> >
> > The patch looks good for me. I also tested it under the IGT tests on my
> > local VM and everything was fine.
Hi,
Patch applied to drm-misc-fixes.
> Hi Rodrigo,
>
> What are IGT tests? How can I run them?
Hi Dmitry,
IGT is a test suite focused on DRM drivers.
You can clone the project using the link below:
https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
In the README, you will find the software dependencies. After you
install all the required package, just use:
mkdir build && meson build && cd build && ninja
Finally, if you want to test VKMS, I recommend you to do it inside a VM.
Best Regards
Rodrigo Siqueira
> >
> > Reviewed-by: Rodrigo Siqueira
> >
> > --
> > Rodrigo Siqueira
> > https://siqueira.tech
> > Graduate Student
> > Department of Computer Science
> > University of São Paulo
> >
> > --
> > You received this message because you are subscribed to the Google Groups
> > "syzkaller-bugs" group.
> > To unsubscribe from this group and stop receiving emails from it, send an
> > email to syzkaller-bugs+unsubscr...@googlegroups.com.
> > To view this discussion on the web visit
> > https://groups.google.com/d/msgid/syzkaller-bugs/20190227231202.tycdbcqtk5ylwp4k%40smtp.gmail.com.
> > For more options, visit https://groups.google.com/d/optout.
--
Rodrigo Siqueira
https://siqueira.tech
Graduate Student
Department of Computer Science
University of São Paulo
signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC v4 00/17] kunit: introduce KUnit, the Linux kernel unit testing framework
On Thu, Feb 14, 2019 at 1:38 PM Brendan Higgins wrote: > > This patch set proposes KUnit, a lightweight unit testing and mocking > framework for the Linux kernel. > > ## More information on KUnit > > There is a bunch of documentation near the end of this patch set that > describes how to use KUnit and best practices for writing unit tests. > For convenience I am hosting the compiled docs here: > https://google.github.io/kunit-docs/third_party/kernel/docs/ > Additionally for convenience, I have applied these patches to a branch: > https://kunit.googlesource.com/linux/+/kunit/rfc/5.0-rc5/v4 > The repo may be cloned with: > git clone https://kunit.googlesource.com/linux > This patchset is on the kunit/rfc/5.0-rc5/v4 branch. > > ## Changes Since Last Version > > - Got KUnit working on (hypothetically) all architectures (tested on >x86), as per Rob's (and other's) request > - Punting all KUnit features/patches depending on UML for now. > - Broke out UML specific support into arch/um/* as per "[RFC v3 01/19] >kunit: test: add KUnit test runner core", as requested by Luis. > - Added support to kunit_tool to allow it to build kernels in external >directories, as suggested by Kieran. > - Added a UML defconfig, and a config fragment for KUnit as suggested >by Kieran and Luis. > - Cleaned up, and reformatted a bunch of stuff. > > -- > 2.21.0.rc0.258.g878e2cd30e-goog > Someone suggested I should send the next revision out as "PATCH" instead of "RFC" since there seems to be general consensus about everything at a high level, with a couple exceptions. At this time I am planning on sending the next revision out as "[PATCH v1 00/NN] kunit: introduce KUnit, the Linux kernel unit testing framework". Initially I wasn't sure if the next revision should be "[PATCH v1 ...]" or "[PATCH v5 ...]". Please let me know if you have a strong objection to the former. In the next revision, I will be dropping the last two of three patches for the DT unit tests as there doesn't seem to be enough features currently available to justify the heavy refactoring I did; however, I will still include the patch that just converts everything over to KUnit without restructuring the test cases: https://lkml.org/lkml/2019/2/14/1133 I should have the next revision out in a week or so. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC v4 08/17] kunit: test: add support for test abort
On Thu, Feb 28, 2019 at 10:02 AM Stephen Boyd wrote:
>
> Quoting Brendan Higgins (2019-02-28 01:03:24)
> > On Tue, Feb 26, 2019 at 12:35 PM Stephen Boyd wrote:
> > >
> > > when they need to abort and then the test runner would detect that error
> > > via the return value from the 'run test' function. That would be a more
> > > direct approach, but also more verbose than a single KUNIT_ASSERT()
> > > line. It would be more kernel idiomatic too because the control flow
> >
> > Yeah, I was intentionally going against that idiom. I think that idiom
> > makes test logic more complicated than it needs to be, especially if
> > the assertion failure happens in a helper function; then you have to
> > pass that error all the way back up. It is important that test code
> > should be as simple as possible to the point of being immediately
> > obviously correct at first glance because there are no tests for
> > tests.
> >
> > The idea with assertions is that you use them to state all the
> > preconditions for your test. Logically speaking, these are the
> > premises of the test case, so if a premise isn't true, there is no
> > point in continuing the test case because there are no conclusions
> > that can be drawn without the premises. Whereas, the expectation is
> > the thing you are trying to prove. It is not used universally in
> > x-unit style test frameworks, but I really like it as a convention.
> > You could still express the idea of a premise using the above idiom,
> > but I think KUNIT_ASSERT_* states the intended idea perfectly.
>
> Fair enough. It would be great if these sorts of things were described
> in the commit text.
Good point. Will fix.
>
> Is the assumption that things like held locks and refcounted elements
> won't exist when one of these assertions is made? It sounds like some of
> the cleanup logic could be fairly complicated if a helper function
> changes some state and then an assert fails and we have to unwind all
> the state from a corrupt location. A similar problem exists for a test
> timeout too. How do we get back to a sane state if the test locks up for
> a long time? Just don't try?
It depends on the situation, if it is part of a KUnit test itself
(probably not code under test), then you can use the kunit_resource
API: https://lkml.org/lkml/2019/2/14/1125; it is inspired by the
devm_* family of functions, such that when a KUnit test case ends, for
any reason, all the kunit_resources are automatically cleaned up.
Similarly, kunit_module.exit is called at the end of every test case,
regardless of how it terminates.
>
> >
> > > isn't hidden inside a macro and it isn't intimately connected with
> > > kthreads and completions.
> >
> > Yeah, I wasn't a fan of that myself, but it was broadly available. My
> > previous version (still the architecture specific version for UML, not
> > in this patchset though) relies on UML_LONGJMP, but is obviously only
> > works on UML. A number of people wanted support for other
> > architectures. Rob and Luis specifically wanted me to provide a
> > version of abort that would work on any architecture, even if it only
> > had reduced functionality; I thought this fit the bill okay.
>
> Ok.
>
> >
> > >
> > > >
> > > > diff --git a/kunit/test.c b/kunit/test.c
> > > > index d18c50d5ed671..6e5244642ab07 100644
> > > > --- a/kunit/test.c
> > > > +++ b/kunit/test.c
> > > [...]
> > > > +
> > > > +static void kunit_generic_throw(struct kunit_try_catch *try_catch)
> > > > +{
> > > > + try_catch->context.try_result = -EFAULT;
> > > > + complete_and_exit(try_catch->context.try_completion, -EFAULT);
> > > > +}
> > > > +
> > > > +static int kunit_generic_run_threadfn_adapter(void *data)
> > > > +{
> > > > + struct kunit_try_catch *try_catch = data;
> > > >
> > > > + try_catch->try(_catch->context);
> > > > +
> > > > + complete_and_exit(try_catch->context.try_completion, 0);
> > >
> > > The exit code doesn't matter, right? If so, it might be clearer to just
> > > return 0 from this function because kthreads exit themselves as far as I
> > > recall.
> >
> > You mean complete and then return?
>
> Yes. I was confused for a minute because I thought the exit code was
> checked, but it isn't. Instead, the try_catch->context.try_result is
> where the test result goes, so calling exit explicitly doesn't seem to
> be important here, but it is important in the throw case.
Yep.
>
> >
> > >
> > > > + else if (exit_code)
> > > > + kunit_err(test, "Unknown error: %d", exit_code);
> > > > +}
> > > > +
> > > > +void kunit_generic_try_catch_init(struct kunit_try_catch *try_catch)
> > > > +{
> > > > + try_catch->run = kunit_generic_run_try_catch;
> > >
> > > Is the idea that 'run' would be anything besides
> > > 'kunit_generic_run_try_catch'? If it isn't going to be different, then
> >
> > Yeah, it can be overridden with an architecture specific version.
> >
> > > maybe it's simpler to just have a function like
> > >
Re: [RFC v4 08/17] kunit: test: add support for test abort
On Thu, Feb 28, 2019 at 5:55 AM Dan Carpenter wrote:
>
> On Thu, Feb 28, 2019 at 01:03:24AM -0800, Brendan Higgins wrote:
> > you could do:
> >
> > if (IS_ERR_OR_NULL(ptr)) {
> > KUNIT_FAIL(test, "ptr is an errno or null: %ld", ptr);
> > return;
> > }
>
> It's best to not mix error pointers and NULL but when we do mix them,
> it means that NULL is a special kind of success. Like we try to load
> a feature and we get back:
>
> valid pointer <-- success
> null <-- feature is disabled. not an error.
> error pointer <-- feature is broken. fail.
Thanks for pointing that out! Will fix.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Freedreno] [PATCH v2 1/4] drm/msm/dpu: add atomic private object to dpu crtc
On Wed, Feb 13, 2019 at 05:52:19PM -0800, Jeykumar Sankaran wrote:
> Subclass drm private object state for DPU for handling driver
> specific data. Adds atomic private object to dpu crtc to
> track hw resources per display. Provide helper function to
> retrieve DPU private data from current atomic state before
> atomic swap.
>
> changes in v2:
> - private objects are maintained in dpu_crtc as
> the resources are tracked per display
Seems like you could just store it in crtc_state if it's per-crtc?
Sean
>
> Signed-off-by: Jeykumar Sankaran
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h | 3 ++
> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 64
> +++-
> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 15
> 3 files changed, 81 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> index e59d62b..be07554 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> @@ -141,6 +141,7 @@ struct dpu_crtc_frame_event {
> * @frame_pending : Whether or not an update is pending
> * @frame_events : static allocation of in-flight frame events
> * @frame_event_list : available frame event list
> + * @priv_obj : private state object to track hw resources
> * @spin_lock : spin lock for frame event, transaction status, etc...
> * @frame_done_comp: for frame_event_done synchronization
> * @event_thread : Pointer to event handler thread
> @@ -176,6 +177,8 @@ struct dpu_crtc {
> spinlock_t spin_lock;
> struct completion frame_done_comp;
>
> + struct drm_private_obj priv_obj;
> +
> /* for handling internal event thread */
> spinlock_t event_lock;
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index 885bf88..1677862 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -459,6 +459,49 @@ static int _dpu_kms_setup_displays(struct drm_device
> *dev,
> return _dpu_kms_initialize_dsi(dev, priv, dpu_kms);
> }
>
> +struct dpu_private_state *dpu_get_private_state(struct drm_atomic_state
> *state,
> + struct dpu_crtc *crtc)
> +{
> + struct drm_private_state *priv_state;
> +
> + priv_state = drm_atomic_get_private_obj_state(state, >priv_obj);
> + if (IS_ERR(priv_state))
> + return ERR_PTR(-ENOMEM);
> +
> + return container_of(priv_state, struct dpu_private_state, base);
> +}
> +
> +static struct drm_private_state *
> +dpu_private_obj_duplicate_state(struct drm_private_obj *obj)
> +{
> + struct dpu_private_state *dpu_priv_state;
> +
> + dpu_priv_state = kmemdup(obj->state, sizeof(*dpu_priv_state),
> + GFP_KERNEL);
> + if (!dpu_priv_state)
> + return NULL;
> +
> + __drm_atomic_helper_private_obj_duplicate_state(obj,
> + _priv_state->base);
> +
> + return _priv_state->base;
> +}
> +
> +static void dpu_private_obj_destroy_state(struct drm_private_obj *obj,
> + struct drm_private_state *state)
> +{
> + struct dpu_private_state *dpu_priv_state = container_of(state,
> + struct dpu_private_state,
> + base);
> +
> + kfree(dpu_priv_state);
> +}
> +
> +static const struct drm_private_state_funcs priv_obj_funcs = {
> + .atomic_duplicate_state = dpu_private_obj_duplicate_state,
> + .atomic_destroy_state = dpu_private_obj_destroy_state,
> +};
> +
> static void _dpu_kms_drm_obj_destroy(struct dpu_kms *dpu_kms)
> {
> struct msm_drm_private *priv;
> @@ -476,8 +519,12 @@ static void _dpu_kms_drm_obj_destroy(struct dpu_kms
> *dpu_kms)
> }
> priv = dpu_kms->dev->dev_private;
>
> - for (i = 0; i < priv->num_crtcs; i++)
> + for (i = 0; i < priv->num_crtcs; i++) {
> + struct dpu_crtc *dpu_crtc = to_dpu_crtc(priv->crtcs[i]);
> +
> + drm_atomic_private_obj_fini(_crtc->priv_obj);
> priv->crtcs[i]->funcs->destroy(priv->crtcs[i]);
> + }
> priv->num_crtcs = 0;
>
> for (i = 0; i < priv->num_planes; i++)
> @@ -499,6 +546,7 @@ static int _dpu_kms_drm_obj_init(struct dpu_kms *dpu_kms)
> struct drm_plane *primary_planes[MAX_PLANES], *plane;
> struct drm_plane *cursor_planes[MAX_PLANES] = { NULL };
> struct drm_crtc *crtc;
> + struct dpu_private_state *dpu_priv_state;
>
> struct msm_drm_private *priv;
> struct dpu_mdss_cfg *catalog;
> @@ -560,11 +608,25 @@ static int _dpu_kms_drm_obj_init(struct dpu_kms
> *dpu_kms)
>
> /* Create one CRTC per encoder */
> for (i = 0; i < max_crtc_count; i++) {
> + struct dpu_crtc *dpu_crtc;
> +
>
Re: [PATCH 1/7] drm/vc4: hdmi: Check that the monitor supports HDMI audio
Stefan Wahren writes: >> Eric Anholt hat am 4. März 2019 um 19:28 geschrieben: >> >> >> Stefan Wahren writes: >> >> > Hi Maxime, >> > >> > Am 04.03.2019 um 15:52 schrieb Maxime Ripard: >> >> The current code assumes as soon as the device is an HDMI one that it >> >> supports an audio sink. However, strictly speaking, this is exposed as a >> >> separate part of EDID. >> >> >> >> This can be checked through the drm_detect_monitor_audio function, so >> >> let's >> >> use it and make sure that we can use the HDMI monitor as an output before >> >> sending sound. >> > >> > does the audio output work in the following setup after applying this >> > patch? >> > >> > VC4 --- HDMI Audio extractor --- Non audio capable monitor >> >> A 1-minute google of audio extractors says they manage the EDID. Do you >> have some reason to think this wouldn't work? > > My only concern is the existence of some audio extractors which doesn't care > about EDID. I that case we need to provide some kind of force switch. I'm having a hard time imagining such a product existing, given that I don't think Windows will have "ignore the EDID, force audio anyway" switches in their UIs. signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RESEND] drm/msm: Truncate the buffer object name if the copy from user failed
On Tue, Feb 19, 2019 at 11:40:19AM -0700, Jordan Crouse wrote:
> (Resend since there was a compile error that I forgot to commit before
> sending)
>
> If there is a error while doing a copy_from_user() for MSM_INFO_SET_NAME
> make sure to truncate the object name so that there isn't a chance that
> we'll have random data in the string.
>
> This is on top of [1] reported and fixed by Dan Carpenter.
>
> [1] https://patchwork.freedesktop.org/series/56656/
>
> Fixes: f05c83e77460 ("drm/msm: add uapi to get/set debug name")
> Reported-by: Dan Carpenter
> Signed-off-by: Jordan Crouse
Reviewed-by: Sean Paul
> ---
>
> drivers/gpu/drm/msm/msm_drv.c | 5 -
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> index 87eae44..906b2bb 100644
> --- a/drivers/gpu/drm/msm/msm_drv.c
> +++ b/drivers/gpu/drm/msm/msm_drv.c
> @@ -852,8 +852,11 @@ static int msm_ioctl_gem_info(struct drm_device *dev,
> void *data,
> break;
> }
> if (copy_from_user(msm_obj->name, u64_to_user_ptr(args->value),
> -args->len))
> +args->len)) {
> + msm_obj->name[0] = '\0';
> ret = -EFAULT;
> + break;
> + }
> msm_obj->name[args->len] = '\0';
> for (i = 0; i < args->len; i++) {
> if (!isprint(msm_obj->name[i])) {
> --
> 2.7.4
>
--
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 1/7] drm/vc4: hdmi: Check that the monitor supports HDMI audio
> Eric Anholt hat am 4. März 2019 um 19:28 geschrieben: > > > Stefan Wahren writes: > > > Hi Maxime, > > > > Am 04.03.2019 um 15:52 schrieb Maxime Ripard: > >> The current code assumes as soon as the device is an HDMI one that it > >> supports an audio sink. However, strictly speaking, this is exposed as a > >> separate part of EDID. > >> > >> This can be checked through the drm_detect_monitor_audio function, so let's > >> use it and make sure that we can use the HDMI monitor as an output before > >> sending sound. > > > > does the audio output work in the following setup after applying this patch? > > > > VC4 --- HDMI Audio extractor --- Non audio capable monitor > > A 1-minute google of audio extractors says they manage the EDID. Do you > have some reason to think this wouldn't work? My only concern is the existence of some audio extractors which doesn't care about EDID. I that case we need to provide some kind of force switch. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/7] drm/vc4: Allow for more boot-time configuration
Maxime Ripard writes: > Hi, > > The proprietary stack for the RaspberryPi allows for a number of video > parameters widely used by their users, but yet don't have any equivalents > in the mainline kernel. > > Those options are detailed here: > https://www.raspberrypi.org/documentation/configuration/config-txt/video.md > > While not all of them are desirable to have in the mainline kernel, some of > them still have value, such as properties to initialise the overscan or > rotation parameters, or the one to deal with broken displays. > > This series is an attempt to support those, and is based on a rewrite of > the command line parser I did a couple of years ago that never reached > upstream (due to a lack of time on my side). While this parser was > initially made to deal with named modes (in order to support TV modes), it > also allowed to extend it more easily, which is why it's resurrected. FWIW for other reviewers, the overscan and rotation are the really important parts of this series. Since Raspberry Pi ends up connected to TVs so frequently, there are many users of the overscan workaround. Rotation is important for supporting the official DSI touchscreen panel, which is unfortunately mounted upside down in most mounts you'll find. > Since a change of the command line parser can pretty easily get things > wrong and introduce regressions, I also worked with a number of unit tests > that you can find here: http://code.bulix.org/tpo7dg-607264?raw Would kselftests be an appropriate way to include these, maybe? signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/7] drm/edid: Allow to ignore the audio EDID data
On Mon, Mar 4, 2019 at 2:53 PM Eric Anholt wrote: > > Maxime Ripard writes: > > > In some cases, in order to accomodate with displays with poor EDIDs, we > > need to ignore that the monitor alledgedly supports audio output and > > disable the audio output. > > > > Signed-off-by: Maxime Ripard > > --- > > drivers/gpu/drm/drm_edid.c | 8 > > 1 file changed, 8 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > > index 990b1909f9d7..c0258b011bb2 100644 > > --- a/drivers/gpu/drm/drm_edid.c > > +++ b/drivers/gpu/drm/drm_edid.c > > @@ -4190,6 +4190,11 @@ bool drm_detect_hdmi_monitor(struct edid *edid) > > } > > EXPORT_SYMBOL(drm_detect_hdmi_monitor); > > > > +static bool ignore_edid_audio = false; > > +module_param(ignore_edid_audio, bool, 0644); > > +MODULE_PARM_DESC(ignore_edid_audio, > > + "Ignore the EDID and always consider that a monitor doesn't > > have audio capabilities"); > > + > > /** > > * drm_detect_monitor_audio - check monitor audio capability > > * @edid: EDID block to scan > > @@ -4209,6 +4214,9 @@ bool drm_detect_monitor_audio(struct edid *edid) > > bool has_audio = false; > > int start_offset, end_offset; > > > > + if (ignore_edid_audio) > > + goto end; > > + > > edid_ext = drm_find_cea_extension(edid); > > if (!edid_ext) > > goto end; > > It looks like the motivation for the original flag on Raspberry Pi was > "I've got a non-audio monitor, but the system comes up trying to play > audio to HDMI instead of the analog jack". Do we have some way for DRM > to communicate to ALSA that this is not the right place to try to play > audio by default? Apparently not. We have users using debug knobs in our drivers to disable display audio because ALSA defaults to that rather than other audio. Alex ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/7] drm/edid: Allow to ignore the HDMI monitor mode
Maxime Ripard writes: > Signed-off-by: Maxime Ripard Googling for users of the firmware's hdmi_drive= flag, I'm seeing lots of people with hdmi_drive=2 (force HDMI mode) due to Raspberry Pi not allowing HDMI audio with DMT modes, which it looks like DRM does allow. The only users of hdmi_drive=1 (force HDMI mode) I'm seeing are people setting things up for non-EDID monitors, but I think the expectation in DRM is that people would provide an EDID. So, I'm not sure it makes sense to support an equivalent of this flag? Or, it should probably be part of specifying a custom mode on the command line. signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] gpu: host1x: avoid IOMMU_API build error
drivers/gpu/host1x/hw/channel_hw.c: In function 'host1x_channel_set_streamid':
drivers/gpu/host1x/hw/channel_hw.c:118:30: error: implicit declaration of
function 'dev_iommu_fwspec_get'; did you mean 'iommu_fwspec_free'?
[-Werror=implicit-function-declaration]
struct iommu_fwspec *spec = dev_iommu_fwspec_get(channel->dev->parent);
^~~~
iommu_fwspec_free
drivers/gpu/host1x/hw/channel_hw.c:118:30: error: initialization of 'struct
iommu_fwspec *' from 'int' makes pointer from integer without a cast
[-Werror=int-conversion]
drivers/gpu/host1x/hw/channel_hw.c:119:23: error: 'struct iommu_fwspec' has no
member named 'ids'
u32 sid = spec ? spec->ids[0] & 0x : 0x7f;
^~
cc1: all warnings being treated as errors
Fixes: de5469c21ff9 ("gpu: host1x: Program the channel stream ID")
Signed-off-by: Arnd Bergmann
---
drivers/gpu/host1x/hw/channel_hw.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/host1x/hw/channel_hw.c
b/drivers/gpu/host1x/hw/channel_hw.c
index 27101c04a827..738dccf4ee3f 100644
--- a/drivers/gpu/host1x/hw/channel_hw.c
+++ b/drivers/gpu/host1x/hw/channel_hw.c
@@ -114,7 +114,7 @@ static inline void synchronize_syncpt_base(struct
host1x_job *job)
static void host1x_channel_set_streamid(struct host1x_channel *channel)
{
-#if HOST1X_HW >= 6
+#if HOST1X_HW >= 6 && defined(CONFIG_IOMMU_API)
struct iommu_fwspec *spec = dev_iommu_fwspec_get(channel->dev->parent);
u32 sid = spec ? spec->ids[0] & 0x : 0x7f;
--
2.20.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] fbdev: mbx: fix a misspelled variable name
A recent cleanup introduced a build failure when a variable
was spelled incorrectly:
In file included from drivers/video/fbdev/mbx/mbxfb.c:881:
drivers/video/fbdev/mbx/mbxdebugfs.c: In function 'mbxfb_debugfs_init':
drivers/video/fbdev/mbx/mbxdebugfs.c:217:2: error: 'mbfi' undeclared (first use
in this function); did you mean 'mfbi'?
mbfi->debugfs_dir = dir;
^~~~
mfbi
drivers/video/fbdev/mbx/mbxdebugfs.c:217:2: note: each undeclared identifier is
reported only once for each function it appears in
drivers/video/fbdev/mbx/mbxdebugfs.c:213:21: error: unused variable 'mfbi'
[-Werror=unused-variable]
struct mbxfb_info *mfbi = fbi->par;
^~~~
Fixes: 72aed9e31344 ("fbdev: mbx: fix up debugfs file creation")
Signed-off-by: Arnd Bergmann
---
drivers/video/fbdev/mbx/mbxdebugfs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/mbx/mbxdebugfs.c
b/drivers/video/fbdev/mbx/mbxdebugfs.c
index 52cfe0132b25..09af721638fb 100644
--- a/drivers/video/fbdev/mbx/mbxdebugfs.c
+++ b/drivers/video/fbdev/mbx/mbxdebugfs.c
@@ -214,7 +214,7 @@ static void mbxfb_debugfs_init(struct fb_info *fbi)
struct dentry *dir;
dir = debugfs_create_dir("mbxfb", NULL);
- mbfi->debugfs_dir = dir;
+ mfbi->debugfs_dir = dir;
debugfs_create_file("sysconf", 0444, dir, fbi, _fops);
debugfs_create_file("clock", 0444, dir, fbi, _fops);
--
2.20.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/7] drm/edid: Allow to ignore the audio EDID data
Maxime Ripard writes: > In some cases, in order to accomodate with displays with poor EDIDs, we > need to ignore that the monitor alledgedly supports audio output and > disable the audio output. > > Signed-off-by: Maxime Ripard > --- > drivers/gpu/drm/drm_edid.c | 8 > 1 file changed, 8 insertions(+) > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > index 990b1909f9d7..c0258b011bb2 100644 > --- a/drivers/gpu/drm/drm_edid.c > +++ b/drivers/gpu/drm/drm_edid.c > @@ -4190,6 +4190,11 @@ bool drm_detect_hdmi_monitor(struct edid *edid) > } > EXPORT_SYMBOL(drm_detect_hdmi_monitor); > > +static bool ignore_edid_audio = false; > +module_param(ignore_edid_audio, bool, 0644); > +MODULE_PARM_DESC(ignore_edid_audio, > + "Ignore the EDID and always consider that a monitor doesn't > have audio capabilities"); > + > /** > * drm_detect_monitor_audio - check monitor audio capability > * @edid: EDID block to scan > @@ -4209,6 +4214,9 @@ bool drm_detect_monitor_audio(struct edid *edid) > bool has_audio = false; > int start_offset, end_offset; > > + if (ignore_edid_audio) > + goto end; > + > edid_ext = drm_find_cea_extension(edid); > if (!edid_ext) > goto end; It looks like the motivation for the original flag on Raspberry Pi was "I've got a non-audio monitor, but the system comes up trying to play audio to HDMI instead of the analog jack". Do we have some way for DRM to communicate to ALSA that this is not the right place to try to play audio by default? signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 10/17] drm/msm: Convert to using __drm_atomic_helper_crtc_reset() for reset.
On Fri, Mar 01, 2019 at 01:56:20PM +0100, Maarten Lankhorst wrote:
> Convert msm to using __drm_atomic_helper_crtc_reset(), instead of
> writing its own version. Instead of open coding
> destroy_state(), call it directly for freeing the old state.
>
> Signed-off-by: Maarten Lankhorst
> Cc: Rob Clark
> Cc: Sean Paul
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 6 ++---
> drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c | 28 +--
> 2 files changed, 13 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index b776fca571f3..eb156cb73dd4 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -753,14 +753,12 @@ void dpu_crtc_commit_kickoff(struct drm_crtc *crtc,
> bool async)
>
> static void dpu_crtc_reset(struct drm_crtc *crtc)
> {
> - struct dpu_crtc_state *cstate;
> + struct dpu_crtc_state *cstate = kzalloc(sizeof(*cstate), GFP_KERNEL);
>
> if (crtc->state)
> dpu_crtc_destroy_state(crtc, crtc->state);
>
> - crtc->state = kzalloc(sizeof(*cstate), GFP_KERNEL);
> - if (crtc->state)
> - crtc->state->crtc = crtc;
> + __drm_atomic_helper_crtc_reset(crtc, >base);
> }
>
> /**
> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
> b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
> index b0cf63c4e3d7..bf24a08feab9 100644
> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
> @@ -1002,23 +1002,6 @@ mdp5_crtc_atomic_print_state(struct drm_printer *p,
> drm_printf(p, "\tcmd_mode=%d\n", mdp5_cstate->cmd_mode);
> }
>
> -static void mdp5_crtc_reset(struct drm_crtc *crtc)
> -{
> - struct mdp5_crtc_state *mdp5_cstate;
> -
> - if (crtc->state) {
> - __drm_atomic_helper_crtc_destroy_state(crtc->state);
> - kfree(to_mdp5_crtc_state(crtc->state));
> - }
> -
> - mdp5_cstate = kzalloc(sizeof(*mdp5_cstate), GFP_KERNEL);
> -
> - if (mdp5_cstate) {
> - mdp5_cstate->base.crtc = crtc;
> - crtc->state = _cstate->base;
> - }
> -}
> -
> static struct drm_crtc_state *
> mdp5_crtc_duplicate_state(struct drm_crtc *crtc)
> {
> @@ -1046,6 +1029,17 @@ static void mdp5_crtc_destroy_state(struct drm_crtc
> *crtc, struct drm_crtc_state
> kfree(mdp5_cstate);
> }
>
> +static void mdp5_crtc_reset(struct drm_crtc *crtc)
> +{
> + struct mdp5_crtc_state *mdp5_cstate =
> + mdp5_cstate = kzalloc(sizeof(*mdp5_cstate), GFP_KERNEL);
Assigned twice for good measure ;)
> +
> + if (crtc->state)
> + mdp5_crtc_destroy_state(crtc, crtc->state);
> +
> + __drm_atomic_helper_crtc_reset(crtc, _cstate->base);
> +}
> +
> static const struct drm_crtc_funcs mdp5_crtc_funcs = {
> .set_config = drm_atomic_helper_set_config,
> .destroy = mdp5_crtc_destroy,
> --
> 2.20.1
>
--
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-next] drm/amdgpu: fix missing assignment of error return code to variable ret
On Sat, Mar 2, 2019 at 5:17 PM Colin King wrote:
>
> From: Colin Ian King
>
> An earlier commit replaced ttm_bo_wait with amdgpu_bo_sync_wait and
> removed the error return assignment to variable ret. Fix this by adding
> the assignment back. Also break line to clean up checkpatch overly
> long line warning.
>
> Detected by CoverityScan, CID#1477327 ("Logically dead code")
>
> Fixes: c60cd590cb7d ("drm/amdgpu: Replace ttm_bo_wait with
> amdgpu_bo_sync_wait")
> Signed-off-by: Colin Ian King
Applied. thanks!
Alex
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> index 1921dec3df7a..92993baac91a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> @@ -906,7 +906,8 @@ static int init_kfd_vm(struct amdgpu_vm *vm, void
> **process_info,
> pr_err("validate_pt_pd_bos() failed\n");
> goto validate_pd_fail;
> }
> - amdgpu_bo_sync_wait(vm->root.base.bo, AMDGPU_FENCE_OWNER_KFD, false);
> + ret = amdgpu_bo_sync_wait(vm->root.base.bo,
> + AMDGPU_FENCE_OWNER_KFD, false);
> if (ret)
> goto wait_pd_fail;
> amdgpu_bo_fence(vm->root.base.bo,
> --
> 2.20.1
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/amdgpu: fix typing in amdgpu_virt_ops::trans_msg
Hi Luc,
I love your patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v5.0 next-20190304]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Luc-Van-Oostenryck/drm-amdgpu-fix-typing-in-amdgpu_virt_ops-trans_msg/20190305-001104
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 8.2.0
reproduce:
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=8.2.0 make.cross ARCH=ia64
All error/warnings (new ones prefixed by >>):
In file included from drivers/gpu/drm/amd/amdgpu/amdgpu.h:77,
from
drivers/gpu/drm/amd/amdgpu/../powerplay/inc/amd_powerplay.h:33,
from drivers/gpu/drm/amd/amdgpu/../powerplay/inc/hwmgr.h:27,
from
drivers/gpu/drm/amd/amdgpu/../powerplay/hwmgr/vega12_thermal.h:27,
from
drivers/gpu/drm/amd/amdgpu/../powerplay/hwmgr/vega12_thermal.c:24:
>> drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h:59:53: warning: 'enum idh_request'
>> declared inside parameter list will not be visible outside of this
>> definition or declaration
void (*trans_msg)(struct amdgpu_device *adev, enum idh_request req, u32
data1, u32 data2, u32 data3);
^~~
--
In file included from drivers/gpu/drm/amd/amdgpu/amdgpu.h:77,
from drivers/gpu/drm/amd/amdgpu/amdgpu_vf_error.c:24:
>> drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h:59:53: warning: 'enum idh_request'
>> declared inside parameter list will not be visible outside of this
>> definition or declaration
void (*trans_msg)(struct amdgpu_device *adev, enum idh_request req, u32
data1, u32 data2, u32 data3);
^~~
drivers/gpu/drm/amd/amdgpu/amdgpu_vf_error.c: In function
'amdgpu_vf_error_trans_all':
>> drivers/gpu/drm/amd/amdgpu/amdgpu_vf_error.c:82:35: error: type of formal
>> parameter 2 is incomplete
adev->virt.ops->trans_msg(adev, IDH_LOG_VF_ERROR, data1, data2, data3);
^~~~
--
In file included from drivers/gpu/drm/amd/amdgpu/amdgpu.h:77,
from drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c:24:
>> drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h:59:53: warning: 'enum idh_request'
>> declared inside parameter list will not be visible outside of this
>> definition or declaration
void (*trans_msg)(struct amdgpu_device *adev, enum idh_request req, u32
data1, u32 data2, u32 data3);
^~~
>> drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c:376:15: error: initialization of 'void
>> (*)(struct amdgpu_device *, enum idh_request, u32, u32, u32)' {aka 'void
>> (*)(struct amdgpu_device *, enum idh_request, unsigned int, unsigned int,
>> unsigned int)'} from incompatible pointer type 'void (*)(struct
>> amdgpu_device *, enum idh_request, u32, u32, u32)' {aka 'void (*)(struct
>> amdgpu_device *, enum idh_request, unsigned int, unsigned int, unsigned
>> int)'} [-Werror=incompatible-pointer-types]
.trans_msg = xgpu_ai_mailbox_trans_msg,
^
drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c:376:15: note: (near initialization for
'xgpu_ai_virt_ops.trans_msg')
cc1: some warnings being treated as errors
vim +376 drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c
f98b617e Monk Liu 2017-04-05 370
c9c9de93 Xiangliang Yu 2017-03-10 371 const struct amdgpu_virt_ops
xgpu_ai_virt_ops = {
c9c9de93 Xiangliang Yu 2017-03-10 372 .req_full_gpu =
xgpu_ai_request_full_gpu_access,
c9c9de93 Xiangliang Yu 2017-03-10 373 .rel_full_gpu =
xgpu_ai_release_full_gpu_access,
f98b617e Monk Liu 2017-04-05 374 .reset_gpu =
xgpu_ai_request_reset,
b5914238 pding 2017-10-24 375 .wait_reset = NULL,
89041940 Gavin Wan 2017-06-23 @376 .trans_msg =
xgpu_ai_mailbox_trans_msg,
:: The code at line 376 was first introduced by commit
:: 890419409a3aba2ca7185a824e47d8ded8df11a2 drm/amdgpu: Support passing
amdgpu critical error to host via GPU Mailbox.
:: TO: Gavin Wan
:: CC: Alex Deucher
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 0/7] move dpu resource parsing to encoder modeset
On Wed, Feb 13, 2019 at 05:19:09PM -0800, Jeykumar Sankaran wrote: > Fixing some of the low hanging fruits by moving the hw resource > parsing and assignment to encoder modeset. This series > prepares DPU resource management to switch to state based > resource tracking which is implemented in the next incoming > changes. Thanks for the set, Jeykumar. I've applied the whole thing to msm-next. Sean > > Thanks. > > Jeykumar Sankaran (7): > drm/msm/dpu: move hw_inf encoder baseclass > drm/msm/dpu: remove phys_vid subclass > drm/msm/dpu: release resources on modeset failure > drm/msm/dpu: dont use encoder->crtc in atomic path > drm/msm/dpu: map mixer/ctl hw blocks in encoder modeset > drm/msm/dpu: assign intf to encoder in mode_set > drm/msm/dpu: check split role for single flush > > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 64 +--- > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c| 73 +++-- > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 15 +- > .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 177 > ++--- > 4 files changed, 118 insertions(+), 211 deletions(-) > > -- > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, > a Linux Foundation Collaborative Project > -- 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 1/7] drm/vc4: hdmi: Check that the monitor supports HDMI audio
Stefan Wahren writes: > Hi Maxime, > > Am 04.03.2019 um 15:52 schrieb Maxime Ripard: >> The current code assumes as soon as the device is an HDMI one that it >> supports an audio sink. However, strictly speaking, this is exposed as a >> separate part of EDID. >> >> This can be checked through the drm_detect_monitor_audio function, so let's >> use it and make sure that we can use the HDMI monitor as an output before >> sending sound. > > does the audio output work in the following setup after applying this patch? > > VC4 --- HDMI Audio extractor --- Non audio capable monitor A 1-minute google of audio extractors says they manage the EDID. Do you have some reason to think this wouldn't work? signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 7/7] drm/msm/dpu: check split role for single flush
On Wed, Feb 13, 2019 at 05:19:16PM -0800, Jeykumar Sankaran wrote:
> Removing unwanted access of crtc_state for finding this information.
> Use split role information to know whether we have slave ctl.
>
> Signed-off-by: Jeykumar Sankaran
Reviewed-by: Sean Paul
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 14 +-
> 1 file changed, 1 insertion(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> index 02362c5..3fc3cc0 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> @@ -347,22 +347,10 @@ static void dpu_encoder_phys_vid_underrun_irq(void
> *arg, int irq_idx)
> phys_enc);
> }
>
> -static bool _dpu_encoder_phys_is_dual_ctl(struct dpu_encoder_phys *phys_enc)
> -{
> - struct dpu_crtc_state *dpu_cstate;
> -
> - if (!phys_enc)
> - return false;
> -
> - dpu_cstate = to_dpu_crtc_state(phys_enc->parent->crtc->state);
> -
> - return dpu_cstate->num_ctls > 1;
> -}
> -
> static bool dpu_encoder_phys_vid_needs_single_flush(
> struct dpu_encoder_phys *phys_enc)
> {
> - return (phys_enc && _dpu_encoder_phys_is_dual_ctl(phys_enc));
> + return phys_enc->split_role != ENC_ROLE_SOLO;
> }
>
> static void _dpu_encoder_phys_vid_setup_irq_hw_idx(
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
--
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 6/7] drm/msm/dpu: assign intf to encoder in mode_set
On Wed, Feb 13, 2019 at 05:19:15PM -0800, Jeykumar Sankaran wrote:
> Iterate and assign HW intf block to physical encoders
> in encoder modeset. Moving all the HW block assignments
> to encoder modeset to allow easy switching to state
> based resource management.
>
> Signed-off-by: Jeykumar Sankaran
Reviewed-by: Sean Paul
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c| 22 +++-
> .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 24
> --
> 2 files changed, 21 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index f648e7f..98ea478 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -968,7 +968,7 @@ static void dpu_encoder_virt_mode_set(struct drm_encoder
> *drm_enc,
> struct dpu_hw_ctl *hw_ctl[MAX_CHANNELS_PER_ENC] = { NULL };
> struct dpu_hw_mixer *hw_lm[MAX_CHANNELS_PER_ENC] = { NULL };
> int num_lm = 0, num_ctl = 0;
> - int i = 0, ret;
> + int i, j, ret;
>
> if (!drm_enc) {
> DPU_ERROR("invalid encoder\n");
> @@ -1065,6 +1065,26 @@ static void dpu_encoder_virt_mode_set(struct
> drm_encoder *drm_enc,
> phys->hw_pp = dpu_enc->hw_pp[i];
> phys->hw_ctl = hw_ctl[i];
>
> + dpu_rm_init_hw_iter(_iter, drm_enc->base.id,
> + DPU_HW_BLK_INTF);
> + for (j = 0; j < MAX_CHANNELS_PER_ENC; j++) {
> + struct dpu_hw_intf *hw_intf;
> +
> + if (!dpu_rm_get_hw(_kms->rm, _iter))
> + break;
> +
> + hw_intf = (struct dpu_hw_intf *)hw_iter.hw;
> + if (hw_intf->idx == phys->intf_idx)
> + phys->hw_intf = hw_intf;
> + }
> +
> + if (!phys->hw_intf) {
> + DPU_ERROR_ENC(dpu_enc,
> + "no intf block assigned at idx:
> %d\n",
> + i);
> + goto error;
> + }
> +
> phys->connector = conn->state->connector;
> if (phys->ops.mode_set)
> phys->ops.mode_set(phys, mode, adj_mode);
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> index ce65521..02362c5 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> @@ -449,35 +449,11 @@ static int dpu_encoder_phys_vid_control_vblank_irq(
>
> static void dpu_encoder_phys_vid_enable(struct dpu_encoder_phys *phys_enc)
> {
> - struct msm_drm_private *priv;
> - struct dpu_rm_hw_iter iter;
> struct dpu_hw_ctl *ctl;
> u32 flush_mask = 0;
>
> - if (!phys_enc || !phys_enc->parent || !phys_enc->parent->dev ||
> - !phys_enc->parent->dev->dev_private) {
> - DPU_ERROR("invalid encoder/device\n");
> - return;
> - }
> - priv = phys_enc->parent->dev->dev_private;
> -
> ctl = phys_enc->hw_ctl;
>
> - dpu_rm_init_hw_iter(, phys_enc->parent->base.id, DPU_HW_BLK_INTF);
> - while (dpu_rm_get_hw(_enc->dpu_kms->rm, )) {
> - struct dpu_hw_intf *hw_intf = (struct dpu_hw_intf *)iter.hw;
> -
> - if (hw_intf->idx == phys_enc->intf_idx) {
> - phys_enc->hw_intf = hw_intf;
> - break;
> - }
> - }
> -
> - if (!phys_enc->hw_intf) {
> - DPU_ERROR("hw_intf not assigned\n");
> - return;
> - }
> -
> DPU_DEBUG_VIDENC(phys_enc, "\n");
>
> if (WARN_ON(!phys_enc->hw_intf->ops.enable_timing))
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
--
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 5/7] drm/msm/dpu: map mixer/ctl hw blocks in encoder modeset
On Wed, Feb 13, 2019 at 05:19:14PM -0800, Jeykumar Sankaran wrote:
> After resource allocation, iterate and populate mixer/ctl
> hw blocks in encoder modeset thereby centralizing all
> the resource mapping to the CRTC. This change is made
> for easy switching to state based allocation using
> private objects later in this series.
>
> Signed-off-by: Jeykumar Sankaran
Reviewed-by: Sean Paul
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c| 64
> +
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 37 +
> 2 files changed, 31 insertions(+), 70 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index 4e4b648..6d30ba9 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -425,65 +425,6 @@ void dpu_crtc_complete_commit(struct drm_crtc *crtc,
> trace_dpu_crtc_complete_commit(DRMID(crtc));
> }
>
> -static void _dpu_crtc_setup_mixer_for_encoder(
> - struct drm_crtc *crtc,
> - struct drm_encoder *enc)
> -{
> - struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc->state);
> - struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc);
> - struct dpu_rm *rm = _kms->rm;
> - struct dpu_crtc_mixer *mixer;
> - struct dpu_hw_ctl *last_valid_ctl = NULL;
> - int i;
> - struct dpu_rm_hw_iter lm_iter, ctl_iter;
> -
> - dpu_rm_init_hw_iter(_iter, enc->base.id, DPU_HW_BLK_LM);
> - dpu_rm_init_hw_iter(_iter, enc->base.id, DPU_HW_BLK_CTL);
> -
> - /* Set up all the mixers and ctls reserved by this encoder */
> - for (i = cstate->num_mixers; i < ARRAY_SIZE(cstate->mixers); i++) {
> - mixer = >mixers[i];
> -
> - if (!dpu_rm_get_hw(rm, _iter))
> - break;
> - mixer->hw_lm = (struct dpu_hw_mixer *)lm_iter.hw;
> -
> - /* CTL may be <= LMs, if <, multiple LMs controlled by 1 CTL */
> - if (!dpu_rm_get_hw(rm, _iter)) {
> - DPU_DEBUG("no ctl assigned to lm %d, using previous\n",
> - mixer->hw_lm->idx - LM_0);
> - mixer->lm_ctl = last_valid_ctl;
> - } else {
> - mixer->lm_ctl = (struct dpu_hw_ctl *)ctl_iter.hw;
> - last_valid_ctl = mixer->lm_ctl;
> - }
> -
> - /* Shouldn't happen, mixers are always >= ctls */
> - if (!mixer->lm_ctl) {
> - DPU_ERROR("no valid ctls found for lm %d\n",
> - mixer->hw_lm->idx - LM_0);
> - return;
> - }
> -
> - cstate->num_mixers++;
> - DPU_DEBUG("setup mixer %d: lm %d\n",
> - i, mixer->hw_lm->idx - LM_0);
> - DPU_DEBUG("setup mixer %d: ctl %d\n",
> - i, mixer->lm_ctl->idx - CTL_0);
> - }
> -}
> -
> -static void _dpu_crtc_setup_mixers(struct drm_crtc *crtc)
> -{
> - struct drm_encoder *enc;
> -
> - WARN_ON(!drm_modeset_is_locked(>mutex));
> -
> - /* Check for mixers on all encoders attached to this crtc */
> - drm_for_each_encoder_mask(enc, crtc->dev, crtc->state->encoder_mask)
> - _dpu_crtc_setup_mixer_for_encoder(crtc, enc);
> -}
> -
> static void _dpu_crtc_setup_lm_bounds(struct drm_crtc *crtc,
> struct drm_crtc_state *state)
> {
> @@ -533,10 +474,7 @@ static void dpu_crtc_atomic_begin(struct drm_crtc *crtc,
> dev = crtc->dev;
> smmu_state = _crtc->smmu_state;
>
> - if (!cstate->num_mixers) {
> - _dpu_crtc_setup_mixers(crtc);
> - _dpu_crtc_setup_lm_bounds(crtc, crtc->state);
> - }
> + _dpu_crtc_setup_lm_bounds(crtc, crtc->state);
>
> if (dpu_crtc->event) {
> WARN_ON(dpu_crtc->event);
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index 0a19124..f648e7f 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -962,9 +962,12 @@ static void dpu_encoder_virt_mode_set(struct drm_encoder
> *drm_enc,
> struct list_head *connector_list;
> struct drm_connector *conn = NULL, *conn_iter;
> struct drm_crtc *drm_crtc;
> - struct dpu_rm_hw_iter pp_iter, ctl_iter;
> + struct dpu_crtc_state *cstate;
> + struct dpu_rm_hw_iter hw_iter;
> struct msm_display_topology topology;
> struct dpu_hw_ctl *hw_ctl[MAX_CHANNELS_PER_ENC] = { NULL };
> + struct dpu_hw_mixer *hw_lm[MAX_CHANNELS_PER_ENC] = { NULL };
> + int num_lm = 0, num_ctl = 0;
> int i = 0, ret;
>
> if (!drm_enc) {
> @@ -1008,21 +1011,41 @@ static void dpu_encoder_virt_mode_set(struct
> drm_encoder *drm_enc,
> return;
> }
>
> - dpu_rm_init_hw_iter(_iter, drm_enc->base.id,
Re: [PATCH v2 4/7] drm/msm/dpu: dont use encoder->crtc in atomic path
On Wed, Feb 13, 2019 at 05:19:13PM -0800, Jeykumar Sankaran wrote:
> encoder->crtc is not really meaningful for atomic path. Use
> crtc->encoder_mask to identify the crtc attached with
> an encoder.
>
> Signed-off-by: Jeykumar Sankaran
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 7 ++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index 45617b9..0a19124 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -961,6 +961,7 @@ static void dpu_encoder_virt_mode_set(struct drm_encoder
> *drm_enc,
> struct dpu_kms *dpu_kms;
> struct list_head *connector_list;
> struct drm_connector *conn = NULL, *conn_iter;
> + struct drm_crtc *drm_crtc;
> struct dpu_rm_hw_iter pp_iter, ctl_iter;
> struct msm_display_topology topology;
> struct dpu_hw_ctl *hw_ctl[MAX_CHANNELS_PER_ENC] = { NULL };
> @@ -992,10 +993,14 @@ static void dpu_encoder_virt_mode_set(struct
> drm_encoder *drm_enc,
> return;
> }
>
> + drm_for_each_crtc(drm_crtc, drm_enc->dev)
> + if (drm_crtc->state->encoder_mask & drm_encoder_mask(drm_enc))
> + break;
You should check whether you actually found the crtc, or are just using the last
one in the list.
Sean
> +
> topology = dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode);
>
> /* Reserve dynamic resources now. Indicating non-AtomicTest phase */
> - ret = dpu_rm_reserve(_kms->rm, drm_enc, drm_enc->crtc->state,
> + ret = dpu_rm_reserve(_kms->rm, drm_enc, drm_crtc->state,
>topology, false);
> if (ret) {
> DPU_ERROR_ENC(dpu_enc,
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
--
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: [Freedreno] [PATCH v2 3/7] drm/msm/dpu: release resources on modeset failure
On Wed, Feb 13, 2019 at 05:19:12PM -0800, Jeykumar Sankaran wrote:
> release resources allocated in mode_set if any of
> the hw check fails. Most of these checks are not
> necessary and they will be removed in the follow up
> patches with state based resource allocations.
>
> Signed-off-by: Jeykumar Sankaran
Reviewed-by: Sean Paul
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 7 +--
> 1 file changed, 5 insertions(+), 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 941ac25..45617b9 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -1025,13 +1025,13 @@ static void dpu_encoder_virt_mode_set(struct
> drm_encoder *drm_enc,
> if (!dpu_enc->hw_pp[i]) {
> DPU_ERROR_ENC(dpu_enc, "no pp block assigned"
>"at idx: %d\n", i);
> - return;
> + goto error;
> }
>
> if (!hw_ctl[i]) {
> DPU_ERROR_ENC(dpu_enc, "no ctl block assigned"
>"at idx: %d\n", i);
> - return;
> + goto error;
> }
>
> phys->hw_pp = dpu_enc->hw_pp[i];
> @@ -1044,6 +1044,9 @@ static void dpu_encoder_virt_mode_set(struct
> drm_encoder *drm_enc,
> }
>
> dpu_enc->mode_set_complete = true;
> +
> +error:
> + dpu_rm_release(_kms->rm, drm_enc);
> }
>
> static void _dpu_encoder_virt_enable_helper(struct drm_encoder *drm_enc)
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
> ___
> Freedreno mailing list
> freedr...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/freedreno
--
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: [Freedreno] [PATCH v2 2/7] drm/msm/dpu: remove phys_vid subclass
On Wed, Feb 13, 2019 at 05:19:11PM -0800, Jeykumar Sankaran wrote:
> Not holding any video encoder specific data. Get rid of it.
>
> Signed-off-by: Jeykumar Sankaran
Reviewed-by: Sean Paul
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 11 ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 18 --
> 2 files changed, 4 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> index acd5956..9b1efd9 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> @@ -254,17 +254,6 @@ static inline int dpu_encoder_phys_inc_pending(struct
> dpu_encoder_phys *phys)
> }
>
> /**
> - * struct dpu_encoder_phys_vid - sub-class of dpu_encoder_phys to handle
> video
> - * mode specific operations
> - * @base:Baseclass physical encoder structure
> - * @timing_params: Current timing parameter
> - */
> -struct dpu_encoder_phys_vid {
> - struct dpu_encoder_phys base;
> - struct intf_timing_params timing_params;
> -};
> -
> -/**
> * struct dpu_encoder_phys_cmd - sub-class of dpu_encoder_phys to handle
> command
> * mode specific operations
> * @base:Baseclass physical encoder structure
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> index e326395..ce65521 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> @@ -240,7 +240,6 @@ static bool dpu_encoder_phys_vid_mode_fixup(
> static void dpu_encoder_phys_vid_setup_timing_engine(
> struct dpu_encoder_phys *phys_enc)
> {
> - struct dpu_encoder_phys_vid *vid_enc;
> struct drm_display_mode mode;
> struct intf_timing_params timing_params = { 0 };
> const struct dpu_format *fmt = NULL;
> @@ -254,7 +253,6 @@ static void dpu_encoder_phys_vid_setup_timing_engine(
> }
>
> mode = phys_enc->cached_mode;
> - vid_enc = to_dpu_encoder_phys_vid(phys_enc);
> if (!phys_enc->hw_intf->ops.setup_timing_gen) {
> DPU_ERROR("timing engine setup is not supported\n");
> return;
> @@ -293,8 +291,6 @@ static void dpu_encoder_phys_vid_setup_timing_engine(
> spin_unlock_irqrestore(phys_enc->enc_spinlock, lock_flags);
>
> programmable_fetch_config(phys_enc, _params);
> -
> - vid_enc->timing_params = timing_params;
> }
>
> static void dpu_encoder_phys_vid_vblank_irq(void *arg, int irq_idx)
> @@ -515,16 +511,13 @@ static void dpu_encoder_phys_vid_enable(struct
> dpu_encoder_phys *phys_enc)
>
> static void dpu_encoder_phys_vid_destroy(struct dpu_encoder_phys *phys_enc)
> {
> - struct dpu_encoder_phys_vid *vid_enc;
> -
> if (!phys_enc) {
> DPU_ERROR("invalid encoder\n");
> return;
> }
>
> - vid_enc = to_dpu_encoder_phys_vid(phys_enc);
> DPU_DEBUG_VIDENC(phys_enc, "\n");
> - kfree(vid_enc);
> + kfree(phys_enc);
> }
>
> static void dpu_encoder_phys_vid_get_hw_resources(
> @@ -747,7 +740,6 @@ struct dpu_encoder_phys *dpu_encoder_phys_vid_init(
> struct dpu_enc_phys_init_params *p)
> {
> struct dpu_encoder_phys *phys_enc = NULL;
> - struct dpu_encoder_phys_vid *vid_enc = NULL;
> struct dpu_encoder_irq *irq;
> int i, ret = 0;
>
> @@ -756,14 +748,12 @@ struct dpu_encoder_phys *dpu_encoder_phys_vid_init(
> goto fail;
> }
>
> - vid_enc = kzalloc(sizeof(*vid_enc), GFP_KERNEL);
> - if (!vid_enc) {
> + phys_enc = kzalloc(sizeof(*phys_enc), GFP_KERNEL);
> + if (!phys_enc) {
> ret = -ENOMEM;
> goto fail;
> }
>
> - phys_enc = _enc->base;
> -
> phys_enc->hw_mdptop = p->dpu_kms->hw_mdp;
> phys_enc->intf_idx = p->intf_idx;
>
> @@ -807,7 +797,7 @@ struct dpu_encoder_phys *dpu_encoder_phys_vid_init(
>
> fail:
> DPU_ERROR("failed to create encoder\n");
> - if (vid_enc)
> + if (phys_enc)
> dpu_encoder_phys_vid_destroy(phys_enc);
>
> return ERR_PTR(ret);
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
> ___
> Freedreno mailing list
> freedr...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/freedreno
--
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: [Freedreno] [PATCH v2 1/7] drm/msm/dpu: move hw_inf encoder baseclass
On Wed, Feb 13, 2019 at 05:19:10PM -0800, Jeykumar Sankaran wrote:
> Both video and command physical encoders will have
> a hw interface assigned to it. So there is really no
> need to track the hw block in specific encoder subclass.
>
> Signed-off-by: Jeykumar Sankaran
Reviewed-by: Sean Paul
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 4 +-
> .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 125
> +
> 2 files changed, 52 insertions(+), 77 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> index 44e6f8b6..acd5956 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> @@ -201,6 +201,7 @@ struct dpu_encoder_irq {
> * @hw_mdptop: Hardware interface to the top registers
> * @hw_ctl: Hardware interface to the ctl registers
> * @hw_pp: Hardware interface to the ping pong registers
> + * @hw_intf: Hardware interface to the intf registers
> * @dpu_kms: Pointer to the dpu_kms top level
> * @cached_mode: DRM mode cached at mode_set time, acted on in enable
> * @enabled: Whether the encoder has enabled and running a mode
> @@ -229,6 +230,7 @@ struct dpu_encoder_phys {
> struct dpu_hw_mdp *hw_mdptop;
> struct dpu_hw_ctl *hw_ctl;
> struct dpu_hw_pingpong *hw_pp;
> + struct dpu_hw_intf *hw_intf;
> struct dpu_kms *dpu_kms;
> struct drm_display_mode cached_mode;
> enum dpu_enc_split_role split_role;
> @@ -255,12 +257,10 @@ static inline int dpu_encoder_phys_inc_pending(struct
> dpu_encoder_phys *phys)
> * struct dpu_encoder_phys_vid - sub-class of dpu_encoder_phys to handle
> video
> * mode specific operations
> * @base:Baseclass physical encoder structure
> - * @hw_intf: Hardware interface to the intf registers
> * @timing_params: Current timing parameter
> */
> struct dpu_encoder_phys_vid {
> struct dpu_encoder_phys base;
> - struct dpu_hw_intf *hw_intf;
> struct intf_timing_params timing_params;
> };
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> index acdab5b0..e326395 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> @@ -18,14 +18,14 @@
> #include "dpu_trace.h"
>
> #define DPU_DEBUG_VIDENC(e, fmt, ...) DPU_DEBUG("enc%d intf%d " fmt, \
> - (e) && (e)->base.parent ? \
> - (e)->base.parent->base.id : -1, \
> + (e) && (e)->parent ? \
> + (e)->parent->base.id : -1, \
> (e) && (e)->hw_intf ? \
> (e)->hw_intf->idx - INTF_0 : -1, ##__VA_ARGS__)
>
> #define DPU_ERROR_VIDENC(e, fmt, ...) DPU_ERROR("enc%d intf%d " fmt, \
> - (e) && (e)->base.parent ? \
> - (e)->base.parent->base.id : -1, \
> + (e) && (e)->parent ? \
> + (e)->parent->base.id : -1, \
> (e) && (e)->hw_intf ? \
> (e)->hw_intf->idx - INTF_0 : -1, ##__VA_ARGS__)
>
> @@ -44,7 +44,7 @@ static bool dpu_encoder_phys_vid_is_master(
> }
>
> static void drm_mode_to_intf_timing_params(
> - const struct dpu_encoder_phys_vid *vid_enc,
> + const struct dpu_encoder_phys *phys_enc,
> const struct drm_display_mode *mode,
> struct intf_timing_params *timing)
> {
> @@ -92,7 +92,7 @@ static void drm_mode_to_intf_timing_params(
> timing->hsync_skew = mode->hskew;
>
> /* DSI controller cannot handle active-low sync signals. */
> - if (vid_enc->hw_intf->cap->type == INTF_DSI) {
> + if (phys_enc->hw_intf->cap->type == INTF_DSI) {
> timing->hsync_polarity = 0;
> timing->vsync_polarity = 0;
> }
> @@ -143,11 +143,11 @@ static u32 get_vertical_total(const struct
> intf_timing_params *timing)
> * lines based on the chip worst case latencies.
> */
> static u32 programmable_fetch_get_num_lines(
> - struct dpu_encoder_phys_vid *vid_enc,
> + struct dpu_encoder_phys *phys_enc,
> const struct intf_timing_params *timing)
> {
> u32 worst_case_needed_lines =
> - vid_enc->hw_intf->cap->prog_fetch_lines_worst_case;
> + phys_enc->hw_intf->cap->prog_fetch_lines_worst_case;
> u32 start_of_frame_lines =
> timing->v_back_porch + timing->vsync_pulse_width;
> u32 needed_vfp_lines = worst_case_needed_lines - start_of_frame_lines;
> @@ -155,26 +155,26 @@ static u32 programmable_fetch_get_num_lines(
>
> /* Fetch must be outside active lines, otherwise undefined. */
> if (start_of_frame_lines >= worst_case_needed_lines) {
> - DPU_DEBUG_VIDENC(vid_enc,
> + DPU_DEBUG_VIDENC(phys_enc,
>
Re: [PATCH v3 0/3] drm/panel: Support Rocktech jh057n00900 DSI panel
Hi Thierry. > Changes from v2 > * As per review comments from Sam Ravnborg > * Lowercase sentinel > * Drop '_panel' postfix > * DRM_DEV_ logging instead of plain DRM_ > * Add Sam's Reviewed-by: > * Add "panel-rocktech-" to the driver name following > the pattern from other drm panel drivers. With the changes done in v2 the patch series is IMO ready to be applied. Sam ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 3/3] drm/panel: Add Rocktech jh057n00900 panel driver
Hi,
On Fri, Mar 01, 2019 at 11:39:06PM +0100, Sam Ravnborg wrote:
> Hi Guido.
>
> Thanks for addressing review comments in first round.
> Just a few nits in this follow-up.
> With these nits addressed:
> Reviewed-by: Sam Ravnborg
Thanks! I made all the suggested changes in v3, on top of that I
prepended 'panel-rockteck-' to the driver name to be more consistent
with other drivers.
-- Guido
>
> On Fri, Mar 01, 2019 at 02:02:04PM +0100, Guido Günther wrote:
>
> > +#include
> This include file is, as far as I could tell, no longer used and can be
> dropped.
>
> > +
> > +static int jh057n_get_modes(struct drm_panel *panel)
> > +{
> > + struct drm_display_mode *mode;
> > +
> > + mode = drm_mode_duplicate(panel->drm, _mode);
> > + if (!mode) {
> > + DRM_ERROR("Failed to add mode %ux%u@%u",
> > + default_mode.hdisplay, default_mode.vdisplay,
> > + default_mode.vrefresh);
> > + return -ENOMEM;
> Use DRM_DEV_ERROR()
> You can find dev via: panel->base.drm-dev
>
> > +
> > +static int jh057n_probe(struct mipi_dsi_device *dsi)
> > +{
> > + struct device *dev = >dev;
> > + struct jh057n *ctx;
> > + int ret;
> > +
> > + ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> > + if (!ctx)
> > + return -ENOMEM;
> > +
> > + ctx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
> > + if (IS_ERR(ctx->reset_gpio)) {
> > + DRM_DEV_ERROR(dev, "cannot get reset gpio");
> > + return PTR_ERR(ctx->reset_gpio);
> > + }
> > +
> > + mipi_dsi_set_drvdata(dsi, ctx);
> > +
> > + ctx->dev = dev;
> > +
> > + dsi->lanes = 4;
> > + dsi->format = MIPI_DSI_FMT_RGB888;
> > + dsi->mode_flags = MIPI_DSI_MODE_VIDEO |
> > + MIPI_DSI_MODE_VIDEO_BURST | MIPI_DSI_MODE_VIDEO_SYNC_PULSE;
> > +
> > + ctx->backlight = devm_of_find_backlight(dev);
> > + if (IS_ERR(ctx->backlight))
> > + return PTR_ERR(ctx->backlight);
> > +
> > + drm_panel_init(>panel);
> > + ctx->panel.dev = dev;
> > + ctx->panel.funcs = _drm_funcs;
> > +
> > + drm_panel_add(>panel);
> > +
> > + ret = mipi_dsi_attach(dsi);
> > + if (ret < 0) {
> > + DRM_DEV_ERROR(dev, "mipi_dsi_attach failed. Is host ready?");
> > + drm_panel_remove(>panel);
> > + return ret;
> > + }
> > +
> > + DRM_INFO(DRV_NAME "_panel %ux%u@%u %ubpp dsi %udl - ready",
> > +default_mode.hdisplay, default_mode.vdisplay,
> > +default_mode.vrefresh,
> > +mipi_dsi_pixel_format_to_bpp(dsi->format), dsi->lanes);
>
> You already have dev, so use DRM_DEV_INFO() and drop DRV_NAME
>
> > +
> > +static const struct of_device_id jh057n_of_match[] = {
> > + { .compatible = "rocktech,jh057n00900" },
> > + { /* Sentinel */ }
> Lower case 's' (sorry, likely my bad)
>
> > + .probe = jh057n_probe,
> > + .remove = jh057n_remove,
> > + .shutdown = jh057n_shutdown,
> > + .driver = {
> > + .name = DRV_NAME "_panel",
> Drop "_panel" postfix. Other drivers do not use it.
>
> Sam
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 3/3] drm/panel: Add Rocktech jh057n00900 panel driver
Support Rocktech jh057n00900 5.5" 720x1440 TFT LCD panel. It is a MIPI
DSI video mode panel.
The panel seems to use a Sitronix ST7703 look alike (most of the
commands look similar to the ST7703's data sheet but use a different
number of parameters). The initial version of the DSI init sequence
(including sleeps) were provided by the vendor. Sleeps were reduced
considerably though to speed up initialization.
Signed-off-by: Guido Günther
Reviewed-by: Sam Ravnborg
---
drivers/gpu/drm/panel/Kconfig | 13 +
drivers/gpu/drm/panel/Makefile| 1 +
.../drm/panel/panel-rocktech-jh057n00900.c| 386 ++
3 files changed, 400 insertions(+)
create mode 100644 drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c
diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index 3e070153ef21..42f541fe57ae 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -149,6 +149,19 @@ config DRM_PANEL_RAYDIUM_RM68200
Say Y here if you want to enable support for Raydium RM68200
720x1280 DSI video mode panel.
+config DRM_PANEL_ROCKTECH_JH057N00900
+ tristate "Rocktech JH057N00900 MIPI touchscreen panel"
+ depends on OF
+ depends on DRM_MIPI_DSI
+ depends on BACKLIGHT_CLASS_DEVICE
+ help
+ Say Y here if you want to enable support for Rocktech JH057N00900
+ MIPI DSI panel as e.g. used in the Librem 5 devkit. It has a
+ resolution of 720x1440 pixels, a built in backlight and touch
+ controller.
+ Touch input support is provided by the goodix driver and needs to be
+ selected separately.
+
config DRM_PANEL_SAMSUNG_S6D16D0
tristate "Samsung S6D16D0 DSI video mode panel"
depends on OF
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index e7ab71968bbf..902e871059d0 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -13,6 +13,7 @@ obj-$(CONFIG_DRM_PANEL_ORISETECH_OTM8009A) +=
panel-orisetech-otm8009a.o
obj-$(CONFIG_DRM_PANEL_PANASONIC_VVX10F034N00) +=
panel-panasonic-vvx10f034n00.o
obj-$(CONFIG_DRM_PANEL_RASPBERRYPI_TOUCHSCREEN) +=
panel-raspberrypi-touchscreen.o
obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM68200) += panel-raydium-rm68200.o
+obj-$(CONFIG_DRM_PANEL_ROCKTECH_JH057N00900) += panel-rocktech-jh057n00900.o
obj-$(CONFIG_DRM_PANEL_SAMSUNG_LD9040) += panel-samsung-ld9040.o
obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6D16D0) += panel-samsung-s6d16d0.o
obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E3HA2) += panel-samsung-s6e3ha2.o
diff --git a/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c
b/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c
new file mode 100644
index ..158a6d548068
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c
@@ -0,0 +1,386 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Rockteck jh057n00900 5.5" MIPI-DSI panel driver
+ *
+ * Copyright (C) Purism SPC 2019
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define DRV_NAME "panel-rocktech-jh057n00900"
+
+/* Manufacturer specific Commands send via DSI */
+#define ST7703_CMD_ALL_PIXEL_OFF 0x22
+#define ST7703_CMD_ALL_PIXEL_ON 0x23
+#define ST7703_CMD_SETDISP 0xB2
+#define ST7703_CMD_SETRGBIF 0xB3
+#define ST7703_CMD_SETCYC 0xB4
+#define ST7703_CMD_SETBGP 0xB5
+#define ST7703_CMD_SETVCOM 0xB6
+#define ST7703_CMD_SETOTP 0xB7
+#define ST7703_CMD_SETPOWER_EXT 0xB8
+#define ST7703_CMD_SETEXTC 0xB9
+#define ST7703_CMD_SETMIPI 0xBA
+#define ST7703_CMD_SETVDC 0xBC
+#define ST7703_CMD_SETSCR 0xC0
+#define ST7703_CMD_SETPOWER 0xC1
+#define ST7703_CMD_SETPANEL 0xCC
+#define ST7703_CMD_SETGAMMA 0xE0
+#define ST7703_CMD_SETEQ0xE3
+#define ST7703_CMD_SETGIP1 0xE9
+#define ST7703_CMD_SETGIP2 0xEA
+
+struct jh057n {
+ struct device *dev;
+ struct drm_panel panel;
+ struct gpio_desc *reset_gpio;
+ struct backlight_device *backlight;
+ bool prepared;
+
+ struct dentry *debugfs;
+};
+
+static inline struct jh057n *panel_to_jh057n(struct drm_panel *panel)
+{
+ return container_of(panel, struct jh057n, panel);
+}
+
+#define dsi_generic_write_seq(dsi, seq...) do {
\
+ static const u8 d[] = { seq }; \
+ int ret;\
+ ret = mipi_dsi_generic_write(dsi, d, ARRAY_SIZE(d));\
+ if (ret < 0)\
+ return ret; \
+ } while (0)
+
+static int jh057n_init_sequence(struct jh057n *ctx)
+{
+ struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
+ struct device *dev = ctx->dev;
+ int ret;
+
+ /*
+
[PATCH v3 1/3] dt-bindings: Add vendor prefix for ROCKTECH DISPLAYS LIMITED
Add ROCKTECH DISPLAYS LIMITED (https://rocktech.com.hk) LCD panel supplier. Signed-off-by: Guido Günther --- Documentation/devicetree/bindings/vendor-prefixes.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt index 413c6f30ce88..cc24619a4249 100644 --- a/Documentation/devicetree/bindings/vendor-prefixes.txt +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt @@ -338,6 +338,7 @@ ricoh Ricoh Co. Ltd. rikomagic Rikomagic Tech Corp. Ltd riscv RISC-V Foundation rockchip Fuzhou Rockchip Electronics Co., Ltd +rocktech ROCKTECH DISPLAYS LIMITED rohm ROHM Semiconductor Co., Ltd roofullShenzhen Roofull Technology Co, Ltd samsungSamsung Semiconductor -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 0/3] drm/panel: Support Rocktech jh057n00900 DSI panel
It's a 5.5" 720x1440 TFT LCD MIPI DSI panel with built in touchscreen and backlight as found in the Librem 5 devkit. These patches are against linux next as of 2019-02-08. Changes from v2 * As per review comments from Sam Ravnborg * Lowercase sentinel * Drop '_panel' postfix * DRM_DEV_ logging instead of plain DRM_ * Add Sam's Reviewed-by: * Add "panel-rocktech-" to the driver name following the pattern from other drm panel drivers. Changes from v1 * As per review comments from Sam Ravnborg * Make SPDX-License-Identifier match MODULE_LICENSE * Sort include files alphabetically * Drop drmP.h and use individual includes * Drop superfuous 'x' in mode printout on error path * Allpixelson_set: Add proper space around '*' * Drop superfluous put_device(>backlight->dev); * Add /* Sentinel */ in jh057n_of_match * Drop jh057n->enabled * Drop drm_display_info_set_bus_formats * Kconfig: Depend on BACKLIGHT_CLASS_DEVICE which somehow got lost * Move jh057n_enable close to jh057n_disable Guido Günther (3): dt-bindings: Add vendor prefix for ROCKTECH DISPLAYS LIMITED dt-bindings: Add Rocktech jh057n00900 panel bindings drm/panel: Add Rocktech jh057n00900 panel driver .../display/panel/rocktech,jh057n00900.txt| 18 + .../devicetree/bindings/vendor-prefixes.txt | 1 + drivers/gpu/drm/panel/Kconfig | 13 + drivers/gpu/drm/panel/Makefile| 1 + .../drm/panel/panel-rocktech-jh057n00900.c| 386 ++ 5 files changed, 419 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/panel/rocktech,jh057n00900.txt create mode 100644 drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 2/3] dt-bindings: Add Rocktech jh057n00900 panel bindings
The Rocktec jh057n00900 is a 5.5" MIPI DSI video mode panel with a
720x1440 resolution and a built in backlight.
Signed-off-by: Guido Günther
---
.../display/panel/rocktech,jh057n00900.txt | 18 ++
1 file changed, 18 insertions(+)
create mode 100644
Documentation/devicetree/bindings/display/panel/rocktech,jh057n00900.txt
diff --git
a/Documentation/devicetree/bindings/display/panel/rocktech,jh057n00900.txt
b/Documentation/devicetree/bindings/display/panel/rocktech,jh057n00900.txt
new file mode 100644
index ..32f4001d2d6f
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/rocktech,jh057n00900.txt
@@ -0,0 +1,18 @@
+Rocktech jh057n00900 5.5" 720x1440 TFT LCD panel
+
+Required properties:
+- compatible: should be "rocktech,jh057n00900"
+- reg: DSI virtual channel of the peripheral
+- reset-gpios: panel reset gpio
+- backlight: phandle of the backlight device attached to the panel
+
+Example:
+
+ _dsi {
+ panel {
+ compatible = "rocktech,jh057n00900";
+ reg = <0>;
+ backlight = <>;
+ reset-gpios = < 13 GPIO_ACTIVE_LOW>;
+ };
+ };
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] tinydrm/mipi-dbi: Use dma-safe buffers for all SPI transfers
Den 04.03.2019 16.10, skrev Andy Shevchenko:
> On Mon, Mar 04, 2019 at 03:45:56PM +0100, Noralf Trønnes wrote:
>>
>>
>> Den 22.02.2019 16.58, skrev Andy Shevchenko:
>>> On Fri, Feb 22, 2019 at 01:43:29PM +0100, Noralf Trønnes wrote:
Buffers passed to spi_sync() must be dma-safe even for tiny buffers since
some SPI controllers use DMA for all transfers.
Example splat with CONFIG_DMA_API_DEBUG enabled:
[ 23.750467] DMA-API: dw_dmac_pci :00:15.0: device driver maps
memory from stack [probable addr=1e49185d]
[ 23.750529] WARNING: CPU: 1 PID: 1296 at kernel/dma/debug.c:1161
check_for_stack+0xb7/0x190
[ 23.750533] Modules linked in: mmc_block(+) spi_pxa2xx_platform(+)
pwm_lpss_pci pwm_lpss spi_pxa2xx_pci sdhci_pci cqhci intel_mrfld_pwrbtn
extcon_intel_mrfld sdhci intel_mrfld_adc led_class mmc_core ili9341
mipi_dbi tinydrm backlight ti_ads7950 industrialio_triggered_buffer
kfifo_buf intel_soc_pmic_mrfld hci_uart btbcm
[ 23.750599] CPU: 1 PID: 1296 Comm: modprobe Not tainted 5.0.0-rc7+ #236
[ 23.750605] Hardware name: Intel Corporation Merrifield/BODEGA BAY,
BIOS 542 2015.01.21:18.19.48
[ 23.750620] RIP: 0010:check_for_stack+0xb7/0x190
[ 23.750630] Code: 8b 6d 50 4d 85 ed 75 04 4c 8b 6d 10 48 89 ef e8 2f 8b
44 00 48 89 c6 4a 8d 0c 23 4c 89 ea 48 c7 c7 88 d0 82 b4 e8 40 7c f9 ff
<0f> 0b 8b 05 79 00 4b 01 85 c0 74 07 5b 5d 41 5c 41 5d c3 8b 05 54
[ 23.750637] RSP: :97bbc0292fa0 EFLAGS: 00010286
[ 23.750646] RAX: RBX: 97bbc029 RCX:
0006
[ 23.750652] RDX: 0007 RSI: 0002 RDI:
94b33e115450
[ 23.750658] RBP: 94b33c8578b0 R08: 0002 R09:
000201c0
[ 23.750664] R10: 0006ecb0ccc6 R11: 00034f38 R12:
316c
[ 23.750670] R13: 94b33c84b250 R14: 94b33dedd5a0 R15:
0001
[ 23.750679] FS: () GS:94b33e10(0063)
knlGS:f7faf690
[ 23.750686] CS: 0010 DS: 002b ES: 002b CR0: 80050033
[ 23.750691] CR2: f7f54faf CR3: 0722c000 CR4:
001006e0
[ 23.750696] Call Trace:
[ 23.750713] debug_dma_map_sg+0x100/0x340
[ 23.750727] ? dma_direct_map_sg+0x3b/0xb0
[ 23.750739] spi_map_buf+0x25a/0x300
[ 23.750751] __spi_pump_messages+0x2a4/0x680
[ 23.750762] __spi_sync+0x1dd/0x1f0
[ 23.750773] spi_sync+0x26/0x40
[ 23.750790] mipi_dbi_typec3_command_read+0x14d/0x240 [mipi_dbi]
[ 23.750802] ? spi_finalize_current_transfer+0x10/0x10
[ 23.750821] mipi_dbi_typec3_command+0x1bc/0x1d0 [mipi_dbi]
>>>
>>> After few runs I don't see the warning anymore. So,
>>> Tested-by: Andy Shevchenko
>>>
>>
>> Can I have an ack as well if you're satisfied with it?
>
> Sure.
>
> Acked-by: Andy Shevchenko
>
> I hope to see this in v5.1-rc1.
The patch doesn't apply cleanly on (to be) 5.1 due to this one:
drm/tinydrm: Use struct drm_rect
https://cgit.freedesktop.org/drm/drm/commit/?id=b051b3459bbae907ef068bcd8b62f73f09ea5016
If it's just a debug warning but no ill effects, it'll show up in 5.2.
Otherwise I will have to backport it.
Noralf.
>>
>> Noralf.
>>
>>> Though I would like to give few more days to monitor the state.
>>> (I believe it's fixed)
>>>
Reported-by: Andy Shevchenko
Signed-off-by: Noralf Trønnes
---
drivers/gpu/drm/tinydrm/ili9225.c | 6 ++--
drivers/gpu/drm/tinydrm/mipi-dbi.c | 58 +-
include/drm/tinydrm/mipi-dbi.h | 5 +--
3 files changed, 48 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/tinydrm/ili9225.c
b/drivers/gpu/drm/tinydrm/ili9225.c
index 3f59cfbd31ba..a87078667e04 100644
--- a/drivers/gpu/drm/tinydrm/ili9225.c
+++ b/drivers/gpu/drm/tinydrm/ili9225.c
@@ -299,7 +299,7 @@ static void ili9225_pipe_disable(struct
drm_simple_display_pipe *pipe)
mipi->enabled = false;
}
-static int ili9225_dbi_command(struct mipi_dbi *mipi, u8 cmd, u8 *par,
+static int ili9225_dbi_command(struct mipi_dbi *mipi, u8 *cmd, u8 *par,
size_t num)
{
struct spi_device *spi = mipi->spi;
@@ -309,11 +309,11 @@ static int ili9225_dbi_command(struct mipi_dbi
*mipi, u8 cmd, u8 *par,
gpiod_set_value_cansleep(mipi->dc, 0);
speed_hz = mipi_dbi_spi_cmd_max_speed(spi, 1);
- ret = tinydrm_spi_transfer(spi, speed_hz, NULL, 8, , 1);
+ ret = tinydrm_spi_transfer(spi, speed_hz, NULL, 8, cmd, 1);
if (ret || !num)
return ret;
- if (cmd == ILI9225_WRITE_DATA_TO_GRAM && !mipi->swap_bytes)
+ if (*cmd == ILI9225_WRITE_DATA_TO_GRAM && !mipi->swap_bytes)
bpw = 16;
Re: [PATCH 4/6] dt-bindings: display: armada: Add display subsystem binding
On Sun, Feb 24, 2019 at 10:15 AM Lubomir Rintel wrote:
>
> On Fri, 2019-02-22 at 14:23 -0600, Rob Herring wrote:
> > On Wed, Feb 13, 2019 at 4:37 PM Lubomir Rintel wrote:
> > > On Mon, 2019-01-21 at 09:35 -0600, Rob Herring wrote:
> > > > On Sun, Jan 20, 2019 at 11:26 AM Lubomir Rintel wrote:
> > > > > The Marvell Armada DRM master device is a virtual device needed to
> > > > > list all
> > > > > nodes that comprise the graphics subsystem.
> > > > >
> > > > > Signed-off-by: Lubomir Rintel
> > > > > ---
> > > > > .../display/armada/marvell-armada-drm.txt | 24
> > > > > +++
> > > > > 1 file changed, 24 insertions(+)
> > > > >
> > > > > diff --git
> > > > > a/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt
> > > > >
> > > > > b/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt
> > > > > index de4cca9432c8..3dbfa8047f0b 100644
> > > > > ---
> > > > > a/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt
> > > > > +++
> > > > > b/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt
> > > > > @@ -1,3 +1,27 @@
> > > > > +Marvell Armada DRM master device
> > > > > +
> > > > > +
> > > > > +The Marvell Armada DRM master device is a virtual device needed to
> > > > > list all
> > > > > +nodes that comprise the graphics subsystem.
> > > > > +
> > > > > +Required properties:
> > > > > +
> > > > > + - compatible: value should be "marvell,dove-display-subsystem",
> > > > > + "marvell,armada-display-subsystem"
> > > > > + - ports: a list of phandles pointing to display interface ports of
> > > > > CRTC
> > > > > + devices
> > > > > + - memory-region: phandle to a node describing memory to be used for
> > > > > the
> > > > > + framebuffer
> > > > > +
> > > > > +Example:
> > > > > +
> > > > > + display-subsystem {
> > > > > + compatible = "marvell,dove-display-subsystem",
> > > > > +"marvell,armada-display-subsystem";
> > > > > + memory-region = <_reserved>;
> > > > > + ports = <_port>;
> > > >
> > > > If there is only one device, you don't need this virtual node.
> > >
> > > Before I follow up on this and submit a version without the virtual
> > > node, I'm wondering: is it okay that the bindings for the LCDC and the
> > > framebuffer are in the same file, or would it be preferrable if they
> > > were separate? Both styles seem to be used for the display bindings.
> >
> > framebuffer as in the kernel fbdev? Really, that should be the same
> > binding. It's the same h/w after all. However, there have been cases
> > where things deviated. So I don't have a good answer.
>
> No, not the fbdev device, that one is managed by drmfb and is not
> expressed in DT. I meant the reserved-memory node that sets aside
> memory for the framebuffers.
>
> See patch "[RFC 03/16] dt-bindings: display: armada: Add framebuffer
> reserved-mem binding". Perhaps that part should even go to
> Documentation/devicetree/bindings/reserved-memory/.
Okay.
A separate file will be better and probably
Documentation/devicetree/bindings/reserved-memory/ is the best
location.
Rob
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 109819] Shadow of Mordor causes gpu freeze ryzen 2200g
https://bugs.freedesktop.org/show_bug.cgi?id=109819 --- Comment #2 from Dominic --- Created attachment 143522 --> https://bugs.freedesktop.org/attachment.cgi?id=143522=edit New 5.0 Kernel Crashlog -- 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 109819] Shadow of Mordor causes gpu freeze ryzen 2200g
https://bugs.freedesktop.org/show_bug.cgi?id=109819 --- Comment #1 from Dominic --- Per Linux Kernel 5.0 release here an updated report with that newest kernel and updated head from git://anongit.freedesktop.org/mesa/drm With the Linux Kernel 5.0 the dmesg log if full of amdgpu spam, that seems to repeat itself all the time, independent of operation - not sure if it's related to the grub debug line. The the freeze though seems to still appear the same way but the error message in dmesg has changed and now just shows two lines: that occur at the freeze point: [ 2501.329358] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring gfx timeout, signaled seq=300047, emitted seq=300049 [ 2501.329419] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* Process information: process ShadowOfMordor pid 3150 thread ShadowOfMo:cs0 pid 3152 [ 2501.329421] [drm] GPU recovery disabled. Full dmesg log and glxinfo output in the new attached dumps_from_dmesg_and_glxinfo_2 -- 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 v3 2/2] drm/lima: driver for ARM Mali4xx GPUs
On Sat, Mar 2, 2019 at 12:23 PM Rob Clark wrote:
>
> On Fri, Mar 1, 2019 at 9:32 PM Qiang Yu wrote:
> >
> > On Thu, Feb 28, 2019 at 5:41 AM Rob Herring wrote:
> > >
> > > On Wed, Feb 27, 2019 at 7:42 AM Qiang Yu wrote:
> > > > diff --git a/drivers/gpu/drm/lima/lima_drv.c
> > > > b/drivers/gpu/drm/lima/lima_drv.c> > > new file mode 100644
> > > > index ..e93bce16ee10
> > > > --- /dev/null
> > > > +++ b/drivers/gpu/drm/lima/lima_drv.c
> > > > @@ -0,0 +1,353 @@
> > > > +// SPDX-License-Identifier: GPL-2.0 OR MIT
> > > > +/* Copyright 2017-2018 Qiang Yu */
> > > > +
> > > > +#include
> > > > +#include
> > > > +#include
> > > > +#include
> > > > +#include
> > > > +
> > > > +#include "lima_drv.h"
> > > > +#include "lima_gem.h"
> > > > +#include "lima_gem_prime.h"
> > > > +#include "lima_vm.h"
> > > > +
> > > > +int lima_sched_timeout_ms = 0;
> > > > +int lima_sched_max_tasks = 32;
> > > > +
> > > > +MODULE_PARM_DESC(sched_timeout_ms, "task run timeout in ms (0 = no
> > > > timeout (default))");
> > > > +module_param_named(sched_timeout_ms, lima_sched_timeout_ms, int, 0444);
> > > > +
> > > > +MODULE_PARM_DESC(sched_max_tasks, "max queued task num in a context
> > > > (default 32)");
> > > > +module_param_named(sched_max_tasks, lima_sched_max_tasks, int, 0444);
> > > > +
> > > > +static int lima_ioctl_info(struct drm_device *dev, void *data, struct
> > > > drm_file *file)
> > > > +{
> > >
> > > For panfrost, we generalized this to "get param" like other drivers.
> > > Looks like you can only add 7 more items.
> > >
> > > What about GPU revisions?
> >
> > Currently I don't know there's any programming difference between GPUs
> > with different revision. Would be appreciate if anyone can tell me before
> > some hard reverse engineering effort.
What does the vendor kernel driver have? I haven't checked utgard, but
there's no shortage of quirks in the midgard/bifrost driver. I'd
imagine utgard to be similar.
> Probably a safe bet there are some revisions that need userspace
> workarounds.. and given that kernel to userspace uabi is something we
> end up having to live with for a long time, better to expose more
> information to userspace just in case.
Right.
More importantly than the 1 example I gave, design the ABI to be
extendable beyond 7 more u32 values. It is quite easy to support 2^32
params.
Rob
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/3] staging/vboxvideo: Drop initial_mode_queried workaround
Drop the initial_mode_queried workaround for kms clients which do not
support hotplug, all kms clients should be able to deal with hotplug.
Suggested-by: Daniel Vetter
Signed-off-by: Hans de Goede
---
drivers/staging/vboxvideo/TODO| 3 ---
drivers/staging/vboxvideo/vbox_drv.c | 25 -
drivers/staging/vboxvideo/vbox_drv.h | 6 --
drivers/staging/vboxvideo/vbox_main.c | 6 +++---
drivers/staging/vboxvideo/vbox_mode.c | 21 ++---
5 files changed, 5 insertions(+), 56 deletions(-)
diff --git a/drivers/staging/vboxvideo/TODO b/drivers/staging/vboxvideo/TODO
index 7f97c47a4042..2e0f99c3f10c 100644
--- a/drivers/staging/vboxvideo/TODO
+++ b/drivers/staging/vboxvideo/TODO
@@ -1,8 +1,5 @@
TODO:
-Get a full review from the drm-maintainers on dri-devel done on this driver
--Drop all the logic around initial_mode_queried, the master_set and
- master_drop callbacks and everything related to this. kms clients can handle
- hotplugs.
-Extend this TODO with the results of that review
Please send any patches to Greg Kroah-Hartman ,
diff --git a/drivers/staging/vboxvideo/vbox_drv.c
b/drivers/staging/vboxvideo/vbox_drv.c
index e7755a179850..fb6a0f0b8167 100644
--- a/drivers/staging/vboxvideo/vbox_drv.c
+++ b/drivers/staging/vboxvideo/vbox_drv.c
@@ -200,36 +200,11 @@ static const struct file_operations vbox_fops = {
.read = drm_read,
};
-static int vbox_master_set(struct drm_device *dev,
- struct drm_file *file_priv, bool from_open)
-{
- struct vbox_private *vbox = dev->dev_private;
-
- /*
-* We do not yet know whether the new owner can handle hotplug, so we
-* do not advertise dynamic modes on the first query and send a
-* tentative hotplug notification after that to see if they query again.
-*/
- vbox->initial_mode_queried = false;
-
- return 0;
-}
-
-static void vbox_master_drop(struct drm_device *dev, struct drm_file
*file_priv)
-{
- struct vbox_private *vbox = dev->dev_private;
-
- /* See vbox_master_set() */
- vbox->initial_mode_queried = false;
-}
-
static struct drm_driver driver = {
.driver_features =
DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME | DRIVER_ATOMIC,
.lastclose = drm_fb_helper_lastclose,
- .master_set = vbox_master_set,
- .master_drop = vbox_master_drop,
.fops = _fops,
.irq_handler = vbox_irq_handler,
diff --git a/drivers/staging/vboxvideo/vbox_drv.h
b/drivers/staging/vboxvideo/vbox_drv.h
index 77f2a4e9000e..6dea8bf5f045 100644
--- a/drivers/staging/vboxvideo/vbox_drv.h
+++ b/drivers/staging/vboxvideo/vbox_drv.h
@@ -83,12 +83,6 @@ struct vbox_private {
} ttm;
struct mutex hw_mutex; /* protects modeset and accel/vbva accesses */
- /*
-* We decide whether or not user-space supports display hot-plug
-* depending on whether they react to a hot-plug event after the initial
-* mode query.
-*/
- bool initial_mode_queried;
struct work_struct hotplug_work;
u32 input_mapping_width;
u32 input_mapping_height;
diff --git a/drivers/staging/vboxvideo/vbox_main.c
b/drivers/staging/vboxvideo/vbox_main.c
index e1fb70a42d32..f4d02de5518a 100644
--- a/drivers/staging/vboxvideo/vbox_main.c
+++ b/drivers/staging/vboxvideo/vbox_main.c
@@ -32,9 +32,9 @@ void vbox_report_caps(struct vbox_private *vbox)
u32 caps = VBVACAPS_DISABLE_CURSOR_INTEGRATION |
VBVACAPS_IRQ | VBVACAPS_USE_VBVA_ONLY;
- if (vbox->initial_mode_queried)
- caps |= VBVACAPS_VIDEO_MODE_HINTS;
-
+ /* The host only accepts VIDEO_MODE_HINTS if it is send separately. */
+ hgsmi_send_caps_info(vbox->guest_pool, caps);
+ caps |= VBVACAPS_VIDEO_MODE_HINTS;
hgsmi_send_caps_info(vbox->guest_pool, caps);
}
diff --git a/drivers/staging/vboxvideo/vbox_mode.c
b/drivers/staging/vboxvideo/vbox_mode.c
index 213551394495..620a6e38f71f 100644
--- a/drivers/staging/vboxvideo/vbox_mode.c
+++ b/drivers/staging/vboxvideo/vbox_mode.c
@@ -736,29 +736,12 @@ static int vbox_get_modes(struct drm_connector *connector)
vbox_connector = to_vbox_connector(connector);
vbox = connector->dev->dev_private;
- /*
-* Heuristic: we do not want to tell the host that we support dynamic
-* resizing unless we feel confident that the user space client using
-* the video driver can handle hot-plug events. So the first time modes
-* are queried after a "master" switch we tell the host that we do not,
-* and immediately after we send the client a hot-plug notification as
-* a test to see if they will respond and query again.
-* That is also the reason why capabilities are reported to the host at
-* this place in the code rather than elsewhere.
-* We need to report the flags location before reporting the IRQ
-*
[PATCH 0/3] drm/vboxvideo: Move the vboxvideo driver out of staging
Hi All, This patch-series addresses the 2 TODO / FIXME items recently reported by Daniel and after that moves the vboxvideo driver out of staging. Note this applies on top of drm-misc-next + this patch: https://patchwork.kernel.org/patch/10824279/ Currently that patch is not yet in drm-misc, I can push it myself before pushing the rest of this series (after review). Greg, the intent is for this series to be merged upstream through drm-misc, may we have your Acked-by for that please? Regards, Hans ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/3] drm/vboxvideo: Move the vboxvideo driver out of staging
The vboxvideo driver has been converted to the atomic modesetting API
and all FIXME and TODO items have been fixed, so it is time to move it out
of staging.
Signed-off-by: Hans de Goede
---
drivers/gpu/drm/Kconfig | 2 ++
drivers/gpu/drm/Makefile | 1 +
drivers/{staging => gpu/drm}/vboxvideo/Kconfig | 0
drivers/{staging => gpu/drm}/vboxvideo/Makefile | 0
drivers/{staging => gpu/drm}/vboxvideo/hgsmi_base.c | 0
drivers/{staging => gpu/drm}/vboxvideo/hgsmi_ch_setup.h | 0
drivers/{staging => gpu/drm}/vboxvideo/hgsmi_channels.h | 0
drivers/{staging => gpu/drm}/vboxvideo/hgsmi_defs.h | 0
drivers/{staging => gpu/drm}/vboxvideo/modesetting.c | 0
drivers/{staging => gpu/drm}/vboxvideo/vbox_drv.c| 0
drivers/{staging => gpu/drm}/vboxvideo/vbox_drv.h| 0
drivers/{staging => gpu/drm}/vboxvideo/vbox_fb.c | 0
drivers/{staging => gpu/drm}/vboxvideo/vbox_hgsmi.c | 0
drivers/{staging => gpu/drm}/vboxvideo/vbox_irq.c| 0
drivers/{staging => gpu/drm}/vboxvideo/vbox_main.c | 0
drivers/{staging => gpu/drm}/vboxvideo/vbox_mode.c | 0
drivers/{staging => gpu/drm}/vboxvideo/vbox_prime.c | 0
drivers/{staging => gpu/drm}/vboxvideo/vbox_ttm.c| 0
drivers/{staging => gpu/drm}/vboxvideo/vboxvideo.h | 0
drivers/{staging => gpu/drm}/vboxvideo/vboxvideo_guest.h | 0
drivers/{staging => gpu/drm}/vboxvideo/vboxvideo_vbe.h | 0
drivers/{staging => gpu/drm}/vboxvideo/vbva_base.c | 0
drivers/staging/Kconfig | 2 --
drivers/staging/Makefile | 1 -
drivers/staging/vboxvideo/TODO | 7 ---
25 files changed, 3 insertions(+), 10 deletions(-)
rename drivers/{staging => gpu/drm}/vboxvideo/Kconfig (100%)
rename drivers/{staging => gpu/drm}/vboxvideo/Makefile (100%)
rename drivers/{staging => gpu/drm}/vboxvideo/hgsmi_base.c (100%)
rename drivers/{staging => gpu/drm}/vboxvideo/hgsmi_ch_setup.h (100%)
rename drivers/{staging => gpu/drm}/vboxvideo/hgsmi_channels.h (100%)
rename drivers/{staging => gpu/drm}/vboxvideo/hgsmi_defs.h (100%)
rename drivers/{staging => gpu/drm}/vboxvideo/modesetting.c (100%)
rename drivers/{staging => gpu/drm}/vboxvideo/vbox_drv.c (100%)
rename drivers/{staging => gpu/drm}/vboxvideo/vbox_drv.h (100%)
rename drivers/{staging => gpu/drm}/vboxvideo/vbox_fb.c (100%)
rename drivers/{staging => gpu/drm}/vboxvideo/vbox_hgsmi.c (100%)
rename drivers/{staging => gpu/drm}/vboxvideo/vbox_irq.c (100%)
rename drivers/{staging => gpu/drm}/vboxvideo/vbox_main.c (100%)
rename drivers/{staging => gpu/drm}/vboxvideo/vbox_mode.c (100%)
rename drivers/{staging => gpu/drm}/vboxvideo/vbox_prime.c (100%)
rename drivers/{staging => gpu/drm}/vboxvideo/vbox_ttm.c (100%)
rename drivers/{staging => gpu/drm}/vboxvideo/vboxvideo.h (100%)
rename drivers/{staging => gpu/drm}/vboxvideo/vboxvideo_guest.h (100%)
rename drivers/{staging => gpu/drm}/vboxvideo/vboxvideo_vbe.h (100%)
rename drivers/{staging => gpu/drm}/vboxvideo/vbva_base.c (100%)
delete mode 100644 drivers/staging/vboxvideo/TODO
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 4385f00e1d05..2aa26dd23271 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -333,6 +333,8 @@ source "drivers/gpu/drm/tve200/Kconfig"
source "drivers/gpu/drm/xen/Kconfig"
+source "drivers/gpu/drm/vboxvideo/Kconfig"
+
# Keep legacy drivers last
menuconfig DRM_LEGACY
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index ce8d1d384319..4d3e101c9f9d 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -109,3 +109,4 @@ obj-$(CONFIG_DRM_TINYDRM) += tinydrm/
obj-$(CONFIG_DRM_PL111) += pl111/
obj-$(CONFIG_DRM_TVE200) += tve200/
obj-$(CONFIG_DRM_XEN) += xen/
+obj-$(CONFIG_DRM_VBOXVIDEO) += vboxvideo/
diff --git a/drivers/staging/vboxvideo/Kconfig
b/drivers/gpu/drm/vboxvideo/Kconfig
similarity index 100%
rename from drivers/staging/vboxvideo/Kconfig
rename to drivers/gpu/drm/vboxvideo/Kconfig
diff --git a/drivers/staging/vboxvideo/Makefile
b/drivers/gpu/drm/vboxvideo/Makefile
similarity index 100%
rename from drivers/staging/vboxvideo/Makefile
rename to drivers/gpu/drm/vboxvideo/Makefile
diff --git a/drivers/staging/vboxvideo/hgsmi_base.c
b/drivers/gpu/drm/vboxvideo/hgsmi_base.c
similarity index 100%
rename from drivers/staging/vboxvideo/hgsmi_base.c
rename to drivers/gpu/drm/vboxvideo/hgsmi_base.c
diff --git a/drivers/staging/vboxvideo/hgsmi_ch_setup.h
b/drivers/gpu/drm/vboxvideo/hgsmi_ch_setup.h
similarity index 100%
rename from drivers/staging/vboxvideo/hgsmi_ch_setup.h
rename to drivers/gpu/drm/vboxvideo/hgsmi_ch_setup.h
diff --git a/drivers/staging/vboxvideo/hgsmi_channels.h
b/drivers/gpu/drm/vboxvideo/hgsmi_channels.h
similarity index 100%
rename from drivers/staging/vboxvideo/hgsmi_channels.h
rename to
[PATCH 2/3] staging/vboxvideo: Refactor vbox_update_mode_hints
Refactor vbox_update_mode_hints to no longer use the obsolete
drm_modeset_lock_all() and switch it over to drm_connector_list_iter
instead of directly accessing the list using list_for_each_entry.
Signed-off-by: Hans de Goede
---
drivers/staging/vboxvideo/vbox_irq.c | 15 +++
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/vboxvideo/vbox_irq.c
b/drivers/staging/vboxvideo/vbox_irq.c
index 89944134ea86..16a1e29f5292 100644
--- a/drivers/staging/vboxvideo/vbox_irq.c
+++ b/drivers/staging/vboxvideo/vbox_irq.c
@@ -105,6 +105,7 @@ static void validate_or_set_position_hints(struct
vbox_private *vbox)
/* Query the host for the most recent video mode hints. */
static void vbox_update_mode_hints(struct vbox_private *vbox)
{
+ struct drm_connector_list_iter conn_iter;
struct drm_device *dev = >ddev;
struct drm_connector *connector;
struct vbox_connector *vbox_conn;
@@ -122,13 +123,10 @@ static void vbox_update_mode_hints(struct vbox_private
*vbox)
}
validate_or_set_position_hints(vbox);
- drm_modeset_lock_all(dev);
- /*
-* FIXME: this needs to use drm_connector_list_iter and some real
-* locking for the actual data it changes, not the deprecated
-* drm_modeset_lock_all() shotgun approach.
-*/
- list_for_each_entry(connector, >mode_config.connector_list, head) {
+
+ drm_modeset_lock(>mode_config.connection_mutex, NULL);
+ drm_connector_list_iter_begin(dev, _iter);
+ drm_for_each_connector_iter(connector, _iter) {
vbox_conn = to_vbox_connector(connector);
hints = >last_mode_hints[vbox_conn->vbox_crtc->crtc_id];
@@ -157,7 +155,8 @@ static void vbox_update_mode_hints(struct vbox_private
*vbox)
vbox_conn->vbox_crtc->disconnected = disconnected;
}
- drm_modeset_unlock_all(dev);
+ drm_connector_list_iter_end(_iter);
+ drm_modeset_unlock(>mode_config.connection_mutex);
}
static void vbox_hotplug_worker(struct work_struct *work)
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: fix spelling mistake "intead" -> "instead"
On Sun, Feb 17, 2019 at 10:55:54PM +, Colin King wrote:
> From: Colin Ian King
>
> There is a spelling mistake in a DRM_NOTE message. Fix this.
>
> Signed-off-by: Colin Ian King
Applied to drm-misc-next, thanks.
Sean
> ---
> drivers/gpu/drm/drm_kms_helper_common.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_kms_helper_common.c
> b/drivers/gpu/drm/drm_kms_helper_common.c
> index 93e2b30fe1a5..9c5ae825c507 100644
> --- a/drivers/gpu/drm/drm_kms_helper_common.c
> +++ b/drivers/gpu/drm/drm_kms_helper_common.c
> @@ -39,7 +39,7 @@ MODULE_LICENSE("GPL and additional rights");
> /* Backward compatibility for drm_kms_helper.edid_firmware */
> static int edid_firmware_set(const char *val, const struct kernel_param *kp)
> {
> - DRM_NOTE("drm_kms_firmware.edid_firmware is deprecated, please use
> drm.edid_firmware intead.\n");
> + DRM_NOTE("drm_kms_firmware.edid_firmware is deprecated, please use
> drm.edid_firmware instead.\n");
>
> return __drm_set_edid_firmware_path(val);
> }
> --
> 2.20.1
>
--
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 0/5] drm/bridge: sii902x: HDMI-audio support and some fixes
Hi Jyri, On Mon, Mar 04, 2019 at 04:29:17PM +0200, Jyri Sarha wrote: > On 04/03/2019 14:42, Laurent Pinchart wrote: > > On Wed, Feb 27, 2019 at 11:54:18PM +0200, Jyri Sarha wrote: > >> Changes since first version: > >> - Moved reviewed patches to front: > >> - drm/bridge: sii902x: add input_bus_flags > >> - drm/bridge: sii902x: Set output mode to HDMI or DVI according to EDID > >> - drm/bridge: sii902x: pixel clock unit is 10kHz instead of 1kHz > >> - Added a new fix: > >> - drm/bridge: sii902x: Select I2C_MUX > >> - Applied some review suggestions to > >> - drm/bridge: sii902x: Implement HDMI audio support > >> - use clock-names property to name mclk > >> - move comment describing added mutex to struct sii902x and improve it > >> - cleanup sii902x_mute() > >> - cleanup sii902x_select_mclk_div() > >> - fix condition for checking ENABLE_BIT from i2s_fifo_routing in > >> sii902x_audio_codec_init() > >> > >> Still to do > >> > >> - Agree on i2s wires to HDMI audio fifo routing in dts. > >> > >> The current scheme is quite straight forward, but there is maybe > >> there is even more straight forward solutions like: > >> > >> audio-fifo-enable = <1 1 1 1>; > >> audio-i2s-pin-to-fifo = <0 1 2 3>; > >> > >> Meaning that all fifos are enabled and SD0 is routed to fifo 0, SD1 > >> to fifo 1, etc. I am not sure if the channel swap functionality > >> should show in dts binding. > > > > Please forgive my lack of audio knowledge, but it this a system > > description that should be encoded in DT, or a policy that should be > > handled purely in software (either fully inside the kernel or with the > > help of userspace) ? > > This property describes how many i2s wires are connected to sii902x and > in what order, so I think it belongs to DTS. That would belong to DT, yes. We have solved a similar problem with CSI-2 receivers, where the number of data lanes and their mapping can often be configured. See the definition of the data-lanes property in Documentation/devicetree/bindings/media/video-interfaces.txt for more information. How about using similar bindings ? It would also solve Andrzej's concerns that you don't describe the audio connection in DT. > One might of course wonder why anybody would put the i2s wires to any > other order than 0 <-> 0, 1 <-> 1, 2 <-> 2, and 3 <-> 3, but then a > again I've seen weirder board designs. It can be useful to simplify signal routing on the board. -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: add __user attribute to ptr_to_compat()
On Fri, Mar 01, 2019 at 12:00:46PM +, Ben Dooks wrote:
> The ptr_to_compat() call takes a "void __user *", so cast
> the compat drm calls that use it to avoid the following
> warnings from sparse:
>
> drivers/gpu/drm/drm_ioc32.c:188:39: warning: incorrect type in argument 1
> (different address spaces)
> drivers/gpu/drm/drm_ioc32.c:188:39:expected void [noderef] *uptr
> drivers/gpu/drm/drm_ioc32.c:188:39:got void *[addressable] [assigned]
> handle
> drivers/gpu/drm/drm_ioc32.c:529:41: warning: incorrect type in argument 1
> (different address spaces)
> drivers/gpu/drm/drm_ioc32.c:529:41:expected void [noderef] *uptr
> drivers/gpu/drm/drm_ioc32.c:529:41:got void *[addressable] [assigned]
> handle
>
> Signed-off-by: Ben Dooks
Thanks for your patch, I've applied it to drm-misc-next-fixes with cc: stable.
Sean
> ---
> drivers/gpu/drm/drm_ioc32.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_ioc32.c b/drivers/gpu/drm/drm_ioc32.c
> index 67b1fca39aa6..0e3043e08c69 100644
> --- a/drivers/gpu/drm/drm_ioc32.c
> +++ b/drivers/gpu/drm/drm_ioc32.c
> @@ -185,7 +185,7 @@ static int compat_drm_getmap(struct file *file, unsigned
> int cmd,
> m32.size = map.size;
> m32.type = map.type;
> m32.flags = map.flags;
> - m32.handle = ptr_to_compat(map.handle);
> + m32.handle = ptr_to_compat((void __user *)map.handle);
> m32.mtrr = map.mtrr;
> if (copy_to_user(argp, , sizeof(m32)))
> return -EFAULT;
> @@ -216,7 +216,7 @@ static int compat_drm_addmap(struct file *file, unsigned
> int cmd,
>
> m32.offset = map.offset;
> m32.mtrr = map.mtrr;
> - m32.handle = ptr_to_compat(map.handle);
> + m32.handle = ptr_to_compat((void __user *)map.handle);
> if (map.handle != compat_ptr(m32.handle))
> pr_err_ratelimited("compat_drm_addmap truncated handle %p for
> type %d offset %x\n",
> map.handle, m32.type, m32.offset);
> @@ -526,7 +526,7 @@ static int compat_drm_getsareactx(struct file *file,
> unsigned int cmd,
> if (err)
> return err;
>
> - req32.handle = ptr_to_compat(req.handle);
> + req32.handle = ptr_to_compat((void __user *)req.handle);
> if (copy_to_user(argp, , sizeof(req32)))
> return -EFAULT;
>
> --
> 2.20.1
>
--
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 4/5] drm/bridge: sii902x: Select I2C_MUX
Hi Jyri,
Thank you for the patch.
On Wed, Feb 27, 2019 at 11:54:22PM +0200, Jyri Sarha wrote:
> "drm/bridge/sii902x: Fix EDID readback"-commit added a dependency to
> I2C_MUX, but not indicate it in the Kconfig entry. Fix it by selecting
> I2C_MUX for DRM_SII902X config option.
>
> Fixes: 88664675239 ("drm/bridge/sii902x: Fix EDID readback")
> Signed-off-by: Jyri Sarha
> ---
> drivers/gpu/drm/bridge/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> index fb0b37918382..a6f6ff8f06b3 100644
> --- a/drivers/gpu/drm/bridge/Kconfig
> +++ b/drivers/gpu/drm/bridge/Kconfig
> @@ -95,6 +95,7 @@ config DRM_SII902X
> depends on OF
> select DRM_KMS_HELPER
> select REGMAP_I2C
> + select I2C_MUX
> ---help---
> Silicon Image sii902x bridge chip driver.
>
This is already present in v5.0.
commit ea6b13e9fed0fda9532ee04d38ed1bef1edbfdbf
Author: Fabrizio Castro
Date: Mon Nov 19 13:26:18 2018 +
drm/bridge/sii902x: Add missing dependency on I2C_MUX
Fabrizio stated in the commit message that "Quite obviously the driver
depends on I2C_MUX, but adding a "depends on" introduces a recursive
dependency, therefore this patch selects I2C_MUX instead.". Given that
I2C_MUX is a tristate user-selectable option, "depend on" should be the
right solution. I wonder if we could fix the root cause of the recursive
dependency.
--
Regards,
Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 3/5] drm/bridge: sii902x: pixel clock unit is 10kHz instead of 1kHz
Hi Jyri, Thank you for the patch. On Wed, Feb 27, 2019 at 11:54:21PM +0200, Jyri Sarha wrote: > The pixel clock unit in the first two registers (0x00 and 0x01) of > sii9022 is 10kHz, not 1kHz as in struct drm_display_mode. Division by > 10 fixes the issue. > > Signed-off-by: Jyri Sarha > Reviewed-by: Andrzej Hajda > --- > drivers/gpu/drm/bridge/sii902x.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/sii902x.c > b/drivers/gpu/drm/bridge/sii902x.c > index 0e21fa419d27..1e91ed72 100644 > --- a/drivers/gpu/drm/bridge/sii902x.c > +++ b/drivers/gpu/drm/bridge/sii902x.c > @@ -248,10 +248,11 @@ static void sii902x_bridge_mode_set(struct drm_bridge > *bridge, > struct regmap *regmap = sii902x->regmap; > u8 buf[HDMI_INFOFRAME_SIZE(AVI)]; > struct hdmi_avi_infoframe frame; > + u16 pixel_clock_10kHz = adj->clock / 10; > int ret; > > - buf[0] = adj->clock; > - buf[1] = adj->clock >> 8; > + buf[0] = pixel_clock_10kHz & 0xFF; Nitpicking, we usually use lowercase hex values. Apart from that, Reviewed-by: Laurent Pinchart > + buf[1] = pixel_clock_10kHz >> 8; > buf[2] = adj->vrefresh; > buf[3] = 0x00; > buf[4] = adj->hdisplay; -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/7] drm/edid: Allow to ignore the audio EDID data
On Mon, Mar 04, 2019 at 03:52:35PM +0100, Maxime Ripard wrote: > In some cases, in order to accomodate with displays with poor EDIDs, we > need to ignore that the monitor alledgedly supports audio output and > disable the audio output. > > Signed-off-by: Maxime Ripard > --- > drivers/gpu/drm/drm_edid.c | 8 > 1 file changed, 8 insertions(+) > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > index 990b1909f9d7..c0258b011bb2 100644 > --- a/drivers/gpu/drm/drm_edid.c > +++ b/drivers/gpu/drm/drm_edid.c > @@ -4190,6 +4190,11 @@ bool drm_detect_hdmi_monitor(struct edid *edid) > } > EXPORT_SYMBOL(drm_detect_hdmi_monitor); > > +static bool ignore_edid_audio = false; > +module_param(ignore_edid_audio, bool, 0644); > +MODULE_PARM_DESC(ignore_edid_audio, > + "Ignore the EDID and always consider that a monitor doesn't > have audio capabilities"); > + I would suggest that this is not the best apporach. Years of experience from i915 says that more modparams means random forums full of people trading cargo culted settings. And as soon as the average user comes across the magic incantation that works they are likely to not file the appropriate bug report. Also years later we still see people using modparams that stopped working five hardware generations ago. So at least for i915 new modparams are generally frowned upon. Bad EDIDs at least should be quirked. Which means we really need the bug reports, and hence a modparam can be somewhat counter productive. For allowing the user to force the DVI vs. HDMI and audio vs. not i915 does have the "audio" connector property. Other drivers could adopt the same thing. Though I'm not sure even i915 should be exposing this for the reasons already mentioned. There is one hardware generation where it can actually be useful on i915 as the hardware is only capably of sending infoframes/audio to a single HDMI port at a time. So with this property the user can at least select which display gets to do those things. I do agree that there is an unfortnate problem with fbcon vs. initial property values. I've sometimes pondered about exposing kms properties in a generic fashion via sysfs and/or kernel cmdline somehow. IIRC devicetree/something similar has also been proposed occasionally to solve this problem. > /** > * drm_detect_monitor_audio - check monitor audio capability > * @edid: EDID block to scan > @@ -4209,6 +4214,9 @@ bool drm_detect_monitor_audio(struct edid *edid) > bool has_audio = false; > int start_offset, end_offset; > > + if (ignore_edid_audio) > + goto end; > + > edid_ext = drm_find_cea_extension(edid); > if (!edid_ext) > goto end; > -- > git-series 0.9.1 > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Ville Syrjälä Intel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/7] drm/vc4: Allow for more boot-time configuration
Hi Peter, On Mon, Mar 04, 2019 at 03:21:35PM +, Peter Stuge wrote: > Hi, > > Maxime Ripard wrote: > > properties to initialise the overscan or rotation parameters, or the > > one to deal with broken displays. > > How does that work on systems with multiple connectors? > > On SBCs with only one output I guess it's fine to have a global option, > but it may be important for new options to also be usable on more > complex systems. And maybe even SPI displays are relevant? These properties are per-connector, so only the display attached to that connector will be affected, and you can have multiple connectors with various properties and values defined if you want to > Also, some of the forward-ported patches still have your old email > address, maybe you want to change that. Good catch, thanks! 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
Re: [PATCH 1/7] drm/vc4: hdmi: Check that the monitor supports HDMI audio
Hi Maxime, Am 04.03.2019 um 15:52 schrieb Maxime Ripard: The current code assumes as soon as the device is an HDMI one that it supports an audio sink. However, strictly speaking, this is exposed as a separate part of EDID. This can be checked through the drm_detect_monitor_audio function, so let's use it and make sure that we can use the HDMI monitor as an output before sending sound. does the audio output work in the following setup after applying this patch? VC4 --- HDMI Audio extractor --- Non audio capable monitor Signed-off-by: Maxime Ripard ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v9 1/5] drm/sun4i: sun6i_mipi_dsi: Fix hsync_porch overflow
On Sun, Mar 03, 2019 at 11:05:23PM +0530, Jagan Teki wrote:
> Loop N1 instruction delay for burst mode devices are computed
> based on horizontal sync and porch timing values.
>
> The current driver is using u16 type for computing this hsync_porch
> value, which would failed to fit within the u16 type for large sync
> and porch timings devices. This would result in hsync_porch overflow
> and eventually computed wrong instruction delay value.
>
> Example, timings, where it produces the overflow
> {
> .hdisplay = 1080,
> .hsync_start= 1080 + 408,
> .hsync_end = 1080 + 408 + 4,
> .htotal = 1080 + 408 + 4 + 38,
> }
>
> It reproduces the desired delay value 65487 but the correct working
> value should be 7.
>
> So, Fix it by computing hsync_porch value separately with u32 type.
>
> Fixes: 1c1a7aa3663c ("drm/sun4i: dsi: Add burst support")
> Signed-off-by: Jagan Teki
> Tested-by: Merlijn Wajer
> ---
> drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> index 6ff585055a07..465e7fc57899 100644
> --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> @@ -457,8 +457,9 @@ static void sun6i_dsi_setup_inst_loop(struct sun6i_dsi
> *dsi,
> u16 delay = 50 - 1;
>
> if (device->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) {
> - delay = (mode->htotal - mode->hdisplay) * 150;
> - delay /= (mode->clock / 1000) * 8;
> + u32 hsync_porch = (mode->htotal - mode->hdisplay);
> +
> + delay = ((hsync_porch * 150) / ((mode->clock / 1000) * 8));
shouldn't we keep the multiplication by 150 in the u32 assignation?
Otherwise, we could keep a u16 for the delay
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
Re: [PATCH 3/5] drm/amd: fix fb references in async update
On 3/4/19 9:49 AM, Helen Koike wrote: > Async update callbacks are expected to set the old_fb in the new_state > so prepare/cleanup framebuffers are balanced. > > Calling drm_atomic_set_fb_for_plane() (which gets a reference of the new > fb and put the old fb) is not required, as it's taken care by > drm_mode_cursor_universal() when calling drm_atomic_helper_update_plane(). > > Suggested-by: Boris Brezillon > Signed-off-by: Helen Koike Reviewed-by: Nicholas Kazlauskas I guess the swap itself should be enough here as per the commit description. It would have been nice if this patch dropped the old_plane_state->fb != new_plane_state->fb check too at the same time, but I suppose I can drop that later. It'll help us pass those failing IGT tests as well. Nicholas Kazlauskas > > --- > Hello, > > As mentioned in the cover letter, > I tested on the rockchip and on i915 using igt plane_cursor_legacy and > kms_cursor_legacy and I didn't see any regressions. > But I couldn't test on AMD because I don't have the hardware and I would > appreciate if anyone could test it. > > Also, I didn't CC to stable here as I saw the async_update function was only > added on v4.20, please let me know if I should CC to stable. > > Thanks! > Helen > > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > index 3a6f595f295e..bc02800254bf 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > @@ -3760,8 +3760,7 @@ static void dm_plane_atomic_async_update(struct > drm_plane *plane, > struct drm_plane_state *old_state = > drm_atomic_get_old_plane_state(new_state->state, plane); > > - if (plane->state->fb != new_state->fb) > - drm_atomic_set_fb_for_plane(plane->state, new_state->fb); > + swap(plane->state->fb, new_state->fb); > > plane->state->src_x = new_state->src_x; > plane->state->src_y = new_state->src_y; > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/7] drm/edid: Allow to ignore the audio EDID data
On Mon, 2019-03-04 at 17:47 +0200, Jani Nikula wrote: > On Mon, 04 Mar 2019, Maxime Ripard wrote: > > In some cases, in order to accomodate with displays with poor EDIDs, we > > need to ignore that the monitor alledgedly supports audio output and > > disable the audio output. > > *sad trombone* > > Trying to figure this out automatically in kernel is better than a > quirk. > > A quirk is better than requiring the user to provide an override EDID > via the firmware loader (drm.edid_firmware parameter). > > Requiring an override EDID is better than adding a module parameter. Also, this and 3/ apply to _every_ monitor attached to the system. No. - ajax ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v9 5/5] drm/sun4i: sun6i_mipi_dsi: Simplify dsi setup timings code
On Sun, Mar 03, 2019 at 11:05:27PM +0530, Jagan Teki wrote:
> DSI timings are varies between burst/non-burst devices and
> current driver is handling this support via if, else statements
> which would difficult to read.
>
> Simplify it by using goto to make code more readable.
>
> Signed-off-by: Jagan Teki
> Tested-by: Merlijn Wajer
> ---
> Note: This change is created based on previous version burst
> changes [1] by addressing comment from [2] by Maxime to make
> code readable.
>
> [1] https://patchwork.kernel.org/cover/10813623/
> [2] https://patchwork.kernel.org/patch/1007/
>
> drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 79 ++
> 1 file changed, 42 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> index 31ec4048804d..898f32319c2d 100644
> --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> @@ -550,7 +550,8 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi,
> {
> struct mipi_dsi_device *device = dsi->device;
> unsigned int Bpp = mipi_dsi_pixel_format_to_bpp(device->format) / 8;
> - u16 hbp = 0, hfp = 0, hsa = 0, hblk = 0, vblk = 0;
> + u16 hbp, hfp, hsa, hblk;
> + u16 vblk = 0;
> u32 basic_ctl = 0;
> size_t bytes;
> u8 *buffer;
> @@ -558,6 +559,7 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi,
> /* Do all timing calculations up front to allocate buffer space */
>
> if (device->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) {
> + hbp = hfp = hsa = 0;
This raises a checkpatch warning and isn't necessary
> hblk = mode->hdisplay * Bpp;
> basic_ctl = SUN6I_DSI_BASIC_CTL_VIDEO_BURST |
> SUN6I_DSI_BASIC_CTL_HSA_HSE_DIS |
> @@ -566,48 +568,51 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi
> *dsi,
> if (device->lanes == 4)
> basic_ctl |= SUN6I_DSI_BASIC_CTL_TRAIL_FILL |
>SUN6I_DSI_BASIC_CTL_TRAIL_INV(0xc);
> - } else {
> - /*
> - * A sync period is composed of a blanking packet (4
> - * bytes + payload + 2 bytes) and a sync event packet
> - * (4 bytes). Its minimal size is therefore 10 bytes
> - */
> +
> + goto alloc_buf;
And I'd rather not have a goto in the middle of the code for no
particular reason.
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
Re: [PATCH 0/7] drm/vc4: Allow for more boot-time configuration
Hi Maxime, Am 04.03.2019 um 15:52 schrieb Maxime Ripard: Hi, The proprietary stack for the RaspberryPi allows for a number of video parameters widely used by their users, but yet don't have any equivalents in the mainline kernel. Those options are detailed here: https://www.raspberrypi.org/documentation/configuration/config-txt/video.md While not all of them are desirable to have in the mainline kernel, some of them still have value, such as properties to initialise the overscan or rotation parameters, or the one to deal with broken displays. This series is an attempt to support those, and is based on a rewrite of the command line parser I did a couple of years ago that never reached upstream (due to a lack of time on my side). While this parser was initially made to deal with named modes (in order to support TV modes), it also allowed to extend it more easily, which is why it's resurrected. Since a change of the command line parser can pretty easily get things wrong and introduce regressions, I also worked with a number of unit tests that you can find here: http://code.bulix.org/tpo7dg-607264?raw Eventually, I guess those tests should be part of the kernel somewhere, but I haven't found a suitable place for them to be included yet. Let me know what you think, Maxime thanks for this series. In case you want some feedback from the Foundation, please add Phil Elwell instead of Eben to CC. Stefan ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v9 3/5] drm/sun4i: sun6i_mipi_dsi: Support vblk timing for 4-lane devices
On Sun, Mar 03, 2019 at 11:05:25PM +0530, Jagan Teki wrote:
> Like other dsi setup timings, or hblk for that matter vblk would
> also require compute the timings based payload equation along with
> packet overhead.
>
> But, on the other hand vblk computation is also depends on device
> lane number.
> - for 4 lane devices, it is computed based on vtotal, packet overhead
> along with hblk value.
> - for others devices, it is simply 0
>
> BSP code from BPI-M64-bsp is computing vblk as for 4-lane devices
> (from linux-sunxi
> drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/de_dsi.c)
>
> tmp = (ht*dsi_pixel_bits[format]/8)*vt-(4+dsi_hblk+2);
> dsi_vblk = (lane-tmp%lane);
>
> So, update the vblk timing calculation to support all type of
> devices.
>
> Tested on 2-lane, 4-lane MIPI-DSI LCD panels.
You should be explaining which issue you faced, in which setup, what
were its symptoms and how that solution is fixing it.
> Signed-off-by: Jagan Teki
> Tested-by: Merlijn Wajer
> ---
> drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 27 +++---
> 1 file changed, 20 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> index 140e55f5ed2e..b38358465d87 100644
> --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> @@ -527,6 +527,24 @@ static void sun6i_dsi_setup_format(struct sun6i_dsi *dsi,
>SUN6I_DSI_PIXEL_CTL0_FORMAT(fmt));
> }
>
> +static u16 sun6i_dsi_get_timings_vblk(struct sun6i_dsi *dsi,
> + struct drm_display_mode *mode, u16 hblk)
> +{
> + struct mipi_dsi_device *device = dsi->device;
> + unsigned int Bpp = mipi_dsi_pixel_format_to_bpp(device->format) / 8;
> + int tmp;
> +
> + /*
> + * The vertical blank is set using a blanking packet (4 bytes +
> + * payload + 2 bytes). Its minimal size is therefore 6 bytes
> + */
> +#define VBLK_PACKET_OVERHEAD 6
> + tmp = (mode->htotal * Bpp) * mode->vtotal -
> + (hblk + VBLK_PACKET_OVERHEAD);
> +
> + return (device->lanes - tmp % device->lanes);
This should have a comment explaining why it's needed
> +}
> +
> static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi,
> struct drm_display_mode *mode)
> {
> @@ -586,13 +604,8 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi
> *dsi,
> (mode->htotal - (mode->hsync_end -
> mode->hsync_start)) * Bpp -
> HBLK_PACKET_OVERHEAD);
>
> - /*
> - * And I'm not entirely sure what vblk is about. The driver in
> - * Allwinner BSP is using a rather convoluted calculation
> - * there only for 4 lanes. However, using 0 (the !4 lanes
> - * case) even with a 4 lanes screen seems to work...
> - */
> - vblk = 0;
> + if (device->lanes == 4)
And that can be done in the function itself.
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
Re: [PATCH 1/5] drm: don't block fb changes for async plane updates
On 3/4/19 9:49 AM, Helen Koike wrote:
> In the case of a normal sync update, the preparation of framebuffers (be
> it calling drm_atomic_helper_prepare_planes() or doing setups with
> drm_framebuffer_get()) are performed in the new_state and the respective
> cleanups are performed in the old_state.
>
> In the case of async updates, the preparation is also done in the
> new_state but the cleanups are done in the new_state (because updates
> are performed in place, i.e. in the current state).
>
> The current code blocks async udpates when the fb is changed, turning
> async updates into sync updates, slowing down cursor updates and
> introducing regressions in igt tests with errors of type:
>
> "CRITICAL: completed 97 cursor updated in a period of 30 flips, we
> expect to complete approximately 15360 updates, with the threshold set
> at 7680"
>
> Fb changes in async updates were prevented to avoid the following scenario:
>
> - Async update, oldfb = NULL, newfb = fb1, prepare fb1, cleanup fb1
> - Async update, oldfb = fb1, newfb = fb2, prepare fb2, cleanup fb2
> - Non-async commit, oldfb = fb2, newfb = fb1, prepare fb1, cleanup fb2 (wrong)
> Where we have a single call to prepare fb2 but double cleanup call to fb2.
>
> To solve the above problems, instead of blocking async fb changes, we
> place the old framebuffer in the new_state object, so when the code
> performs cleanups in the new_state it will cleanup the old_fb and we
> will have the following scenario instead:
>
> - Async update, oldfb = NULL, newfb = fb1, prepare fb1, no cleanup
> - Async update, oldfb = fb1, newfb = fb2, prepare fb2, cleanup fb1
> - Non-async commit, oldfb = fb2, newfb = fb1, prepare fb1, cleanup fb2
>
> Where calls to prepare/cleanup are ballanced.
>
> Cc: # v4.14+: 25dc194b34dd: drm: Block fb changes
> for async plane updates
> Fixes: 25dc194b34dd ("drm: Block fb changes for async plane updates")
> Suggested-by: Boris Brezillon
> Signed-off-by: Helen Koike
>
> ---
> Hello,
>
> As mentioned in the cover letter,
> I tested on the rockchip and on i915 (with a patch I am still working on for
> replacing cursors by async update), with igt plane_cursor_legacy and
> kms_cursor_legacy and I didn't see any regressions.
> I couldn't test on MSM and AMD because I don't have the hardware (and I am
> having some issues testing on vc4) and I would appreciate if anyone could help
> me testing those.
>
> I also think it would be a better solution if, instead of having async
> to do in-place updates in the current state, the async path should be
> equivalent to a syncronous update, i.e., modifying new_state and
> performing a flip
> IMHO, the only difference between sync and async should be that async update
> doesn't wait for vblank and applies the changes immeditally to the hw,
> but the code path could be almost the same.
> But for now I think this solution is ok (swaping new_fb/old_fb), and
> then we can adjust things little by little, what do you think?
>
> Thanks!
> Helen
>
> drivers/gpu/drm/drm_atomic_helper.c | 20 ++--
> 1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c
> index 540a77a2ade9..e7eb96f1efc2 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1608,15 +1608,6 @@ int drm_atomic_helper_async_check(struct drm_device
> *dev,
> old_plane_state->crtc != new_plane_state->crtc)
> return -EINVAL;
>
> - /*
> - * FIXME: Since prepare_fb and cleanup_fb are always called on
> - * the new_plane_state for async updates we need to block framebuffer
> - * changes. This prevents use of a fb that's been cleaned up and
> - * double cleanups from occuring.
> - */
> - if (old_plane_state->fb != new_plane_state->fb)
> - return -EINVAL;
> -
> funcs = plane->helper_private;
> if (!funcs->atomic_async_update)
> return -EINVAL;
> @@ -1657,6 +1648,9 @@ void drm_atomic_helper_async_commit(struct drm_device
> *dev,
> int i;
>
> for_each_new_plane_in_state(state, plane, plane_state, i) {
> + struct drm_framebuffer *new_fb = plane_state->fb;
> + struct drm_framebuffer *old_fb = plane->state->fb;
> +
> funcs = plane->helper_private;
> funcs->atomic_async_update(plane, plane_state);
>
> @@ -1665,11 +1659,17 @@ void drm_atomic_helper_async_commit(struct drm_device
> *dev,
>* plane->state in-place, make sure at least common
>* properties have been properly updated.
>*/
> - WARN_ON_ONCE(plane->state->fb != plane_state->fb);
> + WARN_ON_ONCE(plane->state->fb != new_fb);
> WARN_ON_ONCE(plane->state->crtc_x != plane_state->crtc_x);
> WARN_ON_ONCE(plane->state->crtc_y != plane_state->crtc_y);
>
Re: [PATCH 3/7] drm/edid: Allow to ignore the HDMI monitor mode
On Mon, 04 Mar 2019, Maxime Ripard wrote: > Signed-off-by: Maxime Ripard > --- > drivers/gpu/drm/drm_edid.c | 8 > 1 file changed, 8 insertions(+) > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > index c0258b011bb2..2f6df10ed9f1 100644 > --- a/drivers/gpu/drm/drm_edid.c > +++ b/drivers/gpu/drm/drm_edid.c > @@ -4156,6 +4156,11 @@ int drm_av_sync_delay(struct drm_connector *connector, > } > EXPORT_SYMBOL(drm_av_sync_delay); > > +static bool force_dvi_monitor = false; > +module_param(force_dvi_monitor, bool, 0644); > +MODULE_PARM_DESC(force_dvi_monitor, > + "Ignore the EDID and always consider the monitor as DVI > instead of HDMI"); > + Same reply as with patch 2/7. BR, Jani. > /** > * drm_detect_hdmi_monitor - detect whether monitor is HDMI > * @edid: monitor EDID information > @@ -4170,6 +4175,9 @@ bool drm_detect_hdmi_monitor(struct edid *edid) > int i; > int start_offset, end_offset; > > + if (force_dvi_monitor) > + return false; > + > edid_ext = drm_find_cea_extension(edid); > if (!edid_ext) > return false; -- Jani Nikula, Intel Open Source Graphics Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/7] drm/edid: Allow to ignore the audio EDID data
On Mon, 04 Mar 2019, Maxime Ripard wrote: > In some cases, in order to accomodate with displays with poor EDIDs, we > need to ignore that the monitor alledgedly supports audio output and > disable the audio output. *sad trombone* Trying to figure this out automatically in kernel is better than a quirk. A quirk is better than requiring the user to provide an override EDID via the firmware loader (drm.edid_firmware parameter). Requiring an override EDID is better than adding a module parameter. I'd much rather we exhausted the other options before adding module parameters to address specific issues with EDIDs. That's a rabbit hole with no end. BR, Jani. > > Signed-off-by: Maxime Ripard > --- > drivers/gpu/drm/drm_edid.c | 8 > 1 file changed, 8 insertions(+) > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > index 990b1909f9d7..c0258b011bb2 100644 > --- a/drivers/gpu/drm/drm_edid.c > +++ b/drivers/gpu/drm/drm_edid.c > @@ -4190,6 +4190,11 @@ bool drm_detect_hdmi_monitor(struct edid *edid) > } > EXPORT_SYMBOL(drm_detect_hdmi_monitor); > > +static bool ignore_edid_audio = false; > +module_param(ignore_edid_audio, bool, 0644); > +MODULE_PARM_DESC(ignore_edid_audio, > + "Ignore the EDID and always consider that a monitor doesn't > have audio capabilities"); > + > /** > * drm_detect_monitor_audio - check monitor audio capability > * @edid: EDID block to scan > @@ -4209,6 +4214,9 @@ bool drm_detect_monitor_audio(struct edid *edid) > bool has_audio = false; > int start_offset, end_offset; > > + if (ignore_edid_audio) > + goto end; > + > edid_ext = drm_find_cea_extension(edid); > if (!edid_ext) > goto end; -- Jani Nikula, Intel Open Source Graphics Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v9 2/5] drm/sun4i: sun6i_mipi_dsi: Fix TCON DRQ set bits
On Sun, Mar 03, 2019 at 11:05:24PM +0530, Jagan Teki wrote: > TCON DRQ for non-burst DSI mode can computed based on horizontal > front porch value, but the current driver trying to include sync > timings along with front porch resulting wrong drq. > > This patch is trying to update the drq by subtracting hsync_start > with hdisplay, which is horizontal front porch. > > Current code: > > mode->hsync_end - mode->hdisplay => horizontal front porch + sync > > With this patch: > > mode->hsync_start - mode->hdisplay => horizontal front porch > > BSP code form BPI-M64-bsp is computing TCON DRQ set bits > for non-burts as (from linux-sunxi/ > drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/de_dsi.c) > > => panel->lcd_ht -panel->lcd_x - panel->lcd_hbp > => (timmings->hor_front_porch + panel->lcd_hbp + panel->lcd_x) ^ + sync length + >- panel->lcd_x - panel->hbp > => timmings->hor_front_porch ^ + sync > => mode->hsync_start - mode->hdisplay s/hsync_start/hsync_end/ Did you encounter any panel where this was fixing something? If so, which one, and what is the matching timings and / or datasheet? 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
[Bug 109808] ROCm OpenCL segfaults on drm-next-5.1-wip
https://bugs.freedesktop.org/show_bug.cgi?id=109808 --- Comment #4 from Philip Yang --- I will change the error message for this specific case to mention the missing kernel config option. I cannot add select ZONE_DEVICE in driver Kconfig file because there will be a circular dependency issue. The old or wired kernel config file may select to don't enable HMM or ZONE_DEVICE. Thanks, Philip -- 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 0/7] drm/vc4: Allow for more boot-time configuration
Hi, Maxime Ripard wrote: > properties to initialise the overscan or rotation parameters, or the > one to deal with broken displays. How does that work on systems with multiple connectors? On SBCs with only one output I guess it's fine to have a global option, but it may be important for new options to also be usable on more complex systems. And maybe even SPI displays are relevant? Also, some of the forward-ported patches still have your old email address, maybe you want to change that. Thanks //Peter ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/7] drm/edid: Allow to ignore the HDMI monitor mode
Hi, On Mon, 2019-03-04 at 15:52 +0100, Maxime Ripard wrote: > Signed-off-by: Maxime Ripard Reviewed-by: Paul Kocialkowski Cheers, Paul > --- > drivers/gpu/drm/drm_edid.c | 8 > 1 file changed, 8 insertions(+) > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > index c0258b011bb2..2f6df10ed9f1 100644 > --- a/drivers/gpu/drm/drm_edid.c > +++ b/drivers/gpu/drm/drm_edid.c > @@ -4156,6 +4156,11 @@ int drm_av_sync_delay(struct drm_connector *connector, > } > EXPORT_SYMBOL(drm_av_sync_delay); > > +static bool force_dvi_monitor = false; > +module_param(force_dvi_monitor, bool, 0644); > +MODULE_PARM_DESC(force_dvi_monitor, > + "Ignore the EDID and always consider the monitor as DVI > instead of HDMI"); > + > /** > * drm_detect_hdmi_monitor - detect whether monitor is HDMI > * @edid: monitor EDID information > @@ -4170,6 +4175,9 @@ bool drm_detect_hdmi_monitor(struct edid *edid) > int i; > int start_offset, end_offset; > > + if (force_dvi_monitor) > + return false; > + > edid_ext = drm_find_cea_extension(edid); > if (!edid_ext) > return false; -- Paul Kocialkowski, 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 1/7] drm/vc4: hdmi: Check that the monitor supports HDMI audio
Hi,
On Mon, 2019-03-04 at 15:52 +0100, Maxime Ripard wrote:
> The current code assumes as soon as the device is an HDMI one that it
> supports an audio sink. However, strictly speaking, this is exposed as a
> separate part of EDID.
>
> This can be checked through the drm_detect_monitor_audio function, so let's
> use it and make sure that we can use the HDMI monitor as an output before
> sending sound.
>
> Signed-off-by: Maxime Ripard
Reviewed-by: Paul Kocialkowski
Cheers,
Paul
> ---
> drivers/gpu/drm/vc4/vc4_hdmi.c | 6 ++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> index 88fd5df7e7dc..a1bdc065c47c 100644
> --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> @@ -109,6 +109,7 @@ struct vc4_hdmi_encoder {
> struct vc4_encoder base;
> bool hdmi_monitor;
> bool limited_rgb_range;
> + bool monitor_has_audio;
> };
>
> static inline struct vc4_hdmi_encoder *
> @@ -278,6 +279,7 @@ static int vc4_hdmi_connector_get_modes(struct
> drm_connector *connector)
> return -ENODEV;
>
> vc4_encoder->hdmi_monitor = drm_detect_hdmi_monitor(edid);
> + vc4_encoder->monitor_has_audio = drm_detect_monitor_audio(edid);
>
> drm_connector_update_edid_property(connector, edid);
> ret = drm_add_edid_modes(connector, edid);
> @@ -785,9 +787,13 @@ static int vc4_hdmi_audio_startup(struct
> snd_pcm_substream *substream,
> {
> struct vc4_hdmi *hdmi = dai_to_hdmi(dai);
> struct drm_encoder *encoder = hdmi->encoder;
> + struct vc4_hdmi_encoder *vc4_encoder = to_vc4_hdmi_encoder(encoder);
> struct vc4_dev *vc4 = to_vc4_dev(encoder->dev);
> int ret;
>
> + if (!vc4_encoder->monitor_has_audio)
> + return -ENODEV;
> +
> if (hdmi->audio.substream && hdmi->audio.substream != substream)
> return -EINVAL;
>
--
Paul Kocialkowski, 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: [EARLY RFC][PATCH] dma-buf: Add dma-buf heaps framework
On 3/1/19 6:06 AM, Brian Starkey wrote: > Hi Andrew, > > Sorry for not managing to comment on this sooner, I've had a crazy few > days. > > As the others have said, I quite like the direction here. > > On Mon, Feb 25, 2019 at 08:36:04AM -0600, Andrew F. Davis wrote: >> This framework allows a unified userspace interface for dma-buf >> exporters, allowing userland to allocate specific types of memory >> for use in dma-buf sharing. >> >> Each heap is given its own device node, which a user can allocate >> a dma-buf fd from using the DMA_HEAP_IOC_ALLOC. >> >> Signed-off-by: Andrew F. Davis >> --- >> >> Hello all, >> >> I had a little less time over the weekend than I thought I would to >> clean this up more and finish the first set of user heaps, but wanted >> to get this out anyway. >> >> ION in its current form assumes a lot about the memory it exports and >> these assumptions lead to restrictions on the full range of operations >> dma-buf's can produce. Due to this if we are to add this as an extension >> of the core dma-buf then it should only be the user-space advertisement >> and allocation front-end. All dma-buf exporting and operation need to be >> placed in the control of the exporting heap. The core becomes rather >> small, just a few hundred lines you see here. This is not to say we >> should not provide helpers to make the heap exporters more simple, but >> they should only be helpers and not enforced by the core framework. >> >> Basic use model here is an exporter (dedicated heap memory driver, CMA, >> System, etc.) registers with the framework by providing a struct >> dma_heap_info which gives us the needed info to export this heap to >> userspace as a device node. Next a user will request an allocation, >> the IOCTL is parsed and the request made to a heap provided alloc() op. >> The heap should return the filled out struct dma_heap_buffer, including >> exporting the buffer as a dma-buf. This dma-buf we then return to the >> user as a fd. Buffer free is still a bit open as we need to update some >> stats and free some memory, but the release operation is controlled by >> the heap exporter, so some hook will have to be created. >> >> It all needs a bit more work, and I'm sure I'll find missing parts when >> I add some more heaps, but hopefully this framework is simple enough that >> it does not impede the implementation of all functionality once provided >> by ION (shrinker, delayed free), nor any new functionality needed for >> future heap exporting devices. >> >> Thanks, >> Andrew >> >> drivers/dma-buf/Kconfig | 12 ++ >> drivers/dma-buf/Makefile | 1 + >> drivers/dma-buf/dma-heap.c| 268 ++ >> include/linux/dma-heap.h | 57 >> include/uapi/linux/dma-heap.h | 54 +++ >> 5 files changed, 392 insertions(+) >> create mode 100644 drivers/dma-buf/dma-heap.c >> create mode 100644 include/linux/dma-heap.h >> create mode 100644 include/uapi/linux/dma-heap.h >> >> diff --git a/drivers/dma-buf/Kconfig b/drivers/dma-buf/Kconfig >> index 2e5a0faa2cb1..30b0d7c83945 100644 >> --- a/drivers/dma-buf/Kconfig >> +++ b/drivers/dma-buf/Kconfig >> @@ -39,4 +39,16 @@ config UDMABUF >>A driver to let userspace turn memfd regions into dma-bufs. >>Qemu can use this to create host dmabufs for guest framebuffers. >> >> +menuconfig DMABUF_HEAPS >> +bool "DMA-BUF Userland Memory Heaps" >> +depends on HAS_DMA && MMU >> +select GENERIC_ALLOCATOR >> +select DMA_SHARED_BUFFER >> +help >> + Choose this option to enable the DMA-BUF userland memory heaps, >> + this allows userspace to allocate dma-bufs that can be shared between >> + drivers. >> + >> +source "drivers/dma-buf/heaps/Kconfig" >> + >> endmenu >> diff --git a/drivers/dma-buf/Makefile b/drivers/dma-buf/Makefile >> index 0913a6ccab5a..b0332f143413 100644 >> --- a/drivers/dma-buf/Makefile >> +++ b/drivers/dma-buf/Makefile >> @@ -1,4 +1,5 @@ >> obj-y := dma-buf.o dma-fence.o dma-fence-array.o reservation.o seqno-fence.o >> +obj-$(CONFIG_DMABUF_HEAPS) += dma-heap.o >> obj-$(CONFIG_SYNC_FILE) += sync_file.o >> obj-$(CONFIG_SW_SYNC) += sw_sync.o sync_debug.o >> obj-$(CONFIG_UDMABUF) += udmabuf.o >> diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c >> new file mode 100644 >> index ..72ed225fa892 >> --- /dev/null >> +++ b/drivers/dma-buf/dma-heap.c >> @@ -0,0 +1,268 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Framework for userspace DMA-BUF allocations >> + * >> + * Copyright (C) 2011 Google, Inc. >> + * Copyright (C) 2019 Linaro Ltd. >> + */ >> + >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +#include >> +#include >> + >> +#define DEVNAME "dma_heap" >> + >> +#define NUM_HEAP_MINORS 128 >> +static DEFINE_IDR(dma_heap_idr); >> +static DEFINE_MUTEX(minor_lock); /* Protect
[PATCH 7/7] drm/modes: Parse overscan properties
Properly configuring the overscan properties might be needed for the
initial setup of the framebuffer for display that still have overscan.
Let's allow for more properties on the kernel command line to setup each
margin.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/drm_fb_helper.c | 47 ++-
drivers/gpu/drm/drm_modes.c | 44 -
include/drm/drm_connector.h | 1 +-
3 files changed, 92 insertions(+)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 1457a1d1a423..3569016111a4 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -2569,6 +2569,51 @@ static void drm_setup_crtc_rotation(struct drm_fb_helper
*fb_helper,
fb_helper->sw_rotations |= DRM_MODE_ROTATE_0;
}
+static void drm_setup_connector_margins(struct drm_connector *connector)
+{
+ struct drm_cmdline_mode *cmdline = >cmdline_mode;
+ struct drm_modeset_acquire_ctx ctx;
+ struct drm_atomic_state *state;
+ struct drm_device *dev = connector->dev;
+ int ret;
+
+ if (!drm_drv_uses_atomic_modeset(dev))
+ return;
+
+ drm_modeset_acquire_init(, 0);
+
+ state = drm_atomic_state_alloc(dev);
+ state->acquire_ctx =
+
+retry:
+ drm_atomic_set_property(state, >base,
+ dev->mode_config.tv_left_margin_property,
+ cmdline->overscan_left);
+
+ drm_atomic_set_property(state, >base,
+ dev->mode_config.tv_right_margin_property,
+ cmdline->overscan_right);
+
+ drm_atomic_set_property(state, >base,
+ dev->mode_config.tv_top_margin_property,
+ cmdline->overscan_top);
+
+ drm_atomic_set_property(state, >base,
+ dev->mode_config.tv_bottom_margin_property,
+ cmdline->overscan_bottom);
+
+ ret = drm_atomic_commit(state);
+ if (ret == -EDEADLK) {
+ drm_atomic_state_clear(state);
+ drm_modeset_backoff();
+ goto retry;
+ }
+
+ drm_atomic_state_put(state);
+ drm_modeset_drop_locks();
+ drm_modeset_acquire_fini();
+}
+
static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
u32 width, u32 height)
{
@@ -2682,6 +2727,8 @@ static void drm_setup_crtcs_fb(struct drm_fb_helper
*fb_helper)
struct drm_connector *connector =
fb_helper->connector_info[i]->connector;
+ drm_setup_connector_margins(connector);
+
/* use first connected connector for the physical dimensions */
if (connector->status == connector_status_connected) {
info->var.width = connector->display_info.width_mm;
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 10c7e9322f76..6613db04cf77 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1586,6 +1586,50 @@ static int drm_mode_parse_cmdline_options(char *str,
size_t len,
} else if (!strncmp(option, "reflect_y", delim - option)) {
rotation |= DRM_MODE_REFLECT_Y;
sep = delim;
+ } else if (!strncmp(option, "overscan_right", delim - option)) {
+ const char *value = delim + 1;
+ unsigned int margin;
+
+ margin = simple_strtol(value, , 10);
+
+ /* Make sure we have parsed something */
+ if (sep == value)
+ return -EINVAL;
+
+ mode->overscan_right = margin;
+ } else if (!strncmp(option, "overscan_left", delim - option)) {
+ const char *value = delim + 1;
+ unsigned int margin;
+
+ margin = simple_strtol(value, , 10);
+
+ /* Make sure we have parsed something */
+ if (sep == value)
+ return -EINVAL;
+
+ mode->overscan_left = margin;
+ } else if (!strncmp(option, "overscan_top", delim - option)) {
+ const char *value = delim + 1;
+ unsigned int margin;
+
+ margin = simple_strtol(value, , 10);
+
+ /* Make sure we have parsed something */
+ if (sep == value)
+ return -EINVAL;
+
+ mode->overscan_top = margin;
+ } else if (!strncmp(option, "overscan_bottom", delim - option))
{
+ const char *value = delim + 1;
+ unsigned int margin;
+
+ margin =
[PATCH 6/7] drm/modes: Allow to specify rotation and reflection on the commandline
Rotations and reflections setup are needed in some scenarios to initialise
properly the initial framebuffer. Some drivers already had a bunch of
quirks to deal with this, such as either a private kernel command line
parameter (omapdss) or on the device tree (various panels).
In order to accomodate this, let's create a video mode parameter to deal
with the rotation and reflexion.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/drm_fb_helper.c | 4 +-
drivers/gpu/drm/drm_modes.c | 110 +++--
include/drm/drm_connector.h | 1 +-
3 files changed, 95 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index cacc4b56344e..1457a1d1a423 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -2523,6 +2523,7 @@ static void drm_setup_crtc_rotation(struct drm_fb_helper
*fb_helper,
struct drm_connector *connector)
{
struct drm_plane *plane = fb_crtc->mode_set.crtc->primary;
+ struct drm_cmdline_mode *mode = >cmdline_mode;
uint64_t valid_mask = 0;
int i, rotation;
@@ -2542,6 +2543,9 @@ static void drm_setup_crtc_rotation(struct drm_fb_helper
*fb_helper,
rotation = DRM_MODE_ROTATE_0;
}
+ if (mode->rotation != DRM_MODE_ROTATE_0)
+ fb_crtc->rotation = mode->rotation;
+
/*
* TODO: support 90 / 270 degree hardware rotation,
* depending on the hardware this may require the framebuffer
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 3d843d17370a..10c7e9322f76 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1531,6 +1531,71 @@ static int drm_mode_parse_cmdline_res_mode(const char
*str, unsigned int length,
return 0;
}
+static int drm_mode_parse_cmdline_options(char *str, size_t len,
+ struct drm_connector *connector,
+ struct drm_cmdline_mode *mode)
+{
+ unsigned int rotation = 0;
+ char *sep = str;
+
+ while ((sep = strchr(sep, ','))) {
+ char *delim, *option;
+
+ option = sep + 1;
+ delim = strchr(option, '=');
+ if (!delim) {
+ delim = strchr(option, ',');
+
+ if (!delim)
+ delim = str + len;
+ }
+
+ if (!strncmp(option, "rotate", delim - option)) {
+ const char *value = delim + 1;
+ unsigned int deg;
+
+ deg = simple_strtol(value, , 10);
+
+ /* Make sure we have parsed something */
+ if (sep == value)
+ return -EINVAL;
+
+ switch (deg) {
+ case 0:
+ rotation |= DRM_MODE_ROTATE_0;
+ break;
+
+ case 90:
+ rotation |= DRM_MODE_ROTATE_90;
+ break;
+
+ case 180:
+ rotation |= DRM_MODE_ROTATE_180;
+ break;
+
+ case 270:
+ rotation |= DRM_MODE_ROTATE_270;
+ break;
+
+ default:
+ return -EINVAL;
+ }
+ } else if (!strncmp(option, "reflect_x", delim - option)) {
+ rotation |= DRM_MODE_REFLECT_X;
+ sep = delim;
+ } else if (!strncmp(option, "reflect_y", delim - option)) {
+ rotation |= DRM_MODE_REFLECT_Y;
+ sep = delim;
+ } else {
+ return -EINVAL;
+ }
+ }
+
+ mode->rotation = rotation;
+
+ return 0;
+}
+
/**
* drm_mode_parse_command_line_for_connector - parse command line modeline for
connector
* @mode_option: optional per connector mode option
@@ -1558,9 +1623,10 @@ bool drm_mode_parse_command_line_for_connector(const
char *mode_option,
{
const char *name;
bool named_mode = false, parse_extras = false;
- unsigned int bpp_off = 0, refresh_off = 0;
+ unsigned int bpp_off = 0, refresh_off = 0, options_off = 0;
unsigned int mode_end = 0;
char *bpp_ptr = NULL, *refresh_ptr = NULL, *extra_ptr = NULL;
+ char *options_ptr = NULL;
char *bpp_end_ptr = NULL, *refresh_end_ptr = NULL;
int ret;
@@ -1601,13 +1667,18 @@ bool drm_mode_parse_command_line_for_connector(const
char *mode_option,
mode->refresh_specified = true;
}
+ /* Locate the start of named options */
+ options_ptr = strchr(name, ',');
+
[PATCH 3/7] drm/edid: Allow to ignore the HDMI monitor mode
Signed-off-by: Maxime Ripard --- drivers/gpu/drm/drm_edid.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index c0258b011bb2..2f6df10ed9f1 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -4156,6 +4156,11 @@ int drm_av_sync_delay(struct drm_connector *connector, } EXPORT_SYMBOL(drm_av_sync_delay); +static bool force_dvi_monitor = false; +module_param(force_dvi_monitor, bool, 0644); +MODULE_PARM_DESC(force_dvi_monitor, +"Ignore the EDID and always consider the monitor as DVI instead of HDMI"); + /** * drm_detect_hdmi_monitor - detect whether monitor is HDMI * @edid: monitor EDID information @@ -4170,6 +4175,9 @@ bool drm_detect_hdmi_monitor(struct edid *edid) int i; int start_offset, end_offset; + if (force_dvi_monitor) + return false; + edid_ext = drm_find_cea_extension(edid); if (!edid_ext) return false; -- git-series 0.9.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 5/7] drm/modes: Support modes names on the command line
From: Maxime Ripard
The drm subsystem also uses the video= kernel parameter, and in the
documentation refers to the fbdev documentation for that parameter.
However, that documentation also says that instead of giving the mode using
its resolution we can also give a name. However, DRM doesn't handle that
case at the moment. Even though in most case it shouldn't make any
difference, it might be useful for analog modes, where different standards
might have the same resolution, but still have a few different parameters
that are not encoded in the modes (NTSC vs NTSC-J vs PAL-M for example).
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/drm_connector.c | 3 +-
drivers/gpu/drm/drm_fb_helper.c | 4 +++-
drivers/gpu/drm/drm_modes.c | 49 +++---
include/drm/drm_connector.h | 1 +-
4 files changed, 41 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index dd40eff0911c..799e932c114d 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -139,8 +139,9 @@ static void drm_connector_get_cmdline_mode(struct
drm_connector *connector)
connector->force = mode->force;
}
- DRM_DEBUG_KMS("cmdline mode for connector %s %dx%d@%dHz%s%s%s\n",
+ DRM_DEBUG_KMS("cmdline mode for connector %s %s %dx%d@%dHz%s%s%s\n",
connector->name,
+ mode->name ? mode->name : "",
mode->xres, mode->yres,
mode->refresh_specified ? mode->refresh : 60,
mode->rb ? " reduced blanking" : "",
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 0e9349ff2d16..cacc4b56344e 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -2158,6 +2158,10 @@ struct drm_display_mode *drm_pick_cmdline_mode(struct
drm_fb_helper_connector *f
prefer_non_interlace = !cmdline_mode->interlace;
again:
list_for_each_entry(mode, _helper_conn->connector->modes, head) {
+ /* Check (optional) mode name first */
+ if (!strcmp(mode->name, cmdline_mode->name))
+ return mode;
+
/* check width/height */
if (mode->hdisplay != cmdline_mode->xres ||
mode->vdisplay != cmdline_mode->yres)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 31d61940c007..3d843d17370a 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1557,7 +1557,7 @@ bool drm_mode_parse_command_line_for_connector(const char
*mode_option,
struct drm_cmdline_mode *mode)
{
const char *name;
- bool parse_extras = false;
+ bool named_mode = false, parse_extras = false;
unsigned int bpp_off = 0, refresh_off = 0;
unsigned int mode_end = 0;
char *bpp_ptr = NULL, *refresh_ptr = NULL, *extra_ptr = NULL;
@@ -1576,8 +1576,14 @@ bool drm_mode_parse_command_line_for_connector(const
char *mode_option,
name = mode_option;
+ /*
+* If the first character is not a digit, then it means that
+* we have a named mode.
+*/
if (!isdigit(name[0]))
- return false;
+ named_mode = true;
+ else
+ named_mode = false;
/* Try to locate the bpp and refresh specifiers, if any */
bpp_ptr = strchr(name, '-');
@@ -1607,12 +1613,16 @@ bool drm_mode_parse_command_line_for_connector(const
char *mode_option,
parse_extras = true;
}
- ret = drm_mode_parse_cmdline_res_mode(name, mode_end,
- parse_extras,
- connector,
- mode);
- if (ret)
- return false;
+ if (named_mode) {
+ strncpy(mode->name, name, mode_end);
+ } else {
+ ret = drm_mode_parse_cmdline_res_mode(name, mode_end,
+ parse_extras,
+ connector,
+ mode);
+ if (ret)
+ return false;
+ }
mode->specified = true;
if (bpp_ptr) {
@@ -1640,14 +1650,23 @@ bool drm_mode_parse_command_line_for_connector(const
char *mode_option,
extra_ptr = refresh_end_ptr;
if (extra_ptr) {
- int remaining = strlen(name) - (extra_ptr - name);
+ if (!named_mode) {
+ int len = strlen(name) - (extra_ptr - name);
- /*
-* We still have characters to process, while
-* we shouldn't have any
-*/
- if (remaining > 0)
- return
[PATCH 2/7] drm/edid: Allow to ignore the audio EDID data
In some cases, in order to accomodate with displays with poor EDIDs, we need to ignore that the monitor alledgedly supports audio output and disable the audio output. Signed-off-by: Maxime Ripard --- drivers/gpu/drm/drm_edid.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 990b1909f9d7..c0258b011bb2 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -4190,6 +4190,11 @@ bool drm_detect_hdmi_monitor(struct edid *edid) } EXPORT_SYMBOL(drm_detect_hdmi_monitor); +static bool ignore_edid_audio = false; +module_param(ignore_edid_audio, bool, 0644); +MODULE_PARM_DESC(ignore_edid_audio, +"Ignore the EDID and always consider that a monitor doesn't have audio capabilities"); + /** * drm_detect_monitor_audio - check monitor audio capability * @edid: EDID block to scan @@ -4209,6 +4214,9 @@ bool drm_detect_monitor_audio(struct edid *edid) bool has_audio = false; int start_offset, end_offset; + if (ignore_edid_audio) + goto end; + edid_ext = drm_find_cea_extension(edid); if (!edid_ext) goto end; -- git-series 0.9.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/7] drm/modes: Rewrite the command line parser
From: Maxime Ripard
Rewrite the command line parser in order to get away from the state machine
parsing the video mode lines.
Hopefully, this will allow to extend it more easily to support named modes
and / or properties set directly on the command line.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/drm_modes.c | 308 +++--
1 file changed, 193 insertions(+), 115 deletions(-)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 869ac6f4671e..31d61940c007 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -30,6 +30,7 @@
* authorization from the copyright holder(s) and author(s).
*/
+#include
#include
#include
#include
@@ -1405,6 +1406,131 @@ void drm_connector_list_update(struct drm_connector
*connector)
}
EXPORT_SYMBOL(drm_connector_list_update);
+static int drm_mode_parse_cmdline_bpp(const char *str, char **end_ptr,
+ struct drm_cmdline_mode *mode)
+{
+ if (str[0] != '-')
+ return -EINVAL;
+
+ mode->bpp = simple_strtol(str + 1, end_ptr, 10);
+ mode->bpp_specified = true;
+
+ return 0;
+}
+
+static int drm_mode_parse_cmdline_refresh(const char *str, char **end_ptr,
+ struct drm_cmdline_mode *mode)
+{
+ if (str[0] != '@')
+ return -EINVAL;
+
+ mode->refresh = simple_strtol(str + 1, end_ptr, 10);
+ mode->refresh_specified = true;
+
+ return 0;
+}
+
+static int drm_mode_parse_cmdline_extra(const char *str, int length,
+ struct drm_connector *connector,
+ struct drm_cmdline_mode *mode)
+{
+ int i;
+
+ for (i = 0; i < length; i++) {
+ switch (str[i]) {
+ case 'i':
+ mode->interlace = true;
+ break;
+ case 'm':
+ mode->margins = true;
+ break;
+ case 'D':
+ if (mode->force != DRM_FORCE_UNSPECIFIED)
+ return -EINVAL;
+
+ if ((connector->connector_type !=
DRM_MODE_CONNECTOR_DVII) &&
+ (connector->connector_type !=
DRM_MODE_CONNECTOR_HDMIB))
+ mode->force = DRM_FORCE_ON;
+ else
+ mode->force = DRM_FORCE_ON_DIGITAL;
+ break;
+ case 'd':
+ if (mode->force != DRM_FORCE_UNSPECIFIED)
+ return -EINVAL;
+
+ mode->force = DRM_FORCE_OFF;
+ break;
+ case 'e':
+ if (mode->force != DRM_FORCE_UNSPECIFIED)
+ return -EINVAL;
+
+ mode->force = DRM_FORCE_ON;
+ break;
+ default:
+ return -EINVAL;
+ }
+ }
+
+ return 0;
+}
+
+static int drm_mode_parse_cmdline_res_mode(const char *str, unsigned int
length,
+ bool extras,
+ struct drm_connector *connector,
+ struct drm_cmdline_mode *mode)
+{
+ bool rb = false, cvt = false;
+ int xres = 0, yres = 0;
+ int remaining, i;
+ char *end_ptr;
+
+ xres = simple_strtol(str, _ptr, 10);
+
+ if (end_ptr[0] != 'x')
+ return -EINVAL;
+ end_ptr++;
+
+ yres = simple_strtol(end_ptr, _ptr, 10);
+
+ remaining = length - (end_ptr - str);
+ if (remaining < 0)
+ return -EINVAL;
+
+ for (i = 0; i < remaining; i++) {
+ switch (end_ptr[i]) {
+ case 'M':
+ cvt = true;
+ break;
+ case 'R':
+ rb = true;
+ break;
+ default:
+ /*
+* Try to pass that to our extras parsing
+* function to handle the case where the
+* extras are directly after the resolution
+*/
+ if (extras) {
+ int ret = drm_mode_parse_cmdline_extra(end_ptr
+ i,
+ 1,
+
connector,
+ mode);
+ if (ret)
+ return ret;
+ } else {
+ return -EINVAL;
+ }
+ }
+ }
+
+ mode->xres = xres;
+ mode->yres =
[PATCH 0/7] drm/vc4: Allow for more boot-time configuration
Hi, The proprietary stack for the RaspberryPi allows for a number of video parameters widely used by their users, but yet don't have any equivalents in the mainline kernel. Those options are detailed here: https://www.raspberrypi.org/documentation/configuration/config-txt/video.md While not all of them are desirable to have in the mainline kernel, some of them still have value, such as properties to initialise the overscan or rotation parameters, or the one to deal with broken displays. This series is an attempt to support those, and is based on a rewrite of the command line parser I did a couple of years ago that never reached upstream (due to a lack of time on my side). While this parser was initially made to deal with named modes (in order to support TV modes), it also allowed to extend it more easily, which is why it's resurrected. Since a change of the command line parser can pretty easily get things wrong and introduce regressions, I also worked with a number of unit tests that you can find here: http://code.bulix.org/tpo7dg-607264?raw Eventually, I guess those tests should be part of the kernel somewhere, but I haven't found a suitable place for them to be included yet. Let me know what you think, Maxime Maxime Ripard (7): drm/vc4: hdmi: Check that the monitor supports HDMI audio drm/edid: Allow to ignore the audio EDID data drm/edid: Allow to ignore the HDMI monitor mode drm/modes: Rewrite the command line parser drm/modes: Support modes names on the command line drm/modes: Allow to specify rotation and reflection on the commandline drm/modes: Parse overscan properties drivers/gpu/drm/drm_connector.c | 3 +- drivers/gpu/drm/drm_edid.c | 16 +- drivers/gpu/drm/drm_fb_helper.c | 55 - drivers/gpu/drm/drm_modes.c | 441 - drivers/gpu/drm/vc4/vc4_hdmi.c | 6 +- include/drm/drm_connector.h | 3 +- 6 files changed, 408 insertions(+), 116 deletions(-) base-commit: e179d8e074e05a913a0915ae3c4b82f1434d9f4e -- git-series 0.9.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/7] drm/vc4: hdmi: Check that the monitor supports HDMI audio
The current code assumes as soon as the device is an HDMI one that it
supports an audio sink. However, strictly speaking, this is exposed as a
separate part of EDID.
This can be checked through the drm_detect_monitor_audio function, so let's
use it and make sure that we can use the HDMI monitor as an output before
sending sound.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 88fd5df7e7dc..a1bdc065c47c 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -109,6 +109,7 @@ struct vc4_hdmi_encoder {
struct vc4_encoder base;
bool hdmi_monitor;
bool limited_rgb_range;
+ bool monitor_has_audio;
};
static inline struct vc4_hdmi_encoder *
@@ -278,6 +279,7 @@ static int vc4_hdmi_connector_get_modes(struct
drm_connector *connector)
return -ENODEV;
vc4_encoder->hdmi_monitor = drm_detect_hdmi_monitor(edid);
+ vc4_encoder->monitor_has_audio = drm_detect_monitor_audio(edid);
drm_connector_update_edid_property(connector, edid);
ret = drm_add_edid_modes(connector, edid);
@@ -785,9 +787,13 @@ static int vc4_hdmi_audio_startup(struct snd_pcm_substream
*substream,
{
struct vc4_hdmi *hdmi = dai_to_hdmi(dai);
struct drm_encoder *encoder = hdmi->encoder;
+ struct vc4_hdmi_encoder *vc4_encoder = to_vc4_hdmi_encoder(encoder);
struct vc4_dev *vc4 = to_vc4_dev(encoder->dev);
int ret;
+ if (!vc4_encoder->monitor_has_audio)
+ return -ENODEV;
+
if (hdmi->audio.substream && hdmi->audio.substream != substream)
return -EINVAL;
--
git-series 0.9.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 5/5] drm/vc4: fix fb references in async update
Async update callbacks are expected to set the old_fb in the new_state
so prepare/cleanup framebuffers are balanced.
Calling drm_atomic_set_fb_for_plane() (which gets a reference of the new
fb and put the old fb) is not required, as it's taken care by
drm_mode_cursor_universal() when calling drm_atomic_helper_update_plane().
Cc: # v4.19+: 25dc194b34dd: drm: Block fb changes for
async plane updates
Cc: # v4.19+: 8105bbaf9afd: drm: don't block fb
changes for async plane updates
Fixes: 25dc194b34dd ("drm: Block fb changes for async plane updates")
Suggested-by: Boris Brezillon
Signed-off-by: Helen Koike
---
Hello,
As mentioned in the cover letter,
I tested on the rockchip and on i915 using igt plane_cursor_legacy and
kms_cursor_legacy and I didn't see any regressions.
But I couldn't test on VC4. I have a Raspberry pi model B rev2, when
FB_SIMPLE is running I can see output on the screen, but when vc4 is
loaded my hdmi display is not detected anymore, I am still debugging
this, probably some config in the firmware, but I would appreciate if
anyone could help me testing it.
Also the Cc statble commit hash dependency needs to be updated once the
refered commit is merged.
Thanks!
Helen
drivers/gpu/drm/vc4/vc4_plane.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c
index 1babfeca0c92..1235e53b22a3 100644
--- a/drivers/gpu/drm/vc4/vc4_plane.c
+++ b/drivers/gpu/drm/vc4/vc4_plane.c
@@ -968,7 +968,7 @@ static void vc4_plane_atomic_async_update(struct drm_plane
*plane,
{
struct vc4_plane_state *vc4_state, *new_vc4_state;
- drm_atomic_set_fb_for_plane(plane->state, state->fb);
+ swap(plane->state->fb, state->fb);
plane->state->crtc_x = state->crtc_x;
plane->state->crtc_y = state->crtc_y;
plane->state->crtc_w = state->crtc_w;
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/5] drm/msm: fix fb references in async update
Async update callbacks are expected to set the old_fb in the new_state
so prepare/cleanup framebuffers are balanced.
Cc: # v4.14+: 25dc194b34dd: drm: Block fb changes for
async plane updates
Cc: # v4.14+: 8105bbaf9afd: drm: don't block fb
changes for async plane updates
Fixes: 25dc194b34dd ("drm: Block fb changes for async plane updates")
Suggested-by: Boris Brezillon
Signed-off-by: Helen Koike
---
Hello,
As mentioned in the cover letter,
I tested on the rockchip and on i915 using igt plane_cursor_legacy and
kms_cursor_legacy and I didn't see any regressions.
But I couldn't test on MSM because I don't have the hardware and I would
appreciate if anyone could test it.
In other platforms (VC4, AMD, Rockchip), there is a hidden
drm_framebuffer_get(new_fb)/drm_framebuffer_put(old_fb) in async_update
that is wrong, but I couldn't identify those here, not sure if it is hidden
somewhere else, but if tests fail this is probably the cause.
Thanks!
Helen
drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
index be13140967b4..b854f471e9e5 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
@@ -502,6 +502,8 @@ static int mdp5_plane_atomic_async_check(struct drm_plane
*plane,
static void mdp5_plane_atomic_async_update(struct drm_plane *plane,
struct drm_plane_state *new_state)
{
+ struct drm_framebuffer *old_fb = plane->state->fb;
+
plane->state->src_x = new_state->src_x;
plane->state->src_y = new_state->src_y;
plane->state->crtc_x = new_state->crtc_x;
@@ -524,6 +526,8 @@ static void mdp5_plane_atomic_async_update(struct drm_plane
*plane,
*to_mdp5_plane_state(plane->state) =
*to_mdp5_plane_state(new_state);
+
+ new_state->fb = old_fb;
}
static const struct drm_plane_helper_funcs mdp5_plane_helper_funcs = {
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/5] drm/amd: fix fb references in async update
Async update callbacks are expected to set the old_fb in the new_state so prepare/cleanup framebuffers are balanced. Calling drm_atomic_set_fb_for_plane() (which gets a reference of the new fb and put the old fb) is not required, as it's taken care by drm_mode_cursor_universal() when calling drm_atomic_helper_update_plane(). Suggested-by: Boris Brezillon Signed-off-by: Helen Koike --- Hello, As mentioned in the cover letter, I tested on the rockchip and on i915 using igt plane_cursor_legacy and kms_cursor_legacy and I didn't see any regressions. But I couldn't test on AMD because I don't have the hardware and I would appreciate if anyone could test it. Also, I didn't CC to stable here as I saw the async_update function was only added on v4.20, please let me know if I should CC to stable. Thanks! Helen drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index 3a6f595f295e..bc02800254bf 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -3760,8 +3760,7 @@ static void dm_plane_atomic_async_update(struct drm_plane *plane, struct drm_plane_state *old_state = drm_atomic_get_old_plane_state(new_state->state, plane); - if (plane->state->fb != new_state->fb) - drm_atomic_set_fb_for_plane(plane->state, new_state->fb); + swap(plane->state->fb, new_state->fb); plane->state->src_x = new_state->src_x; plane->state->src_y = new_state->src_y; -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/5] drm: don't block fb changes for async plane updates
In the case of a normal sync update, the preparation of framebuffers (be
it calling drm_atomic_helper_prepare_planes() or doing setups with
drm_framebuffer_get()) are performed in the new_state and the respective
cleanups are performed in the old_state.
In the case of async updates, the preparation is also done in the
new_state but the cleanups are done in the new_state (because updates
are performed in place, i.e. in the current state).
The current code blocks async udpates when the fb is changed, turning
async updates into sync updates, slowing down cursor updates and
introducing regressions in igt tests with errors of type:
"CRITICAL: completed 97 cursor updated in a period of 30 flips, we
expect to complete approximately 15360 updates, with the threshold set
at 7680"
Fb changes in async updates were prevented to avoid the following scenario:
- Async update, oldfb = NULL, newfb = fb1, prepare fb1, cleanup fb1
- Async update, oldfb = fb1, newfb = fb2, prepare fb2, cleanup fb2
- Non-async commit, oldfb = fb2, newfb = fb1, prepare fb1, cleanup fb2 (wrong)
Where we have a single call to prepare fb2 but double cleanup call to fb2.
To solve the above problems, instead of blocking async fb changes, we
place the old framebuffer in the new_state object, so when the code
performs cleanups in the new_state it will cleanup the old_fb and we
will have the following scenario instead:
- Async update, oldfb = NULL, newfb = fb1, prepare fb1, no cleanup
- Async update, oldfb = fb1, newfb = fb2, prepare fb2, cleanup fb1
- Non-async commit, oldfb = fb2, newfb = fb1, prepare fb1, cleanup fb2
Where calls to prepare/cleanup are ballanced.
Cc: # v4.14+: 25dc194b34dd: drm: Block fb changes for
async plane updates
Fixes: 25dc194b34dd ("drm: Block fb changes for async plane updates")
Suggested-by: Boris Brezillon
Signed-off-by: Helen Koike
---
Hello,
As mentioned in the cover letter,
I tested on the rockchip and on i915 (with a patch I am still working on for
replacing cursors by async update), with igt plane_cursor_legacy and
kms_cursor_legacy and I didn't see any regressions.
I couldn't test on MSM and AMD because I don't have the hardware (and I am
having some issues testing on vc4) and I would appreciate if anyone could help
me testing those.
I also think it would be a better solution if, instead of having async
to do in-place updates in the current state, the async path should be
equivalent to a syncronous update, i.e., modifying new_state and
performing a flip
IMHO, the only difference between sync and async should be that async update
doesn't wait for vblank and applies the changes immeditally to the hw,
but the code path could be almost the same.
But for now I think this solution is ok (swaping new_fb/old_fb), and
then we can adjust things little by little, what do you think?
Thanks!
Helen
drivers/gpu/drm/drm_atomic_helper.c | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c
b/drivers/gpu/drm/drm_atomic_helper.c
index 540a77a2ade9..e7eb96f1efc2 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1608,15 +1608,6 @@ int drm_atomic_helper_async_check(struct drm_device *dev,
old_plane_state->crtc != new_plane_state->crtc)
return -EINVAL;
- /*
-* FIXME: Since prepare_fb and cleanup_fb are always called on
-* the new_plane_state for async updates we need to block framebuffer
-* changes. This prevents use of a fb that's been cleaned up and
-* double cleanups from occuring.
-*/
- if (old_plane_state->fb != new_plane_state->fb)
- return -EINVAL;
-
funcs = plane->helper_private;
if (!funcs->atomic_async_update)
return -EINVAL;
@@ -1657,6 +1648,9 @@ void drm_atomic_helper_async_commit(struct drm_device
*dev,
int i;
for_each_new_plane_in_state(state, plane, plane_state, i) {
+ struct drm_framebuffer *new_fb = plane_state->fb;
+ struct drm_framebuffer *old_fb = plane->state->fb;
+
funcs = plane->helper_private;
funcs->atomic_async_update(plane, plane_state);
@@ -1665,11 +1659,17 @@ void drm_atomic_helper_async_commit(struct drm_device
*dev,
* plane->state in-place, make sure at least common
* properties have been properly updated.
*/
- WARN_ON_ONCE(plane->state->fb != plane_state->fb);
+ WARN_ON_ONCE(plane->state->fb != new_fb);
WARN_ON_ONCE(plane->state->crtc_x != plane_state->crtc_x);
WARN_ON_ONCE(plane->state->crtc_y != plane_state->crtc_y);
WARN_ON_ONCE(plane->state->src_x != plane_state->src_x);
WARN_ON_ONCE(plane->state->src_y != plane_state->src_y);
+
+ /*
+* Make sure the FBs have been swapped so that cleanups in
[PATCH 0/5] drm: Fix fb changes for async updates
Hello, This series is a first attempt to fix the slow down in performance introduced by "[PATCH v2] drm: Block fb changes for async plane updates" where async update falls back to a sync update, causing igt failures of type: "CRITICAL: completed 97 cursor updated in a period of 30 flips, we expect to complete approximately 15360 updates, with the threshold set at 7680" Please read the commit message of the first patch to understand how it works. I tested on the rockchip and on i915 (with a patch I am still working on for replacing cursors by async update), with igt plane_cursor_legacy and kms_cursor_legacy and I didn't see any regressions. I couldn't test on MSM and AMD because I don't have the hardware and my vc4 raspberry pi is not recognizing my display for some reason, I would appreciate if anyone could help me testing those. I also separated the patches per platform to be easier to get the tested-by tags, please let me know if it should be a single patch. Also, I added CC stable (as the "drm: Block fb changes for async plane updates" was also CCed to stable). I am not used to CC stable, please let me know if anything is off. Thanks! Helen Helen Koike (5): drm: don't block fb changes for async plane updates drm/rockchip: fix fb references in async update drm/amd: fix fb references in async update drm/msm: fix fb references in async update drm/vc4: fix fb references in async update .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +- drivers/gpu/drm/drm_atomic_helper.c | 20 - drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c| 4 ++ drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 42 +++ drivers/gpu/drm/vc4/vc4_plane.c | 2 +- 5 files changed, 40 insertions(+), 31 deletions(-) -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/5] drm/rockchip: fix fb references in async update
In the case of async update, modifications are done in place, i.e. in the
current plane state, so the new_state is prepared and the new_state is
cleanup up (instead of the old_state, diferrently on what happen in a
normal sync update).
To cleanup the old_fb properly, it needs to be placed in the new_state
in the end of async_update, so cleanup call will unreference the old_fb
correctly.
Also, the previous code had a:
plane_state = plane->funcs->atomic_duplicate_state(plane);
...
swap(plane_state, plane->state);
if (plane->state->fb && plane->state->fb != new_state->fb) {
...
}
Which was wrong, as the fb were just assigned to be equal, so this if
statement nevers evaluates to true.
Another details is that the function drm_crtc_vblank_get() can only be
called when vop->is_enabled is true, otherwise it has no effect and
trows a WARN_ON().
Calling drm_atomic_set_fb_for_plane() (which get a referent of the new
fb and pus the old fb) is not required, as it is taken care by
drm_mode_cursor_universal() when calling
drm_atomic_helper_update_plane().
Signed-off-by: Helen Koike
---
Hello,
I tested on the rockchip ficus v1.1 using igt plane_cursor_legacy and
kms_cursor_legacy and I didn't see any regressions.
It was me who worked on the first version and I am really sorry about that
dead code in the if statement.
Now I understand drm better and I know better how to properly test things with
more care/details.
Also, I didn't CC to stable here as I saw the async_update function was only
added on v4.20, please let me know if I should CC to stable.
Thanks!
Helen
drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 42 -
1 file changed, 24 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index c7d4c6073ea5..a1ee8c156a7b 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -912,30 +912,31 @@ static void vop_plane_atomic_async_update(struct
drm_plane *plane,
struct drm_plane_state *new_state)
{
struct vop *vop = to_vop(plane->state->crtc);
- struct drm_plane_state *plane_state;
+ struct drm_framebuffer *old_fb = plane->state->fb;
- plane_state = plane->funcs->atomic_duplicate_state(plane);
- plane_state->crtc_x = new_state->crtc_x;
- plane_state->crtc_y = new_state->crtc_y;
- plane_state->crtc_h = new_state->crtc_h;
- plane_state->crtc_w = new_state->crtc_w;
- plane_state->src_x = new_state->src_x;
- plane_state->src_y = new_state->src_y;
- plane_state->src_h = new_state->src_h;
- plane_state->src_w = new_state->src_w;
-
- if (plane_state->fb != new_state->fb)
- drm_atomic_set_fb_for_plane(plane_state, new_state->fb);
-
- swap(plane_state, plane->state);
-
- if (plane->state->fb && plane->state->fb != new_state->fb) {
+ /*
+* A scanout can still be occurring, so we can't drop the reference to
+* the old framebuffer. To solve this we get a reference to old_fb and
+* set a worker to release it later.
+*/
+ if (vop->is_enabled &&
+ plane->state->fb && plane->state->fb != new_state->fb) {
drm_framebuffer_get(plane->state->fb);
WARN_ON(drm_crtc_vblank_get(plane->state->crtc) != 0);
drm_flip_work_queue(>fb_unref_work, plane->state->fb);
set_bit(VOP_PENDING_FB_UNREF, >pending);
}
+ plane->state->crtc_x = new_state->crtc_x;
+ plane->state->crtc_y = new_state->crtc_y;
+ plane->state->crtc_h = new_state->crtc_h;
+ plane->state->crtc_w = new_state->crtc_w;
+ plane->state->src_x = new_state->src_x;
+ plane->state->src_y = new_state->src_y;
+ plane->state->src_h = new_state->src_h;
+ plane->state->src_w = new_state->src_w;
+ plane->state->fb = new_state->fb;
+
if (vop->is_enabled) {
rockchip_drm_psr_inhibit_get_state(new_state->state);
vop_plane_atomic_update(plane, plane->state);
@@ -945,7 +946,12 @@ static void vop_plane_atomic_async_update(struct drm_plane
*plane,
rockchip_drm_psr_inhibit_put_state(new_state->state);
}
- plane->funcs->atomic_destroy_state(plane, plane_state);
+ /*
+* In async update we perform inplace modifications and release the
+* new_state. The following is required so we release the reference of
+* the old framebuffer.
+*/
+ new_state->fb = old_fb;
}
static const struct drm_plane_helper_funcs plane_helper_funcs = {
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] tinydrm/mipi-dbi: Use dma-safe buffers for all SPI transfers
Den 22.02.2019 16.58, skrev Andy Shevchenko:
> On Fri, Feb 22, 2019 at 01:43:29PM +0100, Noralf Trønnes wrote:
>> Buffers passed to spi_sync() must be dma-safe even for tiny buffers since
>> some SPI controllers use DMA for all transfers.
>>
>> Example splat with CONFIG_DMA_API_DEBUG enabled:
>>
>> [ 23.750467] DMA-API: dw_dmac_pci :00:15.0: device driver maps memory
>> from stack [probable addr=1e49185d]
>> [ 23.750529] WARNING: CPU: 1 PID: 1296 at kernel/dma/debug.c:1161
>> check_for_stack+0xb7/0x190
>> [ 23.750533] Modules linked in: mmc_block(+) spi_pxa2xx_platform(+)
>> pwm_lpss_pci pwm_lpss spi_pxa2xx_pci sdhci_pci cqhci intel_mrfld_pwrbtn
>> extcon_intel_mrfld sdhci intel_mrfld_adc led_class mmc_core ili9341 mipi_dbi
>> tinydrm backlight ti_ads7950 industrialio_triggered_buffer kfifo_buf
>> intel_soc_pmic_mrfld hci_uart btbcm
>> [ 23.750599] CPU: 1 PID: 1296 Comm: modprobe Not tainted 5.0.0-rc7+ #236
>> [ 23.750605] Hardware name: Intel Corporation Merrifield/BODEGA BAY, BIOS
>> 542 2015.01.21:18.19.48
>> [ 23.750620] RIP: 0010:check_for_stack+0xb7/0x190
>> [ 23.750630] Code: 8b 6d 50 4d 85 ed 75 04 4c 8b 6d 10 48 89 ef e8 2f 8b
>> 44 00 48 89 c6 4a 8d 0c 23 4c 89 ea 48 c7 c7 88 d0 82 b4 e8 40 7c f9 ff <0f>
>> 0b 8b 05 79 00 4b 01 85 c0 74 07 5b 5d 41 5c 41 5d c3 8b 05 54
>> [ 23.750637] RSP: :97bbc0292fa0 EFLAGS: 00010286
>> [ 23.750646] RAX: RBX: 97bbc029 RCX:
>> 0006
>> [ 23.750652] RDX: 0007 RSI: 0002 RDI:
>> 94b33e115450
>> [ 23.750658] RBP: 94b33c8578b0 R08: 0002 R09:
>> 000201c0
>> [ 23.750664] R10: 0006ecb0ccc6 R11: 00034f38 R12:
>> 316c
>> [ 23.750670] R13: 94b33c84b250 R14: 94b33dedd5a0 R15:
>> 0001
>> [ 23.750679] FS: () GS:94b33e10(0063)
>> knlGS:f7faf690
>> [ 23.750686] CS: 0010 DS: 002b ES: 002b CR0: 80050033
>> [ 23.750691] CR2: f7f54faf CR3: 0722c000 CR4:
>> 001006e0
>> [ 23.750696] Call Trace:
>> [ 23.750713] debug_dma_map_sg+0x100/0x340
>> [ 23.750727] ? dma_direct_map_sg+0x3b/0xb0
>> [ 23.750739] spi_map_buf+0x25a/0x300
>> [ 23.750751] __spi_pump_messages+0x2a4/0x680
>> [ 23.750762] __spi_sync+0x1dd/0x1f0
>> [ 23.750773] spi_sync+0x26/0x40
>> [ 23.750790] mipi_dbi_typec3_command_read+0x14d/0x240 [mipi_dbi]
>> [ 23.750802] ? spi_finalize_current_transfer+0x10/0x10
>> [ 23.750821] mipi_dbi_typec3_command+0x1bc/0x1d0 [mipi_dbi]
>>
>
> After few runs I don't see the warning anymore. So,
> Tested-by: Andy Shevchenko
>
Can I have an ack as well if you're satisfied with it?
Noralf.
> Though I would like to give few more days to monitor the state.
> (I believe it's fixed)
>
>> Reported-by: Andy Shevchenko
>> Signed-off-by: Noralf Trønnes
>> ---
>> drivers/gpu/drm/tinydrm/ili9225.c | 6 ++--
>> drivers/gpu/drm/tinydrm/mipi-dbi.c | 58 +-
>> include/drm/tinydrm/mipi-dbi.h | 5 +--
>> 3 files changed, 48 insertions(+), 21 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/tinydrm/ili9225.c
>> b/drivers/gpu/drm/tinydrm/ili9225.c
>> index 3f59cfbd31ba..a87078667e04 100644
>> --- a/drivers/gpu/drm/tinydrm/ili9225.c
>> +++ b/drivers/gpu/drm/tinydrm/ili9225.c
>> @@ -299,7 +299,7 @@ static void ili9225_pipe_disable(struct
>> drm_simple_display_pipe *pipe)
>> mipi->enabled = false;
>> }
>>
>> -static int ili9225_dbi_command(struct mipi_dbi *mipi, u8 cmd, u8 *par,
>> +static int ili9225_dbi_command(struct mipi_dbi *mipi, u8 *cmd, u8 *par,
>> size_t num)
>> {
>> struct spi_device *spi = mipi->spi;
>> @@ -309,11 +309,11 @@ static int ili9225_dbi_command(struct mipi_dbi *mipi,
>> u8 cmd, u8 *par,
>>
>> gpiod_set_value_cansleep(mipi->dc, 0);
>> speed_hz = mipi_dbi_spi_cmd_max_speed(spi, 1);
>> -ret = tinydrm_spi_transfer(spi, speed_hz, NULL, 8, , 1);
>> +ret = tinydrm_spi_transfer(spi, speed_hz, NULL, 8, cmd, 1);
>> if (ret || !num)
>> return ret;
>>
>> -if (cmd == ILI9225_WRITE_DATA_TO_GRAM && !mipi->swap_bytes)
>> +if (*cmd == ILI9225_WRITE_DATA_TO_GRAM && !mipi->swap_bytes)
>> bpw = 16;
>>
>> gpiod_set_value_cansleep(mipi->dc, 1);
>> diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c
>> b/drivers/gpu/drm/tinydrm/mipi-dbi.c
>> index 246e708a9ff7..4a4cd7e72938 100644
>> --- a/drivers/gpu/drm/tinydrm/mipi-dbi.c
>> +++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c
>> @@ -153,16 +153,42 @@ EXPORT_SYMBOL(mipi_dbi_command_read);
>> */
>> int mipi_dbi_command_buf(struct mipi_dbi *mipi, u8 cmd, u8 *data, size_t
>> len)
>> {
>> +u8 *cmdbuf;
>> int ret;
>>
>> +/* SPI requires dma-safe buffers */
>> +cmdbuf = kmemdup(, 1, GFP_KERNEL);
>> +if (!cmdbuf)
>> +return -ENOMEM;
>> +
>> mutex_lock(>cmdlock);
>> -ret =
Re: [PATCH v3 0/7] drm/tinydrm: Remove tinydrm_device
Den 25.02.2019 15.42, skrev Noralf Trønnes: > This patchset is part of the effort to remove tinydrm.ko. It removes > struct tinydrm_device and tinydrm.h. > > Only one change in this version and that is expanding the driver > example. > > The drm_dev_unplug() dependency series has been applied together with > some patches from the previous version. > > I've cc'ed intel-gfx so the Intel CI can verify the parent device ref > patch. > > Noralf. > > Noralf Trønnes (7): > drm/drv: Hold ref on parent device during drm_device lifetime > drm: Add devm_drm_dev_init() > drm/drv: DOC: Add driver example code > drm/tinydrm/repaper: Drop using tinydrm_device > drm/tinydrm: Drop using tinydrm_device > drm/tinydrm: Remove tinydrm_device > drm/tinydrm: Use drm_dev_enter/exit() > Series is applied to drm-misc-next, thanks for reviewing! Noralf. > Documentation/driver-model/devres.txt | 3 + > Documentation/gpu/tinydrm.rst | 32 +--- > Documentation/gpu/todo.rst| 4 - > drivers/gpu/drm/drm_drv.c | 176 +- > drivers/gpu/drm/tinydrm/core/Makefile | 2 +- > drivers/gpu/drm/tinydrm/core/tinydrm-core.c | 169 - > .../gpu/drm/tinydrm/core/tinydrm-helpers.c| 2 + > drivers/gpu/drm/tinydrm/hx8357d.c | 49 - > drivers/gpu/drm/tinydrm/ili9225.c | 63 ++- > drivers/gpu/drm/tinydrm/ili9341.c | 49 - > drivers/gpu/drm/tinydrm/mi0283qt.c| 49 - > drivers/gpu/drm/tinydrm/mipi-dbi.c| 109 --- > drivers/gpu/drm/tinydrm/repaper.c | 130 + > drivers/gpu/drm/tinydrm/st7586.c | 129 - > drivers/gpu/drm/tinydrm/st7735r.c | 49 - > include/drm/drm_drv.h | 3 + > include/drm/tinydrm/mipi-dbi.h| 26 ++- > include/drm/tinydrm/tinydrm.h | 42 - > 18 files changed, 688 insertions(+), 398 deletions(-) > delete mode 100644 drivers/gpu/drm/tinydrm/core/tinydrm-core.c > delete mode 100644 include/drm/tinydrm/tinydrm.h > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 0/5] drm/bridge: sii902x: HDMI-audio support and some fixes
On 04/03/2019 14:42, Laurent Pinchart wrote: > Hi Jyri, > > On Wed, Feb 27, 2019 at 11:54:18PM +0200, Jyri Sarha wrote: >> Changes since first version: >> - Moved reviewed patches to front: >> - drm/bridge: sii902x: add input_bus_flags >> - drm/bridge: sii902x: Set output mode to HDMI or DVI according to EDID >> - drm/bridge: sii902x: pixel clock unit is 10kHz instead of 1kHz >> - Added a new fix: >> - drm/bridge: sii902x: Select I2C_MUX >> - Applied some review suggestions to >> - drm/bridge: sii902x: Implement HDMI audio support >> - use clock-names property to name mclk >> - move comment describing added mutex to struct sii902x and improve it >> - cleanup sii902x_mute() >> - cleanup sii902x_select_mclk_div() >> - fix condition for checking ENABLE_BIT from i2s_fifo_routing in >> sii902x_audio_codec_init() >> >> Still to do >> >> - Agree on i2s wires to HDMI audio fifo routing in dts. >> >> The current scheme is quite straight forward, but there is maybe >> there is even more straight forward solutions like: >> >> audio-fifo-enable = <1 1 1 1>; >> audio-i2s-pin-to-fifo = <0 1 2 3>; >> >> Meaning that all fifos are enabled and SD0 is routed to fifo 0, SD1 >> to fifo 1, etc. I am not sure if the channel swap functionality >> should show in dts binding. > Please forgive my lack of audio knowledge, but it this a system > description that should be encoded in DT, or a policy that should be > handled purely in software (either fully inside the kernel or with the > help of userspace) ? > This property describes how many i2s wires are connected to sii902x and in what order, so I think it belongs to DTS. One might of course wonder why anybody would put the i2s wires to any other order than 0 <-> 0, 1 <-> 1, 2 <-> 2, and 3 <-> 3, but then a again I've seen weirder board designs. Best regards, Jyri -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 2/5] drm/bridge: sii902x: Set output mode to HDMI or DVI according to EDID
On 04/03/2019 14:52, Laurent Pinchart wrote:
> Hi Jyri,
>
> Thank you for the patch.
> On Wed, Feb 27, 2019 at 11:54:20PM +0200, Jyri Sarha wrote:
>> Set output mode to HDMI or DVI according to EDID HDMI signature.
>>
>> Signed-off-by: Jyri Sarha
>> Reviewed-by: Andrzej Hajda
>> ---
>> drivers/gpu/drm/bridge/sii902x.c | 9 +
>> 1 file changed, 9 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/bridge/sii902x.c
>> b/drivers/gpu/drm/bridge/sii902x.c
>> index 1afa000141d5..0e21fa419d27 100644
>> --- a/drivers/gpu/drm/bridge/sii902x.c
>> +++ b/drivers/gpu/drm/bridge/sii902x.c
>> @@ -181,11 +181,15 @@ static int sii902x_get_modes(struct drm_connector
>> *connector)
>> struct sii902x *sii902x = connector_to_sii902x(connector);
>> u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24;
>> struct edid *edid;
>> +u8 output_mode = SII902X_SYS_CTRL_OUTPUT_DVI;
>
> I'd move this one line up, but that's certainly a matter of taste :-)
I usually sort the local variables by length too. I wonder why I did not
do it this time... I'll fix it :).
>> int num = 0, ret;
>>
>> edid = drm_get_edid(connector, sii902x->i2cmux->adapter[0]);
>> drm_connector_update_edid_property(connector, edid);
>> if (edid) {
>> +if (drm_detect_hdmi_monitor(edid))
>> +output_mode = SII902X_SYS_CTRL_OUTPUT_HDMI;
>> +
>> num = drm_add_edid_modes(connector, edid);
>> kfree(edid);
>> }
>> @@ -195,6 +199,11 @@ static int sii902x_get_modes(struct drm_connector
>> *connector)
>> if (ret)
>> return ret;
>>
>> +ret = regmap_update_bits(sii902x->regmap, SII902X_SYS_CTRL_DATA,
>> + SII902X_SYS_CTRL_OUTPUT_MODE, output_mode);
>> +if (ret)
>> +return ret;
>> +
>
> Is this the right place to update the register, shouldn't this be done
> in sii902x_bridge_enable() instead ? I could foresee cases where the
> chip could be powered down between get_modes() and enable(), losing its
> internal state.
>
I have a spec (unfortunately I can not share it) that describes the
sequence of handling a hot plug event on sii9022. There it is said that
the HDMI mode, if HDMI signature is found, should be set at the same
time while releasing the DCC access by setting SII902X_SYS_CTRL_DATA
bits #1 and #2 to zero.
However, I did not dare to change sii902x_i2c_bypass_deselect()
function, so I set the HDMI mode right after the DCC pass trough mode is
disabled.
Having it there is logical too, since the HDMI/DVI-mode should not
change without a hot plug event. In practice sii9022 does not appear to
be too picky when the bit is set.
>> return num;
>> }
>>
>
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 15/17] drm/vc4: Convert to using __drm_atomic_helper_crtc_reset() for reset.
Op 01-03-2019 om 23:47 schreef Eric Anholt:
> Maarten Lankhorst writes:
>
>> Convert vc4 to using __drm_atomic_helper_crtc_reset(), instead of
>> writing its own version. Instead of open coding destroy_state(),
>> call it directly for freeing the old state.
>>
>> Signed-off-by: Maarten Lankhorst
>> Cc: Eric Anholt
>> ---
>> drivers/gpu/drm/vc4/vc4_crtc.c | 9 +
>> 1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
>> index e7c04a9eb219..fdf21594b050 100644
>> --- a/drivers/gpu/drm/vc4/vc4_crtc.c
>> +++ b/drivers/gpu/drm/vc4/vc4_crtc.c
>> @@ -1041,12 +1041,13 @@ static void vc4_crtc_destroy_state(struct drm_crtc
>> *crtc,
>> static void
>> vc4_crtc_reset(struct drm_crtc *crtc)
>> {
>> -if (crtc->state)
>> -vc4_crtc_destroy_state(crtc->state);
>> +struct vc4_crtc_state *crtc_state =
>> +kzalloc(sizeof(*crtc_state), GFP_KERNEL);
>>
>> -crtc->state = kzalloc(sizeof(struct vc4_crtc_state), GFP_KERNEL);
>> if (crtc->state)
>> -crtc->state->crtc = crtc;
>> +vc4_crtc_destroy_state(crtc, crtc->state);
>> +
>> +__drm_atomic_helper_crtc_reset(crtc, _state->base);
> Wouldn't it be much easier if __drm_atomic_helper_crtc_reset took in a
> new state and destroyed the old state for you? It seems like hardly a
> helper as is.
Yes it would, but the plane/connector reset is the same. If you want to convert
them all it would be nice. :)
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 202735] CONFIG_DRM_TTM does not have name in Kconfig, so it is invisble for menuconfig
https://bugzilla.kernel.org/show_bug.cgi?id=202735 --- Comment #4 from Ilya (kazakevichi...@gmail.com) --- Not a bug at all. I'd call it "usability problem") User can't enable TTM (which is required for virtualbox) unless she enables some redundant driver. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [linux-sunxi] [RESEND PATCH] drm/sun4i: hdmi: Improve compatibility with hpd-less HDMI displays
On Mon, Mar 04, 2019 at 03:06:16PM +0200, Priit Laes wrote: > From: Priit Laes > > Even though HDMI connector features hotplug detect pin (HPD), there > are older devices which do not support it. For these devices fall > back to additional check on I2C bus to probe for EDID data. > > One known example is HDMI/DVI display with following edid: > > $ xxd -p display.edid > 000005a1e0030100150f0103800f05780a0f6ea05748 > 9a2610474f20010101010101010101010101010101012a08804520e0 > 0b10200040009536001800fd0034441a2403000a202020202020 > 001000310a202020202020202020202000102a4030701300 > 782d111e006b > > $ edid-decode display.edid > EDID version: 1.3 > Manufacturer: AMA Model 3e0 Serial Number 1 > Digital display > Maximum image size: 15 cm x 5 cm > Gamma: 2.20 > RGB color display > First detailed timing is preferred timing > Display x,y Chromaticity: > Red: 0.6250, 0.3398 > Green: 0.2841, 0.6044 > Blue: 0.1494, 0.0644 > White: 0.2802, 0.3105 > > Established timings supported: > 640x480@60Hz 4:3 HorFreq: 31469 Hz Clock: 25.175 MHz > Standard timings supported: > Detailed mode: Clock 20.900 MHz, 149 mm x 54 mm >640 672 672 709 hborder 0 >480 484 484 491 vborder 0 >-hsync -vsync >VertFreq: 60 Hz, HorFreq: 29478 Hz > Monitor ranges (GTF): 52-68Hz V, 26-36kHz H, max dotclock 30MHz > Dummy block > Dummy block > Checksum: 0x6b (valid) > > Now, current implementation is still flawed, as HDMI uses the > HPD signal to indicate that the source should re-read the EDID > due to change in device capabilities. With current HPD polling > implementation we would most certainly miss those notifications > as one can try just swapping two HDMI monitors really fast. > > Proper fix would be skipping the HPD pin detection and relying > on just EDID fetching and acting on its changes. HPD has been a hard requirement since DDWG came up with DVI somewhere in the late 90s. This monitor is plainly broken, and should not get an expensive i2c address polling based workaround at the driver level. Luc Verhaegen. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 14/17] drm/tegra: Convert to using __drm_atomic_helper_crtc_reset() for reset.
On Fri, Mar 01, 2019 at 01:56:24PM +0100, Maarten Lankhorst wrote:
> Convert tegra to using __drm_atomic_helper_crtc_reset(), instead of
> writing its own version. Instead of open coding destroy_state(),
> call it directly for freeing the old state.
>
> Signed-off-by: Maarten Lankhorst
> Cc: Thierry Reding
> Cc: Jonathan Hunter
> Cc: linux-te...@vger.kernel.org
> ---
> drivers/gpu/drm/tegra/dc.c | 30 +++---
> 1 file changed, 11 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> index 607a6ea17ecc..57c88d78cdaa 100644
> --- a/drivers/gpu/drm/tegra/dc.c
> +++ b/drivers/gpu/drm/tegra/dc.c
> @@ -1153,25 +1153,6 @@ static void tegra_dc_destroy(struct drm_crtc *crtc)
> drm_crtc_cleanup(crtc);
> }
>
> -static void tegra_crtc_reset(struct drm_crtc *crtc)
> -{
> - struct tegra_dc_state *state;
> -
> - if (crtc->state)
> - __drm_atomic_helper_crtc_destroy_state(crtc->state);
> -
> - kfree(crtc->state);
> - crtc->state = NULL;
> -
> - state = kzalloc(sizeof(*state), GFP_KERNEL);
> - if (state) {
> - crtc->state = >base;
> - crtc->state->crtc = crtc;
> - }
> -
> - drm_crtc_vblank_reset(crtc);
> -}
> -
> static struct drm_crtc_state *
> tegra_crtc_atomic_duplicate_state(struct drm_crtc *crtc)
> {
> @@ -1198,6 +1179,17 @@ static void tegra_crtc_atomic_destroy_state(struct
> drm_crtc *crtc,
> kfree(state);
> }
>
> +static void tegra_crtc_reset(struct drm_crtc *crtc)
> +{
> + struct tegra_dc_state *state = kzalloc(sizeof(*state), GFP_KERNEL);
> +
> + if (crtc->state)
> + tegra_crtc_atomic_destroy_state(crtc, crtc->state);
> +
> + __drm_atomic_helper_crtc_reset(crtc, >base);
> + drm_crtc_vblank_reset(crtc);
> +}
> +
I would preferred a predeclaration of tegra_crtc_atomic_destroy_state()
in this case because the implementations are in the same order as their
use in tegra_crtc_funcs, but I think I can live with it, so either way:
Acked-by: Thierry Reding
signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 2/2] drm/lima: driver for ARM Mali4xx GPUs
Hi Sam, Thanks, I'll fix them in next version. Regards, Qiang On Sun, Mar 3, 2019 at 11:02 PM Sam Ravnborg wrote: > > Hi Qiang. > > Good to see you do prompt follow-up on the feedback you get. > I applied the patch and noticed that git compains about > a few whitespace errors. > So for good measure I throw checkpatch after it. > $ scripts/checkpatch.pl --max-line-length=120 ~/lima > > ... > total: 45 errors, 36 warnings, 4197 lines checked > > Several (most) of these looks like legitimate issues and should > be fixed. > It is much easier to do it now, rather than handle the trivial > patches later. > > Sam ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 202735] CONFIG_DRM_TTM does not have name in Kconfig, so it is invisble for menuconfig
https://bugzilla.kernel.org/show_bug.cgi?id=202735 --- Comment #3 from Jani Nikula (jani.nik...@intel.com) --- Arguably not a bug in kernel. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 2/5] drm/bridge: sii902x: Set output mode to HDMI or DVI according to EDID
Hi Jyri,
Thank you for the patch.
On Wed, Feb 27, 2019 at 11:54:20PM +0200, Jyri Sarha wrote:
> Set output mode to HDMI or DVI according to EDID HDMI signature.
>
> Signed-off-by: Jyri Sarha
> Reviewed-by: Andrzej Hajda
> ---
> drivers/gpu/drm/bridge/sii902x.c | 9 +
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/sii902x.c
> b/drivers/gpu/drm/bridge/sii902x.c
> index 1afa000141d5..0e21fa419d27 100644
> --- a/drivers/gpu/drm/bridge/sii902x.c
> +++ b/drivers/gpu/drm/bridge/sii902x.c
> @@ -181,11 +181,15 @@ static int sii902x_get_modes(struct drm_connector
> *connector)
> struct sii902x *sii902x = connector_to_sii902x(connector);
> u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24;
> struct edid *edid;
> + u8 output_mode = SII902X_SYS_CTRL_OUTPUT_DVI;
I'd move this one line up, but that's certainly a matter of taste :-)
> int num = 0, ret;
>
> edid = drm_get_edid(connector, sii902x->i2cmux->adapter[0]);
> drm_connector_update_edid_property(connector, edid);
> if (edid) {
> + if (drm_detect_hdmi_monitor(edid))
> + output_mode = SII902X_SYS_CTRL_OUTPUT_HDMI;
> +
> num = drm_add_edid_modes(connector, edid);
> kfree(edid);
> }
> @@ -195,6 +199,11 @@ static int sii902x_get_modes(struct drm_connector
> *connector)
> if (ret)
> return ret;
>
> + ret = regmap_update_bits(sii902x->regmap, SII902X_SYS_CTRL_DATA,
> + SII902X_SYS_CTRL_OUTPUT_MODE, output_mode);
> + if (ret)
> + return ret;
> +
Is this the right place to update the register, shouldn't this be done
in sii902x_bridge_enable() instead ? I could foresee cases where the
chip could be powered down between get_modes() and enable(), losing its
internal state.
> return num;
> }
>
--
Regards,
Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC PATCH 1/2] drm/panel: Add Ilitek ILI9341 parallel RGB panel driver
---
MAINTAINERS | 6 +
drivers/gpu/drm/panel/Kconfig| 7 +
drivers/gpu/drm/panel/Makefile | 1 +
drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 320 +++
4 files changed, 334 insertions(+)
create mode 100644 drivers/gpu/drm/panel/panel-ilitek-ili9341.c
diff --git a/MAINTAINERS b/MAINTAINERS
index a4a4bf563f94..d2e03c5ad04d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4821,6 +4821,12 @@ S: Maintained
F: drivers/gpu/drm/tinydrm/ili9225.c
F: Documentation/devicetree/bindings/display/ilitek,ili9225.txt
+DRM DRIVER FOR ILITEK ILI9341 PANELS
+M: Josef Lusticky
+S: Maintained
+F: drivers/gpu/drm/panel/panel-ilitek-ili9341.c
+F: Documentation/devicetree/bindings/display/panel/ilitek,ili9341.txt
+
DRM DRIVER FOR HX8357D PANELS
M: Eric Anholt
T: git git://anongit.freedesktop.org/drm/drm-misc
diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index f53f817356db..a59cfff614c0 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -46,6 +46,13 @@ config DRM_PANEL_ILITEK_IL9322
Say Y here if you want to enable support for Ilitek IL9322
QVGA (320x240) RGB, YUV and ITU-T BT.656 panels.
+config DRM_PANEL_ILITEK_IL9341
+ tristate "Ilitek ILI9341 240x320 panels"
+ depends on OF && SPI
+ help
+ Say Y here if you want to enable support for Ilitek IL9341
+ QVGA (240x320) RGB panel.
+
config DRM_PANEL_ILITEK_ILI9881C
tristate "Ilitek ILI9881C-based panels"
depends on OF
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index 7834947a53b0..1ce3ff8d6204 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -3,6 +3,7 @@ obj-$(CONFIG_DRM_PANEL_ARM_VERSATILE) += panel-arm-versatile.o
obj-$(CONFIG_DRM_PANEL_LVDS) += panel-lvds.o
obj-$(CONFIG_DRM_PANEL_SIMPLE) += panel-simple.o
obj-$(CONFIG_DRM_PANEL_ILITEK_IL9322) += panel-ilitek-ili9322.o
+obj-$(CONFIG_DRM_PANEL_ILITEK_IL9341) += panel-ilitek-ili9341.o
obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9881C) += panel-ilitek-ili9881c.o
obj-$(CONFIG_DRM_PANEL_INNOLUX_P079ZCA) += panel-innolux-p079zca.o
obj-$(CONFIG_DRM_PANEL_JDI_LT070ME05000) += panel-jdi-lt070me05000.o
diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
new file mode 100644
index ..51ed03140f8d
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
@@ -0,0 +1,320 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Ilitek ILI9341 drm_panel driver
+ * 240RGBx320 dots resolution TFT LCD display
+ *
+ * This driver support the following panel configurations:
+ * - 18-bit parallel RGB interface
+ * - 8-bit SPI with Data/Command GPIO
+ *
+ * Copyright (C) 2019 Josef Lusticky
+ *
+ */
+
+#include
+#include
+#include
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+
+/* ILI9341 extended register set */
+#define ILI9341_IFMODE 0xB0 // clock polarity
+#define ILI9341_IFCTL 0xF6 // interface conrol
+#define ILI9341_PGAMCTRL 0xE0 // positive gamma control
+#define ILI9341_NGAMCTRL 0xE1 // negative gamma control
+
+#define ILI9341_MADCTL_MV BIT(5)
+#define ILI9341_MADCTL_MX BIT(6)
+#define ILI9341_MADCTL_MY BIT(7)
+
+/**
+ * struct ili9341_config - the display specific configuration
+ * @width_mm: physical panel width [mm]
+ * @height_mm: physical panel height [mm]
+ */
+struct ili9341_config {
+ u32 width_mm;
+ u32 height_mm;
+};
+
+struct ili9341 {
+ const struct ili9341_config *conf;
+ struct drm_panel panel;
+ struct spi_device *spi;
+ struct backlight_device *backlight;
+ struct gpio_desc *dc_gpio;
+ struct gpio_desc *reset_gpio;
+};
+
+static inline struct ili9341 *panel_to_ili9341(struct drm_panel *panel)
+{
+ return container_of(panel, struct ili9341, panel);
+}
+
+static int ili9341_spi_write_command(struct ili9341 *ili, u8 cmd)
+{
+ struct spi_transfer xfer = {
+ .tx_buf = ,
+ .bits_per_word = 8,
+ .len = 1
+ };
+ struct spi_message msg;
+ spi_message_init();
+ spi_message_add_tail(, );
+
+ gpiod_set_value(ili->dc_gpio, 0);
+
+ return spi_sync(ili->spi, );
+}
+
+static int ili9341_spi_write_data(struct ili9341 *ili, u8 *data, size_t size)
+{
+ struct spi_transfer xfer = {
+ .tx_buf = data,
+ .bits_per_word = 8,
+ .len = size
+ };
+
+ struct spi_message msg;
+ spi_message_init();
+ spi_message_add_tail(, );
+
+ gpiod_set_value(ili->dc_gpio, 1);
+
+ return spi_sync(ili->spi, );
+}
+
+#define ili9341_spi_write(ili, cmd, data...) \
+({ \
+ u8 d[] = { data }; \
+ ili9341_spi_write_command(ili, cmd); \
+ if (ARRAY_SIZE(d) > 0) \
+
[RFC PATCH 2/2] dt-bindings: panel: Add Ilitek ILI9341 panel documentation
---
.../bindings/display/panel/ilitek,ili9341.txt | 33 +++
1 file changed, 33 insertions(+)
create mode 100644
Documentation/devicetree/bindings/display/panel/ilitek,ili9341.txt
diff --git a/Documentation/devicetree/bindings/display/panel/ilitek,ili9341.txt
b/Documentation/devicetree/bindings/display/panel/ilitek,ili9341.txt
new file mode 100644
index ..4e0e483bc12e
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/ilitek,ili9341.txt
@@ -0,0 +1,33 @@
+Ilitek ILI9341 TFT panel driver with SPI control bus
+
+This is a driver for 240x320 TFT panels with parallel RGB color input.
+
+Required properties:
+ - compatible: "displaytech,dt024ctft", "ilitek,ili9341"
+ - backlight: phandle of the backlight device attached to the panel
+
+Optional properties:
+ - dc-gpios: a GPIO spec for the Data/Command pin, see gpio/gpio.txt
+ - reset-gpios: a GPIO spec for the reset pin, see gpio/gpio.txt
+
+The panel must obey the rules for a SPI slave device as specified in
+spi/spi-bus.txt
+
+The device node can contain one 'port' child node with one child
+'endpoint' node, according to the bindings defined in
+media/video-interfaces.txt. This node should describe panel's video bus.
+
+Example:
+
+panel@0 {
+ compatible = "displaytech,dt024ctft", "ilitek,ili9341";
+ reg = <0>;
+ backlight = <>;
+ dc-gpios = < 4 9 GPIO_ACTIVE_HIGH>;
+
+ port {
+ panel_in: endpoint {
+ remote-endpoint = <_out>;
+ };
+ };
+};
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC PATCH 0/2] Add DRM panel driver for Ilitek ILI9341 based panels in parallel RGB mode
These patches add panel driver for ili9341-based panels in parallel RGB mode. The driver was developed for DispleyTech DT024CTFT LCD panel [1] which features ILI9341 chip [2]. The driver was tested on the Allwinner A13 (sun5i) platform. The driver supports 240x320 pixel resolution with 18-bit RGB (6 wires per color) and SPI control bus with Data/Command GPIO pin: DisplayTech DT024CTFT panel is configured with the IM[0:3] pins set to "1110" - see page 10 in datasheet [2]. The Data/Command GPIO is optional, however at the moment the driver requires it: The display itself is capable of 9-bit SPI without the Data/Command GPIO. Support for such configuration can be added later to the driver. Optional HW reset gpio can be specified, otherwise SW reset command is used during the initialization. The ILI9341 displays have two command sets: Level 1 conforms to MIPI specs Level 2 is outside the MIPI specs - custom defines are used in the driver The ILI9341 supports various RGB modes (e.g. NVSYNC, DE_LOW, clock freq, etc.), however values that are presented in the ILI9341 datasheet [2] are used by the driver in struct drm_display_mode. General note on ILI9341-based displays: The ILI9341 chip can be used in parallel RGB with SPI control or in SPI only mode where the image data itself is also transferred via SPI. This driver supports parallel RGB displays - it works with displays wired with 18-bit RGB input, it does not support SPI data mode (i.e. Multi-inno mi0283qt or Adafruit yx240qv29 are not supported by this driver). The SPI data mode is naturally much slower than the parallel RGB mode. General note on DisplayTech DT024CTFT panel: The panel supports different configuation options (18/16/6-bit RGB or 9/8-bit SPI) depending on the IM[0:3] wiring. To keep this patch small for reveiw, at the moment only 18-bit RGB mode and 8-bit SPI with Data/Command GPIO is supported by this driver. I kindly ask you for a patch review. Links to datasheet: [1] https://www.displaytech-us.com/sites/default/files/display-data-sheet/DT024CTFT-v10_0.pdf [2] https://cdn-shop.adafruit.com/datasheets/ILI9341.pdf Best regards, Josef Lusticky Josef Lusticky (2): drm/panel: Add Ilitek ILI9341 parallel RGB panel driver dt-bindings: panel: Add Ilitek ILI9341 panel documentation .../bindings/display/panel/ilitek,ili9341.txt | 33 ++ MAINTAINERS | 6 + drivers/gpu/drm/panel/Kconfig | 7 + drivers/gpu/drm/panel/Makefile| 1 + drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 320 ++ 5 files changed, 367 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/panel/ilitek,ili9341.txt create mode 100644 drivers/gpu/drm/panel/panel-ilitek-ili9341.c -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
