[PATCH] drm/atomic_helper: Add drm_atomic_helper_disable_planes_on_crtc()

2015-11-23 Thread Jyri Sarha
On 11/23/15 18:09, Daniel Vetter wrote:
> On Mon, Nov 23, 2015 at 12:44:35PM +0200, Jyri Sarha wrote:
>> Add drm_atomic_helper_disable_planes_on_crtc() for disabling all
>> planes associated with the given CRTC. This can be used for instance
>> in the CRTC helper disable callback to disable all planes before
>> shutting down the display pipeline.
>>
>> Signed-off-by: Jyri Sarha 
>> ---
>> This a follow version to "[PATCH RFC] drm/crtc_helper: Add
>> drm_crtc_helper_disable_planes()"-patch [1], with Daniel Vetter's
>> review comments [2] implemented. Hope I got everything right this
>> time. Thanks a lot for the prompt review.
>
> When resending a patch please add a revision log to remind reviewers of
> what you changed. Otherwise they have to dig out the old thread again to
> figure that out. E.g.
>
> v2 per Daniel's feedback:
> - Improve kerneldoc.
> - WARN_ON when atomic_disable is missing.
> - Also use crtc_funcs->atomic_begin/atomic_flush optionally.

I usually do that, but not when I drop RFC off, but I'll do it next time 
even then.

>>
>> Best regards,
>> Jyri
>>
>> [1] http://www.spinics.net/lists/dri-devel/msg94720.html
>> [2] http://www.spinics.net/lists/dri-devel/msg94722.html
>>
>>   drivers/gpu/drm/drm_atomic_helper.c | 52 
>> +
>>   include/drm/drm_atomic_helper.h |  2 ++
>>   2 files changed, 54 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
>> b/drivers/gpu/drm/drm_atomic_helper.c
>> index a53ed7a..229c810 100644
>> --- a/drivers/gpu/drm/drm_atomic_helper.c
>> +++ b/drivers/gpu/drm/drm_atomic_helper.c
>> @@ -1281,6 +1281,58 @@ void drm_atomic_helper_commit_planes(struct 
>> drm_device *dev,
>>   EXPORT_SYMBOL(drm_atomic_helper_commit_planes);
>>
>>   /**
>> + * drm_atomic_helper_disable_planes_on_crtc - helper to disable CRTC's 
>> planes
>> + * @crtc: CRTC
>> + * @atomic: if set, synchronize with CRTC's atomic_begin/flush hooks
>> + *
>> + * Disables all planes associated with the given CRTC. This can be
>> + * used for instance in the CRTC helper disable callback to disable
>> + * all planes before shutting down the display pipeline.
>> + *
>> + * If there are planes to disable and the atomic-parameter is set the
>> + * function calls the CRTC's atomic_begin hook before and atomic_flush
>> + * hook after disabling the planes.
>> + *
>> + * It is a bug to call this function without having implemented the
>> + * ->atomic_disable() plane hook.
>> + */
>> +void drm_atomic_helper_disable_planes_on_crtc(struct drm_crtc *crtc,
>> +  bool atomic)
>> +{
>> +const struct drm_crtc_helper_funcs *crtc_funcs =
>> +crtc->helper_private;
>> +struct drm_plane *plane;
>> +bool flush = false;
>> +
>> +if (!crtc_funcs || !crtc_funcs->atomic_begin)
>> +atomic = false;
>> +
>> +drm_for_each_plane(plane, crtc->dev) {
>> +const struct drm_plane_helper_funcs *plane_funcs =
>> +plane->helper_private;
>> +
>> +if (plane->state->crtc != crtc || !plane_funcs)
>> +continue;
>> +
>> +if (atomic && !flush) {
>> +crtc_funcs->atomic_begin(crtc, NULL);
>> +flush = true;
>
> I think it's clearer if you do that upfront with a simple
>
>   if (crtc_funcs->atomic_begin && atomic)
>   crtc_funcs->atomic_begin();
>

That would cause ->atomic_begin() and ->atomic_flush() cycle even when 
there is no planes to disable. At least with omapdrm that causes a write 
HW and crtc_disable() is called once at probe time when the piece of HW 
not yet enabled.

I am not that familiar with either drm or omapdrm yet to tell if that is 
wrong, but in any case, calling ->atomic_begin() and ->atomic_flush() 
for nothing makes no sense to me.

Would you like it better if there would be two drm_for_each_plane() 
loops, one for just checking if there is anything to do and the second 
doing actual job and ->atomic_begin() there in between?


>> +}
>> +
>> +WARN_ON(!plane_funcs->atomic_disable);
>> +if (plane_funcs->atomic_disable)
>> +plane_funcs->atomic_disable(plane, NULL);
>> +}
>> +
>> +if (flush) {
>> +WARN_ON(!crtc_funcs->atomic_flush);
>> +if (crtc_funcs->atomic_flush)
>> +crtc_funcs->atomic_flush(crtc, NULL);
>> +}
>
> Same here below. Imo the tracking you do in flush isn't required, since
> drivers can opt to not implement either atomic_begin or atomic_flush (on
> some hw you only need atomic_flush, and your code would break such a
> driver here).
>

Ok, thanks I did not know that. I'll fix that.

> In short the code flow for atomic_begin/flush should look exactly like in
> drm_atomic_helper_commit_planes_on_crtc, except for the added && atomic
> check.
>
> Otherwise looks good.
>
> Cheers, Daniel
>

Thanks,
Jyri

>> +}
>> 

[Bug 93080] War Thunder launcher no longer loads

2015-11-23 Thread bugzilla-dae...@freedesktop.org
https://bugs.freedesktop.org/show_bug.cgi?id=93080

--- Comment #3 from bellamorte42 at gmail.com ---
Of course.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20151123/94855dbe/attachment.html>


Funky new vblank counter regressions in Linux 4.4-rc1

2015-11-23 Thread Ville Syrjälä
On Mon, Nov 23, 2015 at 06:58:34PM +0100, Mario Kleiner wrote:
> 
> 
> On 11/23/2015 04:51 PM, Ville Syrjälä wrote:
> > On Mon, Nov 23, 2015 at 04:23:21PM +0100, Mario Kleiner wrote:
> >> On 11/20/2015 04:34 PM, Ville Syrjälä wrote:
> >>> On Fri, Nov 20, 2015 at 04:24:50PM +0100, Mario Kleiner wrote:
> >>
> >> ...
> >> Ok, but why would that be a bad thing? I think we want it to think it is
> >> in the previous frame if it is called outside the vblank irq context.
> >> The only reason we fudge it to the next frames vblank if i vblank irq is
> >> because we know the vblank irq handler we are executing atm. was meant
> >> to execute within the upcoming vblank for the next frame, so we fudge
> >> the scanout positions and thereby timestamp to correspond to that new
> >> frame. But if something called outside irq context it should get a
> >> scanout position/timestamp that corresponds to "reality".
> >
> > It would be a bad thing since it would cause the timestamp to jump
> > backwards, and that would also cause the frame count guesstimate to go
> > backwards.
> >
> 
> But only if we don't use the dev->driver->get_vblank_counter() method, 
> which we try to use on AMD.

Well, if you do it that way then you have the problem of the hw counter
seeming to jump forward by one after crossing the start of vblank (when
compared to the value you sampled when you processed the early vblank
interrupt).

I guess one silly idea would be to defer the vblank interrupt processing
to a timer, and just schedule it a bit into the future from the actual
interrupt handler.

> Also dev->vblank_time_lock should protect us 
> from concurrent execution of the vblank counting/timestamping in irq 
> context and non-irq context? At least that was one of the purposes of 
> that lock in the past?

The scenarion I outlined doesn't require anything to happen
concurrently.

> 
> -mario
> 
> >>
> >> It would maybe be a problem to get different answers for a query at the
> >> same scanout position if we used .get_scanout_position() for something
> >> else than for calculating vblank timestamps, but we don't do that atm.
> >>
> >> Maybe i'm overlooking something here related to the somewhat rewritten
> >> code from the last year or so? But in the original design this would be
> >> exactly what i intended?
> >>
> >> ...
> >>
>  So it's good enough for typical desktop
>  applications/compositors/games/media players, and a nice improvement
>  over the previous state, but not quite sufficient for applications that
>  need long time consistent vblank counts for long waits of multiple
>  seconds or even minutes. So it's still good to have hw counters if we
>  can get some that are reliable enough.
> >>>
> >>> Ah, I didn't realize you care about small errors in the counter for
> >>> such long periods of vblank off.
> >>>
> >>
> >> Actually, you are right, i was stupid - not enough sleep last friday. I
> >> do care about such small errors, but the long vblank off periods don't
> >> matter at all the way my software works. I query the current count and
> >> timestamp (glXGetSyncValuesOML), calculate a target count based on those
> >> and then schedule a swap for that target count via glXSwapBuffersMscOML.
> >> That swapbuffers call will keep vblank irqs on until the kms pageflip is
> >> queued. So i only care about vblank counts/timestamps being consistent
> >> for short amounts of time, typically 1 video refresh from vblank query
> >> to queueing a vblank event, and then from reception of that
> >> event/queuing the pageflip to pageflip completion event. So even if the
> >> system would be heavily loaded and my code would have big preemption
> >> delays i think counts that are consistent over a few seconds would be
> >> enough to keep things working. Otherwise it wouldn't work now either
> >> with a vblank off after 5 seconds and nouveau not having vblank hw 
> >> counters.
> >>
> >> -mario
> >>
> >>
> 
>  -mario
> 
> 
> >>
> >> -mario
> >>
> 
>  It almost sort of works on the rs600 code path, but i need a bit of 
>  info
>  from you:
> 
>  1. There's this register from the old specs for m76.pdf, which is not
>  part of the current register defines for radeon-kms:
> 
>  "D1CRTC_STATUS_VF_COUNT - RW - 32 bits - [GpuF0MMReg:0x60A8]"
> 
>  It contains the lower 16 bits of framecounter and the 13 bits of
>  vertical scanout position. It seems to give the same readings as the 
>  24
>  bit R_0060A4_D1CRTC_STATUS_FRAME_COUNT we use for the hw counter. 
>  This
>  would come handy.
> 
>  Does Evergreen and later have a same/similar register and where is 
>  it?
> 
>  2. The hw framecounter seems to increment when the vertical scanout
>  position wraps back from (VTOTAL - 1) to 0, at least on the one DCE-3
>  gpu i tested so 

Funky new vblank counter regressions in Linux 4.4-rc1

2015-11-23 Thread Mario Kleiner
On 11/23/2015 09:04 PM, Harry Wentland wrote:
> Hi Mario,
>
> when we've had issues with this on amdgpu Christian fixed it by enabling
> page flip irq all the time, rather than turning it on when usermode
> request a flip and turning it back off after we handled it. I believe
> that fix exists on radeon already. Michel should have more info on that.
>
> See other comments inline.
>
> Thanks,
> Harry
>
>
> On 2015-11-23 11:02 AM, Mario Kleiner wrote:
>> On 11/20/2015 04:42 AM, Alex Deucher wrote:
>>> On Thu, Nov 19, 2015 at 12:46 PM, Mario Kleiner
>>>  wrote:
 Hi Alex and Michel and Ville,

 it's "fix vblank stuff" time again ;-)
>>>
>>> Adding Harry from our display team.  He might be able to fill in the
>>> blanks of on some of this better than I can.  It might also be worth
>>> checking to see how our DAL (our new display code which is being
>>> developed directly by our display team) code handles this.  It could
>>> be that we are just missing register settings:
>>
>> Thanks Alex! And hello Harry :)
>>
>>> http://cgit.freedesktop.org/~agd5f/linux/log/?h=DAL-wip
>>
>> I'll have a look at this.
>>
>>> Additionally we've published full registers headers for the display
>>> block:
>>> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/amd/include/asic_reg/dce
>>>
>>> The DCE8 stuff should generally apply back to DCE4.  If you have
>>> questions about registers older asics not covered in the hw docs, let
>>> me know.  Note the new headers are dword aligned rather than byte
>>> aligned.
>>>
>>
>> I've tested now with two different progressive modes on DCE3 and one
>> progressive mode on DCE4, the only cards i have atm. So far it seems
>> that the framecounter indeed increments when the vpos from the scanout
>> position query jumps back to zero. Attached for reference is my
>> current patch to radeon-kms. This one seems to work reliably so far,
>> also if i enable the immediate vblank irq disable which we so far only
>> used on intel-kms.
>>
>> But according to this patch the framecounter increment happens
>> somewhere in the middle of vblank.
>>
>> Essentially from the vpos extracted from
>> EVERGREEN_CRTC_STATUS_POSITION which defines start of vblank ("Start"
>> part of EVERGREEN_CRTC_V_BLANK_START_END) until maximum ie. VTOTAL-1
>> the framecounter stays on the count of the old previous scanout cycle.
>> Then when vpos wraps to zero the framecounter increments by 1. And
>> then we have another couple of dozen lines inside vblank until
>> reaching the "End" part of EVERGREEN_CRTC_V_BLANK_START_END and
>> entering active scanout for the new frame.
>>
>> So the position of observed framecounter increment seems to be not
>> close to the end of vblank ("start of frame" indicator?), but a couple
>> of scanlines after start of vblank.
>>
>> E.g., for a 2560x1440 video mode at 60 Hz, start of vblank is 1478,
>> vtotal is 1481, end of vblank is 38. So i enter the vblank and see the
>> old framecounter for vpos = 1478, 1479, 1480, then it wraps to 0 and
>> the framecounter increments by 1, then 38 scanlines later the vblank
>> ends.
>>
>> So i seem to have something that seems to work in practice and this
>> "increment framecounter if vpos wraps back to zero" behavior makes
>> some sense. It just doesn't conform to what those descriptions for
>> start_line and "start of frame" indicator describe?
>>
> This is correct. Our HW doesn't really have a vblank counter but a frame
> counter. The framecounter increments at the start of vsync, which is
> when we wrap to zero which doesn't coincide with the start of vblank.
>
> What we're trying to do with get_vblank_counter isn't the same as what
> framecount gives us, but we could probably do something like:
>
> if (get_scanout_pos > vblank_start)
>return frame_count + 1;
> else
>return frame_count;
>

Great! That's what my current patch does and it seems to work well with 
different video modes on both DCE3 and DCE4. So theory agrees with 
practice again :) - thanks for clarifying this.

So the other problem we have since forever is the vblank irq firing 
before the start of vblank. We are typically 1-2 scanlines before 
vblank_start when we sample the scanout position and framecounter. This 
needs a slightly ugly workaround. Is there a way, maybe some config 
register, to fire the irq at leading edge of vblank instead of a bit too 
early?

thanks,
-mario

> Harry
>
>> I'll test with a few more video modes.
>>
>> thanks,
>> -mario
>>
>>

 Ville's changes to the DRM's drm_handle_vblank() /
 drm_update_vblank_count()
 code in Linux 4.4 not only made that code more elegant, but also
 removed the
 robustness against the vblank irq quirks in AMD hw and similar
 hardware. So
 now i get tons of off-by-one errors and

 "[   432.345] (WW) RADEON(1): radeon_dri2_flip_event_handler: Pageflip
 completion event has impossible msc 24803 < target_msc 24804" XOrg
 messages
 from that 

nouveau: iowrite32 oops & warning at drivers/gpu/drm/nouveau/nouveau_fence.c:198

2015-11-23 Thread Tommi Rantala
2015-11-22 22:49 GMT+02:00 Ilia Mirkin :
> Not sure if these apply here but there are a couple of outstanding
> locking fixes available in
> http://cgit.freedesktop.org/~darktama/nouveau/ -- specifically these
> two:
>
> http://cgit.freedesktop.org/~darktama/nouveau/commit/?id=2f3a56ad019e378a352e9cb7a559f478826f1a87
> http://cgit.freedesktop.org/~darktama/nouveau/commit/?id=4179b15c6e9fcfb253e811e5477debe46c84c395
>
> Not sure if they affect this particular issue, but thought I'd point
> it out. Are you fuzzing with multiple threads, or just one at a time?
> Do you have a branch somewhere public with the changes to add nouveau
> ioctl support to trinity?

Hi!

I applied those two on top of v4.4-rc2, but the same warning and oops
are still easily reproducible. I can test with older kernels and/or
try to bisect when I have more time, unless anyone has better ideas.

I'm actually running unmodified trinity, and for this purpose only
fuzzing the ioctl() syscall from multiple processes, and opening only
the files from /dev/dri/:

$ ./trinity -q -loff -C20 -c ioctl -V /dev/dri/

Trinity knows about a bunch of DRM ioctl commands, but the rest of the
ioctl arguments will be garbage:
https://github.com/kernelslacker/trinity/blob/master/ioctls/drm.c

Tommi


[Bug 80419] XCOM: Enemy Unknown Causes lockup

2015-11-23 Thread bugzilla-dae...@freedesktop.org
https://bugs.freedesktop.org/show_bug.cgi?id=80419

--- Comment #63 from Paulo Dias  ---
sorry, my bad, it was a typo, i meant for the OS sync , like for example the
kf5 (kde 5.4) vsync setting.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20151123/fff2f402/attachment.html>


[Bug 93080] War Thunder launcher no longer loads

2015-11-23 Thread bugzilla-dae...@freedesktop.org
https://bugs.freedesktop.org/show_bug.cgi?id=93080

--- Comment #2 from Ernst Sjöstrand  ---
There is a pinned thread in the War Thunder support forums with a topic like
this:

https://forum.warthunder.com/index.php?/forum/923-linux-related-problems-and-advice/

Are you running with MESA_GL_VERSION_OVERRIDE=4.1COMPAT ?

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20151123/ad0d70e6/attachment.html>


[Bug 82109] Firefox crashes Google Maps, likely related to graphics driver

2015-11-23 Thread bugzilla-dae...@freedesktop.org
https://bugs.freedesktop.org/show_bug.cgi?id=82109

--- Comment #7 from Fabrizio Gennari  ---
It is not happening any more

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20151123/6663deb8/attachment.html>


[Bug 76130] Radeon HD 4570 set dpm state fails after suspend

2015-11-23 Thread bugzilla-dae...@freedesktop.org
https://bugs.freedesktop.org/show_bug.cgi?id=76130

--- Comment #17 from Alex Deucher  ---
Created attachment 120072
  --> https://bugs.freedesktop.org/attachment.cgi?id=120072=edit
possible fix

Does this patch help?  You will still get the error messages, but everything
else should work.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20151123/cd91a067/attachment-0001.html>


[PATCH 13/29] drm/armada: Use a private mutex to protect priv->linear

2015-11-23 Thread Daniel Vetter
On Mon, Nov 23, 2015 at 6:40 PM, Russell King - ARM Linux
 wrote:
> On Mon, Nov 23, 2015 at 10:32:46AM +0100, Daniel Vetter wrote:
>> diff --git a/drivers/gpu/drm/armada/armada_gem.c 
>> b/drivers/gpu/drm/armada/armada_gem.c
>> index e3a86b96af2a..a43690ab18b9 100644
>> --- a/drivers/gpu/drm/armada/armada_gem.c
>> +++ b/drivers/gpu/drm/armada/armada_gem.c
>> @@ -46,7 +46,7 @@ static size_t roundup_gem_size(size_t size)
>>   return roundup(size, PAGE_SIZE);
>>  }
>>
>> -/* dev->struct_mutex is held here */
>> +/* dev_priv->linear_lock is held here */
>>  void armada_gem_free_object(struct drm_gem_object *obj)
>
> This is wrong (unless there's changes which I'm not aware of.)
>
> This function is called from drm_gem_object_free(), which is called from
> drm_gem_object_unreference(), both of which contain:
>
> WARN_ON(!mutex_is_locked(>struct_mutex));
>
> Now, unless you're changing the conditions on which
> drm_gem_object_unreference() to be under dev_priv->linear_lock, the
> above comment becomes misleading.
>
> It'd also need drm_gem_object_unreference_unlocked() to become per-
> driver, so it can take dev_priv->linear_lock, and I don't see anything
> in the patches which I've received which does that.
>
> So, I suspect the new comment is basically false.

Well the patch is false. I should have grabbed the new
dev_priv->linear_lock around the cleanup functions in
armada_gem_free_object. Which is possible since armada never calls an
gem unref function while holding the new lock. Sorry for not
double-checking what I've done, I'll revise.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch


[Bug 93079] Tonga faults and oopses on agd5f drm-fixes-4.4 since fix incorrect mutex usage v3

2015-11-23 Thread bugzilla-dae...@freedesktop.org
https://bugs.freedesktop.org/show_bug.cgi?id=93079

Andy Furniss  changed:

   What|Removed |Added

Summary|Tonga faults and oopses on  |Tonga faults and oopses on
   |agd5f drm-fixes-4.4 |agd5f drm-fixes-4.4 since
   ||fix incorrect mutex usage
   ||v3

--- Comment #2 from Andy Furniss  ---
Unless I eventually manage to lock the one before - and I am failing to so far,
it looks like this starts with -

commit e284022163716ecf11c37fd1057c35d689ef2c11
Author: Christian König 
Date:   Thu Nov 5 19:49:48 2015 +0100

drm/amdgpu: fix incorrect mutex usage v3

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20151123/44d4aa29/attachment.html>


Funky new vblank counter regressions in Linux 4.4-rc1

2015-11-23 Thread Mario Kleiner


On 11/23/2015 04:51 PM, Ville Syrjälä wrote:
> On Mon, Nov 23, 2015 at 04:23:21PM +0100, Mario Kleiner wrote:
>> On 11/20/2015 04:34 PM, Ville Syrjälä wrote:
>>> On Fri, Nov 20, 2015 at 04:24:50PM +0100, Mario Kleiner wrote:
>>
>> ...
>> Ok, but why would that be a bad thing? I think we want it to think it is
>> in the previous frame if it is called outside the vblank irq context.
>> The only reason we fudge it to the next frames vblank if i vblank irq is
>> because we know the vblank irq handler we are executing atm. was meant
>> to execute within the upcoming vblank for the next frame, so we fudge
>> the scanout positions and thereby timestamp to correspond to that new
>> frame. But if something called outside irq context it should get a
>> scanout position/timestamp that corresponds to "reality".
>
> It would be a bad thing since it would cause the timestamp to jump
> backwards, and that would also cause the frame count guesstimate to go
> backwards.
>

But only if we don't use the dev->driver->get_vblank_counter() method, 
which we try to use on AMD. Also dev->vblank_time_lock should protect us 
from concurrent execution of the vblank counting/timestamping in irq 
context and non-irq context? At least that was one of the purposes of 
that lock in the past?

-mario

>>
>> It would maybe be a problem to get different answers for a query at the
>> same scanout position if we used .get_scanout_position() for something
>> else than for calculating vblank timestamps, but we don't do that atm.
>>
>> Maybe i'm overlooking something here related to the somewhat rewritten
>> code from the last year or so? But in the original design this would be
>> exactly what i intended?
>>
>> ...
>>
 So it's good enough for typical desktop
 applications/compositors/games/media players, and a nice improvement
 over the previous state, but not quite sufficient for applications that
 need long time consistent vblank counts for long waits of multiple
 seconds or even minutes. So it's still good to have hw counters if we
 can get some that are reliable enough.
>>>
>>> Ah, I didn't realize you care about small errors in the counter for
>>> such long periods of vblank off.
>>>
>>
>> Actually, you are right, i was stupid - not enough sleep last friday. I
>> do care about such small errors, but the long vblank off periods don't
>> matter at all the way my software works. I query the current count and
>> timestamp (glXGetSyncValuesOML), calculate a target count based on those
>> and then schedule a swap for that target count via glXSwapBuffersMscOML.
>> That swapbuffers call will keep vblank irqs on until the kms pageflip is
>> queued. So i only care about vblank counts/timestamps being consistent
>> for short amounts of time, typically 1 video refresh from vblank query
>> to queueing a vblank event, and then from reception of that
>> event/queuing the pageflip to pageflip completion event. So even if the
>> system would be heavily loaded and my code would have big preemption
>> delays i think counts that are consistent over a few seconds would be
>> enough to keep things working. Otherwise it wouldn't work now either
>> with a vblank off after 5 seconds and nouveau not having vblank hw counters.
>>
>> -mario
>>
>>

 -mario


>>
>> -mario
>>

 It almost sort of works on the rs600 code path, but i need a bit of 
 info
 from you:

 1. There's this register from the old specs for m76.pdf, which is not
 part of the current register defines for radeon-kms:

 "D1CRTC_STATUS_VF_COUNT - RW - 32 bits - [GpuF0MMReg:0x60A8]"

 It contains the lower 16 bits of framecounter and the 13 bits of
 vertical scanout position. It seems to give the same readings as the 24
 bit R_0060A4_D1CRTC_STATUS_FRAME_COUNT we use for the hw counter. This
 would come handy.

 Does Evergreen and later have a same/similar register and where is it?

 2. The hw framecounter seems to increment when the vertical scanout
 position wraps back from (VTOTAL - 1) to 0, at least on the one DCE-3
 gpu i tested so far. Is this so on all asics? And is the hw counter
 increment happening exactly at the moment that vertical scanout 
 position
 jumps back to zero, ie. both events are driven by the same signal? Or 
 is
 the framecounter increment just happening somewhere inside either
 scanline VTOTAL-1 or scanline 0?


 If we can fix this and get it into rc2 or rc3 then we could avoid a bad
 regression and with a bit of luck at the same time improve by being 
 able
 to set dev->vblank_disable_immediate = true then and allow vblank irqs
 to get turned off more aggressively for a bit of extra power saving.

 thanks,
 -mario
>>>
 -- 

[PATCH v2 4/5] drm/exynos: mixer: do blending setup in mixer_cfg_layer()

2015-11-23 Thread Tobias Jakobi
Hey Inki,


Inki Dae wrote:
> 
> 
> 2015년 11월 23일 01:09에 Tobias Jakobi 이(가) 쓴 글:
>> This updates the blending setup when the layer configuration
>> changes (triggered by mixer_win_{commit,disable}).
>>
>> To avoid unnecesary reconfigurations we cache the layer
>> state in the mixer context.
>>
>> Extra care has to be taken for the layer that is currently
>> being enabled/disabled.
>>
>> Signed-off-by: Tobias Jakobi 
>> ---
>>  drivers/gpu/drm/exynos/exynos_mixer.c | 41 
>> +--
>>  1 file changed, 39 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c 
>> b/drivers/gpu/drm/exynos/exynos_mixer.c
>> index ec9659e..1c24fb5 100644
>> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
>> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
>> @@ -99,6 +99,7 @@ struct mixer_context {
>>  struct exynos_drm_plane planes[MIXER_WIN_NR];
>>  const struct layer_cfg  *layer_cfg;
>>  unsigned intnum_layer;
>> +u32 layer_state;
>>  int pipe;
>>  unsigned long   flags;
>>  boolinterlace;
>> @@ -189,6 +190,27 @@ static inline bool is_alpha_format(const struct 
>> mixer_context* ctx, unsigned int
>>  }
>>  }
>>  
>> +static inline u32 get_layer_state(const struct mixer_context *ctx,
>> +unsigned int win, bool enable)
>> +{
>> +u32 enable_state, alpha_state;
>> +
>> +enable_state = ctx->layer_state & 0x;
>> +alpha_state = ctx->layer_state >> 16;
>> +
>> +if (enable)
>> +enable_state |= (1 << win);
>> +else
>> +enable_state &= ~(1 << win);
>> +
>> +if (enable && is_alpha_format(ctx, win))
>> +alpha_state |= (1 << win);
>> +else
>> +alpha_state &= ~(1 << win);
>> +
>> +return ((alpha_state << 16) | enable_state);
>> +}
>> +
>>  static inline u32 vp_reg_read(struct mixer_resources *res, u32 reg_id)
>>  {
>>  return readl(res->vp_regs + reg_id);
>> @@ -370,8 +392,9 @@ static void mixer_general_layer(struct mixer_context 
>> *ctx,
>>  {
>>  u32 val;
>>  struct mixer_resources *res = >mixer_res;
>> +const u32 alpha_state = ctx->layer_state >> 16;
>>  
>> -if (is_alpha_format(ctx, cfg->index)) {
>> +if (alpha_state & (1 << cfg->index)) {
>>  val  = MXR_GRP_CFG_COLOR_KEY_DISABLE; /* no blank key */
>>  val |= MXR_GRP_CFG_BLEND_PRE_MUL;
>>  val |= MXR_GRP_CFG_PIXEL_BLEND_EN; /* blending based on pixel 
>> alpha */
>> @@ -397,10 +420,11 @@ static void mixer_general_layer(struct mixer_context 
>> *ctx,
>>  }
>>  }
>>  
>> -static void mixer_layer_blending(struct mixer_context *ctx, unsigned int 
>> enable_state)
>> +static void mixer_layer_blending(struct mixer_context *ctx)
>>  {
>>  unsigned int i, index;
>>  bool bottom_layer = false;
>> +const u32 enable_state = ctx->layer_state & 0x;
>>  
>>  for (i = 0; i < ctx->num_layer; ++i) {
>>  index = ctx->layer_cfg[i].index;
>> @@ -503,8 +527,19 @@ static void mixer_cfg_layer(struct mixer_context *ctx, 
>> unsigned int win,
>>  bool enable)
>>  {
>>  struct mixer_resources *res = >mixer_res;
>> +u32 new_layer_state;
>>  u32 val = enable ? ~0 : 0;
>>  
>> +new_layer_state = get_layer_state(ctx, win, enable);
>> +if (new_layer_state == ctx->layer_state)
>> +return;
>> +
>> +/*
>> + * Update the layer state so that mixer_layer_blending()
>> + * below can use it.
>> + */
>> +ctx->layer_state = new_layer_state;
> 
> It may be trivial but I think it'd be better to move above line to most 
> bottom of this function.
Sure, I can do that, but I would rather know what's the rationale here.


With best wishes,
Tobias

>> +
>>  switch (win) {
>>  case 0:
>>  mixer_reg_writemask(res, MXR_CFG, val, MXR_CFG_GRP0_ENABLE);
>> @@ -520,6 +555,8 @@ static void mixer_cfg_layer(struct mixer_context *ctx, 
>> unsigned int win,
>>  }
>>  break;
>>  }
>> +
>> +mixer_layer_blending(ctx);
> 
> Here.
> 
> Thanks,
> Inki Dae
> 
>>  }
>>  
>>  static void mixer_run(struct mixer_context *ctx)
>>



[PATCH v2 2/5] drm/exynos: mixer: introduce mixer_layer_blending()

2015-11-23 Thread Tobias Jakobi
Hey Inki,


Inki Dae wrote:
> 
> 
> 2015년 11월 23일 01:09에 Tobias Jakobi 이(가) 쓴 글:
>> This analyses the current layer configuration (which layers
>> are enabled, which have alpha-pixelformat, etc.) and setups
>> blending accordingly.
>>
>> We currently disable all kinds of blending for the bottom-most
>> layer, since configuration of the mixer background is not
>> yet exposed.
>> Also blending is only enabled when the layer has a pixelformat
>> with alpha attached.
>>
>> Signed-off-by: Tobias Jakobi 
>> ---
>>  drivers/gpu/drm/exynos/exynos_mixer.c | 88 
>> +++
>>  drivers/gpu/drm/exynos/regs-mixer.h   |  1 +
>>  2 files changed, 89 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c 
>> b/drivers/gpu/drm/exynos/exynos_mixer.c
>> index 2c1cea3..ec77aad 100644
>> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
>> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
>> @@ -174,6 +174,21 @@ static const u8 filter_cr_horiz_tap4[] = {
>>  70, 59, 48, 37, 27, 19, 11, 5,
>>  };
>>  
>> +static inline bool is_alpha_format(const struct mixer_context* ctx, 
>> unsigned int win)
>> +{
>> +const struct drm_plane_state *state = ctx->planes[win].base.state;
>> +const struct drm_framebuffer *fb = state->fb;
>> +
>> +switch (fb->pixel_format) {
>> +case DRM_FORMAT_ARGB:
>> +case DRM_FORMAT_ARGB1555:
>> +case DRM_FORMAT_ARGB:
>> +return true;
>> +default:
>> +return false;
>> +}
>> +}
>> +
>>  static inline u32 vp_reg_read(struct mixer_resources *res, u32 reg_id)
>>  {
>>  return readl(res->vp_regs + reg_id);
>> @@ -331,6 +346,79 @@ static void mixer_layer_priority(struct mixer_context 
>> *ctx)
>>  mixer_reg_write(>mixer_res, MXR_LAYER_CFG, val);
>>  }
>>  
>> +/* Configure blending for bottom-most layer. */
>> +static void mixer_bottom_layer(struct mixer_context *ctx,
>> +const struct layer_cfg *cfg)
>> +{
>> +u32 val;
>> +struct mixer_resources *res = >mixer_res;
>> +
>> +if (cfg->index == 2) {
>> +val = 0; /* use defaults for video layer */
>> +mixer_reg_write(res, MXR_VIDEO_CFG, val);
>> +} else {
>> +val = MXR_GRP_CFG_COLOR_KEY_DISABLE; /* no blank key */
>> +
>> +/* Don't blend bottom-most layer onto the mixer background. */
>> +mixer_reg_writemask(res, MXR_GRAPHIC_CFG(cfg->index),
>> +val, MXR_GRP_CFG_MISC_MASK);
>> +}
>> +}
>> +
>> +static void mixer_general_layer(struct mixer_context *ctx,
>> +const struct layer_cfg *cfg)
>> +{
>> +u32 val;
>> +struct mixer_resources *res = >mixer_res;
>> +
>> +if (is_alpha_format(ctx, cfg->index)) {
>> +val  = MXR_GRP_CFG_COLOR_KEY_DISABLE; /* no blank key */
>> +val |= MXR_GRP_CFG_BLEND_PRE_MUL;
>> +val |= MXR_GRP_CFG_PIXEL_BLEND_EN; /* blending based on pixel 
>> alpha */
>> +
>> +/* The video layer never has an alpha pixelformat. */
> 
> Looks like above comment isn't related to graphics layer.
Why do you think so? Because it certainly is.


>> +mixer_reg_writemask(res, MXR_GRAPHIC_CFG(cfg->index),
>> +val, MXR_GRP_CFG_MISC_MASK);
>> +} else {
>> +if (cfg->index == 2) {
>> +/*
>> + * No blending at the moment since the NV12/NV21 
>> pixelformats don't
>> + * have an alpha channel. However the mixer supports a 
>> global alpha
>> + * value for a layer. Once this functionality is 
>> exposed, we can
>> + * support blending of the video layer through this.
>> + */
>> +val = 0;
>> +mixer_reg_write(res, MXR_VIDEO_CFG, val);
> 
> Above 'if statement' wouldn't be called because cfg->index has always a value 
> less than 2
> so it should be removed.
Can you explain why you think that index is always < 2 here?


>> +} else {
>> +val = MXR_GRP_CFG_COLOR_KEY_DISABLE;
>> +mixer_reg_writemask(res, MXR_GRAPHIC_CFG(cfg->index),
>> +val, MXR_GRP_CFG_MISC_MASK);
>> +}
>> +}
>> +}
>> +
>> +static void mixer_layer_blending(struct mixer_context *ctx, unsigned int 
>> enable_state)
>> +{
>> +unsigned int i, index;
>> +bool bottom_layer = false;
>> +
>> +for (i = 0; i < ctx->num_layer; ++i) {
>> +index = ctx->layer_cfg[i].index;
>> +
>> +/* Skip layer if it's not enabled. */
>> +if (!(enable_state & (1 << index)))
>> +continue;
>> +
>> +/* Bottom layer needs special handling. */
>> +if (bottom_layer) {
>> +mixer_general_layer(ctx, >layer_cfg[i]);
>> +} else {
>> +mixer_bottom_layer(ctx, 

[PATCH v2 1/5] drm/exynos: mixer: refactor layer setup

2015-11-23 Thread Tobias Jakobi
Hey Inki,


Inki Dae wrote:
> Hi Tobias,
> 
> 2015년 11월 23일 01:09에 Tobias Jakobi 이(가) 쓴 글:
>> First step in allowing a more generic way to setup complex
>> blending for the different layers.
>>
>> Signed-off-by: Tobias Jakobi 
>> ---
>>  drivers/gpu/drm/exynos/exynos_mixer.c | 84 
>> ++-
>>  1 file changed, 73 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c 
>> b/drivers/gpu/drm/exynos/exynos_mixer.c
>> index 7498c6e..2c1cea3 100644
>> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
>> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
>> @@ -63,6 +63,11 @@ struct mixer_resources {
>>  struct clk  *mout_mixer;
>>  };
>>  
>> +struct layer_cfg {
>> +unsigned int index;
>> +unsigned int priority;
>> +};
>> +
>>  enum mixer_version_id {
>>  MXR_VER_0_0_0_16,
>>  MXR_VER_16_0_33_0,
>> @@ -92,6 +97,8 @@ struct mixer_context {
>>  struct drm_device   *drm_dev;
>>  struct exynos_drm_crtc  *crtc;
>>  struct exynos_drm_plane planes[MIXER_WIN_NR];
>> +const struct layer_cfg  *layer_cfg;
>> +unsigned intnum_layer;
>>  int pipe;
>>  unsigned long   flags;
>>  boolinterlace;
>> @@ -110,6 +117,34 @@ struct mixer_drv_data {
>>  boolhas_sclk;
>>  };
>>  
>> +/*
>> + * The default layer priorities for non-VP (video processor)
>> + * and VP mixer configurations.
>> + *
>> + * A higher priority means that the layer is at the top of
>> + * the layer stack.
>> + * Configurations have to be specified with its entries
>> + * sorted with increasing priority.
>> + *
>> + * The default config assumes the following usage scenario:
>> + * layer1: OSD [top]
>> + * layer0: main framebuffer
>> + * video layer: video overlay [bottom]
>> + * Note that the video layer is only usable when the
>> + * VP is available.
>> + */
>> +
>> +static const struct layer_cfg nonvp_default_cfg[] = {
>> +{ .index = 0, .priority = 1 },  /* layer0 */
>> +{ .index = 1, .priority = 2 },  /* layer1 */
>> +};
>> +
>> +static const struct layer_cfg vp_default_cfg[] = {
>> +{ .index = 2, .priority = 1 },  /* video layer */
>> +{ .index = 0, .priority = 2 },  /* layer0 */
>> +{ .index = 1, .priority = 3 },  /* layer1 */
>> +};
>> +
>>  static const u8 filter_y_horiz_tap8[] = {
>>  0,  -1, -1, -1, -1, -1, -1, -1,
>>  -1, -1, -1, -1, -1, 0,  0,  0,
>> @@ -268,6 +303,34 @@ static void vp_default_filter(struct mixer_resources 
>> *res)
>>  filter_cr_horiz_tap4, sizeof(filter_cr_horiz_tap4));
>>  }
>>  
>> +static void mixer_layer_priority(struct mixer_context *ctx)
>> +{
>> +u32 val = 0;
>> +unsigned int i, priority;
>> +
>> +for (i = 0; i < ctx->num_layer; ++i) {
>> +priority = ctx->layer_cfg[i].priority;
>> +BUG_ON(priority > 15);
> 
> What doesn constant, 15 mean? You need to clarify the meaning
> and please, use a macro instead.
If I read the specs correctly the layer priority is encoded with just
4-bits in the corresponding register. I'll add a define to make this
more clear.


> And it'd better to just return in this case so that the layer
> configuration can be set with a default value.
The point is that these (currently) are the default values that
mixer_layer_priority() sets.
The BUG_ON() is there to make sure that nobody changes
{vp,nonvp}_default_cfg[] later with illegal values.


With best wishes,
Tobias


> I think it'd be enough to print out warning message.
> 
> Thanks,
> Inki Dae
> 
>> +
>> +switch (ctx->layer_cfg[i].index) {
>> +case 0:
>> +val |= MXR_LAYER_CFG_GRP0_VAL(priority);
>> +break;
>> +case 1:
>> +val |= MXR_LAYER_CFG_GRP1_VAL(priority);
>> +break;
>> +case 2:
>> +val |= MXR_LAYER_CFG_VP_VAL(priority);
>> +break;
>> +default:
>> +BUG_ON(true);
>> +break;
>> +}
>> +}
>> +
>> +mixer_reg_write(>mixer_res, MXR_LAYER_CFG, val);
>> +}
>> +
>>  static void mixer_vsync_set_update(struct mixer_context *ctx, bool enable)
>>  {
>>  struct mixer_resources *res = >mixer_res;
>> @@ -673,17 +736,7 @@ static void mixer_win_reset(struct mixer_context *ctx)
>>  mixer_reg_writemask(res, MXR_STATUS, MXR_STATUS_16_BURST,
>>  MXR_STATUS_BURST_MASK);
>>  
>> -/* setting default layer priority: layer1 > layer0 > video
>> - * because typical usage scenario would be
>> - * layer1 - OSD
>> - * layer0 - framebuffer
>> - * video - video overlay
>> - */
>> -val = MXR_LAYER_CFG_GRP1_VAL(3);
>> -val |= MXR_LAYER_CFG_GRP0_VAL(2);
>> -if (ctx->vp_enabled)
>> -val 

[PATCH v4 01/13] clk: rockchip: add id for mipidsi sclk on rk3288

2015-11-23 Thread Heiko Stübner
Hi Chris,

Am Freitag, 20. November 2015, 16:15:27 schrieb Chris Zhong:
> Adds a new id for the sclk supplying the mipidsi on rk3288 socs.
> 
> Signed-off-by: Chris Zhong 
> 
> ---
> 
> Changes in v4: None
> Changes in v3: None
> Changes in v2:
> add the mipi clk id in a single patch
> 
>  include/dt-bindings/clock/rk3288-cru.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/include/dt-bindings/clock/rk3288-cru.h
> b/include/dt-bindings/clock/rk3288-cru.h index c719aac..b07cdd3 100644
> --- a/include/dt-bindings/clock/rk3288-cru.h
> +++ b/include/dt-bindings/clock/rk3288-cru.h
> @@ -86,6 +86,7 @@
>  #define SCLK_USBPHY480M_SRC  122
>  #define SCLK_PVTM_CORE   123
>  #define SCLK_PVTM_GPU124
> +#define SCLK_MIPI_24M125

in the TRM (and therefore in the clock-controller) this is labeled as
sclk_mipidsi_24m, so I'd think the clock-id shouls also contain the
DSI part as well, making it SCLK_MIPIDSI_24M


Heiko


[Bug 92944] [Fiji/LLVM/RadeonSI] CS:GO segfaults in llvm

2015-11-23 Thread bugzilla-dae...@freedesktop.org
https://bugs.freedesktop.org/show_bug.cgi?id=92944

--- Comment #4 from Bogar Boris  ---
Created attachment 120065
  --> https://bugs.freedesktop.org/attachment.cgi?id=120065=edit
cs_go console log with R600_DEBUG=vs,gs,ps with patched LLVM-git and mesa-svn

Hi, i tried to recompile llvm-git and mesa-svn including both the patch that i
found in  Michel Dänzer message. The attachment contain the last two mesa debug
dump from the console. Honestly I don't know if you need the full log from the
game start to the crash, but the full log is over 3Mb. I hope this can help,
and this is the dmesg generated from the crash.

[nov23 19:01] csgo_linux[8027]: segfault at c ip f17cb7ed sp
d3d56a0c error 4 in libLLVM-3.8svn.so[f11b6000+29d3000]

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20151123/1174b33c/attachment.html>


[PATCH v5 2/2] drm/bridge: Add I2C based driver for ps8640 bridge

2015-11-23 Thread Jitao Shi
This patch adds drm_bridge driver for parade DSI to eDP bridge chip.

Signed-off-by: Jitao Shi 
---
Change since v4
-fix build error, change Kconfig DRM_PARADE_PS8640 from bool to tristate
---
 drivers/gpu/drm/bridge/Kconfig |   10 +
 drivers/gpu/drm/bridge/Makefile|1 +
 drivers/gpu/drm/bridge/parade-ps8640.c |  471 
 3 files changed, 482 insertions(+)
 create mode 100644 drivers/gpu/drm/bridge/parade-ps8640.c

diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index 639..dcfdbc9 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -41,4 +41,14 @@ config DRM_PARADE_PS8622
---help---
  Parade eDP-LVDS bridge chip driver.

+config DRM_PARADE_PS8640
+   tristate "Parade PS8640 MIPI DSI to eDP Converter"
+   depends on OF
+   select DRM_KMS_HELPER
+   select DRM_PANEL
+   ---help---
+ Choose this option if you have PS8640 for display
+ The PS8640 is a high-performance and low-power
+ MIPI DSI to eDP converter
+
 endmenu
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index d4e28be..272e3c01 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -4,3 +4,4 @@ obj-$(CONFIG_DRM_DW_HDMI) += dw_hdmi.o
 obj-$(CONFIG_DRM_DW_HDMI_AHB_AUDIO) += dw_hdmi-ahb-audio.o
 obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o
 obj-$(CONFIG_DRM_PARADE_PS8622) += parade-ps8622.o
+obj-$(CONFIG_DRM_PARADE_PS8640) += parade-ps8640.o
diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c 
b/drivers/gpu/drm/bridge/parade-ps8640.c
new file mode 100644
index 000..7c83afa
--- /dev/null
+++ b/drivers/gpu/drm/bridge/parade-ps8640.c
@@ -0,0 +1,471 @@
+/*
+ * Copyright (c) 2014 MediaTek Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define PAGE2_GPIO_L   0xa6
+#define PAGE2_GPIO_H   0xa7
+#define PS_GPIO9   BIT(1)
+#define PAGE2_I2C_BYPASS   0xea
+#define I2C_BYPASS_EN  0xd0
+
+#define PAGE3_SET_ADD  0xfe
+#define PAGE3_SET_VAL  0xff
+#define VDO_CTL_ADD0x13
+#define VDO_DIS0x18
+#define VDO_EN 0x1c
+
+#define PAGE4_REV_L0xf0
+#define PAGE4_REV_H0xf1
+#define PAGE4_CHIP_L   0xf2
+#define PAGE4_CHIP_H   0xf3
+
+#define bridge_to_ps8640(e)container_of(e, struct ps8640, bridge)
+#define connector_to_ps8640(e) container_of(e, struct ps8640, connector)
+
+struct ps8640 {
+   struct drm_connector connector;
+   struct drm_bridge bridge;
+   struct i2c_client *page[6];
+   struct ps8640_driver_data *driver_data;
+   struct regulator *pwr_1v2_supply;
+   struct regulator *pwr_3v3_supply;
+   struct drm_panel *panel;
+   struct gpio_desc *gpio_rst_n;
+   struct gpio_desc *gpio_slp_n;
+   struct gpio_desc *gpio_mode_sel_n;
+   bool enabled;
+};
+
+static int ps8640_regr(struct i2c_client *client, u8 reg, u8 *value)
+{
+   int ret;
+
+   ret = i2c_master_send(client, , 1);
+   if (ret <= 0) {
+   dev_err(>dev, "Failed to send i2c command, ret=%d\n",
+   ret);
+   return ret;
+   }
+
+   ret = i2c_master_recv(client, value, 1);
+   if (ret <= 0) {
+   dev_err(>dev, "Failed to recv i2c data, ret=%d\n", ret);
+   return ret;
+   }
+
+   return 0;
+}
+
+static int ps8640_regw(struct i2c_client *client, u8 reg, u8 value)
+{
+   int ret;
+   char buf[2];
+
+   buf[0] = reg;
+   buf[1] = value;
+   ret = i2c_master_send(client, buf, ARRAY_SIZE(buf));
+   if (ret <= 0) {
+   dev_err(>dev, "Failed to send i2c command, ret=%d\n",
+   ret);
+   return ret;
+   }
+
+   return 0;
+}
+
+static bool ps8640_check_valid_id(struct ps8640 *ps_bridge)
+{
+   struct i2c_client *client = ps_bridge->page[4];
+   u8 rev_id_low, rev_id_high, chip_id_low, chip_id_high;
+   int retry_cnt = 0;
+
+   do {
+   ps8640_regr(client, PAGE4_CHIP_H, _id_high);
+   if (chip_id_high != 0x30)
+   dev_info(>dev, "chip_id_high = 0x%x\n",
+chip_id_high);
+   } while ((retry_cnt++ < 2) && (chip_id_high != 0x30));
+
+   

[PATCH v5 1/2] Documentation: bridge: Add documentation for ps8640 DT properties

2015-11-23 Thread Jitao Shi
Add documentation for DT properties supported by
ps8640 DSI-eDP converter.

Signed-off-by: Jitao Shi 
Acked-by: Rob Herring 
Reviewed-by: Philipp Zabel 
---
Changes since v4
-no change
---
 .../devicetree/bindings/display/bridge/ps8640.txt  |   43 
 1 file changed, 43 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/bridge/ps8640.txt

diff --git a/Documentation/devicetree/bindings/display/bridge/ps8640.txt 
b/Documentation/devicetree/bindings/display/bridge/ps8640.txt
new file mode 100644
index 000..a3e0971
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/ps8640.txt
@@ -0,0 +1,43 @@
+ps8640-bridge bindings
+
+Required properties:
+   - compatible: "parade,ps8640"
+   - reg: first page address of the bridge.
+   - sleep-gpios: OF device-tree gpio specification for PD_ pin.
+   - reset-gpios: OF device-tree gpio specification for reset pin.
+   - mode-sel-gpios: OF device-tree gpio specification for mode-sel pin.
+   - vdd12-supply: OF device-tree regulator specification for 1.2V power.
+   - vdd33-supply: OF device-tree regulator specification for 3.3V power.
+   - ports: The device node can contain video interface port nodes per
+the video-interfaces bind[1]. For port at 0,set the reg = <0> 
as
+ps8640 dsi in and port at 1,set the reg = <1> as ps8640 eDP 
out.
+
+[1]: Documentation/devicetree/bindings/media/video-interfaces.txt
+
+Example:
+   edp-bridge at 18 {
+   compatible = "parade,ps8640";
+   reg = <0x18>;
+   sleep-gpios = < 116 GPIO_ACTIVE_HIGH>;
+   reset-gpios = < 115 GPIO_ACTIVE_HIGH>;
+   mode-sel-gpios = < 92 GPIO_ACTIVE_HIGH>;
+   vdd12-supply = <_fixed_1v2>;
+   vdd33-supply = <_vgp2_reg>;
+
+   ports {
+   #address-cells = <1>;
+   #size-cells = <0>;
+   port at 0 {
+   reg = <0>;
+   ps8640_in: endpoint {
+   remote-endpoint = <_out>;
+   };
+   };
+   port at 1 {
+   reg = <1>;
+   ps8640_out: endpoint {
+   remote-endpoint = <_in>;
+   };
+   };
+   };
+   };
-- 
1.7.9.5



Funky new vblank counter regressions in Linux 4.4-rc1

2015-11-23 Thread Ville Syrjälä
On Mon, Nov 23, 2015 at 04:23:21PM +0100, Mario Kleiner wrote:
> On 11/20/2015 04:34 PM, Ville Syrjälä wrote:
> > On Fri, Nov 20, 2015 at 04:24:50PM +0100, Mario Kleiner wrote:
> 
> ...
> 
> >> What we do in radeon-kms is similar. If DRM_CALLED_FROM_VBLIRQ and we
> >> are no more than 1% of the display height away from start of vblank we
> >> fudge scanout position in a way so that the timestamp gets computed for
> >> the soon-to-begin vblank, not the old one.
> >
> > The problem with basing that fudging purely on DRM_CALLED_FROM_VBLIRQ is
> > that if you call .get_scanout_positon() from a non-irq context between
> > the irq firing and start of vblank, you'll think you're still in the
> > previous frame. Hence my suggestion to note down the frame counter when
> > called from the irq, and then keep doing the fudging until you've truly
> > crossed the start of vblank.
> >
> 
> Ok, but why would that be a bad thing? I think we want it to think it is 
> in the previous frame if it is called outside the vblank irq context. 
> The only reason we fudge it to the next frames vblank if i vblank irq is 
> because we know the vblank irq handler we are executing atm. was meant 
> to execute within the upcoming vblank for the next frame, so we fudge 
> the scanout positions and thereby timestamp to correspond to that new 
> frame. But if something called outside irq context it should get a 
> scanout position/timestamp that corresponds to "reality".

It would be a bad thing since it would cause the timestamp to jump
backwards, and that would also cause the frame count guesstimate to go
backwards.

> 
> It would maybe be a problem to get different answers for a query at the 
> same scanout position if we used .get_scanout_position() for something 
> else than for calculating vblank timestamps, but we don't do that atm.
> 
> Maybe i'm overlooking something here related to the somewhat rewritten 
> code from the last year or so? But in the original design this would be 
> exactly what i intended?
> 
> ...
> 
> >> So it's good enough for typical desktop
> >> applications/compositors/games/media players, and a nice improvement
> >> over the previous state, but not quite sufficient for applications that
> >> need long time consistent vblank counts for long waits of multiple
> >> seconds or even minutes. So it's still good to have hw counters if we
> >> can get some that are reliable enough.
> >
> > Ah, I didn't realize you care about small errors in the counter for
> > such long periods of vblank off.
> >
> 
> Actually, you are right, i was stupid - not enough sleep last friday. I 
> do care about such small errors, but the long vblank off periods don't 
> matter at all the way my software works. I query the current count and 
> timestamp (glXGetSyncValuesOML), calculate a target count based on those 
> and then schedule a swap for that target count via glXSwapBuffersMscOML. 
> That swapbuffers call will keep vblank irqs on until the kms pageflip is 
> queued. So i only care about vblank counts/timestamps being consistent 
> for short amounts of time, typically 1 video refresh from vblank query 
> to queueing a vblank event, and then from reception of that 
> event/queuing the pageflip to pageflip completion event. So even if the 
> system would be heavily loaded and my code would have big preemption 
> delays i think counts that are consistent over a few seconds would be 
> enough to keep things working. Otherwise it wouldn't work now either 
> with a vblank off after 5 seconds and nouveau not having vblank hw counters.
> 
> -mario
> 
> 
> >>
> >> -mario
> >>
> >>
> 
>  -mario
> 
> >>
> >> It almost sort of works on the rs600 code path, but i need a bit of 
> >> info
> >> from you:
> >>
> >> 1. There's this register from the old specs for m76.pdf, which is not
> >> part of the current register defines for radeon-kms:
> >>
> >> "D1CRTC_STATUS_VF_COUNT - RW - 32 bits - [GpuF0MMReg:0x60A8]"
> >>
> >> It contains the lower 16 bits of framecounter and the 13 bits of
> >> vertical scanout position. It seems to give the same readings as the 24
> >> bit R_0060A4_D1CRTC_STATUS_FRAME_COUNT we use for the hw counter. This
> >> would come handy.
> >>
> >> Does Evergreen and later have a same/similar register and where is it?
> >>
> >> 2. The hw framecounter seems to increment when the vertical scanout
> >> position wraps back from (VTOTAL - 1) to 0, at least on the one DCE-3
> >> gpu i tested so far. Is this so on all asics? And is the hw counter
> >> increment happening exactly at the moment that vertical scanout 
> >> position
> >> jumps back to zero, ie. both events are driven by the same signal? Or 
> >> is
> >> the framecounter increment just happening somewhere inside either
> >> scanline VTOTAL-1 or scanline 0?
> >>
> >>
> >> If we can fix this and get it into rc2 or rc3 then we could avoid a bad
> 

[PATCH] staging/android: add TODO to de-stage android sync framework

2015-11-23 Thread Gustavo Padovan
From: Gustavo Padovan 

 - remove sw_sync, it is used only for testing/debugging and should not
be upstreamed.
 - port sw_sync testcases to use debugfs somehow
 - clean up and ABI check for security issues
 - move the sync framework to drivers/base/dma-buf

Cc: Arve Hjønnevåg 
Cc: Riley Andrews 
Cc: Daniel Vetter 
Cc: Rob Clark 
Cc: Greg Hackmann 
Cc: John Harrison 
Signed-off-by: Gustavo Padovan 
---
 drivers/staging/android/TODO | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/staging/android/TODO b/drivers/staging/android/TODO
index 8f3ac37..2375dae 100644
--- a/drivers/staging/android/TODO
+++ b/drivers/staging/android/TODO
@@ -25,5 +25,12 @@ ion/
exposes existing cma regions and doesn't reserve unecessarily memory when
booting a system which doesn't use ion.

+sync framework:
+ - remove sw_sync, it is used only for testing/debugging and should not be
+upstreamed.
+ - port sw_sync testcases to use debugfs somehow
+ - clean up and ABI check for security issues
+ - move it to drivers/base/dma-buf
+
 Please send patches to Greg Kroah-Hartman  and Cc:
 Arve Hjønnevåg  and Riley Andrews 
-- 
2.1.0



[PATCH 13/29] drm/armada: Use a private mutex to protect priv->linear

2015-11-23 Thread Russell King - ARM Linux
On Mon, Nov 23, 2015 at 10:32:46AM +0100, Daniel Vetter wrote:
> diff --git a/drivers/gpu/drm/armada/armada_gem.c 
> b/drivers/gpu/drm/armada/armada_gem.c
> index e3a86b96af2a..a43690ab18b9 100644
> --- a/drivers/gpu/drm/armada/armada_gem.c
> +++ b/drivers/gpu/drm/armada/armada_gem.c
> @@ -46,7 +46,7 @@ static size_t roundup_gem_size(size_t size)
>   return roundup(size, PAGE_SIZE);
>  }
>  
> -/* dev->struct_mutex is held here */
> +/* dev_priv->linear_lock is held here */
>  void armada_gem_free_object(struct drm_gem_object *obj)

This is wrong (unless there's changes which I'm not aware of.)

This function is called from drm_gem_object_free(), which is called from
drm_gem_object_unreference(), both of which contain:

WARN_ON(!mutex_is_locked(>struct_mutex));

Now, unless you're changing the conditions on which
drm_gem_object_unreference() to be under dev_priv->linear_lock, the
above comment becomes misleading.

It'd also need drm_gem_object_unreference_unlocked() to become per-
driver, so it can take dev_priv->linear_lock, and I don't see anything
in the patches which I've received which does that.

So, I suspect the new comment is basically false.

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.


[PATCH v2 2/2] drm/rockchip: Convert the probe function to the generic drm_of_component_probe()

2015-11-23 Thread Mark yao
On 2015年11月20日 22:22, Liviu Dudau wrote:
> Initial attempt to convert rockchip to drm_of_component_probe() missed the
> difference between ports and encoders when using the compare_of() function.
> Now that drm_of_component_probe() has been enhanced, let's try again the
> conversion.
>
> Signed-off-by: Liviu Dudau

Looks good for me, and it works on popmetal board, so
  Acked-by: Mark Yao 

Thanks.

-- 
ï¼­ark Yao




[PATCH v2 2/5] drm/exynos: mixer: introduce mixer_layer_blending()

2015-11-23 Thread Inki Dae


2015년 11월 23일 01:09에 Tobias Jakobi 이(가) 쓴 글:
> This analyses the current layer configuration (which layers
> are enabled, which have alpha-pixelformat, etc.) and setups
> blending accordingly.
> 
> We currently disable all kinds of blending for the bottom-most
> layer, since configuration of the mixer background is not
> yet exposed.
> Also blending is only enabled when the layer has a pixelformat
> with alpha attached.
> 
> Signed-off-by: Tobias Jakobi 
> ---
>  drivers/gpu/drm/exynos/exynos_mixer.c | 88 
> +++
>  drivers/gpu/drm/exynos/regs-mixer.h   |  1 +
>  2 files changed, 89 insertions(+)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c 
> b/drivers/gpu/drm/exynos/exynos_mixer.c
> index 2c1cea3..ec77aad 100644
> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
> @@ -174,6 +174,21 @@ static const u8 filter_cr_horiz_tap4[] = {
>   70, 59, 48, 37, 27, 19, 11, 5,
>  };
>  
> +static inline bool is_alpha_format(const struct mixer_context* ctx, unsigned 
> int win)
> +{
> + const struct drm_plane_state *state = ctx->planes[win].base.state;
> + const struct drm_framebuffer *fb = state->fb;
> +
> + switch (fb->pixel_format) {
> + case DRM_FORMAT_ARGB:
> + case DRM_FORMAT_ARGB1555:
> + case DRM_FORMAT_ARGB:
> + return true;
> + default:
> + return false;
> + }
> +}
> +
>  static inline u32 vp_reg_read(struct mixer_resources *res, u32 reg_id)
>  {
>   return readl(res->vp_regs + reg_id);
> @@ -331,6 +346,79 @@ static void mixer_layer_priority(struct mixer_context 
> *ctx)
>   mixer_reg_write(>mixer_res, MXR_LAYER_CFG, val);
>  }
>  
> +/* Configure blending for bottom-most layer. */
> +static void mixer_bottom_layer(struct mixer_context *ctx,
> + const struct layer_cfg *cfg)
> +{
> + u32 val;
> + struct mixer_resources *res = >mixer_res;
> +
> + if (cfg->index == 2) {
> + val = 0; /* use defaults for video layer */
> + mixer_reg_write(res, MXR_VIDEO_CFG, val);
> + } else {
> + val = MXR_GRP_CFG_COLOR_KEY_DISABLE; /* no blank key */
> +
> + /* Don't blend bottom-most layer onto the mixer background. */
> + mixer_reg_writemask(res, MXR_GRAPHIC_CFG(cfg->index),
> + val, MXR_GRP_CFG_MISC_MASK);
> + }
> +}
> +
> +static void mixer_general_layer(struct mixer_context *ctx,
> + const struct layer_cfg *cfg)
> +{
> + u32 val;
> + struct mixer_resources *res = >mixer_res;
> +
> + if (is_alpha_format(ctx, cfg->index)) {
> + val  = MXR_GRP_CFG_COLOR_KEY_DISABLE; /* no blank key */
> + val |= MXR_GRP_CFG_BLEND_PRE_MUL;
> + val |= MXR_GRP_CFG_PIXEL_BLEND_EN; /* blending based on pixel 
> alpha */
> +
> + /* The video layer never has an alpha pixelformat. */

Looks like above comment isn't related to graphics layer.

> + mixer_reg_writemask(res, MXR_GRAPHIC_CFG(cfg->index),
> + val, MXR_GRP_CFG_MISC_MASK);
> + } else {
> + if (cfg->index == 2) {
> + /*
> +  * No blending at the moment since the NV12/NV21 
> pixelformats don't
> +  * have an alpha channel. However the mixer supports a 
> global alpha
> +  * value for a layer. Once this functionality is 
> exposed, we can
> +  * support blending of the video layer through this.
> +  */
> + val = 0;
> + mixer_reg_write(res, MXR_VIDEO_CFG, val);

Above 'if statement' wouldn't be called because cfg->index has always a value 
less than 2
so it should be removed.

> + } else {
> + val = MXR_GRP_CFG_COLOR_KEY_DISABLE;
> + mixer_reg_writemask(res, MXR_GRAPHIC_CFG(cfg->index),
> + val, MXR_GRP_CFG_MISC_MASK);
> + }
> + }
> +}
> +
> +static void mixer_layer_blending(struct mixer_context *ctx, unsigned int 
> enable_state)
> +{
> + unsigned int i, index;
> + bool bottom_layer = false;
> +
> + for (i = 0; i < ctx->num_layer; ++i) {
> + index = ctx->layer_cfg[i].index;
> +
> + /* Skip layer if it's not enabled. */
> + if (!(enable_state & (1 << index)))
> + continue;
> +
> + /* Bottom layer needs special handling. */
> + if (bottom_layer) {
> + mixer_general_layer(ctx, >layer_cfg[i]);
> + } else {
> + mixer_bottom_layer(ctx, >layer_cfg[i]);
> + bottom_layer = true;
> + }

I think above codes could be more cleanned up like below if I understood 
correctly,

if (cfg->index == 2) {
 

[PATCH] drm/atomic_helper: Add drm_atomic_helper_disable_planes_on_crtc()

2015-11-23 Thread Daniel Vetter
On Mon, Nov 23, 2015 at 12:44:35PM +0200, Jyri Sarha wrote:
> Add drm_atomic_helper_disable_planes_on_crtc() for disabling all
> planes associated with the given CRTC. This can be used for instance
> in the CRTC helper disable callback to disable all planes before
> shutting down the display pipeline.
> 
> Signed-off-by: Jyri Sarha 
> ---
> This a follow version to "[PATCH RFC] drm/crtc_helper: Add
> drm_crtc_helper_disable_planes()"-patch [1], with Daniel Vetter's
> review comments [2] implemented. Hope I got everything right this
> time. Thanks a lot for the prompt review.

When resending a patch please add a revision log to remind reviewers of
what you changed. Otherwise they have to dig out the old thread again to
figure that out. E.g.

v2 per Daniel's feedback:
- Improve kerneldoc.
- WARN_ON when atomic_disable is missing.
- Also use crtc_funcs->atomic_begin/atomic_flush optionally.
> 
> Best regards,
> Jyri
> 
> [1] http://www.spinics.net/lists/dri-devel/msg94720.html
> [2] http://www.spinics.net/lists/dri-devel/msg94722.html
> 
>  drivers/gpu/drm/drm_atomic_helper.c | 52 
> +
>  include/drm/drm_atomic_helper.h |  2 ++
>  2 files changed, 54 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
> b/drivers/gpu/drm/drm_atomic_helper.c
> index a53ed7a..229c810 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1281,6 +1281,58 @@ void drm_atomic_helper_commit_planes(struct drm_device 
> *dev,
>  EXPORT_SYMBOL(drm_atomic_helper_commit_planes);
>  
>  /**
> + * drm_atomic_helper_disable_planes_on_crtc - helper to disable CRTC's planes
> + * @crtc: CRTC
> + * @atomic: if set, synchronize with CRTC's atomic_begin/flush hooks
> + *
> + * Disables all planes associated with the given CRTC. This can be
> + * used for instance in the CRTC helper disable callback to disable
> + * all planes before shutting down the display pipeline.
> + *
> + * If there are planes to disable and the atomic-parameter is set the
> + * function calls the CRTC's atomic_begin hook before and atomic_flush
> + * hook after disabling the planes.
> + *
> + * It is a bug to call this function without having implemented the
> + * ->atomic_disable() plane hook.
> + */
> +void drm_atomic_helper_disable_planes_on_crtc(struct drm_crtc *crtc,
> +   bool atomic)
> +{
> + const struct drm_crtc_helper_funcs *crtc_funcs =
> + crtc->helper_private;
> + struct drm_plane *plane;
> + bool flush = false;
> +
> + if (!crtc_funcs || !crtc_funcs->atomic_begin)
> + atomic = false;
> +
> + drm_for_each_plane(plane, crtc->dev) {
> + const struct drm_plane_helper_funcs *plane_funcs =
> + plane->helper_private;
> +
> + if (plane->state->crtc != crtc || !plane_funcs)
> + continue;
> +
> + if (atomic && !flush) {
> + crtc_funcs->atomic_begin(crtc, NULL);
> + flush = true;

I think it's clearer if you do that upfront with a simple

if (crtc_funcs->atomic_begin && atomic)
crtc_funcs->atomic_begin();

> + }
> +
> + WARN_ON(!plane_funcs->atomic_disable);
> + if (plane_funcs->atomic_disable)
> + plane_funcs->atomic_disable(plane, NULL);
> + }
> +
> + if (flush) {
> + WARN_ON(!crtc_funcs->atomic_flush);
> + if (crtc_funcs->atomic_flush)
> + crtc_funcs->atomic_flush(crtc, NULL);
> + }

Same here below. Imo the tracking you do in flush isn't required, since
drivers can opt to not implement either atomic_begin or atomic_flush (on
some hw you only need atomic_flush, and your code would break such a
driver here).

In short the code flow for atomic_begin/flush should look exactly like in
drm_atomic_helper_commit_planes_on_crtc, except for the added && atomic
check.

Otherwise looks good.

Cheers, Daniel

> +}
> +EXPORT_SYMBOL(drm_atomic_helper_disable_planes_on_crtc);
> +
> +/**
>   * drm_atomic_helper_commit_planes_on_crtc - commit plane state for a crtc
>   * @old_crtc_state: atomic state object with the old crtc state
>   *
> diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
> index 8cba54a..b7d4237 100644
> --- a/include/drm/drm_atomic_helper.h
> +++ b/include/drm/drm_atomic_helper.h
> @@ -62,6 +62,8 @@ void drm_atomic_helper_commit_planes(struct drm_device *dev,
>  void drm_atomic_helper_cleanup_planes(struct drm_device *dev,
> struct drm_atomic_state *old_state);
>  void drm_atomic_helper_commit_planes_on_crtc(struct drm_crtc_state 
> *old_crtc_state);
> +void drm_atomic_helper_disable_planes_on_crtc(struct drm_crtc *crtc,
> +   bool atomic);
>  
>  void drm_atomic_helper_swap_state(struct drm_device *dev,
>  

Funky new vblank counter regressions in Linux 4.4-rc1

2015-11-23 Thread Mario Kleiner
in
> interlaced).
>
> When progressive output display modes are enabled
> (DxCRTC_INTERLACE_ENABLE = 0) and the CRTC timing generator is enabled
> (DxCRTC_MASTER_EN = 1), the “start of frame” indicator occurs 2 lines
> before the end of the vertical blank occurs (DxCRTC_V_BLANK_END - 2).
> Similar to interlaced output mode, there is one exception to the
> previous statement.  If scaling is enabled (DxSCL_SCALE_EN = 1) and
> the early start line is enabled (DxCRTC_PROGRESSIVE_START_LINE_EARLY =
> 1) in progressive output mode then the “start of frame” indicator
> happens 3 lines before the end of the vertical blank
> (DxCRTC_V_BLANK_END – 3)
>
> I hope this helps,
>
> Alex
>
>>
>>
>> If we can fix this and get it into rc2 or rc3 then we could avoid a bad
>> regression and with a bit of luck at the same time improve by being able to
>> set dev->vblank_disable_immediate = true then and allow vblank irqs to get
>> turned off more aggressively for a bit of extra power saving.
>>
>> thanks,
>> -mario
-- next part --
A non-text attachment was scrubbed...
Name: radeonframecounter.patch
Type: text/x-patch
Size: 5332 bytes
Desc: not available
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20151123/38a6d7ab/attachment-0001.bin>


[Bug 92953] Segfault loading UT4 map/server

2015-11-23 Thread bugzilla-dae...@freedesktop.org
https://bugs.freedesktop.org/show_bug.cgi?id=92953

--- Comment #15 from bellamorte42 at gmail.com ---
Well, I guess the definition of non-standard is relative.  I didn't change
anything, how ever it is, is the way it came.  So I'm not sure what to
'revert'.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20151123/8501a607/attachment.html>


[PATCH] drm/imx: parallel-display: allow to determine bus format from the connected panel

2015-11-23 Thread Gary Bisson
Philipp, All,

On Fri, Nov 20, 2015 at 01:46:39PM +0100, Philipp Zabel wrote:
> Similarly to commit 5e501ed7253b3 ("drm/imx: imx-ldb: allow to determine
> bus format from the connected panel"), if a panel is connected to the ldb
> output port via the of_graph bindings, the data mapping is determined from
> the display_info.bus_format field provided by the panel instead of from the
> optional interface_pix_fmt device tree property.

Tested with a SabreLite + Okaya rs800480t LCD display on 4.4-rc1 kernel.

Tested-by: Gary Bisson 

One question though, do you want people to remove the interface_pix_fmt
property from the device trees to make it clear the interface depends on
the panel selected?

Regards,
Gary


[Bug 93017] complete system stalls while changing displays resolutions on a hybrid (intel/radeon) system

2015-11-23 Thread bugzilla-dae...@freedesktop.org
https://bugs.freedesktop.org/show_bug.cgi?id=93017

--- Comment #8 from polo  ---
I see you have same laptop as me zbook 14,  DP on docking station are conected
only to AMD GPU.

I have  DRI_PRIME=1 issue  with new kernel (probably start with 3.19) maybe is
related 

https://bugzilla.opensuse.org/show_bug.cgi?id=954783

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20151123/68b07bde/attachment.html>


[PATCH v2 4/5] drm/exynos: mixer: do blending setup in mixer_cfg_layer()

2015-11-23 Thread Inki Dae


2015년 11월 23일 01:09에 Tobias Jakobi 이(가) 쓴 글:
> This updates the blending setup when the layer configuration
> changes (triggered by mixer_win_{commit,disable}).
> 
> To avoid unnecesary reconfigurations we cache the layer
> state in the mixer context.
> 
> Extra care has to be taken for the layer that is currently
> being enabled/disabled.
> 
> Signed-off-by: Tobias Jakobi 
> ---
>  drivers/gpu/drm/exynos/exynos_mixer.c | 41 
> +--
>  1 file changed, 39 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c 
> b/drivers/gpu/drm/exynos/exynos_mixer.c
> index ec9659e..1c24fb5 100644
> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
> @@ -99,6 +99,7 @@ struct mixer_context {
>   struct exynos_drm_plane planes[MIXER_WIN_NR];
>   const struct layer_cfg  *layer_cfg;
>   unsigned intnum_layer;
> + u32 layer_state;
>   int pipe;
>   unsigned long   flags;
>   boolinterlace;
> @@ -189,6 +190,27 @@ static inline bool is_alpha_format(const struct 
> mixer_context* ctx, unsigned int
>   }
>  }
>  
> +static inline u32 get_layer_state(const struct mixer_context *ctx,
> + unsigned int win, bool enable)
> +{
> + u32 enable_state, alpha_state;
> +
> + enable_state = ctx->layer_state & 0x;
> + alpha_state = ctx->layer_state >> 16;
> +
> + if (enable)
> + enable_state |= (1 << win);
> + else
> + enable_state &= ~(1 << win);
> +
> + if (enable && is_alpha_format(ctx, win))
> + alpha_state |= (1 << win);
> + else
> + alpha_state &= ~(1 << win);
> +
> + return ((alpha_state << 16) | enable_state);
> +}
> +
>  static inline u32 vp_reg_read(struct mixer_resources *res, u32 reg_id)
>  {
>   return readl(res->vp_regs + reg_id);
> @@ -370,8 +392,9 @@ static void mixer_general_layer(struct mixer_context *ctx,
>  {
>   u32 val;
>   struct mixer_resources *res = >mixer_res;
> + const u32 alpha_state = ctx->layer_state >> 16;
>  
> - if (is_alpha_format(ctx, cfg->index)) {
> + if (alpha_state & (1 << cfg->index)) {
>   val  = MXR_GRP_CFG_COLOR_KEY_DISABLE; /* no blank key */
>   val |= MXR_GRP_CFG_BLEND_PRE_MUL;
>   val |= MXR_GRP_CFG_PIXEL_BLEND_EN; /* blending based on pixel 
> alpha */
> @@ -397,10 +420,11 @@ static void mixer_general_layer(struct mixer_context 
> *ctx,
>   }
>  }
>  
> -static void mixer_layer_blending(struct mixer_context *ctx, unsigned int 
> enable_state)
> +static void mixer_layer_blending(struct mixer_context *ctx)
>  {
>   unsigned int i, index;
>   bool bottom_layer = false;
> + const u32 enable_state = ctx->layer_state & 0x;
>  
>   for (i = 0; i < ctx->num_layer; ++i) {
>   index = ctx->layer_cfg[i].index;
> @@ -503,8 +527,19 @@ static void mixer_cfg_layer(struct mixer_context *ctx, 
> unsigned int win,
>   bool enable)
>  {
>   struct mixer_resources *res = >mixer_res;
> + u32 new_layer_state;
>   u32 val = enable ? ~0 : 0;
>  
> + new_layer_state = get_layer_state(ctx, win, enable);
> + if (new_layer_state == ctx->layer_state)
> + return;
> +
> + /*
> +  * Update the layer state so that mixer_layer_blending()
> +  * below can use it.
> +  */
> + ctx->layer_state = new_layer_state;

It may be trivial but I think it'd be better to move above line to most bottom 
of this function.

> +
>   switch (win) {
>   case 0:
>   mixer_reg_writemask(res, MXR_CFG, val, MXR_CFG_GRP0_ENABLE);
> @@ -520,6 +555,8 @@ static void mixer_cfg_layer(struct mixer_context *ctx, 
> unsigned int win,
>   }
>   break;
>   }
> +
> + mixer_layer_blending(ctx);

Here.

Thanks,
Inki Dae

>  }
>  
>  static void mixer_run(struct mixer_context *ctx)
> 


[Bug 92258] [regression] Opening context menu in Steam running via DRI_PRIME with enabled DRI3 could lead to radeon kernel module crash

2015-11-23 Thread bugzilla-dae...@freedesktop.org
https://bugs.freedesktop.org/show_bug.cgi?id=92258

--- Comment #12 from polo  ---
how to use this patch? 

I reported this bug on opensuse bugzilla, there's more info
https://bugzilla.opensuse.org/show_bug.cgi?id=954783

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20151123/4e2951a5/attachment.html>


Funky new vblank counter regressions in Linux 4.4-rc1

2015-11-23 Thread Mario Kleiner
On 11/20/2015 04:34 PM, Ville Syrjälä wrote:
> On Fri, Nov 20, 2015 at 04:24:50PM +0100, Mario Kleiner wrote:

...

>> What we do in radeon-kms is similar. If DRM_CALLED_FROM_VBLIRQ and we
>> are no more than 1% of the display height away from start of vblank we
>> fudge scanout position in a way so that the timestamp gets computed for
>> the soon-to-begin vblank, not the old one.
>
> The problem with basing that fudging purely on DRM_CALLED_FROM_VBLIRQ is
> that if you call .get_scanout_positon() from a non-irq context between
> the irq firing and start of vblank, you'll think you're still in the
> previous frame. Hence my suggestion to note down the frame counter when
> called from the irq, and then keep doing the fudging until you've truly
> crossed the start of vblank.
>

Ok, but why would that be a bad thing? I think we want it to think it is 
in the previous frame if it is called outside the vblank irq context. 
The only reason we fudge it to the next frames vblank if i vblank irq is 
because we know the vblank irq handler we are executing atm. was meant 
to execute within the upcoming vblank for the next frame, so we fudge 
the scanout positions and thereby timestamp to correspond to that new 
frame. But if something called outside irq context it should get a 
scanout position/timestamp that corresponds to "reality".

It would maybe be a problem to get different answers for a query at the 
same scanout position if we used .get_scanout_position() for something 
else than for calculating vblank timestamps, but we don't do that atm.

Maybe i'm overlooking something here related to the somewhat rewritten 
code from the last year or so? But in the original design this would be 
exactly what i intended?

...

>> So it's good enough for typical desktop
>> applications/compositors/games/media players, and a nice improvement
>> over the previous state, but not quite sufficient for applications that
>> need long time consistent vblank counts for long waits of multiple
>> seconds or even minutes. So it's still good to have hw counters if we
>> can get some that are reliable enough.
>
> Ah, I didn't realize you care about small errors in the counter for
> such long periods of vblank off.
>

Actually, you are right, i was stupid - not enough sleep last friday. I 
do care about such small errors, but the long vblank off periods don't 
matter at all the way my software works. I query the current count and 
timestamp (glXGetSyncValuesOML), calculate a target count based on those 
and then schedule a swap for that target count via glXSwapBuffersMscOML. 
That swapbuffers call will keep vblank irqs on until the kms pageflip is 
queued. So i only care about vblank counts/timestamps being consistent 
for short amounts of time, typically 1 video refresh from vblank query 
to queueing a vblank event, and then from reception of that 
event/queuing the pageflip to pageflip completion event. So even if the 
system would be heavily loaded and my code would have big preemption 
delays i think counts that are consistent over a few seconds would be 
enough to keep things working. Otherwise it wouldn't work now either 
with a vblank off after 5 seconds and nouveau not having vblank hw counters.

-mario


>>
>> -mario
>>
>>

 -mario

>>
>> It almost sort of works on the rs600 code path, but i need a bit of info
>> from you:
>>
>> 1. There's this register from the old specs for m76.pdf, which is not
>> part of the current register defines for radeon-kms:
>>
>> "D1CRTC_STATUS_VF_COUNT - RW - 32 bits - [GpuF0MMReg:0x60A8]"
>>
>> It contains the lower 16 bits of framecounter and the 13 bits of
>> vertical scanout position. It seems to give the same readings as the 24
>> bit R_0060A4_D1CRTC_STATUS_FRAME_COUNT we use for the hw counter. This
>> would come handy.
>>
>> Does Evergreen and later have a same/similar register and where is it?
>>
>> 2. The hw framecounter seems to increment when the vertical scanout
>> position wraps back from (VTOTAL - 1) to 0, at least on the one DCE-3
>> gpu i tested so far. Is this so on all asics? And is the hw counter
>> increment happening exactly at the moment that vertical scanout position
>> jumps back to zero, ie. both events are driven by the same signal? Or is
>> the framecounter increment just happening somewhere inside either
>> scanline VTOTAL-1 or scanline 0?
>>
>>
>> If we can fix this and get it into rc2 or rc3 then we could avoid a bad
>> regression and with a bit of luck at the same time improve by being able
>> to set dev->vblank_disable_immediate = true then and allow vblank irqs
>> to get turned off more aggressively for a bit of extra power saving.
>>
>> thanks,
>> -mario
>
>> -- a/drivers/gpu/drm/drm_irq.c
>> +++ b/drivers/gpu/drm/drm_irq.c
>> @@ -172,9 +172,11 @@ static void drm_update_vblank_count(struct 
>> drm_device 

[PATCH v2 1/5] drm/exynos: mixer: refactor layer setup

2015-11-23 Thread Inki Dae
Hi Tobias,

2015년 11월 23일 01:09에 Tobias Jakobi 이(가) 쓴 글:
> First step in allowing a more generic way to setup complex
> blending for the different layers.
> 
> Signed-off-by: Tobias Jakobi 
> ---
>  drivers/gpu/drm/exynos/exynos_mixer.c | 84 
> ++-
>  1 file changed, 73 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c 
> b/drivers/gpu/drm/exynos/exynos_mixer.c
> index 7498c6e..2c1cea3 100644
> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
> @@ -63,6 +63,11 @@ struct mixer_resources {
>   struct clk  *mout_mixer;
>  };
>  
> +struct layer_cfg {
> + unsigned int index;
> + unsigned int priority;
> +};
> +
>  enum mixer_version_id {
>   MXR_VER_0_0_0_16,
>   MXR_VER_16_0_33_0,
> @@ -92,6 +97,8 @@ struct mixer_context {
>   struct drm_device   *drm_dev;
>   struct exynos_drm_crtc  *crtc;
>   struct exynos_drm_plane planes[MIXER_WIN_NR];
> + const struct layer_cfg  *layer_cfg;
> + unsigned intnum_layer;
>   int pipe;
>   unsigned long   flags;
>   boolinterlace;
> @@ -110,6 +117,34 @@ struct mixer_drv_data {
>   boolhas_sclk;
>  };
>  
> +/*
> + * The default layer priorities for non-VP (video processor)
> + * and VP mixer configurations.
> + *
> + * A higher priority means that the layer is at the top of
> + * the layer stack.
> + * Configurations have to be specified with its entries
> + * sorted with increasing priority.
> + *
> + * The default config assumes the following usage scenario:
> + * layer1: OSD [top]
> + * layer0: main framebuffer
> + * video layer: video overlay [bottom]
> + * Note that the video layer is only usable when the
> + * VP is available.
> + */
> +
> +static const struct layer_cfg nonvp_default_cfg[] = {
> + { .index = 0, .priority = 1 },  /* layer0 */
> + { .index = 1, .priority = 2 },  /* layer1 */
> +};
> +
> +static const struct layer_cfg vp_default_cfg[] = {
> + { .index = 2, .priority = 1 },  /* video layer */
> + { .index = 0, .priority = 2 },  /* layer0 */
> + { .index = 1, .priority = 3 },  /* layer1 */
> +};
> +
>  static const u8 filter_y_horiz_tap8[] = {
>   0,  -1, -1, -1, -1, -1, -1, -1,
>   -1, -1, -1, -1, -1, 0,  0,  0,
> @@ -268,6 +303,34 @@ static void vp_default_filter(struct mixer_resources 
> *res)
>   filter_cr_horiz_tap4, sizeof(filter_cr_horiz_tap4));
>  }
>  
> +static void mixer_layer_priority(struct mixer_context *ctx)
> +{
> + u32 val = 0;
> + unsigned int i, priority;
> +
> + for (i = 0; i < ctx->num_layer; ++i) {
> + priority = ctx->layer_cfg[i].priority;
> + BUG_ON(priority > 15);

What doesn constant, 15 mean? You need to clarify the meaning
and please, use a macro instead.
And it'd better to just return in this case so that the layer
configuration can be set with a default value.
I think it'd be enough to print out warning message.

Thanks,
Inki Dae

> +
> + switch (ctx->layer_cfg[i].index) {
> + case 0:
> + val |= MXR_LAYER_CFG_GRP0_VAL(priority);
> + break;
> + case 1:
> + val |= MXR_LAYER_CFG_GRP1_VAL(priority);
> + break;
> + case 2:
> + val |= MXR_LAYER_CFG_VP_VAL(priority);
> + break;
> + default:
> + BUG_ON(true);
> + break;
> + }
> + }
> +
> + mixer_reg_write(>mixer_res, MXR_LAYER_CFG, val);
> +}
> +
>  static void mixer_vsync_set_update(struct mixer_context *ctx, bool enable)
>  {
>   struct mixer_resources *res = >mixer_res;
> @@ -673,17 +736,7 @@ static void mixer_win_reset(struct mixer_context *ctx)
>   mixer_reg_writemask(res, MXR_STATUS, MXR_STATUS_16_BURST,
>   MXR_STATUS_BURST_MASK);
>  
> - /* setting default layer priority: layer1 > layer0 > video
> -  * because typical usage scenario would be
> -  * layer1 - OSD
> -  * layer0 - framebuffer
> -  * video - video overlay
> -  */
> - val = MXR_LAYER_CFG_GRP1_VAL(3);
> - val |= MXR_LAYER_CFG_GRP0_VAL(2);
> - if (ctx->vp_enabled)
> - val |= MXR_LAYER_CFG_VP_VAL(1);
> - mixer_reg_write(res, MXR_LAYER_CFG, val);
> + mixer_layer_priority(ctx);
>  
>   /* setting background color */
>   mixer_reg_write(res, MXR_BG_COLOR0, 0x008080);
> @@ -1209,6 +1262,15 @@ static int mixer_probe(struct platform_device *pdev)
>   ctx->vp_enabled = drv->is_vp_enabled;
>   ctx->has_sclk = drv->has_sclk;
>   ctx->mxr_ver = drv->version;
> +
> + if (drv->is_vp_enabled) {
> + ctx->layer_cfg = vp_default_cfg;

[Bug 93080] War Thunder launcher no longer loads

2015-11-23 Thread bugzilla-dae...@freedesktop.org
https://bugs.freedesktop.org/show_bug.cgi?id=93080

--- Comment #1 from bellamorte42 at gmail.com ---
Created attachment 120053
  --> https://bugs.freedesktop.org/attachment.cgi?id=120053=edit
apitrace

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20151123/5d274203/attachment.html>


[Bug 93080] War Thunder launcher no longer loads

2015-11-23 Thread bugzilla-dae...@freedesktop.org
https://bugs.freedesktop.org/show_bug.cgi?id=93080

Bug ID: 93080
   Summary: War Thunder launcher no longer loads
   Product: Mesa
   Version: git
  Hardware: x86-64 (AMD64)
OS: Linux (All)
Status: NEW
  Severity: major
  Priority: medium
 Component: Drivers/Gallium/radeonsi
  Assignee: dri-devel at lists.freedesktop.org
  Reporter: bellamorte42 at gmail.com
QA Contact: dri-devel at lists.freedesktop.org

Created attachment 120052
  --> https://bugs.freedesktop.org/attachment.cgi?id=120052=edit
backtrace

An empty window will pop up and quickly disappear.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20151123/fd6683b5/attachment-0001.html>


[PATCH 4/4] component: add support for releasing match data

2015-11-23 Thread Russell King
The component helper treats the void match data pointer as an opaque
object which needs no further management.  When device nodes being
passed, this is not true: the caller should pass its refcount to the
component helper, and there should be a way to drop the refcount when
the matching information is destroyed.

This patch provides a per-match release method in addition to the match
method to solve this issue.  Rather than using component_match_add(),
users should use component_match_add_release() which takes an additional
function pointer for releasing this reference.

Signed-off-by: Russell King 
---
 drivers/base/component.c  | 101 ++
 include/linux/component.h |  28 +
 2 files changed, 87 insertions(+), 42 deletions(-)

diff --git a/drivers/base/component.c b/drivers/base/component.c
index d99b06b341fb..89f5cf68d80a 100644
--- a/drivers/base/component.c
+++ b/drivers/base/component.c
@@ -20,15 +20,18 @@

 struct component;

+struct component_match_array {
+   void *data;
+   int (*compare)(struct device *, void *);
+   void (*release)(struct device *, void *);
+   struct component *component;
+   bool duplicate;
+};
+
 struct component_match {
size_t alloc;
size_t num;
-   struct {
-   void *data;
-   int (*fn)(struct device *, void *);
-   struct component *component;
-   bool duplicate;
-   } compare[0];
+   struct component_match_array *compare;
 };

 struct master {
@@ -92,6 +95,7 @@ static int find_components(struct master *master)
 * any components which are found to this master.
 */
for (i = 0; i < match->num; i++) {
+   struct component_match_array *mc = >compare[i];
struct component *c;

dev_dbg(master->dev, "Looking for component %zu\n", i);
@@ -99,8 +103,7 @@ static int find_components(struct master *master)
if (match->compare[i].component)
continue;

-   c = find_component(master, match->compare[i].fn,
-  match->compare[i].data);
+   c = find_component(master, mc->compare, mc->data);
if (!c) {
ret = -ENXIO;
break;
@@ -192,41 +195,55 @@ static void take_down_master(struct master *master)
}
 }

-static size_t component_match_size(size_t num)
+static void component_match_release(struct device *master,
+   struct component_match *match)
+{
+   unsigned int i;
+
+   for (i = 0; i < match->num; i++) {
+   struct component_match_array *mc = >compare[i];
+
+   if (mc->release)
+   mc->release(master, mc->data);
+   }
+}
+
+static void devm_component_match_release(struct device *dev, void *res)
 {
-   return offsetof(struct component_match, compare[num]);
+   component_match_release(dev, res);
 }

-static struct component_match *component_match_realloc(struct device *dev,
+static int component_match_realloc(struct device *dev,
struct component_match *match, size_t num)
 {
-   struct component_match *new;
+   struct component_match_array *new;

-   if (match && match->alloc == num)
-   return match;
+   if (match->alloc == num)
+   return 0;

-   new = devm_kmalloc(dev, component_match_size(num), GFP_KERNEL);
+   new = devm_kmalloc_array(dev, num, sizeof(*new), GFP_KERNEL);
if (!new)
-   return ERR_PTR(-ENOMEM);
+   return -ENOMEM;

-   if (match) {
-   memcpy(new, match, component_match_size(min(match->num, num)));
-   devm_kfree(dev, match);
-   } else {
-   new->num = 0;
+   if (match->compare) {
+   memcpy(new, match->compare, sizeof(*new) *
+   min(match->num, num));
+   devm_kfree(dev, match->compare);
}
+   match->compare = new;
+   match->alloc = num;

-   new->alloc = num;
-
-   return new;
+   return 0;
 }

 /*
- * Add a component to be matched.
+ * Add a component to be matched, with a release function.
  *
  * The match array is first created or extended if necessary.
  */
-void component_match_add(struct device *dev, struct component_match **matchptr,
+void component_match_add_release(struct device *master,
+   struct component_match **matchptr,
+   void (*release)(struct device *, void *),
int (*compare)(struct device *, void *), void *compare_data)
 {
struct component_match *match = *matchptr;
@@ -234,23 +251,37 @@ void component_match_add(struct device *dev, struct 
component_match **matchptr,
if (IS_ERR(match))
return;

-   if (!match || match->num == match->alloc) {
-   size_t new_size = match ? match->alloc + 16 : 

[PATCH 3/4] component: track components via array rather than list

2015-11-23 Thread Russell King
Since we now have an array which defines each component, maintain the
components to be bound in the array rather than a separate list.  We
also need duplicate tracking so we can eliminate multiple bind calls
for the same component: we preserve the list-based component order in
that the first match which adds the component determines its position.

Signed-off-by: Russell King 
---
 drivers/base/component.c | 154 ---
 1 file changed, 80 insertions(+), 74 deletions(-)

diff --git a/drivers/base/component.c b/drivers/base/component.c
index cd70b68d9780..d99b06b341fb 100644
--- a/drivers/base/component.c
+++ b/drivers/base/component.c
@@ -18,18 +18,21 @@
 #include 
 #include 

+struct component;
+
 struct component_match {
size_t alloc;
size_t num;
struct {
void *data;
int (*fn)(struct device *, void *);
+   struct component *component;
+   bool duplicate;
} compare[0];
 };

 struct master {
struct list_head node;
-   struct list_head components;
bool bound;

const struct component_master_ops *ops;
@@ -39,7 +42,6 @@ struct master {

 struct component {
struct list_head node;
-   struct list_head master_node;
struct master *master;
bool bound;

@@ -63,46 +65,20 @@ static struct master *__master_find(struct device *dev,
return NULL;
 }

-/* Attach an unattached component to a master. */
-static void component_attach_master(struct master *master, struct component *c)
-{
-   c->master = master;
-
-   list_add_tail(>master_node, >components);
-}
-
-/* Detach a component from a master. */
-static void component_detach_master(struct master *master, struct component *c)
-{
-   list_del(>master_node);
-
-   c->master = NULL;
-}
-
-/*
- * Add a component to a master, finding the component via the compare
- * function and compare data.  This is safe to call for duplicate matches
- * and will not result in the same component being added multiple times.
- */
-static int component_master_add_child(struct master *master,
+static struct component *find_component(struct master *master,
int (*compare)(struct device *, void *), void *compare_data)
 {
struct component *c;
-   int ret = -ENXIO;

list_for_each_entry(c, _list, node) {
if (c->master && c->master != master)
continue;

-   if (compare(c->dev, compare_data)) {
-   if (!c->master)
-   component_attach_master(master, c);
-   ret = 0;
-   break;
-   }
+   if (compare(c->dev, compare_data))
+   return c;
}

-   return ret;
+   return NULL;
 }

 static int find_components(struct master *master)
@@ -116,26 +92,39 @@ static int find_components(struct master *master)
 * any components which are found to this master.
 */
for (i = 0; i < match->num; i++) {
-   ret = component_master_add_child(master,
-match->compare[i].fn,
-match->compare[i].data);
-   if (ret)
+   struct component *c;
+
+   dev_dbg(master->dev, "Looking for component %zu\n", i);
+
+   if (match->compare[i].component)
+   continue;
+
+   c = find_component(master, match->compare[i].fn,
+  match->compare[i].data);
+   if (!c) {
+   ret = -ENXIO;
break;
+   }
+
+   dev_dbg(master->dev, "found component %s, duplicate %u\n", 
dev_name(c->dev), !!c->master);
+
+   /* Attach this component to the master */
+   match->compare[i].duplicate = !!c->master;
+   match->compare[i].component = c;
+   c->master = master;
}
return ret;
 }

-/* Detach all attached components from this master */
-static void master_remove_components(struct master *master)
+/* Detach component from associated master */
+static void remove_component(struct master *master, struct component *c)
 {
-   while (!list_empty(>components)) {
-   struct component *c = list_first_entry(>components,
-   struct component, master_node);
-
-   WARN_ON(c->master != master);
+   size_t i;

-   component_detach_master(master, c);
-   }
+   /* Detach the component from this master. */
+   for (i = 0; i < master->match->num; i++)
+   if (master->match->compare[i].component == c)
+   master->match->compare[i].component = NULL;
 }

 /*
@@ -150,37 +139,32 @@ static int try_to_bring_up_master(struct master 

[PATCH 2/4] component: move check for unbound master into try_to_bring_up_masters()

2015-11-23 Thread Russell King
Clean up the code a little; we don't need to check that the master is
unbound for every invocation of try_to_bring_up_master(), so let's move
it to where it's really needed - try_to_bring_up_masters(), where we may
encounter already bound masters.

Reviewed-by: Thierry Reding 
Signed-off-by: Russell King 
---
 drivers/base/component.c | 15 +--
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/base/component.c b/drivers/base/component.c
index 2ca22738ae92..cd70b68d9780 100644
--- a/drivers/base/component.c
+++ b/drivers/base/component.c
@@ -150,13 +150,6 @@ static int try_to_bring_up_master(struct master *master,
 {
int ret;

-   if (master->bound)
-   return 0;
-
-   /*
-* Search the list of components, looking for components that
-* belong to this master, and attach them to the master.
-*/
if (find_components(master)) {
/* Failed to find all components */
ret = 0;
@@ -196,9 +189,11 @@ static int try_to_bring_up_masters(struct component 
*component)
int ret = 0;

list_for_each_entry(m, , node) {
-   ret = try_to_bring_up_master(m, component);
-   if (ret != 0)
-   break;
+   if (!m->bound) {
+   ret = try_to_bring_up_master(m, component);
+   if (ret != 0)
+   break;
+   }
}

return ret;
-- 
2.1.0



[PATCH 1/4] component: remove old add_components method

2015-11-23 Thread Russell King
Now that drivers create an array of component matches at probe time, we
can retire the old methods.  This involves removing the add_components
master method, and removing component_master_add_child() from public
view.  We also remove component_add_master() as that interface is no
longer useful.

Signed-off-by: Russell King 
---
 drivers/base/component.c  | 31 +--
 include/linux/component.h |  5 -
 2 files changed, 5 insertions(+), 31 deletions(-)

diff --git a/drivers/base/component.c b/drivers/base/component.c
index f748430bb654..2ca22738ae92 100644
--- a/drivers/base/component.c
+++ b/drivers/base/component.c
@@ -84,7 +84,7 @@ static void component_detach_master(struct master *master, 
struct component *c)
  * function and compare data.  This is safe to call for duplicate matches
  * and will not result in the same component being added multiple times.
  */
-int component_master_add_child(struct master *master,
+static int component_master_add_child(struct master *master,
int (*compare)(struct device *, void *), void *compare_data)
 {
struct component *c;
@@ -104,7 +104,6 @@ int component_master_add_child(struct master *master,

return ret;
 }
-EXPORT_SYMBOL_GPL(component_master_add_child);

 static int find_components(struct master *master)
 {
@@ -112,14 +111,6 @@ static int find_components(struct master *master)
size_t i;
int ret = 0;

-   if (!match) {
-   /*
-* Search the list of components, looking for components that
-* belong to this master, and attach them to the master.
-*/
-   return master->ops->add_components(master->dev, master);
-   }
-
/*
 * Scan the array of match functions and attach
 * any components which are found to this master.
@@ -290,15 +281,10 @@ int component_master_add_with_match(struct device *dev,
struct master *master;
int ret;

-   if (ops->add_components && match)
-   return -EINVAL;
-
-   if (match) {
-   /* Reallocate the match array for its true size */
-   match = component_match_realloc(dev, match, match->num);
-   if (IS_ERR(match))
-   return PTR_ERR(match);
-   }
+   /* Reallocate the match array for its true size */
+   match = component_match_realloc(dev, match, match->num);
+   if (IS_ERR(match))
+   return PTR_ERR(match);

master = kzalloc(sizeof(*master), GFP_KERNEL);
if (!master)
@@ -326,13 +312,6 @@ int component_master_add_with_match(struct device *dev,
 }
 EXPORT_SYMBOL_GPL(component_master_add_with_match);

-int component_master_add(struct device *dev,
-   const struct component_master_ops *ops)
-{
-   return component_master_add_with_match(dev, ops, NULL);
-}
-EXPORT_SYMBOL_GPL(component_master_add);
-
 void component_master_del(struct device *dev,
const struct component_master_ops *ops)
 {
diff --git a/include/linux/component.h b/include/linux/component.h
index c00dcc302611..71c434a6a5ee 100644
--- a/include/linux/component.h
+++ b/include/linux/component.h
@@ -17,18 +17,13 @@ void component_unbind_all(struct device *, void *);
 struct master;

 struct component_master_ops {
-   int (*add_components)(struct device *, struct master *);
int (*bind)(struct device *);
void (*unbind)(struct device *);
 };

-int component_master_add(struct device *, const struct component_master_ops *);
 void component_master_del(struct device *,
const struct component_master_ops *);

-int component_master_add_child(struct master *master,
-   int (*compare)(struct device *, void *), void *compare_data);
-
 struct component_match;

 int component_master_add_with_match(struct device *,
-- 
2.1.0



[PATCH 0/4] Component helper updates

2015-11-23 Thread Russell King - ARM Linux
Greg,

These four patches update the component helper by:
* Removing the legacy matching code with the .add_components method
  in the master's operation structure.  Nothing in -rc1 appears to be
  using the legacy functions or method, according to my git greps.

* A slight code reorganisation which results in slightly easier to read
  code.

* Switch to tracking components via an array rather than a list, which
  allows us to keep the matching and matched components together, and
  more importantly allows us to reduce the amount of matching - with
  this structure, we can incrementally add the component devices as
  they become available, rather than re-running the list of matches
  each time something changes.

* Fix the lack of match release functionality, which Liviu Dudau
  reminded me was missing.  This allows people who want to pass
  device_node structures in to (correctly) retain the reference to the
  node, and drop the node when the need to do matches is no longer
  required.

The first three patches have been well tested over the last year as I've
had them in my tree that long.  I hadn't considered them important enough
to send as they're only removing and cleaning up functionality.  However,
with the need to fix something, it now makes sense to get them merged.

The last patch has been tested with etnaviv DRM to prove that the match
release works by unloading the module.  On unload, the release function
is correctly called.

This is intended for the next merge window.  Please apply.

 drivers/base/component.c  | 281 --
 include/linux/component.h |  33 --
 2 files changed, 167 insertions(+), 147 deletions(-)

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.


[Bug 92850] Segfault loading War Thunder

2015-11-23 Thread bugzilla-dae...@freedesktop.org
https://bugs.freedesktop.org/show_bug.cgi?id=92850

--- Comment #14 from bellamorte42 at gmail.com ---
Link to apitrace
https://www.dropbox.com/s/wxoywdk1pa0el2p/aces.7.trace?dl=0

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20151123/148ecd83/attachment.html>


[Bug 93079] Tonga faults and oopses on agd5f drm-fixes-4.4

2015-11-23 Thread bugzilla-dae...@freedesktop.org
https://bugs.freedesktop.org/show_bug.cgi?id=93079

--- Comment #1 from Andy Furniss  ---
Created attachment 120051
  --> https://bugs.freedesktop.org/attachment.cgi?id=120051=edit
dmesg

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20151123/734510ca/attachment.html>


[Bug 93079] Tonga faults and oopses on agd5f drm-fixes-4.4

2015-11-23 Thread bugzilla-dae...@freedesktop.org
https://bugs.freedesktop.org/show_bug.cgi?id=93079

Bug ID: 93079
   Summary: Tonga faults and oopses on agd5f drm-fixes-4.4
   Product: DRI
   Version: DRI git
  Hardware: Other
OS: All
Status: NEW
  Severity: normal
  Priority: medium
 Component: DRM/AMDgpu
  Assignee: dri-devel at lists.freedesktop.org
  Reporter: adf.lists at gmail.com

Created attachment 120050
  --> https://bugs.freedesktop.org/attachment.cgi?id=120050=edit
some backtraces

Haven't tried agd5f drm-fixes-4.4 before today.

It seems that it is unstable on R9 285.

Twice I've been close to submitting a bisect, but false goods have spoilt
things.

Varying backtraces attached.

Will try to bisect again but it's not going to be quick.

I am stable on powerplay and was OK on 4.4-next-wip.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20151123/de477148/attachment.html>


[PATCH 2/2] drm/rockchip: Send events for same-fb flips

2015-11-23 Thread Caesar Wang
Hi,

于 2015年11月16日 20:50, Daniel Stone 写道:
> Rockchip previously treated a pageflip to the same framebuffer as a
> no-op, discarding the event if one was requested. This breaks Weston,
> which, when idle, sends a no-op vblank event to discover vblank
> timings if the vblank query interface is not usable.
>
> Silently dropping events is also quite a hostile thing to do to
> userspace in general.
>
> Signed-off-by: Daniel Stone 
> Cc: Sjoerd Simons 
> Cc: Heiko Stuebner 
> ---
>   drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 30 
> ++---
>   1 file changed, 14 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c 
> b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index ddf6dc2..dad607e 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -916,25 +916,23 @@ static int vop_update_plane_event(struct drm_plane 
> *plane,
>* unreference any previous framebuffers.
>*/
>   mutex_lock(>vsync_mutex);
> - if (fb != vop_win_last_pending_fb(vop_win)) {

There is a warning for building.

CC drivers/gpu/drm/rockchip/rockchip_drm_vop.o
drivers/gpu/drm/rockchip/rockchip_drm_vop.c:753:32: warning: 
'vop_win_last_pending_fb' defined but not used [-Wunused-function]

Maybe, we can also remove the vop_win_last_pending_fb() function.

> - ret = drm_vblank_get(plane->dev, vop->pipe);
> - if (ret) {
> - DRM_ERROR("failed to get vblank, %d\n", ret);
> - mutex_unlock(>vsync_mutex);
> - return ret;
> - }
> + ret = drm_vblank_get(plane->dev, vop->pipe);
> + if (ret) {
> + DRM_ERROR("failed to get vblank, %d\n", ret);
> + mutex_unlock(>vsync_mutex);
> + return ret;
> + }
>   
> - drm_framebuffer_reference(fb);
> + drm_framebuffer_reference(fb);
>   
> - ret = vop_win_queue_fb(vop_win, fb, yrgb_mst, event);
> - if (ret) {
> - drm_vblank_put(plane->dev, vop->pipe);
> - mutex_unlock(>vsync_mutex);
> - return ret;
> - }
> -
> - vop->vsync_work_pending = true;
> + ret = vop_win_queue_fb(vop_win, fb, yrgb_mst, event);
> + if (ret) {
> + drm_vblank_put(plane->dev, vop->pipe);
> + mutex_unlock(>vsync_mutex);
> + return ret;
>   }
> +
> + vop->vsync_work_pending = true;
>   mutex_unlock(>vsync_mutex);
>   
>   spin_lock(>reg_lock);



[PATCH v2 0/7] drm/exynos: add pm runtime support

2015-11-23 Thread Javier Martinez Canillas
Hello Inki,

On 11/23/2015 01:47 PM, Inki Dae wrote:
> 2015-11-23 21:25 GMT+09:00 Javier Martinez Canillas  osg.samsung.com>:
>> Hello,
>>
>> On 11/21/2015 11:59 AM, Inki Dae wrote:
>>> Hi Daniel,
>>>
>>>
>>> 2015-11-21 22:40 GMT+09:00 Daniel Stone :
 Hi Inki,

 On 21 November 2015 at 09:38, Inki Dae  wrote:
> 2015-11-21 1:44 GMT+09:00 Javier Martinez Canillas  osg.samsung.com>:
>> On 11/20/2015 08:13 AM, Inki Dae wrote:
>>> The boot log says,
>>> [5.754493] vdd_ldo9: supplied by vdd_2v
>>> [5.765510] of_graph_get_next_endpoint(): no port node found in 
>>> /dp-controller at 145B
>>>
>>
>> This message is a red herring for the reported issue, the message is also
>> present when the machine boots and the display is brought correctly.
>>
>>> Seems this error is because exynos5800-peach-pit.dts file doesn't have 
>>> 'ports' node in dp node.
>>>
>>> Below is dp node description of exynos5420-peach-pit.dts file.
>>>  {
>>>   status = "okay";
>>>   pinctrl-names = "default";
>>>   pinctrl-0 = <_hpd_gpio>;
>>>   samsung,color-space = <0>;
>>>   samsung,dynamic-range = <0>;
>>>   samsung,ycbcr-coeff = <0>;
>>>   samsung,color-depth = <1>;
>>>   samsung,link-rate = <0x06>;
>>>   samsung,lane-count = <2>;
>>>   samsung,hpd-gpio = < 6 GPIO_ACTIVE_HIGH>;
>>>
>>>   ports {
>>>   port at 0 {
>>>   dp_out: endpoint {
>>>   remote-endpoint = <_in>;
>>>   };
>>>   };
>>>   };
>>> };
>>>
>>> And below is for exynos5800-peash-pit.dts,
>>>  {
>>>   status = "okay";
>>>   pinctrl-names = "default";
>>>   pinctrl-0 = <_hpd_gpio>;
>>>   samsung,color-space = <0>;
>>>   samsung,dynamic-range = <0>;
>>>   samsung,ycbcr-coeff = <0>;
>>>   samsung,color-depth = <1>;
>>>   samsung,link-rate = <0x0a>;
>>>   samsung,lane-count = <2>;
>>>   samsung,hpd-gpio = < 6 GPIO_ACTIVE_HIGH>;
>>>   panel = <>;
>>> };
>>>
>>
>> The difference is because the Exynos5420 Peach Pit Display Port is not
>> attached directly to the display panel, there is an eDP/LVDS bridge chip
>> in the middle (PS8622) while the Exynos5800 Peach Pi doesn't have that.
>>
>> The Exynos DP driver lookups for either a panel phandle or an OF graph
>> endpoint that points to a bridge chip and the bridge enpoint has a port
>> that points to the panel.
>>
>> So the DT is correct but of_graph_get_next_endpoint() always prints an
>
> Then, the DT is really incorrect. As you mentioned, if the Exynos5800 
> Peach PI
> board doesn't use eDP, then the dp node __should be removed__ from
> exynos5800-peach-pit.dts.
>
> From a common-sense standpoint, there is no any reason to build
> and probe dp driver if the board doesn't use dp hardware.

 I agree with what you say, but unfortunately you've slightly misread
 what Javier has said. :) exynos5420-peach-pit has an LVDS panel, with
 the eDP -> LVDS bridge in between (ps8622). exynos5800-peach-pi (from
 which I am writing this) has an eDP panel directly connected. The DT
>>
>> Thanks a lot Daniel for clarifying my comments to Inki :)
>>
 describes both the eDP connector from FIMD and the eDP panel, except
 that there is no connection between the DT nodes.
>>
>> There *is* a connection between the FIMD eDP connector and the eDP panel
>> nodes but these are connected using a phandle while the connection for
>> the FIMD eDP connector and the eDP/LVDS bridge is using the OF graph DT
>> bindings (Documentation/devicetree/bindings/graph.txt).
>>
>> And also the connection between the eDP/LVDS bridge and the LVDS panel
>> is using an OF graph node, so what I meant is that it would be much more
>> consistent if both the eDP -> panel and eDP -> eDP/LVDS bridge -> panel
>> connections used the OF graph DT bindings.
>>
>>>
>>> Right. I misread what Javier said. :)
>>>

>> error if the port so OF graph endpoints it seems can't be optional as
>> used in this driver. Maybe that message should be change to debug then?
>>
>> Another option is to extend the DP driver DT binding to be more generic
>> supporting having a port to a panel besides a bridge, so we could have
>> something like this for Exynos5800 Peach and be consistent in both cases:
>
> It's really not good. This would make it more complex. The best
> solution is just to
> remove the dt node from the device tree file.

 Given the above, not really. Javier's patch seems correct to me - as
 you can see, there is a panel node, and that is the panel that's
 really connected.
>>>
>>> Indeed. Javier's patch will correct it.
>>>
>>

[Bug 92996] [Fiji/amdgpu/Powerplay] Problems with vsync, interactivity, performance, EQ overflow with Powerplay

2015-11-23 Thread bugzilla-dae...@freedesktop.org
https://bugs.freedesktop.org/show_bug.cgi?id=92996

--- Comment #5 from Ernst Sjöstrand  ---
My system only support PCI-Express 2.0, not 3.0. Could that be something?
Does someone _not_ have these problems with Fiji?

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20151123/d861bdea/attachment-0001.html>


[PATCH i915 v6 2/2] i915: wait for fence in prepare_plane_fb

2015-11-23 Thread Alex Goins
In intel_prepare_plane_fb, if fb is backed by dma-buf, wait for exclusive
fence

v2: First commit
v3: Remove object_name_lock acquire
Move wait from intel_atomic_commit() to intel_prepare_plane_fb()
v4: Wait only on exclusive fences, interruptible with no timeout
v5: Style tweaks to more closely match rest of file
v6: Properly handle interrupted waits

Signed-off-by: Alex Goins 
---
 drivers/gpu/drm/i915/intel_display.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index bacf336..604186b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13347,6 +13347,17 @@ intel_prepare_plane_fb(struct drm_plane *plane,
if (!obj)
return 0;

+   /* For framebuffer backed by dmabuf, wait for fence */
+   if (obj->base.dma_buf) {
+   ret = 
reservation_object_wait_timeout_rcu(obj->base.dma_buf->resv,
+ false, true,
+ MAX_SCHEDULE_TIMEOUT);
+   if (ret == -ERESTARTSYS)
+   return ret;
+
+   WARN_ON(ret < 0);
+   }
+
mutex_lock(>struct_mutex);

if (plane->type == DRM_PLANE_TYPE_CURSOR &&
-- 
1.9.1



[PATCH i915 v6 1/2] i915: wait for fence in mmio_flip_work_func

2015-11-23 Thread Alex Goins
If a buffer is backed by dmabuf, wait on its reservation object's exclusive
fence before flipping.

v2: First commit
v3: Remove object_name_lock acquire
v4: Move wait ahead of mark_page_flip_active
Use crtc->primary->fb to get GEM object instead of pending_flip_obj
use_mmio_flip() return true when exclusive fence is attached
Wait only on exclusive fences, interruptible with no timeout
v5: Move wait from do_mmio_flip to mmio_flip_work_func
Style tweaks to more closely match rest of file
v6: Change back to unintteruptible wait to match __i915_wait_request due to
inability to properly handle interrupted wait.
Warn on error code from waiting.

Signed-off-by: Alex Goins 
---
 drivers/gpu/drm/i915/intel_display.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index b2270d5..bacf336 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -44,6 +44,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 

 /* Primary plane formats for gen <= 3 */
 static const uint32_t i8xx_primary_formats[] = {
@@ -11088,6 +11090,8 @@ static bool use_mmio_flip(struct intel_engine_cs *ring,
return true;
else if (i915.enable_execlists)
return true;
+   else if (obj->base.dma_buf && obj->base.dma_buf->resv->fence_excl)
+   return true;
else
return ring != i915_gem_request_get_ring(obj->last_write_req);
 }
@@ -11189,6 +11193,9 @@ static void intel_mmio_flip_work_func(struct 
work_struct *work)
 {
struct intel_mmio_flip *mmio_flip =
container_of(work, struct intel_mmio_flip, work);
+   struct intel_framebuffer *intel_fb =
+   to_intel_framebuffer(mmio_flip->crtc->base.primary->fb);
+   struct drm_i915_gem_object *obj = intel_fb->obj;

if (mmio_flip->req)
WARN_ON(__i915_wait_request(mmio_flip->req,
@@ -11196,6 +11203,12 @@ static void intel_mmio_flip_work_func(struct 
work_struct *work)
false, NULL,
_flip->i915->rps.mmioflips));

+   /* For framebuffer backed by dmabuf, wait for fence */
+   if (obj->base.dma_buf)
+   
WARN_ON(reservation_object_wait_timeout_rcu(obj->base.dma_buf->resv,
+   false, false,
+   
MAX_SCHEDULE_TIMEOUT) < 0);
+
intel_do_mmio_flip(mmio_flip->crtc);

i915_gem_request_unreference__unlocked(mmio_flip->req);
-- 
1.9.1



[PATCH i915 v6 0/2] PRIME Synchronization

2015-11-23 Thread Alex Goins
Hello all,

For a while now, I've been working to fix tearing with PRIME. This is the
sixth version of the DRM component for PRIME synchronization.

In this version I modified the wait in prepare_plane_fb to properly handle
interrupts by returning -ERESTARTSYS, and warn on other error codes before
continuing.

Due to the inability to properly handle interrupts in mmio_flip_work_func,
I changed it back to an uninterruptible wait, matching the semantics used
by __i915_wait_request(). Like prepare_plane_fb, I also changed it to warn
on error codes.

Repeat of overview below:

v1 was a more complicated patch set that added an additional fenced
interface to page flipping. To avoid adding additional interfaces on top of
a legacy path, v2 scrapped those patches and changed i915 page flipping
paths to wait on fences attached to DMA-BUF-backed fbs. Subsequent versions
involve incremental changes outlined in the patch descriptions.

I have two patches, one that implements fencing for i915's legacy mmio_flip
path, and one for atomic modesetting for futureproofing. Currently the
mmio_flip path is the one ultimately used by the X patches, due to the lack
of asynchronous atomic modesetting support in i915.

With my synchronization patches to X, it is possible to export two shared
buffers per crtc instead of just one. The sink driver uses the legacy
drmModePageFlip() to flip between the buffers, as the rest of the driver
has yet to be ported to atomics. In the pageflip/vblank event handler, the
sink driver requests a present from the source using the new X ABI function
pScreen->PresentTrackedFlippingPixmap(). If the call returns successfully,
it uses drmModePageFlip() to flip to the updated buffer, otherwise it waits
until the next vblank and tries again.

When the source driver presents on a given buffer, it first attaches a
fence.  The source driver is responsible for either using software
signaling or hardware semaphore-backed fences to ensure the fence is
signaled when the present is finished. If the sink's DRM driver implements
fencing in the flipping path, it will guarantee that that flip won't occur
until the present has finished.

This means that DRM drivers that don't implement fencing in their flipping
paths won't be able to guarantee 100% tear-free PRIME with my X patches.
However, the good news is that even without fencing, tearing is rare.
Generally presenting finishes before the next vblank, so there is no need
to wait on the fence. The X patches are a drastic improvement with or
without fencing, but the fencing is nonetheless important to guarantee
tear-free under all conditions.

To give some greater context, I've uploaded my branches for DRM and the X
server to Github. I'll move forward with upstreaming the X changes if and
when these DRM patches go in.

DRM Tree:https://github.com/GoinsWithTheWind/drm-prime-sync
X Tree:  https://github.com/GoinsWithTheWind/xserver-prime-sync

(branch agoins-prime-v6)

Thanks, Alex @ NVIDIA Linux Driver Team

Alex Goins (2):
  i915: wait for fence in mmio_flip_work_func
  i915: wait for fence in prepare_plane_fb

 drivers/gpu/drm/i915/intel_display.c | 24 
 1 file changed, 24 insertions(+)

-- 
1.9.1



Funky new vblank counter regressions in Linux 4.4-rc1

2015-11-23 Thread Harry Wentland
Hi Mario,

when we've had issues with this on amdgpu Christian fixed it by enabling 
page flip irq all the time, rather than turning it on when usermode 
request a flip and turning it back off after we handled it. I believe 
that fix exists on radeon already. Michel should have more info on that.

See other comments inline.

Thanks,
Harry


On 2015-11-23 11:02 AM, Mario Kleiner wrote:
> On 11/20/2015 04:42 AM, Alex Deucher wrote:
>> On Thu, Nov 19, 2015 at 12:46 PM, Mario Kleiner
>>  wrote:
>>> Hi Alex and Michel and Ville,
>>>
>>> it's "fix vblank stuff" time again ;-)
>>
>> Adding Harry from our display team.  He might be able to fill in the
>> blanks of on some of this better than I can.  It might also be worth
>> checking to see how our DAL (our new display code which is being
>> developed directly by our display team) code handles this.  It could
>> be that we are just missing register settings:
>
> Thanks Alex! And hello Harry :)
>
>> http://cgit.freedesktop.org/~agd5f/linux/log/?h=DAL-wip
>
> I'll have a look at this.
>
>> Additionally we've published full registers headers for the display 
>> block:
>> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/amd/include/asic_reg/dce
>>  
>>
>> The DCE8 stuff should generally apply back to DCE4.  If you have
>> questions about registers older asics not covered in the hw docs, let
>> me know.  Note the new headers are dword aligned rather than byte
>> aligned.
>>
>
> I've tested now with two different progressive modes on DCE3 and one 
> progressive mode on DCE4, the only cards i have atm. So far it seems 
> that the framecounter indeed increments when the vpos from the scanout 
> position query jumps back to zero. Attached for reference is my 
> current patch to radeon-kms. This one seems to work reliably so far, 
> also if i enable the immediate vblank irq disable which we so far only 
> used on intel-kms.
>
> But according to this patch the framecounter increment happens 
> somewhere in the middle of vblank.
>
> Essentially from the vpos extracted from 
> EVERGREEN_CRTC_STATUS_POSITION which defines start of vblank ("Start" 
> part of EVERGREEN_CRTC_V_BLANK_START_END) until maximum ie. VTOTAL-1 
> the framecounter stays on the count of the old previous scanout cycle. 
> Then when vpos wraps to zero the framecounter increments by 1. And 
> then we have another couple of dozen lines inside vblank until 
> reaching the "End" part of EVERGREEN_CRTC_V_BLANK_START_END and 
> entering active scanout for the new frame.
>
> So the position of observed framecounter increment seems to be not 
> close to the end of vblank ("start of frame" indicator?), but a couple 
> of scanlines after start of vblank.
>
> E.g., for a 2560x1440 video mode at 60 Hz, start of vblank is 1478, 
> vtotal is 1481, end of vblank is 38. So i enter the vblank and see the 
> old framecounter for vpos = 1478, 1479, 1480, then it wraps to 0 and 
> the framecounter increments by 1, then 38 scanlines later the vblank 
> ends.
>
> So i seem to have something that seems to work in practice and this 
> "increment framecounter if vpos wraps back to zero" behavior makes 
> some sense. It just doesn't conform to what those descriptions for 
> start_line and "start of frame" indicator describe?
>
This is correct. Our HW doesn't really have a vblank counter but a frame 
counter. The framecounter increments at the start of vsync, which is 
when we wrap to zero which doesn't coincide with the start of vblank.

What we're trying to do with get_vblank_counter isn't the same as what 
framecount gives us, but we could probably do something like:

if (get_scanout_pos > vblank_start)
   return frame_count + 1;
else
   return frame_count;

Harry

> I'll test with a few more video modes.
>
> thanks,
> -mario
>
>
>>>
>>> Ville's changes to the DRM's drm_handle_vblank() / 
>>> drm_update_vblank_count()
>>> code in Linux 4.4 not only made that code more elegant, but also 
>>> removed the
>>> robustness against the vblank irq quirks in AMD hw and similar 
>>> hardware. So
>>> now i get tons of off-by-one errors and
>>>
>>> "[   432.345] (WW) RADEON(1): radeon_dri2_flip_event_handler: Pageflip
>>> completion event has impossible msc 24803 < target_msc 24804" XOrg 
>>> messages
>>> from that kernel.
>>>
>>> One of the reasons for trouble is that AMD hw quirk where the hw 
>>> fires an
>>> extra vblank irq shortly after vblank irq's get enabled, not 
>>> synchronized to
>>> vblank, but typically in the middle of active scanout, so we get a 
>>> redundant
>>> call to drm_handle_vblank in the middle of scanout.
>>>
>>> To fix that i have a minor patch to make drm_update_vblank_count() 
>>> again
>>> robust against such redundant calls, which i will send out later to the
>>> mailing list. Diff attached for reference.
>>>
>>> The second quirk of AMD hw is that the vblank interrupt fires a few
>>> scanlines before start of vblank, so drm_handle_vblank ->
>>> drm_update_vblank_count() -> 

[Bug 108221] amdgpu: mouse cursor flickers + black boxes

2015-11-23 Thread bugzilla-dae...@bugzilla.kernel.org
https://bugzilla.kernel.org/show_bug.cgi?id=108221

Alex Deucher  changed:

   What|Removed |Added

 CC||alexdeucher at gmail.com

--- Comment #1 from Alex Deucher  ---
Can you bisect?

-- 
You are receiving this mail because:
You are watching the assignee of the bug.


[PATCH] drm/radeon: Retry DDC probing on DVI on failure if we got an HPD interrupt

2015-11-23 Thread Deucher, Alexander
> -Original Message-
> From: Lyude [mailto:cpaul at redhat.com]
> Sent: Sunday, November 22, 2015 9:45 PM
> To: Koenig, Christian; Daniel Stone
> Cc: Deucher, Alexander; David Airlie; dri-devel; Linux Kernel Mailing List;
> Jerome Glisse; Benjamin Tissoires
> Subject: Re: [PATCH] drm/radeon: Retry DDC probing on DVI on failure if we
> got an HPD interrupt
> 
> On Sat, 2015-11-21 at 16:22 +0100, Christian König wrote:
> > On 21.11.2015 15:49, Daniel Stone wrote:
> > > Hi,
> > >
> > > On 21 November 2015 at 14:22, Christian König  > > .com> wrote:
> > > > On 20.11.2015 16:52, cpaul at redhat.com wrote:
> > > > > This is somewhat rare on most cards (depending on what angle
> > > > > you plug
> > > > > the DVI connector in), but on some cards it happens constantly.
> > > > > The
> > > > > Radeon R5 on the machine used for testing this patch for
> > > > > instance, runs
> > > > > into this issue just about every time I try to hotplug a DVI
> > > > > monitor and
> > > > > as a result hotplugging almost never works.
> > > > >
> > > > > Rescheduling the hotplug work for a second when we run into an
> > > > > HPD
> > > > > signal with a failing DDC probe usually gives enough time for
> > > > > the rest
> > > > > of the connector's pins to make contact, and fixes this issue.
> > > > >
> > > > > Signed-off-by: Stephen Chandler Paul 
> > > >
> > > > Yeah, that's something I always wondered a about bit as well.
> > > >
> > > > Debouncing is something very common done in electronics, but as
> > > > far as I
> > > > know the HPD pins don't necessary have an RC circuit so we might
> > > > need to
> > > > handle this case in software here.
> > > >
> > > > A delay of something between 10-30ms between the last HPD
> > > > interrupt and
> > > > further processing of the signal doesn't sounds like such a bad
> > > > idea.
> Unfortunately the delay needed to make hotplugging work on the system
> mentioned in the commit log can actually be over 700ms.
> > > >
> > > > Retrying on the other hand doesn't necessarily improve the
> > > > situation cause
> > > > the delay introduced by this might not be enough.
> Yeah, but I would think it would make sense to retry here so long as we
> back off after a certain time. This would also have the benefit of
> skipping this delay on systems that don't need it.
> > > >
> > > > So I would rather vote for a fixed delay between an HPD interrupt
> > > > and
> > > > actually starting to process anything.
> > > Yes-ish. Debouncing is useful, and ignoring buggy devices (e.g.
> > > those
> > > on marginal power) which send you HPD storms as well. But DP relies
> > > on
> > > 'short HPD' pulses which can be as brief as 2ms. So attempting to
> > > totally debounce all HPD won't work.
> > Well the discussion so far was about HPD on DVI only.
> >
> > I'm not so deep into DP, but why should it uses HPD pulses of less
> > than 2ms?
> This is part of the DP spec iirc. This being said though, the issue
> here with the HPD signal coming before the connector is ready only
> happens on DVI. I haven't ever run into this issue with any HDMI cables
> or DP cables, so I'm against imposing this on all connectors.
> 
> One of the solutions I've been thinking about with this: In
> radeon_dvi_detect(), if we get a real hotplug signal retry the DDC
> probe until at least a second has passed, after which we back off and
> assume the port is disconnected.

FWIW, there are registers to adjust how long the hpd needs to be asserted 
before the hpd connection and short pulse interrupts are triggered.  See 
DC_HPDx_CONTROL.  Maybe adjusting them would help.  We currently just write the 
default value, but it might be better to RMW the value in case there is a 
special golden value set by the vbios at init time.

Alex

> >
> > Regards,
> > Christian.
> >
> > >
> > > Cheers,
> > > Daniel
> >
> --
> Cheers,
>   Lyude



[PATCH v2 00/13] drm/exynos: async G2D and g2d_move()

2015-11-23 Thread Inki Dae


2015년 11월 23일 11:35에 Hyungwon Hwang 이(가) 쓴 글:
> Hello Tobias,
> 
> I reviewed this whole patchset, and it looks good to me. Also I tested
> it on my odroid u3, and all works fine. Thanks you for your effort.
> 
> Tested-by: Hyungwon Hwang 
> Reviewed-by: Hyungwon Hwang 

Acked-by: Inki Dae 

Thanks,
Inki Dae

> 
> BRs,
> Hyungwon Hwang
> 
> On Sun, 22 Nov 2015 19:48:30 +0100
> Tobias Jakobi  wrote:
> 
>> Hello,
>>
>> this series mostly touches G2D code. It introduces the following:
>>
>> (1) drmHandleEvent2() is added to enable processing of vendor-specific
>> events. This will be used to expose asynchronous operation of the
>> G2D. The necessary kernel infrastructure is already there since
>> a lot of kernel versions. [This touches libdrm core code!]
>>
>> (2) The necessary infrastructure to handle G2D events. This includes
>> adding g2d_config_event() and g2d_exec2() to the public API.
>> A test application is provided to ensure that everything works
>> as expected.
>>
>> (3) A small performance test application which can be used to measure
>> the speed of solid color clear operations. Interesting for
>> benchmarking and plotting colorful graphs (e.g. through
>> Mathematica).
>>
>> (4) g2d_move() which works similar to g2d_copy() but like the C
>> memmove() properly handles overlapping buffer copies.
>> Again a test application is present to check that this
>> indeed does what it should.
>>
>> (5) Various small changes. A framebuffer colorformat fix for the
>> general G2D test application. Moving the currently unused
>> g2d_reset() to the public API. Adding a counterpart to
>> exynos_bo_map() to unmap buffers again.
>>
>> (6) Last but not least a small bump of the Exynos version number.
>>
>> Please review and let me know what I should change/improve.
>>
>>
>> With best wishes,
>> Tobias
>>
>> P.S.: Most patches were submitted already some time ago but never
>> made it upstream. So if something looks familiar, don't worry! ;)
>>
>> Changes since v1:
>> - Added wording changes suggested by Hyungwon Hwang.
>> - Added binaries for new test applications to .gitignore.
>> - Collected r-b and t-b tags.
>>
>> Tobias Jakobi (13):
>>   drm: Implement drmHandleEvent2()
>>   exynos: Introduce exynos_handle_event()
>>   tests/exynos: add fimg2d performance analysis
>>   exynos/fimg2d: add g2d_config_event
>>   exynos: fimg2d: add g2d_exec2
>>   tests/exynos: add fimg2d event test
>>   tests/exynos: use XRGB for framebuffer
>>   exynos: fimg2d: add g2d_set_direction
>>   exynos/fimg2d: add g2d_move
>>   tests/exynos: add test for g2d_move
>>   exynos/fimg2d: add exynos_bo_unmap()
>>   exynos/fimg2d: add g2d_reset() to public API
>>   exynos: bump version number
>>
>>  .gitignore |   2 +
>>  exynos/exynos-symbol-check |   5 +
>>  exynos/exynos_drm.c|  48 ++
>>  exynos/exynos_drm.h|  12 ++
>>  exynos/exynos_drmif.h  |  27 +++
>>  exynos/exynos_fimg2d.c | 165 +--
>>  exynos/exynos_fimg2d.h |  49 ++
>>  exynos/libdrm_exynos.pc.in |   2 +-
>>  tests/exynos/Makefile.am   |  26 ++-
>>  tests/exynos/exynos_fimg2d_event.c | 326
>> 
>> tests/exynos/exynos_fimg2d_perf.c  | 327
>> +
>> tests/exynos/exynos_fimg2d_test.c  | 134 ++-
>> xf86drm.h  |  21 +++
>> xf86drmMode.c  |  10 +- 14 files changed, 1138
>> insertions(+), 16 deletions(-) create mode 100644
>> tests/exynos/exynos_fimg2d_event.c create mode 100644
>> tests/exynos/exynos_fimg2d_perf.c
>>
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" 
> in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


[PATCH] drm/radeon: remove UMS support

2015-11-23 Thread Alex Deucher
It's been deprecated behind a kconfig option for almost
two years and hasn't really been supported for years before
that.  DDX support was dropped more than three years ago.

Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/radeon/Kconfig|9 -
 drivers/gpu/drm/radeon/Makefile   |4 -
 drivers/gpu/drm/radeon/drm_buffer.c   |  177 --
 drivers/gpu/drm/radeon/drm_buffer.h   |  148 --
 drivers/gpu/drm/radeon/r300_cmdbuf.c  | 1186 
 drivers/gpu/drm/radeon/r600_blit.c|  874 -
 drivers/gpu/drm/radeon/r600_cp.c  | 2660 ---
 drivers/gpu/drm/radeon/r600_cs.c  |   95 -
 drivers/gpu/drm/radeon/radeon_cp.c| 2243 ---
 drivers/gpu/drm/radeon/radeon_drv.c   |   97 -
 drivers/gpu/drm/radeon/radeon_drv.h   | 2048 -
 drivers/gpu/drm/radeon/radeon_irq.c   |  402 
 drivers/gpu/drm/radeon/radeon_mem.c   |  302 ---
 drivers/gpu/drm/radeon/radeon_state.c | 3261 -
 14 files changed, 13506 deletions(-)
 delete mode 100644 drivers/gpu/drm/radeon/drm_buffer.c
 delete mode 100644 drivers/gpu/drm/radeon/drm_buffer.h
 delete mode 100644 drivers/gpu/drm/radeon/r300_cmdbuf.c
 delete mode 100644 drivers/gpu/drm/radeon/r600_blit.c
 delete mode 100644 drivers/gpu/drm/radeon/r600_cp.c
 delete mode 100644 drivers/gpu/drm/radeon/radeon_cp.c
 delete mode 100644 drivers/gpu/drm/radeon/radeon_irq.c
 delete mode 100644 drivers/gpu/drm/radeon/radeon_mem.c
 delete mode 100644 drivers/gpu/drm/radeon/radeon_state.c

diff --git a/drivers/gpu/drm/radeon/Kconfig b/drivers/gpu/drm/radeon/Kconfig
index 421ae13..9909f5c 100644
--- a/drivers/gpu/drm/radeon/Kconfig
+++ b/drivers/gpu/drm/radeon/Kconfig
@@ -5,12 +5,3 @@ config DRM_RADEON_USERPTR
help
  This option selects CONFIG_MMU_NOTIFIER if it isn't already
  selected to enabled full userptr support.
-
-config DRM_RADEON_UMS
-   bool "Enable userspace modesetting on radeon (DEPRECATED)"
-   depends on DRM_RADEON
-   help
- Choose this option if you still need userspace modesetting.
-
- Userspace modesetting is deprecated for quite some time now, so
- enable this only if you have ancient versions of the DDX drivers.
diff --git a/drivers/gpu/drm/radeon/Makefile b/drivers/gpu/drm/radeon/Makefile
index dea53e3..08bd17d 100644
--- a/drivers/gpu/drm/radeon/Makefile
+++ b/drivers/gpu/drm/radeon/Makefile
@@ -58,10 +58,6 @@ $(obj)/evergreen_cs.o: $(obj)/evergreen_reg_safe.h 
$(obj)/cayman_reg_safe.h

 radeon-y := radeon_drv.o

-# add UMS driver
-radeon-$(CONFIG_DRM_RADEON_UMS)+= radeon_cp.o radeon_state.o radeon_mem.o \
-   radeon_irq.o r300_cmdbuf.o r600_cp.o r600_blit.o drm_buffer.o
-
 # add KMS driver
 radeon-y += radeon_device.o radeon_asic.o radeon_kms.o \
radeon_atombios.o radeon_agp.o atombios_crtc.o radeon_combios.o \
diff --git a/drivers/gpu/drm/radeon/drm_buffer.c 
b/drivers/gpu/drm/radeon/drm_buffer.c
deleted file mode 100644
index f4e0f3a..000
--- a/drivers/gpu/drm/radeon/drm_buffer.c
+++ /dev/null
@@ -1,177 +0,0 @@
-/**
- *
- * Copyright 2010 Pauli Nieminen.
- * All Rights Reserved.
- *
- * Permission is hereby granted, free of charge, to any person obtaining a
- * copy of this software and associated documentation files (the
- * "Software"), to deal in the Software without restriction, including
- * without limitation the rights to use, copy, modify, merge, publish,
- * distribute, sub license, and/or sell copies of the Software, and to
- * permit persons to whom the Software is furnished to do so, subject to
- * the following conditions:
- *
- * The above copyright notice and this permission notice (including the
- * next paragraph) shall be included in all copies or substantial portions
- * of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
- * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM,
- * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
- * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
- * USE OR OTHER DEALINGS IN THE SOFTWARE.
- *
- *
- **/
-/*
- * Multipart buffer for coping data which is larger than the page size.
- *
- * Authors:
- * Pauli Nieminen 
- */
-
-#include 
-#include "drm_buffer.h"
-
-/**
- * Allocate the drm buffer object.
- *
- *   buf: Pointer to a pointer where the object is stored.
- *   size: The number of bytes to allocate.
- */
-int drm_buffer_alloc(struct drm_buffer **buf, int size)
-{
-   int nr_pages = size / PAGE_SIZE + 1;
-   int idx;
-
-   /* Allocating pointer table to end of structure makes drm_buffer
-

[PATCH 5/5] drm/imx: ipuv3 plane: Replace dev_info with dev_dbg if a plane's CRTC changes

2015-11-23 Thread Philipp Zabel
Am Freitag, den 20.11.2015, 16:14 +0800 schrieb Liu Ying:
> This patch changes the dev_info() call to dev_dbg() in ipu_plane_update()
> to print out the information that a plane's CRTC is changed, because this
> kind of information is only useful for debugging.
> 
> Signed-off-by: Liu Ying 
> ---
> This patch applies to the imx-drm/fixes branch of Philipp Zabel's open git.
> 
>  drivers/gpu/drm/imx/ipuv3-plane.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c 
> b/drivers/gpu/drm/imx/ipuv3-plane.c
> index b3ed207..b24bf94 100644
> --- a/drivers/gpu/drm/imx/ipuv3-plane.c
> +++ b/drivers/gpu/drm/imx/ipuv3-plane.c
> @@ -338,7 +338,7 @@ static int ipu_update_plane(struct drm_plane *plane, 
> struct drm_crtc *crtc,
>   }
>  
>   if (crtc != plane->crtc)
> - dev_info(plane->dev->dev, "crtc change: %p -> %p\n",
> + dev_dbg(plane->dev->dev, "crtc change: %p -> %p\n",
>   plane->crtc, crtc);
>   plane->crtc = crtc;

This change is separate from the others, I have applied it.

thanks
Philipp



[PATCH] drm/radeon: Retry DDC probing on DVI on failure if we got an HPD interrupt

2015-11-23 Thread Lyude
On Mon, 2015-11-23 at 10:43 -0500, Lyude wrote:
> On Mon, 2015-11-23 at 14:20 +, Deucher, Alexander wrote:
> > > -Original Message-
> > > From: Lyude [mailto:cpaul at redhat.com]
> > > Sent: Sunday, November 22, 2015 9:45 PM
> > > To: Koenig, Christian; Daniel Stone
> > > Cc: Deucher, Alexander; David Airlie; dri-devel; Linux Kernel Mailing
> > > List;
> > > Jerome Glisse; Benjamin Tissoires
> > > Subject: Re: [PATCH] drm/radeon: Retry DDC probing on DVI on failure if we
> > > got an HPD interrupt
> > > 
> > > On Sat, 2015-11-21 at 16:22 +0100, Christian König wrote:
> > > > On 21.11.2015 15:49, Daniel Stone wrote:
> > > > > Hi,
> > > > > 
> > > > > On 21 November 2015 at 14:22, Christian König  > > > > amd
> > > > > .com> wrote:
> > > > > > On 20.11.2015 16:52, cpaul at redhat.com wrote:
> > > > > > > This is somewhat rare on most cards (depending on what angle
> > > > > > > you plug
> > > > > > > the DVI connector in), but on some cards it happens constantly.
> > > > > > > The
> > > > > > > Radeon R5 on the machine used for testing this patch for
> > > > > > > instance, runs
> > > > > > > into this issue just about every time I try to hotplug a DVI
> > > > > > > monitor and
> > > > > > > as a result hotplugging almost never works.
> > > > > > > 
> > > > > > > Rescheduling the hotplug work for a second when we run into an
> > > > > > > HPD
> > > > > > > signal with a failing DDC probe usually gives enough time for
> > > > > > > the rest
> > > > > > > of the connector's pins to make contact, and fixes this issue.
> > > > > > > 
> > > > > > > Signed-off-by: Stephen Chandler Paul 
> > > > > > 
> > > > > > Yeah, that's something I always wondered a about bit as well.
> > > > > > 
> > > > > > Debouncing is something very common done in electronics, but as
> > > > > > far as I
> > > > > > know the HPD pins don't necessary have an RC circuit so we might
> > > > > > need to
> > > > > > handle this case in software here.
> > > > > > 
> > > > > > A delay of something between 10-30ms between the last HPD
> > > > > > interrupt and
> > > > > > further processing of the signal doesn't sounds like such a bad
> > > > > > idea.
> > > Unfortunately the delay needed to make hotplugging work on the system
> > > mentioned in the commit log can actually be over 700ms.
> > > > > > 
> > > > > > Retrying on the other hand doesn't necessarily improve the
> > > > > > situation cause
> > > > > > the delay introduced by this might not be enough.
> > > Yeah, but I would think it would make sense to retry here so long as we
> > > back off after a certain time. This would also have the benefit of
> > > skipping this delay on systems that don't need it.
> > > > > > 
> > > > > > So I would rather vote for a fixed delay between an HPD interrupt
> > > > > > and
> > > > > > actually starting to process anything.
> > > > > Yes-ish. Debouncing is useful, and ignoring buggy devices (e.g.
> > > > > those
> > > > > on marginal power) which send you HPD storms as well. But DP relies
> > > > > on
> > > > > 'short HPD' pulses which can be as brief as 2ms. So attempting to
> > > > > totally debounce all HPD won't work.
> > > > Well the discussion so far was about HPD on DVI only.
> > > > 
> > > > I'm not so deep into DP, but why should it uses HPD pulses of less
> > > > than 2ms?
> > > This is part of the DP spec iirc. This being said though, the issue
> > > here with the HPD signal coming before the connector is ready only
> > > happens on DVI. I haven't ever run into this issue with any HDMI cables
> > > or DP cables, so I'm against imposing this on all connectors.
> > > 
> > > One of the solutions I've been thinking about with this: In
> > > radeon_dvi_detect(), if we get a real hotplug signal retry the DDC
> > > probe until at least a second has passed, after which we back off and
> > > assume the port is disconnected.
> > 
> > FWIW, there are registers to adjust how long the hpd needs to be asserted
> > before the hpd connection and short pulse interrupts are triggered.  See
> > DC_HPDx_CONTROL.  Maybe adjusting them would help.  We currently just 
> > write
> > the default value, but it might be better to RMW the value in case there is
> > a
> > special golden value set by the vbios at init time.
> > 
> This sounds like a much better idea. I'm going to try to come up with a patch
> that adjusts these by default for DVI ports.

Ignore this; we can't get a long enough delay using those registers to fix the
issue so we'll still have to handle this in software.
> > Alex
> > 
> > > > 
> > > > Regards,
> > > > Christian.
> > > > 
> > > > > 
> > > > > Cheers,
> > > > > Daniel
> > > > 
> > > --
> > > Cheers,
> > >   Lyude
> > 
-- 
Cheers,
Lyude



[PATCH 3/5] drm/imx: ipuv3 crtc: Cleanup ipu planes in ipu_drm_unbind()

2015-11-23 Thread Philipp Zabel
Am Freitag, den 20.11.2015, 16:14 +0800 schrieb Liu Ying:
> To avoid memory leakage, we need to cleanup ipu planes in ipu_drm_unbind().
> 
> Signed-off-by: Liu Ying 
> ---
> This patch applies to the imx-drm/fixes branch of Philipp Zabel's open git.
> 
>  drivers/gpu/drm/imx/ipuv3-crtc.c | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c 
> b/drivers/gpu/drm/imx/ipuv3-crtc.c
> index 59f44df..467905c 100644
> --- a/drivers/gpu/drm/imx/ipuv3-crtc.c
> +++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
> @@ -446,6 +446,11 @@ static void ipu_drm_unbind(struct device *dev, struct 
> device *master,
>   imx_drm_remove_crtc(ipu_crtc->imx_crtc);
>  
>   ipu_plane_put_resources(ipu_crtc->plane[0]);
> +
> + if (ipu_crtc->plane[1])
> + ipu_plane_cleanup(ipu_crtc->plane[1]);
> + ipu_plane_cleanup(ipu_crtc->plane[0]);
> +
>   ipu_put_resources(ipu_crtc);
>  }
>  

Shouldn't this already be handled by the DRM core calling the
plane->destroy callbacks from drm_mode_config_cleanup (called by
imx_drm_driver_unload shortly after component_unbind_all)?

regards
Philipp



[PATCH 1/5] drm/imx: ipuv3 plane: Introduce ipu_plane_cleanup()

2015-11-23 Thread Philipp Zabel
Am Freitag, den 20.11.2015, 16:14 +0800 schrieb Liu Ying:
> This patch adds a helper ipu_plane_cleanup() to cleanup a IPU plane.
> It can be used in the bailout path of ipu_crtc_init(), for instance.
> 
> Signed-off-by: Liu Ying 
> ---
> This patch applies to the imx-drm/fixes branch of Philipp Zabel's open git.
> 
>  drivers/gpu/drm/imx/ipuv3-plane.c | 6 ++
>  drivers/gpu/drm/imx/ipuv3-plane.h | 2 ++
>  2 files changed, 8 insertions(+)
> 
> diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c 
> b/drivers/gpu/drm/imx/ipuv3-plane.c
> index e2ff410..e60b382 100644
> --- a/drivers/gpu/drm/imx/ipuv3-plane.c
> +++ b/drivers/gpu/drm/imx/ipuv3-plane.c
> @@ -410,3 +410,9 @@ struct ipu_plane *ipu_plane_init(struct drm_device *dev, 
> struct ipu_soc *ipu,
>  
>   return ipu_plane;
>  }
> +
> +void ipu_plane_cleanup(struct ipu_plane *ipu_plane)
> +{
> + drm_plane_cleanup(_plane->base);
> + kfree(ipu_plane);
> +}

The name says cleanup, but that's not what it does. This function should
be named ipu_plane_free, or ipu_plane_destroy. Actually, we have that
already.

regards
Philipp



[PATCH 4/5] drm/imx: ipuv3 plane: Use the helper ipu_plane_cleanup() in ipu_plane_destroy()

2015-11-23 Thread Philipp Zabel
Am Freitag, den 20.11.2015, 16:14 +0800 schrieb Liu Ying:
> To reduce code duplication, we can use the helper ipu_plane_cleanup() in
> ipu_plane_destroy().
> 
> Signed-off-by: Liu Ying 
> ---
> This patch applies to the imx-drm/fixes branch of Philipp Zabel's open git.
> 
>  drivers/gpu/drm/imx/ipuv3-plane.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c 
> b/drivers/gpu/drm/imx/ipuv3-plane.c
> index e60b382..b3ed207 100644
> --- a/drivers/gpu/drm/imx/ipuv3-plane.c
> +++ b/drivers/gpu/drm/imx/ipuv3-plane.c
> @@ -369,8 +369,7 @@ static void ipu_plane_destroy(struct drm_plane *plane)
>   DRM_DEBUG_KMS("[%d] %s\n", __LINE__, __func__);
>  
>   ipu_disable_plane(plane);
> - drm_plane_cleanup(plane);
> - kfree(ipu_plane);
> + ipu_plane_cleanup(ipu_plane);
>  }
>  
>  static struct drm_plane_funcs ipu_plane_funcs = {

This could be merged into the first patch, but I don't think
ipu_plane_cleanup is necessary at all.

regards
Philipp



[PATCH 2/5] drm/imx: ipuv3 crtc: Cleanup ipu planes in ipu_crtc_init() when necessary

2015-11-23 Thread Philipp Zabel
Am Freitag, den 20.11.2015, 16:14 +0800 schrieb Liu Ying:
> To avoid memory leakage, we need to cleanup the initialized ipu planes in
> the bailout path of ipu_crtc_init().
> 
> Signed-off-by: Liu Ying 
> ---
> This patch applies to the imx-drm/fixes branch of Philipp Zabel's open git.
> 
>  drivers/gpu/drm/imx/ipuv3-crtc.c | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c 
> b/drivers/gpu/drm/imx/ipuv3-crtc.c
> index 67813ca..59f44df 100644
> --- a/drivers/gpu/drm/imx/ipuv3-crtc.c
> +++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
> @@ -371,7 +371,7 @@ static int ipu_crtc_init(struct ipu_crtc *ipu_crtc,
>   ipu_crtc->dev->of_node);
>   if (ret) {
>   dev_err(ipu_crtc->dev, "adding crtc failed with %d.\n", ret);
> - goto err_put_resources;
> + goto err_cleanup_plane0;
>   }
>  
>   ret = ipu_plane_get_resources(ipu_crtc->plane[0]);
> @@ -402,9 +402,14 @@ static int ipu_crtc_init(struct ipu_crtc *ipu_crtc,
>   return 0;
>  
>  err_put_plane_res:
> + if (ipu_crtc->plane[1])
> + ipu_plane_cleanup(ipu_crtc->plane[1]);
> +
>   ipu_plane_put_resources(ipu_crtc->plane[0]);
>  err_remove_crtc:
>   imx_drm_remove_crtc(ipu_crtc->imx_crtc);
> +err_cleanup_plane0:
> + ipu_plane_cleanup(ipu_crtc->plane[0]);
>  err_put_resources:
>   ipu_put_resources(ipu_crtc);

I think we can use ipu_plane_destroy as-is instead of
ipu_plane_put_resources + ipu_plane_cleanup.

regards
Philipp



[PATCH 3/3] drm/panel: Add Panasonic VVX10F034N00 MIPI DSI panel

2015-11-23 Thread Bjorn Andersson
On Mon, Nov 23, 2015 at 3:33 AM, Thierry Reding
 wrote:
> On Fri, Oct 30, 2015 at 05:38:28PM -0700, bjorn at kryo.se wrote:
>> From: Werner Johansson 
>>
>> This adds support for the Panasonic panel found in some Xperia Z2
>> tablets.
>>
>> Signed-off-by: Werner Johansson 
>> Signed-off-by: Bjorn Andersson 
>> ---
>>  drivers/gpu/drm/panel/Kconfig  |  10 +
>>  drivers/gpu/drm/panel/Makefile |   1 +
>>  .../gpu/drm/panel/panel-panasonic-vvx10f034n00.c   | 334 
>> +
>>  3 files changed, 345 insertions(+)
>>  create mode 100644 drivers/gpu/drm/panel/panel-panasonic-vvx10f034n00.c
>
> Applied, though I had to reorder Makefile and Kconfig entries to keep
> alphabetic ordering.
>

Thanks, now we're close to play Quake3 on the mainline kernel on the
Xperia Z2 Tablet :)

Did you pull patch 1/3 from this set as well? I didn't get a reply on
it and wanted to check as this patch (3/3) has a build dependency on
it.

Patch 1/3: https://patchwork.kernel.org/patch/7530121/

Regards,
Bjorn


[PATCH] drm/atomic_helper: Add drm_atomic_helper_disable_planes_on_crtc()

2015-11-23 Thread Jyri Sarha
Add drm_atomic_helper_disable_planes_on_crtc() for disabling all
planes associated with the given CRTC. This can be used for instance
in the CRTC helper disable callback to disable all planes before
shutting down the display pipeline.

Signed-off-by: Jyri Sarha 
---
This a follow version to "[PATCH RFC] drm/crtc_helper: Add
drm_crtc_helper_disable_planes()"-patch [1], with Daniel Vetter's
review comments [2] implemented. Hope I got everything right this
time. Thanks a lot for the prompt review.

Best regards,
Jyri

[1] http://www.spinics.net/lists/dri-devel/msg94720.html
[2] http://www.spinics.net/lists/dri-devel/msg94722.html

 drivers/gpu/drm/drm_atomic_helper.c | 52 +
 include/drm/drm_atomic_helper.h |  2 ++
 2 files changed, 54 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
b/drivers/gpu/drm/drm_atomic_helper.c
index a53ed7a..229c810 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1281,6 +1281,58 @@ void drm_atomic_helper_commit_planes(struct drm_device 
*dev,
 EXPORT_SYMBOL(drm_atomic_helper_commit_planes);

 /**
+ * drm_atomic_helper_disable_planes_on_crtc - helper to disable CRTC's planes
+ * @crtc: CRTC
+ * @atomic: if set, synchronize with CRTC's atomic_begin/flush hooks
+ *
+ * Disables all planes associated with the given CRTC. This can be
+ * used for instance in the CRTC helper disable callback to disable
+ * all planes before shutting down the display pipeline.
+ *
+ * If there are planes to disable and the atomic-parameter is set the
+ * function calls the CRTC's atomic_begin hook before and atomic_flush
+ * hook after disabling the planes.
+ *
+ * It is a bug to call this function without having implemented the
+ * ->atomic_disable() plane hook.
+ */
+void drm_atomic_helper_disable_planes_on_crtc(struct drm_crtc *crtc,
+ bool atomic)
+{
+   const struct drm_crtc_helper_funcs *crtc_funcs =
+   crtc->helper_private;
+   struct drm_plane *plane;
+   bool flush = false;
+
+   if (!crtc_funcs || !crtc_funcs->atomic_begin)
+   atomic = false;
+
+   drm_for_each_plane(plane, crtc->dev) {
+   const struct drm_plane_helper_funcs *plane_funcs =
+   plane->helper_private;
+
+   if (plane->state->crtc != crtc || !plane_funcs)
+   continue;
+
+   if (atomic && !flush) {
+   crtc_funcs->atomic_begin(crtc, NULL);
+   flush = true;
+   }
+
+   WARN_ON(!plane_funcs->atomic_disable);
+   if (plane_funcs->atomic_disable)
+   plane_funcs->atomic_disable(plane, NULL);
+   }
+
+   if (flush) {
+   WARN_ON(!crtc_funcs->atomic_flush);
+   if (crtc_funcs->atomic_flush)
+   crtc_funcs->atomic_flush(crtc, NULL);
+   }
+}
+EXPORT_SYMBOL(drm_atomic_helper_disable_planes_on_crtc);
+
+/**
  * drm_atomic_helper_commit_planes_on_crtc - commit plane state for a crtc
  * @old_crtc_state: atomic state object with the old crtc state
  *
diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
index 8cba54a..b7d4237 100644
--- a/include/drm/drm_atomic_helper.h
+++ b/include/drm/drm_atomic_helper.h
@@ -62,6 +62,8 @@ void drm_atomic_helper_commit_planes(struct drm_device *dev,
 void drm_atomic_helper_cleanup_planes(struct drm_device *dev,
  struct drm_atomic_state *old_state);
 void drm_atomic_helper_commit_planes_on_crtc(struct drm_crtc_state 
*old_crtc_state);
+void drm_atomic_helper_disable_planes_on_crtc(struct drm_crtc *crtc,
+ bool atomic);

 void drm_atomic_helper_swap_state(struct drm_device *dev,
  struct drm_atomic_state *state);
-- 
1.9.1



[PATCH v2 2/2] drm/panel: Add Sharp LS043T1LE01 MIPI DSI panel

2015-11-23 Thread Thierry Reding
On Fri, Oct 30, 2015 at 03:34:30PM -0700, Bjorn Andersson wrote:
> From: Werner Johansson 
> 
> This adds support for the Sharp panel found on the Qualcomm
> Snapdragon 800 Dragonboard (APQ8074)
> 
> Signed-off-by: Werner Johansson 
> Signed-off-by: Bjorn Andersson 
> ---
> 
> Change since v1:
> - Dropped -vid suffix from compatible

Applied, thanks.

Thierry
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20151123/9eb76c67/attachment.sig>


[PATCH v2 1/2] drm/panel: Add Sharp LS043T1LE01 panel binding documentation

2015-11-23 Thread Thierry Reding
On Fri, Oct 30, 2015 at 03:34:29PM -0700, Bjorn Andersson wrote:
> From: Werner Johansson 
> 
> Signed-off-by: Werner Johansson 
> Signed-off-by: Bjorn Andersson 
> ---
> 
> Change since v1:
> - Dropped -vid suffix from compatible
> 
>  .../bindings/display/panel/sharp,ls043t1le01.txt   | 22 
> ++
>  1 file changed, 22 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/display/panel/sharp,ls043t1le01.txt

Applied, thanks.

Thierry
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20151123/7d017ad9/attachment.sig>


[PATCH 3/3] drm/panel: Add Panasonic VVX10F034N00 MIPI DSI panel

2015-11-23 Thread Thierry Reding
On Fri, Oct 30, 2015 at 05:38:28PM -0700, bjorn at kryo.se wrote:
> From: Werner Johansson 
> 
> This adds support for the Panasonic panel found in some Xperia Z2
> tablets.
> 
> Signed-off-by: Werner Johansson 
> Signed-off-by: Bjorn Andersson 
> ---
>  drivers/gpu/drm/panel/Kconfig  |  10 +
>  drivers/gpu/drm/panel/Makefile |   1 +
>  .../gpu/drm/panel/panel-panasonic-vvx10f034n00.c   | 334 
> +
>  3 files changed, 345 insertions(+)
>  create mode 100644 drivers/gpu/drm/panel/panel-panasonic-vvx10f034n00.c

Applied, though I had to reorder Makefile and Kconfig entries to keep
alphabetic ordering.

Thanks,
Thierry
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20151123/3fe29412/attachment.sig>


[PATCH 2/3] panel/panasonic-vvx10f034n00: Add DT bindings

2015-11-23 Thread Thierry Reding
On Fri, Oct 30, 2015 at 05:38:27PM -0700, bjorn at kryo.se wrote:
> From: Werner Johansson 
> 
> This patch adds bindings for the Panasonic VVX10F034N00
> WUXGA panel.
> 
> Signed-off-by: Werner Johansson 
> Signed-off-by: Bjorn Andersson 
> ---
>  .../bindings/panel/panasonic,vvx10f034n00.txt| 20 
> 
>  1 file changed, 20 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/panel/panasonic,vvx10f034n00.txt

Applied, thanks.

Thierry
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20151123/2d2400f0/attachment.sig>


[PATCH] drm/panel: simple: Add Innolux N133HSE panel support

2015-11-23 Thread Thierry Reding
On Wed, Nov 18, 2015 at 10:47:43AM +0100, Marek Vasut wrote:
> From: Sean Cross 
> 
> The Innolux N133HSE panel is a 13.3" 1920x1080 panel that contains an
> integrated backlight, and connects via eDP.
> 
> It is used in the Kosagi Novena.
> 
> Signed-off-by: Sean Cross 
> Cc: Shawn Guo 
> Cc: Fabio Estevam 
> Cc: Thierry Reding 
> ---
>  drivers/gpu/drm/panel/panel-simple.c | 26 ++
>  1 file changed, 26 insertions(+)

This is missing a device tree binding document. Otherwise looks good,
except one minor nit below.

> diff --git a/drivers/gpu/drm/panel/panel-simple.c 
> b/drivers/gpu/drm/panel/panel-simple.c
> index f97b73e..d0fd427 100644
> --- a/drivers/gpu/drm/panel/panel-simple.c
> +++ b/drivers/gpu/drm/panel/panel-simple.c
> @@ -856,6 +856,29 @@ static const struct panel_desc innolux_n116bge = {
>   },
>  };
>  
> +static const struct drm_display_mode innolux_n133hse_ea1_mode = {
> + .clock = 138500,
> + .hdisplay = 1920,
> + .hsync_start = 1920 + 46,
> + .hsync_end = 1920 + 46 + 30,
> + .htotal = 1920 + 160,

Can you split out the .htotal into its various components, for
consistency with the entries for other panels?

> + .vdisplay = 1080,
> + .vsync_start = 1080 + 2,
> + .vsync_end = 1080 + 2 + 4,
> + .vtotal = 1080 + 32,

Same for .vtotal here.

Thierry
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20151123/4592bcf8/attachment.sig>


[PATCH] drm/panel: simple: Add support for G121X1-L03

2015-11-23 Thread Thierry Reding
On Wed, Nov 18, 2015 at 03:57:47PM -0500, Akshay Bhat wrote:
> Add support for Innolux CheMei 12" G121X1-L03 XGA LVDS display.
> 
> Datasheet: http://www.azdisplays.com/PDF/G121X1-L03.pdf
> Signed-off-by: Akshay Bhat 
> ---
>  .../bindings/display/panel/innolux,g121x1-l03.txt  |  7 +
>  drivers/gpu/drm/panel/panel-simple.c   | 31 
> ++
>  2 files changed, 38 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/display/panel/innolux,g121x1-l03.txt

Applied, thanks.

Thierry
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20151123/dd98523b/attachment-0001.sig>


[Intel-gfx] [PATCH] v2: Also add a DRIVER_* check like for all other maps functions to really short-circuit the code. And give drm_legacy_rmmap used by the dev unregister code the same treatment.

2015-11-23 Thread Jani Nikula

Accidental patch?

On Mon, 23 Nov 2015, Daniel Vetter  wrote:
> ---
>  drivers/gpu/drm/drm_bufs.c | 14 ++
>  include/drm/drm_legacy.h   |  2 +-
>  2 files changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_bufs.c b/drivers/gpu/drm/drm_bufs.c
> index de18b8b78a26..5a51633da033 100644
> --- a/drivers/gpu/drm/drm_bufs.c
> +++ b/drivers/gpu/drm/drm_bufs.c
> @@ -542,15 +542,17 @@ int drm_legacy_rmmap_locked(struct drm_device *dev, 
> struct drm_local_map *map)
>  }
>  EXPORT_SYMBOL(drm_legacy_rmmap_locked);
>  
> -int drm_legacy_rmmap(struct drm_device *dev, struct drm_local_map *map)
> +void drm_legacy_rmmap(struct drm_device *dev, struct drm_local_map *map)
>  {
> - int ret;
> + if (!drm_core_check_feature(dev, DRIVER_KMS_LEGACY_CONTEXT) &&
> + drm_core_check_feature(dev, DRIVER_MODESET))
> + return;
>  
>   mutex_lock(>struct_mutex);
> - ret = drm_legacy_rmmap_locked(dev, map);
> + drm_legacy_rmmap_locked(dev, map);
>   mutex_unlock(>struct_mutex);
>  
> - return ret;
> + return;
>  }
>  EXPORT_SYMBOL(drm_legacy_rmmap);
>  
> @@ -558,6 +560,10 @@ void drm_legacy_master_rmmaps(struct drm_device *dev, 
> struct drm_master *master)
>  {
>   struct drm_map_list *r_list, *list_temp;
>  
> + if (!drm_core_check_feature(dev, DRIVER_KMS_LEGACY_CONTEXT) &&
> + drm_core_check_feature(dev, DRIVER_MODESET))
> + return;
> +
>   mutex_lock(>struct_mutex);
>   list_for_each_entry_safe(r_list, list_temp, >maplist, head) {
>   if (r_list->master == master) {
> diff --git a/include/drm/drm_legacy.h b/include/drm/drm_legacy.h
> index 24504b0eafea..a5ef2c7e40f8 100644
> --- a/include/drm/drm_legacy.h
> +++ b/include/drm/drm_legacy.h
> @@ -154,7 +154,7 @@ struct drm_map_list {
>  int drm_legacy_addmap(struct drm_device *d, resource_size_t offset,
> unsigned int size, enum drm_map_type type,
> enum drm_map_flags flags, struct drm_local_map **map_p);
> -int drm_legacy_rmmap(struct drm_device *d, struct drm_local_map *map);
> +void drm_legacy_rmmap(struct drm_device *d, struct drm_local_map *map);
>  int drm_legacy_rmmap_locked(struct drm_device *d, struct drm_local_map *map);
>  void drm_legacy_master_rmmaps(struct drm_device *dev,
> struct drm_master *master);

-- 
Jani Nikula, Intel Open Source Technology Center


[PATCH 7/8] drm/i915: Fix random aux transactions failures.

2015-11-23 Thread Jani Nikula

I don't see how the subject matches the commit.

On Sat, 21 Nov 2015, Rodrigo Vivi  wrote:
> This read wake with retries were initially added by 2 commits:
>
> commit 61da5fab ("drm/i915/dp: retry link status read 3 times on failure")
> commit 899526d9 ("drm/i915/dp: try to read receiver capabilities 3 times when 
> detecting")
>
> Both mentioning section 9.1 of the 1.1a DisplayPort spec, that actually
> tell us to retry three times on certain case when "writing 01h to DPCD 
> address 600h"
> and this code is already in place in our driver. Added by:
>
> commit c7ad3810 ("drm/i915/dp: manage sink power state if possible")

I still think what we currently do for the sink power state management
works by coincidence. We should still look into it.

However, I think this series overall (apart from patch 6/8 which really
is a bummer, the comment inline below, and the minor other comments)
looks like worthwhile changes. We can leave the power state management
for later. Or rip it out for now...

> At this point we have no visibility if those patches were added to workaround 
> certain
> corner cases like lazy dongles or what, but also at that time there wasn't 
> enough
> retries on the proper places.
>
> So my proposal is to remove these retries for now that we have drm handling 
> the retries
> and if we face any corner case back again we study it to return EAGAIN or 
> EBUSY
> to force retries at drm instead of handling them here.
>
> v2: Improve commit message trying to explain the origin of the retries.
>
> Cc: Daniel Vetter 
> Cc: Jani Nikula 
> Cc: Jesse Barnes 
> Signed-off-by: Rodrigo Vivi 
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 95 
> ++---
>  1 file changed, 32 insertions(+), 63 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index c87e937..2ce6527 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -985,7 +985,8 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct 
> drm_dp_aux_msg *msg)
>   if (WARN_ON(rxsize > 20))
>   return -E2BIG;
>  
> - ret = intel_dp_aux_ch(intel_dp, txbuf, txsize, rxbuf, rxsize);
> + ret = intel_dp_aux_ch(intel_dp, txbuf, txsize,
> +   rxbuf, rxsize);
>   if (ret > 0) {
>   msg->reply = rxbuf[0] >> 4;
>   /*
> @@ -3150,47 +3151,16 @@ static void chv_dp_post_pll_disable(struct 
> intel_encoder *encoder)
>  }
>  
>  /*
> - * Native read with retry for link status and receiver capability reads for
> - * cases where the sink may still be asleep.
> - *
> - * Sinks are *supposed* to come up within 1ms from an off state, but we're 
> also
> - * supposed to retry 3 times per the spec.
> - */
> -static ssize_t
> -intel_dp_dpcd_read_wake(struct drm_dp_aux *aux, unsigned int offset,
> - void *buffer, size_t size)
> -{
> - ssize_t ret;
> - int i;
> -
> - /*
> -  * Sometime we just get the same incorrect byte repeated
> -  * over the entire buffer. Doing just one throw away read
> -  * initially seems to "solve" it.
> -  */
> - drm_dp_dpcd_read(aux, DP_DPCD_REV, buffer, 1);

This still needs to be addressed somehow. Maybe it's sufficient for
Ville to test with his monitor?

commit f6a1906674005377b64ee5431c1418077c1b2425
Author: Ville Syrjälä 
Date:   Thu Oct 16 20:46:09 2014 +0300

drm/i915: Do a dummy DPCD read before the actual read

> -
> - for (i = 0; i < 3; i++) {
> - ret = drm_dp_dpcd_read(aux, offset, buffer, size);
> - if (ret == size)
> - return ret;
> - msleep(1);
> - }
> -
> - return ret;
> -}
> -
> -/*
>   * Fetch AUX CH registers 0x202 - 0x207 which contain
>   * link status information
>   */
>  bool
>  intel_dp_get_link_status(struct intel_dp *intel_dp, uint8_t 
> link_status[DP_LINK_STATUS_SIZE])
>  {
> - return intel_dp_dpcd_read_wake(_dp->aux,
> -DP_LANE0_1_STATUS,
> -link_status,
> -DP_LINK_STATUS_SIZE) == 
> DP_LINK_STATUS_SIZE;
> + return drm_dp_dpcd_read(_dp->aux,
> + DP_LANE0_1_STATUS,
> + link_status,
> + DP_LINK_STATUS_SIZE) == DP_LINK_STATUS_SIZE;
>  }
>  
>  /* These are source-specific values. */
> @@ -3825,8 +3795,8 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
>   struct drm_i915_private *dev_priv = dev->dev_private;
>   uint8_t rev;
>  
> - if (intel_dp_dpcd_read_wake(_dp->aux, 0x000, intel_dp->dpcd,
> - sizeof(intel_dp->dpcd)) < 0)
> + if (drm_dp_dpcd_read(_dp->aux, 0x000, intel_dp->dpcd,
> +  sizeof(intel_dp->dpcd)) < 0)
>   return false; /* aux transfer failed */
>  
>   

[PATCH 7/8] drm/i915: Fix random aux transactions failures.

2015-11-23 Thread Jani Nikula
On Sat, 21 Nov 2015, Rodrigo Vivi  wrote:
> Mainly aux communications on sink_crc
> were failing a lot randomly on recent platforms.
> The first solution was to try to use intel_dp_dpcd_read_wake, but then
> it was suggested to move retries to drm level.
>
> Since drm level was already taking care of retries and didn't want
> to through random retries on that level the second solution was to
> put the retries at aux_transfer layer what was nacked.
>
> So I realized we had so many retries in different places and
> started to organize that a bit. During this organization I noticed
> that we weren't handing at all the case were the message size was
> zeroed. And this was exactly the case that was affecting sink_crc.
>
> Also we weren't respect BSPec who says this size message = 0 or > 20
> are forbidden.
>
> It is a fact that we still have no clue why we are getting this
> forbidden value there. But anyway we need to handle that for now
> so we return -EBUSY and drm level takes care of the retries that
> are already in place.
>
> Cc: Jani Nikula 
> Cc: Daniel Vetter 
> Signed-off-by: Rodrigo Vivi 
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 11 +++
>  1 file changed, 11 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 35048d6..c87e937 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -905,6 +905,17 @@ done:
>   /* Unload any bytes sent back from the other side */
>   recv_bytes = ((status & DP_AUX_CH_CTL_MESSAGE_SIZE_MASK) >>
> DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT);
> +
> + /*
> +  * By BSpec: "Message sizes of 0 or >20 are not allowed."
> +  * We have no idea of what happened so we return -EBUSY so
> +  * drm layer takes care for the necessary retries.
> +  */
> + if (recv_bytes == 0 || recv_bytes > 20) {
> + ret = -EBUSY;

This deserves debug logging at the very least.

BR,
Jani.


> + goto out;
> + }
> +
>   if (recv_bytes > recv_size)
>   recv_bytes = recv_size;

-- 
Jani Nikula, Intel Open Source Technology Center


[PATCH 6/8] drm/i915: Remove remaining retries from intel_dp_aux_ch.

2015-11-23 Thread Jani Nikula
On Sat, 21 Nov 2015, Rodrigo Vivi  wrote:
> drm level already takes care of the needed retries so instead of
> duplicate the effort here.
>
> If the retry is possible immediately we return EAGAIN. Otherwise,
> if we have no idea what caused the error let's assume something
> was busy and let drm level handle the wait and retries.
>
> Signed-off-by: Rodrigo Vivi 
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 64 
> ++---
>  1 file changed, 35 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index a8ba243..35048d6 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -795,7 +795,7 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
>   uint32_t aux_clock_divider;
>   int i, ret, recv_bytes;
>   uint32_t status;
> - int try, clock = 0;
> + int clock = 0;
>   bool has_aux_irq = HAS_AUX_IRQ(dev);
>   bool vdd;
>  
> @@ -835,41 +835,47 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
> send_bytes,
> aux_clock_divider);
>  
> - /* Must try at least 3 times according to DP spec */
> - for (try = 0; try < 5; try++) {

This now inverses the retries wrt the aux clock divider retries, which
probably breaks the non-ULT HSW workaround:

"WA: For the PCH Aux channels (Aux B/C/D) use an aux divider value of 63
decimal (03Fh). If there is a failure, retry at least three times with
63, then retry at least three times with 72 decimal (048h). See South
Display Engine Registers, DP_AUX_CTL."

It is sad.

BR,
Jani.


> - /* Load the send data into the aux channel data 
> registers */
> - for (i = 0; i < send_bytes; i += 4)
> - I915_WRITE(intel_dp->aux_ch_data_reg[i >> 2],
> -intel_dp_pack_aux(send + i,
> -  send_bytes - i));
> + /* Load the send data into the aux channel data registers */
> + for (i = 0; i < send_bytes; i += 4)
> + I915_WRITE(intel_dp->aux_ch_data_reg[i >> 2],
> +intel_dp_pack_aux(send + i,
> +  send_bytes - i));
>  
> - /* Send the command and wait for it to complete */
> - I915_WRITE(ch_ctl, send_ctl);
> + /* Send the command and wait for it to complete */
> + I915_WRITE(ch_ctl, send_ctl);
>  
> - status = intel_dp_aux_wait_done(intel_dp, has_aux_irq);
> + status = intel_dp_aux_wait_done(intel_dp, has_aux_irq);
>  
> - /* Clear done status and any errors */
> - I915_WRITE(ch_ctl,
> -status |
> -DP_AUX_CH_CTL_DONE |
> -DP_AUX_CH_CTL_TIME_OUT_ERROR |
> -DP_AUX_CH_CTL_RECEIVE_ERROR);
> + /* Clear done status and any errors */
> + I915_WRITE(ch_ctl,
> +status |
> +DP_AUX_CH_CTL_DONE |
> +DP_AUX_CH_CTL_TIME_OUT_ERROR |
> +DP_AUX_CH_CTL_RECEIVE_ERROR);
>  
> - if (status & DP_AUX_CH_CTL_TIME_OUT_ERROR)
> - continue;
> + if (status & DP_AUX_CH_CTL_TIME_OUT_ERROR) {
> + /*
> +  * DP CTS 1.2 Core Rev 1.1, 4.2.1.1 & 4.2.1.2
> +  * 400us delay required for errors and timeouts
> +  * Timeout errors from the HW already meet this
> +  * requirement so skip to next iteration
> +  */
> + ret = -EAGAIN;
> + goto out;
> + }
>  
> - /* DP CTS 1.2 Core Rev 1.1, 4.2.1.1 & 4.2.1.2
> -  *   400us delay required for errors and timeouts
> -  *   Timeout errors from the HW already meet this
> -  *   requirement so skip to next iteration
> + if (status & DP_AUX_CH_CTL_RECEIVE_ERROR) {
> + /*
> +  * We don't know what caused the error, so let's
> +  * return -EBUSY so drm level takes care of
> +  * the necessary wait for recover and retries
>*/
> - if (status & DP_AUX_CH_CTL_RECEIVE_ERROR) {
> - usleep_range(400, 500);
> - continue;
> - }
> - if (status & DP_AUX_CH_CTL_DONE)
> - goto done;
> + ret = -EBUSY;
> 

[Bug 80419] XCOM: Enemy Unknown Causes lockup

2015-11-23 Thread bugzilla-dae...@freedesktop.org
https://bugs.freedesktop.org/show_bug.cgi?id=80419

--- Comment #62 from Kamil Páral  ---
Paulo, what's "the SO sync"?

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20151123/e864bcd2/attachment.html>


[Intel-gfx] [PATCH 4/8] drm: Wait 1ms before retrying aux transactions on EBUSY.

2015-11-23 Thread Jani Nikula
On Sat, 21 Nov 2015, Rodrigo Vivi  wrote:
> DP Specs isn't really clear about the amount of retries,
> but for cases it mentions retries it also mention times that
> vary from 300us to 1ms.
>
> For many cases hardware can handled the timeouts before retry
> is possible and allowed, but for many other cases it is better
> to wait and give time so the aux channels can recover.
>
> For instance one general case there is when downstream device
> is waking up from sleep states generating HPD so it might take
> up to 1ms before getting responsive.
>
> I believe with this msleep we could minimize the 32 times retries
> and still let Dell monitors happy, but I don't have this monitor
> to test here so let's just add the sleep for now and still retry
> 32 times.
>
> Cc: Dave Airlie 
> Signed-off-by: Rodrigo Vivi 
> ---
>  drivers/gpu/drm/drm_dp_helper.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index ee7c955..1d6016d 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -202,9 +202,11 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 
> request,
>   if (err == -EAGAIN)
>   continue;
>  
> - /* FIXME: On BUSY we could wait before retrying */
> - if (err == -EBUSY)
> + /* Give a time for aux channels to recover */
> + if (err == -EBUSY) {
> + msleep(1);

usleep_range please; msleep(1) may take *much* longer than 1 ms, and
that could throw us off with our retry logic.

BR,
Jani.

>   continue;
> + }
>  
>   return err;
>   }

-- 
Jani Nikula, Intel Open Source Technology Center


[PATCH v2 10/10] dt-bindings: Add DSIv2 documentation

2015-11-23 Thread Archit Taneja


On 11/21/2015 1:29 AM, Rob Herring wrote:
> +Stephen
>
> On Wed, Nov 18, 2015 at 9:24 AM, Archit Taneja  
> wrote:
>> Hi Rob,
>>
>> On 11/18/2015 6:48 PM, Rob Herring wrote:
>>>
>>> +dt list
>>>
>>> On Wed, Nov 18, 2015 at 4:55 AM, Archit Taneja 
>>> wrote:

 Add additional property info needed for DSIv2 DT.
>>>
>>>
>>> Please use get_maintainers.pl.
>>
>>
>> Sorry about that, missed out doing that posting this time.
>>
>>>
 Signed-off-by: Archit Taneja 
 ---
Documentation/devicetree/bindings/display/msm/dsi.txt | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)

 diff --git a/Documentation/devicetree/bindings/display/msm/dsi.txt
 b/Documentation/devicetree/bindings/display/msm/dsi.txt
 index f344b9e..ca65a34 100644
 --- a/Documentation/devicetree/bindings/display/msm/dsi.txt
 +++ b/Documentation/devicetree/bindings/display/msm/dsi.txt
 @@ -13,18 +13,25 @@ Required properties:
- power-domains: Should be < MDSS_GDSC>.
- clocks: device clocks
  See Documentation/devicetree/bindings/clocks/clock-bindings.txt for
 details.
 -- clock-names: the following clocks are required:
 +- clock-names: these vary based on the DSI version. For DSI6G:
  * "bus_clk"
  * "byte_clk"
 +  * "byte_clk_src
>>>
>>>
>>> This sounds like the parent of byte_clk. Is that really a clock within
>>> the block?
>>
>>
>> byte_clk_src isn't in the block, but byte_clk_src's parent is one of
>> the PLLs in this block. We take this clock so that we can re-parent
>> it to an appropriate PLL. The decision of what PLL to choose needs to
>> be done by the DSI block's driver.
>
> Seems like abuse to me. The list of clocks should match what are
> inputs to the block, not what the driver happens to need. Without a
> full understanding of the clock tree here, I don't have a suggestion.
> Maybe Stephen does.

We don't need specify byte_clk_src (and other xyz_clk_src clocks) via
DT. There is a static link set up between byte_clk and byte_clk_src by
our clock driver that never changes. We can retrieve it in the driver
itself using clk_get_parent(byte_clk). This way we stick to only
input clocks.

Stephen, does that sound okay?

>
  * "core_clk"
  * "core_mmss_clk"
  * "iface_clk"
  * "mdp_core_clk"
  * "pixel_clk"
 +  * "pixel_clk_src"
 +  For DSIv2, we need a few more:
>>>
>>>
>>> What is the overall order of clocks? As listed?
>>
>>
>> Order in which the driver does clk_get? It uses the clock
>> name to get each one individually, so the order doesn't matter
>> as such.
>
> The order in DT. You may use the names, but the order should still be 
> specified.

Okay. I'll cross check and make sure the order is as in our DT files.

Archit

>
> Rob
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project


[PATCH 3/8] drm/i915: Use EAGAIN instead EBUSY for aux retry.

2015-11-23 Thread Jani Nikula
On Mon, 23 Nov 2015, Daniel Vetter  wrote:
> On Fri, Nov 20, 2015 at 04:46:25PM -0800, Rodrigo Vivi wrote:
>> Current EBUSY meaning is immediately retry, but this is
>> about to change. DRM aux transfer is about to change and
>> make EAGAIN the immediately retry and use EBUSY to wait
>> a bit for aux channels to recover for any error or wake up.
>> 
>> This has no functional change if the EAGAIN support is in
>> place already for drm aux transfer.
>> 
>> Signed-off-by: Rodrigo Vivi 
>
> Please document the EAGAIN and EBUSY return codes for the ->transfer
> member fo drm_dp_aux in the kerneldoc properly. While at it would be great
> if you could document any other error codes that are treated specially for
> this function.

See my comment in reply to patch 1 about the difference in handling of
the ->transfer return values for native aux and i2c-over-aux.

BR,
Jani.

>
> Since this will be a bit more text please convert the kerneldoc to the new
> per-member comment layout that 4.4 supports, i.e.
>
> /**
>  * struct foo - foo structure
>  *
>  * top-level blabla
>  */
> struct foo {
>   /**
>* @bar:
>*
>* Long text (with paragraphs) explaining bar.
>*/
> };
>
> Otherwise this looks really tidy and I really like how this allows us to
> get rid of the intel_read_wake hack.
> -Daniel
>
>> ---
>>  drivers/gpu/drm/i915/intel_dp.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c 
>> b/drivers/gpu/drm/i915/intel_dp.c
>> index bec443a..ed07f0a 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -835,7 +835,7 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
>>  last_status = status;
>>  }
>>  
>> -ret = -EBUSY;
>> +ret = -EAGAIN;
>>  goto out;
>>  }
>>  
>> @@ -890,7 +890,7 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
>>  
>>  if ((status & DP_AUX_CH_CTL_DONE) == 0) {
>>  DRM_ERROR("dp_aux_ch not done status 0x%08x\n", status);
>> -ret = -EBUSY;
>> +ret = -EAGAIN;
>>  goto out;
>>  }
>>  
>> -- 
>> 2.4.3
>> 
>> ___
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Jani Nikula, Intel Open Source Technology Center


[Intel-gfx] [PATCH 1/8] drm: Introduce EAGAIN handling for immediatelly aux retries

2015-11-23 Thread Jani Nikula
On Sat, 21 Nov 2015, Rodrigo Vivi  wrote:
> The goal here is to offload aux retries handling from the
> drivers to drm.
>
> However there are 2 differents cases for retry:
> 1. Immediately retry - Hardware already took care of needed timeouts
>  before answering that a retry is possible.
> 2. Busy - Wait some time and retry.
>
> This driver introduce the support for EAGAIN that can handle the
> first case and a following patch will introduce EBUSY waits,
> after all drivers counting on immediately retries are changed.
>
> Cc: Dave Airlie 
> Signed-off-by: Rodrigo Vivi 
> ---
>  drivers/gpu/drm/drm_dp_helper.c | 5 +
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index 9535c5b..ee7c955 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -198,6 +198,11 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 
> request,
>   err = aux->transfer(aux, );
>   mutex_unlock(>hw_mutex);
>   if (err < 0) {
> + /* Immediately retry */
> + if (err == -EAGAIN)
> + continue;

How about the ->transfer call in drm_dp_i2c_do_msg? If we leave that for
the i2c layer (which does do retries on -EAGAIN, but also handles
timeouts for repeated -EAGAINs) it deserves a comment in in
drm_dp_i2c_do_msg.

BR,
Jani.

> +
> + /* FIXME: On BUSY we could wait before retrying */
>   if (err == -EBUSY)
>   continue;

-- 
Jani Nikula, Intel Open Source Technology Center


[PATCH v2 00/13] drm/exynos: async G2D and g2d_move()

2015-11-23 Thread Hyungwon Hwang
Hello Tobias,

I reviewed this whole patchset, and it looks good to me. Also I tested
it on my odroid u3, and all works fine. Thanks you for your effort.

Tested-by: Hyungwon Hwang 
Reviewed-by: Hyungwon Hwang 

BRs,
Hyungwon Hwang

On Sun, 22 Nov 2015 19:48:30 +0100
Tobias Jakobi  wrote:

> Hello,
> 
> this series mostly touches G2D code. It introduces the following:
> 
> (1) drmHandleEvent2() is added to enable processing of vendor-specific
> events. This will be used to expose asynchronous operation of the
> G2D. The necessary kernel infrastructure is already there since
> a lot of kernel versions. [This touches libdrm core code!]
> 
> (2) The necessary infrastructure to handle G2D events. This includes
> adding g2d_config_event() and g2d_exec2() to the public API.
> A test application is provided to ensure that everything works
> as expected.
> 
> (3) A small performance test application which can be used to measure
> the speed of solid color clear operations. Interesting for
> benchmarking and plotting colorful graphs (e.g. through
> Mathematica).
> 
> (4) g2d_move() which works similar to g2d_copy() but like the C
> memmove() properly handles overlapping buffer copies.
> Again a test application is present to check that this
> indeed does what it should.
> 
> (5) Various small changes. A framebuffer colorformat fix for the
> general G2D test application. Moving the currently unused
> g2d_reset() to the public API. Adding a counterpart to
> exynos_bo_map() to unmap buffers again.
> 
> (6) Last but not least a small bump of the Exynos version number.
> 
> Please review and let me know what I should change/improve.
> 
> 
> With best wishes,
> Tobias
> 
> P.S.: Most patches were submitted already some time ago but never
> made it upstream. So if something looks familiar, don't worry! ;)
> 
> Changes since v1:
> - Added wording changes suggested by Hyungwon Hwang.
> - Added binaries for new test applications to .gitignore.
> - Collected r-b and t-b tags.
> 
> Tobias Jakobi (13):
>   drm: Implement drmHandleEvent2()
>   exynos: Introduce exynos_handle_event()
>   tests/exynos: add fimg2d performance analysis
>   exynos/fimg2d: add g2d_config_event
>   exynos: fimg2d: add g2d_exec2
>   tests/exynos: add fimg2d event test
>   tests/exynos: use XRGB for framebuffer
>   exynos: fimg2d: add g2d_set_direction
>   exynos/fimg2d: add g2d_move
>   tests/exynos: add test for g2d_move
>   exynos/fimg2d: add exynos_bo_unmap()
>   exynos/fimg2d: add g2d_reset() to public API
>   exynos: bump version number
> 
>  .gitignore |   2 +
>  exynos/exynos-symbol-check |   5 +
>  exynos/exynos_drm.c|  48 ++
>  exynos/exynos_drm.h|  12 ++
>  exynos/exynos_drmif.h  |  27 +++
>  exynos/exynos_fimg2d.c | 165 +--
>  exynos/exynos_fimg2d.h |  49 ++
>  exynos/libdrm_exynos.pc.in |   2 +-
>  tests/exynos/Makefile.am   |  26 ++-
>  tests/exynos/exynos_fimg2d_event.c | 326
> 
> tests/exynos/exynos_fimg2d_perf.c  | 327
> +
> tests/exynos/exynos_fimg2d_test.c  | 134 ++-
> xf86drm.h  |  21 +++
> xf86drmMode.c  |  10 +- 14 files changed, 1138
> insertions(+), 16 deletions(-) create mode 100644
> tests/exynos/exynos_fimg2d_event.c create mode 100644
> tests/exynos/exynos_fimg2d_perf.c
> 



[PATCH 1/1] apple-gmux: Assign apple_gmux_data before registering

2015-11-23 Thread Darren Hart
On Mon, Nov 16, 2015 at 09:38:40PM +0100, Lukas Wunner wrote:
> From: Matthew Garrett 
> 
> Registering the handler after both GPUs will trigger a DDC switch for
> connector reprobing. This will oops if apple_gmux_data hasn't already
> been assigned. Reorder the code to do that.
> 
> [Lukas: More generally, this commit fixes a race condition that
> is triggered by invoking a handler callback between the call to
> vga_switcheroo_register_handler() and the assignment of
> apple_gmux_data.]
> 
> Tested-by: Pierre Moreau 
> [MBP  5,3 2009  nvidia MCP79 + G96pre-retina  15"]
> Tested-by: Paul Hordiienko 
> [MBP  6,2 2010  intel ILK + nvidia GT216  pre-retina  15"]
> Tested-by: Lukas Wunner 
> [MBP  9,1 2012  intel IVB + nvidia GK107  pre-retina  15"]
> Tested-by: William Brown 
> [MBP  8,2 2011  intel SNB + amd turks pre-retina  15"]
> Tested-by: Bruno Bierbaumer 
> [MBP 11,3 2013  intel HSW + nvidia GK107  retina  15"]
> 
> Signed-off-by: Matthew Garrett 
> Reviewed-by: Lukas Wunner 
> Signed-off-by: Lukas Wunner 

My apologies for the delay. Thank you for the testing data and submitting.

I have queued this to testing. Pending success on 0-day, it will land in
linux-next shortly (tomorrow most likely) where I hope it will receive
additional testing.

-- 
Darren Hart
Intel Open Source Technology Center


[Intel-gfx] [PATCH] v2: Also add a DRIVER_* check like for all other maps functions to really short-circuit the code. And give drm_legacy_rmmap used by the dev unregister code the same treatment.

2015-11-23 Thread Daniel Vetter
On Mon, Nov 23, 2015 at 12:15:18PM +0200, Jani Nikula wrote:
> 
> Accidental patch?

Yup, dunno how that one ended up in there ...
-Daniel

> 
> On Mon, 23 Nov 2015, Daniel Vetter  wrote:
> > ---
> >  drivers/gpu/drm/drm_bufs.c | 14 ++
> >  include/drm/drm_legacy.h   |  2 +-
> >  2 files changed, 11 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_bufs.c b/drivers/gpu/drm/drm_bufs.c
> > index de18b8b78a26..5a51633da033 100644
> > --- a/drivers/gpu/drm/drm_bufs.c
> > +++ b/drivers/gpu/drm/drm_bufs.c
> > @@ -542,15 +542,17 @@ int drm_legacy_rmmap_locked(struct drm_device *dev, 
> > struct drm_local_map *map)
> >  }
> >  EXPORT_SYMBOL(drm_legacy_rmmap_locked);
> >  
> > -int drm_legacy_rmmap(struct drm_device *dev, struct drm_local_map *map)
> > +void drm_legacy_rmmap(struct drm_device *dev, struct drm_local_map *map)
> >  {
> > -   int ret;
> > +   if (!drm_core_check_feature(dev, DRIVER_KMS_LEGACY_CONTEXT) &&
> > +   drm_core_check_feature(dev, DRIVER_MODESET))
> > +   return;
> >  
> > mutex_lock(>struct_mutex);
> > -   ret = drm_legacy_rmmap_locked(dev, map);
> > +   drm_legacy_rmmap_locked(dev, map);
> > mutex_unlock(>struct_mutex);
> >  
> > -   return ret;
> > +   return;
> >  }
> >  EXPORT_SYMBOL(drm_legacy_rmmap);
> >  
> > @@ -558,6 +560,10 @@ void drm_legacy_master_rmmaps(struct drm_device *dev, 
> > struct drm_master *master)
> >  {
> > struct drm_map_list *r_list, *list_temp;
> >  
> > +   if (!drm_core_check_feature(dev, DRIVER_KMS_LEGACY_CONTEXT) &&
> > +   drm_core_check_feature(dev, DRIVER_MODESET))
> > +   return;
> > +
> > mutex_lock(>struct_mutex);
> > list_for_each_entry_safe(r_list, list_temp, >maplist, head) {
> > if (r_list->master == master) {
> > diff --git a/include/drm/drm_legacy.h b/include/drm/drm_legacy.h
> > index 24504b0eafea..a5ef2c7e40f8 100644
> > --- a/include/drm/drm_legacy.h
> > +++ b/include/drm/drm_legacy.h
> > @@ -154,7 +154,7 @@ struct drm_map_list {
> >  int drm_legacy_addmap(struct drm_device *d, resource_size_t offset,
> >   unsigned int size, enum drm_map_type type,
> >   enum drm_map_flags flags, struct drm_local_map **map_p);
> > -int drm_legacy_rmmap(struct drm_device *d, struct drm_local_map *map);
> > +void drm_legacy_rmmap(struct drm_device *d, struct drm_local_map *map);
> >  int drm_legacy_rmmap_locked(struct drm_device *d, struct drm_local_map 
> > *map);
> >  void drm_legacy_master_rmmaps(struct drm_device *dev,
> >   struct drm_master *master);
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch


[PATCH 15/29] drm/tegra: Use drm_gem_object_unreference_unlocked

2015-11-23 Thread Thierry Reding
On Mon, Nov 23, 2015 at 10:32:48AM +0100, Daniel Vetter wrote:
> This only grabs the mutex when really needed, but still has a
> might-acquire lockdep check to make sure that's always possible.
> With this patch tegra is officially struct_mutex free, yay!
> 
> v2: refernce_unlocked doesn't exist as kbuild spotted.
> 
> Cc: Thierry Reding 
> Acked-by: Thierry Reding 
> Signed-off-by: Daniel Vetter 
> ---
>  drivers/gpu/drm/tegra/drm.c | 4 +---
>  drivers/gpu/drm/tegra/gem.c | 6 +-
>  2 files changed, 2 insertions(+), 8 deletions(-)

Applied, thanks.

Thierry
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20151123/47e04a6b/attachment.sig>


[PATCH 14/29] drm/tegra: don't take dev->struct_mutex in mmap offset ioctl

2015-11-23 Thread Thierry Reding
On Mon, Nov 23, 2015 at 10:32:47AM +0100, Daniel Vetter wrote:
> Since David Herrmann's mmap vma manager rework we don't need to grab
> dev->struct_mutex any more to prevent races when looking up the mmap
> offset. Drop it and instead don't forget to use the unref_unlocked
> variant (since the drm core still cares).
> 
> v2: Finally get rid of the copypasta from another commit in this
> commit message. And convert to _unlocked like we need to (Patrik).
> 
> Cc: Patrik Jakobsson 
> Cc: Thierry Reding 
> Acked-by: Thierry Reding 
> Reviewed-by: Patrik Jakobsson 
> Signed-off-by: Daniel Vetter 
> ---
>  drivers/gpu/drm/tegra/gem.c | 7 +--
>  1 file changed, 1 insertion(+), 6 deletions(-)

Applied, thanks.

Thierry
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20151123/ef1ff25c/attachment.sig>


[PATCH 07/29] drm/tegra: Use unlocked gem unreferencing

2015-11-23 Thread Thierry Reding
On Mon, Nov 23, 2015 at 10:32:40AM +0100, Daniel Vetter wrote:
> For drm_gem_object_unreference callers are required to hold
> dev->struct_mutex, which these paths don't. Enforcing this requirement
> has become a bit more strict with
> 
> commit ef4c6270bf2867e2f8032e9614d1a8cfc6c71663
> Author: Daniel Vetter 
> Date:   Thu Oct 15 09:36:25 2015 +0200
> 
> drm/gem: Check locking in drm_gem_object_unreference
> 
> Cc: Thierry Reding 
> Signed-off-by: Daniel Vetter 
> ---
>  drivers/gpu/drm/tegra/drm.c | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)

Applied, thanks.

Thierry
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20151123/3d5abad2/attachment.sig>


[PATCH 04/29] drm/amdgpu: Use unlocked gem unreferencing

2015-11-23 Thread Christian König
On 23.11.2015 10:32, Daniel Vetter wrote:
> For drm_gem_object_unreference callers are required to hold
> dev->struct_mutex, which these paths don't. Enforcing this requirement
> has become a bit more strict with
>
> commit ef4c6270bf2867e2f8032e9614d1a8cfc6c71663
> Author: Daniel Vetter 
> Date:   Thu Oct 15 09:36:25 2015 +0200
>
>  drm/gem: Check locking in drm_gem_object_unreference
>
> Cc: Alex Deucher 
> Signed-off-by: Daniel Vetter 

For this and the radeon patch Reviewed-by: Christian König 


> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
> index 6fcbbcc2e99e..cfb6caad2a73 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
> @@ -263,7 +263,7 @@ out_unref:
>   
>   }
>   if (fb && ret) {
> - drm_gem_object_unreference(gobj);
> + drm_gem_object_unreference_unlocked(gobj);
>   drm_framebuffer_unregister_private(fb);
>   drm_framebuffer_cleanup(fb);
>   kfree(fb);




[PATCH] drm/radeon: Retry DDC probing on DVI on failure if we got an HPD interrupt

2015-11-23 Thread Lyude
On Mon, 2015-11-23 at 14:20 +, Deucher, Alexander wrote:
> > -Original Message-
> > From: Lyude [mailto:cpaul at redhat.com]
> > Sent: Sunday, November 22, 2015 9:45 PM
> > To: Koenig, Christian; Daniel Stone
> > Cc: Deucher, Alexander; David Airlie; dri-devel; Linux Kernel Mailing List;
> > Jerome Glisse; Benjamin Tissoires
> > Subject: Re: [PATCH] drm/radeon: Retry DDC probing on DVI on failure if we
> > got an HPD interrupt
> > 
> > On Sat, 2015-11-21 at 16:22 +0100, Christian König wrote:
> > > On 21.11.2015 15:49, Daniel Stone wrote:
> > > > Hi,
> > > > 
> > > > On 21 November 2015 at 14:22, Christian König  > > > .com> wrote:
> > > > > On 20.11.2015 16:52, cpaul at redhat.com wrote:
> > > > > > This is somewhat rare on most cards (depending on what angle
> > > > > > you plug
> > > > > > the DVI connector in), but on some cards it happens constantly.
> > > > > > The
> > > > > > Radeon R5 on the machine used for testing this patch for
> > > > > > instance, runs
> > > > > > into this issue just about every time I try to hotplug a DVI
> > > > > > monitor and
> > > > > > as a result hotplugging almost never works.
> > > > > > 
> > > > > > Rescheduling the hotplug work for a second when we run into an
> > > > > > HPD
> > > > > > signal with a failing DDC probe usually gives enough time for
> > > > > > the rest
> > > > > > of the connector's pins to make contact, and fixes this issue.
> > > > > > 
> > > > > > Signed-off-by: Stephen Chandler Paul 
> > > > > 
> > > > > Yeah, that's something I always wondered a about bit as well.
> > > > > 
> > > > > Debouncing is something very common done in electronics, but as
> > > > > far as I
> > > > > know the HPD pins don't necessary have an RC circuit so we might
> > > > > need to
> > > > > handle this case in software here.
> > > > > 
> > > > > A delay of something between 10-30ms between the last HPD
> > > > > interrupt and
> > > > > further processing of the signal doesn't sounds like such a bad
> > > > > idea.
> > Unfortunately the delay needed to make hotplugging work on the system
> > mentioned in the commit log can actually be over 700ms.
> > > > > 
> > > > > Retrying on the other hand doesn't necessarily improve the
> > > > > situation cause
> > > > > the delay introduced by this might not be enough.
> > Yeah, but I would think it would make sense to retry here so long as we
> > back off after a certain time. This would also have the benefit of
> > skipping this delay on systems that don't need it.
> > > > > 
> > > > > So I would rather vote for a fixed delay between an HPD interrupt
> > > > > and
> > > > > actually starting to process anything.
> > > > Yes-ish. Debouncing is useful, and ignoring buggy devices (e.g.
> > > > those
> > > > on marginal power) which send you HPD storms as well. But DP relies
> > > > on
> > > > 'short HPD' pulses which can be as brief as 2ms. So attempting to
> > > > totally debounce all HPD won't work.
> > > Well the discussion so far was about HPD on DVI only.
> > > 
> > > I'm not so deep into DP, but why should it uses HPD pulses of less
> > > than 2ms?
> > This is part of the DP spec iirc. This being said though, the issue
> > here with the HPD signal coming before the connector is ready only
> > happens on DVI. I haven't ever run into this issue with any HDMI cables
> > or DP cables, so I'm against imposing this on all connectors.
> > 
> > One of the solutions I've been thinking about with this: In
> > radeon_dvi_detect(), if we get a real hotplug signal retry the DDC
> > probe until at least a second has passed, after which we back off and
> > assume the port is disconnected.
> 
> FWIW, there are registers to adjust how long the hpd needs to be asserted
> before the hpd connection and short pulse interrupts are triggered.  See
> DC_HPDx_CONTROL.  Maybe adjusting them would help.  We currently just 
> write
> the default value, but it might be better to RMW the value in case there is a
> special golden value set by the vbios at init time.
> 
This sounds like a much better idea. I'm going to try to come up with a patch
that adjusts these by default for DVI ports.
> Alex
> 
> > > 
> > > Regards,
> > > Christian.
> > > 
> > > > 
> > > > Cheers,
> > > > Daniel
> > > 
> > --
> > Cheers,
> > Lyude
> 
-- 
Cheers,
Lyude



[PATCH] v2: Also add a DRIVER_* check like for all other maps functions to really short-circuit the code. And give drm_legacy_rmmap used by the dev unregister code the same treatment.

2015-11-23 Thread Daniel Vetter
---
 drivers/gpu/drm/drm_bufs.c | 14 ++
 include/drm/drm_legacy.h   |  2 +-
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_bufs.c b/drivers/gpu/drm/drm_bufs.c
index de18b8b78a26..5a51633da033 100644
--- a/drivers/gpu/drm/drm_bufs.c
+++ b/drivers/gpu/drm/drm_bufs.c
@@ -542,15 +542,17 @@ int drm_legacy_rmmap_locked(struct drm_device *dev, 
struct drm_local_map *map)
 }
 EXPORT_SYMBOL(drm_legacy_rmmap_locked);

-int drm_legacy_rmmap(struct drm_device *dev, struct drm_local_map *map)
+void drm_legacy_rmmap(struct drm_device *dev, struct drm_local_map *map)
 {
-   int ret;
+   if (!drm_core_check_feature(dev, DRIVER_KMS_LEGACY_CONTEXT) &&
+   drm_core_check_feature(dev, DRIVER_MODESET))
+   return;

mutex_lock(>struct_mutex);
-   ret = drm_legacy_rmmap_locked(dev, map);
+   drm_legacy_rmmap_locked(dev, map);
mutex_unlock(>struct_mutex);

-   return ret;
+   return;
 }
 EXPORT_SYMBOL(drm_legacy_rmmap);

@@ -558,6 +560,10 @@ void drm_legacy_master_rmmaps(struct drm_device *dev, 
struct drm_master *master)
 {
struct drm_map_list *r_list, *list_temp;

+   if (!drm_core_check_feature(dev, DRIVER_KMS_LEGACY_CONTEXT) &&
+   drm_core_check_feature(dev, DRIVER_MODESET))
+   return;
+
mutex_lock(>struct_mutex);
list_for_each_entry_safe(r_list, list_temp, >maplist, head) {
if (r_list->master == master) {
diff --git a/include/drm/drm_legacy.h b/include/drm/drm_legacy.h
index 24504b0eafea..a5ef2c7e40f8 100644
--- a/include/drm/drm_legacy.h
+++ b/include/drm/drm_legacy.h
@@ -154,7 +154,7 @@ struct drm_map_list {
 int drm_legacy_addmap(struct drm_device *d, resource_size_t offset,
  unsigned int size, enum drm_map_type type,
  enum drm_map_flags flags, struct drm_local_map **map_p);
-int drm_legacy_rmmap(struct drm_device *d, struct drm_local_map *map);
+void drm_legacy_rmmap(struct drm_device *d, struct drm_local_map *map);
 int drm_legacy_rmmap_locked(struct drm_device *d, struct drm_local_map *map);
 void drm_legacy_master_rmmaps(struct drm_device *dev,
  struct drm_master *master);
-- 
2.5.1



[PATCH 29/29] drm/vma_manage: Drop has_offset

2015-11-23 Thread Daniel Vetter
It's racy, creating mmap offsets is a slowpath, so better to remove it
to avoid drivers doing broken things.

The only user is i915, and it's ok there because everything (well
almost) is protected by dev->struct_mutex in i915-gem.

While at it add a note in the create_mmap_offset kerneldoc that
drivers must release it again. And then I also noticed that
drm_gem_object_release entirely lacks kerneldoc.

Cc: David Herrmann 
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/drm_gem.c   | 17 +
 drivers/gpu/drm/i915/i915_gem.c |  3 ---
 include/drm/drm_vma_manager.h   | 15 +--
 3 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 2e10bba4468b..5c5001171dd3 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -392,6 +392,10 @@ EXPORT_SYMBOL(drm_gem_handle_create);
  * @obj: obj in question
  *
  * This routine frees fake offsets allocated by drm_gem_create_mmap_offset().
+ *
+ * Note that drm_gem_object_release() already calls this function, so drivers
+ * don't have to take care of releasing the mmap offset themselves when freeing
+ * the GEM object.
  */
 void
 drm_gem_free_mmap_offset(struct drm_gem_object *obj)
@@ -415,6 +419,9 @@ EXPORT_SYMBOL(drm_gem_free_mmap_offset);
  * This routine allocates and attaches a fake offset for @obj, in cases where
  * the virtual size differs from the physical size (ie. obj->size).  Otherwise
  * just use drm_gem_create_mmap_offset().
+ *
+ * This function is idempotent and handles an already allocated mmap offset
+ * transparently. Drivers do not need to check for this case.
  */
 int
 drm_gem_create_mmap_offset_size(struct drm_gem_object *obj, size_t size)
@@ -436,6 +443,9 @@ EXPORT_SYMBOL(drm_gem_create_mmap_offset_size);
  * structures.
  *
  * This routine allocates and attaches a fake offset for @obj.
+ *
+ * Drivers can call drm_gem_free_mmap_offset() before freeing @obj to release
+ * the fake offset again.
  */
 int drm_gem_create_mmap_offset(struct drm_gem_object *obj)
 {
@@ -754,6 +764,13 @@ drm_gem_release(struct drm_device *dev, struct drm_file 
*file_private)
idr_destroy(_private->object_idr);
 }

+/**
+ * drm_gem_object_release - release GEM buffer object resources
+ * @obj: GEM buffer object
+ *
+ * This releases any structures and resources used by @obj and is the invers of
+ * drm_gem_object_init().
+ */
 void
 drm_gem_object_release(struct drm_gem_object *obj)
 {
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 33adc8f8ab20..f10a5d57377e 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1972,9 +1972,6 @@ static int i915_gem_object_create_mmap_offset(struct 
drm_i915_gem_object *obj)
struct drm_i915_private *dev_priv = obj->base.dev->dev_private;
int ret;

-   if (drm_vma_node_has_offset(>base.vma_node))
-   return 0;
-
dev_priv->mm.shrinker_no_lock_stealing = true;

ret = drm_gem_create_mmap_offset(>base);
diff --git a/include/drm/drm_vma_manager.h b/include/drm/drm_vma_manager.h
index 2f63dd5e05eb..06ea8e077ec2 100644
--- a/include/drm/drm_vma_manager.h
+++ b/include/drm/drm_vma_manager.h
@@ -176,19 +176,6 @@ static inline unsigned long drm_vma_node_size(struct 
drm_vma_offset_node *node)
 }

 /**
- * drm_vma_node_has_offset() - Check whether node is added to offset manager
- * @node: Node to be checked
- *
- * RETURNS:
- * true iff the node was previously allocated an offset and added to
- * an vma offset manager.
- */
-static inline bool drm_vma_node_has_offset(struct drm_vma_offset_node *node)
-{
-   return drm_mm_node_allocated(>vm_node);
-}
-
-/**
  * drm_vma_node_offset_addr() - Return sanitized offset for user-space mmaps
  * @node: Linked offset node
  *
@@ -220,7 +207,7 @@ static inline __u64 drm_vma_node_offset_addr(struct 
drm_vma_offset_node *node)
 static inline void drm_vma_node_unmap(struct drm_vma_offset_node *node,
  struct address_space *file_mapping)
 {
-   if (drm_vma_node_has_offset(node))
+   if (drm_mm_node_allocated(>vm_node))
unmap_mapping_range(file_mapping,
drm_vma_node_offset_addr(node),
drm_vma_node_size(node) << PAGE_SHIFT, 1);
-- 
2.5.1



[PATCH 28/29] drm/vgem: Drop dev->struct_mutex

2015-11-23 Thread Daniel Vetter
With the previous two changes it doesn't protect anything any more.

v2: Use _unlocked unreference variant.

Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/vgem/vgem_drv.c | 16 
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c
index 1a609347236b..807a24d3c0a8 100644
--- a/drivers/gpu/drm/vgem/vgem_drv.c
+++ b/drivers/gpu/drm/vgem/vgem_drv.c
@@ -103,12 +103,8 @@ static int vgem_gem_fault(struct vm_area_struct *vma, 
struct vm_fault *vmf)
if (page_offset > num_pages)
return VM_FAULT_SIGBUS;

-   mutex_lock(>struct_mutex);
-
ret = vm_insert_page(vma, (unsigned long)vmf->virtual_address,
 obj->pages[page_offset]);
-
-   mutex_unlock(>struct_mutex);
switch (ret) {
case 0:
return VM_FAULT_NOPAGE;
@@ -205,12 +201,9 @@ int vgem_gem_dumb_map(struct drm_file *file, struct 
drm_device *dev,
int ret = 0;
struct drm_gem_object *obj;

-   mutex_lock(>struct_mutex);
obj = drm_gem_object_lookup(dev, file, handle);
-   if (!obj) {
-   ret = -ENOENT;
-   goto unlock;
-   }
+   if (!obj)
+   return -ENOENT;

ret = drm_gem_create_mmap_offset(obj);
if (ret)
@@ -223,9 +216,8 @@ int vgem_gem_dumb_map(struct drm_file *file, struct 
drm_device *dev,
*offset = drm_vma_node_offset_addr(>vma_node);

 unref:
-   drm_gem_object_unreference(obj);
-unlock:
-   mutex_unlock(>struct_mutex);
+   drm_gem_object_unreference_unlocked(obj);
+
return ret;
 }

-- 
2.5.1



[PATCH 27/29] drm/vgem: Move get_pages to gem_create

2015-11-23 Thread Daniel Vetter
vgem doesn't have a shrinker or anything like that and drops backing
storage only at object_free time. There's no use in trying to be
clever and allocating backing storage delayed, it only causes trouble
by requiring locking.

Instead grab pages when we allocate the object right away.

Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/vgem/vgem_drv.c | 12 
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c
index 63026d4324ad..1a609347236b 100644
--- a/drivers/gpu/drm/vgem/vgem_drv.c
+++ b/drivers/gpu/drm/vgem/vgem_drv.c
@@ -154,6 +154,10 @@ static struct drm_gem_object *vgem_gem_create(struct 
drm_device *dev,
if (err)
goto out;

+   ret = vgem_gem_get_pages(to_vgem_bo(obj));
+   if (ret)
+   goto out;
+
err = drm_gem_handle_create(file, gem_object, handle);
if (err)
goto handle_out;
@@ -216,16 +220,8 @@ int vgem_gem_dumb_map(struct drm_file *file, struct 
drm_device *dev,

obj->filp->private_data = obj;

-   ret = vgem_gem_get_pages(to_vgem_bo(obj));
-   if (ret)
-   goto fail_get_pages;
-
*offset = drm_vma_node_offset_addr(>vma_node);

-   goto unref;
-
-fail_get_pages:
-   drm_gem_free_mmap_offset(obj);
 unref:
drm_gem_object_unreference(obj);
 unlock:
-- 
2.5.1



[PATCH 26/29] drm/vgem: Simplify dum_map

2015-11-23 Thread Daniel Vetter
The offset manager already checks for existing offsets internally,
while holding suitable locks. We can drop this check.

Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/vgem/vgem_drv.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c
index 02ae60c395b7..63026d4324ad 100644
--- a/drivers/gpu/drm/vgem/vgem_drv.c
+++ b/drivers/gpu/drm/vgem/vgem_drv.c
@@ -208,11 +208,9 @@ int vgem_gem_dumb_map(struct drm_file *file, struct 
drm_device *dev,
goto unlock;
}

-   if (!drm_vma_node_has_offset(>vma_node)) {
-   ret = drm_gem_create_mmap_offset(obj);
-   if (ret)
-   goto unref;
-   }
+   ret = drm_gem_create_mmap_offset(obj);
+   if (ret)
+   goto unref;

BUG_ON(!obj->filp);

-- 
2.5.1



[PATCH 25/29] drm/exynos: drop struct_mutex from fbdev setup

2015-11-23 Thread Daniel Vetter
Doesn't protect anything at all, and probably just here because a long
time ago dev->struct_mutex was required to allocate gem objects.

With this patch exynos is completely struct_mutex free!

Cc: Inki Dae 
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 22 --
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c 
b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
index f6118baa8e3e..e1f7abaa61df 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
@@ -138,8 +138,6 @@ static int exynos_drm_fbdev_create(struct drm_fb_helper 
*helper,
mode_cmd.pixel_format = drm_mode_legacy_fb_format(sizes->surface_bpp,
  sizes->surface_depth);

-   mutex_lock(>struct_mutex);
-
size = mode_cmd.pitches[0] * mode_cmd.height;

exynos_gem = exynos_drm_gem_create(dev, EXYNOS_BO_CONTIG, size);
@@ -154,10 +152,8 @@ static int exynos_drm_fbdev_create(struct drm_fb_helper 
*helper,
   size);
}

-   if (IS_ERR(exynos_gem)) {
-   ret = PTR_ERR(exynos_gem);
-   goto out;
-   }
+   if (IS_ERR(exynos_gem))
+   return PTR_ERR(exynos_gem);

exynos_fbdev->exynos_gem = exynos_gem;

@@ -173,7 +169,6 @@ static int exynos_drm_fbdev_create(struct drm_fb_helper 
*helper,
if (ret < 0)
goto err_destroy_framebuffer;

-   mutex_unlock(>struct_mutex);
return ret;

 err_destroy_framebuffer:
@@ -181,13 +176,12 @@ err_destroy_framebuffer:
 err_destroy_gem:
exynos_drm_gem_destroy(exynos_gem);

-/*
- * if failed, all resources allocated above would be released by
- * drm_mode_config_cleanup() when drm_load() had been called prior
- * to any specific driver such as fimd or hdmi driver.
- */
-out:
-   mutex_unlock(>struct_mutex);
+   /*
+* if failed, all resources allocated above would be released by
+* drm_mode_config_cleanup() when drm_load() had been called prior
+* to any specific driver such as fimd or hdmi driver.
+*/
+
return ret;
 }

-- 
2.5.1



[PATCH 24/29] drm/exynos: drop struct_mutex from exynos_drm_gem_get_ioctl

2015-11-23 Thread Daniel Vetter
The only things this protects is reading ->flags and ->size, both of
which are invariant over the lifetime of an exynos gem bo. So no
locking needed at all (besides that, nothing protects the writers
anyway).

Aside: exynos_gem_obj->size is redundant with
exynos_gem_obj->base.size and probably should be removed.

v2: Use _unlocked unreference (Daniel Stone).

Cc: Daniel Stone 
Cc: Inki Dae 
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/exynos/exynos_drm_gem.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c 
b/drivers/gpu/drm/exynos/exynos_drm_gem.c
index b198fa560106..e735827e0c6d 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
@@ -352,12 +352,9 @@ int exynos_drm_gem_get_ioctl(struct drm_device *dev, void 
*data,
struct drm_exynos_gem_info *args = data;
struct drm_gem_object *obj;

-   mutex_lock(>struct_mutex);
-
obj = drm_gem_object_lookup(dev, file_priv, args->handle);
if (!obj) {
DRM_ERROR("failed to lookup gem object.\n");
-   mutex_unlock(>struct_mutex);
return -EINVAL;
}

@@ -366,8 +363,7 @@ int exynos_drm_gem_get_ioctl(struct drm_device *dev, void 
*data,
args->flags = exynos_gem->flags;
args->size = exynos_gem->size;

-   drm_gem_object_unreference(obj);
-   mutex_unlock(>struct_mutex);
+   drm_gem_object_unreference_unlocked(obj);

return 0;
 }
-- 
2.5.1



[PATCH 23/29] drm/exynos: drop struct_mutex from exynos_gem_map_sgt_with_dma

2015-11-23 Thread Daniel Vetter
The sg table isn't refcounted, there's no corresponding locking for
unmapping and drm_map_sg is ok with being called concurrently.

So drop the locking since it doesn't protect anything.

Cc: Inki Dae 
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/exynos/exynos_drm_gem.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c 
b/drivers/gpu/drm/exynos/exynos_drm_gem.c
index 44e710ab84de..b198fa560106 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
@@ -378,16 +378,12 @@ int exynos_gem_map_sgt_with_dma(struct drm_device 
*drm_dev,
 {
int nents;

-   mutex_lock(_dev->struct_mutex);
-
nents = dma_map_sg(drm_dev->dev, sgt->sgl, sgt->nents, dir);
if (!nents) {
DRM_ERROR("failed to map sgl with dma.\n");
-   mutex_unlock(_dev->struct_mutex);
return nents;
}

-   mutex_unlock(_dev->struct_mutex);
return 0;
 }

-- 
2.5.1



[PATCH 22/29] drm/exynos: Drop dev->struct_mutex from mmap offset function

2015-11-23 Thread Daniel Vetter
Simply forgotten about this when I was doing my general cleansing of
simple gem mmap offset functions. There's nothing but core functions
called here, and they all have their own protection already.

Aside: DRM_ERROR for userspace controlled input isn't great, but
that's for another patch.

v2: Use _unlocked unreference (Daniel Stone).

Cc: Daniel Stone 
Cc: Inki Dae 
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/exynos/exynos_drm_gem.c | 9 ++---
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c 
b/drivers/gpu/drm/exynos/exynos_drm_gem.c
index 252eb301470c..44e710ab84de 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
@@ -448,8 +448,6 @@ int exynos_drm_gem_dumb_map_offset(struct drm_file 
*file_priv,
struct drm_gem_object *obj;
int ret = 0;

-   mutex_lock(>struct_mutex);
-
/*
 * get offset of memory allocated for drm framebuffer.
 * - this callback would be called by user application
@@ -459,16 +457,13 @@ int exynos_drm_gem_dumb_map_offset(struct drm_file 
*file_priv,
obj = drm_gem_object_lookup(dev, file_priv, handle);
if (!obj) {
DRM_ERROR("failed to lookup gem object.\n");
-   ret = -EINVAL;
-   goto unlock;
+   return -EINVAL;
}

*offset = drm_vma_node_offset_addr(>vma_node);
DRM_DEBUG_KMS("offset = 0x%lx\n", (unsigned long)*offset);

-   drm_gem_object_unreference(obj);
-unlock:
-   mutex_unlock(>struct_mutex);
+   drm_gem_object_unreference_unlocked(obj);
return ret;
 }

-- 
2.5.1



[PATCH 21/29] drm/nouveau: Drop dev->struct_mutex from fbdev init

2015-11-23 Thread Daniel Vetter
Doesn't protect anything at all.

With this patch nouveau is completely dev->struct_mutex free!

Cc: Ben Skeggs 
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/nouveau/nouveau_fbcon.c | 5 -
 1 file changed, 5 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c 
b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
index 59f27e774acb..3bae706126bd 100644
--- a/drivers/gpu/drm/nouveau/nouveau_fbcon.c
+++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
@@ -386,8 +386,6 @@ nouveau_fbcon_create(struct drm_fb_helper *helper,
}
}

-   mutex_lock(>struct_mutex);
-
info = drm_fb_helper_alloc_fbi(helper);
if (IS_ERR(info)) {
ret = PTR_ERR(info);
@@ -426,8 +424,6 @@ nouveau_fbcon_create(struct drm_fb_helper *helper,

/* Use default scratch pixmap (info->pixmap.flags = FB_PIXMAP_SYSTEM) */

-   mutex_unlock(>struct_mutex);
-
if (chan)
nouveau_fbcon_accel_init(dev);
nouveau_fbcon_zfill(dev, fbcon);
@@ -441,7 +437,6 @@ nouveau_fbcon_create(struct drm_fb_helper *helper,
return 0;

 out_unlock:
-   mutex_unlock(>struct_mutex);
if (chan)
nouveau_bo_vma_del(nvbo, >nouveau_fb.vma);
nouveau_bo_unmap(nvbo);
-- 
2.5.1



[PATCH 20/29] drm/gma500: Add driver private mutex for the fault handler

2015-11-23 Thread Daniel Vetter
There's currently two places where the gma500 fault handler relies
upon dev->struct_mutex:
- To protect r->mappping
- To make sure vm_insert_pfn isn't called concurrently (in which case
  the 2nd thread would get an error code).

Everything else (specifically psb_gtt_pin) is already protected by
some other locks. Hence just create a new driver-private mmap_mutex
just for this function.

With this gma500 is complete dev->struct_mutex free!

Cc: Patrik Jakobsson 
Acked-by: Patrik Jakobsson 
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/gma500/gem.c | 4 ++--
 drivers/gpu/drm/gma500/gtt.c | 1 +
 drivers/gpu/drm/gma500/psb_drv.h | 2 ++
 3 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/gma500/gem.c b/drivers/gpu/drm/gma500/gem.c
index f0357f525f56..506224b3a0ad 100644
--- a/drivers/gpu/drm/gma500/gem.c
+++ b/drivers/gpu/drm/gma500/gem.c
@@ -182,7 +182,7 @@ int psb_gem_fault(struct vm_area_struct *vma, struct 
vm_fault *vmf)

/* Make sure we don't parallel update on a fault, nor move or remove
   something from beneath our feet */
-   mutex_lock(>struct_mutex);
+   mutex_lock(_priv->mmap_mutex);

/* For now the mmap pins the object and it stays pinned. As things
   stand that will do us no harm */
@@ -208,7 +208,7 @@ int psb_gem_fault(struct vm_area_struct *vma, struct 
vm_fault *vmf)
ret = vm_insert_pfn(vma, (unsigned long)vmf->virtual_address, pfn);

 fail:
-   mutex_unlock(>struct_mutex);
+   mutex_unlock(_priv->mmap_mutex);
switch (ret) {
case 0:
case -ERESTARTSYS:
diff --git a/drivers/gpu/drm/gma500/gtt.c b/drivers/gpu/drm/gma500/gtt.c
index ce015db59dc6..8f69225ce2b4 100644
--- a/drivers/gpu/drm/gma500/gtt.c
+++ b/drivers/gpu/drm/gma500/gtt.c
@@ -425,6 +425,7 @@ int psb_gtt_init(struct drm_device *dev, int resume)

if (!resume) {
mutex_init(_priv->gtt_mutex);
+   mutex_init(_priv->mmap_mutex);
psb_gtt_alloc(dev);
}

diff --git a/drivers/gpu/drm/gma500/psb_drv.h b/drivers/gpu/drm/gma500/psb_drv.h
index e21726ecac32..3bd2c726dd61 100644
--- a/drivers/gpu/drm/gma500/psb_drv.h
+++ b/drivers/gpu/drm/gma500/psb_drv.h
@@ -465,6 +465,8 @@ struct drm_psb_private {
struct mutex gtt_mutex;
struct resource *gtt_mem;   /* Our PCI resource */

+   struct mutex mmap_mutex;
+
struct psb_mmu_driver *mmu;
struct psb_mmu_pd *pf_pd;

-- 
2.5.1



[PATCH 19/29] drm/gma500: Drop dev->struct_mutex from mmap offset function

2015-11-23 Thread Daniel Vetter
Simply forgotten about this when I was doing my general cleansing of
simple gem mmap offset functions. There's nothing but core functions
called here, and they all have their own protection already.

Cc: Patrik Jakobsson 
Acked-by: Patrik Jakobsson 
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/gma500/gem.c | 13 +++--
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/gma500/gem.c b/drivers/gpu/drm/gma500/gem.c
index e3bdc8b1c32c..f0357f525f56 100644
--- a/drivers/gpu/drm/gma500/gem.c
+++ b/drivers/gpu/drm/gma500/gem.c
@@ -62,15 +62,10 @@ int psb_gem_dumb_map_gtt(struct drm_file *file, struct 
drm_device *dev,
int ret = 0;
struct drm_gem_object *obj;

-   mutex_lock(>struct_mutex);
-
/* GEM does all our handle to object mapping */
obj = drm_gem_object_lookup(dev, file, handle);
-   if (obj == NULL) {
-   ret = -ENOENT;
-   goto unlock;
-   }
-   /* What validation is needed here ? */
+   if (obj == NULL)
+   return -ENOENT;

/* Make it mmapable */
ret = drm_gem_create_mmap_offset(obj);
@@ -78,9 +73,7 @@ int psb_gem_dumb_map_gtt(struct drm_file *file, struct 
drm_device *dev,
goto out;
*offset = drm_vma_node_offset_addr(>vma_node);
 out:
-   drm_gem_object_unreference(obj);
-unlock:
-   mutex_unlock(>struct_mutex);
+   drm_gem_object_unreference_unlocked(obj);
return ret;
 }

-- 
2.5.1



[PATCH 18/29] drm/gma500: Drop dev->struct_mutex from fbdev init/teardown code

2015-11-23 Thread Daniel Vetter
This is init/teardown code, locking is just to appease locking checks.
And since gem create/free doesn't need this any more there's really no
reason for grabbing dev->struct_mutex.

Again important to switch obj_unref to _unlocked variants.

Cc: Patrik Jakobsson 
Acked-by: Patrik Jakobsson 
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/gma500/framebuffer.c | 12 ++--
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/gma500/framebuffer.c 
b/drivers/gpu/drm/gma500/framebuffer.c
index dc0508dca1d4..ee95c03a8c54 100644
--- a/drivers/gpu/drm/gma500/framebuffer.c
+++ b/drivers/gpu/drm/gma500/framebuffer.c
@@ -406,8 +406,6 @@ static int psbfb_create(struct psb_fbdev *fbdev,

memset(dev_priv->vram_addr + backing->offset, 0, size);

-   mutex_lock(>struct_mutex);
-
info = drm_fb_helper_alloc_fbi(>psb_fb_helper);
if (IS_ERR(info)) {
ret = PTR_ERR(info);
@@ -463,17 +461,15 @@ static int psbfb_create(struct psb_fbdev *fbdev,
dev_dbg(dev->dev, "allocated %dx%d fb\n",
psbfb->base.width, psbfb->base.height);

-   mutex_unlock(>struct_mutex);
return 0;
 out_unref:
if (backing->stolen)
psb_gtt_free_range(dev, backing);
else
-   drm_gem_object_unreference(>gem);
+   drm_gem_object_unreference_unlocked(>gem);

drm_fb_helper_release_fbi(>psb_fb_helper);
 out_err1:
-   mutex_unlock(>struct_mutex);
psb_gtt_free_range(dev, backing);
return ret;
 }
@@ -569,7 +565,7 @@ static int psb_fbdev_destroy(struct drm_device *dev, struct 
psb_fbdev *fbdev)
drm_framebuffer_cleanup(>base);

if (psbfb->gtt)
-   drm_gem_object_unreference(>gtt->gem);
+   drm_gem_object_unreference_unlocked(>gtt->gem);
return 0;
 }

@@ -784,12 +780,8 @@ void psb_modeset_cleanup(struct drm_device *dev)
 {
struct drm_psb_private *dev_priv = dev->dev_private;
if (dev_priv->modeset) {
-   mutex_lock(>struct_mutex);
-
drm_kms_helper_poll_fini(dev);
psb_fbdev_fini(dev);
drm_mode_config_cleanup(dev);
-
-   mutex_unlock(>struct_mutex);
}
 }
-- 
2.5.1



[PATCH 17/29] drm/gma500: Drop dev->struct_mutex from modeset code

2015-11-23 Thread Daniel Vetter
It's either init code or already protected by other means. Note
that psb_gtt_pin/unpin has it's own lock, and that's really the
only piece of driver private state - all the modeset state is
protected with modeset locks already.

The only important bit is to switch all gem_obj_unref calls to the
_unlocked variant.

Cc: Patrik Jakobsson 
Acked-by: Patrik Jakobsson 
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/gma500/gma_display.c | 13 +++--
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/gma500/gma_display.c 
b/drivers/gpu/drm/gma500/gma_display.c
index 001b450b27b3..ff17af4cfc64 100644
--- a/drivers/gpu/drm/gma500/gma_display.c
+++ b/drivers/gpu/drm/gma500/gma_display.c
@@ -349,8 +349,6 @@ int gma_crtc_cursor_set(struct drm_crtc *crtc,
/* If we didn't get a handle then turn the cursor off */
if (!handle) {
temp = CURSOR_MODE_DISABLE;
-   mutex_lock(>struct_mutex);
-
if (gma_power_begin(dev, false)) {
REG_WRITE(control, temp);
REG_WRITE(base, 0);
@@ -362,11 +360,9 @@ int gma_crtc_cursor_set(struct drm_crtc *crtc,
gt = container_of(gma_crtc->cursor_obj,
  struct gtt_range, gem);
psb_gtt_unpin(gt);
-   drm_gem_object_unreference(gma_crtc->cursor_obj);
+   
drm_gem_object_unreference_unlocked(gma_crtc->cursor_obj);
gma_crtc->cursor_obj = NULL;
}
-
-   mutex_unlock(>struct_mutex);
return 0;
}

@@ -376,7 +372,6 @@ int gma_crtc_cursor_set(struct drm_crtc *crtc,
return -EINVAL;
}

-   mutex_lock(>struct_mutex);
obj = drm_gem_object_lookup(dev, file_priv, handle);
if (!obj) {
ret = -ENOENT;
@@ -441,17 +436,15 @@ int gma_crtc_cursor_set(struct drm_crtc *crtc,
if (gma_crtc->cursor_obj) {
gt = container_of(gma_crtc->cursor_obj, struct gtt_range, gem);
psb_gtt_unpin(gt);
-   drm_gem_object_unreference(gma_crtc->cursor_obj);
+   drm_gem_object_unreference_unlocked(gma_crtc->cursor_obj);
}

gma_crtc->cursor_obj = obj;
 unlock:
-   mutex_unlock(>struct_mutex);
return ret;

 unref_cursor:
-   drm_gem_object_unreference(obj);
-   mutex_unlock(>struct_mutex);
+   drm_gem_object_unreference_unlocked(obj);
return ret;
 }

-- 
2.5.1



[PATCH 16/29] drm/gma500: Use correct unref in the gem bo create function

2015-11-23 Thread Daniel Vetter
This is called without dev->struct_mutex held, we need to use the
_unlocked variant.

Never caught in the wild since you'd need an evil userspace which
races a gem_close ioctl call with the in-progress open.

Cc: Patrik Jakobsson 
Acked-by: Patrik Jakobsson 
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/gma500/gem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/gma500/gem.c b/drivers/gpu/drm/gma500/gem.c
index c707fa6fca85..e3bdc8b1c32c 100644
--- a/drivers/gpu/drm/gma500/gem.c
+++ b/drivers/gpu/drm/gma500/gem.c
@@ -130,7 +130,7 @@ int psb_gem_create(struct drm_file *file, struct drm_device 
*dev, u64 size,
return ret;
}
/* We have the initial and handle reference but need only one now */
-   drm_gem_object_unreference(>gem);
+   drm_gem_object_unreference_unlocked(>gem);
*handlep = handle;
return 0;
 }
-- 
2.5.1



  1   2   >