[git pull] drm fixes

2015-05-04 Thread Dave Airlie

Hi Linus,

one intel fix, one rockchip fix, and a bunch of radeon fixes for
some regressions from audio rework and vm stability.

Dave.

The following changes since commit b787f68c36d49bb1d9236f403813641efa74a031:

  Linux 4.1-rc1 (2015-04-26 17:59:10 -0700)

are available in the git repository at:

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

for you to fetch changes up to 71aee81937963ccb07b3fa1b912e4cc6cd77dfa8:

  Merge tag 'drm-intel-fixes-2015-04-30' of 
git://anongit.freedesktop.org/drm-intel into drm-fixes (2015-05-04 08:56:47 
+1000)


Alex Deucher (7):
  drm/radeon: fix ordering of AVI packet setup
  drm/radeon: drop dce6_dp_enable
  drm/radeon/audio: don't enable packets until the end
  drm/radeon: only mark audio as connected if the monitor supports it (v3)
  drm/radeon: only enable audio streams if the monitor supports it
  drm/radeon: adjust pll when audio is not enabled
  drm/radeon: add SI DPM quirk for Sapphire R9 270 Dual-X 2G GDDR5

Christian König (4):
  drm/radeon: fix lockup when BOs aren't part of the VM on release
  drm/radeon: reset BOs address after clearing it.
  drm/radeon: check new address before removing old one
  drm/radeon: fix userptr return value checking (v2)

Dave Airlie (3):
  Merge branch 'drm-fixes-4.1' of git://people.freedesktop.org/~agd5f/linux 
into drm-fixes
  Merge branch 'drm-next0420' of 
https://github.com/markyzq/kernel-drm-rockchip into drm-fixes
  Merge tag 'drm-intel-fixes-2015-04-30' of 
git://anongit.freedesktop.org/drm-intel into drm-fixes

Deepak S (1):
  drm/i915/chv: Implement WaDisableShadowRegForCpd

Heiko Stuebner (2):
  MAINTAINERS: add entry for Rockchip drm drivers
  drm/rockchip: fix error check when getting irq

Michel Dänzer (1):
  drm/radeon: Use drm_calloc_ab for CS relocs

 MAINTAINERS |  7 
 drivers/gpu/drm/i915/i915_reg.h |  2 ++
 drivers/gpu/drm/i915/intel_uncore.c |  8 +
 drivers/gpu/drm/radeon/atombios_crtc.c  |  3 ++
 drivers/gpu/drm/radeon/atombios_encoders.c  |  6 ++--
 drivers/gpu/drm/radeon/dce6_afmt.c  | 25 --
 drivers/gpu/drm/radeon/evergreen_hdmi.c | 53 ++---
 drivers/gpu/drm/radeon/r600_hdmi.c  |  9 ++---
 drivers/gpu/drm/radeon/radeon_audio.c   | 30 
 drivers/gpu/drm/radeon/radeon_connectors.c  |  8 +++--
 drivers/gpu/drm/radeon/radeon_cs.c  |  4 +--
 drivers/gpu/drm/radeon/radeon_mn.c  | 10 +++---
 drivers/gpu/drm/radeon/radeon_vm.c  | 36 
 drivers/gpu/drm/radeon/si_dpm.c |  1 +
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c |  9 ++---
 15 files changed, 117 insertions(+), 94 deletions(-)


Tiny fixes - resent

2015-05-04 Thread Mario Kleiner
Hi, a resend of updated versions of the earlier patches:

Patch 1 should really get into Linux 4.1 to avoid tegra breaking
user-space clients.

Patch 2 reviewed-by Michel, updated to take his feedback into account.

Patch 3 is modified to not conflict with Daniel Vetter's patch
"drm/vblank: Fixup and document timestamp update/read barriers"

Patch 4 will be needed for drm/qxl to compile with Daniel's fixup
patch applied.

thanks,
-mario



[PATCH 1/5] drm/tegra: Don't use vblank_disable_immediate on incapable driver.

2015-05-04 Thread Mario Kleiner
Tegra would not only need a hardware vblank counter that
increments at leading edge of vblank, but also support
for instantaneous high precision vblank timestamp queries, ie.
a proper implementation of dev->driver->get_vblank_timestamp().

Without these, there can be off-by-one errors during vblank
disable/enable if the scanout is inside vblank at en/disable
time, and additionally clients will never see any useable
vblank timestamps when querying via drmWaitVblank ioctl. This
would negatively affect swap scheduling under X11 and Wayland.

Signed-off-by: Mario Kleiner 
Cc: Thierry Reding 
Cc: Dave Airlie 
---
 drivers/gpu/drm/tegra/drm.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index 1833abd..bfad15a 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -173,7 +173,6 @@ static int tegra_drm_load(struct drm_device *drm, unsigned 
long flags)
drm->irq_enabled = true;

/* syncpoints are used for full 32-bit hardware VBLANK counters */
-   drm->vblank_disable_immediate = true;
drm->max_vblank_count = 0x;

err = drm_vblank_init(drm, drm->mode_config.num_crtc);
-- 
1.9.1



[PATCH 2/5] drm: Prevent invalid use of vblank_disable_immediate. (v2)

2015-05-04 Thread Mario Kleiner
For a kms driver to support immediate disable of vblank
irq's reliably without introducing off by one errors or
other mayhem for clients, it must not only support a
hardware vblank counter query, but also high precision
vblank timestamping, so vblank count and timestamp can be
instantaneously reinitialzed to valid values. Additionally
the exposed hardware counter must behave as if it is
incrementing at leading edge of vblank to avoid off by
one errors during reinitialization of the counter while
the display happens to be inside or close to vblank.

Check during drm_vblank_init that a driver which claims to
be capable of vblank_disable_immediate at least supports
high precision timestamping and prevent use of instant
disable if that isn't present as a minimum requirement.

v2: Changed from DRM_ERROR to DRM_INFO and made message
more clear, as suggested by Michel Dänzer.

Signed-off-by: Mario Kleiner 
Reviewed-by: Michel Dänzer 

Cc: Dave Airlie 
---
 drivers/gpu/drm/drm_irq.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 9c166b4..152d1de 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -352,6 +352,13 @@ int drm_vblank_init(struct drm_device *dev, int num_crtcs)
else
DRM_INFO("No driver support for vblank timestamp query.\n");

+   /* Must have precise timestamping for reliable vblank instant disable */
+   if (dev->vblank_disable_immediate && 
!dev->driver->get_vblank_timestamp) {
+   dev->vblank_disable_immediate = false;
+   DRM_INFO("Setting vblank_disable_immediate to false because "
+"get_vblank_timestamp == NULL\n");
+   }
+
dev->vblank_disable_allowed = false;

return 0;
-- 
1.9.1



[PATCH 3/5] drm: Zero out invalid vblank timestamp in drm_update_vblank_count. (v2)

2015-05-04 Thread Mario Kleiner
Since commit 844b03f27739135fe1fed2fef06da0ffc4c7a081 we make
sure that after vblank irq off, we return the last valid
(vblank count, vblank timestamp) pair to clients, e.g., during
modesets, which is good.

An overlooked side effect of that commit for kms drivers without
support for precise vblank timestamping is that at vblank irq
enable, when we update the vblank counter from the hw counter, we
can't update the corresponding vblank timestamp, so now we have a
totally mismatched timestamp for the new count to confuse clients.

Restore old client visible behaviour from before Linux 3.18, but
zero out the timestamp at vblank counter update (instead of disable
as in original implementation) if we can't generate a meaningful
timestamp immediately for the new vblank counter. This will fix
this regression, so callers know they need to retry again later
if they need a valid timestamp, but at the same time preserves
the improvements made in the commit mentioned above.

v2: Rebased on top of Daniel Vetter's fixup and documentation
patch for timestamp updates. Drop request for stable kernel
backport as this would be more difficult, unless the original
patch would get applied to stable kernels.

Signed-off-by: Mario Kleiner 

Cc: Ville Syrjälä 
Cc: Daniel Vetter 
Cc: Dave Airlie 
---
 drivers/gpu/drm/drm_irq.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 152d1de..44e6a20b 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -161,10 +161,13 @@ static void drm_update_vblank_count(struct drm_device 
*dev, int crtc)

/*
 * Only reinitialize corresponding vblank timestamp if high-precision 
query
-* available and didn't fail. Will reinitialize delayed at next vblank
-* interrupt in that case.
+* available and didn't fail. Otherwise reinitialize delayed at next 
vblank
+* interrupt and assign 0 for now, to mark the vblanktimestamp as 
invalid.
 */
-   store_vblank(dev, crtc, diff, rc ? &t_vblank : NULL);
+   if (!rc)
+   t_vblank = (struct timeval) {0, 0};
+
+   store_vblank(dev, crtc, diff, &t_vblank);
 }

 /*
-- 
1.9.1



[PATCH 4/5] drm/qxl: Fix qxl_noop_get_vblank_counter()

2015-05-04 Thread Mario Kleiner
This breaks under the vblank timestamp cleanup patch
by Daniel Vetter. Also it is pointless to return anything
but zero (or any other constant) if the function doesn't
actually query a hw vblank counter. The bogus return of
the current drm vblank counter via direct readout or via
drm_vblank_count() is found in many of the new kms drivers,
but it does exactly nothing different from returning any
arbitrary constant - it's a no operation.

Let's simply return 0 - Easy and fast.

Signed-off-by: Mario Kleiner 
Cc: Dave Airlie 
---
 drivers/gpu/drm/qxl/qxl_drv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
index 1d9b80c..577dc45 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.c
+++ b/drivers/gpu/drm/qxl/qxl_drv.c
@@ -198,7 +198,7 @@ static int qxl_pm_restore(struct device *dev)

 static u32 qxl_noop_get_vblank_counter(struct drm_device *dev, int crtc)
 {
-   return dev->vblank[crtc].count.counter;
+   return 0;
 }

 static int qxl_noop_enable_vblank(struct drm_device *dev, int crtc)
-- 
1.9.1



[Intel-gfx] [PATCH] drm/vblank: Fixup and document timestamp update/read barriers

2015-05-04 Thread Mario Kleiner
On 04/16/2015 03:03 PM, Daniel Vetter wrote:
> On Thu, Apr 16, 2015 at 08:30:55AM -0400, Peter Hurley wrote:
>> On 04/15/2015 01:31 PM, Daniel Vetter wrote:
>>> On Wed, Apr 15, 2015 at 09:00:04AM -0400, Peter Hurley wrote:
 Hi Daniel,

 On 04/15/2015 03:17 AM, Daniel Vetter wrote:
> This was a bit too much cargo-culted, so lets make it solid:
> - vblank->count doesn't need to be an atomic, writes are always done
>under the protection of dev->vblank_time_lock. Switch to an unsigned
>long instead and update comments. Note that atomic_read is just a
>normal read of a volatile variable, so no need to audit all the
>read-side access specifically.
>
> - The barriers for the vblank counter seqlock weren't complete: The
>read-side was missing the first barrier between the counter read and
>the timestamp read, it only had a barrier between the ts and the
>counter read. We need both.
>
> - Barriers weren't properly documented. Since barriers only work if
>you have them on boths sides of the transaction it's prudent to
>reference where the other side is. To avoid duplicating the
>write-side comment 3 times extract a little store_vblank() helper.
>In that helper also assert that we do indeed hold
>dev->vblank_time_lock, since in some cases the lock is acquired a
>few functions up in the callchain.
>
> Spotted while reviewing a patch from Chris Wilson to add a fastpath to
> the vblank_wait ioctl.
>
> Cc: Chris Wilson 
> Cc: Mario Kleiner 
> Cc: Ville Syrjälä 
> Cc: Michel Dänzer 
> Signed-off-by: Daniel Vetter 
> ---
>   drivers/gpu/drm/drm_irq.c | 92 
> ---
>   include/drm/drmP.h|  8 +++--
>   2 files changed, 54 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> index c8a34476570a..23bfbc61a494 100644
> --- a/drivers/gpu/drm/drm_irq.c
> +++ b/drivers/gpu/drm/drm_irq.c
> @@ -74,6 +74,33 @@ module_param_named(vblankoffdelay, 
> drm_vblank_offdelay, int, 0600);
>   module_param_named(timestamp_precision_usec, drm_timestamp_precision, 
> int, 0600);
>   module_param_named(timestamp_monotonic, drm_timestamp_monotonic, int, 
> 0600);
>
> +static void store_vblank(struct drm_device *dev, int crtc,
> +  unsigned vblank_count_inc,
> +  struct timeval *t_vblank)
> +{
> + struct drm_vblank_crtc *vblank = &dev->vblank[crtc];
> + u32 tslot;
> +
> + assert_spin_locked(&dev->vblank_time_lock);
> +
> + if (t_vblank) {
> + tslot = vblank->count + vblank_count_inc;
> + vblanktimestamp(dev, crtc, tslot) = *t_vblank;
> + }
> +
> + /*
> +  * vblank timestamp updates are protected on the write side with
> +  * vblank_time_lock, but on the read side done locklessly using a
> +  * sequence-lock on the vblank counter. Ensure correct ordering using
> +  * memory barrriers. We need the barrier both before and also after the
> +  * counter update to synchronize with the next timestamp write.
> +  * The read-side barriers for this are in drm_vblank_count_and_time.
> +  */
> + smp_wmb();
> + vblank->count += vblank_count_inc;
> + smp_wmb();

 The comment and the code are each self-contradictory.

 If vblank->count writes are always protected by vblank_time_lock 
 (something I
 did not verify but that the comment above asserts), then the trailing write
 barrier is not required (and the assertion that it is in the comment is 
 incorrect).

 A spin unlock operation is always a write barrier.
>>>
>>> Hm yeah. Otoh to me that's bordering on "code too clever for my own good".
>>> That the spinlock is held I can assure. That no one goes around and does
>>> multiple vblank updates (because somehow that code raced with the hw
>>> itself) I can't easily assure with a simple assert or something similar.
>>> It's not the case right now, but that can changes.
>>
>> The algorithm would be broken if multiple updates for the same vblank
>> count were allowed; that's why it checks to see if the vblank count has
>> not advanced before storing a new timestamp.
>>
>> Otherwise, the read side would not be able to determine that the
>> timestamp is valid by double-checking that the vblank count has not
>> changed.
>>
>> And besides, even if the code looped without dropping the spinlock,
>> the correct write order would still be observed because it would still
>> be executing on the same cpu.
>>
>> My objection to the write memory barrier is not about optimization;
>> it's about correct code.
>
> Well diff=0 is not allowed, I guess I could enforce this with some
> WARN_ON. And I still think my point of non-local correctness is solid.
> Wit

[PATCH] drm: Defer disabling the vblank IRQ until the next interrupt (for instant-off)

2015-05-04 Thread Mario Kleiner


On 04/15/2015 03:03 AM, Mario Kleiner wrote:
> On 04/02/2015 01:34 PM, Chris Wilson wrote:
>> On vblank instant-off systems, we can get into a situation where the cost
>> of enabling and disabling the vblank IRQ around a drmWaitVblank query
>> dominates. However, we know that if the user wants the current vblank
>> counter, they are also very likely to immediately queue a vblank wait
>> and so we can keep the interrupt around and only turn it off if we have
>> no further vblank requests in the interrupt interval.
>>
>> After vblank event delivery there is a shadow of one vblank where the
>> interrupt is kept alive for the user to query and queue another vblank
>> event. Similarly, if the user is using blocking drmWaitVblanks, the
>> interrupt will be disabled on the IRQ following the wait completion.
>> However, if the user is simply querying the current vblank counter and
>> timestamp, the interrupt will be disabled after every IRQ and the user
>> will enabled it again on the first query following the IRQ.
>>
>> Testcase: igt/kms_vblank
>> Signed-off-by: Chris Wilson 
>> Cc: Ville Syrjälä 
>> Cc: Daniel Vetter 
>> Cc: Michel Dänzer 
>> Cc: Laurent Pinchart 
>> Cc: Dave Airlie ,
>> Cc: Mario Kleiner 
>> ---
>>   drivers/gpu/drm/drm_irq.c | 15 +--
>>   1 file changed, 13 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
>> index c8a34476570a..6f5dc18779e2 100644
>> --- a/drivers/gpu/drm/drm_irq.c
>> +++ b/drivers/gpu/drm/drm_irq.c
>> @@ -1091,9 +1091,9 @@ void drm_vblank_put(struct drm_device *dev, int
>> crtc)
>>   if (atomic_dec_and_test(&vblank->refcount)) {
>>   if (drm_vblank_offdelay == 0)
>>   return;
>> -else if (dev->vblank_disable_immediate || drm_vblank_offdelay
>> < 0)
>> +else if (drm_vblank_offdelay < 0)
>>   vblank_disable_fn((unsigned long)vblank);
>> -else
>> +else if (!dev->vblank_disable_immediate)
>>   mod_timer(&vblank->disable_timer,
>> jiffies + ((drm_vblank_offdelay * HZ)/1000));
>>   }
>> @@ -1697,6 +1697,17 @@ bool drm_handle_vblank(struct drm_device *dev,
>> int crtc)
>>
>>   spin_lock_irqsave(&dev->event_lock, irqflags);
>>
>
> You could move the code before the spin_lock_irqsave(&dev->event_lock,
> irqflags); i think it doesn't need that lock?
>
>> +if (dev->vblank_disable_immediate &&
>> !atomic_read(&vblank->refcount)) {
>
> Also check for (drm_vblank_offdelay > 0) to make sure we have a way out
> of instant disable here, and the same meaning of of drm_vblank_offdelay
> like we have in the current implementation.
>
> This hunk ...
>
>> +unsigned long vbl_lock_irqflags;
>> +
>> +spin_lock_irqsave(&dev->vbl_lock, vbl_lock_irqflags);
>> +if (atomic_read(&vblank->refcount) == 0 && vblank->enabled) {
>> +DRM_DEBUG("disabling vblank on crtc %d\n", crtc);
>> +vblank_disable_and_save(dev, crtc);
>> +}
>> +spin_unlock_irqrestore(&dev->vbl_lock, vbl_lock_irqflags);
>
> ... is the same as a call to vblank_disable_fn((unsigned long) vblank);
> Maybe replace by that call?
>
> You could also return here already, as the code below will just take a
> lock, realize vblanks are now disabled and then release the locks and exit.
>
>> +}
>> +
>>   /* Need timestamp lock to prevent concurrent execution with
>>* vblank enable/disable, as this would cause inconsistent
>>* or corrupted timestamps and vblank counts.
>>
>
> I think the logic itself is fine and at least basic testing of the patch
> on a Intel HD Ironlake didn't show problems, so with the above taken
> into account it would have my slightly uneasy reviewed-by.
>
> One thing that worries me a little bit about the disable inside vblank
> irq are the potential races between the disable code and the display
> engine which could cause really bad off-by-one errors for clients on a
> imperfect driver. These races can only happen if vblank enable or
> disable happens close to or inside the vblank. This approach lets the
> instant disable happen exactly inside vblank when there is the highest
> chance of triggering that condition.
>
> This doesn't seem to be a problem for intel kms, but other drivers don't
> have instant disable yet, so we don't know how well we could do it
> there. Additionally things like dynamic power management tend to operate
> inside vblank, sometimes with "funny" side effects to other stuff, e.g.,
> dpm on AMD, as i remember from some long debug session with Michel and
> Alex last summer where dpm played a role. Therefore it seems more safe
> to me to avoid actions inside vblank that could be done outside. E.g.,
> instead of doing the disable inside the vblank irq one could maybe just
> schedule an exact timer to do the disable a few milliseconds later in
> the middle of active scanout to avoid these potential issues?
>
> -mario

After testing this, one more 

[PATCH 1/2] Add device enumeration interface (v2)

2015-05-04 Thread Zhou, Jammy
> 1) work for vendor A but not for B or 2) will be OK for B but will produce 
> false positives for A.
For such kind of cases, IMHO we probably can have vendor specific 
implementations.

> A trivial lookup in sysfs will be able to provide all the required 
> information, won't you agree ?
Yes, I agree. I think we can use the udev interfaces for such kind of 
enumeration by doing the match on the drm subsystem (the assumption is that the 
drm driver is loaded for the graphics device already). We will do some 
prototyping with udev for this.

Regards,
Jammy

-Original Message-
From: Emil Velikov [mailto:emil.l.veli...@gmail.com] 
Sent: Wednesday, April 29, 2015 12:18 AM
To: Zhou, Jammy
Cc: dri-devel at lists.freedesktop.org; Min, Frank
Subject: Re: [PATCH 1/2] Add device enumeration interface (v2)

On 28 April 2015 at 04:26, Zhou, Jammy  wrote:
> Hi Emil,
>
> This interface is intended for multiple GPU support. For example, we need to 
> know how many GPU devices are available on the system (and for some specific 
> vendor) in the client drivers, and then we can select proper devices for 
> rendering/compute/etc. For current mesa and Xserver implementations, the 
> device enumeration is done separately. I think it will be helpful if we can 
> have such kind of function in libdrm core, which can also be leveraged by 
> other new APIs requiring multi GPU support.
>
Hmm I'm not sure how the proposed interface will ease either mesa or xserver's 
implementation. The former is used only for clover(opencl) and already handles 
platform devices. While for the server I believe (haven't checked) that it just 
"throws" the PCI device information to the ddx and lets the latter do its thing.

>> Any particular reason why "3D controller" (0x32000) is omitted ?
> No. For AMD cards, we currently have 0x3 and 0x38000. Is 0x32000 used by 
> Nvidia cards? If so, I think we should add it as well.
>
What I am thinking is that using heuristics such as these will either
1) work for vendor A but not for B or 2) will be OK for B but will produce 
false positives for A.

>> Using libpciaccess, will give you the number of PCI devices available on the 
>> system rather than the ones accessible - think about platform devices and/or 
>> devices without a drm driver.
> This interface is just to enumerate the PCIE GPU devices on the system. With 
> regard to which ones are accessible, we can use drmOpen/drmOpenWithType to 
> check, and I don't want to have duplicated functionalities for these 
> interfaces. And for those non-PCIE platform devices (mostly on ARM 
> platforms), this interface shouldn't be used, and instead the client drivers 
> should handle it by themselves.
>
I am against duplication, to the point that I may have alienated a person or 
two :-\ Although this function as-is won't bring much benefit to mesa/xserver 
afaict. Plus it would be nice to keep an open mind for platform world, so that 
things will just work when AMD decides to go that road. Not to mention that 
iterating through all the devices in drmOpen* just to find that the device at 
pci:X:Y provides only a primary/render node seems a bit wasteful.


A trivial lookup in sysfs will be able to provide all the required information, 
won't you agree ?


Cheers,
Emil


[PATCH v2 1/4] drm/exynos: fb: use drm_format_num_planes to get buffer count

2015-05-04 Thread Inki Dae
On 2015년 04월 28일 06:10, Tobias Jakobi wrote:
> The previous code had some special case handling for the buffer
> count in exynos_drm_format_num_buffers().
> 
> This code was incorrect though, since this special case doesn't
> exist for DRM. It stemmed from the existence of the special NV12M
> V4L2 format. NV12 is a bi-planar format (separate planes for luma
> and chroma) and V4L2 differentiates between a NV12 buffer where
> luma and chroma is contiguous in memory (so no data between
> luma/chroma), and a NV12 buffer where luma and chroma have two
> explicit memory locations (which is then called NV12M).
> 
> This distinction doesn't exist for DRM. A bi-planar format always
> explicitly comes with the information about its two planes (even
> if these planes should be contiguous).
> 

For all patches, Applied.

Thanks,
Inki Dae

> Signed-off-by: Tobias Jakobi 
> ---
>  drivers/gpu/drm/exynos/exynos_drm_fb.c | 39 
> +-
>  1 file changed, 1 insertion(+), 38 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fb.c 
> b/drivers/gpu/drm/exynos/exynos_drm_fb.c
> index 929cb03..142eb4e 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fb.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fb.c
> @@ -171,43 +171,6 @@ exynos_drm_framebuffer_init(struct drm_device *dev,
>   return &exynos_fb->fb;
>  }
>  
> -static u32 exynos_drm_format_num_buffers(struct drm_mode_fb_cmd2 *mode_cmd)
> -{
> - unsigned int cnt = 0;
> -
> - if (mode_cmd->pixel_format != DRM_FORMAT_NV12)
> - return drm_format_num_planes(mode_cmd->pixel_format);
> -
> - while (cnt != MAX_FB_BUFFER) {
> - if (!mode_cmd->handles[cnt])
> - break;
> - cnt++;
> - }
> -
> - /*
> -  * check if NV12 or NV12M.
> -  *
> -  * NV12
> -  * handles[0] = base1, offsets[0] = 0
> -  * handles[1] = base1, offsets[1] = Y_size
> -  *
> -  * NV12M
> -  * handles[0] = base1, offsets[0] = 0
> -  * handles[1] = base2, offsets[1] = 0
> -  */
> - if (cnt == 2) {
> - /*
> -  * in case of NV12 format, offsets[1] is not 0 and
> -  * handles[0] is same as handles[1].
> -  */
> - if (mode_cmd->offsets[1] &&
> - mode_cmd->handles[0] == mode_cmd->handles[1])
> - cnt = 1;
> - }
> -
> - return cnt;
> -}
> -
>  static struct drm_framebuffer *
>  exynos_user_fb_create(struct drm_device *dev, struct drm_file *file_priv,
> struct drm_mode_fb_cmd2 *mode_cmd)
> @@ -230,7 +193,7 @@ exynos_user_fb_create(struct drm_device *dev, struct 
> drm_file *file_priv,
>  
>   drm_helper_mode_fill_fb_struct(&exynos_fb->fb, mode_cmd);
>   exynos_fb->exynos_gem_obj[0] = to_exynos_gem_obj(obj);
> - exynos_fb->buf_cnt = exynos_drm_format_num_buffers(mode_cmd);
> + exynos_fb->buf_cnt = drm_format_num_planes(mode_cmd->pixel_format);
>  
>   DRM_DEBUG_KMS("buf_cnt = %d\n", exynos_fb->buf_cnt);
>  
> 



[PATCH v3] drm/exynos: mixer: cleanup pixelformat handling

2015-05-04 Thread Inki Dae
On 2015년 04월 28일 06:11, Tobias Jakobi wrote:
> Move the defines for the pixelformats that the mixer supports out
> of mixer_graph_buffer() to the top of the source.
> Then select the mixer pixelformat (pf) in mixer_graph_buffer() based on
> the plane's pf (and not bpp).
> Also add handling of RGB565 and XRGB1555 to the switch statement and
> exit early if the plane has an unsupported pf.

Applied.

Thanks,
Inki Dae

> 
> Partially based on 'drm/exynos: enable/disable blend based on pixel
> format' by Gustavo Padovan .
> 
> v2: Use the shorter MXR_FORMAT as prefix.
> v3: Re-add ARGB because of compatibility reasons
> (suggested by Joonyoung Shim).
> 
> Reviewed-by: Gustavo Padovan 
> Signed-off-by: Tobias Jakobi 
> ---
>  drivers/gpu/drm/exynos/exynos_mixer.c | 33 +++--
>  1 file changed, 23 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c 
> b/drivers/gpu/drm/exynos/exynos_mixer.c
> index fbec750..0474fd3 100644
> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
> @@ -44,6 +44,12 @@
>  #define MIXER_WIN_NR 3
>  #define MIXER_DEFAULT_WIN0
>  
> +/* The pixelformats that are natively supported by the mixer. */
> +#define MXR_FORMAT_RGB5654
> +#define MXR_FORMAT_ARGB1555  5
> +#define MXR_FORMAT_ARGB  6
> +#define MXR_FORMAT_ARGB  7
> +
>  struct mixer_resources {
>   int irq;
>   void __iomem*mixer_regs;
> @@ -531,20 +537,27 @@ static void mixer_graph_buffer(struct mixer_context 
> *ctx, int win)
>  
>   plane = &ctx->planes[win];
>  
> - #define RGB565 4
> - #define ARGB1555 5
> - #define ARGB 6
> - #define ARGB 7
> + switch (plane->pixel_format) {
> + case DRM_FORMAT_XRGB:
> + fmt = MXR_FORMAT_ARGB;
> + break;
> +
> + case DRM_FORMAT_XRGB1555:
> + fmt = MXR_FORMAT_ARGB1555;
> + break;
>  
> - switch (plane->bpp) {
> - case 16:
> - fmt = ARGB;
> + case DRM_FORMAT_RGB565:
> + fmt = MXR_FORMAT_RGB565;
>   break;
> - case 32:
> - fmt = ARGB;
> +
> + case DRM_FORMAT_XRGB:
> + case DRM_FORMAT_ARGB:
> + fmt = MXR_FORMAT_ARGB;
>   break;
> +
>   default:
> - fmt = ARGB;
> + DRM_DEBUG_KMS("pixelformat unsupported by mixer\n");
> + return;
>   }
>  
>   /* check if mixer supports requested scaling setup */
> 



[RFC] drm/exynos: rework layer blending setup

2015-05-04 Thread Inki Dae
Hi Tobias,

To make patch files, you could use below command,
#git format-patch --cover-letter from..to

With this command, a cover file will be created and you could describe
what this patch series mean in the cover file.

Thanks,
Inki Dae

On 2015년 04월 30일 23:56, Tobias Jakobi wrote:
> Hello,
> 
> here's the rework of the layer blending setup that I discussed with Joonyoung 
> in the past days. There's still 
> some TODOs in the code, but more or less it does what it's supposed to do. 
> What still bothers me a bit is that I 
> currently call blending reconfig in mixer_cfg_layer(). It would be nice if 
> this could be reduced to one call per 
> "frame" (so with the last win_{commit,disable} call). Maybe atomic provides 
> such an infrastructure?
> 
> With best wishes,
> Tobias
> 
> 



[Y2038] [PATCH v4 06/10] cec: add HDMI CEC framework: y2038 question

2015-05-04 Thread Hans Verkuil
Ping! (Added Arnd to the CC list)

On 04/27/2015 09:40 AM, Hans Verkuil wrote:
> Added the y2038 mailinglist since I would like to get their input for
> this API.
> 
> Y2038 experts, can you take a look at my comment in the code below?
> 
> Thanks!

Arnd, I just saw your patch series adding struct __kernel_timespec to
uapi/linux/time.h. I get the feeling that it might take a few kernel
cycles before we have a timespec64 available in userspace. Based on that
I think this CEC API should drop the timestamps for now and wait until
timespec64 becomes available before adding it.

The timestamps are a nice-to-have, but not critical. So adding it later
shouldn't be a problem. What is your opinion?

Hans

> 
> On 04/23/2015 03:03 PM, Kamil Debski wrote:
>> From: Hans Verkuil 
>>
>> The added HDMI CEC framework provides a generic kernel interface for
>> HDMI CEC devices.
>>
>> Signed-off-by: Hans Verkuil 
>> [k.debski at samsung.com: Merged CEC Updates commit by Hans Verkuil]
>> [k.debski at samsung.com: Merged Update author commit by Hans Verkuil]
>> [k.debski at samsung.com: change kthread handling when setting logical
>> address]
>> [k.debski at samsung.com: code cleanup and fixes]
>> [k.debski at samsung.com: add missing CEC commands to match spec]
>> [k.debski at samsung.com: add RC framework support]
>> [k.debski at samsung.com: move and edit documentation]
>> [k.debski at samsung.com: add vendor id reporting]
>> [k.debski at samsung.com: add possibility to clear assigned logical
>> addresses]
>> [k.debski at samsung.com: documentation fixes, clenaup and expansion]
>> [k.debski at samsung.com: reorder of API structs and add reserved fields]
>> [k.debski at samsung.com: fix handling of events and fix 32/64bit timespec
>> problem]
>> [k.debski at samsung.com: add cec.h to include/uapi/linux/Kbuild]
>> Signed-off-by: Kamil Debski 
>> ---
>>  Documentation/cec.txt |  396 
>>  drivers/media/Kconfig |6 +
>>  drivers/media/Makefile|2 +
>>  drivers/media/cec.c   | 1161 
>> +
>>  include/media/cec.h   |  140 ++
>>  include/uapi/linux/Kbuild |1 +
>>  include/uapi/linux/cec.h  |  303 
>>  7 files changed, 2009 insertions(+)
>>  create mode 100644 Documentation/cec.txt
>>  create mode 100644 drivers/media/cec.c
>>  create mode 100644 include/media/cec.h
>>  create mode 100644 include/uapi/linux/cec.h
>>
> 
> 
> 
>> diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild
>> index 4842a98..5854cfd 100644
>> --- a/include/uapi/linux/Kbuild
>> +++ b/include/uapi/linux/Kbuild
>> @@ -81,6 +81,7 @@ header-y += capi.h
>>  header-y += cciss_defs.h
>>  header-y += cciss_ioctl.h
>>  header-y += cdrom.h
>> +header-y += cec.h
>>  header-y += cgroupstats.h
>>  header-y += chio.h
>>  header-y += cm4000_cs.h
>> diff --git a/include/uapi/linux/cec.h b/include/uapi/linux/cec.h
>> new file mode 100644
>> index 000..bb6d66c
>> --- /dev/null
>> +++ b/include/uapi/linux/cec.h
>> @@ -0,0 +1,303 @@
>> +#ifndef _CEC_H
>> +#define _CEC_H
>> +
>> +#include 
>> +
>> +struct cec_time {
>> +__u64 sec;
>> +__u64 nsec;
>> +};
> 
> I don't like having to introduce a new struct for time here. Basically we are
> only doing this because there is still no public struct timespec64.
> 
> When will that struct become available for use in a public API? If it is 4.2,
> then we can start using it. If it will take longer, then what alternative can
> we use to prevent having to change the API later?
> 
> One alternative might be to drop it for now and just reserve space in the
> structs to add it later.
> 
> Input from the y2038 experts will be welcome!
> 
> Regards,
> 
>   Hans
> 
>> +
>> +struct cec_msg {
>> +struct cec_time ts;
>> +__u32 len;
>> +__u32 status;
>> +__u32 timeout;
>> +/* timeout (in ms) is used to timeout CEC_RECEIVE.
>> +   Set to 0 if you want to wait forever. */
>> +__u8  msg[16];
>> +__u8  reply;
>> +/* If non-zero, then wait for a reply with this opcode.
>> +   If there was an error when sending the msg or FeatureAbort
>> +   was returned, then reply is set to 0.
>> +   If reply is non-zero upon return, then len/msg are set to
>> +   the received message.
>> +   If reply is zero upon return and status has the
>> +   CEC_TX_STATUS_FEATURE_ABORT bit set, then len/msg are set to the
>> +   received feature abort message.
>> +   If reply is zero upon return and status has the
>> +   CEC_TX_STATUS_REPLY_TIMEOUT
>> +   bit set, then no reply was seen at all.
>> +   This field is ignored with CEC_RECEIVE.
>> +   If reply is non-zero for CEC_TRANSMIT and the message is a broadcast,
>> +   then -EINVAL is returned.
>> +   if reply is non-zero, then timeout is set to 1000 (the required
>> +   maximum response time).
>> + */
>> +__u8 reserved[31];
>> +};
>> +
>> +static inline __u8 cec_msg_initiator(const struct

[PATCH] drm/exynos: Fix build breakage on !DRM_EXYNOS_FIMD

2015-05-04 Thread Inki Dae
On 2015년 05월 02일 13:08, Krzysztof Kozlowski wrote:
> Selecting CONFIG_FB_S3C disables CONFIG_DRM_EXYNOS_FIMD leading to build
> error:

No, eDP has no any dependency of FIMD but DECON. Just add dependency
code like below,

 config DRM_EXYNOS7_DECON
bool "Exynos DRM DECON"
-   depends on DRM_EXYNOS
+   depends on DRM_EXYNOS && !FB_S3C

> 
> drivers/built-in.o: In function `exynos_dp_dpms':
> binder.c:(.text+0xd6a840): undefined reference to `fimd_dp_clock_enable'
> binder.c:(.text+0xd6ab54): undefined reference to `fimd_dp_clock_enable'
> 
> Signed-off-by: Krzysztof Kozlowski 
> ---
>  drivers/gpu/drm/exynos/exynos_drm_fimd.h | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.h 
> b/drivers/gpu/drm/exynos/exynos_drm_fimd.h
> index b4fcaa568456..db67f3d9786d 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.h
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.h
> @@ -10,6 +10,10 @@
>  #ifndef _EXYNOS_DRM_FIMD_H_
>  #define _EXYNOS_DRM_FIMD_H_
>  
> +#ifdef CONFIG_DRM_EXYNOS_FIMD
>  extern void fimd_dp_clock_enable(struct exynos_drm_crtc *crtc, bool enable);
> +#else
> +static inline void fimd_dp_clock_enable(struct exynos_drm_crtc *crtc, bool 
> enable) {};
> +#endif

So above codes are unnecessary. It's really not good to add #ifdef ~ #endif.

Thanks,
Inki Dae

>  
>  #endif /* _EXYNOS_DRM_FIMD_H_ */
> 



[PATCH] drm/exynos: mixer: Constify platform_device_id

2015-05-04 Thread Inki Dae
On 2015년 05월 02일 00:56, Krzysztof Kozlowski wrote:
> The platform_device_id is not modified by the driver and core uses it as
> const.

Applied.

Thanks,
Inki Dae

> 
> Signed-off-by: Krzysztof Kozlowski 
> ---
>  drivers/gpu/drm/exynos/exynos_mixer.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c 
> b/drivers/gpu/drm/exynos/exynos_mixer.c
> index fbec750574e6..512549a35e96 100644
> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
> @@ -1156,7 +1156,7 @@ static struct mixer_drv_data exynos4210_mxr_drv_data = {
>   .has_sclk = 1,
>  };
>  
> -static struct platform_device_id mixer_driver_types[] = {
> +static const struct platform_device_id mixer_driver_types[] = {
>   {
>   .name   = "s5p-mixer",
>   .driver_data= (unsigned long)&exynos4210_mxr_drv_data,
> 



[Bug 73530] Asus U38N: Black screen with Radeon driver in Linux

2015-05-04 Thread bugzilla-dae...@freedesktop.org
https://bugs.freedesktop.org/show_bug.cgi?id=73530

--- Comment #75 from N.Leiten  ---
Can anyone provide me with some devel tools to make research of this problem?

I've already tried to play with delays in linux-4.0.1, tried drm-next with no
success.

At this point I have two variants - to look/disassemble fglrx and/or get track
of driver kms-init to solve this 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/20150504/d15fc996/attachment.html>


[PATCH v2] drm: Defer disabling the vblank IRQ until the next interrupt (for instant-off)

2015-05-04 Thread Chris Wilson
On vblank instant-off systems, we can get into a situation where the cost
of enabling and disabling the vblank IRQ around a drmWaitVblank query
dominates. However, we know that if the user wants the current vblank
counter, they are also very likely to immediately queue a vblank wait
and so we can keep the interrupt around and only turn it off if we have
no further vblank requests in the interrupt interval.

After vblank event delivery there is a shadow of one vblank where the
interrupt is kept alive for the user to query and queue another vblank
event. Similarly, if the user is using blocking drmWaitVblanks, the
interrupt will be disabled on the IRQ following the wait completion.
However, if the user is simply querying the current vblank counter and
timestamp, the interrupt will be disabled after every IRQ and the user
will enabled it again on the first query following the IRQ.

v2: Mario Kleiner -
After testing this, one more thing that would make sense is to move
the disable block at the end of drm_handle_vblank() instead of at the
top.

Turns out that if high precision timestaming is disabled or doesn't
work for some reason (as can be simulated by echo 0 >
/sys/module/drm/parameters/timestamp_precision_usec), then with your
delayed disable code at its current place, the vblank counter won't
increment anymore at all for instant queries, ie. with your other
"instant query" patches. Clients which repeatedly query the counter
and wait for it to progress will simply hang, spinning in an endless
query loop. There's that comment in vblank_disable_and_save:

"* Skip this step if there isn't any high precision timestamp
 * available. In that case we can't account for this and just
 * hope for the best.
 */

With the disable happening after leading edge of vblank (== hw counter
increment already happened) but before the vblank counter/timestamp
handling in drm_handle_vblank, that step is needed to keep the counter
progressing, so skipping it is bad.

Now without high precision timestamping support, a kms driver must not
set dev->vblank_disable_immediate = true, as this would cause problems
for clients, so this shouldn't matter, but it would be good to still
make this robust against a future kms driver which might have
unreliable high precision timestamping, e.g., high precision
timestamping that intermittently doesn't work.

Testcase: igt/kms_vblank
Signed-off-by: Chris Wilson 
Cc: Ville Syrjälä 
Cc: Daniel Vetter 
Cc: Michel Dänzer 
Cc: Laurent Pinchart 
Cc: Dave Airlie ,
Cc: Mario Kleiner 
---
 drivers/gpu/drm/drm_irq.c | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index c8a34476570a..d27b91f8a357 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -1091,9 +1091,9 @@ void drm_vblank_put(struct drm_device *dev, int crtc)
if (atomic_dec_and_test(&vblank->refcount)) {
if (drm_vblank_offdelay == 0)
return;
-   else if (dev->vblank_disable_immediate || drm_vblank_offdelay < 
0)
+   else if (drm_vblank_offdelay < 0)
vblank_disable_fn((unsigned long)vblank);
-   else
+   else if (!dev->vblank_disable_immediate)
mod_timer(&vblank->disable_timer,
  jiffies + ((drm_vblank_offdelay * HZ)/1000));
}
@@ -1751,6 +1751,16 @@ bool drm_handle_vblank(struct drm_device *dev, int crtc)
wake_up(&vblank->queue);
drm_handle_vblank_events(dev, crtc);

+   /* With instant-off, we defer disabling the interrupt until after
+* we finish processing the following vblank. The disable has to
+* be last (after drm_handle_vblank_events) so that the timestamp
+* is always accurate.
+*/
+   if (dev->vblank_disable_immediate &
+   drm_vblank_offdelay > 0 &&
+   !atomic_read(&vblank->refcount))
+   vblank_disable_fn(vblank);
+
spin_unlock_irqrestore(&dev->event_lock, irqflags);

return true;
-- 
2.1.4



Tiny fixes - resent

2015-05-04 Thread Daniel Vetter
On Mon, May 04, 2015 at 06:29:43AM +0200, Mario Kleiner wrote:
> Hi, a resend of updated versions of the earlier patches:
> 
> Patch 1 should really get into Linux 4.1 to avoid tegra breaking
> user-space clients.
> 
> Patch 2 reviewed-by Michel, updated to take his feedback into account.
> 
> Patch 3 is modified to not conflict with Daniel Vetter's patch
> "drm/vblank: Fixup and document timestamp update/read barriers"
> 
> Patch 4 will be needed for drm/qxl to compile with Daniel's fixup
> patch applied.

Merged patch 2-4 to topic/drm-misc on top of my memory barrier patch. I'll
leave 1 to Thierry for tegra-fixes. And there doesn't seem to be a patch 5
somehow, was that intentional? The patches are numbered n/5.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch


[RFC] drm/exynos: rework layer blending setup

2015-05-04 Thread Tobias Jakobi
Hello Inki!

Inki Dae wrote:
> Hi Tobias,
> 
> To make patch files, you could use below command,
> #git format-patch --cover-letter from..to
Thanks, I'm going to add the cover letter to the next revision.


> With this command, a cover file will be created and you could describe
> what this patch series mean in the cover file.
I left out a detailed description since the series isn't really finished
yet and I more or less wanted to hear Joonyoung' thoughts, who I
discussed stuff with.

I still have to change some details and remove the TODOs. Once that's
done I'm going to write a full description.


With best wishes,
Tobias

> 
> Thanks,
> Inki Dae
> 
> On 2015년 04월 30일 23:56, Tobias Jakobi wrote:
>> Hello,
>>
>> here's the rework of the layer blending setup that I discussed with 
>> Joonyoung in the past days. There's still 
>> some TODOs in the code, but more or less it does what it's supposed to do. 
>> What still bothers me a bit is that I 
>> currently call blending reconfig in mixer_cfg_layer(). It would be nice if 
>> this could be reduced to one call per 
>> "frame" (so with the last win_{commit,disable} call). Maybe atomic provides 
>> such an infrastructure?
>>
>> With best wishes,
>> Tobias
>>
>>
> 



[Y2038] [PATCH v4 06/10] cec: add HDMI CEC framework: y2038 question

2015-05-04 Thread Arnd Bergmann
On Monday 04 May 2015 09:42:36 Hans Verkuil wrote:
> Ping! (Added Arnd to the CC list)

Hi Hans,

sorry I missed this the first time

> On 04/27/2015 09:40 AM, Hans Verkuil wrote:
> > Added the y2038 mailinglist since I would like to get their input for
> > this API.
> > 
> > Y2038 experts, can you take a look at my comment in the code below?
> > 
> > Thanks!
> 
> Arnd, I just saw your patch series adding struct __kernel_timespec to
> uapi/linux/time.h. I get the feeling that it might take a few kernel
> cycles before we have a timespec64 available in userspace. Based on that
> I think this CEC API should drop the timestamps for now and wait until
> timespec64 becomes available before adding it.
> 
> The timestamps are a nice-to-have, but not critical. So adding it later
> shouldn't be a problem. What is your opinion?

It will take a little while for the patches to make it in, I would guess
4.3 at the earliest. Using your own struct works just as well and would
be less ambiguous.

However, for timestamps, I would recommend not using timespec anyway.
Instead, just use a single 64-bit nanosecond value from ktime_get_ns()
(or ktime_get_boot_ns() if you need a time that keeps ticking across
suspend). This is more efficient to get and simpler to use as long
as you don't need to convert from nanosecond to timespec.

Arnd


[BUG] i915: suspend by closing Laptop lid broken

2015-05-04 Thread Jani Nikula
On Mon, 04 May 2015, Martin Kepplinger  wrote:
> So. -rc1 broke suspending by closing my laptop lid and it's not fixed in
> -rc2. It works exactly *one* first time and every subsequent lid-closing
> is ignored.
>
> Biscted and tested first bad commit:
> 14aa02449064541217836b9f3d3295e241d5ae9c
>
> This pulls in i915 changes as well as ACPI changes. I don't know the
> driver but I'm sure you can find the mistake. I'm happy to test changes.
>
> There are no log differences.

Any chance you could bisect into the merge? It would be helpful.

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Technology Center


[PATCH] drm/exynos: Fix build breakage on !DRM_EXYNOS_FIMD

2015-05-04 Thread Daniel Stone
Hi,

On 4 May 2015 at 08:43, Inki Dae  wrote:
> On 2015년 05월 02일 13:08, Krzysztof Kozlowski wrote:
>> Selecting CONFIG_FB_S3C disables CONFIG_DRM_EXYNOS_FIMD leading to build
>> error:
>
> No, eDP has no any dependency of FIMD but DECON. Just add dependency
> code like below,
>
>  config DRM_EXYNOS7_DECON
> bool "Exynos DRM DECON"
> -   depends on DRM_EXYNOS
> +   depends on DRM_EXYNOS && !FB_S3C

But it does clearly and explicitly call fimd_dp_clock_enable from
exynos_dp_powero{n,ff}. So the dependency you're proposing seems
backwards: it's not an expression of the requirements of the current
code (that FIMD DP code be available, i.e. CONFIG_DRM_EXYNOS_FIMD is
selected), but an indirect expression of another dependency
(CONFIG_FB_S3C disables CONFIG_DRM_EXYNOS_FIMD, so disable
CONFIG_FB_S3C).

Additionally, as the call comes from exynos_dp_core.c, which is built
by CONFIG_DRM_EXYNOS_DP (an explicitly user-selectable option), why
shouldn't the dependency be there? Ah, because the dependency on DP is
for (DECON || FIMD), but as DECON doesn't provide
fimd_dp_clock_enable(), it doesn't seem like it would compile if you
selected DECON and not FIMD.

So, for me, the cleanest solution would be config DRM_EXYNOS_DP gains
a hard dependency on DRM_EXYNOS_FIMD, at least until it can be fixed
to compile without FIMD.

Cheers,
Daniel


[PATCH] drm/amdkfd: reformat some debug prints

2015-05-04 Thread Oded Gabbay
Signed-off-by: Oded Gabbay 
---
 drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c | 15 +++
 drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c |  5 +++--
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c
index 17e56dc..e621eba 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c
@@ -142,14 +142,13 @@ int kfd_doorbell_mmap(struct kfd_process *process, struct 
vm_area_struct *vma)

vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);

-   pr_debug("kfd: mapping doorbell page in kfd_doorbell_mmap\n"
-" target user address == 0x%08llX\n"
-" physical address== 0x%08llX\n"
-" vm_flags== 0x%04lX\n"
-" size== 0x%04lX\n",
-(unsigned long long) vma->vm_start, address, vma->vm_flags,
-doorbell_process_allocation());
-
+   pr_debug("mapping doorbell page:\n");
+   pr_debug(" target user address == 0x%08llX\n",
+   (unsigned long long) vma->vm_start);
+   pr_debug(" physical address== 0x%08llX\n", address);
+   pr_debug(" vm_flags== 0x%04lX\n", vma->vm_flags);
+   pr_debug(" size== 0x%04lX\n",
+doorbell_process_allocation());

return io_remap_pfn_range(vma,
vma->vm_start,
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
index c7d298e..8fa8941 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
@@ -215,8 +215,9 @@ static int acquire_packet_buffer(struct kernel_queue *kq,
queue_address = (unsigned int *)kq->pq_kernel_addr;
queue_size_dwords = kq->queue->properties.queue_size / sizeof(uint32_t);

-   pr_debug("amdkfd: In func %s\nrptr: %d\nwptr: %d\nqueue_address 0x%p\n",
-   __func__, rptr, wptr, queue_address);
+   pr_debug("rptr: %d\n", rptr);
+   pr_debug("wptr: %d\n", wptr);
+   pr_debug("queue_address 0x%p\n", queue_address);

available_size = (rptr - 1 - wptr + queue_size_dwords) %
queue_size_dwords;
-- 
1.9.1



[PATCH] drm/exynos: Fix build breakage on !DRM_EXYNOS_FIMD

2015-05-04 Thread Andrzej Hajda
Hi,


On 05/04/2015 02:43 PM, Krzysztof Kozlowski wrote:
> 2015-05-04 20:34 GMT+09:00 Daniel Stone :
>> Hi,
>>
>> On 4 May 2015 at 08:43, Inki Dae  wrote:
>>> On 2015년 05월 02일 13:08, Krzysztof Kozlowski wrote:
 Selecting CONFIG_FB_S3C disables CONFIG_DRM_EXYNOS_FIMD leading to build
 error:
>>>
>>> No, eDP has no any dependency of FIMD but DECON. Just add dependency
>>> code like below,
>>>
>>>  config DRM_EXYNOS7_DECON
>>> bool "Exynos DRM DECON"
>>> -   depends on DRM_EXYNOS
>>> +   depends on DRM_EXYNOS && !FB_S3C
> 
> Actually my commit message was not detailed enough. The FB_S3C here
> won't solve the issue because you may:
> 1, disable FIMD and FB_S3C,
> 2, enabke DECON and DP,
> and it won't compile.
> 
> Currently the FIMD must be enabled if DRM_EXYNOS_DP is enabled.
> 
>>
>> But it does clearly and explicitly call fimd_dp_clock_enable from
>> exynos_dp_powero{n,ff}. So the dependency you're proposing seems
>> backwards: it's not an expression of the requirements of the current
>> code (that FIMD DP code be available, i.e. CONFIG_DRM_EXYNOS_FIMD is
>> selected), but an indirect expression of another dependency
>> (CONFIG_FB_S3C disables CONFIG_DRM_EXYNOS_FIMD, so disable
>> CONFIG_FB_S3C).
>>
>> Additionally, as the call comes from exynos_dp_core.c, which is built
>> by CONFIG_DRM_EXYNOS_DP (an explicitly user-selectable option), why
>> shouldn't the dependency be there? Ah, because the dependency on DP is
>> for (DECON || FIMD), but as DECON doesn't provide
>> fimd_dp_clock_enable(), it doesn't seem like it would compile if you
>> selected DECON and not FIMD.
>>
>> So, for me, the cleanest solution would be config DRM_EXYNOS_DP gains
>> a hard dependency on DRM_EXYNOS_FIMD, at least until it can be fixed
>> to compile without FIMD.
> 
> Right, you correctly pointed current dependencies. Still it looks little
> hacky because EXYNOS_DP may work with FIMD or DECON. 

Are you sure? I have not seen any chipset having DECON and DP. In all
chipsets known to me DP is always accompanied by FIMD. I guess it can
change in the future, but for now hard dependency on FIMD seems to be OK
- it just reflects hardware design.
Of course this is just my humble opinion :)

Regards
Andrzej

It does not really
> need FIMD. Using ifdefs in headers is not uncommon - many core
> subsystems do this that way to provide stubs.
> 
> Probably the cleanest way would be to provide by FIMD and DECON a common
> interface for DP for such operation, something like:
> struct exynos_drm_crtc {
>   struct drm_crtc base;
>   ...
>   void (*clock_enable)(struct exynos_drm_crtc *crtc, bool enable)
> );
> 
> which, if non-NULL, will be called by exynos_dp_core.c:
> static void exynos_dp_poweron(struct exynos_dp_device *dp)
> {
>   ...
>   if (crtc->clock_enable)
>   crtc->clock_enable(crtc, true);
> }
> 
> What do you think?
> 
> Best regards,
> Krzysztof
> 



[PATCH] drm/atomic-helper: Really recover pre-atomic plane/cursor behavior

2015-05-04 Thread Daniel Vetter
I've fumbled this in

commit f02ad907cd9e7fe3a6405d2d005840912f1ed258
Author: Daniel Vetter 
Date:   Thu Jan 22 16:36:23 2015 +0100

drm/atomic-helpers: Recover full cursor plane behaviour

and accidentally put the assignment for legacy_cursor_upate after the
atomic commit, where it is pretty useless.

Reported-by: Maarten Lankhorst 
Cc: Maarten Lankhorst 
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/drm_atomic_helper.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
b/drivers/gpu/drm/drm_atomic_helper.c
index d536817699c1..9f216ff61af3 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1310,13 +1310,13 @@ retry:
plane_state->src_h = src_h;
plane_state->src_w = src_w;

+   if (plane == crtc->cursor)
+   state->legacy_cursor_update = true;
+
ret = drm_atomic_commit(state);
if (ret != 0)
goto fail;

-   if (plane == crtc->cursor)
-   state->legacy_cursor_update = true;
-
/* Driver takes ownership of state on successful commit. */
return 0;
 fail:
-- 
2.1.4



[Intel-gfx] [PATCH i-g-t] tests/drm_hw_lock: Tests for hw_lock fixes.

2015-05-04 Thread Daniel Vetter
On Tue, Apr 28, 2015 at 09:28:51AM +, Antoine, Peter wrote:
> On Mon, 2015-04-27 at 16:33 +0100, Chris Wilson wrote:
> > On Mon, Apr 27, 2015 at 04:24:37PM +0100, Thomas Wood wrote:
> > > On 23 April 2015 at 15:07, Peter Antoine  
> > > wrote:
> > > > There are several issues with the hardware locks functions that stretch
> > > > from kernel crashes to priority escalations. This new test will test the
> > > > the fixes for these features.
> > > >
> > > > This test will cause a driver/kernel crash on un-patched kernels, the
> > > > following patches should be applied to stop the crashes:
> > > >
> > > >   drm: Kernel Crash in drm_unlock
> > > >   drm: Fixes unsafe deference in locks.
> > > >
> > > > Issue: VIZ-5485
> > > > Signed-off-by: Peter Antoine 
> > > > ---
> > > >  lib/ioctl_wrappers.c   |  19 +
> > > >  lib/ioctl_wrappers.h   |   1 +
> > > >  tests/Makefile.sources |   1 +
> > > >  tests/drm_hw_lock.c| 207 
> > > > +
> > > >  4 files changed, 228 insertions(+)
> > > >  create mode 100644 tests/drm_hw_lock.c
> > > >
> > > > diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
> > > > index 000d394..ad8b3d3 100644
> > > > --- a/lib/ioctl_wrappers.c
> > > > +++ b/lib/ioctl_wrappers.c
> > > > @@ -964,6 +964,25 @@ bool gem_has_bsd2(int fd)
> > > >  {
> > > > return gem_has_enable_ring(fd,LOCAL_I915_PARAM_HAS_BSD2);
> > > >  }
> > > > +#define I915_PARAM_HAS_LEGACY_CONTEXT   35
> > > 
> > > 
> > > Please add some API documentation for this new function here.
> > > 
> > > > +bool drm_has_legacy_context(int fd)
> > > > +{
> > > > +   int tmp = 0;
> > > > +   drm_i915_getparam_t gp;
> > > > +
> > > > +   memset(&gp, 0, sizeof(gp));
> > > > +   gp.value = &tmp;
> > > > +   gp.param = I915_PARAM_HAS_LEGACY_CONTEXT;
> > > > +
> > > > +   /*
> > > > +* if legacy context param is not supported, then it's old and 
> > > > we
> > > > +* can assume that the HW_LOCKS are supported.
> > > > +*/
> > > > +   if (drmIoctl(fd, DRM_IOCTL_I915_GETPARAM, &gp) != 0)
> > > > +   return true;
> > 
> > Would not a simpler test be to try and legally acquire a hwlock?
> > If it fails, hwlocks are not supported. No need for a PARAM.
> > -Chris
> > 
> 
> The test relies on the hw_lock not being created (f the HW-LOCKs are
> supported). If I grab, then delete the lock I might find more problems
> with this code. :)
> 
> As you have to be root to create and delete the hwlock the security
> issues are minimal as you have root already you don't really need to use
> these ioctls to harm the system. So as the exposure is minimal, any more
> fixes on code that is legacy and being turned off seems to be a bit of a
> time waste.
> 
> Also, the PARAM should give legitimate code the opportunity to avoid the
> problems but avoiding these features completely.

root != kernel, at least in secure boot enviroments. Which means not even
root is allowed to crash/exploit the kernel ... Yes there's a pile of
.config options and stuff you need to set correctly to fully lock out root
from the kernel, but in general drivers really shouldn't have their own
root2kernel backdoors.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch


[Intel-gfx] [PATCH 1/5] drm: Kernel Crash in drm_unlock

2015-05-04 Thread Daniel Vetter
On Tue, Apr 28, 2015 at 10:52:32AM +0100, chris at chris-wilson.co.uk wrote:
> On Tue, Apr 28, 2015 at 10:21:49AM +0100, Dave Gordon wrote:
> > On 24/04/15 06:52, Antoine, Peter wrote:
> > > I picked up this work due to the following Jira ticket created by the
> > > security team (on Android) and was asked to give it a second look and
> > > found a few more issues with the hw lock code.
> > > 
> > > https://jira01.devtools.intel.com/browse/GMINL-5388
> > > I/O control on /dev/dri/card0 crashes the kernel (0x4008642b)
> > > 
> > > It also stops Linux as it kills the driver, I guess it might be possible
> > > to reload the gfx driver. On a unpatched system the test that is
> > > included in the issue or the igt test that has been posted for the issue
> > > will show the problem.
> > > 
> > > I ran the test on an unpatched system here and the gui stopped and the
> > > keyboard stopped responding, so I rebooted. With the patched system I
> > > did not need to reboot.
> > > 
> > > Should I change the SIGTERM to SIGSEGV, not quite the same thing but
> > > tooling is better at handling a segfault than a SIGTERM and the
> > > application that calls this IOCTL is using an uninitialised hw lock so
> > > it is kind of the same as differencing an uninitialised pointer (kind
> > > of). Or, I could just remove it, but the bug has been in the code for at
> > > least two years (and known about), and I would guess that any code that
> > > is calling this is fuzzing the IOCTLs (as this is how the security team
> > > found it) and we should reward them with a application exit.
> > > 
> > > Peter. 
> > 
> > SIGSEGV would be a better choice.
> > 
> > SIGTERM is normally sent by a user -- it's the default signal sent by
> > kill(1). It's also commonly used to tell a long-running daemon process
> > to tidy up and exit cleanly.
> > 
> > SIGSEGV commonly means "you accessed something that doesn't exist/isn't
> > mapped/you don't have permissions for". There are specific subcases that
> > can be indicated via the siginfo data; this is from the sigaction(1)
> > manpage:
> > 
> > The following values can be placed in si_code for a SIGSEGV signal:
> > 
> > SEGV_MAPERRaddress not mapped to object
> > 
> > SEGV_ACCERRinvalid permissions for mapped object
> > 
> > SIGBUS would also be a possibility but that's generally taken to mean
> > that an access got all the way to some physical bus and then faulted,
> > whereas SIGSEGV suggests the access was rejected during the
> > virtual-to-physical mapping process.
> 
> None of the above. Just return -EINVAL, -EPERM, -EACCESS as appropriate.

Seconded, we really don't want to be in the business of fixing up the drm
design mistakes of the past 15 years. As long as we can fully lock out
this particular dragon when running i915 we're imo good enough. The dri1
design of a kernel shim driver cooperating with the ums driver for hw
ownership is fundamentally unfixable.

Also we can't change any of it for drivers actually using it since it'll
break them, which is a big no-go.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch


[Intel-gfx] [PATCH 3/5] drm: Possible lock priority escalation.

2015-05-04 Thread Daniel Vetter
On Mon, Apr 27, 2015 at 07:52:46PM +0300, Ville Syrjälä wrote:
> On Thu, Apr 23, 2015 at 03:07:56PM +0100, Peter Antoine wrote:
> > If an application that has a driver lock created, wants the lock the
> > kernel context, it is not allowed to. If the call to drm_lock has a
> > context of 0, it is rejected. If you set the context to _DRM_LOCK_CONT
> > then call drm lock, it will pass the context == DRM_KERNEL_CONTEXT checks.
> > But as the DRM_LOCK_CONT bits are not part of the context id this allows
> > operations on the DRM_KERNEL_CONTEXT.
> > 
> > Issue: VIZ-5485
> > Signed-off-by: Peter Antoine 

If you're touching code with drm_legacy_ prefix of in such a file you've
ended up in the horrible corners of the dri1 dungeons and should head back
out pronto ;-)

If we can actually run into this code on production i915 then we need to
improve the locks at the door of these dungeons for kms drivers, not try
to fix up the mess behind them. That's just plain impossible.

If you want to make really sure we get this right some simple drm igt
tests to make sure these codepaths are really dead for kms driver might be
good. But otherwise we really can only annotate this as wontfix in
code security issue scanners.
-Daniel

> > ---
> >  drivers/gpu/drm/drm_context.c | 6 +++---
> >  drivers/gpu/drm/drm_lock.c| 4 ++--
> >  2 files changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_context.c b/drivers/gpu/drm/drm_context.c
> > index 96350d1..1febcd3 100644
> > --- a/drivers/gpu/drm/drm_context.c
> > +++ b/drivers/gpu/drm/drm_context.c
> > @@ -123,7 +123,7 @@ void drm_legacy_ctxbitmap_flush(struct drm_device *dev, 
> > struct drm_file *file)
> >  
> > list_for_each_entry_safe(pos, tmp, &dev->ctxlist, head) {
> > if (pos->tag == file &&
> > -   pos->handle != DRM_KERNEL_CONTEXT) {
> > +   _DRM_LOCKING_CONTEXT(pos->handle) != DRM_KERNEL_CONTEXT) {
> > if (dev->driver->context_dtor)
> > dev->driver->context_dtor(dev, pos->handle);
> >  
> > @@ -342,7 +342,7 @@ int drm_legacy_addctx(struct drm_device *dev, void 
> > *data,
> > struct drm_ctx *ctx = data;
> >  
> > ctx->handle = drm_legacy_ctxbitmap_next(dev);
> > -   if (ctx->handle == DRM_KERNEL_CONTEXT) {
> > +   if (_DRM_LOCKING_CONTEXT(ctx->handle) == DRM_KERNEL_CONTEXT) {
> > /* Skip kernel's context and get a new one. */
> > ctx->handle = drm_legacy_ctxbitmap_next(dev);
> > }
> > @@ -449,7 +449,7 @@ int drm_legacy_rmctx(struct drm_device *dev, void *data,
> > struct drm_ctx *ctx = data;
> >  
> > DRM_DEBUG("%d\n", ctx->handle);
> > -   if (ctx->handle != DRM_KERNEL_CONTEXT) {
> > +   if (_DRM_LOCKING_CONTEXT(ctx->handle) != DRM_KERNEL_CONTEXT) {
> > if (dev->driver->context_dtor)
> > dev->driver->context_dtor(dev, ctx->handle);
> > drm_legacy_ctxbitmap_free(dev, ctx->handle);
> 
> How about just fixing the end parameter passed to idr_alloc()? AFAICS
> that would take care of the context code.
> 
> Well, there are a few more issues with the code:
> - not properly checking for negative return value from idr_alloc()
> - leaking the ctx id on kmalloc() error
> - pointless check for idr_alloc() returning 0 even though the min is 1
> 
> > diff --git a/drivers/gpu/drm/drm_lock.c b/drivers/gpu/drm/drm_lock.c
> > index 070dd5d..94500930 100644
> > --- a/drivers/gpu/drm/drm_lock.c
> > +++ b/drivers/gpu/drm/drm_lock.c
> > @@ -63,7 +63,7 @@ int drm_legacy_lock(struct drm_device *dev, void *data,
> >  
> > ++file_priv->lock_count;
> 
> While you're poking around this dungeopn, maybe you can kill lock_count?
> We never seem to decrement it, and it's only checked in 
> drm_legacy_i_have_hw_lock().
> 
> >  
> > -   if (lock->context == DRM_KERNEL_CONTEXT) {
> > +   if (_DRM_LOCKING_CONTEXT(lock->context) == DRM_KERNEL_CONTEXT) {
> > DRM_ERROR("Process %d using kernel context %d\n",
> >   task_pid_nr(current), lock->context);
> > return -EINVAL;
> > @@ -153,7 +153,7 @@ int drm_legacy_unlock(struct drm_device *dev, void 
> > *data, struct drm_file *file_
> > struct drm_lock *lock = data;
> > struct drm_master *master = file_priv->master;
> >  
> > -   if (lock->context == DRM_KERNEL_CONTEXT) {
> > +   if (_DRM_LOCKING_CONTEXT(lock->context) == DRM_KERNEL_CONTEXT) {
> > DRM_ERROR("Process %d using kernel context %d\n",
> >   task_pid_nr(current), lock->context);
> > return -EINVAL;
> 
> These two changes look OK to me.
> 
> > -- 
> > 1.9.1
> > 
> > ___
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC

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


[Intel-gfx] [PATCH 4/5] drm: Make HW_LOCK access functions optional.

2015-05-04 Thread Daniel Vetter
On Tue, Apr 28, 2015 at 01:29:21PM +, Antoine, Peter wrote:
> On Tue, 2015-04-28 at 16:08 +0300, Ville Syrjälä wrote:
> > On Tue, Apr 28, 2015 at 11:29:06AM +, Antoine, Peter wrote:
> > > > > > diff --git a/include/drm/drmP.h b/include/drm/drmP.h index 
> > > > > > 62c40777..367e42f 100644
> > > > > > --- a/include/drm/drmP.h
> > > > > > +++ b/include/drm/drmP.h
> > > > > > @@ -137,17 +137,18 @@ void drm_err(const char *format, ...);  /*@{*/
> > > > > >  
> > > > > >  /* driver capabilities and requirements mask */
> > > > > > -#define DRIVER_USE_AGP 0x1
> > > > > > -#define DRIVER_PCI_DMA 0x8
> > > > > > -#define DRIVER_SG  0x10
> > > > > > -#define DRIVER_HAVE_DMA0x20
> > > > > > -#define DRIVER_HAVE_IRQ0x40
> > > > > > -#define DRIVER_IRQ_SHARED  0x80
> > > > > > -#define DRIVER_GEM 0x1000
> > > > > > -#define DRIVER_MODESET 0x2000
> > > > > > -#define DRIVER_PRIME   0x4000
> > > > > > -#define DRIVER_RENDER  0x8000
> > > > > > -#define DRIVER_ATOMIC  0x1
> > > > > > +#define DRIVER_USE_AGP 0x1
> > > > > > +#define DRIVER_PCI_DMA 0x8
> > > > > > +#define DRIVER_SG  0x10
> > > > > > +#define DRIVER_HAVE_DMA0x20
> > > > > > +#define DRIVER_HAVE_IRQ0x40
> > > > > > +#define DRIVER_IRQ_SHARED  0x80
> > > > > > +#define DRIVER_GEM 0x1000
> > > > > > +#define DRIVER_MODESET 0x2000
> > > > > > +#define DRIVER_PRIME   0x4000
> > > > > > +#define DRIVER_RENDER  0x8000
> > > > > > +#define DRIVER_ATOMIC  0x1
> > > > > > +#define DRIVER_KMS_LEGACY_CONTEXT  0x2
> > > > > 
> > > > > Why is there KMS in the name?
> > > > > 
> > > > > By suggestion of Daniel.
> > > > > 
> > > > > I was thinking just checking for GEM, but I think there was some
> > > > > gem+dri1 userland for i915 at some point in time. ums and dri1 are
> > > > > now dead as far as i915 is concerned, so in theory it should be fine.
> > > > > But I'm not sure if some other driver might have the same baggage.
> > > > > 
> > > > > Other drivers have the same baggage.
> > > > > 
> > > > > I suppose one option would be to check for MODESET instead. kms+dri1 
> > > > > doesn't sound like an entirely sane combination to me.
> > > > > 
> > > > > Can't use the MODESET as this was how it was turned off in the 
> > > > > previous incarnation and was reverted by Dave Airle.
> > > > 
> > > > Reference?
> > > 
> > > From the next commit [5/5] as it is the one that actually turns off the
> > > functions that were turned off before.
> > > 
> > > These changes are based on the two patches:
> > >   commit c21eb21cb50d58e7cbdcb8b9e7ff68b85cfa5095
> > >   Author: Dave Airlie 
> > > 
> > > And the commit that the above patch reverts:
> > >   commit 7c510133d93dd6f15ca040733ba7b2891ed61fd1
> > >   Author: Daniel Vetter 

These two commits definitely should be referenced from the commit message,
othwerise no one will understand the history.

> > Looking at ancient libdrm sources makes me think nouveau just used to
> > create and destroy the context, but not actually use it for anything.
> > So nopping out the ioctls should be good enough AFAICS. Or am I missing
> > something?
> > 
> 
> An old version of libdrm that still requires support needs them, it's
> the reason that David Airlie reverted the patch that Daniel did to
> remove the functions. Do they still need support, I don't know? David
> Airlie is on the cc list.
> 
> A discussion was had and this was the way that it was suggested it be
> done. This seems a good half-way house, the actual security holes that
> have been found have been fixed and the functions (that seem to have a
> lot more security issues in them) are turned off for the drivers that
> don't use them, and if a driver does require them, it will be a one line
> change to reintroduce them. Are we carrying code we don't need to
> support, probably.

Iirc it was in the ddx, and it was actually using the mmap code. Leftovers
from ums, but unfortunately X crashes if we take them away. If I recall
correctly nouveau was in staging still, but per Linus staging or not
doesn't matter when distros are shipping with the code already. I did dig
out the details way back out of curiosity, but lost them since then again.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch


[PATCH 1/3] drm: drop unused 'magicfree' list

2015-05-04 Thread David Herrmann
This list is write-only. It's never used for read-access, so no reason to
keep it around. Drop it!

Signed-off-by: David Herrmann 
---
 drivers/gpu/drm/drm_auth.c | 3 ---
 drivers/gpu/drm/drm_drv.c  | 1 -
 include/drm/drmP.h | 2 --
 3 files changed, 6 deletions(-)

diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
index fc8e8aa..8a37524 100644
--- a/drivers/gpu/drm/drm_auth.c
+++ b/drivers/gpu/drm/drm_auth.c
@@ -37,7 +37,6 @@
 #include "drm_internal.h"

 struct drm_magic_entry {
-   struct list_head head;
struct drm_hash_item hash_item;
struct drm_file *priv;
 };
@@ -93,7 +92,6 @@ static int drm_add_magic(struct drm_master *master, struct 
drm_file *priv,
entry->hash_item.key = (unsigned long)magic;
mutex_lock(&dev->struct_mutex);
drm_ht_insert_item(&master->magiclist, &entry->hash_item);
-   list_add_tail(&entry->head, &master->magicfree);
mutex_unlock(&dev->struct_mutex);

return 0;
@@ -123,7 +121,6 @@ int drm_remove_magic(struct drm_master *master, drm_magic_t 
magic)
}
pt = drm_hash_entry(hash, struct drm_magic_entry, hash_item);
drm_ht_remove_item(&master->magiclist, hash);
-   list_del(&pt->head);
mutex_unlock(&dev->struct_mutex);

kfree(pt);
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 48f7359..26ed9fe 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -109,7 +109,6 @@ struct drm_master *drm_master_create(struct drm_minor 
*minor)
kfree(master);
return NULL;
}
-   INIT_LIST_HEAD(&master->magicfree);
master->minor = minor;

return master;
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 62c40777..80be07a 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -356,7 +356,6 @@ struct drm_lock_data {
  * @unique: Unique identifier: e.g. busid. Protected by drm_global_mutex.
  * @unique_len: Length of unique field. Protected by drm_global_mutex.
  * @magiclist: Hash of used authentication tokens. Protected by struct_mutex.
- * @magicfree: List of used authentication tokens. Protected by struct_mutex.
  * @lock: DRI lock information.
  * @driver_priv: Pointer to driver-private information.
  */
@@ -366,7 +365,6 @@ struct drm_master {
char *unique;
int unique_len;
struct drm_open_hash magiclist;
-   struct list_head magicfree;
struct drm_lock_data lock;
void *driver_priv;
 };
-- 
2.3.7



[PATCH 2/3] drm: simplify authentication management

2015-05-04 Thread David Herrmann
The magic auth tokens we have are a simple map from cyclic IDs to drm_file
objects. Remove all the old bulk of code and replace it with a simple,
direct IDR.

The previous behavior is kept. Especially calling authmagic multiple times
on the same magic results in EINVAL except on the first call. The only
difference in behavior is that we never allocate IDs multiple times, even
if they were already authenticated. To trigger that, you need 2^31 open
DRM file descriptions at the same time, though.

Diff-stat:
   5 files changed, 45 insertions(+), 157 deletions(-)

Signed-off-by: David Herrmann 
---
 drivers/gpu/drm/drm_auth.c | 178 +
 drivers/gpu/drm/drm_drv.c  |  12 +--
 drivers/gpu/drm/drm_fops.c |   7 +-
 drivers/gpu/drm/drm_internal.h |   1 -
 include/drm/drmP.h |   4 +-
 5 files changed, 45 insertions(+), 157 deletions(-)

diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
index 8a37524..ee365e8 100644
--- a/drivers/gpu/drm/drm_auth.c
+++ b/drivers/gpu/drm/drm_auth.c
@@ -1,11 +1,3 @@
-/**
- * \file drm_auth.c
- * IOCTLs for authentication
- *
- * \author Rickard E. (Rik) Faith 
- * \author Gareth Hughes 
- */
-
 /*
  * Created: Tue Feb  2 08:37:54 1999 by faith at valinux.com
  *
@@ -13,6 +5,9 @@
  * Copyright 2000 VA Linux Systems, Inc., Sunnyvale, California.
  * All Rights Reserved.
  *
+ * Author Rickard E. (Rik) Faith 
+ * Author Gareth Hughes 
+ *
  * Permission is hereby granted, free of charge, to any person obtaining a
  * copy of this software and associated documentation files (the "Software"),
  * to deal in the Software without restriction, including without limitation
@@ -36,151 +31,47 @@
 #include 
 #include "drm_internal.h"

-struct drm_magic_entry {
-   struct drm_hash_item hash_item;
-   struct drm_file *priv;
-};
-
-/**
- * Find the file with the given magic number.
- *
- * \param dev DRM device.
- * \param magic magic number.
- *
- * Searches in drm_device::magiclist within all files with the same hash key
- * the one with matching magic number, while holding the 
drm_device::struct_mutex
- * lock.
- */
-static struct drm_file *drm_find_file(struct drm_master *master, drm_magic_t 
magic)
-{
-   struct drm_file *retval = NULL;
-   struct drm_magic_entry *pt;
-   struct drm_hash_item *hash;
-   struct drm_device *dev = master->minor->dev;
-
-   mutex_lock(&dev->struct_mutex);
-   if (!drm_ht_find_item(&master->magiclist, (unsigned long)magic, &hash)) 
{
-   pt = drm_hash_entry(hash, struct drm_magic_entry, hash_item);
-   retval = pt->priv;
-   }
-   mutex_unlock(&dev->struct_mutex);
-   return retval;
-}
-
 /**
- * Adds a magic number.
+ * drm_getmagic - Get unique magic of a client
+ * @dev: DRM device to operate on
+ * @data: ioctl data containing the drm_auth object
+ * @file_priv: DRM file that performs the operation
  *
- * \param dev DRM device.
- * \param priv file private data.
- * \param magic magic number.
+ * This looks up the unique magic of the passed client and returns it. If the
+ * client did not have a magic assigned, yet, a new one is registered. The 
magic
+ * is stored in the passed drm_auth object.
  *
- * Creates a drm_magic_entry structure and appends to the linked list
- * associated the magic number hash key in drm_device::magiclist, while holding
- * the drm_device::struct_mutex lock.
- */
-static int drm_add_magic(struct drm_master *master, struct drm_file *priv,
-drm_magic_t magic)
-{
-   struct drm_magic_entry *entry;
-   struct drm_device *dev = master->minor->dev;
-   DRM_DEBUG("%d\n", magic);
-
-   entry = kzalloc(sizeof(*entry), GFP_KERNEL);
-   if (!entry)
-   return -ENOMEM;
-   entry->priv = priv;
-   entry->hash_item.key = (unsigned long)magic;
-   mutex_lock(&dev->struct_mutex);
-   drm_ht_insert_item(&master->magiclist, &entry->hash_item);
-   mutex_unlock(&dev->struct_mutex);
-
-   return 0;
-}
-
-/**
- * Remove a magic number.
- *
- * \param dev DRM device.
- * \param magic magic number.
- *
- * Searches and unlinks the entry in drm_device::magiclist with the magic
- * number hash key, while holding the drm_device::struct_mutex lock.
- */
-int drm_remove_magic(struct drm_master *master, drm_magic_t magic)
-{
-   struct drm_magic_entry *pt;
-   struct drm_hash_item *hash;
-   struct drm_device *dev = master->minor->dev;
-
-   DRM_DEBUG("%d\n", magic);
-
-   mutex_lock(&dev->struct_mutex);
-   if (drm_ht_find_item(&master->magiclist, (unsigned long)magic, &hash)) {
-   mutex_unlock(&dev->struct_mutex);
-   return -EINVAL;
-   }
-   pt = drm_hash_entry(hash, struct drm_magic_entry, hash_item);
-   drm_ht_remove_item(&master->magiclist, hash);
-   mutex_unlock(&dev->struct_mutex);
-
-   kfree(pt);
-
-   return 0;
-}
-
-/**
- * Get a unique magic number (

[PATCH 3/3] drm: simplify master cleanup

2015-05-04 Thread David Herrmann
In drm_master_destroy() we _free_ the master object. There is no reason to
hold any locks while dropping its static members, nor do we have to reset
it to 0.

Furthermore, kfree() already does NULL checks, so call it directly on
master->unique and drop the redundant reset-code.

Signed-off-by: David Herrmann 
---
 drivers/gpu/drm/drm_drv.c | 7 +--
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 88b594c..3b3c4f5 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -132,15 +132,10 @@ static void drm_master_destroy(struct kref *kref)
r_list = NULL;
}
}
-
-   if (master->unique) {
-   kfree(master->unique);
-   master->unique = NULL;
-   master->unique_len = 0;
-   }
mutex_unlock(&dev->struct_mutex);

idr_destroy(&master->magic_map);
+   kfree(master->unique);
kfree(master);
 }

-- 
2.3.7



[Intel-gfx] [PATCH 1/1 linux-next] drm/i915: use ERR_CAST instead of ERR_PTR/PTR_ERR

2015-05-04 Thread Daniel Vetter
On Mon, Apr 27, 2015 at 10:43:19AM +0300, Jani Nikula wrote:
> On Sat, 25 Apr 2015, Fabian Frederick  wrote:
> > Inspired by scripts/coccinelle/api/err_cast.cocci
> >
> > Signed-off-by: Fabian Frederick 
> 
> Reviewed-by: Jani Nikula 

Queued for -next, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch


[PATCH] drm/exynos: Fix build breakage on !DRM_EXYNOS_FIMD

2015-05-04 Thread Inki Dae
2015-05-04 20:34 GMT+09:00 Daniel Stone :
> Hi,
>
> On 4 May 2015 at 08:43, Inki Dae  wrote:
>> On 2015년 05월 02일 13:08, Krzysztof Kozlowski wrote:
>>> Selecting CONFIG_FB_S3C disables CONFIG_DRM_EXYNOS_FIMD leading to build
>>> error:
>>
>> No, eDP has no any dependency of FIMD but DECON. Just add dependency
>> code like below,
>>
>>  config DRM_EXYNOS7_DECON
>> bool "Exynos DRM DECON"
>> -   depends on DRM_EXYNOS
>> +   depends on DRM_EXYNOS && !FB_S3C
>
> But it does clearly and explicitly call fimd_dp_clock_enable from
> exynos_dp_powero{n,ff}. So the dependency you're proposing seems
> backwards: it's not an expression of the requirements of the current
> code (that FIMD DP code be available, i.e. CONFIG_DRM_EXYNOS_FIMD is
> selected), but an indirect expression of another dependency
> (CONFIG_FB_S3C disables CONFIG_DRM_EXYNOS_FIMD, so disable
> CONFIG_FB_S3C).
>
> Additionally, as the call comes from exynos_dp_core.c, which is built
> by CONFIG_DRM_EXYNOS_DP (an explicitly user-selectable option), why
> shouldn't the dependency be there? Ah, because the dependency on DP is
> for (DECON || FIMD), but as DECON doesn't provide
> fimd_dp_clock_enable(), it doesn't seem like it would compile if you

Please know that the output data of DECON and FIMD can go to DSI or
Panel directly not through eDP. This means that they - FIMD and eDP or
DECON and eDP - have hardware dependency each other. DECON driver
missed this - only one of Linux framebuffer and DRM KMS driver should
be selected because Linux framebuffer and DRM KMS drivers control same
hardware. So the reason that DECON driver should have dependency on
eDP is not because dp core functions are called by FIMD driver.

In addition, from hw point of view, eDP has depencency on DECON and
FIMD because eDP cannot work itself without DECON or FIMD. Therefore,
if DECON or FIMD is not selected, then eDP should also be disabled  so
that user cannot disable eDP with FIMD because FIMD driver calls eDP's
functions directly. My missing point was that even though we add
!S3C_FB to DECON Kconfig, we can disable eDP with FIMD so build error
can still occur. The best way to resolve this issue would be to move
the functions called by FIMD driver directly to Exynos specific KMS
structure such as exynos crtc - eDP driver implements encoder which
know crtc to FIMD or DECON. The directly call was not good way.

Thanks,
Inki Dae

> selected DECON and not FIMD.
>
> So, for me, the cleanest solution would be config DRM_EXYNOS_DP gains
> a hard dependency on DRM_EXYNOS_FIMD, at least until it can be fixed
> to compile without FIMD.
>
> Cheers,
> Daniel
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH 1/3] drm: drop unused 'magicfree' list

2015-05-04 Thread Chris Wilson
On Mon, May 04, 2015 at 04:05:12PM +0200, David Herrmann wrote:
> This list is write-only. It's never used for read-access, so no reason to
> keep it around. Drop it!
> 
> Signed-off-by: David Herrmann 
Reviewed-by: Chris Wilson 
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre


[PATCH] drm/exynos: Fix build breakage on !DRM_EXYNOS_FIMD

2015-05-04 Thread Inki Dae
2015-05-04 21:43 GMT+09:00 Krzysztof Kozlowski :
> 2015-05-04 20:34 GMT+09:00 Daniel Stone :
>> Hi,
>>
>> On 4 May 2015 at 08:43, Inki Dae  wrote:
>>> On 2015년 05월 02일 13:08, Krzysztof Kozlowski wrote:
 Selecting CONFIG_FB_S3C disables CONFIG_DRM_EXYNOS_FIMD leading to build
 error:
>>>
>>> No, eDP has no any dependency of FIMD but DECON. Just add dependency
>>> code like below,
>>>
>>>  config DRM_EXYNOS7_DECON
>>> bool "Exynos DRM DECON"
>>> -   depends on DRM_EXYNOS
>>> +   depends on DRM_EXYNOS && !FB_S3C
>
> Actually my commit message was not detailed enough. The FB_S3C here
> won't solve the issue because you may:
> 1, disable FIMD and FB_S3C,
> 2, enabke DECON and DP,
> and it won't compile.
>
> Currently the FIMD must be enabled if DRM_EXYNOS_DP is enabled.
>
>>
>> But it does clearly and explicitly call fimd_dp_clock_enable from
>> exynos_dp_powero{n,ff}. So the dependency you're proposing seems
>> backwards: it's not an expression of the requirements of the current
>> code (that FIMD DP code be available, i.e. CONFIG_DRM_EXYNOS_FIMD is
>> selected), but an indirect expression of another dependency
>> (CONFIG_FB_S3C disables CONFIG_DRM_EXYNOS_FIMD, so disable
>> CONFIG_FB_S3C).
>>
>> Additionally, as the call comes from exynos_dp_core.c, which is built
>> by CONFIG_DRM_EXYNOS_DP (an explicitly user-selectable option), why
>> shouldn't the dependency be there? Ah, because the dependency on DP is
>> for (DECON || FIMD), but as DECON doesn't provide
>> fimd_dp_clock_enable(), it doesn't seem like it would compile if you
>> selected DECON and not FIMD.
>>
>> So, for me, the cleanest solution would be config DRM_EXYNOS_DP gains
>> a hard dependency on DRM_EXYNOS_FIMD, at least until it can be fixed
>> to compile without FIMD.
>
> Right, you correctly pointed current dependencies. Still it looks little
> hacky because EXYNOS_DP may work with FIMD or DECON. It does not really
> need FIMD. Using ifdefs in headers is not uncommon - many core
> subsystems do this that way to provide stubs.
>
> Probably the cleanest way would be to provide by FIMD and DECON a common
> interface for DP for such operation, something like:
> struct exynos_drm_crtc {
> struct drm_crtc base;
> ...
> void (*clock_enable)(struct exynos_drm_crtc *crtc, bool enable)
> );
>
> which, if non-NULL, will be called by exynos_dp_core.c:
> static void exynos_dp_poweron(struct exynos_dp_device *dp)
> {
> ...
> if (crtc->clock_enable)
> crtc->clock_enable(crtc, true);
> }
>
> What do you think?

Good idea and you should have done so. the directly call was not good
way. Anyway, with this, fimd driver can set fimd's callbacks to
exynos_drm_crtc structure and eDP driver can call the callbacks
through its encoder. But please, keep in mind that you should set the
callbacks to Exynos specific structure - exynos_drm_crtc - because
this hardware dependency is really specific to Exynos SoC.

Thanks,
Inki Dae

>
> Best regards,
> Krzysztof
>
> ___
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel


[PATCH 2/3] drm: simplify authentication management

2015-05-04 Thread Chris Wilson
On Mon, May 04, 2015 at 04:05:13PM +0200, David Herrmann wrote:
> The magic auth tokens we have are a simple map from cyclic IDs to drm_file
> objects. Remove all the old bulk of code and replace it with a simple,
> direct IDR.
> 
> The previous behavior is kept. Especially calling authmagic multiple times
> on the same magic results in EINVAL except on the first call. The only
> difference in behavior is that we never allocate IDs multiple times, even
> if they were already authenticated. To trigger that, you need 2^31 open
> DRM file descriptions at the same time, though.
> 
> Diff-stat:
>5 files changed, 45 insertions(+), 157 deletions(-)
> 
> Signed-off-by: David Herrmann 

Just one quibble,

> @@ -189,10 +80,17 @@ int drm_authmagic(struct drm_device *dev, void *data,
>   struct drm_file *file;
>  
>   DRM_DEBUG("%u\n", auth->magic);
> - if ((file = drm_find_file(file_priv->master, auth->magic))) {
> +
> + if (auth->magic >= INT_MAX)
> + return -EINVAL;

The idr upper bound is INT_MAX [inclusive].

> + mutex_lock(&dev->struct_mutex);
> + file = idr_find(&file_priv->master->magic_map, auth->magic);

But given that it will return NULL anyway, do you really want to
short-circuit the erroneous lookup, and just leave it to the idr to
validate it itself?

So... How efficient is the cyclic idr for a long running system that has
accumulated several hundred thousand stale magics?

Maybe an igt to create a couple of billion shortlived clients (with
overlapping lifetimes) and check the behaviour?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre


[PATCH 3/3] drm: simplify master cleanup

2015-05-04 Thread Chris Wilson
On Mon, May 04, 2015 at 04:05:14PM +0200, David Herrmann wrote:
> In drm_master_destroy() we _free_ the master object. There is no reason to
> hold any locks while dropping its static members, nor do we have to reset
> it to 0.
> 
> Furthermore, kfree() already does NULL checks, so call it directly on
> master->unique and drop the redundant reset-code.
> 
> Signed-off-by: David Herrmann 
Reviewed-by: Chris Wilson 
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre


[PATCH 2/3] drm: simplify authentication management

2015-05-04 Thread David Herrmann
Hi

On Mon, May 4, 2015 at 4:46 PM, Chris Wilson  
wrote:
> On Mon, May 04, 2015 at 04:05:13PM +0200, David Herrmann wrote:
>> The magic auth tokens we have are a simple map from cyclic IDs to drm_file
>> objects. Remove all the old bulk of code and replace it with a simple,
>> direct IDR.
>>
>> The previous behavior is kept. Especially calling authmagic multiple times
>> on the same magic results in EINVAL except on the first call. The only
>> difference in behavior is that we never allocate IDs multiple times, even
>> if they were already authenticated. To trigger that, you need 2^31 open
>> DRM file descriptions at the same time, though.
>>
>> Diff-stat:
>>5 files changed, 45 insertions(+), 157 deletions(-)
>>
>> Signed-off-by: David Herrmann 
>
> Just one quibble,
>
>> @@ -189,10 +80,17 @@ int drm_authmagic(struct drm_device *dev, void *data,
>>   struct drm_file *file;
>>
>>   DRM_DEBUG("%u\n", auth->magic);
>> - if ((file = drm_find_file(file_priv->master, auth->magic))) {
>> +
>> + if (auth->magic >= INT_MAX)
>> + return -EINVAL;
>
> The idr upper bound is INT_MAX [inclusive].
>
>> + mutex_lock(&dev->struct_mutex);
>> + file = idr_find(&file_priv->master->magic_map, auth->magic);
>
> But given that it will return NULL anyway, do you really want to
> short-circuit the erroneous lookup, and just leave it to the idr to
> validate it itself?

Indeed, I can just change it to "auth->magic > INT_MAX". Fixed!

> So... How efficient is the cyclic idr for a long running system that has
> accumulated several hundred thousand stale magics?
>
> Maybe an igt to create a couple of billion shortlived clients (with
> overlapping lifetimes) and check the behaviour?

I'm not sure this is an interesting benchmark. I mean, you usually
have a dozen DRM clients max, anyway. So the only interesting detail
is whether the cyclic behavior causes the IDR tree to degenerate. But
in that case, we should either fix IDR or see whether we can avoid the
cyclic behavior.

Given that I use idr_replace(idr, NULL, id) on auth, instead of an
immediate idr_remove(), we can just use idr_alloc() instead of
idr_alloc_cyclic(). However, idr_alloc_cyclic() seems to work fine for
inotify, so it should be fine for auth-magics, I guess (and
auth-magics aren't part of any fast-path, right?).

Anyway, I can ping Tejun and ask whether the cyclic IDR has any
visible downsides compared to normal allocation. He should know.

Thanks!
David


[PATCH 2/3] drm: simplify authentication management

2015-05-04 Thread Chris Wilson
On Mon, May 04, 2015 at 05:02:37PM +0200, David Herrmann wrote:
> Hi
> 
> On Mon, May 4, 2015 at 4:46 PM, Chris Wilson  
> wrote:
> > On Mon, May 04, 2015 at 04:05:13PM +0200, David Herrmann wrote:
> >> The magic auth tokens we have are a simple map from cyclic IDs to drm_file
> >> objects. Remove all the old bulk of code and replace it with a simple,
> >> direct IDR.
> >>
> >> The previous behavior is kept. Especially calling authmagic multiple times
> >> on the same magic results in EINVAL except on the first call. The only
> >> difference in behavior is that we never allocate IDs multiple times, even
> >> if they were already authenticated. To trigger that, you need 2^31 open
> >> DRM file descriptions at the same time, though.
> >>
> >> Diff-stat:
> >>5 files changed, 45 insertions(+), 157 deletions(-)
> >>
> >> Signed-off-by: David Herrmann 
> >
> > Just one quibble,
> >
> >> @@ -189,10 +80,17 @@ int drm_authmagic(struct drm_device *dev, void *data,
> >>   struct drm_file *file;
> >>
> >>   DRM_DEBUG("%u\n", auth->magic);
> >> - if ((file = drm_find_file(file_priv->master, auth->magic))) {
> >> +
> >> + if (auth->magic >= INT_MAX)
> >> + return -EINVAL;
> >
> > The idr upper bound is INT_MAX [inclusive].
> >
> >> + mutex_lock(&dev->struct_mutex);
> >> + file = idr_find(&file_priv->master->magic_map, auth->magic);
> >
> > But given that it will return NULL anyway, do you really want to
> > short-circuit the erroneous lookup, and just leave it to the idr to
> > validate it itself?
> 
> Indeed, I can just change it to "auth->magic > INT_MAX". Fixed!
> 
> > So... How efficient is the cyclic idr for a long running system that has
> > accumulated several hundred thousand stale magics?
> >
> > Maybe an igt to create a couple of billion shortlived clients (with
> > overlapping lifetimes) and check the behaviour?
> 
> I'm not sure this is an interesting benchmark. I mean, you usually
> have a dozen DRM clients max, anyway. So the only interesting detail
> is whether the cyclic behavior causes the IDR tree to degenerate. But
> in that case, we should either fix IDR or see whether we can avoid the
> cyclic behavior.

I was actually more concerned about what happens if we end up with 2
billion stale clients - do we get 2 billion entries? Can we even
allocate that many? I suspect we end up with a DoS.

> Given that I use idr_replace(idr, NULL, id) on auth, instead of an
> immediate idr_remove(), we can just use idr_alloc() instead of
> idr_alloc_cyclic(). However, idr_alloc_cyclic() seems to work fine for
> inotify, so it should be fine for auth-magics, I guess (and
> auth-magics aren't part of any fast-path, right?).

Right, not concerned about speed here. Presuming no anomalous behaviour!

> Anyway, I can ping Tejun and ask whether the cyclic IDR has any
> visible downsides compared to normal allocation. He should know.

Anyway, I just thought the test would be reasonably easy to write and
useful to compare behaviour of long running systems - and helps to
improve our test coverage just that little bit.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre


[PATCH 2/9] mm: Provide new get_vaddr_pfns() helper

2015-05-04 Thread Jan Kara
On Thu 30-04-15 16:55:31, Mel Gorman wrote:
> On Tue, Mar 17, 2015 at 12:56:32PM +0100, Jan Kara wrote:
> > Provide new function get_vaddr_pfns().  This function maps virtual
> > addresses from given start and fills given array with page frame numbers of
> > the corresponding pages. If given start belongs to a normal vma, the 
> > function
> > grabs reference to each of the pages to pin them in memory. If start
> > belongs to VM_IO | VM_PFNMAP vma, we don't touch page structures. Caller
> > should make sure pfns aren't reused for anything else while he is using
> > them.
> > 
> > This function is created for various drivers to simplify handling of
> > their buffers.
> > 
> > Signed-off-by: Jan Kara 
> > ---
> >  include/linux/mm.h |  38 +++
> >  mm/gup.c   | 180 
> > +
> >  2 files changed, 218 insertions(+)
> > 
> > diff --git a/include/linux/mm.h b/include/linux/mm.h
> > index 47a93928b90f..a5045df92454 100644
> > --- a/include/linux/mm.h
> > +++ b/include/linux/mm.h
> > @@ -1279,6 +1279,44 @@ long get_user_pages_unlocked(struct task_struct 
> > *tsk, struct mm_struct *mm,
> > int write, int force, struct page **pages);
> >  int get_user_pages_fast(unsigned long start, int nr_pages, int write,
> > struct page **pages);
> > +
> > +/* Container for pinned pfns / pages */
> > +struct pinned_pfns {
> > +   unsigned int nr_allocated_pfns; /* Number of pfns we have space for */
> > +   unsigned int nr_pfns;   /* Number of pfns stored in pfns array 
> > */
> > +   unsigned int got_ref:1; /* Did we pin pfns by getting page ref? 
> > */
> > +   unsigned int is_pages:1;/* Does array contain pages or pfns? */
> 
> The bit field is probably overkill as I expect it'll get padded out for
> pointer alignment anyway. Just use bools.
  Makes sense.

> is_pfns is less ambiguous than is_pages but not very important.
> 
> The naming is not great in general. Only struct pages are pinned in the
> traditional meaning of the word. The raw PFNs are not so there is no such
> thing as a "pinned pfns". It might be better just to call it frame_vectors
> and document that it's either raw PFNs that the caller should be responsible
> for or struct pages that are pinned.
  Good point. I'll try to come up with a better name.

 - I agree with minor comments there.

> >  /**
> > + * get_vaddr_pfns() - map virtual addresses to pfns
> > + * @start: starting user address
> > + * @nr_pfns:   number of pfns from start to map
> > + * @write: whether pages will be written to by the caller
> > + * @force: whether to force write access even if user mapping is
> > + * readonly. This will result in the page being COWed even
> > + * in MAP_SHARED mappings. You do not want this.
> > + * @pfns:  structure which receives pfns of the pages mapped.
> > + * It should have space for at least nr_pfns pfns.
> > + *
> > + * This function maps virtual addresses from @start and fills @pfns 
> > structure
> > + * with page frame numbers of corresponding pages. If @start belongs to a
> > + * normal vma, the function grabs reference to each of the pages to pin 
> > them in
> > + * memory. If @start belongs to VM_IO | VM_PFNMAP vma, we don't touch page
> > + * structures. Caller should make sure pfns aren't reused for anything else
> > + * while he is using them.
> > + *
> > + * This function takes care of grabbing mmap_sem as necessary.
> > + */
> > +int get_vaddr_pfns(unsigned long start, int nr_pfns, int write, int force,
> > +  struct pinned_pfns *pfns)
> > +{
> > +   struct mm_struct *mm = current->mm;
> > +   struct vm_area_struct *vma;
> > +   int ret = 0;
> > +   int err;
> > +
> > +   if (nr_pfns <= 0)
> > +   return 0;
> > +
> 
> I know I suggested that nr_pfns should be unsigned earlier and then I
> saw this. Is there any valid use of the API that would pass in a
> negative number here?
  No. It was there just because I had ints everywhere without too much
thought (it's shorter to type *grin*). I'll put unsigned types where it
makes sense and add a test for INT_MAX so that get_vaddr_pfns() can still
return negative errors.

> > +   if (nr_pfns > pfns->nr_allocated_pfns)
> > +   nr_pfns = pfns->nr_allocated_pfns;
> > +
> 
> Should this be a WARN_ON_ONCE? You recover from it obviously but the return
> value is not documented to say that it could return less than requested.
> Of course, this is implied but a caller might assume it's due to a transient
> error instead of broken API usage.
  Yep, good idea.

> > +   down_read(&mm->mmap_sem);
> > +   vma = find_vma_intersection(mm, start, start + 1);
> > +   if (!vma) {
> > +   ret = -EFAULT;
> > +   goto out;
> > +   }
> 
> Returning -EFAULT means that the return value has to be signed but the
> structure itself has unsigned int for counters so the maximum number of
> PFNs supported is not available. We'd never wan

[PATCH 2/3] drm: simplify authentication management

2015-05-04 Thread David Herrmann
Hi

On Mon, May 4, 2015 at 5:11 PM, Chris Wilson  
wrote:
> On Mon, May 04, 2015 at 05:02:37PM +0200, David Herrmann wrote:
>> Hi
>>
>> On Mon, May 4, 2015 at 4:46 PM, Chris Wilson  
>> wrote:
>> > On Mon, May 04, 2015 at 04:05:13PM +0200, David Herrmann wrote:
>> >> The magic auth tokens we have are a simple map from cyclic IDs to drm_file
>> >> objects. Remove all the old bulk of code and replace it with a simple,
>> >> direct IDR.
>> >>
>> >> The previous behavior is kept. Especially calling authmagic multiple times
>> >> on the same magic results in EINVAL except on the first call. The only
>> >> difference in behavior is that we never allocate IDs multiple times, even
>> >> if they were already authenticated. To trigger that, you need 2^31 open
>> >> DRM file descriptions at the same time, though.
>> >>
>> >> Diff-stat:
>> >>5 files changed, 45 insertions(+), 157 deletions(-)
>> >>
>> >> Signed-off-by: David Herrmann 
>> >
>> > Just one quibble,
>> >
>> >> @@ -189,10 +80,17 @@ int drm_authmagic(struct drm_device *dev, void *data,
>> >>   struct drm_file *file;
>> >>
>> >>   DRM_DEBUG("%u\n", auth->magic);
>> >> - if ((file = drm_find_file(file_priv->master, auth->magic))) {
>> >> +
>> >> + if (auth->magic >= INT_MAX)
>> >> + return -EINVAL;
>> >
>> > The idr upper bound is INT_MAX [inclusive].
>> >
>> >> + mutex_lock(&dev->struct_mutex);
>> >> + file = idr_find(&file_priv->master->magic_map, auth->magic);
>> >
>> > But given that it will return NULL anyway, do you really want to
>> > short-circuit the erroneous lookup, and just leave it to the idr to
>> > validate it itself?
>>
>> Indeed, I can just change it to "auth->magic > INT_MAX". Fixed!
>>
>> > So... How efficient is the cyclic idr for a long running system that has
>> > accumulated several hundred thousand stale magics?
>> >
>> > Maybe an igt to create a couple of billion shortlived clients (with
>> > overlapping lifetimes) and check the behaviour?
>>
>> I'm not sure this is an interesting benchmark. I mean, you usually
>> have a dozen DRM clients max, anyway. So the only interesting detail
>> is whether the cyclic behavior causes the IDR tree to degenerate. But
>> in that case, we should either fix IDR or see whether we can avoid the
>> cyclic behavior.
>
> I was actually more concerned about what happens if we end up with 2
> billion stale clients - do we get 2 billion entries? Can we even
> allocate that many? I suspect we end up with a DoS.

Ehh, the magic-entries are dropped on close(). So to actually keep 2
billion entries, you also need 2 billion "struct file*", "struct
drm_file*", ...

The idr_alloc_cyclic() does _not_ mark old entries as "used". All it
does is try to remember new IDs in a cyclic order. On wrap-around, old
IDs will get re-used.

Thanks
David


[Bug 90266] Unigine Heaven 4.0 logging vm faults since radeon/llvm: Run LLVM's instruction combining pass

2015-05-04 Thread bugzilla-dae...@freedesktop.org
https://bugs.freedesktop.org/show_bug.cgi?id=90266

--- Comment #4 from Tom Stellard  ---
(In reply to Andy Furniss from comment #0)
> Related to 
> 
> https://bugs.freedesktop.org/show_bug.cgi?id=90056
> 
> Unlike valley I thought heaven 4.0 wasn't affected by above as it runs and
> looks OK.
> 
> There is however an issue, again "fixed" by reverting mesa commit 
> 

Thanks for the report, I just have a few qustions:

Are there any problems with Valley after the fix for Bug 90056 was committed?

Does the problem reported in this bug for Heaven still happen if you keep the
InsCombine pass enabled, but revert the fix from Bug 90056?

-- 
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/20150504/d066d565/attachment.html>


[PATCH 2/3] drm: simplify authentication management

2015-05-04 Thread Chris Wilson
On Mon, May 04, 2015 at 05:14:51PM +0200, David Herrmann wrote:
> Hi
> 
> On Mon, May 4, 2015 at 5:11 PM, Chris Wilson  
> wrote:
> > On Mon, May 04, 2015 at 05:02:37PM +0200, David Herrmann wrote:
> >> Hi
> >>
> >> On Mon, May 4, 2015 at 4:46 PM, Chris Wilson  
> >> wrote:
> >> > On Mon, May 04, 2015 at 04:05:13PM +0200, David Herrmann wrote:
> >> >> The magic auth tokens we have are a simple map from cyclic IDs to 
> >> >> drm_file
> >> >> objects. Remove all the old bulk of code and replace it with a simple,
> >> >> direct IDR.
> >> >>
> >> >> The previous behavior is kept. Especially calling authmagic multiple 
> >> >> times
> >> >> on the same magic results in EINVAL except on the first call. The only
> >> >> difference in behavior is that we never allocate IDs multiple times, 
> >> >> even
> >> >> if they were already authenticated. To trigger that, you need 2^31 open
> >> >> DRM file descriptions at the same time, though.
> >> >>
> >> >> Diff-stat:
> >> >>5 files changed, 45 insertions(+), 157 deletions(-)
> >> >>
> >> >> Signed-off-by: David Herrmann 
> >> >
> >> > Just one quibble,
> >> >
> >> >> @@ -189,10 +80,17 @@ int drm_authmagic(struct drm_device *dev, void 
> >> >> *data,
> >> >>   struct drm_file *file;
> >> >>
> >> >>   DRM_DEBUG("%u\n", auth->magic);
> >> >> - if ((file = drm_find_file(file_priv->master, auth->magic))) {
> >> >> +
> >> >> + if (auth->magic >= INT_MAX)
> >> >> + return -EINVAL;
> >> >
> >> > The idr upper bound is INT_MAX [inclusive].
> >> >
> >> >> + mutex_lock(&dev->struct_mutex);
> >> >> + file = idr_find(&file_priv->master->magic_map, auth->magic);
> >> >
> >> > But given that it will return NULL anyway, do you really want to
> >> > short-circuit the erroneous lookup, and just leave it to the idr to
> >> > validate it itself?
> >>
> >> Indeed, I can just change it to "auth->magic > INT_MAX". Fixed!
> >>
> >> > So... How efficient is the cyclic idr for a long running system that has
> >> > accumulated several hundred thousand stale magics?
> >> >
> >> > Maybe an igt to create a couple of billion shortlived clients (with
> >> > overlapping lifetimes) and check the behaviour?
> >>
> >> I'm not sure this is an interesting benchmark. I mean, you usually
> >> have a dozen DRM clients max, anyway. So the only interesting detail
> >> is whether the cyclic behavior causes the IDR tree to degenerate. But
> >> in that case, we should either fix IDR or see whether we can avoid the
> >> cyclic behavior.
> >
> > I was actually more concerned about what happens if we end up with 2
> > billion stale clients - do we get 2 billion entries? Can we even
> > allocate that many? I suspect we end up with a DoS.
> 
> Ehh, the magic-entries are dropped on close(). So to actually keep 2
> billion entries, you also need 2 billion "struct file*", "struct
> drm_file*", ...
> 
> The idr_alloc_cyclic() does _not_ mark old entries as "used". All it
> does is try to remember new IDs in a cyclic order. On wrap-around, old
> IDs will get re-used.

But the layers are only freed if all below them are empty (iiui).
Basically, I am not entirely familar with how idr will cope on a long
running system, and would like some elaboration (and a test for future
reference!).
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre


[Bug 97701] New: [regression] won't boot unless radeon.audio=0

2015-05-04 Thread bugzilla-dae...@bugzilla.kernel.org
https://bugzilla.kernel.org/show_bug.cgi?id=97701

Bug ID: 97701
   Summary: [regression] won't boot unless radeon.audio=0
   Product: Drivers
   Version: 2.5
Kernel Version: 4.1-rc2
  Hardware: All
OS: Linux
  Tree: Mainline
Status: NEW
  Severity: normal
  Priority: P1
 Component: Video(DRI - non Intel)
  Assignee: drivers_video-dri at kernel-bugs.osdl.org
  Reporter: jtmettala at gmail.com
Regression: No

Created attachment 175791
  --> https://bugzilla.kernel.org/attachment.cgi?id=175791&action=edit
picture of trace

Boot regression between 4.1-rc1 and 4.1-rc2

04:00.0 VGA compatible controller: Advanced Micro Devices, Inc. [AMD/ATI] RV350
[Radeon 9550/9600/X1050 Series]

Attached picture shows trace.

I guess full bisect is not needed.

ps. https://lkml.org/lkml/2015/5/4/491 looks like it might be related.

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


[Bug 90266] Unigine Heaven 4.0 logging vm faults since radeon/llvm: Run LLVM's instruction combining pass

2015-05-04 Thread bugzilla-dae...@freedesktop.org
https://bugs.freedesktop.org/show_bug.cgi?id=90266

--- Comment #5 from Andy Furniss  ---
(In reply to Tom Stellard from comment #4)
> (In reply to Andy Furniss from comment #0)
> > Related to 
> > 
> > https://bugs.freedesktop.org/show_bug.cgi?id=90056
> > 
> > Unlike valley I thought heaven 4.0 wasn't affected by above as it runs and
> > looks OK.
> > 
> > There is however an issue, again "fixed" by reverting mesa commit 
> > 
> 
> Thanks for the report, I just have a few qustions:
> 
> Are there any problems with Valley after the fix for Bug 90056 was committed?

No, Valley is OK

> Does the problem reported in this bug for Heaven still happen if you keep
> the InsCombine pass enabled, but revert the fix from Bug 90056?

Yes with or without the fix Heaven logs the errors.

Initially tested on an older llvm with and without the patch from -

https://bugs.freedesktop.org/show_bug.cgi?id=90056#c14

and again just now by reverting "R600/SI: Fix verifier errors from the
SIAnnotateControlFlow" on a more recent llvm.

-- 
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/20150504/aa67ae39/attachment.html>


[PATCH 2/3] drm: simplify authentication management

2015-05-04 Thread David Herrmann
Hi

On Mon, May 4, 2015 at 5:25 PM, Chris Wilson  
wrote:
> On Mon, May 04, 2015 at 05:14:51PM +0200, David Herrmann wrote:
>> Hi
>>
>> On Mon, May 4, 2015 at 5:11 PM, Chris Wilson  
>> wrote:
>> > On Mon, May 04, 2015 at 05:02:37PM +0200, David Herrmann wrote:
>> >> Hi
>> >>
>> >> On Mon, May 4, 2015 at 4:46 PM, Chris Wilson > >> chris-wilson.co.uk> wrote:
>> >> > On Mon, May 04, 2015 at 04:05:13PM +0200, David Herrmann wrote:
>> >> >> The magic auth tokens we have are a simple map from cyclic IDs to 
>> >> >> drm_file
>> >> >> objects. Remove all the old bulk of code and replace it with a simple,
>> >> >> direct IDR.
>> >> >>
>> >> >> The previous behavior is kept. Especially calling authmagic multiple 
>> >> >> times
>> >> >> on the same magic results in EINVAL except on the first call. The only
>> >> >> difference in behavior is that we never allocate IDs multiple times, 
>> >> >> even
>> >> >> if they were already authenticated. To trigger that, you need 2^31 open
>> >> >> DRM file descriptions at the same time, though.
>> >> >>
>> >> >> Diff-stat:
>> >> >>5 files changed, 45 insertions(+), 157 deletions(-)
>> >> >>
>> >> >> Signed-off-by: David Herrmann 
>> >> >
>> >> > Just one quibble,
>> >> >
>> >> >> @@ -189,10 +80,17 @@ int drm_authmagic(struct drm_device *dev, void 
>> >> >> *data,
>> >> >>   struct drm_file *file;
>> >> >>
>> >> >>   DRM_DEBUG("%u\n", auth->magic);
>> >> >> - if ((file = drm_find_file(file_priv->master, auth->magic))) {
>> >> >> +
>> >> >> + if (auth->magic >= INT_MAX)
>> >> >> + return -EINVAL;
>> >> >
>> >> > The idr upper bound is INT_MAX [inclusive].
>> >> >
>> >> >> + mutex_lock(&dev->struct_mutex);
>> >> >> + file = idr_find(&file_priv->master->magic_map, auth->magic);
>> >> >
>> >> > But given that it will return NULL anyway, do you really want to
>> >> > short-circuit the erroneous lookup, and just leave it to the idr to
>> >> > validate it itself?
>> >>
>> >> Indeed, I can just change it to "auth->magic > INT_MAX". Fixed!
>> >>
>> >> > So... How efficient is the cyclic idr for a long running system that has
>> >> > accumulated several hundred thousand stale magics?
>> >> >
>> >> > Maybe an igt to create a couple of billion shortlived clients (with
>> >> > overlapping lifetimes) and check the behaviour?
>> >>
>> >> I'm not sure this is an interesting benchmark. I mean, you usually
>> >> have a dozen DRM clients max, anyway. So the only interesting detail
>> >> is whether the cyclic behavior causes the IDR tree to degenerate. But
>> >> in that case, we should either fix IDR or see whether we can avoid the
>> >> cyclic behavior.
>> >
>> > I was actually more concerned about what happens if we end up with 2
>> > billion stale clients - do we get 2 billion entries? Can we even
>> > allocate that many? I suspect we end up with a DoS.
>>
>> Ehh, the magic-entries are dropped on close(). So to actually keep 2
>> billion entries, you also need 2 billion "struct file*", "struct
>> drm_file*", ...
>>
>> The idr_alloc_cyclic() does _not_ mark old entries as "used". All it
>> does is try to remember new IDs in a cyclic order. On wrap-around, old
>> IDs will get re-used.
>
> But the layers are only freed if all below them are empty (iiui).
> Basically, I am not entirely familar with how idr will cope on a long
> running system, and would like some elaboration (and a test for future
> reference!).

IDR uses a tree of idr_layer objects (basically an optimized suffix
tree on the binary ID). Tree depth is increased dynamically if a layer
runs full. Max depths of layers is 4. Each layer manages 256 children
(leafs store the void* data, nodes store pointers to sub-layers). Size
of a layer is roughly 2k on 64bit.

If the ID-range is small and packed, the storage is used optimally.
But a sparse ID-range might occupy one layer per ID in worst case.
Lookup speed should be negligible as we are limited to a depth of 4
layers. However, memory consumption might get huge if the IDs are
spread even. But this really depends on the allocation scheme.

I'm not sure how to write a benchmark for this. I mean, I can easily
craft one that causes the IDR to degenerate, but it requires us to
keep huge numbers of files open.
But this fact makes IDR rather suboptimal for cyclic allocations, so I
switched to idr_alloc() now. This guarantees tight/packed ID ranges
and user-space cannot degenerate the layers, anymore. That is, unless
you open more than 256 FDs on a device in parallel, we're fine with a
single IDR layer; always. This should work fine, right?

Thanks
David


[Bug 97701] [regression] won't boot unless radeon.audio=0

2015-05-04 Thread bugzilla-dae...@bugzilla.kernel.org
https://bugzilla.kernel.org/show_bug.cgi?id=97701

Jouni Mettälä  changed:

   What|Removed |Added

 CC||jtmettala at gmail.com
 Regression|No  |Yes

--- Comment #1 from Jouni Mettälä  ---
Reverting commit 0f55db36d49d45b80eff0c0a2a498766016f458b fixes it for me too.

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


[PATCH 2/3] drm: simplify authentication management

2015-05-04 Thread Chris Wilson
On Mon, May 04, 2015 at 06:28:25PM +0200, David Herrmann wrote:
> I'm not sure how to write a benchmark for this. I mean, I can easily
> craft one that causes the IDR to degenerate, but it requires us to
> keep huge numbers of files open.
> But this fact makes IDR rather suboptimal for cyclic allocations, so I
> switched to idr_alloc() now. This guarantees tight/packed ID ranges
> and user-space cannot degenerate the layers, anymore. That is, unless
> you open more than 256 FDs on a device in parallel, we're fine with a
> single IDR layer; always. This should work fine, right?

That pretty much circumvents my only worry! If there is a client leak,
the system will eventually keel under the load, and that we have a huge
number of magics open is insignificant.

As far as test coverage I would focus on

(a) authenticating up to vfs-file-max fds (i.e. check that we hit
the system limits without authmagic failing)

(b) churn through a small number of clients for a few minutes, just
basically checking for anomalous behaviour and that allocation times
every minute or so remain constant.

(c) just check that we can authenticate! always useful for patches that
touch the authmagic system

I was thinking of a few more, but they basically serve to show the holes
in the authmagic scheme.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre


[Bug 89405] radeon: *ERROR* invalid ioctl running weston-launch while X is running

2015-05-04 Thread bugzilla-dae...@freedesktop.org
https://bugs.freedesktop.org/show_bug.cgi?id=89405

--- Comment #1 from fosero at gmail.com ---
Maybe unrelated, but there seems to be issue initializing given gfx chip when
running anything non X.

On a fresh boot trying to run any wayland session (weston directly or current
stable gdm) results in just a black screen. No obvious error in the logs.
Booting with plymouth enabled also just results in a black screen.

However when X is started first any subsequent wayland session shows just fine.

-- 
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/20150504/0e39a7d2/attachment.html>


[PATCH v6 00/11]

2015-05-04 Thread Kamil Debski
Hi,

The sixth version of this patchset addresses recent comments on the mailing
list. Please see the changelog below for details.

Best wishes,
Kamil Debski

Changes since v5

- drop struct cec_timeval in favour of a __u64 that keeps the timestamp in ns
- remove userspace documentation from Documentation/cec.txt as userspace API
  is described in the DocBook
- add missing documentation for the passthrough mode to the DocBook
- add information about the number of events that can be queued
- fix misspelling of reply
- fix behaviour of posting an event in cec_received_msg, such that the behaviour
  is consistent with the documentation

Changes since v4

- add sequence numbering to transmitted messages
- add sequence number handling to event hanlding
- add passthrough mode
- change reserved field sizes
- fixed CEC version defines and addec CEC 2.0 commands
- add DocBook documentation

Changes since v3

- remove the promiscuous mode
- rewrite the devicetree patches
- fixes, expansion and partial rewrite of the documentation
- reorder of API structures and addition of reserved fields
- use own struct to report time (32/64 bit safe)
- fix of handling events
- add cec.h to include/uapi/linux/Kbuild
- fixes in the adv76xx driver (add missing methods, change adv7604 to adv76xx)
- cleanup of debug messages in s5p-cec driver
- remove non necessary claiming of a gpio in the s5p-cec driver
- cleanup headers of the s5p-cec driver

Changes since v2
===-
- added promiscuous mode
- added new key codes to the input framework
- add vendor ID reporting
- add the possibility to clear assigned logical addresses
- cleanup of the rc cec map

Changes since v1

- documentation edited and moved to the Documentation folder
- added key up/down message handling
- add missing CEC commands to the cec.h file

Background
==

The work on a common CEC framework was started over three years ago by Hans
Verkuil. Unfortunately the work has stalled. As I have received the task of
creating a driver for the CEC interface module present on the Exynos range of
SoCs, I got in touch with Hans. He replied that the work stalled due to his
lack of time.

Original RFC by Hans Verkuil/Martin Bugge
=
https://www.mail-archive.com/linux-media at vger.kernel.org/msg28735.html


Hans Verkuil (5):
  cec: add HDMI CEC framework
  DocBook/media: add CEC documentation
  v4l2-subdev: add HDMI CEC ops
  cec: adv7604: add cec support.
  cec: adv7511: add cec support.

Kamil Debski (6):
  dts: exynos4*: add HDMI CEC pin definition to pinctrl
  dts: exynos4: add node for the HDMI CEC device
  dts: exynos4412-odroid*: enable the HDMI CEC device
  HID: add HDMI CEC specific keycodes
  rc: Add HDMI CEC protoctol handling
  cec: s5p-cec: Add s5p-cec driver

 Documentation/DocBook/media/Makefile   |4 +-
 Documentation/DocBook/media/v4l/biblio.xml |   10 +
 Documentation/DocBook/media/v4l/cec-api.xml|   74 ++
 Documentation/DocBook/media/v4l/cec-func-close.xml |   59 +
 Documentation/DocBook/media/v4l/cec-func-ioctl.xml |   73 ++
 Documentation/DocBook/media/v4l/cec-func-open.xml  |   94 ++
 Documentation/DocBook/media/v4l/cec-func-poll.xml  |   89 ++
 .../DocBook/media/v4l/cec-ioc-g-adap-log-addrs.xml |  275 +
 .../DocBook/media/v4l/cec-ioc-g-adap-phys-addr.xml |   78 ++
 .../DocBook/media/v4l/cec-ioc-g-adap-state.xml |   87 ++
 Documentation/DocBook/media/v4l/cec-ioc-g-caps.xml |  173 +++
 .../DocBook/media/v4l/cec-ioc-g-event.xml  |  125 ++
 .../DocBook/media/v4l/cec-ioc-g-passthrough.xml|   88 ++
 .../DocBook/media/v4l/cec-ioc-g-vendor-id.xml  |   70 ++
 .../DocBook/media/v4l/cec-ioc-receive.xml  |  185 +++
 Documentation/DocBook/media_api.tmpl   |6 +-
 Documentation/cec.txt  |  165 +++
 .../devicetree/bindings/media/s5p-cec.txt  |   33 +
 arch/arm/boot/dts/exynos4.dtsi |   12 +
 arch/arm/boot/dts/exynos4210-pinctrl.dtsi  |7 +
 arch/arm/boot/dts/exynos4412-odroid-common.dtsi|4 +
 arch/arm/boot/dts/exynos4x12-pinctrl.dtsi  |7 +
 drivers/media/Kconfig  |6 +
 drivers/media/Makefile |2 +
 drivers/media/cec.c| 1191 
 drivers/media/i2c/adv7511.c|  347 +-
 drivers/media/i2c/adv7604.c|  207 +++-
 drivers/media/platform/Kconfig |   10 +
 drivers/media/platform/Makefile|1 +
 drivers/media/platform/s5p-cec/Makefile|4 +
 drivers/media/platform/s5p-cec/exynos_hdmi_cec.h   |   37 +
 .../media/platform/s5p-cec/exynos_hdmi_cecctrl.c   |  208 
 drivers/media/platform/s5p-cec/regs-cec.h  |   96 ++
 drivers/media/platform/s5p-cec/s5p_cec.c   |  283 +
 drive

[PATCH v6 01/11] dts: exynos4*: add HDMI CEC pin definition to pinctrl

2015-05-04 Thread Kamil Debski
Add pinctrl nodes for the HDMI CEC device to the Exynos4210 and
Exynos4x12 SoCs. These are required by the HDMI CEC device.

Signed-off-by: Kamil Debski 
---
 arch/arm/boot/dts/exynos4210-pinctrl.dtsi |7 +++
 arch/arm/boot/dts/exynos4x12-pinctrl.dtsi |7 +++
 2 files changed, 14 insertions(+)

diff --git a/arch/arm/boot/dts/exynos4210-pinctrl.dtsi 
b/arch/arm/boot/dts/exynos4210-pinctrl.dtsi
index a7c2128..9331c62 100644
--- a/arch/arm/boot/dts/exynos4210-pinctrl.dtsi
+++ b/arch/arm/boot/dts/exynos4210-pinctrl.dtsi
@@ -820,6 +820,13 @@
samsung,pin-pud = <1>;
samsung,pin-drv = <0>;
};
+
+   hdmi_cec: hdmi-cec {
+   samsung,pins = "gpx3-6";
+   samsung,pin-function = <3>;
+   samsung,pin-pud = <0>;
+   samsung,pin-drv = <0>;
+   };
};

pinctrl at 0386 {
diff --git a/arch/arm/boot/dts/exynos4x12-pinctrl.dtsi 
b/arch/arm/boot/dts/exynos4x12-pinctrl.dtsi
index c141931..875464e 100644
--- a/arch/arm/boot/dts/exynos4x12-pinctrl.dtsi
+++ b/arch/arm/boot/dts/exynos4x12-pinctrl.dtsi
@@ -885,6 +885,13 @@
samsung,pin-pud = <0>;
samsung,pin-drv = <0>;
};
+
+   hdmi_cec: hdmi-cec {
+   samsung,pins = "gpx3-6";
+   samsung,pin-function = <3>;
+   samsung,pin-pud = <0>;
+   samsung,pin-drv = <0>;
+   };
};

pinctrl at 0386 {
-- 
1.7.9.5



[PATCH v6 02/11] dts: exynos4: add node for the HDMI CEC device

2015-05-04 Thread Kamil Debski
This patch adds HDMI CEC node specific to the Exynos4210/4x12 SoC series.

Signed-off-by: Kamil Debski 
---
 arch/arm/boot/dts/exynos4.dtsi |   12 
 1 file changed, 12 insertions(+)

diff --git a/arch/arm/boot/dts/exynos4.dtsi b/arch/arm/boot/dts/exynos4.dtsi
index e20cdc2..8776db9 100644
--- a/arch/arm/boot/dts/exynos4.dtsi
+++ b/arch/arm/boot/dts/exynos4.dtsi
@@ -704,6 +704,18 @@
status = "disabled";
};

+   hdmicec: cec at 100B {
+   compatible = "samsung,s5p-cec";
+   reg = <0x100B 0x200>;
+   interrupts = <0 114 0>;
+   clocks = <&clock CLK_HDMI_CEC>;
+   clock-names = "hdmicec";
+   samsung,syscon-phandle = <&pmu_system_controller>;
+   pinctrl-names = "default";
+   pinctrl-0 = <&hdmi_cec>;
+   status = "disabled";
+   };
+
mixer: mixer at 12C1 {
compatible = "samsung,exynos4210-mixer";
interrupts = <0 91 0>;
-- 
1.7.9.5



[PATCH v6 03/11] dts: exynos4412-odroid*: enable the HDMI CEC device

2015-05-04 Thread Kamil Debski
Add a dts node entry and enable the HDMI CEC device present in the Exynos4
family of SoCs.

Signed-off-by: Kamil Debski 
---
 arch/arm/boot/dts/exynos4412-odroid-common.dtsi |4 
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi 
b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
index 8de12af..e50862d 100644
--- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
+++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
@@ -469,6 +469,10 @@
status = "okay";
};

+   cec at 100B {
+   status = "okay";
+   };
+
hdmi_ddc: i2c at 1388 {
status = "okay";
pinctrl-names = "default";
-- 
1.7.9.5



[PATCH v6 04/11] HID: add HDMI CEC specific keycodes

2015-05-04 Thread Kamil Debski
Add HDMI CEC specific keycodes to the keycodes definition.

Signed-off-by: Kamil Debski 
---
 include/uapi/linux/input.h |   12 
 1 file changed, 12 insertions(+)

diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
index 731417c..7430a3f 100644
--- a/include/uapi/linux/input.h
+++ b/include/uapi/linux/input.h
@@ -752,6 +752,18 @@ struct input_keymap_entry {
 #define KEY_KBDINPUTASSIST_ACCEPT  0x264
 #define KEY_KBDINPUTASSIST_CANCEL  0x265

+#define KEY_RIGHT_UP   0x266
+#define KEY_RIGHT_DOWN 0x267
+#define KEY_LEFT_UP0x268
+#define KEY_LEFT_DOWN  0x269
+
+#define KEY_NEXT_FAVORITE  0x270
+#define KEY_STOP_RECORD0x271
+#define KEY_PAUSE_RECORD   0x272
+#define KEY_VOD0x273
+#define KEY_UNMUTE 0x274
+#define KEY_DVB0x275
+
 #define BTN_TRIGGER_HAPPY  0x2c0
 #define BTN_TRIGGER_HAPPY1 0x2c0
 #define BTN_TRIGGER_HAPPY2 0x2c1
-- 
1.7.9.5



[PATCH v6 05/11] rc: Add HDMI CEC protoctol handling

2015-05-04 Thread Kamil Debski
Add handling of remote control events coming from the HDMI CEC bus.
This patch includes a new keymap that maps values found in the CEC
messages to the keys pressed and released. Also, a new protocol has
been added to the core.

Signed-off-by: Kamil Debski 
---
 drivers/media/rc/keymaps/Makefile |1 +
 drivers/media/rc/keymaps/rc-cec.c |  144 +
 drivers/media/rc/rc-main.c|1 +
 include/media/rc-core.h   |1 +
 include/media/rc-map.h|5 +-
 5 files changed, 151 insertions(+), 1 deletion(-)
 create mode 100644 drivers/media/rc/keymaps/rc-cec.c

diff --git a/drivers/media/rc/keymaps/Makefile 
b/drivers/media/rc/keymaps/Makefile
index abf6079..56f10d6 100644
--- a/drivers/media/rc/keymaps/Makefile
+++ b/drivers/media/rc/keymaps/Makefile
@@ -18,6 +18,7 @@ obj-$(CONFIG_RC_MAP) += rc-adstech-dvb-t-pci.o \
rc-behold.o \
rc-behold-columbus.o \
rc-budget-ci-old.o \
+   rc-cec.o \
rc-cinergy-1400.o \
rc-cinergy.o \
rc-delock-61959.o \
diff --git a/drivers/media/rc/keymaps/rc-cec.c 
b/drivers/media/rc/keymaps/rc-cec.c
new file mode 100644
index 000..cc5b318
--- /dev/null
+++ b/drivers/media/rc/keymaps/rc-cec.c
@@ -0,0 +1,144 @@
+/* Keytable for the CEC remote control
+ *
+ * Copyright (c) 2015 by Kamil Debski
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include 
+#include 
+
+/* CEC Spec "High-Definition Multimedia Interface Specification" can be 
obtained
+ * here: http://xtreamerdev.googlecode.com/files/CEC_Specs.pdf
+ * The list of control codes is listed in Table 27: User Control Codes p. 95 */
+
+static struct rc_map_table cec[] = {
+   { 0x00, KEY_OK },
+   { 0x01, KEY_UP },
+   { 0x02, KEY_DOWN },
+   { 0x03, KEY_LEFT },
+   { 0x04, KEY_RIGHT },
+   { 0x05, KEY_RIGHT_UP },
+   { 0x06, KEY_RIGHT_DOWN },
+   { 0x07, KEY_LEFT_UP },
+   { 0x08, KEY_LEFT_DOWN },
+   { 0x09, KEY_CONTEXT_MENU }, /* CEC Spec: Root Menu - see Note 2 */
+   /* Note 2: This is the initial display that a device shows. It is
+* device-dependent and can be, for example, a contents menu, setup
+* menu, favorite menu or other menu. The actual menu displayed
+* may also depend on the device’s current state. */
+   { 0x0a, KEY_SETUP },
+   { 0x0b, KEY_MENU }, /* CEC Spec: Contents Menu */
+   { 0x0c, KEY_FAVORITES }, /* CEC Spec: Favorite Menu */
+   { 0x0d, KEY_EXIT },
+   /* 0x0e-0x1f: Reserved */
+   /* 0x20-0x29: Keys 0 to 9 */
+   { 0x20, KEY_NUMERIC_0 },
+   { 0x21, KEY_NUMERIC_1 },
+   { 0x22, KEY_NUMERIC_2 },
+   { 0x23, KEY_NUMERIC_3 },
+   { 0x24, KEY_NUMERIC_4 },
+   { 0x25, KEY_NUMERIC_5 },
+   { 0x26, KEY_NUMERIC_6 },
+   { 0x27, KEY_NUMERIC_7 },
+   { 0x28, KEY_NUMERIC_8 },
+   { 0x29, KEY_NUMERIC_9 },
+   { 0x2a, KEY_DOT },
+   { 0x2b, KEY_ENTER },
+   { 0x2c, KEY_CLEAR },
+   /* 0x2d-0x2e: Reserved */
+   { 0x2f, KEY_NEXT_FAVORITE }, /* CEC Spec: Next Favorite */
+   { 0x30, KEY_CHANNELUP },
+   { 0x31, KEY_CHANNELDOWN },
+   { 0x32, KEY_PREVIOUS }, /* CEC Spec: Previous Channel */
+   { 0x33, KEY_SOUND }, /* CEC Spec: Sound Select */
+   { 0x34, KEY_VIDEO }, /* 0x34: CEC Spec: Input Select */
+   { 0x35, KEY_INFO }, /* CEC Spec: Display Information */
+   { 0x36, KEY_HELP },
+   { 0x37, KEY_PAGEUP },
+   { 0x38, KEY_PAGEDOWN },
+   /* 0x39-0x3f: Reserved */
+   { 0x40, KEY_POWER },
+   { 0x41, KEY_VOLUMEUP },
+   { 0x42, KEY_VOLUMEDOWN },
+   { 0x43, KEY_MUTE },
+   { 0x44, KEY_PLAY },
+   { 0x45, KEY_STOP },
+   { 0x46, KEY_PAUSE },
+   { 0x47, KEY_RECORD },
+   { 0x48, KEY_REWIND },
+   { 0x49, KEY_FASTFORWARD },
+   { 0x4a, KEY_EJECTCD }, /* CEC Spec: Eject */
+   { 0x4b, KEY_FORWARD },
+   { 0x4c, KEY_BACK },
+   { 0x4d, KEY_STOP_RECORD }, /* CEC Spec: Stop-Record */
+   { 0x4e, KEY_PAUSE_RECORD }, /* CEC Spec: Pause-Record */
+   /* 0x4f: Reserved */
+   { 0x50, KEY_ANGLE },
+   { 0x51, KEY_TV2 },
+   { 0x52, KEY_VOD }, /* CEC Spec: Video on Demand */
+   { 0x53, KEY_EPG },
+   { 0x54, KEY_TIME }, /* CEC Spec: Timer */
+   { 0x55, KEY_CONFIG },
+   /* 0x56-0x5f: Reserved */
+   { 0x60, KEY_PLAY }, /* CEC Spec: Play Function */
+   { 0x6024, KEY_PLAY },
+   { 0x6020, KEY_PAUSE },
+   { 0x61, KEY_PLAYPAUSE }, /* CEC Spec: Pause-Play Function */
+   { 0x62, KEY_RECORD }, /* Spec: Record Function */
+   { 0x63, KEY_PAUSE }, /* CEC Spec: Pause-Record Function */
+   

[PATCH v6 06/11] cec: add HDMI CEC framework

2015-05-04 Thread Kamil Debski
From: Hans Verkuil 

The added HDMI CEC framework provides a generic kernel interface for
HDMI CEC devices.

Signed-off-by: Hans Verkuil 
[k.debski at samsung.com: Merged CEC Updates commit by Hans Verkuil]
[k.debski at samsung.com: Merged Update author commit by Hans Verkuil]
[k.debski at samsung.com: change kthread handling when setting logical
address]
[k.debski at samsung.com: code cleanup and fixes]
[k.debski at samsung.com: add missing CEC commands to match spec]
[k.debski at samsung.com: add RC framework support]
[k.debski at samsung.com: move and edit documentation]
[k.debski at samsung.com: add vendor id reporting]
[k.debski at samsung.com: add possibility to clear assigned logical
addresses]
[k.debski at samsung.com: documentation fixes, clenaup and expansion]
[k.debski at samsung.com: reorder of API structs and add reserved fields]
[k.debski at samsung.com: fix handling of events and fix 32/64bit timespec
problem]
[k.debski at samsung.com: add cec.h to include/uapi/linux/Kbuild]
[k.debski at samsung.com: add sequence number handling]
[k.debski at samsung.com: add passthrough mode]
[k.debski at samsung.com: fix CEC defines, add missing CEC 2.0 commands]
[k.debski at samsung.com: add DocBook documentation by Hans Verkuil, with
minor additions]
Signed-off-by: Kamil Debski 
---
 Documentation/cec.txt |  165 +++
 drivers/media/Kconfig |6 +
 drivers/media/Makefile|2 +
 drivers/media/cec.c   | 1191 +
 include/media/cec.h   |  142 ++
 include/uapi/linux/Kbuild |1 +
 include/uapi/linux/cec.h  |  332 +
 7 files changed, 1839 insertions(+)
 create mode 100644 Documentation/cec.txt
 create mode 100644 drivers/media/cec.c
 create mode 100644 include/media/cec.h
 create mode 100644 include/uapi/linux/cec.h

diff --git a/Documentation/cec.txt b/Documentation/cec.txt
new file mode 100644
index 000..a52017c2
--- /dev/null
+++ b/Documentation/cec.txt
@@ -0,0 +1,165 @@
+CEC Kernel Support
+==
+
+The CEC framework provides a unified kernel interface for use with HDMI CEC
+hardware. It is designed to handle a multiple variants of hardware. Adding to
+the flexibility of the framework it enables to set which parts of the CEC
+protocol processing is handled by the hardware, by the driver and by the
+userspace application.
+
+
+The CEC Protocol
+
+
+The CEC protocol enables consumer electronic devices to communicate with each
+other through the HDMI connection. The protocol uses logical addresses in the
+communication. The logical address is strictly connected with the functionality
+provided by the device. The TV acting as the communication hub is always
+assigned address 0. The physical address is determined by the physical
+connection between devices.
+
+The protocol enables control of compatible devices with a single remote.
+Synchronous power on/standby, instant playback with changing the content source
+on the TV.
+
+The Kernel Interface
+
+
+CEC Adapter
+---
+
+#define CEC_LOG_ADDR_INVALID 0xff
+
+/* The maximum number of logical addresses one device can be assigned to.
+ * The CEC 2.0 spec allows for only 2 logical addresses at the moment. The
+ * Analog Devices CEC hardware supports 3. So let's go wild and go for 4. */
+#define CEC_MAX_LOG_ADDRS 4
+
+/* The "Primary Device Type" */
+#define CEC_PRIM_DEVTYPE_TV0
+#define CEC_PRIM_DEVTYPE_RECORD1
+#define CEC_PRIM_DEVTYPE_TUNER 3
+#define CEC_PRIM_DEVTYPE_PLAYBACK  4
+#define CEC_PRIM_DEVTYPE_AUDIOSYSTEM   5
+#define CEC_PRIM_DEVTYPE_SWITCH6
+#define CEC_PRIM_DEVTYPE_VIDEOPROC 7
+
+/* The "All Device Types" flags (CEC 2.0) */
+#define CEC_FL_ALL_DEVTYPE_TV  (1 << 7)
+#define CEC_FL_ALL_DEVTYPE_RECORD  (1 << 6)
+#define CEC_FL_ALL_DEVTYPE_TUNER   (1 << 5)
+#define CEC_FL_ALL_DEVTYPE_PLAYBACK(1 << 4)
+#define CEC_FL_ALL_DEVTYPE_AUDIOSYSTEM (1 << 3)
+#define CEC_FL_ALL_DEVTYPE_SWITCH  (1 << 2)
+/* And if you wondering what happened to VIDEOPROC devices: those should
+ * be mapped to a SWITCH. */
+
+/* The logical address types that the CEC device wants to claim */
+#define CEC_LOG_ADDR_TYPE_TV   0
+#define CEC_LOG_ADDR_TYPE_RECORD   1
+#define CEC_LOG_ADDR_TYPE_TUNER2
+#define CEC_LOG_ADDR_TYPE_PLAYBACK 3
+#define CEC_LOG_ADDR_TYPE_AUDIOSYSTEM  4
+#define CEC_LOG_ADDR_TYPE_SPECIFIC 5
+#define CEC_LOG_ADDR_TYPE_UNREGISTERED 6
+/* Switches should use UNREGISTERED.
+ * Video processors should use SPECIFIC. */
+
+/* The CEC version */
+#define CEC_VERSION_1_4B   5
+#define CEC_VERSION_2_06
+
+struct cec_adapter {
+   /* internal fields removed */
+
+   u16 phys_addr;
+   u32 capabilities;
+   u8 version;
+   u8 num_log_addrs;
+   u8 prim_device[CEC_MAX_LOG_ADDRS];
+   u8 log_addr_type[CEC_MAX_LOG_ADDRS];
+   u8 log_addr[CEC_MAX_

[PATCH v6 07/11] DocBook/media: add CEC documentation

2015-05-04 Thread Kamil Debski
From: Hans Verkuil 

Add DocBook documentation for the CEC API.

Signed-off-by: Hans Verkuil 
[k.debski at samsung.com: add documentation for passthrough mode]
[k.debski at samsung.com: minor fixes and change of reserved field sizes]
Signed-off-by: Kamil Debski 
---
 Documentation/DocBook/media/Makefile   |4 +-
 Documentation/DocBook/media/v4l/biblio.xml |   10 +
 Documentation/DocBook/media/v4l/cec-api.xml|   74 ++
 Documentation/DocBook/media/v4l/cec-func-close.xml |   59 +
 Documentation/DocBook/media/v4l/cec-func-ioctl.xml |   73 ++
 Documentation/DocBook/media/v4l/cec-func-open.xml  |   94 +++
 Documentation/DocBook/media/v4l/cec-func-poll.xml  |   89 +++
 .../DocBook/media/v4l/cec-ioc-g-adap-log-addrs.xml |  275 
 .../DocBook/media/v4l/cec-ioc-g-adap-phys-addr.xml |   78 ++
 .../DocBook/media/v4l/cec-ioc-g-adap-state.xml |   87 +++
 Documentation/DocBook/media/v4l/cec-ioc-g-caps.xml |  173 
 .../DocBook/media/v4l/cec-ioc-g-event.xml  |  125 +
 .../DocBook/media/v4l/cec-ioc-g-passthrough.xml|   88 +++
 .../DocBook/media/v4l/cec-ioc-g-vendor-id.xml  |   70 +
 .../DocBook/media/v4l/cec-ioc-receive.xml  |  185 +
 Documentation/DocBook/media_api.tmpl   |6 +-
 16 files changed, 1487 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/DocBook/media/v4l/cec-api.xml
 create mode 100644 Documentation/DocBook/media/v4l/cec-func-close.xml
 create mode 100644 Documentation/DocBook/media/v4l/cec-func-ioctl.xml
 create mode 100644 Documentation/DocBook/media/v4l/cec-func-open.xml
 create mode 100644 Documentation/DocBook/media/v4l/cec-func-poll.xml
 create mode 100644 Documentation/DocBook/media/v4l/cec-ioc-g-adap-log-addrs.xml
 create mode 100644 Documentation/DocBook/media/v4l/cec-ioc-g-adap-phys-addr.xml
 create mode 100644 Documentation/DocBook/media/v4l/cec-ioc-g-adap-state.xml
 create mode 100644 Documentation/DocBook/media/v4l/cec-ioc-g-caps.xml
 create mode 100644 Documentation/DocBook/media/v4l/cec-ioc-g-event.xml
 create mode 100644 Documentation/DocBook/media/v4l/cec-ioc-g-passthrough.xml
 create mode 100644 Documentation/DocBook/media/v4l/cec-ioc-g-vendor-id.xml
 create mode 100644 Documentation/DocBook/media/v4l/cec-ioc-receive.xml

diff --git a/Documentation/DocBook/media/Makefile 
b/Documentation/DocBook/media/Makefile
index 8bf7c61..9650332 100644
--- a/Documentation/DocBook/media/Makefile
+++ b/Documentation/DocBook/media/Makefile
@@ -64,6 +64,7 @@ IOCTLS = \
$(shell perl -ne 'print "$$1 " if /\#define\s+([A-Z][^\s]+)\s+_IO/' 
$(srctree)/include/uapi/linux/dvb/net.h) \
$(shell perl -ne 'print "$$1 " if /\#define\s+([^\s]+)\s+_IO/' 
$(srctree)/include/uapi/linux/dvb/video.h) \
$(shell perl -ne 'print "$$1 " if /\#define\s+([^\s]+)\s+_IO/' 
$(srctree)/include/uapi/linux/media.h) \
+   $(shell perl -ne 'print "$$1 " if /\#define\s+([^\s]+)\s+_IO/' 
$(srctree)/include/uapi/linux/cec.h) \
$(shell perl -ne 'print "$$1 " if /\#define\s+([^\s]+)\s+_IO/' 
$(srctree)/include/uapi/linux/v4l2-subdev.h) \
VIDIOC_SUBDEV_G_FRAME_INTERVAL \
VIDIOC_SUBDEV_S_FRAME_INTERVAL \
@@ -98,6 +99,7 @@ STRUCTS = \
$(shell perl -ne 'print "$$1 " if (/^struct\s+([A-Z][^\s]+)\s+/)' 
$(srctree)/include/uapi/linux/dvb/net.h) \
$(shell perl -ne 'print "$$1 " if (/^struct\s+([^\s]+)\s+/)' 
$(srctree)/include/uapi/linux/dvb/video.h) \
$(shell perl -ne 'print "$$1 " if /^struct\s+([^\s]+)\s+/' 
$(srctree)/include/uapi/linux/media.h) \
+   $(shell perl -ne 'print "$$1 " if /^struct\s+([^\s]+)\s+/' 
$(srctree)/include/uapi/linux/cec.h) \
$(shell perl -ne 'print "$$1 " if /^struct\s+([^\s]+)\s+/' 
$(srctree)/include/uapi/linux/v4l2-subdev.h) \
$(shell perl -ne 'print "$$1 " if /^struct\s+([^\s]+)\s+/' 
$(srctree)/include/uapi/linux/v4l2-mediabus.h)

@@ -300,7 +302,7 @@ $(MEDIA_OBJ_DIR)/media-entities.tmpl: 
$(MEDIA_OBJ_DIR)/v4l2.xml
@(  \
for ident in $(IOCTLS) ; do \
  entity=`echo $$ident | tr _ -` ;  \
- id=`grep "$$ident" $(MEDIA_OBJ_DIR)/vidioc-*.xml 
$(MEDIA_OBJ_DIR)/media-ioc-*.xml | sed -r s,"^.*/(.*).xml.*","\1",` ; \
+ id=`grep "$$ident" $(MEDIA_OBJ_DIR)/vidioc-*.xml 
$(MEDIA_OBJ_DIR)/media-ioc-*.xml $(MEDIA_OBJ_DIR)/cec-ioc-*.xml | sed -r 
s,"^.*/(.*).xml.*","\1",` ; \
  echo "$$ident\">" \
  >>$@ ;\
diff --git a/Documentation/DocBook/media/v4l/biblio.xml 
b/Documentation/DocBook/media/v4l/biblio.xml
index fdee6b3..bed940b 100644
--- a/Documentation/DocBook/media/v4l/biblio.xml
+++ b/Documentation/DocBook/media/v4l/biblio.xml
@@ -324,6 +324,16 @@ in the frequency range from 87,5 to 108,0 MHz
   Specification Version 1.4a

[PATCH v6 08/11] v4l2-subdev: add HDMI CEC ops

2015-05-04 Thread Kamil Debski
From: Hans Verkuil 

Add callbacks to the v4l2_subdev_video_ops.

Signed-off-by: Hans Verkuil 
[k.debski at samsung.com: Merged changes from CEC Updates commit by Hans 
Verkuil]
Signed-off-by: Kamil Debski 
---
 include/media/v4l2-subdev.h |8 
 1 file changed, 8 insertions(+)

diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index 2f0a345..9323e10 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -40,6 +40,9 @@
 #define V4L2_SUBDEV_IR_TX_NOTIFY   _IOW('v', 1, u32)
 #define V4L2_SUBDEV_IR_TX_FIFO_SERVICE_REQ 0x0001

+#define V4L2_SUBDEV_CEC_TX_DONE_IOW('v', 2, u32)
+#define V4L2_SUBDEV_CEC_RX_MSG _IOW('v', 3, struct cec_msg)
+
 struct v4l2_device;
 struct v4l2_ctrl_handler;
 struct v4l2_event_subscription;
@@ -48,6 +51,7 @@ struct v4l2_subdev;
 struct v4l2_subdev_fh;
 struct tuner_setup;
 struct v4l2_mbus_frame_desc;
+struct cec_msg;

 /* decode_vbi_line */
 struct v4l2_decode_vbi_line {
@@ -352,6 +356,10 @@ struct v4l2_subdev_video_ops {
 const struct v4l2_mbus_config *cfg);
int (*s_rx_buffer)(struct v4l2_subdev *sd, void *buf,
   unsigned int *size);
+   int (*cec_enable)(struct v4l2_subdev *sd, bool enable);
+   int (*cec_log_addr)(struct v4l2_subdev *sd, u8 logical_addr);
+   int (*cec_transmit)(struct v4l2_subdev *sd, struct cec_msg *msg);
+   void (*cec_transmit_timed_out)(struct v4l2_subdev *sd);
 };

 /*
-- 
1.7.9.5



[PATCH v6 09/11] cec: adv7604: add cec support.

2015-05-04 Thread Kamil Debski
From: Hans Verkuil 

Add CEC support to the adv7604 driver.

Signed-off-by: Hans Verkuil 
[k.debski at samsung.com: Merged changes from CEC Updates commit by Hans 
Verkuil]
[k.debski at samsung.com: add missing methods cec/io_write_and_or]
[k.debski at samsung.com: change adv7604 to adv76xx in added functions]
Signed-off-by: Kamil Debski 
---
 drivers/media/i2c/adv7604.c |  207 ++-
 1 file changed, 206 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
index 60ffcf0..4921276 100644
--- a/drivers/media/i2c/adv7604.c
+++ b/drivers/media/i2c/adv7604.c
@@ -38,6 +38,7 @@
 #include 

 #include 
+#include 
 #include 
 #include 
 #include 
@@ -77,6 +78,8 @@ MODULE_LICENSE("GPL");

 #define ADV76XX_OP_SWAP_CB_CR  (1 << 0)

+#define ADV76XX_MAX_ADDRS (3)
+
 enum adv76xx_type {
ADV7604,
ADV7611,
@@ -159,6 +162,10 @@ struct adv76xx_state {
u16 spa_port_a[2];
struct v4l2_fract aspect_ratio;
u32 rgb_quantization_range;
+   u8   cec_addr[ADV76XX_MAX_ADDRS];
+   u8   cec_valid_addrs;
+   bool cec_enabled_adap;
+
struct workqueue_struct *work_queues;
struct delayed_work delayed_work_enable_hotplug;
bool restart_stdi_once;
@@ -424,7 +431,15 @@ static inline int io_write(struct v4l2_subdev *sd, u8 reg, 
u8 val)
return adv_smbus_write_byte_data(state, ADV76XX_PAGE_IO, reg, val);
 }

-static inline int io_write_clr_set(struct v4l2_subdev *sd, u8 reg, u8 mask, u8 
val)
+static inline int io_write_and_or(struct v4l2_subdev *sd, u8 reg, u8 mask,
+ u8 val)
+{
+   return io_write(sd, reg, (io_read(sd, reg) & mask) | val);
+}
+
+
+static inline int io_write_clr_set(struct v4l2_subdev *sd, u8 reg, u8 mask,
+  u8 val)
 {
return io_write(sd, reg, (io_read(sd, reg) & ~mask) | val);
 }
@@ -457,6 +472,12 @@ static inline int cec_write(struct v4l2_subdev *sd, u8 
reg, u8 val)
return adv_smbus_write_byte_data(state, ADV76XX_PAGE_CEC, reg, val);
 }

+static inline int cec_write_and_or(struct v4l2_subdev *sd, u8 reg, u8 mask,
+  u8 val)
+{
+   return cec_write(sd, reg, (cec_read(sd, reg) & mask) | val);
+}
+
 static inline int infoframe_read(struct v4l2_subdev *sd, u8 reg)
 {
struct adv76xx_state *state = to_state(sd);
@@ -1865,6 +1886,183 @@ static int adv76xx_set_format(struct v4l2_subdev *sd,
return 0;
 }

+static void adv76xx_cec_tx_raw_status(struct v4l2_subdev *sd, u8 tx_raw_status)
+{
+   if ((cec_read(sd, 0x11) & 0x01) == 0) {
+   v4l2_dbg(1, debug, sd, "%s: tx raw: tx disabled\n", __func__);
+   return;
+   }
+
+   if (tx_raw_status & 0x02) {
+   v4l2_dbg(1, debug, sd, "%s: tx raw: arbitration lost\n",
+__func__);
+   v4l2_subdev_notify(sd, V4L2_SUBDEV_CEC_TX_DONE,
+  (void *)CEC_TX_STATUS_ARB_LOST);
+   return;
+   }
+   if (tx_raw_status & 0x04) {
+   v4l2_dbg(1, debug, sd, "%s: tx raw: retry failed\n", __func__);
+   v4l2_subdev_notify(sd, V4L2_SUBDEV_CEC_TX_DONE,
+  (void *)CEC_TX_STATUS_RETRY_TIMEOUT);
+   return;
+   }
+   if (tx_raw_status & 0x01) {
+   v4l2_dbg(1, debug, sd, "%s: tx raw: ready ok\n", __func__);
+   v4l2_subdev_notify(sd, V4L2_SUBDEV_CEC_TX_DONE,
+  (void *)CEC_TX_STATUS_OK);
+   return;
+   }
+}
+
+static void adv76xx_cec_isr(struct v4l2_subdev *sd, bool *handled)
+{
+   struct cec_msg msg;
+   u8 cec_irq;
+
+   /* cec controller */
+   cec_irq = io_read(sd, 0x4d) & 0x0f;
+   if (!cec_irq)
+   return;
+
+   v4l2_dbg(1, debug, sd, "%s: cec: irq 0x%x\n", __func__, cec_irq);
+   adv76xx_cec_tx_raw_status(sd, cec_irq);
+   if (cec_irq & 0x08) {
+   msg.len = cec_read(sd, 0x25) & 0x1f;
+   if (msg.len > 16)
+   msg.len = 16;
+
+   if (msg.len) {
+   u8 i;
+
+   for (i = 0; i < msg.len; i++)
+   msg.msg[i] = cec_read(sd, i + 0x15);
+   cec_write(sd, 0x26, 0x01); /* re-enable rx */
+   v4l2_subdev_notify(sd, V4L2_SUBDEV_CEC_RX_MSG, &msg);
+   }
+   }
+
+   /* note: the bit order is swapped between 0x4d and 0x4e */
+   cec_irq = ((cec_irq & 0x08) >> 3) | ((cec_irq & 0x04) >> 1) |
+ ((cec_irq & 0x02) << 1) | ((cec_irq & 0x01) << 3);
+   io_write(sd, 0x4e, cec_irq);
+
+   if (handled)
+   *handled = true;
+}
+
+static int adv76xx_cec_enable(struct v4l2_subdev *sd, bool enable)
+{
+   struct adv76xx_state *state = to_state(sd);
+
+   if (!stat

[PATCH v6 10/11] cec: adv7511: add cec support.

2015-05-04 Thread Kamil Debski
From: Hans Verkuil 

Add CEC support to the adv7511 driver.

Signed-off-by: Hans Verkuil 
[k.debski at samsung.com: Merged changes from CEC Updates commit by Hans 
Verkuil]
Signed-off-by: Kamil Debski 
---
 drivers/media/i2c/adv7511.c |  347 ++-
 include/media/adv7511.h |6 +-
 2 files changed, 343 insertions(+), 10 deletions(-)

diff --git a/drivers/media/i2c/adv7511.c b/drivers/media/i2c/adv7511.c
index 12d9320..d56e110 100644
--- a/drivers/media/i2c/adv7511.c
+++ b/drivers/media/i2c/adv7511.c
@@ -33,6 +33,7 @@
 #include 
 #include 
 #include 
+#include 

 static int debug;
 module_param(debug, int, 0644);
@@ -91,6 +92,12 @@ struct adv7511_state {
int chip_revision;
uint8_t i2c_edid_addr;
uint8_t i2c_cec_addr;
+
+   struct i2c_client *i2c_cec;
+   u8   cec_addr[3];
+   u8   cec_valid_addrs;
+   bool cec_enabled_adap;
+
/* Is the adv7511 powered on? */
bool power_on;
/* Did we receive hotplug and rx-sense signals? */
@@ -222,7 +229,7 @@ static int adv_smbus_read_i2c_block_data(struct i2c_client 
*client,
return ret;
 }

-static inline void adv7511_edid_rd(struct v4l2_subdev *sd, uint16_t len, 
uint8_t *buf)
+static void adv7511_edid_rd(struct v4l2_subdev *sd, uint16_t len, uint8_t *buf)
 {
struct adv7511_state *state = get_adv7511_state(sd);
int i;
@@ -237,6 +244,34 @@ static inline void adv7511_edid_rd(struct v4l2_subdev *sd, 
uint16_t len, uint8_t
v4l2_err(sd, "%s: i2c read error\n", __func__);
 }

+static inline int cec_read(struct v4l2_subdev *sd, u8 reg)
+{
+   struct adv7511_state *state = get_adv7511_state(sd);
+
+   return i2c_smbus_read_byte_data(state->i2c_cec, reg);
+}
+
+static int cec_write(struct v4l2_subdev *sd, u8 reg, u8 val)
+{
+   struct adv7511_state *state = get_adv7511_state(sd);
+   int ret;
+   int i;
+
+   for (i = 0; i < 3; i++) {
+   ret = i2c_smbus_write_byte_data(state->i2c_cec, reg, val);
+   if (ret == 0)
+   return 0;
+   }
+   v4l2_err(sd, "%s: I2C Write Problem\n", __func__);
+   return ret;
+}
+
+static inline int cec_write_and_or(struct v4l2_subdev *sd, u8 reg, u8 mask,
+  u8 val)
+{
+   return cec_write(sd, reg, (cec_read(sd, reg) & mask) | val);
+}
+
 static inline bool adv7511_have_hotplug(struct v4l2_subdev *sd)
 {
return adv7511_rd(sd, 0x42) & MASK_ADV7511_HPD_DETECT;
@@ -381,16 +416,28 @@ static const struct v4l2_ctrl_ops adv7511_ctrl_ops = {
 #ifdef CONFIG_VIDEO_ADV_DEBUG
 static void adv7511_inv_register(struct v4l2_subdev *sd)
 {
+   struct adv7511_state *state = get_adv7511_state(sd);
+
v4l2_info(sd, "0x000-0x0ff: Main Map\n");
+   if (state->i2c_cec)
+   v4l2_info(sd, "0x100-0x1ff: CEC Map\n");
 }

 static int adv7511_g_register(struct v4l2_subdev *sd, struct v4l2_dbg_register 
*reg)
 {
+   struct adv7511_state *state = get_adv7511_state(sd);
+
reg->size = 1;
switch (reg->reg >> 8) {
case 0:
reg->val = adv7511_rd(sd, reg->reg & 0xff);
break;
+   case 1:
+   if (state->i2c_cec) {
+   reg->val = cec_read(sd, reg->reg & 0xff);
+   break;
+   }
+   /* fall through */
default:
v4l2_info(sd, "Register %03llx not supported\n", reg->reg);
adv7511_inv_register(sd);
@@ -401,10 +448,18 @@ static int adv7511_g_register(struct v4l2_subdev *sd, 
struct v4l2_dbg_register *

 static int adv7511_s_register(struct v4l2_subdev *sd, const struct 
v4l2_dbg_register *reg)
 {
+   struct adv7511_state *state = get_adv7511_state(sd);
+
switch (reg->reg >> 8) {
case 0:
adv7511_wr(sd, reg->reg & 0xff, reg->val & 0xff);
break;
+   case 1:
+   if (state->i2c_cec) {
+   cec_write(sd, reg->reg & 0xff, reg->val & 0xff);
+   break;
+   }
+   /* fall through */
default:
v4l2_info(sd, "Register %03llx not supported\n", reg->reg);
adv7511_inv_register(sd);
@@ -418,6 +473,7 @@ static int adv7511_log_status(struct v4l2_subdev *sd)
 {
struct adv7511_state *state = get_adv7511_state(sd);
struct adv7511_state_edid *edid = &state->edid;
+   int i;

static const char * const states[] = {
"in reset",
@@ -486,7 +542,21 @@ static int adv7511_log_status(struct v4l2_subdev *sd)
else
v4l2_info(sd, "no timings set\n");
v4l2_info(sd, "i2c edid addr: 0x%x\n", state->i2c_edid_addr);
+
+   if (state->i2c_cec == NULL)
+   return 0;
+
v4l2_info(sd, "i2c cec addr: 0x%x\n", state->i2c_cec_addr);
+
+   if (cec_read(sd, 0x4e) & 0x01) {
+   v4l2_info(sd, "cec: en

[PATCH v6 11/11] cec: s5p-cec: Add s5p-cec driver

2015-05-04 Thread Kamil Debski
Add CEC interface driver present in the Samsung Exynos range of
SoCs.

The following files were based on work by SangPil Moon:
- exynos_hdmi_cec.h
- exynos_hdmi_cecctl.c

Signed-off-by: Kamil Debski 
---
 .../devicetree/bindings/media/s5p-cec.txt  |   33 +++
 drivers/media/platform/Kconfig |   10 +
 drivers/media/platform/Makefile|1 +
 drivers/media/platform/s5p-cec/Makefile|4 +
 drivers/media/platform/s5p-cec/exynos_hdmi_cec.h   |   37 +++
 .../media/platform/s5p-cec/exynos_hdmi_cecctrl.c   |  208 ++
 drivers/media/platform/s5p-cec/regs-cec.h  |   96 +++
 drivers/media/platform/s5p-cec/s5p_cec.c   |  283 
 drivers/media/platform/s5p-cec/s5p_cec.h   |   76 ++
 9 files changed, 748 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/s5p-cec.txt
 create mode 100644 drivers/media/platform/s5p-cec/Makefile
 create mode 100644 drivers/media/platform/s5p-cec/exynos_hdmi_cec.h
 create mode 100644 drivers/media/platform/s5p-cec/exynos_hdmi_cecctrl.c
 create mode 100644 drivers/media/platform/s5p-cec/regs-cec.h
 create mode 100644 drivers/media/platform/s5p-cec/s5p_cec.c
 create mode 100644 drivers/media/platform/s5p-cec/s5p_cec.h

diff --git a/Documentation/devicetree/bindings/media/s5p-cec.txt 
b/Documentation/devicetree/bindings/media/s5p-cec.txt
new file mode 100644
index 000..97ca664
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/s5p-cec.txt
@@ -0,0 +1,33 @@
+* Samsung HDMI CEC driver
+
+The HDMI CEC module is present is Samsung SoCs and its purpose is to
+handle communication between HDMI connected devices over the CEC bus.
+
+Required properties:
+  - compatible : value should be follwoing
+   "samsung,s5p-cec"
+
+  - reg : Physical base address of the IP registers and length of memory
+ mapped region.
+
+  - interrupts : HDMI CEC interrupt number to the CPU.
+  - clocks : from common clock binding: handle to HDMI CEC clock.
+  - clock-names : from common clock binding: must contain "hdmicec",
+ corresponding to entry in the clocks property.
+  - samsung,syscon-phandle - phandle to the PMU system controller
+
+Example:
+
+hdmicec: cec at 100B {
+   compatible = "samsung,s5p-cec";
+   reg = <0x100B 0x200>;
+   interrupts = <0 114 0>;
+   clocks = <&clock CLK_HDMI_CEC>;
+   clock-names = "hdmicec";
+   samsung,syscon-phandle = <&pmu_system_controller>;
+   pinctrl-names = "default";
+   pinctrl-0 = <&hdmi_cec>;
+   status = "okay";
+};
+
+
diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index 421f531..203bc06 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -157,6 +157,16 @@ config VIDEO_MEM2MEM_DEINTERLACE
help
Generic deinterlacing V4L2 driver.

+config VIDEO_SAMSUNG_S5P_CEC
+   tristate "Samsung S5P CEC driver"
+   depends on CEC && VIDEO_DEV && VIDEO_V4L2 && (PLAT_S5P || ARCH_EXYNOS)
+   default n
+   ---help---
+ This is a driver for Samsung S5P HDMI CEC interface. It uses the
+ generic CEC framework interface.
+ CEC bus is present in the HDMI connector and enables communication
+ between compatible devices.
+
 config VIDEO_SAMSUNG_S5P_G2D
tristate "Samsung S5P and EXYNOS4 G2D 2d graphics accelerator driver"
depends on VIDEO_DEV && VIDEO_V4L2
diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
index 8f85561..f96c9a3 100644
--- a/drivers/media/platform/Makefile
+++ b/drivers/media/platform/Makefile
@@ -27,6 +27,7 @@ obj-$(CONFIG_VIDEO_MEM2MEM_DEINTERLACE)   += 
m2m-deinterlace.o

 obj-$(CONFIG_VIDEO_S3C_CAMIF)  += s3c-camif/
 obj-$(CONFIG_VIDEO_SAMSUNG_EXYNOS4_IS) += exynos4-is/
+obj-$(CONFIG_VIDEO_SAMSUNG_S5P_CEC)+= s5p-cec/
 obj-$(CONFIG_VIDEO_SAMSUNG_S5P_JPEG)   += s5p-jpeg/
 obj-$(CONFIG_VIDEO_SAMSUNG_S5P_MFC)+= s5p-mfc/
 obj-$(CONFIG_VIDEO_SAMSUNG_S5P_TV) += s5p-tv/
diff --git a/drivers/media/platform/s5p-cec/Makefile 
b/drivers/media/platform/s5p-cec/Makefile
new file mode 100644
index 000..7f84226
--- /dev/null
+++ b/drivers/media/platform/s5p-cec/Makefile
@@ -0,0 +1,4 @@
+obj-$(CONFIG_VIDEO_SAMSUNG_S5P_CEC)+= s5p-cec.o
+s5p-cec-y += s5p_cec.o exynos_hdmi_cecctrl.o
+
+
diff --git a/drivers/media/platform/s5p-cec/exynos_hdmi_cec.h 
b/drivers/media/platform/s5p-cec/exynos_hdmi_cec.h
new file mode 100644
index 000..d008695
--- /dev/null
+++ b/drivers/media/platform/s5p-cec/exynos_hdmi_cec.h
@@ -0,0 +1,37 @@
+/* drivers/media/platform/s5p-cec/exynos_hdmi_cec.h
+ *
+ * Copyright (c) 2010, 2014 Samsung Electronics
+ * http://www.samsung.com/
+ *
+ * Header file for interface of Samsung Exynos hdmi cec hardware
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public Licens

[PATCH v6 00/11] HDMI CEC framework

2015-05-04 Thread Kamil Debski
Hi,

Sorry, I missed the subject for this cover-letter. Added it in this reply.

Best wishes,
-- 
Kamil Debski
Samsung R&D Institute Poland


> -Original Message-
> From: Kamil Debski [mailto:k.debski at samsung.com]
> Sent: Monday, May 04, 2015 7:33 PM
> To: dri-devel at lists.freedesktop.org; linux-media at vger.kernel.org
> Cc: m.szyprowski at samsung.com; k.debski at samsung.com;
> mchehab at osg.samsung.com; hverkuil at xs4all.nl; kyungmin.park at 
> samsung.com;
> thomas at tommie-lie.de; sean at mess.org; dmitry.torokhov at gmail.com; 
> linux-
> input at vger.kernel.org; linux-samsung-soc at vger.kernel.org;
> lars at opdenkamp.eu
> Subject: [PATCH v6 00/11]
> 
> Hi,
> 
> The sixth version of this patchset addresses recent comments on the
> mailing list. Please see the changelog below for details.
> 
> Best wishes,
> Kamil Debski
> 
> Changes since v5
> 
> - drop struct cec_timeval in favour of a __u64 that keeps the timestamp
> in ns
> - remove userspace documentation from Documentation/cec.txt as
> userspace API
>   is described in the DocBook
> - add missing documentation for the passthrough mode to the DocBook
> - add information about the number of events that can be queued
> - fix misspelling of reply
> - fix behaviour of posting an event in cec_received_msg, such that the
> behaviour
>   is consistent with the documentation
> 
> Changes since v4
> 
> - add sequence numbering to transmitted messages
> - add sequence number handling to event hanlding
> - add passthrough mode
> - change reserved field sizes
> - fixed CEC version defines and addec CEC 2.0 commands
> - add DocBook documentation
> 
> Changes since v3
> 
> - remove the promiscuous mode
> - rewrite the devicetree patches
> - fixes, expansion and partial rewrite of the documentation
> - reorder of API structures and addition of reserved fields
> - use own struct to report time (32/64 bit safe)
> - fix of handling events
> - add cec.h to include/uapi/linux/Kbuild
> - fixes in the adv76xx driver (add missing methods, change adv7604 to
> adv76xx)
> - cleanup of debug messages in s5p-cec driver
> - remove non necessary claiming of a gpio in the s5p-cec driver
> - cleanup headers of the s5p-cec driver
> 
> Changes since v2
> ===-
> - added promiscuous mode
> - added new key codes to the input framework
> - add vendor ID reporting
> - add the possibility to clear assigned logical addresses
> - cleanup of the rc cec map
> 
> Changes since v1
> 
> - documentation edited and moved to the Documentation folder
> - added key up/down message handling
> - add missing CEC commands to the cec.h file
> 
> Background
> ==
> 
> The work on a common CEC framework was started over three years ago by
> Hans Verkuil. Unfortunately the work has stalled. As I have received
> the task of creating a driver for the CEC interface module present on
> the Exynos range of SoCs, I got in touch with Hans. He replied that the
> work stalled due to his lack of time.
> 
> Original RFC by Hans Verkuil/Martin Bugge
> =
> https://www.mail-archive.com/linux-media at vger.kernel.org/msg28735.html
> 
> 
> Hans Verkuil (5):
>   cec: add HDMI CEC framework
>   DocBook/media: add CEC documentation
>   v4l2-subdev: add HDMI CEC ops
>   cec: adv7604: add cec support.
>   cec: adv7511: add cec support.
> 
> Kamil Debski (6):
>   dts: exynos4*: add HDMI CEC pin definition to pinctrl
>   dts: exynos4: add node for the HDMI CEC device
>   dts: exynos4412-odroid*: enable the HDMI CEC device
>   HID: add HDMI CEC specific keycodes
>   rc: Add HDMI CEC protoctol handling
>   cec: s5p-cec: Add s5p-cec driver
> 
>  Documentation/DocBook/media/Makefile   |4 +-
>  Documentation/DocBook/media/v4l/biblio.xml |   10 +
>  Documentation/DocBook/media/v4l/cec-api.xml|   74 ++
>  Documentation/DocBook/media/v4l/cec-func-close.xml |   59 +
>  Documentation/DocBook/media/v4l/cec-func-ioctl.xml |   73 ++
>  Documentation/DocBook/media/v4l/cec-func-open.xml  |   94 ++
>  Documentation/DocBook/media/v4l/cec-func-poll.xml  |   89 ++
>  .../DocBook/media/v4l/cec-ioc-g-adap-log-addrs.xml |  275 +
>  .../DocBook/media/v4l/cec-ioc-g-adap-phys-addr.xml |   78 ++
>  .../DocBook/media/v4l/cec-ioc-g-adap-state.xml |   87 ++
>  Documentation/DocBook/media/v4l/cec-ioc-g-caps.xml |  173 +++
>  .../DocBook/media/v4l/cec-ioc-g-event.xml  |  125 ++
>  .../DocBook/media/v4l/cec-ioc-g-passthrough.xml|   88 ++
>  .../DocBook/media/v4l/cec-ioc-g-vendor-id.xml  |   70 ++
>  .../DocBook/media/v4l/cec-ioc-receive.xml  |  185 +++
>  Documentation/DocBook/media_api.tmpl   |6 +-
>  Documentation/cec.txt  |  165 +++
>  .../devicetree/bindings/media/s5p-cec.txt  |   33 +
>  arch/arm/boot/dts/exynos4.dtsi |   12 +
>  arch/arm/boot/dts/exynos4210-pinctrl.dtsi  

[PATCH v2] libgencec: Add userspace library for the generic CEC kernel interface

2015-05-04 Thread Kamil Debski
This is the first version of the libGenCEC library. It was designed to
act as an interface between the generic CEC kernel API and userspace
applications. It provides a simple interface for applications and an
example application that can be used to test the CEC functionality.

signed-off-by: Kamil Debski 
---
 AUTHORS  |1 +
 INSTALL  |9 +
 LICENSE  |  202 
 Makefile.am  |4 +
 README   |   22 ++
 configure.ac |   24 ++
 doc/index.html   |  345 +++
 examples/Makefile.am |4 +
 examples/cectest.c   |  631 ++
 include/gencec.h |  255 
 libgencec.pc.in  |   10 +
 src/Makefile.am  |4 +
 src/gencec.c |  445 +++
 13 files changed, 1956 insertions(+)
 create mode 100644 AUTHORS
 create mode 100644 INSTALL
 create mode 100644 LICENSE
 create mode 100644 Makefile.am
 create mode 100644 README
 create mode 100644 configure.ac
 create mode 100644 doc/index.html
 create mode 100644 examples/Makefile.am
 create mode 100644 examples/cectest.c
 create mode 100644 include/gencec.h
 create mode 100644 libgencec.pc.in
 create mode 100644 src/Makefile.am
 create mode 100644 src/gencec.c

diff --git a/AUTHORS b/AUTHORS
new file mode 100644
index 000..e4b7117
--- /dev/null
+++ b/AUTHORS
@@ -0,0 +1 @@
+Kamil Debski 
diff --git a/INSTALL b/INSTALL
new file mode 100644
index 000..aac6101
--- /dev/null
+++ b/INSTALL
@@ -0,0 +1,9 @@
+To install libgencec run following commands:
+
+autoreconf -i
+./configure
+make
+make install
+
+A cross compilation example for ARM:
+CFLAGS=-I ./configure --host=arm-linux-gnueabi 
--prefix=
diff --git a/LICENSE b/LICENSE
new file mode 100644
index 000..d645695
--- /dev/null
+++ b/LICENSE
@@ -0,0 +1,202 @@
+
+ Apache License
+   Version 2.0, January 2004
+http://www.apache.org/licenses/
+
+   TERMS AND CONDITIONS FOR USE, REPRODUCTION, AND DISTRIBUTION
+
+   1. Definitions.
+
+  "License" shall mean the terms and conditions for use, reproduction,
+  and distribution as defined by Sections 1 through 9 of this document.
+
+  "Licensor" shall mean the copyright owner or entity authorized by
+  the copyright owner that is granting the License.
+
+  "Legal Entity" shall mean the union of the acting entity and all
+  other entities that control, are controlled by, or are under common
+  control with that entity. For the purposes of this definition,
+  "control" means (i) the power, direct or indirect, to cause the
+  direction or management of such entity, whether by contract or
+  otherwise, or (ii) ownership of fifty percent (50%) or more of the
+  outstanding shares, or (iii) beneficial ownership of such entity.
+
+  "You" (or "Your") shall mean an individual or Legal Entity
+  exercising permissions granted by this License.
+
+  "Source" form shall mean the preferred form for making modifications,
+  including but not limited to software source code, documentation
+  source, and configuration files.
+
+  "Object" form shall mean any form resulting from mechanical
+  transformation or translation of a Source form, including but
+  not limited to compiled object code, generated documentation,
+  and conversions to other media types.
+
+  "Work" shall mean the work of authorship, whether in Source or
+  Object form, made available under the License, as indicated by a
+  copyright notice that is included in or attached to the work
+  (an example is provided in the Appendix below).
+
+  "Derivative Works" shall mean any work, whether in Source or Object
+  form, that is based on (or derived from) the Work and for which the
+  editorial revisions, annotations, elaborations, or other modifications
+  represent, as a whole, an original work of authorship. For the purposes
+  of this License, Derivative Works shall not include works that remain
+  separable from, or merely link (or bind by name) to the interfaces of,
+  the Work and Derivative Works thereof.
+
+  "Contribution" shall mean any work of authorship, including
+  the original version of the Work and any modifications or additions
+  to that Work or Derivative Works thereof, that is intentionally
+  submitted to Licensor for inclusion in the Work by the copyright owner
+  or by an individual or Legal Entity authorized to submit on behalf of
+  the copyright owner. For the purposes of this definition, "submitted"
+  means any form of electronic, verbal, or written communication sent
+  to the Licensor or its representatives, including but not limited to
+  communication on electronic mailing lists, source code control systems,
+  and issue tracking systems th

[PATCH v2 2/3] drm: simplify authentication management

2015-05-04 Thread David Herrmann
The magic auth tokens we have are a simple map from cyclic IDs to drm_file
objects. Remove all the old bulk of code and replace it with a simple,
direct IDR.

The previous behavior is kept. Especially calling authmagic multiple times
on the same magic results in EINVAL except on the first call. The only
difference in behavior is that we never allocate IDs multiple times as
long as a client has its FD open.

Diff-stat:
   5 files changed, 45 insertions(+), 157 deletions(-)

Signed-off-by: David Herrmann 
---
v2:
 - Fix return code of GetMagic()
 - Use non-cyclic IDR allocator
 - fix off-by-one in "magic > INT_MAX" sanity check

 drivers/gpu/drm/drm_auth.c | 178 +
 drivers/gpu/drm/drm_drv.c  |  12 +--
 drivers/gpu/drm/drm_fops.c |   7 +-
 drivers/gpu/drm/drm_internal.h |   1 -
 include/drm/drmP.h |   4 +-
 5 files changed, 45 insertions(+), 157 deletions(-)

diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
index 8a37524..f5f77db 100644
--- a/drivers/gpu/drm/drm_auth.c
+++ b/drivers/gpu/drm/drm_auth.c
@@ -1,11 +1,3 @@
-/**
- * \file drm_auth.c
- * IOCTLs for authentication
- *
- * \author Rickard E. (Rik) Faith 
- * \author Gareth Hughes 
- */
-
 /*
  * Created: Tue Feb  2 08:37:54 1999 by faith at valinux.com
  *
@@ -13,6 +5,9 @@
  * Copyright 2000 VA Linux Systems, Inc., Sunnyvale, California.
  * All Rights Reserved.
  *
+ * Author Rickard E. (Rik) Faith 
+ * Author Gareth Hughes 
+ *
  * Permission is hereby granted, free of charge, to any person obtaining a
  * copy of this software and associated documentation files (the "Software"),
  * to deal in the Software without restriction, including without limitation
@@ -36,151 +31,47 @@
 #include 
 #include "drm_internal.h"

-struct drm_magic_entry {
-   struct drm_hash_item hash_item;
-   struct drm_file *priv;
-};
-
-/**
- * Find the file with the given magic number.
- *
- * \param dev DRM device.
- * \param magic magic number.
- *
- * Searches in drm_device::magiclist within all files with the same hash key
- * the one with matching magic number, while holding the 
drm_device::struct_mutex
- * lock.
- */
-static struct drm_file *drm_find_file(struct drm_master *master, drm_magic_t 
magic)
-{
-   struct drm_file *retval = NULL;
-   struct drm_magic_entry *pt;
-   struct drm_hash_item *hash;
-   struct drm_device *dev = master->minor->dev;
-
-   mutex_lock(&dev->struct_mutex);
-   if (!drm_ht_find_item(&master->magiclist, (unsigned long)magic, &hash)) 
{
-   pt = drm_hash_entry(hash, struct drm_magic_entry, hash_item);
-   retval = pt->priv;
-   }
-   mutex_unlock(&dev->struct_mutex);
-   return retval;
-}
-
 /**
- * Adds a magic number.
+ * drm_getmagic - Get unique magic of a client
+ * @dev: DRM device to operate on
+ * @data: ioctl data containing the drm_auth object
+ * @file_priv: DRM file that performs the operation
  *
- * \param dev DRM device.
- * \param priv file private data.
- * \param magic magic number.
+ * This looks up the unique magic of the passed client and returns it. If the
+ * client did not have a magic assigned, yet, a new one is registered. The 
magic
+ * is stored in the passed drm_auth object.
  *
- * Creates a drm_magic_entry structure and appends to the linked list
- * associated the magic number hash key in drm_device::magiclist, while holding
- * the drm_device::struct_mutex lock.
- */
-static int drm_add_magic(struct drm_master *master, struct drm_file *priv,
-drm_magic_t magic)
-{
-   struct drm_magic_entry *entry;
-   struct drm_device *dev = master->minor->dev;
-   DRM_DEBUG("%d\n", magic);
-
-   entry = kzalloc(sizeof(*entry), GFP_KERNEL);
-   if (!entry)
-   return -ENOMEM;
-   entry->priv = priv;
-   entry->hash_item.key = (unsigned long)magic;
-   mutex_lock(&dev->struct_mutex);
-   drm_ht_insert_item(&master->magiclist, &entry->hash_item);
-   mutex_unlock(&dev->struct_mutex);
-
-   return 0;
-}
-
-/**
- * Remove a magic number.
- *
- * \param dev DRM device.
- * \param magic magic number.
- *
- * Searches and unlinks the entry in drm_device::magiclist with the magic
- * number hash key, while holding the drm_device::struct_mutex lock.
- */
-int drm_remove_magic(struct drm_master *master, drm_magic_t magic)
-{
-   struct drm_magic_entry *pt;
-   struct drm_hash_item *hash;
-   struct drm_device *dev = master->minor->dev;
-
-   DRM_DEBUG("%d\n", magic);
-
-   mutex_lock(&dev->struct_mutex);
-   if (drm_ht_find_item(&master->magiclist, (unsigned long)magic, &hash)) {
-   mutex_unlock(&dev->struct_mutex);
-   return -EINVAL;
-   }
-   pt = drm_hash_entry(hash, struct drm_magic_entry, hash_item);
-   drm_ht_remove_item(&master->magiclist, hash);
-   mutex_unlock(&dev->struct_mutex);
-
-   kfree(pt);
-
-   return 0;
-}
-
-/*

[PATCH 2/3] drm: simplify authentication management

2015-05-04 Thread David Herrmann
Hi

On Mon, May 4, 2015 at 6:49 PM, Chris Wilson  
wrote:
> On Mon, May 04, 2015 at 06:28:25PM +0200, David Herrmann wrote:
>> I'm not sure how to write a benchmark for this. I mean, I can easily
>> craft one that causes the IDR to degenerate, but it requires us to
>> keep huge numbers of files open.
>> But this fact makes IDR rather suboptimal for cyclic allocations, so I
>> switched to idr_alloc() now. This guarantees tight/packed ID ranges
>> and user-space cannot degenerate the layers, anymore. That is, unless
>> you open more than 256 FDs on a device in parallel, we're fine with a
>> single IDR layer; always. This should work fine, right?
>
> That pretty much circumvents my only worry! If there is a client leak,
> the system will eventually keel under the load, and that we have a huge
> number of magics open is insignificant.
>
> As far as test coverage I would focus on
>
> (a) authenticating up to vfs-file-max fds (i.e. check that we hit
> the system limits without authmagic failing)
>
> (b) churn through a small number of clients for a few minutes, just
> basically checking for anomalous behaviour and that allocation times
> every minute or so remain constant.
>
> (c) just check that we can authenticate! always useful for patches that
> touch the authmagic system
>
> I was thinking of a few more, but they basically serve to show the holes
> in the authmagic scheme.

Attached is an i-g-t patch to test for basic drm-auth/magic behavior.
Comments welcome!

I also sent v2 of this patch seconds ago.

Thanks
David
-- next part --
A non-text attachment was scrubbed...
Name: 0001-tests-add-drm_auth-tests-for-generic-DRM-auth-magic-.patch
Type: text/x-patch
Size: 5733 bytes
Desc: not available
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20150504/02debdeb/attachment.bin>


[PATCH v2 2/3] drm: simplify authentication management

2015-05-04 Thread Chris Wilson
On Mon, May 04, 2015 at 08:19:33PM +0200, David Herrmann wrote:
> The magic auth tokens we have are a simple map from cyclic IDs to drm_file
> objects. Remove all the old bulk of code and replace it with a simple,
> direct IDR.
> 
> The previous behavior is kept. Especially calling authmagic multiple times
> on the same magic results in EINVAL except on the first call. The only
> difference in behavior is that we never allocate IDs multiple times as
> long as a client has its FD open.
> 
> Diff-stat:
>5 files changed, 45 insertions(+), 157 deletions(-)
> 
> Signed-off-by: David Herrmann 
> ---
> v2:
>  - Fix return code of GetMagic()
>  - Use non-cyclic IDR allocator
>  - fix off-by-one in "magic > INT_MAX" sanity check

I'm being nitpicking because I don't understand the rationale for doing
the range check ourselves. idr_find() will return NULL quite happily if
the id wraps around to negative.

However, that's a nit and I like its simplicity (or at least hiding the
complexity in widely used lib code!),

Reviewed-by: Chris Wilson 
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre


[PATCH v2 2/3] drm: simplify authentication management

2015-05-04 Thread David Herrmann
Hi

On Mon, May 4, 2015 at 8:33 PM, Chris Wilson  
wrote:
> On Mon, May 04, 2015 at 08:19:33PM +0200, David Herrmann wrote:
>> The magic auth tokens we have are a simple map from cyclic IDs to drm_file
>> objects. Remove all the old bulk of code and replace it with a simple,
>> direct IDR.
>>
>> The previous behavior is kept. Especially calling authmagic multiple times
>> on the same magic results in EINVAL except on the first call. The only
>> difference in behavior is that we never allocate IDs multiple times as
>> long as a client has its FD open.
>>
>> Diff-stat:
>>5 files changed, 45 insertions(+), 157 deletions(-)
>>
>> Signed-off-by: David Herrmann 
>> ---
>> v2:
>>  - Fix return code of GetMagic()
>>  - Use non-cyclic IDR allocator
>>  - fix off-by-one in "magic > INT_MAX" sanity check
>
> I'm being nitpicking because I don't understand the rationale for doing
> the range check ourselves. idr_find() will return NULL quite happily if
> the id wraps around to negative.

...my bad. I thought you were talking about the off-by-one. I actually
thought drm_magic_t might be 64bit under some circumstances, so I
wanted to avoid the double-mapping. But you're right, it's just an
unsigned int so we can forward it unchanged to the idr helpers.

Fixed!

Thanks
David

> However, that's a nit and I like its simplicity (or at least hiding the
> complexity in widely used lib code!),
>
> Reviewed-by: Chris Wilson 
> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre


[Bug 97701] [regression] won't boot unless radeon.audio=0

2015-05-04 Thread bugzilla-dae...@bugzilla.kernel.org
https://bugzilla.kernel.org/show_bug.cgi?id=97701

Alex Deucher  changed:

   What|Removed |Added

 CC||alexdeucher at gmail.com

--- Comment #2 from Alex Deucher  ---
Created attachment 175801
  --> https://bugzilla.kernel.org/attachment.cgi?id=175801&action=edit
possible fix

This patch should fix it.

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


[PATCH v3 2/3] drm: simplify authentication management

2015-05-04 Thread David Herrmann
The magic auth tokens we have are a simple map from cyclic IDs to drm_file
objects. Remove all the old bulk of code and replace it with a simple,
direct IDR.

The previous behavior is kept. Especially calling authmagic multiple times
on the same magic results in EINVAL except on the first call. The only
difference in behavior is that we never allocate IDs multiple times as
long as a client has its FD open.

Diff-stat:
   5 files changed, 42 insertions(+), 157 deletions(-)

Signed-off-by: David Herrmann 
Reviewed-by: Chris Wilson 
---
v3:
 - drop redundant "magic > INT_MAX" check

v2:
 - Fix return code of GetMagic()
 - Use non-cyclic IDR allocator
 - fix off-by-one in "magic > INT_MAX" sanity check

 drivers/gpu/drm/drm_auth.c | 175 +
 drivers/gpu/drm/drm_drv.c  |  12 +--
 drivers/gpu/drm/drm_fops.c |   7 +-
 drivers/gpu/drm/drm_internal.h |   1 -
 include/drm/drmP.h |   4 +-
 5 files changed, 42 insertions(+), 157 deletions(-)

diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
index 8a37524..50d0baa 100644
--- a/drivers/gpu/drm/drm_auth.c
+++ b/drivers/gpu/drm/drm_auth.c
@@ -1,11 +1,3 @@
-/**
- * \file drm_auth.c
- * IOCTLs for authentication
- *
- * \author Rickard E. (Rik) Faith 
- * \author Gareth Hughes 
- */
-
 /*
  * Created: Tue Feb  2 08:37:54 1999 by faith at valinux.com
  *
@@ -13,6 +5,9 @@
  * Copyright 2000 VA Linux Systems, Inc., Sunnyvale, California.
  * All Rights Reserved.
  *
+ * Author Rickard E. (Rik) Faith 
+ * Author Gareth Hughes 
+ *
  * Permission is hereby granted, free of charge, to any person obtaining a
  * copy of this software and associated documentation files (the "Software"),
  * to deal in the Software without restriction, including without limitation
@@ -36,151 +31,47 @@
 #include 
 #include "drm_internal.h"

-struct drm_magic_entry {
-   struct drm_hash_item hash_item;
-   struct drm_file *priv;
-};
-
-/**
- * Find the file with the given magic number.
- *
- * \param dev DRM device.
- * \param magic magic number.
- *
- * Searches in drm_device::magiclist within all files with the same hash key
- * the one with matching magic number, while holding the 
drm_device::struct_mutex
- * lock.
- */
-static struct drm_file *drm_find_file(struct drm_master *master, drm_magic_t 
magic)
-{
-   struct drm_file *retval = NULL;
-   struct drm_magic_entry *pt;
-   struct drm_hash_item *hash;
-   struct drm_device *dev = master->minor->dev;
-
-   mutex_lock(&dev->struct_mutex);
-   if (!drm_ht_find_item(&master->magiclist, (unsigned long)magic, &hash)) 
{
-   pt = drm_hash_entry(hash, struct drm_magic_entry, hash_item);
-   retval = pt->priv;
-   }
-   mutex_unlock(&dev->struct_mutex);
-   return retval;
-}
-
-/**
- * Adds a magic number.
- *
- * \param dev DRM device.
- * \param priv file private data.
- * \param magic magic number.
- *
- * Creates a drm_magic_entry structure and appends to the linked list
- * associated the magic number hash key in drm_device::magiclist, while holding
- * the drm_device::struct_mutex lock.
- */
-static int drm_add_magic(struct drm_master *master, struct drm_file *priv,
-drm_magic_t magic)
-{
-   struct drm_magic_entry *entry;
-   struct drm_device *dev = master->minor->dev;
-   DRM_DEBUG("%d\n", magic);
-
-   entry = kzalloc(sizeof(*entry), GFP_KERNEL);
-   if (!entry)
-   return -ENOMEM;
-   entry->priv = priv;
-   entry->hash_item.key = (unsigned long)magic;
-   mutex_lock(&dev->struct_mutex);
-   drm_ht_insert_item(&master->magiclist, &entry->hash_item);
-   mutex_unlock(&dev->struct_mutex);
-
-   return 0;
-}
-
-/**
- * Remove a magic number.
- *
- * \param dev DRM device.
- * \param magic magic number.
- *
- * Searches and unlinks the entry in drm_device::magiclist with the magic
- * number hash key, while holding the drm_device::struct_mutex lock.
- */
-int drm_remove_magic(struct drm_master *master, drm_magic_t magic)
-{
-   struct drm_magic_entry *pt;
-   struct drm_hash_item *hash;
-   struct drm_device *dev = master->minor->dev;
-
-   DRM_DEBUG("%d\n", magic);
-
-   mutex_lock(&dev->struct_mutex);
-   if (drm_ht_find_item(&master->magiclist, (unsigned long)magic, &hash)) {
-   mutex_unlock(&dev->struct_mutex);
-   return -EINVAL;
-   }
-   pt = drm_hash_entry(hash, struct drm_magic_entry, hash_item);
-   drm_ht_remove_item(&master->magiclist, hash);
-   mutex_unlock(&dev->struct_mutex);
-
-   kfree(pt);
-
-   return 0;
-}
-
 /**
- * Get a unique magic number (ioctl).
+ * drm_getmagic - Get unique magic of a client
+ * @dev: DRM device to operate on
+ * @data: ioctl data containing the drm_auth object
+ * @file_priv: DRM file that performs the operation
  *
- * \param inode device inode.
- * \param file_priv DRM file private.
- * \param cmd com

[Bug 97701] [regression] won't boot unless radeon.audio=0

2015-05-04 Thread bugzilla-dae...@bugzilla.kernel.org
https://bugzilla.kernel.org/show_bug.cgi?id=97701

--- Comment #3 from Jouni Mettälä  ---
Patch fixed it.

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


[Bug 89069] Lack of grass in The Talos Principle on radeonsi (native\wine\nine)

2015-05-04 Thread bugzilla-dae...@freedesktop.org
https://bugs.freedesktop.org/show_bug.cgi?id=89069

--- Comment #34 from Daniel Scharrer  ---
(In reply to smoki from comment #28)
>  After
> http://cgit.freedesktop.org/mesa/mesa/commit/
> ?id=c6d79ed289a75f13c65f011be870f7e43a0fedc7 which improve code quality i
> can enable unsafe-fp-math and grass is now there with trace from Comment 7
> 
>  Daniel and others, you might want to recheck this.

I can confirm that there is still grass with Mesa git-e1ae0c3 + LLVM r236436
and

diff --git a/src/gallium/drivers/radeon/radeon_llvm_emit.c.old
b/src/gallium/drivers/radeon/radeon_llvm_emit.c
index 624077c..0f9dbab 100644
--- a/src/gallium/drivers/radeon/radeon_llvm_emit.c.old
+++ b/src/gallium/drivers/radeon/radeon_llvm_emit.c
@@ -80,6 +80,10 @@ void radeon_llvm_shader_type(LLVMValueRef F, unsigned type)
sprintf(Str, "%1d", llvm_type);

LLVMAddTargetDependentFunctionAttr(F, "ShaderType", Str);
+
+   if (type != TGSI_PROCESSOR_COMPUTE) {
+   LLVMAddTargetDependentFunctionAttr(F, "unsafe-fp-math",
"true");
+   }
 }

I still get GPU faults with The Talos Principle, even with "unsafe-fp-math"
enabled now - although not with either of the traces I posted. I also get some
garbage rendered / flickering stuff, but I don't think it's related to
"unsafe-fp-math". I'll test more and post a new trace when I have time.

-- 
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/20150504/c78b9b47/attachment.html>


[PATCH] drm/cma: Fix 64-bit size_t build warnings

2015-05-04 Thread Geert Uytterhoeven
From: Magnus Damm 

Fix warnings related to size_t when building for 64-bit architectures:

drivers/gpu/drm/drm_gem_cma_helper.c: In function ‘drm_gem_cma_create’:
drivers/gpu/drm/drm_gem_cma_helper.c:114:4: warning: format ‘%d’ expects 
argument of type ‘int’, but argument 3 has type ‘size_t’ [-Wformat=]
size);
^
drivers/gpu/drm/drm_gem_cma_helper.c: In function ‘drm_gem_cma_describe’:
drivers/gpu/drm/drm_gem_cma_helper.c:393:4: warning: format ‘%d’ expects 
argument of type ‘int’, but argument 8 has type ‘size_t’ [-Wformat=]
off, &cma_obj->paddr, cma_obj->vaddr, obj->size);

Signed-off-by: Magnus Damm 
Signed-off-by: Geert Uytterhoeven 
---
 drivers/gpu/drm/drm_gem_cma_helper.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c 
b/drivers/gpu/drm/drm_gem_cma_helper.c
index e419eedf751de6cb..bd75f303da63d9c6 100644
--- a/drivers/gpu/drm/drm_gem_cma_helper.c
+++ b/drivers/gpu/drm/drm_gem_cma_helper.c
@@ -110,7 +110,7 @@ struct drm_gem_cma_object *drm_gem_cma_create(struct 
drm_device *drm,
cma_obj->vaddr = dma_alloc_writecombine(drm->dev, size,
&cma_obj->paddr, GFP_KERNEL | __GFP_NOWARN);
if (!cma_obj->vaddr) {
-   dev_err(drm->dev, "failed to allocate buffer with size %d\n",
+   dev_err(drm->dev, "failed to allocate buffer with size %zu\n",
size);
ret = -ENOMEM;
goto error;
@@ -388,7 +388,7 @@ void drm_gem_cma_describe(struct drm_gem_cma_object 
*cma_obj,

off = drm_vma_node_start(&obj->vma_node);

-   seq_printf(m, "%2d (%2d) %08llx %pad %p %d",
+   seq_printf(m, "%2d (%2d) %08llx %pad %p %zu",
obj->name, obj->refcount.refcount.counter,
off, &cma_obj->paddr, cma_obj->vaddr, obj->size);

-- 
1.9.1



[PATCH] drm/exynos: Fix build breakage on !DRM_EXYNOS_FIMD

2015-05-04 Thread Krzysztof Kozlowski
W dniu 04.05.2015 o 22:15, Andrzej Hajda pisze:
> Hi,
> 
> 
> On 05/04/2015 02:43 PM, Krzysztof Kozlowski wrote:
>> 2015-05-04 20:34 GMT+09:00 Daniel Stone :
>>> Hi,
>>>
>>> On 4 May 2015 at 08:43, Inki Dae  wrote:
 On 2015년 05월 02일 13:08, Krzysztof Kozlowski wrote:
> Selecting CONFIG_FB_S3C disables CONFIG_DRM_EXYNOS_FIMD leading to build
> error:

 No, eDP has no any dependency of FIMD but DECON. Just add dependency
 code like below,

  config DRM_EXYNOS7_DECON
 bool "Exynos DRM DECON"
 -   depends on DRM_EXYNOS
 +   depends on DRM_EXYNOS && !FB_S3C
>>
>> Actually my commit message was not detailed enough. The FB_S3C here
>> won't solve the issue because you may:
>> 1, disable FIMD and FB_S3C,
>> 2, enabke DECON and DP,
>> and it won't compile.
>>
>> Currently the FIMD must be enabled if DRM_EXYNOS_DP is enabled.
>>
>>>
>>> But it does clearly and explicitly call fimd_dp_clock_enable from
>>> exynos_dp_powero{n,ff}. So the dependency you're proposing seems
>>> backwards: it's not an expression of the requirements of the current
>>> code (that FIMD DP code be available, i.e. CONFIG_DRM_EXYNOS_FIMD is
>>> selected), but an indirect expression of another dependency
>>> (CONFIG_FB_S3C disables CONFIG_DRM_EXYNOS_FIMD, so disable
>>> CONFIG_FB_S3C).
>>>
>>> Additionally, as the call comes from exynos_dp_core.c, which is built
>>> by CONFIG_DRM_EXYNOS_DP (an explicitly user-selectable option), why
>>> shouldn't the dependency be there? Ah, because the dependency on DP is
>>> for (DECON || FIMD), but as DECON doesn't provide
>>> fimd_dp_clock_enable(), it doesn't seem like it would compile if you
>>> selected DECON and not FIMD.
>>>
>>> So, for me, the cleanest solution would be config DRM_EXYNOS_DP gains
>>> a hard dependency on DRM_EXYNOS_FIMD, at least until it can be fixed
>>> to compile without FIMD.
>>
>> Right, you correctly pointed current dependencies. Still it looks little
>> hacky because EXYNOS_DP may work with FIMD or DECON. 
> 
> Are you sure? I have not seen any chipset having DECON and DP. In all
> chipsets known to me DP is always accompanied by FIMD. I guess it can
> change in the future, but for now hard dependency on FIMD seems to be OK
> - it just reflects hardware design.
> Of course this is just my humble opinion :)

OK, so my next question would be: does DECON requires similar clock
handling like FIMD on certain SoCs? In other words - does something like
fimd_dp_clock_enable() have any sense in context of DECON?

Best regards,
Krzysztof



[PATCH 1/2 linux-next] drm/msm: use IS_ERR() to check msm_ioremap() return

2015-05-04 Thread Fabian Frederick
msm_ioremap() never returns NULL. There's no need for IS_ERR_OR_NULL()

Signed-off-by: Fabian Frederick 
---
 drivers/gpu/drm/msm/dsi/dsi_phy.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi_phy.c 
b/drivers/gpu/drm/msm/dsi/dsi_phy.c
index f0cea89..61af193 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_phy.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_phy.c
@@ -300,12 +300,12 @@ struct msm_dsi_phy *msm_dsi_phy_init(struct 
platform_device *pdev,
return NULL;

phy->base = msm_ioremap(pdev, "dsi_phy", "DSI_PHY");
-   if (IS_ERR_OR_NULL(phy->base)) {
+   if (IS_ERR(phy->base)) {
pr_err("%s: failed to map phy base\n", __func__);
return NULL;
}
phy->reg_base = msm_ioremap(pdev, "dsi_phy_regulator", "DSI_PHY_REG");
-   if (IS_ERR_OR_NULL(phy->reg_base)) {
+   if (IS_ERR(phy->reg_base)) {
pr_err("%s: failed to map phy regulator base\n", __func__);
return NULL;
}
-- 
1.9.1



[PATCH 2/2 linux-next] drm/msm: use IS_ERR() to check regulator_get() return

2015-05-04 Thread Fabian Frederick
regulator_get() never returns NULL. There's no need for IS_ERR_OR_NULL()

Signed-off-by: Fabian Frederick 
---
 drivers/gpu/drm/msm/dsi/dsi_host.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c 
b/drivers/gpu/drm/msm/dsi/dsi_host.c
index 956b224..642cbaf 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -273,7 +273,7 @@ static const struct dsi_config *dsi_get_config(struct 
msm_dsi_host *msm_host)
u32 major = 0, minor = 0;

gdsc_reg = regulator_get(&msm_host->pdev->dev, "gdsc");
-   if (IS_ERR_OR_NULL(gdsc_reg)) {
+   if (IS_ERR(gdsc_reg)) {
pr_err("%s: cannot get gdsc\n", __func__);
goto fail;
}
-- 
1.9.1



[BUG] i915: suspend by closing Laptop lid broken

2015-05-04 Thread Martin Kepplinger
So. -rc1 broke suspending by closing my laptop lid and it's not fixed in
-rc2. It works exactly *one* first time and every subsequent lid-closing
is ignored.

Biscted and tested first bad commit:
14aa02449064541217836b9f3d3295e241d5ae9c

This pulls in i915 changes as well as ACPI changes. I don't know the
driver but I'm sure you can find the mistake. I'm happy to test changes.

There are no log differences.


[PATCH] drm/exynos: Fix build breakage on !DRM_EXYNOS_FIMD

2015-05-04 Thread Krzysztof Kozlowski
2015-05-04 20:34 GMT+09:00 Daniel Stone :
> Hi,
>
> On 4 May 2015 at 08:43, Inki Dae  wrote:
>> On 2015년 05월 02일 13:08, Krzysztof Kozlowski wrote:
>>> Selecting CONFIG_FB_S3C disables CONFIG_DRM_EXYNOS_FIMD leading to build
>>> error:
>>
>> No, eDP has no any dependency of FIMD but DECON. Just add dependency
>> code like below,
>>
>>  config DRM_EXYNOS7_DECON
>> bool "Exynos DRM DECON"
>> -   depends on DRM_EXYNOS
>> +   depends on DRM_EXYNOS && !FB_S3C

Actually my commit message was not detailed enough. The FB_S3C here
won't solve the issue because you may:
1, disable FIMD and FB_S3C,
2, enabke DECON and DP,
and it won't compile.

Currently the FIMD must be enabled if DRM_EXYNOS_DP is enabled.

>
> But it does clearly and explicitly call fimd_dp_clock_enable from
> exynos_dp_powero{n,ff}. So the dependency you're proposing seems
> backwards: it's not an expression of the requirements of the current
> code (that FIMD DP code be available, i.e. CONFIG_DRM_EXYNOS_FIMD is
> selected), but an indirect expression of another dependency
> (CONFIG_FB_S3C disables CONFIG_DRM_EXYNOS_FIMD, so disable
> CONFIG_FB_S3C).
>
> Additionally, as the call comes from exynos_dp_core.c, which is built
> by CONFIG_DRM_EXYNOS_DP (an explicitly user-selectable option), why
> shouldn't the dependency be there? Ah, because the dependency on DP is
> for (DECON || FIMD), but as DECON doesn't provide
> fimd_dp_clock_enable(), it doesn't seem like it would compile if you
> selected DECON and not FIMD.
>
> So, for me, the cleanest solution would be config DRM_EXYNOS_DP gains
> a hard dependency on DRM_EXYNOS_FIMD, at least until it can be fixed
> to compile without FIMD.

Right, you correctly pointed current dependencies. Still it looks little
hacky because EXYNOS_DP may work with FIMD or DECON. It does not really
need FIMD. Using ifdefs in headers is not uncommon - many core
subsystems do this that way to provide stubs.

Probably the cleanest way would be to provide by FIMD and DECON a common
interface for DP for such operation, something like:
struct exynos_drm_crtc {
struct drm_crtc base;
...
void (*clock_enable)(struct exynos_drm_crtc *crtc, bool enable)
);

which, if non-NULL, will be called by exynos_dp_core.c:
static void exynos_dp_poweron(struct exynos_dp_device *dp)
{
...
if (crtc->clock_enable)
crtc->clock_enable(crtc, true);
}

What do you think?

Best regards,
Krzysztof