Re: [RFC PATCH] drm/virtio: Export resource handles via DMA-buf API
Hi Daniel, Gerd,
On Tue, Sep 17, 2019 at 10:23 PM Daniel Vetter wrote:
>
> On Thu, Sep 12, 2019 at 06:41:21PM +0900, Tomasz Figa wrote:
> > This patch is an early RFC to judge the direction we are following in
> > our virtualization efforts in Chrome OS. The purpose is to start a
> > discussion on how to handle buffer sharing between multiple virtio
> > devices.
> >
> > On a side note, we are also working on a virtio video decoder interface
> > and implementation, with a V4L2 driver for Linux. Those will be posted
> > for review in the near future as well.
> >
> > Any feedback will be appreciated! Thanks in advance.
> >
> > ===
> >
> > With the range of use cases for virtualization expanding, there is going
> > to be more virtio devices added to the ecosystem. Devices such as video
> > decoders, encoders, cameras, etc. typically work together with the
> > display and GPU in a pipeline manner, which can only be implemented
> > efficiently by sharing the buffers between producers and consumers.
> >
> > Existing buffer management framework in Linux, such as the videobuf2
> > framework in V4L2, implements all the DMA-buf handling inside generic
> > code and do not expose any low level information about the buffers to
> > the drivers.
> >
> > To seamlessly enable buffer sharing with drivers using such frameworks,
> > make the virtio-gpu driver expose the resource handle as the DMA address
> > of the buffer returned from the DMA-buf mapping operation. Arguably, the
> > resource handle is a kind of DMA address already, as it is the buffer
> > identifier that the device needs to access the backing memory, which is
> > exactly the same role a DMA address provides for native devices.
> >
> > A virtio driver that does memory management fully on its own would have
> > code similar to following. The code is identical to what a regular
> > driver for real hardware would do to import a DMA-buf.
> >
> > static int virtio_foo_get_resource_handle(struct virtio_foo *foo,
> > struct dma_buf *dma_buf, u32 *id)
> > {
> > struct dma_buf_attachment *attach;
> > struct sg_table *sgt;
> > int ret = 0;
> >
> > attach = dma_buf_attach(dma_buf, foo->dev);
> > if (IS_ERR(attach))
> > return PTR_ERR(attach);
> >
> > sgt = dma_buf_map_attachment(attach, DMA_BIDIRECTIONAL);
> > if (IS_ERR(sgt)) {
> > ret = PTR_ERR(sgt);
> > goto err_detach;
> > }
> >
> > if (sgt->nents != 1) {
> > ret = -EINVAL;
> > goto err_unmap;
> > }
> >
> > *id = sg_dma_address(sgt->sgl);
>
> I agree with Gerd, this looks pretty horrible to me.
>
> The usual way we've done these kind of special dma-bufs is:
>
> - They all get allocated at the same place, through some library or
> whatever.
>
> - You add a dma_buf_is_virtio(dma_buf) function, or maybe something that
> also upcasts or returns NULL, which checks for dma_buf->ops.
>
Thanks for a lot of valuable feedback and sorry for the late reply.
While I agree that stuffing the resource ID in sg_dma_address() is
quite ugly (for example, the regular address arithmetic doesn't work),
I still believe we need to convey information about these buffers
using regular kernel interfaces.
Drivers in some subsystems like DRM tend to open code any buffer
management and then it wouldn't be any problem to do what you
suggested. However, other subsystems have generic frameworks for
buffer management, like videobuf2 for V4L2. Those assume regular
DMA-bufs that can be handled with regular dma_buf_() API and described
using sgtables and/or pfn vectors and/or DMA addresses.
> - Once you've upcasted at runtime by checking for ->ops, you can add
> whatever fancy interfaces you want. Including a real&proper interface to
> get at whatever underlying id you need to for real buffer sharing
> between virtio devices.
>
> In a way virtio buffer/memory ids are a kind of private bus, entirely
> distinct from the dma_addr_t bus. So can't really stuff them under this
> same thing like we e.g. do with pci peer2peer.
As I mentioned earlier, there is no single "dma_addr_t bus". Each
device (as in struct device) can be on its own different DMA bus, with
a different DMA address space. There is not even a guarantee that a
DMA address obtained for one PCI device will be valid for another if
they are on different buses, which could have different address
mappings.
Putting that aside, we're thinking about a different approach, as Gerd
suggested in another thread, the one about the Virtio Video Decoder
protocol. I'm going to reply there, making sure to CC everyone
involved here.
Best regards,
Tomasz
> -Daniel
>
> >
> > err_unmap:
> > dma_buf_unmap_attachment(attach, sgt, DMA_BIDIRECTIONAL);
> > err_detach:
> > dma_buf_detach(dma_buf, attach);
> >
> > return ret;
> > }
> >
> > On the other hand, a virtio driver that uses an existing kernel
> > framew
Re: [PATCH v18 00/19] kunit: introduce KUnit, the Linux kernel unit testing framework
On Fri, Oct 4, 2019 at 5:49 PM shuah wrote: > > On 10/4/19 6:33 PM, Brendan Higgins wrote: > > On Fri, Oct 4, 2019 at 4:57 PM shuah wrote: > >> > >> On 10/4/19 5:52 PM, Brendan Higgins wrote: > >>> On Fri, Oct 4, 2019 at 4:30 PM Theodore Y. Ts'o wrote: > > On Fri, Oct 04, 2019 at 04:47:09PM -0600, shuah wrote: > >> However, if I encourage arbitrary tests/improvements into my KUnit > >> branch, it further diverges away from torvalds/master, and is more > >> likely that there will be a merge conflict or issue that is not related > >> to the core KUnit changes that will cause the whole thing to be > >> rejected again in v5.5. > > > > The idea is that the new development will happen based on kunit in > > linux-kselftest next. It will work just fine. As we accepts patches, > > they will go on top of kunit that is in linux-next now. > > I don't see how this would work. If I add unit tests to ext4, they > would be in fs/ext4. And to the extent that I need to add test mocks > to allow the unit tests to work, they will involve changes to existing > files in fs/ext4. I can't put them in the ext4.git tree without > pulling in the kunit changes into the ext4 git tree. And if they ext4 > unit tests land in the kunit tree, it would be a receipe for large > numbers of merge conflicts. > >>> > >>> That's where I was originally coming from. > >>> > >>> So here's a dumb idea: what if we merged KUnit through the ext4 tree? > >>> I imagine that could potentially get very confusing when we go back to > >>> sending changes in through the kselftest tree, but at least it means > >>> that ext4 can use it in the meantime, which means that it at least > >>> gets to be useful to one group of people. Also, since Ted seems pretty > >>> keen on using this, I imagine it is much more likely to produce real > >>> world, immediately useful tests not written by me (I'm not being lazy, > >>> I just think it is better to get other people's experiences other than > >>> my own). > >>> > >> > >> That doesn't make sense does it? The tests might not be limited to > >> fs/ext4. We might have other sub-systems that add tests. > > > > Well, I have some small isolated examples that I think would probably > > work no matter what, so we can get some usage outside of ext4. Also, > > if we want to limit the number of tests, then we don't expect to get > > much usage outside of ext4 before v5.5 anyway. I just figure, it's > > better to make it work for one person than no one, right? > > > > In any case, I admit it is not a great idea. I just thought it had > > some interesting advantages over going in through linux-kselftest that > > were worth exploring. > > > >> So, we will have to work to make linux-next as the base for new > >> development and limit the number of tests to where it will be > >> easier work in this mode for 5.5. We can stage the pull requests > >> so that kunit lands first followed by tests. > > > > So we are going to encourage maintainers to allow tests in their tree > > based on KUnit on the assumption that KUnit will get merged before > > there changes? That sounds like a huge burden, not just on us, but on > > other maintainers and users. > > > > I think if we are going to allow tests before KUnit is merged, we > > should have the tests come in through the same tree as KUnit. > > > >> We have a similar situation with kselftest as well. Sub-systems > >> send tests that depend on their tress separately. > > > > Well it is different if the maintainer wants to send the test in > > through their tree, right? Otherwise, it won't matter what the state > > of linux-next is and it won't matter when linux-kselftest gets merged. > > Or am I not understanding you? > > > > Let's talk about current state. Right now kunit is in linux-next and > we want to add a few more tests. We will have to coordinate the effort. > Once kunit get into mainline, then the need for this coordination goes > down. Sure, I was just thinking that getting other people to write the tests would be better. Since not only is it then useful to someone else, it provides the best possible exercise of KUnit. Nevertheless, it would probably just be easier to get a handful of example tests, and it is less likely to result in any issues for v5.5, so that's probably better. (I think that is what you are hinting at here. ;-) ) Hey Ted, do you know if that ext4 timestamp test can go in through linux-kselftest? It seemed fairly self-contained. Or is that what you were saying wouldn't work for you? > Let's focus on the next few weeks first so we can get this into mainline > in 5.5. I agree. That is the most important thing. > The two of us can chat next week and come up with a plan. Sure. Cheers!
Re: [PATCH v18 00/19] kunit: introduce KUnit, the Linux kernel unit testing framework
On 10/4/19 6:33 PM, Brendan Higgins wrote: On Fri, Oct 4, 2019 at 4:57 PM shuah wrote: On 10/4/19 5:52 PM, Brendan Higgins wrote: On Fri, Oct 4, 2019 at 4:30 PM Theodore Y. Ts'o wrote: On Fri, Oct 04, 2019 at 04:47:09PM -0600, shuah wrote: However, if I encourage arbitrary tests/improvements into my KUnit branch, it further diverges away from torvalds/master, and is more likely that there will be a merge conflict or issue that is not related to the core KUnit changes that will cause the whole thing to be rejected again in v5.5. The idea is that the new development will happen based on kunit in linux-kselftest next. It will work just fine. As we accepts patches, they will go on top of kunit that is in linux-next now. I don't see how this would work. If I add unit tests to ext4, they would be in fs/ext4. And to the extent that I need to add test mocks to allow the unit tests to work, they will involve changes to existing files in fs/ext4. I can't put them in the ext4.git tree without pulling in the kunit changes into the ext4 git tree. And if they ext4 unit tests land in the kunit tree, it would be a receipe for large numbers of merge conflicts. That's where I was originally coming from. So here's a dumb idea: what if we merged KUnit through the ext4 tree? I imagine that could potentially get very confusing when we go back to sending changes in through the kselftest tree, but at least it means that ext4 can use it in the meantime, which means that it at least gets to be useful to one group of people. Also, since Ted seems pretty keen on using this, I imagine it is much more likely to produce real world, immediately useful tests not written by me (I'm not being lazy, I just think it is better to get other people's experiences other than my own). That doesn't make sense does it? The tests might not be limited to fs/ext4. We might have other sub-systems that add tests. Well, I have some small isolated examples that I think would probably work no matter what, so we can get some usage outside of ext4. Also, if we want to limit the number of tests, then we don't expect to get much usage outside of ext4 before v5.5 anyway. I just figure, it's better to make it work for one person than no one, right? In any case, I admit it is not a great idea. I just thought it had some interesting advantages over going in through linux-kselftest that were worth exploring. So, we will have to work to make linux-next as the base for new development and limit the number of tests to where it will be easier work in this mode for 5.5. We can stage the pull requests so that kunit lands first followed by tests. So we are going to encourage maintainers to allow tests in their tree based on KUnit on the assumption that KUnit will get merged before there changes? That sounds like a huge burden, not just on us, but on other maintainers and users. I think if we are going to allow tests before KUnit is merged, we should have the tests come in through the same tree as KUnit. We have a similar situation with kselftest as well. Sub-systems send tests that depend on their tress separately. Well it is different if the maintainer wants to send the test in through their tree, right? Otherwise, it won't matter what the state of linux-next is and it won't matter when linux-kselftest gets merged. Or am I not understanding you? Let's talk about current state. Right now kunit is in linux-next and we want to add a few more tests. We will have to coordinate the effort. Once kunit get into mainline, then the need for this coordination goes down. Let's focus on the next few weeks first so we can get this into mainline in 5.5. The two of us can chat next week and come up with a plan. thanks, -- Shuah ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v18 00/19] kunit: introduce KUnit, the Linux kernel unit testing framework
On Fri, Oct 4, 2019 at 4:57 PM shuah wrote: > > On 10/4/19 5:52 PM, Brendan Higgins wrote: > > On Fri, Oct 4, 2019 at 4:30 PM Theodore Y. Ts'o wrote: > >> > >> On Fri, Oct 04, 2019 at 04:47:09PM -0600, shuah wrote: > However, if I encourage arbitrary tests/improvements into my KUnit > branch, it further diverges away from torvalds/master, and is more > likely that there will be a merge conflict or issue that is not related > to the core KUnit changes that will cause the whole thing to be > rejected again in v5.5. > >>> > >>> The idea is that the new development will happen based on kunit in > >>> linux-kselftest next. It will work just fine. As we accepts patches, > >>> they will go on top of kunit that is in linux-next now. > >> > >> I don't see how this would work. If I add unit tests to ext4, they > >> would be in fs/ext4. And to the extent that I need to add test mocks > >> to allow the unit tests to work, they will involve changes to existing > >> files in fs/ext4. I can't put them in the ext4.git tree without > >> pulling in the kunit changes into the ext4 git tree. And if they ext4 > >> unit tests land in the kunit tree, it would be a receipe for large > >> numbers of merge conflicts. > > > > That's where I was originally coming from. > > > > So here's a dumb idea: what if we merged KUnit through the ext4 tree? > > I imagine that could potentially get very confusing when we go back to > > sending changes in through the kselftest tree, but at least it means > > that ext4 can use it in the meantime, which means that it at least > > gets to be useful to one group of people. Also, since Ted seems pretty > > keen on using this, I imagine it is much more likely to produce real > > world, immediately useful tests not written by me (I'm not being lazy, > > I just think it is better to get other people's experiences other than > > my own). > > > > That doesn't make sense does it? The tests might not be limited to > fs/ext4. We might have other sub-systems that add tests. Well, I have some small isolated examples that I think would probably work no matter what, so we can get some usage outside of ext4. Also, if we want to limit the number of tests, then we don't expect to get much usage outside of ext4 before v5.5 anyway. I just figure, it's better to make it work for one person than no one, right? In any case, I admit it is not a great idea. I just thought it had some interesting advantages over going in through linux-kselftest that were worth exploring. > So, we will have to work to make linux-next as the base for new > development and limit the number of tests to where it will be > easier work in this mode for 5.5. We can stage the pull requests > so that kunit lands first followed by tests. So we are going to encourage maintainers to allow tests in their tree based on KUnit on the assumption that KUnit will get merged before there changes? That sounds like a huge burden, not just on us, but on other maintainers and users. I think if we are going to allow tests before KUnit is merged, we should have the tests come in through the same tree as KUnit. > We have a similar situation with kselftest as well. Sub-systems > send tests that depend on their tress separately. Well it is different if the maintainer wants to send the test in through their tree, right? Otherwise, it won't matter what the state of linux-next is and it won't matter when linux-kselftest gets merged. Or am I not understanding you? ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 204241] amdgpu fails to resume from suspend
https://bugzilla.kernel.org/show_bug.cgi?id=204241 a...@tutanota.com changed: What|Removed |Added CC||a...@tutanota.com --- Comment #14 from a...@tutanota.com --- Created attachment 285349 --> https://bugzilla.kernel.org/attachment.cgi?id=285349&action=edit Patch to prevent frequent resume failures While this issue happens rather randomly, it can be quite reliably reproduced on linux 5.2 and later by performing successive suspend-resume cycles. Usually the error occurs after less than 10 cycles, but occasionally only after more than 20. Thus one can use the following command to reproduce it almost certainly: $ for i in $(seq 30); do sudo rtcwake -m mem -s 5; sleep 15; done A bisection using this method lead to: commit 533aed278afeaa68bb5d0600856ab02268cfa3b8 Author: Andrey Grodzovsky Date: Wed Mar 6 16:16:28 2019 -0500 drm/amdgpu: Move IB pool init and fini v2 Problem: Using SDMA for TLB invalidation in certain ASICs exposed a problem of IB pool not being ready while SDMA already up on Init and already shutt down while SDMA still running on Fini. This caused IB allocation failure. Temproary fix was commited into a bringup branch but this is the generic fix. Fix: Init IB pool rigth after GMC is ready but before SDMA is ready. Do th opposite for Fini. v2: Remove restriction on SDMA early init and move amdgpu_ib_pool_fini Reviewed-by: Christian König Signed-off-by: Andrey Grodzovsky Signed-off-by: Alex Deucher Reverting this commit makes the problem unreproducible with above command. Another way to prevent these frequent resume failures, while preserving the intention of this commit, is to simply call amdgpu_ib_pool_init directly after calling amdgpu_ucode_create_bo instead of directly before that. Attached is a patch doing it that way. -- 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 v18 00/19] kunit: introduce KUnit, the Linux kernel unit testing framework
On 10/4/19 5:52 PM, Brendan Higgins wrote: On Fri, Oct 4, 2019 at 4:30 PM Theodore Y. Ts'o wrote: On Fri, Oct 04, 2019 at 04:47:09PM -0600, shuah wrote: However, if I encourage arbitrary tests/improvements into my KUnit branch, it further diverges away from torvalds/master, and is more likely that there will be a merge conflict or issue that is not related to the core KUnit changes that will cause the whole thing to be rejected again in v5.5. The idea is that the new development will happen based on kunit in linux-kselftest next. It will work just fine. As we accepts patches, they will go on top of kunit that is in linux-next now. I don't see how this would work. If I add unit tests to ext4, they would be in fs/ext4. And to the extent that I need to add test mocks to allow the unit tests to work, they will involve changes to existing files in fs/ext4. I can't put them in the ext4.git tree without pulling in the kunit changes into the ext4 git tree. And if they ext4 unit tests land in the kunit tree, it would be a receipe for large numbers of merge conflicts. That's where I was originally coming from. So here's a dumb idea: what if we merged KUnit through the ext4 tree? I imagine that could potentially get very confusing when we go back to sending changes in through the kselftest tree, but at least it means that ext4 can use it in the meantime, which means that it at least gets to be useful to one group of people. Also, since Ted seems pretty keen on using this, I imagine it is much more likely to produce real world, immediately useful tests not written by me (I'm not being lazy, I just think it is better to get other people's experiences other than my own). That doesn't make sense does it? The tests might not be limited to fs/ext4. We might have other sub-systems that add tests. So, we will have to work to make linux-next as the base for new development and limit the number of tests to where it will be easier work in this mode for 5.5. We can stage the pull requests so that kunit lands first followed by tests. We have a similar situation with kselftest as well. Sub-systems send tests that depend on their tress separately. thanks, -- Shuah
Re: [PATCH v18 00/19] kunit: introduce KUnit, the Linux kernel unit testing framework
On Fri, Oct 4, 2019 at 4:30 PM Theodore Y. Ts'o wrote: > > On Fri, Oct 04, 2019 at 04:47:09PM -0600, shuah wrote: > > > However, if I encourage arbitrary tests/improvements into my KUnit > > > branch, it further diverges away from torvalds/master, and is more > > > likely that there will be a merge conflict or issue that is not related > > > to the core KUnit changes that will cause the whole thing to be > > > rejected again in v5.5. > > > > The idea is that the new development will happen based on kunit in > > linux-kselftest next. It will work just fine. As we accepts patches, > > they will go on top of kunit that is in linux-next now. > > I don't see how this would work. If I add unit tests to ext4, they > would be in fs/ext4. And to the extent that I need to add test mocks > to allow the unit tests to work, they will involve changes to existing > files in fs/ext4. I can't put them in the ext4.git tree without > pulling in the kunit changes into the ext4 git tree. And if they ext4 > unit tests land in the kunit tree, it would be a receipe for large > numbers of merge conflicts. That's where I was originally coming from. So here's a dumb idea: what if we merged KUnit through the ext4 tree? I imagine that could potentially get very confusing when we go back to sending changes in through the kselftest tree, but at least it means that ext4 can use it in the meantime, which means that it at least gets to be useful to one group of people. Also, since Ted seems pretty keen on using this, I imagine it is much more likely to produce real world, immediately useful tests not written by me (I'm not being lazy, I just think it is better to get other people's experiences other than my own). Thoughts? ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v18 00/19] kunit: introduce KUnit, the Linux kernel unit testing framework
On Fri, Oct 04, 2019 at 04:47:09PM -0600, shuah wrote: > > However, if I encourage arbitrary tests/improvements into my KUnit > > branch, it further diverges away from torvalds/master, and is more > > likely that there will be a merge conflict or issue that is not related > > to the core KUnit changes that will cause the whole thing to be > > rejected again in v5.5. > > The idea is that the new development will happen based on kunit in > linux-kselftest next. It will work just fine. As we accepts patches, > they will go on top of kunit that is in linux-next now. I don't see how this would work. If I add unit tests to ext4, they would be in fs/ext4. And to the extent that I need to add test mocks to allow the unit tests to work, they will involve changes to existing files in fs/ext4. I can't put them in the ext4.git tree without pulling in the kunit changes into the ext4 git tree. And if they ext4 unit tests land in the kunit tree, it would be a receipe for large numbers of merge conflicts. - Ted ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v18 00/19] kunit: introduce KUnit, the Linux kernel unit testing framework
On 10/4/19 5:10 PM, Brendan Higgins wrote: On Fri, Oct 4, 2019 at 3:47 PM shuah wrote: On 10/4/19 4:27 PM, Brendan Higgins wrote: On Fri, Oct 04, 2019 at 03:59:10PM -0600, shuah wrote: On 10/4/19 3:42 PM, Linus Torvalds wrote: On Fri, Oct 4, 2019 at 2:39 PM Theodore Y. Ts'o wrote: This question is primarily directed at Shuah and Linus What's the current status of the kunit series now that Brendan has moved it out of the top-level kunit directory as Linus has requested? The move happened smack in the middle of merge window and landed in linux-next towards the end of the merge window. We seemed to decide to just wait for 5.5, but there is nothing that looks to block that. And I encouraged Shuah to find more kunit cases for when it _does_ get merged. Right. I communicated that to Brendan that we could work on adding more kunit based tests which would help get more mileage on the kunit. So if the kunit branch is stable, and people want to start using it for their unit tests, then I think that would be a good idea, and then during the 5.5 merge window we'll not just get the infrastructure, we'll get a few more users too and not just examples. I was planning on holding off on accepting more tests/changes until KUnit is in torvalds/master. As much as I would like to go around promoting it, I don't really want to promote too much complexity in a non-upstream branch before getting it upstream because I don't want to risk adding something that might cause it to get rejected again. To be clear, I can understand from your perspective why getting more tests/usage before accepting it is a good thing. The more people that play around with it, the more likely that someone will find an issue with it, and more likely that what is accepted into torvalds/master is of high quality. However, if I encourage arbitrary tests/improvements into my KUnit branch, it further diverges away from torvalds/master, and is more likely that there will be a merge conflict or issue that is not related to the core KUnit changes that will cause the whole thing to be rejected again in v5.5. The idea is that the new development will happen based on kunit in linux-kselftest next. It will work just fine. As we accepts patches, they will go on top of kunit that is in linux-next now. But then wouldn't we want to limit what KUnit changes are going into linux-kselftest next for v5.5? For example, we probably don't want to do anymore feature development on it until it is in v5.5, since the goal is to make it more stable, right? I am guessing that it will probably be fine, but it still sounds like we need to establish some ground rules, and play it *very* safe. How about we identify a small number tests that can add value and focus on them. I am thinking a number between 2 and 5. This way we get a feel for the API, if it changes for the better great, if it doesn't have to, then you know you already did a great job. Does that sound reasonable to you? thanks, -- Shuah ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v18 00/19] kunit: introduce KUnit, the Linux kernel unit testing framework
On Fri, Oct 4, 2019 at 3:47 PM shuah wrote: > > On 10/4/19 4:27 PM, Brendan Higgins wrote: > > On Fri, Oct 04, 2019 at 03:59:10PM -0600, shuah wrote: > >> On 10/4/19 3:42 PM, Linus Torvalds wrote: > >>> On Fri, Oct 4, 2019 at 2:39 PM Theodore Y. Ts'o wrote: > > This question is primarily directed at Shuah and Linus > > What's the current status of the kunit series now that Brendan has > moved it out of the top-level kunit directory as Linus has requested? > >>> > >> > >> The move happened smack in the middle of merge window and landed in > >> linux-next towards the end of the merge window. > >> > >>> We seemed to decide to just wait for 5.5, but there is nothing that > >>> looks to block that. And I encouraged Shuah to find more kunit cases > >>> for when it _does_ get merged. > >>> > >> > >> Right. I communicated that to Brendan that we could work on adding more > >> kunit based tests which would help get more mileage on the kunit. > >> > >>> So if the kunit branch is stable, and people want to start using it > >>> for their unit tests, then I think that would be a good idea, and then > >>> during the 5.5 merge window we'll not just get the infrastructure, > >>> we'll get a few more users too and not just examples. > > > > I was planning on holding off on accepting more tests/changes until > > KUnit is in torvalds/master. As much as I would like to go around > > promoting it, I don't really want to promote too much complexity in a > > non-upstream branch before getting it upstream because I don't want to > > risk adding something that might cause it to get rejected again. > > > > To be clear, I can understand from your perspective why getting more > > tests/usage before accepting it is a good thing. The more people that > > play around with it, the more likely that someone will find an issue > > with it, and more likely that what is accepted into torvalds/master is > > of high quality. > > > > However, if I encourage arbitrary tests/improvements into my KUnit > > branch, it further diverges away from torvalds/master, and is more > > likely that there will be a merge conflict or issue that is not related > > to the core KUnit changes that will cause the whole thing to be > > rejected again in v5.5. > > > > The idea is that the new development will happen based on kunit in > linux-kselftest next. It will work just fine. As we accepts patches, > they will go on top of kunit that is in linux-next now. But then wouldn't we want to limit what KUnit changes are going into linux-kselftest next for v5.5? For example, we probably don't want to do anymore feature development on it until it is in v5.5, since the goal is to make it more stable, right? I am guessing that it will probably be fine, but it still sounds like we need to establish some ground rules, and play it *very* safe. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v18 00/19] kunit: introduce KUnit, the Linux kernel unit testing framework
On 10/4/19 4:27 PM, Brendan Higgins wrote: On Fri, Oct 04, 2019 at 03:59:10PM -0600, shuah wrote: On 10/4/19 3:42 PM, Linus Torvalds wrote: On Fri, Oct 4, 2019 at 2:39 PM Theodore Y. Ts'o wrote: This question is primarily directed at Shuah and Linus What's the current status of the kunit series now that Brendan has moved it out of the top-level kunit directory as Linus has requested? The move happened smack in the middle of merge window and landed in linux-next towards the end of the merge window. We seemed to decide to just wait for 5.5, but there is nothing that looks to block that. And I encouraged Shuah to find more kunit cases for when it _does_ get merged. Right. I communicated that to Brendan that we could work on adding more kunit based tests which would help get more mileage on the kunit. So if the kunit branch is stable, and people want to start using it for their unit tests, then I think that would be a good idea, and then during the 5.5 merge window we'll not just get the infrastructure, we'll get a few more users too and not just examples. I was planning on holding off on accepting more tests/changes until KUnit is in torvalds/master. As much as I would like to go around promoting it, I don't really want to promote too much complexity in a non-upstream branch before getting it upstream because I don't want to risk adding something that might cause it to get rejected again. To be clear, I can understand from your perspective why getting more tests/usage before accepting it is a good thing. The more people that play around with it, the more likely that someone will find an issue with it, and more likely that what is accepted into torvalds/master is of high quality. However, if I encourage arbitrary tests/improvements into my KUnit branch, it further diverges away from torvalds/master, and is more likely that there will be a merge conflict or issue that is not related to the core KUnit changes that will cause the whole thing to be rejected again in v5.5. The idea is that the new development will happen based on kunit in linux-kselftest next. It will work just fine. As we accepts patches, they will go on top of kunit that is in linux-next now. thanks, -- Shuah
Re: [PATCH v18 00/19] kunit: introduce KUnit, the Linux kernel unit testing framework
On Fri, Oct 04, 2019 at 03:59:10PM -0600, shuah wrote: > On 10/4/19 3:42 PM, Linus Torvalds wrote: > > On Fri, Oct 4, 2019 at 2:39 PM Theodore Y. Ts'o wrote: > > > > > > This question is primarily directed at Shuah and Linus > > > > > > What's the current status of the kunit series now that Brendan has > > > moved it out of the top-level kunit directory as Linus has requested? > > > > The move happened smack in the middle of merge window and landed in > linux-next towards the end of the merge window. > > > We seemed to decide to just wait for 5.5, but there is nothing that > > looks to block that. And I encouraged Shuah to find more kunit cases > > for when it _does_ get merged. > > > > Right. I communicated that to Brendan that we could work on adding more > kunit based tests which would help get more mileage on the kunit. > > > So if the kunit branch is stable, and people want to start using it > > for their unit tests, then I think that would be a good idea, and then > > during the 5.5 merge window we'll not just get the infrastructure, > > we'll get a few more users too and not just examples. I was planning on holding off on accepting more tests/changes until KUnit is in torvalds/master. As much as I would like to go around promoting it, I don't really want to promote too much complexity in a non-upstream branch before getting it upstream because I don't want to risk adding something that might cause it to get rejected again. To be clear, I can understand from your perspective why getting more tests/usage before accepting it is a good thing. The more people that play around with it, the more likely that someone will find an issue with it, and more likely that what is accepted into torvalds/master is of high quality. However, if I encourage arbitrary tests/improvements into my KUnit branch, it further diverges away from torvalds/master, and is more likely that there will be a merge conflict or issue that is not related to the core KUnit changes that will cause the whole thing to be rejected again in v5.5. I don't know. I guess we could maybe address that situation by splitting up the pull request into features and tests when we go to send it in, but that seems to invite a lot of unnecessary complexity. I actually already had some other tests/changes ready to send for review, but was holding off until the initial set of patches mad it in. Looking forward to hearing other people's thoughts.
Re: [PATCH 1/2] drm/mcde: Fix reference to DOC comment
On Thu, Oct 3, 2019 at 5:15 PM Maxime Ripard wrote:
> > > > Fixes: 5fc537bfd000 ("drm/mcde: Add new driver for ST-Ericsson MCDE")
> > > > Signed-off-by: Jonathan Neuschäfer
> > >
> > > Both patches applied!
> >
> > ...but I can't push the changes:
> >
> > $ dim push-branch drm-misc-next
> > dim: 9fa1f9734e40 ("Revert "drm/sun4i: dsi: Change the start delay
> > calculation""): committer Signed-off-by missing.
> > dim: ERROR: issues in commits detected, aborting
> >
> > Not even my commit, apart from that it looks like it does have
> > the committer Signed-off-by. I'm confused and don't know what
> > to do... anyone has some hints?
>
> Yeah, it's pretty weird, I just pushed without any trouble.
>
> Did you rebase or something?
Nope... even tried to reset hard to origin :/
I guess just try again...
Yours,
Linus Walleij
Re: [PATCH v18 00/19] kunit: introduce KUnit, the Linux kernel unit testing framework
On 10/4/19 3:42 PM, Linus Torvalds wrote: On Fri, Oct 4, 2019 at 2:39 PM Theodore Y. Ts'o wrote: This question is primarily directed at Shuah and Linus What's the current status of the kunit series now that Brendan has moved it out of the top-level kunit directory as Linus has requested? The move happened smack in the middle of merge window and landed in linux-next towards the end of the merge window. We seemed to decide to just wait for 5.5, but there is nothing that looks to block that. And I encouraged Shuah to find more kunit cases for when it _does_ get merged. Right. I communicated that to Brendan that we could work on adding more kunit based tests which would help get more mileage on the kunit. So if the kunit branch is stable, and people want to start using it for their unit tests, then I think that would be a good idea, and then during the 5.5 merge window we'll not just get the infrastructure, we'll get a few more users too and not just examples. thanks, -- Shuah ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH][next] drm/komeda: remove redundant assignment to pointer disable_done
On 04/10/2019 20:27, Liviu Dudau wrote:
> On Fri, Oct 04, 2019 at 05:21:56PM +0100, Colin King wrote:
>> From: Colin Ian King
>>
>> The pointer disable_done is being initialized with a value that
>> is never read and is being re-assigned a little later on. The
>> assignment is redundant and hence can be removed.
>
> Not really true, isn't it? The re-assignment is done under the condition that
> crtc->state->active is true. disable_done will be used regardless after the if
> block, so we can't skip this initialisation.
>
> Not sure why Coverity flags this, but I would NAK this patch.
I'm patching against the driver from linux-next so I believe this is OK
for that. I believe your statement is true against linux which does not
have commit:
d6cb013579e743bc7bc5590ca35a1943f2b8f3c8
Author: Lowry Li (Arm Technology China)
Date: Fri Sep 6 07:18:06 2019 +
Colin.
>
> Best regards,
> Liviu
>
>>
>> Addresses-Coverity: ("Unused value")
>> Signed-off-by: Colin Ian King
>> ---
>> drivers/gpu/drm/arm/display/komeda/komeda_crtc.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
>> b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
>> index 75263d8cd0bd..9beeda04818b 100644
>> --- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
>> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
>> @@ -296,7 +296,7 @@ komeda_crtc_atomic_disable(struct drm_crtc *crtc,
>> struct komeda_crtc_state *old_st = to_kcrtc_st(old);
>> struct komeda_pipeline *master = kcrtc->master;
>> struct komeda_pipeline *slave = kcrtc->slave;
>> -struct completion *disable_done = &crtc->state->commit->flip_done;
>> +struct completion *disable_done;
>> bool needs_phase2 = false;
>>
>> DRM_DEBUG_ATOMIC("CRTC%d_DISABLE: active_pipes: 0x%x, affected: 0x%x\n",
>> --
>> 2.20.1
>>
>
Re: [PATCH v4 1/7] backlight: gpio: remove unneeded include
On Tue, Oct 1, 2019 at 2:59 PM Bartosz Golaszewski wrote: > From: Bartosz Golaszewski > > We no longer use any symbols from of_gpio.h. Remove this include. > > Signed-off-by: Bartosz Golaszewski Reviewed-by: Linus Walleij Thanks Bartosz, Linus Walleij ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 00/11] Add support for software nodes to gpiolib
On Tue, Oct 1, 2019 at 12:45 AM Dmitry Torokhov wrote: > So I guess we missed -rc1. Any chance we could get an immutable branch > off -rc1 that you will pull into your main branch and I hopefully can > persuade other maintainers to pull as well so we do not need to drag it > over 2+ merge windows? Yes I'm sorry. I was swamped with stabilizing the kernel. I made an immutable branch and tried to use zeroday for testing but it timed out so I folded it in for-next anyways after som basic tests. Yours, Linus Walleij
Re: [PATCH v18 00/19] kunit: introduce KUnit, the Linux kernel unit testing framework
On Fri, Oct 4, 2019 at 2:39 PM Theodore Y. Ts'o wrote: > > This question is primarily directed at Shuah and Linus > > What's the current status of the kunit series now that Brendan has > moved it out of the top-level kunit directory as Linus has requested? We seemed to decide to just wait for 5.5, but there is nothing that looks to block that. And I encouraged Shuah to find more kunit cases for when it _does_ get merged. So if the kunit branch is stable, and people want to start using it for their unit tests, then I think that would be a good idea, and then during the 5.5 merge window we'll not just get the infrastructure, we'll get a few more users too and not just examples. Linus ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 111481] AMD Navi GPU frequent freezes on both Manjaro/Ubuntu with kernel 5.3 and mesa 19.2 -git/llvm9
https://bugs.freedesktop.org/show_bug.cgi?id=111481 --- Comment #72 from Marko Popovic --- (In reply to Shmerl from comment #71) > (In reply to Marko Popovic from comment #70) > > > > Yes, I can confirm that with 5.4 RC1 and MESA-git from 04.10. (with radv > > patches included) I can reproduce all 4 types of hangs, random desktop hang, > > Rise of the Tomb Raider Hang, Starcraft II hang and even Citra hang > > (eventhough those patches supposedly fix the ngg) so that's a huge bummer. > > Just to clarify, those fixes were added post rc1 tag, so you'd need to build > the master branch of Linus's repo (it would produce 5.4-rc1+). Sorry I wrote poorly, I'm using 5.4 daily build. These hangs on Navi seem to be quite a hard nut to crack for AMD it seems, they are trying with different types of patches from amdgpu, firmware, kernel and even mesa, and yet nothing ever changes :( Maybe this issue should get a high priority at least considering that hangs basically render desktop unusable for many things, quite a few dxvk games produce hangs even with nodma and nongg applied, so no idea what could be going on there. Why do those flags work for some things and not for the others... -- 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 v18 00/19] kunit: introduce KUnit, the Linux kernel unit testing framework
On Mon, Sep 23, 2019 at 02:02:30AM -0700, Brendan Higgins wrote: > ## TL;DR > > This revision addresses comments from Linus[1] and Randy[2], by moving > top level `kunit/` directory to `lib/kunit/` and likewise moves top > level Kconfig entry under lib/Kconfig.debug, so the KUnit submenu now > shows up under the "Kernel Hacking" menu. This question is primarily directed at Shuah and Linus What's the current status of the kunit series now that Brendan has moved it out of the top-level kunit directory as Linus has requested? There doesn't appear to have been many comments or changes since since September 23rd, and I was very much hoping they could land before -rc2, since I've been hoping to add unit tests for ext4. Is kunit likely to be able to be landed in Linus's tree during this development cycle? Many thanks! - Ted
[Bug 111481] AMD Navi GPU frequent freezes on both Manjaro/Ubuntu with kernel 5.3 and mesa 19.2 -git/llvm9
https://bugs.freedesktop.org/show_bug.cgi?id=111481 --- Comment #71 from Shmerl --- (In reply to Marko Popovic from comment #70) > > Yes, I can confirm that with 5.4 RC1 and MESA-git from 04.10. (with radv > patches included) I can reproduce all 4 types of hangs, random desktop hang, > Rise of the Tomb Raider Hang, Starcraft II hang and even Citra hang > (eventhough those patches supposedly fix the ngg) so that's a huge bummer. Just to clarify, those fixes were added post rc1 tag, so you'd need to build the master branch of Linus's repo (it would produce 5.4-rc1+). -- 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 111481] AMD Navi GPU frequent freezes on both Manjaro/Ubuntu with kernel 5.3 and mesa 19.2 -git/llvm9
https://bugs.freedesktop.org/show_bug.cgi?id=111481 --- Comment #70 from Marko Popovic --- (In reply to Shmerl from comment #69) > I just tried recent kernel 5.4-rc1+ from here: > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git > > It supposedly already has fixes for amdgpu metrics, in this commit: > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/ > ?id=0f83eb690d77f76d27d803488c1047fc9a10 > > However I just experienced another hang when adding amdgpu fans sensors to > ksysguard, so apparently it didn't fix the problem. > > And there was also a hang with Firefox, when it was started without > AMD_DEBUG=nodma, so that issue is not fixed either yet. > > cc Alex Deucher. Yes, I can confirm that with 5.4 RC1 and MESA-git from 04.10. (with radv patches included) I can reproduce all 4 types of hangs, random desktop hang, Rise of the Tomb Raider Hang, Starcraft II hang and even Citra hang (eventhough those patches supposedly fix the ngg) so that's a huge bummer. -- 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 111481] AMD Navi GPU frequent freezes on both Manjaro/Ubuntu with kernel 5.3 and mesa 19.2 -git/llvm9
https://bugs.freedesktop.org/show_bug.cgi?id=111481 --- Comment #69 from Shmerl --- I just tried recent kernel 5.4-rc1+ from here: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git It supposedly already has fixes for amdgpu metrics, in this commit: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0f83eb690d77f76d27d803488c1047fc9a10 However I just experienced another hang when adding amdgpu fans sensors to ksysguard, so apparently it didn't fix the problem. And there was also a hang with Firefox, when it was started without AMD_DEBUG=nodma, so that issue is not fixed either yet. cc Alex Deucher. -- 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 v8 1/5] leds: populate the device's of_node when possible
Hi Jean,
On 10/3/19 10:28 AM, Jean-Jacques Hiblot wrote:
> If initialization data is available and its fwnode is actually a of_node,
> store this information in the led device's structure. This will allow the
> device to use or provide OF-based API such (devm_xxx).
>
> Signed-off-by: Jean-Jacques Hiblot
> ---
> drivers/leds/led-class.c | 5 -
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
> index 647b1263c579..c2167b66b61f 100644
> --- a/drivers/leds/led-class.c
> +++ b/drivers/leds/led-class.c
> @@ -276,8 +276,11 @@ int led_classdev_register_ext(struct device *parent,
> mutex_unlock(&led_cdev->led_access);
> return PTR_ERR(led_cdev->dev);
> }
> - if (init_data && init_data->fwnode)
> + if (init_data && init_data->fwnode) {
> led_cdev->dev->fwnode = init_data->fwnode;
> + if (is_of_node(init_data->fwnode))
This check is made inside to_of_node() macro so here we
need only below assignment.
> + led_cdev->dev->of_node = to_of_node(init_data->fwnode);
> + }
>
> if (ret)
> dev_warn(parent, "Led %s renamed to %s due to name collision",
>
--
Best regards,
Jacek Anaszewski
RE: [Outreachy][VKMS] Apply to VKMS
Hi Lorrany, Its great to hear that you are interested in VKMS project. Is this your first time in the DRM subsystem or the Linux kernel? First of all, welcome! In order to improve your experience, we selected a set of tutorials that may help you to get into the DRM subsystem: 1. This project is about VKMS and IGT, which means that you don't need real hardware for work in this project. You need to work with a virtual machine; in this sense, we recommend the following tutorial for preparing your work environment: https://flusp.ime.usp.br/others/2019/02/15/use-qemu-to-play-with-linux/ 2. If you had success in setup your VM, you could start the most exciting part: compile and install a kernel. Take a look at this tutorial: https://flusp.ime.usp.br/others/2019/02/16/Kernel-compilation-and-installation/ 3. Get the IGT tools and compile and run some tests on VKMS: https://gitlab.freedesktop.org/drm/igt-gpu-tools 4. Now, it is time to work with a single module. Take a look at this tutorial: https://flusp.ime.usp.br/others/2019/04/07/play_with_modules/ 5. Finally, prepare yourself to send a patch. Take a look at: https://kernelnewbies.org/FirstKernelPatch Please let me or Rodrigo Siqueira know if you have any questions. Regards Manasi Navare (mdnavare@Freenode) Hi guys, I'm Lorrany Azevedo, and I'm participating in the application phase for the outreachy internships. I'm interested in making a contribution to VKMS, and I would like to ask for tips on how I can do this, I'm new to the community and I never made any contribution to the FOSS. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Should regulator core support parsing OF based fwnode?
On 10/4/19 7:42 PM, Mark Brown wrote: > On Fri, Oct 04, 2019 at 06:12:52PM +0200, Jean-Jacques Hiblot wrote: >> On 04/10/2019 17:58, Mark Brown wrote: > >>> Regulator supplies are supposed to be defined at the chip level rather >>> than subfunctions with names corresponding to the names on the chip. > > ... > >>> good chance that they come up with the same mapping. The supply_alias >>> interface is there to allow mapping these through to subfunctions if >>> needed, it looks like the LED framework should be using this. > >> In case of current-sink LED drivers, each LED can be powered by a different >> regulator, because the driver is only a switch between the LED cathod and >> the ground. > > Sure, it's common for devices to have supplies that are only needed by > one part of the chip which is why we have the supply_alias interface for > mapping things through. > >>> That said if you are doing the above and the LEDs are appearing as >>> devices it's extremely surprising that their of_node might not be >>> initialized. > >> That is because this is usually done by the platform core which is not >> involved here. > > The surprise is more that it got instantiated from the DT without > keeping the node around than how it happened. This is LED class driver that is instantiated from DT and it in turn registers LED class devices - one per corresponding DT child node found in the parent LED controller node. LED class device is created via device_create_with_groups() that returns struct device with uninitialized of_node. This is the point we're discussing. In order to be able to obtain regulator handle in the LED core from DT we need to have exactly of_node (of course provided it contains *-supply property). Here the question may arise why it is not the LED core that instantiates LED class devices per child nodes - well it is tricky to implement due to disparate possible LED channel routings across devices. We can think of adding it to the TODO list, but this is another story. This is the background. However, since Jean pointed to the few other similar cases when of_node is being initialized in addition to fwnode I deem there is no issue and we can do the same in the LED core. No action in regulator core is required then. -- Best regards, Jacek Anaszewski
Re: [PATCH 07/11] vhost: convert vhost_umem_interval_tree to half closed intervals
On Fri, 04 Oct 2019, Michel Lespinasse wrote:
On Thu, Oct 03, 2019 at 01:18:54PM -0700, Davidlohr Bueso wrote:
@@ -1320,15 +1320,14 @@ static bool iotlb_access_ok(struct vhost_virtqueue *vq,
{
const struct vhost_umem_node *node;
struct vhost_umem *umem = vq->iotlb;
- u64 s = 0, size, orig_addr = addr, last = addr + len - 1;
+ u64 s = 0, size, orig_addr = addr, last = addr + len;
maybe "end" or "end_addr" instead of "last".
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index e9ed2722b633..bb36cb9ed5ec 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -53,13 +53,13 @@ struct vhost_log {
};
#define START(node) ((node)->start)
-#define LAST(node) ((node)->last)
+#define END(node) ((node)->end)
struct vhost_umem_node {
struct rb_node rb;
struct list_head link;
__u64 start;
- __u64 last;
+ __u64 end;
__u64 size;
__u64 userspace_addr;
__u32 perm;
Preferably also rename __subtree_last to __subtree_end
Yes, this was was another one that I had in mind renaming, but
didn't want to grow the series -- all custom interval trees
name _last for the subtree iirc. Like my previous reply, I'd
rather leave this stuff for a followup series.
Thanks,
Davidlohr
Re: [PATCH 05/11] IB/hfi1: convert __mmu_int_rb to half closed intervals
On Fri, 04 Oct 2019, Michel Lespinasse wrote:
On Thu, Oct 03, 2019 at 01:18:52PM -0700, Davidlohr Bueso wrote:
diff --git a/drivers/infiniband/hw/hfi1/mmu_rb.c
b/drivers/infiniband/hw/hfi1/mmu_rb.c
index 14d2a90964c3..fb6382b2d44e 100644
--- a/drivers/infiniband/hw/hfi1/mmu_rb.c
+++ b/drivers/infiniband/hw/hfi1/mmu_rb.c
@@ -47,7 +47,7 @@
#include
#include
#include
-#include
+#include
#include "mmu_rb.h"
#include "trace.h"
@@ -89,7 +89,7 @@ static unsigned long mmu_node_start(struct mmu_rb_node *node)
static unsigned long mmu_node_last(struct mmu_rb_node *node)
{
- return PAGE_ALIGN(node->addr + node->len) - 1;
+ return PAGE_ALIGN(node->addr + node->len);
}
May as well rename the function mmu_node_end(). I was worried if it
was used anywhere else, but it turned out it's only used when defining
the interval tree.
Right.
In general I tried not to rename everything to end because I wanted to
avoid bloating the diffstat, albeit having naming discrepancies within
the code (which isn't new either fwiw).
I would also suggest moving this function (as well as mmu_node_first)
right before its use, rather than just after, which would allow you to
also remove the function prototype a few lines earlier.
Indeed, but again I don't want to unnecessarily grow the patch. I have
several notes to come back to once/if this series is settled.
Looks good to me otherwise.
Reviewed-by: Michel Lespinasse
Thanks,
Davidlohr
Re: [PATCH -next 00/11] lib/interval-tree: move to half closed intervals
On Fri, 04 Oct 2019, Matthew Wilcox wrote: On Fri, Oct 04, 2019 at 06:15:11AM -0700, Michel Lespinasse wrote: My take is that this (Davidlohr's) patch series does not necessarily need to be applied all at once - we could get the first change in (adding the interval_tree_gen.h header), and convert the first few users, without getting them all at once, as long as we have a plan for finishing the work. So, if you have cleanups in progress in some of the files, just tell us which ones and we can leave them out from the first pass. Since we have users which do need to use the full ULONG_MAX range (as pointed out by Christian Koenig), I don't think adding a second implementation which is half-open is a good idea. It'll only lead to confusion. Right, we should not have two implementations. Thanks, Davidlohr
Re: [PATCH] drm: Fix comment doc for format_modifiers
On Fri, Oct 04, 2019 at 08:08:26PM +0100, Eric Engestrom wrote: > On Thursday, 2019-10-03 16:53:18 +0300, Ville Syrjälä wrote: > > On Thu, Oct 03, 2019 at 09:51:18AM +0200, Andrzej Pietrasiewicz wrote: > > > To human readers > > > > The commit message is always for human readers, no need to point that > > out... > > > > > > > > "array of struct drm_format modifiers" is almost indistinguishable from > > > "array of struct drm_format_modifiers", especially given that > > > struct drm_format_modifier does exist. > > > > ..but this paragraph still manages to 100% confuse this particular human. > > There's an underscore instead of a space on the second line > (s/drm_format modifiers/drm_format_modifiers/). OK, so I guess I really am blind. > > It should definitely be reworded to be much clearer. > > > > > The actual code changes lgtm, so with the commit message reworded > > this patch is > > Reviewed-by: Ville Syrjälä > > > > > > > > And indeed the parameter passes an array of uint64_t rather than an array > > > of structs, but the first words of the comment suggest that it passes > > > > > > Signed-off-by: Andrzej Pietrasiewicz > > > --- > > > drivers/gpu/drm/drm_plane.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c > > > index d6ad60ab0d38..0d4f9172c0dd 100644 > > > --- a/drivers/gpu/drm/drm_plane.c > > > +++ b/drivers/gpu/drm/drm_plane.c > > > @@ -160,7 +160,7 @@ static int create_in_format_blob(struct drm_device > > > *dev, struct drm_plane *plane > > > * @funcs: callbacks for the new plane > > > * @formats: array of supported formats (DRM_FORMAT\_\*) > > > * @format_count: number of elements in @formats > > > - * @format_modifiers: array of struct drm_format modifiers terminated by > > > + * @format_modifiers: array of format modifiers terminated by > > > *DRM_FORMAT_MOD_INVALID > > > * @type: type of plane (overlay, primary, cursor) > > > * @name: printf style format string for the plane name, or NULL for > > > default name > > > -- > > > 2.17.1 > > > > > > ___ > > > dri-devel mailing list > > > dri-devel@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- Ville Syrjälä Intel
Re: [PATCH][next] drm/komeda: remove redundant assignment to pointer disable_done
On Fri, Oct 04, 2019 at 05:21:56PM +0100, Colin King wrote:
> From: Colin Ian King
>
> The pointer disable_done is being initialized with a value that
> is never read and is being re-assigned a little later on. The
> assignment is redundant and hence can be removed.
Not really true, isn't it? The re-assignment is done under the condition that
crtc->state->active is true. disable_done will be used regardless after the if
block, so we can't skip this initialisation.
Not sure why Coverity flags this, but I would NAK this patch.
Best regards,
Liviu
>
> Addresses-Coverity: ("Unused value")
> Signed-off-by: Colin Ian King
> ---
> drivers/gpu/drm/arm/display/komeda/komeda_crtc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> index 75263d8cd0bd..9beeda04818b 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> @@ -296,7 +296,7 @@ komeda_crtc_atomic_disable(struct drm_crtc *crtc,
> struct komeda_crtc_state *old_st = to_kcrtc_st(old);
> struct komeda_pipeline *master = kcrtc->master;
> struct komeda_pipeline *slave = kcrtc->slave;
> - struct completion *disable_done = &crtc->state->commit->flip_done;
> + struct completion *disable_done;
> bool needs_phase2 = false;
>
> DRM_DEBUG_ATOMIC("CRTC%d_DISABLE: active_pipes: 0x%x, affected: 0x%x\n",
> --
> 2.20.1
>
--
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---
¯\_(ツ)_/¯
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/imx: fix memory leak in imx_pd_bind
In imx_pd_bind, the duplicated memory for imxpd->edid via kmemdup should
be released in drm_of_find_panel_or_bridge or imx_pd_register fail.
Fixes: ebc944613567 ("drm: convert drivers to use drm_of_find_panel_or_bridge")
Fixes: 19022aaae677 ("staging: drm/imx: Add parallel display support")
Signed-off-by: Navid Emamdoost
---
drivers/gpu/drm/imx/parallel-display.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/imx/parallel-display.c
b/drivers/gpu/drm/imx/parallel-display.c
index e7ce17503ae1..9522d2cb0ad5 100644
--- a/drivers/gpu/drm/imx/parallel-display.c
+++ b/drivers/gpu/drm/imx/parallel-display.c
@@ -227,14 +227,18 @@ static int imx_pd_bind(struct device *dev, struct device
*master, void *data)
/* port@1 is the output port */
ret = drm_of_find_panel_or_bridge(np, 1, 0, &imxpd->panel,
&imxpd->bridge);
- if (ret && ret != -ENODEV)
+ if (ret && ret != -ENODEV) {
+ kfree(imxpd->edid);
return ret;
+ }
imxpd->dev = dev;
ret = imx_pd_register(drm, imxpd);
- if (ret)
+ if (ret) {
+ kfree(imxpd->edid);
return ret;
+ }
dev_set_drvdata(dev, imxpd);
--
2.17.1
Re: [PATCH] drm: Fix comment doc for format_modifiers
On Thursday, 2019-10-03 16:53:18 +0300, Ville Syrjälä wrote: > On Thu, Oct 03, 2019 at 09:51:18AM +0200, Andrzej Pietrasiewicz wrote: > > To human readers > > The commit message is always for human readers, no need to point that > out... > > > > > "array of struct drm_format modifiers" is almost indistinguishable from > > "array of struct drm_format_modifiers", especially given that > > struct drm_format_modifier does exist. > > ..but this paragraph still manages to 100% confuse this particular human. There's an underscore instead of a space on the second line (s/drm_format modifiers/drm_format_modifiers/). It should definitely be reworded to be much clearer. > > The actual code changes lgtm, so with the commit message reworded > this patch is > Reviewed-by: Ville Syrjälä > > > > > And indeed the parameter passes an array of uint64_t rather than an array > > of structs, but the first words of the comment suggest that it passes > > > > Signed-off-by: Andrzej Pietrasiewicz > > --- > > drivers/gpu/drm/drm_plane.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c > > index d6ad60ab0d38..0d4f9172c0dd 100644 > > --- a/drivers/gpu/drm/drm_plane.c > > +++ b/drivers/gpu/drm/drm_plane.c > > @@ -160,7 +160,7 @@ static int create_in_format_blob(struct drm_device > > *dev, struct drm_plane *plane > > * @funcs: callbacks for the new plane > > * @formats: array of supported formats (DRM_FORMAT\_\*) > > * @format_count: number of elements in @formats > > - * @format_modifiers: array of struct drm_format modifiers terminated by > > + * @format_modifiers: array of format modifiers terminated by > > *DRM_FORMAT_MOD_INVALID > > * @type: type of plane (overlay, primary, cursor) > > * @name: printf style format string for the plane name, or NULL for > > default name > > -- > > 2.17.1 > > > > ___ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel >
[Bug 110886] After S3 resume, kernel: [drm] psp command failed and response status is (-65529) at 27th time of S3. 28th time of S3 freeze the system.
https://bugs.freedesktop.org/show_bug.cgi?id=110886 --- Comment #24 from Andrey Grodzovsky --- (In reply to Kai-Heng Feng from comment #23) > Created attachment 145576 [details] > journalctl last boot kernel message Can u retry with latest FW from https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git and also load kernel with drm.debug=1 as there seems a failure in PSP command submission during FW loading but the actual code of failure is now under debug log level. -- 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] drm/i915: customize DPCD brightness control for specific panel
On Fri, 04 Oct 2019, Adam Jackson wrote:
> On Sat, 2019-10-05 at 05:58 +0800, Lee Shawn C wrote:
>> This panel (manufacturer is SDC, product ID is 0x4141)
>> used manufacturer defined DPCD register to control brightness
>> that not defined in eDP spec so far. This change follow panel
>> vendor's instruction to support brightness adjustment.
>
> I'm sure this works, but this smells a little funny to me.
That was kindly put. ;)
>> +/* Samsung eDP panel */
>> +{ "SDC", 0x4141, EDID_QUIRK_NON_STD_BRIGHTNESS_CONTROL },
>
> It feels a bit like a layering violation to identify eDP behavior
> changes based on EDID. But I'm not sure there's any obvious way to
> identify this device by its DPCD. The Sink OUI (from the linked
> bugzilla) seems to be 0011F8, which doesn't match up to anything in my
> oui.txt...
We have the DPCD quirk stuff in drm_dp_helper.c, but IIUC in this case
there's only the OUI, and the device id etc. are all zeros. Otherwise I
think that would be the natural thing to do, and all this could be
better hidden away in i915.
>
>> @@ -1953,6 +1956,7 @@ static u32 edid_get_quirks(const struct edid *edid)
>>
>> return 0;
>> }
>> +EXPORT_SYMBOL(edid_get_quirks);
>
> If we're going to export this it should probably get a drm_ prefix.
>
>> +#define DPCD_EDP_GETSET_CTRL_PARAMS 0x344
>> +#define DPCD_EDP_CONTENT_LUMINANCE 0x346
>> +#define DPCD_EDP_PANEL_LUMINANCE_OVERRIDE 0x34a
>> +#define DPCD_EDP_BRIGHTNESS_NITS0x354
>> +#define DPCD_EDP_BRIGHTNESS_OPTIMIZATION0x358
>> +
>> +#define EDP_CUSTOMIZE_MAX_BRIGHTNESS_LEVEL (512)
>
> This also seems a bit weird, the 0x300-0x3FF registers belong to the
> _source_ DP device. But then later...
>
>> +/* write source OUI */
>> +write_val[0] = 0x00;
>> +write_val[1] = 0xaa;
>> +write_val[2] = 0x01;
>
> Oh hey, you're writing (an) Intel OUI to the Source OUI, so now it
> makes sense that you're writing to registers whose behavior the source
> defines. But this does raise the question: is this just a convention
> between Intel and this particular panel? Would we expect this to work
> with other similar panels? Is there any way to know to expect this
> convention from DPCD instead?
For one thing, it's not standard. I honestly don't know, but I'd assume
you wouldn't find behaviour with Intel OUI in non-Intel designs... and a
quirk of some sort seems like the only way to make this work.
I suppose we could start off with a DPCD quirk that only looks at the
sink OUI, and then, if needed, limit by DMI matching or by checking for
some DPCD registers (what, I am not sure, perhaps write the source OUI
and see how it behaves).
That would avoid the mildly annoying change in the EDID quirk interface
and how it's being used.
Thoughts?
BR,
Jani.
--
Jani Nikula, Intel Open Source Graphics Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 111729] RX480 : random NULL pointer dereference on resume from suspend
https://bugs.freedesktop.org/show_bug.cgi?id=111729 --- Comment #8 from m...@cschwarz.com --- There is a correspnding bug report on the Gentoo users forum: https://forums.gentoo.org/viewtopic-p-8375988.html#8375988 -- 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 110994] [vega10] *ERROR* Failed to initialize parser -125! , running libreoffice
https://bugs.freedesktop.org/show_bug.cgi?id=110994 --- Comment #8 from Kimmo --- (In reply to Jason Playne from comment #2) > This is has just started effecting me too. Playing "Hellblade: Senuas > Sacrifice" on steam (so steam play / radv / dxvk) > > mesa via pkppa > > kernel 5.1.15-050115-generic (ubuntu kernel ppa) > libdrm-amdgpu1:amd642.4.98+git1906260630.4 > mesa-vulkan-drivers:amd64 19.2~git1906240730.3b6 > > this wasn't happening 2 days ago, now it happens consistantly Distro: Arch linux with latest mesa/git/valve aco builds Experiencing the same hangups with World of Warcraft / Lutris / radv /dxvk mesa-aco-git 1:19.3.0_devel.115981.f2bed9b344c-1 Was using libdrm-2.4.99-1 stable build but dont know if there is any difference to that. Installed just latest libdrm-git from valve-repository Kernel 5.3.1-arch1-1-ARCH Hardware: RX480 4GB -- 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 111848] AMDGPU and display fails after resume from suspend
https://bugs.freedesktop.org/show_bug.cgi?id=111848 --- Comment #18 from m...@cschwarz.com --- Bisection turns out to be harder than anticipated because there is another bug that makes the system freeze on suspend (not reported yet). Anyways, this seems to be a duplicate of https://bugs.freedesktop.org/show_bug.cgi?id=111729#c7 -- 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: [git pull] drm fixes for 5.4-rc2
The pull request you sent on Fri, 4 Oct 2019 17:55:16 +1000: > git://anongit.freedesktop.org/drm/drm tags/drm-fixes-2019-10-04 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/768b47b7a9bca52101ff48081cfcecd3ebc5351a Thank you! -- Deet-doot-dot, I am a bot. https://korg.wiki.kernel.org/userdoc/prtracker
Re: [PATCH -next 00/11] lib/interval-tree: move to half closed intervals
On Fri, Oct 04, 2019 at 06:15:11AM -0700, Michel Lespinasse wrote: > Hi Jason, > > On Thu, Oct 3, 2019 at 5:26 PM Jason Gunthorpe wrote: > > Hurm, this is not entirely accurate. Most users do actually want > > overlapping and multiple ranges. I just studied this extensively: > > (Just curious, are you the person we discussed this with after the > Maple Tree talk at LPC 2019 ?) Possibly! > I think we have two separate API problems there: > - overlapping vs non-overlapping intervals (the interval tree API > supports overlapping intervals, but some users are confused about > this) I think we just have a bunch of confused drivers, ie the two drm drivers sure look confused to me. > - closed vs half-open interval definitions I'm not sure why this is a big problem.. We may actually just have bugs in handling the '-1' as it is supposed to be written as start + (size-1) so that start + size == ULONG_MAX+1 works properly. > > hfi1/mmu_rb definitely needs overlapping as it is dealing with > > userspace VA ranges under control of userspace. As do the other > > infiniband users. > > Do you have a handle on what usnic is doing with its intervals ? > usnic_uiom_insert_interval() has some complicated logic to avoid > having overlapping intervals, which is very confusing to me. I don't know why it is so complicated, but I can say that it is storing userspace VA's in that tree. I have some feeling this driver is trying to use the IOMMU to create a mirror of the userspace VA Userspace can request the HW be able to access any set of overlapping regions and so the driver must intersect all the ranges and compute a list of VA pages to IOMMU map. Just guessing. Jason
Re: Should regulator core support parsing OF based fwnode?
On Fri, Oct 04, 2019 at 06:12:52PM +0200, Jean-Jacques Hiblot wrote: > On 04/10/2019 17:58, Mark Brown wrote: > > Regulator supplies are supposed to be defined at the chip level rather > > than subfunctions with names corresponding to the names on the chip. ... > > good chance that they come up with the same mapping. The supply_alias > > interface is there to allow mapping these through to subfunctions if > > needed, it looks like the LED framework should be using this. > In case of current-sink LED drivers, each LED can be powered by a different > regulator, because the driver is only a switch between the LED cathod and > the ground. Sure, it's common for devices to have supplies that are only needed by one part of the chip which is why we have the supply_alias interface for mapping things through. > > That said if you are doing the above and the LEDs are appearing as > > devices it's extremely surprising that their of_node might not be > > initialized. > That is because this is usually done by the platform core which is not > involved here. The surprise is more that it got instantiated from the DT without keeping the node around than how it happened. signature.asc Description: PGP signature
Re: drm_sched with panfrost crash on T820
Am 04.10.2019 18:02 schrieb Steven Price :
On 04/10/2019 16:34, Koenig, Christian wrote:
> Am 04.10.19 um 17:27 schrieb Steven Price:
>> On 04/10/2019 16:03, Neil Armstrong wrote:
>>> On 04/10/2019 16:53, Grodzovsky, Andrey wrote:
On 10/3/19 4:34 AM, Neil Armstrong wrote:
> Hi Andrey,
>
> Le 02/10/2019 à 16:40, Grodzovsky, Andrey a écrit :
>> On 9/30/19 10:52 AM, Hillf Danton wrote:
>>> On Mon, 30 Sep 2019 11:17:45 +0200 Neil Armstrong wrote:
Did a new run from 5.3:
[ 35.971972] Call trace:
[ 35.974391] drm_sched_increase_karma+0x5c/0xf0
10667f3810667F94
drivers/gpu/drm/scheduler/sched_main.c:335
The crashing line is :
if (bad->s_fence->scheduled.context
==
entity->fence_context) {
Doesn't seem related to guilty job.
>>> Bail out if s_fence is no longer fresh.
>>>
>>> --- a/drivers/gpu/drm/scheduler/sched_main.c
>>> +++ b/drivers/gpu/drm/scheduler/sched_main.c
>>> @@ -333,6 +333,10 @@ void drm_sched_increase_karma(struct drm
>>>
>>> spin_lock(&rq->lock);
>>> list_for_each_entry_safe(entity, tmp,
>>> &rq->entities, list) {
>>> + if (!smp_load_acquire(&bad->s_fence)) {
>>> + spin_unlock(&rq->lock);
>>> + return;
>>> + }
>>> if (bad->s_fence->scheduled.context ==
>>> entity->fence_context) {
>>> if (atomic_read(&bad->karma) >
>>> @@ -543,7 +547,7 @@ EXPORT_SYMBOL(drm_sched_job_init);
>>> void drm_sched_job_cleanup(struct drm_sched_job *job)
>>> {
>>> dma_fence_put(&job->s_fence->finished);
>>> - job->s_fence = NULL;
>>> + smp_store_release(&job->s_fence, 0);
>>> }
>>> EXPORT_SYMBOL(drm_sched_job_cleanup);
> This fixed the problem on the 10 CI runs.
>
> Neil
These are good news but I still fail to see how this fixes the problem -
Hillf, do you mind explaining how you came up with this particular fix -
what was the bug you saw ?
>>> As Steven explained, seems the same job was submitted on both HW slots,
>>> and then when timeout occurs each thread calls panfrost_job_timedout
>>> which leads to drm_sched_stop() on the first call and on the
>>> second call the job was already freed.
>>>
>>> Steven proposed a working fix, and this one does the same but on
>>> the drm_sched side. This one looks cleaner, but panfrost should
>>> not call drm_sched_stop() twice for the same job.
>> I'm not sure that Hillf's fix is sufficient. In particular in
>> drm_sched_increase_karma() I don't understand how the smp_load_acquire()
>> call prevents bad->s_fence becoming NULL immediately afterwards (but
>> admittedly the window is much smaller). But really this is just a
>> Panfrost bug (calling drm_sched_stop() twice on the same job).
>>
>> The part of my change that I'd welcome feedback on is changing
>> cancel_delayed_work() to cancel_delayed_work_sync() in drm_sched_stop()
>> when called on different scheduler to the bad job. It's not clear to me
>> exactly what the semantics of the function should be, and I haven't
>> tested the effect of the change on drivers other than Panfrost.
>
> Yeah, at least of hand that change doesn't seem to make sense to me.
We need to ensure that any other timeouts that might have started
processing are complete before actually resetting the hardware.
Otherwise after the reset another thread could come along and attempt to
reset the hardware again (and cause a double free of a job).
This is intentional behaviour. If you don't want the double reset in Panfrost
you should probably call the cancel_sync yourself.
Regards,
Christian.
My change
to use the _sync() variant prevents this happening.
> Multiple timeout workers can perfectly run in parallel, Panfrost needs
> to make sure that they don't affect each other.
>
> The simplest way of doing this is to have a mutex you trylock when
> starting the reset.
>
> The first one grabbing it wins, all other just silently return.
Ah that would simplify my change removing the need for the new variable.
I hadn't thought to use trylock. I'll give that a spin and post a new
simpler version.
Thanks,
Steve
> Regards,
> Christian.
>
>>
>> Steve
>>
>>> Neil
>>>
Andrey
>> Does this change help the problem ? Note that drm_sched_job_cleanup is
>> called from scheduler thread which is stopped at all times when work_tdr
>> thread is running and anyway the 'bad' job is still in the
>> ring_mirro
[PATCH][next] drm/komeda: remove redundant assignment to pointer disable_done
From: Colin Ian King
The pointer disable_done is being initialized with a value that
is never read and is being re-assigned a little later on. The
assignment is redundant and hence can be removed.
Addresses-Coverity: ("Unused value")
Signed-off-by: Colin Ian King
---
drivers/gpu/drm/arm/display/komeda/komeda_crtc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
index 75263d8cd0bd..9beeda04818b 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
@@ -296,7 +296,7 @@ komeda_crtc_atomic_disable(struct drm_crtc *crtc,
struct komeda_crtc_state *old_st = to_kcrtc_st(old);
struct komeda_pipeline *master = kcrtc->master;
struct komeda_pipeline *slave = kcrtc->slave;
- struct completion *disable_done = &crtc->state->commit->flip_done;
+ struct completion *disable_done;
bool needs_phase2 = false;
DRM_DEBUG_ATOMIC("CRTC%d_DISABLE: active_pipes: 0x%x, affected: 0x%x\n",
--
2.20.1
Re: Should regulator core support parsing OF based fwnode?
On 04/10/2019 17:58, Mark Brown wrote:
On Fri, Oct 04, 2019 at 05:13:13PM +0200, Jean-Jacques Hiblot wrote:
On 04/10/2019 16:40, Mark Brown wrote:
Why is the LED core populating anything? Is the LED core copying bits
out of the struct device for the actual device into a synthetic device
rather than passing the actual device in? That really doesn't seem like
a good idea, it's likely to lead to things like this where you don't
copy something that's required (or worse where something directly in the
struct device that can't be copied is needed).
This is not a copy of a device of parent's of_node or something like that.
You can think of a LED controller as a bus. It 'enumerates' its children
LED, create the children devices (one per LED) and provides the functions to
interact with them.
The device node we are talking about here is a per-LED thing, it is a child
node of the node of the LED controller.
here is an example:
tlc59108: tlc59116@40 { /* this is the node for the LED controller */
status = "okay";
#address-cells = <1>;
#size-cells = <0>;
compatible = "ti,tlc59108";
reg = <0x40>;
backlight_led: led@2 { /* this is the node of one LED attached to
pin#2 of the LED controller */
power-supply = <&bkl_fixed>;
reg = <0x2>;
};
Regulator supplies are supposed to be defined at the chip level rather
than subfunctions with names corresponding to the names on the chip.
This ensures that all chips look similar when you're mapping the
schematic into a DT, you shouldn't need to know about the binding for a
given chip to write a DT for it and if multiple people (perhaps working
on different OSs) write bindings for the same chip there should be a
good chance that they come up with the same mapping. The supply_alias
interface is there to allow mapping these through to subfunctions if
needed, it looks like the LED framework should be using this.
In case of current-sink LED drivers, each LED can be powered by a
different regulator, because the driver is only a switch between the LED
cathod and the ground.
That said if you are doing the above and the LEDs are appearing as
devices it's extremely surprising that their of_node might not be
initialized.
That is because this is usually done by the platform core which is not
involved here.
JJ
[Bug 111599] [CI][RESUME] igt@gem_ctx_isolation@* - skip - Test requirement: !(gen > LAST_KNOWN_GEN), SKIP
https://bugs.freedesktop.org/show_bug.cgi?id=111599 Chris Wilson changed: What|Removed |Added Resolution|--- |FIXED Status|NEW |RESOLVED --- Comment #3 from Chris Wilson --- commit d17a484b3c22706b2b004ef1577f367d79235e43 (upstream/master, origin/master, origin/HEAD) Author: Chris Wilson Date: Wed Oct 2 12:22:29 2019 +0100 i915/gem_ctx_isolation: Bump support for Tigerlake There's very little variation in non-privileged registers for Tigerlake, so we can mostly inherit the set from gen11. There is no whitelist at present, so we do not need to add any special registers. v2: Add COMMON_SLICE_CHICKEN2, GEN9_SLICE_COMMON_ECO_CHICKEN1 and a variety of huc readonly status registers. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111599 Signed-off-by: Chris Wilson Cc: Mika Kuoppala Reviewed-by: Mika Kuoppala -- 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] drm/i810: Prevent underflow in ioctl
Quoting Chris Wilson (2019-10-04 15:08:57) > Quoting Dan Carpenter (2019-10-04 11:22:51) > > The "used" variables here come from the user in the ioctl and it can be > > negative. It could result in an out of bounds write. > > > > Signed-off-by: Dan Carpenter > > --- > > drivers/gpu/drm/i810/i810_dma.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i810/i810_dma.c > > b/drivers/gpu/drm/i810/i810_dma.c > > index 2a77823b8e9a..e66c38332df4 100644 > > --- a/drivers/gpu/drm/i810/i810_dma.c > > +++ b/drivers/gpu/drm/i810/i810_dma.c > > @@ -728,7 +728,7 @@ static void i810_dma_dispatch_vertex(struct drm_device > > *dev, > > if (nbox > I810_NR_SAREA_CLIPRECTS) > > nbox = I810_NR_SAREA_CLIPRECTS; > > > > - if (used > 4 * 1024) > > + if (used < 0 || used > 4 * 1024) > > used = 0; > > Yes, as passed to the GPU instruction, negative used is invalid. > > Then it is used as an offset into a memblock, where a negative offset > would be very bad. > > Reviewed-by: Chris Wilson Applied to drm-misc-next with cc'ed stable. -Chris ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH -next 00/11] lib/interval-tree: move to half closed intervals
On Fri, Oct 04, 2019 at 06:15:11AM -0700, Michel Lespinasse wrote: > My take is that this (Davidlohr's) patch series does not necessarily > need to be applied all at once - we could get the first change in > (adding the interval_tree_gen.h header), and convert the first few > users, without getting them all at once, as long as we have a plan for > finishing the work. So, if you have cleanups in progress in some of > the files, just tell us which ones and we can leave them out from the > first pass. Since we have users which do need to use the full ULONG_MAX range (as pointed out by Christian Koenig), I don't think adding a second implementation which is half-open is a good idea. It'll only lead to confusion.
Re: drm_sched with panfrost crash on T820
On 04/10/2019 16:34, Koenig, Christian wrote:
> Am 04.10.19 um 17:27 schrieb Steven Price:
>> On 04/10/2019 16:03, Neil Armstrong wrote:
>>> On 04/10/2019 16:53, Grodzovsky, Andrey wrote:
On 10/3/19 4:34 AM, Neil Armstrong wrote:
> Hi Andrey,
>
> Le 02/10/2019 à 16:40, Grodzovsky, Andrey a écrit :
>> On 9/30/19 10:52 AM, Hillf Danton wrote:
>>> On Mon, 30 Sep 2019 11:17:45 +0200 Neil Armstrong wrote:
Did a new run from 5.3:
[ 35.971972] Call trace:
[ 35.974391] drm_sched_increase_karma+0x5c/0xf0
10667f3810667F94
drivers/gpu/drm/scheduler/sched_main.c:335
The crashing line is :
if (bad->s_fence->scheduled.context
==
entity->fence_context) {
Doesn't seem related to guilty job.
>>> Bail out if s_fence is no longer fresh.
>>>
>>> --- a/drivers/gpu/drm/scheduler/sched_main.c
>>> +++ b/drivers/gpu/drm/scheduler/sched_main.c
>>> @@ -333,6 +333,10 @@ void drm_sched_increase_karma(struct drm
>>>
>>> spin_lock(&rq->lock);
>>> list_for_each_entry_safe(entity, tmp,
>>> &rq->entities, list) {
>>> + if (!smp_load_acquire(&bad->s_fence)) {
>>> + spin_unlock(&rq->lock);
>>> + return;
>>> + }
>>> if (bad->s_fence->scheduled.context ==
>>> entity->fence_context) {
>>> if (atomic_read(&bad->karma) >
>>> @@ -543,7 +547,7 @@ EXPORT_SYMBOL(drm_sched_job_init);
>>> void drm_sched_job_cleanup(struct drm_sched_job *job)
>>> {
>>> dma_fence_put(&job->s_fence->finished);
>>> - job->s_fence = NULL;
>>> + smp_store_release(&job->s_fence, 0);
>>> }
>>> EXPORT_SYMBOL(drm_sched_job_cleanup);
> This fixed the problem on the 10 CI runs.
>
> Neil
These are good news but I still fail to see how this fixes the problem -
Hillf, do you mind explaining how you came up with this particular fix -
what was the bug you saw ?
>>> As Steven explained, seems the same job was submitted on both HW slots,
>>> and then when timeout occurs each thread calls panfrost_job_timedout
>>> which leads to drm_sched_stop() on the first call and on the
>>> second call the job was already freed.
>>>
>>> Steven proposed a working fix, and this one does the same but on
>>> the drm_sched side. This one looks cleaner, but panfrost should
>>> not call drm_sched_stop() twice for the same job.
>> I'm not sure that Hillf's fix is sufficient. In particular in
>> drm_sched_increase_karma() I don't understand how the smp_load_acquire()
>> call prevents bad->s_fence becoming NULL immediately afterwards (but
>> admittedly the window is much smaller). But really this is just a
>> Panfrost bug (calling drm_sched_stop() twice on the same job).
>>
>> The part of my change that I'd welcome feedback on is changing
>> cancel_delayed_work() to cancel_delayed_work_sync() in drm_sched_stop()
>> when called on different scheduler to the bad job. It's not clear to me
>> exactly what the semantics of the function should be, and I haven't
>> tested the effect of the change on drivers other than Panfrost.
>
> Yeah, at least of hand that change doesn't seem to make sense to me.
We need to ensure that any other timeouts that might have started
processing are complete before actually resetting the hardware.
Otherwise after the reset another thread could come along and attempt to
reset the hardware again (and cause a double free of a job). My change
to use the _sync() variant prevents this happening.
> Multiple timeout workers can perfectly run in parallel, Panfrost needs
> to make sure that they don't affect each other.
>
> The simplest way of doing this is to have a mutex you trylock when
> starting the reset.
>
> The first one grabbing it wins, all other just silently return.
Ah that would simplify my change removing the need for the new variable.
I hadn't thought to use trylock. I'll give that a spin and post a new
simpler version.
Thanks,
Steve
> Regards,
> Christian.
>
>>
>> Steve
>>
>>> Neil
>>>
Andrey
>> Does this change help the problem ? Note that drm_sched_job_cleanup is
>> called from scheduler thread which is stopped at all times when work_tdr
>> thread is running and anyway the 'bad' job is still in the
>> ring_mirror_list while it's being accessed from
>> drm_sched_increase_karma so I don't think drm_sched_job_cleanup can be
>> called for it BEFORE or while drm_sched_increase_karma is executed.
>
Re: Should regulator core support parsing OF based fwnode?
On Fri, Oct 04, 2019 at 05:13:13PM +0200, Jean-Jacques Hiblot wrote:
> On 04/10/2019 16:40, Mark Brown wrote:
> > Why is the LED core populating anything? Is the LED core copying bits
> > out of the struct device for the actual device into a synthetic device
> > rather than passing the actual device in? That really doesn't seem like
> > a good idea, it's likely to lead to things like this where you don't
> > copy something that's required (or worse where something directly in the
> > struct device that can't be copied is needed).
> This is not a copy of a device of parent's of_node or something like that.
> You can think of a LED controller as a bus. It 'enumerates' its children
> LED, create the children devices (one per LED) and provides the functions to
> interact with them.
> The device node we are talking about here is a per-LED thing, it is a child
> node of the node of the LED controller.
> here is an example:
>
> tlc59108: tlc59116@40 { /* this is the node for the LED controller */
> status = "okay";
> #address-cells = <1>;
> #size-cells = <0>;
> compatible = "ti,tlc59108";
> reg = <0x40>;
>
> backlight_led: led@2 { /* this is the node of one LED attached to
> pin#2 of the LED controller */
> power-supply = <&bkl_fixed>;
> reg = <0x2>;
> };
Regulator supplies are supposed to be defined at the chip level rather
than subfunctions with names corresponding to the names on the chip.
This ensures that all chips look similar when you're mapping the
schematic into a DT, you shouldn't need to know about the binding for a
given chip to write a DT for it and if multiple people (perhaps working
on different OSs) write bindings for the same chip there should be a
good chance that they come up with the same mapping. The supply_alias
interface is there to allow mapping these through to subfunctions if
needed, it looks like the LED framework should be using this.
That said if you are doing the above and the LEDs are appearing as
devices it's extremely surprising that their of_node might not be
initialized.
signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v7 5/5] backlight: add led-backlight driver
Hi Lee, On 04/10/2019 16:39, Lee Jones wrote: On Wed, 18 Sep 2019, Jean-Jacques Hiblot wrote: From: Tomi Valkeinen This patch adds a led-backlight driver (led_bl), which is similar to pwm_bl except the driver uses a LED class driver to adjust the brightness in the HW. Multiple LEDs can be used for a single backlight. Signed-off-by: Tomi Valkeinen Signed-off-by: Jean-Jacques Hiblot Acked-by: Pavel Machek Reviewed-by: Daniel Thompson --- drivers/video/backlight/Kconfig | 7 + drivers/video/backlight/Makefile | 1 + drivers/video/backlight/led_bl.c | 260 +++ 3 files changed, 268 insertions(+) create mode 100644 drivers/video/backlight/led_bl.c Applied, thanks. It will break the build because it relies on functions not yet in the LED core (devm_led_get() for v7 or devm_of_led_get() for v8) JJ ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: drm_sched with panfrost crash on T820
Am 04.10.19 um 17:27 schrieb Steven Price:
> On 04/10/2019 16:03, Neil Armstrong wrote:
>> On 04/10/2019 16:53, Grodzovsky, Andrey wrote:
>>> On 10/3/19 4:34 AM, Neil Armstrong wrote:
Hi Andrey,
Le 02/10/2019 à 16:40, Grodzovsky, Andrey a écrit :
> On 9/30/19 10:52 AM, Hillf Danton wrote:
>> On Mon, 30 Sep 2019 11:17:45 +0200 Neil Armstrong wrote:
>>> Did a new run from 5.3:
>>>
>>> [ 35.971972] Call trace:
>>> [ 35.974391] drm_sched_increase_karma+0x5c/0xf0
>>> 10667f3810667F94
>>> drivers/gpu/drm/scheduler/sched_main.c:335
>>>
>>> The crashing line is :
>>>if (bad->s_fence->scheduled.context
>>> ==
>>>entity->fence_context) {
>>>
>>> Doesn't seem related to guilty job.
>> Bail out if s_fence is no longer fresh.
>>
>> --- a/drivers/gpu/drm/scheduler/sched_main.c
>> +++ b/drivers/gpu/drm/scheduler/sched_main.c
>> @@ -333,6 +333,10 @@ void drm_sched_increase_karma(struct drm
>>
>> spin_lock(&rq->lock);
>> list_for_each_entry_safe(entity, tmp,
>> &rq->entities, list) {
>> +if (!smp_load_acquire(&bad->s_fence)) {
>> +spin_unlock(&rq->lock);
>> +return;
>> +}
>> if (bad->s_fence->scheduled.context ==
>> entity->fence_context) {
>> if (atomic_read(&bad->karma) >
>> @@ -543,7 +547,7 @@ EXPORT_SYMBOL(drm_sched_job_init);
>> void drm_sched_job_cleanup(struct drm_sched_job *job)
>> {
>> dma_fence_put(&job->s_fence->finished);
>> -job->s_fence = NULL;
>> +smp_store_release(&job->s_fence, 0);
>> }
>> EXPORT_SYMBOL(drm_sched_job_cleanup);
This fixed the problem on the 10 CI runs.
Neil
>>>
>>> These are good news but I still fail to see how this fixes the problem -
>>> Hillf, do you mind explaining how you came up with this particular fix -
>>> what was the bug you saw ?
>> As Steven explained, seems the same job was submitted on both HW slots,
>> and then when timeout occurs each thread calls panfrost_job_timedout
>> which leads to drm_sched_stop() on the first call and on the
>> second call the job was already freed.
>>
>> Steven proposed a working fix, and this one does the same but on
>> the drm_sched side. This one looks cleaner, but panfrost should
>> not call drm_sched_stop() twice for the same job.
> I'm not sure that Hillf's fix is sufficient. In particular in
> drm_sched_increase_karma() I don't understand how the smp_load_acquire()
> call prevents bad->s_fence becoming NULL immediately afterwards (but
> admittedly the window is much smaller). But really this is just a
> Panfrost bug (calling drm_sched_stop() twice on the same job).
>
> The part of my change that I'd welcome feedback on is changing
> cancel_delayed_work() to cancel_delayed_work_sync() in drm_sched_stop()
> when called on different scheduler to the bad job. It's not clear to me
> exactly what the semantics of the function should be, and I haven't
> tested the effect of the change on drivers other than Panfrost.
Yeah, at least of hand that change doesn't seem to make sense to me.
Multiple timeout workers can perfectly run in parallel, Panfrost needs
to make sure that they don't affect each other.
The simplest way of doing this is to have a mutex you trylock when
starting the reset.
The first one grabbing it wins, all other just silently return.
Regards,
Christian.
>
> Steve
>
>> Neil
>>
>>> Andrey
>>>
> Does this change help the problem ? Note that drm_sched_job_cleanup is
> called from scheduler thread which is stopped at all times when work_tdr
> thread is running and anyway the 'bad' job is still in the
> ring_mirror_list while it's being accessed from
> drm_sched_increase_karma so I don't think drm_sched_job_cleanup can be
> called for it BEFORE or while drm_sched_increase_karma is executed.
>
> Andrey
>
>
>>
>> --
>>
>> ___
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
Re: drm_sched with panfrost crash on T820
On 04/10/2019 16:03, Neil Armstrong wrote:
> On 04/10/2019 16:53, Grodzovsky, Andrey wrote:
>>
>> On 10/3/19 4:34 AM, Neil Armstrong wrote:
>>> Hi Andrey,
>>>
>>> Le 02/10/2019 à 16:40, Grodzovsky, Andrey a écrit :
On 9/30/19 10:52 AM, Hillf Danton wrote:
> On Mon, 30 Sep 2019 11:17:45 +0200 Neil Armstrong wrote:
>> Did a new run from 5.3:
>>
>> [ 35.971972] Call trace:
>> [ 35.974391] drm_sched_increase_karma+0x5c/0xf0
>> 10667f3810667F94
>> drivers/gpu/drm/scheduler/sched_main.c:335
>>
>> The crashing line is :
>> if (bad->s_fence->scheduled.context ==
>> entity->fence_context) {
>>
>> Doesn't seem related to guilty job.
> Bail out if s_fence is no longer fresh.
>
> --- a/drivers/gpu/drm/scheduler/sched_main.c
> +++ b/drivers/gpu/drm/scheduler/sched_main.c
> @@ -333,6 +333,10 @@ void drm_sched_increase_karma(struct drm
>
> spin_lock(&rq->lock);
> list_for_each_entry_safe(entity, tmp,
> &rq->entities, list) {
> + if (!smp_load_acquire(&bad->s_fence)) {
> + spin_unlock(&rq->lock);
> + return;
> + }
> if (bad->s_fence->scheduled.context ==
> entity->fence_context) {
> if (atomic_read(&bad->karma) >
> @@ -543,7 +547,7 @@ EXPORT_SYMBOL(drm_sched_job_init);
>void drm_sched_job_cleanup(struct drm_sched_job *job)
>{
> dma_fence_put(&job->s_fence->finished);
> - job->s_fence = NULL;
> + smp_store_release(&job->s_fence, 0);
>}
>EXPORT_SYMBOL(drm_sched_job_cleanup);
>>> This fixed the problem on the 10 CI runs.
>>>
>>> Neil
>>
>>
>> These are good news but I still fail to see how this fixes the problem -
>> Hillf, do you mind explaining how you came up with this particular fix -
>> what was the bug you saw ?
>
> As Steven explained, seems the same job was submitted on both HW slots,
> and then when timeout occurs each thread calls panfrost_job_timedout
> which leads to drm_sched_stop() on the first call and on the
> second call the job was already freed.
>
> Steven proposed a working fix, and this one does the same but on
> the drm_sched side. This one looks cleaner, but panfrost should
> not call drm_sched_stop() twice for the same job.
I'm not sure that Hillf's fix is sufficient. In particular in
drm_sched_increase_karma() I don't understand how the smp_load_acquire()
call prevents bad->s_fence becoming NULL immediately afterwards (but
admittedly the window is much smaller). But really this is just a
Panfrost bug (calling drm_sched_stop() twice on the same job).
The part of my change that I'd welcome feedback on is changing
cancel_delayed_work() to cancel_delayed_work_sync() in drm_sched_stop()
when called on different scheduler to the bad job. It's not clear to me
exactly what the semantics of the function should be, and I haven't
tested the effect of the change on drivers other than Panfrost.
Steve
> Neil
>
>>
>> Andrey
>>
>>>
Does this change help the problem ? Note that drm_sched_job_cleanup is
called from scheduler thread which is stopped at all times when work_tdr
thread is running and anyway the 'bad' job is still in the
ring_mirror_list while it's being accessed from
drm_sched_increase_karma so I don't think drm_sched_job_cleanup can be
called for it BEFORE or while drm_sched_increase_karma is executed.
Andrey
>
> --
>
>
> ___
> 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/i915: customize DPCD brightness control for specific panel
On Sat, 2019-10-05 at 05:58 +0800, Lee Shawn C wrote:
> This panel (manufacturer is SDC, product ID is 0x4141)
> used manufacturer defined DPCD register to control brightness
> that not defined in eDP spec so far. This change follow panel
> vendor's instruction to support brightness adjustment.
I'm sure this works, but this smells a little funny to me.
> + /* Samsung eDP panel */
> + { "SDC", 0x4141, EDID_QUIRK_NON_STD_BRIGHTNESS_CONTROL },
It feels a bit like a layering violation to identify eDP behavior
changes based on EDID. But I'm not sure there's any obvious way to
identify this device by its DPCD. The Sink OUI (from the linked
bugzilla) seems to be 0011F8, which doesn't match up to anything in my
oui.txt...
> @@ -1953,6 +1956,7 @@ static u32 edid_get_quirks(const struct edid *edid)
>
> return 0;
> }
> +EXPORT_SYMBOL(edid_get_quirks);
If we're going to export this it should probably get a drm_ prefix.
> +#define DPCD_EDP_GETSET_CTRL_PARAMS 0x344
> +#define DPCD_EDP_CONTENT_LUMINANCE 0x346
> +#define DPCD_EDP_PANEL_LUMINANCE_OVERRIDE0x34a
> +#define DPCD_EDP_BRIGHTNESS_NITS 0x354
> +#define DPCD_EDP_BRIGHTNESS_OPTIMIZATION 0x358
> +
> +#define EDP_CUSTOMIZE_MAX_BRIGHTNESS_LEVEL (512)
This also seems a bit weird, the 0x300-0x3FF registers belong to the
_source_ DP device. But then later...
> + /* write source OUI */
> + write_val[0] = 0x00;
> + write_val[1] = 0xaa;
> + write_val[2] = 0x01;
Oh hey, you're writing (an) Intel OUI to the Source OUI, so now it
makes sense that you're writing to registers whose behavior the source
defines. But this does raise the question: is this just a convention
between Intel and this particular panel? Would we expect this to work
with other similar panels? Is there any way to know to expect this
convention from DPCD instead?
- ajax
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/3] dt-bindings: arm: samsung: Force clkoutN names to be unique in PMU
The clkoutN names of clocks must be unique because they represent unique inputs of clock multiplexer. Signed-off-by: Krzysztof Kozlowski --- Documentation/devicetree/bindings/arm/samsung/pmu.yaml | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/arm/samsung/pmu.yaml b/Documentation/devicetree/bindings/arm/samsung/pmu.yaml index 73b56fc5bf58..d8e03716f5d2 100644 --- a/Documentation/devicetree/bindings/arm/samsung/pmu.yaml +++ b/Documentation/devicetree/bindings/arm/samsung/pmu.yaml @@ -53,8 +53,10 @@ properties: List of clock names for particular CLKOUT mux inputs minItems: 1 maxItems: 32 -items: - pattern: '^clkout([0-9]|[12][0-9]|3[0-1])$' +allOf: + - items: + pattern: '^clkout([0-9]|[12][0-9]|3[0-1])$' + - uniqueItems: true clocks: minItems: 1 -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/3] dt-bindings: serial: Convert Samsung UART bindings to json-schema
Convert Samsung S3C/S5P/Exynos Serial/UART bindings to DT schema format
using json-schema.
Signed-off-by: Krzysztof Kozlowski
---
.../bindings/mfd/samsung,exynos5433-lpass.txt | 2 +-
.../bindings/serial/samsung_uart.txt | 58 ---
.../bindings/serial/samsung_uart.yaml | 148 ++
3 files changed, 149 insertions(+), 59 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/serial/samsung_uart.txt
create mode 100644 Documentation/devicetree/bindings/serial/samsung_uart.yaml
diff --git a/Documentation/devicetree/bindings/mfd/samsung,exynos5433-lpass.txt
b/Documentation/devicetree/bindings/mfd/samsung,exynos5433-lpass.txt
index d759da606f75..30ea27c3936d 100644
--- a/Documentation/devicetree/bindings/mfd/samsung,exynos5433-lpass.txt
+++ b/Documentation/devicetree/bindings/mfd/samsung,exynos5433-lpass.txt
@@ -18,7 +18,7 @@ an optional sub-node. For "samsung,exynos5433-lpass"
compatible this includes:
UART, SLIMBUS, PCM, I2S, DMAC, Timers 0...4, VIC, WDT 0...1 devices.
Bindings of the sub-nodes are described in:
- ../serial/samsung_uart.txt
+ ../serial/samsung_uart.yaml
../sound/samsung-i2s.txt
../dma/arm-pl330.txt
diff --git a/Documentation/devicetree/bindings/serial/samsung_uart.txt
b/Documentation/devicetree/bindings/serial/samsung_uart.txt
deleted file mode 100644
index e85f37ec33f0..
--- a/Documentation/devicetree/bindings/serial/samsung_uart.txt
+++ /dev/null
@@ -1,58 +0,0 @@
-* Samsung's UART Controller
-
-The Samsung's UART controller is used for interfacing SoC with serial
-communicaion devices.
-
-Required properties:
-- compatible: should be one of following:
- - "samsung,exynos4210-uart" - Exynos4210 SoC,
- - "samsung,s3c2410-uart" - compatible with ports present on S3C2410 SoC,
- - "samsung,s3c2412-uart" - compatible with ports present on S3C2412 SoC,
- - "samsung,s3c2440-uart" - compatible with ports present on S3C2440 SoC,
- - "samsung,s3c6400-uart" - compatible with ports present on S3C6400 SoC,
- - "samsung,s5pv210-uart" - compatible with ports present on S5PV210 SoC.
-
-- reg: base physical address of the controller and length of memory mapped
- region.
-
-- interrupts: a single interrupt signal to SoC interrupt controller,
- according to interrupt bindings documentation [1].
-
-- clock-names: input names of clocks used by the controller:
- - "uart" - controller bus clock,
- - "clk_uart_baudN" - Nth baud base clock input (N = 0, 1, ...),
-according to SoC User's Manual (only N = 0 is allowedfor SoCs without
-internal baud clock mux).
-- clocks: phandles and specifiers for all clocks specified in "clock-names"
- property, in the same order, according to clock bindings documentation [2].
-
-[1] Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
-[2] Documentation/devicetree/bindings/clock/clock-bindings.txt
-
-Optional properties:
-- samsung,uart-fifosize: The fifo size supported by the UART channel
-
-Note: Each Samsung UART should have an alias correctly numbered in the
-"aliases" node, according to serialN format, where N is the port number
-(non-negative decimal integer) as specified by User's Manual of respective
-SoC.
-
-Example:
- aliases {
- serial0 = &uart0;
- serial1 = &uart1;
- serial2 = &uart2;
- };
-
-Example:
- uart1: serial@7f005400 {
- compatible = "samsung,s3c6400-uart";
- reg = <0x7f005400 0x100>;
- interrupt-parent = <&vic1>;
- interrupts = <6>;
- clock-names = "uart", "clk_uart_baud2",
- "clk_uart_baud3";
- clocks = <&clocks PCLK_UART1>, <&clocks PCLK_UART1>,
- <&clocks SCLK_UART>;
- samsung,uart-fifosize = <16>;
- };
diff --git a/Documentation/devicetree/bindings/serial/samsung_uart.yaml
b/Documentation/devicetree/bindings/serial/samsung_uart.yaml
new file mode 100644
index ..276bea1c231a
--- /dev/null
+++ b/Documentation/devicetree/bindings/serial/samsung_uart.yaml
@@ -0,0 +1,148 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/serial/samsung_uart.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Samsung S3C, S5P and Exynos SoC UART Controller
+
+maintainers:
+ - Krzysztof Kozlowski
+ - Greg Kroah-Hartman
+
+description: |+
+ Each Samsung UART should have an alias correctly numbered in the "aliases"
+ node, according to serialN format, where N is the port number (non-negative
+ decimal integer) as specified by User's Manual of respective SoC.
+
+properties:
+ compatible:
+items:
+ - enum:
+ - samsung,s3c2410-uart
+ - samsung,s3c2412-uart
+ - samsung,s3c2440-uart
+ - samsung,s3c6400-uart
+ - samsung,s5pv210-uart
+ - samsung,exynos4210-uart
+
+ reg:
+maxItems: 1
+
+ clocks:
+minIte
[PATCH 2/3] dt-bindings: gpu: samsung-rotator: Fix indentation
Array elements under 'items' should be indented. Signed-off-by: Krzysztof Kozlowski --- Documentation/devicetree/bindings/gpu/samsung-rotator.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/gpu/samsung-rotator.yaml b/Documentation/devicetree/bindings/gpu/samsung-rotator.yaml index 45ce562435fa..f4dfa6fc724c 100644 --- a/Documentation/devicetree/bindings/gpu/samsung-rotator.yaml +++ b/Documentation/devicetree/bindings/gpu/samsung-rotator.yaml @@ -27,7 +27,7 @@ properties: clock-names: items: -- const: rotator + - const: rotator required: - compatible -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Should regulator core support parsing OF based fwnode?
On 04/10/2019 16:40, Mark Brown wrote:
On Fri, Oct 04, 2019 at 03:33:13PM +0200, Jean-Jacques Hiblot wrote:
On 04/10/2019 13:39, Mark Brown wrote:
Consumers should just be able to request a regulator without having to
worry about how that's being provided - they should have no knowledge at
all of firmware bindings or platform data for defining this. If they
do that suggests there's an abstraction issue somewhere, what makes you
think that doing something with of_node is required?
The regulator core accesses consumer->of_node to get a phandle to a
regulator's node. The trouble arises from the fact that the LED core does
not populate of_node anymore, instead it populates fwnode. This allows the
LED core to be agnostic of ACPI or OF to get the properties of a LED.
Why is the LED core populating anything? Is the LED core copying bits
out of the struct device for the actual device into a synthetic device
rather than passing the actual device in? That really doesn't seem like
a good idea, it's likely to lead to things like this where you don't
copy something that's required (or worse where something directly in the
struct device that can't be copied is needed).
This is not a copy of a device of parent's of_node or something like that.
You can think of a LED controller as a bus. It 'enumerates' its children
LED, create the children devices (one per LED) and provides the
functions to interact with them.
The device node we are talking about here is a per-LED thing, it is a
child node of the node of the LED controller.
here is an example:
tlc59108: tlc59116@40 { /* this is the node for the LED controller */
status = "okay";
#address-cells = <1>;
#size-cells = <0>;
compatible = "ti,tlc59108";
reg = <0x40>;
backlight_led: led@2 { /* this is the node of one LED attached
to pin#2 of the LED controller */
power-supply = <&bkl_fixed>;
reg = <0x2>;
};
other_led: led@3 { /* this is the node another LED attached to
pin #3 of the LED controller */
power-supply = <®_3v3>;
reg = <0x3>;
};
};
IMO it is better to populate both of_node and fwnode in the LED core at the
moment. It has already been fixed this way for the platform driver [0], MTD
[1] and PCI-OF [2].
Yeah, if you're going to be copying stuff out of the real device I'd
copy the of_node as well.
Further, unless you have LEDs that work without power you probably
shouldn't be using _get_optional() for their supply. That interface is
intended only for supplies that may be physically absent.
Not all LEDs have a regulator to provide the power. The power can be
supplied by the LED controller for example.
This code probably shouldn't be being run at all for LEDs like that, I
was assuming this was just for GPIO LEDs and similar rather than all
LEDs.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: linux-next: Tree for Oct 4 (amdgpu)
On Fri, Oct 4, 2019 at 11:08 AM Randy Dunlap wrote: > > On 10/3/19 10:59 PM, Stephen Rothwell wrote: > > Hi all, > > > > Changes since 20191003: > > > > on x86_64: > CONFIG_DRM_AMDGPU=y > # CONFIG_DRM_AMDGPU_SI is not set > # CONFIG_DRM_AMDGPU_CIK is not set > CONFIG_DRM_AMDGPU_USERPTR=y > CONFIG_DRM_AMDGPU_GART_DEBUGFS=y > > ld: drivers/gpu/drm/amd/amdkfd/kfd_device.o:(.rodata+0xf60): undefined > reference to `gfx_v7_kfd2kgd' > Fixed: https://patchwork.freedesktop.org/patch/334412/ Alex > -- > ~Randy > ___ > amd-gfx mailing list > amd-...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: drm_sched with panfrost crash on T820
On 04/10/2019 16:53, Grodzovsky, Andrey wrote:
>
> On 10/3/19 4:34 AM, Neil Armstrong wrote:
>> Hi Andrey,
>>
>> Le 02/10/2019 à 16:40, Grodzovsky, Andrey a écrit :
>>> On 9/30/19 10:52 AM, Hillf Danton wrote:
On Mon, 30 Sep 2019 11:17:45 +0200 Neil Armstrong wrote:
> Did a new run from 5.3:
>
> [ 35.971972] Call trace:
> [ 35.974391] drm_sched_increase_karma+0x5c/0xf0
> 10667f3810667F94
> drivers/gpu/drm/scheduler/sched_main.c:335
>
> The crashing line is :
> if (bad->s_fence->scheduled.context ==
> entity->fence_context) {
>
> Doesn't seem related to guilty job.
Bail out if s_fence is no longer fresh.
--- a/drivers/gpu/drm/scheduler/sched_main.c
+++ b/drivers/gpu/drm/scheduler/sched_main.c
@@ -333,6 +333,10 @@ void drm_sched_increase_karma(struct drm
spin_lock(&rq->lock);
list_for_each_entry_safe(entity, tmp,
&rq->entities, list) {
+ if (!smp_load_acquire(&bad->s_fence)) {
+ spin_unlock(&rq->lock);
+ return;
+ }
if (bad->s_fence->scheduled.context ==
entity->fence_context) {
if (atomic_read(&bad->karma) >
@@ -543,7 +547,7 @@ EXPORT_SYMBOL(drm_sched_job_init);
void drm_sched_job_cleanup(struct drm_sched_job *job)
{
dma_fence_put(&job->s_fence->finished);
- job->s_fence = NULL;
+ smp_store_release(&job->s_fence, 0);
}
EXPORT_SYMBOL(drm_sched_job_cleanup);
>> This fixed the problem on the 10 CI runs.
>>
>> Neil
>
>
> These are good news but I still fail to see how this fixes the problem -
> Hillf, do you mind explaining how you came up with this particular fix -
> what was the bug you saw ?
As Steven explained, seems the same job was submitted on both HW slots,
and then when timeout occurs each thread calls panfrost_job_timedout
which leads to drm_sched_stop() on the first call and on the
second call the job was already freed.
Steven proposed a working fix, and this one does the same but on
the drm_sched side. This one looks cleaner, but panfrost should
not call drm_sched_stop() twice for the same job.
Neil
>
> Andrey
>
>>
>>> Does this change help the problem ? Note that drm_sched_job_cleanup is
>>> called from scheduler thread which is stopped at all times when work_tdr
>>> thread is running and anyway the 'bad' job is still in the
>>> ring_mirror_list while it's being accessed from
>>> drm_sched_increase_karma so I don't think drm_sched_job_cleanup can be
>>> called for it BEFORE or while drm_sched_increase_karma is executed.
>>>
>>> Andrey
>>>
>>>
--
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RESEND TRIVIAL 3/3] treewide: arch: Fix Kconfig indentation
Adjust indentation from spaces to tab (+optional two spaces) as in coding style with command like: $ sed -e 's/^/\t/' -i */Kconfig Signed-off-by: Krzysztof Kozlowski --- arch/Kconfig | 4 ++-- arch/alpha/Kconfig | 2 +- arch/arm/Kconfig.debug | 4 ++-- arch/arm/mach-ep93xx/Kconfig | 8 arch/arm/mach-hisi/Kconfig | 2 +- arch/arm/mach-ixp4xx/Kconfig | 16 arch/arm/mach-mmp/Kconfig | 2 +- arch/arm/mach-omap1/Kconfig| 14 +++--- arch/arm/mach-prima2/Kconfig | 6 +++--- arch/arm/mach-s3c24xx/Kconfig | 4 ++-- arch/arm/mach-s3c64xx/Kconfig | 6 +++--- arch/arm/plat-samsung/Kconfig | 2 +- arch/arm64/Kconfig | 6 +++--- arch/arm64/Kconfig.debug | 2 +- arch/h8300/Kconfig | 4 ++-- arch/h8300/Kconfig.cpu | 4 ++-- arch/m68k/Kconfig.bus | 2 +- arch/m68k/Kconfig.debug| 16 arch/m68k/Kconfig.machine | 8 arch/nds32/Kconfig.cpu | 18 +- arch/openrisc/Kconfig | 26 +- arch/powerpc/Kconfig.debug | 18 +- arch/powerpc/platforms/Kconfig.cputype | 2 +- arch/riscv/Kconfig.socs| 2 +- arch/sh/boards/Kconfig | 2 +- arch/sh/mm/Kconfig | 2 +- arch/um/Kconfig| 2 +- arch/x86/Kconfig | 18 +- 28 files changed, 101 insertions(+), 101 deletions(-) diff --git a/arch/Kconfig b/arch/Kconfig index 5f8a5d84dbbe..8d4f77bbed29 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -76,7 +76,7 @@ config JUMP_LABEL depends on HAVE_ARCH_JUMP_LABEL depends on CC_HAS_ASM_GOTO help - This option enables a transparent branch optimization that +This option enables a transparent branch optimization that makes certain almost-always-true or almost-always-false branch conditions even cheaper to execute within the kernel. @@ -84,7 +84,7 @@ config JUMP_LABEL scheduler functionality, networking code and KVM have such branches and include support for this optimization technique. - If it is detected that the compiler has support for "asm goto", +If it is detected that the compiler has support for "asm goto", the kernel will compile such branches with just a nop instruction. When the condition flag is toggled to true, the nop will be converted to a jump instruction to execute the diff --git a/arch/alpha/Kconfig b/arch/alpha/Kconfig index ef179033a7c2..30a6291355cb 100644 --- a/arch/alpha/Kconfig +++ b/arch/alpha/Kconfig @@ -545,7 +545,7 @@ config NR_CPUS default "4" if !ALPHA_GENERIC && !ALPHA_MARVEL help MARVEL support can handle a maximum of 32 CPUs, all the others - with working support have a maximum of 4 CPUs. + with working support have a maximum of 4 CPUs. config ARCH_DISCONTIGMEM_ENABLE bool "Discontiguous Memory Support" diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug index 8bcbd0cd739b..0e5d52fbddbd 100644 --- a/arch/arm/Kconfig.debug +++ b/arch/arm/Kconfig.debug @@ -274,7 +274,7 @@ choice select DEBUG_UART_8250 help Say Y here if you want the debug print routines to direct - their output to the CNS3xxx UART0. + their output to the CNS3xxx UART0. config DEBUG_DAVINCI_DA8XX_UART1 bool "Kernel low-level debugging on DaVinci DA8XX using UART1" @@ -828,7 +828,7 @@ choice select DEBUG_UART_8250 help Say Y here if you want kernel low-level debugging support - on Rockchip RV1108 based platforms. + on Rockchip RV1108 based platforms. config DEBUG_RV1108_UART1 bool "Kernel low-level debugging messages via Rockchip RV1108 UART1" diff --git a/arch/arm/mach-ep93xx/Kconfig b/arch/arm/mach-ep93xx/Kconfig index f2db5fd38145..bf81dfab7f1b 100644 --- a/arch/arm/mach-ep93xx/Kconfig +++ b/arch/arm/mach-ep93xx/Kconfig @@ -126,10 +126,10 @@ config MACH_MICRO9S Contec Micro9-Slim board. config MACH_SIM_ONE -bool "Support Simplemachines Sim.One board" -help - Say 'Y' here if you want your kernel to support the - Simplemachines Sim.One board. + bool "Support Simplemachines Sim.One board" + help + Say 'Y' here if you want your kernel to support the + Simplemachines Sim.One board. config MACH_SNAPPER_CL15 bool "Support Bluewater Systems Snapper CL15 Module" diff --git a/arch/arm/mach-hisi/Kconfig b/arch/arm/mach-hi
[RESEND TRIVIAL 2/3] treewide: Fix Kconfig indentation
Adjust indentation from spaces to tab (+optional two spaces) as in coding style with command like: $ sed -e 's/^/\t/' -i */Kconfig Signed-off-by: Krzysztof Kozlowski --- certs/Kconfig | 14 ++--- init/Kconfig | 28 +- kernel/trace/Kconfig | 8 lib/Kconfig| 2 +- lib/Kconfig.debug | 36 +- lib/Kconfig.kgdb | 8 mm/Kconfig | 28 +- samples/Kconfig| 2 +- security/apparmor/Kconfig | 2 +- security/integrity/Kconfig | 24 +++ security/integrity/ima/Kconfig | 12 ++-- security/safesetid/Kconfig | 24 +++ 12 files changed, 94 insertions(+), 94 deletions(-) diff --git a/certs/Kconfig b/certs/Kconfig index c94e93d8bccf..0358c66d3d7c 100644 --- a/certs/Kconfig +++ b/certs/Kconfig @@ -6,14 +6,14 @@ config MODULE_SIG_KEY default "certs/signing_key.pem" depends on MODULE_SIG help - Provide the file name of a private key/certificate in PEM format, - or a PKCS#11 URI according to RFC7512. The file should contain, or - the URI should identify, both the certificate and its corresponding - private key. +Provide the file name of a private key/certificate in PEM format, +or a PKCS#11 URI according to RFC7512. The file should contain, or +the URI should identify, both the certificate and its corresponding +private key. - If this option is unchanged from its default "certs/signing_key.pem", - then the kernel will automatically generate the private key and - certificate as described in Documentation/admin-guide/module-signing.rst +If this option is unchanged from its default "certs/signing_key.pem", +then the kernel will automatically generate the private key and +certificate as described in Documentation/admin-guide/module-signing.rst config SYSTEM_TRUSTED_KEYRING bool "Provide system-wide ring of trusted keys" diff --git a/init/Kconfig b/init/Kconfig index b4daad2bac23..e1a6f31da281 100644 --- a/init/Kconfig +++ b/init/Kconfig @@ -169,10 +169,10 @@ config BUILD_SALT string "Build ID Salt" default "" help - The build ID is used to link binaries and their debug info. Setting - this option will use the value in the calculation of the build id. - This is mostly useful for distributions which want to ensure the - build is unique between builds. It's safe to leave the default. + The build ID is used to link binaries and their debug info. Setting + this option will use the value in the calculation of the build id. + This is mostly useful for distributions which want to ensure the + build is unique between builds. It's safe to leave the default. config HAVE_KERNEL_GZIP bool @@ -1327,9 +1327,9 @@ menuconfig EXPERT select DEBUG_KERNEL help This option allows certain base kernel options and settings - to be disabled or tweaked. This is for specialized - environments which can tolerate a "non-standard" kernel. - Only use this if you really know what you are doing. + to be disabled or tweaked. This is for specialized + environments which can tolerate a "non-standard" kernel. + Only use this if you really know what you are doing. config UID16 bool "Enable 16-bit UID system calls" if EXPERT @@ -1439,11 +1439,11 @@ config BUG bool "BUG() support" if EXPERT default y help - Disabling this option eliminates support for BUG and WARN, reducing - the size of your kernel image and potentially quietly ignoring - numerous fatal conditions. You should only consider disabling this - option for embedded systems with no facilities for reporting errors. - Just say Y. + Disabling this option eliminates support for BUG and WARN, reducing + the size of your kernel image and potentially quietly ignoring + numerous fatal conditions. You should only consider disabling this + option for embedded systems with no facilities for reporting errors. + Just say Y. config ELF_CORE depends on COREDUMP @@ -1459,8 +1459,8 @@ config PCSPKR_PLATFORM select I8253_LOCK default y help - This option allows to disable the internal PC-Speaker - support, saving some memory. + This option allows to disable the internal PC-Speaker + support, saving some memory. config BASE_FULL default y diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig index e08527f50d2a..0393003f102f 100644 --- a/kernel/trace/Kconfig +++ b/kernel/trace/Kconfig @@ -76,7 +76,7 @
Re: drm_sched with panfrost crash on T820
On 10/3/19 4:34 AM, Neil Armstrong wrote:
> Hi Andrey,
>
> Le 02/10/2019 à 16:40, Grodzovsky, Andrey a écrit :
>> On 9/30/19 10:52 AM, Hillf Danton wrote:
>>> On Mon, 30 Sep 2019 11:17:45 +0200 Neil Armstrong wrote:
Did a new run from 5.3:
[ 35.971972] Call trace:
[ 35.974391] drm_sched_increase_karma+0x5c/0xf0
10667f3810667F94
drivers/gpu/drm/scheduler/sched_main.c:335
The crashing line is :
if (bad->s_fence->scheduled.context ==
entity->fence_context) {
Doesn't seem related to guilty job.
>>> Bail out if s_fence is no longer fresh.
>>>
>>> --- a/drivers/gpu/drm/scheduler/sched_main.c
>>> +++ b/drivers/gpu/drm/scheduler/sched_main.c
>>> @@ -333,6 +333,10 @@ void drm_sched_increase_karma(struct drm
>>>
>>> spin_lock(&rq->lock);
>>> list_for_each_entry_safe(entity, tmp, &rq->entities,
>>> list) {
>>> + if (!smp_load_acquire(&bad->s_fence)) {
>>> + spin_unlock(&rq->lock);
>>> + return;
>>> + }
>>> if (bad->s_fence->scheduled.context ==
>>> entity->fence_context) {
>>> if (atomic_read(&bad->karma) >
>>> @@ -543,7 +547,7 @@ EXPORT_SYMBOL(drm_sched_job_init);
>>>void drm_sched_job_cleanup(struct drm_sched_job *job)
>>>{
>>> dma_fence_put(&job->s_fence->finished);
>>> - job->s_fence = NULL;
>>> + smp_store_release(&job->s_fence, 0);
>>>}
>>>EXPORT_SYMBOL(drm_sched_job_cleanup);
> This fixed the problem on the 10 CI runs.
>
> Neil
These are good news but I still fail to see how this fixes the problem -
Hillf, do you mind explaining how you came up with this particular fix -
what was the bug you saw ?
Andrey
>
>> Does this change help the problem ? Note that drm_sched_job_cleanup is
>> called from scheduler thread which is stopped at all times when work_tdr
>> thread is running and anyway the 'bad' job is still in the
>> ring_mirror_list while it's being accessed from
>> drm_sched_increase_karma so I don't think drm_sched_job_cleanup can be
>> called for it BEFORE or while drm_sched_increase_karma is executed.
>>
>> Andrey
>>
>>
>>>
>>> --
>>>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH TRIVIAL v2] gpu: Fix Kconfig indentation
Adjust indentation from spaces to tab (+optional two spaces) as in coding style with command like: $ sed -e 's/^/\t/' -i */Kconfig Signed-off-by: Krzysztof Kozlowski --- Changes since v1: 1. Fix also DRM_AMD_DC_HDCP (new arrival since v1). --- drivers/gpu/drm/Kconfig | 10 +- drivers/gpu/drm/amd/display/Kconfig | 32 ++--- drivers/gpu/drm/bridge/Kconfig | 8 +- drivers/gpu/drm/i915/Kconfig | 12 +- drivers/gpu/drm/i915/Kconfig.debug | 144 +++ drivers/gpu/drm/lima/Kconfig | 2 +- drivers/gpu/drm/mgag200/Kconfig | 8 +- drivers/gpu/drm/nouveau/Kconfig | 2 +- drivers/gpu/drm/omapdrm/displays/Kconfig | 6 +- drivers/gpu/drm/omapdrm/dss/Kconfig | 12 +- drivers/gpu/drm/rockchip/Kconfig | 8 +- drivers/gpu/drm/udl/Kconfig | 2 +- drivers/gpu/vga/Kconfig | 2 +- 13 files changed, 124 insertions(+), 124 deletions(-) diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index e67c194c2aca..7cb6e4eb99e8 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -207,8 +207,8 @@ config DRM_RADEON tristate "ATI Radeon" depends on DRM && PCI && MMU select FW_LOADER -select DRM_KMS_HELPER -select DRM_TTM + select DRM_KMS_HELPER + select DRM_TTM select POWER_SUPPLY select HWMON select BACKLIGHT_CLASS_DEVICE @@ -226,9 +226,9 @@ config DRM_AMDGPU tristate "AMD GPU" depends on DRM && PCI && MMU select FW_LOADER -select DRM_KMS_HELPER + select DRM_KMS_HELPER select DRM_SCHED -select DRM_TTM + select DRM_TTM select POWER_SUPPLY select HWMON select BACKLIGHT_CLASS_DEVICE @@ -266,7 +266,7 @@ config DRM_VKMS If M is selected the module will be called vkms. config DRM_ATI_PCIGART -bool + bool source "drivers/gpu/drm/exynos/Kconfig" diff --git a/drivers/gpu/drm/amd/display/Kconfig b/drivers/gpu/drm/amd/display/Kconfig index 1bbe762ee6ba..313183b80032 100644 --- a/drivers/gpu/drm/amd/display/Kconfig +++ b/drivers/gpu/drm/amd/display/Kconfig @@ -23,16 +23,16 @@ config DRM_AMD_DC_DCN2_0 depends on DRM_AMD_DC && X86 depends on DRM_AMD_DC_DCN1_0 help - Choose this option if you want to have - Navi support for display engine + Choose this option if you want to have + Navi support for display engine config DRM_AMD_DC_DCN2_1 -bool "DCN 2.1 family" -depends on DRM_AMD_DC && X86 -depends on DRM_AMD_DC_DCN2_0 -help -Choose this option if you want to have -Renoir support for display engine + bool "DCN 2.1 family" + depends on DRM_AMD_DC && X86 + depends on DRM_AMD_DC_DCN2_0 + help + Choose this option if you want to have + Renoir support for display engine config DRM_AMD_DC_DSC_SUPPORT bool "DSC support" @@ -41,16 +41,16 @@ config DRM_AMD_DC_DSC_SUPPORT depends on DRM_AMD_DC_DCN1_0 depends on DRM_AMD_DC_DCN2_0 help - Choose this option if you want to have - Dynamic Stream Compression support + Choose this option if you want to have + Dynamic Stream Compression support config DRM_AMD_DC_HDCP -bool "Enable HDCP support in DC" -depends on DRM_AMD_DC -help - Choose this option - if you want to support - HDCP authentication + bool "Enable HDCP support in DC" + depends on DRM_AMD_DC + help +Choose this option +if you want to support +HDCP authentication config DEBUG_KERNEL_DC bool "Enable kgdb break in DC" diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig index 1cc9f502c1f2..a5aa7ec16000 100644 --- a/drivers/gpu/drm/bridge/Kconfig +++ b/drivers/gpu/drm/bridge/Kconfig @@ -60,10 +60,10 @@ config DRM_MEGACHIPS_STDP_GE_B850V3_FW select DRM_KMS_HELPER select DRM_PANEL ---help--- - This is a driver for the display bridges of - GE B850v3 that convert dual channel LVDS - to DP++. This is used with the i.MX6 imx-ldb - driver. You are likely to say N here. + This is a driver for the display bridges of + GE B850v3 that convert dual channel LVDS + to DP++. This is used with the i.MX6 imx-ldb + driver. You are likely to say N here. config DRM_NXP_PTN3460 tristate "NXP PTN3460 DP/LVDS bridge" diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig index 0d21402945ab..3c6d57df262d 100644 --- a/drivers/gpu/drm/i915/Kconfig +++ b/drivers/gpu/drm/i915/Kconfig @@ -76,7 +76,7 @@ config DRM_I915_CAPTURE_ERROR This option enables capturing the GPU state when a hang is detected. This inf
[PATCH] drm/panfrost: Remove NULL check for regulator
devm_regulator_get() is used to populate pfdev->regulator which ensures
that this cannot be NULL (a dummy regulator will be returned if
necessary). So remove the check in panfrost_devfreq_target().
Signed-off-by: Steven Price
---
This looks like it was accidentally reintroduced by the merge from
drm-next into drm-misc-next due to the duplication of "drm/panfrost: Add
missing check for pfdev-regulator" (commits c90f30812a79 and
52282163dfa6).
---
drivers/gpu/drm/panfrost/panfrost_devfreq.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
index c1eb8cfe6aeb..12ff77dacc95 100644
--- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
+++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
@@ -53,10 +53,8 @@ static int panfrost_devfreq_target(struct device *dev,
unsigned long *freq,
if (err) {
dev_err(dev, "Cannot set frequency %lu (%d)\n", target_rate,
err);
- if (pfdev->regulator)
- regulator_set_voltage(pfdev->regulator,
- pfdev->devfreq.cur_volt,
- pfdev->devfreq.cur_volt);
+ regulator_set_voltage(pfdev->regulator, pfdev->devfreq.cur_volt,
+ pfdev->devfreq.cur_volt);
return err;
}
--
2.20.1
Re: Should regulator core support parsing OF based fwnode?
On Fri, Oct 04, 2019 at 03:33:13PM +0200, Jean-Jacques Hiblot wrote: > On 04/10/2019 13:39, Mark Brown wrote: > > Consumers should just be able to request a regulator without having to > > worry about how that's being provided - they should have no knowledge at > > all of firmware bindings or platform data for defining this. If they > > do that suggests there's an abstraction issue somewhere, what makes you > > think that doing something with of_node is required? > The regulator core accesses consumer->of_node to get a phandle to a > regulator's node. The trouble arises from the fact that the LED core does > not populate of_node anymore, instead it populates fwnode. This allows the > LED core to be agnostic of ACPI or OF to get the properties of a LED. Why is the LED core populating anything? Is the LED core copying bits out of the struct device for the actual device into a synthetic device rather than passing the actual device in? That really doesn't seem like a good idea, it's likely to lead to things like this where you don't copy something that's required (or worse where something directly in the struct device that can't be copied is needed). > IMO it is better to populate both of_node and fwnode in the LED core at the > moment. It has already been fixed this way for the platform driver [0], MTD > [1] and PCI-OF [2]. Yeah, if you're going to be copying stuff out of the real device I'd copy the of_node as well. > > Further, unless you have LEDs that work without power you probably > > shouldn't be using _get_optional() for their supply. That interface is > > intended only for supplies that may be physically absent. > Not all LEDs have a regulator to provide the power. The power can be > supplied by the LED controller for example. This code probably shouldn't be being run at all for LEDs like that, I was assuming this was just for GPIO LEDs and similar rather than all LEDs. signature.asc Description: PGP signature
Re: [PATCH v7 5/5] backlight: add led-backlight driver
On Wed, 18 Sep 2019, Jean-Jacques Hiblot wrote: > From: Tomi Valkeinen > > This patch adds a led-backlight driver (led_bl), which is similar to > pwm_bl except the driver uses a LED class driver to adjust the > brightness in the HW. Multiple LEDs can be used for a single backlight. > > Signed-off-by: Tomi Valkeinen > Signed-off-by: Jean-Jacques Hiblot > Acked-by: Pavel Machek > Reviewed-by: Daniel Thompson > --- > drivers/video/backlight/Kconfig | 7 + > drivers/video/backlight/Makefile | 1 + > drivers/video/backlight/led_bl.c | 260 +++ > 3 files changed, 268 insertions(+) > create mode 100644 drivers/video/backlight/led_bl.c Applied, thanks. -- Lee Jones [李琼斯] Linaro Services Technical Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v7 4/5] dt-bindings: backlight: Add led-backlight binding
On Wed, 18 Sep 2019, Jean-Jacques Hiblot wrote: > Add DT binding for led-backlight. > > Signed-off-by: Jean-Jacques Hiblot > Reviewed-by: Daniel Thompson > --- > .../bindings/leds/backlight/led-backlight.txt | 28 +++ > 1 file changed, 28 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/leds/backlight/led-backlight.txt Applied, thanks. -- Lee Jones [李琼斯] Linaro Services Technical Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/3] drm/komeda: Memory manage struct komeda_drv in probe/remove
Some fields of komeda_drv members will be useful very early
in probe code, so make sure an instance is available.
Signed-off-by: Mihail Atanassov
---
.../gpu/drm/arm/display/komeda/komeda_drv.c | 30 +++
1 file changed, 17 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_drv.c
b/drivers/gpu/drm/arm/display/komeda/komeda_drv.c
index 660181bdc008..9ed25ffe0e22 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_drv.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_drv.c
@@ -32,22 +32,15 @@ static void komeda_unbind(struct device *dev)
if (!mdrv)
return;
- komeda_kms_fini(mdrv->kms);
- komeda_dev_fini(mdrv->mdev);
-
- dev_set_drvdata(dev, NULL);
- devm_kfree(dev, mdrv);
+ komeda_kms_fini(&mdrv->kms);
+ komeda_dev_fini(&mdrv->mdev);
}
static int komeda_bind(struct device *dev)
{
- struct komeda_drv *mdrv;
+ struct komeda_drv *mdrv = dev_get_drvdata(dev);
int err;
- mdrv = devm_kzalloc(dev, sizeof(*mdrv), GFP_KERNEL);
- if (!mdrv)
- return -ENOMEM;
-
err = komeda_dev_init(&mdrv->mdev, dev);
if (err)
goto free_mdrv;
@@ -56,8 +49,6 @@ static int komeda_bind(struct device *dev)
if (err)
goto fini_mdev;
- dev_set_drvdata(dev, mdrv);
-
return 0;
fini_mdev:
@@ -97,10 +88,15 @@ static int komeda_platform_probe(struct platform_device
*pdev)
struct device *dev = &pdev->dev;
struct component_match *match = NULL;
struct device_node *child;
+ struct komeda_drv *mdrv;
if (!dev->of_node)
return -ENODEV;
+ mdrv = devm_kzalloc(dev, sizeof(*mdrv), GFP_KERNEL);
+ if (!mdrv)
+ return -ENOMEM;
+
for_each_available_child_of_node(dev->of_node, child) {
if (of_node_cmp(child->name, "pipeline") != 0)
continue;
@@ -110,12 +106,20 @@ static int komeda_platform_probe(struct platform_device
*pdev)
komeda_add_slave(dev, &match, child, KOMEDA_OF_PORT_OUTPUT, 1);
}
+ dev_set_drvdata(dev, mdrv);
+
return component_master_add_with_match(dev, &komeda_master_ops, match);
}
static int komeda_platform_remove(struct platform_device *pdev)
{
- component_master_del(&pdev->dev, &komeda_master_ops);
+ struct device *dev = &pdev->dev;
+ struct komeda_drv *mdrv = dev_get_drvdata(dev);
+
+ component_master_del(dev, &komeda_master_ops);
+
+ dev_set_drvdata(dev, NULL);
+ devm_kfree(dev, mdrv);
return 0;
}
--
2.23.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/3] drm/komeda: Consolidate struct komeda_drv allocations
struct komeda_drv has two pointer members only, and both
it and its members get allocated separately. Since both
members' types are in scope, there's not a lot to gain by
keeping the indirection around.
To avoid double-free issues, provide a barebones ->release()
hook for the driver.
Signed-off-by: Mihail Atanassov
---
.../gpu/drm/arm/display/komeda/komeda_dev.c | 21 ---
.../gpu/drm/arm/display/komeda/komeda_dev.h | 4 +--
.../gpu/drm/arm/display/komeda/komeda_drv.c | 36 +--
.../gpu/drm/arm/display/komeda/komeda_kms.c | 26 --
.../gpu/drm/arm/display/komeda/komeda_kms.h | 4 +--
5 files changed, 42 insertions(+), 49 deletions(-)
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
index 937a6d4c4865..c84f978cacc3 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
@@ -179,28 +179,23 @@ static int komeda_parse_dt(struct device *dev, struct
komeda_dev *mdev)
return ret;
}
-struct komeda_dev *komeda_dev_create(struct device *dev)
+int komeda_dev_init(struct komeda_dev *mdev, struct device *dev)
{
struct platform_device *pdev = to_platform_device(dev);
const struct komeda_product_data *product;
- struct komeda_dev *mdev;
struct resource *io_res;
int err = 0;
product = of_device_get_match_data(dev);
if (!product)
- return ERR_PTR(-ENODEV);
+ return -ENODEV;
io_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!io_res) {
DRM_ERROR("No registers defined.\n");
- return ERR_PTR(-ENODEV);
+ return -ENODEV;
}
- mdev = devm_kzalloc(dev, sizeof(*mdev), GFP_KERNEL);
- if (!mdev)
- return ERR_PTR(-ENOMEM);
-
mutex_init(&mdev->lock);
mdev->dev = dev;
@@ -284,16 +279,16 @@ struct komeda_dev *komeda_dev_create(struct device *dev)
komeda_debugfs_init(mdev);
#endif
- return mdev;
+ return 0;
disable_clk:
clk_disable_unprepare(mdev->aclk);
err_cleanup:
- komeda_dev_destroy(mdev);
- return ERR_PTR(err);
+ komeda_dev_fini(mdev);
+ return err;
}
-void komeda_dev_destroy(struct komeda_dev *mdev)
+void komeda_dev_fini(struct komeda_dev *mdev)
{
struct device *dev = mdev->dev;
const struct komeda_dev_funcs *funcs = mdev->funcs;
@@ -335,8 +330,6 @@ void komeda_dev_destroy(struct komeda_dev *mdev)
devm_clk_put(dev, mdev->aclk);
mdev->aclk = NULL;
}
-
- devm_kfree(dev, mdev);
}
int komeda_dev_resume(struct komeda_dev *mdev)
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
b/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
index 414200233b64..e392b8ffc372 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
@@ -213,8 +213,8 @@ komeda_product_match(struct komeda_dev *mdev, u32 target)
const struct komeda_dev_funcs *
d71_identify(u32 __iomem *reg, struct komeda_chip_info *chip);
-struct komeda_dev *komeda_dev_create(struct device *dev);
-void komeda_dev_destroy(struct komeda_dev *mdev);
+int komeda_dev_init(struct komeda_dev *mdev, struct device *dev);
+void komeda_dev_fini(struct komeda_dev *mdev);
struct komeda_dev *dev_to_mdev(struct device *dev);
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_drv.c
b/drivers/gpu/drm/arm/display/komeda/komeda_drv.c
index d6cc5d33..660181bdc008 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_drv.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_drv.c
@@ -14,15 +14,15 @@
#include "komeda_kms.h"
struct komeda_drv {
- struct komeda_dev *mdev;
- struct komeda_kms_dev *kms;
+ struct komeda_dev mdev;
+ struct komeda_kms_dev kms;
};
struct komeda_dev *dev_to_mdev(struct device *dev)
{
struct komeda_drv *mdrv = dev_get_drvdata(dev);
- return mdrv ? mdrv->mdev : NULL;
+ return mdrv ? &mdrv->mdev : NULL;
}
static void komeda_unbind(struct device *dev)
@@ -32,8 +32,8 @@ static void komeda_unbind(struct device *dev)
if (!mdrv)
return;
- komeda_kms_detach(mdrv->kms);
- komeda_dev_destroy(mdrv->mdev);
+ komeda_kms_fini(mdrv->kms);
+ komeda_dev_fini(mdrv->mdev);
dev_set_drvdata(dev, NULL);
devm_kfree(dev, mdrv);
@@ -48,24 +48,20 @@ static int komeda_bind(struct device *dev)
if (!mdrv)
return -ENOMEM;
- mdrv->mdev = komeda_dev_create(dev);
- if (IS_ERR(mdrv->mdev)) {
- err = PTR_ERR(mdrv->mdev);
+ err = komeda_dev_init(&mdrv->mdev, dev);
+ if (err)
goto free_mdrv;
- }
- mdrv->kms = komeda_kms_attach(mdrv->mdev);
- if (IS_ERR(mdrv->kms)) {
- err = PTR_ERR(mdrv->kms)
[PATCH 0/3] drm/komeda: Support for drm_bridge endpoints
Greetings, This series attempts to add support for endpoints (in the DT sense) whose drivers only have a drm_bridge interface, unlike the tda998x, which uses the component framework and provides all of drm_encoder, drm_bridge, drm_connector. 1 & 2 should be fairly non-contentious, and I believe are valuable enough on their own as they remove some pointer chasing and a few allocations at the top level of komeda. 3 is the meat of this series, where I add the drm_bridge endpoint code. This was tested with ti_tfp410 on the back end of a D71. I've tagged it with an RFC since I drew inspiration from tilcdc, which does similar shenanigans to detect tda998x vs non-tda998x, and is hence a precedent for including similar handling, but I wasn't sure if there's a more well established pattern. Note that I opted not to remove the previous way of doing things for tda998x, even though it could work with the drm_bridge handling directly, since AFAIUI, device links haven't been implemented for drm_bridge (see [1] for an attempt at doing that that never landed, and I'm not aware of any refcounting having been added since), and therefore getting a drm_bridge driver rmmod'ed while in use would be Bad(tm). [1] https://lore.kernel.org/lkml/20180426223139.16740-1-p...@axentia.se/ Cc: Liviu Dudau Cc: Brian Starkey Cc: James (Qian) Wang Cc: Daniel Vetter Cc: David Airlie Cc: Maxime Ripard Cc: Maarten Lankhorst Cc: Sean Paul Mihail Atanassov (3): drm/komeda: Consolidate struct komeda_drv allocations drm/komeda: Memory manage struct komeda_drv in probe/remove drm/komeda: Allow non-component drm_bridge only endpoints .../gpu/drm/arm/display/komeda/komeda_dev.c | 21 +-- .../gpu/drm/arm/display/komeda/komeda_dev.h | 9 +- .../gpu/drm/arm/display/komeda/komeda_drv.c | 118 - .../gpu/drm/arm/display/komeda/komeda_kms.c | 159 -- .../gpu/drm/arm/display/komeda/komeda_kms.h | 9 +- 5 files changed, 243 insertions(+), 73 deletions(-) -- 2.23.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC PATCH 3/3] drm/komeda: Allow non-component drm_bridge only endpoints
To support transmitters other than the tda998x, we need to allow
non-component framework bridges to be attached to a dummy drm_encoder in
our driver.
For the existing supported encoder (tda998x), keep the behaviour as-is,
since there's no way to unbind if a drm_bridge module goes away under
our feet.
Signed-off-by: Mihail Atanassov
---
.../gpu/drm/arm/display/komeda/komeda_dev.h | 5 +
.../gpu/drm/arm/display/komeda/komeda_drv.c | 58 ++--
.../gpu/drm/arm/display/komeda/komeda_kms.c | 133 +-
.../gpu/drm/arm/display/komeda/komeda_kms.h | 5 +
4 files changed, 187 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
b/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
index e392b8ffc372..64d2902e2e0c 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
@@ -176,6 +176,11 @@ struct komeda_dev {
/** @irq: irq number */
int irq;
+ /** @has_components:
+*
+* if true, use the component framework to bind/unbind driver
+*/
+ bool has_components;
/** @lock: used to protect dpmode */
struct mutex lock;
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_drv.c
b/drivers/gpu/drm/arm/display/komeda/komeda_drv.c
index 9ed25ffe0e22..34cfc6a4c3e4 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_drv.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_drv.c
@@ -10,6 +10,8 @@
#include
#include
#include
+#include
+#include
#include "komeda_dev.h"
#include "komeda_kms.h"
@@ -69,18 +71,35 @@ static int compare_of(struct device *dev, void *data)
return dev->of_node == data;
}
-static void komeda_add_slave(struct device *master,
-struct component_match **match,
-struct device_node *np,
-u32 port, u32 endpoint)
+static int komeda_add_slave(struct device *master,
+ struct komeda_drv *mdrv,
+ struct component_match **match,
+ struct device_node *np,
+ u32 port, u32 endpoint)
{
struct device_node *remote;
+ struct drm_bridge *bridge;
+ int ret = 0;
remote = of_graph_get_remote_node(np, port, endpoint);
- if (remote) {
+ if (!remote)
+ return 0;
+
+ if (komeda_remote_device_is_component(np, port, endpoint)) {
+ mdrv->mdev.has_components = true;
drm_of_component_match_add(master, match, compare_of, remote);
- of_node_put(remote);
+ goto exit;
+ }
+
+ bridge = of_drm_find_bridge(remote);
+ if (!bridge) {
+ ret = -EPROBE_DEFER;
+ goto exit;
}
+
+exit:
+ of_node_put(remote);
+ return ret;
}
static int komeda_platform_probe(struct platform_device *pdev)
@@ -89,6 +108,7 @@ static int komeda_platform_probe(struct platform_device
*pdev)
struct component_match *match = NULL;
struct device_node *child;
struct komeda_drv *mdrv;
+ int ret;
if (!dev->of_node)
return -ENODEV;
@@ -101,14 +121,27 @@ static int komeda_platform_probe(struct platform_device
*pdev)
if (of_node_cmp(child->name, "pipeline") != 0)
continue;
- /* add connector */
- komeda_add_slave(dev, &match, child, KOMEDA_OF_PORT_OUTPUT, 0);
- komeda_add_slave(dev, &match, child, KOMEDA_OF_PORT_OUTPUT, 1);
+ /* attach any remote devices if present */
+ ret = komeda_add_slave(dev, mdrv, &match, child,
+ KOMEDA_OF_PORT_OUTPUT, 0);
+ if (ret)
+ goto free_mdrv;
+ ret = komeda_add_slave(dev, mdrv, &match, child,
+ KOMEDA_OF_PORT_OUTPUT, 1);
+ if (ret)
+ goto free_mdrv;
}
dev_set_drvdata(dev, mdrv);
- return component_master_add_with_match(dev, &komeda_master_ops, match);
+ return mdrv->mdev.has_components
+ ? component_master_add_with_match(
+ dev, &komeda_master_ops, match)
+ : komeda_bind(dev);
+
+free_mdrv:
+ devm_kfree(dev, mdrv);
+ return ret;
}
static int komeda_platform_remove(struct platform_device *pdev)
@@ -116,7 +149,10 @@ static int komeda_platform_remove(struct platform_device
*pdev)
struct device *dev = &pdev->dev;
struct komeda_drv *mdrv = dev_get_drvdata(dev);
- component_master_del(dev, &komeda_master_ops);
+ if (mdrv->mdev.has_components)
+ component_master_del(dev, &komeda_master_ops);
+ else
+ komeda_unbind(dev);
dev_set_drvdata(dev, NULL);
devm_kfree(dev, mdrv);
dif
Re: [PATCH v3] drm/fourcc: Add Arm 16x16 block modifier
On Fri, Oct 04, 2019 at 02:12:38PM +, Ayan Halder wrote:
> From: Raymond Smith
>
> Add the DRM_FORMAT_MOD_ARM_16X16_BLOCK_U_INTERLEAVED modifier to
> denote the 16x16 block u-interleaved format used in Arm Utgard and
> Midgard GPUs.
>
> Changes from v1:-
> 1. Reserved the upper four bits (out of the 56 bits assigned to each vendor)
> to denote the category of Arm specific modifiers. Currently, we have two
> categories ie AFBC and MISC.
>
> Changes from v2:-
> 1. Preserved Ray's authorship
> 2. Cleanups/changes suggested by Brian
> 3. Added r-bs of Brian and Qiang
>
> Signed-off-by: Raymond Smith
> Signed-off-by: Ayan kumar halder
> Reviewed-by: Brian Starkey
> Reviewed-by: Qiang Yu
Pushed to drm-misc-next - ba2a1c8706151ac3234d2d020873feab498ab1bb
> ---
> include/uapi/drm/drm_fourcc.h | 26 +-
> 1 file changed, 25 insertions(+), 1 deletion(-)
>
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index 3feeaa3f987a..2376d36ea573 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -648,7 +648,21 @@ extern "C" {
> * Further information on the use of AFBC modifiers can be found in
> * Documentation/gpu/afbc.rst
> */
> -#define DRM_FORMAT_MOD_ARM_AFBC(__afbc_mode) fourcc_mod_code(ARM,
> __afbc_mode)
> +
> +/*
> + * The top 4 bits (out of the 56 bits alloted for specifying vendor specific
> + * modifiers) denote the category for modifiers. Currently we have only two
> + * categories of modifiers ie AFBC and MISC. We can have a maximum of sixteen
> + * different categories.
> + */
> +#define DRM_FORMAT_MOD_ARM_CODE(__type, __val) \
> + fourcc_mod_code(ARM, ((__u64)(__type) << 52) | ((__val) &
> 0x000fULL))
> +
> +#define DRM_FORMAT_MOD_ARM_TYPE_AFBC 0x00
> +#define DRM_FORMAT_MOD_ARM_TYPE_MISC 0x01
> +
> +#define DRM_FORMAT_MOD_ARM_AFBC(__afbc_mode) \
> + DRM_FORMAT_MOD_ARM_CODE(DRM_FORMAT_MOD_ARM_TYPE_AFBC, __afbc_mode)
>
> /*
> * AFBC superblock size
> @@ -742,6 +756,16 @@ extern "C" {
> */
> #define AFBC_FORMAT_MOD_BCH (1ULL << 11)
>
> +/*
> + * Arm 16x16 Block U-Interleaved modifier
> + *
> + * This is used by Arm Mali Utgard and Midgard GPUs. It divides the image
> + * into 16x16 pixel blocks. Blocks are stored linearly in order, but pixels
> + * in the block are reordered.
> + */
> +#define DRM_FORMAT_MOD_ARM_16X16_BLOCK_U_INTERLEAVED \
> + DRM_FORMAT_MOD_ARM_CODE(DRM_FORMAT_MOD_ARM_TYPE_MISC, 1ULL)
> +
> /*
> * Allwinner tiled modifier
> *
> --
> 2.23.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/drm_edid: correct VIC and HDMI_VIC under HDMI 2.0
On Fri, Oct 04, 2019 at 10:41:20AM +, Lin, Wayne wrote: > > > > From: Ville Syrjälä > Sent: Thursday, October 3, 2019 21:29 > To: Lin, Wayne > Cc: dri-devel@lists.freedesktop.org ; > amd-...@lists.freedesktop.org ; Li, Sun peng > (Leo) ; Kazlauskas, Nicholas > Subject: Re: [PATCH] drm/drm_edid: correct VIC and HDMI_VIC under HDMI 2.0 > > On Thu, Oct 03, 2019 at 06:49:05AM +, Lin, Wayne wrote: > > > > > > > > From: Ville Syrjälä > > Sent: Wednesday, October 2, 2019 19:58 > > To: Lin, Wayne > > Cc: dri-devel@lists.freedesktop.org ; > > amd-...@lists.freedesktop.org ; Li, Sun peng > > (Leo) ; Kazlauskas, Nicholas > > > > Subject: Re: [PATCH] drm/drm_edid: correct VIC and HDMI_VIC under HDMI 2.0 > > > > On Tue, Sep 24, 2019 at 01:26:21PM +0800, Wayne Lin wrote: > > > In HDMI 1.4 defines 4k modes without specific aspect ratio. > > > However, in HDMI 2.0, adds aspect ratio attribute to distinguish different > > > 4k modes. > > > > > > According to Appendix E of HDMI 2.0 spec, source should use VSIF to > > > indicate VIC mode only when the mode is one defined in HDMI 1.4b 4K modes. > > > Otherwise, use AVI infoframes to convey VIC. > > > > > > eg: VIC_103 should use AVI infoframes and VIC_93 use VSIF > > > > > > When the sink is HDMI 2.0, current code in > > > drm_hdmi_avi_infoframe_from_display_mode will also force mode VIC_103 to > > > have VIC value 0. This violates the spec and needs to be corrected. > > > > > Where is that being done? We only set the AVI VIC to zero if we're going > > > to use the HDMI VIC instead. > > > > Appreciate for your time and apologize for not explaining it clearly. > > Current code in drm_hdmi_avi_infoframe_from_display_mode() will call > > drm_match_hdmi_mode() to set up vendor_if_vic. By checking > > drm_valid_hdmi_vic(vendor_if_vic) to see if the vic info should be conveyed > > by avi > > or not. > > > > But in drm_match_hdmi_mode(), code doesn't enable match_flags with > > DRM_MODE_MATCH_ASPECT_RATIO. I think it's due to HDMI1.4b doesn't specify > > 4K mode conveyed by HDMI VIC with particular aspect ratio. But in Appendix > > E of > > HDMI 2.0 spec, it specify only 4k modes with particular aspect ratio should > > use VSIF to convey. > > Hence, when the sink support HDMI 2.0 and set the mode to be VIC_103, > > calling > > drm_match_hdmi_mode(mode) will return vendor_if_vic = 3 (VIC_93 and VIC_103 > > are having > > the same timing but different aspect ratio). Thereafter will set the > > frame->video_code to 0. > > However, VIC_103 should use AVI VIC according to HDMI 2.0 spec (only VIC: > > 93, 94, 95 & > > 98 should use VSIF). > > > > This patch try to revise that, when the sink support HDMI 2.0, > > drm_match_hdmi_mode() > > should also take aspect ratio into consideration. > > But for easy reading, I add another function "drm_match_hdmi_1_4_mode" to > > do so. > > > Seems rather convoluted. I think we should just add the aspect ratios > > to edid_4k_modes[]. Or is there some problem with that approach? > > Thanks for your time. > > Since HDMI 1.4b doesn't require edid_4k_modes[] with specific aspect ratio, > modes as the same > timing in edid_4k_modes[] but with different aspect ratios are also expected > to convey VIC by > VSIF to HDMI 1.4 sink. Might can't guarantee that there wont't be any > compatibility side effect with > that approach when the sink is HDMI 1.4b . I think adding them should be fine. But while checking the existing code I noticed a few problems, so I sent out some fixes (cc:d you). -- Ville Syrjälä Intel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/4] drm/edid: Make drm_get_cea_aspect_ratio() static
From: Ville Syrjälä
drm_get_cea_aspect_ratio() is not used outside drm_edid.c.
Make it static.
Cc: Wayne Lin
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/drm_edid.c | 10 +-
include/drm/drm_edid.h | 1 -
2 files changed, 1 insertion(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 0552175313cb..3df8267adab9 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3205,18 +3205,10 @@ static bool drm_valid_cea_vic(u8 vic)
return vic > 0 && vic < ARRAY_SIZE(edid_cea_modes);
}
-/**
- * drm_get_cea_aspect_ratio - get the picture aspect ratio corresponding to
- * the input VIC from the CEA mode list
- * @video_code: ID given to each of the CEA modes
- *
- * Returns picture aspect ratio
- */
-enum hdmi_picture_aspect drm_get_cea_aspect_ratio(const u8 video_code)
+static enum hdmi_picture_aspect drm_get_cea_aspect_ratio(const u8 video_code)
{
return edid_cea_modes[video_code].picture_aspect_ratio;
}
-EXPORT_SYMBOL(drm_get_cea_aspect_ratio);
/*
* Calculate the alternate clock for HDMI modes (those from the HDMI vendor
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index b9719418c3d2..efce675abf07 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -481,7 +481,6 @@ int drm_add_edid_modes(struct drm_connector *connector,
struct edid *edid);
int drm_add_override_edid_modes(struct drm_connector *connector);
u8 drm_match_cea_mode(const struct drm_display_mode *to_match);
-enum hdmi_picture_aspect drm_get_cea_aspect_ratio(const u8 video_code);
bool drm_detect_hdmi_monitor(struct edid *edid);
bool drm_detect_monitor_audio(struct edid *edid);
enum hdmi_quantization_range
--
2.21.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/4] drm/edid: Fix HDMI VIC handling
From: Ville Syrjälä
Extract drm_mode_hdmi_vic() to correctly calculate the final HDMI
VIC for us. Currently this is being done a bit differently between
the AVI and HDMI infoframes. Let's get both to agree on this.
We need to allow the case where a mode is both 3D and has a HDMI
VIC. Currently we'll just refuse to generate the HDMI infoframe when
we really should be setting HDMI VIC to 0 and instead enabling 3D
stereo signalling.
If the sink doesn't even support the HDMI infoframe we should
not be picking the HDMI VIC in favor of the CEA VIC, because then
we'll end up not sending either VIC in the end.
Cc: Wayne Lin
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/drm_edid.c | 37 +
1 file changed, 21 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 495b7fb4d9ef..c7f9f7ca75a2 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -5160,11 +5160,25 @@ drm_hdmi_infoframe_set_hdr_metadata(struct
hdmi_drm_infoframe *frame,
}
EXPORT_SYMBOL(drm_hdmi_infoframe_set_hdr_metadata);
+static u8 drm_mode_hdmi_vic(struct drm_connector *connector,
+ const struct drm_display_mode *mode)
+{
+ bool has_hdmi_infoframe = connector ?
+ connector->display_info.has_hdmi_infoframe : false;
+
+ if (!has_hdmi_infoframe)
+ return 0;
+
+ /* No HDMI VIC when signalling 3D video format */
+ if (mode->flags & DRM_MODE_FLAG_3D_MASK)
+ return 0;
+
+ return drm_match_hdmi_mode(mode);
+}
+
static u8 drm_mode_cea_vic(struct drm_connector *connector,
const struct drm_display_mode *mode)
{
- u8 vendor_if_vic = drm_match_hdmi_mode(mode);
- bool is_s3d = mode->flags & DRM_MODE_FLAG_3D_MASK;
u8 vic;
/*
@@ -5173,7 +5187,7 @@ static u8 drm_mode_cea_vic(struct drm_connector
*connector,
* VIC in AVI infoframes. Lets check if this mode is present in
* HDMI 1.4b 4K modes
*/
- if (drm_valid_hdmi_vic(vendor_if_vic) && !is_s3d)
+ if (drm_mode_hdmi_vic(connector, mode))
return 0;
vic = drm_match_cea_mode(mode);
@@ -5433,8 +5447,6 @@ drm_hdmi_vendor_infoframe_from_display_mode(struct
hdmi_vendor_infoframe *frame,
bool has_hdmi_infoframe = connector ?
connector->display_info.has_hdmi_infoframe : false;
int err;
- u32 s3d_flags;
- u8 vic;
if (!frame || !mode)
return -EINVAL;
@@ -5442,8 +5454,9 @@ drm_hdmi_vendor_infoframe_from_display_mode(struct
hdmi_vendor_infoframe *frame,
if (!has_hdmi_infoframe)
return -EINVAL;
- vic = drm_match_hdmi_mode(mode);
- s3d_flags = mode->flags & DRM_MODE_FLAG_3D_MASK;
+ err = hdmi_vendor_infoframe_init(frame);
+ if (err < 0)
+ return err;
/*
* Even if it's not absolutely necessary to send the infoframe
@@ -5454,15 +5467,7 @@ drm_hdmi_vendor_infoframe_from_display_mode(struct
hdmi_vendor_infoframe *frame,
* mode if the source simply stops sending the infoframe when
* it wants to switch from 3D to 2D.
*/
-
- if (vic && s3d_flags)
- return -EINVAL;
-
- err = hdmi_vendor_infoframe_init(frame);
- if (err < 0)
- return err;
-
- frame->vic = vic;
+ frame->vic = drm_mode_hdmi_vic(connector, mode);
frame->s3d_struct = s3d_structure_from_display_mode(mode);
return 0;
--
2.21.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/4] drm/edid: Extract drm_mode_cea_vic()
From: Ville Syrjälä
Extract the logic to compute the final CEA VIC to a small helper.
We'll reorder it a bit to make future modifications more
straightforward. No function changes.
Cc: Wayne Lin
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/drm_edid.c | 53 +-
1 file changed, 30 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 3df8267adab9..495b7fb4d9ef 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -5160,6 +5160,35 @@ drm_hdmi_infoframe_set_hdr_metadata(struct
hdmi_drm_infoframe *frame,
}
EXPORT_SYMBOL(drm_hdmi_infoframe_set_hdr_metadata);
+static u8 drm_mode_cea_vic(struct drm_connector *connector,
+ const struct drm_display_mode *mode)
+{
+ u8 vendor_if_vic = drm_match_hdmi_mode(mode);
+ bool is_s3d = mode->flags & DRM_MODE_FLAG_3D_MASK;
+ u8 vic;
+
+ /*
+* HDMI spec says if a mode is found in HDMI 1.4b 4K modes
+* we should send its VIC in vendor infoframes, else send the
+* VIC in AVI infoframes. Lets check if this mode is present in
+* HDMI 1.4b 4K modes
+*/
+ if (drm_valid_hdmi_vic(vendor_if_vic) && !is_s3d)
+ return 0;
+
+ vic = drm_match_cea_mode(mode);
+
+ /*
+* HDMI 1.4 VIC range: 1 <= VIC <= 64 (CEA-861-D) but
+* HDMI 2.0 VIC range: 1 <= VIC <= 107 (CEA-861-F). So we
+* have to make sure we dont break HDMI 1.4 sinks.
+*/
+ if (!is_hdmi2_sink(connector) && vic > 64)
+ return 0;
+
+ return vic;
+}
+
/**
* drm_hdmi_avi_infoframe_from_display_mode() - fill an HDMI AVI infoframe with
* data from a DRM display mode
@@ -5187,29 +5216,7 @@ drm_hdmi_avi_infoframe_from_display_mode(struct
hdmi_avi_infoframe *frame,
if (mode->flags & DRM_MODE_FLAG_DBLCLK)
frame->pixel_repeat = 1;
- frame->video_code = drm_match_cea_mode(mode);
-
- /*
-* HDMI 1.4 VIC range: 1 <= VIC <= 64 (CEA-861-D) but
-* HDMI 2.0 VIC range: 1 <= VIC <= 107 (CEA-861-F). So we
-* have to make sure we dont break HDMI 1.4 sinks.
-*/
- if (!is_hdmi2_sink(connector) && frame->video_code > 64)
- frame->video_code = 0;
-
- /*
-* HDMI spec says if a mode is found in HDMI 1.4b 4K modes
-* we should send its VIC in vendor infoframes, else send the
-* VIC in AVI infoframes. Lets check if this mode is present in
-* HDMI 1.4b 4K modes
-*/
- if (frame->video_code) {
- u8 vendor_if_vic = drm_match_hdmi_mode(mode);
- bool is_s3d = mode->flags & DRM_MODE_FLAG_3D_MASK;
-
- if (drm_valid_hdmi_vic(vendor_if_vic) && !is_s3d)
- frame->video_code = 0;
- }
+ frame->video_code = drm_mode_cea_vic(connector, mode);
frame->picture_aspect = HDMI_PICTURE_ASPECT_NONE;
--
2.21.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/4] drm/edid: Prep for HDMI VIC aspect ratio (WIP)
From: Ville Syrjälä
I think this should provide most of necessary logic for
adding aspecr ratios to the HDMI 4k modes.
Cc: Wayne Lin
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/drm_edid.c | 37 +++--
1 file changed, 31 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index c7f9f7ca75a2..c76814edc784 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3210,6 +3210,11 @@ static enum hdmi_picture_aspect
drm_get_cea_aspect_ratio(const u8 video_code)
return edid_cea_modes[video_code].picture_aspect_ratio;
}
+static enum hdmi_picture_aspect drm_get_hdmi_aspect_ratio(const u8 video_code)
+{
+ return edid_4k_modes[video_code].picture_aspect_ratio;
+}
+
/*
* Calculate the alternate clock for HDMI modes (those from the HDMI vendor
* specific block).
@@ -3236,6 +3241,9 @@ static u8 drm_match_hdmi_mode_clock_tolerance(const
struct drm_display_mode *to_
if (!to_match->clock)
return 0;
+ if (to_match->picture_aspect_ratio)
+ match_flags |= DRM_MODE_MATCH_ASPECT_RATIO;
+
for (vic = 1; vic < ARRAY_SIZE(edid_4k_modes); vic++) {
const struct drm_display_mode *hdmi_mode = &edid_4k_modes[vic];
unsigned int clock1, clock2;
@@ -3271,6 +3279,9 @@ static u8 drm_match_hdmi_mode(const struct
drm_display_mode *to_match)
if (!to_match->clock)
return 0;
+ if (to_match->picture_aspect_ratio)
+ match_flags |= DRM_MODE_MATCH_ASPECT_RATIO;
+
for (vic = 1; vic < ARRAY_SIZE(edid_4k_modes); vic++) {
const struct drm_display_mode *hdmi_mode = &edid_4k_modes[vic];
unsigned int clock1, clock2;
@@ -5218,6 +5229,7 @@ drm_hdmi_avi_infoframe_from_display_mode(struct
hdmi_avi_infoframe *frame,
const struct drm_display_mode *mode)
{
enum hdmi_picture_aspect picture_aspect;
+ u8 vic, hdmi_vic;
int err;
if (!frame || !mode)
@@ -5230,7 +5242,8 @@ drm_hdmi_avi_infoframe_from_display_mode(struct
hdmi_avi_infoframe *frame,
if (mode->flags & DRM_MODE_FLAG_DBLCLK)
frame->pixel_repeat = 1;
- frame->video_code = drm_mode_cea_vic(connector, mode);
+ vic = drm_mode_cea_vic(connector, mode);
+ hdmi_vic = drm_mode_hdmi_vic(connector, mode);
frame->picture_aspect = HDMI_PICTURE_ASPECT_NONE;
@@ -5244,11 +5257,15 @@ drm_hdmi_avi_infoframe_from_display_mode(struct
hdmi_avi_infoframe *frame,
/*
* Populate picture aspect ratio from either
-* user input (if specified) or from the CEA mode list.
+* user input (if specified) or from the CEA/HDMI mode lists.
*/
picture_aspect = mode->picture_aspect_ratio;
- if (picture_aspect == HDMI_PICTURE_ASPECT_NONE)
- picture_aspect = drm_get_cea_aspect_ratio(frame->video_code);
+ if (picture_aspect == HDMI_PICTURE_ASPECT_NONE) {
+ if (vic)
+ picture_aspect = drm_get_cea_aspect_ratio(vic);
+ else if (hdmi_vic)
+ picture_aspect = drm_get_hdmi_aspect_ratio(hdmi_vic);
+ }
/*
* The infoframe can't convey anything but none, 4:3
@@ -5256,12 +5273,20 @@ drm_hdmi_avi_infoframe_from_display_mode(struct
hdmi_avi_infoframe *frame,
* we can only satisfy it by specifying the right VIC.
*/
if (picture_aspect > HDMI_PICTURE_ASPECT_16_9) {
- if (picture_aspect !=
- drm_get_cea_aspect_ratio(frame->video_code))
+ if (vic) {
+ if (picture_aspect != drm_get_cea_aspect_ratio(vic))
+ return -EINVAL;
+ } else if (hdmi_vic) {
+ if (picture_aspect !=
drm_get_hdmi_aspect_ratio(hdmi_vic))
+ return -EINVAL;
+ } else {
return -EINVAL;
+ }
+
picture_aspect = HDMI_PICTURE_ASPECT_NONE;
}
+ frame->video_code = vic;
frame->picture_aspect = picture_aspect;
frame->active_aspect = HDMI_ACTIVE_ASPECT_PICTURE;
frame->scan_mode = HDMI_SCAN_MODE_UNDERSCAN;
--
2.21.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Outreachy][VKMS] Apply to VKMS
Hi guys, I'm Lorrany Azevedo, and I'm participating in the application phase for the outreachy internships. I'm interested in making a contribution to VKMS, and I would like to ask for tips on how I can do this, I'm new to the community and I never made any contribution to the FOSS. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3] drm/fourcc: Add Arm 16x16 block modifier
From: Raymond Smith
Add the DRM_FORMAT_MOD_ARM_16X16_BLOCK_U_INTERLEAVED modifier to
denote the 16x16 block u-interleaved format used in Arm Utgard and
Midgard GPUs.
Changes from v1:-
1. Reserved the upper four bits (out of the 56 bits assigned to each vendor)
to denote the category of Arm specific modifiers. Currently, we have two
categories ie AFBC and MISC.
Changes from v2:-
1. Preserved Ray's authorship
2. Cleanups/changes suggested by Brian
3. Added r-bs of Brian and Qiang
Signed-off-by: Raymond Smith
Signed-off-by: Ayan kumar halder
Reviewed-by: Brian Starkey
Reviewed-by: Qiang Yu
---
include/uapi/drm/drm_fourcc.h | 26 +-
1 file changed, 25 insertions(+), 1 deletion(-)
diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index 3feeaa3f987a..2376d36ea573 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -648,7 +648,21 @@ extern "C" {
* Further information on the use of AFBC modifiers can be found in
* Documentation/gpu/afbc.rst
*/
-#define DRM_FORMAT_MOD_ARM_AFBC(__afbc_mode) fourcc_mod_code(ARM,
__afbc_mode)
+
+/*
+ * The top 4 bits (out of the 56 bits alloted for specifying vendor specific
+ * modifiers) denote the category for modifiers. Currently we have only two
+ * categories of modifiers ie AFBC and MISC. We can have a maximum of sixteen
+ * different categories.
+ */
+#define DRM_FORMAT_MOD_ARM_CODE(__type, __val) \
+ fourcc_mod_code(ARM, ((__u64)(__type) << 52) | ((__val) &
0x000fULL))
+
+#define DRM_FORMAT_MOD_ARM_TYPE_AFBC 0x00
+#define DRM_FORMAT_MOD_ARM_TYPE_MISC 0x01
+
+#define DRM_FORMAT_MOD_ARM_AFBC(__afbc_mode) \
+ DRM_FORMAT_MOD_ARM_CODE(DRM_FORMAT_MOD_ARM_TYPE_AFBC, __afbc_mode)
/*
* AFBC superblock size
@@ -742,6 +756,16 @@ extern "C" {
*/
#define AFBC_FORMAT_MOD_BCH (1ULL << 11)
+/*
+ * Arm 16x16 Block U-Interleaved modifier
+ *
+ * This is used by Arm Mali Utgard and Midgard GPUs. It divides the image
+ * into 16x16 pixel blocks. Blocks are stored linearly in order, but pixels
+ * in the block are reordered.
+ */
+#define DRM_FORMAT_MOD_ARM_16X16_BLOCK_U_INTERLEAVED \
+ DRM_FORMAT_MOD_ARM_CODE(DRM_FORMAT_MOD_ARM_TYPE_MISC, 1ULL)
+
/*
* Allwinner tiled modifier
*
--
2.23.0
Re: [PATCH] drm/i810: Prevent underflow in ioctl
Quoting Dan Carpenter (2019-10-04 11:22:51) > The "used" variables here come from the user in the ioctl and it can be > negative. It could result in an out of bounds write. > > Signed-off-by: Dan Carpenter > --- > drivers/gpu/drm/i810/i810_dma.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i810/i810_dma.c b/drivers/gpu/drm/i810/i810_dma.c > index 2a77823b8e9a..e66c38332df4 100644 > --- a/drivers/gpu/drm/i810/i810_dma.c > +++ b/drivers/gpu/drm/i810/i810_dma.c > @@ -728,7 +728,7 @@ static void i810_dma_dispatch_vertex(struct drm_device > *dev, > if (nbox > I810_NR_SAREA_CLIPRECTS) > nbox = I810_NR_SAREA_CLIPRECTS; > > - if (used > 4 * 1024) > + if (used < 0 || used > 4 * 1024) > used = 0; Yes, as passed to the GPU instruction, negative used is invalid. Then it is used as an offset into a memblock, where a negative offset would be very bad. Reviewed-by: Chris Wilson -Chris ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm: bridge: adv7511: Enable SPDIF DAI
Hi Bogdan,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[cannot apply to v5.4-rc1 next-20191004]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url:
https://github.com/0day-ci/linux/commits/Bogdan-Togorean/drm-bridge-adv7511-Enable-SPDIF-DAI/20191004-205455
config: xtensa-allyesconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 7.4.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=7.4.0 make.cross ARCH=xtensa
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot
All warnings (new ones prefixed by >>):
drivers/gpu/drm/bridge/adv7511/adv7511_audio.c: In function
'adv7511_hdmi_hw_params':
>> drivers/gpu/drm/bridge/adv7511/adv7511_audio.c:123:16: warning: this
>> statement may fall through [-Wimplicit-fallthrough=]
audio_source = ADV7511_AUDIO_SOURCE_SPDIF;
drivers/gpu/drm/bridge/adv7511/adv7511_audio.c:124:2: note: here
default:
^~~
vim +123 drivers/gpu/drm/bridge/adv7511/adv7511_audio.c
55
56 int adv7511_hdmi_hw_params(struct device *dev, void *data,
57 struct hdmi_codec_daifmt *fmt,
58 struct hdmi_codec_params *hparms)
59 {
60 struct adv7511 *adv7511 = dev_get_drvdata(dev);
61 unsigned int audio_source, i2s_format = 0;
62 unsigned int invert_clock;
63 unsigned int rate;
64 unsigned int len;
65
66 switch (hparms->sample_rate) {
67 case 32000:
68 rate = ADV7511_SAMPLE_FREQ_32000;
69 break;
70 case 44100:
71 rate = ADV7511_SAMPLE_FREQ_44100;
72 break;
73 case 48000:
74 rate = ADV7511_SAMPLE_FREQ_48000;
75 break;
76 case 88200:
77 rate = ADV7511_SAMPLE_FREQ_88200;
78 break;
79 case 96000:
80 rate = ADV7511_SAMPLE_FREQ_96000;
81 break;
82 case 176400:
83 rate = ADV7511_SAMPLE_FREQ_176400;
84 break;
85 case 192000:
86 rate = ADV7511_SAMPLE_FREQ_192000;
87 break;
88 default:
89 return -EINVAL;
90 }
91
92 switch (hparms->sample_width) {
93 case 16:
94 len = ADV7511_I2S_SAMPLE_LEN_16;
95 break;
96 case 18:
97 len = ADV7511_I2S_SAMPLE_LEN_18;
98 break;
99 case 20:
100 len = ADV7511_I2S_SAMPLE_LEN_20;
101 break;
102 case 24:
103 len = ADV7511_I2S_SAMPLE_LEN_24;
104 break;
105 default:
106 return -EINVAL;
107 }
108
109 switch (fmt->fmt) {
110 case HDMI_I2S:
111 audio_source = ADV7511_AUDIO_SOURCE_I2S;
112 i2s_format = ADV7511_I2S_FORMAT_I2S;
113 break;
114 case HDMI_RIGHT_J:
115 audio_source = ADV7511_AUDIO_SOURCE_I2S;
116 i2s_format = ADV7511_I2S_FORMAT_RIGHT_J;
117 break;
118 case HDMI_LEFT_J:
119 audio_source = ADV7511_AUDIO_SOURCE_I2S;
120 i2s_format = ADV7511_I2S_FORMAT_LEFT_J;
121 break;
122 case HDMI_SPDIF:
> 123 audio_source = ADV7511_AUDIO_SOURCE_SPDIF;
124 default:
125 return -EINVAL;
126 }
127
128 invert_clock = fmt->bit_clk_inv;
129
130 regmap_update_bits(adv7511->regmap, ADV7511_REG_AUDIO_SOURCE,
0x70,
131 audio_source << 4);
132 regmap_update_bits(adv7511->regmap, ADV7511_REG_AUDIO_CONFIG,
BIT(6),
133 invert_clock << 6);
134 regmap_update_bits(adv7511->regmap, ADV7511_REG_I2S_CONFIG,
0x03,
135 i2s_format);
136
137 adv7511->audio_source = audio_source;
138
139 adv7511->f_audio = hparms->sample_rate;
140
141 adv7511_update_cts_n(adv7511);
142
[PATCH] drm/i915: customize DPCD brightness control for specific panel
This panel (manufacturer is SDC, product ID is 0x4141)
used manufacturer defined DPCD register to control brightness
that not defined in eDP spec so far. This change follow panel
vendor's instruction to support brightness adjustment.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97883
Cc: Jani Nikula
Cc: Maarten Lankhorst
Cc: Gustavo Padovan
Cc: Cooper Chiou
Signed-off-by: Lee Shawn C
---
drivers/gpu/drm/drm_edid.c| 6 +-
drivers/gpu/drm/i915/display/intel_dp.c | 7 +
.../drm/i915/display/intel_dp_aux_backlight.c | 143 +-
include/drm/drm_edid.h| 3 +
4 files changed, 153 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 3c9703b08491..5aee0ebc200e 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -211,6 +211,9 @@ static const struct edid_quirk {
/* OSVR HDK and HDK2 VR Headsets */
{ "SVR", 0x1019, EDID_QUIRK_NON_DESKTOP },
+
+ /* Samsung eDP panel */
+ { "SDC", 0x4141, EDID_QUIRK_NON_STD_BRIGHTNESS_CONTROL },
};
/*
@@ -1938,7 +1941,7 @@ static bool edid_vendor(const struct edid *edid, const
char *vendor)
*
* This tells subsequent routines what fixes they need to apply.
*/
-static u32 edid_get_quirks(const struct edid *edid)
+u32 edid_get_quirks(const struct edid *edid)
{
const struct edid_quirk *quirk;
int i;
@@ -1953,6 +1956,7 @@ static u32 edid_get_quirks(const struct edid *edid)
return 0;
}
+EXPORT_SYMBOL(edid_get_quirks);
#define MODE_SIZE(m) ((m)->hdisplay * (m)->vdisplay)
#define MODE_REFRESH_DIFF(c,t) (abs((c) - (t)))
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
b/drivers/gpu/drm/i915/display/intel_dp.c
index 2b1e71f992b0..89193bd2d8ea 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -7097,6 +7097,13 @@ static bool intel_edp_init_connector(struct intel_dp
*intel_dp,
}
intel_connector->edid = edid;
+ if (edid_get_quirks(intel_connector->edid) ==
+ EDID_QUIRK_NON_STD_BRIGHTNESS_CONTROL) {
+ i915_modparams.enable_dpcd_backlight = true;
+ i915_modparams.fastboot = false;
+ DRM_DEBUG_KMS("Using specific DPCD to control brightness\n");
+ }
+
fixed_mode = intel_panel_edid_fixed_mode(intel_connector);
if (fixed_mode)
downclock_mode = intel_dp_drrs_init(intel_connector,
fixed_mode);
diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
index 020422da2ae2..7d9a2249cfb1 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
@@ -25,6 +25,117 @@
#include "intel_display_types.h"
#include "intel_dp_aux_backlight.h"
+#define DPCD_EDP_GETSET_CTRL_PARAMS0x344
+#define DPCD_EDP_CONTENT_LUMINANCE 0x346
+#define DPCD_EDP_PANEL_LUMINANCE_OVERRIDE 0x34a
+#define DPCD_EDP_BRIGHTNESS_NITS 0x354
+#define DPCD_EDP_BRIGHTNESS_OPTIMIZATION 0x358
+
+#define EDP_CUSTOMIZE_MAX_BRIGHTNESS_LEVEL (512)
+
+static uint32_t intel_dp_aux_get_customize_backlight(struct intel_connector
*connector)
+{
+ struct intel_dp *intel_dp = enc_to_intel_dp(&connector->encoder->base);
+ uint8_t read_val[2] = { 0x0 };
+
+ if (drm_dp_dpcd_read(&intel_dp->aux, DPCD_EDP_BRIGHTNESS_NITS,
+&read_val, sizeof(read_val)) < 0) {
+ DRM_DEBUG_KMS("Failed to read DPCD register %x\n",
+ DPCD_EDP_BRIGHTNESS_NITS);
+ return 0;
+ }
+
+ return (read_val[1] << 8 | read_val[0]);
+}
+
+static void
+intel_dp_aux_set_customize_backlight(const struct drm_connector_state
*conn_state, u32 level)
+{
+ struct intel_connector *connector =
to_intel_connector(conn_state->connector);
+ struct intel_dp *intel_dp = enc_to_intel_dp(&connector->encoder->base);
+ struct intel_panel *panel = &connector->panel;
+ uint8_t new_vals[4];
+
+ if (level > panel->backlight.max)
+ level = panel->backlight.max;
+
+ new_vals[0] = level & 0xFF;
+ new_vals[1] = (level & 0xFF00) >> 8;
+ new_vals[2] = 0;
+ new_vals[3] = 0;
+
+ if (drm_dp_dpcd_write(&intel_dp->aux, DPCD_EDP_BRIGHTNESS_NITS,
new_vals, 4) < 0)
+ DRM_DEBUG_KMS("Failed to write aux backlight level\n");
+}
+
+static void intel_dp_aux_enable_customize_backlight(const struct
intel_crtc_state *crtc_state,
+ const struct drm_connector_state
*conn_state)
+{
+ struct intel_connector *connector =
to_intel_connector(conn_state->connector);
+ struct intel_dp *intel_dp = enc_to_intel_dp(&connector->encoder->base);
+ uint8_t read_val[4], i;
+ uint8_t write_val[8] = {0x00, 0x00, 0xF0
Re: general protection fault in veth_get_stats64
On Wed, Oct 2, 2019 at 5:45 PM syzbot
wrote:
>
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit:a32db7e1 Add linux-next specific files for 20191002
> git tree: linux-next
> console output: https://syzkaller.appspot.com/x/log.txt?x=175ab7cd60
> kernel config: https://syzkaller.appspot.com/x/.config?x=599cf05035799eef
> dashboard link: https://syzkaller.appspot.com/bug?extid=3f3e5e77d793c7a6fe6c
> compiler: gcc (GCC) 9.0.0 20181231 (experimental)
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=12f8b94360
> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=16981a2560
>
> The bug was bisected to:
>
> commit 84da111de0b4be15bd500deff773f5116f39f7be
> Author: Linus Torvalds
> Date: Sat Sep 21 17:07:42 2019 +
>
> Merge tag 'for-linus-hmm' of
> git://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma
>
> bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=17c5584760
> final crash:https://syzkaller.appspot.com/x/report.txt?x=1425584760
> console output: https://syzkaller.appspot.com/x/log.txt?x=1025584760
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+3f3e5e77d793c7a6f...@syzkaller.appspotmail.com
> Fixes: 84da111de0b4 ("Merge tag 'for-linus-hmm' of
> git://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma")
>
> RSP: 002b:7fff0ba6c998 EFLAGS: 0246 ORIG_RAX: 002e
> RAX: ffda RBX: 0003 RCX: 004424a9
> RDX: RSI: 2140 RDI: 0003
> RBP: R08: 0002 R09:
> R10: R11: 0246 R12:
> R13: 0004 R14: R15:
> kasan: CONFIG_KASAN_INLINE enabled
> kasan: GPF could be caused by NULL-ptr deref or user memory access
> general protection fault: [#1] PREEMPT SMP KASAN
> CPU: 1 PID: 8605 Comm: syz-executor330 Not tainted 5.4.0-rc1-next-20191002
> #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> Google 01/01/2011
> RIP: 0010:veth_stats_rx drivers/net/veth.c:322 [inline]
> RIP: 0010:veth_get_stats64+0x523/0x900 drivers/net/veth.c:356
> Code: 89 85 60 ff ff ff e8 6c 74 31 fd 49 63 c7 48 69 c0 c0 02 00 00 48 03
> 85 60 ff ff ff 48 8d b8 a0 01 00 00 48 89 fa 48 c1 ea 03 <42> 80 3c 32 00
> 0f 85 c9 02 00 00 48 8d b8 a8 01 00 00 48 8b 90 a0
> RSP: 0018:88809996ed00 EFLAGS: 00010202
> RAX: RBX: 0001 RCX: 84418daf
> RDX: 0034 RSI: 84418e04 RDI: 01a0
> RBP: 88809996ede0 R08: 888093182180 R09: ed1013202d6a
> R10: ed1013202d69 R11: 888099016b4f R12:
> R13: R14: dc00 R15:
> FS: 01f4a880() GS:8880ae90() knlGS:
> CS: 0010 DS: ES: CR0: 80050033
> CR2: 2140 CR3: 9a80b000 CR4: 001406e0
> DR0: DR1: DR2:
> DR3: DR6: fffe0ff0 DR7: 0400
> Call Trace:
> dev_get_stats+0x8e/0x280 net/core/dev.c:9220
> rtnl_fill_stats+0x4d/0xac0 net/core/rtnetlink.c:1191
> rtnl_fill_ifinfo+0x10ad/0x3af0 net/core/rtnetlink.c:1717
> rtmsg_ifinfo_build_skb+0xc9/0x1a0 net/core/rtnetlink.c:3635
> rtmsg_ifinfo_event.part.0+0x43/0xe0 net/core/rtnetlink.c:3667
> rtmsg_ifinfo_event net/core/rtnetlink.c:3678 [inline]
> rtmsg_ifinfo+0x8d/0xa0 net/core/rtnetlink.c:3676
> __dev_notify_flags+0x235/0x2c0 net/core/dev.c:7757
> rtnl_configure_link+0x175/0x250 net/core/rtnetlink.c:2968
> __rtnl_newlink+0x10c4/0x16d0 net/core/rtnetlink.c:3285
> rtnl_newlink+0x69/0xa0 net/core/rtnetlink.c:3325
> rtnetlink_rcv_msg+0x463/0xb00 net/core/rtnetlink.c:5386
> netlink_rcv_skb+0x177/0x450 net/netlink/af_netlink.c:2477
> rtnetlink_rcv+0x1d/0x30 net/core/rtnetlink.c:5404
> netlink_unicast_kernel net/netlink/af_netlink.c:1302 [inline]
> netlink_unicast+0x531/0x710 net/netlink/af_netlink.c:1328
> netlink_sendmsg+0x8a5/0xd60 net/netlink/af_netlink.c:1917
> sock_sendmsg_nosec net/socket.c:638 [inline]
> sock_sendmsg+0xd7/0x130 net/socket.c:658
> ___sys_sendmsg+0x803/0x920 net/socket.c:2312
> __sys_sendmsg+0x105/0x1d0 net/socket.c:2357
> __do_sys_sendmsg net/socket.c:2366 [inline]
> __se_sys_sendmsg net/socket.c:2364 [inline]
> __x64_sys_sendmsg+0x78/0xb0 net/socket.c:2364
> do_syscall_64+0xfa/0x760 arch/x86/entry/common.c:290
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
> RIP: 0033:0x4424a9
> Code: e8 9c 07 03 00 48 83 c4 18 c3 0f 1f 80 00 00 00 00 48 89 f8 48 89 f7
> 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff
> ff 0f 83 3b 0a fc ff c3 66 2e 0f 1f 84 00 00 00 00
> RSP: 002b:7fff0ba6c998 EFLAGS: 0246 ORIG_RAX: 002e
> RAX: ffda RBX: 0003 RCX: 0044
[Bug 111244] amdgpu kernel 5.2 blank display after resume from suspend
https://bugs.freedesktop.org/show_bug.cgi?id=111244 --- Comment #33 from Andrey Grodzovsky --- OOO today -- 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 111244] amdgpu kernel 5.2 blank display after resume from suspend
https://bugs.freedesktop.org/show_bug.cgi?id=111244 --- Comment #32 from mib...@gmx.at --- Cannot reproduce it anymore as of 5.4rc1, seems like it's fixed for me! -- 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 111729] RX480 : random NULL pointer dereference on resume from suspend
https://bugs.freedesktop.org/show_bug.cgi?id=111729 --- Comment #7 from m...@cschwarz.com --- I began bisecting yesterday and discovered another bug that happens on suspend, which makes it hard to determine the good/bad status of a build with regards to _this_ bug in a timely manner. Hence aborting any bisection attempts. Maybe a new crowd of people runs into this when upgrading their Ubuntu LTS systems :( -- 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: Should regulator core support parsing OF based fwnode?
On 04/10/2019 13:39, Mark Brown wrote: On Thu, Oct 03, 2019 at 10:27:26PM +0200, Jacek Anaszewski wrote: On 10/3/19 9:41 PM, Mark Brown wrote: Why would we want to do that? We'd continue to support only DT systems, just with code that's less obviously DT only and would need to put checks in. I'm not seeing an upside here. For instance few weeks ago we had a patch [0] in the LED core switching from using struct device's of_node property to fwnode for conveying device property data. And this transition to fwnode property API can be observed as a frequent pattern across subsystems. For most subsystems the intent is to reuse DT bindings on embedded ACPI systems via _DSD. Recently there is an ongoing effort aiming to add generic support for handling regulators in the LED core [1], but it turns out to require bringing back initialization of of_node property for devm_regulator_get_optional() to work properly. Consumers should just be able to request a regulator without having to worry about how that's being provided - they should have no knowledge at all of firmware bindings or platform data for defining this. If they do that suggests there's an abstraction issue somewhere, what makes you think that doing something with of_node is required? The regulator core accesses consumer->of_node to get a phandle to a regulator's node. The trouble arises from the fact that the LED core does not populate of_node anymore, instead it populates fwnode. This allows the LED core to be agnostic of ACPI or OF to get the properties of a LED. IMO it is better to populate both of_node and fwnode in the LED core at the moment. It has already been fixed this way for the platform driver [0], MTD [1] and PCI-OF [2]. Further, unless you have LEDs that work without power you probably shouldn't be using _get_optional() for their supply. That interface is intended only for supplies that may be physically absent. Not all LEDs have a regulator to provide the power. The power can be supplied by the LED controller for example. [0] f94277af03ead0d3bf2 of/platform: Initialise dev->fwnode appropriately [1] c176c6d7e932662668 mfd: core: Set fwnode for created devices [2] 9b099a6c75e4ddceea PCI: OF: Initialize dev->fwnode appropriately
Re: [PATCH -next 00/11] lib/interval-tree: move to half closed intervals
Hi Jason, On Thu, Oct 3, 2019 at 5:26 PM Jason Gunthorpe wrote: > Hurm, this is not entirely accurate. Most users do actually want > overlapping and multiple ranges. I just studied this extensively: (Just curious, are you the person we discussed this with after the Maple Tree talk at LPC 2019 ?) I think we have two separate API problems there: - overlapping vs non-overlapping intervals (the interval tree API supports overlapping intervals, but some users are confused about this) - closed vs half-open interval definitions It looks like you have been looking mostly at the first issue, which I expect could simplify several interval tree users considerably, while Davidlohr is addressing the second issue here. > radeon_mn actually wants overlapping but seems to mis-understand the > interval_tree API and actively tries hard to prevent overlapping at > great cost and complexity. I have a patch to delete all of this and > just be overlapping. > > amdgpu_mn copied the wrongness from radeon_mn > > All the DRM drivers are basically the same here, tracking userspace > controlled VAs, so overlapping is essential > > hfi1/mmu_rb definitely needs overlapping as it is dealing with > userspace VA ranges under control of userspace. As do the other > infiniband users. Do you have a handle on what usnic is doing with its intervals ? usnic_uiom_insert_interval() has some complicated logic to avoid having overlapping intervals, which is very confusing to me. > vhost probably doesn't overlap in the normal case, but again userspace > could trigger overlap in some pathalogical case. > > The [start,last] allows the interval to cover up to ULONG_MAX. I don't > know if this is needed however. Many users are using userspace VAs > here. Is there any kernel configuration where ULONG_MAX is a valid > userspace pointer? Ie 32 bit 4G userspace? I don't know. > > Many users seemed to have bugs where they were taking a userspace > controlled start + length and converting them into a start/end for > interval tree without overflow protection (woops) > > Also I have a series already cooking to delete several of these > interval tree users, which will terribly conflict with this :\ > > Is it really necessary to make such churn for such a tiny API change? My take is that this (Davidlohr's) patch series does not necessarily need to be applied all at once - we could get the first change in (adding the interval_tree_gen.h header), and convert the first few users, without getting them all at once, as long as we have a plan for finishing the work. So, if you have cleanups in progress in some of the files, just tell us which ones and we can leave them out from the first pass. Thanks, -- Michel "Walken" Lespinasse A program is never fully debugged until the last user dies.
Re: [PATCH] drm: atomic helper: fix W=1 warnings
On Fri, Oct 04, 2019 at 12:36:56PM +, Benjamin GAIGNARD wrote:
>
> On 10/4/19 2:27 PM, Ville Syrjälä wrote:
> > On Fri, Oct 04, 2019 at 12:48:02PM +0200, Benjamin Gaignard wrote:
> >> Le jeu. 3 oct. 2019 à 17:46, Ville Syrjälä
> >> a écrit :
> >>> On Thu, Oct 03, 2019 at 05:37:15PM +0200, Benjamin Gaignard wrote:
> Le jeu. 3 oct. 2019 à 17:05, Ville Syrjälä
> a écrit :
> > On Thu, Oct 03, 2019 at 04:46:54PM +0200, Benjamin Gaignard wrote:
> >> Le jeu. 3 oct. 2019 à 16:27, Ville Syrjälä
> >> a écrit :
> >>> On Mon, Sep 09, 2019 at 03:52:05PM +0200, Benjamin Gaignard wrote:
> Fix warnings with W=1.
> Few for_each macro set variables that are never used later.
> Prevent warning by marking these variables as __maybe_unused.
>
> Signed-off-by: Benjamin Gaignard
> ---
> drivers/gpu/drm/drm_atomic_helper.c | 36
> ++--
> 1 file changed, 18 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c
> index aa16ea17ff9b..b69d17b0b9bd 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -262,7 +262,7 @@ steal_encoder(struct drm_atomic_state *state,
> struct drm_encoder *encoder)
> {
> struct drm_crtc_state *crtc_state;
> - struct drm_connector *connector;
> + struct drm_connector __maybe_unused *connector;
> >>> Rather ugly. IMO would be nicer if we could hide something inside
> >>> the iterator macros to suppress the warning.
> >> Ok but how ?
> >> connector is assigned in the macros but not used later and we can't
> >> set "__maybe_unused"
> >> in the macro.
> >> Does another keyword exist for that ?
> > Stick a (void)(connector) into the macro?
> That could work but it will look strange inside the macro.
>
> > Another (arguably cleaner) idea would be to remove the
> > connector/crtc/plane
> > argument from the iterators entirely since it's redundant, and instead
> > just
> > extract it from the appropriate new/old state as needed.
> >
> > We could then also add a for_each_connector_in_state()/etc. which omit
> > s the state arguments and just has the connector argument, for cases
> > where
> > you don't care about the states when iterating.
> That may lead to get a macro for each possible combination of used
> variables.
> >>> We already have new/old/oldnew, so would "just" add one more.
> >> Not just one, it will be one each new/old/oldnew macro to be able to
> >> distinguish
> >> when connector is used or not.
> > What I'm suggesting is this:
> > for_each_connector_in_state(state, connector, i)
> > for_each_old_connector_in_state(state, old_conn_state, i)
> > for_each_new_connector_in_state(state, new_conn_state, i)
> > for_each_oldnew_connector_in_state(state, old_conn_state, new_conn_state, i)
> >
> > So only four in total for each object type, instead of the current
> > three.
>
> You are missing these cases: old and connector, new and connector,
>
> old and new and connector are needed together.
No, that's redundant. You can always get the connector from
old/new_conn_state->connector if you need it.
--
Ville Syrjälä
Intel
Re: [PATCH][next] drm/amdgpu: remove redundant variable r and redundant return statement
On Fri, Oct 4, 2019 at 3:29 AM Koenig, Christian
wrote:
>
> Am 03.10.19 um 23:40 schrieb Colin King:
> > From: Colin Ian King
> >
> > There is a return statement that is not reachable and a variable that
> > is not used. Remove them.
> >
> > Addresses-Coverity: ("Structurally dead code")
> > Fixes: de7b45babd9b ("drm/amdgpu: cleanup creating BOs at fixed location
> > (v2)")
> > Signed-off-by: Colin Ian King
>
> Reviewed-by: Christian König
Applied. Thanks!
Alex
>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 2 --
> > 1 file changed, 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > index 481e4c381083..814159f15633 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > @@ -1636,7 +1636,6 @@ static void amdgpu_ttm_fw_reserve_vram_fini(struct
> > amdgpu_device *adev)
> > static int amdgpu_ttm_fw_reserve_vram_init(struct amdgpu_device *adev)
> > {
> > uint64_t vram_size = adev->gmc.visible_vram_size;
> > - int r;
> >
> > adev->fw_vram_usage.va = NULL;
> > adev->fw_vram_usage.reserved_bo = NULL;
> > @@ -1651,7 +1650,6 @@ static int amdgpu_ttm_fw_reserve_vram_init(struct
> > amdgpu_device *adev)
> > AMDGPU_GEM_DOMAIN_VRAM,
> > &adev->fw_vram_usage.reserved_bo,
> > &adev->fw_vram_usage.va);
> > - return r;
> > }
> >
> > /**
>
> ___
> 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] cec: add cec_adapter to cec_notifier_cec_adap_unregister()
On Fri, Oct 04, 2019 at 01:04:24PM +0200, Hans Verkuil wrote:
> It is possible for one HDMI connector to have multiple CEC adapters. The
> typical real-world scenario is that where one adapter is used when the device
> is in standby, and one that's better/smarter when the device is powered up.
>
> The cec-notifier changes were made with that in mind, but I missed that in
> order to support this you need to tell cec_notifier_cec_adap_unregister()
> which adapter you are unregistering from the notifier.
>
> Add this additional argument. It is currently unused, but once all drivers
> use this, the CEC core will be adapted for these use-cases.
>
> Signed-off-by: Hans Verkuil
> ---
> This patch should go in via the drm subsystem since all CEC adapters in the
> drm subsystem have been converted to use cec_notifier_cec_adap_unregister().
> The media subsystem still has older drm drivers that weren't converted to use
> cec_notifier_cec_adap_unregister().
>
> This will only be a problem if a new CEC adapter driver is added to the media
> subsystem for v5.5, but I am not aware of any plans for that. Should it
> happen,
> then that just means that the media subsystem needs to resolve a fairly
> trivial
> merge conflict.
>
> Ville, Mauro, can you review/ack?
Looks harmless enough to me :)
Reviewed-by: Ville Syrjälä
> ---
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c
> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c
> index ac1e001d0882..70ab4fbdc23e 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c
> @@ -285,7 +285,7 @@ static int dw_hdmi_cec_probe(struct platform_device *pdev)
>
> ret = cec_register_adapter(cec->adap, pdev->dev.parent);
> if (ret < 0) {
> - cec_notifier_cec_adap_unregister(cec->notify);
> + cec_notifier_cec_adap_unregister(cec->notify, cec->adap);
> return ret;
> }
>
> @@ -302,7 +302,7 @@ static int dw_hdmi_cec_remove(struct platform_device
> *pdev)
> {
> struct dw_hdmi_cec *cec = platform_get_drvdata(pdev);
>
> - cec_notifier_cec_adap_unregister(cec->notify);
> + cec_notifier_cec_adap_unregister(cec->notify, cec->adap);
> cec_unregister_adapter(cec->adap);
>
> return 0;
> diff --git a/drivers/gpu/drm/i2c/tda9950.c b/drivers/gpu/drm/i2c/tda9950.c
> index a5a75bdeb7a5..5b03fdd1eaa4 100644
> --- a/drivers/gpu/drm/i2c/tda9950.c
> +++ b/drivers/gpu/drm/i2c/tda9950.c
> @@ -465,7 +465,7 @@ static int tda9950_probe(struct i2c_client *client,
>
> ret = cec_register_adapter(priv->adap, priv->hdmi);
> if (ret < 0) {
> - cec_notifier_cec_adap_unregister(priv->notify);
> + cec_notifier_cec_adap_unregister(priv->notify, priv->adap);
> return ret;
> }
>
> @@ -482,7 +482,7 @@ static int tda9950_remove(struct i2c_client *client)
> {
> struct tda9950_priv *priv = i2c_get_clientdata(client);
>
> - cec_notifier_cec_adap_unregister(priv->notify);
> + cec_notifier_cec_adap_unregister(priv->notify, priv->adap);
> cec_unregister_adapter(priv->adap);
>
> return 0;
> diff --git a/drivers/media/cec/cec-notifier.c
> b/drivers/media/cec/cec-notifier.c
> index 4d82a5522072..7cf42b133dbc 100644
> --- a/drivers/media/cec/cec-notifier.c
> +++ b/drivers/media/cec/cec-notifier.c
> @@ -153,13 +153,14 @@ cec_notifier_cec_adap_register(struct device *hdmi_dev,
> const char *conn_name,
> }
> EXPORT_SYMBOL_GPL(cec_notifier_cec_adap_register);
>
> -void cec_notifier_cec_adap_unregister(struct cec_notifier *n)
> +void cec_notifier_cec_adap_unregister(struct cec_notifier *n,
> + struct cec_adapter *adap)
> {
> if (!n)
> return;
>
> mutex_lock(&n->lock);
> - n->cec_adap->notifier = NULL;
> + adap->notifier = NULL;
Maybe assert that the notifier and adapter know each other?
> n->cec_adap = NULL;
> n->callback = NULL;
> mutex_unlock(&n->lock);
> diff --git a/drivers/media/platform/cros-ec-cec/cros-ec-cec.c
> b/drivers/media/platform/cros-ec-cec/cros-ec-cec.c
> index 4a3b3810fd89..f048e8994785 100644
> --- a/drivers/media/platform/cros-ec-cec/cros-ec-cec.c
> +++ b/drivers/media/platform/cros-ec-cec/cros-ec-cec.c
> @@ -314,7 +314,8 @@ static int cros_ec_cec_probe(struct platform_device *pdev)
> return 0;
>
> out_probe_notify:
> - cec_notifier_cec_adap_unregister(cros_ec_cec->notify);
> + cec_notifier_cec_adap_unregister(cros_ec_cec->notify,
> + cros_ec_cec->adap);
> out_probe_adapter:
> cec_delete_adapter(cros_ec_cec->adap);
> return ret;
> @@ -335,7 +336,8 @@ static int cros_ec_cec_remove(struct platform_device
> *pdev)
> return ret;
> }
>
> - cec_notifier_cec_adap_unregister(cros_ec_cec->notify);
> + cec_notifier_cec_adap_unregister(cros_ec_cec->notify,
> +
Re: [PATCH][next] drm/amdgpu: fix uninitialized variable pasid_mapping_needed
On Fri, Oct 4, 2019 at 3:28 AM Koenig, Christian
wrote:
>
> Am 03.10.19 um 23:52 schrieb Colin King:
> > From: Colin Ian King
> >
> > The boolean variable pasid_mapping_needed is not initialized and
> > there are code paths that do not assign it any value before it is
> > is read later. Fix this by initializing pasid_mapping_needed to
> > false.
> >
> > Addresses-Coverity: ("Uninitialized scalar variable")
> > Fixes: 6817bf283b2b ("drm/amdgpu: grab the id mgr lock while accessing
> > passid_mapping")
> > Signed-off-by: Colin Ian King
>
> Reviewed-by: Christian König
>
Applied. thanks!
Alex
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> > index a2c797e34a29..be10e4b9a94d 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> > @@ -1055,7 +1055,7 @@ int amdgpu_vm_flush(struct amdgpu_ring *ring, struct
> > amdgpu_job *job,
> > id->oa_size != job->oa_size);
> > bool vm_flush_needed = job->vm_needs_flush;
> > struct dma_fence *fence = NULL;
> > - bool pasid_mapping_needed;
> > + bool pasid_mapping_needed = false;
> > unsigned patch_offset = 0;
> > int r;
> >
>
> ___
> 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/amd/display: Make some functions static
On Fri, Oct 4, 2019 at 8:25 AM zhengbin wrote:
>
> Fix sparse warnings:
>
> drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_hdcp.c:32:6:
> warning: symbol 'lp_write_i2c' was not declared. Should it be static?
> drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_hdcp.c:42:6:
> warning: symbol 'lp_read_i2c' was not declared. Should it be static?
> drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_hdcp.c:52:6:
> warning: symbol 'lp_write_dpcd' was not declared. Should it be static?
> drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_hdcp.c:59:6:
> warning: symbol 'lp_read_dpcd' was not declared. Should it be static?
>
> Reported-by: Hulk Robot
> Signed-off-by: zhengbin
Applied. thanks!
Alex
> ---
> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c | 12
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c
> index 2443c23..77181dd 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c
> @@ -29,7 +29,8 @@
> #include "dm_helpers.h"
> #include
>
> -bool lp_write_i2c(void *handle, uint32_t address, const uint8_t *data,
> uint32_t size)
> +static bool
> +lp_write_i2c(void *handle, uint32_t address, const uint8_t *data, uint32_t
> size)
> {
>
> struct dc_link *link = handle;
> @@ -39,7 +40,8 @@ bool lp_write_i2c(void *handle, uint32_t address, const
> uint8_t *data, uint32_t
> return dm_helpers_submit_i2c(link->ctx, link, &cmd);
> }
>
> -bool lp_read_i2c(void *handle, uint32_t address, uint8_t offset, uint8_t
> *data, uint32_t size)
> +static bool
> +lp_read_i2c(void *handle, uint32_t address, uint8_t offset, uint8_t *data,
> uint32_t size)
> {
> struct dc_link *link = handle;
>
> @@ -49,14 +51,16 @@ bool lp_read_i2c(void *handle, uint32_t address, uint8_t
> offset, uint8_t *data,
> return dm_helpers_submit_i2c(link->ctx, link, &cmd);
> }
>
> -bool lp_write_dpcd(void *handle, uint32_t address, const uint8_t *data,
> uint32_t size)
> +static bool
> +lp_write_dpcd(void *handle, uint32_t address, const uint8_t *data, uint32_t
> size)
> {
> struct dc_link *link = handle;
>
> return dm_helpers_dp_write_dpcd(link->ctx, link, address, data, size);
> }
>
> -bool lp_read_dpcd(void *handle, uint32_t address, uint8_t *data, uint32_t
> size)
> +static bool
> +lp_read_dpcd(void *handle, uint32_t address, uint8_t *data, uint32_t size)
> {
> struct dc_link *link = handle;
>
> --
> 2.7.4
>
> ___
> amd-gfx mailing list
> amd-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3] drm/amd/display: fix struct init in update_bounding_box
On Thu, Oct 3, 2019 at 4:35 PM Raul E Rangel wrote:
>
> dcn20_resource.c:2636:9: error: missing braces around initializer
> [-Werror=missing-braces]
> struct _vcs_dpi_voltage_scaling_st
> calculated_states[MAX_CLOCK_LIMIT_STATES] = {0};
> ^
>
> Fixes: 7ed4e6352c16f ("drm/amd/display: Add DCN2 HW Sequencer and Resource")
>
> Signed-off-by: Raul E Rangel
Applied. thanks!
Alex
>
> ---
>
> Changes in v3:
> - Use memset
>
> Changes in v2:
> - Use {{0}} instead of {}
>
> drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
> b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
> index b949e202d6cb7..f72c26ae41def 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
> @@ -2633,7 +2633,7 @@ static void cap_soc_clocks(
> static void update_bounding_box(struct dc *dc, struct
> _vcs_dpi_soc_bounding_box_st *bb,
> struct pp_smu_nv_clock_table *max_clocks, unsigned int
> *uclk_states, unsigned int num_states)
> {
> - struct _vcs_dpi_voltage_scaling_st
> calculated_states[MAX_CLOCK_LIMIT_STATES] = {0};
> + struct _vcs_dpi_voltage_scaling_st
> calculated_states[MAX_CLOCK_LIMIT_STATES];
> int i;
> int num_calculated_states = 0;
> int min_dcfclk = 0;
> @@ -2641,6 +2641,8 @@ static void update_bounding_box(struct dc *dc, struct
> _vcs_dpi_soc_bounding_box_
> if (num_states == 0)
> return;
>
> + memset(calculated_states, 0, sizeof(calculated_states));
> +
> if (dc->bb_overrides.min_dcfclk_mhz > 0)
> min_dcfclk = dc->bb_overrides.min_dcfclk_mhz;
> else
> --
> 2.23.0.444.g18eeb5a265-goog
>
> ___
> amd-gfx mailing list
> amd-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH -next 00/11] lib/interval-tree: move to half closed intervals
On Thu, Oct 03, 2019 at 01:32:50PM -0700, Matthew Wilcox wrote: > On Thu, Oct 03, 2019 at 01:18:47PM -0700, Davidlohr Bueso wrote: > > It has been discussed[1,2] that almost all users of interval trees would > > better > > be served if the intervals were actually not [a,b], but instead [a, b). This > > So how does a user represent a range from ULONG_MAX to ULONG_MAX now? > > I think the problem is that large parts of the kernel just don't consider > integer overflow. Because we write in C, it's natural to write: > > for (i = start; i < end; i++) > > and just assume that we never need to hit ULONG_MAX or UINT_MAX. > If we're storing addresses, that's generally true -- most architectures > don't allow addresses in the -PAGE_SIZE to ULONG_MAX range (or they'd > have trouble with PTR_ERR). If you're looking at file sizes, that's > not true on 32-bit machines, and we've definitely seen filesystem bugs > with files nudging up on 16TB (on 32 bit with 4k page size). Or block > driver bugs with similarly sized block devices. > > So, yeah, easier to use. But damning corner cases. Yeah, I wanted to ask - is the case where pgoff == ULONG_MAX (i.e., last block of a file that is exactly 16TB) currently supported on 32-bit archs ? I have no idea if I am supposed to care about this or not... -- Michel "Walken" Lespinasse A program is never fully debugged until the last user dies.
[Bug 110674] Crashes / Resets From AMDGPU / Radeon VII
https://bugs.freedesktop.org/show_bug.cgi?id=110674 --- Comment #155 from ReddestDream --- So, I've done some tests with 5.4-rc1 and it seems like I'm getting similar results to line...@xcpp.org and sehell...@gmail.com. I'm using GNOME with Wayland (which works fine with only 1 display). Sometimes it works for a while. Sometimes I can't see the mouse cursor. Sometimes I get glitches all over the screen containing pieces and parts of previous framebuffers. But, I mean, it's better than 5.3 was, which was so bad I never could see anything and I would get stuck on blackscreen. At least on 5.4-rc1 I've been able to manually switch to a virtual console and reboot rather than force a reboot with the power button. Still hoping for some fix for this, but it's become less important to me as further improvements to GNOME and MESA have made the Radeon VII + iGPU setup I've been using run smoother. I've also discovered further issues on Windows regarding the high memory clock when using multiple monitors with Radeon VII, and it's been affecting performance there too. I'm considering just sticking with 1 monitor only with for this machine/card. lol -- 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 03/11] drm/amdgpu: convert amdgpu_vm_it to half closed intervals
Hi Michel, Am 04.10.19 um 13:36 schrieb Michel Lespinasse: On Fri, Oct 04, 2019 at 06:54:54AM +, Koenig, Christian wrote: Am 03.10.19 um 22:18 schrieb Davidlohr Bueso: The amdgpu_vm interval tree really wants [a, b) intervals, NAK, we explicitly do need an [a, b[ interval here. Hi Christian, Just wanted to confirm where you stand on this patch, since I think you reconsidered your initial position after first looking at 9/11 from this series. I do not know the amdgpu code well, but I think the changes should be fine - in struct amdgpu_bo_va_mapping, the "end" field will hold what was previously stored in the "last" field, plus one. The expectation is that overflows should not be an issue there, as "end" is explicitly declared as an uint64, and as the code was previously computing "last + 1" in many places. Does that seem workable to you ? No, we computed last + 1 in a couple of debug places were it doesn't hurt us and IIRC we currently cheat a bit because we use pfn instead of addresses on some other places. But that is only a leftover from radeon and we need to fix that sooner or later, cause essentially the physical address space of the device is really full 64bits, e.g. 0x0-0x. So that only fits into a 64bit int when we use half open/closed intervals, but would wrap around to zero if we use a closed interval. I initially thought that the set was changing the interval tree into always using a closed interval, but that seems to have been a misunderstanding. Regards, Christian. Thanks, ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: atomic helper: fix W=1 warnings
On 10/4/19 2:27 PM, Ville Syrjälä wrote:
> On Fri, Oct 04, 2019 at 12:48:02PM +0200, Benjamin Gaignard wrote:
>> Le jeu. 3 oct. 2019 à 17:46, Ville Syrjälä
>> a écrit :
>>> On Thu, Oct 03, 2019 at 05:37:15PM +0200, Benjamin Gaignard wrote:
Le jeu. 3 oct. 2019 à 17:05, Ville Syrjälä
a écrit :
> On Thu, Oct 03, 2019 at 04:46:54PM +0200, Benjamin Gaignard wrote:
>> Le jeu. 3 oct. 2019 à 16:27, Ville Syrjälä
>> a écrit :
>>> On Mon, Sep 09, 2019 at 03:52:05PM +0200, Benjamin Gaignard wrote:
Fix warnings with W=1.
Few for_each macro set variables that are never used later.
Prevent warning by marking these variables as __maybe_unused.
Signed-off-by: Benjamin Gaignard
---
drivers/gpu/drm/drm_atomic_helper.c | 36
++--
1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c
b/drivers/gpu/drm/drm_atomic_helper.c
index aa16ea17ff9b..b69d17b0b9bd 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -262,7 +262,7 @@ steal_encoder(struct drm_atomic_state *state,
struct drm_encoder *encoder)
{
struct drm_crtc_state *crtc_state;
- struct drm_connector *connector;
+ struct drm_connector __maybe_unused *connector;
>>> Rather ugly. IMO would be nicer if we could hide something inside
>>> the iterator macros to suppress the warning.
>> Ok but how ?
>> connector is assigned in the macros but not used later and we can't
>> set "__maybe_unused"
>> in the macro.
>> Does another keyword exist for that ?
> Stick a (void)(connector) into the macro?
That could work but it will look strange inside the macro.
> Another (arguably cleaner) idea would be to remove the
> connector/crtc/plane
> argument from the iterators entirely since it's redundant, and instead
> just
> extract it from the appropriate new/old state as needed.
>
> We could then also add a for_each_connector_in_state()/etc. which omit
> s the state arguments and just has the connector argument, for cases where
> you don't care about the states when iterating.
That may lead to get a macro for each possible combination of used
variables.
>>> We already have new/old/oldnew, so would "just" add one more.
>> Not just one, it will be one each new/old/oldnew macro to be able to
>> distinguish
>> when connector is used or not.
> What I'm suggesting is this:
> for_each_connector_in_state(state, connector, i)
> for_each_old_connector_in_state(state, old_conn_state, i)
> for_each_new_connector_in_state(state, new_conn_state, i)
> for_each_oldnew_connector_in_state(state, old_conn_state, new_conn_state, i)
>
> So only four in total for each object type, instead of the current
> three.
You are missing these cases: old and connector, new and connector,
old and new and connector are needed together.
>
Re: [PATCH] drm/amdgpu: Fix build error when !CONFIG_PERF_EVENTS
On Thu, Oct 3, 2019 at 10:13 PM Huacai Chen wrote:
>
> In previous release amdgpu_pmu.o is only built when CONFIG_PERF_EVENTS
> is selected. But after commit 64f55e629237e4752db1 ("drm/amdgpu: Add RAS
> EEPROM table.") it is duplicated in amdgpu-y. This change causes a build
> error when !CONFIG_PERF_EVENTS, so fix it.
>
> Fixes: 64f55e629237e4752db1 ("drm/amdgpu: Add RAS EEPROM table.")
> Cc: Andrey Grodzovsky
> Cc: Luben Tuikov
> Signed-off-by: Huacai Chen
Already fixed:
https://cgit.freedesktop.org/drm/drm/commit/?h=drm-fixes&id=ec3e5c0f0c2b716e768c0eee0fec30d572939ef5
Thanks!
Alex
> ---
> drivers/gpu/drm/amd/amdgpu/Makefile | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile
> b/drivers/gpu/drm/amd/amdgpu/Makefile
> index 42e2c1f..00962a6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/Makefile
> +++ b/drivers/gpu/drm/amd/amdgpu/Makefile
> @@ -54,7 +54,7 @@ amdgpu-y += amdgpu_device.o amdgpu_kms.o \
> amdgpu_gtt_mgr.o amdgpu_vram_mgr.o amdgpu_virt.o
> amdgpu_atomfirmware.o \
> amdgpu_vf_error.o amdgpu_sched.o amdgpu_debugfs.o amdgpu_ids.o \
> amdgpu_gmc.o amdgpu_xgmi.o amdgpu_csa.o amdgpu_ras.o amdgpu_vm_cpu.o \
> - amdgpu_vm_sdma.o amdgpu_pmu.o amdgpu_discovery.o amdgpu_ras_eeprom.o
> smu_v11_0_i2c.o
> + amdgpu_vm_sdma.o amdgpu_discovery.o amdgpu_ras_eeprom.o
> smu_v11_0_i2c.o
>
> amdgpu-$(CONFIG_PERF_EVENTS) += amdgpu_pmu.o
>
> --
> 2.7.0
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: atomic helper: fix W=1 warnings
On Fri, Oct 04, 2019 at 12:48:02PM +0200, Benjamin Gaignard wrote:
> Le jeu. 3 oct. 2019 à 17:46, Ville Syrjälä
> a écrit :
> >
> > On Thu, Oct 03, 2019 at 05:37:15PM +0200, Benjamin Gaignard wrote:
> > > Le jeu. 3 oct. 2019 à 17:05, Ville Syrjälä
> > > a écrit :
> > > >
> > > > On Thu, Oct 03, 2019 at 04:46:54PM +0200, Benjamin Gaignard wrote:
> > > > > Le jeu. 3 oct. 2019 à 16:27, Ville Syrjälä
> > > > > a écrit :
> > > > > >
> > > > > > On Mon, Sep 09, 2019 at 03:52:05PM +0200, Benjamin Gaignard wrote:
> > > > > > > Fix warnings with W=1.
> > > > > > > Few for_each macro set variables that are never used later.
> > > > > > > Prevent warning by marking these variables as __maybe_unused.
> > > > > > >
> > > > > > > Signed-off-by: Benjamin Gaignard
> > > > > > > ---
> > > > > > > drivers/gpu/drm/drm_atomic_helper.c | 36
> > > > > > > ++--
> > > > > > > 1 file changed, 18 insertions(+), 18 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> > > > > > > b/drivers/gpu/drm/drm_atomic_helper.c
> > > > > > > index aa16ea17ff9b..b69d17b0b9bd 100644
> > > > > > > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > > > > > > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > > > > > > @@ -262,7 +262,7 @@ steal_encoder(struct drm_atomic_state *state,
> > > > > > > struct drm_encoder *encoder)
> > > > > > > {
> > > > > > > struct drm_crtc_state *crtc_state;
> > > > > > > - struct drm_connector *connector;
> > > > > > > + struct drm_connector __maybe_unused *connector;
> > > > > >
> > > > > > Rather ugly. IMO would be nicer if we could hide something inside
> > > > > > the iterator macros to suppress the warning.
> > > > >
> > > > > Ok but how ?
> > > > > connector is assigned in the macros but not used later and we can't
> > > > > set "__maybe_unused"
> > > > > in the macro.
> > > > > Does another keyword exist for that ?
> > > >
> > > > Stick a (void)(connector) into the macro?
> > >
> > > That could work but it will look strange inside the macro.
> > >
> > > >
> > > > Another (arguably cleaner) idea would be to remove the
> > > > connector/crtc/plane
> > > > argument from the iterators entirely since it's redundant, and instead
> > > > just
> > > > extract it from the appropriate new/old state as needed.
> > > >
> > > > We could then also add a for_each_connector_in_state()/etc. which omit
> > > > s the state arguments and just has the connector argument, for cases
> > > > where
> > > > you don't care about the states when iterating.
> > >
> > > That may lead to get a macro for each possible combination of used
> > > variables.
> >
> > We already have new/old/oldnew, so would "just" add one more.
>
> Not just one, it will be one each new/old/oldnew macro to be able to
> distinguish
> when connector is used or not.
What I'm suggesting is this:
for_each_connector_in_state(state, connector, i)
for_each_old_connector_in_state(state, old_conn_state, i)
for_each_new_connector_in_state(state, new_conn_state, i)
for_each_oldnew_connector_in_state(state, old_conn_state, new_conn_state, i)
So only four in total for each object type, instead of the current
three.
--
Ville Syrjälä
Intel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
