[Bug 76490] Hang during boot when DPM is on (R9 270X)
https://bugs.freedesktop.org/show_bug.cgi?id=76490 --- Comment #63 from Stefan Ott --- I appear to have the same issue on an ASUS STRIX R7 370. It's also a factory-overclocked card and radeon.dpm=0 seems to work. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151112/343a02bf/attachment.html>
[Bug 76490] Hang during boot when DPM is on (R9 270X)
https://bugs.freedesktop.org/show_bug.cgi?id=76490 almos changed: What|Removed |Added CC||aaalmosss at gmail.com --- Comment #62 from almos --- I also have problems with dpm on my ASUS R9 270X. Under no load and high load it seems stable, but with low load (e.g. playing an old game, or watching a video with mpv -vo opengl) it is very unstable. It suddenly switches to white screen, and the machine is hardlocked. I couldn't reach more than 2-3 days of uptime. Since I activated the profile method and I switch manually between low and high states, it hasn't crashed. It also seems stable in windows. My guess is that it can't properly handle frequent switching between power level 0 and 1, where all clocks and voltages change at once (or maybe it's just the memory reclocking?). -- 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/20151112/4c5100b7/attachment.html>
__i915_spin_request() sucks
On Thu, Nov 12, 2015 at 01:40:33PM -0700, Jens Axboe wrote: > On 11/12/2015 01:36 PM, Jens Axboe wrote: > >Hi, > > > >So a few months ago I got an XPS13 laptop, the one with the high res > >screen. GUI performance was never really that great, I attributed it to > >coming from a more powerful laptop, and the i915 driving a lot of > >pixels. But yesterday I browsed from my wife's macbook, and was blown > >away. Wow, scrolling in chrome SUCKS on the xps13. Not just scrolling, > >basically anything in chrome. Molasses. So I got sick of it, fired up a > >quick perf record, did a bunch of stuff in chrome. No super smoking > >guns, but one thing did stick out - the path leading to > >__i915_spin_request(). smoking gun pointing at the messenger normally. > >So today, I figured I'd try just killing that spin. If it fails, we'll > >punt to normal completions, so easy change. And wow, MASSIVE difference. > >I can now scroll in chrome and not rage! It's like the laptop is 10x > >faster now. > > > >Ran git blame, and found: > > > >commit 2def4ad99befa25775dd2f714fdd4d92faec6e34 > >Author: Chris Wilson > >Date: Tue Apr 7 16:20:41 2015 +0100 > > > > drm/i915: Optimistically spin for the request completion > > > >and read the commit message. Doesn't sound that impressive. Especially > >not for something that screws up interactive performance by a LOT. > > > >What's the deal? Revert? The tests that it improved the most were the latency sensitive tests and since my Broadwell xps13 behaves itself, I'd like to understand how it culminates in an interactivity loss. 1. Maybe it is the uninterruptible nature of the polling, making X's SIGIO jerky: gitt a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 19e8f5442cf8..8099c2a9f88e 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -1146,7 +1146,7 @@ static bool missed_irq(struct drm_i915_private *dev_priv, return test_bit(ring->id, _priv->gpu_error.missed_irq_rings); } -static int __i915_spin_request(struct drm_i915_gem_request *req) +static int __i915_spin_request(struct drm_i915_gem_request *req, int state) { unsigned long timeout; @@ -1161,6 +1161,9 @@ static int __i915_spin_request(struct drm_i915_gem_request *req) if (time_after_eq(jiffies, timeout)) break; + if (signal_pending_state(state, current)) + break; + cpu_relax_lowlatency(); } if (i915_gem_request_completed(req, false)) @@ -1197,6 +1200,7 @@ int __i915_wait_request(struct drm_i915_gem_request *req, struct drm_i915_private *dev_priv = dev->dev_private; const bool irq_test_in_progress = ACCESS_ONCE(dev_priv->gpu_error.test_irq_rings) & intel_ring_flag(ring); + int state = interruptible ? TASK_INTERRUPTIBLE : TASK_UNINTERRUPTIBLE; DEFINE_WAIT(wait); unsigned long timeout_expire; s64 before, now; @@ -1221,7 +1225,7 @@ int __i915_wait_request(struct drm_i915_gem_request *req, before = ktime_get_raw_ns(); /* Optimistic spin for the next jiffie before touching IRQs */ - ret = __i915_spin_request(req); + ret = __i915_spin_request(req, state); if (ret == 0) goto out; @@ -1233,8 +1237,7 @@ int __i915_wait_request(struct drm_i915_gem_request *req, for (;;) { struct timer_list timer; - prepare_to_wait(>irq_queue, , - interruptible ? TASK_INTERRUPTIBLE : TASK_UNINTERRUPTIBLE); + prepare_to_wait(>irq_queue, , state); /* We need to check whether any gpu reset happened in between * the caller grabbing the seqno and now ... */ @@ -1252,7 +1255,7 @@ int __i915_wait_request(struct drm_i915_gem_request *req, break; } - if (interruptible && signal_pending(current)) { + if (signal_pending_state(state, current)) { ret = -ERESTARTSYS; break; } 2. Or maybe it is increased mutex contention: diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index a275b0478200..1e52a7444e0c 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2973,9 +2973,11 @@ void __i915_add_request(struct drm_i915_gem_request *req, __i915_add_request(req, NULL, false) int __i915_wait_request(struct drm_i915_gem_request *req, unsigned reset_counter, - bool interruptible, + unsigned flags, s64 *timeout, struct intel_rps_client *rps); +#define WAIT_INTERRUPTIBLE 0x1 +#define WAIT_UNLOCKED 0x2 int __must_check i915_wait_request(struct drm_i915_gem_request *req); int i915_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf);
[Bug 92907] ARB_copy_image breaks recording with obs-studio
https://bugs.freedesktop.org/show_bug.cgi?id=92907 Ilia Mirkin changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |NOTOURBUG --- Comment #8 from Ilia Mirkin --- Closing this for now. Feel free to reopen should analysis on the obs-studio side disagree with my findings. -- 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/20151112/d1c65cca/attachment.html>
[PATCH v2 8/8] drm/i915: Give meaningful names to all the planes
From: Ville SyrjäläLet's name our planes in a way that makes sense wrt. the spec: - skl+ -> "plane 1A", "plane 2A", "plane 1C", "cursor A" etc. - g4x+ -> "primary A", "primary B", "sprite A", "cursor C" etc. - pre-g4x -> "plane A", "cursor B" etc. v2: Rebase on top of the fixed/cleaned error paths Use a local 'name' variable to make things easier Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_display.c | 30 ++ drivers/gpu/drm/i915/intel_sprite.c | 14 ++ 2 files changed, 44 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index bb49d7d..15b133e 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -13787,10 +13787,18 @@ static void intel_finish_crtc_commit(struct drm_crtc *crtc, */ void intel_plane_destroy(struct drm_plane *plane) { + char *name; + if (!plane) return; + /* +* drm_plane_cleanup() zeroes the structure, so +* need an extra dance to avoid leaking the name. +*/ + name = plane->name; drm_plane_cleanup(plane); + kfree(name); kfree(to_intel_plane(plane)); } @@ -13811,6 +13819,7 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev, { struct intel_plane *primary = NULL; struct intel_plane_state *state = NULL; + char *name = NULL; const uint32_t *intel_primary_formats; unsigned int num_formats; int ret; @@ -13839,6 +13848,19 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev, if (HAS_FBC(dev) && INTEL_INFO(dev)->gen < 4) primary->plane = !pipe; + if (INTEL_INFO(dev)->gen >= 9) + name = kasprintf(GFP_KERNEL, "plane 1%c", +pipe_name(pipe)); + else if (INTEL_INFO(dev)->gen >= 5 || IS_G4X(dev)) + name = kasprintf(GFP_KERNEL, "primary %c", +pipe_name(pipe)); + else + name = kasprintf(GFP_KERNEL, "plane %c", +plane_name(primary->plane)); + if (!name) + goto fail; + primary->base.name = name; + if (INTEL_INFO(dev)->gen >= 9) { intel_primary_formats = skl_primary_formats; num_formats = ARRAY_SIZE(skl_primary_formats); @@ -13865,6 +13887,7 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev, return >base; fail: + kfree(name); kfree(state); kfree(primary); @@ -13975,6 +13998,7 @@ static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev, { struct intel_plane *cursor = NULL; struct intel_plane_state *state = NULL; + char *name = NULL; int ret; cursor = kzalloc(sizeof(*cursor), GFP_KERNEL); @@ -13995,6 +14019,11 @@ static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev, cursor->commit_plane = intel_commit_cursor_plane; cursor->disable_plane = intel_disable_cursor_plane; + name = kasprintf(GFP_KERNEL, "cursor %c", pipe_name(pipe)); + if (!name) + goto fail; + cursor->base.name = name; + ret = drm_universal_plane_init(dev, >base, 0, _plane_funcs, intel_cursor_formats, @@ -14023,6 +14052,7 @@ static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev, return >base; fail: + kfree(name); kfree(state); kfree(cursor); diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 44e1e39..50e53ee 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -1040,6 +1040,7 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane) { struct intel_plane *intel_plane = NULL; struct intel_plane_state *state = NULL; + char *name = NULL; unsigned long possible_crtcs; const uint32_t *plane_formats; int num_plane_formats; @@ -1123,6 +1124,18 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane) intel_plane->check_plane = intel_check_sprite_plane; intel_plane->commit_plane = intel_commit_sprite_plane; + if (INTEL_INFO(dev)->gen >= 9) + name = kasprintf(GFP_KERNEL, "plane %d%c", +plane + 2, pipe_name(pipe)); + else + name = kasprintf(GFP_KERNEL, "sprite %c", +sprite_name(pipe, plane)); + if (!name) { + ret = -ENOMEM; + goto fail; + } + intel_plane->base.name = name; + possible_crtcs = (1 << pipe); ret = drm_universal_plane_init(dev, _plane->base, possible_crtcs,
[PATCH 7/8] drm/i915: Don't leak primary/cursor planes on crtc init failure
From: Ville SyrjäläCall intel_plane_destroy() instead of drm_plane_cleanup() so that we also free the plane struct itself when bailing out of the crtc init. And make intel_plane_destroy() NULL tolerant to avoid having to check for it in the caller. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_display.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 752283c..bb49d7d 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -13787,9 +13787,11 @@ static void intel_finish_crtc_commit(struct drm_crtc *crtc, */ void intel_plane_destroy(struct drm_plane *plane) { - struct intel_plane *intel_plane = to_intel_plane(plane); + if (!plane) + return; + drm_plane_cleanup(plane); - kfree(intel_plane); + kfree(to_intel_plane(plane)); } const struct drm_plane_funcs intel_plane_funcs = { @@ -14127,10 +14129,8 @@ static void intel_crtc_init(struct drm_device *dev, int pipe) return; fail: - if (primary) - drm_plane_cleanup(primary); - if (cursor) - drm_plane_cleanup(cursor); + intel_plane_destroy(primary); + intel_plane_destroy(cursor); kfree(name); kfree(crtc_state); kfree(intel_crtc); -- 2.4.10
[PATCH 6/8] drm/i915: Fix plane init failure paths
From: Ville SyrjäläDeal with errors from drm_universal_plane_init() in primary and cursor plane init paths (sprites were already covered). Also make the code neater by using goto for error handling. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_display.c | 64 ++-- drivers/gpu/drm/i915/intel_sprite.c | 34 +++ 2 files changed, 60 insertions(+), 38 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 972e17f..752283c 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -13807,20 +13807,19 @@ const struct drm_plane_funcs intel_plane_funcs = { static struct drm_plane *intel_primary_plane_create(struct drm_device *dev, int pipe) { - struct intel_plane *primary; - struct intel_plane_state *state; + struct intel_plane *primary = NULL; + struct intel_plane_state *state = NULL; const uint32_t *intel_primary_formats; unsigned int num_formats; + int ret; primary = kzalloc(sizeof(*primary), GFP_KERNEL); - if (primary == NULL) - return NULL; + if (!primary) + goto fail; state = intel_create_plane_state(>base); - if (!state) { - kfree(primary); - return NULL; - } + if (!state) + goto fail; primary->base.state = >base; primary->can_scale = false; @@ -13849,10 +13848,12 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev, num_formats = ARRAY_SIZE(i8xx_primary_formats); } - drm_universal_plane_init(dev, >base, 0, -_plane_funcs, -intel_primary_formats, num_formats, -DRM_PLANE_TYPE_PRIMARY); + ret = drm_universal_plane_init(dev, >base, 0, + _plane_funcs, + intel_primary_formats, num_formats, + DRM_PLANE_TYPE_PRIMARY); + if (ret) + goto fail; if (INTEL_INFO(dev)->gen >= 4) intel_create_rotation_property(dev, primary); @@ -13860,6 +13861,12 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev, drm_plane_helper_add(>base, _plane_helper_funcs); return >base; + +fail: + kfree(state); + kfree(primary); + + return NULL; } void intel_create_rotation_property(struct drm_device *dev, struct intel_plane *plane) @@ -13964,18 +13971,17 @@ update: static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev, int pipe) { - struct intel_plane *cursor; - struct intel_plane_state *state; + struct intel_plane *cursor = NULL; + struct intel_plane_state *state = NULL; + int ret; cursor = kzalloc(sizeof(*cursor), GFP_KERNEL); - if (cursor == NULL) - return NULL; + if (!cursor) + goto fail; state = intel_create_plane_state(>base); - if (!state) { - kfree(cursor); - return NULL; - } + if (!state) + goto fail; cursor->base.state = >base; cursor->can_scale = false; @@ -13987,11 +13993,13 @@ static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev, cursor->commit_plane = intel_commit_cursor_plane; cursor->disable_plane = intel_disable_cursor_plane; - drm_universal_plane_init(dev, >base, 0, -_plane_funcs, -intel_cursor_formats, -ARRAY_SIZE(intel_cursor_formats), -DRM_PLANE_TYPE_CURSOR); + ret = drm_universal_plane_init(dev, >base, 0, + _plane_funcs, + intel_cursor_formats, + ARRAY_SIZE(intel_cursor_formats), + DRM_PLANE_TYPE_CURSOR); + if (ret) + goto fail; if (INTEL_INFO(dev)->gen >= 4) { if (!dev->mode_config.rotation_property) @@ -14011,6 +14019,12 @@ static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev, drm_plane_helper_add(>base, _plane_helper_funcs); return >base; + +fail: + kfree(state); + kfree(cursor); + + return NULL; } static void skl_init_scalers(struct drm_device *dev, struct intel_crtc *intel_crtc, diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index a2c15f8..44e1e39 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++
[PATCH v2 5/8] drm/i915: Set crtc->name to "pipe A", "pipe B", etc.
From: Ville Syrjäläv2: Fix intel_crtc leak on failure to allocate the name Use a local 'name' variable to make things easier Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_display.c | 21 + 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index b628dab..972e17f 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -10711,6 +10711,7 @@ static void intel_crtc_destroy(struct drm_crtc *crtc) struct intel_crtc *intel_crtc = to_intel_crtc(crtc); struct drm_device *dev = crtc->dev; struct intel_unpin_work *work; + char *name; spin_lock_irq(>event_lock); work = intel_crtc->unpin_work; @@ -10722,8 +10723,13 @@ static void intel_crtc_destroy(struct drm_crtc *crtc) kfree(work); } + /* +* drm_crtc_cleanup() zeroes the structure, so +* need an extra dance to avoid leaking the name. +*/ + name = crtc->name; drm_crtc_cleanup(crtc); - + kfree(name); kfree(intel_crtc); } @@ -14026,15 +14032,16 @@ static void skl_init_scalers(struct drm_device *dev, struct intel_crtc *intel_cr static void intel_crtc_init(struct drm_device *dev, int pipe) { struct drm_i915_private *dev_priv = dev->dev_private; - struct intel_crtc *intel_crtc; + struct intel_crtc *intel_crtc = NULL; struct intel_crtc_state *crtc_state = NULL; struct drm_plane *primary = NULL; struct drm_plane *cursor = NULL; + char *name = NULL; int i, ret; intel_crtc = kzalloc(sizeof(*intel_crtc), GFP_KERNEL); - if (intel_crtc == NULL) - return; + if (!intel_crtc) + goto fail; crtc_state = kzalloc(sizeof(*crtc_state), GFP_KERNEL); if (!crtc_state) @@ -14043,6 +14050,11 @@ static void intel_crtc_init(struct drm_device *dev, int pipe) intel_crtc->base.state = _state->base; crtc_state->base.crtc = _crtc->base; + name = kasprintf(GFP_KERNEL, "pipe %c", pipe_name(pipe)); + if (!name) + goto fail; + intel_crtc->base.name = name; + /* initialize shared scalers */ if (INTEL_INFO(dev)->gen >= 9) { if (pipe == PIPE_C) @@ -14105,6 +14117,7 @@ fail: drm_plane_cleanup(primary); if (cursor) drm_plane_cleanup(cursor); + kfree(name); kfree(crtc_state); kfree(intel_crtc); } -- 2.4.10
[PATCH 4/8] drm/i915: Use plane->name in debug prints
From: Ville SyrjäläSigned-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_display.c | 38 +--- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 9845687..b628dab 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4433,9 +4433,9 @@ static int skl_update_scaler_plane(struct intel_crtc_state *crtc_state, bool force_detach = !fb || !plane_state->visible; - DRM_DEBUG_KMS("Updating scaler for [PLANE:%d] scaler_user index %u.%u\n", - intel_plane->base.base.id, intel_crtc->pipe, - drm_plane_index(_plane->base)); + DRM_DEBUG_KMS("Updating scaler for [PLANE:%d:%s] scaler_user index %u.%u\n", + intel_plane->base.base.id, intel_plane->base.name, + intel_crtc->pipe, drm_plane_index(_plane->base)); ret = skl_update_scaler(crtc_state, force_detach, drm_plane_index(_plane->base), @@ -4451,8 +4451,9 @@ static int skl_update_scaler_plane(struct intel_crtc_state *crtc_state, /* check colorkey */ if (plane_state->ckey.flags != I915_SET_COLORKEY_NONE) { - DRM_DEBUG_KMS("[PLANE:%d] scaling with color key not allowed", - intel_plane->base.base.id); + DRM_DEBUG_KMS("[PLANE:%d:%s] scaling with color key not allowed", + intel_plane->base.base.id, + intel_plane->base.name); return -EINVAL; } @@ -4471,8 +4472,9 @@ static int skl_update_scaler_plane(struct intel_crtc_state *crtc_state, case DRM_FORMAT_VYUY: break; default: - DRM_DEBUG_KMS("[PLANE:%d] FB:%d unsupported scaling format 0x%x\n", - intel_plane->base.base.id, fb->base.id, fb->pixel_format); + DRM_DEBUG_KMS("[PLANE:%d:%s] FB:%d unsupported scaling format 0x%x\n", + intel_plane->base.base.id, intel_plane->base.name, + fb->base.id, fb->pixel_format); return -EINVAL; } @@ -11679,13 +11681,15 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state, turn_off = was_visible && (!visible || mode_changed); turn_on = visible && (!was_visible || mode_changed); - DRM_DEBUG_ATOMIC("[CRTC:%d:%s] has [PLANE:%i] with fb %i\n", + DRM_DEBUG_ATOMIC("[CRTC:%d:%s] has [PLANE:%d:%s] with fb %i\n", intel_crtc->base.base.id, intel_crtc->base.name, -plane->base.id, fb ? fb->base.id : -1); +plane->base.id, plane->name, +fb ? fb->base.id : -1); - DRM_DEBUG_ATOMIC("[PLANE:%i] visible %i -> %i, off %i, on %i, ms %i\n", -plane->base.id, was_visible, visible, + DRM_DEBUG_ATOMIC("[PLANE:%d:%s] visible %i -> %i, off %i, on %i, ms %i\n", +plane->base.id, plane->name, +was_visible, visible, turn_off, turn_on, mode_changed); if (turn_on) { @@ -12088,18 +12092,20 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc, state = to_intel_plane_state(plane->state); fb = state->base.fb; if (!fb) { - DRM_DEBUG_KMS("%s PLANE:%d plane: %u.%u idx: %d " - "disabled, scaler_id = %d\n", + DRM_DEBUG_KMS("%s [PLANE:%d:%s] plane: %u.%u idx: %d " + "disabled, scaler_id = %d\n", plane->type == DRM_PLANE_TYPE_CURSOR ? "CURSOR" : "STANDARD", - plane->base.id, intel_plane->pipe, + plane->base.id, plane->name, + intel_plane->pipe, (crtc->base.primary == plane) ? 0 : intel_plane->plane + 1, drm_plane_index(plane), state->scaler_id); continue; } - DRM_DEBUG_KMS("%s PLANE:%d plane: %u.%u idx: %d enabled", + DRM_DEBUG_KMS("%s [PLANE:%d:%s] plane: %u.%u idx: %d enabled", plane->type == DRM_PLANE_TYPE_CURSOR ? "CURSOR" : "STANDARD", - plane->base.id, intel_plane->pipe, + plane->base.id, plane->name, + intel_plane->pipe, crtc->base.primary == plane ? 0 : intel_plane->plane + 1, drm_plane_index(plane)); DRM_DEBUG_KMS("\tFB:%d, fb = %ux%u format = 0x%x", -- 2.4.10
[PATCH 3/8] drm/i915: Use crtc->name in debug messages
From: Ville SyrjäläSigned-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_display.c | 55 drivers/gpu/drm/i915/intel_fbdev.c | 5 ++-- 2 files changed, 33 insertions(+), 27 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index b5f7493..9845687 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4220,8 +4220,9 @@ struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc, i = (enum intel_dpll_id) crtc->pipe; pll = _priv->shared_dplls[i]; - DRM_DEBUG_KMS("CRTC:%d using pre-allocated %s\n", - crtc->base.base.id, pll->name); + DRM_DEBUG_KMS("[CRTC:%d:%s] using pre-allocated %s\n", + crtc->base.base.id, crtc->base.name, + pll->name); WARN_ON(shared_dpll[i].crtc_mask); @@ -4241,8 +4242,9 @@ struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc, /* 1:1 mapping between ports and PLLs */ i = (enum intel_dpll_id)intel_dig_port->port; pll = _priv->shared_dplls[i]; - DRM_DEBUG_KMS("CRTC:%d using pre-allocated %s\n", - crtc->base.base.id, pll->name); + DRM_DEBUG_KMS("[CRTC:%d:%s] using pre-allocated %s\n", + crtc->base.base.id, crtc->base.name, + pll->name); WARN_ON(shared_dpll[i].crtc_mask); goto found; @@ -4258,9 +4260,9 @@ struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc, if (memcmp(_state->dpll_hw_state, _dpll[i].hw_state, sizeof(crtc_state->dpll_hw_state)) == 0) { - DRM_DEBUG_KMS("CRTC:%d sharing existing %s (crtc mask 0x%08x, ative %d)\n", - crtc->base.base.id, pll->name, - shared_dpll[i].crtc_mask, + DRM_DEBUG_KMS("[CRTC:%d:%s] sharing existing %s (crtc mask 0x%08x, ative %d)\n", + crtc->base.base.id, crtc->base.name, + pll->name, shared_dpll[i].crtc_mask, pll->active); goto found; } @@ -4270,8 +4272,9 @@ struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc, for (i = 0; i < dev_priv->num_shared_dpll; i++) { pll = _priv->shared_dplls[i]; if (shared_dpll[i].crtc_mask == 0) { - DRM_DEBUG_KMS("CRTC:%d allocated %s\n", - crtc->base.base.id, pll->name); + DRM_DEBUG_KMS("[CRTC:%d:%s] allocated %s\n", + crtc->base.base.id, crtc->base.name, + pll->name); goto found; } } @@ -4398,8 +4401,9 @@ int skl_update_scaler_crtc(struct intel_crtc_state *state) struct intel_crtc *intel_crtc = to_intel_crtc(state->base.crtc); const struct drm_display_mode *adjusted_mode = >base.adjusted_mode; - DRM_DEBUG_KMS("Updating scaler for [CRTC:%i] scaler_user index %u.%u\n", - intel_crtc->base.base.id, intel_crtc->pipe, SKL_CRTC_INDEX); + DRM_DEBUG_KMS("Updating scaler for [CRTC:%d:%s] scaler_user index %u.%u\n", + intel_crtc->base.base.id, intel_crtc->base.name, + intel_crtc->pipe, SKL_CRTC_INDEX); return skl_update_scaler(state, !state->base.active, SKL_CRTC_INDEX, >scaler_state.scaler_id, DRM_ROTATE_0, @@ -11643,13 +11647,13 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state, struct drm_i915_private *dev_priv = dev->dev_private; struct intel_plane_state *old_plane_state = to_intel_plane_state(plane->state); - int idx = intel_crtc->base.base.id, ret; int i = drm_plane_index(plane); bool mode_changed = needs_modeset(crtc_state); bool was_crtc_enabled = crtc->state->active; bool is_crtc_enabled = crtc_state->active; bool turn_off, turn_on, visible, was_visible; struct drm_framebuffer *fb = plane_state->fb; + int ret; if (crtc_state && INTEL_INFO(dev)->gen >= 9 && plane->type != DRM_PLANE_TYPE_CURSOR) { @@ -11675,7 +11679,9 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state, turn_off = was_visible && (!visible || mode_changed); turn_on = visible && (!was_visible || mode_changed); - DRM_DEBUG_ATOMIC("[CRTC:%i] has [PLANE:%i] with fb %i\n", idx, + DRM_DEBUG_ATOMIC("[CRTC:%d:%s]
[PATCH 2/8] drm: Add plane->name and use it in debug prints
From: Ville SyrjäläSigned-off-by: Ville Syrjälä --- drivers/gpu/drm/drm_atomic.c| 12 ++-- drivers/gpu/drm/drm_atomic_helper.c | 4 ++-- drivers/gpu/drm/drm_crtc.c | 3 +++ include/drm/drm_crtc.h | 2 ++ 4 files changed, 13 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 2944655..df84060 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -543,8 +543,8 @@ drm_atomic_get_plane_state(struct drm_atomic_state *state, state->planes[index] = plane; plane_state->state = state; - DRM_DEBUG_ATOMIC("Added [PLANE:%d] %p state to %p\n", -plane->base.id, plane_state, state); + DRM_DEBUG_ATOMIC("Added [PLANE:%d:%s] %p state to %p\n", +plane->base.id, plane->name, plane_state, state); if (plane_state->crtc) { struct drm_crtc_state *crtc_state; @@ -755,8 +755,8 @@ static int drm_atomic_plane_check(struct drm_plane *plane, } if (plane_switching_crtc(state->state, plane, state)) { - DRM_DEBUG_ATOMIC("[PLANE:%d] switching CRTC directly\n", -plane->base.id); + DRM_DEBUG_ATOMIC("[PLANE:%d:%s] switching CRTC directly\n", +plane->base.id, plane->name); return -EINVAL; } @@ -1229,8 +1229,8 @@ int drm_atomic_check_only(struct drm_atomic_state *state) for_each_plane_in_state(state, plane, plane_state, i) { ret = drm_atomic_plane_check(plane, plane_state); if (ret) { - DRM_DEBUG_ATOMIC("[PLANE:%d] atomic core check failed\n", -plane->base.id); + DRM_DEBUG_ATOMIC("[PLANE:%d:%s] atomic core check failed\n", +plane->base.id, plane->name); return ret; } } diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index fc90af50..387d95c 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -503,8 +503,8 @@ drm_atomic_helper_check_planes(struct drm_device *dev, ret = funcs->atomic_check(plane, plane_state); if (ret) { - DRM_DEBUG_ATOMIC("[PLANE:%d] atomic driver check failed\n", -plane->base.id); + DRM_DEBUG_ATOMIC("[PLANE:%d:%s] atomic driver check failed\n", +plane->base.id, plane->name); return ret; } } diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index ea00a69..8dc4052 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -1174,6 +1174,9 @@ int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane, drm_modeset_lock_init(>mutex); + if (!plane->name) + plane->name = ""; + plane->base.properties = >properties; plane->dev = dev; plane->funcs = funcs; diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index a8279b4..a08e256 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -848,6 +848,8 @@ struct drm_plane { struct drm_device *dev; struct list_head head; + char *name; + struct drm_modeset_lock mutex; struct drm_mode_object base; -- 2.4.10
[PATCH 1/8] drm: Add crtc->name and use it in debug messages
From: Ville SyrjäläSigned-off-by: Ville Syrjälä --- drivers/gpu/drm/drm_atomic.c| 41 ++- drivers/gpu/drm/drm_atomic_helper.c | 56 +++-- drivers/gpu/drm/drm_crtc.c | 8 -- drivers/gpu/drm/drm_crtc_helper.c | 24 include/drm/drm_crtc.h | 2 ++ 5 files changed, 71 insertions(+), 60 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 7bb3845..2944655 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -288,8 +288,8 @@ drm_atomic_get_crtc_state(struct drm_atomic_state *state, state->crtcs[index] = crtc; crtc_state->state = state; - DRM_DEBUG_ATOMIC("Added [CRTC:%d] %p state to %p\n", -crtc->base.id, crtc_state, state); + DRM_DEBUG_ATOMIC("Added [CRTC:%d:%s] %p state to %p\n", +crtc->base.id, crtc->name, crtc_state, state); return crtc_state; } @@ -480,8 +480,8 @@ static int drm_atomic_crtc_check(struct drm_crtc *crtc, */ if (state->active && !state->enable) { - DRM_DEBUG_ATOMIC("[CRTC:%d] active without enabled\n", -crtc->base.id); + DRM_DEBUG_ATOMIC("[CRTC:%d:%s] active without enabled\n", +crtc->base.id, crtc->name); return -EINVAL; } @@ -490,15 +490,15 @@ static int drm_atomic_crtc_check(struct drm_crtc *crtc, * 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); + DRM_DEBUG_ATOMIC("[CRTC:%d:%s] enabled without mode blob\n", +crtc->base.id, crtc->name); 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); + DRM_DEBUG_ATOMIC("[CRTC:%d:%s] disabled with mode blob\n", +crtc->base.id, crtc->name); return -EINVAL; } @@ -980,8 +980,8 @@ drm_atomic_set_crtc_for_plane(struct drm_plane_state *plane_state, } if (crtc) - DRM_DEBUG_ATOMIC("Link plane state %p to [CRTC:%d]\n", -plane_state, crtc->base.id); + DRM_DEBUG_ATOMIC("Link plane state %p to [CRTC:%d:%s]\n", +plane_state, crtc->base.id, crtc->name); else DRM_DEBUG_ATOMIC("Link plane state %p to [NOCRTC]\n", plane_state); @@ -1048,8 +1048,8 @@ drm_atomic_set_crtc_for_connector(struct drm_connector_state *conn_state, conn_state->crtc = crtc; if (crtc) - DRM_DEBUG_ATOMIC("Link connector state %p to [CRTC:%d]\n", -conn_state, crtc->base.id); + DRM_DEBUG_ATOMIC("Link connector state %p to [CRTC:%d:%s]\n", +conn_state, crtc->base.id, crtc->name); else DRM_DEBUG_ATOMIC("Link connector state %p to [NOCRTC]\n", conn_state); @@ -1088,8 +1088,8 @@ drm_atomic_add_affected_connectors(struct drm_atomic_state *state, if (ret) return ret; - DRM_DEBUG_ATOMIC("Adding all current connectors for [CRTC:%d] to %p\n", -crtc->base.id, state); + DRM_DEBUG_ATOMIC("Adding all current connectors for [CRTC:%d:%s] to %p\n", +crtc->base.id, crtc->name, state); /* * Changed connectors are already in @state, so only need to look at the @@ -1169,8 +1169,9 @@ drm_atomic_connectors_for_crtc(struct drm_atomic_state *state, num_connected_connectors++; } - DRM_DEBUG_ATOMIC("State %p has %i connectors for [CRTC:%d]\n", -state, num_connected_connectors, crtc->base.id); + DRM_DEBUG_ATOMIC("State %p has %i connectors for [CRTC:%d:%s]\n", +state, num_connected_connectors, +crtc->base.id, crtc->name); return num_connected_connectors; } @@ -1237,8 +1238,8 @@ int drm_atomic_check_only(struct drm_atomic_state *state) for_each_crtc_in_state(state, crtc, crtc_state, i) { ret = drm_atomic_crtc_check(crtc, crtc_state); if (ret) { - DRM_DEBUG_ATOMIC("[CRTC:%d] atomic core check failed\n", -crtc->base.id); +
[PATCH v2 0/8] drm: Give crtcs and planes actual names (v2)
From: Ville SyrjäläOK, so another attempt. This time the error handling and cleanup should be more solid. Previus attempt was at http://lists.freedesktop.org/archives/dri-devel/2015-November/094331.html I pushed v2 to: git://github.com/vsyrjala/linux.git crtc_plane_name_2 Ville Syrjälä (8): drm: Add crtc->name and use it in debug messages drm: Add plane->name and use it in debug prints drm/i915: Use crtc->name in debug messages drm/i915: Use plane->name in debug prints drm/i915: Set crtc->name to "pipe A", "pipe B", etc. drm/i915: Fix plane init failure paths drm/i915: Don't leak primary/cursor planes on crtc init failure drm/i915: Give meaningful names to all the planes drivers/gpu/drm/drm_atomic.c | 53 - drivers/gpu/drm/drm_atomic_helper.c | 60 +- drivers/gpu/drm/drm_crtc.c | 11 +- drivers/gpu/drm/drm_crtc_helper.c| 24 ++-- drivers/gpu/drm/i915/intel_display.c | 218 +++ drivers/gpu/drm/i915/intel_fbdev.c | 5 +- drivers/gpu/drm/i915/intel_sprite.c | 48 +--- include/drm/drm_crtc.h | 4 + 8 files changed, 265 insertions(+), 158 deletions(-) -- 2.4.10
[PATCH v2 1/4] drm: arm: Add DT bindings documentation for HDLCD driver.
On Thu, Nov 12, 2015 at 4:42 AM, Liviu Dudau wrote: > On Wed, Nov 11, 2015 at 12:48:50PM -0600, Rob Herring wrote: >> On Wed, Nov 11, 2015 at 04:06:47PM +, Liviu Dudau wrote: >> > Cc: Rob Herring>> > Cc: Pawel Moll >> > Cc: Mark Rutland >> > Cc: Ian Campbell >> > Cc: Kumar Gala >> > >> > Signed-off-by: Liviu Dudau >> >> Looks pretty good, but a few comments. >> >> > --- >> > .../devicetree/bindings/drm/arm/arm,hdlcd.txt | 74 >> > ++ >> > 1 file changed, 74 insertions(+) >> > create mode 100644 Documentation/devicetree/bindings/drm/arm/arm,hdlcd.txt >> > >> > diff --git a/Documentation/devicetree/bindings/drm/arm/arm,hdlcd.txt >> > b/Documentation/devicetree/bindings/drm/arm/arm,hdlcd.txt >> > new file mode 100644 >> > index 000..b57f1b9 >> > --- /dev/null >> > +++ b/Documentation/devicetree/bindings/drm/arm/arm,hdlcd.txt >> > @@ -0,0 +1,74 @@ >> > +ARM HDLCD >> > + >> > +This is a display controller found on several development platforms >> > produced >> > +by ARM Ltd and in more modern of its' Fast Models. The HDLCD is an RGB >> > +streamer that reads the data from a framebuffer and sends it to a single >> > +digital encoder (DVI or HDMI). >> > + >> > +Required properties: >> > + - compatible: "arm,hdlcd" >> >> Kind of generic. Something more specific please. > > "There can be only one!" (hdlcd) :) This is going to be a "one version only" > HW part. > ARM has now switched to a new display hardware that has more features and a > new name, > and work on mainlining support for that will start once I get the HDLCD > driver accepted. So there is never going to be a single difference across platforms. Variations in max clock for different FPGAs? >> > + - reg: Physical base address and length of the controller's registers. >> > +If a second pair of address and length values is present this >> > specifies >> > +the presence of a DMA coherent memory area that the HDLCD can use as >> > +framebuffer instead of normal CMA memory. >> >> This is on-chip RAM or nornal system RAM? We already have bindings for >> both. > > Juno has a set of TLX (ThinLinks) connectors on the board where an FPGA can > be attached. On r1 > the code running on FPGA can even participate as an AXI master with full > coherency. The FPGA > has local memory that we want to share with the HDLCD to be used as a > framebuffer. So describe the memory region and then use a memory-region phandle to the memory here. >> > + - interrupts: One interrupt used by the display controller to notify the >> > +interrupt controller when any of the interrupt sources programmed in >> > +the interrupt mask register have activated. >> > + - clocks: A list of phandle + clock-specifier pairs, one for each >> > +entry in 'clock-names'. >> > + - clock-names: A list of clock names. For HDLD it should contain: typo: HDLD >> > + - "pxlclk" for the clock feeding the output PLL of the controller. >> > + - port: The HDLCD connection to an encoder chip. The connection is >> > modelled s/modelled/modeled/ Rob
[Intel-gfx] [PATCH 6/6] drm/i915: Give meaningful names to all the planes
On Thu, Nov 12, 2015 at 07:49:19PM +0200, Ville Syrjälä wrote: > On Thu, Nov 12, 2015 at 05:38:48PM +, Emil Velikov wrote: > > Hi Ville, > > > > On 12 November 2015 at 16:52, wrote: > > > From: Ville Syrjälä > > > > > > Let's name our planes in a way that makes sense wrt. the spec: > > > - skl+ -> "plane 1A", "plane 2A", "plane 1C", "cursor A" etc. > > > - g4x+ -> "primary A", "primary B", "sprite A", "cursor C" etc. > > > - pre-g4x -> "plane A", "cursor B" etc. > > > > > > Signed-off-by: Ville Syrjälä > > > --- > > > drivers/gpu/drm/i915/intel_display.c | 35 > > > +-- > > > drivers/gpu/drm/i915/intel_sprite.c | 14 ++ > > > 2 files changed, 47 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c > > > b/drivers/gpu/drm/i915/intel_display.c > > > index 2b5e81a..82b2f58 100644 > > > --- a/drivers/gpu/drm/i915/intel_display.c > > > +++ b/drivers/gpu/drm/i915/intel_display.c > > > @@ -13788,7 +13788,15 @@ static void intel_finish_crtc_commit(struct > > > drm_crtc *crtc, > > > void intel_plane_destroy(struct drm_plane *plane) > > > { > > > struct intel_plane *intel_plane = to_intel_plane(plane); > > > + char *name; > > > + > > > + /* > > > +* drm_plane_cleanup() zeroes the structure, so > > > +* need an extra dance to avoid leaking the name. > > > +*/ > > > + name = plane->name; > > > drm_plane_cleanup(plane); > > > + kfree(name); > > > kfree(intel_plane); > > > } > > > > > > @@ -13838,6 +13846,21 @@ static struct drm_plane > > > *intel_primary_plane_create(struct drm_device *dev, > > > if (HAS_FBC(dev) && INTEL_INFO(dev)->gen < 4) > > > primary->plane = !pipe; > > > > > > + if (INTEL_INFO(dev)->gen >= 9) > > > + primary->base.name = kasprintf(GFP_KERNEL, "plane 1%c", > > > + pipe_name(pipe)); > > > + else if (INTEL_INFO(dev)->gen >= 5 || IS_G4X(dev)) > > > + primary->base.name = kasprintf(GFP_KERNEL, "primary %c", > > > + pipe_name(pipe)); > > > + else > > > + primary->base.name = kasprintf(GFP_KERNEL, "plane %c", > > > + > > > plane_name(primary->plane)); > > > + if (!primary->base.name) { > > > + kfree(state); > > > + kfree(primary); > > > + return NULL; > > Worth adding a label and doing all the teardown there ? (same goes for > > the rest of the patch) > > Dunno. Was feeling lazy, and so didn't go the extra mile. After a better look I saw that I fumbled the error paths in the crtc name patch too. So I went on to clean things up a bit. I think I'll repost the lot since there are now more patches. > > > > > > + } > > > + > > > if (INTEL_INFO(dev)->gen >= 9) { > > > intel_primary_formats = skl_primary_formats; > > > num_formats = ARRAY_SIZE(skl_primary_formats); > > > @@ -13987,6 +14010,14 @@ static struct drm_plane > > > *intel_cursor_plane_create(struct drm_device *dev, > > > cursor->commit_plane = intel_commit_cursor_plane; > > > cursor->disable_plane = intel_disable_cursor_plane; > > > > > > + cursor->base.name = kasprintf(GFP_KERNEL, "cursor %c", > > > + pipe_name(pipe)); > > > + if (!cursor->base.name) { > > > + kfree(state); > > > + kfree(cursor); > > > + return NULL; > > > + } > > > + > > > drm_universal_plane_init(dev, >base, 0, > > > _plane_funcs, > > > intel_cursor_formats, > > > @@ -14113,9 +14144,9 @@ static void intel_crtc_init(struct drm_device > > > *dev, int pipe) > > > > > > fail: > > > if (primary) > > > - drm_plane_cleanup(primary); > > > + intel_plane_destroy(primary); > > > if (cursor) > > > - drm_plane_cleanup(cursor); > > > + intel_plane_destroy(cursor); > > Something feels strange here. We are either leaking memory before or > > we'll end up with double free after your patch. Worth > > checking/mentioning in the commit message ? > > Yeah, I think we were leaking here. Forgot to add a note. > > -- > Ville Syrjälä > Intel OTC > ___ > Intel-gfx mailing list > Intel-gfx at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC
[Nouveau] [PATCH] drm/nouveau: Fix pre-nv50 pageflip events (v3) -> v4
On 12.11.2015 14:48, Thierry Reding wrote: > On Wed, Nov 11, 2015 at 09:12:33PM +0100, poma wrote: >> On 10.11.2015 17:25, Mario Kleiner wrote: >>> On 11/10/2015 05:00 PM, Thierry Reding wrote: On Tue, Nov 10, 2015 at 03:54:52PM +0100, Mario Kleiner wrote: > From: Daniel Vetter > > Apparently pre-nv50 pageflip events happen before the actual vblank > period. Therefore that functionality got semi-disabled in > > commit af4870e406126b7ac0ae7c7ce5751f25ebe60f28 > Author: Mario Kleiner > Date: Tue May 13 00:42:08 2014 +0200 > > drm/nouveau/kms/nv04-nv40: fix pageflip events via special case. > > Unfortunately that hack got uprooted in > > commit cc1ef118fc099295ae6aabbacc8af94d8d8885eb > Author: Thierry Reding > Date: Wed Aug 12 17:00:31 2015 +0200 > > drm/irq: Make pipe unsigned and name consistent > > Trigering a warning when trying to sample the vblank timestamp for a > non-existing pipe. There's a few ways to fix this: > > - Open-code the old behaviour, which just enshrines this slight >breakage of the userspace ABI. > > - Revert Mario's commit and again inflict broken timestamps, again not >pretty. > > - Fix this for real by delaying the pageflip TS until the next vblank >interrupt, thereby making it accurate. > > This patch implements the third option. Since having a page flip > interrupt that happens when the pageflip gets armed and not when it > completes in the next vblank seems to be fairly common (older i915 hw > works very similarly) create a new helper to arm vblank events for > such drivers. > > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=106431 > Cc: Thierry Reding > Cc: Mario Kleiner > Cc: Ben Skeggs > Cc: Ilia Mirkin > > v2 (mario): Integrate my own review comments into Daniels patch. > - Fix function prototypes in drmP.h > - Add missing vblank_put() for pageflip completion without > pageflip event. > - Initialize sequence number for queued pageflip event to avoidng > trouble in drm_handle_vblank_events(). > - Remove dead code and spelling fix. > > v3 (mario): Add a signed-off-by and cc stable tag per Ilja's advice. > > Signed-off-by: Daniel Vetter > (v1) Reviewed-by: Mario Kleiner > (v2/v3) Signed-off-by: Mario Kleiner > > Cc: stable at vger.kernel.org # v4.3 > --- > drivers/gpu/drm/drm_irq.c | 54 > ++- > drivers/gpu/drm/nouveau/nouveau_display.c | 19 ++- > include/drm/drmP.h| 4 +++ > 3 files changed, 68 insertions(+), 9 deletions(-) This looks good to me. Let me clean this up a little and submit it to Dave. Thierry >>> >>> Btw., if somebody has a functional old card for testing this, it should >>> be easy to verify if it works on pre-nv50. If it would not work it would >>> deliver the pageflip event 1 frame delayed, so at least on standard >>> nouveau + default DRI2 + default double-buffering the rate for a tight >>> loop of page-flipped swaps should go down to 30 fps on a 60 Hz display, >>> quite noticeable. Afaik we also have Piglit tests for OML_sync_control >>> which would likely fail if this would be broken. >>> >>> Oh and if someone has tips on how to resurrect an old nv-40 PC (booted >>> with BIOS only) graphics card in a MacPro (EFI boot), i wouldn't mind >>> hearing them. It would be nice to still be able to use that card for >>> testing. >>> >>> thanks, >>> -mario >> >> >> [ cut here ] >> WARNING: CPU: 0 PID: 313 at lib/dma-debug.c:1205 check_sync+0x169/0x6e0() >> nouveau :01:00.0: DMA-API: device driver tries to sync DMA memory it has >> not allocated [device address=0xc0bf6468] [size=4096 bytes] >> Modules linked in: nouveau(+) ... >> CPU: 0 PID: 313 Comm: systemd-udevd Not tainted 4.3.0-3.fc22.i686+debug #1 >> ... >> Call Trace: >> [] dump_stack+0x48/0x69 >> [] warn_slowpath_common+0x87/0xc0 >> [] ? check_sync+0x169/0x6e0 >> [] ? check_sync+0x169/0x6e0 >> [] warn_slowpath_fmt+0x3e/0x60 >> [] check_sync+0x169/0x6e0 >> [] debug_dma_sync_single_for_device+0x7d/0x90 >> [] ? ttm_bo_del_sub_from_lru+0x18/0x50 [ttm] >> [] ? text_poke_bp+0xd0/0xd0 >> [] nouveau_bo_sync_for_device+0x8b/0xa0 [nouveau] >> [] nouveau_bo_validate+0x34/0x40 [nouveau] >> [] nouveau_bo_pin+0x188/0x290 [nouveau] >> [] ? nv10_bo_put_tile_region+0x80/0x80 [nouveau] >> [] nouveau_channel_prep+0xfd/0x2c0 [nouveau] >> [] nouveau_channel_new+0x57/0x7a0 [nouveau] >> [] ? kfree+0xdc/0x280 >> [] ? nvif_object_sclass_put+0x12/0x20 [nouveau] >> [] nouveau_drm_load+0x596/0x8d0 [nouveau] >> [] ? trace_hardirqs_on_caller+0x12c/0x1d0 >> [] ? drm_minor_register+0x89/0x120 [drm] >> [] drm_dev_register+0x96/0xa0 [drm] >>
[PATCH 6/6] drm/i915: Give meaningful names to all the planes
On Thu, Nov 12, 2015 at 05:38:48PM +, Emil Velikov wrote: > Hi Ville, > > On 12 November 2015 at 16:52, wrote: > > From: Ville Syrjälä > > > > Let's name our planes in a way that makes sense wrt. the spec: > > - skl+ -> "plane 1A", "plane 2A", "plane 1C", "cursor A" etc. > > - g4x+ -> "primary A", "primary B", "sprite A", "cursor C" etc. > > - pre-g4x -> "plane A", "cursor B" etc. > > > > Signed-off-by: Ville Syrjälä > > --- > > drivers/gpu/drm/i915/intel_display.c | 35 > > +-- > > drivers/gpu/drm/i915/intel_sprite.c | 14 ++ > > 2 files changed, 47 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c > > b/drivers/gpu/drm/i915/intel_display.c > > index 2b5e81a..82b2f58 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -13788,7 +13788,15 @@ static void intel_finish_crtc_commit(struct > > drm_crtc *crtc, > > void intel_plane_destroy(struct drm_plane *plane) > > { > > struct intel_plane *intel_plane = to_intel_plane(plane); > > + char *name; > > + > > + /* > > +* drm_plane_cleanup() zeroes the structure, so > > +* need an extra dance to avoid leaking the name. > > +*/ > > + name = plane->name; > > drm_plane_cleanup(plane); > > + kfree(name); > > kfree(intel_plane); > > } > > > > @@ -13838,6 +13846,21 @@ static struct drm_plane > > *intel_primary_plane_create(struct drm_device *dev, > > if (HAS_FBC(dev) && INTEL_INFO(dev)->gen < 4) > > primary->plane = !pipe; > > > > + if (INTEL_INFO(dev)->gen >= 9) > > + primary->base.name = kasprintf(GFP_KERNEL, "plane 1%c", > > + pipe_name(pipe)); > > + else if (INTEL_INFO(dev)->gen >= 5 || IS_G4X(dev)) > > + primary->base.name = kasprintf(GFP_KERNEL, "primary %c", > > + pipe_name(pipe)); > > + else > > + primary->base.name = kasprintf(GFP_KERNEL, "plane %c", > > + plane_name(primary->plane)); > > + if (!primary->base.name) { > > + kfree(state); > > + kfree(primary); > > + return NULL; > Worth adding a label and doing all the teardown there ? (same goes for > the rest of the patch) Dunno. Was feeling lazy, and so didn't go the extra mile. > > > + } > > + > > if (INTEL_INFO(dev)->gen >= 9) { > > intel_primary_formats = skl_primary_formats; > > num_formats = ARRAY_SIZE(skl_primary_formats); > > @@ -13987,6 +14010,14 @@ static struct drm_plane > > *intel_cursor_plane_create(struct drm_device *dev, > > cursor->commit_plane = intel_commit_cursor_plane; > > cursor->disable_plane = intel_disable_cursor_plane; > > > > + cursor->base.name = kasprintf(GFP_KERNEL, "cursor %c", > > + pipe_name(pipe)); > > + if (!cursor->base.name) { > > + kfree(state); > > + kfree(cursor); > > + return NULL; > > + } > > + > > drm_universal_plane_init(dev, >base, 0, > > _plane_funcs, > > intel_cursor_formats, > > @@ -14113,9 +14144,9 @@ static void intel_crtc_init(struct drm_device *dev, > > int pipe) > > > > fail: > > if (primary) > > - drm_plane_cleanup(primary); > > + intel_plane_destroy(primary); > > if (cursor) > > - drm_plane_cleanup(cursor); > > + intel_plane_destroy(cursor); > Something feels strange here. We are either leaking memory before or > we'll end up with double free after your patch. Worth > checking/mentioning in the commit message ? Yeah, I think we were leaking here. Forgot to add a note. -- Ville Syrjälä Intel OTC
[PATCH 0/9] uapi: drm: fixes for userspace compilation
Hi Gabriel, Interestingly enough we had a person (gaby) asking about this over at #dri-devel, and I forwarded him to the work of Mikko Rapeli. You don't happen to be that person do you ? On 12 November 2015 at 18:14, Gabriel Laskar wrote: > Public headers should use types from include/uapi/linux/types.h. > Please don't do this. As mentioned to Mikko, these headers are meant to be used in more places than just Linux. All the compatibility is handled in drm.h so we might as well use it. That aside I'm a strong supporter of this type of work and I'm curious why Dave hasn't picked up the existing series ? Last time I've asked he did not have any objections. Dave, Did you see any issues with these that changed your mind or did it slip through the cracks ? Regards, Emil
[PATCH 9/9] include/uapi/drm/omap_drm.h: use __u{32, 64} types instead of uint{32, 64}_t
Kernel headers should use linux/types.h Signed-off-by: Gabriel Laskar --- include/uapi/drm/omap_drm.h | 39 --- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/include/uapi/drm/omap_drm.h b/include/uapi/drm/omap_drm.h index 1d0b117..926e42a 100644 --- a/include/uapi/drm/omap_drm.h +++ b/include/uapi/drm/omap_drm.h @@ -21,6 +21,7 @@ #define __OMAP_DRM_H__ #include +#include /* Please note that modifications to all structs defined here are * subject to backwards-compatibility constraints. @@ -29,8 +30,8 @@ #define OMAP_PARAM_CHIPSET_ID 1 /* ie. 0x3430, 0x4430, etc */ struct drm_omap_param { - uint64_t param; /* in */ - uint64_t value; /* in (set_param), out (get_param) */ + __u64 param;/* in */ + __u64 value;/* in (set_param), out (get_param) */ }; #define OMAP_BO_SCANOUT0x0001 /* scanout capable (phys contiguous) */ @@ -49,18 +50,18 @@ struct drm_omap_param { #define OMAP_BO_TILED (OMAP_BO_TILED_8 | OMAP_BO_TILED_16 | OMAP_BO_TILED_32) union omap_gem_size { - uint32_t bytes; /* (for non-tiled formats) */ + __u32 bytes;/* (for non-tiled formats) */ struct { - uint16_t width; - uint16_t height; + __u16 width; + __u16 height; } tiled;/* (for tiled formats) */ }; struct drm_omap_gem_new { union omap_gem_size size; /* in */ - uint32_t flags; /* in */ - uint32_t handle;/* out */ - uint32_t __pad; + __u32 flags;/* in */ + __u32 handle; /* out */ + __u32 __pad; }; /* mask of operations: */ @@ -70,33 +71,33 @@ enum omap_gem_op { }; struct drm_omap_gem_cpu_prep { - uint32_t handle;/* buffer handle (in) */ - uint32_t op;/* mask of omap_gem_op (in) */ + __u32 handle; /* buffer handle (in) */ + __u32 op; /* mask of omap_gem_op (in) */ }; struct drm_omap_gem_cpu_fini { - uint32_t handle;/* buffer handle (in) */ - uint32_t op;/* mask of omap_gem_op (in) */ + __u32 handle; /* buffer handle (in) */ + __u32 op; /* mask of omap_gem_op (in) */ /* TODO maybe here we pass down info about what regions are touched * by sw so we can be clever about cache ops? For now a placeholder, * set to zero and we just do full buffer flush.. */ - uint32_t nregions; - uint32_t __pad; + __u32 nregions; + __u32 __pad; }; struct drm_omap_gem_info { - uint32_t handle;/* buffer handle (in) */ - uint32_t pad; - uint64_t offset;/* mmap offset (out) */ + __u32 handle; /* buffer handle (in) */ + __u32 pad; + __u64 offset; /* mmap offset (out) */ /* note: in case of tiled buffers, the user virtual size can be * different from the physical size (ie. how many pages are needed * to back the object) which is returned in DRM_IOCTL_GEM_OPEN.. * This size here is the one that should be used if you want to * mmap() the buffer: */ - uint32_t size; /* virtual size for mmap'ing (out) */ - uint32_t __pad; + __u32 size; /* virtual size for mmap'ing (out) */ + __u32 __pad; }; #define DRM_OMAP_GET_PARAM 0x00 -- 2.6.2
[PATCH 8/9] include/uapi/drm/vmwgfx_drm.h: use __u{32, 64} types instead of uint{32, 64}_t
Kernel headers should use linux/types.h Signed-off-by: Gabriel Laskar --- include/uapi/drm/vmwgfx_drm.h | 264 +- 1 file changed, 132 insertions(+), 132 deletions(-) diff --git a/include/uapi/drm/vmwgfx_drm.h b/include/uapi/drm/vmwgfx_drm.h index 05b2049..ef31f44 100644 --- a/include/uapi/drm/vmwgfx_drm.h +++ b/include/uapi/drm/vmwgfx_drm.h @@ -111,9 +111,9 @@ enum drm_vmw_handle_type { */ struct drm_vmw_getparam_arg { - uint64_t value; - uint32_t param; - uint32_t pad64; + __u64 value; + __u32 param; + __u32 pad64; }; /*/ @@ -134,8 +134,8 @@ struct drm_vmw_getparam_arg { */ struct drm_vmw_context_arg { - int32_t cid; - uint32_t pad64; + __s32 cid; + __u32 pad64; }; /*/ @@ -165,7 +165,7 @@ struct drm_vmw_context_arg { * @mip_levels: Number of mip levels for each face. * An unused face should have 0 encoded. * @size_addr: Address of a user-space array of sruct drm_vmw_size - * cast to an uint64_t for 32-64 bit compatibility. + * cast to an __u64 for 32-64 bit compatibility. * The size of the array should equal the total number of mipmap levels. * @shareable: Boolean whether other clients (as identified by file descriptors) * may reference this surface. @@ -177,12 +177,12 @@ struct drm_vmw_context_arg { */ struct drm_vmw_surface_create_req { - uint32_t flags; - uint32_t format; - uint32_t mip_levels[DRM_VMW_MAX_SURFACE_FACES]; - uint64_t size_addr; - int32_t shareable; - int32_t scanout; + __u32 flags; + __u32 format; + __u32 mip_levels[DRM_VMW_MAX_SURFACE_FACES]; + __u64 size_addr; + __s32 shareable; + __s32 scanout; }; /** @@ -197,7 +197,7 @@ struct drm_vmw_surface_create_req { */ struct drm_vmw_surface_arg { - int32_t sid; + __s32 sid; enum drm_vmw_handle_type handle_type; }; @@ -213,10 +213,10 @@ struct drm_vmw_surface_arg { */ struct drm_vmw_size { - uint32_t width; - uint32_t height; - uint32_t depth; - uint32_t pad64; + __u32 width; + __u32 height; + __u32 depth; + __u32 pad64; }; /** @@ -284,13 +284,13 @@ union drm_vmw_surface_reference_arg { /** * struct drm_vmw_execbuf_arg * - * @commands: User-space address of a command buffer cast to an uint64_t. + * @commands: User-space address of a command buffer cast to an __u64. * @command-size: Size in bytes of the command buffer. * @throttle-us: Sleep until software is less than @throttle_us * microseconds ahead of hardware. The driver may round this value * to the nearest kernel tick. * @fence_rep: User-space address of a struct drm_vmw_fence_rep cast to an - * uint64_t. + * __u64. * @version: Allows expanding the execbuf ioctl parameters without breaking * backwards compatibility, since user-space will always tell the kernel * which version it uses. @@ -302,14 +302,14 @@ union drm_vmw_surface_reference_arg { #define DRM_VMW_EXECBUF_VERSION 2 struct drm_vmw_execbuf_arg { - uint64_t commands; - uint32_t command_size; - uint32_t throttle_us; - uint64_t fence_rep; - uint32_t version; - uint32_t flags; - uint32_t context_handle; - uint32_t pad64; + __u64 commands; + __u32 command_size; + __u32 throttle_us; + __u64 fence_rep; + __u32 version; + __u32 flags; + __u32 context_handle; + __u32 pad64; }; /** @@ -338,12 +338,12 @@ struct drm_vmw_execbuf_arg { */ struct drm_vmw_fence_rep { - uint32_t handle; - uint32_t mask; - uint32_t seqno; - uint32_t passed_seqno; - uint32_t pad64; - int32_t error; + __u32 handle; + __u32 mask; + __u32 seqno; + __u32 passed_seqno; + __u32 pad64; + __s32 error; }; /*/ @@ -373,8 +373,8 @@ struct drm_vmw_fence_rep { */ struct drm_vmw_alloc_dmabuf_req { - uint32_t size; - uint32_t pad64; + __u32 size; + __u32 pad64; }; /** @@ -391,11 +391,11 @@ struct drm_vmw_alloc_dmabuf_req { */ struct drm_vmw_dmabuf_rep { - uint64_t map_handle; - uint32_t handle; - uint32_t cur_gmr_id; - uint32_t cur_gmr_offset; - uint32_t pad64; + __u64 map_handle; + __u32 handle; + __u32 cur_gmr_id; + __u32 cur_gmr_offset; + __u32 pad64; }; /** @@ -428,8 +428,8 @@ union drm_vmw_alloc_dmabuf_arg { */ struct drm_vmw_unref_dmabuf_arg { - uint32_t handle; - uint32_t pad64; + __u32 handle; + __u32 pad64; }; /*/ @@ -452,10 +452,10 @@ struct drm_vmw_unref_dmabuf_arg { */
[PATCH 7/9] include/uapi/drm/drm_mode.h: use __u{32, 64} types instead of uint{32, 64}_t
Kernel headers should use linux/types.h Signed-off-by: Gabriel Laskar --- include/uapi/drm/drm_mode.h | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index 6c11ca4..44ad794 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -526,14 +526,14 @@ struct drm_mode_crtc_page_flip { /* create a dumb scanout buffer */ struct drm_mode_create_dumb { - uint32_t height; - uint32_t width; - uint32_t bpp; - uint32_t flags; + __u32 height; + __u32 width; + __u32 bpp; + __u32 flags; /* handle, pitch, size will be returned */ - uint32_t handle; - uint32_t pitch; - uint64_t size; + __u32 handle; + __u32 pitch; + __u64 size; }; /* set up for mmap of a dumb scanout buffer */ @@ -550,7 +550,7 @@ struct drm_mode_map_dumb { }; struct drm_mode_destroy_dumb { - uint32_t handle; + __u32 handle; }; /* page-flip flags are valid, plus: */ -- 2.6.2
[PATCH 6/9] include/uapi/drm/armada_drm.h: use __u{32, 64} types instead of uint{32, 64}_t
Kernel headers should use linux/types.h Signed-off-by: Gabriel Laskar --- include/uapi/drm/radeon_drm.h | 130 +- 1 file changed, 66 insertions(+), 64 deletions(-) diff --git a/include/uapi/drm/radeon_drm.h b/include/uapi/drm/radeon_drm.h index 01aa2a8..84a32ce 100644 --- a/include/uapi/drm/radeon_drm.h +++ b/include/uapi/drm/radeon_drm.h @@ -35,6 +35,8 @@ #include "drm.h" +#include + /* WARNING: If you change any of these defines, make sure to change the * defines in the X server file (radeon_sarea.h) */ @@ -793,9 +795,9 @@ typedef struct drm_radeon_surface_free { #define RADEON_GEM_DOMAIN_VRAM 0x4 struct drm_radeon_gem_info { - uint64_tgart_size; - uint64_tvram_size; - uint64_tvram_visible; + __u64 gart_size; + __u64 vram_size; + __u64 vram_visible; }; #define RADEON_GEM_NO_BACKING_STORE(1 << 0) @@ -807,11 +809,11 @@ struct drm_radeon_gem_info { #define RADEON_GEM_NO_CPU_ACCESS (1 << 4) struct drm_radeon_gem_create { - uint64_tsize; - uint64_talignment; - uint32_thandle; - uint32_tinitial_domain; - uint32_tflags; + __u64 size; + __u64 alignment; + __u32 handle; + __u32 initial_domain; + __u32 flags; }; /* @@ -825,10 +827,10 @@ struct drm_radeon_gem_create { #define RADEON_GEM_USERPTR_REGISTER(1 << 3) struct drm_radeon_gem_userptr { - uint64_taddr; - uint64_tsize; - uint32_tflags; - uint32_thandle; + __u64 addr; + __u64 size; + __u32 flags; + __u32 handle; }; #define RADEON_TILING_MACRO0x1 @@ -850,72 +852,72 @@ struct drm_radeon_gem_userptr { #define RADEON_TILING_EG_STENCIL_TILE_SPLIT_MASK 0xf struct drm_radeon_gem_set_tiling { - uint32_thandle; - uint32_ttiling_flags; - uint32_tpitch; + __u32 handle; + __u32 tiling_flags; + __u32 pitch; }; struct drm_radeon_gem_get_tiling { - uint32_thandle; - uint32_ttiling_flags; - uint32_tpitch; + __u32 handle; + __u32 tiling_flags; + __u32 pitch; }; struct drm_radeon_gem_mmap { - uint32_thandle; - uint32_tpad; - uint64_toffset; - uint64_tsize; - uint64_taddr_ptr; + __u32 handle; + __u32 pad; + __u64 offset; + __u64 size; + __u64 addr_ptr; }; struct drm_radeon_gem_set_domain { - uint32_thandle; - uint32_tread_domains; - uint32_twrite_domain; + __u32 handle; + __u32 read_domains; + __u32 write_domain; }; struct drm_radeon_gem_wait_idle { - uint32_thandle; - uint32_tpad; + __u32 handle; + __u32 pad; }; struct drm_radeon_gem_busy { - uint32_thandle; - uint32_tdomain; + __u32 handle; + __u32domain; }; struct drm_radeon_gem_pread { /** Handle for the object being read. */ - uint32_t handle; - uint32_t pad; + __u32 handle; + __u32 pad; /** Offset into the object to read from */ - uint64_t offset; + __u64 offset; /** Length of data to read */ - uint64_t size; + __u64 size; /** Pointer to write the data into. */ /* void *, but pointers are not 32/64 compatible */ - uint64_t data_ptr; + __u64 data_ptr; }; struct drm_radeon_gem_pwrite { /** Handle for the object being written to. */ - uint32_t handle; - uint32_t pad; + __u32 handle; + __u32 pad; /** Offset into the object to write to */ - uint64_t offset; + __u64 offset; /** Length of data to write */ - uint64_t size; + __u64 size; /** Pointer to read the data from. */ /* void *, but pointers are not 32/64 compatible */ - uint64_t data_ptr; + __u64 data_ptr; }; /* Sets or returns a value associated with a buffer. */ struct drm_radeon_gem_op { - uint32_thandle; /* buffer */ - uint32_top; /* RADEON_GEM_OP_* */ - uint64_tvalue; /* input or return value */ + __u32 handle; /* buffer */ + __u32 op; /* RADEON_GEM_OP_* */ + __u64 value; /* input or return value */ }; #define RADEON_GEM_OP_GET_INITIAL_DOMAIN 0 @@ -935,11 +937,11 @@ struct drm_radeon_gem_op { #define RADEON_VM_PAGE_SNOOPED (1 << 4) struct drm_radeon_gem_va { - uint32_thandle; - uint32_toperation; - uint32_tvm_id; - uint32_t
[PATCH 5/9] include/uapi/drm/amdgpu_drm.h: use __u{32, 64} types instead of uint{32, 64}_t
Kernel headers should use linux/types.h Signed-off-by: Gabriel Laskar --- include/uapi/drm/amdgpu_drm.h | 292 +- 1 file changed, 147 insertions(+), 145 deletions(-) diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h index e52933a..7f169c3 100644 --- a/include/uapi/drm/amdgpu_drm.h +++ b/include/uapi/drm/amdgpu_drm.h @@ -34,6 +34,8 @@ #include "drm.h" +#include + #define DRM_AMDGPU_GEM_CREATE 0x00 #define DRM_AMDGPU_GEM_MMAP0x01 #define DRM_AMDGPU_CTX 0x02 @@ -76,19 +78,19 @@ struct drm_amdgpu_gem_create_in { /** the requested memory size */ - uint64_t bo_size; + __u64 bo_size; /** physical start_addr alignment in bytes for some HW requirements */ - uint64_t alignment; + __u64 alignment; /** the requested memory domains */ - uint64_t domains; + __u64 domains; /** allocation flags */ - uint64_t domain_flags; + __u64 domain_flags; }; struct drm_amdgpu_gem_create_out { /** returned GEM object handle */ - uint32_t handle; - uint32_t _pad; + __u32 handle; + __u32 _pad; }; union drm_amdgpu_gem_create { @@ -105,28 +107,28 @@ union drm_amdgpu_gem_create { struct drm_amdgpu_bo_list_in { /** Type of operation */ - uint32_t operation; + __u32 operation; /** Handle of list or 0 if we want to create one */ - uint32_t list_handle; + __u32 list_handle; /** Number of BOs in list */ - uint32_t bo_number; + __u32 bo_number; /** Size of each element describing BO */ - uint32_t bo_info_size; + __u32 bo_info_size; /** Pointer to array describing BOs */ - uint64_t bo_info_ptr; + __u64 bo_info_ptr; }; struct drm_amdgpu_bo_list_entry { /** Handle of BO */ - uint32_t bo_handle; + __u32 bo_handle; /** New (if specified) BO priority to be used during migration */ - uint32_t bo_priority; + __u32 bo_priority; }; struct drm_amdgpu_bo_list_out { /** Handle of resource list */ - uint32_t list_handle; - uint32_t _pad; + __u32 list_handle; + __u32 _pad; }; union drm_amdgpu_bo_list { @@ -150,26 +152,26 @@ union drm_amdgpu_bo_list { struct drm_amdgpu_ctx_in { /** AMDGPU_CTX_OP_* */ - uint32_top; + __u32 op; /** For future use, no flags defined so far */ - uint32_tflags; - uint32_tctx_id; - uint32_t_pad; + __u32 flags; + __u32 ctx_id; + __u32 _pad; }; union drm_amdgpu_ctx_out { struct { - uint32_tctx_id; - uint32_t_pad; + __u32 ctx_id; + __u32 _pad; } alloc; struct { /** For future use, no flags defined so far */ - uint64_tflags; + __u64 flags; /** Number of resets caused by this context so far. */ - uint32_thangs; + __u32 hangs; /** Reset status since the last call of the ioctl. */ - uint32_treset_status; + __u32 reset_status; } state; }; @@ -189,12 +191,12 @@ union drm_amdgpu_ctx { #define AMDGPU_GEM_USERPTR_REGISTER(1 << 3) struct drm_amdgpu_gem_userptr { - uint64_taddr; - uint64_tsize; + __u64 addr; + __u64 size; /* AMDGPU_GEM_USERPTR_* */ - uint32_tflags; + __u32 flags; /* Resulting GEM handle */ - uint32_thandle; + __u32 handle; }; /* same meaning as the GB_TILE_MODE and GL_MACRO_TILE_MODE fields */ @@ -226,28 +228,28 @@ struct drm_amdgpu_gem_userptr { /** The same structure is shared for input/output */ struct drm_amdgpu_gem_metadata { /** GEM Object handle */ - uint32_thandle; + __u32 handle; /** Do we want get or set metadata */ - uint32_top; + __u32 op; struct { /** For future use, no flags defined so far */ - uint64_tflags; + __u64 flags; /** family specific tiling info */ - uint64_ttiling_info; - uint32_tdata_size_bytes; - uint32_tdata[64]; + __u64 tiling_info; + __u32 data_size_bytes; + __u32 data[64]; } data; }; struct drm_amdgpu_gem_mmap_in { /** the GEM object handle */ - uint32_t handle; - uint32_t _pad; + __u32 handle; + __u32 _pad; };
[PATCH 4/9] include/uapi/drm/nouveau_drm.h: use __u{32, 64} types instead of uint{32, 64}_t
Kernel headers should use linux/types.h Signed-off-by: Gabriel Laskar --- include/uapi/drm/nouveau_drm.h | 84 +- 1 file changed, 42 insertions(+), 42 deletions(-) diff --git a/include/uapi/drm/nouveau_drm.h b/include/uapi/drm/nouveau_drm.h index fd594cc..722b6a6 100644 --- a/include/uapi/drm/nouveau_drm.h +++ b/include/uapi/drm/nouveau_drm.h @@ -41,34 +41,34 @@ #define NOUVEAU_GEM_TILE_NONCONTIG 0x0008 struct drm_nouveau_gem_info { - uint32_t handle; - uint32_t domain; - uint64_t size; - uint64_t offset; - uint64_t map_handle; - uint32_t tile_mode; - uint32_t tile_flags; + __u32 handle; + __u32 domain; + __u64 size; + __u64 offset; + __u64 map_handle; + __u32 tile_mode; + __u32 tile_flags; }; struct drm_nouveau_gem_new { struct drm_nouveau_gem_info info; - uint32_t channel_hint; - uint32_t align; + __u32 channel_hint; + __u32 align; }; #define NOUVEAU_GEM_MAX_BUFFERS 1024 struct drm_nouveau_gem_pushbuf_bo_presumed { - uint32_t valid; - uint32_t domain; - uint64_t offset; + __u32 valid; + __u32 domain; + __u64 offset; }; struct drm_nouveau_gem_pushbuf_bo { - uint64_t user_priv; - uint32_t handle; - uint32_t read_domains; - uint32_t write_domains; - uint32_t valid_domains; + __u64 user_priv; + __u32 handle; + __u32 read_domains; + __u32 write_domains; + __u32 valid_domains; struct drm_nouveau_gem_pushbuf_bo_presumed presumed; }; @@ -77,46 +77,46 @@ struct drm_nouveau_gem_pushbuf_bo { #define NOUVEAU_GEM_RELOC_OR (1 << 2) #define NOUVEAU_GEM_MAX_RELOCS 1024 struct drm_nouveau_gem_pushbuf_reloc { - uint32_t reloc_bo_index; - uint32_t reloc_bo_offset; - uint32_t bo_index; - uint32_t flags; - uint32_t data; - uint32_t vor; - uint32_t tor; + __u32 reloc_bo_index; + __u32 reloc_bo_offset; + __u32 bo_index; + __u32 flags; + __u32 data; + __u32 vor; + __u32 tor; }; #define NOUVEAU_GEM_MAX_PUSH 512 struct drm_nouveau_gem_pushbuf_push { - uint32_t bo_index; - uint32_t pad; - uint64_t offset; - uint64_t length; + __u32 bo_index; + __u32 pad; + __u64 offset; + __u64 length; }; struct drm_nouveau_gem_pushbuf { - uint32_t channel; - uint32_t nr_buffers; - uint64_t buffers; - uint32_t nr_relocs; - uint32_t nr_push; - uint64_t relocs; - uint64_t push; - uint32_t suffix0; - uint32_t suffix1; - uint64_t vram_available; - uint64_t gart_available; + __u32 channel; + __u32 nr_buffers; + __u64 buffers; + __u32 nr_relocs; + __u32 nr_push; + __u64 relocs; + __u64 push; + __u32 suffix0; + __u32 suffix1; + __u64 vram_available; + __u64 gart_available; }; #define NOUVEAU_GEM_CPU_PREP_NOWAIT 0x0001 #define NOUVEAU_GEM_CPU_PREP_WRITE 0x0004 struct drm_nouveau_gem_cpu_prep { - uint32_t handle; - uint32_t flags; + __u32 handle; + __u32 flags; }; struct drm_nouveau_gem_cpu_fini { - uint32_t handle; + __u32 handle; }; #define DRM_NOUVEAU_GETPARAM 0x00 /* deprecated */ -- 2.6.2
[PATCH 3/9] include/uapi/drm/qxl_drm.h: use __u{32, 64} types instead of uint{32, 64}_t
Kernel headers should use linux/types.h Signed-off-by: Gabriel Laskar --- include/uapi/drm/qxl_drm.h | 74 +++--- 1 file changed, 37 insertions(+), 37 deletions(-) diff --git a/include/uapi/drm/qxl_drm.h b/include/uapi/drm/qxl_drm.h index ebebd36..2aa6376 100644 --- a/include/uapi/drm/qxl_drm.h +++ b/include/uapi/drm/qxl_drm.h @@ -30,7 +30,7 @@ /* Please note that modifications to all structs defined here are * subject to backwards-compatibility constraints. * - * Do not use pointers, use uint64_t instead for 32 bit / 64 bit user/kernel + * Do not use pointers, use __u64 instead for 32 bit / 64 bit user/kernel * compatibility Keep fields aligned to their size */ @@ -48,14 +48,14 @@ #define DRM_QXL_ALLOC_SURF 0x06 struct drm_qxl_alloc { - uint32_t size; - uint32_t handle; /* 0 is an invalid handle */ + __u32 size; + __u32 handle; /* 0 is an invalid handle */ }; struct drm_qxl_map { - uint64_t offset; /* use for mmap system call */ - uint32_t handle; - uint32_t pad; + __u64 offset; /* use for mmap system call */ + __u32 handle; + __u32 pad; }; /* @@ -68,59 +68,59 @@ struct drm_qxl_map { #define QXL_RELOC_TYPE_SURF 2 struct drm_qxl_reloc { - uint64_t src_offset; /* offset into src_handle or src buffer */ - uint64_t dst_offset; /* offset in dest handle */ - uint32_t src_handle; /* dest handle to compute address from */ - uint32_t dst_handle; /* 0 if to command buffer */ - uint32_t reloc_type; - uint32_t pad; + __u64 src_offset; /* offset into src_handle or src buffer */ + __u64 dst_offset; /* offset in dest handle */ + __u32 src_handle; /* dest handle to compute address from */ + __u32 dst_handle; /* 0 if to command buffer */ + __u32 reloc_type; + __u32 pad; }; struct drm_qxl_command { - uint64_t __user command; /* void* */ - uint64_t __user relocs; /* struct drm_qxl_reloc* */ - uint32_ttype; - uint32_tcommand_size; - uint32_trelocs_num; - uint32_tpad; + __u64__user command; /* void* */ + __u64__user relocs; /* struct drm_qxl_reloc* */ + __u32 type; + __u32 command_size; + __u32 relocs_num; + __u32 pad; }; /* XXX: call it drm_qxl_commands? */ struct drm_qxl_execbuffer { - uint32_tflags; /* for future use */ - uint32_tcommands_num; - uint64_t __user commands; /* struct drm_qxl_command* */ + __u32 flags; /* for future use */ + __u32 commands_num; + __u64__user commands; /* struct drm_qxl_command* */ }; struct drm_qxl_update_area { - uint32_t handle; - uint32_t top; - uint32_t left; - uint32_t bottom; - uint32_t right; - uint32_t pad; + __u32 handle; + __u32 top; + __u32 left; + __u32 bottom; + __u32 right; + __u32 pad; }; #define QXL_PARAM_NUM_SURFACES 1 /* rom->n_surfaces */ #define QXL_PARAM_MAX_RELOCS 2 struct drm_qxl_getparam { - uint64_t param; - uint64_t value; + __u64 param; + __u64 value; }; /* these are one bit values */ struct drm_qxl_clientcap { - uint32_t index; - uint32_t pad; + __u32 index; + __u32 pad; }; struct drm_qxl_alloc_surf { - uint32_t format; - uint32_t width; - uint32_t height; - int32_t stride; - uint32_t handle; - uint32_t pad; + __u32 format; + __u32 width; + __u32 height; + __s32 stride; + __u32 handle; + __u32 pad; }; #define DRM_IOCTL_QXL_ALLOC \ -- 2.6.2
[PATCH 2/9] include/uapi/drm/virtgpu_drm.h: use __u{32, 64} types instead of uint{32, 64}_t
Kernel headers should use linux/types.h Signed-off-by: Gabriel Laskar --- include/uapi/drm/virtgpu_drm.h | 99 +- 1 file changed, 50 insertions(+), 49 deletions(-) diff --git a/include/uapi/drm/virtgpu_drm.h b/include/uapi/drm/virtgpu_drm.h index fc9e2d6..60b11e7 100644 --- a/include/uapi/drm/virtgpu_drm.h +++ b/include/uapi/drm/virtgpu_drm.h @@ -25,12 +25,13 @@ #define VIRTGPU_DRM_H #include +#include #include "drm/drm.h" /* Please note that modifications to all structs defined here are * subject to backwards-compatibility constraints. * - * Do not use pointers, use uint64_t instead for 32 bit / 64 bit user/kernel + * Do not use pointers, use __u64 instead for 32 bit / 64 bit user/kernel * compatibility Keep fields aligned to their size */ @@ -45,88 +46,88 @@ #define DRM_VIRTGPU_GET_CAPS 0x09 struct drm_virtgpu_map { - uint64_t offset; /* use for mmap system call */ - uint32_t handle; - uint32_t pad; + __u64 offset; /* use for mmap system call */ + __u32 handle; + __u32 pad; }; struct drm_virtgpu_execbuffer { - uint32_tflags; /* for future use */ - uint32_t size; - uint64_t command; /* void* */ - uint64_t bo_handles; - uint32_t num_bo_handles; - uint32_t pad; + __u32 flags; /* for future use */ + __u32 size; + __u64 command; /* void* */ + __u64 bo_handles; + __u32 num_bo_handles; + __u32 pad; }; #define VIRTGPU_PARAM_3D_FEATURES 1 /* do we have 3D features in the hw */ struct drm_virtgpu_getparam { - uint64_t param; - uint64_t value; + __u64 param; + __u64 value; }; /* NO_BO flags? NO resource flag? */ /* resource flag for y_0_top */ struct drm_virtgpu_resource_create { - uint32_t target; - uint32_t format; - uint32_t bind; - uint32_t width; - uint32_t height; - uint32_t depth; - uint32_t array_size; - uint32_t last_level; - uint32_t nr_samples; - uint32_t flags; - uint32_t bo_handle; /* if this is set - recreate a new resource attached to this bo ? */ - uint32_t res_handle; /* returned by kernel */ - uint32_t size;/* validate transfer in the host */ - uint32_t stride; /* validate transfer in the host */ + __u32 target; + __u32 format; + __u32 bind; + __u32 width; + __u32 height; + __u32 depth; + __u32 array_size; + __u32 last_level; + __u32 nr_samples; + __u32 flags; + __u32 bo_handle; /* if this is set - recreate a new resource attached to this bo ? */ + __u32 res_handle; /* returned by kernel */ + __u32 size;/* validate transfer in the host */ + __u32 stride; /* validate transfer in the host */ }; struct drm_virtgpu_resource_info { - uint32_t bo_handle; - uint32_t res_handle; - uint32_t size; - uint32_t stride; + __u32 bo_handle; + __u32 res_handle; + __u32 size; + __u32 stride; }; struct drm_virtgpu_3d_box { - uint32_t x; - uint32_t y; - uint32_t z; - uint32_t w; - uint32_t h; - uint32_t d; + __u32 x; + __u32 y; + __u32 z; + __u32 w; + __u32 h; + __u32 d; }; struct drm_virtgpu_3d_transfer_to_host { - uint32_t bo_handle; + __u32 bo_handle; struct drm_virtgpu_3d_box box; - uint32_t level; - uint32_t offset; + __u32 level; + __u32 offset; }; struct drm_virtgpu_3d_transfer_from_host { - uint32_t bo_handle; + __u32 bo_handle; struct drm_virtgpu_3d_box box; - uint32_t level; - uint32_t offset; + __u32 level; + __u32 offset; }; #define VIRTGPU_WAIT_NOWAIT 1 /* like it */ struct drm_virtgpu_3d_wait { - uint32_t handle; /* 0 is an invalid handle */ - uint32_t flags; + __u32 handle; /* 0 is an invalid handle */ + __u32 flags; }; struct drm_virtgpu_get_caps { - uint32_t cap_set_id; - uint32_t cap_set_ver; - uint64_t addr; - uint32_t size; - uint32_t pad; + __u32 cap_set_id; + __u32 cap_set_ver; + __u64 addr; + __u32 size; + __u32 pad; }; #define DRM_IOCTL_VIRTGPU_MAP \ -- 2.6.2
[PATCH 1/9] include/uapi/drm/armada_drm.h: use __u{32, 64} types instead of uint{32, 64}_t
Kernel headers should use linux/types.h Signed-off-by: Gabriel Laskar --- include/uapi/drm/armada_drm.h | 24 +--- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/include/uapi/drm/armada_drm.h b/include/uapi/drm/armada_drm.h index 8dec3fd..77184b7 100644 --- a/include/uapi/drm/armada_drm.h +++ b/include/uapi/drm/armada_drm.h @@ -9,6 +9,8 @@ #ifndef DRM_ARMADA_IOCTL_H #define DRM_ARMADA_IOCTL_H +#include + #define DRM_ARMADA_GEM_CREATE 0x00 #define DRM_ARMADA_GEM_MMAP0x02 #define DRM_ARMADA_GEM_PWRITE 0x03 @@ -17,27 +19,27 @@ DRM_##dir(DRM_COMMAND_BASE + DRM_ARMADA_##name, struct drm_armada_##str) struct drm_armada_gem_create { - uint32_t handle; - uint32_t size; + __u32 handle; + __u32 size; }; #define DRM_IOCTL_ARMADA_GEM_CREATE \ ARMADA_IOCTL(IOWR, GEM_CREATE, gem_create) struct drm_armada_gem_mmap { - uint32_t handle; - uint32_t pad; - uint64_t offset; - uint64_t size; - uint64_t addr; + __u32 handle; + __u32 pad; + __u64 offset; + __u64 size; + __u64 addr; }; #define DRM_IOCTL_ARMADA_GEM_MMAP \ ARMADA_IOCTL(IOWR, GEM_MMAP, gem_mmap) struct drm_armada_gem_pwrite { - uint64_t ptr; - uint32_t handle; - uint32_t offset; - uint32_t size; + __u64 ptr; + __u32 handle; + __u32 offset; + __u32 size; }; #define DRM_IOCTL_ARMADA_GEM_PWRITE \ ARMADA_IOCTL(IOW, GEM_PWRITE, gem_pwrite) -- 2.6.2
[PATCH 0/9] uapi: drm: fixes for userspace compilation
Public headers should use types from include/uapi/linux/types.h. This series fixes that, and allow out-of-the-box compilation of thoses headers from userspace. Some programs need to parse and use these headers, in order to gather informations about the public kernel API, for example strace generation scripts, some abi checkers, and so on. Gabriel Laskar (9): include/uapi/drm/armada_drm.h: use __u{32,64} types instead of uint{32,64}_t include/uapi/drm/virtgpu_drm.h: use __u{32,64} types instead of uint{32,64}_t include/uapi/drm/qxl_drm.h: use __u{32,64} types instead of uint{32,64}_t include/uapi/drm/nouveau_drm.h: use __u{32,64} types instead of uint{32,64}_t include/uapi/drm/amdgpu_drm.h: use __u{32,64} types instead of uint{32,64}_t include/uapi/drm/armada_drm.h: use __u{32,64} types instead of uint{32,64}_t include/uapi/drm/drm_mode.h: use __u{32,64} types instead of uint{32,64}_t include/uapi/drm/vmwgfx_drm.h: use __u{32,64} types instead of uint{32,64}_t include/uapi/drm/omap_drm.h: use __u{32,64} types instead of uint{32,64}_t include/uapi/drm/amdgpu_drm.h | 292 + include/uapi/drm/armada_drm.h | 24 ++-- include/uapi/drm/drm_mode.h| 16 +-- include/uapi/drm/nouveau_drm.h | 84 ++-- include/uapi/drm/omap_drm.h| 39 +++--- include/uapi/drm/qxl_drm.h | 74 +-- include/uapi/drm/radeon_drm.h | 130 +- include/uapi/drm/virtgpu_drm.h | 99 +++--- include/uapi/drm/vmwgfx_drm.h | 264 ++--- 9 files changed, 515 insertions(+), 507 deletions(-) -- 2.6.2
[Bug 92900] [regression bisected] About 700 piglit regressions is what could go wrong
https://bugs.freedesktop.org/show_bug.cgi?id=92900 --- Comment #5 from Roland Scheidegger --- (In reply to Ian Romanick from comment #4) > > That fixes the problem here. If you want to send that as a patch, you can > put > > Tested-by: Ian Romanick > Cc: "11.0" > > on it. Thanks for testing! Patches sent. -- 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/20151112/dd8cc708/attachment.html>
drm: Bogus WARN() in drm_atomic_helper_update_legacy_modeset_state() ?
On 2015å¹´11æ12æ¥ 18:34, Liviu Dudau wrote: > Can you switch your email client to text mode and make it do proper reply > quoting? It is > rather difficult to follow where your reply comes when you don't use HTML > MUAs. > > Best regards, > Liviu Hi Liviu I'm sorry about that, now I switch my email into text mode, but I don't known whether it take effect, everything look the same. Thanks. -- ï¼ark Yao
[PATCH 6/6] drm/i915: Give meaningful names to all the planes
From: Ville SyrjäläLet's name our planes in a way that makes sense wrt. the spec: - skl+ -> "plane 1A", "plane 2A", "plane 1C", "cursor A" etc. - g4x+ -> "primary A", "primary B", "sprite A", "cursor C" etc. - pre-g4x -> "plane A", "cursor B" etc. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_display.c | 35 +-- drivers/gpu/drm/i915/intel_sprite.c | 14 ++ 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 2b5e81a..82b2f58 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -13788,7 +13788,15 @@ static void intel_finish_crtc_commit(struct drm_crtc *crtc, void intel_plane_destroy(struct drm_plane *plane) { struct intel_plane *intel_plane = to_intel_plane(plane); + char *name; + + /* +* drm_plane_cleanup() zeroes the structure, so +* need an extra dance to avoid leaking the name. +*/ + name = plane->name; drm_plane_cleanup(plane); + kfree(name); kfree(intel_plane); } @@ -13838,6 +13846,21 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev, if (HAS_FBC(dev) && INTEL_INFO(dev)->gen < 4) primary->plane = !pipe; + if (INTEL_INFO(dev)->gen >= 9) + primary->base.name = kasprintf(GFP_KERNEL, "plane 1%c", + pipe_name(pipe)); + else if (INTEL_INFO(dev)->gen >= 5 || IS_G4X(dev)) + primary->base.name = kasprintf(GFP_KERNEL, "primary %c", + pipe_name(pipe)); + else + primary->base.name = kasprintf(GFP_KERNEL, "plane %c", + plane_name(primary->plane)); + if (!primary->base.name) { + kfree(state); + kfree(primary); + return NULL; + } + if (INTEL_INFO(dev)->gen >= 9) { intel_primary_formats = skl_primary_formats; num_formats = ARRAY_SIZE(skl_primary_formats); @@ -13987,6 +14010,14 @@ static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev, cursor->commit_plane = intel_commit_cursor_plane; cursor->disable_plane = intel_disable_cursor_plane; + cursor->base.name = kasprintf(GFP_KERNEL, "cursor %c", + pipe_name(pipe)); + if (!cursor->base.name) { + kfree(state); + kfree(cursor); + return NULL; + } + drm_universal_plane_init(dev, >base, 0, _plane_funcs, intel_cursor_formats, @@ -14113,9 +14144,9 @@ static void intel_crtc_init(struct drm_device *dev, int pipe) fail: if (primary) - drm_plane_cleanup(primary); + intel_plane_destroy(primary); if (cursor) - drm_plane_cleanup(cursor); + intel_plane_destroy(cursor); kfree(crtc_state); kfree(intel_crtc->base.name); kfree(intel_crtc); diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index a2c15f8..b1520f1 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -1119,7 +1119,21 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane) intel_plane->frontbuffer_bit = INTEL_FRONTBUFFER_SPRITE(pipe, plane); intel_plane->check_plane = intel_check_sprite_plane; intel_plane->commit_plane = intel_commit_sprite_plane; + + if (INTEL_INFO(dev)->gen >= 9) + intel_plane->base.name = kasprintf(GFP_KERNEL, "plane %d%c", + plane + 2, pipe_name(pipe)); + else + intel_plane->base.name = kasprintf(GFP_KERNEL, "sprite %c", + sprite_name(pipe, plane)); + if (!intel_plane->base.name) { + kfree(state); + kfree(intel_plane); + return -ENOMEM; + } + possible_crtcs = (1 << pipe); + ret = drm_universal_plane_init(dev, _plane->base, possible_crtcs, _plane_funcs, plane_formats, num_plane_formats, -- 2.4.10
[PATCH 5/6] drm/i915: Set crtc->name to "pipe A", "pipe B", etc.
From: Ville SyrjäläSigned-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_display.c | 14 +- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index b628dab..2b5e81a 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -10711,6 +10711,7 @@ static void intel_crtc_destroy(struct drm_crtc *crtc) struct intel_crtc *intel_crtc = to_intel_crtc(crtc); struct drm_device *dev = crtc->dev; struct intel_unpin_work *work; + char *name; spin_lock_irq(>event_lock); work = intel_crtc->unpin_work; @@ -10722,8 +10723,13 @@ static void intel_crtc_destroy(struct drm_crtc *crtc) kfree(work); } + /* +* drm_crtc_cleanup() zeroes the structure, so +* need an extra dance to avoid leaking the name. +*/ + name = crtc->name; drm_crtc_cleanup(crtc); - + kfree(name); kfree(intel_crtc); } @@ -14036,6 +14042,11 @@ static void intel_crtc_init(struct drm_device *dev, int pipe) if (intel_crtc == NULL) return; + intel_crtc->base.name = kasprintf(GFP_KERNEL, "pipe %c", + pipe_name(pipe)); + if (!intel_crtc->base.name) + return; + crtc_state = kzalloc(sizeof(*crtc_state), GFP_KERNEL); if (!crtc_state) goto fail; @@ -14106,6 +14117,7 @@ fail: if (cursor) drm_plane_cleanup(cursor); kfree(crtc_state); + kfree(intel_crtc->base.name); kfree(intel_crtc); } -- 2.4.10
[PATCH 4/6] drm/i915: Use plane->name in debug prints
From: Ville SyrjäläSigned-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_display.c | 38 +--- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 9845687..b628dab 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4433,9 +4433,9 @@ static int skl_update_scaler_plane(struct intel_crtc_state *crtc_state, bool force_detach = !fb || !plane_state->visible; - DRM_DEBUG_KMS("Updating scaler for [PLANE:%d] scaler_user index %u.%u\n", - intel_plane->base.base.id, intel_crtc->pipe, - drm_plane_index(_plane->base)); + DRM_DEBUG_KMS("Updating scaler for [PLANE:%d:%s] scaler_user index %u.%u\n", + intel_plane->base.base.id, intel_plane->base.name, + intel_crtc->pipe, drm_plane_index(_plane->base)); ret = skl_update_scaler(crtc_state, force_detach, drm_plane_index(_plane->base), @@ -4451,8 +4451,9 @@ static int skl_update_scaler_plane(struct intel_crtc_state *crtc_state, /* check colorkey */ if (plane_state->ckey.flags != I915_SET_COLORKEY_NONE) { - DRM_DEBUG_KMS("[PLANE:%d] scaling with color key not allowed", - intel_plane->base.base.id); + DRM_DEBUG_KMS("[PLANE:%d:%s] scaling with color key not allowed", + intel_plane->base.base.id, + intel_plane->base.name); return -EINVAL; } @@ -4471,8 +4472,9 @@ static int skl_update_scaler_plane(struct intel_crtc_state *crtc_state, case DRM_FORMAT_VYUY: break; default: - DRM_DEBUG_KMS("[PLANE:%d] FB:%d unsupported scaling format 0x%x\n", - intel_plane->base.base.id, fb->base.id, fb->pixel_format); + DRM_DEBUG_KMS("[PLANE:%d:%s] FB:%d unsupported scaling format 0x%x\n", + intel_plane->base.base.id, intel_plane->base.name, + fb->base.id, fb->pixel_format); return -EINVAL; } @@ -11679,13 +11681,15 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state, turn_off = was_visible && (!visible || mode_changed); turn_on = visible && (!was_visible || mode_changed); - DRM_DEBUG_ATOMIC("[CRTC:%d:%s] has [PLANE:%i] with fb %i\n", + DRM_DEBUG_ATOMIC("[CRTC:%d:%s] has [PLANE:%d:%s] with fb %i\n", intel_crtc->base.base.id, intel_crtc->base.name, -plane->base.id, fb ? fb->base.id : -1); +plane->base.id, plane->name, +fb ? fb->base.id : -1); - DRM_DEBUG_ATOMIC("[PLANE:%i] visible %i -> %i, off %i, on %i, ms %i\n", -plane->base.id, was_visible, visible, + DRM_DEBUG_ATOMIC("[PLANE:%d:%s] visible %i -> %i, off %i, on %i, ms %i\n", +plane->base.id, plane->name, +was_visible, visible, turn_off, turn_on, mode_changed); if (turn_on) { @@ -12088,18 +12092,20 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc, state = to_intel_plane_state(plane->state); fb = state->base.fb; if (!fb) { - DRM_DEBUG_KMS("%s PLANE:%d plane: %u.%u idx: %d " - "disabled, scaler_id = %d\n", + DRM_DEBUG_KMS("%s [PLANE:%d:%s] plane: %u.%u idx: %d " + "disabled, scaler_id = %d\n", plane->type == DRM_PLANE_TYPE_CURSOR ? "CURSOR" : "STANDARD", - plane->base.id, intel_plane->pipe, + plane->base.id, plane->name, + intel_plane->pipe, (crtc->base.primary == plane) ? 0 : intel_plane->plane + 1, drm_plane_index(plane), state->scaler_id); continue; } - DRM_DEBUG_KMS("%s PLANE:%d plane: %u.%u idx: %d enabled", + DRM_DEBUG_KMS("%s [PLANE:%d:%s] plane: %u.%u idx: %d enabled", plane->type == DRM_PLANE_TYPE_CURSOR ? "CURSOR" : "STANDARD", - plane->base.id, intel_plane->pipe, + plane->base.id, plane->name, + intel_plane->pipe, crtc->base.primary == plane ? 0 : intel_plane->plane + 1, drm_plane_index(plane)); DRM_DEBUG_KMS("\tFB:%d, fb = %ux%u format = 0x%x", -- 2.4.10
[PATCH 3/6] drm/i915: Use crtc->name in debug messages
From: Ville SyrjäläSigned-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_display.c | 55 drivers/gpu/drm/i915/intel_fbdev.c | 5 ++-- 2 files changed, 33 insertions(+), 27 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index b5f7493..9845687 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4220,8 +4220,9 @@ struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc, i = (enum intel_dpll_id) crtc->pipe; pll = _priv->shared_dplls[i]; - DRM_DEBUG_KMS("CRTC:%d using pre-allocated %s\n", - crtc->base.base.id, pll->name); + DRM_DEBUG_KMS("[CRTC:%d:%s] using pre-allocated %s\n", + crtc->base.base.id, crtc->base.name, + pll->name); WARN_ON(shared_dpll[i].crtc_mask); @@ -4241,8 +4242,9 @@ struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc, /* 1:1 mapping between ports and PLLs */ i = (enum intel_dpll_id)intel_dig_port->port; pll = _priv->shared_dplls[i]; - DRM_DEBUG_KMS("CRTC:%d using pre-allocated %s\n", - crtc->base.base.id, pll->name); + DRM_DEBUG_KMS("[CRTC:%d:%s] using pre-allocated %s\n", + crtc->base.base.id, crtc->base.name, + pll->name); WARN_ON(shared_dpll[i].crtc_mask); goto found; @@ -4258,9 +4260,9 @@ struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc, if (memcmp(_state->dpll_hw_state, _dpll[i].hw_state, sizeof(crtc_state->dpll_hw_state)) == 0) { - DRM_DEBUG_KMS("CRTC:%d sharing existing %s (crtc mask 0x%08x, ative %d)\n", - crtc->base.base.id, pll->name, - shared_dpll[i].crtc_mask, + DRM_DEBUG_KMS("[CRTC:%d:%s] sharing existing %s (crtc mask 0x%08x, ative %d)\n", + crtc->base.base.id, crtc->base.name, + pll->name, shared_dpll[i].crtc_mask, pll->active); goto found; } @@ -4270,8 +4272,9 @@ struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc, for (i = 0; i < dev_priv->num_shared_dpll; i++) { pll = _priv->shared_dplls[i]; if (shared_dpll[i].crtc_mask == 0) { - DRM_DEBUG_KMS("CRTC:%d allocated %s\n", - crtc->base.base.id, pll->name); + DRM_DEBUG_KMS("[CRTC:%d:%s] allocated %s\n", + crtc->base.base.id, crtc->base.name, + pll->name); goto found; } } @@ -4398,8 +4401,9 @@ int skl_update_scaler_crtc(struct intel_crtc_state *state) struct intel_crtc *intel_crtc = to_intel_crtc(state->base.crtc); const struct drm_display_mode *adjusted_mode = >base.adjusted_mode; - DRM_DEBUG_KMS("Updating scaler for [CRTC:%i] scaler_user index %u.%u\n", - intel_crtc->base.base.id, intel_crtc->pipe, SKL_CRTC_INDEX); + DRM_DEBUG_KMS("Updating scaler for [CRTC:%d:%s] scaler_user index %u.%u\n", + intel_crtc->base.base.id, intel_crtc->base.name, + intel_crtc->pipe, SKL_CRTC_INDEX); return skl_update_scaler(state, !state->base.active, SKL_CRTC_INDEX, >scaler_state.scaler_id, DRM_ROTATE_0, @@ -11643,13 +11647,13 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state, struct drm_i915_private *dev_priv = dev->dev_private; struct intel_plane_state *old_plane_state = to_intel_plane_state(plane->state); - int idx = intel_crtc->base.base.id, ret; int i = drm_plane_index(plane); bool mode_changed = needs_modeset(crtc_state); bool was_crtc_enabled = crtc->state->active; bool is_crtc_enabled = crtc_state->active; bool turn_off, turn_on, visible, was_visible; struct drm_framebuffer *fb = plane_state->fb; + int ret; if (crtc_state && INTEL_INFO(dev)->gen >= 9 && plane->type != DRM_PLANE_TYPE_CURSOR) { @@ -11675,7 +11679,9 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state, turn_off = was_visible && (!visible || mode_changed); turn_on = visible && (!was_visible || mode_changed); - DRM_DEBUG_ATOMIC("[CRTC:%i] has [PLANE:%i] with fb %i\n", idx, + DRM_DEBUG_ATOMIC("[CRTC:%d:%s]
[PATCH 2/6] drm: Add plane->name and use it in debug prints
From: Ville SyrjäläSigned-off-by: Ville Syrjälä --- drivers/gpu/drm/drm_atomic.c| 12 ++-- drivers/gpu/drm/drm_atomic_helper.c | 4 ++-- drivers/gpu/drm/drm_crtc.c | 3 +++ include/drm/drm_crtc.h | 2 ++ 4 files changed, 13 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 2944655..df84060 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -543,8 +543,8 @@ drm_atomic_get_plane_state(struct drm_atomic_state *state, state->planes[index] = plane; plane_state->state = state; - DRM_DEBUG_ATOMIC("Added [PLANE:%d] %p state to %p\n", -plane->base.id, plane_state, state); + DRM_DEBUG_ATOMIC("Added [PLANE:%d:%s] %p state to %p\n", +plane->base.id, plane->name, plane_state, state); if (plane_state->crtc) { struct drm_crtc_state *crtc_state; @@ -755,8 +755,8 @@ static int drm_atomic_plane_check(struct drm_plane *plane, } if (plane_switching_crtc(state->state, plane, state)) { - DRM_DEBUG_ATOMIC("[PLANE:%d] switching CRTC directly\n", -plane->base.id); + DRM_DEBUG_ATOMIC("[PLANE:%d:%s] switching CRTC directly\n", +plane->base.id, plane->name); return -EINVAL; } @@ -1229,8 +1229,8 @@ int drm_atomic_check_only(struct drm_atomic_state *state) for_each_plane_in_state(state, plane, plane_state, i) { ret = drm_atomic_plane_check(plane, plane_state); if (ret) { - DRM_DEBUG_ATOMIC("[PLANE:%d] atomic core check failed\n", -plane->base.id); + DRM_DEBUG_ATOMIC("[PLANE:%d:%s] atomic core check failed\n", +plane->base.id, plane->name); return ret; } } diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index fc90af50..387d95c 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -503,8 +503,8 @@ drm_atomic_helper_check_planes(struct drm_device *dev, ret = funcs->atomic_check(plane, plane_state); if (ret) { - DRM_DEBUG_ATOMIC("[PLANE:%d] atomic driver check failed\n", -plane->base.id); + DRM_DEBUG_ATOMIC("[PLANE:%d:%s] atomic driver check failed\n", +plane->base.id, plane->name); return ret; } } diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index ea00a69..8dc4052 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -1174,6 +1174,9 @@ int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane, drm_modeset_lock_init(>mutex); + if (!plane->name) + plane->name = ""; + plane->base.properties = >properties; plane->dev = dev; plane->funcs = funcs; diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index a8279b4..a08e256 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -848,6 +848,8 @@ struct drm_plane { struct drm_device *dev; struct list_head head; + char *name; + struct drm_modeset_lock mutex; struct drm_mode_object base; -- 2.4.10
[PATCH 1/6] drm: Add crtc->name and use it in debug messages
From: Ville SyrjäläSigned-off-by: Ville Syrjälä --- drivers/gpu/drm/drm_atomic.c| 41 ++- drivers/gpu/drm/drm_atomic_helper.c | 56 +++-- drivers/gpu/drm/drm_crtc.c | 8 -- drivers/gpu/drm/drm_crtc_helper.c | 24 include/drm/drm_crtc.h | 2 ++ 5 files changed, 71 insertions(+), 60 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 7bb3845..2944655 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -288,8 +288,8 @@ drm_atomic_get_crtc_state(struct drm_atomic_state *state, state->crtcs[index] = crtc; crtc_state->state = state; - DRM_DEBUG_ATOMIC("Added [CRTC:%d] %p state to %p\n", -crtc->base.id, crtc_state, state); + DRM_DEBUG_ATOMIC("Added [CRTC:%d:%s] %p state to %p\n", +crtc->base.id, crtc->name, crtc_state, state); return crtc_state; } @@ -480,8 +480,8 @@ static int drm_atomic_crtc_check(struct drm_crtc *crtc, */ if (state->active && !state->enable) { - DRM_DEBUG_ATOMIC("[CRTC:%d] active without enabled\n", -crtc->base.id); + DRM_DEBUG_ATOMIC("[CRTC:%d:%s] active without enabled\n", +crtc->base.id, crtc->name); return -EINVAL; } @@ -490,15 +490,15 @@ static int drm_atomic_crtc_check(struct drm_crtc *crtc, * 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); + DRM_DEBUG_ATOMIC("[CRTC:%d:%s] enabled without mode blob\n", +crtc->base.id, crtc->name); 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); + DRM_DEBUG_ATOMIC("[CRTC:%d:%s] disabled with mode blob\n", +crtc->base.id, crtc->name); return -EINVAL; } @@ -980,8 +980,8 @@ drm_atomic_set_crtc_for_plane(struct drm_plane_state *plane_state, } if (crtc) - DRM_DEBUG_ATOMIC("Link plane state %p to [CRTC:%d]\n", -plane_state, crtc->base.id); + DRM_DEBUG_ATOMIC("Link plane state %p to [CRTC:%d:%s]\n", +plane_state, crtc->base.id, crtc->name); else DRM_DEBUG_ATOMIC("Link plane state %p to [NOCRTC]\n", plane_state); @@ -1048,8 +1048,8 @@ drm_atomic_set_crtc_for_connector(struct drm_connector_state *conn_state, conn_state->crtc = crtc; if (crtc) - DRM_DEBUG_ATOMIC("Link connector state %p to [CRTC:%d]\n", -conn_state, crtc->base.id); + DRM_DEBUG_ATOMIC("Link connector state %p to [CRTC:%d:%s]\n", +conn_state, crtc->base.id, crtc->name); else DRM_DEBUG_ATOMIC("Link connector state %p to [NOCRTC]\n", conn_state); @@ -1088,8 +1088,8 @@ drm_atomic_add_affected_connectors(struct drm_atomic_state *state, if (ret) return ret; - DRM_DEBUG_ATOMIC("Adding all current connectors for [CRTC:%d] to %p\n", -crtc->base.id, state); + DRM_DEBUG_ATOMIC("Adding all current connectors for [CRTC:%d:%s] to %p\n", +crtc->base.id, crtc->name, state); /* * Changed connectors are already in @state, so only need to look at the @@ -1169,8 +1169,9 @@ drm_atomic_connectors_for_crtc(struct drm_atomic_state *state, num_connected_connectors++; } - DRM_DEBUG_ATOMIC("State %p has %i connectors for [CRTC:%d]\n", -state, num_connected_connectors, crtc->base.id); + DRM_DEBUG_ATOMIC("State %p has %i connectors for [CRTC:%d:%s]\n", +state, num_connected_connectors, +crtc->base.id, crtc->name); return num_connected_connectors; } @@ -1237,8 +1238,8 @@ int drm_atomic_check_only(struct drm_atomic_state *state) for_each_crtc_in_state(state, crtc, crtc_state, i) { ret = drm_atomic_crtc_check(crtc, crtc_state); if (ret) { - DRM_DEBUG_ATOMIC("[CRTC:%d] atomic core check failed\n", -crtc->base.id); +
[PATCH 0/6] drm: Give crtcs and planes actual names
From: Ville SyrjäläI got sick and tired of staring at the line noise produced by drm.debug=0x1e, so I decided to give the crtcs and planes human readable names. Each driver can give whatever names it wants, and for i915 I gave something that makes some sense w.r.t. to the spec. The only magic gotcha here is that if the driver dynamically allocates the name, it must be careful around drm_{crtc,plane}_cleanup() cause those guys just memset the entire structure to 0. I didn't want to put the kfree() into the cleanup functions to avoid having to kstrdup("") in the fallback case or forcing drivers to always use a dynamic allocation. Ville Syrjälä (6): drm: Add crtc->name and use it in debug messages drm: Add plane->name and use it in debug prints drm/i915: Use crtc->name in debug messages drm/i915: Use plane->name in debug prints drm/i915: Set crtc->name to "pipe A", "pipe B", etc. drm/i915: Give meaningful names to all the planes drivers/gpu/drm/drm_atomic.c | 53 --- drivers/gpu/drm/drm_atomic_helper.c | 60 + drivers/gpu/drm/drm_crtc.c | 11 ++- drivers/gpu/drm/drm_crtc_helper.c| 24 --- drivers/gpu/drm/i915/intel_display.c | 127 +++ drivers/gpu/drm/i915/intel_fbdev.c | 5 +- drivers/gpu/drm/i915/intel_sprite.c | 14 include/drm/drm_crtc.h | 4 ++ 8 files changed, 185 insertions(+), 113 deletions(-) -- 2.4.10
drm: Bogus WARN() in drm_atomic_helper_update_legacy_modeset_state() ?
On 2015å¹´11æ12æ¥ 18:36, Liviu Dudau wrote: > On Thu, Nov 12, 2015 at 04:32:33PM +0800, Mark yao wrote: >> On 2015å¹´11æ10æ¥ 23:01, Liviu Dudau wrote: >> >> Hello, >> >> When booting my Juno board with the HDLCD driver that I have converted to >> atomic operations I'm getting the following warning: >> >> [ cut here ] >> WARNING: at >> /work/repositories/kernel/drivers/gpu/drm/drm_atomic_helper.c:674 >> Modules linked in: hdlcd(+) clk_scpi >> >> CPU: 3 PID: 1375 Comm: systemd-udevd Not tainted 4.3.0-next-20151109+ #5 >> Hardware name: ARM Juno development board (r0) (DT) >> task: ffc974888b00 ti: ffc9755dc000 task.ti: ffc9755dc000 >> PC is at drm_atomic_helper_update_legacy_modeset_state+0x204/0x20c >> LR is at drm_atomic_helper_commit_modeset_disables+0x1c0/0x394 >> pc : [] lr : [] pstate: 2145 >> sp : ffc9755df430 >> x29: ffc9755df430 x28: ffc975703600 >> x27: x26: ffc976253960 >> x25: ffc976254040 x24: ffc000819000 >> x23: ffc000689ea0 x22: ffc976251800 >> x21: ffc976251800 x20: >> x19: ffc9766b1f80 x18: 715fe015 >> x17: 007fb4b855b0 x16: 0220 >> x15: 0001 x14: 0ffe >> x13: 0008 x12: 0101010101010101 >> x11: ffc000964000 x10: ffc0009d2000 >> x9 : x8 : ffc97ff5f700 >> x7 : ffc97566cb80 x6 : ffc9766b1700 >> x5 : ffc975665100 x4 : >> x3 : ffc976253960 x2 : ffc97566cd00 >> x1 : ffc976253900 x0 : >> >> ---[ end trace 9fe289f798e7178e ]--- >> Call trace: >> [] >> drm_atomic_helper_update_legacy_modeset_state+0x204/0x20c >> [] drm_atomic_helper_commit_modeset_disables+0x1c0/0x394 >> [] drm_atomic_helper_commit+0xe0/0x150 >> [] drm_atomic_commit+0x40/0x6c >> [] restore_fbdev_mode+0x294/0x2d4 >> [] drm_fb_helper_restore_fbdev_mode_unlocked+0x34/0x8c >> [] drm_fb_helper_set_par+0x2c/0x58 >> [] fbcon_init+0x4d4/0x534 >> [] visual_init+0xac/0x104 >> [] do_bind_con_driver+0x16c/0x398 >> [] do_take_over_console+0xd8/0x1f4 >> [] do_fbcon_takeover+0x74/0xf8 >> [] fbcon_event_notify+0x8a4/0x8f8 >> [] notifier_call_chain+0x4c/0x88 >> [] __blocking_notifier_call_chain+0x44/0x74 >> [] blocking_notifier_call_chain+0x14/0x1c >> [] fb_notifier_call_chain+0x1c/0x24 >> [] register_framebuffer+0x1c0/0x2ac >> [] drm_fb_helper_initial_config+0x25c/0x3ec >> [] drm_fbdev_cma_init+0x98/0x134 >> [] hdlcd_drm_bind+0x180/0x498 [hdlcd] >> [] try_to_bring_up_master.part.5+0xd4/0x118 >> [] component_master_add_with_match+0xc4/0x148 >> [] component_master_add+0x10/0x18 >> [] hdlcd_probe+0x14/0x28 [hdlcd] >> [] platform_drv_probe+0x54/0xc0 >> [] driver_probe_device+0x1ec/0x2e8 >> [] __driver_attach+0x9c/0xa0 >> [] bus_for_each_dev+0x58/0x98 >> [] driver_attach+0x20/0x28 >> [] bus_add_driver+0x1c8/0x22c >> [] driver_register+0x68/0x108 >> [] __platform_driver_register+0x4c/0x54 >> [] hdlcd_init+0x18/0x30 [hdlcd] >> [] do_one_initcall+0x90/0x1a8 >> [] do_init_module+0x60/0x1c8 >> [] load_module+0x1554/0x1c98 >> [] SyS_finit_module+0x7c/0x88 >> [] el0_svc_naked+0x24/0x28 >> >> >> The line that triggers the warning is: >> >> 674:WARN_ON(!connector->encoder->crtc); >> >> As far as I can see the encoder->crtc value is being set to a non-NULL >> value only >> in two places: >>- in drm_atomic_helper_update_legacy_modeset_state() after WARN_ON() >> encoder->crtc = connector->state->crtc; >>- in drm_crtc_helper_set_config(drm_mode_set *set): >> encoder->crtc = new_crtc; >> >> Nothing in the call path from drm_atomic_commit() calls >> crtc_funcs->set_config() or >> drm_crtc_helper_set_config() directly, so the question is if this WARN() >> is actually >> valid. >> >> Call path from drm_atomic_commit: >> >> drm_atomic_helper_commit() >> - drm_atomic_helper_prepare_planes() >> - drm_atomic_helper_swap_state() >> - drm_atomic_helper_commit_modeset_disables() >>- disable_outputs() >>- drm_atomic_helper_update_legacy_modeset_state() >> - WARN_ON(!connector->encoder->crtc) >> >> Best regards, >> Liviu >> >> >> Hi Liviu >> I solved this problem by following change, you can check if your >> driver do the same things: >>drivers/gpu/drm/bridge/dw_hdmi.c: >> - hdmi->connector.encoder = encoder; >> +// hdmi->connector.encoder = encoder; >> >>drm_mode_connector_attach_encoder(>connector, >> encoder); >> >> I found some other drivers set connector.encoder before >> drm_mode_connector_attach_encoder, some are not. >> >> drm_mode_connector_attach_encoder already describe the link of >> connector and encoder, >> so I think
[Bug 92923] SGPR spilling
https://bugs.freedesktop.org/show_bug.cgi?id=92923 --- Comment #1 from pat --- Created attachment 119607 --> https://bugs.freedesktop.org/attachment.cgi?id=119607=edit backtrace from game log file -- 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/20151112/719bd96a/attachment.html>
[Bug 92923] SGPR spilling
https://bugs.freedesktop.org/show_bug.cgi?id=92923 Bug ID: 92923 Summary: SGPR spilling Product: Mesa Version: 11.0 Hardware: x86-64 (AMD64) OS: Linux (All) Status: NEW Severity: normal Priority: medium Component: Drivers/Gallium/radeonsi Assignee: dri-devel at lists.freedesktop.org Reporter: freedesktop.org at psycho3d.de QA Contact: dri-devel at lists.freedesktop.org Created attachment 119606 --> https://bugs.freedesktop.org/attachment.cgi?id=119606=edit stacktrace from game log file "LLVM triggered Diagnostic Handler: Ran out of VGPRs for spilling SGPR" The game Planet Explorers crashes after a few minutes, the log file is flooded with this error message. Additional error messages: radeon_llvm_compile: Processing Diag Flag LLVM failed to compile shader EE si_state_shaders.c:647 si_shader_select - Failed to build shader variant (type=1) 1 The problem may have started with mesa 11.0 but I'm not sure, I don't play it very often. The game runs on unity 5. Attaching backtrace and stacktrace from the log file. There is also a memory map I could attach. OpenGL info from the log file: Version: OpenGL 3.0 [3.0 Mesa 11.0.5] Renderer: Gallium 0.4 on AMD PITCAIRN (DRM 2.43.0, LLVM 3.7.0) Vendor: X.Org VRAM: 2048 MB Hardware is a Radeon HD 7870 -- 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/20151112/226d5448/attachment-0001.html>
[PATCH i915 v3 2/2] i915: wait for fence in prepare_plane_fb
In intel_prepare_plane_fb, if fb is backed by dma-buf, wait for fence. Signed-off-by: Alex Goins --- drivers/gpu/drm/i915/intel_display.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index acec108a..36c558a 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -13345,6 +13345,13 @@ intel_prepare_plane_fb(struct drm_plane *plane, if (!obj) return 0; + /* For framebuffer backed by dmabuf, wait for fence */ + if (obj->base.dma_buf) { + reservation_object_wait_timeout_rcu( + obj->base.dma_buf->resv, + true, false, msecs_to_jiffies(96)); + } + mutex_lock(>struct_mutex); if (plane->type == DRM_PLANE_TYPE_CURSOR && -- 1.9.1
[PATCH i915 v3 1/2] i915: wait for fences in mmio_flip()
If a buffer is backed by dmabuf, wait on its reservation object's fences before flipping. Signed-off-by: Alex Goins --- drivers/gpu/drm/i915/intel_display.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index b2270d5..acec108a 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -44,6 +44,8 @@ #include #include #include +#include +#include /* Primary plane formats for gen <= 3 */ static const uint32_t i8xx_primary_formats[] = { @@ -11170,10 +11172,19 @@ static void ilk_do_mmio_flip(struct intel_crtc *intel_crtc) static void intel_do_mmio_flip(struct intel_crtc *intel_crtc) { struct drm_device *dev = intel_crtc->base.dev; + struct drm_i915_gem_object *pending_flip_obj = + intel_crtc->unpin_work->pending_flip_obj; u32 start_vbl_count; intel_mark_page_flip_active(intel_crtc); + /* For framebuffer backed by dmabuf, wait for fence */ + if (pending_flip_obj->base.dma_buf) { + reservation_object_wait_timeout_rcu( + pending_flip_obj->base.dma_buf->resv, + true, false, msecs_to_jiffies(96)); + } + intel_pipe_update_start(intel_crtc, _vbl_count); if (INTEL_INFO(dev)->gen >= 9) -- 1.9.1
[PATCH i915 v3 0/2] PRIME Synchronization
Hello all, For a while now, I've been working to fix tearing with PRIME. This is my third version of the solution, revised according to Chris Wilson and Maarten Lankhorst's suggestions to remove the unnecessary lock on object_name_lock and to move fence waiting from intel_atomic_commit() to intel_prepare_plane_fb(). Repeat of overview below: I have two patches, one that implements fencing for i915's legacy mmio_flip path, and one for atomic modesetting for futureproofing. Currently the mmio_flip path is the one ultimately used by the X patches, due to the lack of asynchronous atomic modesetting support in i915. With my synchronization patches to X, it is possible to export two shared buffers per crtc instead of just one. The sink driver uses the legacy drmModePageFlip() to flip between the buffers, as the rest of the driver has yet to be ported to atomics. In the pageflip/vblank event handler, the sink driver requests a present from the source using the new X ABI function pScreen->PresentTrackedFlippingPixmap(). If the call returns successfully, it uses drmModePageFlip() to flip to the updated buffer, otherwise it waits until the next vblank and tries again. When the source driver presents on a given buffer, it first attaches a fence. The source driver is responsible for either using software signaling or hardware semaphore-backed fences to ensure the fence is signaled when the present is finished. If the sink's DRM driver implements fencing in the flipping path, it will guarantee that that flip won't occur until the present has finished. This means that DRM drivers that don't implement fencing in their flipping paths won't be able to guarantee 100% tear-free PRIME with my X patches. However, the good news is that even without fencing, tearing is rare. Generally presenting finishes before the next vblank, so there is no need to wait on the fence. The X patches are a drastic improvement with or without fencing, but the fencing is nonetheless important to guarantee tear-free under all conditions. To give some greater context, I've uploaded my branches for DRM and the X server to Github. I'll move forward with upstreaming the X changes if and when these DRM patches go in. DRM Tree:https://github.com/GoinsWithTheWind/drm-prime-sync X Tree: https://github.com/GoinsWithTheWind/xserver-prime-sync (branch agoins-prime-v3) Thanks, Alex @ NVIDIA Linux Driver Team Alex Goins (2): i915: wait for fences in mmio_flip() i915: wait for fence in prepare_plane_fb drivers/gpu/drm/i915/intel_display.c | 18 ++ 1 file changed, 18 insertions(+) -- 1.9.1
[PATCH 6/6] drm/i915: Give meaningful names to all the planes
Hi Ville, On 12 November 2015 at 16:52, wrote: > From: Ville Syrjälä > > Let's name our planes in a way that makes sense wrt. the spec: > - skl+ -> "plane 1A", "plane 2A", "plane 1C", "cursor A" etc. > - g4x+ -> "primary A", "primary B", "sprite A", "cursor C" etc. > - pre-g4x -> "plane A", "cursor B" etc. > > Signed-off-by: Ville Syrjälä > --- > drivers/gpu/drm/i915/intel_display.c | 35 +-- > drivers/gpu/drm/i915/intel_sprite.c | 14 ++ > 2 files changed, 47 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 2b5e81a..82b2f58 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -13788,7 +13788,15 @@ static void intel_finish_crtc_commit(struct drm_crtc > *crtc, > void intel_plane_destroy(struct drm_plane *plane) > { > struct intel_plane *intel_plane = to_intel_plane(plane); > + char *name; > + > + /* > +* drm_plane_cleanup() zeroes the structure, so > +* need an extra dance to avoid leaking the name. > +*/ > + name = plane->name; > drm_plane_cleanup(plane); > + kfree(name); > kfree(intel_plane); > } > > @@ -13838,6 +13846,21 @@ static struct drm_plane > *intel_primary_plane_create(struct drm_device *dev, > if (HAS_FBC(dev) && INTEL_INFO(dev)->gen < 4) > primary->plane = !pipe; > > + if (INTEL_INFO(dev)->gen >= 9) > + primary->base.name = kasprintf(GFP_KERNEL, "plane 1%c", > + pipe_name(pipe)); > + else if (INTEL_INFO(dev)->gen >= 5 || IS_G4X(dev)) > + primary->base.name = kasprintf(GFP_KERNEL, "primary %c", > + pipe_name(pipe)); > + else > + primary->base.name = kasprintf(GFP_KERNEL, "plane %c", > + plane_name(primary->plane)); > + if (!primary->base.name) { > + kfree(state); > + kfree(primary); > + return NULL; Worth adding a label and doing all the teardown there ? (same goes for the rest of the patch) > + } > + > if (INTEL_INFO(dev)->gen >= 9) { > intel_primary_formats = skl_primary_formats; > num_formats = ARRAY_SIZE(skl_primary_formats); > @@ -13987,6 +14010,14 @@ static struct drm_plane > *intel_cursor_plane_create(struct drm_device *dev, > cursor->commit_plane = intel_commit_cursor_plane; > cursor->disable_plane = intel_disable_cursor_plane; > > + cursor->base.name = kasprintf(GFP_KERNEL, "cursor %c", > + pipe_name(pipe)); > + if (!cursor->base.name) { > + kfree(state); > + kfree(cursor); > + return NULL; > + } > + > drm_universal_plane_init(dev, >base, 0, > _plane_funcs, > intel_cursor_formats, > @@ -14113,9 +14144,9 @@ static void intel_crtc_init(struct drm_device *dev, > int pipe) > > fail: > if (primary) > - drm_plane_cleanup(primary); > + intel_plane_destroy(primary); > if (cursor) > - drm_plane_cleanup(cursor); > + intel_plane_destroy(cursor); Something feels strange here. We are either leaking memory before or we'll end up with double free after your patch. Worth checking/mentioning in the commit message ? Regards, Emil
[PATCH v9 15/17] drm: bridge: analogix/dp: try force hpd after plug in lookup failed
On Thu, Nov 12, 2015 at 09:27:21AM +0800, Yakir Yang wrote: > Hi Rob, > > On 11/12/2015 07:10 AM, Rob Herring wrote: > >On Fri, Oct 30, 2015 at 09:09:15AM +0800, Yakir Yang wrote: > >>Some edp screen do not have hpd signal, so we can't just return > >>failed when hpd plug in detect failed. > >> > >>This is an hardware property, so we need add a devicetree property > >>"analogix,need-force-hpd" to indicate this sutiation. > >> > >>Tested-by: Heiko Stuebner > >>Tested-by: Javier Martinez Canillas > >>Signed-off-by: Yakir Yang > >>--- > >I didn't find this one in v10. Did it get dropped? > > You are in my 'to' list, but I haven't send the whole v10 out, > most of them don't need update :) Okay, it's just gmail's inability to follow threading... > This series should be: > [v8 0/17 ...] > | [v8 1/17 ...] > | [v8 2/17 ...] > | [v8 3/17 ...] > | [...] > | [v8 15/17 ...] > | [v8 16/17 ...] > | [v8 17/17 ...] > | > | [v9 10/17 ...] > | [v9 15/17 ...] > | We are here > | [v9 09/17 ...] > | [v10 09/17 ...] > | Received an acked from you > >>diff --git > >>a/Documentation/devicetree/bindings/display/bridge/analogix_dp.txt > >>b/Documentation/devicetree/bindings/display/bridge/analogix_dp.txt > >>index 7659a7a..74f0e80 100644 > >>--- a/Documentation/devicetree/bindings/display/bridge/analogix_dp.txt > >>+++ b/Documentation/devicetree/bindings/display/bridge/analogix_dp.txt > >>@@ -22,6 +22,9 @@ Required properties for dp-controller: > >>from general PHY binding: Should be "dp". > >> Optional properties for dp-controller: > >>+ -analogix,need-force-hpd: > >>+ Indicate driver need force hpd when hpd detect failed, this > >>+ is used for some eDP screen which don't have hpd signal. > >This should be a generic property. > > This property would only need in some no-hpd case, it would be dropped if > panel keep the hotplug signal, that's why I thought it should be optional. I agree it is optional. I just mean drop the analogix and put in a common binding doc (i.e. create a display/bridge/common.txt). Needing to ignore HPD is probably a common problem. > I thought if we put this a property to generic property, then we must need > to define it in normal device node, not sure whether it is right :) Sorry, I don't follow. Rob
[RESEND] drm/rockchip: Fix module autoload for OF platform driver
From: Luis de BethencourtThis platform driver has a OF device ID table but the OF module alias information is not created so module autoloading won't work. Signed-off-by: Luis de Bethencourt --- Hi, This is a Resend of a patch from 20 Oct [0] I am adding Andrew Morton to the CC list as was recommended at the Korea Linux Forum. This patch adds the missing MODULE_DEVICE_TABLE() for OF to export that information so modules have the correct aliases built-in and autoloading works correctly. A longer explanation by Javier Canillas can be found here: https://lkml.org/lkml/2015/7/30/519 Thanks, Luis [0] https://lkml.org/lkml/2015/10/20/485 drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c index 5d8ae5e..5897851 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c @@ -374,6 +374,7 @@ static const struct of_device_id vop_driver_dt_match[] = { .data = _vop }, {}, }; +MODULE_DEVICE_TABLE(of, vop_driver_dt_match); static inline void vop_writel(struct vop *vop, uint32_t offset, uint32_t v) {
[RESEND] drm: imx: imx-tve: Fix module autoload for OF platform driver
From: Luis de BethencourtThis platform driver has a OF device ID table but the OF module alias information is not created so module autoloading won't work. Signed-off-by: Luis de Bethencourt --- Hi, This is a Resend of a patch from 20 Oct [0] I am adding Andrew Morton to the CC list as was recommended at the Korea Linux Forum. This patch adds the missing MODULE_DEVICE_TABLE() for OF to export that information so modules have the correct aliases built-in and autoloading works correctly. A longer explanation by Javier Canillas can be found here: https://lkml.org/lkml/2015/7/30/519 Thanks, Luis [0] https://lkml.org/lkml/2015/10/20/478 drivers/gpu/drm/imx/imx-tve.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/imx/imx-tve.c b/drivers/gpu/drm/imx/imx-tve.c index e671ad3..f959714 100644 --- a/drivers/gpu/drm/imx/imx-tve.c +++ b/drivers/gpu/drm/imx/imx-tve.c @@ -721,6 +721,7 @@ static const struct of_device_id imx_tve_dt_ids[] = { { .compatible = "fsl,imx53-tve", }, { /* sentinel */ } }; +MODULE_DEVICE_TABLE(of, imx_tve_dt_ids); static struct platform_driver imx_tve_driver = { .probe = imx_tve_probe,
drm: Bogus WARN() in drm_atomic_helper_update_legacy_modeset_state() ?
On Thu, Nov 12, 2015 at 01:57:11PM +, Liviu Dudau wrote: > On Thu, Nov 12, 2015 at 01:16:55PM +0100, Thierry Reding wrote: > > On Wed, Nov 11, 2015 at 04:09:42PM +, Liviu Dudau wrote: > > > On Tue, Nov 10, 2015 at 05:56:15PM +0100, Thierry Reding wrote: > > > > On Tue, Nov 10, 2015 at 03:01:03PM +, Liviu Dudau wrote: > > > > > Hello, > > > > > > > > > > When booting my Juno board with the HDLCD driver that I have > > > > > converted to > > > > > atomic operations I'm getting the following warning: > > > > > > > > Perhaps you can provide pointers to the source code, that might make it > > > > easier for people to spot what's going wrong. > > > > > > > > Thierry > > > > > > Hi Thierry, > > > > > > I have just pushed to the mailing lists my patch series. [1] [2] > > > > > > If you want to checkout the code I also have a branch here: > > > > > > git://git://linux-arm.org/linux-ld testing/hdlcd > > > > Okay, so if I understand correctly you're using the tda998x as encoder/ > > connector attached to your new HDLCD driver. I think the problem isn't > > so much that nothing has set up the CRTC for the encoder, but rather > > that the tda998x encoder tries to statically associate the encoder with > > the connector. While that might be correct in that it represents the > > hardware state (i.e. there is no way to physically route it anywhere > > else), the DRM logical state is that it's not connected until a complete > > pipeline has been set up, in which case a CRTC would have been assigned > > to the encoder. > > > > If your setup were working correctly you'd never reach the WARN_ON > > because the > > > > if (connector->encoder) { > > > > conditional (on line 673 in drivers/gpu/drm/drm_atomic_helper.c) would > > have evaluated to false already, since logically there'd be no encoder > > associated with the connector yet. > > Your analysis is spot on and matches my conditions. However ... > > > > > Does the patch below help? Let me know and I can produce a proper patch. > > > > Thierry > > > > --- >8 --- > > diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c > > b/drivers/gpu/drm/i2c/tda998x_drv.c > > index 896b6aaf8c4d..8b0a402190eb 100644 > > --- a/drivers/gpu/drm/i2c/tda998x_drv.c > > +++ b/drivers/gpu/drm/i2c/tda998x_drv.c > > @@ -1453,7 +1453,6 @@ static int tda998x_bind(struct device *dev, struct > > device *master, void *data) > > if (ret) > > goto err_sysfs; > > > > - priv->connector.encoder = >encoder; > > while this patch helps (no WARN() printed) I'm not sure this is the right fix. > TDA998x is also used by armada DRM driver which has not been converted (yet) > to > atomic modesetting. And judging by the code and comment of > drm_connector_get_encoder() > in drm_crtc.c, having access to the encoder through the connector is the > non-atomic > way of doing things: > > static struct drm_encoder *drm_connector_get_encoder(struct drm_connector > *connector) > { > /* For atomic drivers only state objects are synchronously updated and >* protected by modeset locks, so check those first. */ > if (connector->state) > return connector->state->best_encoder; > return connector->encoder; > } > > To me, it looks like the WARN() is bogus when the atomic modesetting is used > in > drm_atomic_helper_update_legacy_modeset_state(). Either we acknowledge that > the "legacy" > code sets the connector in the encoder structure before complete pipeline is > setup > and we remove the WARN(), or we find a better way of detecting that some sort > of mixed > support is in place (i.e. atomic and non-atomic aware code running) and we > clean up > in a way that is compatible with both worlds. I think the problem you're seeing here is specifically caused by drivers setting up the connector->encoder explicitly. There should be no reason to do that. The DRM core will automatically do that when setting up a default configuration, or as a result of userspace setting up the wanted configuration. You're also likely only seeing this the first time around and subsequent calls will not trigger this anymore because at that point the core will have reset connector->encoder to NULL as part of tearing down the pipeline. To corroborate that, the Tegra driver doesn't trigger this WARN_ON() and also never sets up the connector->encoder manually. The same is probably true for many other drivers that have already been converted. That said, from a brief look it seems like many more drivers get this wrong and may run into this WARN when they get converted to atomic. Thierry -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151112/0ed8a97c/attachment.sig>
drm: Bogus WARN() in drm_atomic_helper_update_legacy_modeset_state() ?
init_module+0x60/0x1c8 > > > >> [] load_module+0x1554/0x1c98 > > > >> [] SyS_finit_module+0x7c/0x88 > > > >> [] el0_svc_naked+0x24/0x28 > > > >> > > > >> > > > >> The line that triggers the warning is: > > > >> > > > >> 674:WARN_ON(!connector->encoder->crtc); > > > >> > > > >> As far as I can see the encoder->crtc value is being set to a > > > >> non-NULL value only > > > >> in two places: > > > >> - in drm_atomic_helper_update_legacy_modeset_state() after WARN_ON() > > > >> encoder->crtc = connector->state->crtc; > > > >> - in drm_crtc_helper_set_config(drm_mode_set *set): > > > >> encoder->crtc = new_crtc; > > > >> > > > >> Nothing in the call path from drm_atomic_commit() calls > > > >> crtc_funcs->set_config() or > > > >> drm_crtc_helper_set_config() directly, so the question is if this > > > >> WARN() is actually > > > >> valid. > > > >> > > > >> Call path from drm_atomic_commit: > > > >> > > > >> drm_atomic_helper_commit() > > > >>- drm_atomic_helper_prepare_planes() > > > >>- drm_atomic_helper_swap_state() > > > >>- drm_atomic_helper_commit_modeset_disables() > > > >> - disable_outputs() > > > >> - drm_atomic_helper_update_legacy_modeset_state() > > > >> - WARN_ON(!connector->encoder->crtc) > > > >> > > > >> Best regards, > > > >> Liviu > > > >> > > > >> > > > >>Hi Liviu > > > >> I solved this problem by following change, you can check if > > > >> your driver do the same things: > > > >> drivers/gpu/drm/bridge/dw_hdmi.c: > > > >> - hdmi->connector.encoder = encoder; > > > >> +// hdmi->connector.encoder = encoder; > > > >> > > > >> > > > >> drm_mode_connector_attach_encoder(>connector, encoder); > > > >> > > > >> I found some other drivers set connector.encoder before > > > >> drm_mode_connector_attach_encoder, some are not. > > > >> > > > >> drm_mode_connector_attach_encoder already describe the link of > > > >> connector and encoder, > > > >>so I think "connector.encoder = encoder" is not needed, is that > > > >> right? > > > >I'll have a look, thanks for pointing this. However, my setup uses the > > > >tda998x driver for encoder, so I will > > > >have to go look there rather than in my driver. > > > > > > > >Best regards, > > > >Liviu > > > > > > > >>Thanks. > > > >> > > > >> -- > > > >> ï¼ark Yao > > > Hi Liviu > > > drivers/gpu/drm/i2c/tda998x_drv.c do the same things: > > > priv->connector.encoder = >encoder; > > > drm_mode_connector_attach_encoder(>connector, >encoder); > > > > > > I believe must be same problem. > > > > Oh, I hadn't noticed this subthread, but you came to the same conclusion > > as I did. I have the below local change, which I think might be good to > > have given that there are at least two drivers that got this wrong. > > > > Thierry > > > > --- >8 --- > > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > > index 24c5434abd1c..56d53106b32d 100644 > > --- a/drivers/gpu/drm/drm_crtc.c > > +++ b/drivers/gpu/drm/drm_crtc.c > > @@ -4983,6 +4983,20 @@ int drm_mode_connector_attach_encoder(struct > > drm_connector *connector, > > { > > int i; > > > > + /* > > +* In the past, drivers have attempted to model the static association > > +* of connector to encoder in simple connector/encoder devices using a > > +* direct assignment of connector->encoder = encoder. This connection > > +* is a logical one and the responsibility of the core, so drivers are > > +* expected not to mess with this. > > +* > > +* Note that the error return should've been enough here, but a large > > +* majority of drivers ignores the return value, so add in a big WARN > > +* to get people's attention. > > +*/ > > + if (WARN_ON(connector->encoder)) > > + return -EINVAL; > > + > > for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) { > > if (connector->encoder_ids[i] == 0) { > > connector->encoder_ids[i] = encoder->base.id; > > Hi Thierry, > > This patch together with your tda998x proposed patch should solve > nicely the problem we are detecting, as long as no one calls > drm_connector_get_encoder() before drm_crtc_helper_set_config(). The only caller of drm_connector_get_encoder() is drm_mode_getconnector() and that handles the NULL case gracefully, so I don't think there are any issues. Thierry -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151112/0f5fd474/attachment.sig>
drm: Bogus WARN() in drm_atomic_helper_update_legacy_modeset_state() ?
On Thu, Nov 12, 2015 at 05:12:33PM +0100, Thierry Reding wrote: > On Thu, Nov 12, 2015 at 02:03:35PM +, Liviu Dudau wrote: > > On Thu, Nov 12, 2015 at 02:34:11PM +0100, Thierry Reding wrote: > > > On Thu, Nov 12, 2015 at 06:49:38PM +0800, Mark yao wrote: > > > > On 2015å¹´11æ12æ¥ 18:36, Liviu Dudau wrote: > > > > >On Thu, Nov 12, 2015 at 04:32:33PM +0800, Mark yao wrote: > > > > >>On 2015å¹´11æ10æ¥ 23:01, Liviu Dudau wrote: > > > > >> > > > > >> Hello, > > > > >> > > > > >> When booting my Juno board with the HDLCD driver that I have > > > > >> converted to > > > > >> atomic operations I'm getting the following warning: > > > > >> > > > > >> [ cut here ] > > > > >> WARNING: at > > > > >> /work/repositories/kernel/drivers/gpu/drm/drm_atomic_helper.c:674 > > > > >> Modules linked in: hdlcd(+) clk_scpi > > > > >> > > > > >> CPU: 3 PID: 1375 Comm: systemd-udevd Not tainted > > > > >> 4.3.0-next-20151109+ #5 > > > > >> Hardware name: ARM Juno development board (r0) (DT) > > > > >> task: ffc974888b00 ti: ffc9755dc000 task.ti: > > > > >> ffc9755dc000 > > > > >> PC is at drm_atomic_helper_update_legacy_modeset_state+0x204/0x20c > > > > >> LR is at drm_atomic_helper_commit_modeset_disables+0x1c0/0x394 > > > > >> pc : [] lr : [] pstate: 2145 > > > > >> sp : ffc9755df430 > > > > >> x29: ffc9755df430 x28: ffc975703600 > > > > >> x27: x26: ffc976253960 > > > > >> x25: ffc976254040 x24: ffc000819000 > > > > >> x23: ffc000689ea0 x22: ffc976251800 > > > > >> x21: ffc976251800 x20: > > > > >> x19: ffc9766b1f80 x18: 715fe015 > > > > >> x17: 007fb4b855b0 x16: 0220 > > > > >> x15: 0001 x14: 0ffe > > > > >> x13: 0008 x12: 0101010101010101 > > > > >> x11: ffc000964000 x10: ffc0009d2000 > > > > >> x9 : x8 : ffc97ff5f700 > > > > >> x7 : ffc97566cb80 x6 : ffc9766b1700 > > > > >> x5 : ffc975665100 x4 : > > > > >> x3 : ffc976253960 x2 : ffc97566cd00 > > > > >> x1 : ffc976253900 x0 : > > > > >> > > > > >> ---[ end trace 9fe289f798e7178e ]--- > > > > >> Call trace: > > > > >> [] > > > > >> drm_atomic_helper_update_legacy_modeset_state+0x204/0x20c > > > > >> [] > > > > >> drm_atomic_helper_commit_modeset_disables+0x1c0/0x394 > > > > >> [] drm_atomic_helper_commit+0xe0/0x150 > > > > >> [] drm_atomic_commit+0x40/0x6c > > > > >> [] restore_fbdev_mode+0x294/0x2d4 > > > > >> [] > > > > >> drm_fb_helper_restore_fbdev_mode_unlocked+0x34/0x8c > > > > >> [] drm_fb_helper_set_par+0x2c/0x58 > > > > >> [] fbcon_init+0x4d4/0x534 > > > > >> [] visual_init+0xac/0x104 > > > > >> [] do_bind_con_driver+0x16c/0x398 > > > > >> [] do_take_over_console+0xd8/0x1f4 > > > > >> [] do_fbcon_takeover+0x74/0xf8 > > > > >> [] fbcon_event_notify+0x8a4/0x8f8 > > > > >> [] notifier_call_chain+0x4c/0x88 > > > > >> [] __blocking_notifier_call_chain+0x44/0x74 > > > > >> [] blocking_notifier_call_chain+0x14/0x1c > > > > >> [] fb_notifier_call_chain+0x1c/0x24 > > > > >> [] register_framebuffer+0x1c0/0x2ac > > > > >> [] drm_fb_helper_initial_config+0x25c/0x3ec > > > > >> [] drm_fbdev_cma_init+0x98/0x134 > > > > >> [] hdlcd_drm_bind+0x180/0x498 [hdlcd] > > > > >> [] try_to_bring_up_master.part.5+0xd4/0x118 > > > > >> [] component_master_add_with_match+0xc4/0x148 > > > > >> [] component_master_add+0x10/0x18 > > > > >> [] hdlcd_probe+0x14/0x28 [hdlcd] > > > > >> [] platform_drv_probe+0x54/0xc0 > > > > >> [] driver_probe_device+0x1ec/0x2e8 > > > > >> [] __driver_attach+0x9c/0xa0 > > > > >> [] bus_for_each_dev+0x58/0x98 > > > > >> [] driver_attach+0x20/0x28 > > > > >> [] bus_add_driver+0x1c8/0x22c > > > > >> [] driver_register+0x68/0x108 > > > > >> [] __platform_driver_register+0x4c/0x54 > > > > >> [] hdlcd_init+0x18/0x30 [hdlcd] > > > > >> [] do_one_initcall+0x90/0x1a8 > > > > >> [] do_init_module+0x60/0x1c8 > > > > >> [] load_module+0x1554/0x1c98 > > > > >> [] SyS_finit_module+0x7c/0x88 > > > > >> [] el0_svc_naked+0x24/0x28 > > > > >> > > > > >> > > > > >> The line that triggers the warning is: > > > > >> > > > > >> 674:WARN_ON(!connector->encoder->crtc); > > > > >> > > > > >> As far as I can see the encoder->crtc value is being set to a > > > > >> non-NULL value only > > > > >> in two places: > > > > >> - in drm_atomic_helper_update_legacy_modeset_state() after > > > > >> WARN_ON() > > > > >> encoder->crtc = connector->state->crtc; > > > > >> - in drm_crtc_helper_set_config(drm_mode_set *set): > > > > >> encoder->crtc = new_crtc; > > > > >> > > > > >> Nothing in the call path from drm_atomic_commit() calls > > > > >> crtc_funcs->set_config() or > > > > >> drm_crtc_helper_set_config() directly, so the question is if this > > > > >> WARN() is actually > > > > >>
drm: Bogus WARN() in drm_atomic_helper_update_legacy_modeset_state() ?
On 2015å¹´11æ10æ¥ 23:01, Liviu Dudau wrote: > Hello, > > When booting my Juno board with the HDLCD driver that I have converted to > atomic operations I'm getting the following warning: > > [ cut here ] > WARNING: at /work/repositories/kernel/drivers/gpu/drm/drm_atomic_helper.c:674 > Modules linked in: hdlcd(+) clk_scpi > > CPU: 3 PID: 1375 Comm: systemd-udevd Not tainted 4.3.0-next-20151109+ #5 > Hardware name: ARM Juno development board (r0) (DT) > task: ffc974888b00 ti: ffc9755dc000 task.ti: ffc9755dc000 > PC is at drm_atomic_helper_update_legacy_modeset_state+0x204/0x20c > LR is at drm_atomic_helper_commit_modeset_disables+0x1c0/0x394 > pc : [] lr : [] pstate: 2145 > sp : ffc9755df430 > x29: ffc9755df430 x28: ffc975703600 > x27: x26: ffc976253960 > x25: ffc976254040 x24: ffc000819000 > x23: ffc000689ea0 x22: ffc976251800 > x21: ffc976251800 x20: > x19: ffc9766b1f80 x18: 715fe015 > x17: 007fb4b855b0 x16: 0220 > x15: 0001 x14: 0ffe > x13: 0008 x12: 0101010101010101 > x11: ffc000964000 x10: ffc0009d2000 > x9 : x8 : ffc97ff5f700 > x7 : ffc97566cb80 x6 : ffc9766b1700 > x5 : ffc975665100 x4 : > x3 : ffc976253960 x2 : ffc97566cd00 > x1 : ffc976253900 x0 : > > ---[ end trace 9fe289f798e7178e ]--- > Call trace: > [] drm_atomic_helper_update_legacy_modeset_state+0x204/0x20c > [] drm_atomic_helper_commit_modeset_disables+0x1c0/0x394 > [] drm_atomic_helper_commit+0xe0/0x150 > [] drm_atomic_commit+0x40/0x6c > [] restore_fbdev_mode+0x294/0x2d4 > [] drm_fb_helper_restore_fbdev_mode_unlocked+0x34/0x8c > [] drm_fb_helper_set_par+0x2c/0x58 > [] fbcon_init+0x4d4/0x534 > [] visual_init+0xac/0x104 > [] do_bind_con_driver+0x16c/0x398 > [] do_take_over_console+0xd8/0x1f4 > [] do_fbcon_takeover+0x74/0xf8 > [] fbcon_event_notify+0x8a4/0x8f8 > [] notifier_call_chain+0x4c/0x88 > [] __blocking_notifier_call_chain+0x44/0x74 > [] blocking_notifier_call_chain+0x14/0x1c > [] fb_notifier_call_chain+0x1c/0x24 > [] register_framebuffer+0x1c0/0x2ac > [] drm_fb_helper_initial_config+0x25c/0x3ec > [] drm_fbdev_cma_init+0x98/0x134 > [] hdlcd_drm_bind+0x180/0x498 [hdlcd] > [] try_to_bring_up_master.part.5+0xd4/0x118 > [] component_master_add_with_match+0xc4/0x148 > [] component_master_add+0x10/0x18 > [] hdlcd_probe+0x14/0x28 [hdlcd] > [] platform_drv_probe+0x54/0xc0 > [] driver_probe_device+0x1ec/0x2e8 > [] __driver_attach+0x9c/0xa0 > [] bus_for_each_dev+0x58/0x98 > [] driver_attach+0x20/0x28 > [] bus_add_driver+0x1c8/0x22c > [] driver_register+0x68/0x108 > [] __platform_driver_register+0x4c/0x54 > [] hdlcd_init+0x18/0x30 [hdlcd] > [] do_one_initcall+0x90/0x1a8 > [] do_init_module+0x60/0x1c8 > [] load_module+0x1554/0x1c98 > [] SyS_finit_module+0x7c/0x88 > [] el0_svc_naked+0x24/0x28 > > > The line that triggers the warning is: > > 674: WARN_ON(!connector->encoder->crtc); > > As far as I can see the encoder->crtc value is being set to a non-NULL value > only > in two places: > - in drm_atomic_helper_update_legacy_modeset_state() after WARN_ON() > encoder->crtc = connector->state->crtc; > - in drm_crtc_helper_set_config(drm_mode_set *set): > encoder->crtc = new_crtc; > > Nothing in the call path from drm_atomic_commit() calls > crtc_funcs->set_config() or > drm_crtc_helper_set_config() directly, so the question is if this WARN() is > actually > valid. > > Call path from drm_atomic_commit: > > drm_atomic_helper_commit() >- drm_atomic_helper_prepare_planes() >- drm_atomic_helper_swap_state() >- drm_atomic_helper_commit_modeset_disables() > - disable_outputs() > - drm_atomic_helper_update_legacy_modeset_state() > - WARN_ON(!connector->encoder->crtc) > > Best regards, > Liviu > Hi Liviu I solved this problem by following change, you can check if your driver do the same things: drivers/gpu/drm/bridge/dw_hdmi.c: - hdmi->connector.encoder = encoder; +// hdmi->connector.encoder = encoder; drm_mode_connector_attach_encoder(>connector, encoder); I found some other drivers set connector.encoder before drm_mode_connector_attach_encoder, some are not. drm_mode_connector_attach_encoder already describe the link of connector and encoder, so I think "connector.encoder = encoder" is not needed, is that right? Thanks. -- ï¼ark Yao -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151112/5ddae3e6/attachment.html>
[PATCH 10/25] drm/exynos: move dma_addr attribute from exynos plane to exynos fb
Hi Marek, 2015-11-10 Marek Szyprowski : > DMA address is a framebuffer attribute and the right place for it is > exynos_drm_framebuffer not exynos_drm_plane. This patch also introduces > helper function for getting dma address of the given framebuffer. > > Signed-off-by: Marek Szyprowski > --- > drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 13 - > drivers/gpu/drm/exynos/exynos7_drm_decon.c| 16 +--- > drivers/gpu/drm/exynos/exynos_drm_drv.h | 3 --- > drivers/gpu/drm/exynos/exynos_drm_fb.c| 16 ++-- > drivers/gpu/drm/exynos/exynos_drm_fb.h| 3 +-- > drivers/gpu/drm/exynos/exynos_drm_fimd.c | 10 ++ > drivers/gpu/drm/exynos/exynos_drm_plane.c | 18 -- > drivers/gpu/drm/exynos/exynos_drm_vidi.c | 5 - > drivers/gpu/drm/exynos/exynos_mixer.c | 7 --- > 9 files changed, 38 insertions(+), 53 deletions(-) Reviewed-by: Gustavo Padovan Gustavo
[PATCH 06/25] drm/exynos: fix to calculate offset of each plane for ipp fimc
Hello, Marek Szyprowski wrote: > From: Seung-Woo Kim > > NV12 and YUV420 formats are need to calculate offset of each plane > for ipp fimc in a gem buffer. Without proper offset, only Y plane > can be processed, so result shows green frame. > This patch fixes to calculate offset for cbcr planes for NV12 and > YUV420 formats. > > Signed-off-by: Seung-Woo Kim > Signed-off-by: Marek Szyprowski > --- > drivers/gpu/drm/exynos/exynos_drm_fimc.c | 106 > +++ > drivers/gpu/drm/exynos/exynos_drm_ipp.c | 15 - > drivers/gpu/drm/exynos/exynos_drm_ipp.h | 2 + > 3 files changed, 121 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c > b/drivers/gpu/drm/exynos/exynos_drm_fimc.c > index c747824f3c98..72a7ca188be5 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c > @@ -403,6 +403,97 @@ static void fimc_handle_lastend(struct fimc_context > *ctx, bool enable) > fimc_write(ctx, cfg, EXYNOS_CIOCTRL); > } > > +static int fimc_set_planar_addr(struct drm_exynos_ipp_buf_info *buf_info, > + u32 fmt, struct drm_exynos_sz *sz) > +{ > + dma_addr_t *base[EXYNOS_DRM_PLANAR_MAX]; I think I've seen this at several other places already, but I never fully understood it. Why are we using dma_addr_t* here, instead of just dma_addr_t? I mean in the following code the pointer is just in the dereferenced form anyway, so why this added indirection? With best wishes, Tobias > + uint64_t size[EXYNOS_DRM_PLANAR_MAX]; > + uint64_t ofs[EXYNOS_DRM_PLANAR_MAX]; > + bool bypass = false; > + uint64_t tsize = 0; > + int i; > + > + for_each_ipp_planar(i) { > + base[i] = _info->base[i]; > + size[i] = buf_info->size[i]; > + ofs[i] = 0; > + tsize += size[i]; > + } > + > + if (!tsize) { > + DRM_INFO("%s:failed to get buffer size.\n", __func__); > + return 0; > + } > + > + switch (fmt) { > + case DRM_FORMAT_NV12: > + case DRM_FORMAT_NV21: > + case DRM_FORMAT_NV16: > + case DRM_FORMAT_NV61: > + ofs[0] = sz->hsize * sz->vsize; > + ofs[1] = ofs[0] >> 1; > + if (*base[0] && *base[1]) { > + if (size[0] + size[1] < ofs[0] + ofs[1]) > + goto err_info; > + bypass = true; > + } > + break; > + case DRM_FORMAT_YUV410: > + case DRM_FORMAT_YVU410: > + case DRM_FORMAT_YUV411: > + case DRM_FORMAT_YVU411: > + case DRM_FORMAT_YUV420: > + case DRM_FORMAT_YVU420: > + case DRM_FORMAT_YUV422: > + case DRM_FORMAT_YVU422: > + case DRM_FORMAT_YUV444: > + case DRM_FORMAT_YVU444: > + ofs[0] = sz->hsize * sz->vsize; > + ofs[1] = ofs[2] = ofs[0] >> 2; > + if (*base[0] && *base[1] && *base[2]) { > + if (size[0]+size[1]+size[2] < ofs[0]+ofs[1]+ofs[2]) > + goto err_info; > + bypass = true; > + } > + break; > + case DRM_FORMAT_XRGB: > + case DRM_FORMAT_ARGB: > + ofs[0] = sz->hsize * sz->vsize << 2; > + if (*base[0]) { > + if (size[0] < ofs[0]) > + goto err_info; > + } > + bypass = true; > + break; > + default: > + bypass = true; > + break; > + } > + > + if (!bypass) { > + *base[1] = *base[0] + ofs[0]; > + if (ofs[1] && ofs[2]) > + *base[2] = *base[1] + ofs[1]; > + } > + > + DRM_DEBUG_KMS("%s:y[0x%x],cb[0x%x],cr[0x%x]\n", __func__, > + *base[0], *base[1], *base[2]); > + > + return 0; > + > +err_info: > + DRM_ERROR("invalid size for fmt[0x%x]\n", fmt); > + > + for_each_ipp_planar(i) { > + base[i] = _info->base[i]; > + size[i] = buf_info->size[i]; > + > + DRM_ERROR("buf[%d] - base[0x%x] sz[%llu] ofs[%llu]\n", > + i, *base[i], size[i], ofs[i]); > + } > + > + return -EINVAL; > +} > > static int fimc_src_set_fmt_order(struct fimc_context *ctx, u32 fmt) > { > @@ -689,6 +780,7 @@ static int fimc_src_set_addr(struct device *dev, > struct drm_exynos_ipp_cmd_node *c_node = ippdrv->c_node; > struct drm_exynos_ipp_property *property; > struct drm_exynos_ipp_config *config; > + int ret; > > if (!c_node) { > DRM_ERROR("failed to get c_node.\n"); > @@ -709,6 +801,12 @@ static int fimc_src_set_addr(struct device *dev, > switch (buf_type) { > case IPP_BUF_ENQUEUE: > config = >config[EXYNOS_DRM_OPS_SRC]; > + ret = fimc_set_planar_addr(buf_info, config->fmt, >sz); > + if (ret) { > + dev_err(dev,
[PATCH 18/25] drm/exynos: fimd: fix dma burst size setting for small plane size
This one looks a bit strange. It only changes the argument list of fimd_win_set_pixfmt() but the commit message that it actually fixes something. The corresponding upstream commit: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=66367461e573321f0fbb0be0391165b5a54d5fe4 With best wishes, Tobias Marek Szyprowski wrote: > This patch fixes trashed display of buffers cropped to very small width. > Even if DMA is unstable and causes tearing when changing the burst size, > it is still better than displaying a garbage. > > Signed-off-by: Marek Szyprowski > --- > drivers/gpu/drm/exynos/exynos_drm_fimd.c | 24 +++- > 1 file changed, 11 insertions(+), 13 deletions(-) > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c > b/drivers/gpu/drm/exynos/exynos_drm_fimd.c > index 44226b2b46c7..6c04ff6432d4 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c > @@ -487,7 +487,7 @@ static void fimd_commit(struct exynos_drm_crtc *crtc) > > > static void fimd_win_set_pixfmt(struct fimd_context *ctx, unsigned int win, > - struct drm_framebuffer *fb) > + uint32_t pixel_format, int width) > { > unsigned long val; > > @@ -498,11 +498,11 @@ static void fimd_win_set_pixfmt(struct fimd_context > *ctx, unsigned int win, >* So the request format is ARGB then change it to XRGB. >*/ > if (ctx->driver_data->has_limited_fmt && !win) { > - if (fb->pixel_format == DRM_FORMAT_ARGB) > - fb->pixel_format = DRM_FORMAT_XRGB; > + if (pixel_format == DRM_FORMAT_ARGB) > + pixel_format = DRM_FORMAT_XRGB; > } > > - switch (fb->pixel_format) { > + switch (pixel_format) { > case DRM_FORMAT_C8: > val |= WINCON0_BPPMODE_8BPP_PALETTE; > val |= WINCONx_BURSTLEN_8WORD; > @@ -538,17 +538,15 @@ static void fimd_win_set_pixfmt(struct fimd_context > *ctx, unsigned int win, > break; > } > > - DRM_DEBUG_KMS("bpp = %d\n", fb->bits_per_pixel); > - > /* > - * In case of exynos, setting dma-burst to 16Word causes permanent > - * tearing for very small buffers, e.g. cursor buffer. Burst Mode > - * switching which is based on plane size is not recommended as > - * plane size varies alot towards the end of the screen and rapid > - * movement causes unstable DMA which results into iommu crash/tear. > + * Setting dma-burst to 16Word causes permanent tearing for very small > + * buffers, e.g. cursor buffer. Burst Mode switching which based on > + * plane size is not recommended as plane size varies alot towards the > + * end of the screen and rapid movement causes unstable DMA, but it is > + * still better to change dma-burst than displaying garbage. >*/ > > - if (fb->width < MIN_FB_WIDTH_FOR_16WORD_BURST) { > + if (width < MIN_FB_WIDTH_FOR_16WORD_BURST) { > val &= ~WINCONx_BURSTLEN_MASK; > val |= WINCONx_BURSTLEN_4WORD; > } > @@ -723,7 +721,7 @@ static void fimd_update_plane(struct exynos_drm_crtc > *crtc, > DRM_DEBUG_KMS("osd size = 0x%x\n", (unsigned int)val); > } > > - fimd_win_set_pixfmt(ctx, win, fb); > + fimd_win_set_pixfmt(ctx, win, fb->pixel_format, state->src.w); > > /* hardware window 0 doesn't support color key. */ > if (win != 0) >
[PATCH 09/25] drm/exynos: exynos7-decon: remove excessive check
Hi Marek, 2015-11-10 Marek Szyprowski : > Display area is already checked by exynos plane core, so there is no > need for such check in driver code. > > Signed-off-by: Marek Szyprowski > --- > drivers/gpu/drm/exynos/exynos7_drm_decon.c | 10 -- > 1 file changed, 10 deletions(-) Reviewed-by: Gustavo Padovan Gustavo
[PATCH 08/25] drm/exynos: rotator: convert to common clock framework
Hi Marek, 2015-11-10 Marek Szyprowski : > This driver was not used after introduction of common clock framework. > This patch adds missing prepare/unprepare calls and allows to use it > again with current kernel code. > > Signed-off-by: Marek Szyprowski > --- > drivers/gpu/drm/exynos/exynos_drm_rotator.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Reviewed-by: Gustavo Padovan Gustavo
[PATCH 00/25] Exynos DRM: new life of IPP (Image Post Processing) subsystem
Hey Daniel, Daniel Stone wrote: > Hi, > > On 12 November 2015 at 12:44, Tobias Jakobi > wrote: >> Daniel Stone wrote: >>> On 10 November 2015 at 13:23, Marek Szyprowski >> samsung.com> wrote: This patch series introduces a new life into Exynos IPP (Image Post Processing) subsystem by integrating it (transparently for userspace applications) with Exynos DRM core plane management. This means that all CRTC drivers transparently get support for standard features of IPP subsystem like rotation and scaling. Support for features not supported natively by CRTC drivers is implemented with a help of temporary framebuffers, where image data is processed by IPP subsystem before performing the scanout by a CRTC driver. >>> >>> Hm, interesting. The RPi has a similar setup - VC4 can work either >>> online (realtime scanout) or offline (mem2mem). Once the scene crosses >>> a certain complexity boundary, it can no longer be composed in >>> realtime and must fall back to mem2mem before it can be displayed. >>> >>> There was talk of having the fallback handled transparently in KMS for >>> VC4 - similar to this - but the conclusion seemed to be that it was an >>> inappropriate level of hidden complexity for KMS, and instead would >>> best be handled by something like HWComposer directing it. Using HWC >>> would then let you more intelligently split the scene from userspace >>> (e.g. flatten some components but retain others as active planes). >> I would be intererested in the performance implications of this >> abstraction as well. >> >> I'd like to use the Exynos FIMC for CSC and scaling, but this operation >> of course takes some time. >> >> I wonder how this interacts with page flipping. If I queue a pageflip >> event with a buffer that needs to go through the IPP for display, where >> does the delay caused by the operation factor it? If I understand this >> correctly drmModePageFlip() still is going to return immediately, but I >> might miss the next vblank period because the FIMC is still working on >> the buffer. > > Hmm, from my reading of the patches, this didn't affect page-flip > timings. In the sync case, it would block until the buffer was > actually displayed, and in the async case, the event would still be > delivered at the right time. But you're right that it does introduce > hugely variable timings, which can be a problem for userspace which > tries to be intelligent. I guess this is an issue then, since the FIMC is mainly used in the context of converting video frames. And a good video player probably wants to have tight control over the presentation of video frames, to control A/V sychronization and so on. > And even then potentially misleading from a > performance point of view: if userspace can rotate natively (e.g. as > part of a composition blit, or when rendering buffers in the first > place), then we can skip the extra work from G2D. > >> My problem here is that this abstraction would take too much control >> from the user. >> >> Correct me if I have this wrong! > > I believe that was the concern previously, yeah. :) That, and encoding > these semantics in a user-visible way could potentially be dangerous. I guess the local path transfers would still make sense though, but if I understand this correctly this would affect FIMD. With best wishes, Tobias > > Cheers, > Daniel >
[PATCH] drm/tegra: fix locking of SET_TILING ioctl
drm_gem_object_unreference() now expects obj->dev->struct_mutex to be held. Use the newly-introduced drm_gem_object_unreference_unlocked() which handles locking for us. If we don't do this drm_gem_object_unreference() will get noisy about struct_mutex not being held every time we call the SET_TILING ioctl. Signed-off-by: Alexandre Courbot --- drivers/gpu/drm/tegra/drm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c index cc48334ef164..c0ae89865958 100644 --- a/drivers/gpu/drm/tegra/drm.c +++ b/drivers/gpu/drm/tegra/drm.c @@ -683,7 +683,7 @@ static int tegra_gem_set_tiling(struct drm_device *drm, void *data, bo->tiling.mode = mode; bo->tiling.value = value; - drm_gem_object_unreference(gem); + drm_gem_object_unreference_unlocked(gem); return 0; } -- 2.6.2
__i915_spin_request() sucks
On 11/12/2015 03:52 PM, Jens Axboe wrote: > On 11/12/2015 03:19 PM, Chris Wilson wrote: So today, I figured I'd try just killing that spin. If it fails, we'll punt to normal completions, so easy change. And wow, MASSIVE difference. I can now scroll in chrome and not rage! It's like the laptop is 10x faster now. Ran git blame, and found: commit 2def4ad99befa25775dd2f714fdd4d92faec6e34 Author: Chris Wilson Date: Tue Apr 7 16:20:41 2015 +0100 drm/i915: Optimistically spin for the request completion and read the commit message. Doesn't sound that impressive. Especially not for something that screws up interactive performance by a LOT. What's the deal? Revert? >> >> The tests that it improved the most were the latency sensitive tests and >> since my Broadwell xps13 behaves itself, I'd like to understand how it >> culminates in an interactivity loss. >> >> 1. Maybe it is the uninterruptible nature of the polling, making X's >> SIGIO jerky: > > This one still feels bad. > >> 2. Or maybe it is increased mutex contention: > > And so does this one... I had to manually apply hunks 2-3, and after > doing seat-of-the-pants testing for both variants, I confirmed with perf > that we're still seeing a ton of time in __i915_wait_request() for both > of them. I don't see how #2 could make any difference, you're passing in 0x3 hard coded for most call sites, so we poll. The ones that don't, pass a bool (?!). I should note that with the basic patch of just never spinning, I don't see __i915_wait_request() in the profiles. At all. -- Jens Axboe
__i915_spin_request() sucks
On 11/12/2015 03:19 PM, Chris Wilson wrote: >>> So today, I figured I'd try just killing that spin. If it fails, we'll >>> punt to normal completions, so easy change. And wow, MASSIVE difference. >>> I can now scroll in chrome and not rage! It's like the laptop is 10x >>> faster now. >>> >>> Ran git blame, and found: >>> >>> commit 2def4ad99befa25775dd2f714fdd4d92faec6e34 >>> Author: Chris Wilson >>> Date: Tue Apr 7 16:20:41 2015 +0100 >>> >>> drm/i915: Optimistically spin for the request completion >>> >>> and read the commit message. Doesn't sound that impressive. Especially >>> not for something that screws up interactive performance by a LOT. >>> >>> What's the deal? Revert? > > The tests that it improved the most were the latency sensitive tests and > since my Broadwell xps13 behaves itself, I'd like to understand how it > culminates in an interactivity loss. > > 1. Maybe it is the uninterruptible nature of the polling, making X's > SIGIO jerky: This one still feels bad. > 2. Or maybe it is increased mutex contention: And so does this one... I had to manually apply hunks 2-3, and after doing seat-of-the-pants testing for both variants, I confirmed with perf that we're still seeing a ton of time in __i915_wait_request() for both of them. > Or maybe it is an indirect effect, such as power balancing between the > CPU and GPU, or just thermal throttling, or it may be the task being > penalised for consuming its timeslice (for which any completion polling > seems susceptible). Look, polls in the 1-10ms range are just insane. Either you botched the commit message and really meant "~1ms at most" and in which case I'd suspect you of smoking something good, or you hacked it up wrong and used jiffies when you really wanted to be using some other time check that really did give you 1us. I'll take an IRQ over 10 msecs of busy looping on my laptop, thanks. >> "Limit the spinning to a single jiffie (~1us) at most" >> >> is totally wrong. I have HZ=100 on my laptop. That's 10ms. 10ms! >> Even if I had HZ=1000, that'd still be 1ms of spinning. That's >> seriously screwed up, guys. > > That's over and above the termination condition for blk_poll(). ?! And this is related, how? Comparing apples and oranges. One is a test opt-in feature for experimentation, the other is unconditionally enabled for everyone. I believe the commit even says so. See the difference? Would I use busy loop spinning waiting for rotating storage completions, which are in the 1-10ms range? No, with the reason being that the potential wins for spins are in the usec range. -- Jens Axboe
[Bug 106271] Switch between AMD hybrid graphics (HD 8650G / HD 8970M) makes hardware reset.
https://bugzilla.kernel.org/show_bug.cgi?id=106271 --- Comment #12 from Alex Deucher --- (In reply to Aneroid from comment #11) > May be the root of that problem is that Radeon HD 8970M is not a Pitcairn > card ? > > It is a Neptune XT, some kind of mobile version: > http://www.techpowerup.com/gpudb/1966/radeon-hd-8970m.html Neptune is the just the code name for mobile Pitcairn chips. -- You are receiving this mail because: You are watching the assignee of the bug.
[PATCH 18/25] drm/exynos: fimd: fix dma burst size setting for small plane size
Hey Tobias, On 12 November 2015 at 15:17, Tobias Jakobi wrote: > This one looks a bit strange. It only changes the argument list of > fimd_win_set_pixfmt() but the commit message that it actually fixes > something. > > The corresponding upstream commit: > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=66367461e573321f0fbb0be0391165b5a54d5fe4 It's subtle: >> - fimd_win_set_pixfmt(ctx, win, fb); >> + fimd_win_set_pixfmt(ctx, win, fb->pixel_format, state->src.w); i.e. rather than checking the total width of the allocated fb, we check the width we are _actually_ scanning out from the fb. So I think the patch is correct. Cheers, Daniel
[PATCH] drm/exynos: only run atomic_check() if crtc is active
On Thu, Nov 12, 2015 at 11:11:20AM -0200, Gustavo Padovan wrote: > From: Gustavo Padovan > > Fixes an regression added by 3ae2436 (drm/exynos/mixer: replace > direct cross-driver call with drm mode). The whole atomic update > was failing if the hdmi display is not present/active. Add a test > to only run atomic_check() if the CRTC is active. > > Signed-off-by: Gustavo Padovan > --- > drivers/gpu/drm/exynos/exynos_drm_crtc.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c > b/drivers/gpu/drm/exynos/exynos_drm_crtc.c > index b3ba27f..1d3ca0a 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c > @@ -55,6 +55,9 @@ static int exynos_crtc_atomic_check(struct drm_crtc *crtc, > { > struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc); > > + if (!state->active) > + return 0; > + > if (exynos_crtc->ops->atomic_check) > return exynos_crtc->ops->atomic_check(exynos_crtc, state); > This looks like something that the core should be doing. Thierry -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151112/06f9101b/attachment.sig>
[Bug 92850] Segfault loading War Thunder
https://bugs.freedesktop.org/show_bug.cgi?id=92850 --- Comment #11 from bellamorte42 at gmail.com --- I rebuilt mesa as instructed (--enable-debug) and that's what I got. -- 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/20151112/4a77a23c/attachment.html>
[Nouveau] [PATCH] drm/nouveau: Fix pre-nv50 pageflip events (v3) -> v4
.i686+debug #1 > ... > Call Trace: > [] dump_stack+0x48/0x69 > [] warn_slowpath_common+0x87/0xc0 > [] ? check_sync+0x169/0x6e0 > [] ? check_sync+0x169/0x6e0 > [] warn_slowpath_fmt+0x3e/0x60 > [] check_sync+0x169/0x6e0 > [] debug_dma_sync_single_for_device+0x7d/0x90 > [] ? ttm_bo_del_sub_from_lru+0x18/0x50 [ttm] > [] ? text_poke_bp+0xd0/0xd0 > [] nouveau_bo_sync_for_device+0x8b/0xa0 [nouveau] > [] nouveau_bo_validate+0x34/0x40 [nouveau] > [] nouveau_bo_pin+0x188/0x290 [nouveau] > [] ? nv10_bo_put_tile_region+0x80/0x80 [nouveau] > [] nouveau_channel_prep+0xfd/0x2c0 [nouveau] > [] nouveau_channel_new+0x57/0x7a0 [nouveau] > [] ? kfree+0xdc/0x280 > [] ? nvif_object_sclass_put+0x12/0x20 [nouveau] > [] nouveau_drm_load+0x596/0x8d0 [nouveau] > [] ? trace_hardirqs_on_caller+0x12c/0x1d0 > [] ? drm_minor_register+0x89/0x120 [drm] > [] drm_dev_register+0x96/0xa0 [drm] > [] drm_get_pci_dev+0x79/0x1c0 [drm] > [] ? pcibios_set_master+0x4e/0xa0 > [] nouveau_drm_probe+0x1ee/0x220 [nouveau] > [] pci_device_probe+0x7b/0xf0 > [] ? devices_kset_move_last+0x56/0xa0 > [] driver_probe_device+0x204/0x490 > [] ? __driver_attach+0x4c/0x90 > [] ? pci_match_device+0xd2/0x100 > [] __driver_attach+0x81/0x90 > [] ? driver_probe_device+0x490/0x490 > [] bus_for_each_dev+0x57/0xa0 > [] driver_attach+0x1e/0x20 > [] ? driver_probe_device+0x490/0x490 > [] bus_add_driver+0x1ef/0x290 > [] driver_register+0x5d/0xf0 > [] __pci_register_driver+0x4a/0x50 > [] drm_pci_init+0xdd/0x100 [drm] > [] nouveau_drm_init+0x1f9/0x1000 [nouveau] > [] ? 0xf7f21000 > [] do_one_initcall+0xaa/0x200 > [] ? 0xf7f21000 > [] ? rcu_read_lock_sched_held+0x42/0x80 > [] ? kmem_cache_alloc_trace+0x23d/0x310 > [] ? do_init_module+0x21/0x1b7 > [] ? do_init_module+0x21/0x1b7 > [] do_init_module+0x50/0x1b7 > [] load_module+0x1ebc/0x2550 > [] ? _raw_spin_unlock_irq+0x27/0x40 > [] ? finish_task_switch+0x8a/0x1d0 > [] SyS_init_module+0x147/0x1a0 > [] ? do_audit_syscall_entry.isra.9+0x44/0x50 > [] ? syscall_trace_enter_phase1+0x107/0x130 > [] syscall_call+0x7/0x7 > ---[ end trace d3c14159641a1388 ]--- > > > NV34 tested with 4.3.0-3.fc22.i686 > i.e. 4.3.0-1.fc24.i686 & drm-nouveau-Fix-pre-nv50-pageflip-events-v4.patch > > http://koji.fedoraproject.org/koji/buildinfo?buildID=695636 > https://patchwork.kernel.org/patch/7591531/mbox This doesn't look at all related and has probably been an issue for quite some time. I /think/ this happens because memory is allocated from the non-DMA pool (i.e. using alloc_page()) and then ends up getting run through the dma_sync_*() API for cache maintenance. But the assumption is that you can only do cache maintenance by the dma_sync_*() API on memory allocated by dma_alloc_*(), hence the warning. There was some discussion about this a while ago, and there was some conclusion that an API was needed to do cache maintenance on non-DMA- allocated pages of memory, but I don't think any work happened towards that API. Adding Alex and Arnd who had been part of that discussion, though possibly in different threads. Guys, I've been doing too many unrelated things lately it seems, because I can't remember where exactly we left off. I vaguely remember that at some point somebody (maybe Russell) had objected to adding such a non-DMA cache-maintenance API, but I can't find a link to the relevant threads. Thierry -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151112/b2802904/attachment.sig>
[PATCH 00/25] Exynos DRM: new life of IPP (Image Post Processing) subsystem
Hi, On 12 November 2015 at 12:44, Tobias Jakobi wrote: > Daniel Stone wrote: >> On 10 November 2015 at 13:23, Marek Szyprowski >> wrote: >>> This patch series introduces a new life into Exynos IPP (Image Post >>> Processing) subsystem by integrating it (transparently for userspace >>> applications) with Exynos DRM core plane management. This means that all >>> CRTC drivers transparently get support for standard features of IPP >>> subsystem like rotation and scaling. >>> >>> Support for features not supported natively by CRTC drivers is >>> implemented with a help of temporary framebuffers, where image data is >>> processed by IPP subsystem before performing the scanout by a CRTC driver. >> >> Hm, interesting. The RPi has a similar setup - VC4 can work either >> online (realtime scanout) or offline (mem2mem). Once the scene crosses >> a certain complexity boundary, it can no longer be composed in >> realtime and must fall back to mem2mem before it can be displayed. >> >> There was talk of having the fallback handled transparently in KMS for >> VC4 - similar to this - but the conclusion seemed to be that it was an >> inappropriate level of hidden complexity for KMS, and instead would >> best be handled by something like HWComposer directing it. Using HWC >> would then let you more intelligently split the scene from userspace >> (e.g. flatten some components but retain others as active planes). > I would be intererested in the performance implications of this > abstraction as well. > > I'd like to use the Exynos FIMC for CSC and scaling, but this operation > of course takes some time. > > I wonder how this interacts with page flipping. If I queue a pageflip > event with a buffer that needs to go through the IPP for display, where > does the delay caused by the operation factor it? If I understand this > correctly drmModePageFlip() still is going to return immediately, but I > might miss the next vblank period because the FIMC is still working on > the buffer. Hmm, from my reading of the patches, this didn't affect page-flip timings. In the sync case, it would block until the buffer was actually displayed, and in the async case, the event would still be delivered at the right time. But you're right that it does introduce hugely variable timings, which can be a problem for userspace which tries to be intelligent. And even then potentially misleading from a performance point of view: if userspace can rotate natively (e.g. as part of a composition blit, or when rendering buffers in the first place), then we can skip the extra work from G2D. > My problem here is that this abstraction would take too much control > from the user. > > Correct me if I have this wrong! I believe that was the concern previously, yeah. :) That, and encoding these semantics in a user-visible way could potentially be dangerous. Cheers, Daniel
[Bug 92850] Segfault loading War Thunder
https://bugs.freedesktop.org/show_bug.cgi?id=92850 --- Comment #10 from bellamorte42 at gmail.com --- Created attachment 119594 --> https://bugs.freedesktop.org/attachment.cgi?id=119594=edit dmesg -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151112/1228ed61/attachment.html>
[Bug 106271] Switch between AMD hybrid graphics (HD 8650G / HD 8970M) makes hardware reset.
https://bugzilla.kernel.org/show_bug.cgi?id=106271 --- Comment #11 from Aneroid --- May be the root of that problem is that Radeon HD 8970M is not a Pitcairn card ? It is a Neptune XT, some kind of mobile version: http://www.techpowerup.com/gpudb/1966/radeon-hd-8970m.html -- You are receiving this mail because: You are watching the assignee of the bug.
[Bug 92850] Segfault loading War Thunder
https://bugs.freedesktop.org/show_bug.cgi?id=92850 --- Comment #9 from bellamorte42 at gmail.com --- Created attachment 119593 --> https://bugs.freedesktop.org/attachment.cgi?id=119593=edit glxinfo -- 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/20151112/cb25d5ee/attachment.html>
drm: Bogus WARN() in drm_atomic_helper_update_legacy_modeset_state() ?
e state ee38eac0 [1.38] [drm:drm_atomic_get_connector_state] Added [CONNECTOR:29] ee38e8c0 state to ee38ebc0 [1.300015] [drm:drm_atomic_add_affected_connectors] Adding all current connectors for [CRTC:20] to ee38ebc0 [1.300021] [drm:drm_atomic_set_crtc_for_connector] Link connector state ee38e8c0 to [CRTC:20] [1.300033] [drm:drm_atomic_get_crtc_state] Added [CRTC:25] ee393e00 state to ee38ebc0 [1.300038] [drm:drm_atomic_set_mode_for_crtc] Set [NOMODE] for CRTC state ee393e00 [1.300043] [drm:drm_atomic_set_crtc_for_plane] Link plane state ee38e9c0 to [NOCRTC] [1.300047] [drm:drm_atomic_set_fb_for_plane] Set [NOFB] for plane state ee38e9c0 [1.300053] [drm:drm_atomic_add_affected_connectors] Adding all current connectors for [CRTC:20] to ee38ebc0 [1.300059] [drm:drm_atomic_add_affected_connectors] Adding all current connectors for [CRTC:25] to ee38ebc0 [1.300067] [drm:drm_atomic_connectors_for_crtc] State ee38ebc0 has 1 connectors for [CRTC:20] [1.300071] [drm:drm_atomic_check_only] checking ee38ebc0 [1.300079] [drm:drm_atomic_helper_check_modeset] [CRTC:20] mode changed [1.300083] [drm:drm_atomic_helper_check_modeset] [CRTC:20] enable changed [1.300088] [drm:update_connector_routing] Updating routing for [CONNECTOR:29:HDMI-A-1] [1.300095] [drm:update_connector_routing] [CONNECTOR:29:HDMI-A-1] using [ENCODER:28:TMDS-28] on [CRTC:20] [1.300100] [drm:drm_atomic_helper_check_modeset] [CRTC:20] active changed [1.300105] [drm:drm_atomic_helper_check_modeset] [CRTC:20] needs all connectors, enable: y, active: y [1.300111] [drm:drm_atomic_add_affected_connectors] Adding all current connectors for [CRTC:20] to ee38ebc0 [1.300117] [drm:drm_atomic_connectors_for_crtc] State ee38ebc0 has 1 connectors for [CRTC:20] [1.300130] [drm:drm_atomic_commit] commiting ee38ebc0 [1.300156] WARNING: CPU: 0 PID: 26 at drivers/gpu/drm/drm_atomic_helper.c:674 drm_atomic_helper_update_legacy_modeset_state+0x3c/0x1f8() -- ï¼ark Yao -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151112/3b3de8a1/attachment-0001.html>
[PATCH] drm: Don't oops in drm_calc_timestamping_constants() if drm_vblank_init() wasn't called
From: Ville SyrjäläSeems the crtc helpers call drm_calc_timestamping_constants() unconditionally even if the driver didn't initialize vblank support by calling drm_vblank_init(). That used to be OK since the constants were stored under drm_crtc. However I broke this with commit eba1f35dfe14 ("drm: Move timestamping constants into drm_vblank_crtc") when I moved the constants to live inside the drm_vblank_crtc struct instead. If drm_vblank_init() isn't called, we don't allocate these structures, and so drm_calc_timestamping_constants() will oops. Fix it by adding a check into drm_calc_timestamping_constants() to see if vblank support was initialized at all. And to keep in line with other such checks, also toss in a check and warn for the case where vblank support was initialized, but the wrong number of crtcs was specified. Fixes the following sort of oops: BUG: unable to handle kernel NULL pointer dereference at 00b0 IP: [] drm_calc_timestamping_constants+0x86/0x130 [drm] PGD 0 Oops: 0002 [#1] SMP Modules linked in: sr_mod cdrom mgag200(+) i2c_algo_bit drm_kms_helper ahci syscopyarea sysfillrect sysimgblt libahci fb_sys_fops bnx2x ttm tg3(+) mdio drm ptp sd_mod libata i2c_core pps_core libcrc32c hpsa dm_mirror dm_region_hash dm_log dm_mod CPU: 0 PID: 418 Comm: kworker/0:2 Not tainted 4.3.0+ #1 Hardware name: HP ProLiant DL380 Gen9, BIOS P89 06/09/2015 Workqueue: events work_for_cpu_fn task: 88046ca95500 ti: 88007830c000 task.ti: 88007830c000 RIP: 0010:[] [] drm_calc_timestamping_constants+0x86/0x130 [drm] RSP: 0018:88007830f4e8 EFLAGS: 00010246 RAX: 00fe4c00 RBX: 88006a849160 RCX: 0540 RDX: RSI: fde8 RDI: 88006a849000 RBP: 88007830f518 R08: 88007830c000 R09: 0001b87e3712 R10: 50c4 R11: R12: 00fe4c00 R13: 88006a849000 R14: R15: fde8 FS: () GS:88046f80() knlGS: CS: 0010 DS: ES: CR0: 80050033 CR2: 00b0 CR3: 019d6000 CR4: 001406f0 Stack: 88007830f518 88006a849000 880c69b90340 880c69b9 880c69b90348 880c69b90340 88007830f748 a042f7e7 88006a849090 88006a849160 Call Trace: [] drm_crtc_helper_set_mode+0x3d7/0x4b0 [drm_kms_helper] [] drm_crtc_helper_set_config+0x8d4/0xb10 [drm_kms_helper] [] drm_mode_set_config_internal+0x64/0x100 [drm] [] drm_fb_helper_pan_display+0xa2/0x280 [drm_kms_helper] [] fb_pan_display+0xbb/0x170 [] bit_update_start+0x20/0x50 [] fbcon_switch+0x39b/0x590 [] redraw_screen+0x1a0/0x240 [] do_bind_con_driver+0x2ee/0x310 [] do_take_over_console+0x141/0x1b0 [] do_fbcon_takeover+0x57/0xb0 [] fbcon_event_notify+0x60b/0x750 [] notifier_call_chain+0x49/0x70 [] __blocking_notifier_call_chain+0x4d/0x70 [] blocking_notifier_call_chain+0x16/0x20 [] fb_notifier_call_chain+0x1b/0x20 [] register_framebuffer+0x1f1/0x330 [] drm_fb_helper_initial_config+0x27a/0x3d0 [drm_kms_helper] [] mgag200_fbdev_init+0xdd/0xf0 [mgag200] [] mgag200_modeset_init+0x176/0x1e0 [mgag200] [] mgag200_driver_load+0x3f9/0x580 [mgag200] [] drm_dev_register+0xa7/0xb0 [drm] [] drm_get_pci_dev+0x8f/0x1e0 [drm] [] mga_pci_probe+0x9b/0xc0 [mgag200] [] local_pci_probe+0x45/0xa0 [] work_for_cpu_fn+0x14/0x20 [] process_one_work+0x14c/0x3c0 [] worker_thread+0x244/0x470 [] ? __schedule+0x2aa/0x760 [] ? rescuer_thread+0x310/0x310 [] kthread+0xd8/0xf0 [] ? kthread_park+0x60/0x60 [] ret_from_fork+0x3f/0x70 [] ? kthread_park+0x60/0x60 Code: f6 31 d2 41 89 c2 8b 83 b4 00 00 00 0f af c1 48 98 48 69 c0 40 42 0f 00 48 f7 f6 f6 43 74 10 41 89 c4 75 26 f6 05 9a 6f 03 00 01 <45> 89 96 b0 00 00 00 45 89 a6 ac 00 00 00 75 35 48 83 c4 08 5b RIP [] drm_calc_timestamping_constants+0x86/0x130 [drm] RSP CR2: 00b0 Cc: Jeff Moyer Reported-by: Jeff Moyer References: http://lists.freedesktop.org/archives/dri-devel/2015-November/094217.html Fixes: eba1f35dfe14 ("drm: Move timestamping constants into drm_vblank_crtc") Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/drm_irq.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index eba6337..2151ea5 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -616,10 +616,18 @@ int drm_control(struct drm_device *dev, void *data, void drm_calc_timestamping_constants(struct drm_crtc *crtc, const struct drm_display_mode *mode) { - struct drm_vblank_crtc *vblank = >dev->vblank[drm_crtc_index(crtc)]; + struct drm_device *dev = crtc->dev; + unsigned int pipe = drm_crtc_index(crtc); + struct drm_vblank_crtc *vblank = >vblank[pipe]; int linedur_ns = 0,
drm: Bogus WARN() in drm_atomic_helper_update_legacy_modeset_state() ?
>> is actually > >> valid. > >> > >> Call path from drm_atomic_commit: > >> > >> drm_atomic_helper_commit() > >>- drm_atomic_helper_prepare_planes() > >>- drm_atomic_helper_swap_state() > >>- drm_atomic_helper_commit_modeset_disables() > >> - disable_outputs() > >> - drm_atomic_helper_update_legacy_modeset_state() > >> - WARN_ON(!connector->encoder->crtc) > >> > >> Best regards, > >> Liviu > >> > >> > >>Hi Liviu > >> I solved this problem by following change, you can check if your > >> driver do the same things: > >> drivers/gpu/drm/bridge/dw_hdmi.c: > >> - hdmi->connector.encoder = encoder; > >> +// hdmi->connector.encoder = encoder; > >> > >> drm_mode_connector_attach_encoder(>connector, > >> encoder); > >> > >> I found some other drivers set connector.encoder before > >> drm_mode_connector_attach_encoder, some are not. > >> > >> drm_mode_connector_attach_encoder already describe the link of > >> connector and encoder, > >>so I think "connector.encoder = encoder" is not needed, is that right? > >I'll have a look, thanks for pointing this. However, my setup uses the > >tda998x driver for encoder, so I will > >have to go look there rather than in my driver. > > > >Best regards, > >Liviu > > > >>Thanks. > >> > >> -- > >> ï¼ark Yao > Hi Liviu > drivers/gpu/drm/i2c/tda998x_drv.c do the same things: > priv->connector.encoder = >encoder; > drm_mode_connector_attach_encoder(>connector, >encoder); > > I believe must be same problem. Oh, I hadn't noticed this subthread, but you came to the same conclusion as I did. I have the below local change, which I think might be good to have given that there are at least two drivers that got this wrong. Thierry --- >8 --- diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 24c5434abd1c..56d53106b32d 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -4983,6 +4983,20 @@ int drm_mode_connector_attach_encoder(struct drm_connector *connector, { int i; + /* +* In the past, drivers have attempted to model the static association +* of connector to encoder in simple connector/encoder devices using a +* direct assignment of connector->encoder = encoder. This connection +* is a logical one and the responsibility of the core, so drivers are +* expected not to mess with this. +* +* Note that the error return should've been enough here, but a large +* majority of drivers ignores the return value, so add in a big WARN +* to get people's attention. +*/ + if (WARN_ON(connector->encoder)) + return -EINVAL; + for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) { if (connector->encoder_ids[i] == 0) { connector->encoder_ids[i] = encoder->base.id; -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151112/ec66b2b8/attachment-0001.sig>
drm: Bogus WARN() in drm_atomic_helper_update_legacy_modeset_state() ?
On 2015å¹´11æ11æ¥ 00:56, Thierry Reding wrote: > On Tue, Nov 10, 2015 at 03:01:03PM +, Liviu Dudau wrote: >> Hello, >> >> When booting my Juno board with the HDLCD driver that I have converted to >> atomic operations I'm getting the following warning: > Perhaps you can provide pointers to the source code, that might make it > easier for people to spot what's going wrong. > > Thierry > > > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel Hi Thierry I encountered the same problem as Liviu. I'm coverting rockchip drm to atomic api, when booting with hdmi connected, get under warning: [1.300156] WARNING: CPU: 0 PID: 26 at drivers/gpu/drm/drm_atomic_helper.c:674 drm_atomic_helper_update_legacy_modeset_state+0x3c/0x1f8() [1.300161] Modules linked in: [1.300171] CPU: 0 PID: 26 Comm: kworker/0:1 Not tainted 4.3.0-rc5+ #160 [1.300174] Hardware name: Rockchip (Device Tree) [1.300189] Workqueue: events output_poll_execute [1.300224] [] (unwind_backtrace) from [] (show_stack+0x10/0x14) [1.300241] [] (show_stack) from [] (dump_stack+0x6c/0x88) [1.300255] [] (dump_stack) from [] (warn_slowpath_common+0x80/0xa8) [1.300265] [] (warn_slowpath_common) from [] (warn_slowpath_null+0x18/0x1c) [1.300277] [] (warn_slowpath_null) from [] (drm_atomic_helper_update_legacy_modeset_state+0x3c/0x1f8) [1.300293] [] (drm_atomic_helper_update_legacy_modeset_state) from [] (drm_atomic_helper_commit_modeset_disables+0x208/0x364) [1.300305] [] (drm_atomic_helper_commit_modeset_disables) from [] (drm_atomic_helper_commit+0xf8/0x140) [1.300320] [] (drm_atomic_helper_commit) from [] (drm_atomic_commit+0x50/0x60) [1.300333] [] (drm_atomic_commit) from [] (restore_fbdev_mode+0xf4/0x278) [1.300345] [] (restore_fbdev_mode) from [] (drm_fb_helper_restore_fbdev_mode_unlocked+0x30/0x68) [1.300357] [] (drm_fb_helper_restore_fbdev_mode_unlocked) from [] (drm_fb_helper_set_par+0x3c/0x54) [1.300368] [] (drm_fb_helper_set_par) from [] (drm_fb_helper_hotplug_event+0xe4/0xfc) [1.300382] [] (drm_fb_helper_hotplug_event) from [] (drm_kms_helper_hotplug_event+0x24/0x28) [1.300396] [] (drm_kms_helper_hotplug_event) from [] (output_poll_execute+0x134/0x18c) [1.300413] [] (output_poll_execute) from [] (process_one_work+0x1e0/0x318) [1.300426] [] (process_one_work) from [] (worker_thread+0x378/0x4c4) [1.300438] [] (worker_thread) from [] (kthread+0xdc/0xf0) [1.300450] [] (kthread) from [] (ret_from_fork+0x14/0x3c) I can't found who can set encoder->crtc value before atomic_commit, what's going wrong? -- ï¼ark Yao -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151112/ee9c93d3/attachment.html>
DRM i2c module or bridge ?
On Thu, Nov 12, 2015 at 12:48:51PM +, Emil Velikov wrote: > Hello Thierry, all, > > Inspired by a recent discussion I was started wondering - where is the > cut between DRM i2c modules (most of which encoders/transmitters) and > bridge drivers (again some of which i2c encoders) ? Does anyone has > some pointers on the topic ? DRM bridge is a superset of I2C encoders, so everything that I2C encoder drivers do they should be able to do with DRM bridges, and more. There isn't a strict guideline here, but I think there's general agreement that new drivers should be using the DRM bridge framework. The primary reason is that bridges integrate seamlessly with the driver model, that is, the drivers that implement them are regular drivers that register with the corresponding bus and get bound to a device, whereas the I2C encoder infrastructure is mostly about manually instantiating devices. For existing drivers I guess they could all be converted, but doing so may require a bit of work. They also tend to work as-is, so finding volunteers to do the conversion is probably going to be difficult given the lack of motivation. Thierry > Based on the above I did a very quick search for third party IP > modules in the DRM subsystem: > > * i915 > dvo_ch7017.c > dvo_ch7xxx.c > dvo_ivch.c > dvo_ns2501.c > dvo_sil164.c > dvo_tfp410.c It looks like these use some framework that's custom to the i915 driver but could otherwise easily be DRM bridges. > * gma500 > tc35876x-dsi-lvds.c This seems to be some sort of hybrid bridge and panel driver. > * sti > sti_hdmi_tx3g0c55phy.c > sti_hdmi_tx3g4c28phy.c These seem to implement some sort of PHY interface, but from a quick look moving these to the PHY framework seems overkill. They seem no good fit for DRM bridge because they are not separate devices, but rather the SoC generation specific bits of the STi HDMI driver. > (and for posterity) > * i2c > adv7511.c > ch7006_drv.c > sil164_drv.c > tda998x_drv.c > > > * bridge > dw_hdmi.c > nxp-ptn3460.c > parade-ps8622.c > > > By the looks of it, we can move rework (some of?) the above into > i2c/bridge modules and in other cases (sil164) just use the existing > one ? I'm neither volunteering nor suggesting people must work of > these, merely curious. My take on this is that it's probably best to keep the above in their current form. If they need to be shared across multiple hardware setups it might make sense to convert them to DRM bridge drivers. For new drivers it's probably best to make them bridge drivers from the start. Thierry -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151112/fa58afd1/attachment.sig>
drm: Bogus WARN() in drm_atomic_helper_update_legacy_modeset_state() ?
On Thu, Nov 12, 2015 at 02:34:11PM +0100, Thierry Reding wrote: > On Thu, Nov 12, 2015 at 06:49:38PM +0800, Mark yao wrote: > > On 2015å¹´11æ12æ¥ 18:36, Liviu Dudau wrote: > > >On Thu, Nov 12, 2015 at 04:32:33PM +0800, Mark yao wrote: > > >>On 2015å¹´11æ10æ¥ 23:01, Liviu Dudau wrote: > > >> > > >> Hello, > > >> > > >> When booting my Juno board with the HDLCD driver that I have converted > > >> to > > >> atomic operations I'm getting the following warning: > > >> > > >> [ cut here ] > > >> WARNING: at > > >> /work/repositories/kernel/drivers/gpu/drm/drm_atomic_helper.c:674 > > >> Modules linked in: hdlcd(+) clk_scpi > > >> > > >> CPU: 3 PID: 1375 Comm: systemd-udevd Not tainted 4.3.0-next-20151109+ #5 > > >> Hardware name: ARM Juno development board (r0) (DT) > > >> task: ffc974888b00 ti: ffc9755dc000 task.ti: ffc9755dc000 > > >> PC is at drm_atomic_helper_update_legacy_modeset_state+0x204/0x20c > > >> LR is at drm_atomic_helper_commit_modeset_disables+0x1c0/0x394 > > >> pc : [] lr : [] pstate: 2145 > > >> sp : ffc9755df430 > > >> x29: ffc9755df430 x28: ffc975703600 > > >> x27: x26: ffc976253960 > > >> x25: ffc976254040 x24: ffc000819000 > > >> x23: ffc000689ea0 x22: ffc976251800 > > >> x21: ffc976251800 x20: > > >> x19: ffc9766b1f80 x18: 715fe015 > > >> x17: 007fb4b855b0 x16: 0220 > > >> x15: 0001 x14: 0ffe > > >> x13: 0008 x12: 0101010101010101 > > >> x11: ffc000964000 x10: ffc0009d2000 > > >> x9 : x8 : ffc97ff5f700 > > >> x7 : ffc97566cb80 x6 : ffc9766b1700 > > >> x5 : ffc975665100 x4 : > > >> x3 : ffc976253960 x2 : ffc97566cd00 > > >> x1 : ffc976253900 x0 : > > >> > > >> ---[ end trace 9fe289f798e7178e ]--- > > >> Call trace: > > >> [] > > >> drm_atomic_helper_update_legacy_modeset_state+0x204/0x20c > > >> [] > > >> drm_atomic_helper_commit_modeset_disables+0x1c0/0x394 > > >> [] drm_atomic_helper_commit+0xe0/0x150 > > >> [] drm_atomic_commit+0x40/0x6c > > >> [] restore_fbdev_mode+0x294/0x2d4 > > >> [] drm_fb_helper_restore_fbdev_mode_unlocked+0x34/0x8c > > >> [] drm_fb_helper_set_par+0x2c/0x58 > > >> [] fbcon_init+0x4d4/0x534 > > >> [] visual_init+0xac/0x104 > > >> [] do_bind_con_driver+0x16c/0x398 > > >> [] do_take_over_console+0xd8/0x1f4 > > >> [] do_fbcon_takeover+0x74/0xf8 > > >> [] fbcon_event_notify+0x8a4/0x8f8 > > >> [] notifier_call_chain+0x4c/0x88 > > >> [] __blocking_notifier_call_chain+0x44/0x74 > > >> [] blocking_notifier_call_chain+0x14/0x1c > > >> [] fb_notifier_call_chain+0x1c/0x24 > > >> [] register_framebuffer+0x1c0/0x2ac > > >> [] drm_fb_helper_initial_config+0x25c/0x3ec > > >> [] drm_fbdev_cma_init+0x98/0x134 > > >> [] hdlcd_drm_bind+0x180/0x498 [hdlcd] > > >> [] try_to_bring_up_master.part.5+0xd4/0x118 > > >> [] component_master_add_with_match+0xc4/0x148 > > >> [] component_master_add+0x10/0x18 > > >> [] hdlcd_probe+0x14/0x28 [hdlcd] > > >> [] platform_drv_probe+0x54/0xc0 > > >> [] driver_probe_device+0x1ec/0x2e8 > > >> [] __driver_attach+0x9c/0xa0 > > >> [] bus_for_each_dev+0x58/0x98 > > >> [] driver_attach+0x20/0x28 > > >> [] bus_add_driver+0x1c8/0x22c > > >> [] driver_register+0x68/0x108 > > >> [] __platform_driver_register+0x4c/0x54 > > >> [] hdlcd_init+0x18/0x30 [hdlcd] > > >> [] do_one_initcall+0x90/0x1a8 > > >> [] do_init_module+0x60/0x1c8 > > >> [] load_module+0x1554/0x1c98 > > >> [] SyS_finit_module+0x7c/0x88 > > >> [] el0_svc_naked+0x24/0x28 > > >> > > >> > > >> The line that triggers the warning is: > > >> > > >> 674:WARN_ON(!connector->encoder->crtc); > > >> > > >> As far as I can see the encoder->crtc value is being set to a non-NULL > > >> value only > > >> in two places: > > >> - in drm_atomic_helper_update_legacy_modeset_state() after WARN_ON() > > >> encoder->crtc = connector->state->crtc; > > >> - in drm_crtc_helper_set_config(drm_mode_set *set): > > >> encoder->crtc = new_crtc; > > >> > > >> Nothing in the call path from drm_atomic_commit() calls > > >> crtc_funcs->set_config() or > > >> drm_crtc_helper_set_config() directly, so the question is if this > > >> WARN() is actually > > >> valid. > > >> > > >> Call path from drm_atomic_commit: > > >> > > >> drm_atomic_helper_commit() > > >>- drm_atomic_helper_prepare_planes() > > >>- drm_atomic_helper_swap_state() > > >>- drm_atomic_helper_commit_modeset_disables() > > >> - disable_outputs() > > >> - drm_atomic_helper_update_legacy_modeset_state() > > >> - WARN_ON(!connector->encoder->crtc) > > >> > > >> Best regards, > > >> Liviu > > >> > > >> > > >>Hi Liviu > > >> I solved this problem by following change, you can check if > > >> your driver do the same things:
drm: Bogus WARN() in drm_atomic_helper_update_legacy_modeset_state() ?
On Thu, Nov 12, 2015 at 01:16:55PM +0100, Thierry Reding wrote: > On Wed, Nov 11, 2015 at 04:09:42PM +, Liviu Dudau wrote: > > On Tue, Nov 10, 2015 at 05:56:15PM +0100, Thierry Reding wrote: > > > On Tue, Nov 10, 2015 at 03:01:03PM +, Liviu Dudau wrote: > > > > Hello, > > > > > > > > When booting my Juno board with the HDLCD driver that I have converted > > > > to > > > > atomic operations I'm getting the following warning: > > > > > > Perhaps you can provide pointers to the source code, that might make it > > > easier for people to spot what's going wrong. > > > > > > Thierry > > > > Hi Thierry, > > > > I have just pushed to the mailing lists my patch series. [1] [2] > > > > If you want to checkout the code I also have a branch here: > > > > git://git://linux-arm.org/linux-ld testing/hdlcd > > Okay, so if I understand correctly you're using the tda998x as encoder/ > connector attached to your new HDLCD driver. I think the problem isn't > so much that nothing has set up the CRTC for the encoder, but rather > that the tda998x encoder tries to statically associate the encoder with > the connector. While that might be correct in that it represents the > hardware state (i.e. there is no way to physically route it anywhere > else), the DRM logical state is that it's not connected until a complete > pipeline has been set up, in which case a CRTC would have been assigned > to the encoder. > > If your setup were working correctly you'd never reach the WARN_ON > because the > > if (connector->encoder) { > > conditional (on line 673 in drivers/gpu/drm/drm_atomic_helper.c) would > have evaluated to false already, since logically there'd be no encoder > associated with the connector yet. Your analysis is spot on and matches my conditions. However ... > > Does the patch below help? Let me know and I can produce a proper patch. > > Thierry > > --- >8 --- > diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c > b/drivers/gpu/drm/i2c/tda998x_drv.c > index 896b6aaf8c4d..8b0a402190eb 100644 > --- a/drivers/gpu/drm/i2c/tda998x_drv.c > +++ b/drivers/gpu/drm/i2c/tda998x_drv.c > @@ -1453,7 +1453,6 @@ static int tda998x_bind(struct device *dev, struct > device *master, void *data) > if (ret) > goto err_sysfs; > > - priv->connector.encoder = >encoder; while this patch helps (no WARN() printed) I'm not sure this is the right fix. TDA998x is also used by armada DRM driver which has not been converted (yet) to atomic modesetting. And judging by the code and comment of drm_connector_get_encoder() in drm_crtc.c, having access to the encoder through the connector is the non-atomic way of doing things: static struct drm_encoder *drm_connector_get_encoder(struct drm_connector *connector) { /* For atomic drivers only state objects are synchronously updated and * protected by modeset locks, so check those first. */ if (connector->state) return connector->state->best_encoder; return connector->encoder; } To me, it looks like the WARN() is bogus when the atomic modesetting is used in drm_atomic_helper_update_legacy_modeset_state(). Either we acknowledge that the "legacy" code sets the connector in the encoder structure before complete pipeline is setup and we remove the WARN(), or we find a better way of detecting that some sort of mixed support is in place (i.e. atomic and non-atomic aware code running) and we clean up in a way that is compatible with both worlds. Best regards, Liviu > drm_mode_connector_attach_encoder(>connector, >encoder); > > return 0; -- | I would like to | | fix the world, | | but they're not | | giving me the | \ source code! / --- ¯\_(ã)_/¯
DRM i2c module or bridge ?
On 12 November 2015 at 13:18, Thierry Reding wrote: > On Thu, Nov 12, 2015 at 12:48:51PM +, Emil Velikov wrote: >> Hello Thierry, all, >> >> Inspired by a recent discussion I was started wondering - where is the >> cut between DRM i2c modules (most of which encoders/transmitters) and >> bridge drivers (again some of which i2c encoders) ? Does anyone has >> some pointers on the topic ? > > DRM bridge is a superset of I2C encoders, so everything that I2C encoder > drivers do they should be able to do with DRM bridges, and more. There > isn't a strict guideline here, but I think there's general agreement > that new drivers should be using the DRM bridge framework. The primary > reason is that bridges integrate seamlessly with the driver model, that > is, the drivers that implement them are regular drivers that register > with the corresponding bus and get bound to a device, whereas the I2C > encoder infrastructure is mostly about manually instantiating devices. > > For existing drivers I guess they could all be converted, but doing so > may require a bit of work. They also tend to work as-is, so finding > volunteers to do the conversion is probably going to be difficult given > the lack of motivation. > > Thierry > >> Based on the above I did a very quick search for third party IP >> modules in the DRM subsystem: >> >> * i915 >> dvo_ch7017.c >> dvo_ch7xxx.c >> dvo_ivch.c >> dvo_ns2501.c >> dvo_sil164.c >> dvo_tfp410.c > > It looks like these use some framework that's custom to the i915 driver > but could otherwise easily be DRM bridges. > >> * gma500 >> tc35876x-dsi-lvds.c > > This seems to be some sort of hybrid bridge and panel driver. > >> * sti >> sti_hdmi_tx3g0c55phy.c >> sti_hdmi_tx3g4c28phy.c > > These seem to implement some sort of PHY interface, but from a quick > look moving these to the PHY framework seems overkill. They seem no good > fit for DRM bridge because they are not separate devices, but rather the > SoC generation specific bits of the STi HDMI driver. > >> (and for posterity) >> * i2c >> adv7511.c >> ch7006_drv.c >> sil164_drv.c >> tda998x_drv.c >> >> >> * bridge >> dw_hdmi.c >> nxp-ptn3460.c >> parade-ps8622.c >> >> >> By the looks of it, we can move rework (some of?) the above into >> i2c/bridge modules and in other cases (sil164) just use the existing >> one ? I'm neither volunteering nor suggesting people must work of >> these, merely curious. > > My take on this is that it's probably best to keep the above in their > current form. If they need to be shared across multiple hardware setups > it might make sense to convert them to DRM bridge drivers. > > For new drivers it's probably best to make them bridge drivers from the > start. > Thanks for the comprehensive reply Thierry. Pretty sure there are other people wondering about these - this should straighten things out. Just a small note: considering that most desktop GPUs are moving (have moved ?) away from third party encoders/transmitters I doubt we'll be seeing any movements on that front. Cheers, Emil
[PATCH 00/25] Exynos DRM: new life of IPP (Image Post Processing) subsystem
Hello, Daniel Stone wrote: > Hi Marek, > > On 10 November 2015 at 13:23, Marek Szyprowski > wrote: >> This patch series introduces a new life into Exynos IPP (Image Post >> Processing) subsystem by integrating it (transparently for userspace >> applications) with Exynos DRM core plane management. This means that all >> CRTC drivers transparently get support for standard features of IPP >> subsystem like rotation and scaling. >> >> Support for features not supported natively by CRTC drivers is >> implemented with a help of temporary framebuffers, where image data is >> processed by IPP subsystem before performing the scanout by a CRTC driver. >> >> This patchset is a first version of this 'new feature' and I would like >> get some comments on the proposed approach. I plan to continue working >> on enhancing Exynos DRM drivers and especially do the cleanup the IPP >> subsystem. >> >> Most of the new features are added by the last 2 patches. All other >> patches are bugfixes in various Exynos DRM subdrivers and significant >> core rewrite - introducing a subclass of drm_plane_state was needed and >> all drivers have been converted to use it. Some initial cleanups in IPP >> subsystem were also needed to let Exynos core to call it internally from >> the driver core. This part will be cleaned even more in the future. > > Hm, interesting. The RPi has a similar setup - VC4 can work either > online (realtime scanout) or offline (mem2mem). Once the scene crosses > a certain complexity boundary, it can no longer be composed in > realtime and must fall back to mem2mem before it can be displayed. > > There was talk of having the fallback handled transparently in KMS for > VC4 - similar to this - but the conclusion seemed to be that it was an > inappropriate level of hidden complexity for KMS, and instead would > best be handled by something like HWComposer directing it. Using HWC > would then let you more intelligently split the scene from userspace > (e.g. flatten some components but retain others as active planes). I would be intererested in the performance implications of this abstraction as well. I'd like to use the Exynos FIMC for CSC and scaling, but this operation of course takes some time. I wonder how this interacts with page flipping. If I queue a pageflip event with a buffer that needs to go through the IPP for display, where does the delay caused by the operation factor it? If I understand this correctly drmModePageFlip() still is going to return immediately, but I might miss the next vblank period because the FIMC is still working on the buffer. My problem here is that this abstraction would take too much control from the user. Correct me if I have this wrong! With best wishes, Tobias > > Dan V, Eric - thoughts? > > Cheers, > Daniel >
__i915_spin_request() sucks
On 11/12/2015 01:36 PM, Jens Axboe wrote: > Hi, > > So a few months ago I got an XPS13 laptop, the one with the high res > screen. GUI performance was never really that great, I attributed it to > coming from a more powerful laptop, and the i915 driving a lot of > pixels. But yesterday I browsed from my wife's macbook, and was blown > away. Wow, scrolling in chrome SUCKS on the xps13. Not just scrolling, > basically anything in chrome. Molasses. So I got sick of it, fired up a > quick perf record, did a bunch of stuff in chrome. No super smoking > guns, but one thing did stick out - the path leading to > __i915_spin_request(). > > So today, I figured I'd try just killing that spin. If it fails, we'll > punt to normal completions, so easy change. And wow, MASSIVE difference. > I can now scroll in chrome and not rage! It's like the laptop is 10x > faster now. > > Ran git blame, and found: > > commit 2def4ad99befa25775dd2f714fdd4d92faec6e34 > Author: Chris Wilson > Date: Tue Apr 7 16:20:41 2015 +0100 > > drm/i915: Optimistically spin for the request completion > > and read the commit message. Doesn't sound that impressive. Especially > not for something that screws up interactive performance by a LOT. > > What's the deal? Revert? BTW, this: "Limit the spinning to a single jiffie (~1us) at most" is totally wrong. I have HZ=100 on my laptop. That's 10ms. 10ms! Even if I had HZ=1000, that'd still be 1ms of spinning. That's seriously screwed up, guys. -- Jens Axboe
__i915_spin_request() sucks
Hi, So a few months ago I got an XPS13 laptop, the one with the high res screen. GUI performance was never really that great, I attributed it to coming from a more powerful laptop, and the i915 driving a lot of pixels. But yesterday I browsed from my wife's macbook, and was blown away. Wow, scrolling in chrome SUCKS on the xps13. Not just scrolling, basically anything in chrome. Molasses. So I got sick of it, fired up a quick perf record, did a bunch of stuff in chrome. No super smoking guns, but one thing did stick out - the path leading to __i915_spin_request(). So today, I figured I'd try just killing that spin. If it fails, we'll punt to normal completions, so easy change. And wow, MASSIVE difference. I can now scroll in chrome and not rage! It's like the laptop is 10x faster now. Ran git blame, and found: commit 2def4ad99befa25775dd2f714fdd4d92faec6e34 Author: Chris Wilson Date: Tue Apr 7 16:20:41 2015 +0100 drm/i915: Optimistically spin for the request completion and read the commit message. Doesn't sound that impressive. Especially not for something that screws up interactive performance by a LOT. What's the deal? Revert? -- Jens Axboe
drm: Bogus WARN() in drm_atomic_helper_update_legacy_modeset_state() ?
On Thu, Nov 12, 2015 at 02:27:29PM +0800, Mark yao wrote: > On 2015å¹´11æ11æ¥ 00:56, Thierry Reding wrote: > >On Tue, Nov 10, 2015 at 03:01:03PM +, Liviu Dudau wrote: > >>Hello, > >> > >>When booting my Juno board with the HDLCD driver that I have converted to > >>atomic operations I'm getting the following warning: > >Perhaps you can provide pointers to the source code, that might make it > >easier for people to spot what's going wrong. > > > >Thierry > > > > > >___ > >dri-devel mailing list > >dri-devel at lists.freedesktop.org > >http://lists.freedesktop.org/mailman/listinfo/dri-devel > Hi Thierry > I encountered the same problem as Liviu. > I'm coverting rockchip drm to atomic api, when booting with hdmi > connected, get under warning: > > [1.300156] WARNING: CPU: 0 PID: 26 at > drivers/gpu/drm/drm_atomic_helper.c:674 > drm_atomic_helper_update_legacy_modeset_state+0x3c/0x1f8() > [1.300161] Modules linked in: > [1.300171] CPU: 0 PID: 26 Comm: kworker/0:1 Not tainted 4.3.0-rc5+ #160 > [1.300174] Hardware name: Rockchip (Device Tree) > [1.300189] Workqueue: events output_poll_execute > [1.300224] [] (unwind_backtrace) from [] > (show_stack+0x10/0x14) > [1.300241] [] (show_stack) from [] > (dump_stack+0x6c/0x88) > [1.300255] [] (dump_stack) from [] > (warn_slowpath_common+0x80/0xa8) > [1.300265] [] (warn_slowpath_common) from [] > (warn_slowpath_null+0x18/0x1c) > [1.300277] [] (warn_slowpath_null) from [] > (drm_atomic_helper_update_legacy_modeset_state+0x3c/0x1f8) > [1.300293] [] (drm_atomic_helper_update_legacy_modeset_state) > from [] (drm_atomic_helper_commit_modeset_disables+0x208/0x364) > [1.300305] [] (drm_atomic_helper_commit_modeset_disables) from > [] (drm_atomic_helper_commit+0xf8/0x140) > [1.300320] [] (drm_atomic_helper_commit) from [] > (drm_atomic_commit+0x50/0x60) > [1.300333] [] (drm_atomic_commit) from [] > (restore_fbdev_mode+0xf4/0x278) > [1.300345] [] (restore_fbdev_mode) from [] > (drm_fb_helper_restore_fbdev_mode_unlocked+0x30/0x68) > [1.300357] [] (drm_fb_helper_restore_fbdev_mode_unlocked) from > [] (drm_fb_helper_set_par+0x3c/0x54) > [1.300368] [] (drm_fb_helper_set_par) from [] > (drm_fb_helper_hotplug_event+0xe4/0xfc) > [1.300382] [] (drm_fb_helper_hotplug_event) from [] > (drm_kms_helper_hotplug_event+0x24/0x28) > [1.300396] [] (drm_kms_helper_hotplug_event) from [] > (output_poll_execute+0x134/0x18c) > [1.300413] [] (output_poll_execute) from [] > (process_one_work+0x1e0/0x318) > [1.300426] [] (process_one_work) from [] > (worker_thread+0x378/0x4c4) > [1.300438] [] (worker_thread) from [] > (kthread+0xdc/0xf0) > [1.300450] [] (kthread) from [] > (ret_from_fork+0x14/0x3c) > > I can't found who can set encoder->crtc value before atomic_commit, what's > going wrong? The explanation here is probably the same as the one I gave to Liviu. Does the below help? Thierry --- >8 --- diff --git a/drivers/gpu/drm/bridge/dw_hdmi.c b/drivers/gpu/drm/bridge/dw_hdmi.c index 56de9f1c95fc..ffef392ccc13 100644 --- a/drivers/gpu/drm/bridge/dw_hdmi.c +++ b/drivers/gpu/drm/bridge/dw_hdmi.c @@ -1648,8 +1648,6 @@ static int dw_hdmi_register(struct drm_device *drm, struct dw_hdmi *hdmi) drm_connector_init(drm, >connector, _hdmi_connector_funcs, DRM_MODE_CONNECTOR_HDMIA); - hdmi->connector.encoder = encoder; - drm_mode_connector_attach_encoder(>connector, encoder); return 0; -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151112/799ea424/attachment.sig>
[PATCHv10 16/16] cobalt: add cec support
From: Hans VerkuilAdd CEC support to the cobalt driver. Signed-off-by: Hans Verkuil --- drivers/media/pci/cobalt/Kconfig | 1 + drivers/media/pci/cobalt/cobalt-driver.c | 108 +- drivers/media/pci/cobalt/cobalt-driver.h | 2 + drivers/media/pci/cobalt/cobalt-irq.c| 3 + drivers/media/pci/cobalt/cobalt-v4l2.c | 126 --- drivers/media/pci/cobalt/cobalt-v4l2.h | 2 + 6 files changed, 216 insertions(+), 26 deletions(-) diff --git a/drivers/media/pci/cobalt/Kconfig b/drivers/media/pci/cobalt/Kconfig index a01f0cc..9125747 100644 --- a/drivers/media/pci/cobalt/Kconfig +++ b/drivers/media/pci/cobalt/Kconfig @@ -4,6 +4,7 @@ config VIDEO_COBALT depends on PCI_MSI && MTD_COMPLEX_MAPPINGS depends on GPIOLIB || COMPILE_TEST depends on SND + select MEDIA_CEC select I2C_ALGOBIT select VIDEO_ADV7604 select VIDEO_ADV7511 diff --git a/drivers/media/pci/cobalt/cobalt-driver.c b/drivers/media/pci/cobalt/cobalt-driver.c index 8fed61e..0b9e194 100644 --- a/drivers/media/pci/cobalt/cobalt-driver.c +++ b/drivers/media/pci/cobalt/cobalt-driver.c @@ -76,6 +76,7 @@ static u8 edid[256] = { 0x0A, 0x0A, 0x0A, 0x0A, 0x00, 0x00, 0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x68, + 0x02, 0x03, 0x1a, 0xc0, 0x48, 0xa2, 0x10, 0x04, 0x02, 0x01, 0x21, 0x14, 0x13, 0x23, 0x09, 0x07, 0x07, 0x65, 0x03, 0x0c, 0x00, 0x10, 0x00, 0xe2, @@ -149,17 +150,44 @@ static void cobalt_notify(struct v4l2_subdev *sd, struct cobalt *cobalt = to_cobalt(sd->v4l2_dev); unsigned sd_nr = cobalt_get_sd_nr(sd); struct cobalt_stream *s = >streams[sd_nr]; - bool hotplug = arg ? *((int *)arg) : false; - if (s->is_output) + switch (notification) { + case V4L2_SUBDEV_CEC_TX_DONE: + cec_transmit_done(s->cec_adap, (unsigned long)arg); + return; + case V4L2_SUBDEV_CEC_RX_MSG: + cec_received_msg(s->cec_adap, arg); + return; + case V4L2_SUBDEV_CEC_CONN_INPUTS: + cec_connected_inputs(s->cec_adap, *(u16 *)arg); + return; + default: + break; + } + + if (s->is_output) { + switch (notification) { + case ADV7511_EDID_DETECT: { + struct adv7511_edid_detect *ed = arg; + + s->cec_adap->phys_addr = ed->phys_addr; + break; + } + } return; + } switch (notification) { - case ADV76XX_HOTPLUG: + case ADV76XX_HOTPLUG: { + bool hotplug = arg ? *((int *)arg) : false; + cobalt_s_bit_sysctrl(cobalt, COBALT_SYS_CTRL_HPD_TO_CONNECTOR_BIT(sd_nr), hotplug); + if (s->cec_adap) + cec_connected_inputs(s->cec_adap, hotplug); cobalt_dbg(1, "Set hotplug for adv %d to %d\n", sd_nr, hotplug); break; + } case V4L2_DEVICE_NOTIFY_EVENT: cobalt_dbg(1, "Format changed for adv %d\n", sd_nr); v4l2_event_queue(>vdev, arg); @@ -438,12 +466,15 @@ static void cobalt_stream_struct_init(struct cobalt *cobalt) for (i = 0; i < COBALT_NUM_STREAMS; i++) { struct cobalt_stream *s = >streams[i]; + struct video_device *vdev = >vdev; s->cobalt = cobalt; s->flags = 0; s->is_audio = false; s->is_output = false; s->is_dummy = true; + snprintf(vdev->name, sizeof(vdev->name), +"%s-%d", cobalt->v4l2_dev.name, i); /* The Memory DMA channels will always get a lower channel * number than the FIFO DMA. Video input should map to the @@ -485,6 +516,23 @@ static void cobalt_stream_struct_init(struct cobalt *cobalt) } } +static int cobalt_create_cec_adap(struct cobalt_stream *s) +{ + u32 caps = CEC_CAP_IO | CEC_CAP_LOG_ADDRS | + CEC_CAP_PASSTHROUGH | CEC_CAP_RC | + CEC_CAP_ARC | CEC_CAP_VENDOR_ID; + u8 nsinks = 0; + + if (s->is_output) + caps |= CEC_CAP_IS_SOURCE | CEC_CAP_CDC_HPD; + else + nsinks = 1; + s->cec_adap = cec_create_adapter(_cec_adap_ops, +s, s->vdev.name, caps, nsinks, +THIS_MODULE, >cobalt->pci_dev->dev); + return PTR_ERR_OR_ZERO(s->cec_adap); +} + static int cobalt_subdevs_init(struct cobalt *cobalt) { static struct adv76xx_platform_data adv7604_pdata = { @@ -508,10 +556,10 @@ static int cobalt_subdevs_init(struct cobalt *cobalt) .platform_data = _pdata, }; -
[PATCHv10 15/16] cec: s5p-cec: Add s5p-cec driver
From: Kamil DebskiAdd CEC interface driver present in the Samsung Exynos range of SoCs. The following files were based on work by SangPil Moon: - exynos_hdmi_cec.h - exynos_hdmi_cecctl.c Signed-off-by: Kamil Debski Signed-off-by: Hans Verkuil --- .../devicetree/bindings/media/s5p-cec.txt | 31 +++ drivers/media/platform/Kconfig | 12 + drivers/media/platform/Makefile| 1 + drivers/media/platform/s5p-cec/Makefile| 2 + drivers/media/platform/s5p-cec/exynos_hdmi_cec.h | 37 +++ .../media/platform/s5p-cec/exynos_hdmi_cecctrl.c | 208 +++ drivers/media/platform/s5p-cec/regs-cec.h | 96 +++ drivers/media/platform/s5p-cec/s5p_cec.c | 289 + drivers/media/platform/s5p-cec/s5p_cec.h | 76 ++ 9 files changed, 752 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/s5p-cec.txt create mode 100644 drivers/media/platform/s5p-cec/Makefile create mode 100644 drivers/media/platform/s5p-cec/exynos_hdmi_cec.h create mode 100644 drivers/media/platform/s5p-cec/exynos_hdmi_cecctrl.c create mode 100644 drivers/media/platform/s5p-cec/regs-cec.h create mode 100644 drivers/media/platform/s5p-cec/s5p_cec.c create mode 100644 drivers/media/platform/s5p-cec/s5p_cec.h diff --git a/Documentation/devicetree/bindings/media/s5p-cec.txt b/Documentation/devicetree/bindings/media/s5p-cec.txt new file mode 100644 index 000..925ab4d --- /dev/null +++ b/Documentation/devicetree/bindings/media/s5p-cec.txt @@ -0,0 +1,31 @@ +* Samsung HDMI CEC driver + +The HDMI CEC module is present is Samsung SoCs and its purpose is to +handle communication between HDMI connected devices over the CEC bus. + +Required properties: + - compatible : value should be following + "samsung,s5p-cec" + + - reg : Physical base address of the IP registers and length of memory + mapped region. + + - interrupts : HDMI CEC interrupt number to the CPU. + - clocks : from common clock binding: handle to HDMI CEC clock. + - clock-names : from common clock binding: must contain "hdmicec", + corresponding to entry in the clocks property. + - samsung,syscon-phandle - phandle to the PMU system controller + +Example: + +hdmicec: cec at 100B { + compatible = "samsung,s5p-cec"; + reg = <0x100B 0x200>; + interrupts = <0 114 0>; + clocks = < CLK_HDMI_CEC>; + clock-names = "hdmicec"; + samsung,syscon-phandle = <_system_controller>; + pinctrl-names = "default"; + pinctrl-0 = <_cec>; + status = "okay"; +}; diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig index ccbc974..91794a6 100644 --- a/drivers/media/platform/Kconfig +++ b/drivers/media/platform/Kconfig @@ -117,6 +117,18 @@ config VIDEO_S3C_CAMIF source "drivers/media/platform/soc_camera/Kconfig" source "drivers/media/platform/exynos4-is/Kconfig" source "drivers/media/platform/s5p-tv/Kconfig" + +config VIDEO_SAMSUNG_S5P_CEC + tristate "Samsung S5P CEC driver" + depends on VIDEO_DEV && VIDEO_V4L2 && (PLAT_S5P || ARCH_EXYNOS || COMPILE_TEST) + select MEDIA_CEC + default n + ---help--- + This is a driver for Samsung S5P HDMI CEC interface. It uses the + generic CEC framework interface. + CEC bus is present in the HDMI connector and enables communication + between compatible devices. + source "drivers/media/platform/am437x/Kconfig" source "drivers/media/platform/xilinx/Kconfig" diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile index efa0295..957af5f 100644 --- a/drivers/media/platform/Makefile +++ b/drivers/media/platform/Makefile @@ -27,6 +27,7 @@ obj-$(CONFIG_VIDEO_MEM2MEM_DEINTERLACE) += m2m-deinterlace.o obj-$(CONFIG_VIDEO_S3C_CAMIF) += s3c-camif/ obj-$(CONFIG_VIDEO_SAMSUNG_EXYNOS4_IS) += exynos4-is/ +obj-$(CONFIG_VIDEO_SAMSUNG_S5P_CEC)+= s5p-cec/ obj-$(CONFIG_VIDEO_SAMSUNG_S5P_JPEG) += s5p-jpeg/ obj-$(CONFIG_VIDEO_SAMSUNG_S5P_MFC)+= s5p-mfc/ obj-$(CONFIG_VIDEO_SAMSUNG_S5P_TV) += s5p-tv/ diff --git a/drivers/media/platform/s5p-cec/Makefile b/drivers/media/platform/s5p-cec/Makefile new file mode 100644 index 000..0e2cf45 --- /dev/null +++ b/drivers/media/platform/s5p-cec/Makefile @@ -0,0 +1,2 @@ +obj-$(CONFIG_VIDEO_SAMSUNG_S5P_CEC)+= s5p-cec.o +s5p-cec-y += s5p_cec.o exynos_hdmi_cecctrl.o diff --git a/drivers/media/platform/s5p-cec/exynos_hdmi_cec.h b/drivers/media/platform/s5p-cec/exynos_hdmi_cec.h new file mode 100644 index 000..d008695 --- /dev/null +++ b/drivers/media/platform/s5p-cec/exynos_hdmi_cec.h @@ -0,0 +1,37 @@ +/* drivers/media/platform/s5p-cec/exynos_hdmi_cec.h + * + * Copyright (c) 2010, 2014 Samsung Electronics + * http://www.samsung.com/ + * + * Header file for interface of Samsung Exynos hdmi cec hardware + * + * This program is free
[PATCHv10 14/16] cec: adv7511: add cec support.
From: Hans VerkuilAdd CEC support to the adv7511 driver. Signed-off-by: Hans Verkuil [k.debski at samsung.com: Merged changes from CEC Updates commit by Hans Verkuil] Signed-off-by: Kamil Debski Signed-off-by: Hans Verkuil --- drivers/media/i2c/adv7511.c | 364 +++- include/media/adv7511.h | 6 +- 2 files changed, 358 insertions(+), 12 deletions(-) diff --git a/drivers/media/i2c/adv7511.c b/drivers/media/i2c/adv7511.c index e4900df..dee73a6 100644 --- a/drivers/media/i2c/adv7511.c +++ b/drivers/media/i2c/adv7511.c @@ -33,6 +33,7 @@ #include #include #include +#include static int debug; module_param(debug, int, 0644); @@ -59,6 +60,8 @@ MODULE_LICENSE("GPL v2"); #define ADV7511_MIN_PIXELCLOCK 2000 #define ADV7511_MAX_PIXELCLOCK 22500 +#define ADV7511_MAX_ADDRS (3) + /* ** * @@ -90,12 +93,19 @@ struct adv7511_state { struct v4l2_ctrl_handler hdl; int chip_revision; u8 i2c_edid_addr; - u8 i2c_cec_addr; u8 i2c_pktmem_addr; + u8 i2c_cec_addr; + + struct i2c_client *i2c_cec; + u8 cec_addr[ADV7511_MAX_ADDRS]; + u8 cec_valid_addrs; + bool cec_enabled_adap; + /* Is the adv7511 powered on? */ bool power_on; /* Did we receive hotplug and rx-sense signals? */ bool have_monitor; + bool enabled_irq; /* timings from s_dv_timings */ struct v4l2_dv_timings dv_timings; u32 fmt_code; @@ -225,7 +235,7 @@ static int adv_smbus_read_i2c_block_data(struct i2c_client *client, return ret; } -static inline void adv7511_edid_rd(struct v4l2_subdev *sd, u16 len, u8 *buf) +static void adv7511_edid_rd(struct v4l2_subdev *sd, uint16_t len, uint8_t *buf) { struct adv7511_state *state = get_adv7511_state(sd); int i; @@ -240,6 +250,34 @@ static inline void adv7511_edid_rd(struct v4l2_subdev *sd, u16 len, u8 *buf) v4l2_err(sd, "%s: i2c read error\n", __func__); } +static inline int adv7511_cec_read(struct v4l2_subdev *sd, u8 reg) +{ + struct adv7511_state *state = get_adv7511_state(sd); + + return i2c_smbus_read_byte_data(state->i2c_cec, reg); +} + +static int adv7511_cec_write(struct v4l2_subdev *sd, u8 reg, u8 val) +{ + struct adv7511_state *state = get_adv7511_state(sd); + int ret; + int i; + + for (i = 0; i < 3; i++) { + ret = i2c_smbus_write_byte_data(state->i2c_cec, reg, val); + if (ret == 0) + return 0; + } + v4l2_err(sd, "%s: I2C Write Problem\n", __func__); + return ret; +} + +static inline int adv7511_cec_write_and_or(struct v4l2_subdev *sd, u8 reg, u8 mask, + u8 val) +{ + return adv7511_cec_write(sd, reg, (adv7511_cec_read(sd, reg) & mask) | val); +} + static int adv7511_pktmem_rd(struct v4l2_subdev *sd, u8 reg) { struct adv7511_state *state = get_adv7511_state(sd); @@ -413,16 +451,28 @@ static const struct v4l2_ctrl_ops adv7511_ctrl_ops = { #ifdef CONFIG_VIDEO_ADV_DEBUG static void adv7511_inv_register(struct v4l2_subdev *sd) { + struct adv7511_state *state = get_adv7511_state(sd); + v4l2_info(sd, "0x000-0x0ff: Main Map\n"); + if (state->i2c_cec) + v4l2_info(sd, "0x100-0x1ff: CEC Map\n"); } static int adv7511_g_register(struct v4l2_subdev *sd, struct v4l2_dbg_register *reg) { + struct adv7511_state *state = get_adv7511_state(sd); + reg->size = 1; switch (reg->reg >> 8) { case 0: reg->val = adv7511_rd(sd, reg->reg & 0xff); break; + case 1: + if (state->i2c_cec) { + reg->val = adv7511_cec_read(sd, reg->reg & 0xff); + break; + } + /* fall through */ default: v4l2_info(sd, "Register %03llx not supported\n", reg->reg); adv7511_inv_register(sd); @@ -433,10 +483,18 @@ static int adv7511_g_register(struct v4l2_subdev *sd, struct v4l2_dbg_register * static int adv7511_s_register(struct v4l2_subdev *sd, const struct v4l2_dbg_register *reg) { + struct adv7511_state *state = get_adv7511_state(sd); + switch (reg->reg >> 8) { case 0: adv7511_wr(sd, reg->reg & 0xff, reg->val & 0xff); break; + case 1: + if (state->i2c_cec) { + adv7511_cec_write(sd, reg->reg & 0xff, reg->val & 0xff); + break; + } + /* fall through */ default: v4l2_info(sd, "Register %03llx not supported\n", reg->reg); adv7511_inv_register(sd); @@ -524,6 +582,7 @@ static int adv7511_log_status(struct v4l2_subdev *sd) { struct adv7511_state *state =
[PATCHv10 13/16] cec: adv7842: add cec support
From: Hans VerkuilAdd CEC support to the adv7842 driver. Signed-off-by: Hans Verkuil --- drivers/media/i2c/adv7842.c | 255 +--- 1 file changed, 238 insertions(+), 17 deletions(-) diff --git a/drivers/media/i2c/adv7842.c b/drivers/media/i2c/adv7842.c index b7269b8..0f29b33 100644 --- a/drivers/media/i2c/adv7842.c +++ b/drivers/media/i2c/adv7842.c @@ -39,6 +39,7 @@ #include #include #include +#include #include #include #include @@ -79,6 +80,8 @@ MODULE_LICENSE("GPL"); #define ADV7842_OP_SWAP_CB_CR (1 << 0) +#define ADV7842_MAX_ADDRS (3) + /* ** * @@ -142,6 +145,10 @@ struct adv7842_state { struct v4l2_ctrl *free_run_color_ctrl_manual; struct v4l2_ctrl *free_run_color_ctrl; struct v4l2_ctrl *rgb_quantization_range_ctrl; + + u8 cec_addr[ADV7842_MAX_ADDRS]; + u8 cec_valid_addrs; + bool cec_enabled_adap; }; /* Unsupported timings. This device cannot support 720p30. */ @@ -418,9 +425,9 @@ static inline int cec_write(struct v4l2_subdev *sd, u8 reg, u8 val) return adv_smbus_write_byte_data(state->i2c_cec, reg, val); } -static inline int cec_write_and_or(struct v4l2_subdev *sd, u8 reg, u8 mask, u8 val) +static inline int cec_write_clr_set(struct v4l2_subdev *sd, u8 reg, u8 mask, u8 val) { - return cec_write(sd, reg, (cec_read(sd, reg) & mask) | val); + return cec_write(sd, reg, (cec_read(sd, reg) & ~mask) | val); } static inline int infoframe_read(struct v4l2_subdev *sd, u8 reg) @@ -696,6 +703,18 @@ adv7842_get_dv_timings_cap(struct v4l2_subdev *sd) /* --- */ +static u16 adv7842_read_cable_det(struct v4l2_subdev *sd) +{ + u8 reg = io_read(sd, 0x6f); + u16 val = 0; + + if (reg & 0x02) + val |= 1; /* port A */ + if (reg & 0x01) + val |= 2; /* port B */ + return val; +} + static void adv7842_delayed_work_enable_hotplug(struct work_struct *work) { struct delayed_work *dwork = to_delayed_work(work); @@ -703,8 +722,13 @@ static void adv7842_delayed_work_enable_hotplug(struct work_struct *work) struct adv7842_state, delayed_work_enable_hotplug); struct v4l2_subdev *sd = >sd; int present = state->hdmi_edid.present; + u16 connected_inputs; u8 mask = 0; + connected_inputs = present & adv7842_read_cable_det(sd); + v4l2_subdev_notify(sd, V4L2_SUBDEV_CEC_CONN_INPUTS, + _inputs); + v4l2_dbg(2, debug, sd, "%s: enable hotplug on ports: 0x%x\n", __func__, present); @@ -983,20 +1007,16 @@ static int adv7842_s_register(struct v4l2_subdev *sd, static int adv7842_s_detect_tx_5v_ctrl(struct v4l2_subdev *sd) { struct adv7842_state *state = to_state(sd); - int prev = v4l2_ctrl_g_ctrl(state->detect_tx_5v_ctrl); - u8 reg_io_6f = io_read(sd, 0x6f); - int val = 0; + u16 cable_det = adv7842_read_cable_det(sd); + u16 connected_inputs; - if (reg_io_6f & 0x02) - val |= 1; /* port A */ - if (reg_io_6f & 0x01) - val |= 2; /* port B */ + v4l2_dbg(1, debug, sd, "%s: 0x%x\n", __func__, cable_det); - v4l2_dbg(1, debug, sd, "%s: 0x%x -> 0x%x\n", __func__, prev, val); + connected_inputs = state->hdmi_edid.present & cable_det; + v4l2_subdev_notify(sd, V4L2_SUBDEV_CEC_CONN_INPUTS, + _inputs); - if (val != prev) - return v4l2_ctrl_s_ctrl(state->detect_tx_5v_ctrl, val); - return 0; + return v4l2_ctrl_s_ctrl(state->detect_tx_5v_ctrl, cable_det); } static int find_and_set_predefined_video_timings(struct v4l2_subdev *sd, @@ -2157,6 +2177,183 @@ static void adv7842_irq_enable(struct v4l2_subdev *sd, bool enable) } } +static void adv7842_cec_tx_raw_status(struct v4l2_subdev *sd, u8 tx_raw_status) +{ + if ((cec_read(sd, 0x11) & 0x01) == 0) { + v4l2_dbg(1, debug, sd, "%s: tx raw: tx disabled\n", __func__); + return; + } + + if (tx_raw_status & 0x02) { + v4l2_dbg(1, debug, sd, "%s: tx raw: arbitration lost\n", +__func__); + v4l2_subdev_notify(sd, V4L2_SUBDEV_CEC_TX_DONE, + (void *)CEC_TX_STATUS_ARB_LOST); + return; + } + if (tx_raw_status & 0x04) { + v4l2_dbg(1, debug, sd, "%s: tx raw: retry failed\n", __func__); + v4l2_subdev_notify(sd, V4L2_SUBDEV_CEC_TX_DONE, + (void *)CEC_TX_STATUS_RETRY_TIMEOUT); + return; + } + if (tx_raw_status & 0x01) { + v4l2_dbg(1, debug, sd, "%s: tx raw: ready ok\n", __func__); +
[PATCHv10 12/16] cec: adv7604: add cec support.
From: Hans VerkuilAdd CEC support to the adv7604 driver. Signed-off-by: Hans Verkuil [k.debski at samsung.com: Merged changes from CEC Updates commit by Hans Verkuil] [k.debski at samsung.com: add missing methods cec/io_write_and_or] [k.debski at samsung.com: change adv7604 to adv76xx in added functions] [hansverk at cisco.com: use _clr_set instead of _and_or] --- drivers/media/i2c/adv7604.c | 241 +++- 1 file changed, 237 insertions(+), 4 deletions(-) diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c index 5631ec0..f039ae6 100644 --- a/drivers/media/i2c/adv7604.c +++ b/drivers/media/i2c/adv7604.c @@ -40,6 +40,7 @@ #include #include +#include #include #include #include @@ -80,6 +81,8 @@ MODULE_LICENSE("GPL"); #define ADV76XX_OP_SWAP_CB_CR (1 << 0) +#define ADV76XX_MAX_ADDRS (3) + enum adv76xx_type { ADV7604, ADV7611, @@ -184,6 +187,10 @@ struct adv76xx_state { u16 spa_port_a[2]; struct v4l2_fract aspect_ratio; u32 rgb_quantization_range; + u8 cec_addr[ADV76XX_MAX_ADDRS]; + u8 cec_valid_addrs; + bool cec_enabled_adap; + struct workqueue_struct *work_queues; struct delayed_work delayed_work_enable_hotplug; bool restart_stdi_once; @@ -430,7 +437,8 @@ static inline int io_write(struct v4l2_subdev *sd, u8 reg, u8 val) return regmap_write(state->regmap[ADV76XX_PAGE_IO], reg, val); } -static inline int io_write_clr_set(struct v4l2_subdev *sd, u8 reg, u8 mask, u8 val) +static inline int io_write_clr_set(struct v4l2_subdev *sd, u8 reg, u8 mask, + u8 val) { return io_write(sd, reg, (io_read(sd, reg) & ~mask) | val); } @@ -463,6 +471,12 @@ static inline int cec_write(struct v4l2_subdev *sd, u8 reg, u8 val) return regmap_write(state->regmap[ADV76XX_PAGE_CEC], reg, val); } +static inline int cec_write_clr_set(struct v4l2_subdev *sd, u8 reg, u8 mask, + u8 val) +{ + return cec_write(sd, reg, (cec_read(sd, reg) & ~mask) | val); +} + static inline int infoframe_read(struct v4l2_subdev *sd, u8 reg) { struct adv76xx_state *state = to_state(sd); @@ -550,8 +564,13 @@ static inline int edid_write_block(struct v4l2_subdev *sd, static void adv76xx_set_hpd(struct adv76xx_state *state, unsigned int hpd) { + u16 connected_inputs; unsigned int i; + connected_inputs = hpd & state->info->read_cable_det(>sd); + v4l2_subdev_notify(>sd, V4L2_SUBDEV_CEC_CONN_INPUTS, + _inputs); + for (i = 0; i < state->info->num_dv_ports; ++i) gpiod_set_value_cansleep(state->hpd_gpio[i], hpd & BIT(i)); @@ -891,9 +910,12 @@ static int adv76xx_s_detect_tx_5v_ctrl(struct v4l2_subdev *sd) { struct adv76xx_state *state = to_state(sd); const struct adv76xx_chip_info *info = state->info; + u16 cable_det = info->read_cable_det(sd); + u16 connected_inputs = state->edid.present & cable_det; - return v4l2_ctrl_s_ctrl(state->detect_tx_5v_ctrl, - info->read_cable_det(sd)); + v4l2_subdev_notify(sd, V4L2_SUBDEV_CEC_CONN_INPUTS, + _inputs); + return v4l2_ctrl_s_ctrl(state->detect_tx_5v_ctrl, cable_det); } static int find_and_set_predefined_video_timings(struct v4l2_subdev *sd, @@ -1914,6 +1936,187 @@ static int adv76xx_set_format(struct v4l2_subdev *sd, return 0; } +static void adv76xx_cec_tx_raw_status(struct v4l2_subdev *sd, u8 tx_raw_status) +{ + if ((cec_read(sd, 0x11) & 0x01) == 0) { + v4l2_dbg(1, debug, sd, "%s: tx raw: tx disabled\n", __func__); + return; + } + + if (tx_raw_status & 0x02) { + v4l2_dbg(1, debug, sd, "%s: tx raw: arbitration lost\n", +__func__); + v4l2_subdev_notify(sd, V4L2_SUBDEV_CEC_TX_DONE, + (void *)CEC_TX_STATUS_ARB_LOST); + return; + } + if (tx_raw_status & 0x04) { + v4l2_dbg(1, debug, sd, "%s: tx raw: retry failed\n", __func__); + v4l2_subdev_notify(sd, V4L2_SUBDEV_CEC_TX_DONE, + (void *)CEC_TX_STATUS_RETRY_TIMEOUT); + return; + } + if (tx_raw_status & 0x01) { + v4l2_dbg(1, debug, sd, "%s: tx raw: ready ok\n", __func__); + v4l2_subdev_notify(sd, V4L2_SUBDEV_CEC_TX_DONE, + (void *)CEC_TX_STATUS_OK); + return; + } +} + +static void adv76xx_cec_isr(struct v4l2_subdev *sd, bool *handled) +{ + u8 cec_irq; + + /* cec controller */ + cec_irq = io_read(sd, 0x4d) & 0x0f; + if (!cec_irq) + return; + + v4l2_dbg(1, debug, sd, "%s: cec: irq 0x%x\n", __func__, cec_irq); +
[PATCHv10 11/16] v4l2-subdev: add HDMI CEC ops
From: Hans VerkuilAdd CEC callbacks to the new v4l2_subdev_cec_ops struct. These are the low-level CEC ops that subdevs that support CEC have to implement. Signed-off-by: Hans Verkuil [k.debski at samsung.com: Merged changes from CEC Updates commit by Hans Verkuil] Signed-off-by: Kamil Debski --- include/media/v4l2-subdev.h | 14 ++ 1 file changed, 14 insertions(+) diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h index b273cf9..e55031e 100644 --- a/include/media/v4l2-subdev.h +++ b/include/media/v4l2-subdev.h @@ -42,6 +42,10 @@ #defineV4L2_DEVICE_NOTIFY_EVENT_IOW('v', 2, struct v4l2_event) +#define V4L2_SUBDEV_CEC_TX_DONE_IOW('v', 3, u32) +#define V4L2_SUBDEV_CEC_RX_MSG _IOW('v', 4, struct cec_msg) +#define V4L2_SUBDEV_CEC_CONN_INPUTS_IOW('v', 5, u16) + struct v4l2_device; struct v4l2_ctrl_handler; struct v4l2_event; @@ -51,6 +55,7 @@ struct v4l2_subdev; struct v4l2_subdev_fh; struct tuner_setup; struct v4l2_mbus_frame_desc; +struct cec_msg; /* decode_vbi_line */ struct v4l2_decode_vbi_line { @@ -642,6 +647,14 @@ struct v4l2_subdev_pad_ops { struct v4l2_mbus_frame_desc *fd); }; +struct v4l2_subdev_cec_ops { + unsigned (*available_log_addrs)(struct v4l2_subdev *sd); + int (*enable)(struct v4l2_subdev *sd, bool enable); + int (*log_addr)(struct v4l2_subdev *sd, u8 logical_addr); + int (*transmit)(struct v4l2_subdev *sd, u32 timeout_ms, + u8 retries, struct cec_msg *msg); +}; + struct v4l2_subdev_ops { const struct v4l2_subdev_core_ops *core; const struct v4l2_subdev_tuner_ops *tuner; @@ -651,6 +664,7 @@ struct v4l2_subdev_ops { const struct v4l2_subdev_ir_ops *ir; const struct v4l2_subdev_sensor_ops *sensor; const struct v4l2_subdev_pad_ops*pad; + const struct v4l2_subdev_cec_ops*cec; }; /* -- 2.6.2
[PATCHv10 10/16] DocBook/media: add CEC documentation
From: Hans VerkuilAdd DocBook documentation for the CEC API. Signed-off-by: Hans Verkuil [k.debski at samsung.com: add documentation for passthrough mode] [k.debski at samsung.com: minor fixes and change of reserved field sizes] Signed-off-by: Kamil Debski Signed-off-by: Hans Verkuil --- Documentation/DocBook/device-drivers.tmpl | 3 + Documentation/DocBook/media/Makefile | 2 + Documentation/DocBook/media/v4l/biblio.xml | 10 + Documentation/DocBook/media/v4l/cec-api.xml| 76 + 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-adap-g-caps.xml | 167 +++ .../DocBook/media/v4l/cec-ioc-adap-g-log-addrs.xml | 306 + .../DocBook/media/v4l/cec-ioc-adap-g-phys-addr.xml | 80 ++ .../DocBook/media/v4l/cec-ioc-adap-g-state.xml | 87 ++ .../DocBook/media/v4l/cec-ioc-adap-g-vendor-id.xml | 70 + Documentation/DocBook/media/v4l/cec-ioc-claim.xml | 71 + .../DocBook/media/v4l/cec-ioc-dqevent.xml | 208 ++ .../DocBook/media/v4l/cec-ioc-g-monitor.xml| 86 ++ .../DocBook/media/v4l/cec-ioc-g-passthrough.xml| 81 ++ .../DocBook/media/v4l/cec-ioc-receive.xml | 190 + Documentation/DocBook/media_api.tmpl | 6 +- 19 files changed, 1757 insertions(+), 1 deletion(-) 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-adap-g-caps.xml create mode 100644 Documentation/DocBook/media/v4l/cec-ioc-adap-g-log-addrs.xml create mode 100644 Documentation/DocBook/media/v4l/cec-ioc-adap-g-phys-addr.xml create mode 100644 Documentation/DocBook/media/v4l/cec-ioc-adap-g-state.xml create mode 100644 Documentation/DocBook/media/v4l/cec-ioc-adap-g-vendor-id.xml create mode 100644 Documentation/DocBook/media/v4l/cec-ioc-claim.xml create mode 100644 Documentation/DocBook/media/v4l/cec-ioc-dqevent.xml create mode 100644 Documentation/DocBook/media/v4l/cec-ioc-g-monitor.xml create mode 100644 Documentation/DocBook/media/v4l/cec-ioc-g-passthrough.xml create mode 100644 Documentation/DocBook/media/v4l/cec-ioc-receive.xml diff --git a/Documentation/DocBook/device-drivers.tmpl b/Documentation/DocBook/device-drivers.tmpl index 42a2d85..fade76c 100644 --- a/Documentation/DocBook/device-drivers.tmpl +++ b/Documentation/DocBook/device-drivers.tmpl @@ -319,6 +319,9 @@ X!Isound/sound_firmware.c !Iinclude/media/media-devnode.h !Iinclude/media/media-entity.h + Consumer Electronics Control devices +!Iinclude/media/cec.h + diff --git a/Documentation/DocBook/media/Makefile b/Documentation/DocBook/media/Makefile index 08527e7..b97fb71 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) \ DEFINES = \ @@ -100,6 +101,7 @@ STRUCTS = \ $(shell perl -ne 'print "$$1 " if (/^struct\s+([^\s]+)\s+/ && !/_old/)' $(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) diff --git a/Documentation/DocBook/media/v4l/biblio.xml b/Documentation/DocBook/media/v4l/biblio.xml index 9beb30f..87f1d24 100644 --- a/Documentation/DocBook/media/v4l/biblio.xml +++ b/Documentation/DocBook/media/v4l/biblio.xml @@ -342,6 +342,16 @@ in the frequency range from 87,5 to 108,0 MHz
[PATCHv10 09/16] cec.txt: add CEC framework documentation
Document the new HDMI CEC framework. Signed-off-by: Hans Verkuil [k.debski at samsung.com: add DocBook documentation by Hans Verkuil, with Signed-off-by: Kamil Debski Signed-off-by: Hans Verkuil --- Documentation/cec.txt | 326 ++ 1 file changed, 326 insertions(+) create mode 100644 Documentation/cec.txt diff --git a/Documentation/cec.txt b/Documentation/cec.txt new file mode 100644 index 000..90c2c7f --- /dev/null +++ b/Documentation/cec.txt @@ -0,0 +1,326 @@ +CEC Kernel Support +== + +The CEC framework provides a unified kernel interface for use with HDMI CEC +hardware. It is designed to handle a multiple types of hardware (receivers, +transmitters, USB dongles). The framework also gives the option to decide +what to do in the kernel driver and what should be handled by userspace +applications. In addition it integrates the remote control passthrough +feature into the kernel's remote control framework. + + +The CEC Protocol + + +The CEC protocol enables consumer electronic devices to communicate with each +other through the HDMI connection. The protocol uses logical addresses in the +communication. The logical address is strictly connected with the functionality +provided by the device. The TV acting as the communication hub is always +assigned address 0. The physical address is determined by the physical +connection between devices. + +The CEC framework described here is up to date with the CEC 2.0 specification. +It is documented in the HDMI 1.4 specification with the new 2.0 bits documented +in the HDMI 2.0 specification. But for most of the features the freely available +HDMI 1.3a specification is sufficient: + +http://www.microprocessor.org/HDMISpecification13a.pdf + + +The Kernel Interface + + +CEC Adapter +--- + +The struct cec_adapter represents the CEC adapter hardware. It is created by +calling cec_create_adapter() and deleted by calling cec_delete_adapter(): + +struct cec_adapter *cec_create_adapter(const struct cec_adap_ops *ops, + void *priv, const char *name, u32 caps, + u8 ninputs, struct module *owner, struct device *parent); +void cec_delete_adapter(struct cec_adapter *adap); + +To create an adapter you need to pass the following information: + +ops: adapter operations which are called by the CEC framework and that you +have to implement. + +priv: will be stored in adap->priv and can be used by the adapter ops. + +name: the name of the CEC adapter + +caps: capabilities of the CEC adapter. These capabilities determine the + capabilities of the hardware and which parts are to be handled + by userspace and which parts are handled by kernelspace. The + capabilities are returned by CEC_ADAP_G_CAPS. + +ninputs: the number of HDMI inputs of the device. This may be 0. This + is returned by CEC_ADAP_G_CAPS. + +owner: the module owner. + +parent: the parent device. + + +After creating the adapter the driver can modify the following fields +in struct cec_adapter: + + u8 available_log_addrs; + +This determines the number of simultaneous logical addresses the hardware +can program. Often this is 1, which is also the default. + + u8 pwr_state; + +The CEC_MSG_GIVE_DEVICE_POWER_STATUS power state. By default this is +CEC_OP_POWER_STATUS_ON (0). The driver can change this to signal power +state transitions. + + u16 phys_addr; + +By default this is 0x, but drivers can change this. The phys_addr field +must be set before the CEC adapter is enabled (see the adap_enable op below). +While the CEC adapter remains enabled it cannot be changed. Drivers never set +this if CEC_CAP_PHYS_ADDR is set. + + u32 vendor_id; + +By default this is CEC_VENDOR_ID_NONE (0x). It should not be changed +once the adapter is configured. Drivers never set this if CEC_CAP_VENDOR_ID +is set. + + u8 cec_version; + +The CEC version that the framework should support. By default this is the +latest version, but it can be changed to an older version, causing attempts +to use later extensions to fail. Obviously this should be set before the +CEC adapter is enabled. + +To register the /dev/cecX device node and the remote control device (if +CEC_CAP_RC is set) you call: + +int cec_register_adapter(struct cec_adapter *adap); + +To unregister the devices call: + +void cec_unregister_adapter(struct cec_adapter *adap); + + +Implementing the Low-Level CEC Adapter +-- + +The following low-level adapter operations have to be implemented in +your driver: + +struct cec_adap_ops { + /* Low-level callbacks */ + int (*adap_enable)(struct cec_adapter *adap, bool enable); + int (*adap_log_addr)(struct cec_adapter *adap, u8 logical_addr); + int (*adap_transmit)(struct cec_adapter *adap, u32 timeout_ms, struct cec_msg *msg); + + /* High-level callbacks */ + ...
[PATCHv10 08/16] cec: add compat32 ioctl support
From: Hans VerkuilThe CEC ioctls didn't have compat32 support, so they returned -ENOTTY when used in a 32 bit application on a 64 bit kernel. Since all the CEC ioctls are 32-bit compatible adding support for this API is trivial. Signed-off-by: Hans Verkuil --- fs/compat_ioctl.c | 19 +++ 1 file changed, 19 insertions(+) diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c index 48851f6..c8651aa 100644 --- a/fs/compat_ioctl.c +++ b/fs/compat_ioctl.c @@ -57,6 +57,7 @@ #include #include #include +#include #include #include @@ -1381,6 +1382,24 @@ COMPATIBLE_IOCTL(VIDEO_GET_NAVI) COMPATIBLE_IOCTL(VIDEO_SET_ATTRIBUTES) COMPATIBLE_IOCTL(VIDEO_GET_SIZE) COMPATIBLE_IOCTL(VIDEO_GET_FRAME_RATE) +/* cec */ +COMPATIBLE_IOCTL(CEC_ADAP_G_CAPS) +COMPATIBLE_IOCTL(CEC_ADAP_G_LOG_ADDRS) +COMPATIBLE_IOCTL(CEC_ADAP_S_LOG_ADDRS) +COMPATIBLE_IOCTL(CEC_ADAP_G_STATE) +COMPATIBLE_IOCTL(CEC_ADAP_S_STATE) +COMPATIBLE_IOCTL(CEC_ADAP_G_PHYS_ADDR) +COMPATIBLE_IOCTL(CEC_ADAP_S_PHYS_ADDR) +COMPATIBLE_IOCTL(CEC_ADAP_G_VENDOR_ID) +COMPATIBLE_IOCTL(CEC_ADAP_S_VENDOR_ID) +COMPATIBLE_IOCTL(CEC_G_MONITOR) +COMPATIBLE_IOCTL(CEC_S_MONITOR) +COMPATIBLE_IOCTL(CEC_CLAIM) +COMPATIBLE_IOCTL(CEC_RELEASE) +COMPATIBLE_IOCTL(CEC_G_PASSTHROUGH) +COMPATIBLE_IOCTL(CEC_TRANSMIT) +COMPATIBLE_IOCTL(CEC_RECEIVE) +COMPATIBLE_IOCTL(CEC_DQEVENT) /* joystick */ COMPATIBLE_IOCTL(JSIOCGVERSION) -- 2.6.2
[PATCHv10 07/16] cec: add HDMI CEC framework
The added HDMI CEC framework provides a generic kernel interface for HDMI CEC devices. Signed-off-by: Hans Verkuil [k.debski at samsung.com: Merged CEC Updates commit by Hans Verkuil] [k.debski at samsung.com: Merged Update author commit by Hans Verkuil] [k.debski at samsung.com: change kthread handling when setting logical address] [k.debski at samsung.com: code cleanup and fixes] [k.debski at samsung.com: add missing CEC commands to match spec] [k.debski at samsung.com: add RC framework support] [k.debski at samsung.com: move and edit documentation] [k.debski at samsung.com: add vendor id reporting] [k.debski at samsung.com: add possibility to clear assigned logical addresses] [k.debski at samsung.com: documentation fixes, clenaup and expansion] [k.debski at samsung.com: reorder of API structs and add reserved fields] [k.debski at samsung.com: fix handling of events and fix 32/64bit timespec problem] [k.debski at samsung.com: add cec.h to include/uapi/linux/Kbuild] [k.debski at samsung.com: add sequence number handling] [k.debski at samsung.com: add passthrough mode] [k.debski at samsung.com: fix CEC defines, add missing CEC 2.0 commands] minor additions] Signed-off-by: Kamil Debski --- MAINTAINERS| 12 + drivers/media/Kconfig |7 + drivers/media/Makefile |2 + drivers/media/cec.c| 1962 include/media/cec.h| 182 include/uapi/linux/Kbuild |2 + include/uapi/linux/cec-funcs.h | 1771 include/uapi/linux/cec.h | 884 ++ 8 files changed, 4822 insertions(+) create mode 100644 drivers/media/cec.c create mode 100644 include/media/cec.h create mode 100644 include/uapi/linux/cec-funcs.h create mode 100644 include/uapi/linux/cec.h diff --git a/MAINTAINERS b/MAINTAINERS index 42e81f9..23fe4c6 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2599,6 +2599,18 @@ F: drivers/net/ieee802154/cc2520.c F: include/linux/spi/cc2520.h F: Documentation/devicetree/bindings/net/ieee802154/cc2520.txt +CEC DRIVER +M: Hans Verkuil +L: linux-media at vger.kernel.org +T: git git://linuxtv.org/media_tree.git +W: http://linuxtv.org +S: Supported +F: drivers/media/cec.c +F: drivers/media/rc/keymaps/rc-cec.c +F: include/media/cec.h +F: include/uapi/linux/cec.h +F: include/uapi/linux/cec-funcs.h + CELL BROADBAND ENGINE ARCHITECTURE M: Arnd Bergmann L: linuxppc-dev at lists.ozlabs.org diff --git a/drivers/media/Kconfig b/drivers/media/Kconfig index 3ef3d6c..ff22089 100644 --- a/drivers/media/Kconfig +++ b/drivers/media/Kconfig @@ -80,6 +80,13 @@ config MEDIA_RC_SUPPORT Say Y when you have a TV or an IR device. +config MEDIA_CEC + tristate "CEC API (EXPERIMENTAL)" + depends on MEDIA_RC_SUPPORT + select RC_CORE + ---help--- + Enable the CEC API. + # # Media controller # Selectable only for webcam/grabbers, as other drivers don't use it diff --git a/drivers/media/Makefile b/drivers/media/Makefile index e608bbc..58e8554 100644 --- a/drivers/media/Makefile +++ b/drivers/media/Makefile @@ -2,6 +2,8 @@ # Makefile for the kernel multimedia device drivers. # +obj-$(CONFIG_MEDIA_CEC) += cec.o + media-objs := media-device.o media-devnode.o media-entity.o # diff --git a/drivers/media/cec.c b/drivers/media/cec.c new file mode 100644 index 000..e979419 --- /dev/null +++ b/drivers/media/cec.c @@ -0,0 +1,1962 @@ +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define CEC_NUM_DEVICES256 +#define CEC_NAME "cec" + +static int debug; +module_param(debug, int, 0644); +MODULE_PARM_DESC(debug, "debug level (0-2)"); + +#define dprintk(lvl, fmt, arg...) \ + do {\ + if (lvl <= debug) \ + pr_info("cec-%s: " fmt, adap->name, ## arg);\ + } while (0) + +static dev_t cec_dev_t; + +/* Active devices */ +static DEFINE_MUTEX(cec_devnode_lock); +static DECLARE_BITMAP(cec_devnode_nums, CEC_NUM_DEVICES); + +/* dev to cec_devnode */ +#define to_cec_devnode(cd) container_of(cd, struct cec_devnode, dev) + +static inline struct cec_devnode *cec_devnode_data(struct file *filp) +{ + struct cec_fh *fh = filp->private_data; + + return >adap->devnode; +} + +static bool cec_pa_are_adjacent(const struct cec_adapter *adap, u16 pa1, u16 pa2) +{ + u16 mask = 0xf000; + int i; + + if (pa1 == CEC_PHYS_ADDR_INVALID || pa2 == CEC_PHYS_ADDR_INVALID) + return false; + for (i = 0; i < 3; i++) { + if ((pa1 & mask) != (pa2 & mask)) + break; + mask = (mask >> 4) | 0xf000; + } + if
[PATCHv10 06/16] rc: Add HDMI CEC protocol handling
From: Kamil DebskiAdd handling of remote control events coming from the HDMI CEC bus. This patch includes a new keymap that maps values found in the CEC messages to the keys pressed and released. Also, a new protocol has been added to the core. Signed-off-by: Kamil Debski Signed-off-by: Hans Verkuil --- drivers/media/rc/keymaps/Makefile | 1 + drivers/media/rc/keymaps/rc-cec.c | 174 ++ drivers/media/rc/rc-main.c| 1 + include/media/rc-core.h | 1 + include/media/rc-map.h| 5 +- 5 files changed, 181 insertions(+), 1 deletion(-) create mode 100644 drivers/media/rc/keymaps/rc-cec.c diff --git a/drivers/media/rc/keymaps/Makefile b/drivers/media/rc/keymaps/Makefile index fbbd3bb..9cffcc6 100644 --- a/drivers/media/rc/keymaps/Makefile +++ b/drivers/media/rc/keymaps/Makefile @@ -18,6 +18,7 @@ obj-$(CONFIG_RC_MAP) += rc-adstech-dvb-t-pci.o \ rc-behold.o \ rc-behold-columbus.o \ rc-budget-ci-old.o \ + rc-cec.o \ rc-cinergy-1400.o \ rc-cinergy.o \ rc-delock-61959.o \ diff --git a/drivers/media/rc/keymaps/rc-cec.c b/drivers/media/rc/keymaps/rc-cec.c new file mode 100644 index 000..193cdca --- /dev/null +++ b/drivers/media/rc/keymaps/rc-cec.c @@ -0,0 +1,174 @@ +/* Keytable for the CEC remote control + * + * Copyright (c) 2015 by Kamil Debski + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + */ + +#include +#include + +/* CEC Spec "High-Definition Multimedia Interface Specification" can be obtained + * here: http://xtreamerdev.googlecode.com/files/CEC_Specs.pdf + * The list of control codes is listed in Table 27: User Control Codes p. 95 */ + +static struct rc_map_table cec[] = { + { 0x00, KEY_OK }, + { 0x01, KEY_UP }, + { 0x02, KEY_DOWN }, + { 0x03, KEY_LEFT }, + { 0x04, KEY_RIGHT }, + { 0x05, KEY_RIGHT_UP }, + { 0x06, KEY_RIGHT_DOWN }, + { 0x07, KEY_LEFT_UP }, + { 0x08, KEY_LEFT_DOWN }, + { 0x09, KEY_ROOT_MENU }, /* CEC Spec: Device Root Menu - see Note 2 */ + /* Note 2: This is the initial display that a device shows. It is +* device-dependent and can be, for example, a contents menu, setup +* menu, favorite menu or other menu. The actual menu displayed +* may also depend on the device's current state. */ + { 0x0a, KEY_SETUP }, + { 0x0b, KEY_MENU }, /* CEC Spec: Contents Menu */ + { 0x0c, KEY_FAVORITES }, /* CEC Spec: Favorite Menu */ + { 0x0d, KEY_EXIT }, + /* 0x0e-0x0f: Reserved */ + { 0x10, KEY_MEDIA_TOP_MENU }, + { 0x11, KEY_CONTEXT_MENU }, + /* 0x12-0x1c: Reserved */ + { 0x1d, KEY_DIGITS }, /* CEC Spec: select/toggle a Number Entry Mode */ + { 0x1e, KEY_NUMERIC_11 }, + { 0x1f, KEY_NUMERIC_12 }, + /* 0x20-0x29: Keys 0 to 9 */ + { 0x20, KEY_NUMERIC_0 }, + { 0x21, KEY_NUMERIC_1 }, + { 0x22, KEY_NUMERIC_2 }, + { 0x23, KEY_NUMERIC_3 }, + { 0x24, KEY_NUMERIC_4 }, + { 0x25, KEY_NUMERIC_5 }, + { 0x26, KEY_NUMERIC_6 }, + { 0x27, KEY_NUMERIC_7 }, + { 0x28, KEY_NUMERIC_8 }, + { 0x29, KEY_NUMERIC_9 }, + { 0x2a, KEY_DOT }, + { 0x2b, KEY_ENTER }, + { 0x2c, KEY_CLEAR }, + /* 0x2d-0x2e: Reserved */ + { 0x2f, KEY_NEXT_FAVORITE }, /* CEC Spec: Next Favorite */ + { 0x30, KEY_CHANNELUP }, + { 0x31, KEY_CHANNELDOWN }, + { 0x32, KEY_PREVIOUS }, /* CEC Spec: Previous Channel */ + { 0x33, KEY_SOUND }, /* CEC Spec: Sound Select */ + { 0x34, KEY_VIDEO }, /* 0x34: CEC Spec: Input Select */ + { 0x35, KEY_INFO }, /* CEC Spec: Display Information */ + { 0x36, KEY_HELP }, + { 0x37, KEY_PAGEUP }, + { 0x38, KEY_PAGEDOWN }, + /* 0x39-0x3f: Reserved */ + { 0x40, KEY_POWER }, + { 0x41, KEY_VOLUMEUP }, + { 0x42, KEY_VOLUMEDOWN }, + { 0x43, KEY_MUTE }, + { 0x44, KEY_PLAYCD }, + { 0x45, KEY_STOPCD }, + { 0x46, KEY_PAUSECD }, + { 0x47, KEY_RECORD }, + { 0x48, KEY_REWIND }, + { 0x49, KEY_FASTFORWARD }, + { 0x4a, KEY_EJECTCD }, /* CEC Spec: Eject */ + { 0x4b, KEY_FORWARD }, + { 0x4c, KEY_BACK }, + { 0x4d, KEY_STOP_RECORD }, /* CEC Spec: Stop-Record */ + { 0x4e, KEY_PAUSE_RECORD }, /* CEC Spec: Pause-Record */ + /* 0x4f: Reserved */ + { 0x50, KEY_ANGLE }, + { 0x51, KEY_TV2 }, + { 0x52, KEY_VOD }, /* CEC Spec: Video on Demand */ + { 0x53, KEY_EPG }, + { 0x54, KEY_TIME }, /* CEC Spec: Timer */ + { 0x55, KEY_CONFIG }, + /* The
[PATCHv10 05/16] HID: add HDMI CEC specific keycodes
From: Kamil DebskiAdd HDMI CEC specific keycodes to the keycodes definition. Signed-off-by: Kamil Debski Signed-off-by: Hans Verkuil --- include/uapi/linux/input.h | 28 1 file changed, 28 insertions(+) diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h index a32bff1..5e7019a 100644 --- a/include/uapi/linux/input.h +++ b/include/uapi/linux/input.h @@ -752,6 +752,34 @@ struct input_keymap_entry { #define KEY_KBDINPUTASSIST_ACCEPT 0x264 #define KEY_KBDINPUTASSIST_CANCEL 0x265 +#define KEY_RIGHT_UP 0x266 +#define KEY_RIGHT_DOWN 0x267 +#define KEY_LEFT_UP0x268 +#define KEY_LEFT_DOWN 0x269 +#define KEY_ROOT_MENU 0x26a /* Show Device's Root Menu */ +#define KEY_MEDIA_TOP_MENU 0x26b /* Show Top Menu of the Media (e.g. DVD) */ +#define KEY_NUMERIC_11 0x26c +#define KEY_NUMERIC_12 0x26d +/* + * Toggle Audio Description: refers to an audio service that helps blind and + * visually impaired consumers understand the action in a program. Note: in + * some countries this is referred to as "Video Description". + */ +#define KEY_AUDIO_DESC 0x26e +#define KEY_3D_MODE0x26f +#define KEY_NEXT_FAVORITE 0x270 +#define KEY_STOP_RECORD0x271 +#define KEY_PAUSE_RECORD 0x272 +#define KEY_VOD0x273 /* Video on Demand */ +#define KEY_UNMUTE 0x274 +#define KEY_FASTREVERSE0x275 +#define KEY_SLOWREVERSE0x276 +/* + * Control a data application associated with the currently viewed channel, + * e.g. teletext or data broadcast application (MHEG, MHP, HbbTV, etc.) + */ +#define KEY_DATA 0x275 + #define BTN_TRIGGER_HAPPY 0x2c0 #define BTN_TRIGGER_HAPPY1 0x2c0 #define BTN_TRIGGER_HAPPY2 0x2c1 -- 2.6.2
[PATCHv10 04/16] input.h: add BUS_CEC type
From: Hans VerkuilInputs can come in over the HDMI CEC bus, so add a new type for this. Signed-off-by: Hans Verkuil Acked-by: Dmitry Torokhov --- include/uapi/linux/input.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h index 731417c..a32bff1 100644 --- a/include/uapi/linux/input.h +++ b/include/uapi/linux/input.h @@ -972,6 +972,7 @@ struct input_keymap_entry { #define BUS_GSC0x1A #define BUS_ATARI 0x1B #define BUS_SPI0x1C +#define BUS_CEC0x1D /* * MT_TOOL types -- 2.6.2
[PATCHv10 03/16] dts: exynos4412-odroid*: enable the HDMI CEC device
From: Kamil DebskiAdd a dts node entry and enable the HDMI CEC device present in the Exynos4 family of SoCs. Signed-off-by: Kamil Debski Signed-off-by: Hans Verkuil Acked-by: Krzysztof Kozlowski --- arch/arm/boot/dts/exynos4210-universal_c210.dts | 4 1 file changed, 4 insertions(+) diff --git a/arch/arm/boot/dts/exynos4210-universal_c210.dts b/arch/arm/boot/dts/exynos4210-universal_c210.dts index eb37952..5c4393d 100644 --- a/arch/arm/boot/dts/exynos4210-universal_c210.dts +++ b/arch/arm/boot/dts/exynos4210-universal_c210.dts @@ -222,6 +222,10 @@ enable-active-high; }; + cec at 100B { + status = "okay"; + }; + hdmi_ddc: i2c-ddc { compatible = "i2c-gpio"; gpios = < 2 0 3 0>; -- 2.6.2
[PATCHv10 02/16] dts: exynos4: add node for the HDMI CEC device
From: Kamil DebskiThis patch adds HDMI CEC node specific to the Exynos4210/4x12 SoC series. Signed-off-by: Kamil Debski Signed-off-by: Hans Verkuil Acked-by: Krzysztof Kozlowski --- arch/arm/boot/dts/exynos4.dtsi | 12 1 file changed, 12 insertions(+) diff --git a/arch/arm/boot/dts/exynos4.dtsi b/arch/arm/boot/dts/exynos4.dtsi index 98c0a36..7baff26 100644 --- a/arch/arm/boot/dts/exynos4.dtsi +++ b/arch/arm/boot/dts/exynos4.dtsi @@ -720,6 +720,18 @@ status = "disabled"; }; + hdmicec: cec at 100B { + compatible = "samsung,s5p-cec"; + reg = <0x100B 0x200>; + interrupts = <0 114 0>; + clocks = < CLK_HDMI_CEC>; + clock-names = "hdmicec"; + samsung,syscon-phandle = <_system_controller>; + pinctrl-names = "default"; + pinctrl-0 = <_cec>; + status = "disabled"; + }; + mixer: mixer at 12C1 { compatible = "samsung,exynos4210-mixer"; interrupts = <0 91 0>; -- 2.6.2
[PATCHv10 01/16] dts: exynos4*: add HDMI CEC pin definition to pinctrl
From: Kamil DebskiAdd pinctrl nodes for the HDMI CEC device to the Exynos4210 and Exynos4x12 SoCs. These are required by the HDMI CEC device. Signed-off-by: Kamil Debski Signed-off-by: Hans Verkuil Acked-by: Krzysztof Kozlowski --- arch/arm/boot/dts/exynos4210-pinctrl.dtsi | 7 +++ arch/arm/boot/dts/exynos4x12-pinctrl.dtsi | 7 +++ 2 files changed, 14 insertions(+) diff --git a/arch/arm/boot/dts/exynos4210-pinctrl.dtsi b/arch/arm/boot/dts/exynos4210-pinctrl.dtsi index a7c2128..9331c62 100644 --- a/arch/arm/boot/dts/exynos4210-pinctrl.dtsi +++ b/arch/arm/boot/dts/exynos4210-pinctrl.dtsi @@ -820,6 +820,13 @@ samsung,pin-pud = <1>; samsung,pin-drv = <0>; }; + + hdmi_cec: hdmi-cec { + samsung,pins = "gpx3-6"; + samsung,pin-function = <3>; + samsung,pin-pud = <0>; + samsung,pin-drv = <0>; + }; }; pinctrl at 0386 { diff --git a/arch/arm/boot/dts/exynos4x12-pinctrl.dtsi b/arch/arm/boot/dts/exynos4x12-pinctrl.dtsi index bac25c6..856b292 100644 --- a/arch/arm/boot/dts/exynos4x12-pinctrl.dtsi +++ b/arch/arm/boot/dts/exynos4x12-pinctrl.dtsi @@ -885,6 +885,13 @@ samsung,pin-pud = <0>; samsung,pin-drv = <0>; }; + + hdmi_cec: hdmi-cec { + samsung,pins = "gpx3-6"; + samsung,pin-function = <3>; + samsung,pin-pud = <0>; + samsung,pin-drv = <0>; + }; }; pinctrl_2: pinctrl at 0386 { -- 2.6.2
[PATCHv10 00/16] HDMI CEC framework
Hi all, The tenth version of this patchset addresses comments I received from Russell King and various bug fixes and enhancements as the result of more testing. The cec.txt has been updated, but before I can make the final version there are three areas that I want to look at more closely: 1) What to do if a cable is disconnected and reconnected for a source: should the CEC adapter be disabled and re-enabled? Which means that you need to reconfigure. Or just update the physical address from the EDID (if necessary)? I think the latter, but I need to analyze this more closely. 2) I am not happy with the event mechanism. It's OK for messages, but for other events it is awkward. 3) The status field gives insufficient information and I want to reorganize that for the next version. The cec-ctl and cec-compliance utilities used to test the CEC framework can be found here: http://git.linuxtv.org/cgit.cgi/hverkuil/v4l-utils.git/log/?h=cec Best regards, Hans Changes since v9 - Updated cec.txt - Added a promiscuous capability to signal those adapters that can monitor all CEC traffic, not just directed and broadcast messages. I have one adapter that can do this. Added code in the framework to handle such messages correctly. - The status field is now value and no longer a bitmask. - Renamed the kernel config from CEC to MEDIA_CEC - The adap_transmit() callback now has a retries argument. - Use the new CEC_MAX_MSG_SIZE define instead of hardcoding it as 16 - Add support to wait for a reply after a broadcast message: this was forbidden, but it is a valid use-case. - Make sure you can't send a message to yourself. - Waiting for a transmit to succeed would never timeout (and couldn't be interrupted). Fixed. - The message status was not updated correctly if it was CEC_MSG_FEATURE_ABORTed. - Fixed a nasty kernel oops when deleting a cec adapter. - Removed the owner check: the module owner is NULL if it is compiled into the kernel instead of as a module. - Added separate register/unregister calls: this is safer and actually made it possible to drop the ugly 'cec_ready' v4l2_subdev op. Suggested by Russell, and that was a good idea. - Added missing support for 32-bit to 64-bit ioctl conversion. - Move the v4l2_subdev cec ops into a v4l2_subdev_cec_ops struct. Changes since v8 - Addressed the comments Russell King made about how the cec character devices should be allocated/freed. - Updated the DocBook documentation. Changes since v7 - I thought that the core thread could handle out-of-order messages, but that turned out to be wrong. After careful analysis I realized that I had to rewrite this part in cec.c in order to make it work. - Added new CEC-specific keys to input.h and use them in the CEC rc keymap. Replaced KEY_PLAY/PAUSE/STOP with KEY_PLAYCD/PAUSECD/STOPCD to clarify that these are media operations and not the Pause key on the keyboard. - Added CEC_PHYS_ADDR_INVALID (0x) - Added monitor support to monitor CEC traffic - Replaced CAP_TRANSMIT and CAP_RECEIVE by a single CAP_IO. - Replaced CAP_CDC by CAP_CDC_HPD since this only applies to the HPD part of the CDC messages. - Add CAP_IS_SOURCE. - Add ninputs field to cec_caps to export the number of inputs of the device. - Drop CEC_LOG_ADDRS_FL_HANDLE_MSGS and the flags field (see next change for more info). - Add CEC_CLAIM and CEC_RELEASE to explicitly start/stop processing CEC messages. This also implies ownership of the CEC interface, so other filehandles can only receive but not transmit. - Reworked event handling: report adapter state changes, input changes and if the message receive queue is full. - cec-funcs.h: added CDC HEC support. - Renamed G/S_ADAP ioctls to ADAP_G/S: this made it clearer which ioctls deal with the adapter configuration and which deal with CEC messages/events. - Clarified which CEC messages are passed on to userspace and which aren't. Specifically if CAP_ARC is set, then all ARC messages are handled by the kernel. If CAP_CDC_HPD is set, then all CDC hotplug messages are handled by the kernel. Otherwise these messages are passed on to userspace. Changes since v6 - added cec-funcs.h to provide wrapper functions that fill in the cec_msg struct. This header is needed both by the kernel and by applications. - fix a missing rc_unregister_device call. - added CEC support for the adv7842 and cobalt drivers. - added CEC operand defines. Rename CEC message defines to CEC_MSG_ and operand defines now use CEC_OP_. - the CEC_VERSION defines are dropped since we now have the CEC_OP_VERSION defines. - ditto: CEC_PRIM_DEVTYPE_ is now CEC_OP_PRIM_DEVTYPE. - ditto: CEC_FL_ALL_DEVTYPE_ is now CEC_OP_ALL_DEVTYPE. - cec-ioc-g-adap-log-addrs.xml: document cec_versions field. - cec-ioc-g-caps.xml: drop vendor_id and version fields. - add MAINTAINERS entry. - add CDC support (not yet fully functional). -
drm: Bogus WARN() in drm_atomic_helper_update_legacy_modeset_state() ?
On Wed, Nov 11, 2015 at 04:09:42PM +, Liviu Dudau wrote: > On Tue, Nov 10, 2015 at 05:56:15PM +0100, Thierry Reding wrote: > > On Tue, Nov 10, 2015 at 03:01:03PM +, Liviu Dudau wrote: > > > Hello, > > > > > > When booting my Juno board with the HDLCD driver that I have converted to > > > atomic operations I'm getting the following warning: > > > > Perhaps you can provide pointers to the source code, that might make it > > easier for people to spot what's going wrong. > > > > Thierry > > Hi Thierry, > > I have just pushed to the mailing lists my patch series. [1] [2] > > If you want to checkout the code I also have a branch here: > > git://git://linux-arm.org/linux-ld testing/hdlcd Okay, so if I understand correctly you're using the tda998x as encoder/ connector attached to your new HDLCD driver. I think the problem isn't so much that nothing has set up the CRTC for the encoder, but rather that the tda998x encoder tries to statically associate the encoder with the connector. While that might be correct in that it represents the hardware state (i.e. there is no way to physically route it anywhere else), the DRM logical state is that it's not connected until a complete pipeline has been set up, in which case a CRTC would have been assigned to the encoder. If your setup were working correctly you'd never reach the WARN_ON because the if (connector->encoder) { conditional (on line 673 in drivers/gpu/drm/drm_atomic_helper.c) would have evaluated to false already, since logically there'd be no encoder associated with the connector yet. Does the patch below help? Let me know and I can produce a proper patch. Thierry --- >8 --- diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index 896b6aaf8c4d..8b0a402190eb 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -1453,7 +1453,6 @@ static int tda998x_bind(struct device *dev, struct device *master, void *data) if (ret) goto err_sysfs; - priv->connector.encoder = >encoder; drm_mode_connector_attach_encoder(>connector, >encoder); return 0; -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151112/c49e1dac/attachment.sig>
[PATCH 04/25] drm/exynos: gsc: fix wrong pm_runtime state
Hi Marek, 2015-11-10 Marek Szyprowski : > From: Seung-Woo Kim > > At probe time, gsc clock is not enabled, so pm_runtime state should > be deactive. So this patch removes pm_runtime_set_active() from > gsc_probe(). > > Signed-off-by: Seung-Woo Kim > Signed-off-by: Marek Szyprowski > --- > drivers/gpu/drm/exynos/exynos_drm_gsc.c | 1 - > 1 file changed, 1 deletion(-) Reviewed-by: Gustavo Padovan Gustavo