[Bug 90056] Unigine Valley regression since radeon/llvm: Run LLVM's instruction combining pass

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

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

--- Comment #18 from Tom Stellard  ---
(In reply to Furkan from comment #17)
> Since mesa 10.6 is going RC soon, what's going to happen with this feature?
> If it remains enabled, and the fix isn't backported to llvm 3.6, this bug
> will end up being present in a stable release (unless llvm 3.7 is also going
> to be released soon).
> 
> Fez is also affected by the same bug (I tested and verified that it is fixed
> by the same patch), so there may be many more games affected:
> 
> Good (mesa 10.5): https://www.dropbox.com/s/h7qtlgln1xlnxrk/fez_good.png?dl=0
> Bad (mesa 10.6): https://www.dropbox.com/s/f5yqqx2l6pz284z/fez_bad.png?dl=0

This bug will be fixed in the LLVM 3.6.1 release.

-- 
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/20150525/1b83792b/attachment.html>

block_all_signals() usage in DRM

[Richard Weinberger]

Am 25.05.2015 um 18:50 schrieb Oleg Nesterov:
> On 05/25, Richard Weinberger wrote:
>>
>> Is this functionality still in use/needed?
> 
> All I can say it doesn't work.
> 
>> Otherwise we could get rid of block_all_signals() and unpuzzle the signaling
>> code a bit. :-)
> 
> Yes. I do not even remember when I reported this the first time. Perhaps
> more than 10 years ago.
> 
> See the last attempt in 2011: https://lkml.org/lkml/2011/7/12/263
> I copied this email below.

Thank you Oleg, this makes sense.
I was actually wondering WTF this function is good for.

Thanks,
//richard

[PATCH v2 4/4] drm/atomic: Add MODE_ID property

[Daniel Stone]

Atomic modesetting: now with modesetting support.

v2: Moved drm_atomic_set_mode_prop_for_crtc from previous patch; removed
state->active fiddling, documented return code. Changed property
type to DRM_MODE_PROP_BLOB.

Signed-off-by: Daniel Stone 
Tested-by: Sean Paul 
---
 drivers/gpu/drm/drm_atomic.c | 57 +++-
 drivers/gpu/drm/drm_crtc.c   |  8 +++
 include/drm/drm_atomic.h |  3 +++
 include/drm/drm_crtc.h   |  1 +
 4 files changed, 68 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 327ccc7..c7e59b0 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -339,6 +339,51 @@ int drm_atomic_set_mode_for_crtc(struct drm_crtc_state 
*state,
 EXPORT_SYMBOL(drm_atomic_set_mode_for_crtc);

 /**
+ * drm_atomic_set_mode_prop_for_crtc - set mode for CRTC
+ * @state: the CRTC whose incoming state to update
+ * @blob: pointer to blob property to use for mode
+ *
+ * Set a mode (originating from a blob property) on the desired CRTC state.
+ * This function will take a reference on the blob property for the CRTC state,
+ * and release the reference held on the state's existing mode property, if any
+ * was set.
+ *
+ * RETURNS:
+ * Zero on success, error code on failure. Cannot return -EDEADLK.
+ */
+int drm_atomic_set_mode_prop_for_crtc(struct drm_crtc_state *state,
+  struct drm_property_blob *blob)
+{
+   if (blob == state->mode_blob)
+   return 0;
+
+   if (state->mode_blob)
+   drm_property_unreference_blob(state->mode_blob);
+   state->mode_blob = NULL;
+
+   if (blob) {
+   if (blob->length != sizeof(struct drm_mode_modeinfo) ||
+   drm_mode_convert_umode(>mode,
+  (const struct drm_mode_modeinfo *)
+   blob->data))
+   return -EINVAL;
+
+   state->mode_blob = drm_property_reference_blob(blob);
+   state->enable = true;
+   DRM_DEBUG_ATOMIC("Set [MODE:%s] for CRTC state %p\n",
+state->mode.name, state);
+   } else {
+   memset(>mode, 0, sizeof(state->mode));
+   state->enable = false;
+   DRM_DEBUG_ATOMIC("Set [NOMODE] for CRTC state %p\n",
+state);
+   }
+
+   return 0;
+}
+EXPORT_SYMBOL(drm_atomic_set_mode_prop_for_crtc);
+
+/**
  * drm_atomic_crtc_set_property - set property on CRTC
  * @crtc: the drm CRTC to set a property on
  * @state: the state object to update with the new property value
@@ -360,10 +405,18 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
 {
struct drm_device *dev = crtc->dev;
struct drm_mode_config *config = >mode_config;
+   int ret;

-   /* FIXME: Mode prop is missing, which also controls ->enable. */
if (property == config->prop_active)
state->active = val;
+   else if (property == config->prop_mode_id) {
+   struct drm_property_blob *mode =
+   drm_property_lookup_blob(dev, val);
+   ret = drm_atomic_set_mode_prop_for_crtc(state, mode);
+   if (mode)
+   drm_property_unreference_blob(mode);
+   return ret;
+   }
else if (crtc->funcs->atomic_set_property)
return crtc->funcs->atomic_set_property(crtc, state, property, 
val);
else
@@ -388,6 +441,8 @@ int drm_atomic_crtc_get_property(struct drm_crtc *crtc,

if (property == config->prop_active)
*val = state->active;
+   else if (property == config->prop_mode_id)
+   *val = (state->mode_blob) ? state->mode_blob->base.id : 0;
else if (crtc->funcs->atomic_get_property)
return crtc->funcs->atomic_get_property(crtc, state, property, 
val);
else
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 2de6edf..b4b365c 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -688,6 +688,7 @@ int drm_crtc_init_with_planes(struct drm_device *dev, 
struct drm_crtc *crtc,

if (drm_core_check_feature(dev, DRIVER_ATOMIC)) {
drm_object_attach_property(>base, config->prop_active, 0);
+   drm_object_attach_property(>base, config->prop_mode_id, 
0);
}

return 0;
@@ -1454,6 +1455,13 @@ static int drm_mode_create_standard_properties(struct 
drm_device *dev)
return -ENOMEM;
dev->mode_config.prop_active = prop;

+   prop = drm_property_create(dev,
+   DRM_MODE_PROP_ATOMIC | DRM_MODE_PROP_BLOB,
+   "MODE_ID", 0);
+   if (!prop)
+   return -ENOMEM;
+   dev->mode_config.prop_mode_id = prop;
+
return 0;
 }

diff --git 

[PATCH v2 3/4] drm/atomic: Add current-mode blob to CRTC state

[Daniel Stone]

Add a blob property tracking the current mode to the CRTC state, and
ensure it is properly updated and referenced.

v2: Continue using crtc_state->mode inside getcrtc, instead of reading
out the mode blob. Use IS_ERR and PTR_ERR from create_blob. Move
set_mode_prop_for_crtc to later patch where it actually gets used.
Enforce !!state->enable == !!state->mode_blob inside
drm_atomic_crtc_check.

Signed-off-by: Daniel Stone 
Tested-by: Sean Paul 
---
 drivers/gpu/drm/drm_atomic.c| 32 +++-
 drivers/gpu/drm/drm_atomic_helper.c | 11 ++-
 include/drm/drm_crtc.h  |  3 +++
 3 files changed, 40 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index e749287..327ccc7 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -304,11 +304,25 @@ EXPORT_SYMBOL(drm_atomic_get_crtc_state);
 int drm_atomic_set_mode_for_crtc(struct drm_crtc_state *state,
 struct drm_display_mode *mode)
 {
+   struct drm_mode_modeinfo umode;
+
/* Early return for no change. */
if (mode && memcmp(>mode, mode, sizeof(*mode)) == 0)
return 0;

+   if (state->mode_blob)
+   drm_property_unreference_blob(state->mode_blob);
+   state->mode_blob = NULL;
+
if (mode) {
+   drm_mode_convert_to_umode(, mode);
+   state->mode_blob =
+   drm_property_create_blob(state->crtc->dev,
+sizeof(umode),
+);
+   if (IS_ERR(state->mode_blob))
+   return PTR_ERR(state->mode_blob);
+
drm_mode_copy(>mode, mode);
state->enable = true;
DRM_DEBUG_ATOMIC("Set [MODE:%s] for CRTC state %p\n",
@@ -324,7 +338,6 @@ int drm_atomic_set_mode_for_crtc(struct drm_crtc_state 
*state,
 }
 EXPORT_SYMBOL(drm_atomic_set_mode_for_crtc);

-
 /**
  * drm_atomic_crtc_set_property - set property on CRTC
  * @crtc: the drm CRTC to set a property on
@@ -410,6 +423,23 @@ static int drm_atomic_crtc_check(struct drm_crtc *crtc,
return -EINVAL;
}

+   /* The state->enable vs. state->mode_blob checks can be WARN_ON,
+* as this is a kernel-internal detail that userspace should never
+* be able to trigger. */
+   if (drm_core_check_feature(crtc->dev, DRIVER_ATOMIC) &&
+   WARN_ON(state->enable && !state->mode_blob)) {
+   DRM_DEBUG_ATOMIC("[CRTC:%d] enabled without mode blob\n",
+crtc->base.id);
+   return -EINVAL;
+   }
+
+   if (drm_core_check_feature(crtc->dev, DRIVER_ATOMIC) &&
+   WARN_ON(!state->enable && state->mode_blob)) {
+   DRM_DEBUG_ATOMIC("[CRTC:%d] disabled with mode blob\n",
+crtc->base.id);
+   return -EINVAL;
+   }
+
return 0;
 }

diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
b/drivers/gpu/drm/drm_atomic_helper.c
index e69d484..a900858 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -2044,6 +2044,8 @@ EXPORT_SYMBOL(drm_atomic_helper_connector_dpms);
  */
 void drm_atomic_helper_crtc_reset(struct drm_crtc *crtc)
 {
+   if (crtc->state && crtc->state->mode_blob)
+   drm_property_unreference_blob(crtc->state->mode_blob);
kfree(crtc->state);
crtc->state = kzalloc(sizeof(*crtc->state), GFP_KERNEL);

@@ -2065,6 +2067,8 @@ void __drm_atomic_helper_crtc_duplicate_state(struct 
drm_crtc *crtc,
 {
memcpy(state, crtc->state, sizeof(*state));

+   if (state->mode_blob)
+   drm_property_reference_blob(state->mode_blob);
state->mode_changed = false;
state->active_changed = false;
state->planes_changed = false;
@@ -2107,11 +2111,8 @@ EXPORT_SYMBOL(drm_atomic_helper_crtc_duplicate_state);
 void __drm_atomic_helper_crtc_destroy_state(struct drm_crtc *crtc,
struct drm_crtc_state *state)
 {
-   /*
-* This is currently a placeholder so that drivers that subclass the
-* state will automatically do the right thing if code is ever added
-* to this function.
-*/
+   if (state->mode_blob)
+   drm_property_unreference_blob(state->mode_blob);
 }
 EXPORT_SYMBOL(__drm_atomic_helper_crtc_destroy_state);

diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 72b60db..c54fa4a 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -299,6 +299,9 @@ struct drm_crtc_state {

struct drm_display_mode mode;

+   /* blob property to expose current mode to atomic userspace */
+   struct drm_property_blob *mode_blob;
+
struct drm_pending_vblank_event *event;

struct drm_atomic_state *state;
-- 

[PATCH v2 2/4] drm: Add drm_atomic_set_mode_for_crtc

[Daniel Stone]

Add a new helper, to be used later for blob property management, that
sets the mode for a CRTC state, as well as updating the CRTC enable/active
state at the same time.

v2: Do not touch active/mode_changed in CRTC state. Document return
value. Remove stray drm_atomic_set_mode_prop_for_crtc declaration.

Signed-off-by: Daniel Stone 
Tested-by: Sean Paul 
---
 drivers/gpu/drm/drm_atomic.c | 36 +
 drivers/gpu/drm/drm_atomic_helper.c  | 11 +---
 drivers/gpu/drm/drm_crtc_helper.c|  5 ++--
 drivers/gpu/drm/i915/intel_display.c | 51 ++--
 include/drm/drm_atomic.h |  3 +++
 5 files changed, 87 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 3134f50..e749287 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -290,6 +290,42 @@ drm_atomic_get_crtc_state(struct drm_atomic_state *state,
 EXPORT_SYMBOL(drm_atomic_get_crtc_state);

 /**
+ * drm_atomic_set_mode_for_crtc - set mode for CRTC
+ * @state: the CRTC whose incoming state to update
+ * @mode: kernel-internal mode to use for the CRTC, or NULL to disable
+ *
+ * Set a mode (originating from the kernel) on the desired CRTC state. Does
+ * not change any other state properties, including enable, active, or
+ * mode_changed.
+ *
+ * RETURNS:
+ * Zero on success, error code on failure. Cannot return -EDEADLK.
+ */
+int drm_atomic_set_mode_for_crtc(struct drm_crtc_state *state,
+struct drm_display_mode *mode)
+{
+   /* Early return for no change. */
+   if (mode && memcmp(>mode, mode, sizeof(*mode)) == 0)
+   return 0;
+
+   if (mode) {
+   drm_mode_copy(>mode, mode);
+   state->enable = true;
+   DRM_DEBUG_ATOMIC("Set [MODE:%s] for CRTC state %p\n",
+mode->name, state);
+   } else {
+   memset(>mode, 0, sizeof(state->mode));
+   state->enable = false;
+   DRM_DEBUG_ATOMIC("Set [NOMODE] for CRTC state %p\n",
+state);
+   }
+
+   return 0;
+}
+EXPORT_SYMBOL(drm_atomic_set_mode_for_crtc);
+
+
+/**
  * drm_atomic_crtc_set_property - set property on CRTC
  * @crtc: the drm CRTC to set a property on
  * @state: the state object to update with the new property value
diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
b/drivers/gpu/drm/drm_atomic_helper.c
index a64bacd..e69d484 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1607,7 +1607,10 @@ retry:
WARN_ON(set->fb);
WARN_ON(set->num_connectors);

-   crtc_state->enable = false;
+   ret = drm_atomic_set_mode_for_crtc(crtc_state, NULL);
+   if (ret != 0)
+   goto fail;
+
crtc_state->active = false;

ret = drm_atomic_set_crtc_for_plane(primary_state, NULL);
@@ -1622,9 +1625,11 @@ retry:
WARN_ON(!set->fb);
WARN_ON(!set->num_connectors);

-   crtc_state->enable = true;
+   ret = drm_atomic_set_mode_for_crtc(crtc_state, set->mode);
+   if (ret != 0)
+   goto fail;
+
crtc_state->active = true;
-   drm_mode_copy(_state->mode, set->mode);

ret = drm_atomic_set_crtc_for_plane(primary_state, crtc);
if (ret != 0)
diff --git a/drivers/gpu/drm/drm_crtc_helper.c 
b/drivers/gpu/drm/drm_crtc_helper.c
index 9297a61..393114d 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -937,10 +937,11 @@ int drm_helper_crtc_mode_set(struct drm_crtc *crtc, 
struct drm_display_mode *mod
crtc_state->crtc = crtc;
}

-   crtc_state->enable = true;
crtc_state->planes_changed = true;
crtc_state->mode_changed = true;
-   drm_mode_copy(_state->mode, mode);
+   ret = drm_atomic_set_mode_for_crtc(crtc_state, mode);
+   if (ret)
+   goto out;
drm_mode_copy(_state->adjusted_mode, adjusted_mode);

if (crtc_funcs->atomic_check) {
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 240092a..d085c81 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9896,7 +9896,9 @@ retry:
if (ret)
goto fail;

-   drm_mode_copy(_state->base.mode, mode);
+   ret = drm_atomic_set_mode_for_crtc(_state->base, mode);
+   if (ret)
+   goto fail;

if (intel_set_mode(crtc, state)) {
DRM_DEBUG_KMS("failed to set mode on load-detect pipe\n");
@@ -12481,9 +12483,6 @@ void intel_crtc_restore_mode(struct drm_crtc *crtc)
}

for_each_intel_crtc(dev, intel_crtc) {
-   if (intel_crtc->new_enabled == intel_crtc->base.enabled)
-   continue;
-

[PATCH v2 1/4] drm: Retain reference to blob properties in lookup

[Daniel Stone]

When we look up a blob property, make sure we retain a reference to the
blob for the lifetime.

v2: Use DRM_MODE_PROP_BLOB, not PROP_OBJECT + OBJECT_BLOB.

Signed-off-by: Daniel Stone 
Tested-by: Sean Paul 
---
 drivers/gpu/drm/drm_crtc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index e548c50..2de6edf 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -4775,7 +4775,8 @@ void drm_property_change_valid_put(struct drm_property 
*property,
if (drm_property_type_is(property, DRM_MODE_PROP_OBJECT)) {
if (property->values[0] == DRM_MODE_OBJECT_FB)
drm_framebuffer_unreference(obj_to_fb(ref));
-   }
+   } else if (drm_property_type_is(property, DRM_MODE_PROP_BLOB))
+   drm_property_unreference_blob(obj_to_blob(ref));
 }

 /**
-- 
2.4.1

[PATCH v2 0/4] CRTC state mode property

[Daniel Stone]

Hi,
Respin with the fixups from Daniel Vetter's review. The two changes
visible to userspace is that we now require anyone clearing MODE_ID to
explicitly set the CRTC active property to false, and that the property
type is BLOB, rather than OBJECT/BLOB.

The rest are fairly minor.

Cheers,
Daniel

block_all_signals() usage in DRM

[Oleg Nesterov]

On 05/25, Richard Weinberger wrote:
>
> Is this functionality still in use/needed?

All I can say it doesn't work.

> Otherwise we could get rid of block_all_signals() and unpuzzle the signaling
> code a bit. :-)

Yes. I do not even remember when I reported this the first time. Perhaps
more than 10 years ago.

See the last attempt in 2011: https://lkml.org/lkml/2011/7/12/263
I copied this email below.

Dave. Lets finally kill this horror? I am going to send a patch unless
you stop me ;)

Oleg.

---
I tried many times to ask about the supposed behaviour of
block_all_signals() in drm, but it seems nobody can answer.

So I am going to send the patch which simply removes
block_all_signals() and friends. There are numeruous problems
with this interace, I can't even enumerate them. But I think
that it is enough to mention that block_all_signals() simply
can not work. AT ALL. I am wondering, was it ever tested and
how.

So. ioctl()->drm_lock() "blocks" the stop signals. Probably to
ensure the task can't be stopped until it does DRM_IOCTL_UNLOCK.
And what does this mean? Yes, the task won't stop if it receives,
say, SIGTSTP. But! Instead it will loop forever in kernel mode
until it receives another unblocked/non-ignored signal which
should be numerically less than SIGSTOP.

Why do we need this? Once again. block_all_signals(SIGTSTP)
only means that the caller will burn cpu instead of sleeping
in TASK_STOPPED after ^Z. What is the point?

And once again, there are other problems. For example, even if
block_all_signals() actually blocked SIGSTOP/etc, this can not
help if the caller is multithreaded.

I strongly believe block_all_signals() should die. Given that
it doesn't work, could somebody please explain me what will
be broken?



Just in case... Please look at the debugging patch below. With
this patch,

$ perl -le 'syscall 157,666 and die $!; sleep 1, print while ++$_'
1
2
3
^Z

Hang. So it does react to ^Z anyway, just it is looping in the
endless loop in the kernel. It can only look as if ^Z is ignored,
because obviously bash doesn't see it stopped.



Now lets look at drm_notifier(). If it returns 0 it does:

/* Otherwise, set flag to force call to
   drmUnlock */

drmUnlock? grep shows nothing...

do {
old = s->lock->lock;
new = old | _DRM_LOCK_CONT;
prev = cmpxchg(>lock->lock, old, new);
} while (prev != old);
return 0;

OK. So, if block_all_signals() makes any sense, it seems that this
is only because we add _DRM_LOCK_CONT.

Who checks _DRM_LOCK_CONT? _DRM_LOCK_IS_CONT(), but it has no users.
Hmm. Looks like via_release_futex() is the only user, but it doesn't
look as "force call to drmUnlock" and it is CONFIG_DRM_VIA only.


I am totally confused. But block_all_signals() should die anyway.

We can probably implement something like 'i-am-going-to-stop' or
even 'can-i-stop' per-thread notifiers, although this all looks
like the user-space problem to me (yes, I know absolutely nothing
about drm/etc).

If nothing else. We can change drm_lock/drm_unlock to literally
block/unblock SIGSTOP/etc (or perhaps we only should worry about
the signals from tty?). This is the awful hack and this can't work
with the multithreaded tasks too, but still it is better than what
we have now.

Oleg.

--- a/kernel/sys.c~ 2011-06-16 20:12:18.0 +0200
+++ b/kernel/sys.c  2011-07-12 16:24:50.0 +0200
@@ -1614,6 +1614,11 @@ SYSCALL_DEFINE1(umask, int, mask)
return mask;
 }

+static int notifier(void *arg)
+{
+   return 0;
+}
+
 SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
unsigned long, arg4, unsigned long, arg5)
 {
@@ -1627,6 +1632,13 @@ SYSCALL_DEFINE5(prctl, int, option, unsi

error = 0;
switch (option) {
+   case 666: {
+   sigset_t *pmask = kmalloc(sizeof(*pmask), GFP_KERNEL);
+   siginitset(pmask, sigmask(SIGTSTP));
+   block_all_signals(notifier, NULL, pmask);
+   break;
+   }
+
case PR_SET_PDEATHSIG:
if (!valid_signal(arg2)) {
error = -EINVAL;

[Bug 90264] [Regression, bisected] Tooltip corruption in Chrome

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

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

--- Comment #8 from AP  ---
I can confirm this issue. Running the latest Mesa built from Pali's 12.04
backports PPA (https://launchpad.net/~pali/+archive/ubuntu/graphics-drivers) I
am seeing the same kind of graphical corruption in tooltips: They will often
look scrambled and, as pointed out above, are sometimes replaced by textures
from other apps (e.g. my dock). As far as I can tell this is restricted to
Chrome for now.

-- 
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/20150525/b13f6803/attachment.html>

[Nouveau] [PATCH 5/8] acpi: Check returned object type by Optimus _DSM locally

[Ilia Mirkin]

On Mon, May 25, 2015 at 6:22 PM, Pierre Moreau  wrote:
> Most _DSM will return an integer value of 0x8002 when given an unknown
> UUID, revision ID or function ID. Checking locally allows us to differentiate
> that case from other ACPI errors, and to not report a "failed to evaluate 
> _DSM"
> if 0x8002 is returned which was confusing.
>
> Signed-off-by: Pierre Moreau 
> ---
>  drm/nouveau/nouveau_acpi.c | 15 ---
>  1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/drm/nouveau/nouveau_acpi.c b/drm/nouveau/nouveau_acpi.c
> index 073f7d7..7aeaf7d 100644
> --- a/drm/nouveau/nouveau_acpi.c
> +++ b/drm/nouveau/nouveau_acpi.c
> @@ -88,12 +88,12 @@ static int nouveau_evaluate_optimus_dsm(acpi_handle 
> handle, int func, int arg, u
> for (i = 0; i < 4; i++)
> args_buff[i] = (arg >> i * 8) & 0xFF;
>
> -   obj = acpi_evaluate_dsm_typed(handle, nouveau_op_dsm_muid, 
> nouveau_op_dsm_rid,
> - func, , ACPI_TYPE_BUFFER);
> +   obj = acpi_evaluate_dsm(handle, nouveau_op_dsm_muid, 
> nouveau_op_dsm_rid,
> +   func, );
> if (!obj) {
> acpi_handle_info(handle, "failed to evaluate _DSM\n");
> return AE_ERROR;
> -   } else {
> +   } else if (obj->type == ACPI_TYPE_BUFFER) {
> if (!result && obj->buffer.length == 4) {
> *result  = obj->buffer.pointer[0];
> *result |= (obj->buffer.pointer[1] << 8);
> @@ -101,6 +101,15 @@ static int nouveau_evaluate_optimus_dsm(acpi_handle 
> handle, int func, int arg, u
> *result |= (obj->buffer.pointer[3] << 24);
> }
> ACPI_FREE(obj);
> +   } else if (obj->type == ACPI_TYPE_INTEGER &&
> +  obj->integer.value == 0x8002) {
> +   acpi_handle_debug(handle, "failed to query Optimus _DSM\n");
> +   ACPI_FREE(obj);
> +   return -ENODEV;

should this be AE_ERROR?

> +   } else {
> +   acpi_handle_err(handle, "unexpected returned value by Optimus 
> _DSM\n");
> +   ACPI_FREE(obj);
> +   return AE_ERROR;
> }
>
> return 0;
> --
> 2.4.1
>
> ___
> Nouveau mailing list
> Nouveau at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/nouveau

[RESEND PATCH v1 2/2] drm: bridge/dw_hdmi-i2s-audio: add audio driver

[Yakir]

Hi Paul,

在 2015/5/25 16:24, Paul Bolle 写道:
> Just a nit: a license mismatch.
>
> On Fri, 2015-05-22 at 10:14 -0500, Yakir Yang wrote:
>> --- /dev/null
>> +++ b/drivers/gpu/drm/bridge/dw_hdmi-i2s-audio.c
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
> This states the license is GPL v2.
>
>> +MODULE_LICENSE("GPL");
> And, according to include/linux/module.h, this states the license is GPL
> v2 or later. So I think that either the comment at the top of this file
> or the ident used in the MODULE_LICENSE() macro should change.

Thanks for your catch, got it  :-)

Yakir Yang

>
> Paul Bolle
>
>
>
>

[PATCH] drm/atomic: fix out of bounds read in for_each_*_in_state helpers

[Jani Nikula]

On Mon, 25 May 2015, Jani Nikula  wrote:
> On Mon, 25 May 2015, Jani Nikula  wrote:
>> On Mon, 25 May 2015, Andrey Ryabinin  wrote:
>>> On 05/25/2015 04:12 PM, Jani Nikula wrote:
 On Mon, 25 May 2015, Andrey Ryabinin  wrote:
> for_each_*_in_state validate array index after
> access to array elements, thus perform out of bounds read.
>
> Fix this by validating index in the first place and read
> array element iff validation was successful.
>
> Fixes: df63b9994eaf ("drm/atomic: Add 
> for_each_{connector,crtc,plane}_in_state helper macros")
> Signed-off-by: Andrey Ryabinin 
> ---
>  include/drm/drm_atomic.h | 24 
>  1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
> index c1571034..3f13b91 100644
> --- a/include/drm/drm_atomic.h
> +++ b/include/drm/drm_atomic.h
> @@ -77,26 +77,26 @@ int __must_check drm_atomic_async_commit(struct 
> drm_atomic_state *state);
>  
>  #define for_each_connector_in_state(state, connector, connector_state, 
> __i) \
>   for ((__i) = 0; \
> -  (connector) = (state)->connectors[__i],\
> -  (connector_state) = (state)->connector_states[__i],\
> -  (__i) < (state)->num_connector;\
> +  (__i) < (state)->num_connector &&  \
> +  ((connector) = (state)->connectors[__i],   \
> +  (connector_state) = (state)->connector_states[__i], 1);\
 
 This will stop at the first NULL connector/connector_state. Similarly
 for the loops below.
 
>>>
>>> This will stop iff (__i) >= (state)->num_connector, because the result of 
>>> expression:
>>>  ((connector) = (state)->connectors[__i], (connector_state) = 
>>> (state)->connector_states[__i], 1)
>>> is always 1.
>>
>> Why do you think it'll always be 1?
>
> That might be because there's the 1 at the end. *blush*.
>
> I do wonder if this is too subtle in general, or if it's just too subtle
> for me.


So in the mean time, I was looking at doing the below, not because of
this patch or the bug it fixes, but because I think the construct

for (...) \
if (...)

in a for_something() style macro is a dangling else disaster waiting to
happen.

It's a bit tedious, I admit, and apparently makes some gcc versions
whine about using uninitialized variables, because they're not smart
enough to realize the pointers are initialized when the loop condition
is met.

BR,
Jani.


diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
index e89db0c377ba..eb81f5930a8c 100644
--- a/include/drm/drm_atomic.h
+++ b/include/drm/drm_atomic.h
@@ -134,28 +134,70 @@ int __must_check drm_atomic_check_only(struct 
drm_atomic_state *state);
 int __must_check drm_atomic_commit(struct drm_atomic_state *state);
 int __must_check drm_atomic_async_commit(struct drm_atomic_state *state);

+static inline int next_connector(struct drm_connector **pconnector,
+struct drm_connector **connectors,
+struct drm_connector_state **pstate,
+struct drm_connector_state **states,
+int index, int max)
+{
+   while (index < max && (connectors[index] == NULL || states[index] == 
NULL))
+   index++;
+
+   if (index < max) {
+   *pconnector = connectors[index];
+   *pstate = states[index];
+   }
+
+   return index;
+}
+
+static inline int next_crtc(struct drm_crtc **pcrtc,
+   struct drm_crtc **crtcs,
+   struct drm_crtc_state **pstate,
+   struct drm_crtc_state **states,
+   int index, int max)
+{
+   while (index < max && (crtcs[index] == NULL || states[index] == NULL))
+   index++;
+
+   if (index < max) {
+   *pcrtc = crtcs[index];
+   *pstate = states[index];
+   }
+
+   return index;
+}
+
+static inline int next_plane(struct drm_plane **pplane,
+   struct drm_plane **planes,
+   struct drm_plane_state **pstate,
+   struct drm_plane_state **states,
+   int index, int max)
+{
+   while (index < max && (planes[index] == NULL || states[index] == NULL))
+   index++;
+
+   if (index < max) {
+   *pplane = planes[index];
+   *pstate = states[index];
+   }
+
+   return index;
+}
+
 #define for_each_connector_in_state(state, connector, connector_state, __i) \
-   for ((__i) = 0; \
-(connector) = (state)->connectors[__i],\
-

block_all_signals() usage in DRM

[Richard Weinberger]

Hi!

drivers/gpu/drm/drm_lock.c is the only remaining user of block_all_signals():
/* don't set the block all signals on the master process for now
 * really probably not the correct answer but lets us debug xkb
 * xserver for now */
if (!file_priv->is_master) {
sigemptyset(>sigmask);
sigaddset(>sigmask, SIGSTOP);
sigaddset(>sigmask, SIGTSTP);
sigaddset(>sigmask, SIGTTIN);
sigaddset(>sigmask, SIGTTOU);
dev->sigdata.context = lock->context;
dev->sigdata.lock = master->lock.hw_lock;
block_all_signals(drm_notifier, dev, >sigmask);
}

Is this functionality still in use/needed?
Otherwise we could get rid of block_all_signals() and unpuzzle the signaling
code a bit. :-)

Thanks,
//richard

[PATCH] drm/atomic: fix out of bounds read in for_each_*_in_state helpers

[Ville Syrjälä]

On Mon, May 25, 2015 at 04:52:31PM +0300, Jani Nikula wrote:
> On Mon, 25 May 2015, Jani Nikula  wrote:
> > On Mon, 25 May 2015, Andrey Ryabinin  wrote:
> >> On 05/25/2015 04:12 PM, Jani Nikula wrote:
> >>> On Mon, 25 May 2015, Andrey Ryabinin  wrote:
>  for_each_*_in_state validate array index after
>  access to array elements, thus perform out of bounds read.
> 
>  Fix this by validating index in the first place and read
>  array element iff validation was successful.
> 
>  Fixes: df63b9994eaf ("drm/atomic: Add 
>  for_each_{connector,crtc,plane}_in_state helper macros")
>  Signed-off-by: Andrey Ryabinin 
>  ---
>   include/drm/drm_atomic.h | 24 
>   1 file changed, 12 insertions(+), 12 deletions(-)
> 
>  diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
>  index c1571034..3f13b91 100644
>  --- a/include/drm/drm_atomic.h
>  +++ b/include/drm/drm_atomic.h
>  @@ -77,26 +77,26 @@ int __must_check drm_atomic_async_commit(struct 
>  drm_atomic_state *state);
>   
>   #define for_each_connector_in_state(state, connector, connector_state, 
>  __i) \
>   for ((__i) = 0; 
>  \
>  - (connector) = (state)->connectors[__i],
>  \
>  - (connector_state) = (state)->connector_states[__i],
>  \
>  - (__i) < (state)->num_connector;
>  \
>  + (__i) < (state)->num_connector &&  
>  \
>  + ((connector) = (state)->connectors[__i],   
>  \
>  + (connector_state) = (state)->connector_states[__i], 1);
>  \
> >>> 
> >>> This will stop at the first NULL connector/connector_state. Similarly
> >>> for the loops below.
> >>> 
> >>
> >> This will stop iff (__i) >= (state)->num_connector, because the result of 
> >> expression:
> >> ((connector) = (state)->connectors[__i], (connector_state) = 
> >> (state)->connector_states[__i], 1)
> >> is always 1.
> >
> > Why do you think it'll always be 1?
> 
> That might be because there's the 1 at the end. *blush*.
> 
> I do wonder if this is too subtle in general, or if it's just too subtle
> for me.

s/1/true/ might make it a bit less subtle, but not by much.

-- 
Ville Syrjälä
Intel OTC

[PATCH 12/21] drm/tegra: Probe dpaux devices on demand

[Tomeu Vizoso]

When looking up a dpaux device through its DT node, ensure that the
corresponding device has been registered.

Signed-off-by: Tomeu Vizoso 
---
 drivers/gpu/drm/tegra/dpaux.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/tegra/dpaux.c b/drivers/gpu/drm/tegra/dpaux.c
index d6b55e3..10bba41 100644
--- a/drivers/gpu/drm/tegra/dpaux.c
+++ b/drivers/gpu/drm/tegra/dpaux.c
@@ -12,6 +12,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -397,6 +398,8 @@ struct tegra_dpaux *tegra_dpaux_find_by_of_node(struct 
device_node *np)
 {
struct tegra_dpaux *dpaux;

+   of_platform_device_ensure(np);
+
mutex_lock(_lock);

list_for_each_entry(dpaux, _list, list)
-- 
2.4.1

[PATCH 11/21] drm: Probe panels on demand

[Tomeu Vizoso]

When looking up a panel through its DT node, ensure that the
corresponding device has been registered.

Signed-off-by: Tomeu Vizoso 
---
 drivers/gpu/drm/drm_panel.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
index 2ef988e..041211e 100644
--- a/drivers/gpu/drm/drm_panel.c
+++ b/drivers/gpu/drm/drm_panel.c
@@ -23,6 +23,7 @@

 #include 
 #include 
+#include 

 #include 
 #include 
@@ -80,6 +81,8 @@ struct drm_panel *of_drm_find_panel(struct device_node *np)
 {
struct drm_panel *panel;

+   of_platform_device_ensure(np);
+
mutex_lock(_lock);

list_for_each_entry(panel, _list, list) {
-- 
2.4.1

[PATCH 00/21] On-demand device registration

[Tomeu Vizoso]

Hello,

I have a problem with the panel on my Tegra Chromebook taking longer than
expected to be ready during boot (Stéphane Marchesin reported what is
basically the same issue in [0]), and have looked into ordered probing as a
better way of solving this than moving nodes around in the DT or playing with
initcall levels.

While reading the thread [1] that Alexander Holler started with his series to
make probing order deterministic, it occurred to me that it should be possible
to achieve the same by registering devices as they are referenced by other
devices.

This basically reuses the information that is already implicit in the probe()
implementations, saving us from refactoring existing drivers or adding
information to DTBs.

Something I'm not completely happy with is that I have had to move the call to
of_platform_populate after all platform drivers have been registered.
Otherwise I don't see how I could register drivers on demand as we don't have
yet each driver's compatible strings.

For machs that don't move of_platform_populate() to a later point, these
patches shouldn't cause any problems but it's not guaranteed that we'll avoid
all the deferred probes as some drivers may not be registered yet.

I have tested this on boards with Tegra, iMX.6 and Exynos SoCs, and these
patches were enough to eliminate all the deferred probes.

With this series I get the kernel to output to the panel in 0.5s, instead of 
2.8s.

Regards,

Tomeu

[0] http://lists.freedesktop.org/archives/dri-devel/2014-August/066527.html

[1] https://lkml.org/lkml/2014/5/12/452

Tomeu Vizoso (21):
  regulator: core: Reduce critical area in _regulator_get
  ARM: tegra: Add gpio-ranges property
  ARM: tegra: Register drivers before devices
  ARM: EXYNOS: Register drivers before devices
  ARM i.MX6q: Register drivers before devices
  of/platform: Add of_platform_device_ensure()
  of/platform: Ensure device registration on lookup
  gpio: Probe GPIO drivers on demand
  gpio: Probe pinctrl devices on demand
  regulator: core: Probe regulators on demand
  drm: Probe panels on demand
  drm/tegra: Probe dpaux devices on demand
  i2c: core: Probe i2c master devices on demand
  pwm: Probe PWM chip devices on demand
  backlight: Probe backlight devices on demand
  usb: phy: Probe phy devices on demand
  clk: Probe clk providers on demand
  pinctrl: Probe pinctrl devices on demand
  phy: core: Probe phy providers on demand
  dma: of: Probe DMA controllers on demand
  power-supply: Probe power supplies on demand

 arch/arm/boot/dts/tegra124.dtsi |  1 +
 arch/arm/mach-exynos/exynos.c   |  4 +--
 arch/arm/mach-imx/mach-imx6q.c  | 12 -
 arch/arm/mach-tegra/tegra.c | 21 ++-
 drivers/clk/clk.c   |  3 +++
 drivers/dma/of-dma.c|  3 +++
 drivers/gpio/gpiolib-of.c   |  5 
 drivers/gpu/drm/drm_panel.c |  3 +++
 drivers/gpu/drm/tegra/dpaux.c   |  3 +++
 drivers/i2c/i2c-core.c  |  3 +++
 drivers/of/platform.c   | 53 +
 drivers/phy/phy-core.c  |  3 +++
 drivers/pinctrl/devicetree.c|  2 ++
 drivers/power/power_supply_core.c   |  3 +++
 drivers/pwm/core.c  |  3 +++
 drivers/regulator/core.c| 45 +++
 drivers/usb/phy/phy.c   |  3 +++
 drivers/video/backlight/backlight.c |  3 +++
 include/linux/of_platform.h |  2 ++
 19 files changed, 130 insertions(+), 45 deletions(-)

-- 
2.4.1

[PATCH] drm/atomic: fix out of bounds read in for_each_*_in_state helpers

[Jani Nikula]

On Mon, 25 May 2015, Jani Nikula  wrote:
> On Mon, 25 May 2015, Andrey Ryabinin  wrote:
>> On 05/25/2015 04:12 PM, Jani Nikula wrote:
>>> On Mon, 25 May 2015, Andrey Ryabinin  wrote:
 for_each_*_in_state validate array index after
 access to array elements, thus perform out of bounds read.

 Fix this by validating index in the first place and read
 array element iff validation was successful.

 Fixes: df63b9994eaf ("drm/atomic: Add 
 for_each_{connector,crtc,plane}_in_state helper macros")
 Signed-off-by: Andrey Ryabinin 
 ---
  include/drm/drm_atomic.h | 24 
  1 file changed, 12 insertions(+), 12 deletions(-)

 diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
 index c1571034..3f13b91 100644
 --- a/include/drm/drm_atomic.h
 +++ b/include/drm/drm_atomic.h
 @@ -77,26 +77,26 @@ int __must_check drm_atomic_async_commit(struct 
 drm_atomic_state *state);
  
  #define for_each_connector_in_state(state, connector, connector_state, 
 __i) \
for ((__i) = 0; \
 -   (connector) = (state)->connectors[__i],\
 -   (connector_state) = (state)->connector_states[__i],\
 -   (__i) < (state)->num_connector;\
 +   (__i) < (state)->num_connector &&  \
 +   ((connector) = (state)->connectors[__i],   \
 +   (connector_state) = (state)->connector_states[__i], 1);\
>>> 
>>> This will stop at the first NULL connector/connector_state. Similarly
>>> for the loops below.
>>> 
>>
>> This will stop iff (__i) >= (state)->num_connector, because the result of 
>> expression:
>>   ((connector) = (state)->connectors[__i], (connector_state) = 
>> (state)->connector_states[__i], 1)
>> is always 1.
>
> Why do you think it'll always be 1?

That might be because there's the 1 at the end. *blush*.

I do wonder if this is too subtle in general, or if it's just too subtle
for me.

BR,
Jani.

>
> BR,
> Jani.
>
>
>>
>>
>
> -- 
> Jani Nikula, Intel Open Source Technology Center

-- 
Jani Nikula, Intel Open Source Technology Center

[PATCH] drm/atomic: fix out of bounds read in for_each_*_in_state helpers

[Jani Nikula]

On Mon, 25 May 2015, Andrey Ryabinin  wrote:
> On 05/25/2015 04:12 PM, Jani Nikula wrote:
>> On Mon, 25 May 2015, Andrey Ryabinin  wrote:
>>> for_each_*_in_state validate array index after
>>> access to array elements, thus perform out of bounds read.
>>>
>>> Fix this by validating index in the first place and read
>>> array element iff validation was successful.
>>>
>>> Fixes: df63b9994eaf ("drm/atomic: Add 
>>> for_each_{connector,crtc,plane}_in_state helper macros")
>>> Signed-off-by: Andrey Ryabinin 
>>> ---
>>>  include/drm/drm_atomic.h | 24 
>>>  1 file changed, 12 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
>>> index c1571034..3f13b91 100644
>>> --- a/include/drm/drm_atomic.h
>>> +++ b/include/drm/drm_atomic.h
>>> @@ -77,26 +77,26 @@ int __must_check drm_atomic_async_commit(struct 
>>> drm_atomic_state *state);
>>>  
>>>  #define for_each_connector_in_state(state, connector, connector_state, 
>>> __i) \
>>> for ((__i) = 0; \
>>> -(connector) = (state)->connectors[__i],\
>>> -(connector_state) = (state)->connector_states[__i],\
>>> -(__i) < (state)->num_connector;\
>>> +(__i) < (state)->num_connector &&  \
>>> +((connector) = (state)->connectors[__i],   \
>>> +(connector_state) = (state)->connector_states[__i], 1);\
>> 
>> This will stop at the first NULL connector/connector_state. Similarly
>> for the loops below.
>> 
>
> This will stop iff (__i) >= (state)->num_connector, because the result of 
> expression:
>((connector) = (state)->connectors[__i], (connector_state) = 
> (state)->connector_states[__i], 1)
> is always 1.

Why do you think it'll always be 1?

BR,
Jani.


>
>

-- 
Jani Nikula, Intel Open Source Technology Center

[Bug 90537] radeonsi bo/va conflict on RADEON_GEM_VA (rscreen->ws->buffer_from_handle returns NULL)

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

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

--- Comment #16 from pstglia at gmail.com ---
> The mesa version is uninteresting, but what kernel version are you using?
> 
> Older kernels have stability problems when you unmap the BO VA. That's why
> we don't use it in mesa by default.
> 
> Regards,
> Christian.

We are using kernel 4.0.3

-- 
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/20150525/fa8e42eb/attachment.html>

[PATCH] drm/atomic: fix out of bounds read in for_each_*_in_state helpers

[Andrey Ryabinin]

On 05/25/2015 04:12 PM, Jani Nikula wrote:
> On Mon, 25 May 2015, Andrey Ryabinin  wrote:
>> for_each_*_in_state validate array index after
>> access to array elements, thus perform out of bounds read.
>>
>> Fix this by validating index in the first place and read
>> array element iff validation was successful.
>>
>> Fixes: df63b9994eaf ("drm/atomic: Add 
>> for_each_{connector,crtc,plane}_in_state helper macros")
>> Signed-off-by: Andrey Ryabinin 
>> ---
>>  include/drm/drm_atomic.h | 24 
>>  1 file changed, 12 insertions(+), 12 deletions(-)
>>
>> diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
>> index c1571034..3f13b91 100644
>> --- a/include/drm/drm_atomic.h
>> +++ b/include/drm/drm_atomic.h
>> @@ -77,26 +77,26 @@ int __must_check drm_atomic_async_commit(struct 
>> drm_atomic_state *state);
>>  
>>  #define for_each_connector_in_state(state, connector, connector_state, __i) 
>> \
>>  for ((__i) = 0; \
>> - (connector) = (state)->connectors[__i],\
>> - (connector_state) = (state)->connector_states[__i],\
>> - (__i) < (state)->num_connector;\
>> + (__i) < (state)->num_connector &&  \
>> + ((connector) = (state)->connectors[__i],   \
>> + (connector_state) = (state)->connector_states[__i], 1);\
> 
> This will stop at the first NULL connector/connector_state. Similarly
> for the loops below.
> 

This will stop iff (__i) >= (state)->num_connector, because the result of 
expression:
 ((connector) = (state)->connectors[__i], (connector_state) = 
(state)->connector_states[__i], 1)
is always 1.

[PATCH] drm/atomic: fix out of bounds read in for_each_*_in_state helpers

[Jani Nikula]

On Mon, 25 May 2015, Andrey Ryabinin  wrote:
> for_each_*_in_state validate array index after
> access to array elements, thus perform out of bounds read.
>
> Fix this by validating index in the first place and read
> array element iff validation was successful.
>
> Fixes: df63b9994eaf ("drm/atomic: Add 
> for_each_{connector,crtc,plane}_in_state helper macros")
> Signed-off-by: Andrey Ryabinin 
> ---
>  include/drm/drm_atomic.h | 24 
>  1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
> index c1571034..3f13b91 100644
> --- a/include/drm/drm_atomic.h
> +++ b/include/drm/drm_atomic.h
> @@ -77,26 +77,26 @@ int __must_check drm_atomic_async_commit(struct 
> drm_atomic_state *state);
>  
>  #define for_each_connector_in_state(state, connector, connector_state, __i) \
>   for ((__i) = 0; \
> -  (connector) = (state)->connectors[__i],\
> -  (connector_state) = (state)->connector_states[__i],\
> -  (__i) < (state)->num_connector;\
> +  (__i) < (state)->num_connector &&  \
> +  ((connector) = (state)->connectors[__i],   \
> +  (connector_state) = (state)->connector_states[__i], 1);\

This will stop at the first NULL connector/connector_state. Similarly
for the loops below.

BR,
Jani.

>(__i)++)   \
>   if (connector)
>  
>  #define for_each_crtc_in_state(state, crtc, crtc_state, __i) \
>   for ((__i) = 0; \
> -  (crtc) = (state)->crtcs[__i],  \
> -  (crtc_state) = (state)->crtc_states[__i],  \
> -  (__i) < (state)->dev->mode_config.num_crtc;\
> +  (__i) < (state)->dev->mode_config.num_crtc &&  \
> +  ((crtc) = (state)->crtcs[__i], \
> +  (crtc_state) = (state)->crtc_states[__i], 1);  \
>(__i)++)   \
>   if (crtc_state)
>  
> -#define for_each_plane_in_state(state, plane, plane_state, __i)  \
> - for ((__i) = 0; \
> -  (plane) = (state)->planes[__i],\
> -  (plane_state) = (state)->plane_states[__i],\
> -  (__i) < (state)->dev->mode_config.num_total_plane; \
> -  (__i)++)   \
> +#define for_each_plane_in_state(state, plane, plane_state, __i)  
> \
> + for ((__i) = 0; \
> +  (__i) < (state)->dev->mode_config.num_total_plane &&   \
> +  ((plane) = (state)->planes[__i],   \
> +  (plane_state) = (state)->plane_states[__i], 1);\
> +  (__i)++)   \
>   if (plane_state)
>  
>  #endif /* DRM_ATOMIC_H_ */
> -- 
> 2.4.1
>
> ___
> 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 0/9 v5] Helper to abstract vma handling in media layer

[Jan Kara]

On Mon 25-05-15 13:37:56, Hans Verkuil wrote:
> Hi Jan,
> 
> On 05/13/2015 03:08 PM, Jan Kara wrote:
> >   Hello,
> > 
> > I'm sending the fifth version of my patch series to abstract vma handling
> > from the various media drivers. The patches got some review from mm people 
> > and
> > testing from device driver guys so unless someone objects, patches will be
> > queued in media tree for the next merge window.
> 
> What is the current status? I saw one comment for patch 9, so I assume it is 
> not
> quite ready yet.
> 
> Let me know when you think it is time to merge.
  There was a minor comment to the exynos patch - I've sent updated version
of that patch as a reply to the comment. Do you want me to resend the whole
series? Otherwise I think you can just pick up the patches and merge them.
Thanks!

Honza

> > After this patch set drivers have to know much less details about vmas, 
> > their
> > types, and locking. Also quite some code is removed from them. As a bonus
> > drivers get automatically VM_FAULT_RETRY handling. The primary motivation 
> > for
> > this series is to remove knowledge about mmap_sem locking from as many 
> > places a
> > possible so that we can change it with reasonable effort.
> > 
> > The core of the series is the new helper get_vaddr_frames() which is given a
> > virtual address and it fills in PFNs / struct page pointers (depending on 
> > VMA
> > type) into the provided array. If PFNs correspond to normal pages it also 
> > grabs
> > references to these pages. The difference from get_user_pages() is that this
> > function can also deal with pfnmap, and io mappings which is what the media
> > drivers need.
> > 
> > I have tested the patches with vivid driver so at least vb2 code got some
> > exposure. Conversion of other drivers was just compile-tested (for x86 so 
> > e.g.
> > exynos driver which is only for Samsung platform is completely untested).
> > 
> > Honza
> > Changes since v4:
> > * Minor cleanups and fixes pointed out by Mel and Vlasta
> > * Added Acked-by tags
> > 
> > Changes since v3:
> > * Added include  into mm/gup.c as it's needed for some 
> > archs
> > * Fixed error path for exynos driver
> > 
> > Changes since v2:
> > * Renamed functions and structures as Mel suggested
> > * Other minor changes suggested by Mel
> > * Rebased on top of 4.1-rc2
> > * Changed functions to get pointer to array of pages / pfns to perform
> >   conversion if necessary. This fixes possible issue in the omap I may have
> >   introduced in v2 and generally makes the API less errorprone.
> > --
> > 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
> > 
> 
-- 
Jan Kara 
SUSE Labs, CR

[GIT PULL FOR v4.1] ADV7511 driver fixes

[Laurent Pinchart]

And the subject line should obviously have mentioned v4.2, not v4.2. Sorry 
about that.

On Monday 25 May 2015 15:40:53 Laurent Pinchart wrote:
> Hi Dave,
> 
> The following changes since commit bdcddf95e82b1c4e370fc1196b1f4f50f775dab4:
> 
>   Backmerge v4.1-rc4 into into drm-next (2015-05-20 16:23:53 +1000)
> 
> are available in the git repository at:
> 
>   git://linuxtv.org/pinchartl/fbdev.git drm/next/adv7511
> 
> for you to fetch changes up to a1d0503d26ea2ef04f3f013d379e8f4d29c27127:
> 
>   drm: adv7511: Fix crash in IRQ handler when no encoder is associated
> (2015-05-25 15:34:00 +0300)
> 
> 
> Laurent Pinchart (1):
>   drm: adv7511: Fix crash in IRQ handler when no encoder is associated
> 
>  drivers/gpu/drm/i2c/adv7511.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

-- 
Regards,

Laurent Pinchart

[GIT PULL FOR v4.1] ADV7511 driver fixes

[Laurent Pinchart]

Hi Dave,

The following changes since commit bdcddf95e82b1c4e370fc1196b1f4f50f775dab4:

  Backmerge v4.1-rc4 into into drm-next (2015-05-20 16:23:53 +1000)

are available in the git repository at:

  git://linuxtv.org/pinchartl/fbdev.git drm/next/adv7511

for you to fetch changes up to a1d0503d26ea2ef04f3f013d379e8f4d29c27127:

  drm: adv7511: Fix crash in IRQ handler when no encoder is associated 
(2015-05-25 15:34:00 +0300)


Laurent Pinchart (1):
  drm: adv7511: Fix crash in IRQ handler when no encoder is associated

 drivers/gpu/drm/i2c/adv7511.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
Regards,

Laurent Pinchart

[GIT PULL FOR v4.2] R-Car DU driver fixes

[Laurent Pinchart]

Hi Dave,

The following changes since commit bdcddf95e82b1c4e370fc1196b1f4f50f775dab4:

  Backmerge v4.1-rc4 into into drm-next (2015-05-20 16:23:53 +1000)

are available in the git repository at:

  git://linuxtv.org/pinchartl/fbdev.git drm/next/rcar-du

for you to fetch changes up to 2610abfb3a72f6e5444a710b695c9460138f9326:

  drm: rcar-du: Split planes pre-association 4/4 between CRTCs (2015-05-25 
15:34:13 +0300)


Laurent Pinchart (10):
  drm: rcar-du: Document the rcar_du_crtc structure
  drm: rcar-du: Document the rcar_du_plane_state structure
  drm: rcar-du: Move properties from rcar_du_planes to rcar_du_device
  drm: rcar-du: Embed rcar_du_planes structure into rcar_du_group
  drm: rcar-du: Rename to_rcar_du_plane_state to to_rcar_plane_state
  drm: rcar-du: Add plane allocation debugging
  drm: rcar-du: Keep plane to CRTC associations when disabling a plane
  drm: rcar-du: Consider plane to CRTC associations in the plane allocator
  drm: rcar-du: Store the number of CRTCs per group in the group structure
  drm: rcar-du: Split planes pre-association 4/4 between CRTCs

 drivers/gpu/drm/rcar-du/rcar_du_crtc.c  |  61 ---
 drivers/gpu/drm/rcar-du/rcar_du_crtc.h  |  14 ++
 drivers/gpu/drm/rcar-du/rcar_du_drv.h   |   6 +++
 drivers/gpu/drm/rcar-du/rcar_du_group.c |   6 +++
 drivers/gpu/drm/rcar-du/rcar_du_group.h |   8 ++-
 drivers/gpu/drm/rcar-du/rcar_du_kms.c   | 105 ++
 drivers/gpu/drm/rcar-du/rcar_du_plane.c |  71 +--
 drivers/gpu/drm/rcar-du/rcar_du_plane.h |  21 
 8 files changed, 195 insertions(+), 97 deletions(-)

-- 
Regards,

Laurent Pinchart

[Bug 90510] [radeon][tahiti xt] IA_MULTI_VGT_PARAM programming seems broken

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

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

--- Comment #5 from Sylvain BERTRAND  ---
Based on work done from the mailing list, it seems the BO move may be related
to some of the remaining glitches.

-- 
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/20150525/1fb02ac5/attachment-0001.html>

[Bug 90510] [radeon][tahiti xt] IA_MULTI_VGT_PARAM programming seems broken

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

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

--- Comment #4 from Sylvain BERTRAND  ---
Thx to the mailing list, I caught the bug related to my issue:
#88978
verbatim llvm won't work properly for SI hardware at all till those
nasty bugs are solved (still some glitches left).

-- 
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/20150525/bd182690/attachment.html>

[PATCH] drm/i2c: tda998x: Use '%zd' for printing 'ssize_t' format

[Russell King - ARM Linux]

On Mon, May 25, 2015 at 10:17:25AM -0300, Fabio Estevam wrote:
> Use '%zd' for printing 'ssize_t' format in order to fix the following
> build warning on ARM64:
> 
> drivers/gpu/drm/i2c/tda998x_drv.c: In function 'tda998x_write_avi':
> drivers/gpu/drm/i2c/tda998x_drv.c:647:3: warning: format '%d' expects
> argument of type 'int', but argument 3 has type 'ssize_t' [-Wformat=]
> 
> Reported-by: Olof's autobuilder 
> Signed-off-by: Fabio Estevam 

I have a patch already queued for this.  I just need to assemble a few
other patches for David and decide how to get them to him

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

[PATCH 11/11] drm: atomic: Add MODE_ID property

[Daniel Stone]

Hi,

On 22 May 2015 at 15:34, Daniel Vetter  wrote:
> On Fri, May 22, 2015 at 01:34:54PM +0100, Daniel Stone wrote:
>> - /* FIXME: Mode prop is missing, which also controls ->enable. */
>>   if (property == config->prop_active)
>>   state->active = val;
>> + else if (property == config->prop_mode_id) {
>> + struct drm_property_blob *mode =
>> + drm_property_lookup_blob(dev, val);
>> + ret = drm_atomic_set_mode_prop_for_crtc(state, mode);
>> + if (mode)
>> + drm_property_unreference_blob(mode);
>
> Hm, maybe I need to revisit whether auto-clamping ->active is a good idea.
> We need it for legacy helpers, but for atomic userspace this code means
> depending upon whether active or mode_id is first in the prop list it will
> get clamped or not, which isn't awesome.
>
> Imo that's a good reason to remove the ->active clamping from
> set_mode_pop_for_crtc. I guess we can keep it for set_mode_for_crtc since
> that's only used internally and in legacy paths. Perhaps with a comment as
> to why (and why not in set_mode_prop).

No argument to not touching mode_changed, but I'm a bit confused about this one.

We don't touch ->active when setting a mode (i.e. if active is true
and you change MODE_ID without changing the ACTIVE prop, active
remains true; if active is false and you change MODE_ID, active
remains false, but it gains a mode).

I've been working on the following assumption:
  - enable is a proxy for having a valid mode (enable == !!MODE_ID)
  - active cannot be true without enable also being true

Setting MODE_ID to 0 removes the current mode, and when it becomes 0,
we can no longer report back a mode that we're scanning out. So how
would we have active == true (a particular mode is enabled and being
displayed), with no mode? Setting MODE_ID == 0 and ACTIVE == true in
the same request is a broken configuration which should be rejected.
Setting ACTIVE == true, MODE_ID == 0, MODE_ID == some_mode, is not
only pretty pathological but impossible with current libdrm, but
you're right that it should be respected.

So, I guess the way forward would be to not clamp either active or
enable, check that the dependencies (active -> enable -> MODE_ID) are
satisfied in drm_atomic_helper_check, and hope that everyone
implementing their own check gets it right too.

Sound good?

Cheers,
Daniel

[PATCH 0/9 v5] Helper to abstract vma handling in media layer

[Hans Verkuil]

Hi Jan,

On 05/13/2015 03:08 PM, Jan Kara wrote:
>   Hello,
> 
> I'm sending the fifth version of my patch series to abstract vma handling
> from the various media drivers. The patches got some review from mm people and
> testing from device driver guys so unless someone objects, patches will be
> queued in media tree for the next merge window.

What is the current status? I saw one comment for patch 9, so I assume it is not
quite ready yet.

Let me know when you think it is time to merge.

Regards,

Hans

> 
> After this patch set drivers have to know much less details about vmas, their
> types, and locking. Also quite some code is removed from them. As a bonus
> drivers get automatically VM_FAULT_RETRY handling. The primary motivation for
> this series is to remove knowledge about mmap_sem locking from as many places 
> a
> possible so that we can change it with reasonable effort.
> 
> The core of the series is the new helper get_vaddr_frames() which is given a
> virtual address and it fills in PFNs / struct page pointers (depending on VMA
> type) into the provided array. If PFNs correspond to normal pages it also 
> grabs
> references to these pages. The difference from get_user_pages() is that this
> function can also deal with pfnmap, and io mappings which is what the media
> drivers need.
> 
> I have tested the patches with vivid driver so at least vb2 code got some
> exposure. Conversion of other drivers was just compile-tested (for x86 so e.g.
> exynos driver which is only for Samsung platform is completely untested).
> 
>   Honza
> Changes since v4:
> * Minor cleanups and fixes pointed out by Mel and Vlasta
> * Added Acked-by tags
> 
> Changes since v3:
> * Added include  into mm/gup.c as it's needed for some archs
> * Fixed error path for exynos driver
> 
> Changes since v2:
> * Renamed functions and structures as Mel suggested
> * Other minor changes suggested by Mel
> * Rebased on top of 4.1-rc2
> * Changed functions to get pointer to array of pages / pfns to perform
>   conversion if necessary. This fixes possible issue in the omap I may have
>   introduced in v2 and generally makes the API less errorprone.
> --
> 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] drm/atomic: fix out of bounds read in for_each_*_in_state helpers

[Andrey Ryabinin]

for_each_*_in_state validate array index after
access to array elements, thus perform out of bounds read.

Fix this by validating index in the first place and read
array element iff validation was successful.

Fixes: df63b9994eaf ("drm/atomic: Add for_each_{connector,crtc,plane}_in_state 
helper macros")
Signed-off-by: Andrey Ryabinin 
---
 include/drm/drm_atomic.h | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
index c1571034..3f13b91 100644
--- a/include/drm/drm_atomic.h
+++ b/include/drm/drm_atomic.h
@@ -77,26 +77,26 @@ int __must_check drm_atomic_async_commit(struct 
drm_atomic_state *state);

 #define for_each_connector_in_state(state, connector, connector_state, __i) \
for ((__i) = 0; \
-(connector) = (state)->connectors[__i],\
-(connector_state) = (state)->connector_states[__i],\
-(__i) < (state)->num_connector;\
+(__i) < (state)->num_connector &&  \
+((connector) = (state)->connectors[__i],   \
+(connector_state) = (state)->connector_states[__i], 1);\
 (__i)++)   \
if (connector)

 #define for_each_crtc_in_state(state, crtc, crtc_state, __i)   \
for ((__i) = 0; \
-(crtc) = (state)->crtcs[__i],  \
-(crtc_state) = (state)->crtc_states[__i],  \
-(__i) < (state)->dev->mode_config.num_crtc;\
+(__i) < (state)->dev->mode_config.num_crtc &&  \
+((crtc) = (state)->crtcs[__i], \
+(crtc_state) = (state)->crtc_states[__i], 1);  \
 (__i)++)   \
if (crtc_state)

-#define for_each_plane_in_state(state, plane, plane_state, __i)\
-   for ((__i) = 0; \
-(plane) = (state)->planes[__i],\
-(plane_state) = (state)->plane_states[__i],\
-(__i) < (state)->dev->mode_config.num_total_plane; \
-(__i)++)   \
+#define for_each_plane_in_state(state, plane, plane_state, __i)
\
+   for ((__i) = 0; \
+(__i) < (state)->dev->mode_config.num_total_plane &&   \
+((plane) = (state)->planes[__i],   \
+(plane_state) = (state)->plane_states[__i], 1);\
+(__i)++)   \
if (plane_state)

 #endif /* DRM_ATOMIC_H_ */
-- 
2.4.1

[Bug 90320] Lenovo ThinkPad E455 (Kaveri A10-7300): Blank built-in screen with radeon kms driver

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

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

--- Comment #13 from Samir Ibradžić  ---
bump. Anything else I can try?

-- 
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/20150525/4d59936b/attachment.html>

[PATCH] Add device enumeration interface (v3)

[Zhou, Jammy]

Hi Emil,

Do you have chance to have a look at this new version? Thanks!

Regards,
Jammy

-Original Message-
From: dri-devel [mailto:dri-devel-boun...@lists.freedesktop.org] On Behalf Of 
Zhou, Jammy
Sent: Tuesday, May 19, 2015 11:31 PM
To: dri-devel at lists.freedesktop.org; emil.l.velikov at gmail.com
Cc: Min, Frank
Subject: [PATCH] Add device enumeration interface (v3)

From: frank 

v3: switch to udev/sysfs for the enumeration

Signed-off-by: Frank Min 
Reviewed-by: Christian König 
Reviewed-by: Alex Deucher 
Reviewed-by: Jammy Zhou 
---
 Makefile.am |   7 +++--
 xf86drm.c   | 103 
 xf86drm.h   |  19 +++
 3 files changed, 127 insertions(+), 2 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 13df80c..ffd334a 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -95,12 +95,15 @@ 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@ \
+   @LIBUDEV_LIBS@

 libdrm_la_CPPFLAGS = -I$(top_srcdir)/include/drm  AM_CFLAGS = \
$(WARN_CFLAGS) \
-   $(VALGRIND_CFLAGS)
+   $(VALGRIND_CFLAGS) \
+   $(LIBUDEV_CFLAGS)

 libdrm_la_SOURCES = $(LIBDRM_FILES)

diff --git a/xf86drm.c b/xf86drm.c
index f7c45f8..d2704de 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -63,6 +63,7 @@

 #include "xf86drm.h"
 #include "libdrm_macros.h"
+#include "libudev.h"

 #if defined(__FreeBSD__) || defined(__FreeBSD_kernel__) || 
defined(__DragonFly__)  #define DRM_MAJOR 145 @@ -2817,3 +2818,105 @@ 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 drmGetPciDevices(drmPciDevicePtr devSet, uint16_t vendorId) {
+   struct udev *udev = NULL;
+   struct udev_enumerate *udev_enumerate;
+   struct udev_list_entry *list_entry;
+   struct udev_device *device;
+   int drmDevCount = 0;
+   int vendor = 0;
+   int devid = 0;
+   int devclass = 0;
+   int subvendor = 0;
+   int subdevid = 0;
+   int revid = 0xff;
+   int domain = 0;
+   int bus = 0;
+   int dev = 0;
+   int func = 0;
+   char config[64] = {0};
+   char node[128] = {'\0'};
+   char slot[] = ":00:00.0";
+   char *info = NULL;
+   int minor = 0xff;
+   int fd = 0;
+   int ret = 0;
+
+   udev = udev_new();
+   if (udev == NULL) {
+   fprintf(stderr, "no context\n");
+   return -EINVAL;
+   }
+   udev_enumerate = udev_enumerate_new(udev);
+   if (udev_enumerate == NULL)
+   return -EINVAL;
+   udev_enumerate_add_match_subsystem(udev_enumerate, "drm");
+   udev_enumerate_add_match_property(udev_enumerate, "DEVTYPE", 
+"drm_minor");
+
+   udev_enumerate_scan_devices(udev_enumerate);
+
+   udev_list_entry_foreach(list_entry, 
udev_enumerate_get_list_entry(udev_enumerate)) {
+   device = 
udev_device_new_from_syspath(udev_enumerate_get_udev(udev_enumerate),
+ 
udev_list_entry_get_name(list_entry));
+   if (device != NULL) {
+   info = udev_device_get_property_value(device, "MINOR");
+   if (!strcmp(info, "0")) {
+   strcpy(node, udev_device_get_syspath(device));
+   info = strstr(node, "/drm");
+   strncpy(slot, info - strlen(slot), strlen(slot));
+   if (sscanf(slot, "%4x:%2x:%2x.%1x", , , 
, ) != 4) {
+   domain = 0;
+   bus = 0;
+   dev = 0;
+   func = 0;
+   }
+   strcpy(node + strlen(node), "/device/config");
+
+   fd = open(node, O_RDONLY);
+   if (fd >= 0) {
+   ret = read(fd, config, 64);
+   if (ret == 64) {
+   vendor = config[0] + (config[1] << 8);
+   devid = config[2] + (config[3] << 8);
+   revid = config[8];
+   devclass = config[9] + (config[10] << 
8) + (config[11] << 16);
+   subdevid = config[44] + (config[45] << 
8);
+   }
+   close(fd);
+   }
+
+   

[Bug 90537] radeonsi bo/va conflict on RADEON_GEM_VA (rscreen->ws->buffer_from_handle returns NULL)

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

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

--- Comment #15 from Christian König  ---
(In reply to pstglia from comment #12)
> It was reported by one member that patch is causing stability problems for
> HD 7950 (TAHITI). I've attached the full dmesg. The volunteer tested the
> patch with Mesa 10.5.4 and 10.6 RC1. Same behavior in both cases according
> to him

The mesa version is uninteresting, but what kernel version are you using?

Older kernels have stability problems when you unmap the BO VA. That's why we
don't use it in mesa by default.

Regards,
Christian.

-- 
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/20150525/7ca61ea2/attachment.html>

[PATCH 3/4] drm/tegra: Add VIC support

[Arto Merilainen]

Hi Thierry,

Thank you for your thorough analysis - and sorry for a bunch of very 
silly mistakes.

I am skipping most trivial parts and focus on the trickier comments and 
questions.

On 05/22/2015 02:47 PM, Thierry Reding wrote:
>> +
>> +struct tegra_bo *ucode_bo;
>> +bool ucode_valid;
>> +void *ucode_vaddr;
>> +
>> +bool booted;
>
> There are a bunch of other drivers that use a Falcon and they will all
> need to use similar data to this to deal with the firmware and all. I
> would like to see that code to be made into a Falcon library so that it
> can be reused in a meaningful way.
>
> Roughly this would look like this:
>
>   struct falcon {
>   struct device *dev;
>   ...
>   };
>
>   int falcon_init(struct falcon *falcon, struct device *dev,
>   void __iomem *regs);
>   int falcon_load_firmware(struct falcon *falcon, const char *filename);
>   int falcon_exit(struct falcon *falcon);
>   int falcon_boot(struct falcon *falcon);
>

There are two issues in above scheme..:
- Memory allocation. Despite I have explicitly mentioned that the series 
has been tested only with iommu disabled, I would prefer trying to keep 
an option to use it later. Depending how well we want to isolate the 
falcon library from other parts of tegradrm, the library may not have 
ability to map anything to the tegradrm iommu domain.
- The firmware images may not hold only Falcon firmware. Already in VIC 
case we have two firmwares: One for Falcon, another for FCE.

To overcome the above issues, I would prefer dropping 
falcon_load_firmware() and keeping firmware image specifics inside the 
client driver and giving data related to Falcon as a parameter for 
falcon_boot(). Would this be ok?

>> +
>> +/* for firewall - this determines if method 1 should be regarded
>> + * as an address register */
>> +bool method_data_is_addr_reg;
>> +};
>
> I think it'd be best to incorporate that functionality into the firewall
> so that we can deal with it more centrally rather that duplicate this in
> all drivers.
>

Do you have a concrete suggestion how this should be done? Firewall has 
no access to the driver specifics and in VIC case the VIC methods 
themselves define whether the METHOD1 includes address or not.

>> +static int vic_dma_pa_to_internal_256b(struct vic *vic, phys_addr_t pa,
>> +   u32 internal_offset, bool imem)
>
> The name is confusing. Without looking at the code I'd expect this to
> perform some kind of conversion from a physical address to some internal
> address, but if I understand correctly this actually copies code into
> the Falcon instruction or data memories. A better name I think would be:
>
>   static int falcon_load_chunk(struct falcon *falcon, phys_addr_t phys,
>unsigned long offset, enum falcon_memory 
> target);
>
> Note that this is now part of the Falcon library because I've seen
> identical code used in several other drivers. Also the bool argument is
> now an enumeration, which makes it much easier to read. Compare:
>
>   err = vic_dma_pa_to_internal_256b(vic, phys, offset, true);
>
> and
>
>   err = falcon_load_chunk(>falcon, phys, offset, FALCON_MEMORY_IMEM);
>

Sounds ok.

> Is there a specific term in Falcon-speak for these 256 byte blocks?
> "chunk" is a little awkward.
>

Unfortunately I am not aware that there would be - maybe "block"?

>> +if (ucode.bin_header->bin_ver != 1) {
>> +dev_err(vic->dev, "unsupported firmware version");
>> +return -ENOENT;
>
> Why not -EINVAL here, too?
>

We can interpret the issue two ways..: The given firmware is invalid 
(-EINVAL) or that there was no supported firmware entry available (-ENOENT).

I do not have strong opinions and will change this to -EINVAL.

>> +vic->ucode_bo = tegra_bo_create(dev, ucode_fw->size, 0);
>> +if (IS_ERR(vic->ucode_bo)) {
>> +dev_err(vic->dev, "dma memory allocation failed");
>> +err = PTR_ERR(vic->ucode_bo);
>> +goto err_alloc_iova;
>> +}
>
> Erm... no. Please don't use tegra_bo_create() to allocate these buffers.
> tegra_bo_create() creates GEM objects and firmware doesn't qualify as a
> GEM object.
>
> Can't you use the DMA API here?
>

The firmware must be mapped to the IOMMU domain into which VIC is 
attached - and I would prefer keeping the door open for enabling iommu 
on VIC. This was the simplest way to get a buffer that is allocated to 
the tegradrm iommu domain.

Should I add a function for allocating memory without making a gem 
object or should I keep memory allocation here and simply add functions 
for mapping it into tegradrm domain?

>> +static int vic_boot(struct device *dev)
>> +{
>> +struct vic *vic = dev_get_drvdata(dev);
>> +u32 offset;
>> +int err = 0;
>> +
>> +if (vic->booted)
>> +return 0;
>> +
>> +if (!vic->ucode_valid) {
>> +

[Bug 98751] radeon: multihead "no signal" on one of the monitors - regression

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

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

Andy Furniss  changed:

   What|Removed |Added

 CC||adf.lists at gmail.com

--- Comment #9 from Andy Furniss  ---
I just hit this one with my R9270X.

Reverting "drm/radeon: adjust pll when audio is not enabled" fixes it.

For me starting with DVI on and enabling HDMI results in loosing DVI. If I off
then auto DVI it comes back but HDMI goes off, repeat for HDMI = DVI off again
and so on.

I did find a way to get both working = xset dpms force off 
then coming out of dpms both will come on an behave as if nothing was ever
wrong (I can flip either on/off without affecting the other).

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

[PATCH] drm/msm/mdp5: Always generate active-high sync signals for DSI

[Archit Taneja]

On 05/22/2015 07:46 PM, Hai Li wrote:
> DSI video mode engine can only take active-high sync signals. This
> change prevents MDP5 sending active-low sync signals to DSI in any
> case.
>
> Signed-off-by: Hai Li 

Tested-by: Archit Taneja 

> ---
>   drivers/gpu/drm/msm/mdp/mdp5/mdp5_encoder.c | 12 
>   1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_encoder.c 
> b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_encoder.c
> index 53bb1f7..2e98ce0 100644
> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_encoder.c
> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_encoder.c
> @@ -144,10 +144,14 @@ static void mdp5_encoder_mode_set(struct drm_encoder 
> *encoder,
>   mode->type, mode->flags);
>
>   ctrl_pol = 0;
> - if (mode->flags & DRM_MODE_FLAG_NHSYNC)
> - ctrl_pol |= MDP5_INTF_POLARITY_CTL_HSYNC_LOW;
> - if (mode->flags & DRM_MODE_FLAG_NVSYNC)
> - ctrl_pol |= MDP5_INTF_POLARITY_CTL_VSYNC_LOW;
> +
> + /* DSI controller cannot handle active-low sync signals. */
> + if (mdp5_encoder->intf.type != INTF_DSI) {
> + if (mode->flags & DRM_MODE_FLAG_NHSYNC)
> + ctrl_pol |= MDP5_INTF_POLARITY_CTL_HSYNC_LOW;
> + if (mode->flags & DRM_MODE_FLAG_NVSYNC)
> + ctrl_pol |= MDP5_INTF_POLARITY_CTL_VSYNC_LOW;
> + }
>   /* probably need to get DATA_EN polarity from panel.. */
>
>   dtv_hsync_skew = 0;  /* get this from panel? */
>

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

[Bug 98751] radeon: multihead "no signal" on one of the monitors - regression

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

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

--- Comment #8 from Mikkel Oscar Lyderik  ---
It's actually a 6950 card I have, same as in #98651.

I called it 6990 because that is what lspci tells me, but that might narrow it
down?

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

[RESEND PATCH v1 2/2] drm: bridge/dw_hdmi-i2s-audio: add audio driver

[Paul Bolle]

Just a nit: a license mismatch.

On Fri, 2015-05-22 at 10:14 -0500, Yakir Yang wrote:
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/dw_hdmi-i2s-audio.c

> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.

This states the license is GPL v2.

> +MODULE_LICENSE("GPL");

And, according to include/linux/module.h, this states the license is GPL
v2 or later. So I think that either the comment at the top of this file
or the ident used in the MODULE_LICENSE() macro should change.


Paul Bolle

[Bug 98751] radeon: multihead "no signal" on one of the monitors - regression

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

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

Robin Bankhead  changed:

   What|Removed |Added

 CC||kernel at headbank.co.uk

--- Comment #7 from Robin Bankhead  ---
Duplicate of #98651.

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

[PATCH] drm/i2c: tda998x: Use '%zd' for printing 'ssize_t' format

[Fabio Estevam]

Use '%zd' for printing 'ssize_t' format in order to fix the following
build warning on ARM64:

drivers/gpu/drm/i2c/tda998x_drv.c: In function 'tda998x_write_avi':
drivers/gpu/drm/i2c/tda998x_drv.c:647:3: warning: format '%d' expects
argument of type 'int', but argument 3 has type 'ssize_t' [-Wformat=]

Reported-by: Olof's autobuilder 
Signed-off-by: Fabio Estevam 
---
 drivers/gpu/drm/i2c/tda998x_drv.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c 
b/drivers/gpu/drm/i2c/tda998x_drv.c
index 011e1ca..d5dc8aa 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -644,7 +644,8 @@ tda998x_write_avi(struct tda998x_priv *priv, struct 
drm_display_mode *mode)

len = hdmi_avi_infoframe_pack(, buf, sizeof(buf));
if (len < 0) {
-   dev_err(>hdmi->dev, "hdmi_avi_infoframe_pack() failed: 
%d\n", len);
+   dev_err(>hdmi->dev, "hdmi_avi_infoframe_pack() failed: 
%zd\n",
+   len);
return;
}

-- 
1.9.1

[PATCH 3/4] drm/tegra: Add VIC support

[Arto Merilainen]

On 05/22/2015 01:25 PM, Thierry Reding wrote:
> * PGP Signed by an unknown key
>
> On Thu, May 21, 2015 at 05:40:31PM +0300, Mikko Perttunen wrote:
>> On 05/21/2015 04:20 PM, Arto Merilainen wrote:
> [...]
>>> +static int vic_is_addr_reg(struct device *dev, u32 class, u32 offset, u32 
>>> val)
>>> +{
>>> +   struct vic *vic = dev_get_drvdata(dev);
>>> +
>>> +   /* handle host class */
>>> +   if (class == HOST1X_CLASS_HOST1X) {
>>> +   if (offset == 0x2b)
>>> +   return true;
>>> +   return false;
>>
>> "return (offset == 0x2b);" perhaps?
>
> I think this should really be extracted into a separate helper. If we
> ever need to take into account additional offsets we would otherwise
> have to extend every driver rather than just the helper.

I agree, that would be better.

>
> Also I think the 0x2b should be replaced by some symbolic name.
> According to the TRM 0x2b is the host1x class method named
> NV_CLASS_HOST_INDCTRL_0. Oddly enough that doesn't seem to be an address
> register. Instead the address seems to be in the INDOFF2 and INDOFF
> methods (0x2c and 0x2d). I also can't tell from the TRM what exactly
> these are supposed to do.
>
> Arto, can you clarify?

This looks like an unfortunate mistake that got reproduced from gr2d and 
gr3d.

The INDCTRL method is used for indirect register accessing and it allows 
Host1x to read registers of an engine - or write data directly to 
memory. It allow implementing context switch for the clients whose state 
should be not change between jobs from the same application.

>
>>> +   if (IS_ERR(vic->rst)) {
>>> +   dev_err(>dev, "cannot get reset\n");
>>> +   return PTR_ERR(vic->rst);
>>> +   }
>>> +
>>> +   platform_set_drvdata(pdev, vic);
>>> +
>>> +   INIT_LIST_HEAD(>client.base.list);
>>> +   vic->client.base.ops = _client_ops;
>>> +   vic->client.base.dev = dev;
>>> +   vic->client.base.class = vic_config->class_id;
>>> +   vic->client.base.syncpts = syncpts;
>>> +   vic->client.base.num_syncpts = 1;
>>> +   vic->dev = dev;
>>> +   vic->config = vic_config;
>>> +
>>> +   INIT_LIST_HEAD(>client.list);
>>> +   vic->client.ops = _ops;
>>> +
>>> +   err = tegra_powergate_sequence_power_up(vic->config->powergate_id,
>>> +   vic->clk, vic->rst);
>>> +   if (err) {
>>> +   dev_err(dev, "cannot turn on the device\n");
>>> +   return err;
>>> +   }
>>> +
>>> +   err = host1x_client_register(>client.base);
>>> +   if (err < 0) {
>>
>> You used 'if (err) {' previously, so maybe also here.
>
> For consistency with other Tegra DRM code these checks should use (at
> least where possible) the (err < 0) notation.
>

Will fix.

- Arto

[Intel-gfx] drivers/gpu/drm/i915/i915_gem_gtt.c

[Jani Nikula]

On Sat, 23 May 2015, Andrew Morton  wrote:
> On Sat, 23 May 2015 14:30:09 +0100 Damien Lespiau  intel.com> wrote:
>
>> On Fri, May 22, 2015 at 02:17:32PM -0700, Andrew Morton wrote:
>> > I'm not sure what's happened to the drm code in linux-next - it's
>> > exploding all over the place.  Did someone turn on -Werror without
>> > doing anywhere near enough testing?
>> > 
>> > Anyway, I don't know how to fix this i386 build error:
>> 
>> Seems like you have CONFIG_DRM_I915_WERROR set?
>
> Yes.
>
>> We explicitely made sure to not enable -Werror by default,
>
> `make allmodconfig' enables CONFIG_DRM_I915_WERROR.

Right.

> I'm not sure what is the approved way of fixing this.  Perhaps
> disabling CONFIG_DRM_I915_WERROR when CONFIG_COMPILE_TEST=y.

Maybe the answer right now is to just drop that config. Daniel?


BR,
Jani.


-- 
Jani Nikula, Intel Open Source Technology Center