[Bug 100426] Trine 3 takes long time to load

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=100426

--- Comment #8 from Timothy Arceri  ---
The radeonsi backend already does this where possible. The glsl compiler front
end is not currently thread safe, there have been a couple of attempts at this
over the years but I don't think anyone is working on this currently.

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

[Bug 100426] Trine 3 takes long time to load

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=100426

--- Comment #7 from Shmerl  ---
(In reply to Timothy Arceri from comment #6)
> its well know that compile times are not
> as good as we would like in Mesa, but this is an ongoing process.

Is there a plan to make compilation more parallelized when the processor allows
it?

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

[Bug 100426] Trine 3 takes long time to load

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=100426

Timothy Arceri  changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |NOTABUG

--- Comment #6 from Timothy Arceri  ---
Thanks for the trace. However taking a look, there is nothing here that is
taking an abnormal amount of cpu, its well know that compile times are not as
good as we would like in Mesa, but this is an ongoing process.

I don't see any need to keep this bug open.

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

[Bug 100426] Trine 3 takes long time to load

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=100426

--- Comment #5 from Shmerl  ---
Sorry, wrong description in the second attachment. It should say: apitrace of
Trine 3 (filled mesa cache).

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

[Bug 100426] Trine 3 takes long time to load

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=100426

--- Comment #4 from Shmerl  ---
Created attachment 13
  --> https://bugs.freedesktop.org/attachment.cgi?id=13=edit
apitrace of Trine 3 (empty mesa cache)

A trace with already filled mesa cache, Mesa 17.1.0-devel (git-af73acca2b). I
interrupted it a couple of seconds before it hit 25 seconds.

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

[Bug 100426] Trine 3 takes long time to load

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=100426

--- Comment #3 from Shmerl  ---
Just for the reference, this is GOG, not Steam version, I don't know if they
differ.

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

[Bug 100426] Trine 3 takes long time to load

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=100426

--- Comment #2 from Shmerl  ---
Created attachment 131110
  --> https://bugs.freedesktop.org/attachment.cgi?id=131110=edit
apitrace of Trine 3 (empty mesa cache)

This is a trace with empty mesa cache, Mesa 17.1.0-devel (git-af73acca2b). I
interrupted it right before it hit 50 seconds, in order to prevent it from
bloating up.

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

[Bug 100426] Trine 3 takes long time to load

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=100426

Timothy Arceri  changed:

   What|Removed |Added

 CC||t_arc...@yahoo.com.au

--- Comment #1 from Timothy Arceri  ---
25 second compile time is not all that bad. This far from the worse example of
compile times in Mesa.

I'm tempted to close this bug as I'm not convinced there is anything special
about Trine 3 here. Are you able to use the perf tool to see where in the
compiler we are spending lots of time? Or at the very least grab an apitrace
[1] of the game at startup and upload it somewhere so that I can use this to
take a quick look.


[1] https://github.com/apitrace/apitrace/wiki/Steam

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

Re: [Intel-gfx] [PATCH v7 4/4] drm/dp: Track MST link bandwidth

[Pandiyan, Dhinakaran]

On Tue, 2017-04-25 at 09:51 +0200, Maarten Lankhorst wrote:
> On 21-04-17 07:51, Dhinakaran Pandiyan wrote:
> > From: "Pandiyan, Dhinakaran" 
> >
> > Use the added helpers to track MST link bandwidth for atomic modesets.
> > Link bw is acquired in the ->atomic_check() phase when CRTCs are being
> > enabled with drm_atomic_find_vcpi_slots() instead of drm_find_vcpi_slots().
> > Similarly, link bw is released during ->atomic_check() with the connector
> > helper callback ->atomic_release() when CRTCs are disabled.
> >
> > v5: Implement connector->atomic_check() in place of atomic_release()
> > v4: Return an int from intel_dp_mst_atomic_release() (Maarten)
> > v3:
> >  Handled the case where ->atomic_release() is called more than once
> >  during an atomic_check() for the same state.
> > v2:
> >  Squashed atomic_release() implementation and caller (Daniel)
> >  Fixed logic for connector-crtc switching case (Daniel)
> >  Fixed logic for suspend-resume case.
> >
> > Cc: Daniel Vetter 
> > Cc: Maarten Lankhorst 
> > Cc: Archit Taneja 
> > Cc: Chris Wilson 
> > Cc: Harry Wentland 
> > Signed-off-by: Dhinakaran Pandiyan 
> > ---
> >  drivers/gpu/drm/i915/intel_dp_mst.c | 57 
> > +
> >  1 file changed, 51 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c 
> > b/drivers/gpu/drm/i915/intel_dp_mst.c
> > index 5af22a7..20c557c 100644
> > --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> > +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> > @@ -39,9 +39,9 @@ static bool intel_dp_mst_compute_config(struct 
> > intel_encoder *encoder,
> > struct intel_dp *intel_dp = _dig_port->dp;
> > struct intel_connector *connector =
> > to_intel_connector(conn_state->connector);
> > -   struct drm_atomic_state *state;
> > +   struct drm_atomic_state *state = pipe_config->base.state;
> > int bpp;
> > -   int lane_count, slots;
> > +   int lane_count, slots = 0;
> > const struct drm_display_mode *adjusted_mode = 
> > _config->base.adjusted_mode;
> > int mst_pbn;
> >  
> > @@ -63,24 +63,68 @@ static bool intel_dp_mst_compute_config(struct 
> > intel_encoder *encoder,
> > pipe_config->pipe_bpp = bpp;
> > pipe_config->port_clock = intel_dp_max_link_rate(intel_dp);
> >  
> > -   state = pipe_config->base.state;
> > -
> > if (drm_dp_mst_port_has_audio(_dp->mst_mgr, connector->port))
> > pipe_config->has_audio = true;
> > -   mst_pbn = drm_dp_calc_pbn_mode(adjusted_mode->crtc_clock, bpp);
> >  
> > +   mst_pbn = drm_dp_calc_pbn_mode(adjusted_mode->crtc_clock, bpp);
> > pipe_config->pbn = mst_pbn;
> > -   slots = drm_dp_find_vcpi_slots(_dp->mst_mgr, mst_pbn);
> >  
> > intel_link_compute_m_n(bpp, lane_count,
> >adjusted_mode->crtc_clock,
> >pipe_config->port_clock,
> >_config->dp_m_n);
> >  
> > +   if (pipe_config->base.active) {
> Is this check needed? If so it needs a comment as to why.
> 
> I know that the output won't actually turn on, but if the crtc is configured 
> vcpi slots should be allocated if possible.
> 
> Else the following will fail in the wrong place:
> 1. Configure CRTC with a valid mode and DP-MST connector, but set 
> active=false. connectors_changed = true, mode_changed = true. Not enough VCPI 
> slots to enable all crtc's at the same time now, but this returns OK.
> 2. Commit with active property set to true. There are too few vcpi slots to 
> make this work, this atomic commit returns -EINVAL. This might happen on a 
> different crtc than the newly configured one in stap 1, if they were all 
> previously set to !active.
> 

I gave this more thought, what you are recommending is not allocating
vcpi for active false->true but instead doing it only for enable
false->true. The problem I see is encoder->compute_config() is called to
configure the link for a modeset with active=false->true transition. If
we don't want to allocate vcpi slots in this case, we should not be
doing other config. computations as well i.e., not call
encoder->compute_config() at all for active_changed. This means we
should complete this FIXME in intel_atomic_check()

 /* FIXME: For only active_changed we shouldn't need to do any
 * state recomputation at all. */ 

Otoh if you want me to remove 'if (pipe_config->base.active)', we can do
this - 

a) allocate slots unconditionally in compute_config()
intel_link_compute_m_n(bpp, lane_count,
   adjusted_mode->crtc_clock,
   pipe_config->port_clock,
   _config->dp_m_n);

+   slots = drm_dp_atomic_find_vcpi_slots(state, _dp->mst_mgr,
+ connector->port, 

Re: [PATCH v2 01/21] scatterlist: Introduce sg_map helper functions

[Logan Gunthorpe]

On 27/04/17 12:53 AM, Christoph Hellwig wrote:
> I think you'll need to follow the existing kmap semantics and never
> fail the iomem version either.  Otherwise you'll have a special case
> that's almost never used that has a different error path.
>
> Again, wrong way.  Suddenly making things fail for your special case
> that normally don't fail is a receipe for bugs.

I don't disagree but these restrictions make the problem impossible to
solve? If there is iomem behind a page in an SGL and someone tries to
map it, we either have to fail or we break iomem safety which was your
original concern.

Logan

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

Re: [PATCH v2 15/21] xen-blkfront: Make use of the new sg_map helper function

[Logan Gunthorpe]

On 27/04/17 02:53 PM, Jason Gunthorpe wrote:
> blkfront is one of the drivers I looked at, and it appears to only be
> memcpying with the bvec_data pointer, so I wonder why it does not use
> sg_copy_X_buffer instead..

Yes, sort of...

But you'd potentially end up calling sg_copy_to_buffer multiple times
per page within the sg (given that gnttab_foreach_grant_in_range might
call blkif_copy_from_grant/blkif_setup_rw_req_grant multiple times).
Even calling sg_copy_to_buffer once per page seems rather inefficient as
it uses sg_miter internally.

Switching the for_each_sg to sg_miter is probably the nicer solution as
it takes care of the mapping and the offset/length accounting for you
and will have similar performance.

But, yes, if performance is not an issue, switching it to
sg_copy_to_buffer would be a less invasive change than sg_miter. Which
the same might be said about a lot of these cases.

Unfortunately, changing from kmap_atomic (which is a null operation in a
lot of cases) to sg_copy_X_buffer is a pretty big performance hit.

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

Re: [PATCH 1/2] drm: Make fbdev inherit the crtc's initial rotation

[Bastien Nocera]

On Thu, 2017-04-27 at 19:24 +0300, Ville Syrjälä wrote:
> On Wed, Apr 26, 2017 at 02:28:32PM +0200, Bastien Nocera wrote:
> > On Mon, 2017-04-24 at 15:48 +0300, Ville Syrjälä wrote:
> > > 
> > 
> > 
> > 
> > > > I've a patch for iio-sensor-proxy which fixes the rotation
> > > > under
> > > > Xorg /
> > > > Wayland when using a desktop environment which honors iio-
> > > > sensor-
> > > > proxy's
> > > > rotation detection:
> > > > https://github.com/hadess/iio-sensor-proxy/pull/162
> > > 
> > > Or is it just this thing that clobbers what the DDX inherited
> > > from
> > > the
> > > kernel as the initial rotation?
> > 
> > I think it's mostly got to do with the compositor (or X) not
> > knowing
> > what "normal" or "0 degrees rotation" corresponds to.
> 
> Well, there are really two cases to consider:
> 
> 1. BIOS/whatever configures display hardware rotation in a way
>    that matches the orientation of the physical display
> 2. BIOS didn't do that. Either the hardware can't do what
>    would be required, or the BIOS just chose not to do it.
> 
> Case 1 should work with these patches as long as the DDX will set up
> the
> initial randr rotation to match what it read out from the kms
> rotation
> property of the primary plane.

Yes. My problem was that instead of fixing the DDX to behave properly,
reusing the same orientation as already configured, we were using iio-
sensor-proxy to trigger the initial rotation. This doesn't work if
there's no accelerometer, or orientation is locked, which is counter-
intuitive.

> Case 2 can't work without some mechanism to query the orientation
> of the display from the firmware/etc.

Yes. I'm not sure where we'd be exporting this quirk though, as we need
it available early enough so that it can be used by boot splashes. DMI
matches in the graphics driver?
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[RFC] drm/amd/amdgpu: get rid of else branch

[Nikola Pajkovsky]

This is super simple elimination of else branch and I should
probably even use unlikely in

if (ring->count_dw < count_dw) {

However, amdgpu_ring_write() has similar if condition, but does not
return after DRM_ERROR and it looks suspicious. On error, we still
adding v to ring and keeping count_dw-- below zero.

if (ring->count_dw <= 0)
DRM_ERROR("amdgpu: writing more dwords to the ring than 
expected!\n");
ring->ring[ring->wptr++] = v;
ring->wptr &= ring->ptr_mask;
ring->count_dw--;

I can obviously be totaly wrong. Hmm?

8<8<8<8<8<8<8<8<8<
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index c1b913541739..c6f4f874ea68 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1596,28 +1596,29 @@ static inline void amdgpu_ring_write_multiple(struct 
amdgpu_ring *ring, void *sr
 
if (ring->count_dw < count_dw) {
DRM_ERROR("amdgpu: writing more dwords to the ring than 
expected!\n");
-   } else {
-   occupied = ring->wptr & ring->ptr_mask;
-   dst = (void *)>ring[occupied];
-   chunk1 = ring->ptr_mask + 1 - occupied;
-   chunk1 = (chunk1 >= count_dw) ? count_dw: chunk1;
-   chunk2 = count_dw - chunk1;
-   chunk1 <<= 2;
-   chunk2 <<= 2;
-
-   if (chunk1)
-   memcpy(dst, src, chunk1);
-
-   if (chunk2) {
-   src += chunk1;
-   dst = (void *)ring->ring;
-   memcpy(dst, src, chunk2);
-   }
-
-   ring->wptr += count_dw;
-   ring->wptr &= ring->ptr_mask;
-   ring->count_dw -= count_dw;
+   return;
}
+
+   occupied = ring->wptr & ring->ptr_mask;
+   dst = (void *)>ring[occupied];
+   chunk1 = ring->ptr_mask + 1 - occupied;
+   chunk1 = (chunk1 >= count_dw) ? count_dw: chunk1;
+   chunk2 = count_dw - chunk1;
+   chunk1 <<= 2;
+   chunk2 <<= 2;
+
+   if (chunk1)
+   memcpy(dst, src, chunk1);
+
+   if (chunk2) {
+   src += chunk1;
+   dst = (void *)ring->ring;
+   memcpy(dst, src, chunk2);
+   }
+
+   ring->wptr += count_dw;
+   ring->wptr &= ring->ptr_mask;
+   ring->count_dw -= count_dw;
 }
 
 static inline struct amdgpu_sdma_instance *
-- 
2.12.2

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

Re: [PATCH v2 15/21] xen-blkfront: Make use of the new sg_map helper function

[Jason Gunthorpe]

On Thu, Apr 27, 2017 at 03:53:37PM -0600, Logan Gunthorpe wrote:
> On 27/04/17 02:53 PM, Jason Gunthorpe wrote:
> > blkfront is one of the drivers I looked at, and it appears to only be
> > memcpying with the bvec_data pointer, so I wonder why it does not use
> > sg_copy_X_buffer instead..
> 
> But you'd potentially end up calling sg_copy_to_buffer multiple times
> per page within the sg (given that gnttab_foreach_grant_in_range might
> call blkif_copy_from_grant/blkif_setup_rw_req_grant multiple times).
> Even calling sg_copy_to_buffer once per page seems rather inefficient as
> it uses sg_miter internally.

Well, that is in the current form, with more users it would make sense
to optimize for the single page case, eg by providing the existing
call, providing a faster single-page-only variant of the copy, perhaps
even one that is inlined.

> Switching the for_each_sg to sg_miter is probably the nicer solution as
> it takes care of the mapping and the offset/length accounting for you
> and will have similar performance.

sg_miter will still fail when the sg contains __iomem, however I would
expect that the sg_copy will work with iomem, by using the __iomem
memcpy variant.

So, sg_copy should always be preferred in this new world with mixed
__iomem since it is the only primitive that can transparently handle
it.

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

Re: [PATCH v2 07/21] crypto: shash, caam: Make use of the new sg_map helper function

[Logan Gunthorpe]

On 26/04/17 09:56 PM, Herbert Xu wrote:
> On Tue, Apr 25, 2017 at 12:20:54PM -0600, Logan Gunthorpe wrote:
>> Very straightforward conversion to the new function in the caam driver
>> and shash library.
>>
>> Signed-off-by: Logan Gunthorpe 
>> Cc: Herbert Xu 
>> Cc: "David S. Miller" 
>> ---
>>  crypto/shash.c| 9 ++---
>>  drivers/crypto/caam/caamalg.c | 8 +++-
>>  2 files changed, 9 insertions(+), 8 deletions(-)
>>
>> diff --git a/crypto/shash.c b/crypto/shash.c
>> index 5e31c8d..5914881 100644
>> --- a/crypto/shash.c
>> +++ b/crypto/shash.c
>> @@ -283,10 +283,13 @@ int shash_ahash_digest(struct ahash_request *req, 
>> struct shash_desc *desc)
>>  if (nbytes < min(sg->length, ((unsigned int)(PAGE_SIZE)) - offset)) {
>>  void *data;
>>  
>> -data = kmap_atomic(sg_page(sg));
>> -err = crypto_shash_digest(desc, data + offset, nbytes,
>> +data = sg_map(sg, 0, SG_KMAP_ATOMIC);
>> +if (IS_ERR(data))
>> +return PTR_ERR(data);
>> +
>> +err = crypto_shash_digest(desc, data, nbytes,
>>req->result);
>> -kunmap_atomic(data);
>> +sg_unmap(sg, data, 0, SG_KMAP_ATOMIC);
>>  crypto_yield(desc->flags);
>>  } else
>>  err = crypto_shash_init(desc) ?:
> 
> Nack.  This is an optimisation for the special case of a single
> SG list entry.  In fact in the common case the kmap_atomic should
> disappear altogether in the no-highmem case.  So replacing it
> with sg_map is not acceptable.

What you seem to have missed is that sg_map is just a thin wrapper
around kmap_atomic. Perhaps with a future check for a mappable page.
This change should have zero impact on performance.

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

Re: drm: fourcc byteorder: add bigendian support to drm_mode_legacy_fb_format

[Jochen Rollwagen]

Hello there,

Since I’m on a big-endian system I’ve been following the discusssion and 
have two (maybe not quite related, and possibly dumb) questions:


1.There’s been a similar discussion on mesa-dev concerning big-endian 
and #ifdef’s, see


https://lists.freedesktop.org/archives/mesa-dev/2016-February/107701.html

Without checking whether the #defines are actually, well, #defined it’s 
doubtful whether the patch would have any effect on all platforms. 
conclusion from the discussion above was that the whole big-endian 
#ifdef story is „/a very fragile situation/„


2. I’ve recently stumbled across a register definition for (older) 
radeon GPU’s, what caught my eye was


*CB:RB3D_COLORPITCH[0-3] · [R/W] · 32 bits · Access: 8/16/32 · 
MMReg:0x4e38-0x4e44 *


*DESCRIPTION: */Color buffer format and tiling control for all the 
multibuffers and the pitch of multibuffer 0. Unpipelined. The cache must 
be empty before any of the registers are changed. /


Field Name



Bits



Default



Description

COLORPITCH



13:1



none



3D destination pitch in multiples of 2-pixels.

COLORTILE



16



none



Denotes whether the 3D destination is in macrotiled format. POSSIBLE 
VALUES: 00 - 3D destination is not macrotiled 01 - 3D destination is 
macrotiled


COLORMICROTILE



18:17



none



Denotes whether the 3D destination is in microtiled format. POSSIBLE 
VALUES: 00 - 3D destination is no microtiled 01 - 3D destination is 
microtiled 02 - 3D destination is square microtiled. Only available in 
16-bit 03 - (reserved)


COLORENDIAN



20:19



none



Specifies endian control for the color buffer

POSSIBLE VALUES: 00 - No swap 01 - Word swap (2 bytes in 16-bit) 02 - 
Dword swap (4 bytes in a 32-bit) 03 - Half-Dword swap (2 16-bit in a 
32-bit)


COLORFORMAT



24:21



0x6



3D destination color format. POSSIBLE VALUES: 00 - (Reserved) 01 - 
(Reserved) 02 - (Reserved) 03 - ARGB1555 04 - RGB565 05 - (Reserved) 06 
- ARGB 07 - ARGB32323232 08 - (Reserved) 09 - I8 10 - ARGB16161616 
11 - YUV422 packed (VYUY) 12 - YUV422 packed (YVYU) 13 - UV88 14 - 
(reserved) 15 - ARGB


Dumb question: Could that register be used for just setting the GPU to 
big-endian mode and leave all the swapping to the GPU ?


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

Re: [PATCH v2 01/21] scatterlist: Introduce sg_map helper functions

[Logan Gunthorpe]

On 27/04/17 09:27 AM, Jason Gunthorpe wrote:
> On Thu, Apr 27, 2017 at 08:53:38AM +0200, Christoph Hellwig wrote:
> How about first switching as many call sites as possible to use
> sg_copy_X_buffer instead of kmap?

Yeah, I could look at doing that first.

One problem is we might get more Naks of the form of Herbert Xu's who
might be concerned with the performance implications.

These are definitely a bit more invasive changes than thin wrappers
around kmap calls.

> A random audit of Logan's series suggests this is actually a fairly
> common thing.

It's not _that_ common but there are a significant fraction. One of my
patches actually did this to two places that seemed to be reimplementing
the sg_copy_X_buffer logic.

Thanks,

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

Re: [PATCH v2 01/21] scatterlist: Introduce sg_map helper functions

[Jason Gunthorpe]

On Thu, Apr 27, 2017 at 08:53:38AM +0200, Christoph Hellwig wrote:

> > The main difficulty we
> > have now is that neither of those functions are expected to fail and we
> > need them to be able to in cases where the page doesn't map to system
> > RAM. This patch series is trying to address it for users of scatterlist.
> > I'm certainly open to other suggestions.
> 
> I think you'll need to follow the existing kmap semantics and never
> fail the iomem version either.  Otherwise you'll have a special case
> that's almost never used that has a different error path.

How about first switching as many call sites as possible to use
sg_copy_X_buffer instead of kmap?

A random audit of Logan's series suggests this is actually a fairly
common thing.

eg drivers/mmc/host/sdhci.c is only doing this:

buffer = sdhci_kmap_atomic(sg, );
memcpy(buffer, align, size);
sdhci_kunmap_atomic(buffer, );

drivers/scsi/mvsas/mv_sas.c is this:

+   to = sg_map(sg_resp, 0, SG_KMAP_ATOMIC);
+   memcpy(to,
+  slot->response + sizeof(struct mvs_err_info),
+  sg_dma_len(sg_resp));
+   sg_unmap(sg_resp, to, 0, SG_KMAP_ATOMIC);

etc.

Lots of other places seem similar, if not sometimes a little bit more
convoluted..

Switching all the trivial cases to use copy might bring more clarity
as to what is actually required for the remaining few users? If there
are only a few then it may no longer matter if the API is not idyllic.

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

Re: [PATCH v2 15/21] xen-blkfront: Make use of the new sg_map helper function

[Jason Gunthorpe]

On Thu, Apr 27, 2017 at 02:19:24PM -0600, Logan Gunthorpe wrote:
> 
> 
> On 26/04/17 01:37 AM, Roger Pau Monné wrote:
> > On Tue, Apr 25, 2017 at 12:21:02PM -0600, Logan Gunthorpe wrote:
> >> Straightforward conversion to the new helper, except due to the lack
> >> of error path, we have to use SG_MAP_MUST_NOT_FAIL which may BUG_ON in
> >> certain cases in the future.
> >>
> >> Signed-off-by: Logan Gunthorpe 
> >> Cc: Boris Ostrovsky 
> >> Cc: Juergen Gross 
> >> Cc: Konrad Rzeszutek Wilk 
> >> Cc: "Roger Pau Monné" 
> >>  drivers/block/xen-blkfront.c | 20 +++-
> >>  1 file changed, 11 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
> >> index 3945963..ed62175 100644
> >> +++ b/drivers/block/xen-blkfront.c
> >> @@ -816,8 +816,9 @@ static int blkif_queue_rw_req(struct request *req, 
> >> struct blkfront_ring_info *ri
> >>BUG_ON(sg->offset + sg->length > PAGE_SIZE);
> >>  
> >>if (setup.need_copy) {
> >> -  setup.bvec_off = sg->offset;
> >> -  setup.bvec_data = kmap_atomic(sg_page(sg));
> >> +  setup.bvec_off = 0;
> >> +  setup.bvec_data = sg_map(sg, 0, SG_KMAP_ATOMIC |
> >> +   SG_MAP_MUST_NOT_FAIL);
> > 
> > I assume that sg_map already adds sg->offset to the address?
> 
> Correct.
> 
> > Also wondering whether we can get rid of bvec_off and just increment 
> > bvec_data,
> > adding Julien who IIRC added this code.
> 
> bvec_off is used to keep track of the offset within the current mapping
> so it's not a great idea given that you'd want to kunmap_atomic the
> original address and not something with an offset. It would be nice if
> this could be converted to use the sg_miter interface but that's a much
> more invasive change that would require someone who knows this code and
> can properly test it. I'd be very grateful if someone actually took that on.

blkfront is one of the drivers I looked at, and it appears to only be
memcpying with the bvec_data pointer, so I wonder why it does not use
sg_copy_X_buffer instead..

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

Re: [PATCH v2 01/21] scatterlist: Introduce sg_map helper functions

[Christoph Hellwig]

On Wed, Apr 26, 2017 at 12:11:33PM -0600, Logan Gunthorpe wrote:
> Ok, well for starters I think you are mistaken about kmap being able to
> fail. I'm having a hard time finding many users of that function that
> bother to check for an error when calling it.

A quick audit of the arch code shows you're right - kmap can't fail
anywhere anymore.

> The main difficulty we
> have now is that neither of those functions are expected to fail and we
> need them to be able to in cases where the page doesn't map to system
> RAM. This patch series is trying to address it for users of scatterlist.
> I'm certainly open to other suggestions.

I think you'll need to follow the existing kmap semantics and never
fail the iomem version either.  Otherwise you'll have a special case
that's almost never used that has a different error path.

> There are a fair number of cases in the kernel that do something like:
> 
> if (something)
> x = kmap(page);
> else
> x = kmap_atomic(page);
> ...
> if (something)
> kunmap(page)
> else
> kunmap_atomic(x)
> 
> Which just seems cumbersome to me.

Passing a different flag based on something isn't really much better.

> In any case, if you can accept an sg_kmap and sg_kmap_atomic api just
> say so and I'll make the change. But I'll still need a flags variable
> for SG_MAP_MUST_NOT_FAIL to support legacy cases that have no fail path
> and both of those functions will need to be pretty nearly replicas of
> each other.

Again, wrong way.  Suddenly making things fail for your special case
that normally don't fail is a receipe for bugs.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 15/21] xen-blkfront: Make use of the new sg_map helper function

[Logan Gunthorpe]

On 26/04/17 01:37 AM, Roger Pau Monné wrote:
> On Tue, Apr 25, 2017 at 12:21:02PM -0600, Logan Gunthorpe wrote:
>> Straightforward conversion to the new helper, except due to the lack
>> of error path, we have to use SG_MAP_MUST_NOT_FAIL which may BUG_ON in
>> certain cases in the future.
>>
>> Signed-off-by: Logan Gunthorpe 
>> Cc: Boris Ostrovsky 
>> Cc: Juergen Gross 
>> Cc: Konrad Rzeszutek Wilk 
>> Cc: "Roger Pau Monné" 
>> ---
>>  drivers/block/xen-blkfront.c | 20 +++-
>>  1 file changed, 11 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
>> index 3945963..ed62175 100644
>> --- a/drivers/block/xen-blkfront.c
>> +++ b/drivers/block/xen-blkfront.c
>> @@ -816,8 +816,9 @@ static int blkif_queue_rw_req(struct request *req, 
>> struct blkfront_ring_info *ri
>>  BUG_ON(sg->offset + sg->length > PAGE_SIZE);
>>  
>>  if (setup.need_copy) {
>> -setup.bvec_off = sg->offset;
>> -setup.bvec_data = kmap_atomic(sg_page(sg));
>> +setup.bvec_off = 0;
>> +setup.bvec_data = sg_map(sg, 0, SG_KMAP_ATOMIC |
>> + SG_MAP_MUST_NOT_FAIL);
> 
> I assume that sg_map already adds sg->offset to the address?

Correct.

> Also wondering whether we can get rid of bvec_off and just increment 
> bvec_data,
> adding Julien who IIRC added this code.

bvec_off is used to keep track of the offset within the current mapping
so it's not a great idea given that you'd want to kunmap_atomic the
original address and not something with an offset. It would be nice if
this could be converted to use the sg_miter interface but that's a much
more invasive change that would require someone who knows this code and
can properly test it. I'd be very grateful if someone actually took that on.

Logan

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

Re: [PATCH v2 01/21] scatterlist: Introduce sg_map helper functions

[Logan Gunthorpe]

On 26/04/17 01:44 AM, Christoph Hellwig wrote:
> I think we'll at least need a draft of those to make sense of these
> patches.  Otherwise they just look very clumsy.

Ok, what follows is a draft patch attempting to show where I'm thinking
of going with this. Obviously it will not compile because it assumes
the users throughout the kernel are a bit different than they are today.
Notably, there is no sg_page anymore.

There's also likely a ton of issues and arguments to have over a bunch
of the specifics below and I'd expect the concept to evolve more
as cleanup occurs. This itself is an evolution of the draft I posted
replying to you in my last RFC thread.

Also, before any of this is truly useful to us, pfn_t would have to
infect a few other places in the kernel.

Thanks,

Logan


diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
index fad170b..85ef928 100644
--- a/include/linux/scatterlist.h
+++ b/include/linux/scatterlist.h
@@ -6,13 +6,14 @@
 #include 
 #include 
 #include 
+#include 
 #include 

 struct scatterlist {
 #ifdef CONFIG_DEBUG_SG
unsigned long   sg_magic;
 #endif
-   unsigned long   page_link;
+   pfn_t   pfn;
unsigned intoffset;
unsigned intlength;
dma_addr_t  dma_address;
@@ -60,15 +61,68 @@ struct sg_table {

 #define SG_MAGIC   0x87654321

-/*
- * We overload the LSB of the page pointer to indicate whether it's
- * a valid sg entry, or whether it points to the start of a new
scatterlist.
- * Those low bits are there for everyone! (thanks mason :-)
- */
-#define sg_is_chain(sg)((sg)->page_link & 0x01)
-#define sg_is_last(sg) ((sg)->page_link & 0x02)
-#define sg_chain_ptr(sg)   \
-   ((struct scatterlist *) ((sg)->page_link & ~0x03))
+static inline bool sg_is_chain(struct scatterlist *sg)
+{
+   return sg->pfn.val & PFN_SG_CHAIN;
+}
+
+static inline bool sg_is_last(struct scatterlist *sg)
+{
+   return sg->pfn.val & PFN_SG_LAST;
+}
+
+static inline struct scatterlist *sg_chain_ptr(struct scatterlist *sg)
+{
+   unsigned long sgl = pfn_t_to_pfn(sg->pfn);
+   return (struct scatterlist *)(sgl << PAGE_SHIFT);
+}
+
+static inline bool sg_is_iomem(struct scatterlist *sg)
+{
+   return pfn_t_is_iomem(sg->pfn);
+}
+
+/**
+ * sg_assign_pfn - Assign a given pfn_t to an SG entry
+ * @sg:SG entry
+ * @pfn:   The pfn
+ *
+ * Description:
+ *   Assign a pfn to sg entry. Also see sg_set_pfn(), the most commonly
used
+ *   variant.w
+ *
+ **/
+static inline void sg_assign_pfn(struct scatterlist *sg, pfn_t pfn)
+{
+#ifdef CONFIG_DEBUG_SG
+   BUG_ON(sg->sg_magic != SG_MAGIC);
+   BUG_ON(sg_is_chain(sg));
+   BUG_ON(pfn.val & (PFN_SG_CHAIN | PFN_SG_LAST));
+#endif
+
+   sg->pfn = pfn;
+}
+
+/**
+ * sg_set_pfn - Set sg entry to point at given pfn
+ * @sg: SG entry
+ * @pfn:The page
+ * @len:Length of data
+ * @offset: Offset into page
+ *
+ * Description:
+ *   Use this function to set an sg entry pointing at a pfn, never assign
+ *   the page directly. We encode sg table information in the lower bits
+ *   of the page pointer. See sg_pfn_t for looking up the pfn_t belonging
+ *   to an sg entry.
+ **/
+static inline void sg_set_pfn(struct scatterlist *sg, pfn_t pfn,
+ unsigned int len, unsigned int offset)
+{
+   sg_assign_pfn(sg, pfn);
+   sg->offset = offset;
+   sg->length = len;
+}

 /**
  * sg_assign_page - Assign a given page to an SG entry
@@ -82,18 +136,13 @@ struct sg_table {
  **/
 static inline void sg_assign_page(struct scatterlist *sg, struct page
*page)
 {
-   unsigned long page_link = sg->page_link & 0x3;
+   if (!page) {
+   pfn_t null_pfn = {0};
+   sg_assign_pfn(sg, null_pfn);
+   return;
+   }

-   /*
-* In order for the low bit stealing approach to work, pages
-* must be aligned at a 32-bit boundary as a minimum.
-*/
-   BUG_ON((unsigned long) page & 0x03);
-#ifdef CONFIG_DEBUG_SG
-   BUG_ON(sg->sg_magic != SG_MAGIC);
-   BUG_ON(sg_is_chain(sg));
-#endif
-   sg->page_link = page_link | (unsigned long) page;
+   sg_assign_pfn(sg, page_to_pfn_t(page));
 }

 /**
@@ -106,8 +155,7 @@ static inline void sg_assign_page(struct scatterlist
*sg, struct page *page)
  * Description:
  *   Use this function to set an sg entry pointing at a page, never assign
  *   the page directly. We encode sg table information in the lower bits
- *   of the page pointer. See sg_page() for looking up the page belonging
- *   to an sg entry.
+ *   of the page pointer.
  *
  **/
 static inline void sg_set_page(struct scatterlist *sg, struct page *page,
@@ -118,13 +166,53 @@ static inline void sg_set_page(struct scatterlist
*sg, struct page *page,
sg->length = len;
 }

-static inline struct page *sg_page(struct scatterlist *sg)
+/**
+ * sg_pfn_t - Return the 

Re: [PATCH v2 07/21] crypto: shash, caam: Make use of the new sg_map helper function

[Herbert Xu]

On Tue, Apr 25, 2017 at 12:20:54PM -0600, Logan Gunthorpe wrote:
> Very straightforward conversion to the new function in the caam driver
> and shash library.
> 
> Signed-off-by: Logan Gunthorpe 
> Cc: Herbert Xu 
> Cc: "David S. Miller" 
> ---
>  crypto/shash.c| 9 ++---
>  drivers/crypto/caam/caamalg.c | 8 +++-
>  2 files changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/crypto/shash.c b/crypto/shash.c
> index 5e31c8d..5914881 100644
> --- a/crypto/shash.c
> +++ b/crypto/shash.c
> @@ -283,10 +283,13 @@ int shash_ahash_digest(struct ahash_request *req, 
> struct shash_desc *desc)
>   if (nbytes < min(sg->length, ((unsigned int)(PAGE_SIZE)) - offset)) {
>   void *data;
>  
> - data = kmap_atomic(sg_page(sg));
> - err = crypto_shash_digest(desc, data + offset, nbytes,
> + data = sg_map(sg, 0, SG_KMAP_ATOMIC);
> + if (IS_ERR(data))
> + return PTR_ERR(data);
> +
> + err = crypto_shash_digest(desc, data, nbytes,
> req->result);
> - kunmap_atomic(data);
> + sg_unmap(sg, data, 0, SG_KMAP_ATOMIC);
>   crypto_yield(desc->flags);
>   } else
>   err = crypto_shash_init(desc) ?:

Nack.  This is an optimisation for the special case of a single
SG list entry.  In fact in the common case the kmap_atomic should
disappear altogether in the no-highmem case.  So replacing it
with sg_map is not acceptable.

Cheers,
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 15/21] xen-blkfront: Make use of the new sg_map helper function

[Logan Gunthorpe]

On 27/04/17 05:20 PM, Jason Gunthorpe wrote:
> It seems the most robust: test for iomem, and jump to a slow path
> copy, otherwise inline the kmap and memcpy
> 
> Every place doing memcpy from sgl will need that pattern to be
> correct.

Ok, sounds like a good place to start to me. I'll see what I can do for
a v3 of this set. Though, I probably won't send anything until after the
merge window.

>>> sg_miter will still fail when the sg contains __iomem, however I would
>>> expect that the sg_copy will work with iomem, by using the __iomem
>>> memcpy variant.
>>
>> Yes, that's true. Any sg_miters that ever see iomem will need to be
>> converted to support it. This isn't much different than the other
>> kmap(sg_page()) users I was converting that will also fail if they see
>> iomem. Though, I suspect an sg_miter user would be easier to convert to
>> iomem than a random kmap user.
> 
> How? sg_miter seems like the next nightmare down this path, what is
> sg_miter_next supposed to do when something hits an iomem sgl?

My proposal is roughly included in the draft I sent upthread. We add an
sg_miter flag indicating the iteratee supports iomem and if miter finds
iomem (with the support flag set) it sets ioaddr which is __iomem. The
iteratee then just needs to null check addr and ioaddr and perform the
appropriate action.

Logan

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

Re: [Intel-gfx] [PATCH v7 0/4] Adding driver-private objects to atomic state

[Pandiyan, Dhinakaran]

On Mon, 2017-04-24 at 11:53 -0400, Harry Wentland wrote:
> Patches 1-3: Reviewed-by: Harry Wentland 
> Patch 4: Acked-by: Harry Wentland 
> 
> Harry
> 

Thanks for the review.

-DK



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

Re: [PATCH v2 15/21] xen-blkfront: Make use of the new sg_map helper function

[Logan Gunthorpe]

On 27/04/17 04:11 PM, Jason Gunthorpe wrote:
> On Thu, Apr 27, 2017 at 03:53:37PM -0600, Logan Gunthorpe wrote:
> Well, that is in the current form, with more users it would make sense
> to optimize for the single page case, eg by providing the existing
> call, providing a faster single-page-only variant of the copy, perhaps
> even one that is inlined.

Ok, does it make sense then to have an sg_copy_page_to_buffer (or some
such... I'm having trouble thinking of a sane name that isn't too long).
That just does k(un)map_atomic and memcpy? I could try that if it makes
sense to people.

>> Switching the for_each_sg to sg_miter is probably the nicer solution as
>> it takes care of the mapping and the offset/length accounting for you
>> and will have similar performance.
> 
> sg_miter will still fail when the sg contains __iomem, however I would
> expect that the sg_copy will work with iomem, by using the __iomem
> memcpy variant.

Yes, that's true. Any sg_miters that ever see iomem will need to be
converted to support it. This isn't much different than the other
kmap(sg_page()) users I was converting that will also fail if they see
iomem. Though, I suspect an sg_miter user would be easier to convert to
iomem than a random kmap user.

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

Re: [PATCH v2 15/21] xen-blkfront: Make use of the new sg_map helper function

[Jason Gunthorpe]

On Thu, Apr 27, 2017 at 05:03:45PM -0600, Logan Gunthorpe wrote:
> 
> 
> On 27/04/17 04:11 PM, Jason Gunthorpe wrote:
> > On Thu, Apr 27, 2017 at 03:53:37PM -0600, Logan Gunthorpe wrote:
> > Well, that is in the current form, with more users it would make sense
> > to optimize for the single page case, eg by providing the existing
> > call, providing a faster single-page-only variant of the copy, perhaps
> > even one that is inlined.
> 
> Ok, does it make sense then to have an sg_copy_page_to_buffer (or some
> such... I'm having trouble thinking of a sane name that isn't too long).
> That just does k(un)map_atomic and memcpy? I could try that if it makes
> sense to people.

It seems the most robust: test for iomem, and jump to a slow path
copy, otherwise inline the kmap and memcpy

Every place doing memcpy from sgl will need that pattern to be
correct.

> > sg_miter will still fail when the sg contains __iomem, however I would
> > expect that the sg_copy will work with iomem, by using the __iomem
> > memcpy variant.
> 
> Yes, that's true. Any sg_miters that ever see iomem will need to be
> converted to support it. This isn't much different than the other
> kmap(sg_page()) users I was converting that will also fail if they see
> iomem. Though, I suspect an sg_miter user would be easier to convert to
> iomem than a random kmap user.

How? sg_miter seems like the next nightmare down this path, what is
sg_miter_next supposed to do when something hits an iomem sgl?

miter.addr is supposed to be a kernel pointer that must not be
__iomem..

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

Re: [PATCH 1/2] drm: Introduce crtc->mode_valid() callback

[Jose Abreu]

Hi Andrzej,


Thanks for your answer!


On 27-04-2017 11:05, Andrzej Hajda wrote:
> Hi Jose,
>
> On 26.04.2017 12:48, Jose Abreu wrote:
>> Some crtc's may have restrictions in the mode they can display. In
>> this patch a new callback (crtc->mode_valid()) is introduced that
>> is called at the same stage of connector->mode_valid() callback.
>>
>> This shall be implemented if the crtc has some sort of restriction
>> so that we don't probe modes that will fail in the commit() stage.
>> For example: A given crtc may be responsible to set a clock value.
>> If the clock can not produce all the values for the available
>> modes then this callback can be used to restrict the number of
>> probbed modes to only the ones that can be displayed.
>>
>> If the crtc does not implement the callback then the behaviour will
>> remain the same. Also, for a given set of crtcs that can be bound to
>> the connector, if at least one can display the mode then the mode
>> will be probbed.
> I see few possible issues/improvements here:
> 1. crtc can have different constraints depending on the encoder it is
> connected to, theoretically reverse dependency is also possible, but I
> am not aware of such hw.
> 2. there also could be similar dependency constrains between connector
> and encoder, I guess.
> 3. encoders and bridges should have also possibility to validate modes,
> they also have constrains, btw encoder_slave have such callback.
>
> Regarding 1st and 2nd point, maybe it would be good to validate modes
> according to topology described in drm_mode_get_connector::encoder_id
> and drm_mode_get_encoder::crtc_id:
> a) if connector is not connected to any encoder report to userspace
> modes filtered only by connector::mode_valid,
> b) if connector is connected to encoder, report modes filtered by
> connector, encoder and attached bridges,
> c) if full pipeline is constructed, report modes filtered by all members
> of the pipeline.

I see your point but when the full pipeline is created we already
handed the modes to userspace, right?

This patch is simple and does not take into account that: it just
iterates over all crtcs that can be bound to the set of encoders
which instead can be bound to the given connector. So, we are
filtering modes from the connector but not taking into account
the pipeline. It will work for a single pipeline, but with
multiple constraints, just like you mentioned, it will probably
make no difference.

Despite this, I think its a good initial step.

>
> Of course with this approach drm_mode_get_connector userspace should be
> aware of the fact that it should re-call drm_mode_get_connector after
> topology change to update supported modes.

I believe we should avoid this, but indeed it seems necessary.

What about a new get_modes(encoder_id, connector_id, crtc_id)
ioctl ? User could call this and know what modes a given pipeline
will support.

Best regards,
Jose Miguel Abreu

>
> Regards
> Andrzej
>

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

[PATCH v8 1/4] drm: Add driver-private objects to atomic state

[Dhinakaran Pandiyan]

From: "Pandiyan, Dhinakaran" 

It is necessary to track states for objects other than connector, crtc
and plane for atomic modesets. But adding objects like DP MST link
bandwidth to drm_atomic_state would mean that a non-core object will be
modified by the core helper functions for swapping and clearing
it's state. So, lets add void * objects and helper functions that operate
on void * types to keep these objects and states private to the core.
Drivers can then implement specific functions to swap and clear states.
The other advantage having just void * for these objects in
drm_atomic_state is that objects of different types can be managed in the
same state array.

v6: More kernel-doc to keep 0-day happy
v5: Remove more NULL checks (Maarten)
v4: Avoid redundant NULL checks when private_objs array is empty (Maarten)
v3: Macro alignment (Chris)
v2: Added docs and new iterator to filter private objects (Daniel)

Cc: Daniel Vetter 
Cc: Maarten Lankhorst 
Cc: Archit Taneja 
Cc: Chris Wilson 
Cc: Harry Wentland 

Acked-by: Harry Wentland 
Suggested-by: Daniel Vetter 
Signed-off-by: Dhinakaran Pandiyan 
---
 drivers/gpu/drm/drm_atomic.c| 65 +
 drivers/gpu/drm/drm_atomic_helper.c |  5 ++
 include/drm/drm_atomic.h| 95 +
 3 files changed, 165 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 30229ab..8e5f3eb 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -57,6 +57,7 @@ void drm_atomic_state_default_release(struct drm_atomic_state 
*state)
kfree(state->connectors);
kfree(state->crtcs);
kfree(state->planes);
+   kfree(state->private_objs);
 }
 EXPORT_SYMBOL(drm_atomic_state_default_release);
 
@@ -184,6 +185,17 @@ void drm_atomic_state_default_clear(struct 
drm_atomic_state *state)
state->planes[i].ptr = NULL;
state->planes[i].state = NULL;
}
+
+   for (i = 0; i < state->num_private_objs; i++) {
+   void *obj_state = state->private_objs[i].obj_state;
+
+   state->private_objs[i].funcs->destroy_state(obj_state);
+   state->private_objs[i].obj = NULL;
+   state->private_objs[i].obj_state = NULL;
+   state->private_objs[i].funcs = NULL;
+   }
+   state->num_private_objs = 0;
+
 }
 EXPORT_SYMBOL(drm_atomic_state_default_clear);
 
@@ -978,6 +990,59 @@ static void drm_atomic_plane_print_state(struct 
drm_printer *p,
 }
 
 /**
+ * drm_atomic_get_private_obj_state - get private object state
+ * @state: global atomic state
+ * @obj: private object to get the state for
+ * @funcs: pointer to the struct of function pointers that identify the object
+ * type
+ *
+ * This function returns the private object state for the given private object,
+ * allocating the state if needed. It does not grab any locks as the caller is
+ * expected to care of any required locking.
+ *
+ * RETURNS:
+ *
+ * Either the allocated state or the error code encoded into a pointer.
+ */
+void *
+drm_atomic_get_private_obj_state(struct drm_atomic_state *state, void *obj,
+ const struct drm_private_state_funcs *funcs)
+{
+   int index, num_objs, i;
+   size_t size;
+   struct __drm_private_objs_state *arr;
+
+   for (i = 0; i < state->num_private_objs; i++)
+   if (obj == state->private_objs[i].obj &&
+   state->private_objs[i].obj_state)
+   return state->private_objs[i].obj_state;
+
+   num_objs = state->num_private_objs + 1;
+   size = sizeof(*state->private_objs) * num_objs;
+   arr = krealloc(state->private_objs, size, GFP_KERNEL);
+   if (!arr)
+   return ERR_PTR(-ENOMEM);
+
+   state->private_objs = arr;
+   index = state->num_private_objs;
+   memset(>private_objs[index], 0, sizeof(*state->private_objs));
+
+   state->private_objs[index].obj_state = funcs->duplicate_state(state, 
obj);
+   if (!state->private_objs[index].obj_state)
+   return ERR_PTR(-ENOMEM);
+
+   state->private_objs[index].obj = obj;
+   state->private_objs[index].funcs = funcs;
+   state->num_private_objs = num_objs;
+
+   DRM_DEBUG_ATOMIC("Added new private object state %p to %p\n",
+state->private_objs[index].obj_state, state);
+
+   return state->private_objs[index].obj_state;
+}
+EXPORT_SYMBOL(drm_atomic_get_private_obj_state);
+
+/**
  * drm_atomic_get_connector_state - get connector state
  * @state: global atomic state object
  * @connector: connector to get state object for
diff --git a/drivers/gpu/drm/drm_atomic_helper.c 

Re: [PATCH] drm/rockchip: Set line flag config register in vop_crtc_enable

[Kristian H. Kristensen]

Jeffy Chen  writes:

> We need to set vop config done after update line flag config, it's a
> new requirement for chips newer than rk3368.
>
> Since we would only use line flag irq for vact_end, let's move it to
> vop_crtc_enable.
>
> Signed-off-by: Jeffy Chen 
>
> ---
>
>  drivers/gpu/drm/rockchip/analogix_dp-rockchip.c |  4 ++--
>  drivers/gpu/drm/rockchip/rockchip_drm_drv.h |  3 +--
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 20 +---
>  3 files changed, 12 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c 
> b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> index d8fa7a9..9bfdbc6 100644
> --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> @@ -115,8 +115,8 @@ static void analogix_dp_psr_work(struct work_struct *work)
>  
>   vact_end = crtc->mode.vtotal - crtc->mode.vsync_start + 
> crtc->mode.vdisplay;

We don't need vact_end anymore here.

Kristian

> - ret = rockchip_drm_wait_line_flag(dp->encoder.crtc, vact_end,
> -   PSR_WAIT_LINE_FLAG_TIMEOUT_MS);
> + ret = rockchip_drm_wait_vact_end(dp->encoder.crtc,
> +  PSR_WAIT_LINE_FLAG_TIMEOUT_MS);
>   if (ret) {
>   dev_err(dp->dev, "line flag interrupt did not arrive\n");
>   return;
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h 
> b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> index a48fcce..47905fa 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> @@ -62,8 +62,7 @@ int rockchip_drm_dma_attach_device(struct drm_device 
> *drm_dev,
>  struct device *dev);
>  void rockchip_drm_dma_detach_device(struct drm_device *drm_dev,
>   struct device *dev);
> -int rockchip_drm_wait_line_flag(struct drm_crtc *crtc, unsigned int line_num,
> - unsigned int mstimeout);
> +int rockchip_drm_wait_vact_end(struct drm_crtc *crtc, unsigned int 
> mstimeout);
>  
>  extern struct platform_driver cdn_dp_driver;
>  extern struct platform_driver dw_hdmi_rockchip_pltfm_driver;
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c 
> b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index 3f7a82d..40a5e6e 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -468,7 +468,7 @@ static bool vop_line_flag_irq_is_enabled(struct vop *vop)
>   return !!line_flag_irq;
>  }
>  
> -static void vop_line_flag_irq_enable(struct vop *vop, int line_num)
> +static void vop_line_flag_irq_enable(struct vop *vop)
>  {
>   unsigned long flags;
>  
> @@ -477,7 +477,6 @@ static void vop_line_flag_irq_enable(struct vop *vop, int 
> line_num)
>  
>   spin_lock_irqsave(>irq_lock, flags);
>  
> - VOP_CTRL_SET(vop, line_flag_num[0], line_num);
>   VOP_INTR_SET_TYPE(vop, clear, LINE_FLAG_INTR, 1);
>   VOP_INTR_SET_TYPE(vop, enable, LINE_FLAG_INTR, 1);
>  
> @@ -981,6 +980,8 @@ static void vop_crtc_enable(struct drm_crtc *crtc)
>   VOP_CTRL_SET(vop, vact_st_end, val);
>   VOP_CTRL_SET(vop, vpost_st_end, val);
>  
> + VOP_CTRL_SET(vop, line_flag_num[0], vact_end);
> +
>   clk_set_rate(vop->dclk, adjusted_mode->clock * 1000);
>  
>   VOP_CTRL_SET(vop, standby, 0);
> @@ -1507,19 +1508,16 @@ static void vop_win_init(struct vop *vop)
>  }
>  
>  /**
> - * rockchip_drm_wait_line_flag - acqiure the give line flag event
> + * rockchip_drm_wait_vact_end
>   * @crtc: CRTC to enable line flag
> - * @line_num: interested line number
>   * @mstimeout: millisecond for timeout
>   *
> - * Driver would hold here until the interested line flag interrupt have
> - * happened or timeout to wait.
> + * Wait for vact_end line flag irq or timeout.
>   *
>   * Returns:
>   * Zero on success, negative errno on failure.
>   */
> -int rockchip_drm_wait_line_flag(struct drm_crtc *crtc, unsigned int line_num,
> - unsigned int mstimeout)
> +int rockchip_drm_wait_vact_end(struct drm_crtc *crtc, unsigned int mstimeout)
>  {
>   struct vop *vop = to_vop(crtc);
>   unsigned long jiffies_left;
> @@ -1527,14 +1525,14 @@ int rockchip_drm_wait_line_flag(struct drm_crtc 
> *crtc, unsigned int line_num,
>   if (!crtc || !vop->is_enabled)
>   return -ENODEV;
>  
> - if (line_num > crtc->mode.vtotal || mstimeout <= 0)
> + if (mstimeout <= 0)
>   return -EINVAL;
>  
>   if (vop_line_flag_irq_is_enabled(vop))
>   return -EBUSY;
>  
>   reinit_completion(>line_flag_completion);
> - vop_line_flag_irq_enable(vop, line_num);
> + vop_line_flag_irq_enable(vop);
>  
>   jiffies_left = wait_for_completion_timeout(>line_flag_completion,
>  

Re: [PATCH] dma-buf: avoid scheduling on fence status query v2

[Gustavo Padovan]

2017-04-26 Christian König :

> Am 26.04.2017 um 16:46 schrieb Andres Rodriguez:
> > When a timeout of zero is specified, the caller is only interested in
> > the fence status.
> > 
> > In the current implementation, dma_fence_default_wait will always call
> > schedule_timeout() at least once for an unsignaled fence. This adds a
> > significant overhead to a fence status query.
> > 
> > Avoid this overhead by returning early if a zero timeout is specified.
> > 
> > v2: move early return after enable_signaling
> > 
> > Signed-off-by: Andres Rodriguez 
> 
> Reviewed-by: Christian König 

pushed to drm-misc-next. Thanks all.

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

[Bug 96897] clpeak OpenCL benchmark hangs during compilation on Clover RadeonSI

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=96897

--- Comment #10 from M. Edward (Ed) Borasky  ---
Created attachment 131104
  --> https://bugs.freedesktop.org/attachment.cgi?id=131104=edit
clinfo for the system

Note: this bug is in Fedora's bugzilla as well -
https://bugzilla.redhat.com/show_bug.cgi?id=1433632

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

[Bug 96897] clpeak OpenCL benchmark hangs during compilation on Clover RadeonSI

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=96897

M. Edward (Ed) Borasky  changed:

   What|Removed |Added

 CC||zn...@znmeb.net

--- Comment #9 from M. Edward (Ed) Borasky  ---
I have something like this on Fedora - both 25 (stable) and 26 (alpha). I type
"clpeak" and the CPU goes to 100% and nothing else happens.

I'll attach a 'clinfo' printout.

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

Re: [RFC v2 3/7] drm/i915: update cursors asynchronously through atomic

[Gustavo Padovan]

Hi Ville,

2017-04-27 Ville Syrjälä :

> On Thu, Apr 27, 2017 at 12:15:15PM -0300, Gustavo Padovan wrote:
> > From: Gustavo Padovan 
> > 
> > Add support to async updates of cursors by using the new atomic
> > interface for that. Basically what this commit does is do what
> > intel_legacy_cursor_update() did but through atomic.
> > 
> > v2: move fb setting to core and use new state (Eric Anholt)
> > 
> > Cc: Daniel Vetter 
> > Signed-off-by: Gustavo Padovan 
> > ---
> >  drivers/gpu/drm/i915/intel_atomic_plane.c |  52 +++
> >  drivers/gpu/drm/i915/intel_display.c  | 147 
> > +-
> >  2 files changed, 73 insertions(+), 126 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c 
> > b/drivers/gpu/drm/i915/intel_atomic_plane.c
> > index cfb4729..c5d0596 100644
> > --- a/drivers/gpu/drm/i915/intel_atomic_plane.c
> > +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
> > @@ -246,11 +246,63 @@ static void intel_plane_atomic_update(struct 
> > drm_plane *plane,
> > }
> >  }
> >  
> > +static int intel_plane_atomic_async_check(struct drm_plane *plane,
> > + struct drm_plane_state *state)
> > +{
> > +   struct drm_crtc *crtc = plane->state->crtc;
> > +   struct drm_crtc_state *crtc_state = crtc->state;
> > +
> > +   if (plane->type != DRM_PLANE_TYPE_CURSOR)
> > +   return -EINVAL;
> > +
> > +   /*
> > +* When crtc is inactive or there is a modeset pending,
> > +* wait for it to complete in the slowpath
> > +*/
> > +   if (!crtc_state->active || to_intel_crtc_state(crtc_state)->update_pipe)
> > +   return -EINVAL;
> > +
> > +   /* Only changing fb should be in the fastpath.  */
> 
> No, we want cursor movement there as well. It's somewhat impossible
> to see from this code now that the core has the size checks. But even
> so the comment should not lie.

Sure, I'll fix the comment.

> 
> > +   if (!plane->state->fb != !state->fb)
> > +   return -EINVAL;
> > +
> > +   return 0;
> > +}
> > +
> > +static void intel_plane_atomic_async_update(struct drm_plane *plane,
> > +   struct drm_plane_state *new_state)
> > +{
> > +   struct intel_plane *intel_plane = to_intel_plane(plane);
> > +   struct drm_crtc *crtc = plane->state->crtc;
> > +
> > +   i915_gem_track_fb(intel_fb_obj(plane->state->fb),
> > + intel_fb_obj(new_state->fb),
> > + intel_plane->frontbuffer_bit);
> > +
> > +   *to_intel_plane_state(plane->state) = *to_intel_plane_state(new_state);
> > +   to_intel_plane_state(new_state)->vma =
> > +   to_intel_plane_state(plane->state)->vma;
> > +
> > +   plane->state->visible = new_state->visible;
> > +
> > +   if (plane->state->visible) {
> > +   trace_intel_update_plane(plane, to_intel_crtc(crtc));
> > +   intel_plane->update_plane(plane,
> > + to_intel_crtc_state(crtc->state),
> > + to_intel_plane_state(new_state));
> > +   } else {
> > +   trace_intel_disable_plane(plane, to_intel_crtc(crtc));
> > +   intel_plane->disable_plane(plane, crtc);
> > +   }
> > +}
> > +
> >  const struct drm_plane_helper_funcs intel_plane_helper_funcs = {
> > .prepare_fb = intel_prepare_plane_fb,
> > .cleanup_fb = intel_cleanup_plane_fb,
> > .atomic_check = intel_plane_atomic_check,
> > .atomic_update = intel_plane_atomic_update,
> > +   .atomic_async_check = intel_plane_atomic_async_check,
> > +   .atomic_async_update = intel_plane_atomic_async_update,
> 
> NAK. We don't want these "async" updates for anything but cursors.

Yes, we do. Async PageFlips will go through here as well. That seems a
VR requirement as well.

Gustavo

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

[Bug 100239] Incorrect rendering in CS:GO

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=100239

--- Comment #3 from Bruno Jacquet (Xaapyks)  ---
Hello,

I have the same problem on my RX 480 and current mesa git.
I produced an apitrace but it is ~734MiB and trimming seems to be corrupting
it, I might be doing something wrong.
How should I share and eventually trim/extract only the relevant part from the
trace?

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

Re: [RFC v2 1/7] drm/atomic: initial support for asynchronous plane update

[Gustavo Padovan]

Hi Ville,

2017-04-27 Ville Syrjälä :

> On Thu, Apr 27, 2017 at 12:15:13PM -0300, Gustavo Padovan wrote:
> > From: Gustavo Padovan 
> > 
> > In some cases, like cursor updates, it is interesting to update the
> > plane in an asynchronous fashion to avoid big delays. The current queued
> > update could be still waiting for a fence to signal and thus block any
> > subsequent update until its scan out. In cases like this if we update the
> > cursor synchronously through the atomic API it will cause significant
> > delays that would even be noticed by the final user.
> > 
> > This patch creates a fast path to jump ahead the current queued state and
> > do single planes updates without going through all atomic step in
> > drm_atomic_helper_commit().
> > 
> > We take this path for legacy cursor updates. Users can also set the
> > DRM_MODE_ATOMIC_ASYNC_UPDATE flag on atomic updates.
> > 
> > For now only single plane updates are supported, but we plan to support
> > multiple planes updates and async PageFlips through this interface as well
> > in the near future.
> > 
> > v2:
> > - allow updates even if there is a queued update for the same
> > plane.
> > - fixes on the documentation (Emil Velikov)
> > - unconditionally call ->atomic_async_update (Emil Velikov)
> > - check for ->atomic_async_update earlier (Daniel Vetter)
> > - make ->atomic_async_check() the last step (Daniel Vetter)
> > - add ASYNC_UPDATE flag (Eric Anholt)
> > - update state in core after ->atomic_async_update (Eric Anholt)
> > - update docs (Eric Anholt)
> > 
> > Cc: Daniel Vetter 
> > Cc: Rob Clark 
> > Cc: Eric Anholt 
> > Signed-off-by: Gustavo Padovan 
> > ---
> >  drivers/gpu/drm/drm_atomic.c | 50 
> > 
> >  drivers/gpu/drm/drm_atomic_helper.c  | 48 
> > ++
> >  include/drm/drm_atomic.h |  2 ++
> >  include/drm/drm_atomic_helper.h  |  2 ++
> >  include/drm/drm_modeset_helper_vtables.h | 45 
> >  include/uapi/drm/drm_mode.h  |  4 ++-
> >  6 files changed, 150 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > index 30229ab..7b60cf8 100644
> > --- a/drivers/gpu/drm/drm_atomic.c
> > +++ b/drivers/gpu/drm/drm_atomic.c
> > @@ -77,6 +77,7 @@ drm_atomic_state_init(struct drm_device *dev, struct 
> > drm_atomic_state *state)
> >  * setting this appropriately?
> >  */
> > state->allow_modeset = true;
> > +   state->async_update = true;
> >  
> > state->crtcs = kcalloc(dev->mode_config.num_crtc,
> >sizeof(*state->crtcs), GFP_KERNEL);
> > @@ -631,6 +632,51 @@ static int drm_atomic_crtc_check(struct drm_crtc *crtc,
> > return 0;
> >  }
> >  
> > +static bool drm_atomic_async_check(struct drm_atomic_state *state)
> > +{
> > +   struct drm_crtc *crtc;
> > +   struct drm_crtc_state *crtc_state;
> > +   struct drm_plane *__plane, *plane = NULL;
> > +   struct drm_plane_state *__plane_state, *plane_state = NULL;
> > +   const struct drm_plane_helper_funcs *funcs;
> > +   int i, n_planes = 0;
> > +
> > +   for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
> > +   if (drm_atomic_crtc_needs_modeset(crtc_state))
> > +   return false;
> > +   }
> > +
> > +   for_each_new_plane_in_state(state, __plane, __plane_state, i) {
> > +   n_planes++;
> > +   plane = __plane;
> > +   plane_state = __plane_state;
> > +   }
> > +
> > +   /* FIXME: we support single plane updates for now */
> > +   if (!plane || n_planes != 1)
> > +   return false;
> > +
> > +   funcs = plane->helper_private;
> > +   if (!funcs->atomic_async_update)
> > +   return false;
> > +
> > +   if (plane_state->fence)
> > +   return false;
> > +
> > +   if (!plane->state->crtc || (plane->state->crtc != plane_state->crtc))
> > +   return false;
> > +
> > +   /* No configuring new scaling in the async path. */
> 
> Those checks aren't really about scaling. Well, they are also about
> scaling, but they're mainly about changing size.

I just copied the comment from the previous code in the drivers...
I can fix it.

> 
> What I don't really understand is why we're enforcing these restrictions
> in the core but leaving other restrictions up to the driver. I don't see
> size changes as anything really special compared to many of the other
> restrictions that would now be up to each driver.

Because I extracted what was common between msm, vc4 and i915 on their
legacy cursor update calls. I didn't want to enforce anything else in
core for now.

> 
> If you're really after some lowest common denominator as far as
> exposed capabilities are concerned then I think 

Re: [RFC v2 0/7] drm: asynchronous atomic plane update

[Gustavo Padovan]

2017-04-27 Ville Syrjälä :

> On Thu, Apr 27, 2017 at 12:15:12PM -0300, Gustavo Padovan wrote:
> > From: Gustavo Padovan 
> > 
> > Hi,
> > 
> > Second take of Asynchronous Plane Updates over Atomic. Here I looked
> > to msm, vc4 and i915 to identify a common pattern to create atomic helpers
> > for async updates. So in patch 1 drm_atomic_async_check() and
> > drm_atomic_helper_async_commit() are introduced along with driver's plane 
> > hooks:
> > ->atomic_async_check() and ->atomic_async_commit().
> > 
> > For now we only support async update for one plane at a time. Also the async
> > update can't modify the CRTC so no modesets are allowed.
> > 
> > Then the other patches add support for it in the drivers. I did virtio 
> > mostly
> > for testing. i915 have been converted and I've been using it without any
> > problem. IGT tests seems to be fine, but there are somewhat random failures
> > with or without the async update changes. msm and vc4 are only 
> > compile-tested.
> > So I think this needs more testing
> > 
> > I started IGT changes to test the Atomic IOCTL with the new flag:
> > 
> > https://git.collabora.com/cgit/user/padovan/intel-gpu-tools.git/
> > 
> > v2:
> > 
> > Apart from all comments on v1 one extra change I made was to remove the
> > constraint of only updating the plane if the queued state didn't touch
> > that plane. I believe it was a too cautious of a change, furthermore this
> > constraint was affecting throughput negatively on i915.
> 
> So you're now allowing reordering the updates? As in update A is
> scheduled before update B, but update B happens before update A.
> That is not a good idea.

That is what already happens with legacy cursor updates. They jump ahead
the scheduled update and apply the cursor update. What we propose here
is to do this over atomic when DRM_MODE_ATOMIC_ASYNC_UPDATE flag is set.
Async PageFlips should use the same infrastructure in the future.

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

[Bug 100784] Fullscreen fade transitions in starsector run at a few frames per second

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=100784

--- Comment #2 from Jon  ---
LIBGL_ALWAYS_SOFTWARE=1 doesn't have the same behaviour.

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

Re: [PATCH 1/2] drm/bridge: Refactor out the panel wrapper from the lvds-encoder bridge.

[Eric Anholt]

Eric Anholt  writes:

> Many DRM drivers have common code to make a stub connector
> implementation that wraps a drm_panel.  By wrapping the panel in a DRM
> bridge, all of the connector code (including calls during encoder
> enable/disable) goes away.
>
> Signed-off-by: Eric Anholt 

> +/**
> + * drm_panel_bridge_add - Creates a drm_bridge and drm_connector that
> + * just call the appropriate functions from drm_panel.
> + *
> + * @dev: The struct device of the panel device.  This is used for
> + * registering the drm_bridge.
> + * @panel: The drm_panel being wrapped.  Must be non-NULL.
> + * @connector_type: The DRM_MODE_CONNECTOR_* for the connector to be
> + * created.
> + *
> + * For drivers converting from directly using drm_panel: The expected
> + * usage pattern is that during either encoder module probe or DSI
> + * host attach, a drm_panel will be looked up through
> + * drm_of_find_panel_or_bridge().  drm_panel_bridge_add() is used to
> + * wrap that panel in the new bridge, and the result can then be
> + * passed to drm_bridge_attach().  The drm_panel_prepare() and related
> + * functions can be dropped from the encoder driver (they're now
> + * called by the KMS helpers before calling into the encoder), along
> + * with connector creation.
> + */
> +struct drm_bridge *drm_panel_bridge_add(struct device *dev,
> + struct drm_panel *panel,
> + u32 connector_type)
> +{
> + struct panel_bridge *panel_bridge =
> + devm_kzalloc(dev, sizeof(*panel_bridge), GFP_KERNEL);
> + int ret;
> +
> + if (!dev || !panel)
> + return ERR_PTR(EINVAL);
> +
> + panel_bridge->dev = dev;
> + panel_bridge->connector_type = connector_type;
> + panel_bridge->panel = panel;
> +
> + panel_bridge->bridge.funcs = _bridge_bridge_funcs;
> + panel_bridge->bridge.of_node = dev->of_node;
> +
> + ret = drm_bridge_add(_bridge->bridge);
> + if (ret)
> + return ERR_PTR(ret);
> +
> + return _bridge->bridge;
> +}
> +EXPORT_SYMBOL(drm_panel_bridge_add);
> +
> +void drm_panel_bridge_remove(struct device *dev, struct drm_bridge *bridge)
> +{
> + drm_bridge_remove(bridge);
> + devm_kfree(dev, bridge);
> +}
> +EXPORT_SYMBOL(drm_panel_bridge_remove);

Working with this interface in another driver, I want to drop the
"struct device" argument, since we can get the struct device from
panel->dev.  It keeps the user of the driver from needing to keep track
of the panel/device it needs to hand to drm_panel_bridge_remove().


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

[PATCH 1/2] drm/bridge: Refactor out the panel wrapper from the lvds-encoder bridge.

[Eric Anholt]

Many DRM drivers have common code to make a stub connector
implementation that wraps a drm_panel.  By wrapping the panel in a DRM
bridge, all of the connector code (including calls during encoder
enable/disable) goes away.

Signed-off-by: Eric Anholt 
---
 Documentation/gpu/drm-kms-helpers.rst |   3 +
 drivers/gpu/drm/bridge/Kconfig|  11 +-
 drivers/gpu/drm/bridge/Makefile   |   1 +
 drivers/gpu/drm/bridge/lvds-encoder.c | 158 
 drivers/gpu/drm/bridge/panel.c| 188 ++
 include/drm/drm_bridge.h  |   8 ++
 6 files changed, 228 insertions(+), 141 deletions(-)
 create mode 100644 drivers/gpu/drm/bridge/panel.c

diff --git a/Documentation/gpu/drm-kms-helpers.rst 
b/Documentation/gpu/drm-kms-helpers.rst
index c075aadd7078..60eb3b41702b 100644
--- a/Documentation/gpu/drm-kms-helpers.rst
+++ b/Documentation/gpu/drm-kms-helpers.rst
@@ -143,6 +143,9 @@ Bridge Helper Reference
 .. kernel-doc:: drivers/gpu/drm/drm_bridge.c
:export:
 
+.. kernel-doc:: drivers/gpu/drm/bridge/panel.c
+   :export:
+
 .. _drm_panel_helper:
 
 Panel Helper Reference
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index f6968d3b4b41..c4daca38743c 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -4,6 +4,14 @@ config DRM_BRIDGE
help
  Bridge registration and lookup framework.
 
+config DRM_PANEL_BRIDGE
+   def_bool y
+   depends on DRM_BRIDGE
+   select DRM_KMS_HELPER
+   select DRM_PANEL
+   help
+ DRM bridge wrapper of DRM panels
+
 menu "Display Interface Bridges"
depends on DRM && DRM_BRIDGE
 
@@ -27,8 +35,7 @@ config DRM_DUMB_VGA_DAC
 config DRM_LVDS_ENCODER
tristate "Transparent parallel to LVDS encoder support"
depends on OF
-   select DRM_KMS_HELPER
-   select DRM_PANEL
+   select DRM_PANEL_BRIDGE
help
  Support for transparent parallel to LVDS encoders that don't require
  any configuration.
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index 3fe2226ee2f2..40a43750ad55 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -5,6 +5,7 @@ obj-$(CONFIG_DRM_DUMB_VGA_DAC) += dumb-vga-dac.o
 obj-$(CONFIG_DRM_LVDS_ENCODER) += lvds-encoder.o
 obj-$(CONFIG_DRM_MEGACHIPS_STDP_GE_B850V3_FW) += 
megachips-stdp-ge-b850v3-fw.o
 obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o
+obj-$(CONFIG_DRM_PANEL_BRIDGE) += panel.o
 obj-$(CONFIG_DRM_PARADE_PS8622) += parade-ps8622.o
 obj-$(CONFIG_DRM_SIL_SII8620) += sil-sii8620.o
 obj-$(CONFIG_DRM_SII902X) += sii902x.o
diff --git a/drivers/gpu/drm/bridge/lvds-encoder.c 
b/drivers/gpu/drm/bridge/lvds-encoder.c
index f1f67a279426..04e1504c4d8f 100644
--- a/drivers/gpu/drm/bridge/lvds-encoder.c
+++ b/drivers/gpu/drm/bridge/lvds-encoder.c
@@ -8,144 +8,18 @@
  */
 
 #include 
-#include 
-#include 
-#include 
-#include 
-#include 
+#include 
 #include 
 
 #include 
 
-struct lvds_encoder {
-   struct device *dev;
-
-   struct drm_bridge bridge;
-   struct drm_connector connector;
-   struct drm_panel *panel;
-};
-
-static inline struct lvds_encoder *
-drm_bridge_to_lvds_encoder(struct drm_bridge *bridge)
-{
-   return container_of(bridge, struct lvds_encoder, bridge);
-}
-
-static inline struct lvds_encoder *
-drm_connector_to_lvds_encoder(struct drm_connector *connector)
-{
-   return container_of(connector, struct lvds_encoder, connector);
-}
-
-static int lvds_connector_get_modes(struct drm_connector *connector)
-{
-   struct lvds_encoder *lvds = drm_connector_to_lvds_encoder(connector);
-
-   return drm_panel_get_modes(lvds->panel);
-}
-
-static const struct drm_connector_helper_funcs lvds_connector_helper_funcs = {
-   .get_modes = lvds_connector_get_modes,
-};
-
-static const struct drm_connector_funcs lvds_connector_funcs = {
-   .dpms = drm_atomic_helper_connector_dpms,
-   .reset = drm_atomic_helper_connector_reset,
-   .fill_modes = drm_helper_probe_single_connector_modes,
-   .destroy = drm_connector_cleanup,
-   .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
-   .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
-};
-
-static int lvds_encoder_attach(struct drm_bridge *bridge)
-{
-   struct lvds_encoder *lvds = drm_bridge_to_lvds_encoder(bridge);
-   struct drm_connector *connector = >connector;
-   int ret;
-
-   if (!bridge->encoder) {
-   DRM_ERROR("Missing encoder\n");
-   return -ENODEV;
-   }
-
-   drm_connector_helper_add(connector, _connector_helper_funcs);
-
-   ret = drm_connector_init(bridge->dev, connector, _connector_funcs,
-DRM_MODE_CONNECTOR_LVDS);
-   if (ret) {
-   DRM_ERROR("Failed to initialize connector\n");
-   return ret;
-   }
-

[PATCH 2/2] drm/vc4: Switch to using the panel-bridge layer, and support bridges.

[Eric Anholt]

The newer version of the RPi panel driver is going to be a combination
of a bridge and a panel, but we should also support panels without a
bridge, so the panel-bridge layer lets us do that cleanly.

Signed-off-by: Eric Anholt 
---
 drivers/gpu/drm/vc4/Kconfig   |   2 +-
 drivers/gpu/drm/vc4/vc4_dsi.c | 158 +++---
 2 files changed, 26 insertions(+), 134 deletions(-)

diff --git a/drivers/gpu/drm/vc4/Kconfig b/drivers/gpu/drm/vc4/Kconfig
index 973b4203c0b2..118def67c9a9 100644
--- a/drivers/gpu/drm/vc4/Kconfig
+++ b/drivers/gpu/drm/vc4/Kconfig
@@ -7,7 +7,7 @@ config DRM_VC4
select DRM_KMS_HELPER
select DRM_KMS_CMA_HELPER
select DRM_GEM_CMA_HELPER
-   select DRM_PANEL
+   select DRM_PANEL_BRIDGE
select SND_PCM
select SND_PCM_ELD
select SND_SOC_GENERIC_DMAENGINE_PCM
diff --git a/drivers/gpu/drm/vc4/vc4_dsi.c b/drivers/gpu/drm/vc4/vc4_dsi.c
index deba62008fd0..a279188e310c 100644
--- a/drivers/gpu/drm/vc4/vc4_dsi.c
+++ b/drivers/gpu/drm/vc4/vc4_dsi.c
@@ -503,8 +503,8 @@ struct vc4_dsi {
 
struct mipi_dsi_host dsi_host;
struct drm_encoder *encoder;
-   struct drm_connector *connector;
-   struct drm_panel *panel;
+   struct drm_bridge *bridge;
+   bool is_panel_bridge;
 
void __iomem *regs;
 
@@ -604,18 +604,6 @@ to_vc4_dsi_encoder(struct drm_encoder *encoder)
return container_of(encoder, struct vc4_dsi_encoder, base.base);
 }
 
-/* VC4 DSI connector KMS struct */
-struct vc4_dsi_connector {
-   struct drm_connector base;
-   struct vc4_dsi *dsi;
-};
-
-static inline struct vc4_dsi_connector *
-to_vc4_dsi_connector(struct drm_connector *connector)
-{
-   return container_of(connector, struct vc4_dsi_connector, base);
-}
-
 #define DSI_REG(reg) { reg, #reg }
 static const struct {
u32 reg;
@@ -723,79 +711,6 @@ int vc4_dsi_debugfs_regs(struct seq_file *m, void *unused)
 }
 #endif
 
-static enum drm_connector_status
-vc4_dsi_connector_detect(struct drm_connector *connector, bool force)
-{
-   struct vc4_dsi_connector *vc4_connector =
-   to_vc4_dsi_connector(connector);
-   struct vc4_dsi *dsi = vc4_connector->dsi;
-
-   if (dsi->panel)
-   return connector_status_connected;
-   else
-   return connector_status_disconnected;
-}
-
-static void vc4_dsi_connector_destroy(struct drm_connector *connector)
-{
-   drm_connector_unregister(connector);
-   drm_connector_cleanup(connector);
-}
-
-static int vc4_dsi_connector_get_modes(struct drm_connector *connector)
-{
-   struct vc4_dsi_connector *vc4_connector =
-   to_vc4_dsi_connector(connector);
-   struct vc4_dsi *dsi = vc4_connector->dsi;
-
-   if (dsi->panel)
-   return drm_panel_get_modes(dsi->panel);
-
-   return 0;
-}
-
-static const struct drm_connector_funcs vc4_dsi_connector_funcs = {
-   .dpms = drm_atomic_helper_connector_dpms,
-   .detect = vc4_dsi_connector_detect,
-   .fill_modes = drm_helper_probe_single_connector_modes,
-   .destroy = vc4_dsi_connector_destroy,
-   .reset = drm_atomic_helper_connector_reset,
-   .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
-   .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
-};
-
-static const struct drm_connector_helper_funcs vc4_dsi_connector_helper_funcs 
= {
-   .get_modes = vc4_dsi_connector_get_modes,
-};
-
-static struct drm_connector *vc4_dsi_connector_init(struct drm_device *dev,
-   struct vc4_dsi *dsi)
-{
-   struct drm_connector *connector;
-   struct vc4_dsi_connector *dsi_connector;
-
-   dsi_connector = devm_kzalloc(dev->dev, sizeof(*dsi_connector),
-GFP_KERNEL);
-   if (!dsi_connector)
-   return ERR_PTR(-ENOMEM);
-
-   connector = _connector->base;
-
-   dsi_connector->dsi = dsi;
-
-   drm_connector_init(dev, connector, _dsi_connector_funcs,
-  DRM_MODE_CONNECTOR_DSI);
-   drm_connector_helper_add(connector, _dsi_connector_helper_funcs);
-
-   connector->polled = 0;
-   connector->interlace_allowed = 0;
-   connector->doublescan_allowed = 0;
-
-   drm_mode_connector_attach_encoder(connector, dsi->encoder);
-
-   return connector;
-}
-
 static void vc4_dsi_encoder_destroy(struct drm_encoder *encoder)
 {
drm_encoder_cleanup(encoder);
@@ -893,12 +808,8 @@ static void vc4_dsi_encoder_disable(struct drm_encoder 
*encoder)
struct vc4_dsi *dsi = vc4_encoder->dsi;
struct device *dev = >pdev->dev;
 
-   drm_panel_disable(dsi->panel);
-
vc4_dsi_ulps(dsi, true);
 
-   drm_panel_unprepare(dsi->panel);
-
clk_disable_unprepare(dsi->pll_phy_clock);
clk_disable_unprepare(dsi->escape_clock);
clk_disable_unprepare(dsi->pixel_clock);
@@ 

[Bug 100802] [regression] mostly blank graphics on Faeria

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=100802

--- Comment #2 from Timo Aaltonen  ---
happens on debian stretch too with mesa 13.0.6

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

Re: [PATCH 1/2] drm: Make fbdev inherit the crtc's initial rotation

[Ville Syrjälä]

On Wed, Apr 26, 2017 at 02:28:32PM +0200, Bastien Nocera wrote:
> On Mon, 2017-04-24 at 15:48 +0300, Ville Syrjälä wrote:
> > 
> 
> 
> > > I've a patch for iio-sensor-proxy which fixes the rotation under
> > > Xorg /
> > > Wayland when using a desktop environment which honors iio-sensor-
> > > proxy's
> > > rotation detection:
> > > https://github.com/hadess/iio-sensor-proxy/pull/162
> > 
> > Or is it just this thing that clobbers what the DDX inherited from
> > the
> > kernel as the initial rotation?
> 
> I think it's mostly got to do with the compositor (or X) not knowing
> what "normal" or "0 degrees rotation" corresponds to.

Well, there are really two cases to consider:

1. BIOS/whatever configures display hardware rotation in a way
   that matches the orientation of the physical display
2. BIOS didn't do that. Either the hardware can't do what
   would be required, or the BIOS just chose not to do it.

Case 1 should work with these patches as long as the DDX will set up the
initial randr rotation to match what it read out from the kms rotation
property of the primary plane.

Case 2 can't work without some mechanism to query the orientation
of the display from the firmware/etc.

-- 
Ville Syrjälä
Intel OTC
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 11/11] drm/amdgpu: use the new TTM bytes moved counter

[Christian König]

From: Christian König 

Instead of the global statistics use the per context bytes moved counter.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 7 +--
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 5 +
 2 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 31613ab..7486827 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -372,9 +372,7 @@ void amdgpu_cs_report_moved_bytes(struct amdgpu_device 
*adev, u64 num_bytes)
 static int amdgpu_cs_bo_validate(struct amdgpu_cs_parser *p,
 struct amdgpu_bo *bo)
 {
-   struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
struct ttm_operation_ctx ctx = { true, false };
-   u64 initial_bytes_moved;
uint32_t domain;
int r;
 
@@ -391,16 +389,13 @@ static int amdgpu_cs_bo_validate(struct amdgpu_cs_parser 
*p,
 
 retry:
amdgpu_ttm_placement_from_domain(bo, domain);
-   initial_bytes_moved = atomic64_read(>num_bytes_moved);
r = ttm_bo_validate(>tbo, >placement, );
-   p->bytes_moved += atomic64_read(>num_bytes_moved) -
-   initial_bytes_moved;
-
if (unlikely(r == -ENOMEM) && domain != bo->allowed_domains) {
domain = bo->allowed_domains;
goto retry;
}
 
+   p->bytes_moved += ctx.bytes_moved;
return r;
 }
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 364d894..8f49b85 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -323,7 +323,6 @@ int amdgpu_bo_create_restricted(struct amdgpu_device *adev,
struct amdgpu_bo *bo;
enum ttm_bo_type type;
unsigned long page_align;
-   u64 initial_bytes_moved;
size_t acc_size;
int r;
 
@@ -395,12 +394,10 @@ int amdgpu_bo_create_restricted(struct amdgpu_device 
*adev,
amdgpu_fill_placement_to_bo(bo, placement);
/* Kernel allocation are uninterruptible */
 
-   initial_bytes_moved = atomic64_read(>num_bytes_moved);
r = ttm_bo_init_reserved(>mman.bdev, >tbo, size, type,
 >placement, page_align, , NULL,
 acc_size, sg, resv, _ttm_bo_destroy);
-   amdgpu_cs_report_moved_bytes(adev,
-   atomic64_read(>num_bytes_moved) - initial_bytes_moved);
+   amdgpu_cs_report_moved_bytes(adev, ctx.bytes_moved);
 
if (unlikely(r != 0))
return r;
-- 
2.5.0

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

[PATCH 06/11] drm/ttm: use an operation context for ttm_bo_mem_space

[Christian König]

From: Christian König 

Instead of specifying interruptible and no_wait_gpu manually.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c |  8 
 drivers/gpu/drm/nouveau/nouveau_bo.c|  6 --
 drivers/gpu/drm/radeon/radeon_ttm.c |  8 
 drivers/gpu/drm/ttm/ttm_bo.c| 22 +++---
 include/drm/ttm/ttm_bo_driver.h |  3 +--
 5 files changed, 24 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 58873cf..e386657 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -369,6 +369,7 @@ static int amdgpu_move_vram_ram(struct ttm_buffer_object 
*bo,
bool no_wait_gpu,
struct ttm_mem_reg *new_mem)
 {
+   struct ttm_operation_ctx ctx = { interruptible, no_wait_gpu };
struct amdgpu_device *adev;
struct ttm_mem_reg *old_mem = >mem;
struct ttm_mem_reg tmp_mem;
@@ -386,8 +387,7 @@ static int amdgpu_move_vram_ram(struct ttm_buffer_object 
*bo,
placements.fpfn = 0;
placements.lpfn = adev->mc.gtt_size >> PAGE_SHIFT;
placements.flags = TTM_PL_MASK_CACHING | TTM_PL_FLAG_TT;
-   r = ttm_bo_mem_space(bo, , _mem,
-interruptible, no_wait_gpu);
+   r = ttm_bo_mem_space(bo, , _mem, );
if (unlikely(r)) {
return r;
}
@@ -416,6 +416,7 @@ static int amdgpu_move_ram_vram(struct ttm_buffer_object 
*bo,
bool no_wait_gpu,
struct ttm_mem_reg *new_mem)
 {
+   struct ttm_operation_ctx ctx = { interruptible, no_wait_gpu };
struct amdgpu_device *adev;
struct ttm_mem_reg *old_mem = >mem;
struct ttm_mem_reg tmp_mem;
@@ -433,8 +434,7 @@ static int amdgpu_move_ram_vram(struct ttm_buffer_object 
*bo,
placements.fpfn = 0;
placements.lpfn = adev->mc.gtt_size >> PAGE_SHIFT;
placements.flags = TTM_PL_MASK_CACHING | TTM_PL_FLAG_TT;
-   r = ttm_bo_mem_space(bo, , _mem,
-interruptible, no_wait_gpu);
+   r = ttm_bo_mem_space(bo, , _mem, );
if (unlikely(r)) {
return r;
}
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c 
b/drivers/gpu/drm/nouveau/nouveau_bo.c
index ad3778b..8486da6 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -1126,6 +1126,7 @@ static int
 nouveau_bo_move_flipd(struct ttm_buffer_object *bo, bool evict, bool intr,
  bool no_wait_gpu, struct ttm_mem_reg *new_mem)
 {
+   struct ttm_operation_ctx ctx = { intr, no_wait_gpu };
struct ttm_place placement_memtype = {
.fpfn = 0,
.lpfn = 0,
@@ -1140,7 +1141,7 @@ nouveau_bo_move_flipd(struct ttm_buffer_object *bo, bool 
evict, bool intr,
 
tmp_mem = *new_mem;
tmp_mem.mm_node = NULL;
-   ret = ttm_bo_mem_space(bo, , _mem, intr, no_wait_gpu);
+   ret = ttm_bo_mem_space(bo, , _mem, );
if (ret)
return ret;
 
@@ -1162,6 +1163,7 @@ static int
 nouveau_bo_move_flips(struct ttm_buffer_object *bo, bool evict, bool intr,
  bool no_wait_gpu, struct ttm_mem_reg *new_mem)
 {
+   struct ttm_operation_ctx ctx = { intr, no_wait_gpu };
struct ttm_place placement_memtype = {
.fpfn = 0,
.lpfn = 0,
@@ -1176,7 +1178,7 @@ nouveau_bo_move_flips(struct ttm_buffer_object *bo, bool 
evict, bool intr,
 
tmp_mem = *new_mem;
tmp_mem.mm_node = NULL;
-   ret = ttm_bo_mem_space(bo, , _mem, intr, no_wait_gpu);
+   ret = ttm_bo_mem_space(bo, , _mem, );
if (ret)
return ret;
 
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c 
b/drivers/gpu/drm/radeon/radeon_ttm.c
index d07ff84..b4bcff5 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -311,6 +311,7 @@ static int radeon_move_vram_ram(struct ttm_buffer_object 
*bo,
bool no_wait_gpu,
struct ttm_mem_reg *new_mem)
 {
+   struct ttm_operation_ctx ctx = { interruptible, no_wait_gpu };
struct radeon_device *rdev;
struct ttm_mem_reg *old_mem = >mem;
struct ttm_mem_reg tmp_mem;
@@ -328,8 +329,7 @@ static int radeon_move_vram_ram(struct ttm_buffer_object 
*bo,
placements.fpfn = 0;
placements.lpfn = 0;
placements.flags = TTM_PL_MASK_CACHING | TTM_PL_FLAG_TT;
-   r = ttm_bo_mem_space(bo, , _mem,
-interruptible, no_wait_gpu);
+   r = ttm_bo_mem_space(bo, , _mem, );
if (unlikely(r)) {
return r;
}
@@ -358,6 +358,7 @@ static int radeon_move_ram_vram(struct ttm_buffer_object 
*bo,

[PATCH 07/11] drm/ttm: use the operation context inside TTM

[Christian König]

From: Christian König 

Instead of passing down the parameters manually to every function.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/ttm/ttm_bo.c | 69 +++-
 1 file changed, 30 insertions(+), 39 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index caddcf6..c288852 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -286,9 +286,8 @@ static int ttm_bo_add_ttm(struct ttm_buffer_object *bo, 
bool zero_alloc)
 }
 
 static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
- struct ttm_mem_reg *mem,
- bool evict, bool interruptible,
- bool no_wait_gpu)
+ struct ttm_mem_reg *mem, bool evict,
+ struct ttm_operation_ctx *ctx)
 {
struct ttm_bo_device *bdev = bo->bdev;
bool old_is_pci = ttm_mem_reg_is_pci(bdev, >mem);
@@ -342,12 +341,14 @@ static int ttm_bo_handle_move_mem(struct 
ttm_buffer_object *bo,
 
if (!(old_man->flags & TTM_MEMTYPE_FLAG_FIXED) &&
!(new_man->flags & TTM_MEMTYPE_FLAG_FIXED))
-   ret = ttm_bo_move_ttm(bo, interruptible, no_wait_gpu, mem);
+   ret = ttm_bo_move_ttm(bo, ctx->interruptible,
+ ctx->no_wait_gpu, mem);
else if (bdev->driver->move)
-   ret = bdev->driver->move(bo, evict, interruptible,
-no_wait_gpu, mem);
+   ret = bdev->driver->move(bo, evict, ctx->interruptible,
+ctx->no_wait_gpu, mem);
else
-   ret = ttm_bo_move_memcpy(bo, interruptible, no_wait_gpu, mem);
+   ret = ttm_bo_move_memcpy(bo, ctx->interruptible,
+ctx->no_wait_gpu, mem);
 
if (ret) {
if (bdev->driver->move_notify) {
@@ -662,10 +663,9 @@ void ttm_bo_unlock_delayed_workqueue(struct ttm_bo_device 
*bdev, int resched)
 }
 EXPORT_SYMBOL(ttm_bo_unlock_delayed_workqueue);
 
-static int ttm_bo_evict(struct ttm_buffer_object *bo, bool interruptible,
-   bool no_wait_gpu)
+static int ttm_bo_evict(struct ttm_buffer_object *bo,
+   struct ttm_operation_ctx *ctx)
 {
-   struct ttm_operation_ctx ctx = { interruptible, no_wait_gpu };
struct ttm_bo_device *bdev = bo->bdev;
struct ttm_mem_reg evict_mem;
struct ttm_placement placement;
@@ -681,7 +681,7 @@ static int ttm_bo_evict(struct ttm_buffer_object *bo, bool 
interruptible,
placement.num_placement = 0;
placement.num_busy_placement = 0;
bdev->driver->evict_flags(bo, );
-   ret = ttm_bo_mem_space(bo, , _mem, );
+   ret = ttm_bo_mem_space(bo, , _mem, ctx);
if (ret) {
if (ret != -ERESTARTSYS) {
pr_err("Failed to find memory space for buffer 0x%p 
eviction\n",
@@ -691,8 +691,7 @@ static int ttm_bo_evict(struct ttm_buffer_object *bo, bool 
interruptible,
goto out;
}
 
-   ret = ttm_bo_handle_move_mem(bo, _mem, true,
-interruptible, no_wait_gpu);
+   ret = ttm_bo_handle_move_mem(bo, _mem, true, ctx);
if (unlikely(ret)) {
if (ret != -ERESTARTSYS)
pr_err("Buffer eviction failed\n");
@@ -719,10 +718,9 @@ bool ttm_bo_eviction_valuable(struct ttm_buffer_object *bo,
 EXPORT_SYMBOL(ttm_bo_eviction_valuable);
 
 static int ttm_mem_evict_first(struct ttm_bo_device *bdev,
-   uint32_t mem_type,
-   const struct ttm_place *place,
-   bool interruptible,
-   bool no_wait_gpu)
+  uint32_t mem_type,
+  const struct ttm_place *place,
+  struct ttm_operation_ctx *ctx)
 {
struct ttm_bo_global *glob = bdev->glob;
struct ttm_mem_type_manager *man = >man[mem_type];
@@ -759,8 +757,8 @@ static int ttm_mem_evict_first(struct ttm_bo_device *bdev,
kref_get(>list_kref);
 
if (!list_empty(>ddestroy)) {
-   ret = ttm_bo_cleanup_refs_and_unlock(bo, interruptible,
-no_wait_gpu);
+   ret = ttm_bo_cleanup_refs_and_unlock(bo, ctx->interruptible,
+ctx->no_wait_gpu);
kref_put(>list_kref, ttm_bo_release_list);
return ret;
}
@@ -772,7 +770,7 @@ static int ttm_mem_evict_first(struct ttm_bo_device *bdev,
 
ttm_bo_list_ref_sub(bo, put_count, true);
 
-   ret = ttm_bo_evict(bo, interruptible, no_wait_gpu);
+   ret = ttm_bo_evict(bo, ctx);

[PATCH 10/11] drm/amdgpu: forward operation context to ttm_bo_mem_space

[Christian König]

From: Christian König 

This way we can finally use some more stats.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 30 --
 1 file changed, 12 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 061fd5f..5c90601 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -364,12 +364,10 @@ static int amdgpu_move_blit(struct ttm_buffer_object *bo,
return r;
 }
 
-static int amdgpu_move_vram_ram(struct ttm_buffer_object *bo,
-   bool evict, bool interruptible,
-   bool no_wait_gpu,
+static int amdgpu_move_vram_ram(struct ttm_buffer_object *bo, bool evict,
+   struct ttm_operation_ctx *ctx,
struct ttm_mem_reg *new_mem)
 {
-   struct ttm_operation_ctx ctx = { interruptible, no_wait_gpu };
struct amdgpu_device *adev;
struct ttm_mem_reg *old_mem = >mem;
struct ttm_mem_reg tmp_mem;
@@ -387,7 +385,7 @@ static int amdgpu_move_vram_ram(struct ttm_buffer_object 
*bo,
placements.fpfn = 0;
placements.lpfn = adev->mc.gtt_size >> PAGE_SHIFT;
placements.flags = TTM_PL_MASK_CACHING | TTM_PL_FLAG_TT;
-   r = ttm_bo_mem_space(bo, , _mem, );
+   r = ttm_bo_mem_space(bo, , _mem, ctx);
if (unlikely(r)) {
return r;
}
@@ -401,22 +399,20 @@ static int amdgpu_move_vram_ram(struct ttm_buffer_object 
*bo,
if (unlikely(r)) {
goto out_cleanup;
}
-   r = amdgpu_move_blit(bo, true, no_wait_gpu, _mem, old_mem);
+   r = amdgpu_move_blit(bo, true, ctx->no_wait_gpu, _mem, old_mem);
if (unlikely(r)) {
goto out_cleanup;
}
-   r = ttm_bo_move_ttm(bo, interruptible, no_wait_gpu, new_mem);
+   r = ttm_bo_move_ttm(bo, ctx->interruptible, ctx->no_wait_gpu, new_mem);
 out_cleanup:
ttm_bo_mem_put(bo, _mem);
return r;
 }
 
-static int amdgpu_move_ram_vram(struct ttm_buffer_object *bo,
-   bool evict, bool interruptible,
-   bool no_wait_gpu,
+static int amdgpu_move_ram_vram(struct ttm_buffer_object *bo, bool evict,
+   struct ttm_operation_ctx *ctx,
struct ttm_mem_reg *new_mem)
 {
-   struct ttm_operation_ctx ctx = { interruptible, no_wait_gpu };
struct amdgpu_device *adev;
struct ttm_mem_reg *old_mem = >mem;
struct ttm_mem_reg tmp_mem;
@@ -434,15 +430,15 @@ static int amdgpu_move_ram_vram(struct ttm_buffer_object 
*bo,
placements.fpfn = 0;
placements.lpfn = adev->mc.gtt_size >> PAGE_SHIFT;
placements.flags = TTM_PL_MASK_CACHING | TTM_PL_FLAG_TT;
-   r = ttm_bo_mem_space(bo, , _mem, );
+   r = ttm_bo_mem_space(bo, , _mem, ctx);
if (unlikely(r)) {
return r;
}
-   r = ttm_bo_move_ttm(bo, interruptible, no_wait_gpu, _mem);
+   r = ttm_bo_move_ttm(bo, ctx->interruptible, ctx->no_wait_gpu, _mem);
if (unlikely(r)) {
goto out_cleanup;
}
-   r = amdgpu_move_blit(bo, true, no_wait_gpu, new_mem, old_mem);
+   r = amdgpu_move_blit(bo, true, ctx->no_wait_gpu, new_mem, old_mem);
if (unlikely(r)) {
goto out_cleanup;
}
@@ -488,12 +484,10 @@ static int amdgpu_bo_move(struct ttm_buffer_object *bo, 
bool evict,
 
if (old_mem->mem_type == TTM_PL_VRAM &&
new_mem->mem_type == TTM_PL_SYSTEM) {
-   r = amdgpu_move_vram_ram(bo, evict, ctx->interruptible,
-   ctx->no_wait_gpu, new_mem);
+   r = amdgpu_move_vram_ram(bo, evict, ctx, new_mem);
} else if (old_mem->mem_type == TTM_PL_SYSTEM &&
   new_mem->mem_type == TTM_PL_VRAM) {
-   r = amdgpu_move_ram_vram(bo, evict, ctx->interruptible,
-   ctx->no_wait_gpu, new_mem);
+   r = amdgpu_move_ram_vram(bo, evict, ctx, new_mem);
} else {
r = amdgpu_move_blit(bo, evict, ctx->no_wait_gpu,
 new_mem, old_mem);
-- 
2.5.0

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

[PATCH 09/11] drm/ttm: add number of bytes moved to the operation context

[Christian König]

From: Christian König 

Add some statistics how many bytes we have moved.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/ttm/ttm_bo.c | 1 +
 include/drm/ttm/ttm_bo_api.h | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 0c3c06a..0c5bc9e 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -378,6 +378,7 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object 
*bo,
else
bo->offset = 0;
 
+   ctx->bytes_moved += bo->num_pages << PAGE_SHIFT;
return 0;
 
 out_err:
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index 4b4e9e0..91b4afb 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -270,6 +270,7 @@ struct ttm_bo_kmap_obj {
 struct ttm_operation_ctx {
bool interruptible;
bool no_wait_gpu;
+   uint64_t bytes_moved;
 };
 
 /**
-- 
2.5.0

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

[PATCH 04/11] drm/ttm: add operation ctx to ttm_bo_validate

[Christian König]

From: Christian König 

Give moving a BO into place an operation context to work with.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c |  9 ++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c|  3 ++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c |  3 ++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 14 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c|  6 --
 drivers/gpu/drm/ast/ast_ttm.c  |  9 ++---
 drivers/gpu/drm/bochs/bochs_mm.c   |  6 --
 drivers/gpu/drm/cirrus/cirrus_ttm.c|  6 --
 drivers/gpu/drm/mgag200/mgag200_ttm.c  |  9 ++---
 drivers/gpu/drm/nouveau/nouveau_bo.c   |  4 ++--
 drivers/gpu/drm/qxl/qxl_ioctl.c|  4 ++--
 drivers/gpu/drm/qxl/qxl_object.c   |  6 --
 drivers/gpu/drm/qxl/qxl_release.c  |  4 ++--
 drivers/gpu/drm/radeon/radeon_gem.c|  3 ++-
 drivers/gpu/drm/radeon/radeon_mn.c |  3 ++-
 drivers/gpu/drm/radeon/radeon_object.c | 14 +-
 drivers/gpu/drm/radeon/radeon_vm.c |  3 ++-
 drivers/gpu/drm/ttm/ttm_bo.c   | 16 +---
 drivers/gpu/drm/virtio/virtgpu_ioctl.c | 11 ++-
 drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c|  3 ++-
 drivers/gpu/drm/vmwgfx/vmwgfx_dmabuf.c | 21 +
 drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c|  9 -
 drivers/gpu/drm/vmwgfx/vmwgfx_resource.c   |  6 --
 drivers/gpu/drm/vmwgfx/vmwgfx_shader.c |  3 ++-
 include/drm/ttm/ttm_bo_api.h   | 20 
 25 files changed, 124 insertions(+), 71 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 26168df..31613ab 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -373,6 +373,7 @@ static int amdgpu_cs_bo_validate(struct amdgpu_cs_parser *p,
 struct amdgpu_bo *bo)
 {
struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
+   struct ttm_operation_ctx ctx = { true, false };
u64 initial_bytes_moved;
uint32_t domain;
int r;
@@ -391,7 +392,7 @@ static int amdgpu_cs_bo_validate(struct amdgpu_cs_parser *p,
 retry:
amdgpu_ttm_placement_from_domain(bo, domain);
initial_bytes_moved = atomic64_read(>num_bytes_moved);
-   r = ttm_bo_validate(>tbo, >placement, true, false);
+   r = ttm_bo_validate(>tbo, >placement, );
p->bytes_moved += atomic64_read(>num_bytes_moved) -
initial_bytes_moved;
 
@@ -408,6 +409,7 @@ static bool amdgpu_cs_try_evict(struct amdgpu_cs_parser *p,
struct amdgpu_bo *validated)
 {
uint32_t domain = validated->allowed_domains;
+   struct ttm_operation_ctx ctx = { true, false };
int r;
 
if (!p->evictable)
@@ -440,7 +442,7 @@ static bool amdgpu_cs_try_evict(struct amdgpu_cs_parser *p,
/* Good we can try to move this BO somewhere else */
amdgpu_ttm_placement_from_domain(bo, other);
initial_bytes_moved = atomic64_read(>num_bytes_moved);
-   r = ttm_bo_validate(>tbo, >placement, true, false);
+   r = ttm_bo_validate(>tbo, >placement, );
p->bytes_moved += atomic64_read(>num_bytes_moved) -
initial_bytes_moved;
 
@@ -1419,6 +1421,7 @@ amdgpu_cs_find_mapping(struct amdgpu_cs_parser *parser,
  */
 int amdgpu_cs_sysvm_access_required(struct amdgpu_cs_parser *parser)
 {
+   struct ttm_operation_ctx ctx = { false, false };
unsigned i;
int r;
 
@@ -1437,7 +1440,7 @@ int amdgpu_cs_sysvm_access_required(struct 
amdgpu_cs_parser *parser)
 
bo->flags |= AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS;
amdgpu_ttm_placement_from_domain(bo, bo->allowed_domains);
-   r = ttm_bo_validate(>tbo, >placement, false, false);
+   r = ttm_bo_validate(>tbo, >placement, );
if (unlikely(r))
return r;
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index 67be795..e5157b5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -306,6 +306,7 @@ int amdgpu_gem_create_ioctl(struct drm_device *dev, void 
*data,
 int amdgpu_gem_userptr_ioctl(struct drm_device *dev, void *data,
 struct drm_file *filp)
 {
+   struct ttm_operation_ctx ctx = { true, false };
struct amdgpu_device *adev = dev->dev_private;
struct drm_amdgpu_gem_userptr *args = data;
struct drm_gem_object *gobj;
@@ -362,7 +363,7 @@ int amdgpu_gem_userptr_ioctl(struct drm_device *dev, void 
*data,
goto free_pages;
 
amdgpu_ttm_placement_from_domain(bo, AMDGPU_GEM_DOMAIN_GTT);
-   r = 

[PATCH 01/11] drm/ttm: cleanup coding style in ttm_bo_api.h

[Christian König]

From: Christian König 

Extern is the default for function declerations anyway and this
solves a bunch of 80char per line issues.

Signed-off-by: Christian König 
---
 include/drm/ttm/ttm_bo_api.h | 135 ++-
 1 file changed, 56 insertions(+), 79 deletions(-)

diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index 3b302a5..425f3b4 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -288,8 +288,7 @@ ttm_bo_reference(struct ttm_buffer_object *bo)
  * Returns -EBUSY if no_wait is true and the buffer is busy.
  * Returns -ERESTARTSYS if interrupted by a signal.
  */
-extern int ttm_bo_wait(struct ttm_buffer_object *bo,
-  bool interruptible, bool no_wait);
+int ttm_bo_wait(struct ttm_buffer_object *bo, bool interruptible, bool 
no_wait);
 
 /**
  * ttm_bo_mem_compat - Check if proposed placement is compatible with a bo
@@ -300,9 +299,8 @@ extern int ttm_bo_wait(struct ttm_buffer_object *bo,
  *
  * Returns true if the placement is compatible
  */
-extern bool ttm_bo_mem_compat(struct ttm_placement *placement,
- struct ttm_mem_reg *mem,
- uint32_t *new_flags);
+bool ttm_bo_mem_compat(struct ttm_placement *placement, struct ttm_mem_reg 
*mem,
+  uint32_t *new_flags);
 
 /**
  * ttm_bo_validate
@@ -320,10 +318,10 @@ extern bool ttm_bo_mem_compat(struct ttm_placement 
*placement,
  * -EBUSY if no_wait is true and buffer busy.
  * -ERESTARTSYS if interrupted by a signal.
  */
-extern int ttm_bo_validate(struct ttm_buffer_object *bo,
-   struct ttm_placement *placement,
-   bool interruptible,
-   bool no_wait_gpu);
+int ttm_bo_validate(struct ttm_buffer_object *bo,
+   struct ttm_placement *placement,
+   bool interruptible,
+   bool no_wait_gpu);
 
 /**
  * ttm_bo_unref
@@ -332,7 +330,7 @@ extern int ttm_bo_validate(struct ttm_buffer_object *bo,
  *
  * Unreference and clear a pointer to a buffer object.
  */
-extern void ttm_bo_unref(struct ttm_buffer_object **bo);
+void ttm_bo_unref(struct ttm_buffer_object **bo);
 
 
 /**
@@ -344,8 +342,8 @@ extern void ttm_bo_unref(struct ttm_buffer_object **bo);
  *
  * Release @count lru list references to this buffer object.
  */
-extern void ttm_bo_list_ref_sub(struct ttm_buffer_object *bo, int count,
-   bool never_free);
+void ttm_bo_list_ref_sub(struct ttm_buffer_object *bo, int count,
+bool never_free);
 
 /**
  * ttm_bo_add_to_lru
@@ -357,7 +355,7 @@ extern void ttm_bo_list_ref_sub(struct ttm_buffer_object 
*bo, int count,
  * This function must be called with struct ttm_bo_global::lru_lock held, and
  * is typically called immediately prior to unreserving a bo.
  */
-extern void ttm_bo_add_to_lru(struct ttm_buffer_object *bo);
+void ttm_bo_add_to_lru(struct ttm_buffer_object *bo);
 
 /**
  * ttm_bo_del_from_lru
@@ -369,7 +367,7 @@ extern void ttm_bo_add_to_lru(struct ttm_buffer_object *bo);
  * and is usually called just immediately after the bo has been reserved to
  * avoid recursive reservation from lru lists.
  */
-extern int ttm_bo_del_from_lru(struct ttm_buffer_object *bo);
+int ttm_bo_del_from_lru(struct ttm_buffer_object *bo);
 
 /**
  * ttm_bo_move_to_lru_tail
@@ -380,7 +378,7 @@ extern int ttm_bo_del_from_lru(struct ttm_buffer_object 
*bo);
  * object. This function must be called with struct ttm_bo_global::lru_lock
  * held, and is used to make a BO less likely to be considered for eviction.
  */
-extern void ttm_bo_move_to_lru_tail(struct ttm_buffer_object *bo);
+void ttm_bo_move_to_lru_tail(struct ttm_buffer_object *bo);
 
 /**
  * ttm_bo_lock_delayed_workqueue
@@ -389,15 +387,14 @@ extern void ttm_bo_move_to_lru_tail(struct 
ttm_buffer_object *bo);
  * Returns
  * True if the workqueue was queued at the time
  */
-extern int ttm_bo_lock_delayed_workqueue(struct ttm_bo_device *bdev);
+int ttm_bo_lock_delayed_workqueue(struct ttm_bo_device *bdev);
 
 /**
  * ttm_bo_unlock_delayed_workqueue
  *
  * Allows the delayed workqueue to run.
  */
-extern void ttm_bo_unlock_delayed_workqueue(struct ttm_bo_device *bdev,
-   int resched);
+void ttm_bo_unlock_delayed_workqueue(struct ttm_bo_device *bdev, int resched);
 
 /**
  * ttm_bo_eviction_valuable
@@ -424,8 +421,7 @@ bool ttm_bo_eviction_valuable(struct ttm_buffer_object *bo,
  * -EBUSY if the buffer is busy and no_wait is true.
  * -ERESTARTSYS if interrupted by a signal.
  */
-extern int
-ttm_bo_synccpu_write_grab(struct ttm_buffer_object *bo, bool no_wait);
+int ttm_bo_synccpu_write_grab(struct ttm_buffer_object *bo, bool no_wait);
 
 /**
  * ttm_bo_synccpu_write_release:
@@ -434,7 +430,7 @@ ttm_bo_synccpu_write_grab(struct ttm_buffer_object *bo, 
bool 

[PATCH 03/11] drm/ttm: remove cur_placement

[Christian König]

From: Christian König 

Not used any more.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/ttm/ttm_bo.c | 5 ++---
 include/drm/ttm/ttm_bo_api.h | 1 -
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 04cbecc..70335a3 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -372,11 +372,10 @@ static int ttm_bo_handle_move_mem(struct 
ttm_buffer_object *bo,
bo->evicted = false;
}
 
-   if (bo->mem.mm_node) {
+   if (bo->mem.mm_node)
bo->offset = (bo->mem.start << PAGE_SHIFT) +
bdev->man[bo->mem.mem_type].gpu_offset;
-   bo->cur_placement = bo->mem.placement;
-   } else
+   else
bo->offset = 0;
 
return 0;
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index 425f3b4..1d99cc1 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -224,7 +224,6 @@ struct ttm_buffer_object {
 */
 
uint64_t offset; /* GPU address space is independent of CPU word size */
-   uint32_t cur_placement;
 
struct sg_table *sg;
 
-- 
2.5.0

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

[PATCH 08/11] drm/ttm: add context to driver move callback as well

[Christian König]

From: Christian König 

Instead of passing the parameters manually.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 21 +++--
 drivers/gpu/drm/nouveau/nouveau_bo.c| 27 ---
 drivers/gpu/drm/qxl/qxl_ttm.c   |  9 -
 drivers/gpu/drm/radeon/radeon_ttm.c | 23 ---
 drivers/gpu/drm/ttm/ttm_bo.c|  3 +--
 drivers/gpu/drm/virtio/virtgpu_ttm.c|  7 +++
 include/drm/ttm/ttm_bo_driver.h |  6 ++
 7 files changed, 49 insertions(+), 47 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index e386657..061fd5f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -451,10 +451,9 @@ static int amdgpu_move_ram_vram(struct ttm_buffer_object 
*bo,
return r;
 }
 
-static int amdgpu_bo_move(struct ttm_buffer_object *bo,
-   bool evict, bool interruptible,
-   bool no_wait_gpu,
-   struct ttm_mem_reg *new_mem)
+static int amdgpu_bo_move(struct ttm_buffer_object *bo, bool evict,
+ struct ttm_operation_ctx *ctx,
+ struct ttm_mem_reg *new_mem)
 {
struct amdgpu_device *adev;
struct amdgpu_bo *abo;
@@ -489,19 +488,21 @@ static int amdgpu_bo_move(struct ttm_buffer_object *bo,
 
if (old_mem->mem_type == TTM_PL_VRAM &&
new_mem->mem_type == TTM_PL_SYSTEM) {
-   r = amdgpu_move_vram_ram(bo, evict, interruptible,
-   no_wait_gpu, new_mem);
+   r = amdgpu_move_vram_ram(bo, evict, ctx->interruptible,
+   ctx->no_wait_gpu, new_mem);
} else if (old_mem->mem_type == TTM_PL_SYSTEM &&
   new_mem->mem_type == TTM_PL_VRAM) {
-   r = amdgpu_move_ram_vram(bo, evict, interruptible,
-   no_wait_gpu, new_mem);
+   r = amdgpu_move_ram_vram(bo, evict, ctx->interruptible,
+   ctx->no_wait_gpu, new_mem);
} else {
-   r = amdgpu_move_blit(bo, evict, no_wait_gpu, new_mem, old_mem);
+   r = amdgpu_move_blit(bo, evict, ctx->no_wait_gpu,
+new_mem, old_mem);
}
 
if (r) {
 memcpy:
-   r = ttm_bo_move_memcpy(bo, interruptible, no_wait_gpu, new_mem);
+   r = ttm_bo_move_memcpy(bo, ctx->interruptible,
+  ctx->no_wait_gpu, new_mem);
if (r) {
return r;
}
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c 
b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 8486da6..39e328e 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -1253,8 +1253,9 @@ nouveau_bo_vm_cleanup(struct ttm_buffer_object *bo,
 }
 
 static int
-nouveau_bo_move(struct ttm_buffer_object *bo, bool evict, bool intr,
-   bool no_wait_gpu, struct ttm_mem_reg *new_mem)
+nouveau_bo_move(struct ttm_buffer_object *bo, bool evict,
+   struct ttm_operation_ctx *ctx,
+   struct ttm_mem_reg *new_mem)
 {
struct nouveau_drm *drm = nouveau_bdev(bo->bdev);
struct nouveau_bo *nvbo = nouveau_bo(bo);
@@ -1262,7 +1263,7 @@ nouveau_bo_move(struct ttm_buffer_object *bo, bool evict, 
bool intr,
struct nouveau_drm_tile *new_tile = NULL;
int ret = 0;
 
-   ret = ttm_bo_wait(bo, intr, no_wait_gpu);
+   ret = ttm_bo_wait(bo, ctx->interruptible, ctx->no_wait_gpu);
if (ret)
return ret;
 
@@ -1286,22 +1287,26 @@ nouveau_bo_move(struct ttm_buffer_object *bo, bool 
evict, bool intr,
/* Hardware assisted copy. */
if (drm->ttm.move) {
if (new_mem->mem_type == TTM_PL_SYSTEM)
-   ret = nouveau_bo_move_flipd(bo, evict, intr,
-   no_wait_gpu, new_mem);
+   ret = nouveau_bo_move_flipd(bo, evict,
+   ctx->interruptible,
+   ctx->no_wait_gpu, new_mem);
else if (old_mem->mem_type == TTM_PL_SYSTEM)
-   ret = nouveau_bo_move_flips(bo, evict, intr,
-   no_wait_gpu, new_mem);
+   ret = nouveau_bo_move_flips(bo, evict,
+   ctx->interruptible,
+   ctx->no_wait_gpu, new_mem);
else
-   ret = nouveau_bo_move_m2mf(bo, evict, intr,
-  no_wait_gpu, new_mem);
+  

[PATCH 02/11] drm/ttm: cleanup ttm_bo_driver.h

[Christian König]

From: Christian König 

Extern is the default for function declerations anyway.

Signed-off-by: Christian König 
---
 include/drm/ttm/ttm_bo_driver.h | 113 +++-
 1 file changed, 54 insertions(+), 59 deletions(-)

diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index cb5072a..9ce2017 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -608,12 +608,12 @@ ttm_flag_masked(uint32_t *old, uint32_t new, uint32_t 
mask)
  * Returns:
  * NULL: Out of memory.
  */
-extern int ttm_tt_init(struct ttm_tt *ttm, struct ttm_bo_device *bdev,
-   unsigned long size, uint32_t page_flags,
-   struct page *dummy_read_page);
-extern int ttm_dma_tt_init(struct ttm_dma_tt *ttm_dma, struct ttm_bo_device 
*bdev,
-  unsigned long size, uint32_t page_flags,
-  struct page *dummy_read_page);
+int ttm_tt_init(struct ttm_tt *ttm, struct ttm_bo_device *bdev,
+   unsigned long size, uint32_t page_flags,
+   struct page *dummy_read_page);
+int ttm_dma_tt_init(struct ttm_dma_tt *ttm_dma, struct ttm_bo_device *bdev,
+   unsigned long size, uint32_t page_flags,
+   struct page *dummy_read_page);
 
 /**
  * ttm_tt_fini
@@ -622,8 +622,8 @@ extern int ttm_dma_tt_init(struct ttm_dma_tt *ttm_dma, 
struct ttm_bo_device *bde
  *
  * Free memory of ttm_tt structure
  */
-extern void ttm_tt_fini(struct ttm_tt *ttm);
-extern void ttm_dma_tt_fini(struct ttm_dma_tt *ttm_dma);
+void ttm_tt_fini(struct ttm_tt *ttm);
+void ttm_dma_tt_fini(struct ttm_dma_tt *ttm_dma);
 
 /**
  * ttm_ttm_bind:
@@ -633,7 +633,7 @@ extern void ttm_dma_tt_fini(struct ttm_dma_tt *ttm_dma);
  *
  * Bind the pages of @ttm to an aperture location identified by @bo_mem
  */
-extern int ttm_tt_bind(struct ttm_tt *ttm, struct ttm_mem_reg *bo_mem);
+int ttm_tt_bind(struct ttm_tt *ttm, struct ttm_mem_reg *bo_mem);
 
 /**
  * ttm_ttm_destroy:
@@ -642,7 +642,7 @@ extern int ttm_tt_bind(struct ttm_tt *ttm, struct 
ttm_mem_reg *bo_mem);
  *
  * Unbind, unpopulate and destroy common struct ttm_tt.
  */
-extern void ttm_tt_destroy(struct ttm_tt *ttm);
+void ttm_tt_destroy(struct ttm_tt *ttm);
 
 /**
  * ttm_ttm_unbind:
@@ -651,7 +651,7 @@ extern void ttm_tt_destroy(struct ttm_tt *ttm);
  *
  * Unbind a struct ttm_tt.
  */
-extern void ttm_tt_unbind(struct ttm_tt *ttm);
+void ttm_tt_unbind(struct ttm_tt *ttm);
 
 /**
  * ttm_tt_swapin:
@@ -660,7 +660,7 @@ extern void ttm_tt_unbind(struct ttm_tt *ttm);
  *
  * Swap in a previously swap out ttm_tt.
  */
-extern int ttm_tt_swapin(struct ttm_tt *ttm);
+int ttm_tt_swapin(struct ttm_tt *ttm);
 
 /**
  * ttm_tt_set_placement_caching:
@@ -675,9 +675,8 @@ extern int ttm_tt_swapin(struct ttm_tt *ttm);
  * hit RAM. This function may be very costly as it involves global TLB
  * and cache flushes and potential page splitting / combining.
  */
-extern int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t 
placement);
-extern int ttm_tt_swapout(struct ttm_tt *ttm,
- struct file *persistent_swap_storage);
+int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t placement);
+int ttm_tt_swapout(struct ttm_tt *ttm, struct file *persistent_swap_storage);
 
 /**
  * ttm_tt_unpopulate - free pages from a ttm
@@ -686,7 +685,7 @@ extern int ttm_tt_swapout(struct ttm_tt *ttm,
  *
  * Calls the driver method to free all pages from a ttm
  */
-extern void ttm_tt_unpopulate(struct ttm_tt *ttm);
+void ttm_tt_unpopulate(struct ttm_tt *ttm);
 
 /*
  * ttm_bo.c
@@ -701,8 +700,7 @@ extern void ttm_tt_unpopulate(struct ttm_tt *ttm);
  * Returns true if the memory described by @mem is PCI memory,
  * false otherwise.
  */
-extern bool ttm_mem_reg_is_pci(struct ttm_bo_device *bdev,
-  struct ttm_mem_reg *mem);
+bool ttm_mem_reg_is_pci(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem);
 
 /**
  * ttm_bo_mem_space
@@ -723,21 +721,20 @@ extern bool ttm_mem_reg_is_pci(struct ttm_bo_device *bdev,
  * fragmentation or concurrent allocators.
  * -ERESTARTSYS: An interruptible sleep was interrupted by a signal.
  */
-extern int ttm_bo_mem_space(struct ttm_buffer_object *bo,
-   struct ttm_placement *placement,
-   struct ttm_mem_reg *mem,
-   bool interruptible,
-   bool no_wait_gpu);
-
-extern void ttm_bo_mem_put(struct ttm_buffer_object *bo,
+int ttm_bo_mem_space(struct ttm_buffer_object *bo,
+struct ttm_placement *placement,
+struct ttm_mem_reg *mem,
+bool interruptible,
+bool no_wait_gpu);
+
+void ttm_bo_mem_put(struct ttm_buffer_object *bo, struct ttm_mem_reg *mem);
+void ttm_bo_mem_put_locked(struct ttm_buffer_object *bo,

[PATCH 05/11] drm/ttm: use an operation ctx for ttm_bo_init_reserved

[Christian König]

From: Christian König 

Instead of specifying if sleeping should be interruptible.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c |  3 ++-
 drivers/gpu/drm/ttm/ttm_bo.c   | 12 +---
 include/drm/ttm/ttm_bo_api.h   |  5 ++---
 3 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 5ef9be6..364d894 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -319,6 +319,7 @@ int amdgpu_bo_create_restricted(struct amdgpu_device *adev,
struct reservation_object *resv,
struct amdgpu_bo **bo_ptr)
 {
+   struct ttm_operation_ctx ctx = { !kernel, false };
struct amdgpu_bo *bo;
enum ttm_bo_type type;
unsigned long page_align;
@@ -396,7 +397,7 @@ int amdgpu_bo_create_restricted(struct amdgpu_device *adev,
 
initial_bytes_moved = atomic64_read(>num_bytes_moved);
r = ttm_bo_init_reserved(>mman.bdev, >tbo, size, type,
->placement, page_align, !kernel, NULL,
+>placement, page_align, , NULL,
 acc_size, sg, resv, _ttm_bo_destroy);
amdgpu_cs_report_moved_bytes(adev,
atomic64_read(>num_bytes_moved) - initial_bytes_moved);
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 10707923..f1e8833 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1130,7 +1130,7 @@ int ttm_bo_init_reserved(struct ttm_bo_device *bdev,
 enum ttm_bo_type type,
 struct ttm_placement *placement,
 uint32_t page_alignment,
-bool interruptible,
+struct ttm_operation_ctx *ctx,
 struct file *persistent_swap_storage,
 size_t acc_size,
 struct sg_table *sg,
@@ -1216,11 +1216,8 @@ int ttm_bo_init_reserved(struct ttm_bo_device *bdev,
WARN_ON(!locked);
}
 
-   if (likely(!ret)) {
-   struct ttm_operation_ctx ctx = { interruptible, false };
-
-   ret = ttm_bo_validate(bo, placement, );
-   }
+   if (likely(!ret))
+   ret = ttm_bo_validate(bo, placement, ctx);
 
if (unlikely(ret)) {
if (!resv)
@@ -1253,10 +1250,11 @@ int ttm_bo_init(struct ttm_bo_device *bdev,
struct reservation_object *resv,
void (*destroy) (struct ttm_buffer_object *))
 {
+   struct ttm_operation_ctx ctx = { interruptible, false };
int ret;
 
ret = ttm_bo_init_reserved(bdev, bo, size, type, placement,
-  page_alignment, interruptible,
+  page_alignment, ,
   persistent_swap_storage, acc_size,
   sg, resv, destroy);
if (ret)
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index 25c1ca8..4b4e9e0 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -468,8 +468,7 @@ size_t ttm_bo_dma_acc_size(struct ttm_bo_device *bdev,
  * @type: Requested type of buffer object.
  * @flags: Initial placement flags.
  * @page_alignment: Data alignment in pages.
- * @interruptible: If needing to sleep to wait for GPU resources,
- * sleep interruptible.
+ * @ctx: TTM operation context for memory allocation.
  * @persistent_swap_storage: Usually the swap storage is deleted for buffers
  * pinned in physical memory. If this behaviour is not desired, this member
  * holds a pointer to a persistent shmem object. Typically, this would
@@ -506,7 +505,7 @@ int ttm_bo_init_reserved(struct ttm_bo_device *bdev,
 enum ttm_bo_type type,
 struct ttm_placement *placement,
 uint32_t page_alignment,
-bool interrubtible,
+struct ttm_operation_ctx *ctx,
 struct file *persistent_swap_storage,
 size_t acc_size,
 struct sg_table *sg,
-- 
2.5.0

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

Re: [RFC v2 0/7] drm: asynchronous atomic plane update

[Ville Syrjälä]

On Thu, Apr 27, 2017 at 12:15:12PM -0300, Gustavo Padovan wrote:
> From: Gustavo Padovan 
> 
> Hi,
> 
> Second take of Asynchronous Plane Updates over Atomic. Here I looked
> to msm, vc4 and i915 to identify a common pattern to create atomic helpers
> for async updates. So in patch 1 drm_atomic_async_check() and
> drm_atomic_helper_async_commit() are introduced along with driver's plane 
> hooks:
> ->atomic_async_check() and ->atomic_async_commit().
> 
> For now we only support async update for one plane at a time. Also the async
> update can't modify the CRTC so no modesets are allowed.
> 
> Then the other patches add support for it in the drivers. I did virtio mostly
> for testing. i915 have been converted and I've been using it without any
> problem. IGT tests seems to be fine, but there are somewhat random failures
> with or without the async update changes. msm and vc4 are only compile-tested.
> So I think this needs more testing
> 
> I started IGT changes to test the Atomic IOCTL with the new flag:
> 
> https://git.collabora.com/cgit/user/padovan/intel-gpu-tools.git/
> 
> v2:
> 
> Apart from all comments on v1 one extra change I made was to remove the
> constraint of only updating the plane if the queued state didn't touch
> that plane. I believe it was a too cautious of a change, furthermore this
> constraint was affecting throughput negatively on i915.

So you're now allowing reordering the updates? As in update A is
scheduled before update B, but update B happens before update A.
That is not a good idea.

> 
> TODO
> 
>  - improve i-g-t tests where needed
>  - support async page flips (that can be done after uptreaming this part)
>  - figure out what to do for hw that do not update the plane until the next
>  vblank. Maybe wait and see what they do and them extract helpers?
> 
> Comments are welcome!
> 
> Gustavo Padovan (7):
>   drm/atomic: initial support for asynchronous plane update
>   drm/virtio: support async cursor updates
>   drm/i915: update cursors asynchronously through atomic
>   drm/i915: remove intel_cursor_plane_funcs
>   drm/msm: update cursors asynchronously through atomic
>   drm/msm: remove mdp5_cursor_plane_funcs
>   drm/vc4: update cursors asynchronously through atomic
> 
>  drivers/gpu/drm/drm_atomic.c  |  50 ++
>  drivers/gpu/drm/drm_atomic_helper.c   |  48 +
>  drivers/gpu/drm/i915/intel_atomic_plane.c |  52 ++
>  drivers/gpu/drm/i915/intel_display.c  | 158 -
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 161 
> ++
>  drivers/gpu/drm/vc4/vc4_plane.c   |  94 +
>  drivers/gpu/drm/virtio/virtgpu_plane.c|  42 
>  include/drm/drm_atomic.h  |   2 +
>  include/drm/drm_atomic_helper.h   |   2 +
>  include/drm/drm_modeset_helper_vtables.h  |  45 +
>  include/uapi/drm/drm_mode.h   |   4 +-
>  11 files changed, 343 insertions(+), 315 deletions(-)
> 
> -- 
> 2.9.3
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel OTC
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC v2 3/7] drm/i915: update cursors asynchronously through atomic

[Ville Syrjälä]

On Thu, Apr 27, 2017 at 12:15:15PM -0300, Gustavo Padovan wrote:
> From: Gustavo Padovan 
> 
> Add support to async updates of cursors by using the new atomic
> interface for that. Basically what this commit does is do what
> intel_legacy_cursor_update() did but through atomic.
> 
> v2: move fb setting to core and use new state (Eric Anholt)
> 
> Cc: Daniel Vetter 
> Signed-off-by: Gustavo Padovan 
> ---
>  drivers/gpu/drm/i915/intel_atomic_plane.c |  52 +++
>  drivers/gpu/drm/i915/intel_display.c  | 147 
> +-
>  2 files changed, 73 insertions(+), 126 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c 
> b/drivers/gpu/drm/i915/intel_atomic_plane.c
> index cfb4729..c5d0596 100644
> --- a/drivers/gpu/drm/i915/intel_atomic_plane.c
> +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
> @@ -246,11 +246,63 @@ static void intel_plane_atomic_update(struct drm_plane 
> *plane,
>   }
>  }
>  
> +static int intel_plane_atomic_async_check(struct drm_plane *plane,
> +   struct drm_plane_state *state)
> +{
> + struct drm_crtc *crtc = plane->state->crtc;
> + struct drm_crtc_state *crtc_state = crtc->state;
> +
> + if (plane->type != DRM_PLANE_TYPE_CURSOR)
> + return -EINVAL;
> +
> + /*
> +  * When crtc is inactive or there is a modeset pending,
> +  * wait for it to complete in the slowpath
> +  */
> + if (!crtc_state->active || to_intel_crtc_state(crtc_state)->update_pipe)
> + return -EINVAL;
> +
> + /* Only changing fb should be in the fastpath.  */

No, we want cursor movement there as well. It's somewhat impossible
to see from this code now that the core has the size checks. But even
so the comment should not lie.

> + if (!plane->state->fb != !state->fb)
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +static void intel_plane_atomic_async_update(struct drm_plane *plane,
> + struct drm_plane_state *new_state)
> +{
> + struct intel_plane *intel_plane = to_intel_plane(plane);
> + struct drm_crtc *crtc = plane->state->crtc;
> +
> + i915_gem_track_fb(intel_fb_obj(plane->state->fb),
> +   intel_fb_obj(new_state->fb),
> +   intel_plane->frontbuffer_bit);
> +
> + *to_intel_plane_state(plane->state) = *to_intel_plane_state(new_state);
> + to_intel_plane_state(new_state)->vma =
> + to_intel_plane_state(plane->state)->vma;
> +
> + plane->state->visible = new_state->visible;
> +
> + if (plane->state->visible) {
> + trace_intel_update_plane(plane, to_intel_crtc(crtc));
> + intel_plane->update_plane(plane,
> +   to_intel_crtc_state(crtc->state),
> +   to_intel_plane_state(new_state));
> + } else {
> + trace_intel_disable_plane(plane, to_intel_crtc(crtc));
> + intel_plane->disable_plane(plane, crtc);
> + }
> +}
> +
>  const struct drm_plane_helper_funcs intel_plane_helper_funcs = {
>   .prepare_fb = intel_prepare_plane_fb,
>   .cleanup_fb = intel_cleanup_plane_fb,
>   .atomic_check = intel_plane_atomic_check,
>   .atomic_update = intel_plane_atomic_update,
> + .atomic_async_check = intel_plane_atomic_async_check,
> + .atomic_async_update = intel_plane_atomic_async_update,

NAK. We don't want these "async" updates for anything but cursors.

>  };
>  
>  /**
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index e217d04..c854c87 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12993,13 +12993,29 @@ static int intel_atomic_commit(struct drm_device 
> *dev,
>   int ret = 0;
>  
>   /*
> -  * The intel_legacy_cursor_update() fast path takes care
> +  * The atomic async update fast path takes care
>* of avoiding the vblank waits for simple cursor
>* movement and flips. For cursor on/off and size changes,
>* we want to perform the vblank waits so that watermark
>* updates happen during the correct frames. Gen9+ have
>* double buffered watermarks and so shouldn't need this.
>*/
> + if (state->async_update) {
> + ret = mutex_lock_interruptible(>struct_mutex);
> + if (ret)
> + return ret;
> +
> + ret = drm_atomic_helper_prepare_planes(dev, state);
> + mutex_unlock(>struct_mutex);
> +
> + drm_atomic_helper_async_commit(dev, state);
> +
> + mutex_lock(>struct_mutex);
> + drm_atomic_helper_cleanup_planes(dev, state);
> + mutex_unlock(>struct_mutex);
> + return 0;
> + }
> +
>   if 

Re: [RFC v2 1/7] drm/atomic: initial support for asynchronous plane update

[Ville Syrjälä]

On Thu, Apr 27, 2017 at 12:15:13PM -0300, Gustavo Padovan wrote:
> From: Gustavo Padovan 
> 
> In some cases, like cursor updates, it is interesting to update the
> plane in an asynchronous fashion to avoid big delays. The current queued
> update could be still waiting for a fence to signal and thus block any
> subsequent update until its scan out. In cases like this if we update the
> cursor synchronously through the atomic API it will cause significant
> delays that would even be noticed by the final user.
> 
> This patch creates a fast path to jump ahead the current queued state and
> do single planes updates without going through all atomic step in
> drm_atomic_helper_commit().
> 
> We take this path for legacy cursor updates. Users can also set the
> DRM_MODE_ATOMIC_ASYNC_UPDATE flag on atomic updates.
> 
> For now only single plane updates are supported, but we plan to support
> multiple planes updates and async PageFlips through this interface as well
> in the near future.
> 
> v2:
>   - allow updates even if there is a queued update for the same
>   plane.
> - fixes on the documentation (Emil Velikov)
> - unconditionally call ->atomic_async_update (Emil Velikov)
> - check for ->atomic_async_update earlier (Daniel Vetter)
> - make ->atomic_async_check() the last step (Daniel Vetter)
> - add ASYNC_UPDATE flag (Eric Anholt)
> - update state in core after ->atomic_async_update (Eric Anholt)
>   - update docs (Eric Anholt)
> 
> Cc: Daniel Vetter 
> Cc: Rob Clark 
> Cc: Eric Anholt 
> Signed-off-by: Gustavo Padovan 
> ---
>  drivers/gpu/drm/drm_atomic.c | 50 
> 
>  drivers/gpu/drm/drm_atomic_helper.c  | 48 ++
>  include/drm/drm_atomic.h |  2 ++
>  include/drm/drm_atomic_helper.h  |  2 ++
>  include/drm/drm_modeset_helper_vtables.h | 45 
>  include/uapi/drm/drm_mode.h  |  4 ++-
>  6 files changed, 150 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 30229ab..7b60cf8 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -77,6 +77,7 @@ drm_atomic_state_init(struct drm_device *dev, struct 
> drm_atomic_state *state)
>* setting this appropriately?
>*/
>   state->allow_modeset = true;
> + state->async_update = true;
>  
>   state->crtcs = kcalloc(dev->mode_config.num_crtc,
>  sizeof(*state->crtcs), GFP_KERNEL);
> @@ -631,6 +632,51 @@ static int drm_atomic_crtc_check(struct drm_crtc *crtc,
>   return 0;
>  }
>  
> +static bool drm_atomic_async_check(struct drm_atomic_state *state)
> +{
> + struct drm_crtc *crtc;
> + struct drm_crtc_state *crtc_state;
> + struct drm_plane *__plane, *plane = NULL;
> + struct drm_plane_state *__plane_state, *plane_state = NULL;
> + const struct drm_plane_helper_funcs *funcs;
> + int i, n_planes = 0;
> +
> + for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
> + if (drm_atomic_crtc_needs_modeset(crtc_state))
> + return false;
> + }
> +
> + for_each_new_plane_in_state(state, __plane, __plane_state, i) {
> + n_planes++;
> + plane = __plane;
> + plane_state = __plane_state;
> + }
> +
> + /* FIXME: we support single plane updates for now */
> + if (!plane || n_planes != 1)
> + return false;
> +
> + funcs = plane->helper_private;
> + if (!funcs->atomic_async_update)
> + return false;
> +
> + if (plane_state->fence)
> + return false;
> +
> + if (!plane->state->crtc || (plane->state->crtc != plane_state->crtc))
> + return false;
> +
> + /* No configuring new scaling in the async path. */

Those checks aren't really about scaling. Well, they are also about
scaling, but they're mainly about changing size.

What I don't really understand is why we're enforcing these restrictions
in the core but leaving other restrictions up to the driver. I don't see
size changes as anything really special compared to many of the other
restrictions that would now be up to each driver.

If you're really after some lowest common denominator as far as
exposed capabilities are concerned then I think the core should do
more checking. OTOH if you're not interested limiting what each
driver exposes then I don't see why the core checks anything at all.

> + if (plane->state->crtc_w != plane_state->crtc_w ||
> + plane->state->crtc_h != plane_state->crtc_h ||
> + plane->state->src_w != plane_state->src_w ||
> + plane->state->src_h != plane_state->src_h) {
> + return false;
> + }
> +
> + return 

[RFC v2 7/7] drm/vc4: update cursors asynchronously through atomic

[Gustavo Padovan]

From: Gustavo Padovan 

Add support to async updates of cursors by using the new atomic
interface for that. Basically what this commit does is do what
vc4_update_plane() did but through atomic.

v2: move fb setting to core and use new state (Eric Anholt)

Cc: Eric Anholt 
Signed-off-by: Gustavo Padovan 
---
 drivers/gpu/drm/vc4/vc4_plane.c | 94 -
 1 file changed, 26 insertions(+), 68 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c
index d34cd53..c42c610 100644
--- a/drivers/gpu/drm/vc4/vc4_plane.c
+++ b/drivers/gpu/drm/vc4/vc4_plane.c
@@ -735,70 +735,25 @@ void vc4_plane_async_set_fb(struct drm_plane *plane, 
struct drm_framebuffer *fb)
vc4_state->dlist[vc4_state->ptr0_offset] = addr;
 }
 
-static const struct drm_plane_helper_funcs vc4_plane_helper_funcs = {
-   .atomic_check = vc4_plane_atomic_check,
-   .atomic_update = vc4_plane_atomic_update,
-};
-
-static void vc4_plane_destroy(struct drm_plane *plane)
+static int vc4_plane_atomic_async_check(struct drm_plane *plane,
+   struct drm_plane_state *state)
 {
-   drm_plane_helper_disable(plane);
-   drm_plane_cleanup(plane);
-}
-
-/* Implements immediate (non-vblank-synced) updates of the cursor
- * position, or falls back to the atomic helper otherwise.
- */
-static int
-vc4_update_plane(struct drm_plane *plane,
-struct drm_crtc *crtc,
-struct drm_framebuffer *fb,
-int crtc_x, int crtc_y,
-unsigned int crtc_w, unsigned int crtc_h,
-uint32_t src_x, uint32_t src_y,
-uint32_t src_w, uint32_t src_h,
-struct drm_modeset_acquire_ctx *ctx)
-{
-   struct drm_plane_state *plane_state;
-   struct vc4_plane_state *vc4_state;
-
-   if (plane != crtc->cursor)
-   goto out;
-
-   plane_state = plane->state;
-   vc4_state = to_vc4_plane_state(plane_state);
-
-   if (!plane_state)
-   goto out;
-
-   /* No configuring new scaling in the fast path. */
-   if (crtc_w != plane_state->crtc_w ||
-   crtc_h != plane_state->crtc_h ||
-   src_w != plane_state->src_w ||
-   src_h != plane_state->src_h) {
-   goto out;
-   }
+   if (plane != state->crtc->cursor)
+   return -EINVAL;
 
-   if (fb != plane_state->fb) {
-   drm_atomic_set_fb_for_plane(plane->state, fb);
-   vc4_plane_async_set_fb(plane, fb);
-   }
+   if (!plane->state)
+   return -EINVAL;
 
-   /* Set the cursor's position on the screen.  This is the
-* expected change from the drm_mode_cursor_universal()
-* helper.
-*/
-   plane_state->crtc_x = crtc_x;
-   plane_state->crtc_y = crtc_y;
+   return 0;
+}
 
-   /* Allow changing the start position within the cursor BO, if
-* that matters.
-*/
-   plane_state->src_x = src_x;
-   plane_state->src_y = src_y;
+static void vc4_plane_atomic_async_update(struct drm_plane *plane,
+ struct drm_plane_state *new_state)
+{
+   struct vc4_plane_state *vc4_state = to_vc4_plane_state(plane->state);
 
-   /* Update the display list based on the new crtc_x/y. */
-   vc4_plane_atomic_check(plane, plane_state);
+   if (plane->state->fb != new_state->fb)
+   vc4_plane_async_set_fb(plane, new_state->fb);
 
/* Note that we can't just call vc4_plane_write_dlist()
 * because that would smash the context data that the HVS is
@@ -810,20 +765,23 @@ vc4_update_plane(struct drm_plane *plane,
   _state->hw_dlist[vc4_state->pos2_offset]);
writel(vc4_state->dlist[vc4_state->ptr0_offset],
   _state->hw_dlist[vc4_state->ptr0_offset]);
+}
 
-   return 0;
+static const struct drm_plane_helper_funcs vc4_plane_helper_funcs = {
+   .atomic_check = vc4_plane_atomic_check,
+   .atomic_update = vc4_plane_atomic_update,
+   .atomic_async_check = vc4_plane_atomic_async_check,
+   .atomic_async_update = vc4_plane_atomic_async_update,
+};
 
-out:
-   return drm_atomic_helper_update_plane(plane, crtc, fb,
- crtc_x, crtc_y,
- crtc_w, crtc_h,
- src_x, src_y,
- src_w, src_h,
- ctx);
+static void vc4_plane_destroy(struct drm_plane *plane)
+{
+   drm_plane_helper_disable(plane);
+   drm_plane_cleanup(plane);
 }
 
 static const struct drm_plane_funcs vc4_plane_funcs = {
-   .update_plane = vc4_update_plane,
+   .update_plane = drm_atomic_helper_update_plane,
.disable_plane = 

[RFC v2 5/7] drm/msm: update cursors asynchronously through atomic

[Gustavo Padovan]

From: Gustavo Padovan 

Add support to async updates of cursors by using the new atomic
interface for that. Basically what this commit does is do what
mdp5_update_cursor_plane_legacy() did but through atomic.

v2: move fb setting to core and use new state (Eric Anholt)

Cc: Rob Clark 
Signed-off-by: Gustavo Padovan 
---
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 137 +++---
 1 file changed, 50 insertions(+), 87 deletions(-)

diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c 
b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
index 60a5451..d232bd5 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
@@ -31,15 +31,6 @@ static int mdp5_plane_mode_set(struct drm_plane *plane,
struct drm_crtc *crtc, struct drm_framebuffer *fb,
struct drm_rect *src, struct drm_rect *dest);
 
-static int mdp5_update_cursor_plane_legacy(struct drm_plane *plane,
-   struct drm_crtc *crtc,
-   struct drm_framebuffer *fb,
-   int crtc_x, int crtc_y,
-   unsigned int crtc_w, unsigned int crtc_h,
-   uint32_t src_x, uint32_t src_y,
-   uint32_t src_w, uint32_t src_h,
-   struct drm_modeset_acquire_ctx *ctx);
-
 static void set_scanout_locked(struct drm_plane *plane,
struct drm_framebuffer *fb);
 
@@ -253,7 +244,7 @@ static const struct drm_plane_funcs mdp5_plane_funcs = {
 };
 
 static const struct drm_plane_funcs mdp5_cursor_plane_funcs = {
-   .update_plane = mdp5_update_cursor_plane_legacy,
+   .update_plane = drm_atomic_helper_update_plane,
.disable_plane = drm_atomic_helper_disable_plane,
.destroy = mdp5_plane_destroy,
.set_property = drm_atomic_helper_plane_set_property,
@@ -430,11 +421,60 @@ static void mdp5_plane_atomic_update(struct drm_plane 
*plane,
}
 }
 
+static int mdp5_plane_atomic_async_check(struct drm_plane *plane,
+struct drm_plane_state *state)
+{
+   struct mdp5_plane_state *mdp5_state = to_mdp5_plane_state(state);
+   struct drm_crtc_state *crtc_state;
+
+   crtc_state = drm_atomic_get_existing_crtc_state(state->state,
+   state->crtc);
+   if (WARN_ON(!crtc_state))
+   return -EINVAL;
+
+   if (!crtc_state->active)
+   return -EINVAL;
+
+   mdp5_state = to_mdp5_plane_state(state);
+
+   /* don't use fast path if we don't have a hwpipe allocated yet */
+   if (!mdp5_state->hwpipe)
+   return -EINVAL;
+
+   /* only allow changing of position(crtc x/y or src x/y) in fast path */
+   if (!state->fb || plane->state->fb != state->fb)
+   return -EINVAL;
+
+   return 0;
+}
+
+static void mdp5_plane_atomic_async_update(struct drm_plane *plane,
+  struct drm_plane_state *new_state)
+{
+   if (plane_enabled(new_state)) {
+   struct mdp5_ctl *ctl;
+   int ret;
+
+   ret = mdp5_plane_mode_set(plane, new_state->crtc, new_state->fb,
+   _state->src, _state->dst);
+   WARN_ON(ret < 0);
+
+   ctl = mdp5_crtc_get_ctl(new_state->crtc);
+
+   mdp5_ctl_commit(ctl, mdp5_plane_get_flush(plane));
+   }
+
+   *to_mdp5_plane_state(plane->state) =
+   *to_mdp5_plane_state(new_state);
+}
+
 static const struct drm_plane_helper_funcs mdp5_plane_helper_funcs = {
.prepare_fb = mdp5_plane_prepare_fb,
.cleanup_fb = mdp5_plane_cleanup_fb,
.atomic_check = mdp5_plane_atomic_check,
.atomic_update = mdp5_plane_atomic_update,
+   .atomic_async_check = mdp5_plane_atomic_async_check,
+   .atomic_async_update = mdp5_plane_atomic_async_update,
 };
 
 static void set_scanout_locked(struct drm_plane *plane,
@@ -882,83 +922,6 @@ static int mdp5_plane_mode_set(struct drm_plane *plane,
return ret;
 }
 
-static int mdp5_update_cursor_plane_legacy(struct drm_plane *plane,
-   struct drm_crtc *crtc, struct drm_framebuffer *fb,
-   int crtc_x, int crtc_y,
-   unsigned int crtc_w, unsigned int crtc_h,
-   uint32_t src_x, uint32_t src_y,
-   uint32_t src_w, uint32_t src_h,
-   struct drm_modeset_acquire_ctx *ctx)
-{
-   struct drm_plane_state *plane_state, *new_plane_state;
-   struct mdp5_plane_state *mdp5_pstate;
-   struct drm_crtc_state *crtc_state = crtc->state;
-   int ret;
-
-   if (!crtc_state->active || drm_atomic_crtc_needs_modeset(crtc_state))
-   goto slow;
-
-   plane_state = plane->state;
-   

[RFC v2 1/7] drm/atomic: initial support for asynchronous plane update

[Gustavo Padovan]

From: Gustavo Padovan 

In some cases, like cursor updates, it is interesting to update the
plane in an asynchronous fashion to avoid big delays. The current queued
update could be still waiting for a fence to signal and thus block any
subsequent update until its scan out. In cases like this if we update the
cursor synchronously through the atomic API it will cause significant
delays that would even be noticed by the final user.

This patch creates a fast path to jump ahead the current queued state and
do single planes updates without going through all atomic step in
drm_atomic_helper_commit().

We take this path for legacy cursor updates. Users can also set the
DRM_MODE_ATOMIC_ASYNC_UPDATE flag on atomic updates.

For now only single plane updates are supported, but we plan to support
multiple planes updates and async PageFlips through this interface as well
in the near future.

v2:
- allow updates even if there is a queued update for the same
plane.
- fixes on the documentation (Emil Velikov)
- unconditionally call ->atomic_async_update (Emil Velikov)
- check for ->atomic_async_update earlier (Daniel Vetter)
- make ->atomic_async_check() the last step (Daniel Vetter)
- add ASYNC_UPDATE flag (Eric Anholt)
- update state in core after ->atomic_async_update (Eric Anholt)
- update docs (Eric Anholt)

Cc: Daniel Vetter 
Cc: Rob Clark 
Cc: Eric Anholt 
Signed-off-by: Gustavo Padovan 
---
 drivers/gpu/drm/drm_atomic.c | 50 
 drivers/gpu/drm/drm_atomic_helper.c  | 48 ++
 include/drm/drm_atomic.h |  2 ++
 include/drm/drm_atomic_helper.h  |  2 ++
 include/drm/drm_modeset_helper_vtables.h | 45 
 include/uapi/drm/drm_mode.h  |  4 ++-
 6 files changed, 150 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 30229ab..7b60cf8 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -77,6 +77,7 @@ drm_atomic_state_init(struct drm_device *dev, struct 
drm_atomic_state *state)
 * setting this appropriately?
 */
state->allow_modeset = true;
+   state->async_update = true;
 
state->crtcs = kcalloc(dev->mode_config.num_crtc,
   sizeof(*state->crtcs), GFP_KERNEL);
@@ -631,6 +632,51 @@ static int drm_atomic_crtc_check(struct drm_crtc *crtc,
return 0;
 }
 
+static bool drm_atomic_async_check(struct drm_atomic_state *state)
+{
+   struct drm_crtc *crtc;
+   struct drm_crtc_state *crtc_state;
+   struct drm_plane *__plane, *plane = NULL;
+   struct drm_plane_state *__plane_state, *plane_state = NULL;
+   const struct drm_plane_helper_funcs *funcs;
+   int i, n_planes = 0;
+
+   for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
+   if (drm_atomic_crtc_needs_modeset(crtc_state))
+   return false;
+   }
+
+   for_each_new_plane_in_state(state, __plane, __plane_state, i) {
+   n_planes++;
+   plane = __plane;
+   plane_state = __plane_state;
+   }
+
+   /* FIXME: we support single plane updates for now */
+   if (!plane || n_planes != 1)
+   return false;
+
+   funcs = plane->helper_private;
+   if (!funcs->atomic_async_update)
+   return false;
+
+   if (plane_state->fence)
+   return false;
+
+   if (!plane->state->crtc || (plane->state->crtc != plane_state->crtc))
+   return false;
+
+   /* No configuring new scaling in the async path. */
+   if (plane->state->crtc_w != plane_state->crtc_w ||
+   plane->state->crtc_h != plane_state->crtc_h ||
+   plane->state->src_w != plane_state->src_w ||
+   plane->state->src_h != plane_state->src_h) {
+   return false;
+   }
+
+   return !funcs->atomic_async_check(plane, plane_state);
+}
+
 static void drm_atomic_crtc_print_state(struct drm_printer *p,
const struct drm_crtc_state *state)
 {
@@ -1591,6 +1637,9 @@ int drm_atomic_check_only(struct drm_atomic_state *state)
}
}
 
+   if (state->async_update || state->legacy_cursor_update)
+   state->async_update = drm_atomic_async_check(state);
+
return ret;
 }
 EXPORT_SYMBOL(drm_atomic_check_only);
@@ -2132,6 +2181,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
 
state->acquire_ctx = 
state->allow_modeset = !!(arg->flags & DRM_MODE_ATOMIC_ALLOW_MODESET);
+   state->async_update = !!(arg->flags & DRM_MODE_ATOMIC_ASYNC_UPDATE);
 
 retry:
plane_mask = 0;
diff --git a/drivers/gpu/drm/drm_atomic_helper.c 

[RFC v2 2/7] drm/virtio: support async cursor updates

[Gustavo Padovan]

From: Gustavo Padovan 

Short circuit the update path for cursors and use the drm async update
infrastructure.

v2: move fb setting to core and use new state (Eric Anholt)

Signed-off-by: Gustavo Padovan 

---
I wrote this mostly for testing purposes, not sure if its something that
we actually need for virtio.
---
 drivers/gpu/drm/virtio/virtgpu_plane.c | 42 ++
 1 file changed, 42 insertions(+)

diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c 
b/drivers/gpu/drm/virtio/virtgpu_plane.c
index 76d5fed..783f146 100644
--- a/drivers/gpu/drm/virtio/virtgpu_plane.c
+++ b/drivers/gpu/drm/virtio/virtgpu_plane.c
@@ -260,6 +260,46 @@ static void virtio_gpu_cursor_plane_update(struct 
drm_plane *plane,
virtio_gpu_cursor_ping(vgdev, output);
 }
 
+static int virtio_gpu_cursor_plane_async_check(struct drm_plane *plane,
+  struct drm_plane_state 
*new_state)
+{
+   struct virtio_gpu_output *output;
+
+   if (!plane->state->crtc)
+   return -EINVAL;
+
+   output = drm_crtc_to_virtio_gpu_output(plane->state->crtc);
+   if (!output)
+   return -EINVAL;
+
+   if (plane->state->fb != new_state->fb)
+   return -EINVAL;
+
+   return 0;
+}
+
+static void virtio_gpu_cursor_plane_async_update(struct drm_plane *plane,
+struct drm_plane_state 
*new_state)
+{
+   struct drm_device *dev = plane->dev;
+   struct virtio_gpu_device *vgdev = dev->dev_private;
+   struct virtio_gpu_output *output = NULL;
+
+   output = drm_crtc_to_virtio_gpu_output(plane->state->crtc);
+   if (WARN_ON(!output))
+   return;
+
+   if (plane->state->fb != new_state->fb)
+   return;
+
+   DRM_DEBUG("move +%d+%d\n", new_state->crtc_x, new_state->crtc_y);
+
+   output->cursor.hdr.type = cpu_to_le32(VIRTIO_GPU_CMD_MOVE_CURSOR);
+   output->cursor.pos.x = cpu_to_le32(new_state->crtc_x);
+   output->cursor.pos.y = cpu_to_le32(new_state->crtc_y);
+   virtio_gpu_cursor_ping(vgdev, output);
+}
+
 static const struct drm_plane_helper_funcs virtio_gpu_primary_helper_funcs = {
.atomic_check   = virtio_gpu_plane_atomic_check,
.atomic_update  = virtio_gpu_primary_plane_update,
@@ -268,6 +308,8 @@ static const struct drm_plane_helper_funcs 
virtio_gpu_primary_helper_funcs = {
 static const struct drm_plane_helper_funcs virtio_gpu_cursor_helper_funcs = {
.atomic_check   = virtio_gpu_plane_atomic_check,
.atomic_update  = virtio_gpu_cursor_plane_update,
+   .atomic_async_check = virtio_gpu_cursor_plane_async_check,
+   .atomic_async_update= virtio_gpu_cursor_plane_async_update,
 };
 
 struct drm_plane *virtio_gpu_plane_init(struct virtio_gpu_device *vgdev,
-- 
2.9.3

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

[RFC v2 6/7] drm/msm: remove mdp5_cursor_plane_funcs

[Gustavo Padovan]

From: Gustavo Padovan 

After converting legacy cursor updates to atomic async commits
mdp5_cursor_plane_funcs just duplicates mdp5_plane_funcs now.

Cc: Rob Clark 
Signed-off-by: Gustavo Padovan 
---
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 26 +++---
 1 file changed, 3 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c 
b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
index d232bd5..563bad5 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
@@ -243,19 +243,6 @@ static const struct drm_plane_funcs mdp5_plane_funcs = {
.atomic_print_state = mdp5_plane_atomic_print_state,
 };
 
-static const struct drm_plane_funcs mdp5_cursor_plane_funcs = {
-   .update_plane = drm_atomic_helper_update_plane,
-   .disable_plane = drm_atomic_helper_disable_plane,
-   .destroy = mdp5_plane_destroy,
-   .set_property = drm_atomic_helper_plane_set_property,
-   .atomic_set_property = mdp5_plane_atomic_set_property,
-   .atomic_get_property = mdp5_plane_atomic_get_property,
-   .reset = mdp5_plane_reset,
-   .atomic_duplicate_state = mdp5_plane_duplicate_state,
-   .atomic_destroy_state = mdp5_plane_destroy_state,
-   .atomic_print_state = mdp5_plane_atomic_print_state,
-};
-
 static int mdp5_plane_prepare_fb(struct drm_plane *plane,
 struct drm_plane_state *new_state)
 {
@@ -961,16 +948,9 @@ struct drm_plane *mdp5_plane_init(struct drm_device *dev,
mdp5_plane->nformats = mdp_get_formats(mdp5_plane->formats,
ARRAY_SIZE(mdp5_plane->formats), false);
 
-   if (type == DRM_PLANE_TYPE_CURSOR)
-   ret = drm_universal_plane_init(dev, plane, 0xff,
-   _cursor_plane_funcs,
-   mdp5_plane->formats, mdp5_plane->nformats,
-   type, NULL);
-   else
-   ret = drm_universal_plane_init(dev, plane, 0xff,
-   _plane_funcs,
-   mdp5_plane->formats, mdp5_plane->nformats,
-   type, NULL);
+   ret = drm_universal_plane_init(dev, plane, 0xff, _plane_funcs,
+  mdp5_plane->formats,
+  mdp5_plane->nformats, type, NULL);
if (ret)
goto fail;
 
-- 
2.9.3

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

[RFC v2 4/7] drm/i915: remove intel_cursor_plane_funcs

[Gustavo Padovan]

From: Gustavo Padovan 

After converting legacy cursor updates to atomic async commits
intel_cursor_plane_funcs just duplicates intel_plane_funcs now.

Cc: Daniel Vetter 
Signed-off-by: Gustavo Padovan 
---
 drivers/gpu/drm/i915/intel_display.c | 13 +
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index c854c87..26d20e5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13379,17 +13379,6 @@ const struct drm_plane_funcs intel_plane_funcs = {
.atomic_destroy_state = intel_plane_destroy_state,
 };
 
-static const struct drm_plane_funcs intel_cursor_plane_funcs = {
-   .update_plane = drm_atomic_helper_update_plane,
-   .disable_plane = drm_atomic_helper_disable_plane,
-   .destroy = intel_plane_destroy,
-   .set_property = drm_atomic_helper_plane_set_property,
-   .atomic_get_property = intel_plane_atomic_get_property,
-   .atomic_set_property = intel_plane_atomic_set_property,
-   .atomic_duplicate_state = intel_plane_duplicate_state,
-   .atomic_destroy_state = intel_plane_destroy_state,
-};
-
 static struct intel_plane *
 intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
 {
@@ -13636,7 +13625,7 @@ intel_cursor_plane_create(struct drm_i915_private 
*dev_priv, enum pipe pipe)
cursor->disable_plane = intel_disable_cursor_plane;
 
ret = drm_universal_plane_init(_priv->drm, >base,
-  0, _cursor_plane_funcs,
+  0, _plane_funcs,
   intel_cursor_formats,
   ARRAY_SIZE(intel_cursor_formats),
   DRM_PLANE_TYPE_CURSOR,
-- 
2.9.3

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

[RFC v2 3/7] drm/i915: update cursors asynchronously through atomic

[Gustavo Padovan]

From: Gustavo Padovan 

Add support to async updates of cursors by using the new atomic
interface for that. Basically what this commit does is do what
intel_legacy_cursor_update() did but through atomic.

v2: move fb setting to core and use new state (Eric Anholt)

Cc: Daniel Vetter 
Signed-off-by: Gustavo Padovan 
---
 drivers/gpu/drm/i915/intel_atomic_plane.c |  52 +++
 drivers/gpu/drm/i915/intel_display.c  | 147 +-
 2 files changed, 73 insertions(+), 126 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c 
b/drivers/gpu/drm/i915/intel_atomic_plane.c
index cfb4729..c5d0596 100644
--- a/drivers/gpu/drm/i915/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
@@ -246,11 +246,63 @@ static void intel_plane_atomic_update(struct drm_plane 
*plane,
}
 }
 
+static int intel_plane_atomic_async_check(struct drm_plane *plane,
+ struct drm_plane_state *state)
+{
+   struct drm_crtc *crtc = plane->state->crtc;
+   struct drm_crtc_state *crtc_state = crtc->state;
+
+   if (plane->type != DRM_PLANE_TYPE_CURSOR)
+   return -EINVAL;
+
+   /*
+* When crtc is inactive or there is a modeset pending,
+* wait for it to complete in the slowpath
+*/
+   if (!crtc_state->active || to_intel_crtc_state(crtc_state)->update_pipe)
+   return -EINVAL;
+
+   /* Only changing fb should be in the fastpath.  */
+   if (!plane->state->fb != !state->fb)
+   return -EINVAL;
+
+   return 0;
+}
+
+static void intel_plane_atomic_async_update(struct drm_plane *plane,
+   struct drm_plane_state *new_state)
+{
+   struct intel_plane *intel_plane = to_intel_plane(plane);
+   struct drm_crtc *crtc = plane->state->crtc;
+
+   i915_gem_track_fb(intel_fb_obj(plane->state->fb),
+ intel_fb_obj(new_state->fb),
+ intel_plane->frontbuffer_bit);
+
+   *to_intel_plane_state(plane->state) = *to_intel_plane_state(new_state);
+   to_intel_plane_state(new_state)->vma =
+   to_intel_plane_state(plane->state)->vma;
+
+   plane->state->visible = new_state->visible;
+
+   if (plane->state->visible) {
+   trace_intel_update_plane(plane, to_intel_crtc(crtc));
+   intel_plane->update_plane(plane,
+ to_intel_crtc_state(crtc->state),
+ to_intel_plane_state(new_state));
+   } else {
+   trace_intel_disable_plane(plane, to_intel_crtc(crtc));
+   intel_plane->disable_plane(plane, crtc);
+   }
+}
+
 const struct drm_plane_helper_funcs intel_plane_helper_funcs = {
.prepare_fb = intel_prepare_plane_fb,
.cleanup_fb = intel_cleanup_plane_fb,
.atomic_check = intel_plane_atomic_check,
.atomic_update = intel_plane_atomic_update,
+   .atomic_async_check = intel_plane_atomic_async_check,
+   .atomic_async_update = intel_plane_atomic_async_update,
 };
 
 /**
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index e217d04..c854c87 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12993,13 +12993,29 @@ static int intel_atomic_commit(struct drm_device *dev,
int ret = 0;
 
/*
-* The intel_legacy_cursor_update() fast path takes care
+* The atomic async update fast path takes care
 * of avoiding the vblank waits for simple cursor
 * movement and flips. For cursor on/off and size changes,
 * we want to perform the vblank waits so that watermark
 * updates happen during the correct frames. Gen9+ have
 * double buffered watermarks and so shouldn't need this.
 */
+   if (state->async_update) {
+   ret = mutex_lock_interruptible(>struct_mutex);
+   if (ret)
+   return ret;
+
+   ret = drm_atomic_helper_prepare_planes(dev, state);
+   mutex_unlock(>struct_mutex);
+
+   drm_atomic_helper_async_commit(dev, state);
+
+   mutex_lock(>struct_mutex);
+   drm_atomic_helper_cleanup_planes(dev, state);
+   mutex_unlock(>struct_mutex);
+   return 0;
+   }
+
if (INTEL_GEN(dev_priv) < 9)
state->legacy_cursor_update = false;
 
@@ -13141,6 +13157,9 @@ intel_prepare_plane_fb(struct drm_plane *plane,
}
}
 
+   if (new_state->state->async_update)
+   return 0;
+
if (!obj && !old_obj)
return 0;
 
@@ -13360,132 +13379,8 @@ const struct drm_plane_funcs intel_plane_funcs = {
.atomic_destroy_state = 

[RFC v2 0/7] drm: asynchronous atomic plane update

[Gustavo Padovan]

From: Gustavo Padovan 

Hi,

Second take of Asynchronous Plane Updates over Atomic. Here I looked
to msm, vc4 and i915 to identify a common pattern to create atomic helpers
for async updates. So in patch 1 drm_atomic_async_check() and
drm_atomic_helper_async_commit() are introduced along with driver's plane hooks:
->atomic_async_check() and ->atomic_async_commit().

For now we only support async update for one plane at a time. Also the async
update can't modify the CRTC so no modesets are allowed.

Then the other patches add support for it in the drivers. I did virtio mostly
for testing. i915 have been converted and I've been using it without any
problem. IGT tests seems to be fine, but there are somewhat random failures
with or without the async update changes. msm and vc4 are only compile-tested.
So I think this needs more testing

I started IGT changes to test the Atomic IOCTL with the new flag:

https://git.collabora.com/cgit/user/padovan/intel-gpu-tools.git/

v2:

Apart from all comments on v1 one extra change I made was to remove the
constraint of only updating the plane if the queued state didn't touch
that plane. I believe it was a too cautious of a change, furthermore this
constraint was affecting throughput negatively on i915.

TODO

 - improve i-g-t tests where needed
 - support async page flips (that can be done after uptreaming this part)
 - figure out what to do for hw that do not update the plane until the next
 vblank. Maybe wait and see what they do and them extract helpers?

Comments are welcome!

Gustavo Padovan (7):
  drm/atomic: initial support for asynchronous plane update
  drm/virtio: support async cursor updates
  drm/i915: update cursors asynchronously through atomic
  drm/i915: remove intel_cursor_plane_funcs
  drm/msm: update cursors asynchronously through atomic
  drm/msm: remove mdp5_cursor_plane_funcs
  drm/vc4: update cursors asynchronously through atomic

 drivers/gpu/drm/drm_atomic.c  |  50 ++
 drivers/gpu/drm/drm_atomic_helper.c   |  48 +
 drivers/gpu/drm/i915/intel_atomic_plane.c |  52 ++
 drivers/gpu/drm/i915/intel_display.c  | 158 -
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 161 ++
 drivers/gpu/drm/vc4/vc4_plane.c   |  94 +
 drivers/gpu/drm/virtio/virtgpu_plane.c|  42 
 include/drm/drm_atomic.h  |   2 +
 include/drm/drm_atomic_helper.h   |   2 +
 include/drm/drm_modeset_helper_vtables.h  |  45 +
 include/uapi/drm/drm_mode.h   |   4 +-
 11 files changed, 343 insertions(+), 315 deletions(-)

-- 
2.9.3

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

[PATCH] drm/atomic: fix doc to use new name for commit types

[Gustavo Padovan]

From: Gustavo Padovan 

Use "non-blocking" and "blocking" instead of old names.

Signed-off-by: Gustavo Padovan 
---
 drivers/gpu/drm/drm_atomic_helper.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
b/drivers/gpu/drm/drm_atomic_helper.c
index cfeda5f..5a3c9c0 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1070,8 +1070,8 @@ EXPORT_SYMBOL(drm_atomic_helper_commit_modeset_enables);
  *
  * Note that @pre_swap is needed since the point where we block for fences 
moves
  * around depending upon whether an atomic commit is blocking or
- * non-blocking. For async commit all waiting needs to happen after
- * drm_atomic_helper_swap_state() is called, but for synchronous commits we 
want
+ * non-blocking. For non-blocking commit all waiting needs to happen after
+ * drm_atomic_helper_swap_state() is called, but for blocking commits we want
  * to wait **before** we do anything that can't be easily rolled back. That is
  * before we call drm_atomic_helper_swap_state().
  *
-- 
2.9.3

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

Re: [RFC 0/4] Exynos DRM: add Picture Processor extension

[Marek Szyprowski]

Hi Tobias and Nicolas,

On 2017-04-26 17:16, Tobias Jakobi wrote:

Nicolas Dufresne wrote:

Le mercredi 26 avril 2017 à 01:21 +0300, Sakari Ailus a écrit :

Hi Marek,

On Thu, Apr 20, 2017 at 01:23:09PM +0200, Marek Szyprowski wrote:

Hi Laurent,

On 2017-04-20 12:25, Laurent Pinchart wrote:

Hi Marek,

(CC'ing Sakari Ailus)

Thank you for the patches.

On Thursday 20 Apr 2017 11:13:36 Marek Szyprowski wrote:

Dear all,

This is an updated proposal for extending EXYNOS DRM API with generic
support for hardware modules, which can be used for processing image data
from the one memory buffer to another. Typical memory-to-memory operations
are: rotation, scaling, colour space conversion or mix of them. This is a
follow-up of my previous proposal "[RFC 0/2] New feature: Framebuffer
processors", which has been rejected as "not really needed in the DRM
core":
http://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg146286.html

In this proposal I moved all the code to Exynos DRM driver, so now this
will be specific only to Exynos DRM. I've also changed the name from
framebuffer processor (fbproc) to picture processor (pp) to avoid confusion
with fbdev API.

Here is a bit more information what picture processors are:

Embedded SoCs are known to have a number of hardware blocks, which perform
such operations. They can be used in paralel to the main GPU module to
offload CPU from processing grapics or video data. One of example use of
such modules is implementing video overlay, which usually requires color
space conversion from NV12 (or similar) to RGB32 color space and scaling to
target window size.

The proposed API is heavily inspired by atomic KMS approach - it is also
based on DRM objects and their properties. A new DRM object is introduced:
picture processor (called pp for convenience). Such objects have a set of
standard DRM properties, which describes the operation to be performed by
respective hardware module. In typical case those properties are a source
fb id and rectangle (x, y, width, height) and destination fb id and
rectangle. Optionally a rotation property can be also specified if
supported by the given hardware. To perform an operation on image data,
userspace provides a set of properties and their values for given fbproc
object in a similar way as object and properties are provided for
performing atomic page flip / mode setting.

The proposed API consists of the 3 new ioctls:
- DRM_IOCTL_EXYNOS_PP_GET_RESOURCES: to enumerate all available picture
   processors,
- DRM_IOCTL_EXYNOS_PP_GET: to query capabilities of given picture
   processor,
- DRM_IOCTL_EXYNOS_PP_COMMIT: to perform operation described by given
   property set.

The proposed API is extensible. Drivers can attach their own, custom
properties to add support for more advanced picture processing (for example
blending).

This proposal aims to replace Exynos DRM IPP (Image Post Processing)
subsystem. IPP API is over-engineered in general, but not really extensible
on the other side. It is also buggy, with significant design flaws - the
biggest issue is the fact that the API covers memory-2-memory picture
operations together with CRTC writeback and duplicating features, which
belongs to video plane. Comparing with IPP subsystem, the PP framework is
smaller (1807 vs 778 lines) and allows driver simplification (Exynos
rotator driver smaller by over 200 lines).

Just a side note, we have written code in GStreamer using the Exnynos 4
FIMC IPP driver. I don't know how many, if any, deployment still exist
(Exynos 4 is relatively old now), but there exist userspace for the
FIMC driver. We use this for color transformation (from tiled to
linear) and scaling. The FIMC driver is in fact quite stable in
upstream kernel today. The GScaler V4L2 M2M driver on Exynos 5 is
largely based on it and has received some maintenance to properly work
in GStreamer. unlike this DRM API, you can reuse the same userspace
code across multiple platforms (which we do already). We have also
integrated this driver in Chromium in the past (not upstream though).

I am well aware that the blitter driver has not got much attention
though. But again, V4L2 offers a generic interface to userspace
application. Fixing this driver could enable some work like this one:

https://bugzilla.gnome.org/show_bug.cgi?id=772766

This work in progress feature is a generic hardware accelerated video
mixer. It has been tested with IMX.6 v4l2 m2m blitter driver (which I
believe is in staging right now). Again, unlike the exynos/drm, this
code could be reused between platforms.

In general, the problem with the DRM approach is that it only targets
displays. We often need to use these IP block for stream pre/post
processing outside a "playback" use case.

just a short note that this is not true. You can use all this
functionality e.g. through render nodes, without needing to have a
display attached to your system.


Yes. As an alternative I also plan to provide generic V4L2-style mem2mem

Re: [RFC 0/4] Exynos DRM: add Picture Processor extension

[Marek Szyprowski]

Hi Sakari,

On 2017-04-26 00:21, Sakari Ailus wrote:

Hi Marek,

On Thu, Apr 20, 2017 at 01:23:09PM +0200, Marek Szyprowski wrote:

Hi Laurent,

On 2017-04-20 12:25, Laurent Pinchart wrote:

Hi Marek,

(CC'ing Sakari Ailus)

Thank you for the patches.

On Thursday 20 Apr 2017 11:13:36 Marek Szyprowski wrote:

Dear all,

This is an updated proposal for extending EXYNOS DRM API with generic
support for hardware modules, which can be used for processing image data

>from the one memory buffer to another. Typical memory-to-memory operations

are: rotation, scaling, colour space conversion or mix of them. This is a
follow-up of my previous proposal "[RFC 0/2] New feature: Framebuffer
processors", which has been rejected as "not really needed in the DRM
core":
http://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg146286.html

In this proposal I moved all the code to Exynos DRM driver, so now this
will be specific only to Exynos DRM. I've also changed the name from
framebuffer processor (fbproc) to picture processor (pp) to avoid confusion
with fbdev API.

Here is a bit more information what picture processors are:

Embedded SoCs are known to have a number of hardware blocks, which perform
such operations. They can be used in paralel to the main GPU module to
offload CPU from processing grapics or video data. One of example use of
such modules is implementing video overlay, which usually requires color
space conversion from NV12 (or similar) to RGB32 color space and scaling to
target window size.

The proposed API is heavily inspired by atomic KMS approach - it is also
based on DRM objects and their properties. A new DRM object is introduced:
picture processor (called pp for convenience). Such objects have a set of
standard DRM properties, which describes the operation to be performed by
respective hardware module. In typical case those properties are a source
fb id and rectangle (x, y, width, height) and destination fb id and
rectangle. Optionally a rotation property can be also specified if
supported by the given hardware. To perform an operation on image data,
userspace provides a set of properties and their values for given fbproc
object in a similar way as object and properties are provided for
performing atomic page flip / mode setting.

The proposed API consists of the 3 new ioctls:
- DRM_IOCTL_EXYNOS_PP_GET_RESOURCES: to enumerate all available picture
   processors,
- DRM_IOCTL_EXYNOS_PP_GET: to query capabilities of given picture
   processor,
- DRM_IOCTL_EXYNOS_PP_COMMIT: to perform operation described by given
   property set.

The proposed API is extensible. Drivers can attach their own, custom
properties to add support for more advanced picture processing (for example
blending).

This proposal aims to replace Exynos DRM IPP (Image Post Processing)
subsystem. IPP API is over-engineered in general, but not really extensible
on the other side. It is also buggy, with significant design flaws - the
biggest issue is the fact that the API covers memory-2-memory picture
operations together with CRTC writeback and duplicating features, which
belongs to video plane. Comparing with IPP subsystem, the PP framework is
smaller (1807 vs 778 lines) and allows driver simplification (Exynos
rotator driver smaller by over 200 lines).

This seems to be the kind of hardware that is typically supported by V4L2.
Stupid question, why DRM ?

Let me elaborate a bit on the reasons for implementing it in Exynos DRM:

1. we want to replace existing Exynos IPP subsystem:
  - it is used only in some internal/vendor trees, not in open-source
  - we want it to have sane and potentially extensible userspace API
  - but we don't want to loose its functionality

2. we want to have simple API for performing single image processing
operation:
  - typically it will be used by compositing window manager, this means that
some parameters of the processing might change on each vblank (like
destination rectangle for example). This api allows such change on each
operation without any additional cost. V4L2 requires to reinitialize
queues with new configuration on such change, what means that a bunch of
ioctls has to be called.

What do you mean by re-initialising the queue? Format, buffers or something
else?


In case of compositor use case, the parameter that is being changed most
frequently is source and/or destination rectangle position and/or size.


If you need a larger buffer than what you have already allocated, you'll
need to re-allocate, V4L2 or not.

We also do lack a way to destroy individual buffers in V4L2. It'd be up to
implementing that and some work in videobuf2.


Well if we would use V4l2, buffers will always come as dmabuf objects. There
is a hard limit of the number of buffers that can be imported to v4l2/vb2
queue to get buffer ids. This also limits easy processing of the buffers
in the compositor, because you would need to reinitialize the v4l2 queues
to get new set of v4l2/vb2 buffer ids.

[Bug 100802] [regression] mostly blank graphics on Faeria

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=100802

--- Comment #1 from Timo Aaltonen  ---
still there in 17.1-rc2

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

[PULL] drm-intel-next-fixes for v4.12

[Jani Nikula]

Hi Dave, here's an assortment of drm/i915 and gvt fixes for
drm-next/v4.12.

BR,
Jani.

The following changes since commit ab6eb211b07a42a6346e284056422fd9a8576a99:

  Merge tag 'drm/panel/for-4.12-rc1' of 
git://anongit.freedesktop.org/tegra/linux into drm-next (2017-04-13 06:17:40 
+1000)

are available in the git repository at:

  git://anongit.freedesktop.org/git/drm-intel 
tags/drm-intel-next-fixes-2017-04-27

for you to fetch changes up to 88326ef05b262f681d837ecf65db10a7edb609f1:

  drm/i915: Confirm the request is still active before adding it to the await 
(2017-04-26 16:28:47 +0300)


drm/i915 and gvt fixes for drm-next/v4.12


Changbin Du (4):
  drm/i915/gvt: Align render mmio list to cacheline
  drm/i915/gvt: remove redundant platform check for mocs load/restore
  drm/i915/gvt: remove redundant ring id check which cause significant CPU 
misprediction
  drm/i915/gvt: use directly assignment for structure copying

Chris Wilson (7):
  drm/i915: Park the signaler before sleeping
  drm/i915: Apply a cond_resched() to the saturated signaler
  drm/i915: Use the right mapping_gfp_mask for final shmem allocation
  drm/i915: Fix use after free in lpe_audio_platdev_destroy()
  drm/i915/selftests: Allocate inode/file dynamically
  drm/i915: Avoid busy-spinning on VLV_GLTC_PW_STATUS mmio
  drm/i915: Confirm the request is still active before adding it to the 
await

Dan Carpenter (2):
  drm/i915/gvt: fix a bounds check in ring_id_to_context_switch_event()
  drm/i915: checking for NULL instead of IS_ERR() in mock selftests

Jani Nikula (1):
  Merge tag 'gvt-next-fixes-2017-04-20' of 
https://github.com/01org/gvt-linux into drm-intel-next-fixes

Mika Kuoppala (1):
  drm/i915: Fix system hang with EI UP masked on Haswell

Pei Zhang (1):
  drm/i915/gvt: add mmio init for virtual display

Ville Syrjälä (2):
  drm/i915: Make legacy cursor updates more unsynced
  drm/i915: Perform link quality check unconditionally during long pulse

Zhenyu Wang (3):
  drm/i915/gvt: cleanup some too chatty scheduler message
  drm/i915/gvt: remove some debug messages in scheduler timer handler
  drm/i915/gvt: Fix PTE write flush for taking runtime pm properly

 drivers/gpu/drm/i915/gvt/cmd_parser.c |  8 +
 drivers/gpu/drm/i915/gvt/display.c| 29 -
 drivers/gpu/drm/i915/gvt/execlist.c   |  8 ++---
 drivers/gpu/drm/i915/gvt/gtt.c|  5 +++
 drivers/gpu/drm/i915/gvt/render.c | 10 ++
 drivers/gpu/drm/i915/gvt/sched_policy.c   | 17 ++
 drivers/gpu/drm/i915/gvt/scheduler.c  |  5 +--
 drivers/gpu/drm/i915/i915_drv.c   | 46 ++-
 drivers/gpu/drm/i915/i915_gem.c   |  2 +-
 drivers/gpu/drm/i915/i915_gem_request.c   |  3 ++
 drivers/gpu/drm/i915/i915_irq.c   |  4 +--
 drivers/gpu/drm/i915/intel_breadcrumbs.c  | 21 +---
 drivers/gpu/drm/i915/intel_display.c  | 31 +++---
 drivers/gpu/drm/i915/intel_dp.c   | 15 +++--
 drivers/gpu/drm/i915/intel_lpe_audio.c|  9 +-
 drivers/gpu/drm/i915/selftests/mock_drm.c | 45 ++
 drivers/gpu/drm/i915/selftests/mock_request.c |  2 +-
 17 files changed, 163 insertions(+), 97 deletions(-)

-- 
Jani Nikula, Intel Open Source Technology Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 100800] With KMS:No link with displayport and A6-3600 APU from 4.4.x to 4.11.0.rc8, unless nomodeset kernel boot parameter

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=100800

--- Comment #4 from abitt...@opensuse.org ---
Created attachment 131095
  --> https://bugs.freedesktop.org/attachment.cgi?id=131095=edit
Xorg log when displayport monitor attached

Xorg log when displayport monitor attached

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

Re: [PATCH] drm/rockchip: Set line flag config register in vop_crtc_enable

[Sean Paul]

On Thu, Apr 27, 2017 at 03:00:29PM +0800, Mark yao wrote:
> On 2017年04月27日 14:54, Jeffy Chen wrote:
> > We need to set vop config done after update line flag config, it's a
> > new requirement for chips newer than rk3368.
> > 
> > Since we would only use line flag irq for vact_end, let's move it to
> > vop_crtc_enable.
> > 
> > Signed-off-by: Jeffy Chen 
> 
> looks good for me:
> 
> Acked-by: Mark Yao 
> 

Thanks for the patch and review.

Pushed to misc-next

Sean

> > 
> > ---
> > 
> >   drivers/gpu/drm/rockchip/analogix_dp-rockchip.c |  4 ++--
> >   drivers/gpu/drm/rockchip/rockchip_drm_drv.h |  3 +--
> >   drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 20 +---
> >   3 files changed, 12 insertions(+), 15 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c 
> > b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> > index d8fa7a9..9bfdbc6 100644
> > --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> > +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> > @@ -115,8 +115,8 @@ static void analogix_dp_psr_work(struct work_struct 
> > *work)
> > vact_end = crtc->mode.vtotal - crtc->mode.vsync_start + 
> > crtc->mode.vdisplay;
> > -   ret = rockchip_drm_wait_line_flag(dp->encoder.crtc, vact_end,
> > - PSR_WAIT_LINE_FLAG_TIMEOUT_MS);
> > +   ret = rockchip_drm_wait_vact_end(dp->encoder.crtc,
> > +PSR_WAIT_LINE_FLAG_TIMEOUT_MS);
> > if (ret) {
> > dev_err(dp->dev, "line flag interrupt did not arrive\n");
> > return;
> > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h 
> > b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> > index a48fcce..47905fa 100644
> > --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> > @@ -62,8 +62,7 @@ int rockchip_drm_dma_attach_device(struct drm_device 
> > *drm_dev,
> >struct device *dev);
> >   void rockchip_drm_dma_detach_device(struct drm_device *drm_dev,
> > struct device *dev);
> > -int rockchip_drm_wait_line_flag(struct drm_crtc *crtc, unsigned int 
> > line_num,
> > -   unsigned int mstimeout);
> > +int rockchip_drm_wait_vact_end(struct drm_crtc *crtc, unsigned int 
> > mstimeout);
> >   extern struct platform_driver cdn_dp_driver;
> >   extern struct platform_driver dw_hdmi_rockchip_pltfm_driver;
> > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c 
> > b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> > index 3f7a82d..40a5e6e 100644
> > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> > @@ -468,7 +468,7 @@ static bool vop_line_flag_irq_is_enabled(struct vop 
> > *vop)
> > return !!line_flag_irq;
> >   }
> > -static void vop_line_flag_irq_enable(struct vop *vop, int line_num)
> > +static void vop_line_flag_irq_enable(struct vop *vop)
> >   {
> > unsigned long flags;
> > @@ -477,7 +477,6 @@ static void vop_line_flag_irq_enable(struct vop *vop, 
> > int line_num)
> > spin_lock_irqsave(>irq_lock, flags);
> > -   VOP_CTRL_SET(vop, line_flag_num[0], line_num);
> > VOP_INTR_SET_TYPE(vop, clear, LINE_FLAG_INTR, 1);
> > VOP_INTR_SET_TYPE(vop, enable, LINE_FLAG_INTR, 1);
> > @@ -981,6 +980,8 @@ static void vop_crtc_enable(struct drm_crtc *crtc)
> > VOP_CTRL_SET(vop, vact_st_end, val);
> > VOP_CTRL_SET(vop, vpost_st_end, val);
> > +   VOP_CTRL_SET(vop, line_flag_num[0], vact_end);
> > +
> > clk_set_rate(vop->dclk, adjusted_mode->clock * 1000);
> > VOP_CTRL_SET(vop, standby, 0);
> > @@ -1507,19 +1508,16 @@ static void vop_win_init(struct vop *vop)
> >   }
> >   /**
> > - * rockchip_drm_wait_line_flag - acqiure the give line flag event
> > + * rockchip_drm_wait_vact_end
> >* @crtc: CRTC to enable line flag
> > - * @line_num: interested line number
> >* @mstimeout: millisecond for timeout
> >*
> > - * Driver would hold here until the interested line flag interrupt have
> > - * happened or timeout to wait.
> > + * Wait for vact_end line flag irq or timeout.
> >*
> >* Returns:
> >* Zero on success, negative errno on failure.
> >*/
> > -int rockchip_drm_wait_line_flag(struct drm_crtc *crtc, unsigned int 
> > line_num,
> > -   unsigned int mstimeout)
> > +int rockchip_drm_wait_vact_end(struct drm_crtc *crtc, unsigned int 
> > mstimeout)
> >   {
> > struct vop *vop = to_vop(crtc);
> > unsigned long jiffies_left;
> > @@ -1527,14 +1525,14 @@ int rockchip_drm_wait_line_flag(struct drm_crtc 
> > *crtc, unsigned int line_num,
> > if (!crtc || !vop->is_enabled)
> > return -ENODEV;
> > -   if (line_num > crtc->mode.vtotal || mstimeout <= 0)
> > +   if (mstimeout <= 0)
> > return -EINVAL;
> > if (vop_line_flag_irq_is_enabled(vop))
> > return 

[Bug 100800] With KMS:No link with displayport and A6-3600 APU from 4.4.x to 4.11.0.rc8, unless nomodeset kernel boot parameter

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=100800

--- Comment #3 from abitt...@opensuse.org ---
Created attachment 131091
  --> https://bugs.freedesktop.org/attachment.cgi?id=131091=edit
dmesg -e output of affected system when booting it with displayport monitor
attached

dmesg -e output of affected system when booting it with displayport monitor
attached

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

[Bug 100800] With KMS:No link with displayport and A6-3600 APU from 4.4.x to 4.11.0.rc8, unless nomodeset kernel boot parameter

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=100800

--- Comment #2 from abitt...@opensuse.org ---
with the kernel

I have following [drm
loglines:

[  +0.011717] [drm] radeon kernel modesetting enabled.
[  +0.000540] [drm] initializing kernel modesetting (SUMO 0x1002:0x964A
0x1043:0x84C8 0x00).
[  +0.16] [drm] register mmio base: 0xFEB0
[  +0.01] [drm] register mmio size: 262144
[  +0.04] [drm] Detected VRAM RAM=512M, BAR=256M
[  +0.00] [drm] RAM width 32bits DDR
[  +0.24] [drm] radeon: 512M of VRAM memory ready
[  +0.01] [drm] radeon: 1024M of GTT memory ready.
[  +0.12] [drm] Loading SUMO Microcode
[  +0.51] [drm] Internal thermal controller without fan control
[  +0.35] [drm] Found smc ucode version: 0x00011200
[  +0.80] [drm] radeon: dpm initialized
[  +0.000114] [drm] GART: num cpu pages 262144, num gpu pages 262144
[  +0.018205] [drm] PCIE GART of 1024M enabled (table at 0x00162000).
[  +0.02] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
[  +0.01] [drm] Driver supports precise vblank timestamp query.
[  +0.25] [drm] radeon: irq initialized.
[  +0.016359] [drm] ring test on 0 succeeded in 0 usecs
[  +0.06] [drm] ring test on 3 succeeded in 3 usecs
[  +0.045683] [drm] ring test on 5 succeeded in 1 usecs
[  +0.009934] [drm] UVD initialized successfully.
[  +0.000385] [drm] ib test on ring 0 succeeded in 0 usecs
[  +0.44] [drm] ib test on ring 3 succeeded in 0 usecs
[  +0.161967] [drm] ib test on ring 5 succeeded
[  +0.010613] [drm] Radeon Display Connectors
[  +0.02] [drm] Connector 0:
[  +0.01] [drm]   VGA-1
[  +0.00] [drm]   HPD2
[  +0.02] [drm]   DDC: 0x6440 0x6440 0x6444 0x6444 0x6448 0x6448 0x644c
0x644c
[  +0.00] [drm]   Encoders:
[  +0.01] [drm] CRT1: INTERNAL_UNIPHY2
[  +0.01] [drm] CRT1: NUTMEG
[  +0.00] [drm] Connector 1:
[  +0.01] [drm]   HDMI-A-1
[  +0.00] [drm]   HPD1
[  +0.01] [drm]   DDC: 0x6430 0x6430 0x6434 0x6434 0x6438 0x6438 0x643c
0x643c
[  +0.01] [drm]   Encoders:
[  +0.00] [drm] DFP1: INTERNAL_UNIPHY2
[  +0.01] [drm] Connector 2:
[  +0.00] [drm]   DP-1
[  +0.01] [drm]   HPD3
[  +0.01] [drm]   DDC: 0x6450 0x6450 0x6454 0x6454 0x6458 0x6458 0x645c
0x645c
[  +0.00] [drm]   Encoders:
[  +0.01] [drm] DFP2: INTERNAL_UNIPHY
[  +0.091523] [drm] fb mappable at 0xC0366000
[  +0.03] [drm] vram apper at 0xC000
[  +0.00] [drm] size 9216000
[  +0.01] [drm] fb depth is 24
[  +0.00] [drm]pitch is 7680
[  +4.944624] [drm:atom_op_jump [radeon]] *ERROR* atombios stuck in loop for
more than 5secs aborting
[  +0.28] [drm:atom_execute_table_locked [radeon]] *ERROR* atombios stuck
executing E262 (len 2585, WS 4, PS 4) @ 0xE94C
[  +0.039430] [drm:radeon_dp_link_train [radeon]] *ERROR* clock recovery
reached max voltage
[  +0.30] [drm:radeon_dp_link_train [radeon]] *ERROR* clock recovery failed
[  +0.022498] [drm:radeon_dp_link_train [radeon]] *ERROR* clock recovery
reached max voltage
[  +0.86] [drm:radeon_dp_link_train [radeon]] *ERROR* clock recovery failed
[  +0.000704] [drm] Initialized radeon 2.49.0 20080528 for :00:01.0 on
minor 0
[  +0.039081] [drm:radeon_dp_link_train [radeon]] *ERROR* clock recovery
reached max voltage
[  +0.76] [drm:radeon_dp_link_train [radeon]] *ERROR* clock recovery failed
[  +0.021913] [drm:radeon_dp_link_train [radeon]] *ERROR* clock recovery
reached max voltage




modinfo:

filename:  
/lib/modules/4.11.0-rc8-1.g20b036c-default/kernel/drivers/gpu/drm/radeon/radeon.ko
license:GPL and additional rights
description:ATI Radeon
author: Gareth Hughes, Keith Whitwell, others.

srcversion: 50E9ACC7FC15ADCFBF5FE57

depends:drm,drm_kms_helper,ttm,i2c-algo-bit
intree: Y
vermagic:   4.11.0-rc8-1.g20b036c-default SMP preempt mod_unload
modversions 
parm:   no_wb:Disable AGP writeback for scratch registers (int)
parm:   modeset:Disable/Enable modesetting (int)
parm:   dynclks:Disable/Enable dynamic clocks (int)
parm:   r4xx_atom:Enable ATOMBIOS modesetting for R4xx (int)
parm:   vramlimit:Restrict VRAM for testing, in megabytes (int)
parm:   agpmode:AGP Mode (-1 == PCI) (int)
parm:   gartsize:Size of PCIE/IGP gart to setup in megabytes (32, 64,
etc., -1 = auto) (int)
parm:   benchmark:Run benchmark (int)
parm:   test:Run tests (int)
parm:   connector_table:Force connector table (int)
parm:   tv:TV enable (0 = disable) (int)
parm:   audio:Audio enable (-1 = auto, 0 = disable, 1 = enable) (int)
parm:   disp_priority:Display Priority (0 = auto, 1 = normal, 2 = high)
(int)
parm:   hw_i2c:hw i2c engine enable (0 = disable) (int)
parm:   pcie_gen2:PCIE Gen2 mode (-1 = auto, 0 = disable, 1 = enable)
(int)
parm:   msi:MSI support (1 = enable, 0 = disable, -1 = auto) (int)
parm:   

Re: [PATCH v2 09/10] drm: omapdrm: Map pages for DMA in DMA_TO_DEVICE direction

[Tomi Valkeinen]

On 21/04/17 00:33, Laurent Pinchart wrote:
> The display engine only reads from memory, there's no need to use
> bidirectional DMA mappings. Use DMA_TO_DEVICE instead.
> 
> Signed-off-by: Laurent Pinchart 
> ---
>  drivers/gpu/drm/omapdrm/omap_gem.c| 11 +--
>  drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c |  4 ++--
>  2 files changed, 7 insertions(+), 8 deletions(-)

Reviewed-by: Tomi Valkeinen 

 Tomi



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

Re: [PATCH v2 08/10] drm: omapdrm: DMA-unmap pages for all buffer types when freeing buffers

[Tomi Valkeinen]

On 21/04/17 00:33, Laurent Pinchart wrote:
> Both coherent (uncached) and non-coherent (cached) buffers can have
> their pages mapped to the device through the DMA mapping API. Make sure
> to unmap any mapped page when freeing a buffer, regardless of its type.
> 
> Signed-off-by: Laurent Pinchart 
> ---
>  drivers/gpu/drm/omapdrm/omap_gem.c | 17 ++---
>  1 file changed, 6 insertions(+), 11 deletions(-)

Reviewed-by: Tomi Valkeinen 

 Tomi



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

Re: [PATCH v2 2/5] drm: omapdrm: Use DRM core's atomic commit helper

[Tomi Valkeinen]

On 15/04/17 12:16, Laurent Pinchart wrote:
> The DRM core atomic helper now supports asynchronous commits natively.
> The custom omapdrm implementation isn't needed anymore, remove it.
> 
> Signed-off-by: Laurent Pinchart 
> ---
>  drivers/gpu/drm/omapdrm/omap_drv.c | 112 
> +
>  drivers/gpu/drm/omapdrm/omap_drv.h |   9 +--
>  2 files changed, 15 insertions(+), 106 deletions(-)

Reviewed-by: Tomi Valkeinen 

 Tomi



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

[PATCH 3/6] drm/omap: remove ovl_set_channel_out

[Tomi Valkeinen]

At the moment we have ovl_set_channel_out() to configure the output
channel of an overlay. It makes sense to have this configuration as part
of the reset of overlay configuration, and in DSS6+ we need the output
channel when doing the other overlay configuration.

This patch adds 'channel' parameter to ovl_setup(), so that all overlay
configuration is done via the same function, and removes the
ovl_set_channel_out().

Signed-off-by: Tomi Valkeinen 
---
 drivers/gpu/drm/omapdrm/dss/dispc.c   | 13 ++---
 drivers/gpu/drm/omapdrm/dss/omapdss.h |  3 ++-
 drivers/gpu/drm/omapdrm/omap_plane.c  |  6 ++
 3 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c 
b/drivers/gpu/drm/omapdrm/dss/dispc.c
index 9fba92973d0e..97f4a1163fbe 100644
--- a/drivers/gpu/drm/omapdrm/dss/dispc.c
+++ b/drivers/gpu/drm/omapdrm/dss/dispc.c
@@ -2834,21 +2834,21 @@ static int dispc_ovl_setup_common(enum omap_plane_id 
plane,
 
 static int dispc_ovl_setup(enum omap_plane_id plane,
const struct omap_overlay_info *oi,
-   const struct videomode *vm, bool mem_to_mem)
+   const struct videomode *vm, bool mem_to_mem,
+   enum omap_channel channel)
 {
int r;
enum omap_overlay_caps caps = dss_feat_get_overlay_caps(plane);
-   enum omap_channel channel;
const bool replication = true;
 
-   channel = dispc_ovl_get_channel_out(plane);
-
DSSDBG("dispc_ovl_setup %d, pa %pad, pa_uv %pad, sw %d, %d,%d, %dx%d ->"
" %dx%d, cmode %x, rot %d, mir %d, chan %d repl %d\n",
plane, >paddr, >p_uv_addr, oi->screen_width, oi->pos_x,
oi->pos_y, oi->width, oi->height, oi->out_width, oi->out_height,
oi->color_mode, oi->rotation, oi->mirror, channel, replication);
 
+   dispc_ovl_set_channel_out(plane, channel);
+
r = dispc_ovl_setup_common(plane, caps, oi->paddr, oi->p_uv_addr,
oi->screen_width, oi->pos_x, oi->pos_y, oi->width, oi->height,
oi->out_width, oi->out_height, oi->color_mode, oi->rotation,
@@ -4304,8 +4304,8 @@ static void dispc_errata_i734_wa(void)
REG_FLD_MOD(DISPC_CONFIG, 0x1f, 8, 4);
 
/* Setup and enable GFX plane */
-   dispc_ovl_set_channel_out(OMAP_DSS_GFX, OMAP_DSS_CHANNEL_LCD);
-   dispc_ovl_setup(OMAP_DSS_GFX, , , false);
+   dispc_ovl_setup(OMAP_DSS_GFX, , , false,
+   OMAP_DSS_CHANNEL_LCD);
dispc_ovl_enable(OMAP_DSS_GFX, true);
 
/* Set up and enable display manager for LCD1 */
@@ -4372,7 +4372,6 @@ static const struct dispc_ops dispc_ops = {
.mgr_set_gamma = dispc_mgr_set_gamma,
 
.ovl_enable = dispc_ovl_enable,
-   .ovl_set_channel_out = dispc_ovl_set_channel_out,
.ovl_setup = dispc_ovl_setup,
.ovl_get_color_modes = dispc_ovl_get_color_modes,
 };
diff --git a/drivers/gpu/drm/omapdrm/dss/omapdss.h 
b/drivers/gpu/drm/omapdrm/dss/omapdss.h
index d9ad33df0455..a2b9b7f29472 100644
--- a/drivers/gpu/drm/omapdrm/dss/omapdss.h
+++ b/drivers/gpu/drm/omapdrm/dss/omapdss.h
@@ -917,7 +917,8 @@ struct dispc_ops {
enum omap_channel channel);
int (*ovl_setup)(enum omap_plane_id plane,
 const struct omap_overlay_info *oi,
-   const struct videomode *vm, bool mem_to_mem);
+   const struct videomode *vm, bool mem_to_mem,
+   enum omap_channel channel);
 
enum omap_color_mode (*ovl_get_color_modes)(enum omap_plane_id plane);
 };
diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c 
b/drivers/gpu/drm/omapdrm/omap_plane.c
index 9168154d749e..81d9822f6f4a 100644
--- a/drivers/gpu/drm/omapdrm/omap_plane.c
+++ b/drivers/gpu/drm/omapdrm/omap_plane.c
@@ -118,12 +118,10 @@ static void omap_plane_atomic_update(struct drm_plane 
*plane,
DBG("%d,%d %pad %pad", info.pos_x, info.pos_y,
, _uv_addr);
 
-   priv->dispc_ops->ovl_set_channel_out(omap_plane->id,
- omap_crtc_channel(state->crtc));
-
/* and finally, update omapdss: */
ret = priv->dispc_ops->ovl_setup(omap_plane->id, ,
- omap_crtc_timings(state->crtc), false);
+ omap_crtc_timings(state->crtc), false,
+ omap_crtc_channel(state->crtc));
if (ret) {
dev_err(plane->dev->dev, "Failed to setup plane %s\n",
omap_plane->name);
-- 
2.7.4

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

[PATCH 2/6] drm/omap: remove unused ovl_enabled()

[Tomi Valkeinen]

ovl_enabled() is not used anywhere, so remove it.

Signed-off-by: Tomi Valkeinen 
---
 drivers/gpu/drm/omapdrm/dss/dispc.c   | 6 --
 drivers/gpu/drm/omapdrm/dss/omapdss.h | 1 -
 2 files changed, 7 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c 
b/drivers/gpu/drm/omapdrm/dss/dispc.c
index 5ac0145fbae6..9fba92973d0e 100644
--- a/drivers/gpu/drm/omapdrm/dss/dispc.c
+++ b/drivers/gpu/drm/omapdrm/dss/dispc.c
@@ -2935,11 +2935,6 @@ static int dispc_ovl_enable(enum omap_plane_id plane, 
bool enable)
return 0;
 }
 
-static bool dispc_ovl_enabled(enum omap_plane_id plane)
-{
-   return REG_GET(DISPC_OVL_ATTRIBUTES(plane), 0, 0);
-}
-
 static enum omap_dss_output_id dispc_mgr_get_supported_outputs(enum 
omap_channel channel)
 {
return dss_feat_get_supported_outputs(channel);
@@ -4377,7 +4372,6 @@ static const struct dispc_ops dispc_ops = {
.mgr_set_gamma = dispc_mgr_set_gamma,
 
.ovl_enable = dispc_ovl_enable,
-   .ovl_enabled = dispc_ovl_enabled,
.ovl_set_channel_out = dispc_ovl_set_channel_out,
.ovl_setup = dispc_ovl_setup,
.ovl_get_color_modes = dispc_ovl_get_color_modes,
diff --git a/drivers/gpu/drm/omapdrm/dss/omapdss.h 
b/drivers/gpu/drm/omapdrm/dss/omapdss.h
index b19dae1fd6c5..d9ad33df0455 100644
--- a/drivers/gpu/drm/omapdrm/dss/omapdss.h
+++ b/drivers/gpu/drm/omapdrm/dss/omapdss.h
@@ -913,7 +913,6 @@ struct dispc_ops {
unsigned int length);
 
int (*ovl_enable)(enum omap_plane_id plane, bool enable);
-   bool (*ovl_enabled)(enum omap_plane_id plane);
void (*ovl_set_channel_out)(enum omap_plane_id plane,
enum omap_channel channel);
int (*ovl_setup)(enum omap_plane_id plane,
-- 
2.7.4

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

[PATCH 0/6] drm/omap: misc changes

[Tomi Valkeinen]

Hi,

Here are a few cleanups, 64bit fixes and connector type improvement for
omapdrm.

 Tomi

Tomi Valkeinen (6):
  drm/omap: add new connector types
  drm/omap: remove unused ovl_enabled()
  drm/omap: remove ovl_set_channel_out
  drm/omap: remove read_irqenable()
  drm/omap: 64bit compile fixes
  drm/omap: define compat_ioctl

 drivers/gpu/drm/omapdrm/dss/dispc.c  | 28 +---
 drivers/gpu/drm/omapdrm/dss/omapdss.h|  5 ++---
 drivers/gpu/drm/omapdrm/omap_dmm_tiler.c |  2 +-
 drivers/gpu/drm/omapdrm/omap_drv.c   |  8 
 drivers/gpu/drm/omapdrm/omap_fb.c|  2 +-
 drivers/gpu/drm/omapdrm/omap_gem.c   |  2 +-
 drivers/gpu/drm/omapdrm/omap_irq.c   |  1 -
 drivers/gpu/drm/omapdrm/omap_plane.c |  6 ++
 8 files changed, 24 insertions(+), 30 deletions(-)

-- 
2.7.4

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

[PATCH 6/6] drm/omap: define compat_ioctl

[Tomi Valkeinen]

Define compat_ioctl in omapdriver_fops to make it possible to use 32bit
apps on 64bit platform.

Signed-off-by: Tomi Valkeinen 
---
 drivers/gpu/drm/omapdrm/omap_drv.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c 
b/drivers/gpu/drm/omapdrm/omap_drv.c
index 58c639e1e8e9..3cfff88c7d79 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -615,6 +615,7 @@ static const struct file_operations omapdriver_fops = {
.owner = THIS_MODULE,
.open = drm_open,
.unlocked_ioctl = drm_ioctl,
+   .compat_ioctl = drm_compat_ioctl,
.release = drm_release,
.mmap = omap_gem_mmap,
.poll = drm_poll,
-- 
2.7.4

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

[PATCH 4/6] drm/omap: remove read_irqenable()

[Tomi Valkeinen]

We only use read_irqenable() to flush posted write. Instead of having a
separate function for this, do the flush implicitly in write_irqenable().
Thus we can remove read_irqenable().

Signed-off-by: Tomi Valkeinen 
---
 drivers/gpu/drm/omapdrm/dss/dispc.c   | 9 +++--
 drivers/gpu/drm/omapdrm/dss/omapdss.h | 1 -
 drivers/gpu/drm/omapdrm/omap_irq.c| 1 -
 3 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c 
b/drivers/gpu/drm/omapdrm/dss/dispc.c
index 97f4a1163fbe..352fad583571 100644
--- a/drivers/gpu/drm/omapdrm/dss/dispc.c
+++ b/drivers/gpu/drm/omapdrm/dss/dispc.c
@@ -3782,11 +3782,6 @@ static void dispc_clear_irqstatus(u32 mask)
dispc_write_reg(DISPC_IRQSTATUS, mask);
 }
 
-static u32 dispc_read_irqenable(void)
-{
-   return dispc_read_reg(DISPC_IRQENABLE);
-}
-
 static void dispc_write_irqenable(u32 mask)
 {
u32 old_mask = dispc_read_reg(DISPC_IRQENABLE);
@@ -3795,6 +3790,9 @@ static void dispc_write_irqenable(u32 mask)
dispc_clear_irqstatus((mask ^ old_mask) & mask);
 
dispc_write_reg(DISPC_IRQENABLE, mask);
+
+   /* flush posted write */
+   dispc_read_reg(DISPC_IRQENABLE);
 }
 
 void dispc_enable_sidle(void)
@@ -4345,7 +4343,6 @@ static void dispc_errata_i734_wa(void)
 static const struct dispc_ops dispc_ops = {
.read_irqstatus = dispc_read_irqstatus,
.clear_irqstatus = dispc_clear_irqstatus,
-   .read_irqenable = dispc_read_irqenable,
.write_irqenable = dispc_write_irqenable,
 
.request_irq = dispc_request_irq,
diff --git a/drivers/gpu/drm/omapdrm/dss/omapdss.h 
b/drivers/gpu/drm/omapdrm/dss/omapdss.h
index a2b9b7f29472..44bf53351e00 100644
--- a/drivers/gpu/drm/omapdrm/dss/omapdss.h
+++ b/drivers/gpu/drm/omapdrm/dss/omapdss.h
@@ -881,7 +881,6 @@ void dss_mgr_unregister_framedone_handler(enum omap_channel 
channel,
 struct dispc_ops {
u32 (*read_irqstatus)(void);
void (*clear_irqstatus)(u32 mask);
-   u32 (*read_irqenable)(void);
void (*write_irqenable)(u32 mask);
 
int (*request_irq)(irq_handler_t handler, void *dev_id);
diff --git a/drivers/gpu/drm/omapdrm/omap_irq.c 
b/drivers/gpu/drm/omapdrm/omap_irq.c
index 115104cdcc59..7afe4b90befd 100644
--- a/drivers/gpu/drm/omapdrm/omap_irq.c
+++ b/drivers/gpu/drm/omapdrm/omap_irq.c
@@ -41,7 +41,6 @@ static void omap_irq_update(struct drm_device *dev)
DBG("irqmask=%08x", irqmask);
 
priv->dispc_ops->write_irqenable(irqmask);
-   priv->dispc_ops->read_irqenable();/* flush posted write */
 }
 
 static void omap_irq_wait_handler(struct omap_irq_wait *wait)
-- 
2.7.4

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

[PATCH 5/6] drm/omap: 64bit compile fixes

[Tomi Valkeinen]

Fix a few type issues that cause compile warnings on 64 bit ARM
compiler. The change should not affect 32bit platforms.

Signed-off-by: Tomi Valkeinen 
---
 drivers/gpu/drm/omapdrm/omap_dmm_tiler.c | 2 +-
 drivers/gpu/drm/omapdrm/omap_fb.c| 2 +-
 drivers/gpu/drm/omapdrm/omap_gem.c   | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c 
b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
index 3cab06661a08..1dd3dafc59af 100644
--- a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
+++ b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
@@ -388,7 +388,7 @@ struct tiler_block *tiler_reserve_2d(enum tiler_fmt fmt, 
uint16_t w,
u32 min_align = 128;
int ret;
unsigned long flags;
-   size_t slot_bytes;
+   u32 slot_bytes;
 
BUG_ON(!validfmt(fmt));
 
diff --git a/drivers/gpu/drm/omapdrm/omap_fb.c 
b/drivers/gpu/drm/omapdrm/omap_fb.c
index 29dc677dd4d3..2dca19a0c2ff 100644
--- a/drivers/gpu/drm/omapdrm/omap_fb.c
+++ b/drivers/gpu/drm/omapdrm/omap_fb.c
@@ -449,7 +449,7 @@ struct drm_framebuffer *omap_framebuffer_init(struct 
drm_device *dev,
 
if (size > omap_gem_mmap_size(bos[i]) - mode_cmd->offsets[i]) {
dev_dbg(dev->dev,
-   "provided buffer object is too small! %d < 
%d\n",
+   "provided buffer object is too small! %zu < 
%d\n",
bos[i]->size - mode_cmd->offsets[i], size);
ret = -EINVAL;
goto fail;
diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c 
b/drivers/gpu/drm/omapdrm/omap_gem.c
index 68a75b829b71..f076c63e5b08 100644
--- a/drivers/gpu/drm/omapdrm/omap_gem.c
+++ b/drivers/gpu/drm/omapdrm/omap_gem.c
@@ -1046,7 +1046,7 @@ void omap_gem_describe(struct drm_gem_object *obj, struct 
seq_file *m)
area->p1.x, area->p1.y);
}
} else {
-   seq_printf(m, " %d", obj->size);
+   seq_printf(m, " %zu", obj->size);
}
 
seq_printf(m, "\n");
-- 
2.7.4

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

[PATCH 1/6] drm/omap: add new connector types

[Tomi Valkeinen]

We have been using DRM_MODE_CONNECTOR_Unknown for many of our outputs
because there has not been a proper connector type for them.

We now have connector type for DPI so let's take it into use. At the
same time, add better connector types for the remaining outputs too.

This patch sets the following outputs to use the following connector
types:

DPI -> DPI
DBI -> DPI (MIPI DBI is very similar to DPI at the bus level)
SDI -> LVDS (SDI is a type of LVDS)
VENC -> SVIDEO (it could also be composite, but we don't have that
information here, so svideo should be quite good match)

Signed-off-by: Tomi Valkeinen 
---
 drivers/gpu/drm/omapdrm/omap_drv.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c 
b/drivers/gpu/drm/omapdrm/omap_drv.c
index e1f47f0b3ccf..58c639e1e8e9 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -214,6 +214,13 @@ static int get_connector_type(struct omap_dss_device 
*dssdev)
return DRM_MODE_CONNECTOR_DVID;
case OMAP_DISPLAY_TYPE_DSI:
return DRM_MODE_CONNECTOR_DSI;
+   case OMAP_DISPLAY_TYPE_DPI:
+   case OMAP_DISPLAY_TYPE_DBI:
+   return DRM_MODE_CONNECTOR_DPI;
+   case OMAP_DISPLAY_TYPE_VENC:
+   return DRM_MODE_CONNECTOR_SVIDEO;
+   case OMAP_DISPLAY_TYPE_SDI:
+   return DRM_MODE_CONNECTOR_LVDS;
default:
return DRM_MODE_CONNECTOR_Unknown;
}
-- 
2.7.4

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

Re: [PATCH 1/2] drm: Introduce crtc->mode_valid() callback

[Andrzej Hajda]

Hi Jose,

On 26.04.2017 12:48, Jose Abreu wrote:
> Some crtc's may have restrictions in the mode they can display. In
> this patch a new callback (crtc->mode_valid()) is introduced that
> is called at the same stage of connector->mode_valid() callback.
>
> This shall be implemented if the crtc has some sort of restriction
> so that we don't probe modes that will fail in the commit() stage.
> For example: A given crtc may be responsible to set a clock value.
> If the clock can not produce all the values for the available
> modes then this callback can be used to restrict the number of
> probbed modes to only the ones that can be displayed.
>
> If the crtc does not implement the callback then the behaviour will
> remain the same. Also, for a given set of crtcs that can be bound to
> the connector, if at least one can display the mode then the mode
> will be probbed.

I see few possible issues/improvements here:
1. crtc can have different constraints depending on the encoder it is
connected to, theoretically reverse dependency is also possible, but I
am not aware of such hw.
2. there also could be similar dependency constrains between connector
and encoder, I guess.
3. encoders and bridges should have also possibility to validate modes,
they also have constrains, btw encoder_slave have such callback.

Regarding 1st and 2nd point, maybe it would be good to validate modes
according to topology described in drm_mode_get_connector::encoder_id
and drm_mode_get_encoder::crtc_id:
a) if connector is not connected to any encoder report to userspace
modes filtered only by connector::mode_valid,
b) if connector is connected to encoder, report modes filtered by
connector, encoder and attached bridges,
c) if full pipeline is constructed, report modes filtered by all members
of the pipeline.

Of course with this approach drm_mode_get_connector userspace should be
aware of the fact that it should re-call drm_mode_get_connector after
topology change to update supported modes.

Regards
Andrzej

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

[PATCH] drm/nouveau/fifo/gk104-: Silence a locking warning

[Dan Carpenter]

Presumably we can never actually hit this return, but static checkers
complain that we should unlock before we return.

Signed-off-by: Dan Carpenter 

diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/fifo/gk104.c 
b/drivers/gpu/drm/nouveau/nvkm/engine/fifo/gk104.c
index 3a24788c3185..a7e55c422501 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/fifo/gk104.c
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/fifo/gk104.c
@@ -148,7 +148,7 @@ gk104_fifo_runlist_commit(struct gk104_fifo *fifo, int runl)
case NVKM_MEM_TARGET_NCOH: target = 3; break;
default:
WARN_ON(1);
-   return;
+   goto unlock;
}
 
nvkm_wr32(device, 0x002270, (nvkm_memory_addr(mem) >> 12) |
@@ -160,6 +160,7 @@ gk104_fifo_runlist_commit(struct gk104_fifo *fifo, int runl)
   & 0x0010),
   msecs_to_jiffies(2000)) == 0)
nvkm_error(subdev, "runlist %d update timeout\n", runl);
+unlock:
mutex_unlock(>mutex);
 }
 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm/vmwgfx: Handle vmalloc() failure in vmw_local_fifo_reserve()

[Dan Carpenter]

If vmalloc() fails then we need to a bit of cleanup before returning.

Fixes: fb1d9738ca05 ("drm/vmwgfx: Add DRM driver for VMware Virtual GPU")
Signed-off-by: Dan Carpenter 

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fifo.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_fifo.c
index b6a0806b06bf..a1c68e6a689e 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_fifo.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fifo.c
@@ -368,6 +368,8 @@ static void *vmw_local_fifo_reserve(struct vmw_private 
*dev_priv,
return fifo_state->static_buffer;
else {
fifo_state->dynamic_buffer = vmalloc(bytes);
+   if (!fifo_state->dynamic_buffer)
+   goto out_err;
return fifo_state->dynamic_buffer;
}
}
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 96897] clpeak OpenCL benchmark hangs during compilation on Clover RadeonSI

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=96897

--- Comment #8 from ricardo.riba...@gmail.com ---
With:

  Device: AMD CARRIZO (DRM 3.9.0 / 4.10.0-qtec-standard, LLVM 4.0.1)
Driver version  : 17.0.3 (Linux x64)
Compute units   : 8
Clock frequency : 800 MHz


I am getting the same error as Vedran: error: call to 'mad' is ambiguous

After reverting:

16e1b207a4d4e81a0c48c77c950437dca1364cb6 is the first bad commit
commit 16e1b207a4d4e81a0c48c77c950437dca1364cb6
Author: espes 
Date:   Mon Jul 18 17:06:15 2016 -0700

I am experiencing an endless loop as reported by Jan.

I get the same endless loop with:

Platform: Clover
  Device: AMD PALM (DRM 2.49.0 / 4.10.0-qtec-standard, LLVM 4.0.1)
Driver version  : 17.0.3 (Linux x64)
Compute units   : 2
Clock frequency : 0 MHz

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

Re: [RFC PATCHv2 0/3] dma_buf import support for vgem

[Chris Wilson]

On Wed, Apr 26, 2017 at 02:12:27PM -0700, Laura Abbott wrote:
> Hi,
> 
> This is v2 of my proposal to add dma_buf import functions for vgem.
> Big changes from v1:
> 
> - A device is required for dma_buf attach to work. The existing vgem driver
> intentionally does not use one as it provides a good way to test the DRM
> framework. This approach instead puts a dummy platform device in the existing
> drm_device->platformdev field and uses that for attaching.
> - Native vgem buffers can still be faulted in a page at a time without
> requiring the entire buffer be resident in memory.
> 
> I'm still marking this as RFC as I haven't had a chance to finish
> a userspace test that can be integrated into igt.

Note, that it will be good to cc:intel-gfx@ so that our CI does run it
over the existing vgem tests.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC PATCHv2 3/3] drm/vgem: Enable dmabuf import interfaces

[Chris Wilson]

On Wed, Apr 26, 2017 at 02:12:30PM -0700, Laura Abbott wrote:
> Enable the GEM dma-buf import interfaces in addition to the export
> interfaces. This lets vgem be used as a test source for other allocators
> (e.g. Ion).
> 
> Signed-off-by: Laura Abbott 
> ---
> v2: Don't require vgem allocated buffers to existing in memory before 
> importing.
> ---
>  drivers/gpu/drm/vgem/vgem_drv.c | 133 
> +++-
>  drivers/gpu/drm/vgem/vgem_drv.h |   2 +
>  2 files changed, 106 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c
> index 1b02e56..73a619a 100644
> --- a/drivers/gpu/drm/vgem/vgem_drv.c
> +++ b/drivers/gpu/drm/vgem/vgem_drv.c
> @@ -34,6 +34,9 @@
>  #include 
>  #include 
>  #include 
> +
> +#include 
> +
>  #include "vgem_drv.h"
>  
>  #define DRIVER_NAME  "vgem"
> @@ -46,6 +49,12 @@ static void vgem_gem_free_object(struct drm_gem_object 
> *obj)
>  {
>   struct drm_vgem_gem_object *vgem_obj = to_vgem_bo(obj);
>  
> + if (vgem_obj->pages)
> + drm_free_large(vgem_obj->pages);

Just drm_free_large(vgem_obj->pages); NULL -> kfree() which is NULL safe.

The series lgtm, doesn't compromise on any of the existing tests.

All 3,
Reviewed-by: Chris Wilson 

You could always get Joonas to nitpick over style...

> +
> + if (obj->import_attach)
> + drm_prime_gem_destroy(obj, vgem_obj->table);
> +
>   drm_gem_object_release(obj);
>   kfree(vgem_obj);
>  }
> @@ -56,26 +65,48 @@ static int vgem_gem_fault(struct vm_fault *vmf)
>   struct drm_vgem_gem_object *obj = vma->vm_private_data;
>   /* We don't use vmf->pgoff since that has the fake offset */
>   unsigned long vaddr = vmf->address;
> - struct page *page;
> -
> - page = shmem_read_mapping_page(file_inode(obj->base.filp)->i_mapping,
> -(vaddr - vma->vm_start) >> PAGE_SHIFT);
> - if (!IS_ERR(page)) {
> - vmf->page = page;
> - return 0;
> - } else switch (PTR_ERR(page)) {
> - case -ENOSPC:
> - case -ENOMEM:
> - return VM_FAULT_OOM;
> - case -EBUSY:
> - return VM_FAULT_RETRY;
> - case -EFAULT:
> - case -EINVAL:
> - return VM_FAULT_SIGBUS;
> - default:
> - WARN_ON_ONCE(PTR_ERR(page));
> - return VM_FAULT_SIGBUS;
> + int ret;
> + loff_t num_pages;
> + pgoff_t page_offset;
> + page_offset = (vaddr - vma->vm_start) >> PAGE_SHIFT;
> +
> + num_pages = DIV_ROUND_UP(obj->base.size, PAGE_SIZE);
> +
> + if (page_offset > num_pages)
> + return VM_FAULT_SIGBUS;
> +
> + if (obj->pages) {
> + get_page(obj->pages[page_offset]);
> + vmf->page = obj->pages[page_offset];
> + ret = 0;
> + } else {
> + struct page *page;
> +
> + page = shmem_read_mapping_page(
> + file_inode(obj->base.filp)->i_mapping,
> + page_offset);
> + if (!IS_ERR(page)) {
> + vmf->page = page;
> + ret = 0;
> + } else switch (PTR_ERR(page)) {
> + case -ENOSPC:
> + case -ENOMEM:
> + ret = VM_FAULT_OOM;
> + break;
> + case -EBUSY:
> + ret = VM_FAULT_RETRY;
> + break;
> + case -EFAULT:
> + case -EINVAL:
> + ret = VM_FAULT_SIGBUS;
> + break;
> + default:
> + WARN_ON(PTR_ERR(page));
> + ret = VM_FAULT_SIGBUS;

break;
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [linux-sunxi] Re: [PATCH v5 02/11] clk: sunxi-ng: add support for DE2 CCU

[Maxime Ripard]

On Mon, Apr 24, 2017 at 06:26:51PM +0800, icen...@aosc.io wrote:
> 在 2017-04-24 16:51，Maxime Ripard 写道：
> > Hi,
> > 
> > On Sun, Apr 23, 2017 at 06:37:45PM +0800, Icenowy Zheng wrote:
> > > +static const struct of_device_id sunxi_de2_clk_ids[] = {
> > > + {
> > > + .compatible = "allwinner,sun8i-a83t-de2-clk",
> > > + .data = _a83t_de2_clk_desc,
> > > + },
> > > + {
> > > + .compatible = "allwinner,sun50i-h5-de2-clk",
> > > + .data = _a64_de2_clk_desc,
> > > + },
> > > + /*
> > > +  * The Allwinner A64 SoC needs some bit to be poke in syscon to make
> > > +  * DE2 really working.
> > > +  * So there's currently no A64 compatible here.
> > > +  * H5 shares the same reset line with A64, so here H5 is using the
> > > +  * clock description of A64.
> > > +  */
> > > + { }
> > > +};
> > 
> > So that A64 driver would require more than just what you defined in
> > the binding in order to operate?
> 
> Yes. When trying to do A64 driver, I will send out first a patch to
> add the needed binding bit.

Then remove the A64 compatible from the binding document.

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


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

Re: [PATCH] drm/i915/gvt: fix typo: "supporte" -> "support"

[Zhenyu Wang]

On 2017.04.25 10:05:12 +0100, Colin King wrote:
> From: Colin Ian King 
> 
> trivial fix to typo in WARN_ONCE message
> 
> Signed-off-by: Colin Ian King 
> ---
>  drivers/gpu/drm/i915/gvt/handlers.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/handlers.c 
> b/drivers/gpu/drm/i915/gvt/handlers.c
> index 0ad1a508e2af..c995e540ff96 100644
> --- a/drivers/gpu/drm/i915/gvt/handlers.c
> +++ b/drivers/gpu/drm/i915/gvt/handlers.c
> @@ -1244,7 +1244,7 @@ static int dma_ctrl_write(struct intel_vgpu *vgpu, 
> unsigned int offset,
>   mode = vgpu_vreg(vgpu, offset);
>  
>   if (GFX_MODE_BIT_SET_IN_MASK(mode, START_DMA)) {
> - WARN_ONCE(1, "VM(%d): iGVT-g doesn't supporte GuC\n",
> + WARN_ONCE(1, "VM(%d): iGVT-g doesn't support GuC\n",
>   vgpu->id);
>   return 0;
>   }
> -- 

applied, thanks!

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827


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/rockchip: Set line flag config register in vop_crtc_enable

[Mark yao]

On 2017年04月27日 14:54, Jeffy Chen wrote:

We need to set vop config done after update line flag config, it's a
new requirement for chips newer than rk3368.

Since we would only use line flag irq for vact_end, let's move it to
vop_crtc_enable.

Signed-off-by: Jeffy Chen 


looks good for me:

Acked-by: Mark Yao 



---

  drivers/gpu/drm/rockchip/analogix_dp-rockchip.c |  4 ++--
  drivers/gpu/drm/rockchip/rockchip_drm_drv.h |  3 +--
  drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 20 +---
  3 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c 
b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
index d8fa7a9..9bfdbc6 100644
--- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
@@ -115,8 +115,8 @@ static void analogix_dp_psr_work(struct work_struct *work)
  
  	vact_end = crtc->mode.vtotal - crtc->mode.vsync_start + crtc->mode.vdisplay;
  
-	ret = rockchip_drm_wait_line_flag(dp->encoder.crtc, vact_end,

- PSR_WAIT_LINE_FLAG_TIMEOUT_MS);
+   ret = rockchip_drm_wait_vact_end(dp->encoder.crtc,
+PSR_WAIT_LINE_FLAG_TIMEOUT_MS);
if (ret) {
dev_err(dp->dev, "line flag interrupt did not arrive\n");
return;
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h 
b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
index a48fcce..47905fa 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
@@ -62,8 +62,7 @@ int rockchip_drm_dma_attach_device(struct drm_device *drm_dev,
   struct device *dev);
  void rockchip_drm_dma_detach_device(struct drm_device *drm_dev,
struct device *dev);
-int rockchip_drm_wait_line_flag(struct drm_crtc *crtc, unsigned int line_num,
-   unsigned int mstimeout);
+int rockchip_drm_wait_vact_end(struct drm_crtc *crtc, unsigned int mstimeout);
  
  extern struct platform_driver cdn_dp_driver;

  extern struct platform_driver dw_hdmi_rockchip_pltfm_driver;
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c 
b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index 3f7a82d..40a5e6e 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -468,7 +468,7 @@ static bool vop_line_flag_irq_is_enabled(struct vop *vop)
return !!line_flag_irq;
  }
  
-static void vop_line_flag_irq_enable(struct vop *vop, int line_num)

+static void vop_line_flag_irq_enable(struct vop *vop)
  {
unsigned long flags;
  
@@ -477,7 +477,6 @@ static void vop_line_flag_irq_enable(struct vop *vop, int line_num)
  
  	spin_lock_irqsave(>irq_lock, flags);
  
-	VOP_CTRL_SET(vop, line_flag_num[0], line_num);

VOP_INTR_SET_TYPE(vop, clear, LINE_FLAG_INTR, 1);
VOP_INTR_SET_TYPE(vop, enable, LINE_FLAG_INTR, 1);
  
@@ -981,6 +980,8 @@ static void vop_crtc_enable(struct drm_crtc *crtc)

VOP_CTRL_SET(vop, vact_st_end, val);
VOP_CTRL_SET(vop, vpost_st_end, val);
  
+	VOP_CTRL_SET(vop, line_flag_num[0], vact_end);

+
clk_set_rate(vop->dclk, adjusted_mode->clock * 1000);
  
  	VOP_CTRL_SET(vop, standby, 0);

@@ -1507,19 +1508,16 @@ static void vop_win_init(struct vop *vop)
  }
  
  /**

- * rockchip_drm_wait_line_flag - acqiure the give line flag event
+ * rockchip_drm_wait_vact_end
   * @crtc: CRTC to enable line flag
- * @line_num: interested line number
   * @mstimeout: millisecond for timeout
   *
- * Driver would hold here until the interested line flag interrupt have
- * happened or timeout to wait.
+ * Wait for vact_end line flag irq or timeout.
   *
   * Returns:
   * Zero on success, negative errno on failure.
   */
-int rockchip_drm_wait_line_flag(struct drm_crtc *crtc, unsigned int line_num,
-   unsigned int mstimeout)
+int rockchip_drm_wait_vact_end(struct drm_crtc *crtc, unsigned int mstimeout)
  {
struct vop *vop = to_vop(crtc);
unsigned long jiffies_left;
@@ -1527,14 +1525,14 @@ int rockchip_drm_wait_line_flag(struct drm_crtc *crtc, 
unsigned int line_num,
if (!crtc || !vop->is_enabled)
return -ENODEV;
  
-	if (line_num > crtc->mode.vtotal || mstimeout <= 0)

+   if (mstimeout <= 0)
return -EINVAL;
  
  	if (vop_line_flag_irq_is_enabled(vop))

return -EBUSY;
  
  	reinit_completion(>line_flag_completion);

-   vop_line_flag_irq_enable(vop, line_num);
+   vop_line_flag_irq_enable(vop);
  
  	jiffies_left = wait_for_completion_timeout(>line_flag_completion,

   msecs_to_jiffies(mstimeout));
@@ -1547,7 +1545,7 @@ int rockchip_drm_wait_line_flag(struct drm_crtc *crtc, 
unsigned int line_num,
  
  	return 0;

  }

Re: [PATCH 3/6] drm: fourcc byteorder: add bigendian support to drm_mode_legacy_fb_format

[Michel Dänzer]

On 27/04/17 03:45 PM, Gerd Hoffmann wrote:
>   Hi,
> 
>>> That is done using the RADEON_TILING_SWAP_{16,32}BIT flag mentioned in
>>> another thread?
>>
>> Right.
>>
>>
>>> What about dumb bos?  You've mentioned the swap flag isn't used for
>>> those.  Which implies they are in little endian byte order (both gpu and
>>> cpu view).
>>
>> Right, AFAICT from looking at the code.
> 
> Ok.
> 
> And I also don't see an easy way to make them big endian (cpu view)
> using swapping with the existing drm interfaces, given we apply a format
> when we put the bo into use as framebuffer, not when creating it.  So
> userspace can: (1) create dumb bo, (2) map bo, (3) write something bo,
> (4) create fb + attach to crtc.  And at (3) we don't know the format
> yet, so we can't configure swapping accordingly.
> 
> So just not using the swapping indeed looks like the only sensible
> option.  Which in turn implies there is no BGRA support for dumb
> bos.  Hmm, I can see the problem.  Userspace expectation appears to be
> that ADDFB configures a native endian framebuffer, which the driver
> simply can't do on bigendian.

... with pre-R600 GPUs.


> So, what can/should the driver do here?  Throw errors for ADDFB and
> force userspace to use ADDFB2?  From a design point of view the best
> option, but in the other hand I suspect that could break the xorg radeon
> driver ...

Yes, it would.

One thing we could do is provide a way for userspace to query the
effective format(s) as seen by the GPU and/or CPU.

It might also make sense for the radeon driver to set the
RADEON_TILING_SWAP_{16,32}BIT flags for dumb BOs. I wonder about the
status of apps using dumb BOs directly wrt this discussion.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm/rockchip: Set line flag config register in vop_crtc_enable

[Jeffy Chen]

We need to set vop config done after update line flag config, it's a
new requirement for chips newer than rk3368.

Since we would only use line flag irq for vact_end, let's move it to
vop_crtc_enable.

Signed-off-by: Jeffy Chen 

---

 drivers/gpu/drm/rockchip/analogix_dp-rockchip.c |  4 ++--
 drivers/gpu/drm/rockchip/rockchip_drm_drv.h |  3 +--
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 20 +---
 3 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c 
b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
index d8fa7a9..9bfdbc6 100644
--- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
@@ -115,8 +115,8 @@ static void analogix_dp_psr_work(struct work_struct *work)
 
vact_end = crtc->mode.vtotal - crtc->mode.vsync_start + 
crtc->mode.vdisplay;
 
-   ret = rockchip_drm_wait_line_flag(dp->encoder.crtc, vact_end,
- PSR_WAIT_LINE_FLAG_TIMEOUT_MS);
+   ret = rockchip_drm_wait_vact_end(dp->encoder.crtc,
+PSR_WAIT_LINE_FLAG_TIMEOUT_MS);
if (ret) {
dev_err(dp->dev, "line flag interrupt did not arrive\n");
return;
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h 
b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
index a48fcce..47905fa 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
@@ -62,8 +62,7 @@ int rockchip_drm_dma_attach_device(struct drm_device *drm_dev,
   struct device *dev);
 void rockchip_drm_dma_detach_device(struct drm_device *drm_dev,
struct device *dev);
-int rockchip_drm_wait_line_flag(struct drm_crtc *crtc, unsigned int line_num,
-   unsigned int mstimeout);
+int rockchip_drm_wait_vact_end(struct drm_crtc *crtc, unsigned int mstimeout);
 
 extern struct platform_driver cdn_dp_driver;
 extern struct platform_driver dw_hdmi_rockchip_pltfm_driver;
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c 
b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index 3f7a82d..40a5e6e 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -468,7 +468,7 @@ static bool vop_line_flag_irq_is_enabled(struct vop *vop)
return !!line_flag_irq;
 }
 
-static void vop_line_flag_irq_enable(struct vop *vop, int line_num)
+static void vop_line_flag_irq_enable(struct vop *vop)
 {
unsigned long flags;
 
@@ -477,7 +477,6 @@ static void vop_line_flag_irq_enable(struct vop *vop, int 
line_num)
 
spin_lock_irqsave(>irq_lock, flags);
 
-   VOP_CTRL_SET(vop, line_flag_num[0], line_num);
VOP_INTR_SET_TYPE(vop, clear, LINE_FLAG_INTR, 1);
VOP_INTR_SET_TYPE(vop, enable, LINE_FLAG_INTR, 1);
 
@@ -981,6 +980,8 @@ static void vop_crtc_enable(struct drm_crtc *crtc)
VOP_CTRL_SET(vop, vact_st_end, val);
VOP_CTRL_SET(vop, vpost_st_end, val);
 
+   VOP_CTRL_SET(vop, line_flag_num[0], vact_end);
+
clk_set_rate(vop->dclk, adjusted_mode->clock * 1000);
 
VOP_CTRL_SET(vop, standby, 0);
@@ -1507,19 +1508,16 @@ static void vop_win_init(struct vop *vop)
 }
 
 /**
- * rockchip_drm_wait_line_flag - acqiure the give line flag event
+ * rockchip_drm_wait_vact_end
  * @crtc: CRTC to enable line flag
- * @line_num: interested line number
  * @mstimeout: millisecond for timeout
  *
- * Driver would hold here until the interested line flag interrupt have
- * happened or timeout to wait.
+ * Wait for vact_end line flag irq or timeout.
  *
  * Returns:
  * Zero on success, negative errno on failure.
  */
-int rockchip_drm_wait_line_flag(struct drm_crtc *crtc, unsigned int line_num,
-   unsigned int mstimeout)
+int rockchip_drm_wait_vact_end(struct drm_crtc *crtc, unsigned int mstimeout)
 {
struct vop *vop = to_vop(crtc);
unsigned long jiffies_left;
@@ -1527,14 +1525,14 @@ int rockchip_drm_wait_line_flag(struct drm_crtc *crtc, 
unsigned int line_num,
if (!crtc || !vop->is_enabled)
return -ENODEV;
 
-   if (line_num > crtc->mode.vtotal || mstimeout <= 0)
+   if (mstimeout <= 0)
return -EINVAL;
 
if (vop_line_flag_irq_is_enabled(vop))
return -EBUSY;
 
reinit_completion(>line_flag_completion);
-   vop_line_flag_irq_enable(vop, line_num);
+   vop_line_flag_irq_enable(vop);
 
jiffies_left = wait_for_completion_timeout(>line_flag_completion,
   msecs_to_jiffies(mstimeout));
@@ -1547,7 +1545,7 @@ int rockchip_drm_wait_line_flag(struct drm_crtc *crtc, 
unsigned int line_num,
 
return 0;
 }
-EXPORT_SYMBOL(rockchip_drm_wait_line_flag);
+EXPORT_SYMBOL(rockchip_drm_wait_vact_end);
 
 static int 

Re: [PATCH 3/6] drm: fourcc byteorder: add bigendian support to drm_mode_legacy_fb_format

[Gerd Hoffmann]

  Hi,

> > That is done using the RADEON_TILING_SWAP_{16,32}BIT flag mentioned in
> > another thread?
> 
> Right.
> 
> 
> > What about dumb bos?  You've mentioned the swap flag isn't used for
> > those.  Which implies they are in little endian byte order (both gpu and
> > cpu view).
> 
> Right, AFAICT from looking at the code.

Ok.

And I also don't see an easy way to make them big endian (cpu view)
using swapping with the existing drm interfaces, given we apply a format
when we put the bo into use as framebuffer, not when creating it.  So
userspace can: (1) create dumb bo, (2) map bo, (3) write something bo,
(4) create fb + attach to crtc.  And at (3) we don't know the format
yet, so we can't configure swapping accordingly.

So just not using the swapping indeed looks like the only sensible
option.  Which in turn implies there is no BGRA support for dumb
bos.  Hmm, I can see the problem.  Userspace expectation appears to be
that ADDFB configures a native endian framebuffer, which the driver
simply can't do on bigendian.

So, what can/should the driver do here?  Throw errors for ADDFB and
force userspace to use ADDFB2?  From a design point of view the best
option, but in the other hand I suspect that could break the xorg radeon
driver ...

cheers,
  Gerd

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