Re: [PATCH 0/8] Convert the intel iommu driver to the dma-iommu api
Hi Tom, On 2019-12-21 8:03 a.m., Tom Murphy wrote: > This patchset converts the intel iommu driver to the dma-iommu api. Just wanted to note that I've rebased your series on recent kernels and have done some testing on my old Sandybridge machine (without the DO NOT MERGE patch) and have found no issues. I hope this can make progress soon and get merged soon. If you like you can add: Tested-By: Logan Gunthorpe > While converting the driver I exposed a bug in the intel i915 driver which > causes a huge amount of artifacts on the screen of my laptop. You can see a > picture of it here: > https://github.com/pippy360/kernelPatches/blob/master/IMG_20191219_225922.jpg > > This issue is most likely in the i915 driver and is most likely caused by the > driver not respecting the return value of the dma_map_ops::map_sg function. > You can see the driver ignoring the return value here: > https://github.com/torvalds/linux/blob/7e0165b2f1a912a06e381e91f0f4e495f4ac3736/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c#L51 > > Previously this didn’t cause issues because the intel map_sg always returned > the same number of elements as the input scatter gather list but with the > change to this dma-iommu api this is no longer the case. I wasn’t able to > track the bug down to a specific line of code unfortunately. I did some digging into this myself and while I don't have full patch, I think I traced it closer to the problem. Sadly, ignoring the number of nents returned by map_sg() is endemic to dma-buf users, but AMD's GPU driver seems to do the same thing, presumably without issues. Digging a bit further, I found that the i915 has an "innovative" way of iterating through SGLs, see [1]. I suspect if __sgt_iter is changed to increment with sg_dma_len() and return NULL when there is no length left, it may fix the issue. But, sorry, I don't really have the means or time to fix and test this myself. Thanks, Logan [1] https://elixir.bootlin.com/linux/v5.7-rc7/source/drivers/gpu/drm/i915/i915_scatterlist.h#L76 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Potential Memory Leak Bugs in drivers/gpu/drm/virtio/virtgpu_vq.c (Linux 5.6).
Ok, thanks! Gerd Hoffmann 于2020年5月28日周四 下午4:25写道： > > On Thu, May 28, 2020 at 03:57:05PM +0800, Dongyang Zhan wrote: > > Hi, > > My name is Dongyang Zhan, I am a security researcher. > > Currently, I found two possible memory bugs in > > drivers/gpu/drm/virtio/virtgpu_vq.c (Linux 5.6). > > I hope you can help me to confirm them. Thank you. > > Sorry. Not confirmed. You should do a better job verifying your > claims before bugging people. > > > The first one is resp_buf will not be release in > > virtio_gpu_cmd_get_display_info() with the condition > > (resp_size <= MAX_INLINE_RESP_SIZE) in virtio_gpu_alloc_cmd_resp(). > > In that code path resp_size equals sizeof(struct > virtio_gpu_resp_display_info) which is larger than MAX_INLINE_RESP_SIZE > so the condition is never true and no leak happens. > > take care, > Gerd > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v6 08/16] soc: mediatek: cmdq: add write_s function
Hi Matthias,
Thanks for your comment.
On Thu, 2020-05-28 at 23:08 +0200, Matthias Brugger wrote:
>
> On 28/05/2020 19:04, Dennis YC Hsieh wrote:
> > add write_s function in cmdq helper functions which
> > writes value contains in internal register to address
> > with large dma access support.
> >
> > Signed-off-by: Dennis YC Hsieh
> > ---
> > drivers/soc/mediatek/mtk-cmdq-helper.c | 21 -
> > include/linux/mailbox/mtk-cmdq-mailbox.h | 1 +
> > include/linux/soc/mediatek/mtk-cmdq.h| 20
> > 3 files changed, 41 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c
> > b/drivers/soc/mediatek/mtk-cmdq-helper.c
> > index 33153d17c9d9..ee24c0ec0a24 100644
> > --- a/drivers/soc/mediatek/mtk-cmdq-helper.c
> > +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
> > @@ -18,6 +18,10 @@ struct cmdq_instruction {
> > union {
> > u32 value;
> > u32 mask;
> > + struct {
> > + u16 arg_c;
> > + u16 src_reg;
> > + };
> > };
> > union {
> > u16 offset;
> > @@ -29,7 +33,7 @@ struct cmdq_instruction {
> > struct {
> > u8 sop:5;
> > u8 arg_c_t:1;
> > - u8 arg_b_t:1;
> > + u8 src_t:1;
>
> This should be part of 7/16.
ok, I'll move it
>
> > u8 dst_t:1;
> > };
> > };
> > @@ -222,6 +226,21 @@ int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8
> > subsys,
> > }
> > EXPORT_SYMBOL(cmdq_pkt_write_mask);
> >
> > +int cmdq_pkt_write_s(struct cmdq_pkt *pkt, u16 high_addr_reg_idx,
> > +u16 addr_low, u16 src_reg_idx)
> > +{
> > + struct cmdq_instruction inst = { {0} };
>
> If you want an empty struct on the stack, I think {}; should be enough, right?
Yes, I'll change the style, thanks
Regards,
Dennis
>
> Regards,
> Matthias
>
> > +
> > + inst.op = CMDQ_CODE_WRITE_S;
> > + inst.src_t = CMDQ_REG_TYPE;
> > + inst.sop = high_addr_reg_idx;
> > + inst.offset = addr_low;
> > + inst.src_reg = src_reg_idx;
> > +
> > + return cmdq_pkt_append_command(pkt, inst);
> > +}
> > +EXPORT_SYMBOL(cmdq_pkt_write_s);
> > +
> > int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event)
> > {
> > struct cmdq_instruction inst = { {0} };
> > diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h
> > b/include/linux/mailbox/mtk-cmdq-mailbox.h
> > index 121c3bb6d3de..ee67dd3b86f5 100644
> > --- a/include/linux/mailbox/mtk-cmdq-mailbox.h
> > +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
> > @@ -59,6 +59,7 @@ enum cmdq_code {
> > CMDQ_CODE_JUMP = 0x10,
> > CMDQ_CODE_WFE = 0x20,
> > CMDQ_CODE_EOC = 0x40,
> > + CMDQ_CODE_WRITE_S = 0x90,
> > CMDQ_CODE_LOGIC = 0xa0,
> > };
> >
> > diff --git a/include/linux/soc/mediatek/mtk-cmdq.h
> > b/include/linux/soc/mediatek/mtk-cmdq.h
> > index 83340211e1d3..d623f1aa7814 100644
> > --- a/include/linux/soc/mediatek/mtk-cmdq.h
> > +++ b/include/linux/soc/mediatek/mtk-cmdq.h
> > @@ -12,6 +12,8 @@
> > #include
> >
> > #define CMDQ_NO_TIMEOUT0xu
> > +#define CMDQ_ADDR_HIGH(addr) ((u32)(((addr) >> 16) & GENMASK(31, 0)))
> > +#define CMDQ_ADDR_LOW(addr)((u16)(addr) | BIT(1))
> >
> > struct cmdq_pkt;
> >
> > @@ -102,6 +104,24 @@ int cmdq_pkt_write(struct cmdq_pkt *pkt, u8 subsys,
> > u16 offset, u32 value);
> > int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8 subsys,
> > u16 offset, u32 value, u32 mask);
> >
> > +/**
> > + * cmdq_pkt_write_s() - append write_s command to the CMDQ packet
> > + * @pkt: the CMDQ packet
> > + * @high_addr_reg_idx: internal register ID which contains high
> > address of pa
> > + * @addr_low: low address of pa
> > + * @src_reg_idx: the CMDQ internal register ID which cache source value
> > + * @mask: the specified target address mask, use U32_MAX if no need
> > + *
> > + * Return: 0 for success; else the error code is returned
> > + *
> > + * Support write value to physical address without subsys. Use
> > CMDQ_ADDR_HIGH()
> > + * to get high address and call cmdq_pkt_assign() to assign value into
> > internal
> > + * reg. Also use CMDQ_ADDR_LOW() to get low address for addr_low parameter
> > when
> > + * call to this function.
> > + */
> > +int cmdq_pkt_write_s(struct cmdq_pkt *pkt, u16 high_addr_reg_idx,
> > +u16 addr_low, u16 src_reg_idx);
> > +
> > /**
> > * cmdq_pkt_wfe() - append wait for event command to the CMDQ packet
> > * @pkt: the CMDQ packet
> >
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC 02/17] dma-fence: basic lockdep annotations
On Thu, May 28, 2020 at 11:54 PM Luben Tuikov wrote:
>
> On 2020-05-12 4:59 a.m., Daniel Vetter wrote:
> > Design is similar to the lockdep annotations for workers, but with
> > some twists:
> >
> > - We use a read-lock for the execution/worker/completion side, so that
> > this explicit annotation can be more liberally sprinkled around.
> > With read locks lockdep isn't going to complain if the read-side
> > isn't nested the same way under all circumstances, so ABBA deadlocks
> > are ok. Which they are, since this is an annotation only.
> >
> > - We're using non-recursive lockdep read lock mode, since in recursive
> > read lock mode lockdep does not catch read side hazards. And we
> > _very_ much want read side hazards to be caught. For full details of
> > this limitation see
> >
> > commit e91498589746065e3ae95d9a00b068e525eec34f
> > Author: Peter Zijlstra
> > Date: Wed Aug 23 13:13:11 2017 +0200
> >
> > locking/lockdep/selftests: Add mixed read-write ABBA tests
> >
> > - To allow nesting of the read-side explicit annotations we explicitly
> > keep track of the nesting. lock_is_held() allows us to do that.
> >
> > - The wait-side annotation is a write lock, and entirely done within
> > dma_fence_wait() for everyone by default.
> >
> > - To be able to freely annotate helper functions I want to make it ok
> > to call dma_fence_begin/end_signalling from soft/hardirq context.
> > First attempt was using the hardirq locking context for the write
> > side in lockdep, but this forces all normal spinlocks nested within
> > dma_fence_begin/end_signalling to be spinlocks. That bollocks.
> >
> > The approach now is to simple check in_atomic(), and for these cases
> > entirely rely on the might_sleep() check in dma_fence_wait(). That
> > will catch any wrong nesting against spinlocks from soft/hardirq
> > contexts.
> >
> > The idea here is that every code path that's critical for eventually
> > signalling a dma_fence should be annotated with
> > dma_fence_begin/end_signalling. The annotation ideally starts right
> > after a dma_fence is published (added to a dma_resv, exposed as a
> > sync_file fd, attached to a drm_syncobj fd, or anything else that
> > makes the dma_fence visible to other kernel threads), up to and
> > including the dma_fence_wait(). Examples are irq handlers, the
> > scheduler rt threads, the tail of execbuf (after the corresponding
> > fences are visible), any workers that end up signalling dma_fences and
> > really anything else. Not annotated should be code paths that only
> > complete fences opportunistically as the gpu progresses, like e.g.
> > shrinker/eviction code.
> >
> > The main class of deadlocks this is supposed to catch are:
> >
> > Thread A:
> >
> > mutex_lock(A);
> > mutex_unlock(A);
> >
> > dma_fence_signal();
> >
> > Thread B:
> >
> > mutex_lock(A);
> > dma_fence_wait();
> > mutex_unlock(A);
> >
> > Thread B is blocked on A signalling the fence, but A never gets around
> > to that because it cannot acquire the lock A.
> >
> > Note that dma_fence_wait() is allowed to be nested within
> > dma_fence_begin/end_signalling sections. To allow this to happen the
> > read lock needs to be upgraded to a write lock, which means that any
> > other lock is acquired between the dma_fence_begin_signalling() call and
> > the call to dma_fence_wait(), and still held, this will result in an
> > immediate lockdep complaint. The only other option would be to not
> > annotate such calls, defeating the point. Therefore these annotations
> > cannot be sprinkled over the code entirely mindless to avoid false
> > positives.
> >
> > v2: handle soft/hardirq ctx better against write side and dont forget
> > EXPORT_SYMBOL, drivers can't use this otherwise.
> >
> > Cc: linux-me...@vger.kernel.org
> > Cc: linaro-mm-...@lists.linaro.org
> > Cc: linux-r...@vger.kernel.org
> > Cc: amd-...@lists.freedesktop.org
> > Cc: intel-...@lists.freedesktop.org
> > Cc: Chris Wilson
> > Cc: Maarten Lankhorst
> > Cc: Christian König
> > Signed-off-by: Daniel Vetter
> > ---
> > drivers/dma-buf/dma-fence.c | 53 +
> > include/linux/dma-fence.h | 12 +
> > 2 files changed, 65 insertions(+)
> >
> > diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
> > index 6802125349fb..d5c0fd2efc70 100644
> > --- a/drivers/dma-buf/dma-fence.c
> > +++ b/drivers/dma-buf/dma-fence.c
> > @@ -110,6 +110,52 @@ u64 dma_fence_context_alloc(unsigned num)
> > }
> > EXPORT_SYMBOL(dma_fence_context_alloc);
> >
> > +#ifdef CONFIG_LOCKDEP
> > +struct lockdep_map dma_fence_lockdep_map = {
> > + .name = "dma_fence_map"
> > +};
> > +
> > +bool dma_fence_begin_signalling(void)
> > +{
> > + /* explicitly nesting ... */
> > + if (lock_is_held_type(&dma_fence_lockdep_map, 1))
> > + return true;
> > +
> > + /* rely on might_sleep check for soft/hardirq locks */
> > + if (in_ato
Re: [RFC PATCH 3/6] dt-bindings: display/bridge/nwl-dsi: Drop mux handling
Hi Rob, On Thu, May 28, 2020 at 01:59:14PM -0600, Rob Herring wrote: > On Fri, May 15, 2020 at 03:12:12PM +0200, Guido Günther wrote: > > No need to encode the SoC specifics in the bridge driver. For the > > imx8mq we can use the mux-input-bridge. > > You can't just change bindings like this. You'd still have to support > the "old" way. But IMO, this way is the right way. My understanding is that binding stability only applies to released kernels and this binding never was in released kernel yet. Does it still apply in this case? Cheers, -- Guido > > > > > Signed-off-by: Guido Günther > > --- > > .../devicetree/bindings/display/bridge/nwl-dsi.yaml | 6 -- > > 1 file changed, 6 deletions(-) > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[git pull] drm fixes for 5.7 final (apologies release)
Hey, Apologies for previous PR, I did build it locally, I just don't build EXPERT kernels, I expect if I ever get a new builder I should add a few more configs to my list. I've just dropped the i915 PR from this completely, I'm sure when they wake up they'll be able to tell us what we are missing due to mistakes made. Dave. drm-fixes-2020-05-29-1: drm fixes for 5.7 final amdgpu: - display atomic test fix - Fix soft hang in display vupdate code ingenic: - fix pointer cast - fix crtc atomic check callback The following changes since commit 9cb1fd0efd195590b828b9b865421ad345a4a145: Linux 5.7-rc7 (2020-05-24 15:32:54 -0700) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm tags/drm-fixes-2020-05-29-1 for you to fetch changes up to ed9244bd0b265c4c0866a9246c6e7cca1cca3acf: Merge tag 'drm-misc-fixes-2020-05-28' of git://anongit.freedesktop.org/drm/drm-misc into drm-fixes (2020-05-29 12:11:11 +1000) drm fixes for 5.7 final amdgpu: - display atomic test fix - Fix soft hang in display vupdate code ingenic: - fix pointer cast - fix crtc atomic check callback Aric Cyr (1): drm/amd/display: Fix potential integer wraparound resulting in a hang Dave Airlie (2): Merge tag 'amd-drm-fixes-5.7-2020-05-27' of git://people.freedesktop.org/~agd5f/linux into drm-fixes Merge tag 'drm-misc-fixes-2020-05-28' of git://anongit.freedesktop.org/drm/drm-misc into drm-fixes Paul Cercueil (2): gpu/drm: ingenic: Fix bogus crtc_atomic_check callback gpu/drm: Ingenic: Fix opaque pointer casted to wrong type Simon Ser (1): drm/amd/display: drop cursor position check in atomic test drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 7 --- drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c | 2 ++ drivers/gpu/drm/ingenic/ingenic-drm.c | 6 +++--- 3 files changed, 5 insertions(+), 10 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [git pull] drm fixes for 5.7-rc8/final
On Fri, 29 May 2020 at 12:02, Dave Airlie wrote:
>
> On Fri, 29 May 2020 at 11:49, Linus Torvalds
> wrote:
> >
> > On Thu, May 28, 2020 at 5:21 PM Dave Airlie wrote:
> > >
> > > Seems to have wound down nicely, a couple of i915 fixes, amdgpu fixes
> > > and minor ingenic fixes.
> >
> > Dave, this doesn't even build. WTF?
> >
> > In drivers/gpu/drm/i915/gt/selftest_lrc.c, there's a
> > engine_heartbeat_disable() function that takes two arguments, but the
> > new "live_timeslice_nopreempt()" gives it only one.
> >
> > I'd blame a merge problem, since the failure is in new code, but the
> > problem exists in your top-of-tree, not just my merge.
> >
> > In fact, it's not even your merge of the i915 tree that is the source
> > of the problem (although the fact that you clearly didn't _test_ the
> > end result most definitely is _part_ of the problem!).
> >
> > Because the problem exists in the commit that introduced that thing:
> > commit 1f65efb624c4 ("drm/i915/gt: Prevent timeslicing into
> > unpreemptable requests").
> >
> > It's garbage, and never compiled.
>
> I thought I'd dropped the ball completely. but I see it's a selftest
> failure, I must not have selftests built in my config here, I don't do
> exhaustive builds randconfig
>
> This has also been built by Intel, but I'm assuming they missed their
> selftest bits as well.
>
> I'll revert that and resend.
I did drop the ball in one way, I see sfr reported it broken this morning
I normally expect stuff coming from Intel has been through their CI,
even their fixes tree generally gets pushed through that system before
I get it, and it usually catches these things.
I might have to push back on intel fixes this late in the day, as
maybe the land on next and cherry-pick to fixes model has made them a
bit lax on how much stuff goes through CI.
I've just drop the whole i915 fixes from the tree and will resend with
it removed.
Dave.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH][next] drm/amdkfd: fix a dereference of pdd before it is null checked
On 2020-05-28 18:24, Colin King wrote:
From: Colin Ian King
Currently pointer pdd is being dereferenced when assigning pointer
dpm and then pdd is being null checked. Fix this by checking if
pdd is null before the dereference of pdd occurs.
Addresses-Coverity: ("Dereference before null check")
Fixes: 522b89c63370 ("drm/amdkfd: Track SDMA utilization per process")
Signed-off-by: Colin Ian King
Reviewed-by: Felix Kuehling
I applied the patch to our internal amd-staging-drm-next.
Regards,
Felix
---
drivers/gpu/drm/amd/amdkfd/kfd_process.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
index 25636789f3d3..bdc58741b32e 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ -103,10 +103,11 @@ static void kfd_sdma_activity_worker(struct work_struct
*work)
return;
pdd = workarea->pdd;
+ if (!pdd)
+ return;
dqm = pdd->dev->dqm;
qpd = &pdd->qpd;
-
- if (!pdd || !dqm || !qpd)
+ if (!dqm || !qpd)
return;
mm = get_task_mm(pdd->process->lead_thread);
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [git pull] drm fixes for 5.7-rc8/final
On Fri, 29 May 2020 at 11:49, Linus Torvalds
wrote:
>
> On Thu, May 28, 2020 at 5:21 PM Dave Airlie wrote:
> >
> > Seems to have wound down nicely, a couple of i915 fixes, amdgpu fixes
> > and minor ingenic fixes.
>
> Dave, this doesn't even build. WTF?
>
> In drivers/gpu/drm/i915/gt/selftest_lrc.c, there's a
> engine_heartbeat_disable() function that takes two arguments, but the
> new "live_timeslice_nopreempt()" gives it only one.
>
> I'd blame a merge problem, since the failure is in new code, but the
> problem exists in your top-of-tree, not just my merge.
>
> In fact, it's not even your merge of the i915 tree that is the source
> of the problem (although the fact that you clearly didn't _test_ the
> end result most definitely is _part_ of the problem!).
>
> Because the problem exists in the commit that introduced that thing:
> commit 1f65efb624c4 ("drm/i915/gt: Prevent timeslicing into
> unpreemptable requests").
>
> It's garbage, and never compiled.
I thought I'd dropped the ball completely. but I see it's a selftest
failure, I must not have selftests built in my config here, I don't do
exhaustive builds randconfig
This has also been built by Intel, but I'm assuming they missed their
selftest bits as well.
I'll revert that and resend.
Sorry for the noise. I'll add self tests to my builds here for future ones.
Dave.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [git pull] drm fixes for 5.7-rc8/final
On Thu, May 28, 2020 at 5:21 PM Dave Airlie wrote:
>
> Seems to have wound down nicely, a couple of i915 fixes, amdgpu fixes
> and minor ingenic fixes.
Dave, this doesn't even build. WTF?
In drivers/gpu/drm/i915/gt/selftest_lrc.c, there's a
engine_heartbeat_disable() function that takes two arguments, but the
new "live_timeslice_nopreempt()" gives it only one.
I'd blame a merge problem, since the failure is in new code, but the
problem exists in your top-of-tree, not just my merge.
In fact, it's not even your merge of the i915 tree that is the source
of the problem (although the fact that you clearly didn't _test_ the
end result most definitely is _part_ of the problem!).
Because the problem exists in the commit that introduced that thing:
commit 1f65efb624c4 ("drm/i915/gt: Prevent timeslicing into
unpreemptable requests").
It's garbage, and never compiled.
See here:
git grep -1wh engine_heartbeat_disable 1f65efb62 \
drivers/gpu/drm/i915/gt/selftest_lrc.c
and you'll see how the definition of that function looks like this:
static void engine_heartbeat_disable(struct intel_engine_cs *engine,
unsigned long *saved)
but then in the middle of that grep, you'll find
engine_heartbeat_disable(engine);
WTF?
That commit seems to be a cherry-pick of another commit, and maybe it
worked in that other context (which I don't see), but it sure doesn't
work in the context it was then cherry-picked into.
So people took that thing, and it went through at least two different
people WHO NEVER EVEN BOTHERED TO TEST IF IT BUILDS!
Christ, people.
This is why I absolutely DO NOT WANT TO SEE random rebases or
cherry-picks and then sending the resulting untested crap on to me.
Because it's exactly that: untested crap.
It doesn't matter *how* well you have tested a commit in some original
context: the moment you rebase it (or cherry-pick it, which is just
another form of rebasing), it's a completely new commit in a
completely new environment, and all your old testing is null and void.
Guys, get your act together! I should not be getting these kinds of
shit pull requests days before a release!
And how the hell did this not get any build testing at any point
before asking me to pull?
Linus
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[git pull] drm fixes for 5.7-rc8/final
Hey Linus, Seems to have wound down nicely, a couple of i915 fixes, amdgpu fixes and minor ingenic fixes. Should be it until the merge window. Dave. drm-fixes-2020-05-29: drm fixes for 5.7 final i915: - gcc 9 compile warning fix - timeslicing fixes amdgpu: - display atomic test fix - Fix soft hang in display vupdate code ingenic: - fix pointer cast - fix crtc atomic check callback The following changes since commit 9cb1fd0efd195590b828b9b865421ad345a4a145: Linux 5.7-rc7 (2020-05-24 15:32:54 -0700) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm tags/drm-fixes-2020-05-29 for you to fetch changes up to d099f415d50c3980339479f56f124f8bfa6875bc: Merge tag 'drm-misc-fixes-2020-05-28' of git://anongit.freedesktop.org/drm/drm-misc into drm-fixes (2020-05-29 09:25:23 +1000) drm fixes for 5.7 final i915: - gcc 9 compile warning fix - timeslicing fixes amdgpu: - display atomic test fix - Fix soft hang in display vupdate code ingenic: - fix pointer cast - fix crtc atomic check callback Aric Cyr (1): drm/amd/display: Fix potential integer wraparound resulting in a hang Arnd Bergmann (2): drm/i915/pmu: avoid an maybe-uninitialized warning drm/i915: work around false-positive maybe-uninitialized warning Chris Wilson (2): drm/i915/gt: Incorporate the virtual engine into timeslicing drm/i915/gt: Prevent timeslicing into unpreemptable requests Dave Airlie (3): Merge tag 'amd-drm-fixes-5.7-2020-05-27' of git://people.freedesktop.org/~agd5f/linux into drm-fixes Merge tag 'drm-intel-fixes-2020-05-28' of git://anongit.freedesktop.org/drm/drm-intel into drm-fixes Merge tag 'drm-misc-fixes-2020-05-28' of git://anongit.freedesktop.org/drm/drm-misc into drm-fixes Paul Cercueil (2): gpu/drm: ingenic: Fix bogus crtc_atomic_check callback gpu/drm: Ingenic: Fix opaque pointer casted to wrong type Simon Ser (1): drm/amd/display: drop cursor position check in atomic test drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 7 -- .../drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c | 2 + drivers/gpu/drm/i915/gt/intel_lrc.c| 31 -- drivers/gpu/drm/i915/gt/selftest_lrc.c | 118 - drivers/gpu/drm/i915/gt/selftest_workarounds.c | 2 + drivers/gpu/drm/i915/i915_pmu.c| 84 +++ drivers/gpu/drm/i915/i915_priolist_types.h | 2 +- drivers/gpu/drm/ingenic/ingenic-drm.c | 6 +- 8 files changed, 192 insertions(+), 60 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] dt-bindings: display: renesas, du: Convert binding to YAML
Hi Rob, On Thu, May 28, 2020 at 12:52:44PM -0600, Rob Herring wrote: > On Fri, May 15, 2020 at 03:33:40AM +0300, Laurent Pinchart wrote: > > Convert the Renesas R-Car DU text binding to YAML. > > > > Signed-off-by: Laurent Pinchart > > --- > > .../bindings/display/renesas,du.txt | 139 --- > > .../bindings/display/renesas,du.yaml | 915 ++ > > A 'normal' conversion is about double the lines. I guess this is a sign > that the original was not well specified. The original was specified in a much more compressed form (for instance there was a table with one line per SoC to describe the port node, the YAML equivalent has to be longer). > Maybe this can be split to reduce some of the if/then? One way is define > a common 'include' file that each specific instance can reference With your recommendation of using pattern instead of enum for the dclkin clock names, we're down to 848 lines, it's already a bit better :-) I could indeed split the file, but I'll then run into naming issues. If you look at the compatible strings for each of the if...then...else, they don't have easy patterns that could be used to name files. I could name the files based on one arbitrarily chosen compat string among the multiple values that are supported, but I think that would become confusing. I could also have one file per SoC, but we'll then end up with lots of files, several of them being totally identical except for the compatible string. Would you mind keeping it as-is, or do you think it really needs to be split ? > [...] > > > diff --git a/Documentation/devicetree/bindings/display/renesas,du.yaml > > b/Documentation/devicetree/bindings/display/renesas,du.yaml > > new file mode 100644 > > index ..ca48065afe1f > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/display/renesas,du.yaml > > @@ -0,0 +1,915 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/display/renesas,du.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Renesas R-Car Display Unit (DU) > > + > > +maintainers: > > + - Laurent Pinchart > > + > > +description: | > > + These DT bindings describe the Display Unit embedded in the Renesas R-Car > > + Gen1, R-Car Gen2, R-Car Gen3, RZ/G1 and RZ/G2 SoCs. > > + > > +properties: > > + compatible: > > +enum: > > + - renesas,du-r8a7743 # for RZ/G1M compatible DU > > + - renesas,du-r8a7744 # for RZ/G1N compatible DU > > + - renesas,du-r8a7745 # for RZ/G1E compatible DU > > + - renesas,du-r8a77470 # for RZ/G1C compatible DU > > + - renesas,du-r8a774a1 # for RZ/G2M compatible DU > > + - renesas,du-r8a774b1 # for RZ/G2N compatible DU > > + - renesas,du-r8a774c0 # for RZ/G2E compatible DU > > + - renesas,du-r8a7779 # for R-Car H1 compatible DU > > + - renesas,du-r8a7790 # for R-Car H2 compatible DU > > + - renesas,du-r8a7791 # for R-Car M2-W compatible DU > > + - renesas,du-r8a7792 # for R-Car V2H compatible DU > > + - renesas,du-r8a7793 # for R-Car M2-N compatible DU > > + - renesas,du-r8a7794 # for R-Car E2 compatible DU > > + - renesas,du-r8a7795 # for R-Car H3 compatible DU > > + - renesas,du-r8a7796 # for R-Car M3-W compatible DU > > + - renesas,du-r8a77965 # for R-Car M3-N compatible DU > > + - renesas,du-r8a77970 # for R-Car V3M compatible DU > > + - renesas,du-r8a77980 # for R-Car V3H compatible DU > > + - renesas,du-r8a77990 # for R-Car E3 compatible DU > > + - renesas,du-r8a77995 # for R-Car D3 compatible DU > > + > > + reg: > > +maxItems: 1 > > + > > + # See compatible-specific constraints below. > > + clocks: true > > + clock-names: true > > + interrupts: true > > + resets: true > > + reset-names: true > > + > > + ports: > > +type: object > > +description: | > > + The connections to the DU output video ports are modeled using the OF > > + graph bindings specified in > > Documentation/devicetree/bindings/graph.txt. > > + The number of ports and their assignment are model-dependent. Each > > port > > + shall have a single endpoint. > > + > > +properties: > > + '#address-cells': > > +const: 1 > > + > > + '#size-cells': > > +const: 0 > > + > > +patternProperties: > > + "^port@[0-3]$": > > +type: object > > + > > +properties: > > + reg: > > +maxItems: 1 > > + > > + endpoint: > > +type: object > > + > > +properties: > > + remote-endpoint: > > +$ref: /schemas/types.yaml#/definitions/phandle > > + > > +required: > > + - remote-endpoint > > + > > +additionalProperties: false > > + > > +additionalProperties: false > > You can drop this and assume there's a generic check for this. Though I > guess this does ensure only 're
Re: [RFC PATCH 1/6] dt-bindings: display/bridge: Add binding for input mux bridge
Hi Rob, On Thu, May 28, 2020 at 01:48:04PM -0600, Rob Herring wrote: > On Fri, May 15, 2020 at 03:12:10PM +0200, Guido Günther wrote: > > The bridge allows to select the input source via a mux controller. > > > > Signed-off-by: Guido Günther > > --- > > .../display/bridge/mux-input-bridge.yaml | 123 ++ > > 1 file changed, 123 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/display/bridge/mux-input-bridge.yaml > > > > diff --git > > a/Documentation/devicetree/bindings/display/bridge/mux-input-bridge.yaml > > b/Documentation/devicetree/bindings/display/bridge/mux-input-bridge.yaml > > new file mode 100644 > > index ..4029cf63ee5c > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/display/bridge/mux-input-bridge.yaml > > @@ -0,0 +1,123 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/display/bridge/mux-input-bridge.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: DRM input source selection via multiplexer > > DRM is not a hardware thing. > > The graph binding is already designed to support muxing. Generally, > multiple endpoints on an input node is a mux. So either the device with > the input ports knows how to select the input, or you just need a > mux-control property for the port to have some other device implement > the control. The mux modelled by this binding is used by Guido with the NWL DSI bridge integrated in the i.MX8. The NWL DSI is an IP core that has a single input. The i.MX8 has an additional mux in front of the NWL DSI, to select between the two display controllers in the SoC (eLCDIF and DCSS). The mux doesn't belong to any of the display controller, it's really glue logic between the display controllers and the NWL DSI bridge. I agree that the bindings shouldn't mention DRM. I would however prefer not adding a mux-control property and multiple input ports to the NWL DSI binding, as the mux isn't internal to that IP core (if we go that route, we would need to add mux control to any IP core that would be integrated in an SoC with a mux in front). As DT should describe the hardware, I think describing the standalone mux between the display controllers and the NWL DSI bridge makex sense. We already have a DT binding for a video mux (Documentation/devicetree/bindings/media/video-mux.txt). From a DT point of view, I see no reason not to reuse that. From a driver point of view that will be messy, as the driver that binds to the video-mux compatible string is part of V4L2. That's a driver issue however (and not a new one, we already have devices that can be part of a video capture pipeline or a video display pipeline), and should be solved on the software side, not the DT side. It will however not be easy work, which explains why, so far, everybody has ignored the issue hoping that someone else would be hit by it first. We may have reached that day. > You could do it like you have below. That would be appropriate if > there's a separate h/w device controlling the muxing. Say for example > some board level device controlled by i2c. -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 00/91] drm/vc4: Support BCM2711 Display Pipelin
Hi Daniel, On Wed, May 27, 2020 at 05:15:12PM +0800, Daniel Drake wrote: > On Wed, May 27, 2020 at 5:13 PM Maxime Ripard wrote: > > I'm about to send a v3 today or tomorrow, I can Cc you (and Jian-Hong) if > > you > > want. > > That would be great, although given the potentially inconsistent > results we've been seeing so far it would be great if you could > additionally push a git branch somewhere. > That way we can have higher confidence that we are applying exactly > the same patches to the same base etc. So I sent a new iteration yesterday, and of course forgot to cc you... Sorry for that. I've pushed my current branch here: https://git.kernel.org/pub/scm/linux/kernel/git/mripard/linux.git/log/?h=rpi4-kms Maxime signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/imx: imx-ldb: Remove the leftover LVDS_BIT_MAP_SPWG/JEIDA enums
Hi Macro,
On Thu, 2020-05-28 at 11:34 +0200, Marco Felsch wrote:
> Hi Liu,
>
> pls check [1], I already send patches for it. Those stuck because we
> need to verify Philipp's proposal.
Yeah, '[PATCH 09/17] drm/imx: imx-ldb: remove useless enum' in your
patch series does the same thing as this patch does. I'm fine to drop
this patch then.
Regarding Philipp's proposal on the use-after-free issue, we've got the
basically the same implemetation in downstream ldb driver to address
the issue.
Regards,
Liu Ying
>
> [1]
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.spinics.net%2Flists%2Farm-kernel%2Fmsg789309.html&data=02%7C01%7Cvictor.liu%40nxp.com%7C8da3e383727f4fe1e0dd08d802ea5458%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637262552748306435&sdata=DWN%2F3jRT07qmPWxlgSTuD9eRiRVoOHWGRmfrgj0ZA3A%3D&reserved=0
>
> Regards,
> Marco
>
> On 20-05-28 10:55, Liu Ying wrote:
> > Gentle ping...
> >
> > On Tue, 2020-05-12 at 15:21 +0800, Liu Ying wrote:
> > > No one is using the LVDS_BIT_MAP_SPWG/JEIDA enums since a
> > > previous
> > > commit which changes the imx_ldb_bit_mappings[] array definition,
> > > so let's remove them.
> > >
> > > Fixes: 5e501ed7253b ("drm/imx: imx-ldb: allow to determine bus
> > > format
> > > from the connected panel")
> > > Cc: Philipp Zabel
> > > Cc: Sascha Hauer
> > > Cc: Pengutronix Kernel Team
> > > Cc: NXP Linux Team
> > > Signed-off-by: Liu Ying
> > > ---
> > > drivers/gpu/drm/imx/imx-ldb.c | 5 -
> > > 1 file changed, 5 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/imx/imx-ldb.c
> > > b/drivers/gpu/drm/imx/imx-
> > > ldb.c
> > > index 4da22a9..19a00eb 100644
> > > --- a/drivers/gpu/drm/imx/imx-ldb.c
> > > +++ b/drivers/gpu/drm/imx/imx-ldb.c
> > > @@ -477,11 +477,6 @@ static int imx_ldb_register(struct
> > > drm_device
> > > *drm,
> > > return 0;
> > > }
> > >
> > > -enum {
> > > - LVDS_BIT_MAP_SPWG,
> > > - LVDS_BIT_MAP_JEIDA
> > > -};
> > > -
> > > struct imx_ldb_bit_mapping {
> > > u32 bus_format;
> > > u32 datawidth;
> >
> > ___
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel&data=02%7C01%7Cvictor.liu%40nxp.com%7C8da3e383727f4fe1e0dd08d802ea5458%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637262552748306435&sdata=isO%2Fp5kg9qJ8%2FLg5zqI8XazuxwHr1hPjiyTHFhy0n2o%3D&reserved=0
> >
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/imx: imx-ldb: Disable both channels for split mode in enc->disable()
Hi Marco,
On Thu, 2020-05-28 at 11:31 +0200, Marco Felsch wrote:
> Hi Liu
>
> On 20-05-28 10:58, Liu Ying wrote:
> > Gentle ping...
>
> Please check my "spring cleanup series" [1] which do the split:
It looks that your series doesn't disable both lvds channels in the
encoder disablement callback for the ldb split mode.
So, I think this patch still stands.
Regards,
Liu Ying
>
> [1]
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.spinics.net%2Flists%2Farm-kernel%2Fmsg789309.html&data=02%7C01%7Cvictor.liu%40nxp.com%7Cd177bf874b8f41c404e108d802e9f179%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637262551086663318&sdata=PIMNspESt%2BYqEMV1Vh0eBn5PNjKPfz5GdJ6NrnM2bUw%3D&reserved=0
>
> Regards,
> Marco
>
> > On Tue, 2020-05-12 at 15:31 +0800, Liu Ying wrote:
> > > Both of the two LVDS channels should be disabled for split mode
> > > in the encoder's ->disable() callback, because they are enabled
> > > in the encoder's ->enable() callback.
> > >
> > > Fixes: 6556f7f82b9c ("drm: imx: Move imx-drm driver out of
> > > staging")
> > > Cc: Philipp Zabel
> > > Cc: Sascha Hauer
> > > Cc: Pengutronix Kernel Team
> > > Cc: NXP Linux Team
> > > Cc:
> > > Signed-off-by: Liu Ying
> > > ---
> > > drivers/gpu/drm/imx/imx-ldb.c | 7 ---
> > > 1 file changed, 4 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/imx/imx-ldb.c
> > > b/drivers/gpu/drm/imx/imx-
> > > ldb.c
> > > index 4da22a9..af4d0d8 100644
> > > --- a/drivers/gpu/drm/imx/imx-ldb.c
> > > +++ b/drivers/gpu/drm/imx/imx-ldb.c
> > > @@ -303,18 +303,19 @@ static void imx_ldb_encoder_disable(struct
> > > drm_encoder *encoder)
> > > {
> > > struct imx_ldb_channel *imx_ldb_ch =
> > > enc_to_imx_ldb_ch(encoder);
> > > struct imx_ldb *ldb = imx_ldb_ch->ldb;
> > > + int dual = ldb->ldb_ctrl & LDB_SPLIT_MODE_EN;
> > > int mux, ret;
> > >
> > > drm_panel_disable(imx_ldb_ch->panel);
> > >
> > > - if (imx_ldb_ch == &ldb->channel[0])
> > > + if (imx_ldb_ch == &ldb->channel[0] || dual)
> > > ldb->ldb_ctrl &= ~LDB_CH0_MODE_EN_MASK;
> > > - else if (imx_ldb_ch == &ldb->channel[1])
> > > + if (imx_ldb_ch == &ldb->channel[1] || dual)
> > > ldb->ldb_ctrl &= ~LDB_CH1_MODE_EN_MASK;
> > >
> > > regmap_write(ldb->regmap, IOMUXC_GPR2, ldb->ldb_ctrl);
> > >
> > > - if (ldb->ldb_ctrl & LDB_SPLIT_MODE_EN) {
> > > + if (dual) {
> > > clk_disable_unprepare(ldb->clk[0]);
> > > clk_disable_unprepare(ldb->clk[1]);
> > > }
> >
> > ___
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel&data=02%7C01%7Cvictor.liu%40nxp.com%7Cd177bf874b8f41c404e108d802e9f179%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637262551086663318&sdata=lNC4lwLUqM0upUxrrBtk1ap423lBIQlAAqDjdHv92LI%3D&reserved=0
> >
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 08/16] soc: mediatek: cmdq: add write_s function
add write_s function in cmdq helper functions which
writes value contains in internal register to address
with large dma access support.
Signed-off-by: Dennis YC Hsieh
---
drivers/soc/mediatek/mtk-cmdq-helper.c | 21 -
include/linux/mailbox/mtk-cmdq-mailbox.h | 1 +
include/linux/soc/mediatek/mtk-cmdq.h| 20
3 files changed, 41 insertions(+), 1 deletion(-)
diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c
b/drivers/soc/mediatek/mtk-cmdq-helper.c
index 33153d17c9d9..ee24c0ec0a24 100644
--- a/drivers/soc/mediatek/mtk-cmdq-helper.c
+++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
@@ -18,6 +18,10 @@ struct cmdq_instruction {
union {
u32 value;
u32 mask;
+ struct {
+ u16 arg_c;
+ u16 src_reg;
+ };
};
union {
u16 offset;
@@ -29,7 +33,7 @@ struct cmdq_instruction {
struct {
u8 sop:5;
u8 arg_c_t:1;
- u8 arg_b_t:1;
+ u8 src_t:1;
u8 dst_t:1;
};
};
@@ -222,6 +226,21 @@ int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8 subsys,
}
EXPORT_SYMBOL(cmdq_pkt_write_mask);
+int cmdq_pkt_write_s(struct cmdq_pkt *pkt, u16 high_addr_reg_idx,
+u16 addr_low, u16 src_reg_idx)
+{
+ struct cmdq_instruction inst = { {0} };
+
+ inst.op = CMDQ_CODE_WRITE_S;
+ inst.src_t = CMDQ_REG_TYPE;
+ inst.sop = high_addr_reg_idx;
+ inst.offset = addr_low;
+ inst.src_reg = src_reg_idx;
+
+ return cmdq_pkt_append_command(pkt, inst);
+}
+EXPORT_SYMBOL(cmdq_pkt_write_s);
+
int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event)
{
struct cmdq_instruction inst = { {0} };
diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h
b/include/linux/mailbox/mtk-cmdq-mailbox.h
index 121c3bb6d3de..ee67dd3b86f5 100644
--- a/include/linux/mailbox/mtk-cmdq-mailbox.h
+++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
@@ -59,6 +59,7 @@ enum cmdq_code {
CMDQ_CODE_JUMP = 0x10,
CMDQ_CODE_WFE = 0x20,
CMDQ_CODE_EOC = 0x40,
+ CMDQ_CODE_WRITE_S = 0x90,
CMDQ_CODE_LOGIC = 0xa0,
};
diff --git a/include/linux/soc/mediatek/mtk-cmdq.h
b/include/linux/soc/mediatek/mtk-cmdq.h
index 83340211e1d3..d623f1aa7814 100644
--- a/include/linux/soc/mediatek/mtk-cmdq.h
+++ b/include/linux/soc/mediatek/mtk-cmdq.h
@@ -12,6 +12,8 @@
#include
#define CMDQ_NO_TIMEOUT0xu
+#define CMDQ_ADDR_HIGH(addr) ((u32)(((addr) >> 16) & GENMASK(31, 0)))
+#define CMDQ_ADDR_LOW(addr)((u16)(addr) | BIT(1))
struct cmdq_pkt;
@@ -102,6 +104,24 @@ int cmdq_pkt_write(struct cmdq_pkt *pkt, u8 subsys, u16
offset, u32 value);
int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8 subsys,
u16 offset, u32 value, u32 mask);
+/**
+ * cmdq_pkt_write_s() - append write_s command to the CMDQ packet
+ * @pkt: the CMDQ packet
+ * @high_addr_reg_idx: internal register ID which contains high address of pa
+ * @addr_low: low address of pa
+ * @src_reg_idx: the CMDQ internal register ID which cache source value
+ * @mask: the specified target address mask, use U32_MAX if no need
+ *
+ * Return: 0 for success; else the error code is returned
+ *
+ * Support write value to physical address without subsys. Use CMDQ_ADDR_HIGH()
+ * to get high address and call cmdq_pkt_assign() to assign value into internal
+ * reg. Also use CMDQ_ADDR_LOW() to get low address for addr_low parameter when
+ * call to this function.
+ */
+int cmdq_pkt_write_s(struct cmdq_pkt *pkt, u16 high_addr_reg_idx,
+u16 addr_low, u16 src_reg_idx);
+
/**
* cmdq_pkt_wfe() - append wait for event command to the CMDQ packet
* @pkt: the CMDQ packet
--
2.18.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 16/16] soc: mediatek: cmdq: add set event function
Add set event function in cmdq helper functions to set specific event.
Signed-off-by: Dennis YC Hsieh
Reviewed-by: CK Hu
---
drivers/soc/mediatek/mtk-cmdq-helper.c | 15 +++
include/linux/mailbox/mtk-cmdq-mailbox.h | 1 +
include/linux/soc/mediatek/mtk-cmdq.h| 9 +
3 files changed, 25 insertions(+)
diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c
b/drivers/soc/mediatek/mtk-cmdq-helper.c
index 9e9a4c81553a..e9b1dd31f63b 100644
--- a/drivers/soc/mediatek/mtk-cmdq-helper.c
+++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
@@ -346,6 +346,21 @@ int cmdq_pkt_clear_event(struct cmdq_pkt *pkt, u16 event)
}
EXPORT_SYMBOL(cmdq_pkt_clear_event);
+int cmdq_pkt_set_event(struct cmdq_pkt *pkt, u16 event)
+{
+ struct cmdq_instruction inst = { {0} };
+
+ if (event >= CMDQ_MAX_EVENT)
+ return -EINVAL;
+
+ inst.op = CMDQ_CODE_WFE;
+ inst.value = CMDQ_WFE_UPDATE | CMDQ_WFE_UPDATE_VALUE;
+ inst.event = event;
+
+ return cmdq_pkt_append_command(pkt, inst);
+}
+EXPORT_SYMBOL(cmdq_pkt_set_event);
+
int cmdq_pkt_poll(struct cmdq_pkt *pkt, u8 subsys,
u16 offset, u32 value)
{
diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h
b/include/linux/mailbox/mtk-cmdq-mailbox.h
index 42d2a30e6a70..ba2d811183a9 100644
--- a/include/linux/mailbox/mtk-cmdq-mailbox.h
+++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
@@ -17,6 +17,7 @@
#define CMDQ_JUMP_PASS CMDQ_INST_SIZE
#define CMDQ_WFE_UPDATEBIT(31)
+#define CMDQ_WFE_UPDATE_VALUE BIT(16)
#define CMDQ_WFE_WAIT BIT(15)
#define CMDQ_WFE_WAIT_VALUE0x1
diff --git a/include/linux/soc/mediatek/mtk-cmdq.h
b/include/linux/soc/mediatek/mtk-cmdq.h
index 3216ccfff4a9..ea26020e63bf 100644
--- a/include/linux/soc/mediatek/mtk-cmdq.h
+++ b/include/linux/soc/mediatek/mtk-cmdq.h
@@ -199,6 +199,15 @@ int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event, bool
clear);
*/
int cmdq_pkt_clear_event(struct cmdq_pkt *pkt, u16 event);
+/**
+ * cmdq_pkt_set_event() - append set event command to the CMDQ packet
+ * @pkt: the CMDQ packet
+ * @event: the desired event to be set
+ *
+ * Return: 0 for success; else the error code is returned
+ */
+int cmdq_pkt_set_event(struct cmdq_pkt *pkt, u16 event);
+
/**
* cmdq_pkt_poll() - Append polling command to the CMDQ packet, ask GCE to
* execute an instruction that wait for a specified
--
2.18.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] xf86drm: add drmOpenByFB
Simon Ser 於 2020年5月25日 週一 上午3:25寫道： > On Sunday, May 24, 2020 8:53 PM, Daniel Vetter wrote: > > On Sat, May 23, 2020 at 5:44 PM Mauro Rossi issor.or...@gmail.com wrote: > > > > > OpenByFB is introduced to overcome GPU driver loading order issue > > > on a device with multiple GPUs, e.g. Intel iGPU and Nvidia dGPU > > > where the first drmfb kernel module loaded will become device file > > > /dev/dri/card0 and the second will become /dev/dri/card1 > > > The use case is to prefer Intel iGPU over dGPU, or viceversa, > > > in a deterministic and reliable manner. > > > OpenByFB function opens the DRM device with specified fb and node type, > > > thus enabling gralloc to open the correct device node of the primary fb, > > > regardless of the GPU driver loading order. > > > > The fbdev load ordering is as inconsistent/random as the dri node > > loading. Well more so, because you might have random firmware fbdev > > drivers hanging out there. Hence I'm not following how this solves > > anything for your problem. > > > > I think usually what userspace does it look at the boot_vga property > > of the underlying device in sysfs, and prefer that one. Thank you for the reply. I'm not the expert so I can't fully understand what you mean. What does 'boot_vga' mean and how could it help our problem? The main problem we're trying to solve is to find the DRM device of the primary framebuffer (fb0). I believe /sys/class/graphics/fb0/device/drm is the correct one we could use. At least it works on all devices I've tested including nvidia optimus notebooks. If there is a simpler way to get the DRM device of fb0 I'm happy to learn. > Yes. See [1] for an example of how to do this. > > [1]: > https://github.com/swaywm/wlroots/blob/5c942bd5972afee9a68cb15c14aa83b4b0aaf82d/backend/session/session.c#L331 Thank you for the example. However, our target platform is Android and Android doesn't have udev. So I'm afraid we can't use it. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/mm: add ig_frag selftest
This patch introduces fragmentation in the address range
and measures time taken by 10k insertions for each modes.
ig_frag() will fail if one of the mode takes more than 1 sec.
Output:
[ 37.326723] drm_mm: igt_sanitycheck - ok!
[ 37.326727] igt_debug 0x-0x0200: 512: free
[ 37.326728] igt_debug 0x0200-0x0600: 1024: used
[ 37.326728] igt_debug 0x0600-0x0a00: 1024: free
[ 37.326729] igt_debug 0x0a00-0x0e00: 1024: used
[ 37.326730] igt_debug 0x0e00-0x1000: 512: free
[ 37.326730] igt_debug total: 4096, used 2048 free 2048
[ 56.040064] drm_mm: best fragmented insert took 504 msecs
[ 56.082184] drm_mm: bottom-up fragmented insert took 40 msecs
[ 56.102255] drm_mm: top-down fragmented insert took 20 msecs
[ 56.107177] drm_mm: evict fragmented insert took 4 msecs
Signed-off-by: Nirmoy Das
---
drivers/gpu/drm/selftests/drm_mm_selftests.h | 1 +
drivers/gpu/drm/selftests/test-drm_mm.c | 59
2 files changed, 60 insertions(+)
diff --git a/drivers/gpu/drm/selftests/drm_mm_selftests.h
b/drivers/gpu/drm/selftests/drm_mm_selftests.h
index 6b943ea1c57d..8c87c964176b 100644
--- a/drivers/gpu/drm/selftests/drm_mm_selftests.h
+++ b/drivers/gpu/drm/selftests/drm_mm_selftests.h
@@ -14,6 +14,7 @@ selftest(insert, igt_insert)
selftest(replace, igt_replace)
selftest(insert_range, igt_insert_range)
selftest(align, igt_align)
+selftest(frag, igt_frag)
selftest(align32, igt_align32)
selftest(align64, igt_align64)
selftest(evict, igt_evict)
diff --git a/drivers/gpu/drm/selftests/test-drm_mm.c
b/drivers/gpu/drm/selftests/test-drm_mm.c
index 9aabe82dcd3a..4c08ca86bb86 100644
--- a/drivers/gpu/drm/selftests/test-drm_mm.c
+++ b/drivers/gpu/drm/selftests/test-drm_mm.c
@@ -1033,6 +1033,65 @@ static int igt_insert_range(void *ignored)
return 0;
}
+static int igt_frag(void *ignored)
+{
+ const struct insert_mode *mode;
+ struct drm_mm mm;
+ struct drm_mm_node *nodes, *node, *next;
+ unsigned int size = 4096, align = 8192;
+ unsigned long start, timeout = 1000;
+ const unsigned int max_count = 1;
+ unsigned int i;
+ int ret = -EINVAL;
+
+ /* For each of the possible insertion modes, we pass an size, alignment
+* value that is known to introduce fragmentation and check that it
+* doesn't take more than 1 sec.
+*/
+
+ nodes = vzalloc(array_size(max_count, sizeof(*nodes)));
+ if (!nodes)
+ goto err;
+
+ drm_mm_init(&mm, 1, U64_MAX - 2);
+
+ for (mode = insert_modes; mode->name; mode++) {
+ start = jiffies;
+
+ for (i = 0; i < max_count; i++) {
+ if (jiffies_to_msecs(jiffies - start) >= timeout) {
+ pr_err("%s fragmented insert took more than %lu
msecs\n",
+ mode->name, timeout);
+ goto out;
+ }
+
+ if (!expect_insert(&mm, &nodes[i],
+ size, align, i, mode)) {
+ pr_err("%s insert failed with alignment=%d",
+ mode->name, align);
+ goto out;
+ }
+ }
+
+ pr_info("%s fragmented insert took %d msecs\n",
+ mode->name, jiffies_to_msecs(jiffies - start));
+ drm_mm_for_each_node_safe(node, next, &mm)
+ drm_mm_remove_node(node);
+ DRM_MM_BUG_ON(!drm_mm_clean(&mm));
+
+ cond_resched();
+ }
+
+ ret = 0;
+out:
+ drm_mm_for_each_node_safe(node, next, &mm)
+ drm_mm_remove_node(node);
+ drm_mm_takedown(&mm);
+ vfree(nodes);
+err:
+ return ret;
+}
+
static int igt_align(void *ignored)
{
const struct insert_mode *mode;
--
2.26.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 04/16] mailbox: mediatek: cmdq: clear task in channel before shutdown
Do success callback in channel when shutdown. For those task not finish,
callback with error code thus client has chance to cleanup or reset.
Signed-off-by: Dennis YC Hsieh
Reviewed-by: CK Hu
---
drivers/mailbox/mtk-cmdq-mailbox.c | 38 ++
1 file changed, 38 insertions(+)
diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c
b/drivers/mailbox/mtk-cmdq-mailbox.c
index 9994ac9426d6..b56d340c8982 100644
--- a/drivers/mailbox/mtk-cmdq-mailbox.c
+++ b/drivers/mailbox/mtk-cmdq-mailbox.c
@@ -387,6 +387,12 @@ static int cmdq_mbox_send_data(struct mbox_chan *chan,
void *data)
if (list_empty(&thread->task_busy_list)) {
WARN_ON(clk_enable(cmdq->clock) < 0);
+ /*
+* The thread reset will clear thread related register to 0,
+* including pc, end, priority, irq, suspend and enable. Thus
+* set CMDQ_THR_ENABLED to CMDQ_THR_ENABLE_TASK will enable
+* thread and make it running.
+*/
WARN_ON(cmdq_thread_reset(cmdq, thread) < 0);
writel(task->pa_base >> cmdq->shift_pa,
@@ -450,6 +456,38 @@ static int cmdq_mbox_startup(struct mbox_chan *chan)
static void cmdq_mbox_shutdown(struct mbox_chan *chan)
{
+ struct cmdq_thread *thread = (struct cmdq_thread *)chan->con_priv;
+ struct cmdq *cmdq = dev_get_drvdata(chan->mbox->dev);
+ struct cmdq_task *task, *tmp;
+ unsigned long flags;
+
+ spin_lock_irqsave(&thread->chan->lock, flags);
+ if (list_empty(&thread->task_busy_list))
+ goto done;
+
+ WARN_ON(cmdq_thread_suspend(cmdq, thread) < 0);
+
+ /* make sure executed tasks have success callback */
+ cmdq_thread_irq_handler(cmdq, thread);
+ if (list_empty(&thread->task_busy_list))
+ goto done;
+
+ list_for_each_entry_safe(task, tmp, &thread->task_busy_list,
+list_entry) {
+ cmdq_task_exec_done(task, CMDQ_CB_ERROR);
+ kfree(task);
+ }
+
+ cmdq_thread_disable(cmdq, thread);
+ clk_disable(cmdq->clock);
+done:
+ /*
+* The thread->task_busy_list empty means thread already disable. The
+* cmdq_mbox_send_data() always reset thread which clear disable and
+* suspend statue when first pkt send to channel, so there is no need
+* to do any operation here, only unlock and leave.
+*/
+ spin_unlock_irqrestore(&thread->chan->lock, flags);
}
static const struct mbox_chan_ops cmdq_mbox_chan_ops = {
--
2.18.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 13/16] soc: mediatek: cmdq: export finalize function
Export finalize function to client which helps append eoc and jump
command to pkt. Let client decide call finalize or not.
Signed-off-by: Dennis YC Hsieh
Reviewed-by: CK Hu
Acked-by: Chun-Kuang Hu
---
drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 1 +
drivers/soc/mediatek/mtk-cmdq-helper.c | 7 ++-
include/linux/soc/mediatek/mtk-cmdq.h | 8
3 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
index 0dfcd1787e65..7daaabc26eb1 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
@@ -490,6 +490,7 @@ static void mtk_drm_crtc_hw_config(struct mtk_drm_crtc
*mtk_crtc)
cmdq_pkt_clear_event(cmdq_handle, mtk_crtc->cmdq_event);
cmdq_pkt_wfe(cmdq_handle, mtk_crtc->cmdq_event);
mtk_crtc_ddp_config(crtc, cmdq_handle);
+ cmdq_pkt_finalize(cmdq_handle);
cmdq_pkt_flush_async(cmdq_handle, ddp_cmdq_cb, cmdq_handle);
}
#endif
diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c
b/drivers/soc/mediatek/mtk-cmdq-helper.c
index 4b07da56c7bb..ebcdd30cd783 100644
--- a/drivers/soc/mediatek/mtk-cmdq-helper.c
+++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
@@ -391,7 +391,7 @@ int cmdq_pkt_assign(struct cmdq_pkt *pkt, u16 reg_idx, u32
value)
}
EXPORT_SYMBOL(cmdq_pkt_assign);
-static int cmdq_pkt_finalize(struct cmdq_pkt *pkt)
+int cmdq_pkt_finalize(struct cmdq_pkt *pkt)
{
struct cmdq_instruction inst = { {0} };
int err;
@@ -411,6 +411,7 @@ static int cmdq_pkt_finalize(struct cmdq_pkt *pkt)
return err;
}
+EXPORT_SYMBOL(cmdq_pkt_finalize);
static void cmdq_pkt_flush_async_cb(struct cmdq_cb_data data)
{
@@ -445,10 +446,6 @@ int cmdq_pkt_flush_async(struct cmdq_pkt *pkt,
cmdq_async_flush_cb cb,
unsigned long flags = 0;
struct cmdq_client *client = (struct cmdq_client *)pkt->cl;
- err = cmdq_pkt_finalize(pkt);
- if (err < 0)
- return err;
-
pkt->cb.cb = cb;
pkt->cb.data = data;
pkt->async_cb.cb = cmdq_pkt_flush_async_cb;
diff --git a/include/linux/soc/mediatek/mtk-cmdq.h
b/include/linux/soc/mediatek/mtk-cmdq.h
index 28a50c3cf6a1..703cffcd55bd 100644
--- a/include/linux/soc/mediatek/mtk-cmdq.h
+++ b/include/linux/soc/mediatek/mtk-cmdq.h
@@ -244,6 +244,14 @@ int cmdq_pkt_poll_mask(struct cmdq_pkt *pkt, u8 subsys,
*/
int cmdq_pkt_assign(struct cmdq_pkt *pkt, u16 reg_idx, u32 value);
+/**
+ * cmdq_pkt_finalize() - Append EOC and jump command to pkt.
+ * @pkt: the CMDQ packet
+ *
+ * Return: 0 for success; else the error code is returned
+ */
+int cmdq_pkt_finalize(struct cmdq_pkt *pkt);
+
/**
* cmdq_pkt_flush_async() - trigger CMDQ to asynchronously execute the CMDQ
* packet and call back at the end of done packet
--
2.18.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/msm/dpu: Fix usage of ERR_PTR()
ERR_PTR() is used in the kernel to encode an usual *negative* errno code
into a pointer. Passing a positive value (ENOMEM) to it will break the
following IS_ERR() check.
Though memory allocation is unlikely to fail, it's still worth fixing.
And grepping shows that this is the only misuse of ERR_PTR() in kernel.
Fixes: 25fdd5933e4c ("drm/msm: Add SDM845 DPU support")
Signed-off-by: Zenghui Yu
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index a1b79ee2bd9d..a2f6b688a976 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -2173,7 +2173,7 @@ struct drm_encoder *dpu_encoder_init(struct drm_device
*dev,
dpu_enc = devm_kzalloc(dev->dev, sizeof(*dpu_enc), GFP_KERNEL);
if (!dpu_enc)
- return ERR_PTR(ENOMEM);
+ return ERR_PTR(-ENOMEM);
rc = drm_encoder_init(dev, &dpu_enc->base, &dpu_encoder_funcs,
drm_enc_mode, NULL);
--
2.19.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 03/16] mailbox: cmdq: support mt6779 gce platform definition
Add gce v4 hardware support with different thread number and shift.
Signed-off-by: Dennis YC Hsieh
Reviewed-by: CK Hu
Reviewed-by: Matthias Brugger
---
drivers/mailbox/mtk-cmdq-mailbox.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c
b/drivers/mailbox/mtk-cmdq-mailbox.c
index 4dbee9258127..9994ac9426d6 100644
--- a/drivers/mailbox/mtk-cmdq-mailbox.c
+++ b/drivers/mailbox/mtk-cmdq-mailbox.c
@@ -572,10 +572,12 @@ static const struct dev_pm_ops cmdq_pm_ops = {
static const struct gce_plat gce_plat_v2 = {.thread_nr = 16};
static const struct gce_plat gce_plat_v3 = {.thread_nr = 24};
+static const struct gce_plat gce_plat_v4 = {.thread_nr = 24, .shift = 3};
static const struct of_device_id cmdq_of_ids[] = {
{.compatible = "mediatek,mt8173-gce", .data = (void *)&gce_plat_v2},
{.compatible = "mediatek,mt8183-gce", .data = (void *)&gce_plat_v3},
+ {.compatible = "mediatek,mt6779-gce", .data = (void *)&gce_plat_v4},
{}
};
--
2.18.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6] support gce on mt6779 platform
This patch support gce on mt6779 platform. Change since v5: - spearate address shift code in client helper and mailbox controller - separate write_s/write_s_mask and write_s_value/write_s_mask_value so that client can decide use mask or not - fix typo in header Change since v4: - do not clear disp event again in drm driver - symbolize value 1 to jump relative [... snip ...] Dennis YC Hsieh (16): dt-binding: gce: add gce header file for mt6779 mailbox: cmdq: variablize address shift in platform mailbox: cmdq: support mt6779 gce platform definition mailbox: mediatek: cmdq: clear task in channel before shutdown soc: mediatek: cmdq: return send msg error code soc: mediatek: cmdq: add address shift in jump soc: mediatek: cmdq: add assign function soc: mediatek: cmdq: add write_s function soc: mediatek: cmdq: add write_s_mask function soc: mediatek: cmdq: add read_s function soc: mediatek: cmdq: add write_s value function soc: mediatek: cmdq: add write_s_mask value function soc: mediatek: cmdq: export finalize function soc: mediatek: cmdq: add jump function soc: mediatek: cmdq: add clear option in cmdq_pkt_wfe api soc: mediatek: cmdq: add set event function .../devicetree/bindings/mailbox/mtk-gce.txt | 8 +- drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 3 +- drivers/mailbox/mtk-cmdq-mailbox.c| 101 ++-- drivers/soc/mediatek/mtk-cmdq-helper.c| 163 - include/dt-bindings/gce/mt6779-gce.h | 222 ++ include/linux/mailbox/mtk-cmdq-mailbox.h | 10 +- include/linux/soc/mediatek/mtk-cmdq.h | 125 +- 7 files changed, 599 insertions(+), 33 deletions(-) create mode 100644 include/dt-bindings/gce/mt6779-gce.h -- 2.18.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 00/16] support gce on mt6779 platform
This patch support gce on mt6779 platform. Change since v5: - spearate address shift code in client helper and mailbox controller - separate write_s/write_s_mask and write_s_value/write_s_mask_value so that client can decide use mask or not - fix typo in header Change since v4: - do not clear disp event again in drm driver - symbolize value 1 to jump relative [... snip ...] *** BLURB HERE *** Dennis YC Hsieh (16): dt-binding: gce: add gce header file for mt6779 mailbox: cmdq: variablize address shift in platform mailbox: cmdq: support mt6779 gce platform definition mailbox: mediatek: cmdq: clear task in channel before shutdown soc: mediatek: cmdq: return send msg error code soc: mediatek: cmdq: add address shift in jump soc: mediatek: cmdq: add assign function soc: mediatek: cmdq: add write_s function soc: mediatek: cmdq: add write_s_mask function soc: mediatek: cmdq: add read_s function soc: mediatek: cmdq: add write_s value function soc: mediatek: cmdq: add write_s_mask value function soc: mediatek: cmdq: export finalize function soc: mediatek: cmdq: add jump function soc: mediatek: cmdq: add clear option in cmdq_pkt_wfe api soc: mediatek: cmdq: add set event function .../devicetree/bindings/mailbox/mtk-gce.txt | 8 +- drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 3 +- drivers/mailbox/mtk-cmdq-mailbox.c| 101 ++-- drivers/soc/mediatek/mtk-cmdq-helper.c| 163 - include/dt-bindings/gce/mt6779-gce.h | 222 ++ include/linux/mailbox/mtk-cmdq-mailbox.h | 10 +- include/linux/soc/mediatek/mtk-cmdq.h | 125 +- 7 files changed, 599 insertions(+), 33 deletions(-) create mode 100644 include/dt-bindings/gce/mt6779-gce.h -- 2.18.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 06/16] soc: mediatek: cmdq: add address shift in jump
Add address shift when compose jump instruction to compatible with 35bit format. Signed-off-by: Dennis YC Hsieh --- drivers/soc/mediatek/mtk-cmdq-helper.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c index c67081759728..98f23ba3ba47 100644 --- a/drivers/soc/mediatek/mtk-cmdq-helper.c +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c @@ -291,7 +291,8 @@ static int cmdq_pkt_finalize(struct cmdq_pkt *pkt) /* JUMP to end */ inst.op = CMDQ_CODE_JUMP; - inst.value = CMDQ_JUMP_PASS; + inst.value = CMDQ_JUMP_PASS >> + cmdq_mbox_shift(((struct cmdq_client *)pkt->cl)->chan); err = cmdq_pkt_append_command(pkt, inst); return err; -- 2.18.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 10/16] soc: mediatek: cmdq: add read_s function
Add read_s function in cmdq helper functions which support read value from
register or dma physical address into gce internal register.
Signed-off-by: Dennis YC Hsieh
Reviewed-by: CK Hu
---
drivers/soc/mediatek/mtk-cmdq-helper.c | 15 +++
include/linux/mailbox/mtk-cmdq-mailbox.h | 1 +
include/linux/soc/mediatek/mtk-cmdq.h| 13 +
3 files changed, 29 insertions(+)
diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c
b/drivers/soc/mediatek/mtk-cmdq-helper.c
index 58a414790471..02ac0ca23f6e 100644
--- a/drivers/soc/mediatek/mtk-cmdq-helper.c
+++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
@@ -226,6 +226,21 @@ int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8 subsys,
}
EXPORT_SYMBOL(cmdq_pkt_write_mask);
+int cmdq_pkt_read_s(struct cmdq_pkt *pkt, u16 high_addr_reg_idx, u16 addr_low,
+ u16 reg_idx)
+{
+ struct cmdq_instruction inst = { {0} };
+
+ inst.op = CMDQ_CODE_READ_S;
+ inst.dst_t = CMDQ_REG_TYPE;
+ inst.sop = high_addr_reg_idx;
+ inst.reg_dst = reg_idx;
+ inst.src_reg = addr_low;
+
+ return cmdq_pkt_append_command(pkt, inst);
+}
+EXPORT_SYMBOL(cmdq_pkt_read_s);
+
int cmdq_pkt_write_s(struct cmdq_pkt *pkt, u16 high_addr_reg_idx,
u16 addr_low, u16 src_reg_idx)
{
diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h
b/include/linux/mailbox/mtk-cmdq-mailbox.h
index 8ef87e1bd03b..3f6bc0dfd5da 100644
--- a/include/linux/mailbox/mtk-cmdq-mailbox.h
+++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
@@ -59,6 +59,7 @@ enum cmdq_code {
CMDQ_CODE_JUMP = 0x10,
CMDQ_CODE_WFE = 0x20,
CMDQ_CODE_EOC = 0x40,
+ CMDQ_CODE_READ_S = 0x80,
CMDQ_CODE_WRITE_S = 0x90,
CMDQ_CODE_WRITE_S_MASK = 0x91,
CMDQ_CODE_LOGIC = 0xa0,
diff --git a/include/linux/soc/mediatek/mtk-cmdq.h
b/include/linux/soc/mediatek/mtk-cmdq.h
index ca9c75fd8125..c07baa1534ca 100644
--- a/include/linux/soc/mediatek/mtk-cmdq.h
+++ b/include/linux/soc/mediatek/mtk-cmdq.h
@@ -104,6 +104,19 @@ int cmdq_pkt_write(struct cmdq_pkt *pkt, u8 subsys, u16
offset, u32 value);
int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8 subsys,
u16 offset, u32 value, u32 mask);
+/*
+ * cmdq_pkt_read_s() - append read_s command to the CMDQ packet
+ * @pkt: the CMDQ packet
+ * @high_addr_reg_idx: internal register ID which contains high address of pa
+ * @addr_low: low address of pa
+ * @addr: the physical address of register or dma to read
+ * @reg_idx: the CMDQ internal register ID to cache read data
+ *
+ * Return: 0 for success; else the error code is returned
+ */
+int cmdq_pkt_read_s(struct cmdq_pkt *pkt, u16 high_addr_reg_idx, u16 addr_low,
+ u16 reg_idx);
+
/**
* cmdq_pkt_write_s() - append write_s command to the CMDQ packet
* @pkt: the CMDQ packet
--
2.18.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 15/16] soc: mediatek: cmdq: add clear option in cmdq_pkt_wfe api
Add clear parameter to let client decide if
event should be clear to 0 after GCE receive it.
Signed-off-by: Dennis YC Hsieh
Reviewed-by: CK Hu
---
drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 2 +-
drivers/soc/mediatek/mtk-cmdq-helper.c | 5 +++--
include/linux/mailbox/mtk-cmdq-mailbox.h | 3 +--
include/linux/soc/mediatek/mtk-cmdq.h| 5 +++--
4 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
index 7daaabc26eb1..a065b3a412cf 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
@@ -488,7 +488,7 @@ static void mtk_drm_crtc_hw_config(struct mtk_drm_crtc
*mtk_crtc)
if (mtk_crtc->cmdq_client) {
cmdq_handle = cmdq_pkt_create(mtk_crtc->cmdq_client, PAGE_SIZE);
cmdq_pkt_clear_event(cmdq_handle, mtk_crtc->cmdq_event);
- cmdq_pkt_wfe(cmdq_handle, mtk_crtc->cmdq_event);
+ cmdq_pkt_wfe(cmdq_handle, mtk_crtc->cmdq_event, false);
mtk_crtc_ddp_config(crtc, cmdq_handle);
cmdq_pkt_finalize(cmdq_handle);
cmdq_pkt_flush_async(cmdq_handle, ddp_cmdq_cb, cmdq_handle);
diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c
b/drivers/soc/mediatek/mtk-cmdq-helper.c
index 5cf9e71b7900..9e9a4c81553a 100644
--- a/drivers/soc/mediatek/mtk-cmdq-helper.c
+++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
@@ -315,15 +315,16 @@ int cmdq_pkt_write_s_mask_value(struct cmdq_pkt *pkt, u8
high_addr_reg_idx,
}
EXPORT_SYMBOL(cmdq_pkt_write_s_mask_value);
-int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event)
+int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event, bool clear)
{
struct cmdq_instruction inst = { {0} };
+ u32 clear_option = clear ? CMDQ_WFE_UPDATE : 0;
if (event >= CMDQ_MAX_EVENT)
return -EINVAL;
inst.op = CMDQ_CODE_WFE;
- inst.value = CMDQ_WFE_OPTION;
+ inst.value = CMDQ_WFE_OPTION | clear_option;
inst.event = event;
return cmdq_pkt_append_command(pkt, inst);
diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h
b/include/linux/mailbox/mtk-cmdq-mailbox.h
index 3f6bc0dfd5da..42d2a30e6a70 100644
--- a/include/linux/mailbox/mtk-cmdq-mailbox.h
+++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
@@ -27,8 +27,7 @@
* bit 16-27: update value
* bit 31: 1 - update, 0 - no update
*/
-#define CMDQ_WFE_OPTION(CMDQ_WFE_UPDATE |
CMDQ_WFE_WAIT | \
- CMDQ_WFE_WAIT_VALUE)
+#define CMDQ_WFE_OPTION(CMDQ_WFE_WAIT |
CMDQ_WFE_WAIT_VALUE)
/** cmdq event maximum */
#define CMDQ_MAX_EVENT 0x3ff
diff --git a/include/linux/soc/mediatek/mtk-cmdq.h
b/include/linux/soc/mediatek/mtk-cmdq.h
index 9494b293bad9..3216ccfff4a9 100644
--- a/include/linux/soc/mediatek/mtk-cmdq.h
+++ b/include/linux/soc/mediatek/mtk-cmdq.h
@@ -183,11 +183,12 @@ int cmdq_pkt_write_s_mask_value(struct cmdq_pkt *pkt, u8
high_addr_reg_idx,
/**
* cmdq_pkt_wfe() - append wait for event command to the CMDQ packet
* @pkt: the CMDQ packet
- * @event: the desired event type to "wait and CLEAR"
+ * @event: the desired event type to wait
+ * @clear: clear event or not after event arrive
*
* Return: 0 for success; else the error code is returned
*/
-int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event);
+int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event, bool clear);
/**
* cmdq_pkt_clear_event() - append clear event command to the CMDQ packet
--
2.18.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 14/16] soc: mediatek: cmdq: add jump function
Add jump function so that client can jump to any address which
contains instruction.
Signed-off-by: Dennis YC Hsieh
Reviewed-by: CK Hu
---
drivers/soc/mediatek/mtk-cmdq-helper.c | 13 +
include/linux/soc/mediatek/mtk-cmdq.h | 11 +++
2 files changed, 24 insertions(+)
diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c
b/drivers/soc/mediatek/mtk-cmdq-helper.c
index ebcdd30cd783..5cf9e71b7900 100644
--- a/drivers/soc/mediatek/mtk-cmdq-helper.c
+++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
@@ -13,6 +13,7 @@
#define CMDQ_POLL_ENABLE_MASK BIT(0)
#define CMDQ_EOC_IRQ_ENBIT(0)
#define CMDQ_REG_TYPE 1
+#define CMDQ_JUMP_RELATIVE 1
struct cmdq_instruction {
union {
@@ -391,6 +392,18 @@ int cmdq_pkt_assign(struct cmdq_pkt *pkt, u16 reg_idx, u32
value)
}
EXPORT_SYMBOL(cmdq_pkt_assign);
+int cmdq_pkt_jump(struct cmdq_pkt *pkt, dma_addr_t addr)
+{
+ struct cmdq_instruction inst = { {0} };
+
+ inst.op = CMDQ_CODE_JUMP;
+ inst.offset = CMDQ_JUMP_RELATIVE;
+ inst.value = addr >>
+ cmdq_mbox_shift(((struct cmdq_client *)pkt->cl)->chan);
+ return cmdq_pkt_append_command(pkt, inst);
+}
+EXPORT_SYMBOL(cmdq_pkt_jump);
+
int cmdq_pkt_finalize(struct cmdq_pkt *pkt)
{
struct cmdq_instruction inst = { {0} };
diff --git a/include/linux/soc/mediatek/mtk-cmdq.h
b/include/linux/soc/mediatek/mtk-cmdq.h
index 703cffcd55bd..9494b293bad9 100644
--- a/include/linux/soc/mediatek/mtk-cmdq.h
+++ b/include/linux/soc/mediatek/mtk-cmdq.h
@@ -244,6 +244,17 @@ int cmdq_pkt_poll_mask(struct cmdq_pkt *pkt, u8 subsys,
*/
int cmdq_pkt_assign(struct cmdq_pkt *pkt, u16 reg_idx, u32 value);
+/**
+ * cmdq_pkt_jump() - Append jump command to the CMDQ packet, ask GCE
+ * to execute an instruction that change current thread PC to
+ * a physical address which should contains more instruction.
+ * @pkt:the CMDQ packet
+ * @addr: physical address of target instruction buffer
+ *
+ * Return: 0 for success; else the error code is returned
+ */
+int cmdq_pkt_jump(struct cmdq_pkt *pkt, dma_addr_t addr);
+
/**
* cmdq_pkt_finalize() - Append EOC and jump command to pkt.
* @pkt: the CMDQ packet
--
2.18.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 12/16] soc: mediatek: cmdq: add write_s_mask value function
add write_s_mask_value function in cmdq helper functions which
writes a constant value to address with mask and large dma
access support.
Signed-off-by: Dennis YC Hsieh
---
drivers/soc/mediatek/mtk-cmdq-helper.c | 21 +
include/linux/soc/mediatek/mtk-cmdq.h | 15 +++
2 files changed, 36 insertions(+)
diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c
b/drivers/soc/mediatek/mtk-cmdq-helper.c
index 12095a1b701b..4b07da56c7bb 100644
--- a/drivers/soc/mediatek/mtk-cmdq-helper.c
+++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
@@ -293,6 +293,27 @@ int cmdq_pkt_write_s_value(struct cmdq_pkt *pkt, u8
high_addr_reg_idx,
}
EXPORT_SYMBOL(cmdq_pkt_write_s_value);
+int cmdq_pkt_write_s_mask_value(struct cmdq_pkt *pkt, u8 high_addr_reg_idx,
+ u16 addr_low, u32 value, u32 mask)
+{
+ struct cmdq_instruction inst = { {0} };
+ int err;
+
+ inst.op = CMDQ_CODE_MASK;
+ inst.mask = ~mask;
+ err = cmdq_pkt_append_command(pkt, inst);
+ if (err < 0)
+ return err;
+
+ inst.op = CMDQ_CODE_WRITE_S_MASK;
+ inst.sop = high_addr_reg_idx;
+ inst.offset = addr_low;
+ inst.value = value;
+
+ return cmdq_pkt_append_command(pkt, inst);
+}
+EXPORT_SYMBOL(cmdq_pkt_write_s_mask_value);
+
int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event)
{
struct cmdq_instruction inst = { {0} };
diff --git a/include/linux/soc/mediatek/mtk-cmdq.h
b/include/linux/soc/mediatek/mtk-cmdq.h
index bb36750be58c..28a50c3cf6a1 100644
--- a/include/linux/soc/mediatek/mtk-cmdq.h
+++ b/include/linux/soc/mediatek/mtk-cmdq.h
@@ -165,6 +165,21 @@ int cmdq_pkt_write_s_mask(struct cmdq_pkt *pkt, u16
high_addr_reg_idx,
int cmdq_pkt_write_s_value(struct cmdq_pkt *pkt, u8 high_addr_reg_idx,
u16 addr_low, u32 value);
+/**
+ * cmdq_pkt_write_s_mask_value() - append write_s command with mask to the CMDQ
+ *packet which write value to a physical
+ *address
+ * @pkt: the CMDQ packet
+ * @high_addr_reg_idx: internal register ID which contains high address of pa
+ * @addr_low: low address of pa
+ * @value: the specified target value
+ * @mask: the specified target mask
+ *
+ * Return: 0 for success; else the error code is returned
+ */
+int cmdq_pkt_write_s_mask_value(struct cmdq_pkt *pkt, u8 high_addr_reg_idx,
+ u16 addr_low, u32 value, u32 mask);
+
/**
* cmdq_pkt_wfe() - append wait for event command to the CMDQ packet
* @pkt: the CMDQ packet
--
2.18.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 09/16] soc: mediatek: cmdq: add write_s_mask function
add write_s_mask function in cmdq helper functions which
writes value contains in internal register to address
with mask and large dma access support.
Signed-off-by: Dennis YC Hsieh
---
drivers/soc/mediatek/mtk-cmdq-helper.c | 23 +++
include/linux/mailbox/mtk-cmdq-mailbox.h | 1 +
include/linux/soc/mediatek/mtk-cmdq.h| 19 ++-
3 files changed, 42 insertions(+), 1 deletion(-)
diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c
b/drivers/soc/mediatek/mtk-cmdq-helper.c
index ee24c0ec0a24..58a414790471 100644
--- a/drivers/soc/mediatek/mtk-cmdq-helper.c
+++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
@@ -241,6 +241,29 @@ int cmdq_pkt_write_s(struct cmdq_pkt *pkt, u16
high_addr_reg_idx,
}
EXPORT_SYMBOL(cmdq_pkt_write_s);
+int cmdq_pkt_write_s_mask(struct cmdq_pkt *pkt, u16 high_addr_reg_idx,
+ u16 addr_low, u16 src_reg_idx, u32 mask)
+{
+ struct cmdq_instruction inst = { {0} };
+ int err;
+
+ inst.op = CMDQ_CODE_MASK;
+ inst.mask = ~mask;
+ err = cmdq_pkt_append_command(pkt, inst);
+ if (err < 0)
+ return err;
+
+ inst.mask = 0;
+ inst.op = CMDQ_CODE_WRITE_S_MASK;
+ inst.src_t = CMDQ_REG_TYPE;
+ inst.sop = high_addr_reg_idx;
+ inst.offset = addr_low;
+ inst.src_reg = src_reg_idx;
+
+ return cmdq_pkt_append_command(pkt, inst);
+}
+EXPORT_SYMBOL(cmdq_pkt_write_s_mask);
+
int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event)
{
struct cmdq_instruction inst = { {0} };
diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h
b/include/linux/mailbox/mtk-cmdq-mailbox.h
index ee67dd3b86f5..8ef87e1bd03b 100644
--- a/include/linux/mailbox/mtk-cmdq-mailbox.h
+++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
@@ -60,6 +60,7 @@ enum cmdq_code {
CMDQ_CODE_WFE = 0x20,
CMDQ_CODE_EOC = 0x40,
CMDQ_CODE_WRITE_S = 0x90,
+ CMDQ_CODE_WRITE_S_MASK = 0x91,
CMDQ_CODE_LOGIC = 0xa0,
};
diff --git a/include/linux/soc/mediatek/mtk-cmdq.h
b/include/linux/soc/mediatek/mtk-cmdq.h
index d623f1aa7814..ca9c75fd8125 100644
--- a/include/linux/soc/mediatek/mtk-cmdq.h
+++ b/include/linux/soc/mediatek/mtk-cmdq.h
@@ -110,7 +110,6 @@ int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8 subsys,
* @high_addr_reg_idx: internal register ID which contains high address of pa
* @addr_low: low address of pa
* @src_reg_idx: the CMDQ internal register ID which cache source value
- * @mask: the specified target address mask, use U32_MAX if no need
*
* Return: 0 for success; else the error code is returned
*
@@ -122,6 +121,24 @@ int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8 subsys,
int cmdq_pkt_write_s(struct cmdq_pkt *pkt, u16 high_addr_reg_idx,
u16 addr_low, u16 src_reg_idx);
+/**
+ * cmdq_pkt_write_s_mask() - append write_s with mask command to the CMDQ
packet
+ * @pkt: the CMDQ packet
+ * @high_addr_reg_idx: internal register ID which contains high address of pa
+ * @addr_low: low address of pa
+ * @src_reg_idx: the CMDQ internal register ID which cache source value
+ * @mask: the specified target address mask, use U32_MAX if no need
+ *
+ * Return: 0 for success; else the error code is returned
+ *
+ * Support write value to physical address without subsys. Use CMDQ_ADDR_HIGH()
+ * to get high address and call cmdq_pkt_assign() to assign value into internal
+ * reg. Also use CMDQ_ADDR_LOW() to get low address for addr_low parameter when
+ * call to this function.
+ */
+int cmdq_pkt_write_s_mask(struct cmdq_pkt *pkt, u16 high_addr_reg_idx,
+ u16 addr_low, u16 src_reg_idx, u32 mask);
+
/**
* cmdq_pkt_wfe() - append wait for event command to the CMDQ packet
* @pkt: the CMDQ packet
--
2.18.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 07/16] soc: mediatek: cmdq: add assign function
Add assign function in cmdq helper which assign constant value into
internal register by index.
Signed-off-by: Dennis YC Hsieh
Reviewed-by: CK Hu
---
drivers/soc/mediatek/mtk-cmdq-helper.c | 24 +++-
include/linux/mailbox/mtk-cmdq-mailbox.h | 1 +
include/linux/soc/mediatek/mtk-cmdq.h| 14 ++
3 files changed, 38 insertions(+), 1 deletion(-)
diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c
b/drivers/soc/mediatek/mtk-cmdq-helper.c
index 98f23ba3ba47..33153d17c9d9 100644
--- a/drivers/soc/mediatek/mtk-cmdq-helper.c
+++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
@@ -12,6 +12,7 @@
#define CMDQ_WRITE_ENABLE_MASK BIT(0)
#define CMDQ_POLL_ENABLE_MASK BIT(0)
#define CMDQ_EOC_IRQ_ENBIT(0)
+#define CMDQ_REG_TYPE 1
struct cmdq_instruction {
union {
@@ -21,8 +22,17 @@ struct cmdq_instruction {
union {
u16 offset;
u16 event;
+ u16 reg_dst;
+ };
+ union {
+ u8 subsys;
+ struct {
+ u8 sop:5;
+ u8 arg_c_t:1;
+ u8 arg_b_t:1;
+ u8 dst_t:1;
+ };
};
- u8 subsys;
u8 op;
};
@@ -277,6 +287,18 @@ int cmdq_pkt_poll_mask(struct cmdq_pkt *pkt, u8 subsys,
}
EXPORT_SYMBOL(cmdq_pkt_poll_mask);
+int cmdq_pkt_assign(struct cmdq_pkt *pkt, u16 reg_idx, u32 value)
+{
+ struct cmdq_instruction inst = { {0} };
+
+ inst.op = CMDQ_CODE_LOGIC;
+ inst.dst_t = CMDQ_REG_TYPE;
+ inst.reg_dst = reg_idx;
+ inst.value = value;
+ return cmdq_pkt_append_command(pkt, inst);
+}
+EXPORT_SYMBOL(cmdq_pkt_assign);
+
static int cmdq_pkt_finalize(struct cmdq_pkt *pkt)
{
struct cmdq_instruction inst = { {0} };
diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h
b/include/linux/mailbox/mtk-cmdq-mailbox.h
index dfe5b2eb85cc..121c3bb6d3de 100644
--- a/include/linux/mailbox/mtk-cmdq-mailbox.h
+++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
@@ -59,6 +59,7 @@ enum cmdq_code {
CMDQ_CODE_JUMP = 0x10,
CMDQ_CODE_WFE = 0x20,
CMDQ_CODE_EOC = 0x40,
+ CMDQ_CODE_LOGIC = 0xa0,
};
enum cmdq_cb_status {
diff --git a/include/linux/soc/mediatek/mtk-cmdq.h
b/include/linux/soc/mediatek/mtk-cmdq.h
index a74c1d5acdf3..83340211e1d3 100644
--- a/include/linux/soc/mediatek/mtk-cmdq.h
+++ b/include/linux/soc/mediatek/mtk-cmdq.h
@@ -152,6 +152,20 @@ int cmdq_pkt_poll(struct cmdq_pkt *pkt, u8 subsys,
*/
int cmdq_pkt_poll_mask(struct cmdq_pkt *pkt, u8 subsys,
u16 offset, u32 value, u32 mask);
+
+/**
+ * cmdq_pkt_assign() - Append logic assign command to the CMDQ packet, ask GCE
+ *to execute an instruction that set a constant value into
+ *internal register and use as value, mask or address in
+ *read/write instruction.
+ * @pkt: the CMDQ packet
+ * @reg_idx: the CMDQ internal register ID
+ * @value: the specified value
+ *
+ * Return: 0 for success; else the error code is returned
+ */
+int cmdq_pkt_assign(struct cmdq_pkt *pkt, u16 reg_idx, u32 value);
+
/**
* cmdq_pkt_flush_async() - trigger CMDQ to asynchronously execute the CMDQ
* packet and call back at the end of done packet
--
2.18.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 01/16] dt-binding: gce: add gce header file for mt6779
Add documentation for the mt6779 gce. Add gce header file defined the gce hardware event, subsys number and constant for mt6779. Signed-off-by: Dennis YC Hsieh Reviewed-by: Rob Herring Reviewed-by: CK Hu --- .../devicetree/bindings/mailbox/mtk-gce.txt | 8 +- include/dt-bindings/gce/mt6779-gce.h | 222 ++ 2 files changed, 227 insertions(+), 3 deletions(-) create mode 100644 include/dt-bindings/gce/mt6779-gce.h diff --git a/Documentation/devicetree/bindings/mailbox/mtk-gce.txt b/Documentation/devicetree/bindings/mailbox/mtk-gce.txt index 7b13787ab13d..82c0a83fed09 100644 --- a/Documentation/devicetree/bindings/mailbox/mtk-gce.txt +++ b/Documentation/devicetree/bindings/mailbox/mtk-gce.txt @@ -9,7 +9,8 @@ CMDQ driver uses mailbox framework for communication. Please refer to mailbox.txt for generic information about mailbox device-tree bindings. Required properties: -- compatible: can be "mediatek,mt8173-gce" or "mediatek,mt8183-gce" +- compatible: can be "mediatek,mt8173-gce", "mediatek,mt8183-gce" or + "mediatek,mt6779-gce". - reg: Address range of the GCE unit - interrupts: The interrupt signal from the GCE block - clock: Clocks according to the common clock binding @@ -36,8 +37,9 @@ Optional properties for a client device: start_offset: the start offset of register address that GCE can access. size: the total size of register address that GCE can access. -Some vaules of properties are defined in 'dt-bindings/gce/mt8173-gce.h' -or 'dt-binding/gce/mt8183-gce.h'. Such as sub-system ids, thread priority, event ids. +Some vaules of properties are defined in 'dt-bindings/gce/mt8173-gce.h', +'dt-binding/gce/mt8183-gce.h' or 'dt-bindings/gce/mt6779-gce.h'. Such as +sub-system ids, thread priority, event ids. Example: diff --git a/include/dt-bindings/gce/mt6779-gce.h b/include/dt-bindings/gce/mt6779-gce.h new file mode 100644 index ..06101316ace4 --- /dev/null +++ b/include/dt-bindings/gce/mt6779-gce.h @@ -0,0 +1,222 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright (c) 2019 MediaTek Inc. + * Author: Dennis-YC Hsieh + */ + +#ifndef _DT_BINDINGS_GCE_MT6779_H +#define _DT_BINDINGS_GCE_MT6779_H + +#define CMDQ_NO_TIMEOUT0x + +/* GCE HW thread priority */ +#define CMDQ_THR_PRIO_LOWEST 0 +#define CMDQ_THR_PRIO_11 +#define CMDQ_THR_PRIO_22 +#define CMDQ_THR_PRIO_33 +#define CMDQ_THR_PRIO_44 +#define CMDQ_THR_PRIO_55 +#define CMDQ_THR_PRIO_66 +#define CMDQ_THR_PRIO_HIGHEST 7 + +/* GCE subsys table */ +#define SUBSYS_13000 +#define SUBSYS_14001 +#define SUBSYS_14012 +#define SUBSYS_14023 +#define SUBSYS_15024 +#define SUBSYS_18805 +#define SUBSYS_18816 +#define SUBSYS_18827 +#define SUBSYS_18838 +#define SUBSYS_18849 +#define SUBSYS_100010 +#define SUBSYS_100111 +#define SUBSYS_100212 +#define SUBSYS_100313 +#define SUBSYS_100414 +#define SUBSYS_100515 +#define SUBSYS_102016 +#define SUBSYS_102817 +#define SUBSYS_170018 +#define SUBSYS_170119 +#define SUBSYS_170220 +#define SUBSYS_170321 +#define SUBSYS_180022 +#define SUBSYS_180123 +#define SUBSYS_180224 +#define SUBSYS_180425 +#define SUBSYS_180526 +#define SUBSYS_180827 +#define SUBSYS_180a28 +#define SUBSYS_180b29 +#define CMDQ_SUBSYS_OFF32 + +/* GCE hardware events */ +#define CMDQ_EVENT_DISP_RDMA0_SOF 0 +#define CMDQ_EVENT_DISP_RDMA1_SOF 1 +#define CMDQ_EVENT_MDP_RDMA0_SOF 2 +#define CMDQ_EVENT_MDP_RDMA1_SOF 3 +#define CMDQ_EVENT_MDP_RSZ0_SOF4 +#define CMDQ_EVENT_MDP_RSZ1_SOF5 +#define CMDQ_EVENT_MDP_TDSHP_SOF 6 +#define CMDQ_EVENT_MDP_WROT0_SOF 7 +#define CMDQ_EVENT_MDP_WROT1_SOF 8 +#define CMDQ_EVENT_DISP_OVL0_SOF 9 +#define CMDQ_EVENT_DISP_2L_OVL0_SOF10 +#define CMDQ_EVENT_DISP_2L_OVL1_SOF11 +#define CMDQ_EVENT_DISP_WDMA0_SOF 12 +#define CMDQ_EVENT_DISP_COLOR0_SOF 13 +#define CMDQ_EVENT_DISP_CCORR0_SOF 14 +#define CMDQ_EVENT_DISP_AAL0_SOF 15 +#define CMDQ_EVENT_DISP_GAMMA0_SOF 16 +#define CMDQ_EVENT_DISP_DITHER0_SOF17 +#define CMDQ_EVENT_DISP_PWM0_SOF 18 +#define CMDQ_EVENT_DISP_DSI0_SOF
Re: [v1] drm/msm: add shutdown support for display platform_driver
Hi Krishna, On 2020-05-28 14:08, Krishna Manikandan wrote: Define shutdown callback for display drm driver, so as to disable all the CRTCS when shutdown notification is received by the driver. Would be nice to add some more context for adding this shutdown callback something like below: If the hardware is still accessing memory after SMMU translation is disabled as part of smmu shutdown callback, then the IOVAs(I/O virtual address) which it was using will go on the bus as the physical addresses which will result in unknown crashes (NoC/interconnect errors on QCOM SoCs). PS: Credits for description: Robin Murphy Thanks, Sai -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PULL] drm-misc-fixes
Hi Dave, Daniel, Here's this week drm-misc-fixes PR. Thanks! Maxime drm-misc-fixes-2020-05-28: Two ingenic fixes, one for a wrong cast, the other for a typo in a comparison The following changes since commit c54a8f1f329197d83d941ad84c4aa38bf282cbbd: drm/meson: pm resume add return errno branch (2020-05-13 12:00:37 +0200) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-fixes-2020-05-28 for you to fetch changes up to abf56fadf0e208abfb13ad1ac0094416058da0ad: gpu/drm: Ingenic: Fix opaque pointer casted to wrong type (2020-05-17 14:16:15 +0200) Two ingenic fixes, one for a wrong cast, the other for a typo in a comparison Paul Cercueil (2): gpu/drm: ingenic: Fix bogus crtc_atomic_check callback gpu/drm: Ingenic: Fix opaque pointer casted to wrong type drivers/gpu/drm/ingenic/ingenic-drm.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Potential Memory Leak Bugs in drivers/gpu/drm/virtio/virtgpu_vq.c (Linux 5.6).
Hi,
My name is Dongyang Zhan, I am a security researcher.
Currently, I found two possible memory bugs in
drivers/gpu/drm/virtio/virtgpu_vq.c (Linux 5.6).
I hope you can help me to confirm them. Thank you.
The first one is resp_buf will not be release in
virtio_gpu_cmd_get_display_info() with the condition
(resp_size <= MAX_INLINE_RESP_SIZE) in virtio_gpu_alloc_cmd_resp().
int virtio_gpu_cmd_get_display_info(struct virtio_gpu_device *vgdev)
{
struct virtio_gpu_ctrl_hdr *cmd_p;
struct virtio_gpu_vbuffer *vbuf;
void *resp_buf;
...
resp_buf = kzalloc(sizeof(struct virtio_gpu_resp_display_info), GFP_KERNEL);
if (!resp_buf)
return -ENOMEM;
cmd_p =virtio_gpu_alloc_cmd_resp(vgdev,&virtio_gpu_cmd_get_display_info_cb,
&vbuf,
sizeof(*cmd_p), sizeof(struct virtio_gpu_resp_display_info),resp_buf); //
memset(cmd_p, 0, sizeof(*cmd_p));
...
return 0;
}
virtio_gpu_get_vbuf(struct virtio_gpu_device *vgdev,
int size, int resp_size, void *resp_buf,
virtio_gpu_resp_cb resp_cb)
{
struct virtio_gpu_vbuffer *vbuf;
vbuf = kmem_cache_zalloc(vgdev->vbufs, GFP_KERNEL);
if (!vbuf)
return ERR_PTR(-ENOMEM);
BUG_ON(size > MAX_INLINE_CMD_SIZE);
vbuf->buf = (void *)vbuf + sizeof(*vbuf);
vbuf->size = size;
vbuf->resp_cb = resp_cb;
vbuf->resp_size = resp_size;
if (resp_size <= MAX_INLINE_RESP_SIZE)
vbuf->resp_buf = (void *)vbuf->buf + size; // resp_buf will never be released.
else
vbuf->resp_buf = resp_buf;
BUG_ON(!vbuf->resp_buf);
return vbuf;
}
The second one is in virtio_gpu_cmd_get_edids(), which is similar with
the last one. resp_buf will not be released.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 11/16] soc: mediatek: cmdq: add write_s value function
add write_s function in cmdq helper functions which
writes a constant value to address with large dma
access support.
Signed-off-by: Dennis YC Hsieh
---
drivers/soc/mediatek/mtk-cmdq-helper.c | 14 ++
include/linux/soc/mediatek/mtk-cmdq.h | 13 +
2 files changed, 27 insertions(+)
diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c
b/drivers/soc/mediatek/mtk-cmdq-helper.c
index 02ac0ca23f6e..12095a1b701b 100644
--- a/drivers/soc/mediatek/mtk-cmdq-helper.c
+++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
@@ -279,6 +279,20 @@ int cmdq_pkt_write_s_mask(struct cmdq_pkt *pkt, u16
high_addr_reg_idx,
}
EXPORT_SYMBOL(cmdq_pkt_write_s_mask);
+int cmdq_pkt_write_s_value(struct cmdq_pkt *pkt, u8 high_addr_reg_idx,
+ u16 addr_low, u32 value)
+{
+ struct cmdq_instruction inst = { {0} };
+
+ inst.op = CMDQ_CODE_WRITE_S;
+ inst.sop = high_addr_reg_idx;
+ inst.offset = addr_low;
+ inst.value = value;
+
+ return cmdq_pkt_append_command(pkt, inst);
+}
+EXPORT_SYMBOL(cmdq_pkt_write_s_value);
+
int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event)
{
struct cmdq_instruction inst = { {0} };
diff --git a/include/linux/soc/mediatek/mtk-cmdq.h
b/include/linux/soc/mediatek/mtk-cmdq.h
index c07baa1534ca..bb36750be58c 100644
--- a/include/linux/soc/mediatek/mtk-cmdq.h
+++ b/include/linux/soc/mediatek/mtk-cmdq.h
@@ -152,6 +152,19 @@ int cmdq_pkt_write_s(struct cmdq_pkt *pkt, u16
high_addr_reg_idx,
int cmdq_pkt_write_s_mask(struct cmdq_pkt *pkt, u16 high_addr_reg_idx,
u16 addr_low, u16 src_reg_idx, u32 mask);
+/**
+ * cmdq_pkt_write_s_value() - append write_s command to the CMDQ packet which
+ * write value to a physical address
+ * @pkt: the CMDQ packet
+ * @high_addr_reg_idx: internal register ID which contains high address of pa
+ * @addr_low: low address of pa
+ * @value: the specified target value
+ *
+ * Return: 0 for success; else the error code is returned
+ */
+int cmdq_pkt_write_s_value(struct cmdq_pkt *pkt, u8 high_addr_reg_idx,
+ u16 addr_low, u32 value);
+
/**
* cmdq_pkt_wfe() - append wait for event command to the CMDQ packet
* @pkt: the CMDQ packet
--
2.18.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 02/16] mailbox: cmdq: variablize address shift in platform
Some gce hardware shift pc and end address in register to support
large dram addressing.
Implement gce address shift when write or read pc and end register.
And add shift bit in platform definition.
Signed-off-by: Dennis YC Hsieh
---
drivers/mailbox/mtk-cmdq-mailbox.c | 61 ++--
include/linux/mailbox/mtk-cmdq-mailbox.h | 2 +
2 files changed, 48 insertions(+), 15 deletions(-)
diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c
b/drivers/mailbox/mtk-cmdq-mailbox.c
index 9a6ce9f5a7db..4dbee9258127 100644
--- a/drivers/mailbox/mtk-cmdq-mailbox.c
+++ b/drivers/mailbox/mtk-cmdq-mailbox.c
@@ -76,8 +76,22 @@ struct cmdq {
struct cmdq_thread *thread;
struct clk *clock;
boolsuspended;
+ u8 shift_pa;
};
+struct gce_plat {
+ u32 thread_nr;
+ u8 shift;
+};
+
+u8 cmdq_mbox_shift(struct mbox_chan *chan)
+{
+ struct cmdq *cmdq = container_of(chan->mbox, struct cmdq, mbox);
+
+ return cmdq->shift_pa;
+}
+EXPORT_SYMBOL(cmdq_mbox_shift);
+
static int cmdq_thread_suspend(struct cmdq *cmdq, struct cmdq_thread *thread)
{
u32 status;
@@ -183,7 +197,7 @@ static void cmdq_task_remove_wfe(struct cmdq_task *task)
for (i = 0; i < CMDQ_NUM_CMD(task->pkt); i++)
if (cmdq_command_is_wfe(base[i]))
base[i] = (u64)CMDQ_JUMP_BY_OFFSET << 32 |
- CMDQ_JUMP_PASS;
+ CMDQ_JUMP_PASS >> task->cmdq->shift_pa;
dma_sync_single_for_device(dev, task->pa_base, task->pkt->cmd_buf_size,
DMA_TO_DEVICE);
}
@@ -221,13 +235,15 @@ static void cmdq_task_handle_error(struct cmdq_task *task)
{
struct cmdq_thread *thread = task->thread;
struct cmdq_task *next_task;
+ struct cmdq *cmdq = task->cmdq;
- dev_err(task->cmdq->mbox.dev, "task 0x%p error\n", task);
- WARN_ON(cmdq_thread_suspend(task->cmdq, thread) < 0);
+ dev_err(cmdq->mbox.dev, "task 0x%p error\n", task);
+ WARN_ON(cmdq_thread_suspend(cmdq, thread) < 0);
next_task = list_first_entry_or_null(&thread->task_busy_list,
struct cmdq_task, list_entry);
if (next_task)
- writel(next_task->pa_base, thread->base + CMDQ_THR_CURR_ADDR);
+ writel(next_task->pa_base >> cmdq->shift_pa,
+ thread->base + CMDQ_THR_CURR_ADDR);
cmdq_thread_resume(thread);
}
@@ -257,7 +273,7 @@ static void cmdq_thread_irq_handler(struct cmdq *cmdq,
else
return;
- curr_pa = readl(thread->base + CMDQ_THR_CURR_ADDR);
+ curr_pa = readl(thread->base + CMDQ_THR_CURR_ADDR) << cmdq->shift_pa;
list_for_each_entry_safe(task, tmp, &thread->task_busy_list,
list_entry) {
@@ -373,16 +389,20 @@ static int cmdq_mbox_send_data(struct mbox_chan *chan,
void *data)
WARN_ON(clk_enable(cmdq->clock) < 0);
WARN_ON(cmdq_thread_reset(cmdq, thread) < 0);
- writel(task->pa_base, thread->base + CMDQ_THR_CURR_ADDR);
- writel(task->pa_base + pkt->cmd_buf_size,
+ writel(task->pa_base >> cmdq->shift_pa,
+ thread->base + CMDQ_THR_CURR_ADDR);
+ writel((task->pa_base + pkt->cmd_buf_size) >> cmdq->shift_pa,
thread->base + CMDQ_THR_END_ADDR);
+
writel(thread->priority, thread->base + CMDQ_THR_PRIORITY);
writel(CMDQ_THR_IRQ_EN, thread->base + CMDQ_THR_IRQ_ENABLE);
writel(CMDQ_THR_ENABLED, thread->base + CMDQ_THR_ENABLE_TASK);
} else {
WARN_ON(cmdq_thread_suspend(cmdq, thread) < 0);
- curr_pa = readl(thread->base + CMDQ_THR_CURR_ADDR);
- end_pa = readl(thread->base + CMDQ_THR_END_ADDR);
+ curr_pa = readl(thread->base + CMDQ_THR_CURR_ADDR) <<
+ cmdq->shift_pa;
+ end_pa = readl(thread->base + CMDQ_THR_END_ADDR) <<
+ cmdq->shift_pa;
/*
* Atomic execution should remove the following wfe, i.e. only
@@ -395,7 +415,7 @@ static int cmdq_mbox_send_data(struct mbox_chan *chan, void
*data)
cmdq_thread_wait_end(thread, end_pa);
WARN_ON(cmdq_thread_suspend(cmdq, thread) < 0);
/* set to this task directly */
- writel(task->pa_base,
+ writel(task->pa_base >> cmdq->shift_pa,
thread->base + CMDQ_THR_CURR_ADDR);
} else {
cmdq_task_insert_into_thread(task);
@@ -407,14 +427,14 @@ static int cmdq_mbox_send_data(struct mbox_chan *chan,
void *data)
if (curr_pa == end_
[v1] drm/msm: add shutdown support for display platform_driver
Define shutdown callback for display drm driver,
so as to disable all the CRTCS when shutdown
notification is received by the driver.
Signed-off-by: Krishna Manikandan
---
drivers/gpu/drm/msm/msm_drv.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index e4b750b..7a8953f 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -1322,6 +1322,18 @@ static int msm_pdev_remove(struct platform_device *pdev)
return 0;
}
+static void msm_pdev_shutdown(struct platform_device *pdev)
+{
+ struct drm_device *drm = platform_get_drvdata(pdev);
+
+ if (!drm) {
+ DRM_ERROR("Invalid drm device node\n");
+ return;
+ }
+
+ drm_atomic_helper_shutdown(drm);
+}
+
static const struct of_device_id dt_match[] = {
{ .compatible = "qcom,mdp4", .data = (void *)KMS_MDP4 },
{ .compatible = "qcom,mdss", .data = (void *)KMS_MDP5 },
@@ -1334,6 +1346,7 @@ static int msm_pdev_remove(struct platform_device *pdev)
static struct platform_driver msm_platform_driver = {
.probe = msm_pdev_probe,
.remove = msm_pdev_remove,
+ .shutdown = msm_pdev_shutdown,
.driver = {
.name = "msm",
.of_match_table = dt_match,
--
1.9.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [v1] drm/msm: add shutdown support for display platform_driver
Quoting Krishna Manikandan (2020-05-28 01:38:23)
> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> index e4b750b..7a8953f 100644
> --- a/drivers/gpu/drm/msm/msm_drv.c
> +++ b/drivers/gpu/drm/msm/msm_drv.c
> @@ -1322,6 +1322,18 @@ static int msm_pdev_remove(struct platform_device
> *pdev)
> return 0;
> }
>
> +static void msm_pdev_shutdown(struct platform_device *pdev)
> +{
> + struct drm_device *drm = platform_get_drvdata(pdev);
> +
> + if (!drm) {
> + DRM_ERROR("Invalid drm device node\n");
> + return;
> + }
When would this ever happen? Please drop this useless check.
> +
> + drm_atomic_helper_shutdown(drm);
> +}
> +
> static const struct of_device_id dt_match[] = {
> { .compatible = "qcom,mdp4", .data = (void *)KMS_MDP4 },
> { .compatible = "qcom,mdss", .data = (void *)KMS_MDP5 },
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] omapfb: Fix reference count leak in display_init_sysfs.
From: Qiushi Wu
kobject_init_and_add() takes reference even when it fails.
If this function returns an error, kobject_put() must be called to
properly clean up the memory associated with the object.
Because function omap_dss_put_device() doesn't handle dssdev->kobj,
thus we need insert kobject_put() to clean up the kobject,
when kobject_init_and_add() fails.
Fixes: f76ee892a99e ("omapfb: copy omapdss & displays for omapfb")
Signed-off-by: Qiushi Wu
---
drivers/video/fbdev/omap2/omapfb/dss/display-sysfs.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/video/fbdev/omap2/omapfb/dss/display-sysfs.c
b/drivers/video/fbdev/omap2/omapfb/dss/display-sysfs.c
index 6dbe265b312d..51322ac7df07 100644
--- a/drivers/video/fbdev/omap2/omapfb/dss/display-sysfs.c
+++ b/drivers/video/fbdev/omap2/omapfb/dss/display-sysfs.c
@@ -316,6 +316,7 @@ int display_init_sysfs(struct platform_device *pdev)
&pdev->dev.kobj, "%s", dssdev->alias);
if (r) {
DSSERR("failed to create sysfs files\n");
+ kobject_put(&dssdev->kobj);
omap_dss_put_device(dssdev);
goto err;
}
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[v1] drm/msm/dpu: allow initialization of encoder locks during encoder init
In the current implementation, mutex initialization for encoder mutex locks are done during encoder setup. This can lead to scenarios where the lock is used before it is initialized. Move mutex_init to dpu_encoder_init to avoid this. Signed-off-by: Krishna Manikandan --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index f8ac3bf..21730a5 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -2145,7 +2145,6 @@ int dpu_encoder_setup(struct drm_device *dev, struct drm_encoder *enc, dpu_enc = to_dpu_encoder_virt(enc); - mutex_init(&dpu_enc->enc_lock); ret = dpu_encoder_setup_display(dpu_enc, dpu_kms, disp_info); if (ret) goto fail; @@ -2160,7 +2159,6 @@ int dpu_encoder_setup(struct drm_device *dev, struct drm_encoder *enc, 0); - mutex_init(&dpu_enc->rc_lock); INIT_DELAYED_WORK(&dpu_enc->delayed_off_work, dpu_encoder_off_work); dpu_enc->idle_timeout = IDLE_TIMEOUT; @@ -2205,6 +2203,8 @@ struct drm_encoder *dpu_encoder_init(struct drm_device *dev, spin_lock_init(&dpu_enc->enc_spinlock); dpu_enc->enabled = false; + mutex_init(&dpu_enc->enc_lock); + mutex_init(&dpu_enc->rc_lock); return &dpu_enc->base; } -- 1.9.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
linux-next: build failure after merge of the drm-intel-fixes tree
Hi all,
After merging the drm-intel-fixes tree, today's linux-next build (x86_64
allmodconfig) failed like this:
In file included from drivers/gpu/drm/i915/gt/intel_lrc.c:5472:
drivers/gpu/drm/i915/gt/selftest_lrc.c: In function 'live_timeslice_nopreempt':
drivers/gpu/drm/i915/gt/selftest_lrc.c:1311:3: error: too few arguments to
function 'engine_heartbeat_disable'
1311 | engine_heartbeat_disable(engine);
| ^~~~
drivers/gpu/drm/i915/gt/selftest_lrc.c:53:13: note: declared here
53 | static void engine_heartbeat_disable(struct intel_engine_cs *engine,
| ^~~~
drivers/gpu/drm/i915/gt/selftest_lrc.c:1380:3: error: too few arguments to
function 'engine_heartbeat_enable'
1380 | engine_heartbeat_enable(engine);
| ^~~
drivers/gpu/drm/i915/gt/selftest_lrc.c:63:13: note: declared here
63 | static void engine_heartbeat_enable(struct intel_engine_cs *engine,
| ^~~
Caused by commit
1f65efb624c4 ("drm/i915/gt: Prevent timeslicing into unpreemptable requests")
I have reverted that commit for today.
--
Cheers,
Stephen Rothwell
pgpdYBxzSIHsY.pgp
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH][next] drm/amdkfd: fix a dereference of pdd before it is null checked
From: Colin Ian King
Currently pointer pdd is being dereferenced when assigning pointer
dpm and then pdd is being null checked. Fix this by checking if
pdd is null before the dereference of pdd occurs.
Addresses-Coverity: ("Dereference before null check")
Fixes: 522b89c63370 ("drm/amdkfd: Track SDMA utilization per process")
Signed-off-by: Colin Ian King
---
drivers/gpu/drm/amd/amdkfd/kfd_process.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
index 25636789f3d3..bdc58741b32e 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ -103,10 +103,11 @@ static void kfd_sdma_activity_worker(struct work_struct
*work)
return;
pdd = workarea->pdd;
+ if (!pdd)
+ return;
dqm = pdd->dev->dqm;
qpd = &pdd->qpd;
-
- if (!pdd || !dqm || !qpd)
+ if (!dqm || !qpd)
return;
mm = get_task_mm(pdd->process->lead_thread);
--
2.25.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC 02/17] dma-fence: basic lockdep annotations
On 2020-05-12 4:59 a.m., Daniel Vetter wrote:
> Design is similar to the lockdep annotations for workers, but with
> some twists:
>
> - We use a read-lock for the execution/worker/completion side, so that
> this explicit annotation can be more liberally sprinkled around.
> With read locks lockdep isn't going to complain if the read-side
> isn't nested the same way under all circumstances, so ABBA deadlocks
> are ok. Which they are, since this is an annotation only.
>
> - We're using non-recursive lockdep read lock mode, since in recursive
> read lock mode lockdep does not catch read side hazards. And we
> _very_ much want read side hazards to be caught. For full details of
> this limitation see
>
> commit e91498589746065e3ae95d9a00b068e525eec34f
> Author: Peter Zijlstra
> Date: Wed Aug 23 13:13:11 2017 +0200
>
> locking/lockdep/selftests: Add mixed read-write ABBA tests
>
> - To allow nesting of the read-side explicit annotations we explicitly
> keep track of the nesting. lock_is_held() allows us to do that.
>
> - The wait-side annotation is a write lock, and entirely done within
> dma_fence_wait() for everyone by default.
>
> - To be able to freely annotate helper functions I want to make it ok
> to call dma_fence_begin/end_signalling from soft/hardirq context.
> First attempt was using the hardirq locking context for the write
> side in lockdep, but this forces all normal spinlocks nested within
> dma_fence_begin/end_signalling to be spinlocks. That bollocks.
>
> The approach now is to simple check in_atomic(), and for these cases
> entirely rely on the might_sleep() check in dma_fence_wait(). That
> will catch any wrong nesting against spinlocks from soft/hardirq
> contexts.
>
> The idea here is that every code path that's critical for eventually
> signalling a dma_fence should be annotated with
> dma_fence_begin/end_signalling. The annotation ideally starts right
> after a dma_fence is published (added to a dma_resv, exposed as a
> sync_file fd, attached to a drm_syncobj fd, or anything else that
> makes the dma_fence visible to other kernel threads), up to and
> including the dma_fence_wait(). Examples are irq handlers, the
> scheduler rt threads, the tail of execbuf (after the corresponding
> fences are visible), any workers that end up signalling dma_fences and
> really anything else. Not annotated should be code paths that only
> complete fences opportunistically as the gpu progresses, like e.g.
> shrinker/eviction code.
>
> The main class of deadlocks this is supposed to catch are:
>
> Thread A:
>
> mutex_lock(A);
> mutex_unlock(A);
>
> dma_fence_signal();
>
> Thread B:
>
> mutex_lock(A);
> dma_fence_wait();
> mutex_unlock(A);
>
> Thread B is blocked on A signalling the fence, but A never gets around
> to that because it cannot acquire the lock A.
>
> Note that dma_fence_wait() is allowed to be nested within
> dma_fence_begin/end_signalling sections. To allow this to happen the
> read lock needs to be upgraded to a write lock, which means that any
> other lock is acquired between the dma_fence_begin_signalling() call and
> the call to dma_fence_wait(), and still held, this will result in an
> immediate lockdep complaint. The only other option would be to not
> annotate such calls, defeating the point. Therefore these annotations
> cannot be sprinkled over the code entirely mindless to avoid false
> positives.
>
> v2: handle soft/hardirq ctx better against write side and dont forget
> EXPORT_SYMBOL, drivers can't use this otherwise.
>
> Cc: linux-me...@vger.kernel.org
> Cc: linaro-mm-...@lists.linaro.org
> Cc: linux-r...@vger.kernel.org
> Cc: amd-...@lists.freedesktop.org
> Cc: intel-...@lists.freedesktop.org
> Cc: Chris Wilson
> Cc: Maarten Lankhorst
> Cc: Christian König
> Signed-off-by: Daniel Vetter
> ---
> drivers/dma-buf/dma-fence.c | 53 +
> include/linux/dma-fence.h | 12 +
> 2 files changed, 65 insertions(+)
>
> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
> index 6802125349fb..d5c0fd2efc70 100644
> --- a/drivers/dma-buf/dma-fence.c
> +++ b/drivers/dma-buf/dma-fence.c
> @@ -110,6 +110,52 @@ u64 dma_fence_context_alloc(unsigned num)
> }
> EXPORT_SYMBOL(dma_fence_context_alloc);
>
> +#ifdef CONFIG_LOCKDEP
> +struct lockdep_map dma_fence_lockdep_map = {
> + .name = "dma_fence_map"
> +};
> +
> +bool dma_fence_begin_signalling(void)
> +{
> + /* explicitly nesting ... */
> + if (lock_is_held_type(&dma_fence_lockdep_map, 1))
> + return true;
> +
> + /* rely on might_sleep check for soft/hardirq locks */
> + if (in_atomic())
> + return true;
> +
> + /* ... and non-recursive readlock */
> + lock_acquire(&dma_fence_lockdep_map, 0, 0, 1, 1, NULL, _RET_IP_);
> +
> + return false;
> +}
> +EXPORT_SYMBOL(dma_fence_begin_signalling);
Hi Daniel,
This is great work
Re: [PATCH v2 4/4] drm/bridge: tfp410: Fix setup and hold time calculation
Hi Ricardo, Thank you for the patch. On Thu, May 14, 2020 at 04:36:12PM +0200, Ricardo Cañuelo wrote: > The tfp410 has a data de-skew feature that allows the user to compensate > the skew between IDCK and the pixel data and control signals. > > In the driver, the setup and hold times are calculated from the de-skew > value. This retrieves the deskew value from the DT using the proper > datatype and range check as described by the binding (u32 from 0 to 7) > and fixes the calculation of the setup and hold times. How about mentioning that the fix results from a change in the DT bindings ? Otherwise it sounds it was a bug in the driver. I would also mention that the patch fixes the min() calculation, which was clearly wrong. I think I would have split this in two patches. With this addressed, Reviewed-by: Laurent Pinchart > Signed-off-by: Ricardo Cañuelo > --- > drivers/gpu/drm/bridge/ti-tfp410.c | 10 +- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c > b/drivers/gpu/drm/bridge/ti-tfp410.c > index e3eb6364c0f7..21d99b4ea0c9 100644 > --- a/drivers/gpu/drm/bridge/ti-tfp410.c > +++ b/drivers/gpu/drm/bridge/ti-tfp410.c > @@ -220,7 +220,7 @@ static int tfp410_parse_timings(struct tfp410 *dvi, bool > i2c) > struct device_node *ep; > u32 pclk_sample = 0; > u32 bus_width = 24; > - s32 deskew = 0; > + u32 deskew = 0; > > /* Start with defaults. */ > *timings = tfp410_default_timings; > @@ -274,12 +274,12 @@ static int tfp410_parse_timings(struct tfp410 *dvi, > bool i2c) > } > > /* Get the setup and hold time from vendor-specific properties. */ > - of_property_read_u32(dvi->dev->of_node, "ti,deskew", (u32 *)&deskew); > - if (deskew < -4 || deskew > 3) > + of_property_read_u32(dvi->dev->of_node, "ti,deskew", &deskew); > + if (deskew > 7) > return -EINVAL; > > - timings->setup_time_ps = min(0, 1200 - 350 * deskew); > - timings->hold_time_ps = min(0, 1300 + 350 * deskew); > + timings->setup_time_ps = 1200 - 350 * ((s32)deskew - 4); > + timings->hold_time_ps = max(0, 1300 + 350 * ((s32)deskew - 4)); > > return 0; > } -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 3/4] dt-bindings: display: ti,tfp410.txt: convert to yaml
Hi Ricardo,
Thank you for the patch.
On Thu, May 14, 2020 at 04:36:11PM +0200, Ricardo Cañuelo wrote:
> Convert the DT binding documentation for the TI TFP410 DPI-to-DVI
> encoder to json-schema.
>
> The 'ti,deskew' is now an unsigned value from 0 to 7 instead of a signed
> value from -4 to 3. The rest of the binding is a direct translation from
> the old one.
I would have modified this in a separate patch.
> Signed-off-by: Ricardo Cañuelo
> ---
> .../bindings/display/bridge/ti,tfp410.txt | 66 --
> .../bindings/display/bridge/ti,tfp410.yaml| 124 ++
> 2 files changed, 124 insertions(+), 66 deletions(-)
> delete mode 100644
> Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt
> create mode 100644
> Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml
>
> diff --git a/Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt
> b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt
> deleted file mode 100644
> index 5ff4f64ef8e8..
> --- a/Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt
> +++ /dev/null
> @@ -1,66 +0,0 @@
> -TFP410 DPI to DVI encoder
> -=
> -
> -Required properties:
> -- compatible: "ti,tfp410"
> -
> -Optional properties:
> -- powerdown-gpios: power-down gpio
> -- reg: I2C address. If and only if present the device node should be placed
> - into the I2C controller node where the TFP410 I2C is connected to.
> -- ti,deskew: data de-skew in 350ps increments, from -4 to +3, as configured
> - through th DK[3:1] pins. This property shall be present only if the TFP410
> - is not connected through I2C.
> -
> -Required nodes:
> -
> -This device has two video ports. Their connections are modeled using the OF
> -graph bindings specified in [1]. Each port node shall have a single endpoint.
> -
> -- Port 0 is the DPI input port. Its endpoint subnode shall contain a
> - pclk-sample and bus-width property and a remote-endpoint property as
> specified
> - in [1].
> - - If pclk-sample is not defined, pclk-sample = 0 should be assumed for
> -backward compatibility.
> - - If bus-width is not defined then bus-width = 24 should be assumed for
> -backward compatibility.
> -bus-width = 24: 24 data lines are connected and single-edge mode
> -bus-width = 12: 12 data lines are connected and dual-edge mode
> -
> -- Port 1 is the DVI output port. Its endpoint subnode shall contain a
> - remote-endpoint property is specified in [1].
> -
> -[1] Documentation/devicetree/bindings/media/video-interfaces.txt
> -
> -
> -Example
>
> -
> -tfp410: encoder@0 {
> - compatible = "ti,tfp410";
> - powerdown-gpios = <&twl_gpio 2 GPIO_ACTIVE_LOW>;
> - ti,deskew = <4>;
> -
> - ports {
> - #address-cells = <1>;
> - #size-cells = <0>;
> -
> - port@0 {
> - reg = <0>;
> -
> - tfp410_in: endpoint@0 {
> - pclk-sample = <1>;
> - bus-width = <24>;
> - remote-endpoint = <&dpi_out>;
> - };
> - };
> -
> - port@1 {
> - reg = <1>;
> -
> - tfp410_out: endpoint@0 {
> - remote-endpoint = <&dvi_connector_in>;
> - };
> - };
> - };
> -};
> diff --git a/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml
> b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml
> new file mode 100644
> index ..a9f4fd8ea621
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml
> @@ -0,0 +1,124 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/bridge/ti,tfp410.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: TFP410 DPI to DVI encoder
> +
> +maintainers:
> + - Tomi Valkeinen
> + - Jyri Sarha
> +
> +properties:
> + compatible:
> +const: ti,tfp410
> +
> + reg:
> +description: I2C address of the device.
> +maxItems: 1
> +
> + powerdown-gpios:
> +maxItems: 1
> +
> + ti,deskew:
> +description:
> + Data de-skew value in 350ps increments, from 0 to 7, as configured
> + through the DK[3:1] pins. The de-skew multiplier is computed as
> + (DK[3:1] - 4), so it ranges from -4 to 3. This property shall be
> + present only if the TFP410 is not connected through I2C.
I'd replace the last sentence with
if:
required:
- reg
then:
properties:
ti,deskew: false
else:
required:
- ti,deskew
(between the required: and additionalProperties: objects below)
> +$ref: /schemas/types.yaml#/definitions/uint32
> +minimum: 0
> +maximum: 7
> +
> + ports:
> +description:
> + A node containing input and output port nodes with endpoint
> + definitions as documented in
> + D
Re: [v1] drm/bridge: ensure bridge suspend happens during PM sleep
Hi, On Tue, May 12, 2020 at 1:26 PM Stephen Boyd wrote: > > The subject is not specific enough. I'd expect it to be something like: > > drm/bridge: ti-sn65dsi86: ensure bridge suspend happens during PM sleep > > Quoting Harigovindan P (2020-04-22 02:04:43) > > ti-sn65dsi86 bridge is enumerated as a runtime device. > > > > Adding sleep ops to force runtime_suspend when PM suspend is > > requested on the device. > > > > This change needs to be taken along with the series: > > https://patchwork.kernel.org/patch/11494309/ > > Why? It doesn't seem like it should be required to go along with a qcom > specific driver patch. > > > > > Signed-off-by: Harigovindan P > > --- > > Besides the subject: > > Reviewed-by: Stephen Boyd Are you planning to re-post with the changes Stephen requested? Maybe CC Sam too who was nice enough to help land some of my recent changes to this driver. -Doug ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: pl111: add CONFIG_VEXPRESS_CONFIG dependency
On Wed, May 27, 2020 at 7:32 AM Arnd Bergmann wrote:
>
> The vexpress_config code fails to link in some configurations:
>
> drivers/gpu/drm/pl111/pl111_versatile.o: in function `pl111_versatile_init':
> (.text+0x1f0): undefined reference to `devm_regmap_init_vexpress_config'
>
> Add a dependency that links to this only if the dependency is there,
> and prevent the configuration where the drm driver is built-in but
> the config is a loadable module.
>
> Fixes: 826fc86b5903 ("drm: pl111: Move VExpress setup into versatile init")
> Signed-off-by: Arnd Bergmann
> ---
> drivers/gpu/drm/pl111/Kconfig | 1 +
> drivers/gpu/drm/pl111/pl111_versatile.c | 3 ++-
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/pl111/Kconfig b/drivers/gpu/drm/pl111/Kconfig
> index 80f6748055e3..33a005816fdd 100644
> --- a/drivers/gpu/drm/pl111/Kconfig
> +++ b/drivers/gpu/drm/pl111/Kconfig
> @@ -3,6 +3,7 @@ config DRM_PL111
> tristate "DRM Support for PL111 CLCD Controller"
> depends on DRM
> depends on ARM || ARM64 || COMPILE_TEST
> + depends on VEXPRESS_CONFIG || !VEXPRESS_CONFIG
That's really non-obvious. Sometimes I hate kconfig. Thanks for fixing my mess.
Rob
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/doc: device hot-unplug for userspace
On 5/27/20 4:25 PM, Daniel Vetter wrote: On Wed, May 27, 2020 at 9:44 PM Christian König wrote: Am 27.05.20 um 17:23 schrieb Andrey Grodzovsky: On 5/27/20 10:39 AM, Daniel Vetter wrote: On Wed, May 27, 2020 at 3:51 PM Andrey Grodzovsky wrote: On 5/27/20 2:44 AM, Pekka Paalanen wrote: On Tue, 26 May 2020 10:30:20 -0400 Andrey Grodzovsky wrote: On 5/19/20 6:06 AM, Pekka Paalanen wrote: From: Pekka Paalanen Set up the expectations on how hot-unplugging a DRM device should look like to userspace. Written by Daniel Vetter's request and largely based on his comments in IRC and from https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Farchives%2Fdri-devel%2F2020-May%2F265484.html&data=02%7C01%7CAndrey.Grodzovsky%40amd.com%7Cd1aab2c6fe71407a287708d8027c0f3c%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637262079143242123&sdata=krqBSHMfzl%2F4TMaAgEPDq8Y%2BPYWJATZyeDPfhtWQmeg%3D&reserved=0 . Signed-off-by: Pekka Paalanen Cc: Daniel Vetter Cc: Andrey Grodzovsky Cc: Dave Airlie Cc: Sean Paul --- Disclaimer: I am a userspace developer writing for other userspace developers. I took some liberties in defining what should happen without knowing what is actually possible or what existing drivers already implement. --- Documentation/gpu/drm-uapi.rst | 75 ++ 1 file changed, 75 insertions(+) diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst index 56fec6ed1ad8..80db4abd2cbd 100644 --- a/Documentation/gpu/drm-uapi.rst +++ b/Documentation/gpu/drm-uapi.rst @@ -1,3 +1,5 @@ +.. Copyright 2020 DisplayLink (UK) Ltd. + === Userland interfaces === @@ -162,6 +164,79 @@ other hand, a driver requires shared state between clients which is visible to user-space and accessible beyond open-file boundaries, they cannot support render nodes. +Device Hot-Unplug += + +.. note:: + The following is the plan. Implementation is not there yet + (2020 May 13). + +Graphics devices (display and/or render) may be connected via USB (e.g. +display adapters or docking stations) or Thunderbolt (e.g. eGPU). An end +user is able to hot-unplug this kind of devices while they are being +used, and expects that the very least the machine does not crash. Any +damage from hot-unplugging a DRM device needs to be limited as much as +possible and userspace must be given the chance to handle it if it wants +to. Ideally, unplugging a DRM device still lets a desktop to continue +running, but that is going to need explicit support throughout the whole +graphics stack: from kernel and userspace drivers, through display +servers, via window system protocols, and in applications and libraries. + +Other scenarios that should lead to the same are: unrecoverable GPU +crash, PCI device disappearing off the bus, or forced unbind of a driver +from the physical device. + +In other words, from userspace perspective everything needs to keep on +working more or less, until userspace stops using the disappeared DRM +device and closes it completely. Userspace will learn of the device +disappearance from the device removed uevent or in some cases specific +ioctls returning EIO. + +This goal raises at least the following requirements for the kernel and +drivers: + +- The kernel must not hang, crash or oops, no matter what userspace was + in the middle of doing when the device disappeared. + +- All GPU jobs that can no longer run must have their fences + force-signalled to avoid inflicting hangs to userspace. + +- KMS connectors must change their status to disconnected. + +- Legacy modesets and pageflips fake success. + +- Atomic commits, both real and TEST_ONLY, fake success. + +- Pending non-blocking KMS operations deliver the DRM events userspace + is expecting. + +- If underlying memory disappears, the mmaps are replaced with harmless + zero pages where access does not raise SIGBUS. Reads return zeros, + writes are ignored. Regarding this paragraph - what about exiting mappings ? In the first patchset we would actively invalidate all the existing CPU mappings to device memory and i think we still should do it otherwise we will see random crashes in applications as was before. I guess it's because TLBs and page tables are not updated to reflect the fact the device is gone. Hi, I was talking about existing mappings. What I forgot to specify is how new mmap() calls after the device disappearance should work (the end result should be the same still, not failure). I'll clarify this in the next revision. Thanks, pq I see, that ok. Next related question is more for Daniel/Christian - about the implementation of this paragraph, I was thinking about something like checking for device disconnect in ttm_bo_vm_fault_reserved and if so remap the entire VA range for the VMA where the fault address belongs to the global zero page (i.e. (remap_pfn_range(vma, vma->vm_start, page_to_pfn(
Re: [PATCH v2 1/4] ARM: dts: dove: Make the DT compliant with the ti,tfp410 binding
Hi Rob,
On Thu, May 28, 2020 at 11:37:55AM -0600, Rob Herring wrote:
> On Thu, May 14, 2020 at 04:36:09PM +0200, Ricardo Cañuelo wrote:
> > Define a 'ports' node for 'dvi: video@39' and use the proper naming for
> > the powerdown-gpios property to make it compliant with the ti,tfp410
> > binding.
> >
> > This fills the minimum requirements to meet the binding requirements,
> > port endpoints are not defined.
>
> Just make 'ports' optional. This isn't really any better unless you add
> endpoints too.
I wonder how this is supposed to work though. The ti-tfp410 driver will
fail to probe if there's no endpoint. I'd rather already add a node for
the DVI connector and connect it to port@1 of the TFP410.
> > Signed-off-by: Ricardo Cañuelo
> > ---
> > arch/arm/boot/dts/dove-sbc-a510.dts | 13 -
> > 1 file changed, 12 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/arm/boot/dts/dove-sbc-a510.dts
> > b/arch/arm/boot/dts/dove-sbc-a510.dts
> > index 2bb85a9b7614..32804c981625 100644
> > --- a/arch/arm/boot/dts/dove-sbc-a510.dts
> > +++ b/arch/arm/boot/dts/dove-sbc-a510.dts
> > @@ -132,7 +132,18 @@
> > dvi: video@39 {
> > compatible = "ti,tfp410";
> > reg = <0x39>;
> > - powerdown-gpio = <&gpio_ext 3 GPIO_ACTIVE_LOW>;
> > + powerdown-gpios = <&gpio_ext 3 GPIO_ACTIVE_LOW>;
> > +
> > + ports {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + port@0 {
> > + reg = <0>;
> > + };
> > + port@1 {
> > + reg = <1>;
> > + };
> > + };
> > };
> > };
> >
--
Regards,
Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v6 08/16] soc: mediatek: cmdq: add write_s function
On 28/05/2020 19:04, Dennis YC Hsieh wrote:
> add write_s function in cmdq helper functions which
> writes value contains in internal register to address
> with large dma access support.
>
> Signed-off-by: Dennis YC Hsieh
> ---
> drivers/soc/mediatek/mtk-cmdq-helper.c | 21 -
> include/linux/mailbox/mtk-cmdq-mailbox.h | 1 +
> include/linux/soc/mediatek/mtk-cmdq.h| 20
> 3 files changed, 41 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c
> b/drivers/soc/mediatek/mtk-cmdq-helper.c
> index 33153d17c9d9..ee24c0ec0a24 100644
> --- a/drivers/soc/mediatek/mtk-cmdq-helper.c
> +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
> @@ -18,6 +18,10 @@ struct cmdq_instruction {
> union {
> u32 value;
> u32 mask;
> + struct {
> + u16 arg_c;
> + u16 src_reg;
> + };
> };
> union {
> u16 offset;
> @@ -29,7 +33,7 @@ struct cmdq_instruction {
> struct {
> u8 sop:5;
> u8 arg_c_t:1;
> - u8 arg_b_t:1;
> + u8 src_t:1;
This should be part of 7/16.
> u8 dst_t:1;
> };
> };
> @@ -222,6 +226,21 @@ int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8 subsys,
> }
> EXPORT_SYMBOL(cmdq_pkt_write_mask);
>
> +int cmdq_pkt_write_s(struct cmdq_pkt *pkt, u16 high_addr_reg_idx,
> + u16 addr_low, u16 src_reg_idx)
> +{
> + struct cmdq_instruction inst = { {0} };
If you want an empty struct on the stack, I think {}; should be enough, right?
Regards,
Matthias
> +
> + inst.op = CMDQ_CODE_WRITE_S;
> + inst.src_t = CMDQ_REG_TYPE;
> + inst.sop = high_addr_reg_idx;
> + inst.offset = addr_low;
> + inst.src_reg = src_reg_idx;
> +
> + return cmdq_pkt_append_command(pkt, inst);
> +}
> +EXPORT_SYMBOL(cmdq_pkt_write_s);
> +
> int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event)
> {
> struct cmdq_instruction inst = { {0} };
> diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h
> b/include/linux/mailbox/mtk-cmdq-mailbox.h
> index 121c3bb6d3de..ee67dd3b86f5 100644
> --- a/include/linux/mailbox/mtk-cmdq-mailbox.h
> +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
> @@ -59,6 +59,7 @@ enum cmdq_code {
> CMDQ_CODE_JUMP = 0x10,
> CMDQ_CODE_WFE = 0x20,
> CMDQ_CODE_EOC = 0x40,
> + CMDQ_CODE_WRITE_S = 0x90,
> CMDQ_CODE_LOGIC = 0xa0,
> };
>
> diff --git a/include/linux/soc/mediatek/mtk-cmdq.h
> b/include/linux/soc/mediatek/mtk-cmdq.h
> index 83340211e1d3..d623f1aa7814 100644
> --- a/include/linux/soc/mediatek/mtk-cmdq.h
> +++ b/include/linux/soc/mediatek/mtk-cmdq.h
> @@ -12,6 +12,8 @@
> #include
>
> #define CMDQ_NO_TIMEOUT 0xu
> +#define CMDQ_ADDR_HIGH(addr) ((u32)(((addr) >> 16) & GENMASK(31, 0)))
> +#define CMDQ_ADDR_LOW(addr) ((u16)(addr) | BIT(1))
>
> struct cmdq_pkt;
>
> @@ -102,6 +104,24 @@ int cmdq_pkt_write(struct cmdq_pkt *pkt, u8 subsys, u16
> offset, u32 value);
> int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8 subsys,
> u16 offset, u32 value, u32 mask);
>
> +/**
> + * cmdq_pkt_write_s() - append write_s command to the CMDQ packet
> + * @pkt: the CMDQ packet
> + * @high_addr_reg_idx: internal register ID which contains high
> address of pa
> + * @addr_low:low address of pa
> + * @src_reg_idx: the CMDQ internal register ID which cache source value
> + * @mask:the specified target address mask, use U32_MAX if no need
> + *
> + * Return: 0 for success; else the error code is returned
> + *
> + * Support write value to physical address without subsys. Use
> CMDQ_ADDR_HIGH()
> + * to get high address and call cmdq_pkt_assign() to assign value into
> internal
> + * reg. Also use CMDQ_ADDR_LOW() to get low address for addr_low parameter
> when
> + * call to this function.
> + */
> +int cmdq_pkt_write_s(struct cmdq_pkt *pkt, u16 high_addr_reg_idx,
> + u16 addr_low, u16 src_reg_idx);
> +
> /**
> * cmdq_pkt_wfe() - append wait for event command to the CMDQ packet
> * @pkt: the CMDQ packet
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 5/8] dt-bindings: display: panel: Add ilitek ili9341 panel bindings
On Mon, 18 May 2020 19:09:22 +0800, dillon.min...@gmail.com wrote: > From: dillon min > > Add documentation for "ilitek,ili9341" panel. > > Signed-off-by: dillon min > --- > .../bindings/display/panel/ilitek,ili9341.yaml | 69 > ++ > 1 file changed, 69 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/panel/ilitek,ili9341.yaml > Reviewed-by: Rob Herring ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 01/12] dt-bindings: display: Convert ingenic,lcd.txt to YAML
On Sat, 16 May 2020 23:50:46 +0200, Paul Cercueil wrote: > Convert the ingenic,lcd.txt to a new ingenic,lcd.yaml file. > > In the process, the new ingenic,jz4780-lcd compatible string has been > added. > > Signed-off-by: Paul Cercueil > --- > > Notes: > This patch comes from a different patchset so it's effectively a V2. > > Changes were: > - lcd_pclk and lcd clocks are in the correct order now, > - Add 'port' and 'ports' properties, and document the valid ports. > > .../bindings/display/ingenic,lcd.txt | 45 --- > .../bindings/display/ingenic,lcd.yaml | 126 ++ > 2 files changed, 126 insertions(+), 45 deletions(-) > delete mode 100644 Documentation/devicetree/bindings/display/ingenic,lcd.txt > create mode 100644 Documentation/devicetree/bindings/display/ingenic,lcd.yaml > Reviewed-by: Rob Herring ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH 3/6] dt-bindings: display/bridge/nwl-dsi: Drop mux handling
On Fri, May 15, 2020 at 03:12:12PM +0200, Guido Günther wrote: > No need to encode the SoC specifics in the bridge driver. For the > imx8mq we can use the mux-input-bridge. You can't just change bindings like this. You'd still have to support the "old" way. But IMO, this way is the right way. > > Signed-off-by: Guido Günther > --- > .../devicetree/bindings/display/bridge/nwl-dsi.yaml | 6 -- > 1 file changed, 6 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH 4/6] drm/bridge/nwl-dsi: Drop mux handling
On Fri, May 15, 2020 at 03:12:13PM +0200, Guido Günther wrote:
> This will be handled via the mux-input-bridge.
You can't do this. What happens booting a kernel with this change and an
un-modified dtb? You just broke it.
>
> Signed-off-by: Guido Günther
> ---
> drivers/gpu/drm/bridge/Kconfig | 1 -
> drivers/gpu/drm/bridge/nwl-dsi.c | 61
> 2 files changed, 62 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> index 3886c0f41bdd..11444f841e35 100644
> --- a/drivers/gpu/drm/bridge/Kconfig
> +++ b/drivers/gpu/drm/bridge/Kconfig
> @@ -78,7 +78,6 @@ config DRM_NWL_MIPI_DSI
> select DRM_PANEL_BRIDGE
> select GENERIC_PHY_MIPI_DPHY
> select MFD_SYSCON
> - select MULTIPLEXER
> select REGMAP_MMIO
> help
> This enables the Northwest Logic MIPI DSI Host controller as
> diff --git a/drivers/gpu/drm/bridge/nwl-dsi.c
> b/drivers/gpu/drm/bridge/nwl-dsi.c
> index b14d725bf609..8839f333f39c 100644
> --- a/drivers/gpu/drm/bridge/nwl-dsi.c
> +++ b/drivers/gpu/drm/bridge/nwl-dsi.c
> @@ -12,7 +12,6 @@
> #include
> #include
> #include
> -#include
> #include
> #include
> #include
> @@ -44,9 +43,6 @@ enum transfer_direction {
> DSI_PACKET_RECEIVE,
> };
>
> -#define NWL_DSI_ENDPOINT_LCDIF 0
> -#define NWL_DSI_ENDPOINT_DCSS 1
> -
> struct nwl_dsi_plat_clk_config {
> const char *id;
> struct clk *clk;
> @@ -94,7 +90,6 @@ struct nwl_dsi {
> struct reset_control *rst_esc;
> struct reset_control *rst_dpi;
> struct reset_control *rst_pclk;
> - struct mux_control *mux;
>
> /* DSI clocks */
> struct clk *phy_ref_clk;
> @@ -1018,14 +1013,6 @@ static int nwl_dsi_parse_dt(struct nwl_dsi *dsi)
> }
> dsi->tx_esc_clk = clk;
>
> - dsi->mux = devm_mux_control_get(dsi->dev, NULL);
> - if (IS_ERR(dsi->mux)) {
> - ret = PTR_ERR(dsi->mux);
> - if (ret != -EPROBE_DEFER)
> - DRM_DEV_ERROR(dsi->dev, "Failed to get mux: %d\n", ret);
> - return ret;
> - }
> -
> base = devm_platform_ioremap_resource(pdev, 0);
> if (IS_ERR(base))
> return PTR_ERR(base);
> @@ -1073,47 +1060,6 @@ static int nwl_dsi_parse_dt(struct nwl_dsi *dsi)
> return 0;
> }
>
> -static int nwl_dsi_select_input(struct nwl_dsi *dsi)
> -{
> - struct device_node *remote;
> - u32 use_dcss = 1;
> - int ret;
> -
> - remote = of_graph_get_remote_node(dsi->dev->of_node, 0,
> - NWL_DSI_ENDPOINT_LCDIF);
> - if (remote) {
> - use_dcss = 0;
> - } else {
> - remote = of_graph_get_remote_node(dsi->dev->of_node, 0,
> - NWL_DSI_ENDPOINT_DCSS);
> - if (!remote) {
> - DRM_DEV_ERROR(dsi->dev,
> - "No valid input endpoint found\n");
> - return -EINVAL;
> - }
> - }
> -
> - DRM_DEV_INFO(dsi->dev, "Using %s as input source\n",
> - (use_dcss) ? "DCSS" : "LCDIF");
> - ret = mux_control_try_select(dsi->mux, use_dcss);
> - if (ret < 0)
> - DRM_DEV_ERROR(dsi->dev, "Failed to select input: %d\n", ret);
> -
> - of_node_put(remote);
> - return ret;
> -}
You could however make these functions generic for any bridge to use.
Define a function that checks for mux-control property and if found sets
up the mux (IIRC, there's already a concept of a default state). That
should be callable from somewhere generic too.
Rob
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH 1/6] dt-bindings: display/bridge: Add binding for input mux bridge
On Fri, May 15, 2020 at 03:12:10PM +0200, Guido Günther wrote: > The bridge allows to select the input source via a mux controller. > > Signed-off-by: Guido Günther > --- > .../display/bridge/mux-input-bridge.yaml | 123 ++ > 1 file changed, 123 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/bridge/mux-input-bridge.yaml > > diff --git > a/Documentation/devicetree/bindings/display/bridge/mux-input-bridge.yaml > b/Documentation/devicetree/bindings/display/bridge/mux-input-bridge.yaml > new file mode 100644 > index ..4029cf63ee5c > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/bridge/mux-input-bridge.yaml > @@ -0,0 +1,123 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/display/bridge/mux-input-bridge.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: DRM input source selection via multiplexer DRM is not a hardware thing. The graph binding is already designed to support muxing. Generally, multiple endpoints on an input node is a mux. So either the device with the input ports knows how to select the input, or you just need a mux-control property for the port to have some other device implement the control. You could do it like you have below. That would be appropriate if there's a separate h/w device controlling the muxing. Say for example some board level device controlled by i2c. Rob ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PULL] drm-intel-fixes
Hi Dave and Daniel, Here goes drm-intel-fixes-2020-05-28: couple compilation fixes for gcc-9+, and couple fixes for timeslicing, one to respect I915_REQUEST_NOPREEMPT flag and another to incorporate virtual engine into timeslicing. Thanks, Rodrigo. The following changes since commit 9cb1fd0efd195590b828b9b865421ad345a4a145: Linux 5.7-rc7 (2020-05-24 15:32:54 -0700) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-intel tags/drm-intel-fixes-2020-05-28 for you to fetch changes up to 1f65efb624c4164bfd2e2a025fc25e8bfb651daa: drm/i915/gt: Prevent timeslicing into unpreemptable requests (2020-05-28 06:06:27 -0700) couple compilation fixes for gcc-9+, and couple fixes for timeslicing, one to respect I915_REQUEST_NOPREEMPT flag and another to incorporate virtual engine into timeslicing. Arnd Bergmann (2): drm/i915/pmu: avoid an maybe-uninitialized warning drm/i915: work around false-positive maybe-uninitialized warning Chris Wilson (2): drm/i915/gt: Incorporate the virtual engine into timeslicing drm/i915/gt: Prevent timeslicing into unpreemptable requests drivers/gpu/drm/i915/gt/intel_lrc.c| 31 +-- drivers/gpu/drm/i915/gt/selftest_lrc.c | 118 - drivers/gpu/drm/i915/gt/selftest_workarounds.c | 2 + drivers/gpu/drm/i915/i915_pmu.c| 84 +- drivers/gpu/drm/i915/i915_priolist_types.h | 2 +- 5 files changed, 187 insertions(+), 50 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 206987] [drm] [amdgpu] Whole system crashes when the driver is in mode_support_and_system_configuration
https://bugzilla.kernel.org/show_bug.cgi?id=206987 --- Comment #19 from Alex Deucher (alexdeuc...@gmail.com) --- Do these patches help? https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=59dfb0c64d3853d20dc84f4561f28d4f5a2ddc7d https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5aa82e35cacfdff7278b7eeffd9575e9c386289e -- 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] dt-bindings: display: renesas, du: Convert binding to YAML
On Fri, May 15, 2020 at 03:33:40AM +0300, Laurent Pinchart wrote: > Convert the Renesas R-Car DU text binding to YAML. > > Signed-off-by: Laurent Pinchart > --- > .../bindings/display/renesas,du.txt | 139 --- > .../bindings/display/renesas,du.yaml | 915 ++ A 'normal' conversion is about double the lines. I guess this is a sign that the original was not well specified. Maybe this can be split to reduce some of the if/then? One way is define a common 'include' file that each specific instance can reference [...] > diff --git a/Documentation/devicetree/bindings/display/renesas,du.yaml > b/Documentation/devicetree/bindings/display/renesas,du.yaml > new file mode 100644 > index ..ca48065afe1f > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/renesas,du.yaml > @@ -0,0 +1,915 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/display/renesas,du.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Renesas R-Car Display Unit (DU) > + > +maintainers: > + - Laurent Pinchart > + > +description: | > + These DT bindings describe the Display Unit embedded in the Renesas R-Car > + Gen1, R-Car Gen2, R-Car Gen3, RZ/G1 and RZ/G2 SoCs. > + > +properties: > + compatible: > +enum: > + - renesas,du-r8a7743 # for RZ/G1M compatible DU > + - renesas,du-r8a7744 # for RZ/G1N compatible DU > + - renesas,du-r8a7745 # for RZ/G1E compatible DU > + - renesas,du-r8a77470 # for RZ/G1C compatible DU > + - renesas,du-r8a774a1 # for RZ/G2M compatible DU > + - renesas,du-r8a774b1 # for RZ/G2N compatible DU > + - renesas,du-r8a774c0 # for RZ/G2E compatible DU > + - renesas,du-r8a7779 # for R-Car H1 compatible DU > + - renesas,du-r8a7790 # for R-Car H2 compatible DU > + - renesas,du-r8a7791 # for R-Car M2-W compatible DU > + - renesas,du-r8a7792 # for R-Car V2H compatible DU > + - renesas,du-r8a7793 # for R-Car M2-N compatible DU > + - renesas,du-r8a7794 # for R-Car E2 compatible DU > + - renesas,du-r8a7795 # for R-Car H3 compatible DU > + - renesas,du-r8a7796 # for R-Car M3-W compatible DU > + - renesas,du-r8a77965 # for R-Car M3-N compatible DU > + - renesas,du-r8a77970 # for R-Car V3M compatible DU > + - renesas,du-r8a77980 # for R-Car V3H compatible DU > + - renesas,du-r8a77990 # for R-Car E3 compatible DU > + - renesas,du-r8a77995 # for R-Car D3 compatible DU > + > + reg: > +maxItems: 1 > + > + # See compatible-specific constraints below. > + clocks: true > + clock-names: true > + interrupts: true > + resets: true > + reset-names: true > + > + ports: > +type: object > +description: | > + The connections to the DU output video ports are modeled using the OF > + graph bindings specified in > Documentation/devicetree/bindings/graph.txt. > + The number of ports and their assignment are model-dependent. Each port > + shall have a single endpoint. > + > +properties: > + '#address-cells': > +const: 1 > + > + '#size-cells': > +const: 0 > + > +patternProperties: > + "^port@[0-3]$": > +type: object > + > +properties: > + reg: > +maxItems: 1 > + > + endpoint: > +type: object > + > +properties: > + remote-endpoint: > +$ref: /schemas/types.yaml#/definitions/phandle > + > +required: > + - remote-endpoint > + > +additionalProperties: false > + > +additionalProperties: false You can drop this and assume there's a generic check for this. Though I guess this does ensure only 'remote-endpoint' is present which a generic schema couldn't do. > + > +required: > + - port@0 > + - port@1 > + > +additionalProperties: false > + > + renesas,cmms: > +$ref: "/schemas/types.yaml#/definitions/phandle-array" > +description: > + A list of phandles to the CMM instances present in the SoC, one for > each > + available DU channel. > + > + renesas,vsps: > +$ref: "/schemas/types.yaml#/definitions/phandle-array" > +description: > + A list of phandle and channel index tuples to the VSPs that handle the > + memory interfaces for the DU channels. The phandle identifies the VSP > + instance that serves the DU channel, and the channel index identifies > + the LIF instance in that VSP. > + > +required: > + - compatible > + - reg > + - clocks > + - interrupts > + - resets > + - ports > + > +allOf: > + - if: > + properties: > +compatible: > + contains: > +const: renesas,du-r8a7779 > +then: > + properties: > +clocks: > + minItems: 1 > + maxItems: 3 > + items: > +- description: Functional clock > +- description: DU_DOTCL
Re: [PATCH] drm/mm: add ig_frag selftest
Am 28.05.20 um 15:05 schrieb Nirmoy Das:
This patch introduces fragmentation in the address range
and measures time taken by 10k insertions for each modes.
ig_frag() will fail if one of the mode takes more than 1 sec.
Output:
[ 37.326723] drm_mm: igt_sanitycheck - ok!
[ 37.326727] igt_debug 0x-0x0200: 512: free
[ 37.326728] igt_debug 0x0200-0x0600: 1024: used
[ 37.326728] igt_debug 0x0600-0x0a00: 1024: free
[ 37.326729] igt_debug 0x0a00-0x0e00: 1024: used
[ 37.326730] igt_debug 0x0e00-0x1000: 512: free
[ 37.326730] igt_debug total: 4096, used 2048 free 2048
[ 56.040064] drm_mm: best fragmented insert took 504 msecs
[ 56.082184] drm_mm: bottom-up fragmented insert took 40 msecs
[ 56.102255] drm_mm: top-down fragmented insert took 20 msecs
[ 56.107177] drm_mm: evict fragmented insert took 4 msecs
Signed-off-by: Nirmoy Das
Off hand that looks like it should work, but I'm really not an expert
for the drm_mm code.
Feel free to add an Acked-by: Christian König
, but I would consider doing what Chris said
as well.
Christian.
---
drivers/gpu/drm/selftests/drm_mm_selftests.h | 1 +
drivers/gpu/drm/selftests/test-drm_mm.c | 59
2 files changed, 60 insertions(+)
diff --git a/drivers/gpu/drm/selftests/drm_mm_selftests.h
b/drivers/gpu/drm/selftests/drm_mm_selftests.h
index 6b943ea1c57d..8c87c964176b 100644
--- a/drivers/gpu/drm/selftests/drm_mm_selftests.h
+++ b/drivers/gpu/drm/selftests/drm_mm_selftests.h
@@ -14,6 +14,7 @@ selftest(insert, igt_insert)
selftest(replace, igt_replace)
selftest(insert_range, igt_insert_range)
selftest(align, igt_align)
+selftest(frag, igt_frag)
selftest(align32, igt_align32)
selftest(align64, igt_align64)
selftest(evict, igt_evict)
diff --git a/drivers/gpu/drm/selftests/test-drm_mm.c
b/drivers/gpu/drm/selftests/test-drm_mm.c
index 9aabe82dcd3a..4c08ca86bb86 100644
--- a/drivers/gpu/drm/selftests/test-drm_mm.c
+++ b/drivers/gpu/drm/selftests/test-drm_mm.c
@@ -1033,6 +1033,65 @@ static int igt_insert_range(void *ignored)
return 0;
}
+static int igt_frag(void *ignored)
+{
+ const struct insert_mode *mode;
+ struct drm_mm mm;
+ struct drm_mm_node *nodes, *node, *next;
+ unsigned int size = 4096, align = 8192;
+ unsigned long start, timeout = 1000;
+ const unsigned int max_count = 1;
+ unsigned int i;
+ int ret = -EINVAL;
+
+ /* For each of the possible insertion modes, we pass an size, alignment
+* value that is known to introduce fragmentation and check that it
+* doesn't take more than 1 sec.
+*/
+
+ nodes = vzalloc(array_size(max_count, sizeof(*nodes)));
+ if (!nodes)
+ goto err;
+
+ drm_mm_init(&mm, 1, U64_MAX - 2);
+
+ for (mode = insert_modes; mode->name; mode++) {
+ start = jiffies;
+
+ for (i = 0; i < max_count; i++) {
+ if (jiffies_to_msecs(jiffies - start) >= timeout) {
+ pr_err("%s fragmented insert took more than %lu
msecs\n",
+ mode->name, timeout);
+ goto out;
+ }
+
+ if (!expect_insert(&mm, &nodes[i],
+ size, align, i, mode)) {
+ pr_err("%s insert failed with alignment=%d",
+ mode->name, align);
+ goto out;
+ }
+ }
+
+ pr_info("%s fragmented insert took %d msecs\n",
+ mode->name, jiffies_to_msecs(jiffies - start));
+ drm_mm_for_each_node_safe(node, next, &mm)
+ drm_mm_remove_node(node);
+ DRM_MM_BUG_ON(!drm_mm_clean(&mm));
+
+ cond_resched();
+ }
+
+ ret = 0;
+out:
+ drm_mm_for_each_node_safe(node, next, &mm)
+ drm_mm_remove_node(node);
+ drm_mm_takedown(&mm);
+ vfree(nodes);
+err:
+ return ret;
+}
+
static int igt_align(void *ignored)
{
const struct insert_mode *mode;
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v6] drm/ioctl: Add a ioctl to set and get a label on GEM objects
On Thu, May 28, 2020 at 10:06 AM Rohan Garg wrote:
>
> DRM_IOCTL_HANDLE_SET_LABEL lets you label buffers associated
> with a handle, making it easier to debug issues in userspace
> applications.
>
> DRM_IOCTL_HANDLE_GET_LABEL lets you read the label associated
> with a buffer.
>
> Changes in v2:
> - Hoist the IOCTL up into the drm_driver framework
>
> Changes in v3:
> - Introduce a drm_gem_set_label for drivers to use internally
> in order to label a GEM object
> - Hoist string copying up into the IOCTL
> - Fix documentation
> - Move actual gem labeling into drm_gem_adopt_label
>
> Changes in v4:
> - Refactor IOCTL call to only perform string duplication and move
> all gem lookup logic into GEM specific call
>
> Changes in v5:
> - Fix issues pointed out by kbuild test robot
> - Cleanup minor issues around kfree and out/err labels
> - Fixed API documentation issues
> - Rename to DRM_IOCTL_HANDLE_SET_LABEL
> - Introduce a DRM_IOCTL_HANDLE_GET_LABEL to read labels
> - Added some documentation for consumers of this IOCTL
> - Ensure that label's cannot be longer than PAGE_SIZE
> - Set a default label value
> - Drop useless warning
> - Properly return length of label to userspace even if
> userspace did not allocate memory for label.
>
> Changes in v6:
> - Wrap code to make better use of 80 char limit
> - Drop redundant copies of the label
> - Protect concurrent access to labels
> - Improved documentation
> - Refactor setter/getter ioctl's to be static
> - Return EINVAL when label length > PAGE_SIZE
> - Simplify code by calling the default GEM label'ing
> function for all drivers using GEM
> - Do not error out when fetching empty labels
> - Refactor flags to the u32 type and add documentation
> - Refactor ioctls to use correct DRM_IOCTL{R,W,WR} macros
> - Return length of copied label to userspace
>
> Signed-off-by: Rohan Garg
I don't think we should land this until it actually does something
with the label, that feels out of the spirit of our uapi merge rules.
I would suggest looking at dma_buf_set_name(), which would produce
useful output in debugfs's /dma_buf/buf_info. But also presumably you
have something in panfrost using this?
> +int drm_gem_get_label(struct drm_device *dev, struct drm_file *file_priv,
> + struct drm_handle_label *args)
> +{
> + struct drm_gem_object *gem_obj;
> + int len, ret;
> +
> + gem_obj = drm_gem_object_lookup(file_priv, args->handle);
> + if (!gem_obj) {
> + DRM_DEBUG("Failed to look up GEM BO %d\n", args->handle);
> + return -ENOENT;
> + }
> +
> + if (!gem_obj->label) {
> + args->label = NULL;
> + args->len = 0;
> + return 0;
> + }
> +
> + mutex_lock(&gem_obj->bo_lock);
> + len = strlen(gem_obj->label);
> + ret = copy_to_user(u64_to_user_ptr(args->label), gem_obj->label,
> + min(args->len, len));
> + mutex_unlock(&gem_obj->bo_lock);
> + args->len = len;
> + drm_gem_object_put(gem_obj);
> + return ret;
> +}
I think the !gem_obj->label code path also needs to be under the lock,
otherwise you could be racing to copy_to_user from a NULL pointer,
right?
> #define DRM_CORE_IOCTL_COUNT ARRAY_SIZE( drm_ioctls )
> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
> index bb924cddc09c..6fcb7b9ff322 100644
> --- a/include/drm/drm_drv.h
> +++ b/include/drm/drm_drv.h
> @@ -540,6 +540,36 @@ struct drm_driver {
> struct drm_device *dev,
> uint32_t handle);
>
> + /**
> +* @set_label:
> +*
> +* Label a buffer object
> +*
> +* Called by the user via ioctl.
> +*
> +* Returns:
> +*
> +* Length of label on success, negative errno on failure.
> +*/
> + int (*set_label)(struct drm_device *dev,
> +struct drm_file *file_priv,
> +struct drm_handle_label *args);
> +
> + /**
> +* @get_label:
> +*
> +* Read the label of a buffer object.
> +*
> +* Called by the user via ioctl.
> +*
> +* Returns:
> +*
> +* Length of label on success, negative errno on failiure.
> +*/
> + char *(*get_label)(struct drm_device *dev,
> + struct drm_file *file_priv,
> + struct drm_handle_label *args);
> +
I think the documentation should note that these are optional.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1.2 4/4] dt-bindings: display: bridge: renesas, lvds: Convert binding to YAML
On Fri, 15 May 2020 00:42:11 +0300, Laurent Pinchart wrote: > Convert the Renesas R-Car LVDS encoder text binding to YAML. > > Signed-off-by: Laurent Pinchart > Acked-by: Maxime Ripard > --- > Changes since v1: > > - Mention RZ/G1 and R2/G2 explicitly > - Drop the part numbers in comments, only keep the SoC names > - Use one address and size cell in the examples > --- > .../bindings/display/bridge/renesas,lvds.txt | 85 -- > .../bindings/display/bridge/renesas,lvds.yaml | 248 ++ > 2 files changed, 248 insertions(+), 85 deletions(-) > delete mode 100644 > Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt > create mode 100644 > Documentation/devicetree/bindings/display/bridge/renesas,lvds.yaml > Reviewed-by: Rob Herring ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 03/16] backlight: add backlight_is_blank()
On 17/05/2020 22.01, Sam Ravnborg wrote:
> The backlight support has two properties that express the state:
> - power
> - state
>
> It is un-documented and easy to get wrong.
> Add backlight_is_blank() helper to make it simpler for drivers
> to get the check of the state correct.
>
> A lot of drivers also includes checks for fb_blank.
> This check is redundant when the state is checked
> and thus not needed in this helper function.
> But added anyway to avoid introducing subtle bug
> due to the creative use in some drivers.
>
> Rolling out this helper to all relevant backlight drivers
> will eliminate almost all accesses to fb_blank.
Reviewed-by: Peter Ujfalusi
> v2:
> - Added fb_blank condition (Daniel)
>
> Signed-off-by: Sam Ravnborg
> Cc: Daniel Vetter
> Cc: Lee Jones
> Cc: Daniel Thompson
> Cc: Jingoo Han
> ---
> include/linux/backlight.h | 19 +++
> 1 file changed, 19 insertions(+)
>
> diff --git a/include/linux/backlight.h b/include/linux/backlight.h
> index c7d6b2e8c3b5..a0a083b35c47 100644
> --- a/include/linux/backlight.h
> +++ b/include/linux/backlight.h
> @@ -175,6 +175,25 @@ static inline void backlight_put(struct backlight_device
> *bd)
> put_device(&bd->dev);
> }
>
> +/**
> + * backlight_is_blank - Return true if display is expected to be blank
> + * @bd: the backlight device
> + *
> + * Display is expected to be blank if any of these is true::
> + *
> + * 1) if power in not UNBLANK
> + * 2) if fb_blank is not UNBLANK
> + * 3) if state indicate BLANK or SUSPENDED
> + *
> + * Returns true if display is expected to be blank, false otherwise.
> + */
> +static inline bool backlight_is_blank(struct backlight_device *bd)
> +{
> + return bd->props.power != FB_BLANK_UNBLANK ||
> +bd->props.fb_blank != FB_BLANK_UNBLANK ||
> +bd->props.state & (BL_CORE_SUSPENDED | BL_CORE_FBBLANK);
> +}
> +
> extern struct backlight_device *backlight_device_register(const char *name,
> struct device *dev, void *devdata, const struct backlight_ops *ops,
> const struct backlight_properties *props);
>
- Péter
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
pEpkey.asc
Description: application/pgp-keys
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 16/16] backlight: use backlight_is_blank() in all backlight drivers
On 28/05/2020 16.39, Peter Ujfalusi wrote:
> Hi Sam,
>
> On 17/05/2020 22.01, Sam Ravnborg wrote:
>> Replaces the open-coded checks of the state etc.,
>> with the backlight_is_blank() helper.
>> This increases readability of the code and align
>> the functionality across the drivers.
>
> Thanks for the cleanup in with the series!
>
> Checked gpio/pwm/led backlight drivers mostly:
> Reviewed-by: Peter Ujfalusi
Interesting, I had a typo in my e-mail address :o
Let's try again:
Reviewed-by: Peter Ujfalusi
>
>> v2:
>> - Fixed so changelog is readable
>>
>> Signed-off-by: Sam Ravnborg
>> Cc: Lee Jones
>> Cc: Daniel Thompson
>> Cc: Jingoo Han
>> Cc: Michael Hennerich
>> Cc: Support Opensource
>> Cc: Thierry Reding
>> Cc: "Uwe Kleine-König"
>> Cc: Andy Gross
>> Cc: Bjorn Andersson
>> Cc: linux-...@vger.kernel.org
>> Cc: linux-arm-...@vger.kernel.org
>> Cc: patc...@opensource.cirrus.com
>> ---
>> drivers/video/backlight/88pm860x_bl.c| 8 +---
>> drivers/video/backlight/adp5520_bl.c | 5 +
>> drivers/video/backlight/adp8860_bl.c | 5 +
>> drivers/video/backlight/adp8870_bl.c | 5 +
>> drivers/video/backlight/as3711_bl.c | 8 +++-
>> drivers/video/backlight/bd6107.c | 4 +---
>> drivers/video/backlight/corgi_lcd.c | 5 +
>> drivers/video/backlight/cr_bllcd.c | 22 +++---
>> drivers/video/backlight/da903x_bl.c | 8 +---
>> drivers/video/backlight/ep93xx_bl.c | 3 +--
>> drivers/video/backlight/gpio_backlight.c | 4 +---
>> drivers/video/backlight/hp680_bl.c | 4 +---
>> drivers/video/backlight/jornada720_bl.c | 2 +-
>> drivers/video/backlight/kb3886_bl.c | 4 +---
>> drivers/video/backlight/led_bl.c | 4 +---
>> drivers/video/backlight/lm3533_bl.c | 4 +---
>> drivers/video/backlight/locomolcd.c | 4 +---
>> drivers/video/backlight/lv5207lp.c | 4 +---
>> drivers/video/backlight/max8925_bl.c | 8 +---
>> drivers/video/backlight/pwm_bl.c | 4 +---
>> drivers/video/backlight/qcom-wled.c | 4 +---
>> drivers/video/backlight/tps65217_bl.c| 4 +---
>> drivers/video/backlight/wm831x_bl.c | 8 +---
>> 23 files changed, 31 insertions(+), 100 deletions(-)
>>
>> diff --git a/drivers/video/backlight/88pm860x_bl.c
>> b/drivers/video/backlight/88pm860x_bl.c
>> index 20d96a5ac384..162c83ab0f5a 100644
>> --- a/drivers/video/backlight/88pm860x_bl.c
>> +++ b/drivers/video/backlight/88pm860x_bl.c
>> @@ -123,13 +123,7 @@ static int pm860x_backlight_update_status(struct
>> backlight_device *bl)
>> {
>> int brightness = bl->props.brightness;
>>
>> -if (bl->props.power != FB_BLANK_UNBLANK)
>> -brightness = 0;
>> -
>> -if (bl->props.fb_blank != FB_BLANK_UNBLANK)
>> -brightness = 0;
>> -
>> -if (bl->props.state & BL_CORE_SUSPENDED)
>> +if (backlight_is_blank(bl))
>> brightness = 0;
>>
>> return pm860x_backlight_set(bl, brightness);
>> diff --git a/drivers/video/backlight/adp5520_bl.c
>> b/drivers/video/backlight/adp5520_bl.c
>> index 0f63f76723a5..d817b0d95c9d 100644
>> --- a/drivers/video/backlight/adp5520_bl.c
>> +++ b/drivers/video/backlight/adp5520_bl.c
>> @@ -67,10 +67,7 @@ static int adp5520_bl_update_status(struct
>> backlight_device *bl)
>> {
>> int brightness = bl->props.brightness;
>>
>> -if (bl->props.power != FB_BLANK_UNBLANK)
>> -brightness = 0;
>> -
>> -if (bl->props.fb_blank != FB_BLANK_UNBLANK)
>> +if (backlight_is_blank(bl))
>> brightness = 0;
>>
>> return adp5520_bl_set(bl, brightness);
>> diff --git a/drivers/video/backlight/adp8860_bl.c
>> b/drivers/video/backlight/adp8860_bl.c
>> index 19968104fc47..a0ce2a3701fa 100644
>> --- a/drivers/video/backlight/adp8860_bl.c
>> +++ b/drivers/video/backlight/adp8860_bl.c
>> @@ -363,10 +363,7 @@ static int adp8860_bl_update_status(struct
>> backlight_device *bl)
>> {
>> int brightness = bl->props.brightness;
>>
>> -if (bl->props.power != FB_BLANK_UNBLANK)
>> -brightness = 0;
>> -
>> -if (bl->props.fb_blank != FB_BLANK_UNBLANK)
>> +if (backlight_is_blank(bl))
>> brightness = 0;
>>
>> return adp8860_bl_set(bl, brightness);
>> diff --git a/drivers/video/backlight/adp8870_bl.c
>> b/drivers/video/backlight/adp8870_bl.c
>> index 4c0032010cfe..ae4269fdb189 100644
>> --- a/drivers/video/backlight/adp8870_bl.c
>> +++ b/drivers/video/backlight/adp8870_bl.c
>> @@ -401,10 +401,7 @@ static int adp8870_bl_update_status(struct
>> backlight_device *bl)
>> {
>> int brightness = bl->props.brightness;
>>
>> -if (bl->props.power != FB_BLANK_UNBLANK)
>> -brightness = 0;
>> -
>> -if (bl->props.fb_blank != FB_BLANK_UNBLANK)
>> +if (backlight_is_blank(bl))
>> brightness = 0;
>>
>> return adp8870_bl_set(bl, brightness);
>> diff --git a/drivers/video/backlight/as3711_bl.c
>
Re: [PATCH v2 16/16] backlight: use backlight_is_blank() in all backlight drivers
Hi Sam,
On 17/05/2020 22.01, Sam Ravnborg wrote:
> Replaces the open-coded checks of the state etc.,
> with the backlight_is_blank() helper.
> This increases readability of the code and align
> the functionality across the drivers.
Thanks for the cleanup in with the series!
Checked gpio/pwm/led backlight drivers mostly:
Reviewed-by: Peter Ujfalusi
> v2:
> - Fixed so changelog is readable
>
> Signed-off-by: Sam Ravnborg
> Cc: Lee Jones
> Cc: Daniel Thompson
> Cc: Jingoo Han
> Cc: Michael Hennerich
> Cc: Support Opensource
> Cc: Thierry Reding
> Cc: "Uwe Kleine-König"
> Cc: Andy Gross
> Cc: Bjorn Andersson
> Cc: linux-...@vger.kernel.org
> Cc: linux-arm-...@vger.kernel.org
> Cc: patc...@opensource.cirrus.com
> ---
> drivers/video/backlight/88pm860x_bl.c| 8 +---
> drivers/video/backlight/adp5520_bl.c | 5 +
> drivers/video/backlight/adp8860_bl.c | 5 +
> drivers/video/backlight/adp8870_bl.c | 5 +
> drivers/video/backlight/as3711_bl.c | 8 +++-
> drivers/video/backlight/bd6107.c | 4 +---
> drivers/video/backlight/corgi_lcd.c | 5 +
> drivers/video/backlight/cr_bllcd.c | 22 +++---
> drivers/video/backlight/da903x_bl.c | 8 +---
> drivers/video/backlight/ep93xx_bl.c | 3 +--
> drivers/video/backlight/gpio_backlight.c | 4 +---
> drivers/video/backlight/hp680_bl.c | 4 +---
> drivers/video/backlight/jornada720_bl.c | 2 +-
> drivers/video/backlight/kb3886_bl.c | 4 +---
> drivers/video/backlight/led_bl.c | 4 +---
> drivers/video/backlight/lm3533_bl.c | 4 +---
> drivers/video/backlight/locomolcd.c | 4 +---
> drivers/video/backlight/lv5207lp.c | 4 +---
> drivers/video/backlight/max8925_bl.c | 8 +---
> drivers/video/backlight/pwm_bl.c | 4 +---
> drivers/video/backlight/qcom-wled.c | 4 +---
> drivers/video/backlight/tps65217_bl.c| 4 +---
> drivers/video/backlight/wm831x_bl.c | 8 +---
> 23 files changed, 31 insertions(+), 100 deletions(-)
>
> diff --git a/drivers/video/backlight/88pm860x_bl.c
> b/drivers/video/backlight/88pm860x_bl.c
> index 20d96a5ac384..162c83ab0f5a 100644
> --- a/drivers/video/backlight/88pm860x_bl.c
> +++ b/drivers/video/backlight/88pm860x_bl.c
> @@ -123,13 +123,7 @@ static int pm860x_backlight_update_status(struct
> backlight_device *bl)
> {
> int brightness = bl->props.brightness;
>
> - if (bl->props.power != FB_BLANK_UNBLANK)
> - brightness = 0;
> -
> - if (bl->props.fb_blank != FB_BLANK_UNBLANK)
> - brightness = 0;
> -
> - if (bl->props.state & BL_CORE_SUSPENDED)
> + if (backlight_is_blank(bl))
> brightness = 0;
>
> return pm860x_backlight_set(bl, brightness);
> diff --git a/drivers/video/backlight/adp5520_bl.c
> b/drivers/video/backlight/adp5520_bl.c
> index 0f63f76723a5..d817b0d95c9d 100644
> --- a/drivers/video/backlight/adp5520_bl.c
> +++ b/drivers/video/backlight/adp5520_bl.c
> @@ -67,10 +67,7 @@ static int adp5520_bl_update_status(struct
> backlight_device *bl)
> {
> int brightness = bl->props.brightness;
>
> - if (bl->props.power != FB_BLANK_UNBLANK)
> - brightness = 0;
> -
> - if (bl->props.fb_blank != FB_BLANK_UNBLANK)
> + if (backlight_is_blank(bl))
> brightness = 0;
>
> return adp5520_bl_set(bl, brightness);
> diff --git a/drivers/video/backlight/adp8860_bl.c
> b/drivers/video/backlight/adp8860_bl.c
> index 19968104fc47..a0ce2a3701fa 100644
> --- a/drivers/video/backlight/adp8860_bl.c
> +++ b/drivers/video/backlight/adp8860_bl.c
> @@ -363,10 +363,7 @@ static int adp8860_bl_update_status(struct
> backlight_device *bl)
> {
> int brightness = bl->props.brightness;
>
> - if (bl->props.power != FB_BLANK_UNBLANK)
> - brightness = 0;
> -
> - if (bl->props.fb_blank != FB_BLANK_UNBLANK)
> + if (backlight_is_blank(bl))
> brightness = 0;
>
> return adp8860_bl_set(bl, brightness);
> diff --git a/drivers/video/backlight/adp8870_bl.c
> b/drivers/video/backlight/adp8870_bl.c
> index 4c0032010cfe..ae4269fdb189 100644
> --- a/drivers/video/backlight/adp8870_bl.c
> +++ b/drivers/video/backlight/adp8870_bl.c
> @@ -401,10 +401,7 @@ static int adp8870_bl_update_status(struct
> backlight_device *bl)
> {
> int brightness = bl->props.brightness;
>
> - if (bl->props.power != FB_BLANK_UNBLANK)
> - brightness = 0;
> -
> - if (bl->props.fb_blank != FB_BLANK_UNBLANK)
> + if (backlight_is_blank(bl))
> brightness = 0;
>
> return adp8870_bl_set(bl, brightness);
> diff --git a/drivers/video/backlight/as3711_bl.c
> b/drivers/video/backlight/as3711_bl.c
> index 33f0f0f2e8b3..7fa76008c7bf 100644
> --- a/drivers/video/backlight/as3711_bl.c
> +++ b/drivers/video/backlight/as3711_bl.c
> @@ -107,13 +107,11 @@ static int as3711_bl_update_status(struct
> backlight_dev
Re: [PATCH v2 1/4] ARM: dts: dove: Make the DT compliant with the ti,tfp410 binding
On Thu, May 14, 2020 at 04:36:09PM +0200, Ricardo Cañuelo wrote:
> Define a 'ports' node for 'dvi: video@39' and use the proper naming for
> the powerdown-gpios property to make it compliant with the ti,tfp410
> binding.
>
> This fills the minimum requirements to meet the binding requirements,
> port endpoints are not defined.
Just make 'ports' optional. This isn't really any better unless you add
endpoints too.
>
> Signed-off-by: Ricardo Cañuelo
> ---
> arch/arm/boot/dts/dove-sbc-a510.dts | 13 -
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/dove-sbc-a510.dts
> b/arch/arm/boot/dts/dove-sbc-a510.dts
> index 2bb85a9b7614..32804c981625 100644
> --- a/arch/arm/boot/dts/dove-sbc-a510.dts
> +++ b/arch/arm/boot/dts/dove-sbc-a510.dts
> @@ -132,7 +132,18 @@
> dvi: video@39 {
> compatible = "ti,tfp410";
> reg = <0x39>;
> - powerdown-gpio = <&gpio_ext 3 GPIO_ACTIVE_LOW>;
> + powerdown-gpios = <&gpio_ext 3 GPIO_ACTIVE_LOW>;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + port@0 {
> + reg = <0>;
> + };
> + port@1 {
> + reg = <1>;
> + };
> + };
> };
> };
>
> --
> 2.18.0
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] Documentation: Fix typo error in todo.rst
Change wrong function name drm_modest_lock_all() to drm_modeset_lock_all() Signed-off-by: Sidong Yang --- Documentation/gpu/todo.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst index 658b52f7ffc6..436489b53fea 100644 --- a/Documentation/gpu/todo.rst +++ b/Documentation/gpu/todo.rst @@ -305,7 +305,7 @@ acquire context. Replace the boilerplate code surrounding drm_modeset_lock_all_ctx() with DRM_MODESET_LOCK_ALL_BEGIN() and DRM_MODESET_LOCK_ALL_END() instead. -This should also be done for all places where drm_modest_lock_all() is still +This should also be done for all places where drm_modeset_lock_all() is still used. As a reference, take a look at the conversions already completed in drm core. -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6] drm/ioctl: Add a ioctl to set and get a label on GEM objects
DRM_IOCTL_HANDLE_SET_LABEL lets you label buffers associated
with a handle, making it easier to debug issues in userspace
applications.
DRM_IOCTL_HANDLE_GET_LABEL lets you read the label associated
with a buffer.
Changes in v2:
- Hoist the IOCTL up into the drm_driver framework
Changes in v3:
- Introduce a drm_gem_set_label for drivers to use internally
in order to label a GEM object
- Hoist string copying up into the IOCTL
- Fix documentation
- Move actual gem labeling into drm_gem_adopt_label
Changes in v4:
- Refactor IOCTL call to only perform string duplication and move
all gem lookup logic into GEM specific call
Changes in v5:
- Fix issues pointed out by kbuild test robot
- Cleanup minor issues around kfree and out/err labels
- Fixed API documentation issues
- Rename to DRM_IOCTL_HANDLE_SET_LABEL
- Introduce a DRM_IOCTL_HANDLE_GET_LABEL to read labels
- Added some documentation for consumers of this IOCTL
- Ensure that label's cannot be longer than PAGE_SIZE
- Set a default label value
- Drop useless warning
- Properly return length of label to userspace even if
userspace did not allocate memory for label.
Changes in v6:
- Wrap code to make better use of 80 char limit
- Drop redundant copies of the label
- Protect concurrent access to labels
- Improved documentation
- Refactor setter/getter ioctl's to be static
- Return EINVAL when label length > PAGE_SIZE
- Simplify code by calling the default GEM label'ing
function for all drivers using GEM
- Do not error out when fetching empty labels
- Refactor flags to the u32 type and add documentation
- Refactor ioctls to use correct DRM_IOCTL{R,W,WR} macros
- Return length of copied label to userspace
Signed-off-by: Rohan Garg
---
drivers/gpu/drm/drm_gem.c | 68 +
drivers/gpu/drm/drm_internal.h | 5 +++
drivers/gpu/drm/drm_ioctl.c| 70 ++
include/drm/drm_drv.h | 30 +++
include/drm/drm_gem.h | 19 +
include/uapi/drm/drm.h | 25 +++-
6 files changed, 216 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index efc0367841e2..942479dc152f 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -154,8 +154,10 @@ void drm_gem_private_object_init(struct drm_device *dev,
obj->dev = dev;
obj->filp = NULL;
+ obj->label = NULL;
kref_init(&obj->refcount);
+ mutex_init(&obj->bo_lock);
obj->handle_count = 0;
obj->size = size;
dma_resv_init(&obj->_resv);
@@ -940,6 +942,69 @@ drm_gem_release(struct drm_device *dev, struct drm_file
*file_private)
idr_destroy(&file_private->object_idr);
}
+int drm_gem_set_label(struct drm_device *dev, struct drm_file *file_priv,
+ struct drm_handle_label *args)
+{
+ struct drm_gem_object *gem_obj;
+ struct pid *_pid = get_task_pid(current, PIDTYPE_PID);
+ char *label;
+
+ gem_obj = drm_gem_object_lookup(file_priv, args->handle);
+ if (!gem_obj) {
+ DRM_DEBUG("Failed to look up GEM BO %d\n", args->handle);
+ return -ENOENT;
+ }
+
+ if (args->len && args->label)
+ label = strndup_user(u64_to_user_ptr(args->label),
+args->len + 1);
+ else
+ label = NULL;
+
+ drm_gem_adopt_label(gem_obj, label);
+ drm_gem_object_put(gem_obj);
+ return args->len;
+}
+EXPORT_SYMBOL(drm_gem_set_label);
+
+void drm_gem_adopt_label(struct drm_gem_object *gem_obj, const char *label)
+{
+ mutex_lock(&gem_obj->bo_lock);
+ kfree(gem_obj->label);
+ gem_obj->label = label;
+ mutex_unlock(&gem_obj->bo_lock);
+}
+EXPORT_SYMBOL(drm_gem_adopt_label);
+
+int drm_gem_get_label(struct drm_device *dev, struct drm_file *file_priv,
+ struct drm_handle_label *args)
+{
+ struct drm_gem_object *gem_obj;
+ int len, ret;
+
+ gem_obj = drm_gem_object_lookup(file_priv, args->handle);
+ if (!gem_obj) {
+ DRM_DEBUG("Failed to look up GEM BO %d\n", args->handle);
+ return -ENOENT;
+ }
+
+ if (!gem_obj->label) {
+ args->label = NULL;
+ args->len = 0;
+ return 0;
+ }
+
+ mutex_lock(&gem_obj->bo_lock);
+ len = strlen(gem_obj->label);
+ ret = copy_to_user(u64_to_user_ptr(args->label), gem_obj->label,
+ min(args->len, len));
+ mutex_unlock(&gem_obj->bo_lock);
+ args->len = len;
+ drm_gem_object_put(gem_obj);
+ return ret;
+}
+EXPORT_SYMBOL(drm_gem_get_label);
+
/**
* drm_gem_object_release - release GEM buffer object resources
* @obj: GEM buffer object
@@ -957,6 +1022,9 @@ drm_gem_object_release(struct drm_gem_object *obj)
dma_resv_fini(&obj->_resv);
[PATCH] drm/kms: Optimize compute_crc(), blend()
Hi Daniel,
I'm newbie in linux kernel and interested in drm module.
Please check this patch and give some advice for me.
I want to participate in developing kernel and community.
Thanks,
Sidong
Optimize looping pixels in compute_crc() and blend(). Calculate
src_offset in start of looping horizontally and increase it.
It's better than calculating in every pixels.
Signed-off-by: Sidong Yang
---
drivers/gpu/drm/vkms/vkms_composer.c | 32 +++-
1 file changed, 17 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/vkms/vkms_composer.c
b/drivers/gpu/drm/vkms/vkms_composer.c
index 4af2f19480f4..9d2a765ca1fb 100644
--- a/drivers/gpu/drm/vkms/vkms_composer.c
+++ b/drivers/gpu/drm/vkms/vkms_composer.c
@@ -28,14 +28,14 @@ static uint32_t compute_crc(void *vaddr_out, struct
vkms_composer *composer)
u32 crc = 0;
for (i = y_src; i < y_src + h_src; ++i) {
- for (j = x_src; j < x_src + w_src; ++j) {
- src_offset = composer->offset
-+ (i * composer->pitch)
-+ (j * composer->cpp);
+ src_offset = composer->offset + (i * composer->pitch)
+ + (x_src * composer->cpp);
+ for (j = 0 ; j < w_src; ++j) {
/* XRGB format ignores Alpha channel */
memset(vaddr_out + src_offset + 24, 0, 8);
crc = crc32_le(crc, vaddr_out + src_offset,
sizeof(u32));
+ src_offset += composer->cpp;
}
}
@@ -61,7 +61,7 @@ static void blend(void *vaddr_dst, void *vaddr_src,
struct vkms_composer *dest_composer,
struct vkms_composer *src_composer)
{
- int i, j, j_dst, i_dst;
+ int i, j, i_dst;
int offset_src, offset_dst;
int x_src = src_composer->src.x1 >> 16;
@@ -73,21 +73,23 @@ static void blend(void *vaddr_dst, void *vaddr_src,
int w_dst = drm_rect_width(&src_composer->dst);
int y_limit = y_src + h_dst;
- int x_limit = x_src + w_dst;
- for (i = y_src, i_dst = y_dst; i < y_limit; ++i) {
- for (j = x_src, j_dst = x_dst; j < x_limit; ++j) {
- offset_dst = dest_composer->offset
-+ (i_dst * dest_composer->pitch)
-+ (j_dst++ * dest_composer->cpp);
- offset_src = src_composer->offset
-+ (i * src_composer->pitch)
-+ (j * src_composer->cpp);
+ for (i = y_src, i_dst = y_dst; i < y_limit; ++i, ++i_dst) {
+ offset_dst = dest_composer->offset
++ (i_dst * dest_composer->pitch)
++ (x_dst * dest_composer->cpp);
+ offset_src = src_composer->offset
++ (i * src_composer->pitch)
++ (x_src * src_composer->cpp);
+
+ for (j = 0; j < w_dst; ++j) {
memcpy(vaddr_dst + offset_dst,
vaddr_src + offset_src, sizeof(u32));
+
+ offset_dst += dest_composer->cpp;
+ offset_src += src_composer->cpp;
}
- i_dst++;
}
}
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] drm/dsi: use stack buffer in mipi_dsi_dcs_write()
On Tue, 5 May 2020 at 17:05, Emil Velikov wrote:
>
> Currently the function heap allocates when we have any payload. Where in
> many case the payload is 1 byte - ouch.
>
> From casual observation, vast majority of the payloads are smaller than
> 8 bytes - so use a stack array tx[8] to avoid the senseless kmalloc and
> kfree dance.
>
> Cc: Jani Nikula
> Cc: Thierry Reding
> Signed-off-by: Emil Velikov
> ---
> drivers/gpu/drm/drm_mipi_dsi.c | 20 ++--
> 1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
> index 55531895dde6..b96d5b4629d7 100644
> --- a/drivers/gpu/drm/drm_mipi_dsi.c
> +++ b/drivers/gpu/drm/drm_mipi_dsi.c
> @@ -748,26 +748,26 @@ ssize_t mipi_dsi_dcs_write(struct mipi_dsi_device *dsi,
> u8 cmd,
> {
> ssize_t err;
> size_t size;
> + u8 stack_tx[8];
> u8 *tx;
>
> - if (len > 0) {
> - size = 1 + len;
> -
> + size = 1 + len;
> + if (len > ARRAY_SIZE(stack_tx) - 1) {
> tx = kmalloc(size, GFP_KERNEL);
> if (!tx)
> return -ENOMEM;
> -
> - /* concatenate the DCS command byte and the payload */
> - tx[0] = cmd;
> - memcpy(&tx[1], data, len);
> } else {
> - tx = &cmd;
> - size = 1;
> + tx = stack_tx;
> }
>
> + /* concatenate the DCS command byte and the payload */
> + tx[0] = cmd;
> + if (data)
> + memcpy(&tx[1], data, len);
> +
> err = mipi_dsi_dcs_write_buffer(dsi, tx, size);
>
> - if (len > 0)
> + if (tx != stack_tx)
> kfree(tx);
>
> return err;
> --
Thierry, others - humble ping.
Can you check through the series?
Thanks
Emil
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/mm: add ig_frag selftest
Quoting Nirmoy Das (2020-05-28 14:05:56) > This patch introduces fragmentation in the address range > and measures time taken by 10k insertions for each modes. > > ig_frag() will fail if one of the mode takes more than 1 sec. If you cc intel-...@lists.freedesktop.org, the test case will be run by our CI. -Chris ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 206987] [drm] [amdgpu] Whole system crashes when the driver is in mode_support_and_system_configuration
https://bugzilla.kernel.org/show_bug.cgi?id=206987 --- Comment #18 from Petteri Aimonen (j...@kernelbug.mail.kapsi.fi) --- As best as I can tell, the crash seems to be caused by some floating point exception (such as underflow/overflow) in this function call in dcn_calc_auto.c line 176: dcn_bw_ceil2(v->byte_per_pixel_in_dety[k], 1.0) In dcn_bw_ceil2() the exception occurs in this instruction: addsd 0x0(%rip),%xmm3 which is performing the addition flr + 0.1. At this point %xmm3 is ((int)(v->byte_per_pixel_in_dety[k] / 1.0)) * 1.0 The variable byte_per_pixel_in_dety is only assigned constant values 1.0, 2.0, 4.0, 8.0 so I don't see any reason for addsd to cause a simd exception. I'm not sure if the exception is precise or if it could be delayed from some prior instruction, but AFAIK it should be precise because in usermode the exception handler would attempt a recovery. Having XMM3 or MXCSR values would help, but they don't seem to get included in the dmesg output and I'm not sure if they are available in a crash dump either. Google search turned up https://beowulf.beowulf.narkive.com/tAHxVcs0/simd-exception-kernel-panic-on-skylake-ep-triggered-by-openfoam where the exception was delayed for some reason. Analyzing the dmesgs attached to this bug report, we have following crash locations: Cyrax2020-03-26 21:36: divss xmm0,DWORD PTR [r14+0x17f8] Cyrax2020-04-04 07:40: divss xmm0,DWORD PTR [r14+0x17f8] Cyrax2020-04-18 13:19: divss xmm0,DWORD PTR [r14+0x17f8] farmboy0 2020-04-19 11:43: not a simd exception Cyrax2020-04-23 05:15: divss xmm0,DWORD PTR [r14+0x17f8] Cyrax2020-04-27 19:20: divss xmm0,DWORD PTR [r14+0x17f8] Cyrax2020-05-02 14:18: divss xmm0,DWORD PTR [r14+0x17f8] PetteriA 2020-05-28 16:05: addsd xmm3,QWORD PTR [rip+0x1de967] So the crash locations appear fairly consistent for Cyrax's machine, but no two machines have the same location. For other users affected by this problem, it could be helpful if you install kernel debugging symbols and use decode_stacktrace.sh to convert the raw stack trace to code locations. Also reported on freedesktop amd bugtracker: https://gitlab.freedesktop.org/drm/amd/-/issues/1154 -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 206987] [drm] [amdgpu] Whole system crashes when the driver is in mode_support_and_system_configuration
https://bugzilla.kernel.org/show_bug.cgi?id=206987 --- Comment #17 from Petteri Aimonen (j...@kernelbug.mail.kapsi.fi) --- Created attachment 289381 --> https://bugzilla.kernel.org/attachment.cgi?id=289381&action=edit dmesg from kernel 5.4.0-31 -- 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] drm/mm: add ig_frag selftest
Quoting Nirmoy Das (2020-05-28 14:05:56)
> This patch introduces fragmentation in the address range
> and measures time taken by 10k insertions for each modes.
>
> ig_frag() will fail if one of the mode takes more than 1 sec.
>
> Output:
>
> [ 37.326723] drm_mm: igt_sanitycheck - ok!
> [ 37.326727] igt_debug 0x-0x0200: 512: free
> [ 37.326728] igt_debug 0x0200-0x0600: 1024: used
> [ 37.326728] igt_debug 0x0600-0x0a00: 1024: free
> [ 37.326729] igt_debug 0x0a00-0x0e00: 1024: used
> [ 37.326730] igt_debug 0x0e00-0x1000: 512: free
> [ 37.326730] igt_debug total: 4096, used 2048 free 2048
> [ 56.040064] drm_mm: best fragmented insert took 504 msecs
> [ 56.082184] drm_mm: bottom-up fragmented insert took 40 msecs
> [ 56.102255] drm_mm: top-down fragmented insert took 20 msecs
> [ 56.107177] drm_mm: evict fragmented insert took 4 msecs
>
>
> Signed-off-by: Nirmoy Das
> ---
> drivers/gpu/drm/selftests/drm_mm_selftests.h | 1 +
> drivers/gpu/drm/selftests/test-drm_mm.c | 59
> 2 files changed, 60 insertions(+)
>
> diff --git a/drivers/gpu/drm/selftests/drm_mm_selftests.h
> b/drivers/gpu/drm/selftests/drm_mm_selftests.h
> index 6b943ea1c57d..8c87c964176b 100644
> --- a/drivers/gpu/drm/selftests/drm_mm_selftests.h
> +++ b/drivers/gpu/drm/selftests/drm_mm_selftests.h
> @@ -14,6 +14,7 @@ selftest(insert, igt_insert)
> selftest(replace, igt_replace)
> selftest(insert_range, igt_insert_range)
> selftest(align, igt_align)
> +selftest(frag, igt_frag)
> selftest(align32, igt_align32)
> selftest(align64, igt_align64)
> selftest(evict, igt_evict)
> diff --git a/drivers/gpu/drm/selftests/test-drm_mm.c
> b/drivers/gpu/drm/selftests/test-drm_mm.c
> index 9aabe82dcd3a..4c08ca86bb86 100644
> --- a/drivers/gpu/drm/selftests/test-drm_mm.c
> +++ b/drivers/gpu/drm/selftests/test-drm_mm.c
> @@ -1033,6 +1033,65 @@ static int igt_insert_range(void *ignored)
> return 0;
> }
>
> +static int igt_frag(void *ignored)
> +{
> + const struct insert_mode *mode;
> + struct drm_mm mm;
> + struct drm_mm_node *nodes, *node, *next;
> + unsigned int size = 4096, align = 8192;
> + unsigned long start, timeout = 1000;
> + const unsigned int max_count = 1;
> + unsigned int i;
> + int ret = -EINVAL;
> +
> + /* For each of the possible insertion modes, we pass an size,
> alignment
> +* value that is known to introduce fragmentation and check that it
> +* doesn't take more than 1 sec.
> +*/
> +
> + nodes = vzalloc(array_size(max_count, sizeof(*nodes)));
> + if (!nodes)
> + goto err;
> +
> + drm_mm_init(&mm, 1, U64_MAX - 2);
> +
> + for (mode = insert_modes; mode->name; mode++) {
> + start = jiffies;
> +
> + for (i = 0; i < max_count; i++) {
> + if (jiffies_to_msecs(jiffies - start) >= timeout) {
> + pr_err("%s fragmented insert took more than
> %lu msecs\n",
> + mode->name, timeout);
> + goto out;
We will run into problems with fixed timeouts on slow devices (emulated
virtual machines and whatnot). And yes there are a few that are known to
run kernel selftests and send bug reports (and complain about slow
tests such as the other drm_mm selftests :).
Off the top of my head, we expect the fragmented search to scale
[sub]linearly, whereas at the moment it's quadratic. How about if we do
something like insert as many as we can within 50ms, and then verify
that if we double that number of inserts it is less than 200ms. We will
still run into fun with the CPU scheduler causing imprecise timing, but
that should have sufficiently wide error margins.
-Chris
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 066/105] drm/vc4: txp: Turn the TXP into a CRTC of its own
Hi Maxime,
Have you considered splitting the series into several parts and
focusing on merging one at a time?
IIRC this the longest series _ever_ submitted to dri-devel, plus it
seems to be growing with each revision.
Due to the sheer volume, it's likely to miss various points - large or
small (like below).
On Thu, 28 May 2020 at 08:47, Maxime Ripard wrote:
> +static int vc4_txp_enable_vblank(struct drm_crtc *crtc)
> +{
> + return 0;
> +}
> +
> +static void vc4_txp_disable_vblank(struct drm_crtc *crtc) {}
> +
Core should handle if these are NULL, so the stubs should not be needed.
HTH
Emil
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3] drm/fourcc: document modifier uniqueness requirements
On most hardware, there is a minimum pitch alignment for linear and any
greater multiple of the alignment is fine.
On Navi, the pitch in bytes for linear must be align(width * bpp / 8, 256).
That's because the hw computes the pitch from the width and doesn't allow
setting a custom pitch. For that reason, multi-GPU sharing might not be
possible if the other GPU doesn't align the pitch in exactly the same way.
Marek
On Thu, May 28, 2020 at 10:38 AM Simon Ser wrote:
> There have suggestions to bake pitch alignment, address alignement,
> contiguous memory or other placement (hidden VRAM, GTT/BAR, etc)
> constraints into modifiers. Last time this was brought up it seemed
> like the consensus was to not allow this. Document this in drm_fourcc.h.
>
> There are several reasons for this.
>
> - Encoding all of these constraints in the modifiers would explode the
> search space pretty quickly (we only have 64 bits to work with).
> - Modifiers need to be unambiguous: a buffer can only have a single
> modifier.
> - Modifier users aren't expected to parse modifiers.
>
> v2: add paragraph about aliases (Daniel)
>
> v3: fix unrelated changes sent with the patch
>
> Signed-off-by: Simon Ser
> Reviewed-by: Daniel Vetter
> Cc: Daniel Stone
> Cc: Bas Nieuwenhuizen
> Cc: Dave Airlie
> Cc: Marek Olšák
> ---
> include/uapi/drm/drm_fourcc.h | 15 +++
> 1 file changed, 15 insertions(+)
>
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index 490143500a50..f41fcb1ed63d 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -58,6 +58,21 @@ extern "C" {
> * may preserve meaning - such as number of planes - from the fourcc code,
> * whereas others may not.
> *
> + * Modifiers must uniquely encode buffer layout. In other words, a buffer
> must
> + * match only a single modifier. A modifier must not be a subset of
> layouts of
> + * another modifier. For instance, it's incorrect to encode pitch
> alignment in
> + * a modifier: a buffer may match a 64-pixel aligned modifier and a
> 32-pixel
> + * aligned modifier. That said, modifiers can have implicit minimal
> + * requirements.
> + *
> + * For modifiers where the combination of fourcc code and modifier can
> alias,
> + * a canonical pair needs to be defined and used by all drivers. An
> example
> + * is AFBC, where both ARGB and ABGR have the exact same compressed
> layout.
> + *
> + * Users see modifiers as opaque tokens they can check for equality and
> + * intersect. Users musn't need to know to reason about the modifier value
> + * (i.e. users are not expected to extract information out of the
> modifier).
> + *
> * Vendors should document their modifier usage in as much detail as
> * possible, to ensure maximum compatibility across devices, drivers and
> * applications.
> --
> 2.26.2
>
>
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/fourcc: Add bayer formats and modifiers
Hi Niklas, On Fri, 22 May 2020 at 07:56, Niklas Söderlund wrote: > > Bayer formats are used with cameras and contain green, red and blue > components, with alternating lines of red and green, and blue and green > pixels in different orders. For each block of 2x2 pixels there is one > pixel with a red filter, two with a green filter, and one with a blue > filter. The filters can be arranged in different patterns. > > Add DRM fourcc formats to describe the most common Bayer formats. Also > add a modifiers to describe the custom packing layouts used by the Intel > IPU3 and in the MIPI (Mobile Industry Processor Interface) CSI-2 > specification. > > Signed-off-by: Niklas Söderlund > --- > * Changes since v1 > - Rename the defines from DRM_FORMAT_SRGGB8 to DRM_FORMAT_BAYER_RGGB8. > - Update the fourcc codes passed to fourcc_code() to avoid a conflict. > - Add diagrams for all Bayer formats memory layout. > - Update documentation. > --- > include/uapi/drm/drm_fourcc.h | 205 ++ > 1 file changed, 205 insertions(+) > Where is the user for these new formats - be that kernel or userspace? Did you forget to update the __drm_format_info() in drivers/gpu/drm/drm_fourcc.c? -Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: MIPI DSI, DBI, and tinydrm drivers
On Sun, 24 May 2020 at 19:35, Daniel Vetter wrote: > > On Sun, May 24, 2020 at 7:46 PM Noralf Trønnes wrote: > > > > > > > > Den 24.05.2020 18.13, skrev Paul Cercueil: > > > Hi list, > > > > > > I'd like to open a discussion about the current support of MIPI DSI and > > > DBI panels. > > > > > > Both are standards from the MIPI alliance, both are communication > > > protocols between a LCD controller and a LCD panel, they generally both > > > use the same commands (DCS), the main difference is that DSI is serial > > > and DBI is generally parallel. > > > > > > In the kernel right now, DSI is pretty well implemented. All the > > > infrastucture to register a DSI host, DSI device etc. is there. DSI > > > panels are implemented as regular drm_panel instances, and their drivers > > > go through the DSI API to communicate with the panel, which makes them > > > independent of the DSI host driver. > > > > > > DBI, on the other hand, does not have any of this. All (?) DBI panels > > > are implemented as tinydrm drivers, which make them impossible to use > > > with regular DRM drivers. Writing a standard drm_panel driver is > > > impossible, as there is no concept of host and device. All these tinydrm > > > drivers register their own DBI host as they all do DBI over SPI. > > > > > > I think this needs a good cleanup. Given that DSI and DBI are so > > > similar, it would probably make sense to fuse DBI support into the > > > current DSI code, as trying to update DBI would result in a lot of code > > > being duplicated. With the proper host/device registration mechanism > > > from DSI code, it would be possible to turn most of the tinydrm drivers > > > into regular drm_panel drivers. > > Do we have drivers with dbi support that actually want to reuse the > tinydrm drivers? Good clean is all good, but we need a solid reason > for changing stuff. Plus we need to make sure we're not just > rediscovering all the old reasons for why we ended up where we are > right now in the first place. > > > > The problem then is that these should still be available as tinydrm > > > drivers. If the DSI/DBI panels can somehow register a .update_fb() > > > callback, it would make it possible to have a panel-agnostic tinydrm > > > driver, which would then probably open a lot of doors, and help a lot to > > > clean the mess. > > > > > > I think I can help with that, I just need some guidance - I am fishing > > > in exotic seas here. > > > > > > Thoughts, comments, are very welcome. > > > > I did look at this a few months back: > > > > drm/mipi-dbi: Support panel drivers > > https://lists.freedesktop.org/archives/dri-devel/2019-August/228966.html > > Coming late to the party - the series looks like a great step forward. > > The problem with DBI is that it has reused other busses which means we > > don't have DBI drivers, we have SPI drivers instead (6800/8080 is not > > avail. as busses in Linux yet). DSI and DPI on the other hand has > > dedicated hw controller drivers not shared with other subsystems. > > > > My initial tinydrm work used drm_panel, but I was not allowed to use it > > (at least not the way I had done it). > > Hm, do we have a summary of all the discussions/reasons from back > then? All I remember is that it's all that simple, you've done a lot > of work exploring all the options, I'm fairly sure I suggested > drm_panel even back then but somehow it didn't really work. Would be > good if we make sure we don't at least repeat history too much :-) > This pretty much ^^. Does anyone have a link/summary of the concerns? From userspace POV - having these as panel makes sense. Currently as new tiny drm _driver_ gets added, userspace has to be updated to deal with it ... every so often. Additionally having both DPI and DBI code for the given panel alongside one another makes the overall picture clearer. -Emil Aside: mipi_dbi API should grow a drm_ prefix. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: drm_fourcc: add NV15, Q410, Q401 YUV formats
Hi Brian,
On 2020-05-26 15:52, Brian Starkey wrote:
> Hi Jonas,
>
> On Mon, May 25, 2020 at 11:08:11AM +, Jonas Karlman wrote:
>> Hi,
>>
>> On 2020-05-15 15:37, Brian Starkey wrote:
>>> Hi Ben,
>>>
>>> On Wed, May 06, 2020 at 03:41:26PM +0100, Ben Davis wrote:
Hi all, any feedback on this patch?
Thanks, Ben
On Wed, Apr 22, 2020 at 12:13:49PM +0100, Ben Davis wrote:
> DRM_FORMAT_NV15 is a 2 plane format suitable for linear and 16x16
> block-linear memory layouts. The format is similar to P010 with 4:2:0
> sub-sampling but has no padding between components. Instead, luminance
> and chrominance samples are grouped into 4s so that each group is packed
> into an integer number of bytes:
>
> = UVUV = 4 * 10 bits = 40 bits = 5 bytes
>
> The '15' suffix refers to the optimum effective bits per pixel which is
> achieved when the total number of luminance samples is a multiple of 8.
>
> Q410 and Q401 are both 3 plane non-subsampled formats with 16 bits per
> component, but only 10 bits are used and 6 are padded. 'Q' is chosen
> as the first letter to denote 3 plane YUV444, (and is the next letter
> along from P which is usually 2 plane).
>
> Signed-off-by: Ben Davis
>>>
>>> The descriptions match my understanding of the formats and the
>>> format_info struct, so feel free to add my r-b:
>>>
>>> Reviewed-by: Brian Starkey
>>>
>>> Can anyone else pass comment on the approach and/or naming? I feel
>>> like we should have some non-Arm eyes on this before we merge it.
>>
>> This pixel format seem to match the memory layout used for 10-bit 4:2:0 by
>> the
>> Rockchip Video Decoder, for the rkvdec a 4:2:2 format is also needed (maybe
>> NV20?).
>>
>> From what I can tell the rockchip specific pixel format has previously been
>> submitted in [1]
>> and GStreamer use NV12_10LE40 (fourcc RK20) for this pixel format.
>>
>> [1] https://patchwork.freedesktop.org/patch/276029/
>>
>
> Yeah you're right, this is the same as the Rockchip version. I see
> Randy's submission has `block_w = { 4, 2, 0 }`... more on that below.
>
> The comment on block_w says "in pixels" - but what's a pixel in a
> subsampled chroma plane? For a 2-plane 4:2:0 format, is one pair of
> chroma samples a single pixel, or one pair of chroma samples is two
> pixels?
>
> Looks like Randy assumed the former and us the latter.
>
>>>
>>> Thanks,
>>> -Brian
>>>
> ---
> drivers/gpu/drm/drm_fourcc.c | 12
> include/uapi/drm/drm_fourcc.h | 24
> 2 files changed, 36 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> index b234bfaeda06..0c0a65481afd 100644
> --- a/drivers/gpu/drm/drm_fourcc.c
> +++ b/drivers/gpu/drm/drm_fourcc.c
> @@ -274,6 +274,18 @@ const struct drm_format_info *__drm_format_info(u32
> format)
> { .format = DRM_FORMAT_YUV420_10BIT,.depth = 0,
> .num_planes = 1, .cpp = { 0, 0, 0 }, .hsub = 2, .vsub = 2,
> .is_yuv = true },
> + { .format = DRM_FORMAT_NV15,.depth = 0,
> + .num_planes = 2, .char_per_block = { 5, 5, 0 },
> + .block_w = { 4, 4, 0 }, .block_h = { 1, 1, 0 }, .hsub = 2,
> + .vsub = 2, .is_yuv = true },
>>
>> For a 4:2:0 format I wonder if the char_per_block value is correct for the
>> second plane,
>> using the following formula to calculate the pitch seem to result in only
>> half expected width.
>> Maybe .char_per_block { 5, 10, 0 } could be correct?
>>
>> pitch = (width * char_per_block[1]) / block_w[1] / hsub
>>
>> for 16x16 this would be
>>
>> pitch[1] = (16 * 5) / 4 / 2 = 10 bytes
>> vs
>> pitch[1] = (16 * 10) / 4 / 2 = 20 bytes
>>
>> height[1] = 16 / 2 = 8
>>
>
> I've talked myself round in circles, I don't know what to think any
> more.
>
> drm_format_info_min_pitch() does:
>
> pitch[1] = width * char_per_block[1] / (block_w[1] * block_h[1])
>
> so:
>
> pitch[1] = 16 * 5 / (4 * 1) = 20 bytes
>
> which implies that it expects the subsampling to be baked in to the
> block size, or that it just doesn't consider subsampling and so is
> broken, or that it expects `width` to be pre-divided.
>
>
> Looking at DRM_FORMAT_NV12, it has cpp = {1, 2, 0}, which means it
> considers a single pair of chromas to be a single pixel - which is
> in-line with Randy.
>
> So, I think our definition is inconsistent here. We should have
> either:
>
> block_w = { 4, 4, 0 }, char_per_block = { 5, 10, 0 }
>
> or:
>
> block_w = { 4, 2, 0 }, char_per_block = { 5, 5, 0 }
>
> Probably leaning more towards the { 4, 2, 0 } option, and with a big
> question-mark over whether drm_format_info_min_pitch() is doing the
> right thing.
After reading your comments I can agree that block_w = { 4, 2, 0 }
do sound like the better option.
I also made a successful runtime test using block_w = { 4, 2, 0 } in my
development br
Re: [PATCH 6/6] dt-bindings: drm/msm/gpu: Document gpu opp table
On Thu, 14 May 2020 16:24:19 +0530, Sharat Masetty wrote: > Update documentation to list the gpu opp table bindings including the > newly added "opp-peak-kBps" needed for GPU-DDR bandwidth scaling. > > Signed-off-by: Sharat Masetty > --- > .../devicetree/bindings/display/msm/gpu.txt| 28 > ++ > 1 file changed, 28 insertions(+) > Acked-by: Rob Herring ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] dt-bindings: display: anx7814.txt: convert to yaml
On Thu, 14 May 2020 12:12:35 +0200, Ricardo Cañuelo wrote: > This converts the Analogix ANX7814 bridge DT binding to yaml. Port > definitions and descriptions were expanded, apart from that it's a > direct translation from the original binding. > > Signed-off-by: Ricardo Cañuelo > Acked-by: Sam Ravnborg > Reviewed-by: Enric Balletbo i Serra > --- > Changes in v4: > - Make interrupts and gpios optional (Enric Balletbo) > - Make hpd-gpios deprecated (Rob Herring) > - Remove maxItems from dvdd10-supply > > Changes in v3 (suggested by Sam Ravnborg): > - Rename example node i2c0 to i2c. > > Changes in v2 (suggested by Enric Balletbo): > - File name change: use full compatible string. > - Binding description removed. > - #address-cells and #size-cells properties removed from ports node. > - Example node renamed: anx7814 -> bridge. > > Tested with: > make dt_binding_check ARCH=arm64 DT_SCHEMA_FILES=<.../analogix,anx7814.yaml> > make dtbs_check ARCH=arm64 DT_SCHEMA_FILES=<.../analogix,anx7814.yaml> > > .../display/bridge/analogix,anx7814.yaml | 119 ++ > .../bindings/display/bridge/anx7814.txt | 42 --- > 2 files changed, 119 insertions(+), 42 deletions(-) > create mode 100644 > Documentation/devicetree/bindings/display/bridge/analogix,anx7814.yaml > delete mode 100644 > Documentation/devicetree/bindings/display/bridge/anx7814.txt > Applied, thanks! ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/doc: device hot-unplug for userspace
On Thu, May 28, 2020 at 03:27:57PM +0300, Pekka Paalanen wrote: > On Mon, 25 May 2020 17:09:55 +0200 > Daniel Vetter wrote: > > > On Mon, May 25, 2020 at 05:55:19PM +0300, Pekka Paalanen wrote: > > > On Mon, 25 May 2020 16:28:04 +0200 > > > Daniel Vetter wrote: > > > > > > > On Wed, May 20, 2020 at 10:50:41AM -0400, Andrey Grodzovsky wrote: > > > > > > > > > > On 5/20/20 8:46 AM, Daniel Vetter wrote: > > > > > > On Wed, May 20, 2020 at 02:19:08PM +0300, Pekka Paalanen wrote: > > > > > > > On Tue, 19 May 2020 10:37:12 -0400 > > > > > > > Andrey Grodzovsky wrote: > > > > > > > > > > > > > > > Thanks for the summary, does put things in order and makes it > > > > > > > > easier to > > > > > > > > comprehend all the TODOs, some questions bellow > > > > > > > > > > > > > > > > On 5/19/20 6:06 AM, Pekka Paalanen wrote: > > > > > > > > > From: Pekka Paalanen > > > > > > > > > > > > > > > > > > Set up the expectations on how hot-unplugging a DRM device > > > > > > > > > should look like to > > > > > > > > > userspace. > > ... > > > > > > So currently drm_ioctl will just check for drm_dev_is_unplugged and > > > > > return > > > > > -ENODEV at the very beginning of the function > > > > > (https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/drm_ioctl.c#L825) > > > > > meaning that currently we blanket failure any IOCTL in case the > > > > > device is > > > > > unplagued (ignoring the race condition if the device unplugged after > > > > > the > > > > > check). So do we want to remove this check and put it only for render > > > > > ioctls > > > > > (which are those ? e.g. for amdgpu there is AMDGPU_CS ioctl) but not > > > > > for > > > > > mode setting/dma_buf_import/dma_buf_export ioctls ? What about other > > > > > types > > > > > of ioctls which are non of the listed above ? > > > > > > > > Hm right, and this goes back all the way to first usb udl support: > > > > > > > > commit 2c07a21d6fb0be47fda696a618b726ea258ed1dd > > > > Author: Dave Airlie > > > > Date: Mon Feb 20 14:18:07 2012 + > > > > > > > > drm: add core support for unplugging a device (v2) > > > > > > > > So I guess we need to change the uapi docs to explain that. Not sure > > > > everyone can cope, but I guess they better do. Since the use-case back > > > > then was just udl, so simple legacy kms only, the damage was probably > > > > rather limited. I'm not sure we can get away with that now, where kms > > > > code > > > > has spread to funny places likey vulkan winsys code. > > > > > > > > Or maybe we want a file priv flag you can set along the lines of "give > > > > me > > > > less shitty hotunplug semantics for ioctls". Or maybe we can just change > > > > the semantics, not crashing&burning shouldn't cause a regression :-) > > > > > > > > For everything else (mmap, dma-buf fd, sync_file fd, syncobj fd) I think > > > > the discussion is still more or less accurate. > > > > > > > > Pekka, any thoughts? > > > > > > Hi, > > > > > > is ENODEV unique to this particular failure? > > > > Not really sure, we'd need to audit all of drm ... > > $ git ngrep ENODEV -- drivers/gpu/drm | wc -l > 762 > > Yeah, grep is not enough. > > > > Returning errors instead of faking success was my first idea, but you > > > already convinced me that faking is at least as good if not better. :-) > > > > > > So as long as the error code returned is unique to hot-unplug or other > > > "oops, the device is gone" conditions, I think I'm fine. Weston does > > > not handle ENODEV any way, it never did, and it certainly cannot be > > > called a kernel regression. > > > > > > As a Weston developer, I don't mind adding checks for ENODEV. But if I > > > don't have to, even better. Weston is going to need more code to handle > > > DRM device unplug in any case. > > > > > > Sorry, no preference from me. ;-) > > > > > > I do agree that replacing ENODEV with fake success is hard to imagine > > > regressing anything. It's something you can do in the kernel at any > > > time easily, but going from fake success to error is going to be > > > painful. Maybe don't change things until there is a good reason to? > > > > > > We need a kernel that doesn't crash before we can properly test what > > > would be best for userspace, fake or error. > > > > One upshot of faking stuff and only bailing in low-level hw code is that > > it makes validating the races when you hotunplug easier - if we remove the > > early bail-out check even an ioctl later on will look like it raced with > > the hotunplug path in the kernel. So better assurance that things won't > > blow up badly. > > > > Otoh the early bail out in the top-level ioctl code increases the odds > > that you'll survive even on a driver that's totally buggy. > > > > So yeah I guess maybe we should just document that currently you get an > > -ENODEV and maybe have the option mentioned that we might change this > > going forward. See also > > > > https://dri.
[PULL] drm-intel-next-fixes
Hi Dave & Daniel, Two bigger fixes to corner case kernel access faults and three workload scheduling fixups this week. CI_DINF_191 at: https://intel-gfx-ci.01.org/tree/drm-intel-next-fixes/combined-alt.html? I got gvt-next-fixes pull today, I'll pull it next week so it has time to run through CI. PS. Update on igt@i915_selftest@live@gt_pm is that subtest was updated and the added part is failing for now. The granularity of the kernel selftests is short one level from normal IGT due to nature of how they integrate to CI. Regards, Joonas *** drm-intel-next-fixes-2020-05-28: One empty list iteration bug (Cc: stable), One use after free fix and three workload scheduling fixups. The following changes since commit d96536f0fe699729a0974eb5b65eb0d87cc747e1: drm/i915: Fix AUX power domain toggling across TypeC mode resets (2020-05-19 17:54:07 +0300) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-intel tags/drm-intel-next-fixes-2020-05-28 for you to fetch changes up to 757a9395f33c51c4e6eff2c7c0fbd50226a58224: drm/i915/gem: Avoid iterating an empty list (2020-05-25 15:40:43 +0300) One empty list iteration bug (Cc: stable), One use after free fix and three workload scheduling fixups. Chris Wilson (5): drm/i915: Don't set queue-priority hint when supressing the reschedule drm/i915/gt: Remove errant assertion in __intel_context_do_pin drm/i915: Disable semaphore inter-engine sync without timeslicing drm/i915: Avoid using rq->engine after free during i915_fence_release drm/i915/gem: Avoid iterating an empty list drivers/gpu/drm/i915/gem/i915_gem_context.c | 4 ++-- drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 15 - drivers/gpu/drm/i915/gt/intel_context.c | 2 -- drivers/gpu/drm/i915/i915_request.c | 35 +++-- drivers/gpu/drm/i915/i915_scheduler.c | 16 ++--- 5 files changed, 52 insertions(+), 20 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3] drm/fourcc: document modifier uniqueness requirements
There have suggestions to bake pitch alignment, address alignement,
contiguous memory or other placement (hidden VRAM, GTT/BAR, etc)
constraints into modifiers. Last time this was brought up it seemed
like the consensus was to not allow this. Document this in drm_fourcc.h.
There are several reasons for this.
- Encoding all of these constraints in the modifiers would explode the
search space pretty quickly (we only have 64 bits to work with).
- Modifiers need to be unambiguous: a buffer can only have a single
modifier.
- Modifier users aren't expected to parse modifiers.
v2: add paragraph about aliases (Daniel)
v3: fix unrelated changes sent with the patch
Signed-off-by: Simon Ser
Reviewed-by: Daniel Vetter
Cc: Daniel Stone
Cc: Bas Nieuwenhuizen
Cc: Dave Airlie
Cc: Marek Olšák
---
include/uapi/drm/drm_fourcc.h | 15 +++
1 file changed, 15 insertions(+)
diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index 490143500a50..f41fcb1ed63d 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -58,6 +58,21 @@ extern "C" {
* may preserve meaning - such as number of planes - from the fourcc code,
* whereas others may not.
*
+ * Modifiers must uniquely encode buffer layout. In other words, a buffer must
+ * match only a single modifier. A modifier must not be a subset of layouts of
+ * another modifier. For instance, it's incorrect to encode pitch alignment in
+ * a modifier: a buffer may match a 64-pixel aligned modifier and a 32-pixel
+ * aligned modifier. That said, modifiers can have implicit minimal
+ * requirements.
+ *
+ * For modifiers where the combination of fourcc code and modifier can alias,
+ * a canonical pair needs to be defined and used by all drivers. An example
+ * is AFBC, where both ARGB and ABGR have the exact same compressed layout.
+ *
+ * Users see modifiers as opaque tokens they can check for equality and
+ * intersect. Users musn't need to know to reason about the modifier value
+ * (i.e. users are not expected to extract information out of the modifier).
+ *
* Vendors should document their modifier usage in as much detail as
* possible, to ensure maximum compatibility across devices, drivers and
* applications.
--
2.26.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 07/21] drm/hisilicon/kirin: Use GEM CMA object functions
On Mon, 25 May 2020 at 13:41, Thomas Zimmermann wrote:
>
> Hi Emil
>
> Am 22.05.20 um 20:11 schrieb Emil Velikov:
> > Hi Thomas,
> >
> > On Fri, 22 May 2020 at 14:53, Thomas Zimmermann wrote:
> >>
> >> The kirin driver uses the default implementation for CMA functions; except
> >> for the .dumb_create callback. The __DRM_GEM_CMA_DRIVER_OPS macro now sets
> >> these defaults and .dumb_create in struct drm_driver. All remaining
> >> operations are provided by CMA GEM object functions.
> >>
> >> Signed-off-by: Thomas Zimmermann
> >> ---
> >> drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 12 +---
> >> 1 file changed, 1 insertion(+), 11 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> >> b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> >> index c339e632522a9..b1ffd7d43e562 100644
> >> --- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> >> +++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> >> @@ -921,17 +921,7 @@ DEFINE_DRM_GEM_CMA_FOPS(ade_fops);
> >> static struct drm_driver ade_driver = {
> >> .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
> >> .fops = &ade_fops,
> >> - .gem_free_object_unlocked = drm_gem_cma_free_object,
> >> - .gem_vm_ops = &drm_gem_cma_vm_ops,
> >> - .dumb_create = drm_gem_cma_dumb_create_internal,
> >
> > This doesn't seem right. The _internal documentation explicitly says
> > that this should _not_ be used as .dumb_create. Instead drivers should
> > use it to implement their callback.
> >
> > Since it yields the same result as drm_gem_cma_dumb_create we can use
> > the default macro below.
>
> I noticed this and thought that the driver authors probably had their
> reasons. Changing the driver to the default macro is probably still a
> good idea.
>
To be on the _extra_ safe side might want to keep that separate patch
explicitly CC-ing the author/reviewers of the offending commit.
Although as said before - it's your call, I'm fine either way.
HTH
Emil
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH i-g-t] panfrost: Test labeling functionality
Hi Rohan, On Thu, 28 May 2020 at 14:38, Rohan Garg wrote: > > Introduce tests to cover the new generic labeling ioctl's > being reviewed here [1]. > > Signed-off-by: Rohan Garg > > [1] https://patchwork.freedesktop.org/series/77267/ > > Signed-off-by: Rohan Garg > --- > include/drm-uapi/drm.h| 23 ++- > tests/meson.build | 1 + > tests/panfrost_bo_label.c | 129 ++ > 3 files changed, 152 insertions(+), 1 deletion(-) > create mode 100644 tests/panfrost_bo_label.c > Listing high-level comments, haven't looked at the test itself: - make/name the test generic - I would imagine we can use dumb buffer on panfrost, vc4 or others - catch the missing locking - a) two writers, b) missing read lock would trigger user-after-free - use invalid handles - to highlight the mismatched gem_object_put (as mentioned off-list) The last two categories would produce various kernel warnings/warn/bugs with earlier revisions. Might be a good idea to run the test against those, to ensure the test triggers correctly. -Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC 02/17] dma-fence: basic lockdep annotations
On Thu, May 28, 2020 at 3:37 PM Thomas Hellström (Intel) wrote: > > On 2020-05-12 10:59, Daniel Vetter wrote: > > Design is similar to the lockdep annotations for workers, but with > > some twists: > > > > - We use a read-lock for the execution/worker/completion side, so that > >this explicit annotation can be more liberally sprinkled around. > >With read locks lockdep isn't going to complain if the read-side > >isn't nested the same way under all circumstances, so ABBA deadlocks > >are ok. Which they are, since this is an annotation only. > > > > - We're using non-recursive lockdep read lock mode, since in recursive > >read lock mode lockdep does not catch read side hazards. And we > >_very_ much want read side hazards to be caught. For full details of > >this limitation see > > > >commit e91498589746065e3ae95d9a00b068e525eec34f > >Author: Peter Zijlstra > >Date: Wed Aug 23 13:13:11 2017 +0200 > > > >locking/lockdep/selftests: Add mixed read-write ABBA tests > > > > - To allow nesting of the read-side explicit annotations we explicitly > >keep track of the nesting. lock_is_held() allows us to do that. > > > > - The wait-side annotation is a write lock, and entirely done within > >dma_fence_wait() for everyone by default. > > > > - To be able to freely annotate helper functions I want to make it ok > >to call dma_fence_begin/end_signalling from soft/hardirq context. > >First attempt was using the hardirq locking context for the write > >side in lockdep, but this forces all normal spinlocks nested within > >dma_fence_begin/end_signalling to be spinlocks. That bollocks. > > > >The approach now is to simple check in_atomic(), and for these cases > >entirely rely on the might_sleep() check in dma_fence_wait(). That > >will catch any wrong nesting against spinlocks from soft/hardirq > >contexts. > > > > The idea here is that every code path that's critical for eventually > > signalling a dma_fence should be annotated with > > dma_fence_begin/end_signalling. The annotation ideally starts right > > after a dma_fence is published (added to a dma_resv, exposed as a > > sync_file fd, attached to a drm_syncobj fd, or anything else that > > makes the dma_fence visible to other kernel threads), up to and > > including the dma_fence_wait(). Examples are irq handlers, the > > scheduler rt threads, the tail of execbuf (after the corresponding > > fences are visible), any workers that end up signalling dma_fences and > > really anything else. Not annotated should be code paths that only > > complete fences opportunistically as the gpu progresses, like e.g. > > shrinker/eviction code. > > > > The main class of deadlocks this is supposed to catch are: > > > > Thread A: > > > > mutex_lock(A); > > mutex_unlock(A); > > > > dma_fence_signal(); > > > > Thread B: > > > > mutex_lock(A); > > dma_fence_wait(); > > mutex_unlock(A); > > > > Thread B is blocked on A signalling the fence, but A never gets around > > to that because it cannot acquire the lock A. > > > > Note that dma_fence_wait() is allowed to be nested within > > dma_fence_begin/end_signalling sections. To allow this to happen the > > read lock needs to be upgraded to a write lock, which means that any > > other lock is acquired between the dma_fence_begin_signalling() call and > > the call to dma_fence_wait(), and still held, this will result in an > > immediate lockdep complaint. The only other option would be to not > > annotate such calls, defeating the point. Therefore these annotations > > cannot be sprinkled over the code entirely mindless to avoid false > > positives. > > > > v2: handle soft/hardirq ctx better against write side and dont forget > > EXPORT_SYMBOL, drivers can't use this otherwise. > > > > Cc: linux-me...@vger.kernel.org > > Cc: linaro-mm-...@lists.linaro.org > > Cc: linux-r...@vger.kernel.org > > Cc: amd-...@lists.freedesktop.org > > Cc: intel-...@lists.freedesktop.org > > Cc: Chris Wilson > > Cc: Maarten Lankhorst > > Cc: Christian König > > Signed-off-by: Daniel Vetter > > LGTM. Perhaps some in-code documentation on how to use the new functions > are called. See cover letter, that's going to be done for next round. For this one here I just wanted to showcase a bit how it's used in a few different places, mostly selected to get as much feedback from across different drivers. Hence e.g. annotating drm/scheduler. > Otherwise for patch 2 and 3, > > Reviewed-by: Thomas Hellstrom I think I'll just cc you for the next round with docs, so you can make sure it looks ok :-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 206987] [drm] [amdgpu] Whole system crashes when the driver is in mode_support_and_system_configuration
https://bugzilla.kernel.org/show_bug.cgi?id=206987 Petteri Aimonen (j...@kernelbug.mail.kapsi.fi) changed: What|Removed |Added CC||j...@kernelbug.mail.kapsi.fi --- Comment #16 from Petteri Aimonen (j...@kernelbug.mail.kapsi.fi) --- I hit the same issue, using Ubuntu 20.04. It happened when switching window to Firefox. For me it only crashed Xorg, ssh to the machine still worked ok. Killing Xorg didn't work and `shutdown -r now` hung up somewhere. Here is a bug report on the Ubuntu package: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1881134 Here is call trace decoded with the debug symbols: -- [455834.385061] Call Trace: [455834.385120] mode_support_and_system_configuration (/build/linux-FFoizL/linux-5.4.0/drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calc_auto.c:176) amdgpu [455834.385174] ? calculate_inits_and_adj_vp (/build/linux-FFoizL/linux-5.4.0/drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_resource.c:950 (discriminator 12)) amdgpu [455834.385230] dcn_validate_bandwidth (/build/linux-FFoizL/linux-5.4.0/drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:1034) amdgpu [455834.385283] dc_validate_global_state (/build/linux-FFoizL/linux-5.4.0/drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_resource.c:2093) amdgpu [455834.385338] amdgpu_dm_atomic_check (/build/linux-FFoizL/linux-5.4.0/drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:7413) amdgpu [455834.385351] drm_atomic_check_only (/build/linux-FFoizL/linux-5.4.0/drivers/gpu/drm/drm_atomic.c:1179) drm [455834.385361] drm_atomic_commit (/build/linux-FFoizL/linux-5.4.0/drivers/gpu/drm/drm_atomic.c:1220) drm [455834.385370] drm_mode_obj_set_property_ioctl (/build/linux-FFoizL/linux-5.4.0/drivers/gpu/drm/drm_mode_object.c:496 /build/linux-FFoizL/linux-5.4.0/drivers/gpu/drm/drm_mode_object.c:533) drm [455834.385379] ? drm_mode_obj_find_prop_id (/build/linux-FFoizL/linux-5.4.0/drivers/gpu/drm/drm_mode_object.c:512) drm [455834.385386] drm_ioctl_kernel (/build/linux-FFoizL/linux-5.4.0/drivers/gpu/drm/drm_ioctl.c:793) drm [455834.385394] drm_ioctl (/build/linux-FFoizL/linux-5.4.0/include/linux/thread_info.h:119 /build/linux-FFoizL/linux-5.4.0/include/linux/thread_info.h:152 /build/linux-FFoizL/linux-5.4.0/include/linux/uaccess.h:151 /build/linux-FFoizL/linux-5.4.0/drivers/gpu/drm/drm_ioctl.c:888) drm [455834.385402] ? drm_mode_obj_find_prop_id (/build/linux-FFoizL/linux-5.4.0/drivers/gpu/drm/drm_mode_object.c:512) drm [455834.385406] ? recalc_sigpending (/build/linux-FFoizL/linux-5.4.0/kernel/signal.c:184) [455834.385440] amdgpu_drm_ioctl (/build/linux-FFoizL/linux-5.4.0/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c:1293) amdgpu [455834.385443] do_vfs_ioctl (/build/linux-FFoizL/linux-5.4.0/fs/ioctl.c:47 /build/linux-FFoizL/linux-5.4.0/fs/ioctl.c:510 /build/linux-FFoizL/linux-5.4.0/fs/ioctl.c:697) [455834.385444] ? recalc_sigpending (/build/linux-FFoizL/linux-5.4.0/kernel/signal.c:184) [455834.385446] ? _copy_from_user (/build/linux-FFoizL/linux-5.4.0/arch/x86/include/asm/uaccess_64.h:46 /build/linux-FFoizL/linux-5.4.0/arch/x86/include/asm/uaccess_64.h:71 /build/linux-FFoizL/linux-5.4.0/lib/usercopy.c:14) [455834.385448] ksys_ioctl (/build/linux-FFoizL/linux-5.4.0/include/linux/file.h:43 /build/linux-FFoizL/linux-5.4.0/fs/ioctl.c:715) [455834.385449] __x64_sys_ioctl (/build/linux-FFoizL/linux-5.4.0/fs/ioctl.c:719) [455834.385451] do_syscall_64 (/build/linux-FFoizL/linux-5.4.0/arch/x86/entry/common.c:290) [455834.385455] entry_SYSCALL_64_after_hwframe (/build/linux-FFoizL/linux-5.4.0/arch/x86/entry/entry_64.S:184) [455834.385456] RIP: 0033:0x7faf3181837b -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 03/10] drm/client: Add drm_client_framebuffer_flush()
Hi all,
I realise this has landed, so a small FYI comment.
On Sat, 9 May 2020 at 15:16, Noralf Trønnes wrote:
> +int drm_client_framebuffer_flush(struct drm_client_buffer *buffer, struct
> drm_rect *rect)
> +{
> + if (!buffer || !buffer->fb || !buffer->fb->funcs->dirty)
Hmm cannot think of a good reason why anyone would call this with
!buffer || !buffer->fb?
Might be a good idea to WARN in those cases - otherwise the user is
given false sense to security.
Looking at the upcoming user (drm/gud) it already has both - so it's
perfectly safe.
-Emil
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC 02/17] dma-fence: basic lockdep annotations
On 2020-05-12 10:59, Daniel Vetter wrote: Design is similar to the lockdep annotations for workers, but with some twists: - We use a read-lock for the execution/worker/completion side, so that this explicit annotation can be more liberally sprinkled around. With read locks lockdep isn't going to complain if the read-side isn't nested the same way under all circumstances, so ABBA deadlocks are ok. Which they are, since this is an annotation only. - We're using non-recursive lockdep read lock mode, since in recursive read lock mode lockdep does not catch read side hazards. And we _very_ much want read side hazards to be caught. For full details of this limitation see commit e91498589746065e3ae95d9a00b068e525eec34f Author: Peter Zijlstra Date: Wed Aug 23 13:13:11 2017 +0200 locking/lockdep/selftests: Add mixed read-write ABBA tests - To allow nesting of the read-side explicit annotations we explicitly keep track of the nesting. lock_is_held() allows us to do that. - The wait-side annotation is a write lock, and entirely done within dma_fence_wait() for everyone by default. - To be able to freely annotate helper functions I want to make it ok to call dma_fence_begin/end_signalling from soft/hardirq context. First attempt was using the hardirq locking context for the write side in lockdep, but this forces all normal spinlocks nested within dma_fence_begin/end_signalling to be spinlocks. That bollocks. The approach now is to simple check in_atomic(), and for these cases entirely rely on the might_sleep() check in dma_fence_wait(). That will catch any wrong nesting against spinlocks from soft/hardirq contexts. The idea here is that every code path that's critical for eventually signalling a dma_fence should be annotated with dma_fence_begin/end_signalling. The annotation ideally starts right after a dma_fence is published (added to a dma_resv, exposed as a sync_file fd, attached to a drm_syncobj fd, or anything else that makes the dma_fence visible to other kernel threads), up to and including the dma_fence_wait(). Examples are irq handlers, the scheduler rt threads, the tail of execbuf (after the corresponding fences are visible), any workers that end up signalling dma_fences and really anything else. Not annotated should be code paths that only complete fences opportunistically as the gpu progresses, like e.g. shrinker/eviction code. The main class of deadlocks this is supposed to catch are: Thread A: mutex_lock(A); mutex_unlock(A); dma_fence_signal(); Thread B: mutex_lock(A); dma_fence_wait(); mutex_unlock(A); Thread B is blocked on A signalling the fence, but A never gets around to that because it cannot acquire the lock A. Note that dma_fence_wait() is allowed to be nested within dma_fence_begin/end_signalling sections. To allow this to happen the read lock needs to be upgraded to a write lock, which means that any other lock is acquired between the dma_fence_begin_signalling() call and the call to dma_fence_wait(), and still held, this will result in an immediate lockdep complaint. The only other option would be to not annotate such calls, defeating the point. Therefore these annotations cannot be sprinkled over the code entirely mindless to avoid false positives. v2: handle soft/hardirq ctx better against write side and dont forget EXPORT_SYMBOL, drivers can't use this otherwise. Cc: linux-me...@vger.kernel.org Cc: linaro-mm-...@lists.linaro.org Cc: linux-r...@vger.kernel.org Cc: amd-...@lists.freedesktop.org Cc: intel-...@lists.freedesktop.org Cc: Chris Wilson Cc: Maarten Lankhorst Cc: Christian König Signed-off-by: Daniel Vetter LGTM. Perhaps some in-code documentation on how to use the new functions are called. Otherwise for patch 2 and 3, Reviewed-by: Thomas Hellstrom ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH i-g-t] panfrost: Test labeling functionality
Introduce tests to cover the new generic labeling ioctl's
being reviewed here [1].
Signed-off-by: Rohan Garg
[1] https://patchwork.freedesktop.org/series/77267/
Signed-off-by: Rohan Garg
---
include/drm-uapi/drm.h| 23 ++-
tests/meson.build | 1 +
tests/panfrost_bo_label.c | 129 ++
3 files changed, 152 insertions(+), 1 deletion(-)
create mode 100644 tests/panfrost_bo_label.c
diff --git a/include/drm-uapi/drm.h b/include/drm-uapi/drm.h
index c7fd2a35..87124882 100644
--- a/include/drm-uapi/drm.h
+++ b/include/drm-uapi/drm.h
@@ -620,6 +620,25 @@ struct drm_gem_open {
__u64 size;
};
+/** struct drm_handle_label - ioctl argument for labelling BOs.
+ *
+ * This label's a BO with a userspace label
+ *
+ */
+struct drm_handle_label {
+ /** Handle for the object being labelled. */
+ __u32 handle;
+
+ /** Label and label length (len includes the trailing NULL).
+* Label lenght *MUST* be smaller than PAGE_SIZE.
+*/
+ __u32 len;
+ __u64 label;
+
+ /** Flags .. Currently no flags are defined */
+ __u32 flags;
+};
+
#define DRM_CAP_DUMB_BUFFER0x1
#define DRM_CAP_VBLANK_HIGH_CRTC 0x2
#define DRM_CAP_DUMB_PREFERRED_DEPTH 0x3
@@ -941,8 +960,10 @@ extern "C" {
#define DRM_IOCTL_SYNCOBJ_QUERYDRM_IOWR(0xCB, struct
drm_syncobj_timeline_array)
#define DRM_IOCTL_SYNCOBJ_TRANSFER DRM_IOWR(0xCC, struct
drm_syncobj_transfer)
#define DRM_IOCTL_SYNCOBJ_TIMELINE_SIGNAL DRM_IOWR(0xCD, struct
drm_syncobj_timeline_array)
-
#define DRM_IOCTL_MODE_GETFB2 DRM_IOWR(0xCE, struct drm_mode_fb_cmd2)
+#define DRM_IOCTL_HANDLE_SET_LABEL DRM_IOWR(0xCF, struct drm_handle_label)
+#define DRM_IOCTL_HANDLE_GET_LABEL DRM_IOWR(0xD0, struct drm_handle_label)
+
/**
* Device specific ioctls should only be in their respective headers
diff --git a/tests/meson.build b/tests/meson.build
index e69bdb7d..ee91bf48 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -72,6 +72,7 @@ test_progs = [
'kms_vblank',
'kms_vrr',
'meta_test',
+'panfrost_bo_label',
'panfrost_get_param',
'panfrost_gem_new',
'panfrost_prime',
diff --git a/tests/panfrost_bo_label.c b/tests/panfrost_bo_label.c
new file mode 100644
index ..d3179458
--- /dev/null
+++ b/tests/panfrost_bo_label.c
@@ -0,0 +1,129 @@
+/*
+ * Copyright © 2020 Collabora Ltd.
+ * Author: Rohan Garg
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#include "igt.h"
+#include "igt_panfrost.h"
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include "vc4_drm.h"
+
+static int
+set_label(int fd, int handle, const char *label)
+{
+ struct drm_handle_label args = { 0 };
+ if (label) {
+ args.handle = handle;
+ args.len = strlen(label) + 1;
+ args.label= (uintptr_t)label;
+ }
+
+ return drmIoctl(fd, DRM_IOCTL_HANDLE_SET_LABEL, &args);
+}
+
+static const char*
+get_label(int fd, int handle)
+{
+ struct drm_handle_label args = {
+.handle = handle,
+ .len = 10,
+ .label = (uintptr_t) malloc(sizeof(char*) * 10)
+};
+
+ drmIoctl(fd, DRM_IOCTL_HANDLE_GET_LABEL, &args);
+
+ if (!args.label)
+ return NULL;
+
+ igt_assert(args.label);
+
+args.label = (uintptr_t) realloc(args.label, args.len);
+
+ drmIoctl(fd, DRM_IOCTL_HANDLE_GET_LABEL, &args);
+
+return args.label;
+}
+
+
+igt_main
+{
+ int fd;
+
+ igt_fixture
+ fd = drm_open_driver(DRIVER_ANY);
+
+ igt_subtest("set-label") {
+ struct panfrost_bo *bo = igt_panfrost_gem_new(fd, 4096);
+ int ret;
+ ret = set_label(fd, bo->handle, "a
Re: [PATCH 08/15] drm/panfrost: move devfreq_init()/fini() in device
On 10/05/2020 17:55, Clément Péron wrote:
Later we will introduce devfreq probing regulator if they
are present. As regulator should be probe only one time we
need to get this logic in the device_init().
panfrost_device is already taking care of devfreq_resume()
and devfreq_suspend(), so it's not totally illogic to move
the devfreq_init() and devfreq_fini() here.
Signed-off-by: Clément Péron
---
drivers/gpu/drm/panfrost/panfrost_device.c | 37 ++
drivers/gpu/drm/panfrost/panfrost_drv.c| 15 ++---
2 files changed, 25 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c
b/drivers/gpu/drm/panfrost/panfrost_device.c
index 8136babd3ba9..f480127205d6 100644
--- a/drivers/gpu/drm/panfrost/panfrost_device.c
+++ b/drivers/gpu/drm/panfrost/panfrost_device.c
@@ -212,59 +212,67 @@ int panfrost_device_init(struct panfrost_device *pfdev)
return err;
}
+ err = panfrost_devfreq_init(pfdev);
+ if (err) {
+ dev_err(pfdev->dev, "devfreq init failed %d\n", err);
+ goto err_out0;
+ }
+
err = panfrost_regulator_init(pfdev);
if (err) {
dev_err(pfdev->dev, "regulator init failed %d\n", err);
- goto err_out0;
+ goto err_out1;
NIT: Rather than just renumbering these can we give them sensible names
so we don't have this sort of refactoring in future?
}
err = panfrost_reset_init(pfdev);
if (err) {
dev_err(pfdev->dev, "reset init failed %d\n", err);
- goto err_out1;
+ goto err_out2;
}
err = panfrost_pm_domain_init(pfdev);
if (err)
- goto err_out2;
+ goto err_out3;
res = platform_get_resource(pfdev->pdev, IORESOURCE_MEM, 0);
pfdev->iomem = devm_ioremap_resource(pfdev->dev, res);
if (IS_ERR(pfdev->iomem)) {
dev_err(pfdev->dev, "failed to ioremap iomem\n");
err = PTR_ERR(pfdev->iomem);
- goto err_out3;
+ goto err_out4;
}
err = panfrost_gpu_init(pfdev);
if (err)
- goto err_out3;
+ goto err_out4;
err = panfrost_mmu_init(pfdev);
if (err)
- goto err_out4;
+ goto err_out5;
err = panfrost_job_init(pfdev);
if (err)
- goto err_out5;
+ goto err_out6;
err = panfrost_perfcnt_init(pfdev);
if (err)
- goto err_out6;
+ goto err_out7;
return 0;
-err_out6:
+err_out7:
panfrost_job_fini(pfdev);
-err_out5:
+err_out6:
panfrost_mmu_fini(pfdev);
-err_out4:
+err_out5:
panfrost_gpu_fini(pfdev);
-err_out3:
+err_out4:
panfrost_pm_domain_fini(pfdev);
-err_out2:
+err_out3:
panfrost_reset_fini(pfdev);
-err_out1:
+err_out2:
panfrost_regulator_fini(pfdev);
+err_out1:
+ panfrost_devfreq_fini(pfdev);
err_out0:
panfrost_clk_fini(pfdev);
return err;
@@ -278,6 +286,7 @@ void panfrost_device_fini(struct panfrost_device *pfdev)
panfrost_gpu_fini(pfdev);
panfrost_pm_domain_fini(pfdev);
panfrost_reset_fini(pfdev);
+ panfrost_devfreq_fini(pfdev);
panfrost_regulator_fini(pfdev);
panfrost_clk_fini(pfdev);
}
diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c
b/drivers/gpu/drm/panfrost/panfrost_drv.c
index 882fecc33fdb..4dda68689015 100644
--- a/drivers/gpu/drm/panfrost/panfrost_drv.c
+++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
@@ -14,7 +14,6 @@
#include
#include "panfrost_device.h"
-#include "panfrost_devfreq.h"
#include "panfrost_gem.h"
#include "panfrost_mmu.h"
#include "panfrost_job.h"
@@ -606,13 +605,6 @@ static int panfrost_probe(struct platform_device *pdev)
goto err_out0;
}
- err = panfrost_devfreq_init(pfdev);
- if (err) {
- if (err != -EPROBE_DEFER)
- dev_err(&pdev->dev, "Fatal error during devfreq
init\n");
You seem to have lost the check for EPROBE_DEFER during the move.
- goto err_out1;
- }
-
pm_runtime_set_active(pfdev->dev);
pm_runtime_mark_last_busy(pfdev->dev);
pm_runtime_enable(pfdev->dev);
@@ -625,16 +617,14 @@ static int panfrost_probe(struct platform_device *pdev)
*/
err = drm_dev_register(ddev, 0);
if (err < 0)
- goto err_out2;
+ goto err_out1;
panfrost_gem_shrinker_init(ddev);
return 0;
-err_out2:
- pm_runtime_disable(pfdev->dev);
- panfrost_devfreq_fini(pfdev);
err_out1:
+ pm_runtime_disable(pfdev->dev);
panfrost_device_fini(pfdev);
err_out0:
drm_dev_put(ddev);
@@ -650,7 +640,6 @@ static int panfrost_remove(struct platform_device *pdev)
panfrost_gem_shrinker_cleanup(ddev);
pm_runtime_get_sync(pfdev->dev
Re: [PATCH 06/15] drm/panfrost: properly handle error in probe
On 10/05/2020 17:55, Clément Péron wrote:
Introduce a boolean to know if opp table has been added.
With this, we can call panfrost_devfreq_fini() in case of error
and release what has been initialised.
Signed-off-by: Clément Péron
LGTM:
Reviewed-by: Steven Price
---
drivers/gpu/drm/panfrost/panfrost_devfreq.c | 25 -
drivers/gpu/drm/panfrost/panfrost_devfreq.h | 1 +
2 files changed, 20 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
index 78753cfb59fb..d9007f44b772 100644
--- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
+++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
@@ -101,6 +101,7 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev)
return 0;
else if (ret)
return ret;
+ pfdevfreq->opp_of_table_added = true;
spin_lock_init(&pfdevfreq->lock);
@@ -109,8 +110,10 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev)
cur_freq = clk_get_rate(pfdev->clock);
opp = devfreq_recommended_opp(dev, &cur_freq, 0);
- if (IS_ERR(opp))
- return PTR_ERR(opp);
+ if (IS_ERR(opp)) {
+ ret = PTR_ERR(opp);
+ goto err_fini;
+ }
panfrost_devfreq_profile.initial_freq = cur_freq;
dev_pm_opp_put(opp);
@@ -119,8 +122,8 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev)
DEVFREQ_GOV_SIMPLE_ONDEMAND, NULL);
if (IS_ERR(devfreq)) {
DRM_DEV_ERROR(dev, "Couldn't initialize GPU devfreq\n");
- dev_pm_opp_of_remove_table(dev);
- return PTR_ERR(devfreq);
+ ret = PTR_ERR(devfreq);
+ goto err_fini;
}
pfdevfreq->devfreq = devfreq;
@@ -131,15 +134,25 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev)
pfdevfreq->cooling = cooling;
return 0;
+
+err_fini:
+ panfrost_devfreq_fini(pfdev);
+ return ret;
}
void panfrost_devfreq_fini(struct panfrost_device *pfdev)
{
struct panfrost_devfreq *pfdevfreq = &pfdev->pfdevfreq;
- if (pfdevfreq->cooling)
+ if (pfdevfreq->cooling) {
devfreq_cooling_unregister(pfdevfreq->cooling);
- dev_pm_opp_of_remove_table(&pfdev->pdev->dev);
+ pfdevfreq->cooling = NULL;
+ }
+
+ if (pfdevfreq->opp_of_table_added) {
+ dev_pm_opp_of_remove_table(&pfdev->pdev->dev);
+ pfdevfreq->opp_of_table_added = false;
+ }
}
void panfrost_devfreq_resume(struct panfrost_device *pfdev)
diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.h
b/drivers/gpu/drm/panfrost/panfrost_devfreq.h
index e6629900a618..add203cb00c2 100644
--- a/drivers/gpu/drm/panfrost/panfrost_devfreq.h
+++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.h
@@ -15,6 +15,7 @@ struct panfrost_device;
struct panfrost_devfreq {
struct devfreq *devfreq;
struct thermal_cooling_device *cooling;
+ bool opp_of_table_added;
ktime_t busy_time;
ktime_t idle_time;
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 10/15] drm/panfrost: add regulators to devfreq
On 10/05/2020 17:55, Clément Péron wrote:
Some OPP tables specify voltage for each frequency. Devfreq can
handle these regulators but they should be get only 1 time to avoid
issue and know who is in charge.
If OPP table is probe don't init regulator.
Signed-off-by: Clément Péron
This looks like it should work - thanks for doing this!
Reviewed-by: Steven Price
---
drivers/gpu/drm/panfrost/panfrost_devfreq.c | 19 +++
drivers/gpu/drm/panfrost/panfrost_devfreq.h | 2 ++
drivers/gpu/drm/panfrost/panfrost_device.c | 11 +++
3 files changed, 28 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
index fce21c682414..9ffea0d4a087 100644
--- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
+++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
@@ -93,6 +93,7 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev)
unsigned long cur_freq;
struct device *dev = &pfdev->pdev->dev;
struct devfreq *devfreq;
+ struct opp_table *opp_table;
struct thermal_cooling_device *cooling;
struct panfrost_devfreq *pfdevfreq = &pfdev->pfdevfreq;
@@ -102,6 +103,19 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev)
spin_lock_init(&pfdevfreq->lock);
+ opp_table = dev_pm_opp_set_regulators(dev, pfdev->comp->supply_names,
+ pfdev->comp->num_supplies);
+ if (IS_ERR(opp_table)) {
+ ret = PTR_ERR(opp_table);
+ /* Continue if the optional regulator is missing */
+ if (ret != -ENODEV) {
+ DRM_DEV_ERROR(dev, "Couldn't set OPP regulators\n");
+ goto err_fini;
+ }
+ } else {
+ pfdevfreq->regulators_opp_table = opp_table;
+ }
+
ret = dev_pm_opp_of_add_table(dev);
if (ret) {
DRM_DEV_ERROR(dev, "Couldn't add OPP table\n");
@@ -157,6 +171,11 @@ void panfrost_devfreq_fini(struct panfrost_device *pfdev)
dev_pm_opp_of_remove_table(&pfdev->pdev->dev);
pfdevfreq->opp_of_table_added = false;
}
+
+ if (pfdevfreq->regulators_opp_table) {
+ dev_pm_opp_put_regulators(pfdevfreq->regulators_opp_table);
+ pfdevfreq->regulators_opp_table = NULL;
+ }
}
void panfrost_devfreq_resume(struct panfrost_device *pfdev)
diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.h
b/drivers/gpu/drm/panfrost/panfrost_devfreq.h
index add203cb00c2..347cde4786cf 100644
--- a/drivers/gpu/drm/panfrost/panfrost_devfreq.h
+++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.h
@@ -8,12 +8,14 @@
#include
struct devfreq;
+struct opp_table;
struct thermal_cooling_device;
struct panfrost_device;
struct panfrost_devfreq {
struct devfreq *devfreq;
+ struct opp_table *regulators_opp_table;
struct thermal_cooling_device *cooling;
bool opp_of_table_added;
diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c b/drivers/gpu/drm/panfrost/panfrost_device.c
index 67eedf64e82d..8b17fb2e3369 100644
--- a/drivers/gpu/drm/panfrost/panfrost_device.c
+++ b/drivers/gpu/drm/panfrost/panfrost_device.c
@@ -222,10 +222,13 @@ int panfrost_device_init(struct panfrost_device *pfdev)
goto err_out0;
}
- err = panfrost_regulator_init(pfdev);
- if (err) {
- dev_err(pfdev->dev, "regulator init failed %d\n", err);
- goto err_out1;
+ /* OPP will handle regulators */
+ if (!pfdev->pfdevfreq.opp_of_table_added) {
+ err = panfrost_regulator_init(pfdev);
+ if (err) {
+ dev_err(pfdev->dev, "regulator init failed %d\n", err);
+ goto err_out1;
+ }
}
err = panfrost_reset_init(pfdev);
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 09/15] drm/panfrost: dynamically alloc regulators
On 10/05/2020 17:55, Clément Péron wrote:
We will later introduce regulators managed by OPP.
Only alloc regulators when it's needed. This also help use
to release the regulators only when they are allocated.
Signed-off-by: Clément Péron
LGTM:
Reviewed-by: Steven Price
---
drivers/gpu/drm/panfrost/panfrost_device.c | 14 +-
drivers/gpu/drm/panfrost/panfrost_device.h | 3 +--
2 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c
b/drivers/gpu/drm/panfrost/panfrost_device.c
index f480127205d6..67eedf64e82d 100644
--- a/drivers/gpu/drm/panfrost/panfrost_device.c
+++ b/drivers/gpu/drm/panfrost/panfrost_device.c
@@ -90,9 +90,11 @@ static int panfrost_regulator_init(struct panfrost_device
*pfdev)
{
int ret, i;
- if (WARN(pfdev->comp->num_supplies > ARRAY_SIZE(pfdev->regulators),
- "Too many supplies in compatible structure.\n"))
- return -EINVAL;
+ pfdev->regulators = devm_kcalloc(pfdev->dev, pfdev->comp->num_supplies,
+sizeof(*pfdev->regulators),
+GFP_KERNEL);
+ if (!pfdev->regulators)
+ return -ENOMEM;
for (i = 0; i < pfdev->comp->num_supplies; i++)
pfdev->regulators[i].supply = pfdev->comp->supply_names[i];
@@ -117,8 +119,10 @@ static int panfrost_regulator_init(struct panfrost_device
*pfdev)
static void panfrost_regulator_fini(struct panfrost_device *pfdev)
{
- regulator_bulk_disable(pfdev->comp->num_supplies,
- pfdev->regulators);
+ if (!pfdev->regulators)
+ return;
+
+ regulator_bulk_disable(pfdev->comp->num_supplies, pfdev->regulators);
}
static void panfrost_pm_domain_fini(struct panfrost_device *pfdev)
diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h
b/drivers/gpu/drm/panfrost/panfrost_device.h
index 2efa59c9d1c5..953f7536a773 100644
--- a/drivers/gpu/drm/panfrost/panfrost_device.h
+++ b/drivers/gpu/drm/panfrost/panfrost_device.h
@@ -22,7 +22,6 @@ struct panfrost_job;
struct panfrost_perfcnt;
#define NUM_JOB_SLOTS 3
-#define MAX_REGULATORS 2
#define MAX_PM_DOMAINS 3
struct panfrost_features {
@@ -81,7 +80,7 @@ struct panfrost_device {
void __iomem *iomem;
struct clk *clock;
struct clk *bus_clock;
- struct regulator_bulk_data regulators[MAX_REGULATORS];
+ struct regulator_bulk_data *regulators;
struct reset_control *rstc;
/* pm_domains for devices with more than one. */
struct device *pm_domain_devs[MAX_PM_DOMAINS];
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 07/15] drm/panfrost: use device_property_present to check for OPP
On 10/05/2020 17:55, Clément Péron wrote:
Instead of expecting an error from dev_pm_opp_of_add_table()
do a simple device_property_present() check.
Signed-off-by: Clément Péron
I'm not sure I understand why this is better. We seem to have more code
to do roughly the same thing just with the hard-coded
"operating-points-v2" name (if there's ever a 'v3' we'll then have to
update this).
Is the desire just to get an error on probe if the table is malformed?
Have you hit this situation? If so this sounds like something which
would be better fixed in the generic OPP code rather than Panfrost itself.
Steve
---
drivers/gpu/drm/panfrost/panfrost_devfreq.c | 14 +-
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
index d9007f44b772..fce21c682414 100644
--- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
+++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
@@ -96,15 +96,19 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev)
struct thermal_cooling_device *cooling;
struct panfrost_devfreq *pfdevfreq = &pfdev->pfdevfreq;
- ret = dev_pm_opp_of_add_table(dev);
- if (ret == -ENODEV) /* Optional, continue without devfreq */
+ if (!device_property_present(dev, "operating-points-v2"))
+ /* Optional, continue without devfreq */
return 0;
- else if (ret)
- return ret;
- pfdevfreq->opp_of_table_added = true;
spin_lock_init(&pfdevfreq->lock);
+ ret = dev_pm_opp_of_add_table(dev);
+ if (ret) {
+ DRM_DEV_ERROR(dev, "Couldn't add OPP table\n");
+ goto err_fini;
+ }
+ pfdevfreq->opp_of_table_added = true;
+
panfrost_devfreq_reset(pfdevfreq);
cur_freq = clk_get_rate(pfdev->clock);
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 11/15] drm/panfrost: set devfreq clock name
On 10/05/2020 17:55, Clément Péron wrote:
Some SoCs have several clocks defined and the OPP core
needs to know the exact name of the clk to use.
Set the clock name to "core".
Signed-off-by: Clément Péron
This is unfortunately a regression for the RK3288. The device tree
binding doesn't require "clock-names", and for the RK3288 it currently
isn't specified. So this breaks the platform.
Adding the "clock-names" to the device tree 'fixes' it, but we really
need to keep backwards compatibility.
Steve
---
drivers/gpu/drm/panfrost/panfrost_devfreq.c | 13 +
drivers/gpu/drm/panfrost/panfrost_devfreq.h | 1 +
2 files changed, 14 insertions(+)
diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
index 9ffea0d4a087..6bf3541b4d53 100644
--- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
+++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
@@ -103,6 +103,14 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev)
spin_lock_init(&pfdevfreq->lock);
+ opp_table = dev_pm_opp_set_clkname(dev, "core");
+ if (IS_ERR(opp_table)) {
+ ret = PTR_ERR(opp_table);
+ goto err_fini;
+ }
+
+ pfdevfreq->clkname_opp_table = opp_table;
+
opp_table = dev_pm_opp_set_regulators(dev, pfdev->comp->supply_names,
pfdev->comp->num_supplies);
if (IS_ERR(opp_table)) {
@@ -176,6 +184,11 @@ void panfrost_devfreq_fini(struct panfrost_device *pfdev)
dev_pm_opp_put_regulators(pfdevfreq->regulators_opp_table);
pfdevfreq->regulators_opp_table = NULL;
}
+
+ if (pfdevfreq->clkname_opp_table) {
+ dev_pm_opp_put_clkname(pfdevfreq->clkname_opp_table);
+ pfdevfreq->clkname_opp_table = NULL;
+ }
}
void panfrost_devfreq_resume(struct panfrost_device *pfdev)
diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.h
b/drivers/gpu/drm/panfrost/panfrost_devfreq.h
index 347cde4786cf..1f2475e1d034 100644
--- a/drivers/gpu/drm/panfrost/panfrost_devfreq.h
+++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.h
@@ -16,6 +16,7 @@ struct panfrost_device;
struct panfrost_devfreq {
struct devfreq *devfreq;
struct opp_table *regulators_opp_table;
+ struct opp_table *clkname_opp_table;
struct thermal_cooling_device *cooling;
bool opp_of_table_added;
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 03/15] drm/panfrost: don't use pfdevfreq.busy_count to know if hw is idle
On 10/05/2020 17:55, Clément Péron wrote:
This use devfreq variable that will be lock with spinlock in future
patches. We should either introduce a function to access this one
but as devfreq is optional let's just remove it.
Signed-off-by: Clément Péron
As far as I can tell this should be safe. As you note this wouldn't work
without devfreq anyway.
Reviewed-by: Steven Price
---
drivers/gpu/drm/panfrost/panfrost_job.c | 4
1 file changed, 4 deletions(-)
diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c
b/drivers/gpu/drm/panfrost/panfrost_job.c
index 7914b1570841..63e32a9f2749 100644
--- a/drivers/gpu/drm/panfrost/panfrost_job.c
+++ b/drivers/gpu/drm/panfrost/panfrost_job.c
@@ -581,10 +581,6 @@ int panfrost_job_is_idle(struct panfrost_device *pfdev)
struct panfrost_job_slot *js = pfdev->js;
int i;
- /* Check whether the hardware is idle */
- if (atomic_read(&pfdev->devfreq.busy_count))
- return false;
-
for (i = 0; i < NUM_JOB_SLOTS; i++) {
/* If there are any jobs in the HW queue, we're not idle */
if (atomic_read(&js->queue[i].sched.hw_rq_count))
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
