3.15-rc2: i915 regression: only top 20% of screen works in X

[Pavel Machek]

Hi!

After update to 3.15-rc2, only top 20% of screen works on X.

00:02.0 VGA compatible controller: Intel Corporation 4 Series Chipset
Integrated Graphics Controller (rev 03)

00:02.1 Display controller: Intel Corporation 4 Series Chipset
Integrated Graphics Controller (rev 03)
   Subsystem: Intel Corporation Device d614
   Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop-
   ParErr- Stepping- SERR- FastB2B- DisINTx-
   Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=fast
   >TAbort- SERR- 

This worked before. I believe it worked in 3.14. It definitely works
in 3.11-rc2.

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[PATCH 2/3] drm/plane-helper: Add drm_primary_helper_check_update()

[Ville Syrjälä]

On Wed, Apr 23, 2014 at 10:04:00AM -0700, Matt Roper wrote:
> Pull the parameter checking from drm_primary_helper_update() out into
> its own function; drivers that provide their own setplane()
> implementations rather than using the helper may still want to share
> this parameter checking logic.
> 
> A few of the checks here were also updated based on suggestions by
> Ville Syrj?l?.
> 
> Cc: Ville Syrj?l? 
> Cc: dri-devel at lists.freedesktop.org
> Signed-off-by: Matt Roper 
> ---
>  drivers/gpu/drm/drm_plane_helper.c | 148 
> +
>  include/drm/drm_plane_helper.h |   9 +++
>  2 files changed, 128 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_plane_helper.c 
> b/drivers/gpu/drm/drm_plane_helper.c
> index 65c4a00..9bbbdf2 100644
> --- a/drivers/gpu/drm/drm_plane_helper.c
> +++ b/drivers/gpu/drm/drm_plane_helper.c
> @@ -66,6 +66,102 @@ static int get_connectors_for_crtc(struct drm_crtc *crtc,
>  }
>  
>  /**
> + * drm_primary_helper_check_update() - Check primary plane update for 
> validity
> + * @plane: plane object to update
> + * @crtc: owning CRTC of owning plane
> + * @fb: framebuffer to flip onto plane
> + * @crtc_x: x offset of primary plane on crtc
> + * @crtc_y: y offset of primary plane on crtc
> + * @crtc_w: width of primary plane rectangle on crtc
> + * @crtc_h: height of primary plane rectangle on crtc
> + * @src_x: x offset of @fb for panning
> + * @src_y: y offset of @fb for panning
> + * @src_w: width of source rectangle in @fb
> + * @src_h: height of source rectangle in @fb
> + * @can_scale: is primary plane scaling legal?
> + * @can_position: is it legal to position the primary plane such that it
> + *doesn't cover the entire crtc?
> + *
> + * Checks that a desired primary plane update is valid.  Drivers that provide
> + * their own primary plane handling may still wish to call this function to
> + * avoid duplication of error checking code.
> + *
> + * RETURNS:
> + * Zero if update appears valid, error code on failure
> + */
> +int drm_primary_helper_check_update(struct drm_plane *plane,
> + struct drm_crtc *crtc,
> + struct drm_framebuffer *fb,
> + int crtc_x, int crtc_y,
> + unsigned int crtc_w, unsigned int crtc_h,
> + uint32_t src_x, uint32_t src_y,
> + uint32_t src_w, uint32_t src_h,
> + bool can_scale,
> + bool can_position)
> +{
> + struct drm_rect dest = {
> + .x1 = crtc_x,
> + .y1 = crtc_y,
> + .x2 = crtc_x + crtc_w,
> + .y2 = crtc_y + crtc_h,
> + };
> + struct drm_rect src = {
> + .x1 = src_x >> 16,
> + .y1 = src_y >> 16,
> + .x2 = (src_x + src_w) >> 16,
> + .y2 = (src_y + src_h) >> 16,

I think you want '(x>>16) + (y>>16)' instead. Otherwise you may end up
increasing the source width/height.

> + };
> + const struct drm_rect clip = {
> + .x2 = crtc->mode.hdisplay,
> + .y2 = crtc->mode.vdisplay,
> + };
> + int hscale, vscale;
> + int visible;
> +
> + if (!crtc->enabled) {
> + DRM_DEBUG_KMS("Cannot update primary plane of a disabled 
> CRTC.\n");
> + return -EINVAL;
> + }

We allow this for sprites, so I'd allow it for everything. I'd be fine
with leaving this restriction in drm_primary_helper_update() simply
because I have no interst in auditing every other driver.

Although I think we still overwrite the primary plane configure during
setcrtc. We should really change that so that the user can pre-configure
all the planes and then watch them pop into view during a modeset as
previously configured.

> +
> + /* setplane API takes shifted source rectangle values; unshift them */
> + src_x >>= 16;
> + src_y >>= 16;
> + src_w >>= 16;
> + src_h >>= 16;
> +
> + /* check scaling */
> + if (!can_scale && (crtc_w != src_w || crtc_h != src_h)) {
> + DRM_DEBUG_KMS("Can't scale primary plane\n");
> + return -EINVAL;
> + }
> +
> + /*
> +  * Drivers that can scale should perform their own min/max scale
> +  * factor checks.
> +  */
> + hscale = drm_rect_calc_hscale(, , 0, INT_MAX);
> + vscale = drm_rect_calc_vscale(, , 0, INT_MAX);
> + visible = drm_rect_clip_scaled(, , , hscale, vscale);

w/o sub-pixel coordinates the scaling factors will be truncated
to integers, which makes the clipping rather bogus if the plane can
actually scale. I think I'd just make this code assume that scaling
isn't supported, and if the driver supports scaling it can just
implent the appropriate code itself. We can try to refactor some of
the scaling aware code from intel_sprite later if warranted.

But I'm starting to 

[Bug 77835] X crash when using xrandr with R9 290X and 7870 for triple head

[bugzilla-dae...@freedesktop.org]

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

--- Comment #4 from Maik Broemme  ---
Alex, I've tried 3.15-rc2 and your new ucode files from
http://people.freedesktop.org/~agd5f/radeon_ucode/ for both cards now, but
still no luck with xrandr and both cards. Firmware is correctly loaded. Same
crash.

[   21.174824] [drm] initializing kernel modesetting (HAWAII 0x1002:0x67B0
0x174B:0xE285).
[   21.436057] [drm] Loading HAWAII Microcode
[   21.442757] [drm] radeon/HAWAII_mc2.bin: 32364 bytes
[   21.698816] [drm] initializing kernel modesetting (PITCAIRN 0x1002:0x6818
0x1682:0x3251).
[   21.699439] [drm] Loading PITCAIRN Microcode
[   21.701027] [drm] radeon/PITCAIRN_mc2.bin: 31100 bytes

-- 
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/20140423/ff641e25/attachment.html>

[Bug 77582] [r600g] ogl-samples GL3.2 and GL3.3 tests doesn't run without overriding GL/GLSL version

[bugzilla-dae...@freedesktop.org]

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

Benjamin Bellec  changed:

   What|Removed |Added

 Resolution|NOTOURBUG   |INVALID
   Assignee|mesa-dev at lists.freedesktop. |dri-devel at 
lists.freedesktop
   |org |.org
  Component|Mesa core   |Drivers/Gallium/r600

--- Comment #7 from Benjamin Bellec  ---
Old libGL in /usr/local/lib64

-- 
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/20140423/32d89040/attachment-0001.html>

[Bug 77835] X crash when using xrandr with R9 290X and 7870 for triple head

[bugzilla-dae...@freedesktop.org]

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

--- Comment #3 from Maik Broemme  ---
This was my experience too but even without acceleration of the R9 290X the X
server crashes when using both cards.

-- 
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/20140423/73a14bdc/attachment.html>

[Bug 77835] X crash when using xrandr with R9 290X and 7870 for triple head

[bugzilla-dae...@freedesktop.org]

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

--- Comment #2 from Alex Deucher  ---
Acceleration is not currently stable on Hawaii cards (R9 290) so it is disabled
by default.

-- 
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/20140423/5c84b337/attachment.html>

[Bug 77835] X crash when using xrandr with R9 290X and 7870 for triple head

[bugzilla-dae...@freedesktop.org]

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

Maik Broemme  changed:

   What|Removed |Added

   Severity|major   |critical

-- 
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/20140423/c9b78537/attachment.html>

[Intel-gfx] [PATCH 1/3] drm: Check CRTC compatibility in setplane

[Daniel Vetter]

On Wed, Apr 23, 2014 at 11:26:04AM -0700, Matt Roper wrote:
> On Wed, Apr 23, 2014 at 08:03:50PM +0200, Daniel Vetter wrote:
> > On Wed, Apr 23, 2014 at 10:03:59AM -0700, Matt Roper wrote:
> > > The DRM core setplane code should check that the plane is usable on the
> > > specified CRTC before calling into the driver.
> > > 
> > > Prior to this patch, a plane's possible_crtcs field was purely
> > > informational for userspace and was never actually verified at the
> > > kernel level (aside from the primary plane helper).
> > > 
> > > Cc: dri-devel at lists.freedesktop.org
> > > Signed-off-by: Matt Roper 
> > 
> > Do you have a nasty igt somewhere which tries to use a plane on the wrong
> > crtc? Especially useful since our i915 code and hw relies on this.
> > -Daniel
> 
> Not yet; I'll add/modify a test to include that.  I'm still working on
> some other i-g-t test updates for the primary plane stuff based on your
> previous feedback.
> 
> Speaking of i-g-t testing, what is the expected behavior if someone
> issues a pageflip ioctl while the primary plane is disabled?  I'd expect
> it to return an error, but the -EBUSY currently returned by the DRM core
> seems a bit confusing/misleading in my opinion.  The comments indicate
> that the existing assumption is that crtc->primary->fb == NULL indicates 
> a hotplug event that userspace hasn't noticed yet, although now we
> clearly have other ways to hit that error, so I'm not sure EBUSY is
> really the right response.

That comment is outdated since nowadays the kernel doesn't randomly kill a
crtc if its outputs gets unplugged. I think a simple -EINVAL should be
good. We'd need to update kms_flip with that new value though.

A quick grep through the intel ddx shows that we don't really depend on
this either way. -EBUSY for a disabled primary plane (whether the crtc is
on or not doesn't matter imo) really makes no sense.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[Bug 77835] X crash when using xrandr with R9 290X and 7870 for triple head

[bugzilla-dae...@freedesktop.org]

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

--- Comment #1 from Maik Broemme  ---
Created attachment 97832
  --> https://bugs.freedesktop.org/attachment.cgi?id=97832=edit
boot log with drm:radeon_atom_get_leakage_vddc_based_on_leakage_params error

-- 
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/20140423/7038db9b/attachment.html>

[Bug 77835] New: X crash when using xrandr with R9 290X and 7870 for triple head

[bugzilla-dae...@freedesktop.org]

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

  Priority: medium
Bug ID: 77835
  Assignee: dri-devel at lists.freedesktop.org
   Summary: X crash when using xrandr with R9 290X and 7870 for
triple head
  Severity: major
Classification: Unclassified
OS: Linux (All)
  Reporter: mbroemme at libmpq.org
  Hardware: x86-64 (AMD64)
Status: NEW
   Version: unspecified
 Component: DRM/Radeon
   Product: DRI

Created attachment 97831
  --> https://bugs.freedesktop.org/attachment.cgi?id=97831=edit
Xorg log of crash

I'm trying to get a system with a R9 290X and a HD 7870 GHz Edition working. If
I use the cards independently everything works fine under X (the 7870 works
with glamor acceleration and the R9 290X without acceleration) but the problem
is to get them working together with single X. Attached is my Xorg.0.log and
the dmesg output. I'm using the following cards:

01:00.0 VGA compatible controller: Advanced Micro Devices, Inc. [AMD/ATI]
Hawaii XT [Radeon HD 8970]
04:00.0 VGA compatible controller: Advanced Micro Devices, Inc. [AMD/ATI]
Pitcairn XT [Radeon HD 7870 GHz Edition]

Steps to reproduce this issue:

#1 startx
#2 xrandr --output DVI-0 --primary
#3 xrandr --output HDMI-1 --right-of DVI-0
#4 xrandr --setprovideroutputsource 1 0
#5 xrandr --output HDMI-1-0 --mode 1920x1080 --right-of HDMI-1

Right now X is crashed with the following:

(EE) 
(EE) Backtrace:
(EE) 0: /usr/bin/X (xorg_backtrace+0x48) [0x584b08]
(EE) 1: /usr/bin/X (0x40+0x1887f9) [0x5887f9]
(EE) 2: /usr/lib/libpthread.so.0 (0x7f2a182c8000+0xf880) [0x7f2a182d7880]
(EE) 
(EE) Segmentation fault at address 0x0
(EE) 
Fatal server error:
(EE) Caught signal 11 (Segmentation fault). Server aborting
(EE) 
(EE) 
Please consult the The X.Org Foundation support 
 at http://wiki.x.org
 for help. 
(EE) Please also check the log file at "/var/log/Xorg.0.log" for additional
information.
(EE) 
(II) AIGLX: Suspending AIGLX clients for VT switch
(EE) Server terminated with error (1). Closing log file.
XIO:  fatal IO error 11 (Resource temporarily unavailable) on X server ":0"
  after 4790 requests (4790 known processed) with 9 events remaining.
xinit: connection to X server lost

The system is running Arch Linux with the following configuration:

ati-dri 10.1.1
glamor-egl 0.6.0
libdrm 2.4.53
linux 3.14.1
linux-firmware 20140316.dec41bc
mesa 10.1.1
mesa-libgl 10.1.1
xf86-video-ati 7.3.0
xorg-server 1.15.1

Does anybody has a hint what I could try next? Also what is needed to get
acceleration working. I tried it with "NoAccel" "False" and "AccelMethod"
"glamor" but it just freezes my X on startup. :)

--Maik

-- 
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/20140423/a16254e8/attachment.html>

[Intel-gfx] [PATCH 1/3] drm: Check CRTC compatibility in setplane

[Daniel Vetter]

On Wed, Apr 23, 2014 at 10:03:59AM -0700, Matt Roper wrote:
> The DRM core setplane code should check that the plane is usable on the
> specified CRTC before calling into the driver.
> 
> Prior to this patch, a plane's possible_crtcs field was purely
> informational for userspace and was never actually verified at the
> kernel level (aside from the primary plane helper).
> 
> Cc: dri-devel at lists.freedesktop.org
> Signed-off-by: Matt Roper 

Do you have a nasty igt somewhere which tries to use a plane on the wrong
crtc? Especially useful since our i915 code and hw relies on this.
-Daniel

> ---
>  drivers/gpu/drm/drm_crtc.c | 7 +++
>  drivers/gpu/drm/drm_plane_helper.c | 6 --
>  2 files changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 461d19b..b6d6c04 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -2140,6 +2140,13 @@ int drm_mode_setplane(struct drm_device *dev, void 
> *data,
>   }
>   crtc = obj_to_crtc(obj);
>  
> + /* Check whether this plane is usable on this CRTC */
> + if (!(plane->possible_crtcs & drm_crtc_mask(crtc))) {
> + DRM_DEBUG_KMS("Invalid crtc for plane\n");
> + ret = -EINVAL;
> + goto out;
> + }
> +
>   fb = drm_framebuffer_lookup(dev, plane_req->fb_id);
>   if (!fb) {
>   DRM_DEBUG_KMS("Unknown framebuffer ID %d\n",
> diff --git a/drivers/gpu/drm/drm_plane_helper.c 
> b/drivers/gpu/drm/drm_plane_helper.c
> index b72736d..65c4a00 100644
> --- a/drivers/gpu/drm/drm_plane_helper.c
> +++ b/drivers/gpu/drm/drm_plane_helper.c
> @@ -137,12 +137,6 @@ int drm_primary_helper_update(struct drm_plane *plane, 
> struct drm_crtc *crtc,
>   return -EINVAL;
>   }
>  
> - /* Primary planes are locked to their owning CRTC */
> - if (plane->possible_crtcs != drm_crtc_mask(crtc)) {
> - DRM_DEBUG_KMS("Cannot change primary plane CRTC\n");
> - return -EINVAL;
> - }
> -
>   /* Disallow scaling */
>   src_w >>= 16;
>   src_h >>= 16;
> -- 
> 1.8.5.1
> 
> ___
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH RFC 3/3] drm/exynos: use pending_components for components tracking

[Russell King - ARM Linux]

On Wed, Apr 23, 2014 at 05:43:28PM +0100, Russell King - ARM Linux wrote:
> So, maybe you would like to finally address *my* point about TDA998x
> and your solution in a way that provides a satisfactory answer.  *Show*
> how it can be done, or *outline* how it can be done.

Let me be absolutely clear *why* I'm very interested in this - and that
is because I'm presently converting TDA998x and Armada DRM to use the
component helpers.  If your solution is better, then I'd want to convert
to that instead, and let's retire the component helpers.

At the moment, my belief is that your solution is *very* substandard and
suboptimal precisely for the reasons I've outlined, especially when it
comes to sharing a component between several drivers.

So, if you *really* care, you'll stop fobbing me off on this point and
provide some real technical input how you'd see your solution being used
in exactly the scenario that I've been outlining several times in this
thread.

For example, you could show what kind of modifications you expect would
be required to the _existing_ TDA998x driver to allow it to participate
as a device declared in DT as an entirely separate entity, probed via the
standard I2C probing methods, and then hook itself into the appropriate
DRM driver.  Remembering, of course, that the TDA998x device is used by
more than _just_ Armada DRM.

I don't care if you show it via pseudo-code or by real patch.  I just
want to know _how_ your solution could be used.  And I won't want some
silly remark like "trivially" or "I've already answered that."  I want
_you_ to _show_ _how_ it can be done.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

[git pull] drm fixes

[Ed Tomlinson]

On Wednesday 23 April 2014 07:54:17 Dave Airlie wrote:
> On Wed, Apr 23, 2014 at 1:59 AM, Linus Torvalds
>  wrote:
> > Dave, mind sending me a pull request for drm fixes?
> >
> > There's now at least these two:
> >
> >  - "drm/radeon/aux: fix hpd assignment for aux bus"
> >  - "drm/radeon: use fixed PPL ref divider if needed"
> >
> > that look like fairly fatal regressions when they affect somebody.
> >
> > The fact that we already had *two* independent bugs be reported within
> > days of that last out-of-merge-window pull request makes me very
> > unhappy with the state of drm pulls.
> >
> > So please make sure that future fixes really are *fixes*. For
> > regressions only. No more games like this.
> 
> The pll fallout is fixes for the initial feature that was in the merge window,
> Tuning plls for monitors is always a pain in the ass, the previous algorithm
> took a couple of kernels a few years back to get where it was, unfortunately
> HDMI came along and showed up a bunch of its shortcomings. I'm happy
> Alex and Christian are on top of things in terms of tracking regressions
> and making sure they get fixed,
> 
> the AUX fix yes I'm a bit pissed off about myself, but I missed a pull
> from a few
> weeks ago, felt guilty, and maybe should have chosen the other path and let it
> wait a merge,
> 
> Christian just sent me a -fixes pull with all of these in it and I'll
> send it on to you
> in a few mins.

Hi

Given the fun I had with rc1 I decided to try this pull before rc2 and its 
working fine here.

Thanks!

Ed Tomlinson

[PATCH RFC 3/3] drm/exynos: use pending_components for components tracking

[Russell King - ARM Linux]

On Wed, Apr 23, 2014 at 05:04:46PM +0200, Andrzej Hajda wrote:
> On 04/22/2014 01:51 PM, Russell King - ARM Linux wrote:
> > Yes, I know that you're desperate to play that down, but you can't get
> 
> Not true. I only try to find the best solution and the approach with
> multiple re-probing just to avoid potential bugs in drivers does not
> look to me correct.
> 
> > away from this fact: your approach _forces_ a split up of the
> > initialisation into dependent two stages and that fact _alone_ adds
> > additional complexity, and along with that additional complexity comes
> > more opportunity for bugs. 
> 
> This sound so funny, just look at componentize patches - every patch
> adds two stage initialization for every component and the master,
> with forced unwinding and two levels of devres management.

*Sigh*.  Why am I bothering discussing this with you.

*NO* it does not, for the VERY SIMPLE reason that NOTHING is done before
the BIND.  NO structures are allocated.  NOTHING is setup.  The *only*
thing that is done is the driver registers with the component helper.

That's not two stage initialisation.  That's *single* stage.

> 'My approach' adds only one call to probe and one call to remove of
> components, and very simple and straightforward interface to the master.

You're talking utter garbage there.

> 'My approach' is very standard - during probe driver probes hardware,
> and registers interfaces which can be used by other drivers, for example
> by drm master. The only addition is reporting its readiness. Comparing to
> 'your approach' it is bloody simple.

More unbelievable crap.

> >  Also with that additional complexity comes
> > the need to perform more tests to find those bugs, and given that most
> > people just say "okay, it boots and seems to work, that's good enough
> > for me" there is a high possibility that these kinds of bugs will take
> > a long time to find.
> 
> Volume of changes for each component and drm device management
> dispersed on all components makes your argument very valid for
> component subsystem.
> 
> Btw have you observed component framework when one of the components
> during bind returns -EPROBE_DEFER ? In my tests it resulted in
> deferred probing of master and unbind/bind of other components.
> So lets say you have N components and the last component will be deferred
> K times, it results in:
> - K times deferring of the last component and the master,
> - (N - 1) * K - unbinds and binds of other components.

True, and you can't get away from that with proper behaviour.

> >> As I wrote already, this framework was proposed for drivers which
> >> are tied together anyway, and this is case of many drivers, not
> >> only exynos.
> > Please name them.

You ignored this.  Therefore, I assume that you *can't* name them because
there *aren't* any.  I called your bluff, I win.

> > At the moment, I don't see a justification for your "simplified"
> > and restrictive solution, which if used will lock drivers into that
> > simplisitic method, and which can't ever be extended without lots of
> > ifdeffery to having other components (such as TDA998x) attached.
> >
> > My objections are entirely based on where imx-drm and armada DRM are
> > going, neither of which could ever use your implementation.
> >
> > Before you say that it isn't meant to deal with stuff like the TDA998x,
> > take a moment to think about this - the Dove video subsystem was
> > designed to support OLPC.  It was primerly designed to drive a LCD
> > screen plus an on-board VGA DAC.  Everything for that is on-SoC.  With
> > that, the hardware is well known, and your solution could be used.
> >
> > However, then SolidRun came along and dropped a TDA998x on the LCD output
> > pins.  Suddenly, things aren't that simple, and your solution falls
> > apart, because it can't cope with a generic component that has no knowledge
> > of the rest of its "master".
> >
> > This kind of scenario can happen /any/ time, and any time it does happen,
> > your simple solution falls apart.
> 
> I think I have answered you two or three times that it is not a problem
> to remove
> 'glued drivers' restriction. I desperately try to avoid accusing you for
> 'desperately
> playing down' on this subject, so I will not comment this anymore.

Right, so what I draw from this is that *you* again refuse to answer this
point because despite your assertions that your solution can do it, you
have no clue as to *how* it can be done.  I've looked at your solution
with respect to this, and I *can't* see how it can be done either.  That's
why I've been asking *you* the question, so that *you* can provide some
technical input to it.

> On the other hand you have not answered quite important question - how
> do you plan to componentize drivers shared by different drms when
> one of drms is not componentized???

Read this, from a message I sent at the beginning of February:
| Here's my changes to the TDA998x driver to add support for the component
| 

[PATCH] drm: Simplify fb refcounting rules around ->update_plane

[Daniel Vetter]

The introduction of primary planes has apparently caused a bit of fb
refcounting fun for people. That makes it a good time to clean up the
arcane rules and slight differences between ->update_plane and
->set_config. The new rules are:

- The core holds a reference for both the new and the old fb (if
  they're non-NULL of course) while calling into the driver through
  either ->update_plane or ->set_config.

- Drivers may not clobber plane->fb if their callback fails. If they
  do that, they need to store a pointer to the old fb in it again.
  When calling into the driver plane->fb still points at the current
  (old) framebuffer.

- The core will update the plane->fb pointer on success. Drivers can
  do that themselves too, but aren't required to any more for the
  primary plane.

- The core will update fb refcounts for the plane->fb pointer,
  presuming the drivers hold up their end of the bargain.

v2: Remove now unused tmpfb (Thierry)

v3: Drop broken changes from drm_mode_setplane (Ville). Also polish
the commit message a bit.

v4: Also fix up the handling of ->disable_plane in
drm_plane_force_disable. The issue was that we didn't save plane->fb
over the ->disable_plane call. Just paranoia, nothing relies on this.

v5: Keep still useful comments about directly calling ->set_config,
which I should have done for v4 already. Requested by Matt.

Cc: Thierry Reding 
Cc: Ville Syrj?l? 
Cc: Matt Roper 
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/drm_crtc.c | 13 +++--
 drivers/gpu/drm/drm_plane_helper.c | 10 +-
 2 files changed, 8 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index d8b7099abece..f6633cb927bc 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -1145,16 +1145,17 @@ EXPORT_SYMBOL(drm_plane_cleanup);
  */
 void drm_plane_force_disable(struct drm_plane *plane)
 {
+   struct drm_framebuffer *old_fb = plane->fb;
int ret;

-   if (!plane->fb)
+   if (!old_fb)
return;

ret = plane->funcs->disable_plane(plane);
if (ret)
DRM_ERROR("failed to disable plane with busy fb\n");
/* disconnect the plane from the fb and crtc: */
-   __drm_framebuffer_unreference(plane->fb);
+   __drm_framebuffer_unreference(old_fb);
plane->fb = NULL;
plane->crtc = NULL;
 }
@@ -2187,16 +2188,18 @@ int drm_mode_setplane(struct drm_device *dev, void 
*data,
}

drm_modeset_lock_all(dev);
+   old_fb = plane->fb;
ret = plane->funcs->update_plane(plane, crtc, fb,
 plane_req->crtc_x, plane_req->crtc_y,
 plane_req->crtc_w, plane_req->crtc_h,
 plane_req->src_x, plane_req->src_y,
 plane_req->src_w, plane_req->src_h);
if (!ret) {
-   old_fb = plane->fb;
plane->crtc = crtc;
plane->fb = fb;
fb = NULL;
+   } else {
+   old_fb = NULL;
}
drm_modeset_unlock_all(dev);

@@ -2239,9 +2242,7 @@ int drm_mode_set_config_internal(struct drm_mode_set *set)
ret = crtc->funcs->set_config(set);
if (ret == 0) {
crtc->primary->crtc = crtc;
-
-   /* crtc->fb must be updated by ->set_config, enforces this. */
-   WARN_ON(fb != crtc->primary->fb);
+   crtc->primary->fb = fb;
}

list_for_each_entry(tmp, >dev->mode_config.crtc_list, head) {
diff --git a/drivers/gpu/drm/drm_plane_helper.c 
b/drivers/gpu/drm/drm_plane_helper.c
index 9540ff9f97fe..d966afa7ecae 100644
--- a/drivers/gpu/drm/drm_plane_helper.c
+++ b/drivers/gpu/drm/drm_plane_helper.c
@@ -124,7 +124,6 @@ int drm_primary_helper_update(struct drm_plane *plane, 
struct drm_crtc *crtc,
.y2 = crtc->mode.vdisplay,
};
struct drm_connector **connector_list;
-   struct drm_framebuffer *tmpfb;
int num_connectors, ret;

if (!crtc->enabled) {
@@ -178,21 +177,14 @@ int drm_primary_helper_update(struct drm_plane *plane, 
struct drm_crtc *crtc,
set.num_connectors = num_connectors;

/*
-* set_config() adjusts crtc->primary->fb; however the DRM setplane
-* code that called us expects to handle the framebuffer update and
-* reference counting; save and restore the current fb before
-* calling it.
-*
-* N.B., we call set_config() directly here rather than using
+* We call set_config() directly here rather than using
 * drm_mode_set_config_internal.  We're reprogramming the same
 * connectors that were already in use, so we shouldn't need the extra
 * cross-CRTC fb refcounting to accomodate stealing connectors.
 * drm_mode_setplane() already handles the basic refcounting for the
 * 

[RFC 0/3] DRM driver for the ATMEL High end LCD controller

[Tim Niemeyer]

Hi Jean-Jacques,

Am Freitag, den 18.04.2014, 11:45 +0200 schrieb Jean-Jacques Hiblot:
> Hi,
> 
> this patch serie implements a simple DRM driver for the ATMEL High end LCD
> controller found in the SAMA5 familly. It's based on the tilcdc driver.
> It uses the cma_helper for memory and fbdev stuff.
> Your comments are welcome !
I applied your and Robert's patches on a 3.15-rc2 kernel and tried the
framebuffer device on the sama5d31-ek. Seems to work.

> Supported features:
> * the base layer (the main framebuffer)
Tested to show a little tux png, but it's appearing only after 
'echo 0 > /sys/devices/ahb.1/apb.2/f003.hlcdc/graphics/fb0/rotate'
is there missing some initializing?

> * a simple panel
> * a backlight driver
Tested to set brightness to 0, 20 and 255. Works.

Thanks for your work.

Best Regards
Tim

> * structure to 'easily' add other connectors (it comes from the tilcdc)
> 
> On the todo list:
> * support overlays as drm_planes
> * support for the hardware cursor
> * support for the SiI9022 HDMI connector (present on sama5d36ek)
> 
> 
> Jean-Jacques Hiblot (3):
>   atmel: drm: added drm driver for the atmel hlcd controller
>   atmel: drm: dt: Added DT entry for the atmel hlcdc found in the sama5
>   atmel: dt: Add supports for the lcdc support on the sama5d36ek
> 
>  arch/arm/boot/dts/sama5d36ek.dts   |  27 +-
>  arch/arm/boot/dts/sama5d3_lcd.dtsi |  11 +
>  drivers/gpu/drm/Kconfig|   2 +
>  drivers/gpu/drm/Makefile   |   1 +
>  drivers/gpu/drm/atmel_hlcdc/Kconfig|  13 +
>  drivers/gpu/drm/atmel_hlcdc/Makefile   |  12 +
>  drivers/gpu/drm/atmel_hlcdc/atmel_hlcdc.h  | 771 
> +
>  .../gpu/drm/atmel_hlcdc/atmel_hlcdc_backlight.c|  92 +++
>  .../gpu/drm/atmel_hlcdc/atmel_hlcdc_backlight.h|  25 +
>  drivers/gpu/drm/atmel_hlcdc/atmel_hlcdc_crtc.c | 702 +++
>  drivers/gpu/drm/atmel_hlcdc/atmel_hlcdc_drv.c  | 586 
>  drivers/gpu/drm/atmel_hlcdc/atmel_hlcdc_drv.h  | 124 
>  drivers/gpu/drm/atmel_hlcdc/atmel_hlcdc_ovl.h  | 190 +
>  drivers/gpu/drm/atmel_hlcdc/atmel_hlcdc_panel.c| 459 
>  drivers/gpu/drm/atmel_hlcdc/atmel_hlcdc_panel.h|  28 +
>  15 files changed, 3042 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/atmel_hlcdc/Kconfig
>  create mode 100644 drivers/gpu/drm/atmel_hlcdc/Makefile
>  create mode 100644 drivers/gpu/drm/atmel_hlcdc/atmel_hlcdc.h
>  create mode 100644 drivers/gpu/drm/atmel_hlcdc/atmel_hlcdc_backlight.c
>  create mode 100644 drivers/gpu/drm/atmel_hlcdc/atmel_hlcdc_backlight.h
>  create mode 100644 drivers/gpu/drm/atmel_hlcdc/atmel_hlcdc_crtc.c
>  create mode 100644 drivers/gpu/drm/atmel_hlcdc/atmel_hlcdc_drv.c
>  create mode 100644 drivers/gpu/drm/atmel_hlcdc/atmel_hlcdc_drv.h
>  create mode 100644 drivers/gpu/drm/atmel_hlcdc/atmel_hlcdc_ovl.h
>  create mode 100644 drivers/gpu/drm/atmel_hlcdc/atmel_hlcdc_panel.c
>  create mode 100644 drivers/gpu/drm/atmel_hlcdc/atmel_hlcdc_panel.h
> 
> --
> 1.9.1
> 
> 
> ___
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[git pull] drm fixes

[Woody Suwalski]

Dave Airlie wrote:
> On Wed, Apr 23, 2014 at 1:59 AM, Linus Torvalds
>  wrote:
>> Dave, mind sending me a pull request for drm fixes?
>>
>> There's now at least these two:
>>
>>   - "drm/radeon/aux: fix hpd assignment for aux bus"
>>   - "drm/radeon: use fixed PPL ref divider if needed"
>>
>> that look like fairly fatal regressions when they affect somebody.
>>
>> The fact that we already had *two* independent bugs be reported within
>> days of that last out-of-merge-window pull request makes me very
>> unhappy with the state of drm pulls.
>>
>> So please make sure that future fixes really are *fixes*. For
>> regressions only. No more games like this.
> The pll fallout is fixes for the initial feature that was in the merge window,
> Tuning plls for monitors is always a pain in the ass, the previous algorithm
> took a couple of kernels a few years back to get where it was, unfortunately
> HDMI came along and showed up a bunch of its shortcomings. I'm happy
> Alex and Christian are on top of things in terms of tracking regressions
> and making sure they get fixed,
>
> the AUX fix yes I'm a bit pissed off about myself, but I missed a pull
> from a few
> weeks ago, felt guilty, and maybe should have chosen the other path and let it
> wait a merge,
>
> Christian just sent me a -fixes pull with all of these in it and I'll
> send it on to you
> in a few mins.
>
> Dave.
>
That patch also fixes a shimmering (water floating) issue on Radeon 
RV635 (Thinkpad T500)
1002:9591

[PATCH 1/2] drm: Simplify fb refcounting rules around ->update_plane

[Daniel Vetter]

On Wed, Apr 23, 2014 at 4:45 PM, Matt Roper  
wrote:
>> @@ -177,22 +176,7 @@ int drm_primary_helper_update(struct drm_plane *plane, 
>> struct drm_crtc *crtc,
>>   set.connectors = connector_list;
>>   set.num_connectors = num_connectors;
>>
>> - /*
>> -  * set_config() adjusts crtc->primary->fb; however the DRM setplane
>> -  * code that called us expects to handle the framebuffer update and
>> -  * reference counting; save and restore the current fb before
>> -  * calling it.
>> -  *
>> -  * N.B., we call set_config() directly here rather than using
>> -  * drm_mode_set_config_internal.  We're reprogramming the same
>> -  * connectors that were already in use, so we shouldn't need the extra
>> -  * cross-CRTC fb refcounting to accomodate stealing connectors.
>> -  * drm_mode_setplane() already handles the basic refcounting for the
>> -  * framebuffers involved in this operation.
>> -  */
>
> I think the second half of this comment (explaining why we call
> set_config() directly rather than calling
> drm_mode_set_config_internal()) still has some value.

Sorry I've forgotten to keep it when respinning the patch. I agree
with you. Will resend.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH] drm/dp: Add missing kernel-doc

[Jani Nikula]

On Wed, 23 Apr 2014, Thierry Reding  wrote:
> From: Thierry Reding 
>
> Commit 9dc4056026e0 (drm/dp: let drivers specify the name of the I2C-
> over-AUX adapter) introduced a new field but didn't add the proper
> kernel-doc for it.

Thanks for fixing this.

Reviewed-by: Jani Nikula 

> Signed-off-by: Thierry Reding 
> ---
>  include/drm/drm_dp_helper.h | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index cfcacec5b89d..f98cebf79738 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -431,6 +431,7 @@ struct drm_dp_aux_msg {
>  
>  /**
>   * struct drm_dp_aux - DisplayPort AUX channel
> + * @name: user-visible name of this AUX channel and the I2C-over-AUX adapter
>   * @ddc: I2C adapter that can be used for I2C-over-AUX communication
>   * @dev: pointer to struct device that is the parent for this AUX channel
>   * @transfer: transfers a message representing a single AUX transaction
> -- 
> 1.9.2
>
> ___
> 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

[PATCH RFC 3/3] drm/exynos: use pending_components for components tracking

[Andrzej Hajda]

On 04/22/2014 01:51 PM, Russell King - ARM Linux wrote:
> On Tue, Apr 22, 2014 at 01:29:54PM +0200, Andrzej Hajda wrote:
>> On 04/18/2014 02:46 PM, Russell King - ARM Linux wrote:
>>> On Fri, Apr 18, 2014 at 02:02:37PM +0200, Andrzej Hajda wrote:
 Separation of the interfaces exposed by the device from the device itself
 seems to me a good thing. I would even consider it as a biggest
 advantage of this solution :)

 The problem of re-initialization does not seems to be relevant here, it
 is classic
 problem of coding correctness nothing more, it can appear here and in
 many different
 places.
>>> It may be a problem of coding correctless, but it's also a maintainability
>>> problem too - it makes it _much_ more difficult to ensure that everything
>>> is correct.
>> But forcibly re-initializing all component devices instead of fixing bugs
>> in specific drivers seems to be 'absolutely absurd' as classic says.
> They're *unnecessary* bugs that wouldn't even exist if it weren't for
> the forced-splitup of the initialisation into two separate parts that
> your approach mandates.
>
> Yes, I know that you're desperate to play that down, but you can't get

Not true. I only try to find the best solution and the approach with
multiple re-probing just to avoid potential bugs in drivers does not
look to me correct.

> away from this fact: your approach _forces_ a split up of the
> initialisation into dependent two stages and that fact _alone_ adds
> additional complexity, and along with that additional complexity comes
> more opportunity for bugs. 

This sound so funny, just look at componentize patches - every patch
adds two stage initialization for every component and the master,
with forced unwinding and two levels of devres management.

'My approach' adds only one call to probe and one call to remove of
components,
and very simple and straightforward interface to the master.

'My approach' is very standard - during probe driver probes hardware,
and registers interfaces which can be used by other drivers, for example
by drm master. The only addition is reporting its readiness. Comparing to
'your approach' it is bloody simple.


>  Also with that additional complexity comes
> the need to perform more tests to find those bugs, and given that most
> people just say "okay, it boots and seems to work, that's good enough
> for me" there is a high possibility that these kinds of bugs will take
> a long time to find.

Volume of changes for each component and drm device management
dispersed on all components makes your argument very valid for
component subsystem.

Btw have you observed component framework when one of the components
during bind returns -EPROBE_DEFER ? In my tests it resulted in
deferred probing of master and unbind/bind of other components.
So lets say you have N components and the last component will be deferred
K times, it results in:
- K times deferring of the last component and the master,
- (N - 1) * K - unbinds and binds of other components.


>
>> As I wrote already, this framework was proposed for drivers which
>> are tied together anyway, and this is case of many drivers, not
>> only exynos.
> Please name them.
>
>> Standalone drivers were not at my sight but I have also described in
>> other mail how the framework can be 'improved' to support standalone
>> drivers also.
> At the moment, I don't see a justification for your "simplified"
> and restrictive solution, which if used will lock drivers into that
> simplisitic method, and which can't ever be extended without lots of
> ifdeffery to having other components (such as TDA998x) attached.
>
> My objections are entirely based on where imx-drm and armada DRM are
> going, neither of which could ever use your implementation.
>
> Before you say that it isn't meant to deal with stuff like the TDA998x,
> take a moment to think about this - the Dove video subsystem was
> designed to support OLPC.  It was primerly designed to drive a LCD
> screen plus an on-board VGA DAC.  Everything for that is on-SoC.  With
> that, the hardware is well known, and your solution could be used.
>
> However, then SolidRun came along and dropped a TDA998x on the LCD output
> pins.  Suddenly, things aren't that simple, and your solution falls
> apart, because it can't cope with a generic component that has no knowledge
> of the rest of its "master".
>
> This kind of scenario can happen /any/ time, and any time it does happen,
> your simple solution falls apart.

I think I have answered you two or three times that it is not a problem
to remove
'glued drivers' restriction. I desperately try to avoid accusing you for
'desperately
playing down' on this subject, so I will not comment this anymore.

On the other hand you have not answered quite important question - how
do you plan to componentize drivers shared by different drms when
one of drms is not componentized???


Regards
Andrzej

[PATCH] drm: Introduce drm_fb_helper_prepare()

[Daniel Vetter]

On Wed, Apr 23, 2014 at 03:40:46PM +0200, Thierry Reding wrote:
> From: Thierry Reding 
> 
> To implement hotplug detection in a race-free manner, drivers must call
> drm_kms_helper_poll_init() before hotplug events can be triggered. Such
> events can be triggered right after any of the encoders or connectors
> are initialized. At the same time, if the drm_fb_helper_hotplug_event()
> helper is used by a driver, then the poll helper requires some parts of
> the FB helper to be initialized to prevent a crash.
> 
> At the same time, drm_fb_helper_init() requires information that is not
> necessarily available at such an early stage (number of CRTCs and
> connectors), so it cannot be used yet.
> 
> Add a new helper, drm_fb_helper_prepare(), that initializes the bare
> minimum needed to allow drm_kms_helper_poll_init() to execute and any
> subsequent hotplug events to be processed properly.
> 
> Signed-off-by: Thierry Reding 
> ---
> Changes in v2:
> - improve kernel-doc (Daniel Vetter)
> 
>  drivers/gpu/drm/armada/armada_fbdev.c |  2 +-
>  drivers/gpu/drm/ast/ast_fb.c  |  4 ++-
>  drivers/gpu/drm/bochs/bochs_fbdev.c   |  3 +-
>  drivers/gpu/drm/cirrus/cirrus_fbdev.c |  4 ++-
>  drivers/gpu/drm/drm_fb_cma_helper.c   |  3 +-
>  drivers/gpu/drm/drm_fb_helper.c   | 47 
> ---
>  drivers/gpu/drm/exynos/exynos_drm_fbdev.c |  3 +-
>  drivers/gpu/drm/gma500/framebuffer.c  |  3 +-
>  drivers/gpu/drm/i915/intel_fbdev.c|  3 +-
>  drivers/gpu/drm/mgag200/mgag200_fb.c  |  3 +-
>  drivers/gpu/drm/msm/msm_fbdev.c   |  2 +-
>  drivers/gpu/drm/nouveau/nouveau_fbcon.c   |  3 +-
>  drivers/gpu/drm/omapdrm/omap_fbdev.c  |  2 +-
>  drivers/gpu/drm/qxl/qxl_fb.c  |  5 +++-
>  drivers/gpu/drm/radeon/radeon_fb.c|  4 ++-
>  drivers/gpu/drm/tegra/fb.c|  4 +--
>  drivers/gpu/drm/udl/udl_fb.c  |  3 +-
>  include/drm/drm_fb_helper.h   |  2 ++
>  18 files changed, 72 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/gpu/drm/armada/armada_fbdev.c 
> b/drivers/gpu/drm/armada/armada_fbdev.c
> index 21aa1261dba2..9437e11d5df1 100644
> --- a/drivers/gpu/drm/armada/armada_fbdev.c
> +++ b/drivers/gpu/drm/armada/armada_fbdev.c
> @@ -149,7 +149,7 @@ int armada_fbdev_init(struct drm_device *dev)
>  
>   priv->fbdev = fbh;
>  
> - fbh->funcs = _fb_helper_funcs;
> + drm_fb_helper_prepare(dev, fbh, _fb_helper_funcs);
>  
>   ret = drm_fb_helper_init(dev, fbh, 1, 1);
>   if (ret) {
> diff --git a/drivers/gpu/drm/ast/ast_fb.c b/drivers/gpu/drm/ast/ast_fb.c
> index 2113894e4ff8..cba45c774552 100644
> --- a/drivers/gpu/drm/ast/ast_fb.c
> +++ b/drivers/gpu/drm/ast/ast_fb.c
> @@ -328,8 +328,10 @@ int ast_fbdev_init(struct drm_device *dev)
>   return -ENOMEM;
>  
>   ast->fbdev = afbdev;
> - afbdev->helper.funcs = _fb_helper_funcs;
>   spin_lock_init(>dirty_lock);
> +
> + drm_fb_helper_prepare(dev, >helper, _fb_helper_funcs);
> +
>   ret = drm_fb_helper_init(dev, >helper,
>1, 1);
>   if (ret) {
> diff --git a/drivers/gpu/drm/bochs/bochs_fbdev.c 
> b/drivers/gpu/drm/bochs/bochs_fbdev.c
> index 17e5c17f2730..19cf3e9413b6 100644
> --- a/drivers/gpu/drm/bochs/bochs_fbdev.c
> +++ b/drivers/gpu/drm/bochs/bochs_fbdev.c
> @@ -189,7 +189,8 @@ int bochs_fbdev_init(struct bochs_device *bochs)
>  {
>   int ret;
>  
> - bochs->fb.helper.funcs = _fb_helper_funcs;
> + drm_fb_helper_prepare(bochs->dev, >fb.helper,
> +   _fb_helper_funcs);
>  
>   ret = drm_fb_helper_init(bochs->dev, >fb.helper,
>1, 1);
> diff --git a/drivers/gpu/drm/cirrus/cirrus_fbdev.c 
> b/drivers/gpu/drm/cirrus/cirrus_fbdev.c
> index 2bd0291168e4..2a135f253e29 100644
> --- a/drivers/gpu/drm/cirrus/cirrus_fbdev.c
> +++ b/drivers/gpu/drm/cirrus/cirrus_fbdev.c
> @@ -306,9 +306,11 @@ int cirrus_fbdev_init(struct cirrus_device *cdev)
>   return -ENOMEM;
>  
>   cdev->mode_info.gfbdev = gfbdev;
> - gfbdev->helper.funcs = _fb_helper_funcs;
>   spin_lock_init(>dirty_lock);
>  
> + drm_fb_helper_prepare(cdev->dev, >helper,
> +   _fb_helper_funcs);
> +
>   ret = drm_fb_helper_init(cdev->dev, >helper,
>cdev->num_crtc, CIRRUSFB_CONN_LIMIT);
>   if (ret) {
> diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c 
> b/drivers/gpu/drm/drm_fb_cma_helper.c
> index b74f9e58b69d..acbbd230e081 100644
> --- a/drivers/gpu/drm/drm_fb_cma_helper.c
> +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
> @@ -354,9 +354,10 @@ struct drm_fbdev_cma *drm_fbdev_cma_init(struct 
> drm_device *dev,
>   return ERR_PTR(-ENOMEM);
>   }
>  
> - fbdev_cma->fb_helper.funcs = _fb_cma_helper_funcs;
>   helper = _cma->fb_helper;
>  
> + drm_fb_helper_prepare(dev, helper, _fb_cma_helper_funcs);
> +
>   ret = 

[Bug 77785] (radeonsi) Some lighting issues in games, textures goes black

[bugzilla-dae...@freedesktop.org]

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

--- Comment #8 from smoki  ---

 Don't have a time for this right now, but better this with turn on/off
headlights i think clearly show behavior, hope it is reproducible :).

 https://dl.dropboxusercontent.com/u/74553632/pttm_trace2.7z

-- 
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/20140423/59e30fdd/attachment.html>

[PATCH] drm/dp: Add missing kernel-doc

[Thierry Reding]

From: Thierry Reding 

Commit 9dc4056026e0 (drm/dp: let drivers specify the name of the I2C-
over-AUX adapter) introduced a new field but didn't add the proper
kernel-doc for it.

Signed-off-by: Thierry Reding 
---
 include/drm/drm_dp_helper.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index cfcacec5b89d..f98cebf79738 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -431,6 +431,7 @@ struct drm_dp_aux_msg {

 /**
  * struct drm_dp_aux - DisplayPort AUX channel
+ * @name: user-visible name of this AUX channel and the I2C-over-AUX adapter
  * @ddc: I2C adapter that can be used for I2C-over-AUX communication
  * @dev: pointer to struct device that is the parent for this AUX channel
  * @transfer: transfers a message representing a single AUX transaction
-- 
1.9.2

[Bug 76564] [AMD Fusion E-350] HDMI refresh rates doesn't match expectations

[bugzilla-dae...@freedesktop.org]

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

--- Comment #65 from Christian K?nig  ---
(In reply to comment #64)
> I will see if I can create a patch for OpenElec to pull in all the changes
> made to drm/radeon and test it

Peter already did so, you should contact him to get the merged patch.

-- 
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/20140423/6bf80fd7/attachment.html>

[PATCH 3/4] drm: Introduce drm_fb_helper_prepare()

[Thierry Reding]

On Wed, Apr 23, 2014 at 09:35:48AM +0200, Daniel Vetter wrote:
> On Wed, Apr 23, 2014 at 09:14:41AM +0200, Thierry Reding wrote:
> > On Tue, Apr 22, 2014 at 05:54:06PM +0200, Daniel Vetter wrote:
> > > On Tue, Apr 22, 2014 at 04:42:20PM +0200, Thierry Reding wrote:
> > [...]
> > > > diff --git a/drivers/gpu/drm/drm_fb_helper.c 
> > > > b/drivers/gpu/drm/drm_fb_helper.c
> > [...]
> > > > @@ -502,6 +503,33 @@ static void drm_fb_helper_crtc_free(struct 
> > > > drm_fb_helper *helper)
> > > >  }
> > > >  
> > > >  /**
> > > > + * drm_fb_helper_prepare - setup a drm_fb_helper structure
> > > > + * @dev: DRM device
> > > > + * @helper: driver-allocated fbdev helper structure to set up
> > > > + * @funcs: pointer to structure of functions associate with this helper
> > > > + *
> > > > + * Sets up the bare minimum to make the framebuffer helper usable. 
> > > > This is
> > > > + * useful to implement race-free initialization of the polling 
> > > > helpers. In
> > > > + * that case a typical sequence would be:
> > > > + *
> > > > + *   - call drm_fb_helper_prepare()
> > > > + *   - set dev->mode_config.funcs
> > > 
> > > This step is done in drm_fb_helper_prepare already.
> > 
> > drm_fb_helper_prepare() sets fb_helper->funcs. dev->mode_config.funcs
> > needs to be set because it gets called by drm_kms_helper_hotplug_event()
> > which in turn is called by output_poll_execute(), which can be called at
> > any point after drm_kms_helper_poll_init(). It could be scheduled
> > immediately by drm_kms_helper_poll_enable().
> > 
> > I wonder if perhaps we should be wrapping that into a function as well.
> > Currently this is only documented in code by the drivers that call this.
> > But since it's only a single step it doesn't seem worth it. Perhaps if
> > we rolled the min/max_width/height into that function as well it would
> > be more worth it? That could be difficult to do since a couple of
> > drivers need to set those depending on the chipset generation.
> 
> Oh I've misread this step for the fb_helper->funcs assignment. Makes sense
> and I don't think we need to augment your kerneldoc more to explain this.
> 
> > > > + *   - perform driver-specific setup
> 
> Hm, maybe clarify this as "initialize modeset objects like crtcs, encoders
> and connectors"? Since that's the important part we need to get done here.
> 
> > > > + *   - call drm_kms_helper_poll_init()
> > > 
> > > Maybe mention that after this you can start to handle hpd events using the
> > > probe helpers?
> > 
> > Isn't that somewhat implied already? The poll helpers call directly the
> > dev->mode_config.funcs->output_poll_changed() function, which has
> > already been set up earlier.
> 
> I've more meant that after this it's save for drivers to enable hpd
> interrupts and start to process them. I.e.
> 
>   - enable interrupts and start processing hpd events
> 
> as an additional step to make it really cleear how it all fits together.
> Otherwise driver authors are left wondering wtf this isn't just one
> function call to do it all for them ;-)
> 
> > > > + *   - call drm_fb_helper_init()
> > > > + *   - call drm_fb_helper_single_add_all_connectors()
> > > > + *   - call drm_fb_helper_initial_config()
> > > 
> > > Does this list look sane in the generated DocBook? Afaik kerneldoc
> > > unfortunately lacks any form of markup, at least afaik :(
> > 
> > I must admit I didn't check. I'll make sure I do that before sending out
> > the next version.
> 
> If it looks ugly I'm ok as-is, just wondered. Kerneldoc is a bit
> simplistic ime.

In the second version I just sent out I ended up moving the description
of this sequence into the fbdev helper section rather than keeping it in
the description of the drm_fb_helper_prepare() function, since the new
function is really just a part of the whole sequence, so it seemed to
fit more nicely. And I've dropped the list formatting and turned it into
prose instead.

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

[Bug 76564] [AMD Fusion E-350] HDMI refresh rates doesn't match expectations

[bugzilla-dae...@freedesktop.org]

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

--- Comment #64 from jeroen  ---
(In reply to comment #63)
> Please try
> http://cgit.freedesktop.org/~deathsimple/linux/log/?h=drm-next-3.16.
> 
> This branch might fix the remaining frame drop problems.

Hi Christian,

I see multiple commits, are you specifically referring to the page flip
commits?  Problems with the vblank handling could explain skipped frames in
XBMC as this indicates the render thread is late as explained by Rainer.

I will see if I can create a patch for OpenElec to pull in all the changes made
to drm/radeon and test it

-- 
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/20140423/034d0931/attachment-0001.html>

[PATCH] drm: Introduce drm_fb_helper_prepare()

[Thierry Reding]

From: Thierry Reding 

To implement hotplug detection in a race-free manner, drivers must call
drm_kms_helper_poll_init() before hotplug events can be triggered. Such
events can be triggered right after any of the encoders or connectors
are initialized. At the same time, if the drm_fb_helper_hotplug_event()
helper is used by a driver, then the poll helper requires some parts of
the FB helper to be initialized to prevent a crash.

At the same time, drm_fb_helper_init() requires information that is not
necessarily available at such an early stage (number of CRTCs and
connectors), so it cannot be used yet.

Add a new helper, drm_fb_helper_prepare(), that initializes the bare
minimum needed to allow drm_kms_helper_poll_init() to execute and any
subsequent hotplug events to be processed properly.

Signed-off-by: Thierry Reding 
---
Changes in v2:
- improve kernel-doc (Daniel Vetter)

 drivers/gpu/drm/armada/armada_fbdev.c |  2 +-
 drivers/gpu/drm/ast/ast_fb.c  |  4 ++-
 drivers/gpu/drm/bochs/bochs_fbdev.c   |  3 +-
 drivers/gpu/drm/cirrus/cirrus_fbdev.c |  4 ++-
 drivers/gpu/drm/drm_fb_cma_helper.c   |  3 +-
 drivers/gpu/drm/drm_fb_helper.c   | 47 ---
 drivers/gpu/drm/exynos/exynos_drm_fbdev.c |  3 +-
 drivers/gpu/drm/gma500/framebuffer.c  |  3 +-
 drivers/gpu/drm/i915/intel_fbdev.c|  3 +-
 drivers/gpu/drm/mgag200/mgag200_fb.c  |  3 +-
 drivers/gpu/drm/msm/msm_fbdev.c   |  2 +-
 drivers/gpu/drm/nouveau/nouveau_fbcon.c   |  3 +-
 drivers/gpu/drm/omapdrm/omap_fbdev.c  |  2 +-
 drivers/gpu/drm/qxl/qxl_fb.c  |  5 +++-
 drivers/gpu/drm/radeon/radeon_fb.c|  4 ++-
 drivers/gpu/drm/tegra/fb.c|  4 +--
 drivers/gpu/drm/udl/udl_fb.c  |  3 +-
 include/drm/drm_fb_helper.h   |  2 ++
 18 files changed, 72 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/armada/armada_fbdev.c 
b/drivers/gpu/drm/armada/armada_fbdev.c
index 21aa1261dba2..9437e11d5df1 100644
--- a/drivers/gpu/drm/armada/armada_fbdev.c
+++ b/drivers/gpu/drm/armada/armada_fbdev.c
@@ -149,7 +149,7 @@ int armada_fbdev_init(struct drm_device *dev)

priv->fbdev = fbh;

-   fbh->funcs = _fb_helper_funcs;
+   drm_fb_helper_prepare(dev, fbh, _fb_helper_funcs);

ret = drm_fb_helper_init(dev, fbh, 1, 1);
if (ret) {
diff --git a/drivers/gpu/drm/ast/ast_fb.c b/drivers/gpu/drm/ast/ast_fb.c
index 2113894e4ff8..cba45c774552 100644
--- a/drivers/gpu/drm/ast/ast_fb.c
+++ b/drivers/gpu/drm/ast/ast_fb.c
@@ -328,8 +328,10 @@ int ast_fbdev_init(struct drm_device *dev)
return -ENOMEM;

ast->fbdev = afbdev;
-   afbdev->helper.funcs = _fb_helper_funcs;
spin_lock_init(>dirty_lock);
+
+   drm_fb_helper_prepare(dev, >helper, _fb_helper_funcs);
+
ret = drm_fb_helper_init(dev, >helper,
 1, 1);
if (ret) {
diff --git a/drivers/gpu/drm/bochs/bochs_fbdev.c 
b/drivers/gpu/drm/bochs/bochs_fbdev.c
index 17e5c17f2730..19cf3e9413b6 100644
--- a/drivers/gpu/drm/bochs/bochs_fbdev.c
+++ b/drivers/gpu/drm/bochs/bochs_fbdev.c
@@ -189,7 +189,8 @@ int bochs_fbdev_init(struct bochs_device *bochs)
 {
int ret;

-   bochs->fb.helper.funcs = _fb_helper_funcs;
+   drm_fb_helper_prepare(bochs->dev, >fb.helper,
+ _fb_helper_funcs);

ret = drm_fb_helper_init(bochs->dev, >fb.helper,
 1, 1);
diff --git a/drivers/gpu/drm/cirrus/cirrus_fbdev.c 
b/drivers/gpu/drm/cirrus/cirrus_fbdev.c
index 2bd0291168e4..2a135f253e29 100644
--- a/drivers/gpu/drm/cirrus/cirrus_fbdev.c
+++ b/drivers/gpu/drm/cirrus/cirrus_fbdev.c
@@ -306,9 +306,11 @@ int cirrus_fbdev_init(struct cirrus_device *cdev)
return -ENOMEM;

cdev->mode_info.gfbdev = gfbdev;
-   gfbdev->helper.funcs = _fb_helper_funcs;
spin_lock_init(>dirty_lock);

+   drm_fb_helper_prepare(cdev->dev, >helper,
+ _fb_helper_funcs);
+
ret = drm_fb_helper_init(cdev->dev, >helper,
 cdev->num_crtc, CIRRUSFB_CONN_LIMIT);
if (ret) {
diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c 
b/drivers/gpu/drm/drm_fb_cma_helper.c
index b74f9e58b69d..acbbd230e081 100644
--- a/drivers/gpu/drm/drm_fb_cma_helper.c
+++ b/drivers/gpu/drm/drm_fb_cma_helper.c
@@ -354,9 +354,10 @@ struct drm_fbdev_cma *drm_fbdev_cma_init(struct drm_device 
*dev,
return ERR_PTR(-ENOMEM);
}

-   fbdev_cma->fb_helper.funcs = _fb_cma_helper_funcs;
helper = _cma->fb_helper;

+   drm_fb_helper_prepare(dev, helper, _fb_cma_helper_funcs);
+
ret = drm_fb_helper_init(dev, helper, num_crtc, max_conn_count);
if (ret < 0) {
dev_err(dev->dev, "Failed to initialize drm fb helper.\n");
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 

[RFC 4/5] drm: Introduce drm_set_unique()

[Thierry Reding]

On Wed, Apr 23, 2014 at 10:40:57AM +0200, Daniel Vetter wrote:
> On Wed, Apr 23, 2014 at 09:17:16AM +0200, Thierry Reding wrote:
> > On Tue, Apr 22, 2014 at 05:48:07PM +0200, Daniel Vetter wrote:
> > > On Tue, Apr 22, 2014 at 05:09:32PM +0200, Thierry Reding wrote:
> > > > From: Thierry Reding 
> > > > 
> > > > Add a helper function that allows drivers to statically set the unique
> > > > name of the device. This will allow platform and USB drivers to get rid
> > > > of their DRM bus implementations and directly use drm_dev_alloc() and
> > > > drm_dev_register().
> > > > 
> > > > Signed-off-by: Thierry Reding 
> > > > ---
> > > >  drivers/gpu/drm/drm_ioctl.c | 37 +++--
> > > >  drivers/gpu/drm/drm_stub.c  |  1 +
> > > >  include/drm/drmP.h  |  3 +++
> > > >  3 files changed, 35 insertions(+), 6 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> > > > index 2dd3a6d8382b..371db3bef60c 100644
> > > > --- a/drivers/gpu/drm/drm_ioctl.c
> > > > +++ b/drivers/gpu/drm/drm_ioctl.c
> > > > @@ -42,6 +42,20 @@
> > > >  #include 
> > > >  #endif
> > > >  
> > > > +int drm_set_unique(struct drm_device *dev, const char *fmt, ...)
> > > 
> > > Can you please add a bit of kerneldoc for this? drm_ioctl.c isn't yet
> > > pulled into the drm reference docbook, but better to have it there
> > > already.
> > 
> > On second thought, wouldn't this be better located in drm_stub.c? It
> > isn't really related to the IOCTL code except that one of the IOCTLs now
> > uses the information set by this function. Logically I think it belongs
> > with the likes of drm_dev_alloc() and drm_dev_register().
> 
> Yeah makes sense. Tbh the entire split-up of these core drm functions is
> still a bit messy, so I don't mind if it's a bit inconsistent really. We
> can do the suffling when someone bothers with the kerneldoc for all of
> them and pulls it into the drm docbook.

I ended up doing exactly that when I wrote the drm_set_unique() docbook
pieces and I've sent out patches based on top of v2 of this patch just
now.

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

[PATCH 2/2] drm: Document how to register devices without struct drm_bus

[Thierry Reding]

From: Thierry Reding 

With the recent addition of the drm_set_unique() function, devices can
now be registered without requiring a drm_bus. Add a brief description
to the DRM docbook to show how that can be achieved.

Signed-off-by: Thierry Reding 
---
 Documentation/DocBook/drm.tmpl | 29 +
 1 file changed, 29 insertions(+)

diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index ced203cef87d..e47b275170ac 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -142,6 +142,12 @@
   to register it with the DRM subsystem.
 
 
+  Newer drivers that no longer require a drm_bus
+  structure can alternatively use the low-level device initialization and
+  registration functions such as drm_dev_alloc() and
+  drm_dev_register() directly.
+
+
   The drm_driver structure contains static
   information that describes the driver and features it supports, and
   pointers to methods that the DRM core will call to implement the DRM API.
@@ -290,6 +296,29 @@ char *date;
 !Fdrivers/gpu/drm/drm_pci.c drm_pci_init drm_pci_exit
 !Fdrivers/gpu/drm/drm_usb.c drm_usb_init drm_usb_exit
 !Fdrivers/gpu/drm/drm_platform.c drm_platform_init
+  
+New drivers that no longer rely on the services provided by the
+drm_bus structure can call the low-level
+device registration functions directly. The
+drm_dev_alloc() function can be used to allocate
+and initialize a new drm_device structure.
+Drivers will typically want to perform some additional setup on this
+structure, such as allocating driver-specific data and storing a
+pointer to it in the DRM device's 
dev_private
+field. Drivers should also set the device's unique name using the
+drm_set_unique() function. After it has been set 
up
+a device can be registered with the DRM subsystem by calling
+drm_dev_register(). This will cause the device to
+be exposed to userspace and will call the driver's
+.load() implementation. When a device is
+removed, the DRM device can safely be unregistered and freed using a
+call to drm_put_dev().
+  
+!Fdrivers/gpu/drm/drm_stub.c drm_dev_alloc
+!Fdrivers/gpu/drm/drm_stub.c drm_set_unique
+!Fdrivers/gpu/drm/drm_stub.c drm_dev_register
+!Fdrivers/gpu/drm/drm_stub.c drm_put_dev
+
 
   Driver Load
   
-- 
1.9.2

[PATCH 1/2] drm: Add device registration documentation

[Thierry Reding]

From: Thierry Reding 

Describe how devices are registered using the drm_*_init() functions.
Adding this to docbook requires a largish set of changes to the comments
in drm_{pci,usb,platform}.c since they are doxygen-style rather than
proper kernel-doc and therefore mess with the docbook generation.

Signed-off-by: Thierry Reding 
---
 Documentation/DocBook/drm.tmpl |  9 +
 drivers/gpu/drm/drm_pci.c  | 80 --
 drivers/gpu/drm/drm_platform.c | 15 
 drivers/gpu/drm/drm_stub.c | 19 +-
 drivers/gpu/drm/drm_usb.c  | 20 ++-
 5 files changed, 81 insertions(+), 62 deletions(-)

diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index 83dd0b043c28..ced203cef87d 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -282,6 +282,15 @@ char *date;
   
 
 
+  Device Registration
+  
+A number of functions are provided to help with device registration.
+The functions deal with PCI, USB and platform devices, respectively.
+  
+!Fdrivers/gpu/drm/drm_pci.c drm_pci_init drm_pci_exit
+!Fdrivers/gpu/drm/drm_usb.c drm_usb_init drm_usb_exit
+!Fdrivers/gpu/drm/drm_platform.c drm_platform_init
+
   Driver Load
   
 The load method is the driver and device
diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
index 112203e7ae49..c2149f680c4b 100644
--- a/drivers/gpu/drm/drm_pci.c
+++ b/drivers/gpu/drm/drm_pci.c
@@ -1,17 +1,3 @@
-/* drm_pci.h -- PCI DMA memory management wrappers for DRM -*- linux-c -*- */
-/**
- * \file drm_pci.c
- * \brief Functions and ioctls to manage PCI memory
- *
- * \warning These interfaces aren't stable yet.
- *
- * \todo Implement the remaining ioctl's for the PCI pools.
- * \todo The wrappers here are so thin that they would be better off inlined..
- *
- * \author Jos? Fonseca 
- * \author Leif Delgass 
- */
-
 /*
  * Copyright 2003 Jos? Fonseca.
  * Copyright 2003 Leif Delgass.
@@ -42,12 +28,14 @@
 #include 
 #include 

-/**/
-/** \name PCI memory */
-/*@{*/
-
 /**
- * \brief Allocate a PCI consistent memory block, for DMA.
+ * drm_pci_alloc - Allocate a PCI consistent memory block, for DMA.
+ * @dev: DRM device
+ * @size: size of block to allocate
+ * @align: alignment of block
+ *
+ * Return: A handle to the allocated memory block on success or NULL on
+ * failure.
  */
 drm_dma_handle_t *drm_pci_alloc(struct drm_device * dev, size_t size, size_t 
align)
 {
@@ -88,8 +76,8 @@ drm_dma_handle_t *drm_pci_alloc(struct drm_device * dev, 
size_t size, size_t ali

 EXPORT_SYMBOL(drm_pci_alloc);

-/**
- * \brief Free a PCI consistent memory block without freeing its descriptor.
+/*
+ * Free a PCI consistent memory block without freeing its descriptor.
  *
  * This function is for internal use in the Linux-specific DRM core code.
  */
@@ -111,7 +99,9 @@ void __drm_pci_free(struct drm_device * dev, 
drm_dma_handle_t * dmah)
 }

 /**
- * \brief Free a PCI consistent memory block
+ * drm_pci_free - Free a PCI consistent memory block
+ * @dev: DRM device
+ * @dmah: handle to memory block
  */
 void drm_pci_free(struct drm_device * dev, drm_dma_handle_t * dmah)
 {
@@ -226,17 +216,16 @@ static int drm_pci_irq_by_busid(struct drm_device *dev, 
struct drm_irq_busid *p)
 }

 /**
- * Get interrupt from bus id.
- *
- * \param inode device inode.
- * \param file_priv DRM file private.
- * \param cmd command.
- * \param arg user argument, pointing to a drm_irq_busid structure.
- * \return zero on success or a negative number on failure.
+ * drm_irq_by_busid - Get interrupt from bus ID
+ * @dev: DRM device
+ * @data: IOCTL parameter pointing to a drm_irq_busid structure
+ * @file_priv: DRM file private.
  *
  * Finds the PCI device with the specified bus id and gets its IRQ number.
  * This IOCTL is deprecated, and will now return EINVAL for any busid not equal
  * to that of the device that this DRM instance attached to.
+ *
+ * Return: 0 on success or a negative error code on failure.
  */
 int drm_irq_by_busid(struct drm_device *dev, void *data,
 struct drm_file *file_priv)
@@ -286,15 +275,16 @@ static struct drm_bus drm_pci_bus = {
 };

 /**
- * Register.
- *
- * \param pdev - PCI device structure
- * \param ent entry from the PCI ID table with device type flags
- * \return zero on success or a negative number on failure.
+ * drm_get_pci_dev - Register a PCI device with the DRM subsystem
+ * @pdev: PCI device
+ * @ent: entry from the PCI ID table that matches @pdev
+ * @driver: DRM device driver
  *
  * Attempt to gets inter module "drm" information. If we are first
  * then register the character device and inter module information.
  * Try and register, if we fail to register, backout previous work.
+ *
+ * Return: 0 on success or a negative error code on failure.
  */
 int drm_get_pci_dev(struct pci_dev 

[PATCH v2] drm: Introduce drm_set_unique()

[Thierry Reding]

From: Thierry Reding 

Add a helper function that allows drivers to statically set the unique
name of the device. This will allow platform and USB drivers to get rid
of their DRM bus implementations and directly use drm_dev_alloc() and
drm_dev_register().

Signed-off-by: Thierry Reding 
---
Changes in v2:
- move drm_set_unique() to drivers/gpu/drm/drm_stub.c
- add kernel-doc

 drivers/gpu/drm/drm_ioctl.c | 23 +--
 drivers/gpu/drm/drm_stub.c  | 24 
 include/drm/drmP.h  |  3 +++
 3 files changed, 44 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 2dd3a6d8382b..1b082aeb4098 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -131,13 +131,24 @@ static int drm_set_busid(struct drm_device *dev, struct 
drm_file *file_priv)
if (master->unique != NULL)
drm_unset_busid(dev, master);

-   ret = dev->driver->bus->set_busid(dev, master);
-   if (ret)
-   goto err;
+   if (dev->driver->bus && dev->driver->bus->set_busid) {
+   ret = dev->driver->bus->set_busid(dev, master);
+   if (ret) {
+   drm_unset_busid(dev, master);
+   return ret;
+   }
+   } else {
+   WARN(dev->unique == NULL,
+"No drm_bus.set_busid() implementation provided by %ps. "
+"Set the unique name explicitly using drm_set_unique().",
+dev->driver);
+
+   master->unique = kstrdup(dev->unique, GFP_KERNEL);
+   if (master->unique)
+   master->unique_len = strlen(dev->unique);
+   }
+
return 0;
-err:
-   drm_unset_busid(dev, master);
-   return ret;
 }

 /**
diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
index 3a8e832ad151..d0c7c78aa7f4 100644
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -532,6 +532,29 @@ static void drm_fs_inode_free(struct inode *inode)
 }

 /**
+ * drm_set_unique - Set the unique name of a DRM device
+ * @dev: device of which to set the unique name
+ * @fmt: format string for unique name
+ *
+ * Sets the unique name of a DRM device using the specified format string and
+ * a variable list of arguments. Drivers can use this at driver probe time if
+ * the unique name of the devices they drive is static.
+ */
+int drm_set_unique(struct drm_device *dev, const char *fmt, ...)
+{
+   va_list ap;
+
+   kfree(dev->unique);
+
+   va_start(ap, fmt);
+   dev->unique = kvasprintf(GFP_KERNEL, fmt, ap);
+   va_end(ap);
+
+   return dev->unique ? 0 : -ENOMEM;
+}
+EXPORT_SYMBOL(drm_set_unique);
+
+/**
  * drm_dev_alloc - Allocate new drm device
  * @driver: DRM driver to allocate device for
  * @parent: Parent device object
@@ -646,6 +669,7 @@ static void drm_dev_release(struct kref *ref)
drm_minor_free(dev, DRM_MINOR_CONTROL);

mutex_destroy(>master_mutex);
+   kfree(dev->unique);
kfree(dev);
 }

diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 8c80c1894b41..d6da03c1ca3c 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1158,6 +1158,8 @@ struct drm_device {
struct drm_vma_offset_manager *vma_offset_manager;
/*@} */
int switch_power_state;
+
+   char *unique;
 };

 #define DRM_SWITCH_POWER_ON 0
@@ -1608,6 +1610,7 @@ static __inline__ void drm_core_dropmap(struct 
drm_local_map *map)

 #include 

+int drm_set_unique(struct drm_device *dev, const char *fmt, ...);
 struct drm_device *drm_dev_alloc(struct drm_driver *driver,
 struct device *parent);
 void drm_dev_ref(struct drm_device *dev);
-- 
1.9.2

[RFC v2 PATCH v4 09/14] ARM: dts: s6e3fa0: add DT bindings

[Andrzej Hajda]

On 04/23/2014 02:55 PM, Laurent Pinchart wrote:
> Hi Andrzej,
>
> On Wednesday 23 April 2014 14:48:31 Andrzej Hajda wrote:
>> On 04/23/2014 01:34 PM, Laurent Pinchart wrote:
>>> On Wednesday 23 April 2014 11:02:21 Andrzej Hajda wrote:
 On 04/21/2014 02:28 PM, YoungJun Cho wrote:
> This patch adds DT bindings for s6e3fa0 panel.
> The bindings describes panel resources, display timings and cpu timings.
>
> Changelog v2:
> - Adds unit address (commented by Sachin Kamat)
> Changelog v3:
> - Removes optional delay, size properties (commented by Laurent
> Pinchart)
> - Adds OLED detection, TE gpio properties
> Changelog v4:
> - Moves CPU timings relevant properties from FIMD DT
>
>   (commeted by Laurent Pinchart, Andrzej Hajda)
>
> Signed-off-by: YoungJun Cho 
> Acked-by: Inki Dae 
> Acked-by: Kyungmin Park 
> ---
>
>  .../devicetree/bindings/panel/samsung,s6e3fa0.txt  |   63
>  +++
>
> 1 file changed, 63 insertions(+)
>
>  create mode 100644
>  Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt>
>
> diff --git a/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt
> b/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt new file
> mode 100644
> index 000..9eeb38b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt
> @@ -0,0 +1,63 @@
> +Samsung S6E3FA0 AMOLED LCD 5.7 inch panel
> +
> +Required properties:
> +  - compatible: "samsung,s6e3fa0"
> +  - reg: the virtual channel number of a DSI peripheral
> +  - vdd3-supply: core voltage supply
> +  - vci-supply: voltage supply for analog circuits
> +  - reset-gpio: a GPIO spec for the reset pin
> +  - det-gpio: a GPIO spec for the OLED detection pin
> +  - te-gpio: a GPIO spec for the TE pin
> +  - display-timings: timings for the connected panel as described by
> [1]
 Which properties of display-timings are relevant for CPU mode?
 I guess width and height. Anything more?

> +  - cpu-timings: CPU interface timings for the connected panel, and it
> contains
> +following optional properties.
> +  - cs-setup: clock cycles for the active period of address
> signal
> +enable until chip select is enable in CPU interface
> +  - wr-setup: clock cycles for the active period of CS signal
> enable
> +until write signal is enable in CPU interface
> +  - wr-act: clock cycles for the active period of CS enable in
> CPU
> +interface
>>> What about calling this property wr-active ? I had called this wr-cycle in
>>> a previous implementation, that could be an option as well.
>>>
> +  - wr-hold: clock cycles for the active period of CS disable
> until
> +write signal is disable in CPU interface
 cpu-timings name does not sound to be related to display.
 I wonder if it would not be better to merge cpu-timings into
 display-timings but this will require more discussion I guess.
>>> The panel has a memory-like interface with chip select, read/write and
>>> access strobe, address (1 bit) and data signals. The interface is defined
>>> in the MIPI DBI specification (a quick search turned up
>>> http://www.scribd.com/doc/206899854/MIPI-DPI-Specification-v2, there might
>>> be direct PDF downloads available), even if some panels implement a
>>> similar interface that predates MIPI DBI (with names such as i80 or
>>> SYS-80 probably due to the similarities with the 8051 external bus).
>>>
>>> The cpu-timings properties describe the read and write access timings for
>>> those signals, unrelated to the display video timings. I thus believe that
>>> they should be separate from the display timings. We will probably need to
>>> add properties for the read signal as well, but that can be done later.
>> cpu-timings suggests these timings are for CPU, but they are for display
>> panel in CPU mode.
>> I though rather about something like display-cpu-timings or i80-timings,
>> just to avoid confusion.
> mipi-dbi-timings maybe ?

It looks OK.

I guess only hactive, and vactive props of display-timings are used,
maybe some flags?
I wonder if it would not be better to drop display-timings node
and place all props into mipi-dbi-timings or mipi-dbi-settings node.
Or rather all timings props should be put into port/endpoint node with
or without
any container node.

Regards
Andrzej

>
 If you want to stay with separate node please consider to make it
 optional as whole node or make some properties required. Making node
 required with all sub-properties optional is weird.
 By the way I hope all timings properties are generic for CPU mode,
 if not they should be prefixed by vendor or model.
>>> The timings description should be pretty generic I 

[PATCH] drm/tegra: restrict plane loops to legacy planes

[Daniel Vetter]

In Matt Ropers primary plane series a set of prep patches like

commit af2b653bfb4ef40931b4d101ca842ce0c5da57ef
Author: Matt Roper 
Date:   Tue Apr 1 15:22:32 2014 -0700

drm/i915: Restrict plane loops to only operate on overlay planes (v2)

ensured that all exisiting users of the mode_config->plane_list
wouldn't change behaviour. Unfortunately tegra seems to have fallen
through the cracks. Fix it.

This regression was introduced in

commit e13161af80c185ecd8dc4641d0f5df58f9e3e0af
Author: Matt Roper 
Date:   Tue Apr 1 15:22:38 2014 -0700

drm: Add drm_crtc_init_with_planes() (v2)

The result was that we've unref'ed the fb for the primary plane twice,
leading to a use-after free bug. This is because the drm core will
already set crtc->primary->fb to NULL and do the unref for us, and the
crtc disable hook is called by the drm crtc helpers for exactly this
case.

Aside: Now that the fbdev helpers clean up planes there's no longer a
need to do this in drivers. So this could probably be nuked entirely
in linux-next.

Cc: Matt Roper 
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/tegra/dc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index 36c717af6cf9..edb871d7d395 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -312,7 +312,7 @@ static void tegra_crtc_disable(struct drm_crtc *crtc)
struct drm_device *drm = crtc->dev;
struct drm_plane *plane;

-   list_for_each_entry(plane, >mode_config.plane_list, head) {
+   drm_for_each_legacy_plane(plane, >mode_config.plane_list) {
if (plane->crtc == crtc) {
tegra_plane_disable(plane);
plane->crtc = NULL;
-- 
1.9.2

[PATCH v2 04/10] drm/nouveau/fb: add GK20A support

[Alexandre Courbot]

On Wed, Apr 23, 2014 at 11:07 AM, Alexandre Courbot  
wrote:
> On 04/22/2014 07:40 PM, Thierry Reding wrote:
>>
>> * PGP Signed by an unknown key
>>
>>
>> On Mon, Apr 21, 2014 at 03:02:16PM +0900, Alexandre Courbot wrote:
>> [...]
>>>
>>> diff --git a/drivers/gpu/drm/nouveau/core/subdev/fb/ramgk20a.c
>>> b/drivers/gpu/drm/nouveau/core/subdev/fb/ramgk20a.c
>>
>> [...]
>>>
>>> +   pages = dma_alloc_from_contiguous(dev, ncmin, order);
>>> +   if (!pages) {
>>> +   gk20a_ram_put(pfb, );
>>> +   return -ENOMEM;
>>> +   }
>>> +
>>> +   dma_addr = pfn_to_dma(nv_device_base(nv_device(pfb)),
>>> + page_to_pfn(pages));
>>
>>
>> This breaks compilation on x86 because neither pfn_to_dma() nor
>> dma_to_pfn() are available. Is there some other way this can be
>> allocated so that these functions don't need to be called?
>
>
> Mmm, this is bad. There is probably another more portable way to do this.
> Let me look for it.

page_to_phys()/phys_to_page() can be used by drivers and will work
just fine here since the CPU and GPU use the same physical addresses
to access memory.

Thanks,
Alex.

[RFC v2 PATCH v4 09/14] ARM: dts: s6e3fa0: add DT bindings

[Laurent Pinchart]

Hi Andrzej,

On Wednesday 23 April 2014 14:48:31 Andrzej Hajda wrote:
> On 04/23/2014 01:34 PM, Laurent Pinchart wrote:
> > On Wednesday 23 April 2014 11:02:21 Andrzej Hajda wrote:
> >> On 04/21/2014 02:28 PM, YoungJun Cho wrote:
> >>> This patch adds DT bindings for s6e3fa0 panel.
> >>> The bindings describes panel resources, display timings and cpu timings.
> >>> 
> >>> Changelog v2:
> >>> - Adds unit address (commented by Sachin Kamat)
> >>> Changelog v3:
> >>> - Removes optional delay, size properties (commented by Laurent
> >>> Pinchart)
> >>> - Adds OLED detection, TE gpio properties
> >>> Changelog v4:
> >>> - Moves CPU timings relevant properties from FIMD DT
> >>> 
> >>>   (commeted by Laurent Pinchart, Andrzej Hajda)
> >>> 
> >>> Signed-off-by: YoungJun Cho 
> >>> Acked-by: Inki Dae 
> >>> Acked-by: Kyungmin Park 
> >>> ---
> >>> 
> >>>  .../devicetree/bindings/panel/samsung,s6e3fa0.txt  |   63
> >>>  +++
> >>> 
> >>> 1 file changed, 63 insertions(+)
> >>> 
> >>>  create mode 100644
> >>>  Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt>
> >>> 
> >>> diff --git a/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt
> >>> b/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt new file
> >>> mode 100644
> >>> index 000..9eeb38b
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt
> >>> @@ -0,0 +1,63 @@
> >>> +Samsung S6E3FA0 AMOLED LCD 5.7 inch panel
> >>> +
> >>> +Required properties:
> >>> +  - compatible: "samsung,s6e3fa0"
> >>> +  - reg: the virtual channel number of a DSI peripheral
> >>> +  - vdd3-supply: core voltage supply
> >>> +  - vci-supply: voltage supply for analog circuits
> >>> +  - reset-gpio: a GPIO spec for the reset pin
> >>> +  - det-gpio: a GPIO spec for the OLED detection pin
> >>> +  - te-gpio: a GPIO spec for the TE pin
> >>> +  - display-timings: timings for the connected panel as described by
> >>> [1]
> >> 
> >> Which properties of display-timings are relevant for CPU mode?
> >> I guess width and height. Anything more?
> >> 
> >>> +  - cpu-timings: CPU interface timings for the connected panel, and it
> >>> contains
> >>> +following optional properties.
> >>> +  - cs-setup: clock cycles for the active period of address
> >>> signal
> >>> +enable until chip select is enable in CPU interface
> >>> +  - wr-setup: clock cycles for the active period of CS signal
> >>> enable
> >>> +until write signal is enable in CPU interface
> >>> +  - wr-act: clock cycles for the active period of CS enable in
> >>> CPU
> >>> +interface
> > 
> > What about calling this property wr-active ? I had called this wr-cycle in
> > a previous implementation, that could be an option as well.
> > 
> >>> +  - wr-hold: clock cycles for the active period of CS disable
> >>> until
> >>> +write signal is disable in CPU interface
> >> 
> >> cpu-timings name does not sound to be related to display.
> >> I wonder if it would not be better to merge cpu-timings into
> >> display-timings but this will require more discussion I guess.
> > 
> > The panel has a memory-like interface with chip select, read/write and
> > access strobe, address (1 bit) and data signals. The interface is defined
> > in the MIPI DBI specification (a quick search turned up
> > http://www.scribd.com/doc/206899854/MIPI-DPI-Specification-v2, there might
> > be direct PDF downloads available), even if some panels implement a
> > similar interface that predates MIPI DBI (with names such as i80 or
> > SYS-80 probably due to the similarities with the 8051 external bus).
> > 
> > The cpu-timings properties describe the read and write access timings for
> > those signals, unrelated to the display video timings. I thus believe that
> > they should be separate from the display timings. We will probably need to
> > add properties for the read signal as well, but that can be done later.
>
> cpu-timings suggests these timings are for CPU, but they are for display
> panel in CPU mode.
> I though rather about something like display-cpu-timings or i80-timings,
> just to avoid confusion.

mipi-dbi-timings maybe ?

> >> If you want to stay with separate node please consider to make it
> >> optional as whole node or make some properties required. Making node
> >> required with all sub-properties optional is weird.
> >> By the way I hope all timings properties are generic for CPU mode,
> >> if not they should be prefixed by vendor or model.
> > 
> > The timings description should be pretty generic I believe, especially
> > given that the interface is standardized by the MIPI alliance. Could you
> > have a quick look at the spec and share your opinion ?
> > 
> >>> +
> >>> +Optional properties:
> >>> +
> >>> +The device node can contain one 'port' child node with one child
> >>> +'endpoint' node, according to the bindings defined in [2]. This
> >>> +node should describe panel's 

[RFC v2 PATCH v4 09/14] ARM: dts: s6e3fa0: add DT bindings

[Andrzej Hajda]

On 04/23/2014 01:34 PM, Laurent Pinchart wrote:
> Hi Andrzej,
>
> On Wednesday 23 April 2014 11:02:21 Andrzej Hajda wrote:
>> On 04/21/2014 02:28 PM, YoungJun Cho wrote:
>>> This patch adds DT bindings for s6e3fa0 panel.
>>> The bindings describes panel resources, display timings and cpu timings.
>>>
>>> Changelog v2:
>>> - Adds unit address (commented by Sachin Kamat)
>>> Changelog v3:
>>> - Removes optional delay, size properties (commented by Laurent Pinchart)
>>> - Adds OLED detection, TE gpio properties
>>> Changelog v4:
>>> - Moves CPU timings relevant properties from FIMD DT
>>>
>>>   (commeted by Laurent Pinchart, Andrzej Hajda)
>>>
>>> Signed-off-by: YoungJun Cho 
>>> Acked-by: Inki Dae 
>>> Acked-by: Kyungmin Park 
>>> ---
>>>
>>>  .../devicetree/bindings/panel/samsung,s6e3fa0.txt  |   63 +++
>>> 1 file changed, 63 insertions(+)
>>>  create mode 100644
>>>  Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt> 
>>> diff --git a/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt
>>> b/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt new file
>>> mode 100644
>>> index 000..9eeb38b
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt
>>> @@ -0,0 +1,63 @@
>>> +Samsung S6E3FA0 AMOLED LCD 5.7 inch panel
>>> +
>>> +Required properties:
>>> +  - compatible: "samsung,s6e3fa0"
>>> +  - reg: the virtual channel number of a DSI peripheral
>>> +  - vdd3-supply: core voltage supply
>>> +  - vci-supply: voltage supply for analog circuits
>>> +  - reset-gpio: a GPIO spec for the reset pin
>>> +  - det-gpio: a GPIO spec for the OLED detection pin
>>> +  - te-gpio: a GPIO spec for the TE pin
>>> +  - display-timings: timings for the connected panel as described by [1]
>> Which properties of display-timings are relevant for CPU mode?
>> I guess width and height. Anything more?
>>
>>> +  - cpu-timings: CPU interface timings for the connected panel, and it
>>> contains
>>> +following optional properties.
>>> +  - cs-setup: clock cycles for the active period of address
>>> signal
>>> +enable until chip select is enable in CPU interface
>>> +  - wr-setup: clock cycles for the active period of CS signal
>>> enable
>>> +until write signal is enable in CPU interface
>>> +  - wr-act: clock cycles for the active period of CS enable in
>>> CPU
>>> +interface
> What about calling this property wr-active ? I had called this wr-cycle in a 
> previous implementation, that could be an option as well.
>
>>> +  - wr-hold: clock cycles for the active period of CS disable
>>> until
>>> +write signal is disable in CPU interface
>> cpu-timings name does not sound to be related to display.
>> I wonder if it would not be better to merge cpu-timings into
>> display-timings but this will require more discussion I guess.
> The panel has a memory-like interface with chip select, read/write and access 
> strobe, address (1 bit) and data signals. The interface is defined in the 
> MIPI 
> DBI specification (a quick search turned up 
> http://www.scribd.com/doc/206899854/MIPI-DPI-Specification-v2, there might be 
> direct PDF downloads available), even if some panels implement a similar 
> interface that predates MIPI DBI (with names such as i80 or SYS-80 probably 
> due to the similarities with the 8051 external bus).
>
> The cpu-timings properties describe the read and write access timings for 
> those signals, unrelated to the display video timings. I thus believe that 
> they should be separate from the display timings. We will probably need to 
> add 
> properties for the read signal as well, but that can be done later.

cpu-timings suggests these timings are for CPU, but they are for display
panel in CPU mode.
I though rather about something like display-cpu-timings or i80-timings,
just to avoid confusion.

Regards
Andrzej

>> If you want to stay with separate node please consider to make it
>> optional as whole node or make some properties required. Making node
>> required with all sub-properties optional is weird.
>> By the way I hope all timings properties are generic for CPU mode,
>> if not they should be prefixed by vendor or model.
> The timings description should be pretty generic I believe, especially given 
> that the interface is standardized by the MIPI alliance. Could you have a 
> quick look at the spec and share your opinion ?
>
>>> +
>>> +Optional properties:
>>> +
>>> +The device node can contain one 'port' child node with one child
>>> +'endpoint' node, according to the bindings defined in [2]. This
>>> +node should describe panel's video bus.
>>> +
>>> +[1]: Documentation/devicetree/bindings/video/display-timing.txt
>>> +[2]: Documentation/devicetree/bindings/media/video-interfaces.txt
>>> +
>>> +Example:
>>> +
>>> +   panel at 0 {
>>> +   compatible = "samsung,s6e3fa0";
>>> +   reg = <0>;
>>> +   vdd3-supply = 

[Bug 77785] (radeonsi) Some lighting issues in games, textures goes black

[bugzilla-dae...@freedesktop.org]

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

--- Comment #7 from smoki  ---

 I think i see nothing in console... Tried to make short traces of both cases
:).

 https://dl.dropboxusercontent.com/u/74553632/traces.zip

-- 
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/20140423/5eb0751b/attachment.html>

[RFC v2 PATCH v4 09/14] ARM: dts: s6e3fa0: add DT bindings

[Laurent Pinchart]

Hi Andrzej,

On Wednesday 23 April 2014 11:02:21 Andrzej Hajda wrote:
> On 04/21/2014 02:28 PM, YoungJun Cho wrote:
> > This patch adds DT bindings for s6e3fa0 panel.
> > The bindings describes panel resources, display timings and cpu timings.
> > 
> > Changelog v2:
> > - Adds unit address (commented by Sachin Kamat)
> > Changelog v3:
> > - Removes optional delay, size properties (commented by Laurent Pinchart)
> > - Adds OLED detection, TE gpio properties
> > Changelog v4:
> > - Moves CPU timings relevant properties from FIMD DT
> > 
> >   (commeted by Laurent Pinchart, Andrzej Hajda)
> > 
> > Signed-off-by: YoungJun Cho 
> > Acked-by: Inki Dae 
> > Acked-by: Kyungmin Park 
> > ---
> > 
> >  .../devicetree/bindings/panel/samsung,s6e3fa0.txt  |   63 +++
> > 1 file changed, 63 insertions(+)
> >  create mode 100644
> >  Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt> 
> > diff --git a/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt
> > b/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt new file
> > mode 100644
> > index 000..9eeb38b
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt
> > @@ -0,0 +1,63 @@
> > +Samsung S6E3FA0 AMOLED LCD 5.7 inch panel
> > +
> > +Required properties:
> > +  - compatible: "samsung,s6e3fa0"
> > +  - reg: the virtual channel number of a DSI peripheral
> > +  - vdd3-supply: core voltage supply
> > +  - vci-supply: voltage supply for analog circuits
> > +  - reset-gpio: a GPIO spec for the reset pin
> > +  - det-gpio: a GPIO spec for the OLED detection pin
> > +  - te-gpio: a GPIO spec for the TE pin
> > +  - display-timings: timings for the connected panel as described by [1]
> 
> Which properties of display-timings are relevant for CPU mode?
> I guess width and height. Anything more?
> 
> > +  - cpu-timings: CPU interface timings for the connected panel, and it
> > contains
> > +following optional properties.
> > +  - cs-setup: clock cycles for the active period of address
> > signal
> > +enable until chip select is enable in CPU interface
> > +  - wr-setup: clock cycles for the active period of CS signal
> > enable
> > +until write signal is enable in CPU interface
> > +  - wr-act: clock cycles for the active period of CS enable in
> > CPU
> > +interface

What about calling this property wr-active ? I had called this wr-cycle in a 
previous implementation, that could be an option as well.

> > +  - wr-hold: clock cycles for the active period of CS disable
> > until
> > +write signal is disable in CPU interface
> 
> cpu-timings name does not sound to be related to display.
> I wonder if it would not be better to merge cpu-timings into
> display-timings but this will require more discussion I guess.

The panel has a memory-like interface with chip select, read/write and access 
strobe, address (1 bit) and data signals. The interface is defined in the MIPI 
DBI specification (a quick search turned up 
http://www.scribd.com/doc/206899854/MIPI-DPI-Specification-v2, there might be 
direct PDF downloads available), even if some panels implement a similar 
interface that predates MIPI DBI (with names such as i80 or SYS-80 probably 
due to the similarities with the 8051 external bus).

The cpu-timings properties describe the read and write access timings for 
those signals, unrelated to the display video timings. I thus believe that 
they should be separate from the display timings. We will probably need to add 
properties for the read signal as well, but that can be done later.

> If you want to stay with separate node please consider to make it
> optional as whole node or make some properties required. Making node
> required with all sub-properties optional is weird.
> By the way I hope all timings properties are generic for CPU mode,
> if not they should be prefixed by vendor or model.

The timings description should be pretty generic I believe, especially given 
that the interface is standardized by the MIPI alliance. Could you have a 
quick look at the spec and share your opinion ?

> > +
> > +Optional properties:
> > +
> > +The device node can contain one 'port' child node with one child
> > +'endpoint' node, according to the bindings defined in [2]. This
> > +node should describe panel's video bus.
> > +
> > +[1]: Documentation/devicetree/bindings/video/display-timing.txt
> > +[2]: Documentation/devicetree/bindings/media/video-interfaces.txt
> > +
> > +Example:
> > +
> > +   panel at 0 {
> > +   compatible = "samsung,s6e3fa0";
> > +   reg = <0>;
> > +   vdd3-supply = <_reg>;
> > +   vci-supply = <_reg>;
> > +   reset-gpio = < 4 0>;
> > +   det-gpio = < 6 0>;
> > +   te-gpio = < 7 0>;
> > +
> > +   display-timings {
> > +   timing0: timing-0 {
> > +   clock-frequency = <0>;
> > +

[RFC PATCH 2/2 with seqcount v3] reservation: add suppport for read-only access using rcu

[Maarten Lankhorst]

This adds 4 more functions to deal with rcu.

reservation_object_get_fences_rcu() will obtain the list of shared
and exclusive fences without obtaining the ww_mutex.

reservation_object_wait_timeout_rcu() will wait on all fences of the
reservation_object, without obtaining the ww_mutex.

reservation_object_test_signaled_rcu() will test if all fences of the
reservation_object are signaled without using the ww_mutex.

reservation_object_get_excl() is added because touching the fence_excl
member directly will trigger a sparse warning.

Signed-off-by: Maarten Lankhorst 
---
Using seqcount and fixing some lockdep bugs.
Changes since v2:
- Fix some crashes, remove some unneeded barriers when provided by seqcount 
writes
- Fix code to work correctly with sparse's RCU annotations.
- Create a global string for the seqcount lock to make lockdep happy.

Can I get this version reviewed? If it looks correct I'll mail the full series
because it's intertwined with the TTM conversion to use this code.

See http://cgit.freedesktop.org/~mlankhorst/linux/log/?h=vmwgfx_wip
---
diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c
index d89a98d2c37b..0df673f812eb 100644
--- a/drivers/base/dma-buf.c
+++ b/drivers/base/dma-buf.c
@@ -137,7 +137,7 @@ static unsigned int dma_buf_poll(struct file *file, 
poll_table *poll)
struct reservation_object_list *fobj;
struct fence *fence_excl;
unsigned long events;
-   unsigned shared_count;
+   unsigned shared_count, seq;

dmabuf = file->private_data;
if (!dmabuf || !dmabuf->resv)
@@ -151,14 +151,20 @@ static unsigned int dma_buf_poll(struct file *file, 
poll_table *poll)
if (!events)
return 0;

-   ww_mutex_lock(>lock, NULL);
+retry:
+   seq = read_seqcount_begin(>seq);
+   rcu_read_lock();

-   fobj = resv->fence;
-   if (!fobj)
-   goto out;
-
-   shared_count = fobj->shared_count;
-   fence_excl = resv->fence_excl;
+   fobj = rcu_dereference(resv->fence);
+   if (fobj)
+   shared_count = fobj->shared_count;
+   else
+   shared_count = 0;
+   fence_excl = rcu_dereference(resv->fence_excl);
+   if (read_seqcount_retry(>seq, seq)) {
+   rcu_read_unlock();
+   goto retry;
+   }

if (fence_excl && (!(events & POLLOUT) || shared_count == 0)) {
struct dma_buf_poll_cb_t *dcb = >cb_excl;
@@ -176,14 +182,20 @@ static unsigned int dma_buf_poll(struct file *file, 
poll_table *poll)
spin_unlock_irq(>poll.lock);

if (events & pevents) {
-   if (!fence_add_callback(fence_excl, >cb,
+   if (!fence_get_rcu(fence_excl)) {
+   /* force a recheck */
+   events &= ~pevents;
+   dma_buf_poll_cb(NULL, >cb);
+   } else if (!fence_add_callback(fence_excl, >cb,
   dma_buf_poll_cb)) {
events &= ~pevents;
+   fence_put(fence_excl);
} else {
/*
 * No callback queued, wake up any additional
 * waiters.
 */
+   fence_put(fence_excl);
dma_buf_poll_cb(NULL, >cb);
}
}
@@ -205,13 +217,26 @@ static unsigned int dma_buf_poll(struct file *file, 
poll_table *poll)
goto out;

for (i = 0; i < shared_count; ++i) {
-   struct fence *fence = fobj->shared[i];
+   struct fence *fence = rcu_dereference(fobj->shared[i]);

+   if (!fence_get_rcu(fence)) {
+   /*
+* fence refcount dropped to zero, this means
+* that fobj has been freed
+*
+* call dma_buf_poll_cb and force a recheck!
+*/
+   events &= ~POLLOUT;
+   dma_buf_poll_cb(NULL, >cb);
+   break;
+   }
if (!fence_add_callback(fence, >cb,
dma_buf_poll_cb)) {
+   fence_put(fence);
events &= ~POLLOUT;
break;
}
+   fence_put(fence);
}

/* No callback queued, wake up any additional waiters. */
@@ -220,7 +245,7 @@ static unsigned int dma_buf_poll(struct file *file, 
poll_table *poll)
}

  out:
-   

[RFC 4/5] drm: Introduce drm_set_unique()

[David Herrmann]

Hi

On Wed, Apr 23, 2014 at 10:40 AM, Daniel Vetter  wrote:
> On Wed, Apr 23, 2014 at 09:17:16AM +0200, Thierry Reding wrote:
>> On second thought, wouldn't this be better located in drm_stub.c? It
>> isn't really related to the IOCTL code except that one of the IOCTLs now
>> uses the information set by this function. Logically I think it belongs
>> with the likes of drm_dev_alloc() and drm_dev_register().
>
> Yeah makes sense. Tbh the entire split-up of these core drm functions is
> still a bit messy, so I don't mind if it's a bit inconsistent really. We
> can do the suffling when someone bothers with the kerneldoc for all of
> them and pulls it into the drm docbook.

During drm_dev_*() cleanup, I tried to keep the following structure:
  drm_drv.c: global drm-module setup
  drm_stub.c: drm_device allocation, registration and lifetime management
  drm_fops.c: reference-implementation of the drm file_operations

The only thing that's wrongly placed is ioctl handling (which somehow
ended up in drm_drv.c instead of drm_fops.c). drm_stub.c is where all
the generic and mandatory DRM device handling is placed so yeah, I'd
put the set_unique() there.

Thanks
David

[RFC v2 PATCH 02/14] drm/exynos: dsi: delay setting clocks after reset

[YoungJun Cho]

Hi again Andrzej,

On 04/23/2014 10:01 AM, YoungJun Cho wrote:
> Hi Andrzej
>
> Thank you for comments.
>
> On 04/22/2014 09:15 PM, Andrzej Hajda wrote:
>> Hi YoungJun,
>>
>> On 04/21/2014 02:28 PM, YoungJun Cho wrote:
>>> Some phy control registers are not kept after software reset.
>>> So this patch makes the clocks containing phy control to be set
>>> after software reset.
>>>
>>> Signed-off-by: YoungJun Cho 
>>> Acked-by: Inki Dae 
>>> Acked-by: Kyungmin Park 
>>> ---
>>>   drivers/gpu/drm/exynos/exynos_drm_dsi.c |2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>>> b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>>> index 956e5f3..2cf1f0b 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>>> @@ -946,10 +946,10 @@ static irqreturn_t exynos_dsi_irq(int irq, void
>>> *dev_id)
>>>
>>>   static int exynos_dsi_init(struct exynos_dsi *dsi)
>>>   {
>>> -exynos_dsi_enable_clock(dsi);
>>>   exynos_dsi_reset(dsi);
>>>   enable_irq(dsi->irq);
>>>   exynos_dsi_wait_for_reset(dsi);
>>> +exynos_dsi_enable_clock(dsi);
>>>   exynos_dsi_init_link(dsi);
>>>
>>>   return 0;
>>
>> I have commented it in the previous version of the patchset. I repeat it
>> again.
>> This is a regression, on exynos4210-trats I have 'timeout waiting for
>> reset' message after dpms off, on.
>
> I'm really sorry for that. I misunderstood last time.
>
> I think the original codes were correct, because the reset timeout would
> be occurred without clock activation.

This is not true.

>
> I'll check and fix again.
> (By the way, why am I ok?)

I have not verified with exynos4210-trats board yet(I have to get it),
but reset timeout is occured in exynos_dsi_wait_for_reset()
with >completed and that is completed by exynos_dsi_irq().

And the regulators and clocks are enabled by exynos_dsi_poweron(),
NOT by exynos_dsi_enable_clock().

So I think the reset timeout is not related with this patch.

Anyway I need more investigation.

Thank you.
Best regards YJ

>
>>
>> I will comment your previous answer here to make the discussion easier:
>>> As I mentioned in description, it came from phy control registers.
>>> Fortunately Exynos4 SoCs are safe, but the DSIM_PHYCTRL_REG,
>>> DSIM_PHYTIMING_REG, DSIM_PHYTIMING1_REG and DSIM_PHYTIMING2_REG are
>>> affected which are used in exynos_dsi_set_pll() for Exynos5 SoCs.
>>>
>>> So this patch is required for Exynos5 SoCs.
>>
>> In the moment this patch is applied exynos_dsi_set_pll do not touch phy
>> registers you have mentioned.
>> Your change would be more clear if it will be merged together with the
>> patch adding PHYCTRL settings.
>>
>> Anyway, solution is simple - please set PHY registers after reset and
>> configure clocks before reset to avoid
>> reset timeouts, is there any reason to not do it this way?
>
> The only reason is that the PHY control is related with PLL control and
> that was in exynos_dsi_enable_clock() call path.
> So I just wanted to keep current sequence.
>
> If there is no way to use previous one, I'll consider your approach.
>
> Thank you.
> Best regards YJ
>
>>
>> Regards
>> Andrzej
>>
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>

[PULL] drm init cleanup for drm-next

[Daniel Vetter]

On Wed, Apr 23, 2014 at 12:26:53PM +0200, Daniel Vetter wrote:
> Hi Dave,
> 
> Next pull request, this time more of the drm de-midlayering work. The big
> thing is that his patch series here removes everything from drm_bus except
> the set_busid callback. Thierry has a few more patches on top of this to
> make that one optional to.
> 
> With that we can ditch all the non-pci drm_bus implementations, which
> Thierry has already done for the fake tegra host1x drm_bus.
> 
> Reviewed by Thierry, Laurent and David and now also survived some testing
> on my intel boxes to make sure the irq fumble is fixed correctly ;-) The
> last minute rebase was just to add the r-b tags from Thierry for the 2
> patches I've redone.
> 
> Cheers, Daniel

Hah, 2nd pull requests without scripts and already screwed it up - the
DRM_WAIT_ON removal shouldn't be in here.
-Daniel

The following changes since commit a798c10faf62a505d24e5f6213fbaf904a39623f:

  Linux 3.15-rc2 (2014-04-20 11:08:50 -0700)

are available in the git repository at:

  git://people.freedesktop.org/~danvet/drm drm-init-cleanup

for you to fetch changes up to 3c8413951cbd8a2d855740823fc547c97b845f6f:

  drm/: don't set driver->dev_priv_size to 0 (2014-04-23 10:32:54 
+0200)


Daniel Vetter (17):
  drm/irq: simplify irq checks in drm_wait_vblank
  drm/pci: fold in irq_by_busid support
  drm/irq: drm_control is a legacy ioctl, so pci devices only
  drm/irq: remove cargo-culted locking from irq_install/uninstall
  drm: remove drm_dev_to_irq from drivers
  drm: kill drm_bus->bus_type
  drm: Rip out totally bogus vga_switcheroo->can_switch locking
  drm: rename dev->count_lock to dev->buf_lock
  drm/irq: track the irq installed in drm_irq_install in dev->irq
  drm/irq: Look up the pci irq directly in the drm_control ioctl
  drm: pass the irq explicitly to drm_irq_install
  drm: remove bus->get_irq implementations
  drm: inline drm_pci_set_unique
  drm: rip out dev->devname
  drm: remove drm_bus->get_name
  drm: Remove dev->kdriver
  drm/: don't set driver->dev_priv_size to 0

 Documentation/DocBook/drm.tmpl   |  10 +---
 drivers/gpu/drm/armada/armada_drv.c  |   2 +-
 drivers/gpu/drm/ast/ast_drv.c|   1 -
 drivers/gpu/drm/drm_bufs.c   |  32 +-
 drivers/gpu/drm/drm_info.c   |   6 ++---
 drivers/gpu/drm/drm_ioctl.c  |  13 ++-
 drivers/gpu/drm/drm_irq.c| 105 

 drivers/gpu/drm/drm_pci.c|  93 
+-
 drivers/gpu/drm/drm_platform.c   |  25 
 drivers/gpu/drm/drm_stub.c   |   7 +-
 drivers/gpu/drm/drm_usb.c|  14 
 drivers/gpu/drm/gma500/psb_drv.c |   2 +-
 drivers/gpu/drm/i915/i915_dma.c  |  13 ++-
 drivers/gpu/drm/i915/i915_drv.c  |   9 ++--
 drivers/gpu/drm/i915/i915_gem.c  |   7 +++---
 drivers/gpu/drm/mga/mga_state.c  |   2 +-
 drivers/gpu/drm/msm/msm_drv.c|   2 +-
 drivers/gpu/drm/nouveau/nouveau_vga.c|  11 +
 drivers/gpu/drm/qxl/qxl_drv.c|   1 -
 drivers/gpu/drm/qxl/qxl_irq.c|   2 +-
 drivers/gpu/drm/r128/r128_state.c|   2 +-
 drivers/gpu/drm/radeon/radeon_device.c   |  11 +
 drivers/gpu/drm/radeon/radeon_drv.c  |   1 -
 drivers/gpu/drm/radeon/radeon_irq_kms.c  |   2 +-
 drivers/gpu/drm/radeon/radeon_state.c|   2 +-
 drivers/gpu/drm/shmobile/shmob_drm_drv.c |   2 +-
 drivers/gpu/drm/tegra/bus.c  |  11 -
 drivers/gpu/drm/tilcdc/tilcdc_drv.c  |   2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c  |   2 +-
 include/drm/drmP.h   |  33 +++
 30 files changed, 158 insertions(+), 267 deletions(-)
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PULL] drm init cleanup for drm-next

[Daniel Vetter]

Hi Dave,

Next pull request, this time more of the drm de-midlayering work. The big
thing is that his patch series here removes everything from drm_bus except
the set_busid callback. Thierry has a few more patches on top of this to
make that one optional to.

With that we can ditch all the non-pci drm_bus implementations, which
Thierry has already done for the fake tegra host1x drm_bus.

Reviewed by Thierry, Laurent and David and now also survived some testing
on my intel boxes to make sure the irq fumble is fixed correctly ;-) The
last minute rebase was just to add the r-b tags from Thierry for the 2
patches I've redone.

Cheers, Daniel


The following changes since commit a798c10faf62a505d24e5f6213fbaf904a39623f:

  Linux 3.15-rc2 (2014-04-20 11:08:50 -0700)

are available in the git repository at:

  git://people.freedesktop.org/~danvet/drm drm-init-cleanup

for you to fetch changes up to 5c35d532cc6d45cf61c58dd384109f900d577698:

  drm: Remove DRM_WAIT_ON from all drivers (2014-04-23 10:32:56 +0200)


Daniel Vetter (19):
  drm/irq: simplify irq checks in drm_wait_vblank
  drm/pci: fold in irq_by_busid support
  drm/irq: drm_control is a legacy ioctl, so pci devices only
  drm/irq: remove cargo-culted locking from irq_install/uninstall
  drm: remove drm_dev_to_irq from drivers
  drm: kill drm_bus->bus_type
  drm: Rip out totally bogus vga_switcheroo->can_switch locking
  drm: rename dev->count_lock to dev->buf_lock
  drm/irq: track the irq installed in drm_irq_install in dev->irq
  drm/irq: Look up the pci irq directly in the drm_control ioctl
  drm: pass the irq explicitly to drm_irq_install
  drm: remove bus->get_irq implementations
  drm: inline drm_pci_set_unique
  drm: rip out dev->devname
  drm: remove drm_bus->get_name
  drm: Remove dev->kdriver
  drm/: don't set driver->dev_priv_size to 0
  drm/irq: Replace DRM_WAIT_ON with wait_event
  drm: Remove DRM_WAIT_ON from all drivers

 Documentation/DocBook/drm.tmpl   |  10 +--
 drivers/gpu/drm/armada/armada_drv.c  |   2 +-
 drivers/gpu/drm/ast/ast_drv.c|   1 -
 drivers/gpu/drm/drm_bufs.c   |  32 +++
 drivers/gpu/drm/drm_info.c   |   6 ++---
 drivers/gpu/drm/drm_ioctl.c  |  13 +-
 drivers/gpu/drm/drm_irq.c| 119 
++--
 drivers/gpu/drm/drm_pci.c|  93 
-
 drivers/gpu/drm/drm_platform.c   |  25 --
 drivers/gpu/drm/drm_stub.c   |   7 +
 drivers/gpu/drm/drm_usb.c|  14 --
 drivers/gpu/drm/gma500/psb_drv.c |   2 +-
 drivers/gpu/drm/i915/i915_dma.c  |  24 +++--
 drivers/gpu/drm/i915/i915_drv.c  |   9 +--
 drivers/gpu/drm/i915/i915_gem.c  |   7 ++---
 drivers/gpu/drm/mga/mga_irq.c|  12 ++---
 drivers/gpu/drm/mga/mga_state.c  |   2 +-
 drivers/gpu/drm/msm/msm_drv.c|   2 +-
 drivers/gpu/drm/nouveau/nouveau_vga.c|  11 
 drivers/gpu/drm/qxl/qxl_drv.c|   1 -
 drivers/gpu/drm/qxl/qxl_irq.c|   2 +-
 drivers/gpu/drm/r128/r128_state.c|   2 +-
 drivers/gpu/drm/radeon/radeon_device.c   |  11 
 drivers/gpu/drm/radeon/radeon_drv.c  |   1 -
 drivers/gpu/drm/radeon/radeon_irq.c  |  11 +---
 drivers/gpu/drm/radeon/radeon_irq_kms.c  |   2 +-
 drivers/gpu/drm/radeon/radeon_state.c|   2 +-
 drivers/gpu/drm/shmobile/shmob_drm_drv.c |   2 +-
 drivers/gpu/drm/tegra/bus.c  |  11 
 drivers/gpu/drm/tilcdc/tilcdc_drv.c  |   2 +-
 drivers/gpu/drm/via/via_dmablit.c|  23 -
 drivers/gpu/drm/via/via_irq.c|  15 +++
 drivers/gpu/drm/via/via_video.c  |  12 ++---
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c  |   2 +-
 include/drm/drmP.h   |  33 ++--
 include/drm/drm_os_linux.h   |  24 -
 36 files changed, 228 insertions(+), 319 deletions(-)
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[RFC v2 PATCH v3 10/14] drm/panel: add S6E3FA0 driver

[Andrzej Hajda]

Hi YoungJun,


On 04/21/2014 02:28 PM, YoungJun Cho wrote:
> This patch adds MIPI-DSI command mode based S6E3FA0 AMOLED LCD Panel driver.
> 
> Changelog v2:
> - Declares delay, size properties in probe routine instead of DT
> Changelog v3:
> - Moves CPU timings relevant properties from FIMD DT
>   (commented by Laurent Pinchart, Andrzej Hajda)
> 
> Signed-off-by: YoungJun Cho 
> Acked-by: Inki Dae 
> Acked-by: Kyungmin Park 
> ---
>  drivers/gpu/drm/panel/Kconfig |7 +
>  drivers/gpu/drm/panel/Makefile|1 +
>  drivers/gpu/drm/panel/panel-s6e3fa0.c |  569 
> +
>  3 files changed, 577 insertions(+)
>  create mode 100644 drivers/gpu/drm/panel/panel-s6e3fa0.c
> 
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> index 4ec874d..be1392e 100644
> --- a/drivers/gpu/drm/panel/Kconfig
> +++ b/drivers/gpu/drm/panel/Kconfig
> @@ -30,4 +30,11 @@ config DRM_PANEL_S6E8AA0
>   select DRM_MIPI_DSI
>   select VIDEOMODE_HELPERS
>  
> +config DRM_PANEL_S6E3FA0
> + tristate "S6E3FA0 DSI command mode panel"
> + depends on DRM && DRM_PANEL
> + depends on OF
> + select DRM_MIPI_DSI
> + select VIDEOMODE_HELPERS
> +
>  endmenu
> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> index 8b92921..85c6738 100644
> --- a/drivers/gpu/drm/panel/Makefile
> +++ b/drivers/gpu/drm/panel/Makefile
> @@ -1,3 +1,4 @@
>  obj-$(CONFIG_DRM_PANEL_SIMPLE) += panel-simple.o
>  obj-$(CONFIG_DRM_PANEL_LD9040) += panel-ld9040.o
>  obj-$(CONFIG_DRM_PANEL_S6E8AA0) += panel-s6e8aa0.o
> +obj-$(CONFIG_DRM_PANEL_S6E3FA0) += panel-s6e3fa0.o
> diff --git a/drivers/gpu/drm/panel/panel-s6e3fa0.c 
> b/drivers/gpu/drm/panel/panel-s6e3fa0.c
> new file mode 100644
> index 000..1282678
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-s6e3fa0.c
> @@ -0,0 +1,569 @@
> +/*
> + * MIPI-DSI based s6e3fa0 AMOLED LCD 5.7 inch panel driver.
> + *
> + * Copyright (c) 2014 Samsung Electronics Co., Ltd
> + *
> + * YoungJun Cho 
> + *
> + * 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.
> +*/
> +
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 


#include  should be enough.


> +#include 
> +
> +#include 
> +#include 
> +#include 
> +
> +#define MTP_ID_LEN   3
> +#define GAMMA_LEVEL_NUM  30
> +
> +struct s6e3fa0 {
> + struct device   *dev;
> + struct drm_panelpanel;
> +
> + struct regulator_bulk_data  supplies[2];
> + struct gpio_desc*reset_gpio;
> + struct gpio_desc*det_gpio;
> + struct gpio_desc*te_gpio;
> + struct videomodevm;
> + struct drm_panel_cpu_timingscpu_timings;
> +
> + unsigned intpower_on_delay;
> + unsigned intreset_delay;
> + unsigned intinit_delay;
> + unsigned intwidth_mm;
> + unsigned intheight_mm;
> +
> + unsigned char   id;
> + unsigned char   vddm;
> + unsigned intbrightness;
> +};
> +
> +#define panel_to_s6e3fa0(p) container_of(p, struct s6e3fa0, panel)
> +
> +static const unsigned char s6e3fa0_vddm_lut[][2] = {
> + {0x00, 0x0d}, {0x01, 0x0d}, {0x02, 0x0e}, {0x03, 0x0f}, {0x04, 0x10},
> + {0x05, 0x11}, {0x06, 0x12}, {0x07, 0x13}, {0x08, 0x14}, {0x09, 0x15},
> + {0x0a, 0x16}, {0x0b, 0x17}, {0x0c, 0x18}, {0x0d, 0x19}, {0x0e, 0x1a},
> + {0x0f, 0x1b}, {0x10, 0x1c}, {0x11, 0x1d}, {0x12, 0x1e}, {0x13, 0x1f},
> + {0x14, 0x20}, {0x15, 0x21}, {0x16, 0x22}, {0x17, 0x23}, {0x18, 0x24},
> + {0x19, 0x25}, {0x1a, 0x26}, {0x1b, 0x27}, {0x1c, 0x28}, {0x1d, 0x29},
> + {0x1e, 0x2a}, {0x1f, 0x2b}, {0x20, 0x2c}, {0x21, 0x2d}, {0x22, 0x2e},
> + {0x23, 0x2f}, {0x24, 0x30}, {0x25, 0x31}, {0x26, 0x32}, {0x27, 0x33},
> + {0x28, 0x34}, {0x29, 0x35}, {0x2a, 0x36}, {0x2b, 0x37}, {0x2c, 0x38},
> + {0x2d, 0x39}, {0x2e, 0x3a}, {0x2f, 0x3b}, {0x30, 0x3c}, {0x31, 0x3d},
> + {0x32, 0x3e}, {0x33, 0x3f}, {0x34, 0x3f}, {0x35, 0x3f}, {0x36, 0x3f},
> + {0x37, 0x3f}, {0x38, 0x3f}, {0x39, 0x3f}, {0x3a, 0x3f}, {0x3b, 0x3f},
> + {0x3c, 0x3f}, {0x3d, 0x3f}, {0x3e, 0x3f}, {0x3f, 0x3f}, {0x40, 0x0c},
> + {0x41, 0x0b}, {0x42, 0x0a}, {0x43, 0x09}, {0x44, 0x08}, {0x45, 0x07},
> + {0x46, 0x06}, {0x47, 0x05}, {0x48, 0x04}, {0x49, 0x03}, {0x4a, 0x02},
> + {0x4b, 0x01}, {0x4c, 0x40}, {0x4d, 0x41}, {0x4e, 0x42}, {0x4f, 0x43},
> + {0x50, 0x44}, {0x51, 0x45}, {0x52, 0x46}, {0x53, 0x47}, {0x54, 0x48},
> + {0x55, 0x49}, {0x56, 0x4a}, {0x57, 0x4b}, {0x58, 0x4c}, {0x59, 0x4d},
> + {0x5a, 0x4e}, {0x5b, 0x4f}, {0x5c, 0x50}, {0x5d, 0x51}, {0x5e, 0x52},
> + {0x5f, 0x53}, {0x60, 0x54}, {0x61, 0x55}, {0x62, 

[Bug 74401] Radeon r600 has a pink-green tint

[bugzilla-dae...@bugzilla.kernel.org]

https://bugzilla.kernel.org/show_bug.cgi?id=74401

Ja?a Bartelj  changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |OBSOLETE

--- Comment #4 from Ja?a Bartelj  ---
The green tint is no longer present in today's
kernel-3.15.0-0.rc2.git0.1.fc21.x86_64

$ rpm -qi kernel-3.15.0-0.rc2.git0.1.fc21.x86_64
Name: kernel
Version : 3.15.0
Release : 0.rc2.git0.1.fc21
Architecture: x86_64
Install Date: wed 23 apr 2014 10:30:32 CEST
Group   : System Environment/Kernel
Size: 143310038
License : GPLv2 and Redistributable, no modification permitted
Signature   : (none)
Source RPM  : kernel-3.15.0-0.rc2.git0.1.fc21.src.rpm
Build Date  : pon 21 apr 2014 16:29:30 CEST
Build Host  : bkernel02
Relocations : (not relocatable)
Packager: Fedora Project
Vendor  : Fedora Project
URL : http://www.kernel.org/
Summary : The Linux kernel

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

[Intel-gfx] [PATCH 1/3] drm: Check CRTC compatibility in setplane

[Matt Roper]

On Wed, Apr 23, 2014 at 08:03:50PM +0200, Daniel Vetter wrote:
> On Wed, Apr 23, 2014 at 10:03:59AM -0700, Matt Roper wrote:
> > The DRM core setplane code should check that the plane is usable on the
> > specified CRTC before calling into the driver.
> > 
> > Prior to this patch, a plane's possible_crtcs field was purely
> > informational for userspace and was never actually verified at the
> > kernel level (aside from the primary plane helper).
> > 
> > Cc: dri-devel at lists.freedesktop.org
> > Signed-off-by: Matt Roper 
> 
> Do you have a nasty igt somewhere which tries to use a plane on the wrong
> crtc? Especially useful since our i915 code and hw relies on this.
> -Daniel

Not yet; I'll add/modify a test to include that.  I'm still working on
some other i-g-t test updates for the primary plane stuff based on your
previous feedback.

Speaking of i-g-t testing, what is the expected behavior if someone
issues a pageflip ioctl while the primary plane is disabled?  I'd expect
it to return an error, but the -EBUSY currently returned by the DRM core
seems a bit confusing/misleading in my opinion.  The comments indicate
that the existing assumption is that crtc->primary->fb == NULL indicates 
a hotplug event that userspace hasn't noticed yet, although now we
clearly have other ways to hit that error, so I'm not sure EBUSY is
really the right response.


Matt

> 
> > ---
> >  drivers/gpu/drm/drm_crtc.c | 7 +++
> >  drivers/gpu/drm/drm_plane_helper.c | 6 --
> >  2 files changed, 7 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> > index 461d19b..b6d6c04 100644
> > --- a/drivers/gpu/drm/drm_crtc.c
> > +++ b/drivers/gpu/drm/drm_crtc.c
> > @@ -2140,6 +2140,13 @@ int drm_mode_setplane(struct drm_device *dev, void 
> > *data,
> > }
> > crtc = obj_to_crtc(obj);
> >  
> > +   /* Check whether this plane is usable on this CRTC */
> > +   if (!(plane->possible_crtcs & drm_crtc_mask(crtc))) {
> > +   DRM_DEBUG_KMS("Invalid crtc for plane\n");
> > +   ret = -EINVAL;
> > +   goto out;
> > +   }
> > +
> > fb = drm_framebuffer_lookup(dev, plane_req->fb_id);
> > if (!fb) {
> > DRM_DEBUG_KMS("Unknown framebuffer ID %d\n",
> > diff --git a/drivers/gpu/drm/drm_plane_helper.c 
> > b/drivers/gpu/drm/drm_plane_helper.c
> > index b72736d..65c4a00 100644
> > --- a/drivers/gpu/drm/drm_plane_helper.c
> > +++ b/drivers/gpu/drm/drm_plane_helper.c
> > @@ -137,12 +137,6 @@ int drm_primary_helper_update(struct drm_plane *plane, 
> > struct drm_crtc *crtc,
> > return -EINVAL;
> > }
> >  
> > -   /* Primary planes are locked to their owning CRTC */
> > -   if (plane->possible_crtcs != drm_crtc_mask(crtc)) {
> > -   DRM_DEBUG_KMS("Cannot change primary plane CRTC\n");
> > -   return -EINVAL;
> > -   }
> > -
> > /* Disallow scaling */
> > src_w >>= 16;
> > src_h >>= 16;
> > -- 
> > 1.8.5.1
> > 
> > ___
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795

[PATCH v2 04/10] drm/nouveau/fb: add GK20A support

[Alexandre Courbot]

On 04/22/2014 07:40 PM, Thierry Reding wrote:
> * PGP Signed by an unknown key
>
> On Mon, Apr 21, 2014 at 03:02:16PM +0900, Alexandre Courbot wrote:
> [...]
>> diff --git a/drivers/gpu/drm/nouveau/core/subdev/fb/ramgk20a.c 
>> b/drivers/gpu/drm/nouveau/core/subdev/fb/ramgk20a.c
> [...]
>> +pages = dma_alloc_from_contiguous(dev, ncmin, order);
>> +if (!pages) {
>> +gk20a_ram_put(pfb, );
>> +return -ENOMEM;
>> +}
>> +
>> +dma_addr = pfn_to_dma(nv_device_base(nv_device(pfb)),
>> +  page_to_pfn(pages));
>
> This breaks compilation on x86 because neither pfn_to_dma() nor
> dma_to_pfn() are available. Is there some other way this can be
> allocated so that these functions don't need to be called?

Mmm, this is bad. There is probably another more portable way to do 
this. Let me look for it.

Thanks,
Alex.

[RFC v2 PATCH v4 09/14] ARM: dts: s6e3fa0: add DT bindings

[Andrzej Hajda]

On 04/21/2014 02:28 PM, YoungJun Cho wrote:
> This patch adds DT bindings for s6e3fa0 panel.
> The bindings describes panel resources, display timings and cpu timings.
> 
> Changelog v2:
> - Adds unit address (commented by Sachin Kamat)
> Changelog v3:
> - Removes optional delay, size properties (commented by Laurent Pinchart)
> - Adds OLED detection, TE gpio properties
> Changelog v4:
> - Moves CPU timings relevant properties from FIMD DT
>   (commeted by Laurent Pinchart, Andrzej Hajda)
> 
> Signed-off-by: YoungJun Cho 
> Acked-by: Inki Dae 
> Acked-by: Kyungmin Park 
> ---
>  .../devicetree/bindings/panel/samsung,s6e3fa0.txt  |   63 
> 
>  1 file changed, 63 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt
> 
> diff --git a/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt 
> b/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt
> new file mode 100644
> index 000..9eeb38b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt
> @@ -0,0 +1,63 @@
> +Samsung S6E3FA0 AMOLED LCD 5.7 inch panel
> +
> +Required properties:
> +  - compatible: "samsung,s6e3fa0"
> +  - reg: the virtual channel number of a DSI peripheral
> +  - vdd3-supply: core voltage supply
> +  - vci-supply: voltage supply for analog circuits
> +  - reset-gpio: a GPIO spec for the reset pin
> +  - det-gpio: a GPIO spec for the OLED detection pin
> +  - te-gpio: a GPIO spec for the TE pin
> +  - display-timings: timings for the connected panel as described by [1]

Which properties of display-timings are relevant for CPU mode?
I guess width and height. Anything more?

> +  - cpu-timings: CPU interface timings for the connected panel, and it 
> contains
> +following optional properties.
> +  - cs-setup: clock cycles for the active period of address signal
> +enable until chip select is enable in CPU interface
> +  - wr-setup: clock cycles for the active period of CS signal enable
> +until write signal is enable in CPU interface
> +  - wr-act: clock cycles for the active period of CS enable in CPU
> +interface
> +  - wr-hold: clock cycles for the active period of CS disable until
> +write signal is disable in CPU interface

cpu-timings name does not sound to be related to display.
I wonder if it would not be better to merge cpu-timings into
display-timings but this will require more discussion I guess.

If you want to stay with separate node please consider to make it
optional as whole node or make some properties required. Making node
required with all sub-properties optional is weird.
By the way I hope all timings properties are generic for CPU mode,
if not they should be prefixed by vendor or model.

Regards
Andrzej

> +
> +Optional properties:
> +
> +The device node can contain one 'port' child node with one child
> +'endpoint' node, according to the bindings defined in [2]. This
> +node should describe panel's video bus.
> +
> +[1]: Documentation/devicetree/bindings/video/display-timing.txt
> +[2]: Documentation/devicetree/bindings/media/video-interfaces.txt
> +
> +Example:
> +
> + panel at 0 {
> + compatible = "samsung,s6e3fa0";
> + reg = <0>;
> + vdd3-supply = <_reg>;
> + vci-supply = <_reg>;
> + reset-gpio = < 4 0>;
> + det-gpio = < 6 0>;
> + te-gpio = < 7 0>;
> +
> + display-timings {
> + timing0: timing-0 {
> + clock-frequency = <0>;
> + hactive = <1080>;
> + vactive = <1920>;
> + hfront-porch = <2>;
> + hback-porch = <2>;
> + hsync-len = <1>;
> + vfront-porch = <1>;
> + vback-porch = <4>;
> + vsync-len = <1>;
> + };
> + };
> +
> + cpu-timings {
> + cs-setup = <0>;
> + wr-setup = <0>;
> + wr-act = <1>;
> + wr-hold = <0>;
> + };
> + };
> 

[RFC 4/5] drm: Introduce drm_set_unique()

[Daniel Vetter]

On Wed, Apr 23, 2014 at 09:17:16AM +0200, Thierry Reding wrote:
> On Tue, Apr 22, 2014 at 05:48:07PM +0200, Daniel Vetter wrote:
> > On Tue, Apr 22, 2014 at 05:09:32PM +0200, Thierry Reding wrote:
> > > From: Thierry Reding 
> > > 
> > > Add a helper function that allows drivers to statically set the unique
> > > name of the device. This will allow platform and USB drivers to get rid
> > > of their DRM bus implementations and directly use drm_dev_alloc() and
> > > drm_dev_register().
> > > 
> > > Signed-off-by: Thierry Reding 
> > > ---
> > >  drivers/gpu/drm/drm_ioctl.c | 37 +++--
> > >  drivers/gpu/drm/drm_stub.c  |  1 +
> > >  include/drm/drmP.h  |  3 +++
> > >  3 files changed, 35 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> > > index 2dd3a6d8382b..371db3bef60c 100644
> > > --- a/drivers/gpu/drm/drm_ioctl.c
> > > +++ b/drivers/gpu/drm/drm_ioctl.c
> > > @@ -42,6 +42,20 @@
> > >  #include 
> > >  #endif
> > >  
> > > +int drm_set_unique(struct drm_device *dev, const char *fmt, ...)
> > 
> > Can you please add a bit of kerneldoc for this? drm_ioctl.c isn't yet
> > pulled into the drm reference docbook, but better to have it there
> > already.
> 
> On second thought, wouldn't this be better located in drm_stub.c? It
> isn't really related to the IOCTL code except that one of the IOCTLs now
> uses the information set by this function. Logically I think it belongs
> with the likes of drm_dev_alloc() and drm_dev_register().

Yeah makes sense. Tbh the entire split-up of these core drm functions is
still a bit messy, so I don't mind if it's a bit inconsistent really. We
can do the suffling when someone bothers with the kerneldoc for all of
them and pulls it into the drm docbook.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH] drm/irq: track the irq installed in drm_irq_install in dev->irq

[Daniel Vetter]

On Wed, Apr 23, 2014 at 09:27:58AM +0200, Thierry Reding wrote:
> On Tue, Apr 22, 2014 at 10:44:20PM +0200, Daniel Vetter wrote:
> [...]
> > diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> > index 589e865832cd..7cf407bbfed5 100644
> > --- a/drivers/gpu/drm/drm_irq.c
> > +++ b/drivers/gpu/drm/drm_irq.c
> > @@ -249,14 +249,16 @@ static inline int drm_dev_to_irq(struct drm_device 
> > *dev)
> >   */
> >  int drm_irq_install(struct drm_device *dev)
> >  {
> > -   int ret;
> > +   int ret, irq;
> > unsigned long sh_flags = 0;
> > char *irqname;
> >  
> > +   irq = drm_dev_to_irq(dev);
> 
> I think the assignment could have happened either when the variable is
> declared, or...
> 
> > +
> > if (!drm_core_check_feature(dev, DRIVER_HAVE_IRQ))
> > return -EINVAL;
> >  
> > -   if (drm_dev_to_irq(dev) == 0)
> > +   if (irq == 0)
> 
> ... right above this, since it is where it is first used (it may not be
> necessary to query it before here at all if the driver doesn't set
> DRIVER_HAVE_IRQ).
> 
> But I realize that that's pure bike-shedding, so either way:

Follow-on patches will move this assignement into drivers and make int irq
an function parameter, so I think I'll leave this ;-)
> 
> Reviewed-by: Thierry Reding 

Thanks for the review, I'll send the pull request to Dave now.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH 2/2] drm: Handle ->disable_plane failures correctly

[Daniel Vetter]

The ->disable_plane hook always had a return value, but only since the
introduction of primary planes was there any implementation that
actually failed.

So handle such failures correctly.

Note that drm_plane_force_disable is special: In the modeset cleanup
case we first disable all crtc, so primary planes should all be freed
already. And in the fb helper we only reset non-primary planes. Still
better be paranoid and add an early return.

I don't see how this could happen, but it might fix the fb refcount
underrun Thierry is seeing. Matt Roper spotted this issue.

Cc: Thierry Reding 
Cc: Ville Syrj?l? 
Cc: Matt Roper 
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/drm_crtc.c | 14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index f6633cb927bc..461d19bd14ee 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -1152,8 +1152,10 @@ void drm_plane_force_disable(struct drm_plane *plane)
return;

ret = plane->funcs->disable_plane(plane);
-   if (ret)
+   if (ret) {
DRM_ERROR("failed to disable plane with busy fb\n");
+   return;
+   }
/* disconnect the plane from the fb and crtc: */
__drm_framebuffer_unreference(old_fb);
plane->fb = NULL;
@@ -2117,9 +2119,13 @@ int drm_mode_setplane(struct drm_device *dev, void *data,
if (!plane_req->fb_id) {
drm_modeset_lock_all(dev);
old_fb = plane->fb;
-   plane->funcs->disable_plane(plane);
-   plane->crtc = NULL;
-   plane->fb = NULL;
+   ret = plane->funcs->disable_plane(plane);
+   if (!ret) {
+   plane->crtc = NULL;
+   plane->fb = NULL;
+   } else {
+   old_fb = NULL;
+   }
drm_modeset_unlock_all(dev);
goto out;
}
-- 
1.9.2

[PATCH 1/2] drm: Simplify fb refcounting rules around ->update_plane

[Daniel Vetter]

The introduction of primary planes has apparently caused a bit of fb
refcounting fun for people. That makes it a good time to clean up the
arcane rules and slight differences between ->update_plane and
->set_config. The new rules are:

- The core holds a reference for both the new and the old fb (if
  they're non-NULL of course) while calling into the driver through
  either ->update_plane or ->set_config.

- Drivers may not clobber plane->fb if their callback fails. If they
  do that, they need to store a pointer to the old fb in it again.
  When calling into the driver plane->fb still points at the current
  (old) framebuffer.

- The core will update the plane->fb pointer on success. Drivers can
  do that themselves too, but aren't required to any more for the
  primary plane.

- The core will update fb refcounts for the plane->fb pointer,
  presuming the drivers hold up their end of the bargain.

v2: Remove now unused tmpfb (Thierry)

v3: Drop broken changes from drm_mode_setplane (Ville). Also polish
the commit message a bit.

v4: Also fix up the handling of ->disable_plane in
drm_plane_force_disable. The issue was that we didn't save plane->fb
over the ->disable_plane call. Just paranoia, nothing relies on this.

Cc: Thierry Reding 
Cc: Ville Syrj?l? 
Cc: Matt Roper 
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/drm_crtc.c | 13 +++--
 drivers/gpu/drm/drm_plane_helper.c | 16 
 2 files changed, 7 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index d8b7099abece..f6633cb927bc 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -1145,16 +1145,17 @@ EXPORT_SYMBOL(drm_plane_cleanup);
  */
 void drm_plane_force_disable(struct drm_plane *plane)
 {
+   struct drm_framebuffer *old_fb = plane->fb;
int ret;

-   if (!plane->fb)
+   if (!old_fb)
return;

ret = plane->funcs->disable_plane(plane);
if (ret)
DRM_ERROR("failed to disable plane with busy fb\n");
/* disconnect the plane from the fb and crtc: */
-   __drm_framebuffer_unreference(plane->fb);
+   __drm_framebuffer_unreference(old_fb);
plane->fb = NULL;
plane->crtc = NULL;
 }
@@ -2187,16 +2188,18 @@ int drm_mode_setplane(struct drm_device *dev, void 
*data,
}

drm_modeset_lock_all(dev);
+   old_fb = plane->fb;
ret = plane->funcs->update_plane(plane, crtc, fb,
 plane_req->crtc_x, plane_req->crtc_y,
 plane_req->crtc_w, plane_req->crtc_h,
 plane_req->src_x, plane_req->src_y,
 plane_req->src_w, plane_req->src_h);
if (!ret) {
-   old_fb = plane->fb;
plane->crtc = crtc;
plane->fb = fb;
fb = NULL;
+   } else {
+   old_fb = NULL;
}
drm_modeset_unlock_all(dev);

@@ -2239,9 +2242,7 @@ int drm_mode_set_config_internal(struct drm_mode_set *set)
ret = crtc->funcs->set_config(set);
if (ret == 0) {
crtc->primary->crtc = crtc;
-
-   /* crtc->fb must be updated by ->set_config, enforces this. */
-   WARN_ON(fb != crtc->primary->fb);
+   crtc->primary->fb = fb;
}

list_for_each_entry(tmp, >dev->mode_config.crtc_list, head) {
diff --git a/drivers/gpu/drm/drm_plane_helper.c 
b/drivers/gpu/drm/drm_plane_helper.c
index 9540ff9f97fe..b72736d5541d 100644
--- a/drivers/gpu/drm/drm_plane_helper.c
+++ b/drivers/gpu/drm/drm_plane_helper.c
@@ -124,7 +124,6 @@ int drm_primary_helper_update(struct drm_plane *plane, 
struct drm_crtc *crtc,
.y2 = crtc->mode.vdisplay,
};
struct drm_connector **connector_list;
-   struct drm_framebuffer *tmpfb;
int num_connectors, ret;

if (!crtc->enabled) {
@@ -177,22 +176,7 @@ int drm_primary_helper_update(struct drm_plane *plane, 
struct drm_crtc *crtc,
set.connectors = connector_list;
set.num_connectors = num_connectors;

-   /*
-* set_config() adjusts crtc->primary->fb; however the DRM setplane
-* code that called us expects to handle the framebuffer update and
-* reference counting; save and restore the current fb before
-* calling it.
-*
-* N.B., we call set_config() directly here rather than using
-* drm_mode_set_config_internal.  We're reprogramming the same
-* connectors that were already in use, so we shouldn't need the extra
-* cross-CRTC fb refcounting to accomodate stealing connectors.
-* drm_mode_setplane() already handles the basic refcounting for the
-* framebuffers involved in this operation.
-*/
-   tmpfb = plane->fb;
ret = crtc->funcs->set_config();
-   plane->fb = tmpfb;

[RFC v2 PATCH 08/14] drm/exynos: dsi: add driver data to support Exynos5420

[Andrzej Hajda]

On 04/21/2014 02:28 PM, YoungJun Cho wrote:
> The offset of register DSIM_PLLTMR_REG in Exynos5420 is different
> from the one in Exynos4 SoC.
>
> In case of Exynos5420 SoC, there is no frequency band bit in DSIM_PLLCTRL_REG,
> and it uses DSIM_PHYCTRL_REG and DSIM_PHYTIMING*_REG instead.
> So this patch adds driver data to distinguish it.
>
> Signed-off-by: YoungJun Cho 
> Acked-by: Inki Dae 
> Acked-by: Kyungmin Park 
> ---
>  drivers/gpu/drm/exynos/exynos_drm_dsi.c |  101 
> ---
>  1 file changed, 80 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c 
> b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> index 179f2fa..fcd577f 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> @@ -17,6 +17,7 @@
>  
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  
> @@ -54,9 +55,12 @@
>  
>  /* FIFO memory AC characteristic register */
>  #define DSIM_PLLCTRL_REG 0x4c/* PLL control register */
> -#define DSIM_PLLTMR_REG  0x50/* PLL timer register */
>  #define DSIM_PHYACCHR_REG0x54/* D-PHY AC characteristic register */
>  #define DSIM_PHYACCHR1_REG   0x58/* D-PHY AC characteristic register1 */
> +#define DSIM_PHYCTRL_REG 0x5c
> +#define DSIM_PHYTIMING_REG   0x64
> +#define DSIM_PHYTIMING1_REG  0x68
> +#define DSIM_PHYTIMING2_REG  0x6c
>  
>  /* DSIM_STATUS */
>  #define DSIM_STOP_STATE_DAT(x)   (((x) & 0xf) << 0)
> @@ -233,6 +237,12 @@ struct exynos_dsi_transfer {
>  #define DSIM_STATE_INITIALIZED   BIT(1)
>  #define DSIM_STATE_CMD_LPM   BIT(2)
>  
> +struct exynos_dsi_driver_data {
> + unsigned int plltmr_reg;
> +
> + unsigned int has_freqband:1;
> +};
> +
>  struct exynos_dsi {
>   struct mipi_dsi_host dsi_host;
>   struct drm_connector connector;
> @@ -262,11 +272,39 @@ struct exynos_dsi {
>  
>   spinlock_t transfer_lock; /* protects transfer_list */
>   struct list_head transfer_list;
> +
> + struct exynos_dsi_driver_data *driver_data;
>  };
>  
>  #define host_to_dsi(host) container_of(host, struct exynos_dsi, dsi_host)
>  #define connector_to_dsi(c) container_of(c, struct exynos_dsi, connector)
>  
> +static struct exynos_dsi_driver_data exynos4_dsi_driver_data = {
> + .plltmr_reg = 0x50,
> + .has_freqband = 1,
> +};
> +
> +static struct exynos_dsi_driver_data exynos5_dsi_driver_data = {
> + .plltmr_reg = 0x58,
> +};
> +
> +static struct of_device_id exynos_dsi_of_match[] = {
> + { .compatible = "samsung,exynos4210-mipi-dsi",
> +   .data = _dsi_driver_data },
> + { .compatible = "samsung,exynos5420-mipi-dsi",
> +   .data = _dsi_driver_data },
> + { }
> +};

I wonder if it wouldn't be better to use "samsung,exynos5410-mipi-dsi"
compatible and distinguish 5410 and 5420 by DSIM_VERSION register.


> +
> +static inline struct exynos_dsi_driver_data *exynos_dsi_get_driver_data(
> + struct platform_device *pdev)
> +{
> + const struct of_device_id *of_id =
> + of_match_device(exynos_dsi_of_match, >dev);
> +
> + return (struct exynos_dsi_driver_data *)of_id->data;
> +}
> +
>  static void exynos_dsi_wait_for_reset(struct exynos_dsi *dsi)
>  {
>   if (wait_for_completion_timeout(>completed, msecs_to_jiffies(300)))
> @@ -340,14 +378,9 @@ static unsigned long exynos_dsi_pll_find_pms(struct 
> exynos_dsi *dsi,
>  static unsigned long exynos_dsi_set_pll(struct exynos_dsi *dsi,
>   unsigned long freq)
>  {
> - static const unsigned long freq_bands[] = {
> - 100 * MHZ, 120 * MHZ, 160 * MHZ, 200 * MHZ,
> - 270 * MHZ, 320 * MHZ, 390 * MHZ, 450 * MHZ,
> - 510 * MHZ, 560 * MHZ, 640 * MHZ, 690 * MHZ,
> - 770 * MHZ, 870 * MHZ, 950 * MHZ,
> - };
> + struct exynos_dsi_driver_data *driver_data = dsi->driver_data;
>   unsigned long fin, fout;
> - int timeout, band;
> + int timeout;
>   u8 p, s;
>   u16 m;
>   u32 reg;
> @@ -368,18 +401,30 @@ static unsigned long exynos_dsi_set_pll(struct 
> exynos_dsi *dsi,
>   "failed to find PLL PMS for requested frequency\n");
>   return -EFAULT;
>   }
> + dev_dbg(dsi->dev, "PLL freq %lu, (p %d, m %d, s %d)\n", fout, p, m, s);
>  
> - for (band = 0; band < ARRAY_SIZE(freq_bands); ++band)
> - if (fout < freq_bands[band])
> - break;
> + writel(500, dsi->reg_base + driver_data->plltmr_reg);
> +
> + reg = DSIM_PLL_EN | DSIM_PLL_P(p) | DSIM_PLL_M(m) | DSIM_PLL_S(s);
> +
> + if (driver_data->has_freqband) {
> + static const unsigned long freq_bands[] = {
> + 100 * MHZ, 120 * MHZ, 160 * MHZ, 200 * MHZ,
> + 270 * MHZ, 320 * MHZ, 390 * MHZ, 450 * MHZ,
> + 510 * MHZ, 560 * MHZ, 640 * MHZ, 690 * MHZ,
> +   

[RFC v2 PATCH v4 09/14] ARM: dts: s6e3fa0: add DT bindings

[YoungJun Cho]

Hi Andrzej

Thank you for comment.

On 04/22/2014 11:02 PM, Andrzej Hajda wrote:
> On 04/21/2014 02:28 PM, YoungJun Cho wrote:
>> This patch adds DT bindings for s6e3fa0 panel.
>> The bindings describes panel resources, display timings and cpu timings.
>>
>> Changelog v2:
>> - Adds unit address (commented by Sachin Kamat)
>> Changelog v3:
>> - Removes optional delay, size properties (commented by Laurent Pinchart)
>> - Adds OLED detection, TE gpio properties
>> Changelog v4:
>> - Moves CPU timings relevant properties from FIMD DT
>>(commeted by Laurent Pinchart, Andrzej Hajda)
>>
>> Signed-off-by: YoungJun Cho 
>> Acked-by: Inki Dae 
>> Acked-by: Kyungmin Park 
>> ---
>>   .../devicetree/bindings/panel/samsung,s6e3fa0.txt  |   63 
>> 
>>   1 file changed, 63 insertions(+)
>>   create mode 100644 
>> Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt
>>
>> diff --git a/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt 
>> b/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt
>> new file mode 100644
>> index 000..9eeb38b
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt
>> @@ -0,0 +1,63 @@
>> +Samsung S6E3FA0 AMOLED LCD 5.7 inch panel
>> +
>> +Required properties:
>> +  - compatible: "samsung,s6e3fa0"
>> +  - reg: the virtual channel number of a DSI peripheral
>> +  - vdd3-supply: core voltage supply
>> +  - vci-supply: voltage supply for analog circuits
>> +  - reset-gpio: a GPIO spec for the reset pin
>> +  - det-gpio: a GPIO spec for the OLED detection pin
>> +  - te-gpio: a GPIO spec for the TE pin
>
> Just FYI, according to DT documentation [1] gpio spec should be in form
> [name]-gpios, however there is plenty bindings with -gpio suffix, so I
> am not sure if it is really enforced. On the other side it is enforced
> by descriptor based gpio framework[2]. Integer-based gpio framework
> used in your driver is obsolete according to [2].

Yes, you're right. That is my mistake.
They should be attached 's'.
At first I used integer-based gpio framework and replaced to descriptor
based one, but did not updated DT bindings.

I'll update again.

Thank you.
Best regards YJ

>
> [1]: Documentation/devicetree/bindings/gpio/gpio.txt
> [2]: Documentation/gpio/gpio.txt
>
> Regards
> Andrzej
>
>> +  - display-timings: timings for the connected panel as described by [1]
>> +  - cpu-timings: CPU interface timings for the connected panel, and it 
>> contains
>> +following optional properties.
>> +  - cs-setup: clock cycles for the active period of address signal
>> +enable until chip select is enable in CPU interface
>> +  - wr-setup: clock cycles for the active period of CS signal enable
>> +until write signal is enable in CPU interface
>> +  - wr-act: clock cycles for the active period of CS enable in CPU
>> +interface
>> +  - wr-hold: clock cycles for the active period of CS disable until
>> +write signal is disable in CPU interface
>> +
>> +Optional properties:
>> +
>> +The device node can contain one 'port' child node with one child
>> +'endpoint' node, according to the bindings defined in [2]. This
>> +node should describe panel's video bus.
>> +
>> +[1]: Documentation/devicetree/bindings/video/display-timing.txt
>> +[2]: Documentation/devicetree/bindings/media/video-interfaces.txt
>> +
>> +Example:
>> +
>> +panel at 0 {
>> +compatible = "samsung,s6e3fa0";
>> +reg = <0>;
>> +vdd3-supply = <_reg>;
>> +vci-supply = <_reg>;
>> +reset-gpio = < 4 0>;
>> +det-gpio = < 6 0>;
>> +te-gpio = < 7 0>;
>> +
>> +display-timings {
>> +timing0: timing-0 {
>> +clock-frequency = <0>;
>> +hactive = <1080>;
>> +vactive = <1920>;
>> +hfront-porch = <2>;
>> +hback-porch = <2>;
>> +hsync-len = <1>;
>> +vfront-porch = <1>;
>> +vback-porch = <4>;
>> +vsync-len = <1>;
>> +};
>> +};
>> +
>> +cpu-timings {
>> +cs-setup = <0>;
>> +wr-setup = <0>;
>> +wr-act = <1>;
>> +wr-hold = <0>;
>> +};
>> +};
>>
>
>

[RFC v2 PATCH v2 06/14] drm/exynos: support MIPI DSI command mode

[YoungJun Cho]

Hi Thierry

Thank you for the comments.

On 04/22/2014 04:34 PM, Thierry Reding wrote:
> On Mon, Apr 21, 2014 at 09:28:33PM +0900, YoungJun Cho wrote:
> [...]
>> diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
>> index 7209df1..244d197 100644
>> --- a/include/drm/drm_mipi_dsi.h
>> +++ b/include/drm/drm_mipi_dsi.h
>> @@ -49,6 +49,7 @@ struct mipi_dsi_msg {
>>* @detach: detach DSI device from DSI host
>>* @transfer: send and/or receive DSI packet, return number of received 
>> bytes,
>>*   or error
>> + * @te_handler: call CRTC TE handler callback from DSI host
>
> Perhaps you can explain some more what this means or why it would be
> necessary.

I'll enhance the explanation.

For your information, the panel generates Tearing Effect signal
to synchronize between the MCU and Frame Memory Writing when displays
video images.
The display controller requires this signal to trigger video images,
and there is no way to receive this signal directly.
So the panel receive this signal as an IRQ and calls te_handler() chain
for the display controller to trigger video image at that time.

>
>> diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
>> index c2ab77a..0ad64ed 100644
>> --- a/include/drm/drm_panel.h
>> +++ b/include/drm/drm_panel.h
>> @@ -46,6 +46,13 @@ struct drm_panel {
>>  struct list_head list;
>>   };
>>
>> +struct drm_panel_cpu_timings {
>> +unsigned int cs_setup;
>> +unsigned int wr_setup;
>> +unsigned int wr_act;
>> +unsigned int wr_hold;
>> +};
>
> Similarily here. What's this?

I'll enhance the explanation also.
In panel DT bindings, I already explained them.

Thank you.
Best regards YJ

>
> Thierry
>

[PATCH 2/3] drm/plane-helper: Add drm_primary_helper_check_update()

[Matt Roper]

Pull the parameter checking from drm_primary_helper_update() out into
its own function; drivers that provide their own setplane()
implementations rather than using the helper may still want to share
this parameter checking logic.

A few of the checks here were also updated based on suggestions by
Ville Syrj?l?.

Cc: Ville Syrj?l? 
Cc: dri-devel at lists.freedesktop.org
Signed-off-by: Matt Roper 
---
 drivers/gpu/drm/drm_plane_helper.c | 148 +
 include/drm/drm_plane_helper.h |   9 +++
 2 files changed, 128 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/drm_plane_helper.c 
b/drivers/gpu/drm/drm_plane_helper.c
index 65c4a00..9bbbdf2 100644
--- a/drivers/gpu/drm/drm_plane_helper.c
+++ b/drivers/gpu/drm/drm_plane_helper.c
@@ -66,6 +66,102 @@ static int get_connectors_for_crtc(struct drm_crtc *crtc,
 }

 /**
+ * drm_primary_helper_check_update() - Check primary plane update for validity
+ * @plane: plane object to update
+ * @crtc: owning CRTC of owning plane
+ * @fb: framebuffer to flip onto plane
+ * @crtc_x: x offset of primary plane on crtc
+ * @crtc_y: y offset of primary plane on crtc
+ * @crtc_w: width of primary plane rectangle on crtc
+ * @crtc_h: height of primary plane rectangle on crtc
+ * @src_x: x offset of @fb for panning
+ * @src_y: y offset of @fb for panning
+ * @src_w: width of source rectangle in @fb
+ * @src_h: height of source rectangle in @fb
+ * @can_scale: is primary plane scaling legal?
+ * @can_position: is it legal to position the primary plane such that it
+ *doesn't cover the entire crtc?
+ *
+ * Checks that a desired primary plane update is valid.  Drivers that provide
+ * their own primary plane handling may still wish to call this function to
+ * avoid duplication of error checking code.
+ *
+ * RETURNS:
+ * Zero if update appears valid, error code on failure
+ */
+int drm_primary_helper_check_update(struct drm_plane *plane,
+   struct drm_crtc *crtc,
+   struct drm_framebuffer *fb,
+   int crtc_x, int crtc_y,
+   unsigned int crtc_w, unsigned int crtc_h,
+   uint32_t src_x, uint32_t src_y,
+   uint32_t src_w, uint32_t src_h,
+   bool can_scale,
+   bool can_position)
+{
+   struct drm_rect dest = {
+   .x1 = crtc_x,
+   .y1 = crtc_y,
+   .x2 = crtc_x + crtc_w,
+   .y2 = crtc_y + crtc_h,
+   };
+   struct drm_rect src = {
+   .x1 = src_x >> 16,
+   .y1 = src_y >> 16,
+   .x2 = (src_x + src_w) >> 16,
+   .y2 = (src_y + src_h) >> 16,
+   };
+   const struct drm_rect clip = {
+   .x2 = crtc->mode.hdisplay,
+   .y2 = crtc->mode.vdisplay,
+   };
+   int hscale, vscale;
+   int visible;
+
+   if (!crtc->enabled) {
+   DRM_DEBUG_KMS("Cannot update primary plane of a disabled 
CRTC.\n");
+   return -EINVAL;
+   }
+
+   /* setplane API takes shifted source rectangle values; unshift them */
+   src_x >>= 16;
+   src_y >>= 16;
+   src_w >>= 16;
+   src_h >>= 16;
+
+   /* check scaling */
+   if (!can_scale && (crtc_w != src_w || crtc_h != src_h)) {
+   DRM_DEBUG_KMS("Can't scale primary plane\n");
+   return -EINVAL;
+   }
+
+   /*
+* Drivers that can scale should perform their own min/max scale
+* factor checks.
+*/
+   hscale = drm_rect_calc_hscale(, , 0, INT_MAX);
+   vscale = drm_rect_calc_vscale(, , 0, INT_MAX);
+   visible = drm_rect_clip_scaled(, , , hscale, vscale);
+   if (!visible)
+   /*
+* Primary plane isn't visible; some drivers can handle this
+* so we just return success here.  Drivers that can't
+* (including those that use the primary plane helper's
+* update function) will return an error from their
+* update_plane handler.
+*/
+   return 0;
+
+   if (!can_position && !drm_rect_equals(, )) {
+   DRM_DEBUG_KMS("Primary plane must cover entire CRTC\n");
+   return -EINVAL;
+   }
+
+   return 0;
+}
+EXPORT_SYMBOL(drm_primary_helper_check_update);
+
+/**
  * drm_primary_helper_update() - Helper for primary plane update
  * @plane: plane object to update
  * @crtc: owning CRTC of owning plane
@@ -113,6 +209,12 @@ int drm_primary_helper_update(struct drm_plane *plane, 
struct drm_crtc *crtc,
.x = src_x >> 16,
.y = src_y >> 16,
};
+   struct drm_rect src = {
+   .x1 = src_x >> 16,
+   .y1 = src_y >> 16,
+   .x2 = (src_x + src_w) >> 16,
+

[PATCH 1/3] drm: Check CRTC compatibility in setplane

[Matt Roper]

The DRM core setplane code should check that the plane is usable on the
specified CRTC before calling into the driver.

Prior to this patch, a plane's possible_crtcs field was purely
informational for userspace and was never actually verified at the
kernel level (aside from the primary plane helper).

Cc: dri-devel at lists.freedesktop.org
Signed-off-by: Matt Roper 
---
 drivers/gpu/drm/drm_crtc.c | 7 +++
 drivers/gpu/drm/drm_plane_helper.c | 6 --
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 461d19b..b6d6c04 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -2140,6 +2140,13 @@ int drm_mode_setplane(struct drm_device *dev, void *data,
}
crtc = obj_to_crtc(obj);

+   /* Check whether this plane is usable on this CRTC */
+   if (!(plane->possible_crtcs & drm_crtc_mask(crtc))) {
+   DRM_DEBUG_KMS("Invalid crtc for plane\n");
+   ret = -EINVAL;
+   goto out;
+   }
+
fb = drm_framebuffer_lookup(dev, plane_req->fb_id);
if (!fb) {
DRM_DEBUG_KMS("Unknown framebuffer ID %d\n",
diff --git a/drivers/gpu/drm/drm_plane_helper.c 
b/drivers/gpu/drm/drm_plane_helper.c
index b72736d..65c4a00 100644
--- a/drivers/gpu/drm/drm_plane_helper.c
+++ b/drivers/gpu/drm/drm_plane_helper.c
@@ -137,12 +137,6 @@ int drm_primary_helper_update(struct drm_plane *plane, 
struct drm_crtc *crtc,
return -EINVAL;
}

-   /* Primary planes are locked to their owning CRTC */
-   if (plane->possible_crtcs != drm_crtc_mask(crtc)) {
-   DRM_DEBUG_KMS("Cannot change primary plane CRTC\n");
-   return -EINVAL;
-   }
-
/* Disallow scaling */
src_w >>= 16;
src_h >>= 16;
-- 
1.8.5.1

[RFC v2 PATCH 02/14] drm/exynos: dsi: delay setting clocks after reset

[YoungJun Cho]

Hi Andrzej

Thank you for comments.

On 04/22/2014 09:15 PM, Andrzej Hajda wrote:
> Hi YoungJun,
>
> On 04/21/2014 02:28 PM, YoungJun Cho wrote:
>> Some phy control registers are not kept after software reset.
>> So this patch makes the clocks containing phy control to be set
>> after software reset.
>>
>> Signed-off-by: YoungJun Cho 
>> Acked-by: Inki Dae 
>> Acked-by: Kyungmin Park 
>> ---
>>   drivers/gpu/drm/exynos/exynos_drm_dsi.c |2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c 
>> b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>> index 956e5f3..2cf1f0b 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>> @@ -946,10 +946,10 @@ static irqreturn_t exynos_dsi_irq(int irq, void 
>> *dev_id)
>>
>>   static int exynos_dsi_init(struct exynos_dsi *dsi)
>>   {
>> -exynos_dsi_enable_clock(dsi);
>>  exynos_dsi_reset(dsi);
>>  enable_irq(dsi->irq);
>>  exynos_dsi_wait_for_reset(dsi);
>> +exynos_dsi_enable_clock(dsi);
>>  exynos_dsi_init_link(dsi);
>>
>>  return 0;
>
> I have commented it in the previous version of the patchset. I repeat it
> again.
> This is a regression, on exynos4210-trats I have 'timeout waiting for
> reset' message after dpms off, on.

I'm really sorry for that. I misunderstood last time.

I think the original codes were correct, because the reset timeout would
be occurred without clock activation.

I'll check and fix again.
(By the way, why am I ok?)

>
> I will comment your previous answer here to make the discussion easier:
>> As I mentioned in description, it came from phy control registers.
>> Fortunately Exynos4 SoCs are safe, but the DSIM_PHYCTRL_REG,
>> DSIM_PHYTIMING_REG, DSIM_PHYTIMING1_REG and DSIM_PHYTIMING2_REG are
>> affected which are used in exynos_dsi_set_pll() for Exynos5 SoCs.
>>
>> So this patch is required for Exynos5 SoCs.
>
> In the moment this patch is applied exynos_dsi_set_pll do not touch phy
> registers you have mentioned.
> Your change would be more clear if it will be merged together with the
> patch adding PHYCTRL settings.
>
> Anyway, solution is simple - please set PHY registers after reset and
> configure clocks before reset to avoid
> reset timeouts, is there any reason to not do it this way?

The only reason is that the PHY control is related with PLL control and
that was in exynos_dsi_enable_clock() call path.
So I just wanted to keep current sequence.

If there is no way to use previous one, I'll consider your approach.

Thank you.
Best regards YJ

>
> Regards
> Andrzej
>

[PATCH V2 2/9] drm/panel: add pre_enable and post_disable routines

[Thierry Reding]

On Wed, Apr 23, 2014 at 09:29:15AM +0200, Daniel Vetter wrote:
> On Tue, Apr 22, 2014 at 08:06:19PM +0530, Ajay kumar wrote:
> > Hi Thierry,
> > 
> > 
> > On Tue, Apr 22, 2014 at 1:49 PM, Thierry Reding  > gmail.com>
> > wrote:
> > > On Tue, Apr 22, 2014 at 04:09:11AM +0530, Ajay Kumar wrote:
> > >> Most of the panels need an init sequence as mentioned below:
> > >>   -- poweron LCD unit/LCD_EN
> > >>   -- start video data
> > >>   -- poweron LED unit/BL_EN
> > >> And, a de-init sequence as mentioned below:
> > >>   -- poweroff LED unit/BL_EN
> > >>   -- stop video data
> > >>   -- poweroff LCD unit/LCD_EN
> > >> With existing callbacks for drm panel, we cannot accomodate such panels,
> > >> since only two callbacks, i.e "panel_enable" and panel_disable are
> > supported.
> > >>
> > >> This patch adds:
> > >>   -- "pre_enable" callback which can be called before
> > >>   the actual video data is on, and then call the "enable"
> > >>   callback after the video data is available.
> > >>
> > >>   -- "post_disable" callback which can be called after
> > >>   the video data is off, and use "disable" callback
> > >>   to do something before switching off the video data.
> > >>
> > >> Now, we can easily map the above scenario as shown below:
> > >>   poweron LCD unit/LCD_EN = "pre_enable" callback
> > >>   poweron LED unit/BL_EN = "enable" callback
> > >>   poweroff LED unit/BL_EN = "disable" callback
> > >>   poweroff LCD unit/LCD_EN = "post_disable" callback
> > >
> > > I don't like this. What happens when the next panel comes around that
> > > has a yet slightly different requirement? Will we introduce a new
> > > pre_pre_enable() and post_post_disable() function then?
> > >
> > As I have already explained, these 2 callbacks are sufficient enough to
> > take care
> > the power up/down sequence for LVDS and eDP panels. And, definitely having
> > just 2 callbacks "enable" and "disable" is not at all sufficient.
> > 
> > I initially thought of using panel_simple_enable from panel-simple driver.
> > But it doesn't go well with case wherein there are 2 regulators(one for LCD
> > and one for LED)
> > a BL_EN signal etc. And, often(Believe me, I have referred to both eDP
> > panel datasheets
> > and LVDS panel datasheets) proper powerup sequence for such panels is as
> > mentioned below:
> > 
> > powerup:
> > -- power up the supply (LCD_VCC).
> > -- start video data.
> > -- enable backlight.
> > 
> > powerdown
> > -- disable backlight.
> > -- stop video data.
> > -- power off the supply (LCD VCC)
> > 
> > For the above cases, if I have to somehow fit in all the required settings,
> > it breaks the sequence and I end up getting glitches during bootup/DPMS.
> > 
> > Also, when the drm_bridge can support pre_enable and post_disable, why not
> > drm_panel provide that both should work in tandem?
> 
> Imo this makes sense, especially if we go through with the idea talked
> about yesterday of creating a drm_bridge to wrap-up a drm_panel with
> sufficient isolation between all components.

I'm not at all comfortable with these. The names are totally confusing.
Next somebody will need to do something after the panel has been enabled
and we'll have to introduce .post_enable() and .pre_disable() functions.
And worse, what if somebody needs something to be done between
.pre_enable() and .enable()? .post_pre_enable()? Where does it end?

According to the above description the only reason why we need this is
to avoid visible glitches when the panel is already on, but the video
stream hasn't started yet. If that's the only reason why we need this,
then perhaps adding a way for a panel to expose the associated backlight
would be better?

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

[RFC v2 PATCH 02/14] drm/exynos: dsi: delay setting clocks after reset

[Andrzej Hajda]

On 04/23/2014 05:45 AM, YoungJun Cho wrote:
> Hi again Andrzej,
> 
> On 04/23/2014 10:01 AM, YoungJun Cho wrote:
>> Hi Andrzej
>>
>> Thank you for comments.
>>
>> On 04/22/2014 09:15 PM, Andrzej Hajda wrote:
>>> Hi YoungJun,
>>>
>>> On 04/21/2014 02:28 PM, YoungJun Cho wrote:
 Some phy control registers are not kept after software reset.
 So this patch makes the clocks containing phy control to be set
 after software reset.

 Signed-off-by: YoungJun Cho 
 Acked-by: Inki Dae 
 Acked-by: Kyungmin Park 
 ---
   drivers/gpu/drm/exynos/exynos_drm_dsi.c |2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
 b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
 index 956e5f3..2cf1f0b 100644
 --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
 +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
 @@ -946,10 +946,10 @@ static irqreturn_t exynos_dsi_irq(int irq, void
 *dev_id)

   static int exynos_dsi_init(struct exynos_dsi *dsi)
   {
 -exynos_dsi_enable_clock(dsi);
   exynos_dsi_reset(dsi);
   enable_irq(dsi->irq);
   exynos_dsi_wait_for_reset(dsi);
 +exynos_dsi_enable_clock(dsi);
   exynos_dsi_init_link(dsi);

   return 0;
>>>
>>> I have commented it in the previous version of the patchset. I repeat it
>>> again.
>>> This is a regression, on exynos4210-trats I have 'timeout waiting for
>>> reset' message after dpms off, on.
>>
>> I'm really sorry for that. I misunderstood last time.
>>
>> I think the original codes were correct, because the reset timeout would
>> be occurred without clock activation.
> 
> This is not true.
> 
>>
>> I'll check and fix again.
>> (By the way, why am I ok?)
> 
> I have not verified with exynos4210-trats board yet(I have to get it),
> but reset timeout is occured in exynos_dsi_wait_for_reset()
> with >completed and that is completed by exynos_dsi_irq().
> 
> And the regulators and clocks are enabled by exynos_dsi_poweron(),
> NOT by exynos_dsi_enable_clock().
> 
> So I think the reset timeout is not related with this patch.


As far as I remember there were at least two issues with init sequence:
- spurious irq storm after power on and before reset,
- irq reset timeouts after reset and without enabled clock.

The current sequence is a result of tests on live hw (documentation were
not helpful in this case). I think it could be improved little bit more
by moving exynos_dsi_enable_clock just after enable_irq this will
eliminate possible timeout when RST_RELEASE irq is signaled but irq is
still disabled. The sequence should look like below:

exynos_dsi_reset(dsi);
enable_irq(dsi->irq);
exynos_dsi_enable_clock(dsi);
exynos_dsi_wait_for_reset(dsi);
exynos_dsi_init_link(dsi);

And PHY related configuration could be put somewhere after
exynos_dsi_wait_for_reset.

I have tested this sequence on trats, it seems to be OK.

Regards
Andrzej


> 
> Anyway I need more investigation.
> 
> Thank you.
> Best regards YJ
> 
>>
>>>
>>> I will comment your previous answer here to make the discussion easier:
 As I mentioned in description, it came from phy control registers.
 Fortunately Exynos4 SoCs are safe, but the DSIM_PHYCTRL_REG,
 DSIM_PHYTIMING_REG, DSIM_PHYTIMING1_REG and DSIM_PHYTIMING2_REG are
 affected which are used in exynos_dsi_set_pll() for Exynos5 SoCs.

 So this patch is required for Exynos5 SoCs.
>>>
>>> In the moment this patch is applied exynos_dsi_set_pll do not touch phy
>>> registers you have mentioned.
>>> Your change would be more clear if it will be merged together with the
>>> patch adding PHYCTRL settings.
>>>
>>> Anyway, solution is simple - please set PHY registers after reset and
>>> configure clocks before reset to avoid
>>> reset timeouts, is there any reason to not do it this way?
>>
>> The only reason is that the PHY control is related with PLL control and
>> that was in exynos_dsi_enable_clock() call path.
>> So I just wanted to keep current sequence.
>>
>> If there is no way to use previous one, I'll consider your approach.
>>
>> Thank you.
>> Best regards YJ
>>
>>>
>>> Regards
>>> Andrzej
>>>
>>
>> ___
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
> 

[PATCH 3/4] drm: Introduce drm_fb_helper_prepare()

[Daniel Vetter]

On Wed, Apr 23, 2014 at 09:14:41AM +0200, Thierry Reding wrote:
> On Tue, Apr 22, 2014 at 05:54:06PM +0200, Daniel Vetter wrote:
> > On Tue, Apr 22, 2014 at 04:42:20PM +0200, Thierry Reding wrote:
> [...]
> > > diff --git a/drivers/gpu/drm/drm_fb_helper.c 
> > > b/drivers/gpu/drm/drm_fb_helper.c
> [...]
> > > @@ -502,6 +503,33 @@ static void drm_fb_helper_crtc_free(struct 
> > > drm_fb_helper *helper)
> > >  }
> > >  
> > >  /**
> > > + * drm_fb_helper_prepare - setup a drm_fb_helper structure
> > > + * @dev: DRM device
> > > + * @helper: driver-allocated fbdev helper structure to set up
> > > + * @funcs: pointer to structure of functions associate with this helper
> > > + *
> > > + * Sets up the bare minimum to make the framebuffer helper usable. This 
> > > is
> > > + * useful to implement race-free initialization of the polling helpers. 
> > > In
> > > + * that case a typical sequence would be:
> > > + *
> > > + *   - call drm_fb_helper_prepare()
> > > + *   - set dev->mode_config.funcs
> > 
> > This step is done in drm_fb_helper_prepare already.
> 
> drm_fb_helper_prepare() sets fb_helper->funcs. dev->mode_config.funcs
> needs to be set because it gets called by drm_kms_helper_hotplug_event()
> which in turn is called by output_poll_execute(), which can be called at
> any point after drm_kms_helper_poll_init(). It could be scheduled
> immediately by drm_kms_helper_poll_enable().
> 
> I wonder if perhaps we should be wrapping that into a function as well.
> Currently this is only documented in code by the drivers that call this.
> But since it's only a single step it doesn't seem worth it. Perhaps if
> we rolled the min/max_width/height into that function as well it would
> be more worth it? That could be difficult to do since a couple of
> drivers need to set those depending on the chipset generation.

Oh I've misread this step for the fb_helper->funcs assignment. Makes sense
and I don't think we need to augment your kerneldoc more to explain this.

> > > + *   - perform driver-specific setup

Hm, maybe clarify this as "initialize modeset objects like crtcs, encoders
and connectors"? Since that's the important part we need to get done here.

> > > + *   - call drm_kms_helper_poll_init()
> > 
> > Maybe mention that after this you can start to handle hpd events using the
> > probe helpers?
> 
> Isn't that somewhat implied already? The poll helpers call directly the
> dev->mode_config.funcs->output_poll_changed() function, which has
> already been set up earlier.

I've more meant that after this it's save for drivers to enable hpd
interrupts and start to process them. I.e.

- enable interrupts and start processing hpd events

as an additional step to make it really cleear how it all fits together.
Otherwise driver authors are left wondering wtf this isn't just one
function call to do it all for them ;-)

> > > + *   - call drm_fb_helper_init()
> > > + *   - call drm_fb_helper_single_add_all_connectors()
> > > + *   - call drm_fb_helper_initial_config()
> > 
> > Does this list look sane in the generated DocBook? Afaik kerneldoc
> > unfortunately lacks any form of markup, at least afaik :(
> 
> I must admit I didn't check. I'll make sure I do that before sending out
> the next version.

If it looks ugly I'm ok as-is, just wondered. Kerneldoc is a bit
simplistic ime.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[RFC v2 PATCH v4 09/14] ARM: dts: s6e3fa0: add DT bindings

[Thierry Reding]

On Wed, Apr 23, 2014 at 10:26:20AM +0900, YoungJun Cho wrote:
> Hi Andrzej
> 
> Thank you for comment.
> 
> On 04/22/2014 11:02 PM, Andrzej Hajda wrote:
> >On 04/21/2014 02:28 PM, YoungJun Cho wrote:
> >>This patch adds DT bindings for s6e3fa0 panel.
> >>The bindings describes panel resources, display timings and cpu timings.
> >>
> >>Changelog v2:
> >>- Adds unit address (commented by Sachin Kamat)
> >>Changelog v3:
> >>- Removes optional delay, size properties (commented by Laurent Pinchart)
> >>- Adds OLED detection, TE gpio properties
> >>Changelog v4:
> >>- Moves CPU timings relevant properties from FIMD DT
> >>   (commeted by Laurent Pinchart, Andrzej Hajda)
> >>
> >>Signed-off-by: YoungJun Cho 
> >>Acked-by: Inki Dae 
> >>Acked-by: Kyungmin Park 
> >>---
> >>  .../devicetree/bindings/panel/samsung,s6e3fa0.txt  |   63 
> >> 
> >>  1 file changed, 63 insertions(+)
> >>  create mode 100644 
> >> Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt
> >>
> >>diff --git a/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt 
> >>b/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt
> >>new file mode 100644
> >>index 000..9eeb38b
> >>--- /dev/null
> >>+++ b/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt
> >>@@ -0,0 +1,63 @@
> >>+Samsung S6E3FA0 AMOLED LCD 5.7 inch panel
> >>+
> >>+Required properties:
> >>+  - compatible: "samsung,s6e3fa0"
> >>+  - reg: the virtual channel number of a DSI peripheral
> >>+  - vdd3-supply: core voltage supply
> >>+  - vci-supply: voltage supply for analog circuits
> >>+  - reset-gpio: a GPIO spec for the reset pin
> >>+  - det-gpio: a GPIO spec for the OLED detection pin
> >>+  - te-gpio: a GPIO spec for the TE pin
> >
> >Just FYI, according to DT documentation [1] gpio spec should be in form
> >[name]-gpios, however there is plenty bindings with -gpio suffix, so I
> >am not sure if it is really enforced. On the other side it is enforced
> >by descriptor based gpio framework[2]. Integer-based gpio framework
> >used in your driver is obsolete according to [2].
> 
> Yes, you're right. That is my mistake.
> They should be attached 's'.
> At first I used integer-based gpio framework and replaced to descriptor
> based one, but did not updated DT bindings.

I've been working on a patch to support both *-gpios and *-gpio variants
with the GPIO descriptor framework. *-gpios makes sense if there can
indeed be several, but for something like hotplug detection I don't
think there's a reason to require the plural.

Furthermore some bindings already use the singular *-gpio anyway, so if
we ever want to convert drivers using those bindings to the GPIO
descriptor API we have to support that form too.

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

[PATCH V2 2/9] drm/panel: add pre_enable and post_disable routines

[Daniel Vetter]

On Tue, Apr 22, 2014 at 08:06:19PM +0530, Ajay kumar wrote:
> Hi Thierry,
> 
> 
> On Tue, Apr 22, 2014 at 1:49 PM, Thierry Reding 
> wrote:
> > On Tue, Apr 22, 2014 at 04:09:11AM +0530, Ajay Kumar wrote:
> >> Most of the panels need an init sequence as mentioned below:
> >>   -- poweron LCD unit/LCD_EN
> >>   -- start video data
> >>   -- poweron LED unit/BL_EN
> >> And, a de-init sequence as mentioned below:
> >>   -- poweroff LED unit/BL_EN
> >>   -- stop video data
> >>   -- poweroff LCD unit/LCD_EN
> >> With existing callbacks for drm panel, we cannot accomodate such panels,
> >> since only two callbacks, i.e "panel_enable" and panel_disable are
> supported.
> >>
> >> This patch adds:
> >>   -- "pre_enable" callback which can be called before
> >>   the actual video data is on, and then call the "enable"
> >>   callback after the video data is available.
> >>
> >>   -- "post_disable" callback which can be called after
> >>   the video data is off, and use "disable" callback
> >>   to do something before switching off the video data.
> >>
> >> Now, we can easily map the above scenario as shown below:
> >>   poweron LCD unit/LCD_EN = "pre_enable" callback
> >>   poweron LED unit/BL_EN = "enable" callback
> >>   poweroff LED unit/BL_EN = "disable" callback
> >>   poweroff LCD unit/LCD_EN = "post_disable" callback
> >
> > I don't like this. What happens when the next panel comes around that
> > has a yet slightly different requirement? Will we introduce a new
> > pre_pre_enable() and post_post_disable() function then?
> >
> As I have already explained, these 2 callbacks are sufficient enough to
> take care
> the power up/down sequence for LVDS and eDP panels. And, definitely having
> just 2 callbacks "enable" and "disable" is not at all sufficient.
> 
> I initially thought of using panel_simple_enable from panel-simple driver.
> But it doesn't go well with case wherein there are 2 regulators(one for LCD
> and one for LED)
> a BL_EN signal etc. And, often(Believe me, I have referred to both eDP
> panel datasheets
> and LVDS panel datasheets) proper powerup sequence for such panels is as
> mentioned below:
> 
> powerup:
> -- power up the supply (LCD_VCC).
> -- start video data.
> -- enable backlight.
> 
> powerdown
> -- disable backlight.
> -- stop video data.
> -- power off the supply (LCD VCC)
> 
> For the above cases, if I have to somehow fit in all the required settings,
> it breaks the sequence and I end up getting glitches during bootup/DPMS.
> 
> Also, when the drm_bridge can support pre_enable and post_disable, why not
> drm_panel provide that both should work in tandem?

Imo this makes sense, especially if we go through with the idea talked
about yesterday of creating a drm_bridge to wrap-up a drm_panel with
sufficient isolation between all components.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH] drm: Rip out totally bogus vga_switcheroo->can_switch locking

[Thierry Reding]

On Tue, Apr 22, 2014 at 10:45:19PM +0200, Daniel Vetter wrote:
> So I just wanted to add a new field to struct drm_device and
> accidentally stumbled over something. According to comments
> dev->open_count is protected by dev->count_lock, but that's totally
> not the case. It's protected by drm_global_mutex.
> 
> Unfortunately the vga switcheroo callbacks took this comment at face
> value. The problem is that we can't just take the drm_global_mutex
> because:
> - It would lead to a locking inversion with the driver load/unload
>   paths.
> - It wouldn't actually protect anything, for that we'd need to wrap
>   the entire vga switcheroo code in the drm_global_mutex. And I'm not
>   sure whether that would actually solve anything.
> 
> What we probably want is a try_to_grab_switcheroo reference kind of
> thing which is used in the driver's ->open callback. Then we could
> move all that ->can_switch madness into the vga switcheroo core where
> it really belongs.
> 
> But since that would amount to real work take the easy way out and
> just add a comment. It's definitely not going to make anything worse
> since doing switcheroo state changes while restarting X just isn't
> recommended. Even though the delayed switching code does exactly that.
> 
> v2:
> - Simplify the ->can_switch implementations more (Thierry)
> - Fix comment about the dev->open_count locking (Thierry)
> 
> Cc: Thierry Reding 
> Reviewed-by: Laurent Pinchart  (v1)
> Signed-off-by: Daniel Vetter 
> ---
>  drivers/gpu/drm/i915/i915_dma.c| 11 ++-
>  drivers/gpu/drm/nouveau/nouveau_vga.c  | 11 ++-
>  drivers/gpu/drm/radeon/radeon_device.c | 11 ++-
>  include/drm/drmP.h |  2 +-
>  4 files changed, 19 insertions(+), 16 deletions(-)

Looks good:

Reviewed-by: Thierry Reding 
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20140423/ba02a92b/attachment.sig>

[PATCH] drm/irq: track the irq installed in drm_irq_install in dev->irq

[Thierry Reding]

On Tue, Apr 22, 2014 at 10:44:20PM +0200, Daniel Vetter wrote:
[...]
> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> index 589e865832cd..7cf407bbfed5 100644
> --- a/drivers/gpu/drm/drm_irq.c
> +++ b/drivers/gpu/drm/drm_irq.c
> @@ -249,14 +249,16 @@ static inline int drm_dev_to_irq(struct drm_device *dev)
>   */
>  int drm_irq_install(struct drm_device *dev)
>  {
> - int ret;
> + int ret, irq;
>   unsigned long sh_flags = 0;
>   char *irqname;
>  
> + irq = drm_dev_to_irq(dev);

I think the assignment could have happened either when the variable is
declared, or...

> +
>   if (!drm_core_check_feature(dev, DRIVER_HAVE_IRQ))
>   return -EINVAL;
>  
> - if (drm_dev_to_irq(dev) == 0)
> + if (irq == 0)

... right above this, since it is where it is first used (it may not be
necessary to query it before here at all if the driver doesn't set
DRIVER_HAVE_IRQ).

But I realize that that's pure bike-shedding, so either way:

Reviewed-by: Thierry Reding 
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20140423/bc29b1c4/attachment.sig>

[Bug 77785] (radeonsi) Some lighting issues in games, textures goes black

[bugzilla-dae...@freedesktop.org]

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

--- Comment #6 from Michel D?nzer  ---
Is there anything interesting in the game's terminal output, e.g. related to
shader compile failures?

If not, can you create an apitrace demonstrating the problem?

-- 
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/20140423/c7a12b87/attachment.html>

[RFC 4/5] drm: Introduce drm_set_unique()

[Thierry Reding]

On Tue, Apr 22, 2014 at 05:48:07PM +0200, Daniel Vetter wrote:
> On Tue, Apr 22, 2014 at 05:09:32PM +0200, Thierry Reding wrote:
> > From: Thierry Reding 
> > 
> > Add a helper function that allows drivers to statically set the unique
> > name of the device. This will allow platform and USB drivers to get rid
> > of their DRM bus implementations and directly use drm_dev_alloc() and
> > drm_dev_register().
> > 
> > Signed-off-by: Thierry Reding 
> > ---
> >  drivers/gpu/drm/drm_ioctl.c | 37 +++--
> >  drivers/gpu/drm/drm_stub.c  |  1 +
> >  include/drm/drmP.h  |  3 +++
> >  3 files changed, 35 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> > index 2dd3a6d8382b..371db3bef60c 100644
> > --- a/drivers/gpu/drm/drm_ioctl.c
> > +++ b/drivers/gpu/drm/drm_ioctl.c
> > @@ -42,6 +42,20 @@
> >  #include 
> >  #endif
> >  
> > +int drm_set_unique(struct drm_device *dev, const char *fmt, ...)
> 
> Can you please add a bit of kerneldoc for this? drm_ioctl.c isn't yet
> pulled into the drm reference docbook, but better to have it there
> already.

On second thought, wouldn't this be better located in drm_stub.c? It
isn't really related to the IOCTL code except that one of the IOCTLs now
uses the information set by this function. Logically I think it belongs
with the likes of drm_dev_alloc() and drm_dev_register().

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

[PATCH 3/4] drm: Introduce drm_fb_helper_prepare()

[Thierry Reding]

On Tue, Apr 22, 2014 at 05:54:06PM +0200, Daniel Vetter wrote:
> On Tue, Apr 22, 2014 at 04:42:20PM +0200, Thierry Reding wrote:
[...]
> > diff --git a/drivers/gpu/drm/drm_fb_helper.c 
> > b/drivers/gpu/drm/drm_fb_helper.c
[...]
> > @@ -502,6 +503,33 @@ static void drm_fb_helper_crtc_free(struct 
> > drm_fb_helper *helper)
> >  }
> >  
> >  /**
> > + * drm_fb_helper_prepare - setup a drm_fb_helper structure
> > + * @dev: DRM device
> > + * @helper: driver-allocated fbdev helper structure to set up
> > + * @funcs: pointer to structure of functions associate with this helper
> > + *
> > + * Sets up the bare minimum to make the framebuffer helper usable. This is
> > + * useful to implement race-free initialization of the polling helpers. In
> > + * that case a typical sequence would be:
> > + *
> > + *   - call drm_fb_helper_prepare()
> > + *   - set dev->mode_config.funcs
> 
> This step is done in drm_fb_helper_prepare already.

drm_fb_helper_prepare() sets fb_helper->funcs. dev->mode_config.funcs
needs to be set because it gets called by drm_kms_helper_hotplug_event()
which in turn is called by output_poll_execute(), which can be called at
any point after drm_kms_helper_poll_init(). It could be scheduled
immediately by drm_kms_helper_poll_enable().

I wonder if perhaps we should be wrapping that into a function as well.
Currently this is only documented in code by the drivers that call this.
But since it's only a single step it doesn't seem worth it. Perhaps if
we rolled the min/max_width/height into that function as well it would
be more worth it? That could be difficult to do since a couple of
drivers need to set those depending on the chipset generation.

> > + *   - perform driver-specific setup
> > + *   - call drm_kms_helper_poll_init()
> 
> Maybe mention that after this you can start to handle hpd events using the
> probe helpers?

Isn't that somewhat implied already? The poll helpers call directly the
dev->mode_config.funcs->output_poll_changed() function, which has
already been set up earlier.

> > + *   - call drm_fb_helper_init()
> > + *   - call drm_fb_helper_single_add_all_connectors()
> > + *   - call drm_fb_helper_initial_config()
> 
> Does this list look sane in the generated DocBook? Afaik kerneldoc
> unfortunately lacks any form of markup, at least afaik :(

I must admit I didn't check. I'll make sure I do that before sending out
the next version.

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

[PATCH] drm: Simplify fb refcounting rules around ->update_plane

[Matt Roper]

On Wed, Apr 23, 2014 at 05:37:30PM +0200, Daniel Vetter wrote:
> The introduction of primary planes has apparently caused a bit of fb
> refcounting fun for people. That makes it a good time to clean up the
> arcane rules and slight differences between ->update_plane and
> ->set_config. The new rules are:
> 
> - The core holds a reference for both the new and the old fb (if
>   they're non-NULL of course) while calling into the driver through
>   either ->update_plane or ->set_config.
> 
> - Drivers may not clobber plane->fb if their callback fails. If they
>   do that, they need to store a pointer to the old fb in it again.
>   When calling into the driver plane->fb still points at the current
>   (old) framebuffer.
> 
> - The core will update the plane->fb pointer on success. Drivers can
>   do that themselves too, but aren't required to any more for the
>   primary plane.
> 
> - The core will update fb refcounts for the plane->fb pointer,
>   presuming the drivers hold up their end of the bargain.
> 
> v2: Remove now unused tmpfb (Thierry)
> 
> v3: Drop broken changes from drm_mode_setplane (Ville). Also polish
> the commit message a bit.
> 
> v4: Also fix up the handling of ->disable_plane in
> drm_plane_force_disable. The issue was that we didn't save plane->fb
> over the ->disable_plane call. Just paranoia, nothing relies on this.
> 
> v5: Keep still useful comments about directly calling ->set_config,
> which I should have done for v4 already. Requested by Matt.
> 
> Cc: Thierry Reding 
> Cc: Ville Syrj?l? 
> Cc: Matt Roper 
> Signed-off-by: Daniel Vetter 

Reviewed-by: Matt Roper 

> ---
>  drivers/gpu/drm/drm_crtc.c | 13 +++--
>  drivers/gpu/drm/drm_plane_helper.c | 10 +-
>  2 files changed, 8 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index d8b7099abece..f6633cb927bc 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -1145,16 +1145,17 @@ EXPORT_SYMBOL(drm_plane_cleanup);
>   */
>  void drm_plane_force_disable(struct drm_plane *plane)
>  {
> + struct drm_framebuffer *old_fb = plane->fb;
>   int ret;
>  
> - if (!plane->fb)
> + if (!old_fb)
>   return;
>  
>   ret = plane->funcs->disable_plane(plane);
>   if (ret)
>   DRM_ERROR("failed to disable plane with busy fb\n");
>   /* disconnect the plane from the fb and crtc: */
> - __drm_framebuffer_unreference(plane->fb);
> + __drm_framebuffer_unreference(old_fb);
>   plane->fb = NULL;
>   plane->crtc = NULL;
>  }
> @@ -2187,16 +2188,18 @@ int drm_mode_setplane(struct drm_device *dev, void 
> *data,
>   }
>  
>   drm_modeset_lock_all(dev);
> + old_fb = plane->fb;
>   ret = plane->funcs->update_plane(plane, crtc, fb,
>plane_req->crtc_x, plane_req->crtc_y,
>plane_req->crtc_w, plane_req->crtc_h,
>plane_req->src_x, plane_req->src_y,
>plane_req->src_w, plane_req->src_h);
>   if (!ret) {
> - old_fb = plane->fb;
>   plane->crtc = crtc;
>   plane->fb = fb;
>   fb = NULL;
> + } else {
> + old_fb = NULL;
>   }
>   drm_modeset_unlock_all(dev);
>  
> @@ -2239,9 +2242,7 @@ int drm_mode_set_config_internal(struct drm_mode_set 
> *set)
>   ret = crtc->funcs->set_config(set);
>   if (ret == 0) {
>   crtc->primary->crtc = crtc;
> -
> - /* crtc->fb must be updated by ->set_config, enforces this. */
> - WARN_ON(fb != crtc->primary->fb);
> + crtc->primary->fb = fb;
>   }
>  
>   list_for_each_entry(tmp, >dev->mode_config.crtc_list, head) {
> diff --git a/drivers/gpu/drm/drm_plane_helper.c 
> b/drivers/gpu/drm/drm_plane_helper.c
> index 9540ff9f97fe..d966afa7ecae 100644
> --- a/drivers/gpu/drm/drm_plane_helper.c
> +++ b/drivers/gpu/drm/drm_plane_helper.c
> @@ -124,7 +124,6 @@ int drm_primary_helper_update(struct drm_plane *plane, 
> struct drm_crtc *crtc,
>   .y2 = crtc->mode.vdisplay,
>   };
>   struct drm_connector **connector_list;
> - struct drm_framebuffer *tmpfb;
>   int num_connectors, ret;
>  
>   if (!crtc->enabled) {
> @@ -178,21 +177,14 @@ int drm_primary_helper_update(struct drm_plane *plane, 
> struct drm_crtc *crtc,
>   set.num_connectors = num_connectors;
>  
>   /*
> -  * set_config() adjusts crtc->primary->fb; however the DRM setplane
> -  * code that called us expects to handle the framebuffer update and
> -  * reference counting; save and restore the current fb before
> -  * calling it.
> -  *
> -  * N.B., we call set_config() directly here rather than using
> +  * We call set_config() directly here rather than using
>* drm_mode_set_config_internal.  We're reprogramming the same
>

An other radeon ring count patch

[Mathias Fröhlich]

Hi Christian, Alex,

While looking over the rest of the ring counts
I noticed an other one, this time we reserve too
much which cannot even be a potential problem.
Anyway, it's nicer to be correct.

Attached that patch to uvd_v1_0_ring_test()

Greetings

Mathias
-- next part --
A non-text attachment was scrubbed...
Name: 0001-drm-radeon-fix-count-in-uvd_v1_0_ring_test.patch
Type: text/x-patch
Size: 1035 bytes
Desc: not available
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20140423/18ddbecc/attachment-0001.bin>

[Bug 76564] [AMD Fusion E-350] HDMI refresh rates doesn't match expectations

[bugzilla-dae...@freedesktop.org]

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

--- Comment #63 from Christian K?nig  ---
Please try http://cgit.freedesktop.org/~deathsimple/linux/log/?h=drm-next-3.16.

This branch might fix the remaining frame drop problems.

-- 
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/20140423/0d0e02f1/attachment.html>

[PATCH] drm: Simplify fb refcounting rules around ->update_plane

[Daniel Vetter]

On Wed, Apr 23, 2014 at 8:36 AM, Daniel Vetter  
wrote:
> Hm, that's indeed a bit fishy. Otoh I don't understand how this blows
> up with existing userspace since we should only hit this if userspace
> does an explicit disable call on the primary plane through the ioctl.
> It's a bug for sure though.

Actually even before

commit b6ccd7b9873dc46becd11838c885d5c783784156
Author: Daniel Vetter 
Date:   Tue Apr 15 10:02:43 2014 +0200

drm/plane-helper: Don't fake-implement primary plane disabling

this was broken since if any other plane was enabled it would return
-EBUSY. So this might actually explain the refcounting woes since the
above patch isn't yet merged into any of the upstream drm trees.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH] drm: Simplify fb refcounting rules around ->update_plane

[Daniel Vetter]

On Wed, Apr 23, 2014 at 3:08 AM, Matt Roper  
wrote:
> On Tue, Apr 22, 2014 at 04:19:45PM +0200, Daniel Vetter wrote:
>> The introduction of primary planes has apparently caused a bit of fb
>> refcounting fun for people. That makes it a good time to clean up the
>> arcane rules and slight differences between ->update_plane and
>> ->set_config. The new rules are:
>>
>> - The core holds a reference for both the new and the old fb (if
>>   they're non-NULL of course) while calling into the driver through
>>   either ->update_plane or ->set_config.
>>
>> - Drivers may not clobber plane->fb if their callback fails. If they
>>   do that, they need to store a pointer to the old fb in it again.
>>   When calling into the driver plane->fb still points at the current
>>   (old) framebuffer.
>>
>> - The core will update the plane->fb pointer on success. Drivers can
>>   do that themselves too, but aren't required to any more for the
>>   primary plane.
>>
>> - The core will update fb refcounts for the plane->fb pointer,
>>   presuming the drivers hold up their end of the bargain.
>>
>> v2: Remove now unused tmpfb (Thierry)
>>
>> v3: Drop broken changes from drm_mode_setplane (Ville). Also polish
>> the commit message a bit.
>
> It looks like we might have some problems around setplane with fbid=0.
> It looks like we're assuming that disabling a plane always succeeds
> (which is no longer true for helper-based primary planes --- they just
> return -EINVAL on disable now), so we wind up setting old_fb to the
> currently scanned out framebuffer and then unref it at the end of the
> function if I'm reading things correctly.  We also clobber the
> plane->crtc and plane->fb pointers too when this happens.
>
> I think the real problem here was introduced on b6ccd7b9 and I gave you
> an r-b tag on that email, so that's my bad for not catching it before.
> :-(

Hm, that's indeed a bit fishy. Otoh I don't understand how this blows
up with existing userspace since we should only hit this if userspace
does an explicit disable call on the primary plane through the ioctl.
It's a bug for sure though.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[Bug 77745] [r600g] Call of Duty 4 crashes under Wine due to running out of memory

[bugzilla-dae...@freedesktop.org]

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

--- Comment #7 from Michel D?nzer  ---
Is this a regression, i.e. did it work with an older snapshot of Mesa Git? If
yes, can you bisect which commit introduced the problem?

-- 
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/20140423/7682cbc0/attachment.html>

[git pull] drm fixes

[Dave Airlie]

On Wed, Apr 23, 2014 at 1:59 AM, Linus Torvalds
 wrote:
> Dave, mind sending me a pull request for drm fixes?
>
> There's now at least these two:
>
>  - "drm/radeon/aux: fix hpd assignment for aux bus"
>  - "drm/radeon: use fixed PPL ref divider if needed"
>
> that look like fairly fatal regressions when they affect somebody.
>
> The fact that we already had *two* independent bugs be reported within
> days of that last out-of-merge-window pull request makes me very
> unhappy with the state of drm pulls.
>
> So please make sure that future fixes really are *fixes*. For
> regressions only. No more games like this.

The pll fallout is fixes for the initial feature that was in the merge window,
Tuning plls for monitors is always a pain in the ass, the previous algorithm
took a couple of kernels a few years back to get where it was, unfortunately
HDMI came along and showed up a bunch of its shortcomings. I'm happy
Alex and Christian are on top of things in terms of tracking regressions
and making sure they get fixed,

the AUX fix yes I'm a bit pissed off about myself, but I missed a pull
from a few
weeks ago, felt guilty, and maybe should have chosen the other path and let it
wait a merge,

Christian just sent me a -fixes pull with all of these in it and I'll
send it on to you
in a few mins.

Dave.

[PATCH] drm/tegra: restrict plane loops to legacy planes

[Matt Roper]

On Wed, Apr 23, 2014 at 03:15:32PM +0200, Daniel Vetter wrote:
> In Matt Ropers primary plane series a set of prep patches like
> 
> commit af2b653bfb4ef40931b4d101ca842ce0c5da57ef
> Author: Matt Roper 
> Date:   Tue Apr 1 15:22:32 2014 -0700
> 
> drm/i915: Restrict plane loops to only operate on overlay planes (v2)
> 
> ensured that all exisiting users of the mode_config->plane_list
> wouldn't change behaviour. Unfortunately tegra seems to have fallen
> through the cracks. Fix it.
> 
> This regression was introduced in
> 
> commit e13161af80c185ecd8dc4641d0f5df58f9e3e0af
> Author: Matt Roper 
> Date:   Tue Apr 1 15:22:38 2014 -0700
> 
> drm: Add drm_crtc_init_with_planes() (v2)
> 
> The result was that we've unref'ed the fb for the primary plane twice,
> leading to a use-after free bug. This is because the drm core will
> already set crtc->primary->fb to NULL and do the unref for us, and the
> crtc disable hook is called by the drm crtc helpers for exactly this
> case.
> 
> Aside: Now that the fbdev helpers clean up planes there's no longer a
> need to do this in drivers. So this could probably be nuked entirely
> in linux-next.
> 
> Cc: Matt Roper 
> Signed-off-by: Daniel Vetter 

Yep, this is definitely the right fix.  I'm not sure how I missed this
plane loop in cscope before.

Reviewed-by: Matt Roper 


> ---
>  drivers/gpu/drm/tegra/dc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> index 36c717af6cf9..edb871d7d395 100644
> --- a/drivers/gpu/drm/tegra/dc.c
> +++ b/drivers/gpu/drm/tegra/dc.c
> @@ -312,7 +312,7 @@ static void tegra_crtc_disable(struct drm_crtc *crtc)
>   struct drm_device *drm = crtc->dev;
>   struct drm_plane *plane;
>  
> - list_for_each_entry(plane, >mode_config.plane_list, head) {
> + drm_for_each_legacy_plane(plane, >mode_config.plane_list) {
>   if (plane->crtc == crtc) {
>   tegra_plane_disable(plane);
>   plane->crtc = NULL;
> -- 
> 1.9.2
> 

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795

[PATCH 2/2] drm: Handle ->disable_plane failures correctly

[Matt Roper]

On Wed, Apr 23, 2014 at 10:30:05AM +0200, Daniel Vetter wrote:
> The ->disable_plane hook always had a return value, but only since the
> introduction of primary planes was there any implementation that
> actually failed.
> 
> So handle such failures correctly.
> 
> Note that drm_plane_force_disable is special: In the modeset cleanup
> case we first disable all crtc, so primary planes should all be freed
> already. And in the fb helper we only reset non-primary planes. Still
> better be paranoid and add an early return.
> 
> I don't see how this could happen, but it might fix the fb refcount
> underrun Thierry is seeing. Matt Roper spotted this issue.
> 
> Cc: Thierry Reding 
> Cc: Ville Syrj?l? 
> Cc: Matt Roper 
> Signed-off-by: Daniel Vetter 

Reviewed-by: Matt Roper 

> ---
>  drivers/gpu/drm/drm_crtc.c | 14 ++
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index f6633cb927bc..461d19bd14ee 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -1152,8 +1152,10 @@ void drm_plane_force_disable(struct drm_plane *plane)
>   return;
>  
>   ret = plane->funcs->disable_plane(plane);
> - if (ret)
> + if (ret) {
>   DRM_ERROR("failed to disable plane with busy fb\n");
> + return;
> + }
>   /* disconnect the plane from the fb and crtc: */
>   __drm_framebuffer_unreference(old_fb);
>   plane->fb = NULL;
> @@ -2117,9 +2119,13 @@ int drm_mode_setplane(struct drm_device *dev, void 
> *data,
>   if (!plane_req->fb_id) {
>   drm_modeset_lock_all(dev);
>   old_fb = plane->fb;
> - plane->funcs->disable_plane(plane);
> - plane->crtc = NULL;
> - plane->fb = NULL;
> + ret = plane->funcs->disable_plane(plane);
> + if (!ret) {
> + plane->crtc = NULL;
> + plane->fb = NULL;
> + } else {
> + old_fb = NULL;
> + }
>   drm_modeset_unlock_all(dev);
>   goto out;
>   }
> -- 
> 1.9.2
> 

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795

[PATCH 1/2] drm: Simplify fb refcounting rules around ->update_plane

[Matt Roper]

On Wed, Apr 23, 2014 at 10:30:04AM +0200, Daniel Vetter wrote:
> The introduction of primary planes has apparently caused a bit of fb
> refcounting fun for people. That makes it a good time to clean up the
> arcane rules and slight differences between ->update_plane and
> ->set_config. The new rules are:
> 
> - The core holds a reference for both the new and the old fb (if
>   they're non-NULL of course) while calling into the driver through
>   either ->update_plane or ->set_config.
> 
> - Drivers may not clobber plane->fb if their callback fails. If they
>   do that, they need to store a pointer to the old fb in it again.
>   When calling into the driver plane->fb still points at the current
>   (old) framebuffer.
> 
> - The core will update the plane->fb pointer on success. Drivers can
>   do that themselves too, but aren't required to any more for the
>   primary plane.
> 
> - The core will update fb refcounts for the plane->fb pointer,
>   presuming the drivers hold up their end of the bargain.
> 
> v2: Remove now unused tmpfb (Thierry)
> 
> v3: Drop broken changes from drm_mode_setplane (Ville). Also polish
> the commit message a bit.
> 
> v4: Also fix up the handling of ->disable_plane in
> drm_plane_force_disable. The issue was that we didn't save plane->fb
> over the ->disable_plane call. Just paranoia, nothing relies on this.
> 
> Cc: Thierry Reding 
> Cc: Ville Syrj?l? 
> Cc: Matt Roper 
> Signed-off-by: Daniel Vetter 
> ---
>  drivers/gpu/drm/drm_crtc.c | 13 +++--
>  drivers/gpu/drm/drm_plane_helper.c | 16 
>  2 files changed, 7 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index d8b7099abece..f6633cb927bc 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -1145,16 +1145,17 @@ EXPORT_SYMBOL(drm_plane_cleanup);
>   */
>  void drm_plane_force_disable(struct drm_plane *plane)
>  {
> + struct drm_framebuffer *old_fb = plane->fb;
>   int ret;
>  
> - if (!plane->fb)
> + if (!old_fb)
>   return;
>  
>   ret = plane->funcs->disable_plane(plane);
>   if (ret)
>   DRM_ERROR("failed to disable plane with busy fb\n");
>   /* disconnect the plane from the fb and crtc: */
> - __drm_framebuffer_unreference(plane->fb);
> + __drm_framebuffer_unreference(old_fb);
>   plane->fb = NULL;
>   plane->crtc = NULL;
>  }

So if disable_plane() fails here, we'd still be scanning out of old_fb,
yet we deref it...that doesn't seem quite right.

As you mention, the changes here are just for paranoia since we can't
hit failure here as far as I know; force_disable() is only called in
drm_framebuffer_remove() (where a CRTC loop has already taken care of
removing the framebuffer from the primary plane) and
drm_fb_helper_restore_fbdev_mode() (where we explicitly skip primary
planes).  Since only primary planes can fail to be disabled, I'd be
inclined to just add a BUG_ON(plane->type == DRM_PLANE_TYPE_PRIMARY) at
the start of this function with a comment update explaining that it
can't be called on primary planes, and a BUG_ON(ret) since
non-primary planes should never fail to be disabled.


...
> @@ -177,22 +176,7 @@ int drm_primary_helper_update(struct drm_plane *plane, 
> struct drm_crtc *crtc,
>   set.connectors = connector_list;
>   set.num_connectors = num_connectors;
>  
> - /*
> -  * set_config() adjusts crtc->primary->fb; however the DRM setplane
> -  * code that called us expects to handle the framebuffer update and
> -  * reference counting; save and restore the current fb before
> -  * calling it.
> -  *
> -  * N.B., we call set_config() directly here rather than using
> -  * drm_mode_set_config_internal.  We're reprogramming the same
> -  * connectors that were already in use, so we shouldn't need the extra
> -  * cross-CRTC fb refcounting to accomodate stealing connectors.
> -  * drm_mode_setplane() already handles the basic refcounting for the
> -  * framebuffers involved in this operation.
> -  */

I think the second half of this comment (explaining why we call
set_config() directly rather than calling
drm_mode_set_config_internal()) still has some value.


Matt

> - tmpfb = plane->fb;
>   ret = crtc->funcs->set_config();
> - plane->fb = tmpfb;
>  
>   kfree(connector_list);
>   return ret;
> -- 
> 1.9.2
> 

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795

[Bug 73521] Screen backlight control fails on ASUS UX51VZ

[bugzilla-dae...@bugzilla.kernel.org]

https://bugzilla.kernel.org/show_bug.cgi?id=73521

Aaron Lu  changed:

   What|Removed |Added

 Status|NEW |ASSIGNED
 CC||aaron.lu at intel.com
  Component|Other   |Video(DRI - non Intel)
   Assignee|acpi_other at kernel-bugs.osdl |drivers_video-dri at 
kernel-bu
   |.org|gs.osdl.org
Product|ACPI|Drivers

--- Comment #1 from Aaron Lu  ---
(In reply to patrick.clara from comment #0)
> Created attachment 131421 [details]
> dsdt.dls
> 
> Hello.
> 
> On the asus ux51vz laptop screen backlight is not working. It is a win8
> laptop and has only one single nvidia gpu.
> 
> By default in /sys/class/backlight i have acpi_video0.
> Pressing the hotkeys actually result in a change in
> acpi_video0/actual_brightness.
> Also writing manually to acpi_video0/brightness results in a change in
> acpi_video0/actual_brightness.
> Apparently everything works except that the actual brightness of the screen
> doesnt change.
> Looking at the the _BCM method in the DSDT i saw that it calls STBR which
> does nothing it think. So the ACPI is broken in my case.

agree.

> I also tested video.use_native_backlight=1 but acpi_video0 still got
> registred, maybe becouse of the backlight_device_registered(BACKLIGHT_RAW)
> in this check:
> 
> if (acpi_osi_is_win8() && use_native_backlight &&
> backlight_device_registered(BACKLIGHT_RAW))

right.

> 
> If i understand correctly, since the only raw backlight device going to be
> registed is nv_backlight, and since nouveau requires the video module and
> therefore gets loaded afterwards, this condition will never become true.
> 
> I tried removing the backlight_device_registered(BACKLIGHT_RAW) and than
> video.use_native_backlight=1 do what it is supposed to do.
> 
> The problem remains in the way asus-wmi and nouveau check if they should
> register their own control devices. They use acpi_video_backlight_support()
> I think which is unaffected by the use_native_backlight parameter.

We will need to talk to nv gpu developers on this.
I don't think the following is the correct behaviour - whether ACPI video has
provided an interface doesn't matter if the gpu driver should provide a control
interface, the only criteria should be: the interface is not broken.

#ifdef CONFIG_ACPI
if (acpi_video_backlight_support()) {
NV_INFO(drm, "ACPI backlight interface available, "
 "not registering our own\n");
return 0;
}
#endif

> 
> Just for testing I tried removing the check from the nouveau driver and
> forcing it to register the nv_backlight.
> This way, booting with use_native_backlight=1 actually gives a system with
> working backlight, altough this solution is far from beeing optimal and also
> far from beeing out of the box.

We can easily add APIs in acpi video module to unregister its backlight
interface without losing its event reporting capability. But we will need to
talk to nv gpu developers on this first - is it the case for win8 systems, the
gpu is in control of the backlight? For many intel gpu based system it is the
case.

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

[PATCH] drm/exynos: use %pad for dma_addr_t

[Daniel Kurtz]

On Tue, Apr 22, 2014 at 1:45 PM, Jingoo Han  wrote:

> Use %pad for dma_addr_t, because a dma_addr_t type can vary
> based on build options. So, it prevents possible build warnings
> in printks.
>
> Signed-off-by: Jingoo Han 
>

Yay!

Reviewed-by: Daniel Kurtz 


> ---
>  drivers/gpu/drm/exynos/exynos_drm_dmabuf.c |2 +-
>  drivers/gpu/drm/exynos/exynos_drm_vidi.c   |2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
> b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
> index 59827cc..9cf71fa 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
> @@ -263,7 +263,7 @@ struct drm_gem_object
> *exynos_dmabuf_prime_import(struct drm_device *drm_dev,
> buffer->sgt = sgt;
> exynos_gem_obj->base.import_attach = attach;
>
> -   DRM_DEBUG_PRIME("dma_addr = 0x%x, size = 0x%lx\n",
> buffer->dma_addr,
> +   DRM_DEBUG_PRIME("dma_addr = %pad, size = 0x%lx\n",
> >dma_addr,
>
> buffer->size);
>
> return _gem_obj->base;
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_vidi.c
> b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
> index 3319289..e4689ef 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_vidi.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
> @@ -221,7 +221,7 @@ static void vidi_win_commit(struct exynos_drm_manager
> *mgr, int zpos)
>
> win_data->enabled = true;
>
> -   DRM_DEBUG_KMS("dma_addr = 0x%x\n", win_data->dma_addr);
> +   DRM_DEBUG_KMS("dma_addr = %pad\n", _data->dma_addr);
>
> if (ctx->vblank_on)
> schedule_work(>work);
> --
> 1.7.10.4
>
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
-- next part --
An HTML attachment was scrubbed...
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20140423/530db239/attachment.html>

[PATCH v2] drm/exynos: remove DRIVER_HAVE_IRQ feature

[Daniel Kurtz]

On Sat, Apr 12, 2014 at 8:39 AM, Joonyoung Shim wrote:

> Exynos drm driver cannot support DRIVER_HAVE_IRQ feature because it uses
> driver specific one instead of routine of drm framework to
> install/uninstall irq handler.
>
> Signed-off-by: Joonyoung Shim 
>

Reviewed-by: Daniel Kurtz 


> ---
> Changelog from v1:
>  - Keep below code
> drm_dev->irq_enabled = true;
>
>  drivers/gpu/drm/exynos/exynos_drm_drv.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> index 2d27ba2..027e32d 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> @@ -323,8 +323,7 @@ static const struct file_operations
> exynos_drm_driver_fops = {
>  };
>
>  static struct drm_driver exynos_drm_driver = {
> -   .driver_features= DRIVER_HAVE_IRQ | DRIVER_MODESET |
> -   DRIVER_GEM | DRIVER_PRIME,
> +   .driver_features= DRIVER_MODESET | DRIVER_GEM |
> DRIVER_PRIME,
> .load   = exynos_drm_load,
> .unload = exynos_drm_unload,
> .suspend= exynos_drm_suspend,
> --
> 1.8.1.2
>
>
-- next part --
An HTML attachment was scrubbed...
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20140423/99449fd9/attachment.html>

[Bug 77784] Certain mono based games hang the system with radeonsi (probably llvm backend)

[bugzilla-dae...@freedesktop.org]

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

Jos? Su?rez  changed:

   What|Removed |Added

  Attachment #97789|Little Racers STREET crash  |Picture of Little Racers
description||STREET crash

-- 
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/20140423/6a5e92a2/attachment-0001.html>

[Bug 77784] Certain mono based games hang the system with radeonsi (probably llvm backend)

[bugzilla-dae...@freedesktop.org]

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

Jos? Su?rez  changed:

   What|Removed |Added

  Attachment #97788|Xorg log|Xorg log (Foosball Street
description||Edition)

-- 
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/20140423/fb679e6a/attachment.html>

[Bug 77784] Certain mono based games hang the system with radeonsi (probably llvm backend)

[bugzilla-dae...@freedesktop.org]

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

Jos? Su?rez  changed:

   What|Removed |Added

  Attachment #97787|dmesg log   |dmesg log (Foosball Street
description||Edition)

-- 
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/20140423/f14f3cc9/attachment.html>

[Bug 77784] Certain mono based games hang the system with radeonsi (probably llvm backend)

[bugzilla-dae...@freedesktop.org]

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

--- Comment #6 from Jos? Su?rez  ---
I have also added a picture of the screen just after Little Racers STREET
crashed. I am afraid that the system hard locks and I can't get any dmesg or
Xorg logs to attach.

-- 
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/20140423/ec07633c/attachment.html>

[Bug 77784] Certain mono based games hang the system with radeonsi (probably llvm backend)

[bugzilla-dae...@freedesktop.org]

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

--- Comment #5 from Jos? Su?rez  ---
Created attachment 97789
  --> https://bugs.freedesktop.org/attachment.cgi?id=97789=edit
Little Racers STREET crash

-- 
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/20140423/26c0a749/attachment.html>

[Bug 77784] Certain mono based games hang the system with radeonsi (probably llvm backend)

[bugzilla-dae...@freedesktop.org]

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

--- Comment #4 from Jos? Su?rez  ---
I managed to switch to a VT between the a GPU recovery and a hang to save a
dmesg log. See the dmesg and Xorg logs in comments 2 and 3.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 

[Bug 77784] Certain mono based games hang the system with radeonsi (probably llvm backend)

[bugzilla-dae...@freedesktop.org]

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

--- Comment #3 from Jos? Su?rez  ---
Created attachment 97788
  --> https://bugs.freedesktop.org/attachment.cgi?id=97788=edit
Xorg log

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 

[Bug 77784] Certain mono based games hang the system with radeonsi (probably llvm backend)

[bugzilla-dae...@freedesktop.org]

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

--- Comment #2 from Jos? Su?rez  ---
Created attachment 97787
  --> https://bugs.freedesktop.org/attachment.cgi?id=97787=edit
dmesg log

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 

[git pull] drm fixes

[Dave Airlie]

Hi Linus,

this is just radeon fixes, primarily the two pll fix and the aux fix,
it also disables dpm on rv770 gpus, fixes driver reloading, and
fixes two issues with runtime PM on some GPUS.

Dave.

The following changes since commit 4d0fa8a0f01272d4de33704f20303dcecdb55df1:

  Merge tag 'gpio-v3.15-2' of 
git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio (2014-04-22 
09:28:02 -0700)

are available in the git repository at:


  git://people.freedesktop.org/~airlied/linux drm-fixes

for you to fetch changes up to abaafc0af9f74f8e6212a3bf54fb907358b40ad7:

  Merge branch 'drm-fixes-3.15' of 
git://people.freedesktop.org/~deathsimple/linux into drm-next (2014-04-23 
07:39:12 +1000)



Alex Deucher (7):
  drm/radeon: disable dpm on rv770 by default
  drm/radeon/aux: fix hpd assignment for aux bus
  drm/radeon: fix count in cik_sdma_ring_test()
  drm/radeon: properly unregister hwmon interface (v2)
  drm/radeon/pm: don't walk the crtc list before it has been initialized 
(v2)
  drm/radeon: fix ATPX detection on non-VGA GPUs
  drm/radeon: don't allow runpm=1 on systems with out ATPX

Christian K?nig (2):
  drm/radeon: use fixed PPL ref divider if needed
  drm/radeon: improve PLL limit handling in post div calculation

Dave Airlie (1):
  Merge branch 'drm-fixes-3.15' of 
git://people.freedesktop.org/~deathsimple/linux into drm-next

 drivers/gpu/drm/radeon/atombios_dp.c |  1 +
 drivers/gpu/drm/radeon/cik_sdma.c|  2 +-
 drivers/gpu/drm/radeon/r600_dpm.c| 35 ++--
 drivers/gpu/drm/radeon/radeon_atpx_handler.c |  7 +++
 drivers/gpu/drm/radeon/radeon_display.c  | 84 +++-
 drivers/gpu/drm/radeon/radeon_kms.c  |  8 +--
 drivers/gpu/drm/radeon/radeon_pm.c   | 51 ++---
 7 files changed, 120 insertions(+), 68 deletions(-)