Build regressions/improvements in v4.1-rc1

[Aaro Koskinen]

Hi,

On Mon, Apr 27, 2015 at 12:03:32PM +0200, Geert Uytterhoeven wrote:
> > *** ERRORS ***
> >
> > 34 regressions:
> 
> The quiet days are over...
> 
> >   + /home/kisskb/slave/src/arch/mips/cavium-octeon/smp.c: error: passing 
> > argument 2 of 'cpumask_clear_cpu' discards 'volatile' qualifier from 
> > pointer target type [-Werror]:  => 242:2
> >   + /home/kisskb/slave/src/arch/mips/kernel/process.c: error: passing 
> > argument 2 of 'cpumask_test_cpu' discards 'volatile' qualifier from pointer 
> > target type [-Werror]:  => 52:2
> >   + /home/kisskb/slave/src/arch/mips/kernel/smp.c: error: passing argument 
> > 2 of 'cpumask_set_cpu' discards 'volatile' qualifier from pointer target 
> > type [-Werror]:  => 149:2, 211:2

For these there is a fix proposal: http://patchwork.linux-mips.org/patch/9828/

A.

[PATCH v10 0/9] ASoC: tda998x: add a codec to the HDMI transmitter

[Jean-Francois Moine]

On Mon, 27 Apr 2015 14:33:45 +0300
Jyri Sarha  wrote:

> Have you done anything about the tda998x audio support lately?
> 
> I was thinking of taking a shot at this now that I finally seem to have 
> some time for it. However, if you are just about to send another series 
> I'll wait for that first and see what makes the most sense after that.
> 
> My plan is to do something really simple first. No graph bindings or 
> anything fancy like that. Just a minimal dt-binding to for audio pin 
> configuration and use simple-card for the rest. I still try to make it 
> possible to support spdif and i2s at the same time, but I can not test 
> it as I do not have such HW.
> 
> I also try to make the ASoC part as generic as possible, so that it 
> could be reused by other HDMI encoders with spdif or i2s interface.

Hi Jyri,

I was busy on an other work, so, I did not advance on the tda998x.

If you look at the patch series, it contains 3 independant parts:
- dynamic building of the DAIs in the kirkwood audio controller,
- audio CODEC of the tda998x
- DT card based on audio graph

It seems that only the last part raised a problem.
Otherwise, the simple card works fine with the 2 first parts.

I am merging my patches in 4.1-rc1, and, as soon as I get an image on
my screen :), I will resubmit a patch series but about the tda998x
codec only.

Using i2s and s/pdif at the same time with the simple card asks for a
patch as the one I submitted in february 2014 (ASoC: simple-card: DT
fix and multi DAI links extension). As I am using my DT card, I will
not resubmit it. Eventually, the kirkwood patches are not critical.

-- 
Ken ar c'hentañ| ** Breizh ha Linux atav! **
Jef |   http://moinejf.free.fr/

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

[Ville Syrjälä]

On Thu, Apr 23, 2015 at 03:07:57PM +0100, Peter Antoine wrote:
> As these functions are only used by one driver and there are security holes
> in these functions. Make the functions optional.
> 
> Issue: VIZ-5485
> Signed-off-by: Peter Antoine 
> ---
>  drivers/gpu/drm/drm_lock.c|  6 ++
>  drivers/gpu/drm/i915/i915_dma.c   |  3 +++
>  drivers/gpu/drm/nouveau/nouveau_drm.c |  3 ++-
>  include/drm/drmP.h| 23 ---
>  include/uapi/drm/i915_drm.h   |  1 +
>  5 files changed, 24 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_lock.c b/drivers/gpu/drm/drm_lock.c
> index 94500930..b61d4c7 100644
> --- a/drivers/gpu/drm/drm_lock.c
> +++ b/drivers/gpu/drm/drm_lock.c
> @@ -61,6 +61,9 @@ int drm_legacy_lock(struct drm_device *dev, void *data,
>   struct drm_master *master = file_priv->master;
>   int ret = 0;
>  
> + if (!drm_core_check_feature(dev, DRIVER_KMS_LEGACY_CONTEXT))
> + return -EINVAL;
> +
>   ++file_priv->lock_count;
>  
>   if (_DRM_LOCKING_CONTEXT(lock->context) == DRM_KERNEL_CONTEXT) {
> @@ -153,6 +156,9 @@ 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 (!drm_core_check_feature(dev, DRIVER_KMS_LEGACY_CONTEXT))
> + return -EINVAL;
> +
>   if (_DRM_LOCKING_CONTEXT(lock->context) == DRM_KERNEL_CONTEXT) {
>   DRM_ERROR("Process %d using kernel context %d\n",
> task_pid_nr(current), lock->context);
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index e44116f..c771ef0 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -163,6 +163,9 @@ static int i915_getparam(struct drm_device *dev, void 
> *data,
>   if (!value)
>   return -ENODEV;
>   break;
> + case I915_PARAM_HAS_LEGACY_CONTEXT:
> + value = drm_core_check_feature(dev, DRIVER_KMS_LEGACY_CONTEXT);
> + break;

Seems pointless to add a parameter that'll always be false.

>   default:
>   DRM_DEBUG("Unknown parameter %d\n", param->param);
>   return -EINVAL;
> diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c 
> b/drivers/gpu/drm/nouveau/nouveau_drm.c
> index 8763deb..936b423 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
> @@ -940,7 +940,8 @@ static struct drm_driver
>  driver_stub = {
>   .driver_features =
>   DRIVER_USE_AGP |
> - DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME | DRIVER_RENDER,
> + DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME | DRIVER_RENDER |
> + DRIVER_KMS_LEGACY_CONTEXT,

Why is this here? AFAICS only the via driver cares about legacy
contexts, and only dri1 drivers care about the hw lock.

>  
>   .load = nouveau_drm_load,
>   .unload = nouveau_drm_unload,
> 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_SG0x10
> +#define DRIVER_HAVE_DMA  0x20
> +#define DRIVER_HAVE_IRQ  0x40
> +#define DRIVER_IRQ_SHARED0x80
> +#define DRIVER_GEM   0x1000
> +#define DRIVER_MODESET   0x2000
> +#define DRIVER_PRIME 0x4000
> +#define DRIVER_RENDER0x8000
> +#define DRIVER_ATOMIC0x1
> +#define DRIVER_KMS_LEGACY_CONTEXT0x2

Why is there KMS in the name?

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.

I suppose one option would be to check for MODESET instead. kms+dri1
doesn't sound like an entirely sane combination to me.

>  
>  /***/
>  /** \name Macros to make printk easier */
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index 4851d66..0ad611a2 

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

[Ville Syrjälä]

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 
> ---
>  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, >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

[Bug 90202] Firefox nightly hangs with accelerated layers and latest r600 driver

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

sage_loop.cc:369
#23 0x717ac917 in MessageLoop::DoWork() (this=0x7fffd81fed50) at
/build/buildd/firefox-trunk-40.0~a1~hg20150422r240313/ipc/chromium/src/base/message_loop.cc:456
#24 0x717a9184 in
base::MessagePumpDefault::Run(base::MessagePump::Delegate*)
(this=0x7fffddf66820, delegate=0x7fffd81fed50) at
/build/buildd/firefox-trunk-40.0~a1~hg20150422r240313/ipc/chromium/src/base/message_pump_default.cc:34
#25 0x717a901b in MessageLoop::Run() (this=0x7fffd81fed50) at
/build/buildd/firefox-trunk-40.0~a1~hg20150422r240313/ipc/chromium/src/base/message_loop.cc:226
#26 0x717a901b in MessageLoop::Run() (this=this at entry=0x7fffd81fed50)
at
/build/buildd/firefox-trunk-40.0~a1~hg20150422r240313/ipc/chromium/src/base/message_loop.cc:200
#27 0x717b0174 in base::Thread::ThreadMain() (this=0x7fffddf667c0) at
/build/buildd/firefox-trunk-40.0~a1~hg20150422r240313/ipc/chromium/src/base/thread.cc:170
#28 0x717add00 in ThreadFunc(void*) (closure=) at
/build/buildd/firefox-trunk-40.0~a1~hg20150422r240313/ipc/chromium/src/base/platform_thread_posix.cc:39
#29 0x77bc26aa in start_thread (arg=0x7fffd81ff700) at
pthread_create.c:333
#30 0x770dceed in clone () at
../sysdeps/unix/sysv/linux/x86_64/clone.S:109

-- 
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/20150427/c9cd6d25/attachment-0001.html>

[Bug 90202] Firefox nightly hangs with accelerated layers and latest r600 driver

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

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

--- Comment #2 from Ernst Sjöstrand  ---
Created attachment 115385
  --> https://bugs.freedesktop.org/attachment.cgi?id=115385=edit
thread apply all bt

-- 
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/20150427/744dada6/attachment.html>

[Bug 90202] Firefox nightly hangs with accelerated layers and latest r600 driver

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

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

--- Comment #1 from Ernst Sjöstrand  ---
Created attachment 115384
  --> https://bugs.freedesktop.org/attachment.cgi?id=115384=edit
bt full

-- 
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/20150427/24215f08/attachment.html>

[Bug 90202] Firefox nightly hangs with accelerated layers and latest r600 driver

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

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

Bug ID: 90202
   Summary: Firefox nightly hangs with accelerated layers and
latest r600 driver
   Product: Mesa
   Version: git
  Hardware: Other
OS: All
Status: NEW
  Severity: normal
  Priority: medium
 Component: Drivers/Gallium/r600
  Assignee: dri-devel at lists.freedesktop.org
  Reporter: ernstp at gmail.com
QA Contact: dri-devel at lists.freedesktop.org

(This is not #81680 btw, this is separate...)

If I enable layers.acceleration.force-enabled in Firefox nightly,
Firefox always hangs after a little while of browsing.
This is a regression somewhere, this worked during a couple of months...

-- 
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/20150427/35d55383/attachment.html>

[Bug 90182] [radeonsi]Qt Applications won't start on the dedicated GPU (SIGABRT)

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

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

--- Comment #13 from Paul  ---
Thanks Emil, I tried that just now but somehow I still got the whole packages
presumably with the symbols included. 
Might be because I wasn't aware that you could add that PKGBUILD option and
edited my makepkg.conf instead. Anyway I hope I got something useful with the
last trace. :-)

-- 
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/20150427/7fbf79a8/attachment.html>

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

[Emil Velikov]

Hi Jammy, Frank

As far as I can see you're trying to get a different version of
drmGetBusid(). With the DRM_IOCTL_{G,S}ET_UNIQUE ioctl being lovely as
it is I do see your point, but I'm not sure that the current design will
be too useful.

Do we have any upcoming users for this new function, can you share a bit
about the usecase ?

On 24/04/15 03:44, Jammy Zhou wrote:
> drmGetDevices interface is added to enumernate GPU devices on the system
> 
> v2: rebase the code and some improvement for the coding style
> 
> Signed-off-by: Frank Min 
> Signed-off-by: Jammy Zhou  (v2)
> ---
>  Makefile.am |  3 ++-
>  xf86drm.c   | 48 
>  xf86drm.h   | 18 ++
>  3 files changed, 68 insertions(+), 1 deletion(-)
> 
> diff --git a/Makefile.am b/Makefile.am
> index 42d3d7f..8236ed8 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -89,7 +89,8 @@ SUBDIRS = \
>  libdrm_la_LTLIBRARIES = libdrm.la
>  libdrm_ladir = $(libdir)
>  libdrm_la_LDFLAGS = -version-number 2:4:0 -no-undefined
> -libdrm_la_LIBADD = @CLOCK_LIB@
> +libdrm_la_LIBADD = @CLOCK_LIB@ \
> + @PCIACCESS_LIBS@
>  
>  libdrm_la_CPPFLAGS = -I$(top_srcdir)/include/drm
>  AM_CFLAGS = \
> diff --git a/xf86drm.c b/xf86drm.c
> index ffc53b8..4d67861 100644
> --- a/xf86drm.c
> +++ b/xf86drm.c
> @@ -63,6 +63,7 @@
>  
>  #include "xf86drm.h"
>  #include "libdrm.h"
> +#include 
>  
>  #if defined(__FreeBSD__) || defined(__FreeBSD_kernel__) || 
> defined(__DragonFly__)
>  #define DRM_MAJOR 145
> @@ -2817,3 +2818,50 @@ char *drmGetRenderDeviceNameFromFd(int fd)
>  {
>   return drmGetMinorNameForFD(fd, DRM_NODE_RENDER);
>  }
> +
> +/**
> + * Enumerate the GPU devices on the system
> + *
> + * \param devs device array set to return the device information
> + * (if NULL, the number of device is returned)
> + * \param vendor the vendor ID for GPU devices to list
> + * (optional, if not specified, all GPU devices are returned)
> + *
> + * \return the number of GPU devices
> + */
> +int drmGetDevices(drmDevicePtr devs, uint16_t vendor)
> +{
> + struct pci_device_iterator * iter;
> + struct pci_device * dev;
> + uint32_t count = 0;
> +
> + if (pci_system_init())
> + return -EINVAL;
> +
> + iter = pci_slot_match_iterator_create(NULL);
> + if (!iter)
> + return -EINVAL;
> +
> + while ((dev = pci_device_next(iter))) {
> + if (((dev->device_class == 0x3) ||
> + (dev->device_class == 0x38000)) &&
Any particular reason why "3D controller" (0x32000) is omitted ?

> + ((vendor == 0) || (dev->vendor_id == vendor))){
> + if (devs) {
> + devs[count].domain = dev->domain;
> + devs[count].bus = dev->bus;
> + devs[count].dev = dev->dev;
> + devs[count].func = dev->func;
> + devs[count].vendor_id = dev->vendor_id;
> + devs[count].device_id = dev->device_id;
> + devs[count].subvendor_id = dev->subvendor_id;
> + devs[count].subdevice_id = dev->subdevice_id;
> + devs[count].revision_id = dev->revision;
> + }
> + count++;
> + }
> + }
> +
> + pci_iterator_destroy(iter);
> + pci_system_cleanup();
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.

Another solution will be to get the information based on the
primary/control/render nodes available. This will also allow one to know
what the device can be used for - be that via a separate parameter set
by the function or having different functions altogether.

Cheers
Emil

[Bug 90182] [radeonsi]Qt Applications won't start on the dedicated GPU (SIGABRT)

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

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

--- Comment #12 from Emil Velikov  ---
Paul small tip from a fellow Arch user:
The quickest/easiest way to get debug symbols is to add 'debug' in the OPTIONS
array of the PKGBUILD and rebuild the package [1]. This way separate *debug*
packages will be created.

The configure's --enable-debug does something a big different :-)

[1] https://wiki.archlinux.org/index.php/Debug_-_Getting_Traces#PKGBUILD

-- 
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/20150427/55084749/attachment.html>

[PATCH 1/4] drm/radeon: fix lockup when BOs aren't part of the VM on release

[Christian König]

On 27.04.2015 17:04, Christian König wrote:
> From: Christian König 
>
> If we unmap BOs before releasing them them the intervall tree locks
> up because we try to remove an entry not inside the tree.
>
> Signed-off-by: Christian König 
> CC: stable at vger.kernel.org

Somehow forgot to mention that Michel is the original author of this 
patch, I just fixed two additional lines which caused list corruption.

Regards,
Christian.

> ---
>   drivers/gpu/drm/radeon/radeon_vm.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_vm.c 
> b/drivers/gpu/drm/radeon/radeon_vm.c
> index 2a5a4a9..16d8e95 100644
> --- a/drivers/gpu/drm/radeon/radeon_vm.c
> +++ b/drivers/gpu/drm/radeon/radeon_vm.c
> @@ -1107,7 +1107,8 @@ void radeon_vm_bo_rmv(struct radeon_device *rdev,
>   list_del(_va->bo_list);
>   
>   mutex_lock(>mutex);
> - interval_tree_remove(_va->it, >va);
> + if (bo_va->it.start || bo_va->it.last)
> + interval_tree_remove(_va->it, >va);
>   spin_lock(>status_lock);
>   list_del(_va->vm_status);
>   

[PATCH 4/4] drm/radeon: fix userptr return value checking

[Christian König]

From: Christian König 

Otherwise we print false warning from time to time.

Signed-off-by: Christian König 
Signed-off-by: Jack Xiao 
CC: stable at vger.kernel.org
---
 drivers/gpu/drm/radeon/radeon_mn.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_mn.c 
b/drivers/gpu/drm/radeon/radeon_mn.c
index 572b4db..30e6207 100644
--- a/drivers/gpu/drm/radeon/radeon_mn.c
+++ b/drivers/gpu/drm/radeon/radeon_mn.c
@@ -122,26 +122,26 @@ static void radeon_mn_invalidate_range_start(struct 
mmu_notifier *mn,
it = interval_tree_iter_first(>objects, start, end);
while (it) {
struct radeon_bo *bo;
-   int r;
+   long r;

bo = container_of(it, struct radeon_bo, mn_it);
it = interval_tree_iter_next(it, start, end);

r = radeon_bo_reserve(bo, true);
if (r) {
-   DRM_ERROR("(%d) failed to reserve user bo\n", r);
+   DRM_ERROR("(%ld) failed to reserve user bo\n", r);
continue;
}

r = reservation_object_wait_timeout_rcu(bo->tbo.resv, true,
false, MAX_SCHEDULE_TIMEOUT);
-   if (r)
-   DRM_ERROR("(%d) failed to wait for user bo\n", r);
+   if (r <= 0)
+   DRM_ERROR("(%ld) failed to wait for user bo\n", r);

radeon_ttm_placement_from_domain(bo, RADEON_GEM_DOMAIN_CPU);
r = ttm_bo_validate(>tbo, >placement, false, false);
if (r)
-   DRM_ERROR("(%d) failed to validate user bo\n", r);
+   DRM_ERROR("(%ld) failed to validate user bo\n", r);

radeon_bo_unreserve(bo);
}
-- 
1.9.1

[PATCH 3/4] drm/radeon: check new address before removing old one

[Christian König]

From: Christian König 

Otherwise the change isn't atomic.

Signed-off-by: Christian König 
CC: stable at vger.kernel.org
---
 drivers/gpu/drm/radeon/radeon_vm.c | 31 +--
 1 file changed, 17 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_vm.c 
b/drivers/gpu/drm/radeon/radeon_vm.c
index cabcb0a..de42fc4 100644
--- a/drivers/gpu/drm/radeon/radeon_vm.c
+++ b/drivers/gpu/drm/radeon/radeon_vm.c
@@ -473,6 +473,23 @@ int radeon_vm_bo_set_addr(struct radeon_device *rdev,
}

mutex_lock(>mutex);
+   soffset /= RADEON_GPU_PAGE_SIZE;
+   eoffset /= RADEON_GPU_PAGE_SIZE;
+   if (soffset || eoffset) {
+   struct interval_tree_node *it;
+   it = interval_tree_iter_first(>va, soffset, eoffset - 1);
+   if (it && it != _va->it) {
+   struct radeon_bo_va *tmp;
+   tmp = container_of(it, struct radeon_bo_va, it);
+   /* bo and tmp overlap, invalid offset */
+   dev_err(rdev->dev, "bo %p va 0x%010Lx conflict with "
+   "(bo %p 0x%010lx 0x%010lx)\n", bo_va->bo,
+   soffset, tmp->bo, tmp->it.start, tmp->it.last);
+   mutex_unlock(>mutex);
+   return -EINVAL;
+   }
+   }
+
if (bo_va->it.start || bo_va->it.last) {
if (bo_va->addr) {
/* add a clone of the bo_va to clear the old address */
@@ -499,21 +516,7 @@ int radeon_vm_bo_set_addr(struct radeon_device *rdev,
bo_va->it.last = 0;
}

-   soffset /= RADEON_GPU_PAGE_SIZE;
-   eoffset /= RADEON_GPU_PAGE_SIZE;
if (soffset || eoffset) {
-   struct interval_tree_node *it;
-   it = interval_tree_iter_first(>va, soffset, eoffset - 1);
-   if (it) {
-   struct radeon_bo_va *tmp;
-   tmp = container_of(it, struct radeon_bo_va, it);
-   /* bo and tmp overlap, invalid offset */
-   dev_err(rdev->dev, "bo %p va 0x%010Lx conflict with "
-   "(bo %p 0x%010lx 0x%010lx)\n", bo_va->bo,
-   soffset, tmp->bo, tmp->it.start, tmp->it.last);
-   mutex_unlock(>mutex);
-   return -EINVAL;
-   }
bo_va->it.start = soffset;
bo_va->it.last = eoffset - 1;
interval_tree_insert(_va->it, >va);
-- 
1.9.1

[PATCH 2/4] drm/radeon: reset BOs address after clearing it.

[Christian König]

From: Christian König 

Otherwise it is possible that we will have page table corruption
if we change a BOs address multiple times.

Signed-off-by: Christian König 
CC: stable at vger.kernel.org
---
 drivers/gpu/drm/radeon/radeon_vm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/radeon/radeon_vm.c 
b/drivers/gpu/drm/radeon/radeon_vm.c
index 16d8e95..cabcb0a 100644
--- a/drivers/gpu/drm/radeon/radeon_vm.c
+++ b/drivers/gpu/drm/radeon/radeon_vm.c
@@ -490,6 +490,8 @@ int radeon_vm_bo_set_addr(struct radeon_device *rdev,
spin_lock(>status_lock);
list_add(>vm_status, >freed);
spin_unlock(>status_lock);
+
+   bo_va->addr = 0;
}

interval_tree_remove(_va->it, >va);
-- 
1.9.1

[PATCH 1/4] drm/radeon: fix lockup when BOs aren't part of the VM on release

[Christian König]

From: Christian König 

If we unmap BOs before releasing them them the intervall tree locks
up because we try to remove an entry not inside the tree.

Signed-off-by: Christian König 
CC: stable at vger.kernel.org
---
 drivers/gpu/drm/radeon/radeon_vm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/radeon/radeon_vm.c 
b/drivers/gpu/drm/radeon/radeon_vm.c
index 2a5a4a9..16d8e95 100644
--- a/drivers/gpu/drm/radeon/radeon_vm.c
+++ b/drivers/gpu/drm/radeon/radeon_vm.c
@@ -1107,7 +1107,8 @@ void radeon_vm_bo_rmv(struct radeon_device *rdev,
list_del(_va->bo_list);

mutex_lock(>mutex);
-   interval_tree_remove(_va->it, >va);
+   if (bo_va->it.start || bo_va->it.last)
+   interval_tree_remove(_va->it, >va);
spin_lock(>status_lock);
list_del(_va->vm_status);

-- 
1.9.1

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

[Chris Wilson]

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(, 0, sizeof(gp));
> > +   gp.value = 
> > +   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, ) != 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

-- 
Chris Wilson, Intel Open Source Technology Centre

[PATCH 2/2] drm/exynos: mixer: remove buffer count handling in vp_video_buffer()

[Joonyoung Shim]

Hi Tobias,

On 04/26/2015 05:31 AM, Tobias Jakobi wrote:
> The video processor (VP) supports four formats: NV12, NV21 and its
> tiled variants. All these formats are bi-planar, so the buffer
> count in vp_video_buffer() is always 2.
> 
> While we're at it, also add support for NV21 and properly exit
> if we're called with an invalid (non-VP) pixelformat.
> 
> Signed-off-by: Tobias Jakobi 
> ---
>  drivers/gpu/drm/exynos/exynos_mixer.c | 20 +---
>  1 file changed, 5 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c 
> b/drivers/gpu/drm/exynos/exynos_mixer.c
> index 534a594..6fb4faa 100644
> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
> @@ -388,7 +388,6 @@ static void vp_video_buffer(struct mixer_context *ctx, 
> int win)
>   struct mixer_resources *res = >mixer_res;
>   unsigned long flags;
>   struct exynos_drm_plane *plane;
> - unsigned int buf_num = 1;
>   dma_addr_t luma_addr[2], chroma_addr[2];
>   bool tiled_mode = false;
>   bool crcb_mode = false;
> @@ -399,27 +398,18 @@ static void vp_video_buffer(struct mixer_context *ctx, 
> int win)
>   switch (plane->pixel_format) {
>   case DRM_FORMAT_NV12:
>   crcb_mode = false;
> - buf_num = 2;
>   break;
> - /* TODO: single buffer format NV12, NV21 */
> + case DRM_FORMAT_NV21:
> + crcb_mode = true;
> + break;

To support NV21, how about make to other patch? because this patch is to
fix buffer number.

>   default:
> - /* ignore pixel format at disable time */
> - if (!plane->dma_addr[0])
> - break;
> -
>   DRM_ERROR("pixel format for vp is wrong [%d].\n",
>   plane->pixel_format);
>   return;
>   }
>  
> - if (buf_num == 2) {
> - luma_addr[0] = plane->dma_addr[0];
> - chroma_addr[0] = plane->dma_addr[1];
> - } else {
> - luma_addr[0] = plane->dma_addr[0];
> - chroma_addr[0] = plane->dma_addr[0]
> - + (plane->pitch * plane->fb_height);
> - }
> + luma_addr[0] = plane->dma_addr[0];
> + chroma_addr[0] = plane->dma_addr[1];

Hmm, dma_addr[0] and dma_addr[1] are same address if it uses just one
buffer for two planes, then need offset information of each plane.

Thanks.

>  
>   if (plane->scan_flag & DRM_MODE_FLAG_INTERLACE) {
>   ctx->interlace = true;
> 

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

[Thomas Wood]

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(, 0, sizeof(gp));
> +   gp.value = 
> +   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, ) != 0)
> +   return true;
> +
> +   return tmp == 1;
> +}
>  /**
>   * gem_available_aperture_size:
>   * @fd: open i915 drm file descriptor
> diff --git a/lib/ioctl_wrappers.h b/lib/ioctl_wrappers.h
> index ced7ef3..3adc700 100644
> --- a/lib/ioctl_wrappers.h
> +++ b/lib/ioctl_wrappers.h
> @@ -120,6 +120,7 @@ bool gem_has_bsd(int fd);
>  bool gem_has_blt(int fd);
>  bool gem_has_vebox(int fd);
>  bool gem_has_bsd2(int fd);
> +bool drm_has_legacy_context(int fd);
>  bool gem_uses_aliasing_ppgtt(int fd);
>  int gem_available_fences(int fd);
>  uint64_t gem_available_aperture_size(int fd);
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index 71de6de..2f69afc 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -84,6 +84,7 @@ TESTS_progs_M = \
> pm_sseu \
> prime_self_import \
> template \
> +   drm_hw_lock \

Please also add the binary name to .gitignore.

> $(NULL)
>
>  TESTS_progs = \
> diff --git a/tests/drm_hw_lock.c b/tests/drm_hw_lock.c
> new file mode 100644
> index 000..aad50ba
> --- /dev/null
> +++ b/tests/drm_hw_lock.c
> @@ -0,0 +1,207 @@
> +/*
> + * Copyright © 2015 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER 
> DEALINGS
> + * IN THE SOFTWARE.
> + *
> + * Authors:
> + *Peter Antoine 
> + */
> +
> +/*
> + * Testcase: Test the HW_LOCKs for correct support and non-crashing.

This would be a good sentence to use in the IGT_TEST_DESCRIPTION
macro, to add a description for the test.


> + *
> + * This test will check that he hw_locks are only g_supported for drivers 
> that
> + * require that support. If it is not g_supported then the functions all 
> return
> + * the correct error code.
> + *
> + * If g_supported it will check that the functions do not crash when the 
> crash
> + * tests are used, also that one of the tests is a security level escalation
> + * that should be rejected.
> + */
> +#include 
> +#include 
> +#include 
> +#include 
> +#include "drmtest.h"
> +#include "igt_core.h"
> +#include "ioctl_wrappers.h"
> +
> +#ifndef DRM_KERNEL_CONTEXT
> +#  define DRM_KERNEL_CONTEXT   (0)
> +#endif
> +
> +#ifndef _DRM_LOCK_HELD
> +#  define _DRM_LOCK_HELD   0x8000U /**< Hardware lock is held */
> +#endif
> +
> +#ifndef _DRM_LOCK_CONT
> 

[PATCH v2 2/2] drm/exynos: cleanup exynos_drm_plane

[Joonyoung Shim]

Hi Tobias,

On 04/26/2015 03:06 AM, Tobias Jakobi wrote:
> Remove the unused fields of struct exynos_drm_plane.
> 
> v2: Remove index_color as well, also unused (thanks Joonyoung).
> 
> Reviewed-by: Gustavo Padovan 
> Signed-off-by: Tobias Jakobi 
> ---
>  drivers/gpu/drm/exynos/exynos_drm_drv.h | 13 -
>  1 file changed, 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h 
> b/drivers/gpu/drm/exynos/exynos_drm_drv.h
> index e12ecb5..b1014c6 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
> @@ -71,13 +71,6 @@ enum exynos_drm_output_type {
>   * @dma_addr: array of bus(accessed by dma) address to the memory region
>   * allocated for a overlay.
>   * @zpos: order of overlay layer(z position).
> - * @index_color: if using color key feature then this value would be used
> - *   as index color.
> - * @default_win: a window to be enabled.
> - * @color_key: color key on or off.
> - * @local_path: in case of lcd type, local path mode on or off.
> - * @transparency: transparency on or off.
> - * @activated: activated or not.
>   * @enabled: enabled or not.
>   * @resume: to resume or not.
>   *
> @@ -108,13 +101,7 @@ struct exynos_drm_plane {
>   uint32_t pixel_format;
>   dma_addr_t dma_addr[MAX_FB_BUFFER];
>   unsigned int zpos;
> - unsigned int index_color;
>  
> - bool default_win:1;
> - bool color_key:1;
> - bool local_path:1;
> - bool transparency:1;
> - bool activated:1;
>   bool enabled:1;
>   bool resume:1;
>  };
> 

Acked-by: Joonyoung Shim 

Thanks.

[PATCH] drm/exynos: fix source data argument for plane

[Joonyoung Shim]

Hi Gustavo,

On 04/25/2015 03:15 AM, Gustavo Padovan wrote:
> Hi Joonyoung,
> 
> 2015-04-16 Joonyoung Shim :
> 
>> The exynos_update_plane function needs 16.16 fixed point source data.
>>
>> Signed-off-by: Joonyoung Shim 
>> ---
>>  drivers/gpu/drm/exynos/exynos_drm_crtc.c | 7 ---
>>  1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c 
>> b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
>> index eb49195..f0bdaca 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
>> @@ -127,7 +127,8 @@ static int exynos_drm_crtc_mode_set_base(struct drm_crtc 
>> *crtc, int x, int y,
>>  crtc_h = fb->height - y;
>>  
>>  return exynos_update_plane(crtc->primary, crtc, fb, 0, 0,
>> -   crtc_w, crtc_h, x, y, crtc_w, crtc_h);
>> +   crtc_w, crtc_h, x << 16, y << 16,
>> +   crtc_w << 16, crtc_h << 16);
>>  }
>>  
>>  static void exynos_drm_crtc_disable(struct drm_crtc *crtc)
>> @@ -202,8 +203,8 @@ static int exynos_drm_crtc_page_flip(struct drm_crtc 
>> *crtc,
>>  crtc_w = fb->width - crtc->x;
>>  crtc_h = fb->height - crtc->y;
>>  ret = exynos_update_plane(crtc->primary, crtc, fb, 0, 0,
>> -  crtc_w, crtc_h, crtc->x, crtc->y,
>> -  crtc_w, crtc_h);
>> +  crtc_w, crtc_h, crtc->x << 16, crtc->y << 16,
>> +  crtc_w << 16, crtc_h << 16);
> 
> Assuming we are pushing the atomic patches soon we don't need the change
> on this patch.  I've asked about the atomic  modesetting patches in the
> other thread.
> 

Yeah, but this fixed patch needs until merged your atomic patches, if
not, i cannot output updated screen from primary plane on current
exynos drm driver.

Thanks.

Help on drmModeSetPlane

[Ville Syrjälä]

On Mon, Apr 27, 2015 at 09:37:54AM +, Xie, William wrote:
> Hello, Syrjälä,
>  For drmModeSetPlane API, regarding the ctrc width, height and src width and 
> heitht,
> Do they must 16 byte aligned?
> For example, for the size of 33x66, will these value be supported?

Depends on the hardware/driver implementation. In general the driver
should fudge things enough that you'll get something reasonably close to
what you requested. For Intel hardware we support pixel accurate
coordinates (well, macropixel in the case of packed YUV 4:2:2).
Sub-pixel accuracy isn't supported on current hardware.

> 
> William
> 
> -Original Message-
> From: Ville Syrjälä [mailto:ville.syrjala at linux.intel.com] 
> Sent: Friday, April 17, 2015 6:08 PM
> To: Xie, William
> Cc: DRI Development
> Subject: Re: Help on drmModeSetPlane
> 
> On Fri, Apr 17, 2015 at 09:30:02AM +, Xie, William wrote:
> > Hello
> > 
> > Does overlay support alpha?
> 
> Depends on the hardware once more. Also we don't have a proper API for it at 
> this time. You may get lucky if you tell it to use a pixel format with alpha. 
> But there's no guarantee what kind of blend equation you will get.
> 
> Also we don't really have an API for the plane Z order either. Some drivers 
> have a property for that, but I don't think it's really well defined.
> 
> > 
> > William
> > 
> > -Original Message-
> > From: Ville Syrjälä [mailto:ville.syrjala at linux.intel.com]
> > Sent: Wednesday, April 15, 2015 10:34 PM
> > To: Xie, William
> > Cc: DRI Development
> > Subject: Re: Help on drmModeSetPlane
> > 
> > On Wed, Apr 15, 2015 at 02:24:14PM +, Xie, William wrote:
> > > Oh, I tried it on BDW.
> > > So we need to use other way to scale the video to full screen?
> > 
> > Yes, unfortunately.
> > 
> > > 
> > > William
> > > 
> > > 
> > > -Original Message-
> > > From: Ville Syrjälä [mailto:ville.syrjala at linux.intel.com]
> > > Sent: Wednesday, April 15, 2015 7:23 PM
> > > To: Xie, William
> > > Cc: DRI Development
> > > Subject: Re: Help on drmModeSetPlane
> > > 
> > > On Wed, Apr 15, 2015 at 08:49:39AM +, Xie, William wrote:
> > > > To make it more specific,
> > > > 
> > > > For example:
> > > > 1: video frame size is 1280x720,
> > > > 2: screen resolution is 3200x1800,
> > > > 
> > > > How can I set crtc and src parameters?
> > > > 
> > > > 1: crtc (0, 0, 3200, 1800)  src (0, 0, 3200<<16, 1800<<16)
> > > > 2: crtc (0, 0, 3200, 1800)  src (0, 0, 1280<<16, 720<<16)
> > > > 
> > > > Which one is correct?
> > > 
> > > 2 is correct.
> > > 
> > > What hardware are you trying this on? Do note that on HSW/BDW/VLV/CHV the 
> > > hardware lost the plane scaling ability. It's coming back on SKL.
> > > 
> > > > 
> > > > William
> > > > 
> > > > From: Xie, William
> > > > Sent: Wednesday, April 15, 2015 3:55 PM
> > > > To: DRI Development; DRI Development
> > > > Subject: Help on drmModeSetPlane
> > > > 
> > > > Question please, could someone share the meaning of the parameters for 
> > > > drmModeSetPlane in libdrm?
> > > > 
> > > > int drmModeSetPlane(int fd, uint32_t plane_id, uint32_t crtc_id,
> > > > uint32_t fb_id, uint32_t flags,
> > > > int32_t crtc_x, int32_t crtc_y,
> > > > uint32_t crtc_w, uint32_t crtc_h,
> > > > uint32_t src_x, uint32_t src_y,
> > > > uint32_t src_w, uint32_t 
> > > > src_h)
> > > > 
> > > > 
> > > > My problem is, whatever value I set, the video is not full screen 
> > > > mode, Anything I missed?
> > > > 
> > > > Thanks
> > > > William
> > > > 
> > > > 
> > > > 
> > > > 
> > > 
> > > > ___
> > > > dri-devel mailing list
> > > > dri-devel at lists.freedesktop.org
> > > > http://lists.freedesktop.org/mailman/listinfo/dri-devel
> > > 
> > > 
> > > --
> > > Ville Syrjälä
> > > Intel OTC
> > 
> > --
> > Ville Syrjälä
> > Intel OTC
> 
> --
> Ville Syrjälä
> Intel OTC

-- 
Ville Syrjälä
Intel OTC

[PATCH 1/2] drm/exynos: mixer: cleanup pixelformat handling

[Joonyoung Shim]

Hi Tobias,

On 04/24/2015 05:10 PM, Tobias Jakobi wrote:
> On 2015-04-24 03:48, Joonyoung Shim wrote:
>> Hi Tobias,
>>
>> On 04/23/2015 09:12 PM, 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.
>>>
>>> Partially based on 'drm/exynos: enable/disable blend based on pixel
>>> format' by Gustavo Padovan .
>>>
>>> Signed-off-by: Tobias Jakobi 
>>> ---
>>>  drivers/gpu/drm/exynos/exynos_mixer.c | 32 ++--
>>>  1 file changed, 22 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c 
>>> b/drivers/gpu/drm/exynos/exynos_mixer.c
>>> index fbec750..1bd23ee 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
>>> @@ -44,6 +44,12 @@
>>>  #define MIXER_WIN_NR3
>>>  #define MIXER_DEFAULT_WIN0
>>>
>>> +/* The pixelformats that are natively supported by the mixer. */
>>> +#define MIXER_PIXELFORMAT_RGB565 4
>>> +#define MIXER_PIXELFORMAT_ARGB1555 5
>>> +#define MIXER_PIXELFORMAT_ARGB 6
>>> +#define MIXER_PIXELFORMAT_ARGB 7
>>> +
>>
>> Seems be long, how about s/PIXELFORMAT/FORMAT or just MIXER_RGB565 ... ?
>> Please use tab between define name and value.
> How about MXR_FORMAT_XYZ, to stay consistent with the regs-mixer header?
> 
> 
>>>  struct mixer_resources {
>>>  intirq;
>>>  void __iomem*mixer_regs;
>>> @@ -531,20 +537,26 @@ static void mixer_graph_buffer(struct mixer_context 
>>> *ctx, int win)
>>>
>>>  plane = >planes[win];
>>>
>>> -#define RGB565 4
>>> -#define ARGB1555 5
>>> -#define ARGB 6
>>> -#define ARGB 7
>>> +switch (plane->pixel_format) {
>>> +case DRM_FORMAT_XRGB:
>>> +fmt = MIXER_PIXELFORMAT_ARGB;
>>> +break;
>>> +
>>> +case DRM_FORMAT_XRGB1555:
>>> +fmt = MIXER_PIXELFORMAT_ARGB1555;
>>> +break;
>>>
>>> -switch (plane->bpp) {
>>> -case 16:
>>> -fmt = ARGB;
>>> +case DRM_FORMAT_RGB565:
>>> +fmt = MIXER_PIXELFORMAT_RGB565;
>>>  break;
>>> -case 32:
>>> -fmt = ARGB;
>>> +
>>> +case DRM_FORMAT_XRGB:
>>> +fmt = MIXER_PIXELFORMAT_ARGB;
>>>  break;
>>> +
>>>  default:
>>> -fmt = ARGB;
>>> +DRM_DEBUG_KMS("pixelformat unsupported by mixer\n");
>>> +return;
>>>  }
>>>
>>>  /* check if mixer supports requested scaling setup */
>>>
>>
>> Hmm missing formats having alpha?
>> DRM_FORMAT_ARGB
>> DRM_FORMAT_ARGB1555
>> DRM_FORMAT_ARGB
> Should be obvious from my other mail. Blending/alpha is currently screwed up, 
> so we shouldn't add these formats at this point. At least not before figuring 
> out how to properly solve the issue.
> 

It shouldn't mean to remove current supported feature(alpha pixel format
plane support of mixer driver).

Thanks.

access "Display Port Helper Functions" trough userspace

[Jani Nikula]

On Sat, 18 Apr 2015, Marc Ludwig  wrote:
> Hi, Folks!
>  
> I'am looking for an opportunity to access the I2C-Interface which is attached
> to my displayport link.
> Can anyone give me a hint aubout an minimal working example for this?
>  
> I tried to figure aut how this could be solved using the linux userspace, but
> for me it looks like i have to implement my own driver
> according to https://www.kernel.org/doc/htmldocs/drm/ch02s05.html#idp10812112
>  this should be done by describing an
> drm_dp_aux_msg structure, which is passed into a driver's .transfer
>  implementation.

You may be conflating two things here. DP has a native AUX channel, and
I2C-over-AUX on top. If it's DDC you're after, you want I2C, and the I2C
adapters are exposed to userspace. You might want to have a look at
ddccontrol [1]. It's not minimal and seems to be abandoned, but last
time I tried I could use it to control my monitor.

If it's DP AUX you're after (why?) there was some discussion about
adding such an interface in the thread starting at [2].

BR,
Jani.


[1] http://ddccontrol.sourceforge.net/
[2] http://mid.gmane.org/1428493301-20293-1-git-send-email-durgadoss.r at 
intel.com


> Furthermore i was looking into some packages (the working machine runs an
> ArchLinux-Distro) which where provided.
> $ extra/libdrm 2.4.60-2 :Userspace interface to kernel DRM services
> $ extra/libva-intel-driver 1.5.1-1 : VA-API implementation for Intel G45 and 
> HD
> Graphics family (the working machine has an Intel HD4000 GPU)
> But none of them does provide such helper functions.
>  
> Thank you for your time, with kind Regards,
>  
>  
> Marc
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Jani Nikula, Intel Open Source Technology Center

[PATCH libdrm] Add missing includes

[Chih-Wei Huang]

2015-04-24 0:56 GMT+08:00 Emil Velikov :
> On 22/04/15 16:19, Greg Hackmann wrote:
>> On Tue, Apr 21, 2015 at 11:31 AM, Emil Velikov  
>> wrote:
>>>
>>> I'm not sure why I haven't hit this while building with Android-x86's
>>> bionic, although it's the right thing to do.
>>
>> Maybe a difference in bionic versions?  I know the bionic team made
>> some recent (post-5.1) changes that unintentionally exposed a handful
>> of other missing #includes in our tree.
>>
> Could be. I've never really bothered checking the bionic (both
> downstream patches and version) that Android-x86 uses.

Each Android-x86 branch is based on the respective
AOSP stable release.
That means bionic of kitkat-x86 is based on
ASOSP kitkat-mr2.2, while lollipop-x86 is based on
the lollipop-mr1 release.
It may be different than the AOSP master branch.

[PATCH v10 0/9] ASoC: tda998x: add a codec to the HDMI transmitter

[Jyri Sarha]

Hi,
Have you done anything about the tda998x audio support lately?

I was thinking of taking a shot at this now that I finally seem to have 
some time for it. However, if you are just about to send another series 
I'll wait for that first and see what makes the most sense after that.

My plan is to do something really simple first. No graph bindings or 
anything fancy like that. Just a minimal dt-binding to for audio pin 
configuration and use simple-card for the rest. I still try to make it 
possible to support spdif and i2s at the same time, but I can not test 
it as I do not have such HW.

I also try to make the ASoC part as generic as possible, so that it 
could be reused by other HDMI encoders with spdif or i2s interface.

Best regards,
Jyri

On 01/20/15 21:35, Jean-Francois Moine wrote:
> Based on broonie/ASoC
>
> v10:
>   - add the generic dt-card
>   - define the audio ports from a DT graph of ports (Russell King)
>   - reuse HDMI constants (Andrew Jackson - Jyri Sarha)
>   - alloc rate_constraints in codec (Jyri Sarha)
>   - fix bad number of channels (Jyri Sarha)
>   - correct codec generation from config (Russell King - Jyri Sarha)
>   - no module init/exit (Russell King)
> v9:
>   - back to a TDA998x specific CODEC
>   - more comments
>   - change magic values to constants
> v8:
>   - change some comments about the patches
> v7:
>   - remove the change of the K predivider (Jyri Sarha)
>   - add S24_3LE and S32_LE as possible audio formats (Jyri Sarha)
>   - don't move the struct priv2 definition and use the
> slave encoder private data as the device private data
> (Russell King)
>   - remove the useless request_module (Russell King/Mark Brown)
>   - don't lock the HDMI module (Russell King)
>   - use platform_device_unregister to remove the codec
> (Russell King)
> v6:
>   - extend the HDMI CODEC instead of using a specific CODEC
> v5:
>   - use the TDA998x private data instead of a specific area
> for the CODEC interface
>   - the CODEC is TDA998x specific (Mark Brown)
> v4:
>   - remove all the TDA998x specific stuff from the CODEC
>   - move the EDID scan from the CODEC to the TDA998x
>   - move the CODEC to sound/soc (Mark Brown)
>   - update the audio_sample_rate from the EDID (Andrew Jackson)
> v3: fix bad rate (Andrew Jackson)
> v2: check double stream start (Mark Brown)
>
> Jean-Francois Moine (9):
>ASoC: kirkwood: dynamically build the DAI array
>ASoC: kirkwood: check the DAI type from the DAI name
>ASoC: kirkwood: accept the DAI definitions from a graph of ports
>drm/i2c: tda998x: Add support of a DT graph of ports
>drm/i2c: tda998x: Change drvdata for audio extension
>ASoC: tda998x: add a codec to the HDMI transmitter
>drm/i2c: tda998x: set cts_n according to the sample width
>ASoC: core: export snd_soc_get_dai_name
>ASoC: add generic dt-card support
>
>   .../devicetree/bindings/drm/i2c/tda998x.txt|  51 
>   .../devicetree/bindings/sound/mvebu-audio.txt  |  30 +++
>   drivers/gpu/drm/i2c/tda998x_drv.c  | 237 +-
>   include/sound/soc.h|   2 +
>   include/sound/tda998x.h|  32 +++
>   sound/soc/codecs/Kconfig   |   5 +
>   sound/soc/codecs/Makefile  |   2 +
>   sound/soc/codecs/tda998x.c | 175 +
>   sound/soc/generic/Kconfig  |   2 +
>   sound/soc/generic/Makefile |   2 +
>   sound/soc/generic/dt-card.c| 275 
> +
>   sound/soc/kirkwood/kirkwood-i2s.c  | 141 ++-
>   sound/soc/kirkwood/kirkwood.h  |   4 +-
>   sound/soc/soc-core.c   |   5 +-
>   14 files changed, 877 insertions(+), 86 deletions(-)
>   create mode 100644 include/sound/tda998x.h
>   create mode 100644 sound/soc/codecs/tda998x.c
>   create mode 100644 sound/soc/generic/dt-card.c
>

[PATCH] DocBook/media: add CEC documentation

[Hans Verkuil]

(Kamil, here is the DocBook documentation for the CEC API. Please add this to 
your
patch series when you post the next version of that.)
---

Add the documentation for the public CEC API.

Signed-off-by: Hans Verkuil 
---
 Documentation/DocBook/media/Makefile   |   4 +-
 Documentation/DocBook/media/v4l/biblio.xml |  10 +
 Documentation/DocBook/media/v4l/cec-api.xml|  73 ++
 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 | 167 +
 .../DocBook/media/v4l/cec-ioc-g-event.xml  | 128 ++
 .../DocBook/media/v4l/cec-ioc-g-vendor-id.xml  |  70 ++
 .../DocBook/media/v4l/cec-ioc-receive.xml  | 176 +
 Documentation/DocBook/media_api.tmpl   |   6 +-
 15 files changed, 1386 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-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
 

+
+  HDMI2
+  
+   HDMI Licensing LLC
+(http://www.hdmi.org;>http://www.hdmi.org)
+  
+  High-Definition Multimedia Interface
+  Specification Version 2.0
+
+
 

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

[Kamil Debski]

Hi Hans,

Thank you so much for all today's comments. I will consider them when
preparing the next version.

From: linux-media-ow...@vger.kernel.org [mailto:linux-media-
owner at vger.kernel.org] On Behalf Of Hans Verkuil
Sent: Monday, April 27, 2015 1:27 PM
> 
> On 04/27/2015 12:25 PM, Hans Verkuil wrote:
> > 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
> >>
> >
> >> +  case CEC_S_ADAP_LOG_ADDRS: {
> >> +  struct cec_log_addrs log_addrs;
> >> +
> >> +  if (!(adap->capabilities & CEC_CAP_LOG_ADDRS))
> >> +  return -ENOTTY;
> >> +  if (copy_from_user(_addrs, parg, sizeof(log_addrs)))
> >> +  return -EFAULT;
> >> +  err = cec_claim_log_addrs(adap, _addrs, true);
> >
> > Currently CEC_S_ADAP_LOG_ADDRS is always blocking, but since we have
> > CEC_EVENT_READY I think it makes sense to just return in non-blocking
> > mode and have cec_claim_log_addrs generate CEC_EVENT_READY when done.
> > Userspace can then call G_ADAP_LOG_ADDRS to discover the result.
> >
> > What do you think?
> >

I am looking into this now. If I see this correctly this involves:
- adding cec_post_event(cla_int->adap, CEC_EVENT_READY); to
cec_config_thread_func
- adding O_NONBLOCK check in CEC_S_ADAP_LOG_ADDRS 
Right?

> On a related topic: non-blocking behavior for CEC_RECEIVE is well
> defined, but for CEC_TRA NSMIT it isn't. If reply == 0, then we need a
> way to inform userspace that the transmit finished (with a possible
> non-zero status code). An event would be suitable for that, but we
> would need a way to associate a transmit message with the event.
> 
> One possibility might be to have the CEC framework assign a sequence
> number to a transmit message which is returned by CEC_TRANSMIT and used
> in the event.
> 
> If reply != 0, then I think the received message should be queued up in
> the receive queue, but with a non-zero reply field and with the
> sequence number of the transmit message it is a reply of.

A sequence number is a good solution, I believe. To recap:
- a sequence number should be set by the framework and returned in the
CEC_TRANSMIT ioctl
- a new event should be added CEC_EVENT_TX_DONE and it should be posted on
each transmission
  finish 
- event struct has to include a sequence field as well
Is this ok?

> 
> Regards,
> 
>   Hans

Best wishes,
-- 
Kamil Debski
Samsung R Institute Poland

[PATCH 2/2] drm/exynos: mixer: remove buffer count handling in vp_video_buffer()

[Tobias Jakobi]

On 2015-04-27 09:24, Joonyoung Shim wrote:
> Hi Tobias,
> 
> On 04/26/2015 05:31 AM, Tobias Jakobi wrote:
>> The video processor (VP) supports four formats: NV12, NV21 and its
>> tiled variants. All these formats are bi-planar, so the buffer
>> count in vp_video_buffer() is always 2.
>> 
>> While we're at it, also add support for NV21 and properly exit
>> if we're called with an invalid (non-VP) pixelformat.
>> 
>> Signed-off-by: Tobias Jakobi 
>> ---
>>  drivers/gpu/drm/exynos/exynos_mixer.c | 20 +---
>>  1 file changed, 5 insertions(+), 15 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c 
>> b/drivers/gpu/drm/exynos/exynos_mixer.c
>> index 534a594..6fb4faa 100644
>> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
>> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
>> @@ -388,7 +388,6 @@ static void vp_video_buffer(struct mixer_context 
>> *ctx, int win)
>>  struct mixer_resources *res = >mixer_res;
>>  unsigned long flags;
>>  struct exynos_drm_plane *plane;
>> -unsigned int buf_num = 1;
>>  dma_addr_t luma_addr[2], chroma_addr[2];
>>  bool tiled_mode = false;
>>  bool crcb_mode = false;
>> @@ -399,27 +398,18 @@ static void vp_video_buffer(struct mixer_context 
>> *ctx, int win)
>>  switch (plane->pixel_format) {
>>  case DRM_FORMAT_NV12:
>>  crcb_mode = false;
>> -buf_num = 2;
>>  break;
>> -/* TODO: single buffer format NV12, NV21 */
>> +case DRM_FORMAT_NV21:
>> +crcb_mode = true;
>> +break;
> 
> To support NV21, how about make to other patch? because this patch is 
> to
> fix buffer number.
Sure, going to split this off.



>>  default:
>> -/* ignore pixel format at disable time */
>> -if (!plane->dma_addr[0])
>> -break;
>> -
>>  DRM_ERROR("pixel format for vp is wrong [%d].\n",
>>  plane->pixel_format);
>>  return;
>>  }
>> 
>> -if (buf_num == 2) {
>> -luma_addr[0] = plane->dma_addr[0];
>> -chroma_addr[0] = plane->dma_addr[1];
>> -} else {
>> -luma_addr[0] = plane->dma_addr[0];
>> -chroma_addr[0] = plane->dma_addr[0]
>> -+ (plane->pitch * plane->fb_height);
>> -}
>> +luma_addr[0] = plane->dma_addr[0];
>> +chroma_addr[0] = plane->dma_addr[1];
> 
> Hmm, dma_addr[0] and dma_addr[1] are same address if it uses just one
> buffer for two planes, then need offset information of each plane.
Good catch! I was already wondering why the colors were wrong, but 
that's of course to be expected if luma/chroma addr are the same.

I should probably apply offsets when writing dma_addr[i] to the plane 
object.


With best wishes,
Tobias


> Thanks.
> 
>> 
>>  if (plane->scan_flag & DRM_MODE_FLAG_INTERLACE) {
>>  ctx->interlace = true;
>> 

[PATCH 1/2] drm/exynos: mixer: cleanup pixelformat handling

[Tobias Jakobi]

On 2015-04-27 08:33, Joonyoung Shim wrote:
> Hi Tobias,
> 
> On 04/24/2015 05:10 PM, Tobias Jakobi wrote:
>> On 2015-04-24 03:48, Joonyoung Shim wrote:
>>> Hi Tobias,
>>> 
>>> On 04/23/2015 09:12 PM, 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.
 
 Partially based on 'drm/exynos: enable/disable blend based on pixel
 format' by Gustavo Padovan .
 
 Signed-off-by: Tobias Jakobi 
 ---
  drivers/gpu/drm/exynos/exynos_mixer.c | 32 
 ++--
  1 file changed, 22 insertions(+), 10 deletions(-)
 
 diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c 
 b/drivers/gpu/drm/exynos/exynos_mixer.c
 index fbec750..1bd23ee 100644
 --- a/drivers/gpu/drm/exynos/exynos_mixer.c
 +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
 @@ -44,6 +44,12 @@
  #define MIXER_WIN_NR3
  #define MIXER_DEFAULT_WIN0
 
 +/* The pixelformats that are natively supported by the mixer. */
 +#define MIXER_PIXELFORMAT_RGB565 4
 +#define MIXER_PIXELFORMAT_ARGB1555 5
 +#define MIXER_PIXELFORMAT_ARGB 6
 +#define MIXER_PIXELFORMAT_ARGB 7
 +
>>> 
>>> Seems be long, how about s/PIXELFORMAT/FORMAT or just MIXER_RGB565 
>>> ... ?
>>> Please use tab between define name and value.
>> How about MXR_FORMAT_XYZ, to stay consistent with the regs-mixer 
>> header?
>> 
>> 
  struct mixer_resources {
  intirq;
  void __iomem*mixer_regs;
 @@ -531,20 +537,26 @@ static void mixer_graph_buffer(struct 
 mixer_context *ctx, int win)
 
  plane = >planes[win];
 
 -#define RGB565 4
 -#define ARGB1555 5
 -#define ARGB 6
 -#define ARGB 7
 +switch (plane->pixel_format) {
 +case DRM_FORMAT_XRGB:
 +fmt = MIXER_PIXELFORMAT_ARGB;
 +break;
 +
 +case DRM_FORMAT_XRGB1555:
 +fmt = MIXER_PIXELFORMAT_ARGB1555;
 +break;
 
 -switch (plane->bpp) {
 -case 16:
 -fmt = ARGB;
 +case DRM_FORMAT_RGB565:
 +fmt = MIXER_PIXELFORMAT_RGB565;
  break;
 -case 32:
 -fmt = ARGB;
 +
 +case DRM_FORMAT_XRGB:
 +fmt = MIXER_PIXELFORMAT_ARGB;
  break;
 +
  default:
 -fmt = ARGB;
 +DRM_DEBUG_KMS("pixelformat unsupported by mixer\n");
 +return;
  }
 
  /* check if mixer supports requested scaling setup */
 
>>> 
>>> Hmm missing formats having alpha?
>>> DRM_FORMAT_ARGB
>>> DRM_FORMAT_ARGB1555
>>> DRM_FORMAT_ARGB
>> Should be obvious from my other mail. Blending/alpha is currently 
>> screwed up, so we shouldn't add these formats at this point. At least 
>> not before figuring out how to properly solve the issue.
>> 
> 
> It shouldn't mean to remove current supported feature(alpha pixel 
> format
> plane support of mixer driver).
Ah, I see what you mean! Going to send an updated version later this 
day.


With best wishes,
Tobias


> 
> Thanks.

[Bug 90194] Fix Valgrind error in drmPrimeHandleToFD

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

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

--- Comment #1 from Guillaume Desmottes  ---
Created attachment 115375
  --> https://bugs.freedesktop.org/attachment.cgi?id=115375=edit
drmPrimeHandleToFD: initialize args.fd to 0

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

[Bug 90194] Fix Valgrind error in drmPrimeHandleToFD

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

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

Bug ID: 90194
   Summary: Fix Valgrind error in drmPrimeHandleToFD
   Product: DRI
   Version: XOrg git
  Hardware: Other
OS: All
Status: NEW
  Severity: normal
  Priority: medium
 Component: libdrm
  Assignee: dri-devel at lists.freedesktop.org
  Reporter: guillaume.desmottes at collabora.co.uk

args.fd is left uninitialized making Valgrind angry.

==14518== Syscall param ioctl(generic) points to uninitialised byte(s)
==14518==at 0x3EB84F6407: ioctl (syscall-template.S:81)
==14518==by 0x3B01803B17: drmIoctl (xf86drm.c:175)
==14518==by 0x3B01806A7B: drmPrimeHandleToFD (xf86drm.c:2701)
==14518==by 0xDF0866D: drm_intel_bo_gem_export_to_prime
(intel_bufmgr_gem.c:2729)
==14518==by 0xDA92DF7: intel_query_image (intel_screen.c:563)
==14518==by 0x32E484E64D: dri3_alloc_render_buffer (dri3_glx.c:926)
==14518==by 0x32E484E64D: dri3_get_buffer.isra.10 (dri3_glx.c:1307)
==14518==by 0x32E484F499: dri3_get_buffers (dri3_glx.c:1466)
==14518==by 0xDAB0486: intel_update_image_buffers (brw_context.c:1475)
==14518==by 0xDAB0486: intel_update_renderbuffers (brw_context.c:1167)
==14518==by 0xDAB07B4: intel_prepare_render (brw_context.c:1188)
==14518==by 0xDAB0906: intelMakeCurrent (brw_context.c:1031)
==14518==by 0xDA51495: driBindContext (dri_util.c:538)
==14518==by 0x32E484DAA9: dri3_bind_context (dri3_glx.c:145)
==14518==by 0x32E481E5E6: MakeContextCurrent (glxcurrent.c:243)
==14518==by 0x8052F3C: gst_gl_context_glx_activate (gstglcontext_glx.c:401)
==14518==by 0x8036D51: gst_gl_context_activate (gstglcontext.c:632)
==14518==by 0x803882C: gst_gl_context_create_thread (gstglcontext.c:1282)
==14518==by 0x3EBB0703D4: g_thread_proxy (gthread.c:764)
==14518==by 0x3EB8807529: start_thread (pthread_create.c:310)
==14518==by 0x3EB850022C: clone (clone.S:109)
==14518==  Address 0xd793a18 is on thread 9's stack
==14518==  in frame #2, created by drmPrimeHandleToFD (xf86drm.c:2695)

-- 
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/20150427/d1950dc7/attachment.html>

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

[Hans Verkuil]

On 04/27/2015 11:22 AM, Hans Verkuil wrote:
> Hi Kamil,
> 
> Sorry for all the replies, but I'm writing the DocBook documentation, so
> whenever I find something missing I'll just reply to this patch.
>> +/* The CEC version */
> 
> Add support for version 1.3a here:
> 
> #define CEC_VERSION_1_3A  4
> 
>> +#define CEC_VERSION_1_4B5

Hmm, reading up on this I see that '5' is used for HDMI 1.4, 1.4a or 1.4b.

I think it should be renamed to VERSION_1_4 (i.e. drop the 'B').

Regards,

Hans

>> +#define CEC_VERSION_2_0 6
> 
> Regards,
> 
>   Hans
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

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

[Hans Verkuil]

On 04/27/2015 12:25 PM, Hans Verkuil wrote:
> 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
>>
> 
>> +case CEC_S_ADAP_LOG_ADDRS: {
>> +struct cec_log_addrs log_addrs;
>> +
>> +if (!(adap->capabilities & CEC_CAP_LOG_ADDRS))
>> +return -ENOTTY;
>> +if (copy_from_user(_addrs, parg, sizeof(log_addrs)))
>> +return -EFAULT;
>> +err = cec_claim_log_addrs(adap, _addrs, true);
> 
> Currently CEC_S_ADAP_LOG_ADDRS is always blocking, but since we have 
> CEC_EVENT_READY
> I think it makes sense to just return in non-blocking mode and have 
> cec_claim_log_addrs
> generate CEC_EVENT_READY when done. Userspace can then call G_ADAP_LOG_ADDRS 
> to discover
> the result.
> 
> What do you think?
> 

On a related topic: non-blocking behavior for CEC_RECEIVE is well defined, but 
for
CEC_TRANSMIT it isn't. If reply == 0, then we need a way to inform userspace 
that
the transmit finished (with a possible non-zero status code). An event would be
suitable for that, but we would need a way to associate a transmit message with
the event.

One possibility might be to have the CEC framework assign a sequence number to
a transmit message which is returned by CEC_TRANSMIT and used in the event.

If reply != 0, then I think the received message should be queued up in the
receive queue, but with a non-zero reply field and with the sequence number of 
the
transmit message it is a reply of.

Regards,

Hans

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

[Hans Verkuil]

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
> 

> + case CEC_S_ADAP_LOG_ADDRS: {
> + struct cec_log_addrs log_addrs;
> +
> + if (!(adap->capabilities & CEC_CAP_LOG_ADDRS))
> + return -ENOTTY;
> + if (copy_from_user(_addrs, parg, sizeof(log_addrs)))
> + return -EFAULT;
> + err = cec_claim_log_addrs(adap, _addrs, true);

Currently CEC_S_ADAP_LOG_ADDRS is always blocking, but since we have 
CEC_EVENT_READY
I think it makes sense to just return in non-blocking mode and have 
cec_claim_log_addrs
generate CEC_EVENT_READY when done. Userspace can then call G_ADAP_LOG_ADDRS to 
discover
the result.

What do you think?

Regards,

Hans

> + if (err)
> + return err;
> +
> + if (copy_to_user(parg, _addrs, sizeof(log_addrs)))
> + return -EFAULT;
> + break;
> + }

[PATCH 4/4] drm/radeon: fix userptr return value checking

[Alex Deucher]

On Mon, Apr 27, 2015 at 11:04 AM, Christian König
 wrote:
> From: Christian König 
>
> Otherwise we print false warning from time to time.
>
> Signed-off-by: Christian König 
> Signed-off-by: Jack Xiao 
> CC: stable at vger.kernel.org

Applied the series to my 4.1 fixes tree.  I had to make minor changes
to 4/4 so that it would apply:
http://cgit.freedesktop.org/~agd5f/linux/log/?h=drm-fixes-4.1-wip

Alex


> ---
>  drivers/gpu/drm/radeon/radeon_mn.c | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_mn.c 
> b/drivers/gpu/drm/radeon/radeon_mn.c
> index 572b4db..30e6207 100644
> --- a/drivers/gpu/drm/radeon/radeon_mn.c
> +++ b/drivers/gpu/drm/radeon/radeon_mn.c
> @@ -122,26 +122,26 @@ static void radeon_mn_invalidate_range_start(struct 
> mmu_notifier *mn,
> it = interval_tree_iter_first(>objects, start, end);
> while (it) {
> struct radeon_bo *bo;
> -   int r;
> +   long r;
>
> bo = container_of(it, struct radeon_bo, mn_it);
> it = interval_tree_iter_next(it, start, end);
>
> r = radeon_bo_reserve(bo, true);
> if (r) {
> -   DRM_ERROR("(%d) failed to reserve user bo\n", r);
> +   DRM_ERROR("(%ld) failed to reserve user bo\n", r);
> continue;
> }
>
> r = reservation_object_wait_timeout_rcu(bo->tbo.resv, true,
> false, MAX_SCHEDULE_TIMEOUT);
> -   if (r)
> -   DRM_ERROR("(%d) failed to wait for user bo\n", r);
> +   if (r <= 0)
> +   DRM_ERROR("(%ld) failed to wait for user bo\n", r);
>
> radeon_ttm_placement_from_domain(bo, RADEON_GEM_DOMAIN_CPU);
> r = ttm_bo_validate(>tbo, >placement, false, false);
> if (r)
> -   DRM_ERROR("(%d) failed to validate user bo\n", r);
> +   DRM_ERROR("(%ld) failed to validate user bo\n", r);
>
> radeon_bo_unreserve(bo);
> }
> --
> 1.9.1
>

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

[Hans Verkuil]

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/Documentation/cec.txt b/Documentation/cec.txt
> new file mode 100644
> index 000..2b6c08a
> --- /dev/null
> +++ b/Documentation/cec.txt
> @@ -0,0 +1,396 @@
> +- CEC_G_ADAP_LOG_ADDRS and CEC_S_ADAP_LOG_ADDRS
> +
> +These ioctl are used to configure the logical addresses of the CEC adapter.
> +
> +#define CEC_G_ADAP_LOG_ADDRS _IOR('a', 3, struct cec_log_addrs)
> +#define CEC_S_ADAP_LOG_ADDRS _IOWR('a', 4, struct cec_log_addrs)
> +
> +The struct cec_log_addrs is following:
> +
> +struct cec_log_addrs {
> + __u8 cec_version;
> + __u8 num_log_addrs;
> + __u8 primary_device_type[CEC_MAX_LOG_ADDRS];
> + __u8 log_addr_type[CEC_MAX_LOG_ADDRS];
> + __u8 log_addr[CEC_MAX_LOG_ADDRS];
> +
> + /* CEC 2.0 */
> + __u8 all_device_types;
> + __u8 features[CEC_MAX_LOG_ADDRS][12];
> +
> + __u8 reserved[9];
> +};
> +
> +The cec_version determines which CEC version should be used.
> +
> +/* The CEC version */
> +#define CEC_VERSION_1_4B 5
> +#define CEC_VERSION_2_0  6
> +
> +It will try to claim num_log_addrs devices. The log_addr_type array has
> +the logical address type that needs to be claimed for that device, and
> +the log_addr array will receive the actual logical address that was
> +claimed for that device or 0xff if no address could be claimed.
> +
> +The primary_device_type contains the primary device for each logical
> +address.
> +
> +For CEC 2.0 devices fill in the all_device_types parameter to use with the
> +Report Features command, and fill in the 'features' which contains the
> +remaining parameters (RC Profile and Device Features) to use in Report
> +Features.
> +
> +An error is returned if the adapter is disabled or if there
> +is no physical address assigned or if the cec_version is unknown.
> +
> +If no logical address of one or more of the given types could be claimed,
> +then log_addr will be set to CEC_LOG_ADDR_INVALID.

This does not appear to be the case looking at the cec_config_log_addrs 
function.
I don't see it being set to INVALID if it couldn't be claimed. I think that is
missing in the cec_config_log_addrs function.

Regards,

Hans

Build regressions/improvements in v4.1-rc1

[Geert Uytterhoeven]

On Mon, Apr 27, 2015 at 11:51 AM, Geert Uytterhoeven
 wrote:
> Below is the list of build error/warning regressions/improvements in
> v4.1-rc1[1] compared to v4.0[2].
>
> Summarized:
>   - build errors: +34/-11
>   - build warnings: +135/-163
>
> As I haven't mastered kup yet, there's no verbose summary at
> http://www.kernel.org/pub/linux/kernel/people/geert/linux-log/v4.1-rc1.summary.gz
>
> Happy fixing! ;-)
>
> Thanks to the linux-next team for providing the build service.
>
> [1] http://kisskb.ellerman.id.au/kisskb/head/8779/ (254 out of 257 configs)
> [2] http://kisskb.ellerman.id.au/kisskb/head/8710/ (254 out of 257 configs)
>
>
> *** ERRORS ***
>
> 34 regressions:

The quiet days are over...

>   + /home/kisskb/slave/src/arch/mips/cavium-octeon/smp.c: error: passing 
> argument 2 of 'cpumask_clear_cpu' discards 'volatile' qualifier from pointer 
> target type [-Werror]:  => 242:2
>   + /home/kisskb/slave/src/arch/mips/kernel/process.c: error: passing 
> argument 2 of 'cpumask_test_cpu' discards 'volatile' qualifier from pointer 
> target type [-Werror]:  => 52:2
>   + /home/kisskb/slave/src/arch/mips/kernel/smp.c: error: passing argument 2 
> of 'cpumask_set_cpu' discards 'volatile' qualifier from pointer target type 
> [-Werror]:  => 149:2, 211:2
>   + /home/kisskb/slave/src/arch/mips/kernel/smp.c: error: passing argument 2 
> of 'cpumask_test_cpu' discards 'volatile' qualifier from pointer target type 
> [-Werror]:  => 221:2

mips/bigsur_defconfig
mips/malta_defconfig
mips/cavium_octeon_defconfig
mips/ip27_defconfig

and related warnings due to lack of -Werror on
ia64-defconfig
tilegx_defconfig
m32r/m32700ut.smp_defconfig

cpumask also gives fishy warnings:

lib/cpumask.c:167:25: warning: the address of 'cpu_all_bits' will
always evaluate as 'true' [-Waddress]

on sparc (e.g. sparc64/sparc64-allmodconfig) and powerpc (e.g.
powerpc/ppc64_defconfig), which seem to have been reported 6 months ago...

Can we throw some bitcoins at the cpumasks? ;-)

>   + /home/kisskb/slave/src/arch/mips/sgi-ip32/ip32-platform.c: error: 
> 'sgio2_cmos_devinit' undeclared here (not in a function):  => 138:1
>   + /home/kisskb/slave/src/arch/mips/sgi-ip32/ip32-platform.c: error: 
> expected identifier or '(' before '+' token:  => 133:1

mips/ip32_defconfig

>   + /home/kisskb/slave/src/arch/powerpc/include/asm/spinlock.h: error: 
> "arch_read_can_lock" redefined [-Werror]:  => 185:0
>   + /home/kisskb/slave/src/arch/powerpc/include/asm/spinlock.h: error: 
> "arch_write_can_lock" redefined [-Werror]:  => 186:0
>   + /home/kisskb/slave/src/arch/powerpc/include/asm/spinlock.h: error: 
> "smp_mb__after_unlock_lock" redefined [-Werror]:  => 31:0
>   + /home/kisskb/slave/src/arch/powerpc/include/asm/spinlock.h: error: 
> 'arch_rwlock_t' has no member named 'lock':  => 267:12, 303:4, 214:12, 
> 295:11, 253:12, 238:25
>   + /home/kisskb/slave/src/arch/powerpc/include/asm/spinlock.h: error: 
> 'arch_spinlock_t' has no member named 'slock':  => 86:27, 59:13
>   + /home/kisskb/slave/src/arch/powerpc/include/asm/spinlock.h: error: 
> expected ')' before '(' token:  => 62:19
>   + /home/kisskb/slave/src/arch/powerpc/include/asm/spinlock.h: error: 
> expected identifier or '(' before 'do':  => 159:20, 139:6, 168:13, 123:20
>   + /home/kisskb/slave/src/arch/powerpc/include/asm/spinlock.h: error: 
> expected identifier or '(' before 'void':  => 62:19
>   + /home/kisskb/slave/src/arch/powerpc/include/asm/spinlock.h: error: 
> expected identifier or '(' before 'while':  => 168:13, 123:20, 139:6, 159:20
>   + /home/kisskb/slave/src/arch/powerpc/include/asm/spinlock.h: error: 
> expected identifier or '(' before '{' token:  => 92:19

powerpc/ppc64_defconfig+UP

>   + 
> /home/kisskb/slave/src/drivers/gpu/drm/nouveau/nvkm/subdev/instmem/gk20a.c: 
> error: implicit declaration of function 'dma_alloc_attrs' 
> [-Werror=implicit-function-declaration]:  => 218:2
>   + 
> /home/kisskb/slave/src/drivers/gpu/drm/nouveau/nvkm/subdev/instmem/gk20a.c: 
> error: implicit declaration of function 'dma_free_attrs' 
> [-Werror=implicit-function-declaration]:  => 154:2

xtensa-allmodconfig
powerpc/mpc85xx_defconfig

>   + /home/kisskb/slave/src/drivers/spi/spi-bcm2835.c: error: dereferencing 
> pointer to incomplete type:  => 356:21, 387:21
>   + /home/kisskb/slave/src/drivers/spi/spi-bcm2835.c: error: implicit 
> declaration of function 'gpiochip_find' 
> [-Werror=implicit-function-declaration]:  => 382:2

m68k-allmodconfig
s390-allyesconfig
s390-allmodconfig
parisc-allmodconfig
sh-allmodconfig
sh-allyesconfig
mips-allmodconfig

(fix stuck in the spi tree?)

>   + /home/kisskb/slave/src/fs/dax.c: error: implicit declaration of function 
> 'copy_user_page' [-Werror=implicit-function-declaration]:  => 265:2

sh-randconfig

>   + /home/kisskb/slave/src/fs/hostfs/hostfs_user.c: error: 'AT_FDCWD' 
> undeclared (first use in this function):  => 373
>   + /home/kisskb/slave/src/fs/hostfs/hostfs_user.c: error: (Each undeclared 
> identifier is reported only 

[PATCH 2/2] drm: fix a memleak on mutex failure path

[Jani Nikula]

On Sun, 26 Apr 2015, green at linuxhacker.ru wrote:
> From: Oleg Drokin 
>
> Need to free just allocated ctx allocation if we cannot
> get our config mutex.
>
> This one has been flagged by kbuild bot all the way back in August,
> but somehow nobody picked it up:
> https://lists.01.org/pipermail/kbuild/2014-August/001691.html
>
> Found with smatch.
>
> Signed-off-by: Oleg Drokin 
> CC: Daniel Vetter 

The function has another leaking failure path, would be nice to have
that fixed too. Maybe with a common out label.

BR,
Jani.



> ---
>  drivers/gpu/drm/drm_modeset_lock.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_modeset_lock.c 
> b/drivers/gpu/drm/drm_modeset_lock.c
> index 51cc47d..1e8c52f 100644
> --- a/drivers/gpu/drm/drm_modeset_lock.c
> +++ b/drivers/gpu/drm/drm_modeset_lock.c
> @@ -80,8 +80,10 @@ int __drm_modeset_lock_all(struct drm_device *dev,
>   return -ENOMEM;
>  
>   if (trylock) {
> - if (!mutex_trylock(>mutex))
> + if (!mutex_trylock(>mutex)) {
> + kfree(ctx);
>   return -EBUSY;
> + }
>   } else {
>   mutex_lock(>mutex);
>   }
> -- 
> 2.1.0
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Jani Nikula, Intel Open Source Technology Center

[Bug 95911] Recursive error in radeon device driver module after resume from hibernation

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

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

--- Comment #7 from gitne at excite.co.jp ---
(In reply to Michel Dänzer from comment #6)
> Is this a regression compared to older kernel versions? If yes, can you
> bisect or at least narrow down which kernel version is the last good / first
> bad one?

Unfortunately, I cannot tell if this bug is a regression or new. I have started
to experience this bug only with my new piece of hardware AMD Radeon R7. I have
not tested specifically on a range different kernel versions either, nor intend
to bisect anything. Sorry, but I am using my machine to do actual work and do
not have time to do this kind of testing nor have the required knowledge to
compile the kernel. So no, I am not a kernel developer. If I were, I would
probably fix this and submit a patch but I am not.

The AMD APU Radeon R7 should be a common piece of hardware by now, and very
similar to any other APU Radeon R* so testing should be relatively easy for
kernel developers.

All I can say is that I have went through all the F21 kernel versions with the
same annoying effect. Since, Fedora kernel versions are somewhat lagging a bit
behind the upstream kernel versions, and hibernation not being a No. 1 test
feature, plus the R7 being a relatively new piece of hardware makes me assume
that this has not been fixed in the latest kernel versions either. I would
really love hibernation to work because it is a very practical and time saving
feature.

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

[PATCH v2] drm: fix a memleak on mutex failure path

[gr...@linuxhacker.ru]

From: Oleg Drokin 

Need to free just allocated ctx allocation if we cannot
get our config mutex.

This one has been flagged by kbuild bot all the way back in August,
but somehow nobody picked it up:
https://lists.01.org/pipermail/kbuild/2014-August/001691.html

In addition there is another failure path that leaks the same
ctx reference that is fixed.

Found with smatch.

Signed-off-by: Oleg Drokin 
CC: Daniel Vetter 
---
 drivers/gpu/drm/drm_modeset_lock.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_modeset_lock.c 
b/drivers/gpu/drm/drm_modeset_lock.c
index 51cc47d..c0a5cd8 100644
--- a/drivers/gpu/drm/drm_modeset_lock.c
+++ b/drivers/gpu/drm/drm_modeset_lock.c
@@ -80,8 +80,10 @@ int __drm_modeset_lock_all(struct drm_device *dev,
return -ENOMEM;

if (trylock) {
-   if (!mutex_trylock(>mutex))
-   return -EBUSY;
+   if (!mutex_trylock(>mutex)) {
+   ret = -EBUSY;
+   goto out;
+   }
} else {
mutex_lock(>mutex);
}
@@ -114,6 +116,8 @@ fail:
goto retry;
}

+out:
+   kfree(ctx);
return ret;
 }
 EXPORT_SYMBOL(__drm_modeset_lock_all);
-- 
2.1.0

[PATCH 2/2] drm: fix a memleak on mutex failure path

[Oleg Drokin]

Hello!

On Apr 27, 2015, at 4:56 AM, Jani Nikula wrote:

> On Sun, 26 Apr 2015, green at linuxhacker.ru wrote:
>> From: Oleg Drokin 
>> 
>> Need to free just allocated ctx allocation if we cannot
>> get our config mutex.
>> 
>> This one has been flagged by kbuild bot all the way back in August,
>> but somehow nobody picked it up:
>> https://lists.01.org/pipermail/kbuild/2014-August/001691.html
>> 
>> Found with smatch.
>> 
>> Signed-off-by: Oleg Drokin 
>> CC: Daniel Vetter 
> 
> The function has another leaking failure path, would be nice to have
> that fixed too. Maybe with a common out label.

Hm, you are right, there's another path, though it's less obvious since
what is done with ctx inside of those other calls, but apparently nothing
if there is an error, so it's also a leak.

I'll do an updated patch in a moment.

Bye,
Oleg

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

[Hans Verkuil]

On 04/27/2015 11:22 AM, Hans Verkuil wrote:
> Hi Kamil,
> 
> Sorry for all the replies, but I'm writing the DocBook documentation, so
> whenever I find something missing I'll just reply to this patch.
>> +/* The CEC version */
> 
> Add support for version 1.3a here:
> 
> #define CEC_VERSION_1_3A  4

To clarify: the core CEC framework will not support 1.3A, but of course
other devices can report it, so we do need the define.

Regards,

Hans

> 
>> +#define CEC_VERSION_1_4B5
>> +#define CEC_VERSION_2_0 6
> 
> Regards,
> 
>   Hans
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

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

[Hans Verkuil]

Hi Kamil,

Sorry for all the replies, but I'm writing the DocBook documentation, so
whenever I find something missing I'll just reply to this patch.
> +/* The CEC version */

Add support for version 1.3a here:

#define CEC_VERSION_1_3A4

> +#define CEC_VERSION_1_4B 5
> +#define CEC_VERSION_2_0  6

Regards,

Hans

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

[Hans Verkuil]

Hi Kamil,

I've added some missing HDMI 2.0 commands:

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/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;
> +};
> +
> +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 cec_msg *msg)
> +{
> + return msg->msg[0] >> 4;
> +}
> +
> +static inline __u8 cec_msg_destination(const struct cec_msg *msg)
> +{
> + return msg->msg[0] & 0xf;
> +}
> +
> +static inline bool cec_msg_is_broadcast(const struct cec_msg *msg)
> +{
> + return (msg->msg[0] & 0xf) == 0xf;
> +}
> +
> +/* cec status field */
> +#define CEC_TX_STATUS_OK(0)
> +#define CEC_TX_STATUS_ARB_LOST  (1 << 0)
> +#define CEC_TX_STATUS_RETRY_TIMEOUT (1 << 1)
> +#define CEC_TX_STATUS_FEATURE_ABORT (1 << 2)
> +#define CEC_TX_STATUS_REPLY_TIMEOUT (1 << 3)
> +#define CEC_RX_STATUS_READY (0)
> +
> +#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_TV  0
> +#define CEC_PRIM_DEVTYPE_RECORD  1
> +#define CEC_PRIM_DEVTYPE_TUNER   3
> +#define CEC_PRIM_DEVTYPE_PLAYBACK4
> +#define CEC_PRIM_DEVTYPE_AUDIOSYSTEM 5
> +#define CEC_PRIM_DEVTYPE_SWITCH  6
> +#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 

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

[Jani Nikula]

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 


> ---
>  drivers/gpu/drm/i915/intel_drv.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h 
> b/drivers/gpu/drm/i915/intel_drv.h
> index 897f17d..d52f267 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1310,7 +1310,7 @@ intel_atomic_get_crtc_state(struct drm_atomic_state 
> *state,
>   struct drm_crtc_state *crtc_state;
>   crtc_state = drm_atomic_get_crtc_state(state, >base);
>   if (IS_ERR(crtc_state))
> - return ERR_PTR(PTR_ERR(crtc_state));
> + return ERR_CAST(crtc_state);
>  
>   return to_intel_crtc_state(crtc_state);
>  }
> -- 
> 1.9.1
>

-- 
Jani Nikula, Intel Open Source Technology Center

[PATCH 2/2] Fix one warning (v2)

[Jammy Zhou]

xf86drm.c:356:2: warning: comparison of unsigned expression >= 0 is always true 
[-Wtype-limits]
  group = (serv_group >= 0) ? serv_group : DRM_DEV_GID;
  ^

v2: do 'int' cast to fix the warning

Signed-off-by: Jammy Zhou 
---
 xf86drm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xf86drm.c b/xf86drm.c
index 4d67861..4ecb24f 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -353,7 +353,7 @@ static int drmOpenDevice(dev_t dev, int minor, int type)
 }

 if (drm_server_info) {
-   group = (serv_group >= 0) ? serv_group : DRM_DEV_GID;
+   group = ((int)serv_group >= 0) ? serv_group : DRM_DEV_GID;
chown_check_return(buf, user, group);
chmod(buf, devmode);
 }
-- 
1.9.1

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

[Hans Verkuil]

Hi Lars,

My thanks as well for your comments.

I'd like to add some background information as well as to why we move
the core CEC support into the kernel: the main reason for doing this
is to support the HEAC part of the CEC protocol. Specifically the ARC
support and handling the hotplug detect CEC/HEAC messages. This has to
be handled in the kernel and cannot be left to userspace. While the
current framework does not yet handle these messages support for this
will appear, probably later this year since I will have to work on ARC.

Out of curiosity: have you ever seen CEC adapters that implement the
ethernet part of HEAC? My understanding is that nobody uses that part
since wifi is the standard these days. But perhaps you know of examples
where it was actually implemented.

Regards,

Hans

On 04/27/2015 10:09 AM, Kamil Debski wrote:
> Hi Lars, 
> 
> Thank you for your comments.
> 
> From: linux-media-owner at vger.kernel.org [mailto:linux-media-
> owner at vger.kernel.org] On Behalf Of Lars Op den Kamp
> Sent: Friday, April 24, 2015 12:04 PM
>  
>> Hi Kamil, Hans,
>>
>> I'm the main developer of libCEC
>> (https://github.com/Pulse-Eight/libcec). Sorry for the late time to
>> jump
>> in here, but I wasn't signed up to this mailing list and someone
>> pointed
>> me to this discussion.
>>
>> Unfortunately this approach will not work with half the TVs that are
>> out
>> there. I'll explain why:
>>
>> * because of how some (common) brands implemented CEC in their TVs,
>> this
>> implementation will not work, as the TV will just reject it. In libCEC,
>> we've created work arounds for brands like this. Without these work
>> arounds, your in-kernel implementation will be very vendor specific.
>> e.g. this implementation will work for Samsung's TVs, but not for the
>> TVs made by another big TV brand. All commands, including CEC_OP_ABORT,
>> should be passed to userspace to make it work with all brands.
>>
>> * it should be made possible to not have the kernel send any CEC
>> message, try to process any received CEC message, or ack to any logical
>> address at all, to allow libraries like libCEC to fully handle all CEC
>> traffic. Some brands only enable routing of some CEC keys when a
>> specific device type is used. libCEC will allocate a logical address of
>> the correct type for the brand that's used. If another address is first
>> allocated by the kernel, and the TV communicates with it to find out
>> it's name and things like that, and libCEC allocates another address a
>> bit later when initialised, then you'll end up with multiple entries in
>> the device list on the TV, and only one of them will work.
> 
> Adding a special mode in the CEC framework that disables parsing and
> processing seems like a good idea for me. This way libCEC could be
> completely
> in charge of how the communication is handled. 
> 
> I discussed this with Hans and he is for this solution. This way there would
> be two modes:
> - One with handling of CEC messages enabled in the kernel, in idea behind
>   this is to have processing adhere to the CEC spec as closely as possible.
>   It should work with equipment that also follows the spec and has little
>   vendor specific quirks.
> - Second, the passthrough mode, in which the handling of CEC messages would
>   be left to userspace application. Kernel would not be sending or
>   receiving messages, unless specifically told to do so. Below you mentioned
>   that allocating logical addresses and sending ACKs could be done in
> kernel.
> 
>   The way I see it is following: If allocation of a logical address is made
>   then ACKs will be handled by the framework. If no allocation is made then
>   the userspace can still send and receive messages. However no filtering is
>   done based on the logical address - all received messages are sent to the
>   userspace.
> 
>>
>> * CEC is *very* vendor specific. The main reason is, in my opinion, the
>> use of the word "should" instead of "shall" in the spec. It's addressed
>> in the new version, but it'll take years before all the non 2.x devices
>> are gone. What works for vendor A will simply not work for vendor B.
>> libCEC aims to address this, in a library that can be used on all major
>> platforms and by all major programming languages. You could duplicate
>> the work done there in the kernel to make make the implementation work
>> with all brands, but I think that this does simply not belong in the
>> kernel when it can be handled in userspace perfectly.
> 
> CEC being very vendor specific is a huge problem. I agree with you that
> there is no need to duplicate the effort to mitigate all the vendor quirks.
> Especially that a working implementation (libCEC) is already done.
> 
>> So I suggest that you limit the in-kernel implementation to handling
>> raw
>> traffic only, to have it do this (and nothing more):
>> * allocate one or more logical addresses, and ack CEC traffic sent to
>> those logical addresses
>> * receive CEC 

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

[Kamil Debski]

Hi Lars, 

Thank you for your comments.

From: linux-media-ow...@vger.kernel.org [mailto:linux-media-
owner at vger.kernel.org] On Behalf Of Lars Op den Kamp
Sent: Friday, April 24, 2015 12:04 PM

> Hi Kamil, Hans,
> 
> I'm the main developer of libCEC
> (https://github.com/Pulse-Eight/libcec). Sorry for the late time to
> jump
> in here, but I wasn't signed up to this mailing list and someone
> pointed
> me to this discussion.
> 
> Unfortunately this approach will not work with half the TVs that are
> out
> there. I'll explain why:
> 
> * because of how some (common) brands implemented CEC in their TVs,
> this
> implementation will not work, as the TV will just reject it. In libCEC,
> we've created work arounds for brands like this. Without these work
> arounds, your in-kernel implementation will be very vendor specific.
> e.g. this implementation will work for Samsung's TVs, but not for the
> TVs made by another big TV brand. All commands, including CEC_OP_ABORT,
> should be passed to userspace to make it work with all brands.
>
> * it should be made possible to not have the kernel send any CEC
> message, try to process any received CEC message, or ack to any logical
> address at all, to allow libraries like libCEC to fully handle all CEC
> traffic. Some brands only enable routing of some CEC keys when a
> specific device type is used. libCEC will allocate a logical address of
> the correct type for the brand that's used. If another address is first
> allocated by the kernel, and the TV communicates with it to find out
> it's name and things like that, and libCEC allocates another address a
> bit later when initialised, then you'll end up with multiple entries in
> the device list on the TV, and only one of them will work.

Adding a special mode in the CEC framework that disables parsing and
processing seems like a good idea for me. This way libCEC could be
completely
in charge of how the communication is handled. 

I discussed this with Hans and he is for this solution. This way there would
be two modes:
- One with handling of CEC messages enabled in the kernel, in idea behind
  this is to have processing adhere to the CEC spec as closely as possible.
  It should work with equipment that also follows the spec and has little
  vendor specific quirks.
- Second, the passthrough mode, in which the handling of CEC messages would
  be left to userspace application. Kernel would not be sending or
  receiving messages, unless specifically told to do so. Below you mentioned
  that allocating logical addresses and sending ACKs could be done in
kernel.

  The way I see it is following: If allocation of a logical address is made
  then ACKs will be handled by the framework. If no allocation is made then
  the userspace can still send and receive messages. However no filtering is
  done based on the logical address - all received messages are sent to the
  userspace.

> 
> * CEC is *very* vendor specific. The main reason is, in my opinion, the
> use of the word "should" instead of "shall" in the spec. It's addressed
> in the new version, but it'll take years before all the non 2.x devices
> are gone. What works for vendor A will simply not work for vendor B.
> libCEC aims to address this, in a library that can be used on all major
> platforms and by all major programming languages. You could duplicate
> the work done there in the kernel to make make the implementation work
> with all brands, but I think that this does simply not belong in the
> kernel when it can be handled in userspace perfectly.

CEC being very vendor specific is a huge problem. I agree with you that
there is no need to duplicate the effort to mitigate all the vendor quirks.
Especially that a working implementation (libCEC) is already done.

> So I suggest that you limit the in-kernel implementation to handling
> raw
> traffic only, to have it do this (and nothing more):
> * allocate one or more logical addresses, and ack CEC traffic sent to
> those logical addresses
> * receive CEC traffic and forward it to userspace (traffic sent to all
> addresses is preferred, not just traffic sent to the logical address
> used by the device running this code)
> * transmit CEC traffic initiated by userspace

As mentioned above, I propose a "passthorugh" mode in which handling of
CEC messages by the kernel CEC framework will be very limited. I think
that the three functions listed above should be enough.

Any comments on this solution?

> 
> thanks,
> 
> Lars Op den Kamp

Best wishes,
-- 
Kamil Debski
Samsung R Institute Poland

> 
> 
> On 23-04-15 15:03, 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
> > 

[Bug 90182] [radeonsi]Qt Applications won't start on the dedicated GPU (SIGABRT)

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

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

--- Comment #11 from Paul  ---
I "think" it worked:

#3  radeon_drm_cs_emit_ioctl (param=param at entry=0x689970) at
radeon_drm_winsys.c:594
ws = 0x689970
cs = 
i = 
#4  0x7fffdf12a157 in impl_thrd_routine (p=) at
../../../../../include/c11/threads_posix.h:87
pack = {func = 0x7fffdf12a920 , arg =
0x689970}

-- 
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/20150427/e32f1f41/attachment.html>

[Bug 90182] [radeonsi]Qt Applications won't start on the dedicated GPU (SIGABRT)

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

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

Paul  changed:

   What|Removed |Added

 Attachment #115367|text/plain  |application/zip
  mime type||

-- 
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/20150427/d0167b6e/attachment.html>

[Bug 90182] [radeonsi]Qt Applications won't start on the dedicated GPU (SIGABRT)

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

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

--- Comment #10 from Paul  ---
Created attachment 115369
  --> https://bugs.freedesktop.org/attachment.cgi?id=115369=edit
Radeonsi debug log (hopefully with symbols)

-- 
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/20150427/0f225526/attachment-0001.html>

[Bug 90182] [radeonsi]Qt Applications won't start on the dedicated GPU (SIGABRT)

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

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

--- Comment #9 from Michel Dänzer  ---
(In reply to Paul from comment #6)
> "/usr/lib/xorg/modules/dri/radeonsi_dri.so" is not a core dump: File format
> not recognized

It says it's not a core dump, which is correct. Have you tried getting a
backtrace of the crash again?

-- 
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/20150427/776f4ff6/attachment.html>

[Bug 90182] [radeonsi]Qt Applications won't start on the dedicated GPU (SIGABRT)

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

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

--- Comment #8 from Paul  ---
Created attachment 115368
  --> https://bugs.freedesktop.org/attachment.cgi?id=115368=edit
Makepkg.conf used for building mesa

-- 
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/20150427/e6ac2526/attachment.html>

[Bug 90182] [radeonsi]Qt Applications won't start on the dedicated GPU (SIGABRT)

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

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

--- Comment #7 from Paul  ---
Created attachment 115367
  --> https://bugs.freedesktop.org/attachment.cgi?id=115367=edit
Modified PKGBUILD for debug build of mesa

-- 
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/20150427/bb606d64/attachment.html>

[Bug 90182] [radeonsi]Qt Applications won't start on the dedicated GPU (SIGABRT)

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

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

--- Comment #6 from Paul  ---
I tried building mesa with debug symbols but somehow gdb says that there are
still none for /usr/lib/xorg/modules/dri/radeonsi_dri.so

This is what I did: 
I used the archlinux PKGBUILD and added --enable-debug to the configure options
and 
added "-g -O2" to CFLAGS and CXXFLAGS respectively. This gave me a few packages
I then installed. The packages were much larger than the original arch ones so
I think the symbols got built.
But when I do LANG=C gdb info /usr/lib/xorg/modules/dri/radeonsi_dri.so
I still get no sysmbols found: 
GNU gdb (GDB) 7.9
Copyright (C) 2015 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-unknown-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
<http://www.gnu.org/software/gdb/documentation/>.
For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from info...(no debugging symbols found)...done.
"/usr/lib/xorg/modules/dri/radeonsi_dri.so" is not a core dump: File format not
recognized

Any idea what I did wrong? (I'll attach the PKGBUILD and my makepkg.conf)

-- 
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/20150427/7a153aee/attachment.html>

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

[Hans Verkuil]

Added the y2038 mailinglist since I would like to get their input for
this API.

Y2038 experts, can you take a look at my comments in the code below?

Thanks!

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 cec_msg *msg)
> +{
> + return msg->msg[0] >> 4;
> +}
> +
> +static inline __u8 cec_msg_destination(const struct cec_msg *msg)
> +{
> + return msg->msg[0] & 0xf;
> +}
> +
> +static inline bool cec_msg_is_broadcast(const struct cec_msg *msg)
> +{
> + return (msg->msg[0] & 0xf) == 0xf;
> +}
> +
> +/* cec status field */
> +#define CEC_TX_STATUS_OK(0)
> +#define CEC_TX_STATUS_ARB_LOST  (1 << 0)
> +#define CEC_TX_STATUS_RETRY_TIMEOUT (1 << 1)
> +#define CEC_TX_STATUS_FEATURE_ABORT (1 << 2)
> +#define CEC_TX_STATUS_REPLY_TIMEOUT (1 << 3)
> +#define CEC_RX_STATUS_READY (0)
> +
> +#define CEC_LOG_ADDR_INVALID 0xff
> +
> +/* 

Help on drmModeSetPlane

[Xie, William]

Hello, Syrjälä,
 For drmModeSetPlane API, regarding the ctrc width, height and src width and 
heitht,
Do they must 16 byte aligned?
For example, for the size of 33x66, will these value be supported?

William

-Original Message-
From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com] 
Sent: Friday, April 17, 2015 6:08 PM
To: Xie, William
Cc: DRI Development
Subject: Re: Help on drmModeSetPlane

On Fri, Apr 17, 2015 at 09:30:02AM +, Xie, William wrote:
> Hello
> 
> Does overlay support alpha?

Depends on the hardware once more. Also we don't have a proper API for it at 
this time. You may get lucky if you tell it to use a pixel format with alpha. 
But there's no guarantee what kind of blend equation you will get.

Also we don't really have an API for the plane Z order either. Some drivers 
have a property for that, but I don't think it's really well defined.

> 
> William
> 
> -Original Message-
> From: Ville Syrjälä [mailto:ville.syrjala at linux.intel.com]
> Sent: Wednesday, April 15, 2015 10:34 PM
> To: Xie, William
> Cc: DRI Development
> Subject: Re: Help on drmModeSetPlane
> 
> On Wed, Apr 15, 2015 at 02:24:14PM +, Xie, William wrote:
> > Oh, I tried it on BDW.
> > So we need to use other way to scale the video to full screen?
> 
> Yes, unfortunately.
> 
> > 
> > William
> > 
> > 
> > -Original Message-
> > From: Ville Syrjälä [mailto:ville.syrjala at linux.intel.com]
> > Sent: Wednesday, April 15, 2015 7:23 PM
> > To: Xie, William
> > Cc: DRI Development
> > Subject: Re: Help on drmModeSetPlane
> > 
> > On Wed, Apr 15, 2015 at 08:49:39AM +, Xie, William wrote:
> > > To make it more specific,
> > > 
> > > For example:
> > > 1: video frame size is 1280x720,
> > > 2: screen resolution is 3200x1800,
> > > 
> > > How can I set crtc and src parameters?
> > > 
> > > 1: crtc (0, 0, 3200, 1800)  src (0, 0, 3200<<16, 1800<<16)
> > > 2: crtc (0, 0, 3200, 1800)  src (0, 0, 1280<<16, 720<<16)
> > > 
> > > Which one is correct?
> > 
> > 2 is correct.
> > 
> > What hardware are you trying this on? Do note that on HSW/BDW/VLV/CHV the 
> > hardware lost the plane scaling ability. It's coming back on SKL.
> > 
> > > 
> > > William
> > > 
> > > From: Xie, William
> > > Sent: Wednesday, April 15, 2015 3:55 PM
> > > To: DRI Development; DRI Development
> > > Subject: Help on drmModeSetPlane
> > > 
> > > Question please, could someone share the meaning of the parameters for 
> > > drmModeSetPlane in libdrm?
> > > 
> > > int drmModeSetPlane(int fd, uint32_t plane_id, uint32_t crtc_id,
> > > uint32_t fb_id, uint32_t flags,
> > > int32_t crtc_x, int32_t crtc_y,
> > > uint32_t crtc_w, uint32_t crtc_h,
> > > uint32_t src_x, uint32_t src_y,
> > > uint32_t src_w, uint32_t 
> > > src_h)
> > > 
> > > 
> > > My problem is, whatever value I set, the video is not full screen 
> > > mode, Anything I missed?
> > > 
> > > Thanks
> > > William
> > > 
> > > 
> > > 
> > > 
> > 
> > > ___
> > > dri-devel mailing list
> > > dri-devel at lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/dri-devel
> > 
> > 
> > --
> > Ville Syrjälä
> > Intel OTC
> 
> --
> Ville Syrjälä
> Intel OTC

--
Ville Syrjälä
Intel OTC

[Bug 90182] [radeonsi]Qt Applications won't start on the dedicated GPU (SIGABRT)

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

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

--- Comment #5 from Michel Dänzer  ---
Please attach at least one new gdb trace with debugging symbols available for
/usr/lib/xorg/modules/dri/radeonsi_dri.so.

-- 
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/20150427/9e86b937/attachment.html>

linux-4.1-rc1/drivers/gpu/drm/msm/dsi/dsi_host.c:1799: possible missing break ?

[David Binderman]

Hello there,

[linux-4.1-rc1/drivers/gpu/drm/msm/dsi/dsi_host.c:1799] -> 
[linux-4.1-rc1/drivers/gpu/drm/msm/dsi/dsi_host.c:1802]: (warning) Variable 
'ret' is reassigned a value before the old one has been used. 'break;' missing?

    switch (cmd) {
    case MIPI_DSI_RX_ACKNOWLEDGE_AND_ERROR_REPORT:
        pr_err("%s: rx ACK_ERR_PACLAGE\n", __func__);
        ret = 0;
    case MIPI_DSI_RX_GENERIC_SHORT_READ_RESPONSE_1BYTE:
    case MIPI_DSI_RX_DCS_SHORT_READ_RESPONSE_1BYTE:
        ret = dsi_short_read1_resp(buf, msg);
        break;

Regards

David Binderman

[git pull] drm intel fixes

[Dave Airlie]

Hi Linus,

playing the race -rc1 game, nothing too urgent if this doesn't get in 
though, Jani sent it after I started my weekend so I felt I should send 
it on.

Just intel fixes 3 of them.

Dave.

The following changes since commit c8b3fd0ce313443731e8fd6d5a541085eb465f99:

  Merge tag 'pm+acpi-4.1-rc1-2' of 
git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm (2015-04-26 
13:56:35 -0700)

are available in the git repository at:

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

for you to fetch changes up to 59fd7e4b0b0769638b5162e56c28bbb027a118d3:

  Merge tag 'drm-intel-next-fixes-2015-04-25' of 
git://anongit.freedesktop.org/drm-intel into drm-fixes (2015-04-27 10:35:15 
+1000)


Dave Airlie (1):
  Merge tag 'drm-intel-next-fixes-2015-04-25' of 
git://anongit.freedesktop.org/drm-intel into drm-fixes

Dmitry Torokhov (1):
  drm/i915: cope with large i2c transfers

Imre Deak (1):
  drm/i915: vlv: fix save/restore of GFX_MAX_REQ_COUNT reg

Michel Thierry (1):
  drm/i915: Workaround to avoid lite restore with HEAD==TAIL

 drivers/gpu/drm/i915/i915_drv.c  |  4 +--
 drivers/gpu/drm/i915/i915_gem.c  |  3 +-
 drivers/gpu/drm/i915/i915_reg.h  |  1 +
 drivers/gpu/drm/i915/intel_i2c.c | 66 ++--
 drivers/gpu/drm/i915/intel_lrc.c | 35 -
 5 files changed, 95 insertions(+), 14 deletions(-)

[PATCH 2/2] Fix one warning

[Zhou, Jammy]

Thanks for sharing the background. For [0], you mentioned that get_perms may 
return -1 in some cases for the group, can you help indicate which case it is?

Since the drmSetServerInfo is seldom used, maybe we can just do the 'int' cast 
at this moment. I will send v2 for this.

Regards,
Jammy

-Original Message-
From: Jan Vesely [mailto:jv...@scarletmail.rutgers.edu] On Behalf Of Jan Vesely
Sent: Friday, April 24, 2015 10:30 PM
To: Zhou, Jammy
Cc: dri-devel at lists.freedesktop.org; Min, Frank
Subject: Re: [PATCH 2/2] Fix one warning

On Fri, 2015-04-24 at 11:44 +0800, Jammy Zhou wrote:
> xf86drm.c:356:2: warning: comparison of unsigned expression >= 0 is always 
> true [-Wtype-limits]
>   group = (serv_group >= 0) ? serv_group : DRM_DEV_GID;
>   ^
> 
> Signed-off-by: Jammy Zhou 
> ---
>  xf86drm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/xf86drm.c b/xf86drm.c
> index 4d67861..fbda2aa 100644
> --- a/xf86drm.c
> +++ b/xf86drm.c
> @@ -353,7 +353,7 @@ static int drmOpenDevice(dev_t dev, int minor, int type)
>  }
>  
>  if (drm_server_info) {
> - group = (serv_group >= 0) ? serv_group : DRM_DEV_GID;
> + group = serv_group;

I don't think this change is correct. get_perms can return errors as negative 
values. I found that xserver does, see [0] for my take on fixing this, as well 
as Emil's response [1].

I think changing the condition to:
((int)serv_group >= 0)

should be ok(I don't think there are systems with GID_MAX greater than 2^31-1), 
but I never bothered sending v2.

jan


[0]http://lists.freedesktop.org/archives/dri-devel/2015-February/077276.html
[1]http://lists.freedesktop.org/archives/dri-devel/2015-February/078171.html



>   chown_check_return(buf, user, group);
>   chmod(buf, devmode);
>  }


--
Jan Vesely