Re: [Intel-gfx] [PATCH 2/3] drm: add async version of hpd_irq_event
On Tue, May 20, 2014 at 03:25:34PM -0700, Jesse Barnes wrote:
In some cases, the callers of this function may not need the return
value and delaying the uevent is ok. So add an async version of the
function for use in those cases.
Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
---
drivers/gpu/drm/drm_probe_helper.c | 8
include/drm/drm_crtc_helper.h | 2 ++
2 files changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/drm_probe_helper.c
b/drivers/gpu/drm/drm_probe_helper.c
index 79f07f2..f3aee4a 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -446,3 +446,11 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev)
return changed;
}
EXPORT_SYMBOL(drm_helper_hpd_irq_event);
+
Kerneldoc is missing.
-Daniel
+void drm_helper_hpd_irq_event_async(void *data, async_cookie_t cookie)
+{
+ struct drm_device *dev = data;
+
+ drm_helper_hpd_irq_event(dev);
+}
+EXPORT_SYMBOL(drm_helper_hpd_irq_event_async);
diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h
index a3d75fe..4f4ed9c 100644
--- a/include/drm/drm_crtc_helper.h
+++ b/include/drm/drm_crtc_helper.h
@@ -33,6 +33,7 @@
#ifndef __DRM_CRTC_HELPER_H__
#define __DRM_CRTC_HELPER_H__
+#include linux/async.h
#include linux/spinlock.h
#include linux/types.h
#include linux/idr.h
@@ -172,6 +173,7 @@ extern int
drm_helper_probe_single_connector_modes_nomerge(struct drm_connector
extern void drm_kms_helper_poll_init(struct drm_device *dev);
extern void drm_kms_helper_poll_fini(struct drm_device *dev);
extern bool drm_helper_hpd_irq_event(struct drm_device *dev);
+extern void drm_helper_hpd_irq_event_async(void *data, async_cookie_t
cookie);
extern void drm_kms_helper_hotplug_event(struct drm_device *dev);
extern void drm_kms_helper_poll_disable(struct drm_device *dev);
--
1.8.4.2
___
dri-devel mailing list
dri-de...@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] uxa: fix getmsc to not fail hard
On Wed, May 21, 2014 at 12:41:58PM +1000, Dave Airlie wrote: From: Dave Airlie airl...@redhat.com If some outputs go away we race with this call and apps get X errors and fall over. Do what SNA does and don't bother trying. Horrible clients is why I gave up trying to report BadMatch for the inactive pipe. Though it has been a long time since the kernel turned off CRTCs without intervention from userspace. This is a useful safeguard, though it angers the OML sync proponents. Pushed, thanks. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/3] drm/i915: use async hpd_irq_event function on resume
On Tue, May 20, 2014 at 03:25:35PM -0700, Jesse Barnes wrote: Gets the detect code (which may take awhile) out of the resume path, speeding things up a bit. Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org --- drivers/gpu/drm/i915/i915_drv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 302495f..571f688 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -606,7 +606,7 @@ static int __i915_drm_thaw(struct drm_device *dev, bool restore_gtt_mappings) intel_hpd_init(dev); dev_priv-enable_hotplug_processing = true; /* Config may have changed between suspend and resume */ - drm_helper_hpd_irq_event(dev); + async_schedule(drm_helper_hpd_irq_event_async, dev); Does that really help all that much? I've thought the driver core sychnronizes all the async workers again once resume is done. I'm better to schedule this as a fully async work with e.g. a 1s delay, like we do with the rps resume work. -Daniel } intel_opregion_init(dev); -- 1.8.4.2 ___ dri-devel mailing list dri-de...@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/i915: drop encoder hot_plug calls at resume
On Tue, May 20, 2014 at 03:25:33PM -0700, Jesse Barnes wrote:
We really just want to go detect displays again and fire off a hotplug
event if things have changed, not go through full hotplug processing.
Requested-by: Daniel Vetter daniel.vet...@ffwll.ch
Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
Yeah, still can't remember why we've done this, and the commit message is
silent.
Queued for -next, thanks for the patch.
-Daniel
---
drivers/gpu/drm/i915/i915_drv.c | 20 +---
1 file changed, 1 insertion(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index b948c71..302495f 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -551,24 +551,6 @@ void intel_console_resume(struct work_struct *work)
console_unlock();
}
-static void intel_resume_hotplug(struct drm_device *dev)
-{
- struct drm_mode_config *mode_config = dev-mode_config;
- struct intel_encoder *encoder;
-
- mutex_lock(mode_config-mutex);
- DRM_DEBUG_KMS(running encoder hotplug functions\n);
-
- list_for_each_entry(encoder, mode_config-encoder_list, base.head)
- if (encoder-hot_plug)
- encoder-hot_plug(encoder);
-
- mutex_unlock(mode_config-mutex);
-
- /* Just fire off a uevent and let userspace tell us what to do */
- drm_helper_hpd_irq_event(dev);
-}
-
static int i915_drm_thaw_early(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev-dev_private;
@@ -624,7 +606,7 @@ static int __i915_drm_thaw(struct drm_device *dev, bool
restore_gtt_mappings)
intel_hpd_init(dev);
dev_priv-enable_hotplug_processing = true;
/* Config may have changed between suspend and resume */
- intel_resume_hotplug(dev);
+ drm_helper_hpd_irq_event(dev);
}
intel_opregion_init(dev);
--
1.8.4.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCHv5] optimize wait for vblank
Implemented the comments based on www.spinics.net/lists/intel-gfx/msg42439.html Arun R Murthy (1): drm/i915: use waitqueue in wait for vblank drivers/gpu/drm/i915/i915_dma.c |2 ++ drivers/gpu/drm/i915/i915_drv.h |1 + drivers/gpu/drm/i915/i915_irq.c | 16 drivers/gpu/drm/i915/intel_display.c | 10 ++ 4 files changed, 21 insertions(+), 8 deletions(-) -- 1.7.9.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCHv5] drm/i915: use waitqueue in wait for vblank
Instead of using sleep functions in wait for vblank, which wakes up on
sleep timeout aften, thereby scheduler provoking scheduler. Hence use
waitqueue in wait for vblank.
Signed-off-by: Arun R Murthy arun.r.mur...@intel.com
---
drivers/gpu/drm/i915/i915_dma.c |2 ++
drivers/gpu/drm/i915/i915_drv.h |1 +
drivers/gpu/drm/i915/i915_irq.c | 17 +
drivers/gpu/drm/i915/intel_display.c | 10 ++
4 files changed, 22 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 258b1be..896620c 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1506,6 +1506,8 @@ int i915_driver_load(struct drm_device *dev, unsigned
long flags)
mutex_init(dev_priv-dpio_lock);
mutex_init(dev_priv-modeset_restore_lock);
+ init_waitqueue_head(dev_priv-wait_vblank);
+
intel_pm_setup(dev);
intel_display_crc_init(dev);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 728b9c3..449bfef 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1590,6 +1590,7 @@ typedef struct drm_i915_private {
struct i915_dri1_state dri1;
/* Old ums support infrastructure, same warning applies. */
struct i915_ums_state ums;
+ wait_queue_head_t wait_vblank;
} drm_i915_private_t;
static inline struct drm_i915_private *to_i915(const struct drm_device *dev)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 56edff3..f97f0fe 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1513,8 +1513,11 @@ static irqreturn_t valleyview_irq_handler(int irq, void
*arg)
spin_unlock_irqrestore(dev_priv-irq_lock, irqflags);
for_each_pipe(pipe) {
- if (pipe_stats[pipe]
PIPE_START_VBLANK_INTERRUPT_STATUS)
+ if (pipe_stats[pipe]
+ PIPE_START_VBLANK_INTERRUPT_STATUS) {
drm_handle_vblank(dev, pipe);
+ wake_up_interruptible(dev_priv-wait_vblank);
+ }
if (pipe_stats[pipe] PLANE_FLIPDONE_INT_STATUS_VLV) {
intel_prepare_page_flip(dev, pipe);
@@ -3269,8 +3272,10 @@ static irqreturn_t i8xx_irq_handler(int irq, void *arg)
plane = !plane;
if (pipe_stats[pipe] PIPE_VBLANK_INTERRUPT_STATUS
- i8xx_handle_vblank(dev, plane, pipe, iir))
+ i8xx_handle_vblank(dev, plane, pipe, iir)) {
flip_mask = ~DISPLAY_PLANE_FLIP_PENDING(plane);
+ wake_up_interruptible(dev_priv-wait_vblank);
+ }
if (pipe_stats[pipe] PIPE_CRC_DONE_INTERRUPT_STATUS)
i9xx_pipe_crc_irq_handler(dev, pipe);
@@ -3464,8 +3469,10 @@ static irqreturn_t i915_irq_handler(int irq, void *arg)
plane = !plane;
if (pipe_stats[pipe] PIPE_VBLANK_INTERRUPT_STATUS
- i915_handle_vblank(dev, plane, pipe, iir))
+ i915_handle_vblank(dev, plane, pipe, iir)) {
flip_mask = ~DISPLAY_PLANE_FLIP_PENDING(plane);
+ wake_up_interruptible(dev_priv-wait_vblank);
+ }
if (pipe_stats[pipe] PIPE_LEGACY_BLC_EVENT_STATUS)
blc_event = true;
@@ -3709,8 +3716,10 @@ static irqreturn_t i965_irq_handler(int irq, void *arg)
for_each_pipe(pipe) {
if (pipe_stats[pipe]
PIPE_START_VBLANK_INTERRUPT_STATUS
- i915_handle_vblank(dev, pipe, pipe, iir))
+ i915_handle_vblank(dev, pipe, pipe, iir)) {
flip_mask = ~DISPLAY_PLANE_FLIP_PENDING(pipe);
+ wake_up_interruptible(dev_priv-wait_vblank);
+ }
if (pipe_stats[pipe] PIPE_LEGACY_BLC_EVENT_STATUS)
blc_event = true;
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 4d4a0d9..e1eb564 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -757,12 +757,14 @@ enum transcoder intel_pipe_to_cpu_transcoder(struct
drm_i915_private *dev_priv,
static void g4x_wait_for_vblank(struct drm_device *dev, int pipe)
{
struct drm_i915_private *dev_priv = dev-dev_private;
- u32 frame, frame_reg = PIPE_FRMCOUNT_GM45(pipe);
+ u32 vblank_cnt;
- frame = I915_READ(frame_reg);
+ vblank_cnt =
Re: [Intel-gfx] [PATCHv5] drm/i915: use waitqueue in wait for vblank
On Wed, May 21, 2014 at 01:09:58PM +0530, Arun R Murthy wrote:
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 56edff3..f97f0fe 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1513,8 +1513,11 @@ static irqreturn_t valleyview_irq_handler(int irq,
void *arg)
spin_unlock_irqrestore(dev_priv-irq_lock, irqflags);
for_each_pipe(pipe) {
- if (pipe_stats[pipe]
PIPE_START_VBLANK_INTERRUPT_STATUS)
+ if (pipe_stats[pipe]
+ PIPE_START_VBLANK_INTERRUPT_STATUS) {
drm_handle_vblank(dev, pipe);
+ wake_up_interruptible(dev_priv-wait_vblank);
We now have intel_handle_vblank() so these chunks can be simplified.
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 4d4a0d9..e1eb564 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -757,12 +757,14 @@ enum transcoder intel_pipe_to_cpu_transcoder(struct
drm_i915_private *dev_priv,
static void g4x_wait_for_vblank(struct drm_device *dev, int pipe)
{
struct drm_i915_private *dev_priv = dev-dev_private;
- u32 frame, frame_reg = PIPE_FRMCOUNT_GM45(pipe);
+ u32 vblank_cnt;
- frame = I915_READ(frame_reg);
+ vblank_cnt = drm_vblank_count(dev, pipe);
- if (wait_for(I915_READ_NOTRACE(frame_reg) != frame, 50))
- DRM_DEBUG_KMS(vblank wait timed out\n);
+ /* TODO: get the vblank time dynamically or from platform data */
+ wait_event_interruptible_timeout(dev_priv-wait_vblank,
+ (vblank_cnt != drm_vblank_count(dev, pipe)),
+ msecs_to_jiffies(16));
Keep the ultimate timeout at 50 until you have evidence you can reduce
it. And then it should be 2x vrefresh interval to be safe. However, you
are likely still hitting the timeout as the vblank irq is not guaranteed
to be enabled here. How safe calling drm_vblank_get() is during modeset
I defer to Ville since he has just taken a pass over the whole mess.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 1/5] drm/i915: Fix mmio vs. CS flip race on ILK+
On Tue, Apr 15, 2014 at 09:41:34PM +0300, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä ville.syrj...@linux.intel.com
Starting from ILK, mmio flips also cause a flip done interrupt to be
signalled. This means if we first do a set_base and follow it
immediately with the CS flip, we might mistake the flip done interrupt
caused by the set_base as the flip done interrupt caused by the CS
flip.
The hardware has a flip counter which increments every time a mmio or
CS flip is issued. It basically counts the number of DSPSURF register
writes. This means we can sample the counter before we put the CS
flip into the ring, and then when we get a flip done interrupt we can
check whether the CS flip has actually performed the surface address
update, or if the interrupt was caused by a previous but yet
unfinished mmio flip.
Even with the flip counter we still have a race condition of the CS flip
base address update happens after the mmio flip done interrupt was
raised but not yet processed by the driver. When the interrupt is
eventually processed, the flip counter will already indicate that the
CS flip has been executed, but it would not actually complete until the
next start of vblank. We can use the DSPSURFLIVE register to check
whether the hardware is actually scanning out of the buffer we expect,
or if we managed hit this race window.
This covers all the cases where the CS flip actually changes the base
address. If the base address remains unchanged, we might still complete
the CS flip before it has actually completed. But since the address
didn't change anyway, the premature flip completion can't result in
userspace overwriting data that's still being scanned out.
CTG already has the flip counter and DSPSURFLIVE registers, and
although the flip done interrupt is still limited to CS flips alone,
the code now also checks the flip counter on CTG as well.
v2: s/dspsurf/gtt_offset/ (Chris)
Testcase: igt/kms_mmio_vs_cs_flip/setcrtc_vs_cs_flip
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=73027
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/i915_reg.h | 1 +
drivers/gpu/drm/i915/intel_display.c | 73
drivers/gpu/drm/i915/intel_drv.h | 2 +
3 files changed, 69 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 8f84555..c28169c 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3638,6 +3638,7 @@ enum punit_power_well {
#define _PIPEA_FRMCOUNT_GM45 0x70040
#define _PIPEA_FLIPCOUNT_GM450x70044
#define PIPE_FRMCOUNT_GM45(pipe) _PIPE2(pipe, _PIPEA_FRMCOUNT_GM45)
+#define PIPE_FLIPCOUNT_GM45(pipe) _PIPE2(pipe, _PIPEA_FLIPCOUNT_GM45)
/* Cursor A B regs */
#define _CURACNTR(dev_priv-info.display_mmio_offset + 0x70080)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 1390ab5..32c6c16 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8577,6 +8577,48 @@ void intel_finish_page_flip_plane(struct drm_device
*dev, int plane)
do_intel_finish_page_flip(dev, crtc);
}
+/* Is 'a' after or equal to 'b'? */
+static bool flip_count_after_eq(u32 a, u32 b)
I've added a g4x prefix here. The wrap-around semantics on earlier chips
(yeah I know doesn't exist, but the vblank counter does) wouldn't work.
-Daniel
+{
+ return !((a - b) 0x8000);
+}
+
+static bool page_flip_finished(struct intel_crtc *crtc)
+{
+ struct drm_device *dev = crtc-base.dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
+
+ /*
+ * The relevant registers doen't exist on pre-ctg.
+ * As the flip done interrupt doesn't trigger for mmio
+ * flips on gmch platforms, a flip count check isn't
+ * really needed there. But since ctg has the registers,
+ * include it in the check anyway.
+ */
+ if (INTEL_INFO(dev)-gen 5 !IS_G4X(dev))
+ return true;
+
+ /*
+ * A DSPSURFLIVE check isn't enough in case the mmio and CS flips
+ * used the same base address. In that case the mmio flip might
+ * have completed, but the CS hasn't even executed the flip yet.
+ *
+ * A flip count check isn't enough as the CS might have updated
+ * the base address just after start of vblank, but before we
+ * managed to process the interrupt. This means we'd complete the
+ * CS flip too soon.
+ *
+ * Combining both checks should get us a good enough result. It may
+ * still happen that the CS flip has been executed, but has not
+ * yet actually completed. But in case the base address is the same
+ * anyway, we don't really care.
+ */
+ return (I915_READ(DSPSURFLIVE(crtc-plane)) ~0xfff) ==
+
Re: [Intel-gfx] [PATCHv5] drm/i915: use waitqueue in wait for vblank
On Wed, May 21, 2014 at 01:09:58PM +0530, Arun R Murthy wrote:
Instead of using sleep functions in wait for vblank, which wakes up on
sleep timeout aften, thereby scheduler provoking scheduler. Hence use
waitqueue in wait for vblank.
Signed-off-by: Arun R Murthy arun.r.mur...@intel.com
You're rebuilding infrastructure that drm_irq.c already has. In latest
nightly all you really need should be
drm_crtc_vblank_get();
wait_event_timeout(dev-vblank[drm_crtc_index()].queue, ...);
drm_crtc_vblank_put();
If you fancy it you could wrap this little sequence up into a new helper
in drm_irq.c called drm_crtc_wait_one_vblank(). Please add kerneldoc for
that, too.
Also latest upstream WARNs if the vblank wait times out, we want that
here, too. And there's no need to optimize the timeout since it's an
exceptional condition which should never happen. If it does it's a driver
bug and needs to be addressed.
Note that you might need to shuffle around the drm_crtc_vblank_on/off
calls a bit to make sure the vblank support is enabled again.
-Daniel
---
drivers/gpu/drm/i915/i915_dma.c |2 ++
drivers/gpu/drm/i915/i915_drv.h |1 +
drivers/gpu/drm/i915/i915_irq.c | 17 +
drivers/gpu/drm/i915/intel_display.c | 10 ++
4 files changed, 22 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 258b1be..896620c 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1506,6 +1506,8 @@ int i915_driver_load(struct drm_device *dev, unsigned
long flags)
mutex_init(dev_priv-dpio_lock);
mutex_init(dev_priv-modeset_restore_lock);
+ init_waitqueue_head(dev_priv-wait_vblank);
+
intel_pm_setup(dev);
intel_display_crc_init(dev);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 728b9c3..449bfef 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1590,6 +1590,7 @@ typedef struct drm_i915_private {
struct i915_dri1_state dri1;
/* Old ums support infrastructure, same warning applies. */
struct i915_ums_state ums;
+ wait_queue_head_t wait_vblank;
} drm_i915_private_t;
static inline struct drm_i915_private *to_i915(const struct drm_device *dev)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 56edff3..f97f0fe 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1513,8 +1513,11 @@ static irqreturn_t valleyview_irq_handler(int irq,
void *arg)
spin_unlock_irqrestore(dev_priv-irq_lock, irqflags);
for_each_pipe(pipe) {
- if (pipe_stats[pipe]
PIPE_START_VBLANK_INTERRUPT_STATUS)
+ if (pipe_stats[pipe]
+ PIPE_START_VBLANK_INTERRUPT_STATUS) {
drm_handle_vblank(dev, pipe);
+ wake_up_interruptible(dev_priv-wait_vblank);
+ }
if (pipe_stats[pipe] PLANE_FLIPDONE_INT_STATUS_VLV) {
intel_prepare_page_flip(dev, pipe);
@@ -3269,8 +3272,10 @@ static irqreturn_t i8xx_irq_handler(int irq, void *arg)
plane = !plane;
if (pipe_stats[pipe] PIPE_VBLANK_INTERRUPT_STATUS
- i8xx_handle_vblank(dev, plane, pipe, iir))
+ i8xx_handle_vblank(dev, plane, pipe, iir)) {
flip_mask = ~DISPLAY_PLANE_FLIP_PENDING(plane);
+ wake_up_interruptible(dev_priv-wait_vblank);
+ }
if (pipe_stats[pipe] PIPE_CRC_DONE_INTERRUPT_STATUS)
i9xx_pipe_crc_irq_handler(dev, pipe);
@@ -3464,8 +3469,10 @@ static irqreturn_t i915_irq_handler(int irq, void *arg)
plane = !plane;
if (pipe_stats[pipe] PIPE_VBLANK_INTERRUPT_STATUS
- i915_handle_vblank(dev, plane, pipe, iir))
+ i915_handle_vblank(dev, plane, pipe, iir)) {
flip_mask = ~DISPLAY_PLANE_FLIP_PENDING(plane);
+ wake_up_interruptible(dev_priv-wait_vblank);
+ }
if (pipe_stats[pipe] PIPE_LEGACY_BLC_EVENT_STATUS)
blc_event = true;
@@ -3709,8 +3716,10 @@ static irqreturn_t i965_irq_handler(int irq, void *arg)
for_each_pipe(pipe) {
if (pipe_stats[pipe]
PIPE_START_VBLANK_INTERRUPT_STATUS
- i915_handle_vblank(dev, pipe, pipe, iir))
+ i915_handle_vblank(dev, pipe, pipe, iir)) {
flip_mask =
Re: [Intel-gfx] [PATCH v2 2/3] drm/i915: Add a brief description of the VLV display PHY internals
On 04/25 20:14, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä ville.syrj...@linux.intel.com
Document the internal structure of the VLV display PHY a bit to help
people understand how the different register blocks relate to each
other.
v2: Add a bit more text
Make it a DOC: comment, but leave the ascii art out since
it would get mangled
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
Documentation/DocBook/drm.tmpl | 4 ++
drivers/gpu/drm/i915/i915_reg.h | 85
+++--
2 files changed, 85 insertions(+), 4 deletions(-)
diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index 4a955b4..e361ccd 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -2942,6 +2942,10 @@ int num_ioctls;/synopsis
probing, so those sections fully apply.
/para
/sect2
+ sect2
+titleDPIO/title
+!Pdrivers/gpu/drm/i915/i915_reg.h DPIO
+ /sect2
/sect1
sect1
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index b6d5045..8e18e8f 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -566,12 +566,89 @@ enum punit_power_well {
#define DSI_PLL_M1_DIV_MASK (0x1ff 0)
#define CCK_DISPLAY_CLOCK_CONTROL0x6b
-/*
- * DPIO - a special bus for various display related registers to hide behind
+/**
+ * DOC: DPIO
+ *
+ * VLV and CHV have slightly peculiar display PHYs for driving DP/HDMI
+ * ports. DPIO is the name given to such a display PHY. These PHYs
+ * don't follow the standard programming model using direct MMIO
+ * registers, and instead their registers must be accessed trough IOSF
+ * sideband. VLV has one such PHY for driving ports B and C, and CHV
+ * adds another PHY for driving port D. Each PHY responds to specific
+ * IOSF-SB port.
+ *
+ * Each display PHY is made up of one or two channels. Each channel
+ * houses a common lane part which contains the PLL and other common
+ * logic. CH0 common lane also contains the IOSF-SB logic for the
+ * Common Register Interface (CRI) ie. the DPIO registers. CRI clock
+ * must be running when any DPIO registers are accessed.
+ *
+ * In addition to having their own registers, the PHYs are also
+ * controlled through some dedicated signals from the display
+ * controller. These include PLL reference clock enable, PLL enable,
+ * and CRI clock selection, for example.
+ *
+ * Eeach channel also has two splines (also called data lanes), and
+ * each spline is made up of one Physical Access Coding Sub-Layer
+ * (PCS) block and two TX lanes. So each channel has two PCS blocks
+ * and four TX lanes. The TX lanes are used as DP lanes or TMDS
+ * data/clock pairs depending on the output type.
+ *
+ * Additionally the PHY also contains an AUX lane with AUX blocks
+ * for each channel. This is used for DP AUX communication, but
+ * this fact isn't really relevant for the driver since AUX is
+ * controlled from the display controller side. No DPIO registers
+ * need to be accessed during AUX communication,
+ *
+ * Generally the common lane corresponds to the pipe and
+ * the spline (PCS/TX) correponds to the port.
+ *
+ * For dual channel PHY (VLV/CHV):
+ *
+ * pipe A == CMN/PLL/REF CH0
*
- * DPIO is VLV only.
+ * pipe B == CMN/PLL/REF CH1
+ *
+ * port B == PCS/TX CH0
+ *
+ * port C == PCS/TX CH1
+ *
+ * This is especially important when we cross the streams
+ * ie. drive port B with pipe B, or port C with pipe A.
+ *
Do you want to add something like the PHY actually allow PLL CH0 to supply clock
to both ports, same as PLL CH1. But this is something i915 not supported yet,
for power saving purpose.
+ * For single channel PHY (CHV):
+ *
+ * pipe C == CMN/PLL/REF CH0
+ *
+ * port D == PCS/TX CH0
+ *
+ * Note: digital port B is DDI0, digital port C is DDI1,
+ * digital port D is DDI2
+ */
+/*
+ * Dual channel PHY (VLV/CHV)
+ * -
+ * | CH0 | CH1 |
+ * | CMN/PLL/REF | CMN/PLL/REF |
There is a AUX for both CH0 and CH1.
Other than this,
Reviewed-by: Chon Ming Lee chon.ming@intel.com
+ * |---|---| Display PHY
+ * | PCS01 | PCS23 | PCS01 | PCS23 |
+ * |---|---|---|---|
+ * |TX0|TX1|TX2|TX3|TX0|TX1|TX2|TX3|
+ * -
+ * | DDI0 | DDI1 | DP/HDMI ports
+ * -
*
- * Note: digital port B is DDI0, digital pot C is DDI1
+ * Single channel PHY (CHV)
+ * -
+ * | CH0 |
+ * | CMN/PLL/REF |
+ * |---| Display PHY
+ * | PCS01 | PCS23 |
+ * |---|---|
+ * |TX0|TX1|TX2|TX3|
+ * -
+ * | DDI2 | DP/HDMI port
+ * -
*/
#define DPIO_DEVFN 0
Re: [Intel-gfx] [PATCH 00/12] irq vblank handling rework
On Wed, May 21, 2014 at 11:26:55AM +0300, Ville Syrjälä wrote: For everything but the reset_vblank_counter() thing: Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com And another caveat: I only glanced at the crtc_helper stuff. It looks sane but I didn't go reading through the drivers to figure out if they would fall over or work. Oh, the rfc was really just that. I don't have any plans to burn my hands trying to merge those patches ;-) Especially since interest from non-i915 hackers seems to be low. About the reset_vblank_counter(), I think we might still need it to keep the counter sane when the power well goes off. But I think we have other problems on that front esp. with suspend to disk. There the counter should definitely get reset on all platforms, so we migth apply any kind of diff to the user visible value. The fix would likely be to skip the diff adjustment when resuming. I tried to take a quick look at that yesterday but there was something really fishy happening as the code didn't seem to observe the wraparound at all, even though I confirmed w/ intel_reg_read that it definitely did appear to wrap. I'll take another look at it today. Another idea might be to rip out the diff adjustment altogether. That should just mean that the user visible counter wouldn't advance at all between drm_vblank_off() and drm_vblank_on(). But that might actually be the sane thing to do. Maybe we should just do a +1 there to make sure we don't report the same value before and after modeset. It should fix both the suspend problems and the power well problem. Hm, like I've mentioned yesterday on irc the tests I have actually pass, at least if I throw your sanitize_crtc patch on top. vblank frame counter values monotonically increase across suspend/resume, runtime pm and anything else I manged to throw at it. And the limit in the test is 100 frames later, but I've only observed a few tens at most. So I think the code as-is actually works. Whether intentional or not is still under dispute though ;-) The real problem I have with the hsw counter reset is that the same issue should affect _any_ platform where we support runtime pm. Like snb or byt. But the code isn't there. Also if we have such a bug then it will also affect hibernate and suspend to disk. Which means that this should be done in drm_crtc_vblank_off/on, not in the guts of some random platforms runtime pm code. So in my opinion the hsw vblank_count reset code needs to go anyway because: - Either it isn't need any more (and we have the tests for this now) and it's just cargo-culted duct-tape. - Or we need, but then it's in the wrong spot. Given that can you reconsider that patch please? Thanks, Daniel On Wed, May 14, 2014 at 08:51:02PM +0200, Daniel Vetter wrote: Hi all, This is Ville's vblank rework series, slightly pimped. I've added kerneldoc, some polish and also some additional nasty igt testcases for these crtc on/off vs vblank issues. Seems all to hold together nicely. Now one thing I wanted to do is roll this out across all drm drivers, but that looks ugly: Many drivers don't support vblanks really, and I couldn't fully audit whether they'd e.g. blow up when userspace tries to use the vblank wait ioctl. But I think we want to have more sanity since otherwise userspace is doomed to carry countless ugly hacks around forever. The last two patches are my attempt at that. By doing the drm_vblank_on/off dance in the crtc helpers after we know that the crtc is on/off we should have at least a somewhat sane baseline behaviour. Note that since drm_vblank_on/off are idempotent drivers are free to call these functions in their callback at an earlier time, where it makes more sense. But at least it's guaranteed to happen. Otoh drivers still need to manually complete pageflips, so this doesn't solve all the issues. But until we have a solid cross-driver testsuite for these corner-cases (all the interesting stuff happens when you race vblank waits against concurrent modesets, dpms, or system/runtime suspend/resume) we're pretty much guaranteed to have some that works at best occasionally and is guaranteed to have different behaviour in corner cases. Note that the patches won't degrade behaviour for existing drivers, the drm core changes simply allow us to finally fix things up correctly. I guess in a way this is a more generic discussion for the drm subsystem at large. Coments and review highly welcome. Cheers, Daniel Daniel Vetter (8): drm/i915: Remove drm_vblank_pre/post_modeset calls drm/doc: Discourage usage of MODESET_CTL ioctl drm/irq: kerneldoc polish drm/irq: Add kms-native crtc interface functions drm/i915: rip our vblank reset hacks for runtime PM drm/i915: Accurately initialize fifo underrun state on gmch platforms [RFC] drm/irq: More robustness in drm_vblank_on|off [RFC]
Re: [Intel-gfx] [PATCH 00/12] irq vblank handling rework
For everything but the reset_vblank_counter() thing: Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com And another caveat: I only glanced at the crtc_helper stuff. It looks sane but I didn't go reading through the drivers to figure out if they would fall over or work. About the reset_vblank_counter(), I think we might still need it to keep the counter sane when the power well goes off. But I think we have other problems on that front esp. with suspend to disk. There the counter should definitely get reset on all platforms, so we migth apply any kind of diff to the user visible value. The fix would likely be to skip the diff adjustment when resuming. I tried to take a quick look at that yesterday but there was something really fishy happening as the code didn't seem to observe the wraparound at all, even though I confirmed w/ intel_reg_read that it definitely did appear to wrap. I'll take another look at it today. Another idea might be to rip out the diff adjustment altogether. That should just mean that the user visible counter wouldn't advance at all between drm_vblank_off() and drm_vblank_on(). But that might actually be the sane thing to do. Maybe we should just do a +1 there to make sure we don't report the same value before and after modeset. It should fix both the suspend problems and the power well problem. On Wed, May 14, 2014 at 08:51:02PM +0200, Daniel Vetter wrote: Hi all, This is Ville's vblank rework series, slightly pimped. I've added kerneldoc, some polish and also some additional nasty igt testcases for these crtc on/off vs vblank issues. Seems all to hold together nicely. Now one thing I wanted to do is roll this out across all drm drivers, but that looks ugly: Many drivers don't support vblanks really, and I couldn't fully audit whether they'd e.g. blow up when userspace tries to use the vblank wait ioctl. But I think we want to have more sanity since otherwise userspace is doomed to carry countless ugly hacks around forever. The last two patches are my attempt at that. By doing the drm_vblank_on/off dance in the crtc helpers after we know that the crtc is on/off we should have at least a somewhat sane baseline behaviour. Note that since drm_vblank_on/off are idempotent drivers are free to call these functions in their callback at an earlier time, where it makes more sense. But at least it's guaranteed to happen. Otoh drivers still need to manually complete pageflips, so this doesn't solve all the issues. But until we have a solid cross-driver testsuite for these corner-cases (all the interesting stuff happens when you race vblank waits against concurrent modesets, dpms, or system/runtime suspend/resume) we're pretty much guaranteed to have some that works at best occasionally and is guaranteed to have different behaviour in corner cases. Note that the patches won't degrade behaviour for existing drivers, the drm core changes simply allow us to finally fix things up correctly. I guess in a way this is a more generic discussion for the drm subsystem at large. Coments and review highly welcome. Cheers, Daniel Daniel Vetter (8): drm/i915: Remove drm_vblank_pre/post_modeset calls drm/doc: Discourage usage of MODESET_CTL ioctl drm/irq: kerneldoc polish drm/irq: Add kms-native crtc interface functions drm/i915: rip our vblank reset hacks for runtime PM drm/i915: Accurately initialize fifo underrun state on gmch platforms [RFC] drm/irq: More robustness in drm_vblank_on|off [RFC] drm/crtc-helper: Enforce sane vblank counter semantics Peter Hurley (1): drm: Use correct spinlock flavor in drm_vblank_get() Ville Syrjälä (3): drm: Make the vblank disable timer per-crtc drm: Make blocking vblank wait return when the vblank interrupts get disabled drm: Add drm_vblank_on() Documentation/DocBook/drm.tmpl | 16 +- drivers/gpu/drm/drm_crtc_helper.c| 12 ++ drivers/gpu/drm/drm_irq.c| 377 ++- drivers/gpu/drm/i915/i915_irq.c | 4 +- drivers/gpu/drm/i915/intel_display.c | 36 ++-- drivers/gpu/drm/i915/intel_drv.h | 2 - drivers/gpu/drm/i915/intel_pm.c | 40 include/drm/drmP.h | 10 +- 8 files changed, 341 insertions(+), 156 deletions(-) -- 1.8.3.1 ___ dri-devel mailing list dri-de...@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCHv5] drm/i915: use waitqueue in wait for vblank
On Wed, May 21, 2014 at 08:54:04AM +0100, Chris Wilson wrote:
On Wed, May 21, 2014 at 01:09:58PM +0530, Arun R Murthy wrote:
diff --git a/drivers/gpu/drm/i915/i915_irq.c
b/drivers/gpu/drm/i915/i915_irq.c
index 56edff3..f97f0fe 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1513,8 +1513,11 @@ static irqreturn_t valleyview_irq_handler(int irq,
void *arg)
spin_unlock_irqrestore(dev_priv-irq_lock, irqflags);
for_each_pipe(pipe) {
- if (pipe_stats[pipe]
PIPE_START_VBLANK_INTERRUPT_STATUS)
+ if (pipe_stats[pipe]
+ PIPE_START_VBLANK_INTERRUPT_STATUS) {
drm_handle_vblank(dev, pipe);
+ wake_up_interruptible(dev_priv-wait_vblank);
We now have intel_handle_vblank() so these chunks can be simplified.
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 4d4a0d9..e1eb564 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -757,12 +757,14 @@ enum transcoder intel_pipe_to_cpu_transcoder(struct
drm_i915_private *dev_priv,
static void g4x_wait_for_vblank(struct drm_device *dev, int pipe)
{
struct drm_i915_private *dev_priv = dev-dev_private;
- u32 frame, frame_reg = PIPE_FRMCOUNT_GM45(pipe);
+ u32 vblank_cnt;
- frame = I915_READ(frame_reg);
+ vblank_cnt = drm_vblank_count(dev, pipe);
- if (wait_for(I915_READ_NOTRACE(frame_reg) != frame, 50))
- DRM_DEBUG_KMS(vblank wait timed out\n);
+ /* TODO: get the vblank time dynamically or from platform data */
+ wait_event_interruptible_timeout(dev_priv-wait_vblank,
+ (vblank_cnt != drm_vblank_count(dev, pipe)),
+ msecs_to_jiffies(16));
Keep the ultimate timeout at 50 until you have evidence you can reduce
it. And then it should be 2x vrefresh interval to be safe. However, you
are likely still hitting the timeout as the vblank irq is not guaranteed
to be enabled here. How safe calling drm_vblank_get() is during modeset
I defer to Ville since he has just taken a pass over the whole mess.
The plan is to make drm_vblank_get() work until drm_vblank_off() has
been called. And when enabling, drm_vblank_get() will succeed only after
drm_vblank_on() has been called. The place where those should end up is
at the start/end of intel_crtc_{disable,enable}_planes(). So you have
access to vblank irqs while planes are getting enabled/disabled, but no
further since we can't guarantee their function once we start shutting
off pipes/ports/etc.
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 2/3] drm/i915: Add a brief description of the VLV display PHY internals
On Wed, May 21, 2014 at 04:31:37PM +0800, Lee, Chon Ming wrote:
On 04/25 20:14, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä ville.syrj...@linux.intel.com
Document the internal structure of the VLV display PHY a bit to help
people understand how the different register blocks relate to each
other.
v2: Add a bit more text
Make it a DOC: comment, but leave the ascii art out since
it would get mangled
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
Documentation/DocBook/drm.tmpl | 4 ++
drivers/gpu/drm/i915/i915_reg.h | 85
+++--
2 files changed, 85 insertions(+), 4 deletions(-)
diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index 4a955b4..e361ccd 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -2942,6 +2942,10 @@ int num_ioctls;/synopsis
probing, so those sections fully apply.
/para
/sect2
+ sect2
+titleDPIO/title
+!Pdrivers/gpu/drm/i915/i915_reg.h DPIO
+ /sect2
/sect1
sect1
diff --git a/drivers/gpu/drm/i915/i915_reg.h
b/drivers/gpu/drm/i915/i915_reg.h
index b6d5045..8e18e8f 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -566,12 +566,89 @@ enum punit_power_well {
#define DSI_PLL_M1_DIV_MASK (0x1ff 0)
#define CCK_DISPLAY_CLOCK_CONTROL 0x6b
-/*
- * DPIO - a special bus for various display related registers to hide
behind
+/**
+ * DOC: DPIO
+ *
+ * VLV and CHV have slightly peculiar display PHYs for driving DP/HDMI
+ * ports. DPIO is the name given to such a display PHY. These PHYs
+ * don't follow the standard programming model using direct MMIO
+ * registers, and instead their registers must be accessed trough IOSF
+ * sideband. VLV has one such PHY for driving ports B and C, and CHV
+ * adds another PHY for driving port D. Each PHY responds to specific
+ * IOSF-SB port.
+ *
+ * Each display PHY is made up of one or two channels. Each channel
+ * houses a common lane part which contains the PLL and other common
+ * logic. CH0 common lane also contains the IOSF-SB logic for the
+ * Common Register Interface (CRI) ie. the DPIO registers. CRI clock
+ * must be running when any DPIO registers are accessed.
+ *
+ * In addition to having their own registers, the PHYs are also
+ * controlled through some dedicated signals from the display
+ * controller. These include PLL reference clock enable, PLL enable,
+ * and CRI clock selection, for example.
+ *
+ * Eeach channel also has two splines (also called data lanes), and
+ * each spline is made up of one Physical Access Coding Sub-Layer
+ * (PCS) block and two TX lanes. So each channel has two PCS blocks
+ * and four TX lanes. The TX lanes are used as DP lanes or TMDS
+ * data/clock pairs depending on the output type.
+ *
+ * Additionally the PHY also contains an AUX lane with AUX blocks
+ * for each channel. This is used for DP AUX communication, but
+ * this fact isn't really relevant for the driver since AUX is
+ * controlled from the display controller side. No DPIO registers
+ * need to be accessed during AUX communication,
+ *
+ * Generally the common lane corresponds to the pipe and
+ * the spline (PCS/TX) correponds to the port.
+ *
+ * For dual channel PHY (VLV/CHV):
+ *
+ * pipe A == CMN/PLL/REF CH0
*
- * DPIO is VLV only.
+ * pipe B == CMN/PLL/REF CH1
+ *
+ * port B == PCS/TX CH0
+ *
+ * port C == PCS/TX CH1
+ *
+ * This is especially important when we cross the streams
+ * ie. drive port B with pipe B, or port C with pipe A.
+ *
Do you want to add something like the PHY actually allow PLL CH0 to supply
clock
to both ports, same as PLL CH1. But this is something i915 not supported yet,
for power saving purpose.
Maybe we can add the comment if and when we implement it.
I'm not even sure if we should implement it since it might lead to
blinking displays if we have to reroute the PLLs for active pipes.
Although maybe it's possible to swap over to the pipe's own PLL from
the shared case w/o blinking, but that would require special code
in the modeset path since we'd need to make sure we fire up the new
PLL first, then swap the PLLs, and only then can reprogram the old
PLL.
So unless the power savings are really significant I'm not sure anyone
will bother with this.
OTOH we already have potential display blinking due to the cdclk change
logic, so maybe we don't care that much.
+ * For single channel PHY (CHV):
+ *
+ * pipe C == CMN/PLL/REF CH0
+ *
+ * port D == PCS/TX CH0
+ *
+ * Note: digital port B is DDI0, digital port C is DDI1,
+ * digital port D is DDI2
+ */
+/*
+ * Dual channel PHY (VLV/CHV)
+ * -
+ * | CH0
Re: [Intel-gfx] [PATCH 00/12] irq vblank handling rework
On Wed, May 21, 2014 at 10:35:59AM +0200, Daniel Vetter wrote: On Wed, May 21, 2014 at 11:26:55AM +0300, Ville Syrjälä wrote: For everything but the reset_vblank_counter() thing: Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com And another caveat: I only glanced at the crtc_helper stuff. It looks sane but I didn't go reading through the drivers to figure out if they would fall over or work. Oh, the rfc was really just that. I don't have any plans to burn my hands trying to merge those patches ;-) Especially since interest from non-i915 hackers seems to be low. About the reset_vblank_counter(), I think we might still need it to keep the counter sane when the power well goes off. But I think we have other problems on that front esp. with suspend to disk. There the counter should definitely get reset on all platforms, so we migth apply any kind of diff to the user visible value. The fix would likely be to skip the diff adjustment when resuming. I tried to take a quick look at that yesterday but there was something really fishy happening as the code didn't seem to observe the wraparound at all, even though I confirmed w/ intel_reg_read that it definitely did appear to wrap. I'll take another look at it today. Another idea might be to rip out the diff adjustment altogether. That should just mean that the user visible counter wouldn't advance at all between drm_vblank_off() and drm_vblank_on(). But that might actually be the sane thing to do. Maybe we should just do a +1 there to make sure we don't report the same value before and after modeset. It should fix both the suspend problems and the power well problem. Hm, like I've mentioned yesterday on irc the tests I have actually pass, at least if I throw your sanitize_crtc patch on top. vblank frame counter values monotonically increase across suspend/resume, runtime pm and anything else I manged to throw at it. And the limit in the test is 100 frames later, but I've only observed a few tens at most. So I think the code as-is actually works. Whether intentional or not is still under dispute though ;-) The real problem I have with the hsw counter reset is that the same issue should affect _any_ platform where we support runtime pm. Like snb or byt. But the code isn't there. Also if we have such a bug then it will also affect hibernate and suspend to disk. Which means that this should be done in drm_crtc_vblank_off/on, not in the guts of some random platforms runtime pm code. So in my opinion the hsw vblank_count reset code needs to go anyway because: - Either it isn't need any more (and we have the tests for this now) and it's just cargo-culted duct-tape. - Or we need, but then it's in the wrong spot. Given that can you reconsider that patch please? Yeah. So as discussed on irc I think the right fix would be to sample the current counter in drm_vblank_on(), stick it into dev-vblank[crtc].last, but skip the diff adjustment to the user visible counter (maybe just +1 to make sure we never report the same value on both sides of a modeset). That should cover both the suspend case and the power well case. I can go and experiment with this a bit... So I agree that the current code isn't the way things should be done. And since I now have an idea how it should be done, I'm fine with ripping the current thing out. So you can add: Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 9/9] drm/i915: Fix gen2 and hsw+ scanline counter
On Fri, May 16, 2014 at 11:54:09AM +0530, sourab gupta wrote: On Fri, May 16, 2014 at 11:03 AM, akash goel akash.go...@gmail.com wrote: Sorry not aware of this specific difference in the starting value of scanline counter for HSW+ ( gen 2), but implementation wise, patch looks fine. Reviewed-by: Akash Goel akash.go...@gmail.com Don't have enough info about the initial scanline counter values for HSW+ and gen2. Otherwise, you can add my r-b tag Reviewed-by: Sourab Gupta sourabgu...@gmail.com Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=78997 -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/4] drm/i915: move power domain init earlier during system resume
From: Imre Deak imre.d...@intel.com
This is commit 76c4b250080fff6e4befaa36199424 upstream.
During resume the intel hda audio driver depends on the i915 driver
reinitializing the audio power domain. Since the order of calling the
i915 resume handler wrt. that of the audio driver is not guaranteed,
move the power domain reinitialization step to the resume_early
handler. This is guaranteed to run before the resume handler of any
other driver.
The power domain initialization in turn requires us to enable the i915
pci device first, so move that part earlier too.
Accordingly disabling of the i915 pci device should happen after the
audio suspend handler ran. So move the disabling later from the i915
resume handler to the resume_late handler.
v2:
- move intel_uncore_sanitize/early_sanitize earlier too, so they don't
get reordered wrt. intel_power_domains_init_hw()
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=76152
Signed-off-by: Imre Deak imre.d...@intel.com
Reviewed-by: Takashi Iwai ti...@suse.de
Cc: sta...@vger.kernel.org
[danvet: Add cc: stable and loud comments that this is just a hack.]
[danvet: Fix Should it be static? sparse warning reported by Wu
Fengguang's kbuilder.]
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/i915_drv.c | 90 ++---
1 file changed, 75 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index ec7bb0fc71bc..9debd6e74439 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -614,15 +614,20 @@ static void intel_resume_hotplug(struct drm_device *dev)
drm_helper_hpd_irq_event(dev);
}
+static int i915_drm_thaw_early(struct drm_device *dev)
+{
+ intel_uncore_early_sanitize(dev);
+ intel_uncore_sanitize(dev);
+ intel_power_domains_init_hw(dev);
+
+ return 0;
+}
+
static int __i915_drm_thaw(struct drm_device *dev, bool restore_gtt_mappings)
{
struct drm_i915_private *dev_priv = dev-dev_private;
int error = 0;
- intel_uncore_early_sanitize(dev);
-
- intel_uncore_sanitize(dev);
-
if (drm_core_check_feature(dev, DRIVER_MODESET)
restore_gtt_mappings) {
mutex_lock(dev-struct_mutex);
@@ -630,8 +635,6 @@ static int __i915_drm_thaw(struct drm_device *dev, bool
restore_gtt_mappings)
mutex_unlock(dev-struct_mutex);
}
- intel_power_domains_init_hw(dev);
-
i915_restore_state(dev);
intel_opregion_setup(dev);
@@ -700,19 +703,33 @@ static int i915_drm_thaw(struct drm_device *dev)
return __i915_drm_thaw(dev, true);
}
-int i915_resume(struct drm_device *dev)
+static int i915_resume_early(struct drm_device *dev)
{
- struct drm_i915_private *dev_priv = dev-dev_private;
- int ret;
-
if (dev-switch_power_state == DRM_SWITCH_POWER_OFF)
return 0;
+ /*
+* We have a resume ordering issue with the snd-hda driver also
+* requiring our device to be power up. Due to the lack of a
+* parent/child relationship we currently solve this with an early
+* resume hook.
+*
+* FIXME: This should be solved with a special hdmi sink device or
+* similar so that power domains can be employed.
+*/
if (pci_enable_device(dev-pdev))
return -EIO;
pci_set_master(dev-pdev);
+ return i915_drm_thaw_early(dev);
+}
+
+int i915_resume(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ int ret;
+
/*
* Platforms with opregion should have sane BIOS, older ones (gen3 and
* earlier) need to restore the GTT mappings since the BIOS might clear
@@ -726,6 +743,14 @@ int i915_resume(struct drm_device *dev)
return 0;
}
+static int i915_resume_legacy(struct drm_device *dev)
+{
+ i915_resume_early(dev);
+ i915_resume(dev);
+
+ return 0;
+}
+
/**
* i915_reset - reset chip after a hang
* @dev: drm device to reset
@@ -846,7 +871,6 @@ static int i915_pm_suspend(struct device *dev)
{
struct pci_dev *pdev = to_pci_dev(dev);
struct drm_device *drm_dev = pci_get_drvdata(pdev);
- int error;
if (!drm_dev || !drm_dev-dev_private) {
dev_err(dev, DRM not initialized, aborting suspend.\n);
@@ -856,9 +880,25 @@ static int i915_pm_suspend(struct device *dev)
if (drm_dev-switch_power_state == DRM_SWITCH_POWER_OFF)
return 0;
- error = i915_drm_freeze(drm_dev);
- if (error)
- return error;
+ return i915_drm_freeze(drm_dev);
+}
+
+static int i915_pm_suspend_late(struct device *dev)
+{
+ struct pci_dev *pdev = to_pci_dev(dev);
+ struct drm_device *drm_dev = pci_get_drvdata(pdev);
+
+ /*
+* We have a suspedn ordering issue with the snd-hda driver also
+*
[Intel-gfx] [PATCH 1/4] drm/i915: Disable self-refresh for untiled fbs on i915gm
This is commit 2ab1bc9df01dbc19b55b2271100db7 upstream.
Apparently it doesn't work. X-tiled self-refresh works flawlessly
otoh. Apparently X still works correctly with linear framebuffers, so
might just be an issue with the initial modeset. It's unclear whether
this just borked wm setup from our side or a hw restriction, but just
disabling gets things going.
Note that this regression was only brought to light with
commit 3f2dc5ac05714711fc14f2bf0ee5e42d5c08c581
Author: Ville Syrjälä ville.syrj...@linux.intel.com
Date: Fri Jan 10 14:06:47 2014 +0200
drm/i915: Fix 915GM self-refresh enable/disable
before that self-refresh for i915GM didn't work at all.
Kudos to Ville for spotting a little bug in the original patch I've
attached to the bug.
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
Cc: Chris Wilson ch...@chris-wilson.co.uk
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=76103
Tested-by: Krzysztof Mazur krzys...@podlesie.net
Cc: Krzysztof Mazur krzys...@podlesie.net
Cc: sta...@vger.kernel.org
Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
[Jani: rebase on top of drm-next with primary plane support.]
Signed-off-by: Jani Nikula jani.nik...@intel.com
---
drivers/gpu/drm/i915/intel_pm.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index e1fc35a72656..bd1b00344dac 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1539,6 +1539,16 @@ static void i9xx_update_wm(struct drm_crtc *unused_crtc)
DRM_DEBUG_KMS(FIFO watermarks - A: %d, B: %d\n, planea_wm, planeb_wm);
+ if (IS_I915GM(dev) enabled) {
+ struct intel_framebuffer *fb;
+
+ fb = to_intel_framebuffer(enabled-fb);
+
+ /* self-refresh seems busted with untiled */
+ if (fb-obj-tiling_mode == I915_TILING_NONE)
+ enabled = NULL;
+ }
+
/*
* Overlay gets an aggressive default since video jitter is bad.
*/
--
1.9.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/4] drm/i915: quirk invert brightness for Acer Aspire 5336
From: Jani Nikula jani.nik...@intel.com
This is commit 0f540c3a7cfb91c9d7a19eb0c95c24 upstream.
Since
commit ee1452d7458451a7508e0663553ce88d63958157
Author: Jani Nikula jani.nik...@intel.com
Date: Fri Sep 20 15:05:30 2013 +0300
drm/i915: assume all GM45 Acer laptops use inverted backlight PWM
failed and was later reverted in
commit be505f643925e257087247b996cd8ece787c12af
Author: Alexander van Heukelum heuke...@fastmail.fm
Date: Sat Dec 28 21:00:39 2013 +0100
Revert drm/i915: assume all GM45 Acer laptops use inverted backlight PWM
fix the individual broken machine instead.
Note to backporters:
http://patchwork.freedesktop.org/patch/17837/
is the patch you want for 3.13 and older.
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=54171
Reference: http://mid.gmane.org/dub115-w7628c7c710ea51aa110cd4a5...@phx.gbl
CC: sta...@vger.kernel.org
Signed-off-by: Jani Nikula jani.nik...@intel.com
Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
[danvet: Patch mangling for 3.14 plus adding the link to the original
for 3.13.]
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/intel_display.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 9b8a7c7ea7fc..9847988949ad 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10839,6 +10839,9 @@ static struct intel_quirk intel_quirks[] = {
/* Acer Aspire 4736Z */
{ 0x2a42, 0x1025, 0x0260, quirk_invert_brightness },
+
+ /* Acer Aspire 5336 */
+ { 0x2a42, 0x1025, 0x048a, quirk_invert_brightness },
};
static void intel_init_quirks(struct drm_device *dev)
--
1.9.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/4] drm/i915: Fix unsafe loop iteration over vma whilst unbinding them
From: Chris Wilson ch...@chris-wilson.co.uk
This is commit df6f783a4ef6790780a67c491897ac upstream.
On non-LLC platforms, when changing the cache level of an object, we may
need to unbind it so that prefetching across page boundaries does not
cross into a different memory domain. This requires us to unbind
conflicting vma, but we did so iterating over the objects vma in an
unsafe manner (as the list was being modified as we iterated).
The regression was introduced in
commit 3089c6f239d7d2c4cb2dd5c353e8984cf79af1d7
Author: Ben Widawsky b...@bwidawsk.net
Date: Wed Jul 31 17:00:03 2013 -0700
drm/i915: make caching operate on all address spaces
apparently as far back as v3.12-rc1, but it has only just begun to
trigger real world bug reports.
Reported-and-tested-by: Nikolay Martynov mar.ko...@gmail.com
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=76384
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Cc: Ben Widawsky b...@bwidawsk.net
Cc: sta...@vger.kernel.org
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/i915_gem.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 00c836154725..3ecb332e7cfa 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3529,7 +3529,7 @@ int i915_gem_object_set_cache_level(struct
drm_i915_gem_object *obj,
{
struct drm_device *dev = obj-base.dev;
drm_i915_private_t *dev_priv = dev-dev_private;
- struct i915_vma *vma;
+ struct i915_vma *vma, *next;
int ret;
if (obj-cache_level == cache_level)
@@ -3540,7 +3540,7 @@ int i915_gem_object_set_cache_level(struct
drm_i915_gem_object *obj,
return -EBUSY;
}
- list_for_each_entry(vma, obj-vma_list, vma_link) {
+ list_for_each_entry_safe(vma, next, obj-vma_list, vma_link) {
if (!i915_gem_valid_gtt_space(dev, vma-node, cache_level)) {
ret = i915_vma_unbind(vma);
if (ret)
--
1.9.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] drm/i915 stable backports
Hi Greg, This is a set of drm/i915 patches which didn't apply cleanly on for 3.14. All absed on 3.14.4. I've left out the bdw patches for now and will sign up someone else for that task. Cheers, Daniel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/3] drm/i915: Re-enable vblank irqs for already active pipes
On Tue, May 20, 2014 at 07:31:33PM +0200, Daniel Vetter wrote: On Tue, May 20, 2014 at 05:20:05PM +0300, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com If a pipe is already active when we init/resume there might not be a full modeset afterwards so drm_vblank_on() may not get called. In such a case if someone is holding a vblank reference across a suspend/resume cycle drm_vblank_get() called after resuming won't re-enable the vblank interrupts. So in order to make sure vblank interrupts get re-enabled post-resume, call drm_vblank_on() in intel_sanitize_crtc() if the crtc is already active. v2: Also drm_vblank_off() if the pipe got disabled magically Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com This seems to duct-tape over the funny failure I'm seeing on my snb where kms_flip/vblank-vs-modeset|dpms-suspend work nicely, but kms_flip/vblank-vs-suspend has some totally hilarious random vblank frame counter after the modeset. Testecase: igt/kms_flip/vblank-vs-suspend Tested-by: Daniel Vetter daniel.vet...@ffwll.ch Now the problem I have: We already call this in the crtc_enable hook. Why does calling this here again add the necessary magic? /me has no clue Well, we can figure this out later. Queued for -next, thanks for the patch. -Daniel Cheers, Daniel --- drivers/gpu/drm/i915/intel_display.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 9420f4f..2e9f0b0 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -11708,6 +11708,12 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc) reg = PIPECONF(crtc-config.cpu_transcoder); I915_WRITE(reg, I915_READ(reg) ~PIPECONF_FRAME_START_DELAY_MASK); + /* restore vblank interrupts to correct state */ + if (crtc-active) + drm_vblank_on(dev, crtc-pipe); + else + drm_vblank_off(dev, crtc-pipe); + /* We need to sanitize the plane - pipe mapping first because this will * disable the crtc (and hence change the state) if it is wrong. Note * that gen4+ has a fixed plane - pipe mapping. */ -- 1.8.5.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 2/3] drm/i915: Add a brief description of the VLV display PHY internals
-Original Message-
From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
Sent: Wednesday, May 21, 2014 4:50 PM
To: Lee, Chon Ming
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH v2 2/3] drm/i915: Add a brief description of
the VLV display PHY internals
On Wed, May 21, 2014 at 04:31:37PM +0800, Lee, Chon Ming wrote:
On 04/25 20:14, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä ville.syrj...@linux.intel.com
Document the internal structure of the VLV display PHY a bit to help
people understand how the different register blocks relate to each
other.
v2: Add a bit more text
Make it a DOC: comment, but leave the ascii art out since
it would get mangled
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
Documentation/DocBook/drm.tmpl | 4 ++
drivers/gpu/drm/i915/i915_reg.h | 85
+++--
2 files changed, 85 insertions(+), 4 deletions(-)
diff --git a/Documentation/DocBook/drm.tmpl
b/Documentation/DocBook/drm.tmpl index 4a955b4..e361ccd 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -2942,6 +2942,10 @@ int num_ioctls;/synopsis
probing, so those sections fully apply.
/para
/sect2
+ sect2
+titleDPIO/title
+!Pdrivers/gpu/drm/i915/i915_reg.h DPIO
+ /sect2
/sect1
sect1
diff --git a/drivers/gpu/drm/i915/i915_reg.h
b/drivers/gpu/drm/i915/i915_reg.h index b6d5045..8e18e8f 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -566,12 +566,89 @@ enum punit_power_well {
#define DSI_PLL_M1_DIV_MASK (0x1ff 0)
#define CCK_DISPLAY_CLOCK_CONTROL0x6b
-/*
- * DPIO - a special bus for various display related registers to
hide behind
+/**
This is something I miss out. The /** should be /* to match what you want to
do in the first patch.
Regards,
Chon Ming
+ * DOC: DPIO
+ *
+ * VLV and CHV have slightly peculiar display PHYs for driving
+DP/HDMI
+ * ports. DPIO is the name given to such a display PHY. These PHYs
+ * don't follow the standard programming model using direct MMIO
+ * registers, and instead their registers must be accessed trough
+IOSF
+ * sideband. VLV has one such PHY for driving ports B and C, and
+CHV
+ * adds another PHY for driving port D. Each PHY responds to
+specific
+ * IOSF-SB port.
+ *
+ * Each display PHY is made up of one or two channels. Each channel
+ * houses a common lane part which contains the PLL and other
+common
+ * logic. CH0 common lane also contains the IOSF-SB logic for the
+ * Common Register Interface (CRI) ie. the DPIO registers. CRI
+clock
+ * must be running when any DPIO registers are accessed.
+ *
+ * In addition to having their own registers, the PHYs are also
+ * controlled through some dedicated signals from the display
+ * controller. These include PLL reference clock enable, PLL
+enable,
+ * and CRI clock selection, for example.
+ *
+ * Eeach channel also has two splines (also called data lanes), and
+ * each spline is made up of one Physical Access Coding Sub-Layer
+ * (PCS) block and two TX lanes. So each channel has two PCS blocks
+ * and four TX lanes. The TX lanes are used as DP lanes or TMDS
+ * data/clock pairs depending on the output type.
+ *
+ * Additionally the PHY also contains an AUX lane with AUX blocks
+ * for each channel. This is used for DP AUX communication, but
+ * this fact isn't really relevant for the driver since AUX is
+ * controlled from the display controller side. No DPIO registers
+ * need to be accessed during AUX communication,
+ *
+ * Generally the common lane corresponds to the pipe and
+ * the spline (PCS/TX) correponds to the port.
+ *
+ * For dual channel PHY (VLV/CHV):
+ *
+ * pipe A == CMN/PLL/REF CH0
*
- * DPIO is VLV only.
+ * pipe B == CMN/PLL/REF CH1
+ *
+ * port B == PCS/TX CH0
+ *
+ * port C == PCS/TX CH1
+ *
+ * This is especially important when we cross the streams
+ * ie. drive port B with pipe B, or port C with pipe A.
+ *
Do you want to add something like the PHY actually allow PLL CH0 to
supply clock to both ports, same as PLL CH1. But this is something
i915 not supported yet, for power saving purpose.
Maybe we can add the comment if and when we implement it.
I'm not even sure if we should implement it since it might lead to blinking
displays if we have to reroute the PLLs for active pipes.
Although maybe it's possible to swap over to the pipe's own PLL from the
shared case w/o blinking, but that would require special code in the modeset
path since we'd need to make sure we fire up the new PLL first, then swap
the PLLs, and only then can
Re: [Intel-gfx] [PATCH v2 2/3] drm/i915: Add a brief description of the VLV display PHY internals
On Wed, May 21, 2014 at 09:58:35AM +, Lee, Chon Ming wrote:
-Original Message-
From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
Sent: Wednesday, May 21, 2014 4:50 PM
To: Lee, Chon Ming
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH v2 2/3] drm/i915: Add a brief description of
the VLV display PHY internals
On Wed, May 21, 2014 at 04:31:37PM +0800, Lee, Chon Ming wrote:
On 04/25 20:14, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä ville.syrj...@linux.intel.com
Document the internal structure of the VLV display PHY a bit to help
people understand how the different register blocks relate to each
other.
v2: Add a bit more text
Make it a DOC: comment, but leave the ascii art out since
it would get mangled
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
Documentation/DocBook/drm.tmpl | 4 ++
drivers/gpu/drm/i915/i915_reg.h | 85
+++--
2 files changed, 85 insertions(+), 4 deletions(-)
diff --git a/Documentation/DocBook/drm.tmpl
b/Documentation/DocBook/drm.tmpl index 4a955b4..e361ccd 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -2942,6 +2942,10 @@ int num_ioctls;/synopsis
probing, so those sections fully apply.
/para
/sect2
+ sect2
+titleDPIO/title
+!Pdrivers/gpu/drm/i915/i915_reg.h DPIO
+ /sect2
/sect1
sect1
diff --git a/drivers/gpu/drm/i915/i915_reg.h
b/drivers/gpu/drm/i915/i915_reg.h index b6d5045..8e18e8f 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -566,12 +566,89 @@ enum punit_power_well {
#define DSI_PLL_M1_DIV_MASK (0x1ff 0)
#define CCK_DISPLAY_CLOCK_CONTROL 0x6b
-/*
- * DPIO - a special bus for various display related registers to
hide behind
+/**
This is something I miss out. The /** should be /* to match what you want
to do in the first patch.
No, this time around we really want this comment in the generated docs.
Regards,
Chon Ming
+ * DOC: DPIO
+ *
+ * VLV and CHV have slightly peculiar display PHYs for driving
+DP/HDMI
+ * ports. DPIO is the name given to such a display PHY. These PHYs
+ * don't follow the standard programming model using direct MMIO
+ * registers, and instead their registers must be accessed trough
+IOSF
+ * sideband. VLV has one such PHY for driving ports B and C, and
+CHV
+ * adds another PHY for driving port D. Each PHY responds to
+specific
+ * IOSF-SB port.
+ *
+ * Each display PHY is made up of one or two channels. Each channel
+ * houses a common lane part which contains the PLL and other
+common
+ * logic. CH0 common lane also contains the IOSF-SB logic for the
+ * Common Register Interface (CRI) ie. the DPIO registers. CRI
+clock
+ * must be running when any DPIO registers are accessed.
+ *
+ * In addition to having their own registers, the PHYs are also
+ * controlled through some dedicated signals from the display
+ * controller. These include PLL reference clock enable, PLL
+enable,
+ * and CRI clock selection, for example.
+ *
+ * Eeach channel also has two splines (also called data lanes), and
+ * each spline is made up of one Physical Access Coding Sub-Layer
+ * (PCS) block and two TX lanes. So each channel has two PCS blocks
+ * and four TX lanes. The TX lanes are used as DP lanes or TMDS
+ * data/clock pairs depending on the output type.
+ *
+ * Additionally the PHY also contains an AUX lane with AUX blocks
+ * for each channel. This is used for DP AUX communication, but
+ * this fact isn't really relevant for the driver since AUX is
+ * controlled from the display controller side. No DPIO registers
+ * need to be accessed during AUX communication,
+ *
+ * Generally the common lane corresponds to the pipe and
+ * the spline (PCS/TX) correponds to the port.
+ *
+ * For dual channel PHY (VLV/CHV):
+ *
+ * pipe A == CMN/PLL/REF CH0
*
- * DPIO is VLV only.
+ * pipe B == CMN/PLL/REF CH1
+ *
+ * port B == PCS/TX CH0
+ *
+ * port C == PCS/TX CH1
+ *
+ * This is especially important when we cross the streams
+ * ie. drive port B with pipe B, or port C with pipe A.
+ *
Do you want to add something like the PHY actually allow PLL CH0 to
supply clock to both ports, same as PLL CH1. But this is something
i915 not supported yet, for power saving purpose.
Maybe we can add the comment if and when we implement it.
I'm not even sure if we should implement it since it might lead to blinking
displays if we have to reroute the PLLs for
Re: [Intel-gfx] [PATCH 3/3] drm/i915: Provide DPIO diagrams as docboox tables
On Fri, Apr 25, 2014 at 08:14:32PM +0300, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com The ascii art version of the DPIO diagram gets mangled by docbook, so we can't use it there. Insted provide another version built using table. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com I guess one could also add the AUX units here as well. But that's still way better than what we currently have. Reviewed-by: Damien Lespiau damien.lesp...@intel.com -- Damien --- Documentation/DocBook/drm.tmpl | 86 ++ 1 file changed, 86 insertions(+) diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index e361ccd..bf11fe5 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -2945,6 +2945,92 @@ int num_ioctls;/synopsis sect2 titleDPIO/title !Pdrivers/gpu/drm/i915/i915_reg.h DPIO + table id=dpiox2 + titleDual channel PHY (VLV/CHV)/title + tgroup cols=8 + colspec colname=c0 / + colspec colname=c1 / + colspec colname=c2 / + colspec colname=c3 / + colspec colname=c4 / + colspec colname=c5 / + colspec colname=c6 / + colspec colname=c7 / + spanspec spanname=ch0 namest=c0 nameend=c3 / + spanspec spanname=ch1 namest=c4 nameend=c7 / + spanspec spanname=ch0pcs01 namest=c0 nameend=c1 / + spanspec spanname=ch0pcs23 namest=c2 nameend=c3 / + spanspec spanname=ch1pcs01 namest=c4 nameend=c5 / + spanspec spanname=ch1pcs23 namest=c6 nameend=c7 / + thead + row + entry spanname=ch0CH0/entry + entry spanname=ch1CH1/entry + /row + /thead + tbody valign=top align=center + row + entry spanname=ch0CMN/PLL/REF/entry + entry spanname=ch1CMN/PLL/REF/entry + /row + row + entry spanname=ch0pcs01PCS01/entry + entry spanname=ch0pcs23PCS23/entry + entry spanname=ch1pcs01PCS01/entry + entry spanname=ch1pcs23PCS23/entry + /row + row + entryTX0/entry + entryTX1/entry + entryTX2/entry + entryTX3/entry + entryTX0/entry + entryTX1/entry + entryTX2/entry + entryTX3/entry + /row + row + entry spanname=ch0DDI0/entry + entry spanname=ch1DDI1/entry + /row + /tbody + /tgroup + /table + table id=dpiox1 + titleSingle channel PHY (CHV)/title + tgroup cols=4 + colspec colname=c0 / + colspec colname=c1 / + colspec colname=c2 / + colspec colname=c3 / + spanspec spanname=ch0 namest=c0 nameend=c3 / + spanspec spanname=ch0pcs01 namest=c0 nameend=c1 / + spanspec spanname=ch0pcs23 namest=c2 nameend=c3 / + thead + row + entry spanname=ch0CH0/entry + /row + /thead + tbody valign=top align=center + row + entry spanname=ch0CMN/PLL/REF/entry + /row + row + entry spanname=ch0pcs01PCS01/entry + entry spanname=ch0pcs23PCS23/entry + /row + row + entryTX0/entry + entryTX1/entry + entryTX2/entry + entryTX3/entry + /row + row + entry spanname=ch0DDI2/entry + /row + /tbody + /tgroup + /table /sect2 /sect1 -- 1.8.3.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm/i915: State readout and cross-checking for dp_m2_n2
Adding relevant read out comparison code, in check_crtc_state, for the new
member of crtc_config, dp_m2_n2, which was introduced to store link_m_n
values for a DP downclock mode (if available). Suggested by Daniel.
v2: Changed patch title.
Daniel's review comments incorporated.
Added relevant state readout code for M2_N2. dp_m2_n2 comparison to be done
only when high RR is not in use (This is because alternate m_n register
programming will be done only when low RR is being used).
v3: Modified call to get_m2_n2 which had dp_m_n as param by mistake.
Compare dp_m_n and dp_m2_n2 for gen 7 and below. compare the structures
based on DRRS state for gen 8 and above.
Save and restore M2 N2 registers for gen 7 and below
v4: For Gen=8, check M_N registers against dp_m_n and dp_m2_n2 as there is
only one set of M_N registers
Signed-off-by: Vandana Kannan vandana.kan...@intel.com
Cc: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/i915_drv.h | 8
drivers/gpu/drm/i915/i915_ums.c | 26 +++
drivers/gpu/drm/i915/intel_ddi.c | 1 +
drivers/gpu/drm/i915/intel_display.c | 83 +---
drivers/gpu/drm/i915/intel_dp.c | 2 +
drivers/gpu/drm/i915/intel_drv.h | 2 +
6 files changed, 117 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b82f157..a06551a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -815,6 +815,14 @@ struct i915_suspend_saved_registers {
u32 savePIPEB_DATA_N1;
u32 savePIPEB_LINK_M1;
u32 savePIPEB_LINK_N1;
+ u32 savePIPEA_DATA_M2;
+ u32 savePIPEA_DATA_N2;
+ u32 savePIPEA_LINK_M2;
+ u32 savePIPEA_LINK_N2;
+ u32 savePIPEB_DATA_M2;
+ u32 savePIPEB_DATA_N2;
+ u32 savePIPEB_LINK_M2;
+ u32 savePIPEB_LINK_N2;
u32 saveMCHBAR_RENDER_STANDBY;
u32 savePCH_PORT_HOTPLUG;
};
diff --git a/drivers/gpu/drm/i915/i915_ums.c b/drivers/gpu/drm/i915/i915_ums.c
index 480da59..82fc08f 100644
--- a/drivers/gpu/drm/i915/i915_ums.c
+++ b/drivers/gpu/drm/i915/i915_ums.c
@@ -141,6 +141,21 @@ void i915_save_display_reg(struct drm_device *dev)
dev_priv-regfile.savePIPEA_LINK_M1 = I915_READ(_PIPEA_LINK_M1);
dev_priv-regfile.savePIPEA_LINK_N1 = I915_READ(_PIPEA_LINK_N1);
+ /* Saving M2_N2 registers only for Gen7 because DRRS will be
+* used only from Gen7 and for Gen8 above there is no
+* M2_N2 register.
+*/
+ if (INTEL_INFO(dev)-gen == 7) {
+ dev_priv-regfile.savePIPEA_DATA_M2 =
+ I915_READ(_PIPEA_DATA_M2);
+ dev_priv-regfile.savePIPEA_DATA_N2 =
+ I915_READ(_PIPEA_DATA_N2);
+ dev_priv-regfile.savePIPEA_LINK_M2 =
+ I915_READ(_PIPEA_LINK_M2);
+ dev_priv-regfile.savePIPEA_LINK_N2 =
+ I915_READ(_PIPEA_LINK_N2);
+ }
+
dev_priv-regfile.saveFDI_TXA_CTL = I915_READ(_FDI_TXA_CTL);
dev_priv-regfile.saveFDI_RXA_CTL = I915_READ(_FDI_RXA_CTL);
@@ -407,6 +422,17 @@ void i915_restore_display_reg(struct drm_device *dev)
I915_WRITE(_PIPEA_LINK_M1, dev_priv-regfile.savePIPEA_LINK_M1);
I915_WRITE(_PIPEA_LINK_N1, dev_priv-regfile.savePIPEA_LINK_N1);
+ if (INTEL_INFO(dev)-gen == 7) {
+ I915_WRITE(_PIPEA_DATA_M2,
+ dev_priv-regfile.savePIPEA_DATA_M2);
+ I915_WRITE(_PIPEA_DATA_N2,
+ dev_priv-regfile.savePIPEA_DATA_N2);
+ I915_WRITE(_PIPEA_LINK_M2,
+ dev_priv-regfile.savePIPEA_LINK_M2);
+ I915_WRITE(_PIPEA_LINK_N2,
+ dev_priv-regfile.savePIPEA_LINK_N2);
+ }
+
I915_WRITE(_FDI_RXA_CTL, dev_priv-regfile.saveFDI_RXA_CTL);
I915_WRITE(_FDI_TXA_CTL, dev_priv-regfile.saveFDI_TXA_CTL);
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 0ad4e96..6784f0b 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1587,6 +1587,7 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
case TRANS_DDI_MODE_SELECT_DP_MST:
pipe_config-has_dp_encoder = true;
intel_dp_get_m_n(intel_crtc, pipe_config);
+ intel_dp_get_m2_n2(intel_crtc, pipe_config);
break;
default:
break;
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index cf3ad87..09fc286 100644
---
[Intel-gfx] [PATCH 1/2] drm/i915: Set M2_N2 registers during mode set
For Gen 8, set M2_N2 registers on every mode set. This is required to make
sure M2_N2 registers are set during boot, resume from sleep for cross-
checking the state. The register is set only if DRRS is supported.
Signed-off-by: Vandana Kannan vandana.kan...@intel.com
Cc: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/i915_drv.h | 3 +++
drivers/gpu/drm/i915/intel_display.c | 30 +++---
drivers/gpu/drm/i915/intel_dp.c | 14 --
drivers/gpu/drm/i915/intel_drv.h | 1 +
4 files changed, 31 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 6b0e174..b82f157 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1956,6 +1956,9 @@ struct drm_i915_cmd_table {
#define HAS_PSR(dev) (IS_HASWELL(dev) || IS_BROADWELL(dev))
#define HAS_RUNTIME_PM(dev)(IS_GEN6(dev) || IS_HASWELL(dev) || \
IS_BROADWELL(dev) || IS_VALLEYVIEW(dev))
+#define HAS_DRRS(dev) (to_i915(dev)-drrs.connector \
+to_i915(dev)-drrs.connector- \
+panel.downclock_mode)
#define INTEL_PCH_DEVICE_ID_MASK 0xff00
#define INTEL_PCH_IBX_DEVICE_ID_TYPE 0x3b00
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 767ca96..cf3ad87 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5278,6 +5278,18 @@ static void intel_dp_set_m_n(struct intel_crtc *crtc)
intel_cpu_transcoder_set_m_n(crtc, crtc-config.dp_m_n);
}
+void intel_dp_set_m2_n2(struct intel_crtc *crtc, struct intel_link_m_n *m_n)
+{
+ struct drm_device *dev = crtc-base.dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ enum transcoder transcoder = crtc-config.cpu_transcoder;
+
+ I915_WRITE(PIPE_DATA_M2(transcoder), TU_SIZE(m_n-tu) | m_n-gmch_m);
+ I915_WRITE(PIPE_DATA_N2(transcoder), m_n-gmch_n);
+ I915_WRITE(PIPE_LINK_M2(transcoder), m_n-link_m);
+ I915_WRITE(PIPE_LINK_N2(transcoder), m_n-link_n);
+}
+
static void vlv_update_pll(struct intel_crtc *crtc)
{
struct drm_device *dev = crtc-base.dev;
@@ -5872,8 +5884,12 @@ skip_dpll:
dspcntr |= DISPPLANE_SEL_PIPE_B;
}
- if (intel_crtc-config.has_dp_encoder)
+ if (intel_crtc-config.has_dp_encoder) {
intel_dp_set_m_n(intel_crtc);
+ if (INTEL_INFO(dev)-gen 8 HAS_DRRS(dev))
+ intel_dp_set_m2_n2(intel_crtc,
+ intel_crtc-config.dp_m2_n2);
+ }
intel_set_pipe_timings(intel_crtc);
@@ -6881,8 +6897,12 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
} else
intel_put_shared_dpll(intel_crtc);
- if (intel_crtc-config.has_dp_encoder)
+ if (intel_crtc-config.has_dp_encoder) {
intel_dp_set_m_n(intel_crtc);
+ if (INTEL_INFO(dev)-gen 8 HAS_DRRS(dev))
+ intel_dp_set_m2_n2(intel_crtc,
+ intel_crtc-config.dp_m2_n2);
+ }
if (is_lvds has_reduced_clock i915.powersave)
intel_crtc-lowfreq_avail = true;
@@ -7377,8 +7397,12 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
return -EINVAL;
intel_ddi_pll_enable(intel_crtc);
- if (intel_crtc-config.has_dp_encoder)
+ if (intel_crtc-config.has_dp_encoder) {
intel_dp_set_m_n(intel_crtc);
+ if (INTEL_INFO(dev)-gen 8 HAS_DRRS(dev))
+ intel_dp_set_m2_n2(intel_crtc,
+ intel_crtc-config.dp_m2_n2);
+ }
intel_crtc-lowfreq_avail = false;
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 9f67b72..bcab4ea 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -780,20 +780,6 @@ intel_dp_set_clock(struct intel_encoder *encoder,
}
}
-static void
-intel_dp_set_m2_n2(struct intel_crtc *crtc, struct intel_link_m_n *m_n)
-{
- struct drm_device *dev = crtc-base.dev;
- struct drm_i915_private *dev_priv = dev-dev_private;
- enum transcoder transcoder = crtc-config.cpu_transcoder;
-
- I915_WRITE(PIPE_DATA_M2(transcoder),
- TU_SIZE(m_n-tu) | m_n-gmch_m);
- I915_WRITE(PIPE_DATA_N2(transcoder), m_n-gmch_n);
- I915_WRITE(PIPE_LINK_M2(transcoder), m_n-link_m);
- I915_WRITE(PIPE_LINK_N2(transcoder), m_n-link_n);
-}
-
bool
intel_dp_compute_config(struct intel_encoder *encoder,
struct intel_crtc_config *pipe_config)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index acfc5c8..5233a3d 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++
Re: [Intel-gfx] [PATCH 3/3] drm/i915: Provide DPIO diagrams as docboox tables
On 04/25 20:14, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com The ascii art version of the DPIO diagram gets mangled by docbook, so we can't use it there. Insted provide another version built using table. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com When generating drm.tmpl to html, it say no structured comments found for i915_reg.h I am getting this error when displaying the link for i915_reg.h in HTML format: Document generation inconsistency. Not sure is some issue on my side because I have issue apply this patch series, so I apply manually. Anyway, the table below is similar to your previous patch. So, Reviewed-by: Chon Ming Lee chon.ming@intel.com --- Documentation/DocBook/drm.tmpl | 86 ++ 1 file changed, 86 insertions(+) diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index e361ccd..bf11fe5 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -2945,6 +2945,92 @@ int num_ioctls;/synopsis sect2 titleDPIO/title !Pdrivers/gpu/drm/i915/i915_reg.h DPIO + table id=dpiox2 + titleDual channel PHY (VLV/CHV)/title + tgroup cols=8 + colspec colname=c0 / + colspec colname=c1 / + colspec colname=c2 / + colspec colname=c3 / + colspec colname=c4 / + colspec colname=c5 / + colspec colname=c6 / + colspec colname=c7 / + spanspec spanname=ch0 namest=c0 nameend=c3 / + spanspec spanname=ch1 namest=c4 nameend=c7 / + spanspec spanname=ch0pcs01 namest=c0 nameend=c1 / + spanspec spanname=ch0pcs23 namest=c2 nameend=c3 / + spanspec spanname=ch1pcs01 namest=c4 nameend=c5 / + spanspec spanname=ch1pcs23 namest=c6 nameend=c7 / + thead + row + entry spanname=ch0CH0/entry + entry spanname=ch1CH1/entry + /row + /thead + tbody valign=top align=center + row + entry spanname=ch0CMN/PLL/REF/entry + entry spanname=ch1CMN/PLL/REF/entry + /row + row + entry spanname=ch0pcs01PCS01/entry + entry spanname=ch0pcs23PCS23/entry + entry spanname=ch1pcs01PCS01/entry + entry spanname=ch1pcs23PCS23/entry + /row + row + entryTX0/entry + entryTX1/entry + entryTX2/entry + entryTX3/entry + entryTX0/entry + entryTX1/entry + entryTX2/entry + entryTX3/entry + /row + row + entry spanname=ch0DDI0/entry + entry spanname=ch1DDI1/entry + /row + /tbody + /tgroup + /table + table id=dpiox1 + titleSingle channel PHY (CHV)/title + tgroup cols=4 + colspec colname=c0 / + colspec colname=c1 / + colspec colname=c2 / + colspec colname=c3 / + spanspec spanname=ch0 namest=c0 nameend=c3 / + spanspec spanname=ch0pcs01 namest=c0 nameend=c1 / + spanspec spanname=ch0pcs23 namest=c2 nameend=c3 / + thead + row + entry spanname=ch0CH0/entry + /row + /thead + tbody valign=top align=center + row + entry spanname=ch0CMN/PLL/REF/entry + /row + row + entry spanname=ch0pcs01PCS01/entry + entry spanname=ch0pcs23PCS23/entry + /row + row + entryTX0/entry + entryTX1/entry + entryTX2/entry + entryTX3/entry + /row + row + entry spanname=ch0DDI2/entry + /row + /tbody + /tgroup + /table /sect2 /sect1 -- 1.8.3.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 4/5] drm/i915: Wait for pending page flips before enabling/disabling the primary plane
From: Ville Syrjälä ville.syrj...@linux.intel.com
We have to write to the primary plane base address registrer when we
enable/disable the primary plane in response to sprite coverage. Those
writes will cause the flip counter to increment which could interfere
with the detection of CS flip completion. We could end up completing
CS flips before the CS has even executed the commands from the ring.
To avoid such issues, wait for CS flips to finish before we toggle the
primary plane on/off.
v2: Rebased due to atomic sprite update changes
Testcase: igt/kms_mmio_vs_cs_flip/setplane_vs_cs_flip
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/intel_display.c | 2 +-
drivers/gpu/drm/i915/intel_drv.h | 2 ++
drivers/gpu/drm/i915/intel_sprite.c | 3 +++
3 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 8ca8b2f..849bc71 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3291,7 +3291,7 @@ bool intel_has_pending_fb_unpin(struct drm_device *dev)
return false;
}
-static void intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc)
+void intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc)
{
struct drm_device *dev = crtc-dev;
struct drm_i915_private *dev_priv = dev-dev_private;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a78eb75..b9b13eb 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -801,6 +801,8 @@ int valleyview_get_vco(struct drm_i915_private *dev_priv);
void intel_mode_from_pipe_config(struct drm_display_mode *mode,
struct intel_crtc_config *pipe_config);
int intel_format_to_fourcc(int format);
+void intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc);
+
/* intel_dp.c */
void intel_dp_init(struct drm_device *dev, int output_reg, enum port port);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c
b/drivers/gpu/drm/i915/intel_sprite.c
index 7780f6c..d6acd6b 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1018,6 +1018,9 @@ intel_update_plane(struct drm_plane *plane, struct
drm_crtc *crtc,
intel_crtc-primary_enabled = primary_enabled;
+ if (primary_was_enabled != primary_enabled)
+ intel_crtc_wait_for_pending_flips(crtc);
+
if (primary_was_enabled !primary_enabled)
intel_pre_disable_primary(crtc);
--
1.8.5.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 01/12] drm: Use correct spinlock flavor in drm_vblank_get()
On Wed, May 14, 2014 at 08:51:03PM +0200, Daniel Vetter wrote: From: Peter Hurley pe...@hurleysoftware.com The irq flags state is already established by the outer spin_lock_irqsave(); re-disabling irqs is redundant. Signed-off-by: Peter Hurley pe...@hurleysoftware.com Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/drm_irq.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) Reviewed-by: Thierry Reding tred...@nvidia.com pgpk47T2UOhHK.pgp Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 02/12] drm: Make the vblank disable timer per-crtc
On Wed, May 14, 2014 at 08:51:04PM +0200, Daniel Vetter wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com Currently there's one per-device vblank disable timer, and it gets reset wheneven the vblank refcount for any crtc drops to zero. That whenever means that one crtc could accidentally be keeping the vblank interrupts for other crtcs enabled even if there are no users for them. Make the disable timer per-crtc to avoid this issue. Very pedantically: s/crtc/CRTC/ and maybe even s/vblank/VBLANK/. Feel free to ignore those, though. =) Also, and I may have asked before, why do we even need this timer? Why not simply disable interrupts when the last vblank reference goes away? Generally, though: Reviewed-by: Thierry Reding tred...@nvidia.com pgp_M10LG3SwK.pgp Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 03/12] drm: Make blocking vblank wait return when the vblank interrupts get disabled
On Wed, May 14, 2014 at 08:51:05PM +0200, Daniel Vetter wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com If there's a blocking vblank wait in progress while the vblank interrupt gets disabled, the current code will just let the vblank wait time out. Instead make it return immediately when vblank interrupts get disabled. Can this even happen? drm_wait_vblank() takes a vblank reference earlier and drops it right before returning. But perhaps this will become obvious since from a quick peek some of the subsequent patches seem like they will make it possible to force VBLANK off? Thierry pgp2L7BMeObA3.pgp Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 03/12] drm: Make blocking vblank wait return when the vblank interrupts get disabled
On Wed, May 21, 2014 at 01:20:55PM +0200, Thierry Reding wrote: On Wed, May 14, 2014 at 08:51:05PM +0200, Daniel Vetter wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com If there's a blocking vblank wait in progress while the vblank interrupt gets disabled, the current code will just let the vblank wait time out. Instead make it return immediately when vblank interrupts get disabled. Can this even happen? drm_wait_vblank() takes a vblank reference earlier and drops it right before returning. But perhaps this will become obvious since from a quick peek some of the subsequent patches seem like they will make it possible to force VBLANK off? Ah, it seems like drm_vblank_off() can already do exactly that, in which case this makes sense: Reviewed-by: Thierry Reding tred...@nvidia.com pgpbrddTyAMfX.pgp Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 05/12] drm/i915: Remove drm_vblank_pre/post_modeset calls
On Wed, May 14, 2014 at 08:51:07PM +0200, Daniel Vetter wrote: Originally these functions have been for user modesetting drivers to ensure vblank processing doesn't fall over completely around modeset changes. This has been carried over ever since then. Now that Ville cleaned our vblank handling with an explicit drm_vblank_off/on braket when disabling/enabling crtcs. So this seems s/braket/bracket/ to be unnecessary now. Should we document that drivers should start converting to this new set of functions? Maybe deprecate the drm_vblank_pre/post_modeset()? The most important side effect was that due to the delayed vblank disabling we have been pretty much guaranteed to receive a vblank interrupt soonish after a crtc was enabled. I don't understand what this sentence means and whether it relates to code prior to or after this patch. Note that our vblank handling across modeset is still fairly decent fubar - we don't actually handle vblank counter all to well. drm_update_vblank_count will make sure that the frame counter always rolls forward, but userspace isn't really all to ready to cope with the big jumps this causes. This isn't a big mostly because the hardware retains the frame Not a big what? counter. But with runtime pm and also across suspend/resume we fall over. Fixing this is a lot more involved and also needs som i-g-ts. So material for another patch series. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_display.c | 5 - 1 file changed, 5 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 858c393b051f..d0eff53a8ad1 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -7207,15 +7207,10 @@ static int intel_crtc_mode_set(struct drm_crtc *crtc, struct intel_encoder *encoder; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); struct drm_display_mode *mode = intel_crtc-config.requested_mode; - int pipe = intel_crtc-pipe; int ret; - drm_vblank_pre_modeset(dev, pipe); - ret = dev_priv-display.crtc_mode_set(crtc, x, y, fb); - drm_vblank_post_modeset(dev, pipe); - if (ret != 0) Nit: There's now a blank line between ret = ... and if (...). Thierry pgpowDAKeoTqC.pgp Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Fix dynamic allocation of physical handles
A single object may be referenced by multiple registers fundamentally
breaking the static allotment of ids in the current design. When the
object is used the second time, the physical address of the first
assignment is relinquished and a second one granted. However, the
hardware is still reading (and possibly writing) to the old physical
address now returned to the system. Eventually hilarity will ensue, but
in the short term, it just means that cursors are broken when using more
than one pipe.
v2: Fix up leak of pci handle when handling an error during attachment,
and avoid a double kmap/kunmap. (Ville)
Rebase against -fixes.
v3: And fix the error handling added in v2 (Ville)
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=77351
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
Cc: Jani Nikula jani.nik...@linux.intel.com
Cc: sta...@vger.kernel.org
Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/i915_dma.c | 1 -
drivers/gpu/drm/i915/i915_drv.h | 24 +--
drivers/gpu/drm/i915/i915_gem.c | 319 ++-
drivers/gpu/drm/i915/intel_display.c | 11 +-
drivers/gpu/drm/i915/intel_overlay.c | 12 +-
5 files changed, 136 insertions(+), 231 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 96177eec0a0e..eedb023af27d 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1833,7 +1833,6 @@ int i915_driver_unload(struct drm_device *dev)
flush_workqueue(dev_priv-wq);
mutex_lock(dev-struct_mutex);
- i915_gem_free_all_phys_object(dev);
i915_gem_cleanup_ringbuffer(dev);
i915_gem_context_fini(dev);
WARN_ON(dev_priv-mm.aliasing_ppgtt);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 108e1ec2fa4b..ec5f6fb42ab3 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -242,18 +242,6 @@ struct intel_ddi_plls {
#define WATCH_LISTS0
#define WATCH_GTT 0
-#define I915_GEM_PHYS_CURSOR_0 1
-#define I915_GEM_PHYS_CURSOR_1 2
-#define I915_GEM_PHYS_OVERLAY_REGS 3
-#define I915_MAX_PHYS_OBJECT (I915_GEM_PHYS_OVERLAY_REGS)
-
-struct drm_i915_gem_phys_object {
- int id;
- struct page **page_list;
- drm_dma_handle_t *handle;
- struct drm_i915_gem_object *cur_obj;
-};
-
struct opregion_header;
struct opregion_acpi;
struct opregion_swsci;
@@ -1187,9 +1175,6 @@ struct i915_gem_mm {
/** Bit 6 swizzling required for Y tiling */
uint32_t bit_6_swizzle_y;
- /* storage for physical objects */
- struct drm_i915_gem_phys_object *phys_objs[I915_MAX_PHYS_OBJECT];
-
/* accounting, useful for userland debugging */
spinlock_t object_stat_lock;
size_t object_memory;
@@ -1769,7 +1754,7 @@ struct drm_i915_gem_object {
struct drm_file *pin_filp;
/** for phy allocated objects */
- struct drm_i915_gem_phys_object *phys_obj;
+ drm_dma_handle_t *phys_handle;
};
#define to_intel_bo(x) container_of(x, struct drm_i915_gem_object, base)
@@ -2334,13 +2319,8 @@ i915_gem_object_pin_to_display_plane(struct
drm_i915_gem_object *obj,
u32 alignment,
struct intel_ring_buffer *pipelined);
void i915_gem_object_unpin_from_display_plane(struct drm_i915_gem_object *obj);
-int i915_gem_attach_phys_object(struct drm_device *dev,
- struct drm_i915_gem_object *obj,
- int id,
+int i915_gem_object_attach_phys(struct drm_i915_gem_object *obj,
int align);
-void i915_gem_detach_phys_object(struct drm_device *dev,
-struct drm_i915_gem_object *obj);
-void i915_gem_free_all_phys_object(struct drm_device *dev);
int i915_gem_open(struct drm_device *dev, struct drm_file *file);
void i915_gem_release(struct drm_device *dev, struct drm_file *file);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 2871ce75f438..b391f30f9985 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -43,10 +43,6 @@ static void i915_gem_object_flush_cpu_write_domain(struct
drm_i915_gem_object *o
static __must_check int
i915_gem_object_wait_rendering(struct drm_i915_gem_object *obj,
bool readonly);
-static int i915_gem_phys_pwrite(struct drm_device *dev,
- struct drm_i915_gem_object *obj,
- struct drm_i915_gem_pwrite *args,
- struct drm_file *file);
static void i915_gem_write_fence(struct drm_device *dev, int reg,
struct drm_i915_gem_object *obj);
@@ -209,6 +205,128 @@
[Intel-gfx] [PATCH] drm/i915: Add null state batch to active list
for proper refcounting to take place as we use
i915_add_request() for it.
i915_add_request() also takes the context for the request
from ring-last_context so move the null state batch
submission after the ring context has been set.
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
Cc: Chris Wilson ch...@chris-wilson.co.uk
Cc: Damien Lespiau damien.lesp...@intel.com
Signed-off-by: Mika Kuoppala mika.kuopp...@intel.com
---
drivers/gpu/drm/i915/i915_drv.h |3 +++
drivers/gpu/drm/i915/i915_gem.c |4 ++--
drivers/gpu/drm/i915/i915_gem_context.c | 16
drivers/gpu/drm/i915/i915_gem_render_state.c |7 +--
4 files changed, 18 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b90ec69..6881f70 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2204,6 +2204,9 @@ static inline void i915_gem_object_unpin_pages(struct
drm_i915_gem_object *obj)
int __must_check i915_mutex_lock_interruptible(struct drm_device *dev);
int i915_gem_object_sync(struct drm_i915_gem_object *obj,
struct intel_ring_buffer *to);
+void i915_gem_object_move_to_active(struct drm_i915_gem_object *obj,
+ struct intel_ring_buffer *ring);
+void i915_gem_object_move_to_inactive(struct drm_i915_gem_object *obj);
void i915_vma_move_to_active(struct i915_vma *vma,
struct intel_ring_buffer *ring);
int i915_gem_dumb_create(struct drm_file *file_priv,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 440979f..d795800 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2038,7 +2038,7 @@ i915_gem_object_get_pages(struct drm_i915_gem_object *obj)
return 0;
}
-static void
+void
i915_gem_object_move_to_active(struct drm_i915_gem_object *obj,
struct intel_ring_buffer *ring)
{
@@ -2084,7 +2084,7 @@ void i915_vma_move_to_active(struct i915_vma *vma,
return i915_gem_object_move_to_active(vma-obj, ring);
}
-static void
+void
i915_gem_object_move_to_inactive(struct drm_i915_gem_object *obj)
{
struct drm_i915_private *dev_priv = obj-base.dev-dev_private;
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index f220c94..d3aad5b 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -700,21 +700,21 @@ static int do_switch(struct intel_ring_buffer *ring,
/* obj is kept alive until the next request by its active ref */
i915_gem_object_ggtt_unpin(from-obj);
i915_gem_context_unreference(from);
- } else {
- if (to-is_initialized == false) {
- ret = i915_gem_render_state_init(ring);
- if (ret)
- DRM_ERROR(init render state: %d\n, ret);
- }
}
- to-is_initialized = true;
-
done:
i915_gem_context_reference(to);
ring-last_context = to;
to-last_ring = ring;
+ if (to-is_initialized == false from == NULL) {
+ ret = i915_gem_render_state_init(ring);
+ if (ret)
+ DRM_ERROR(init render state: %d\n, ret);
+ }
+
+ to-is_initialized = true;
+
return 0;
unpin_out:
diff --git a/drivers/gpu/drm/i915/i915_gem_render_state.c
b/drivers/gpu/drm/i915/i915_gem_render_state.c
index 392aa7b..d29e6b2 100644
--- a/drivers/gpu/drm/i915/i915_gem_render_state.c
+++ b/drivers/gpu/drm/i915/i915_gem_render_state.c
@@ -164,7 +164,6 @@ int i915_gem_render_state_init(struct intel_ring_buffer
*ring)
const int gen = INTEL_INFO(ring-dev)-gen;
struct i915_render_state *so;
const struct intel_renderstate_rodata *rodata;
- u32 seqno;
int ret;
rodata = render_state_get_rodata(ring-dev, gen);
@@ -186,7 +185,11 @@ int i915_gem_render_state_init(struct intel_ring_buffer
*ring)
if (ret)
goto out;
- ret = i915_add_request(ring, seqno);
+ i915_gem_object_move_to_active(so-obj, ring);
+
+ ret = __i915_add_request(ring, NULL, so-obj, NULL);
+ if (ret)
+ i915_gem_object_move_to_inactive(so-obj);
out:
render_state_free(so);
--
1.7.9.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 02/12] drm: Make the vblank disable timer per-crtc
On Wed, May 21, 2014 at 01:17:49PM +0200, Thierry Reding wrote: On Wed, May 14, 2014 at 08:51:04PM +0200, Daniel Vetter wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com Currently there's one per-device vblank disable timer, and it gets reset wheneven the vblank refcount for any crtc drops to zero. That whenever means that one crtc could accidentally be keeping the vblank interrupts for other crtcs enabled even if there are no users for them. Make the disable timer per-crtc to avoid this issue. Very pedantically: s/crtc/CRTC/ and maybe even s/vblank/VBLANK/. Feel free to ignore those, though. =) Also, and I may have asked before, why do we even need this timer? Why not simply disable interrupts when the last vblank reference goes away? Without intricate knowledge of where exactly the vblank interrupt fires wrt the hw frame counter the enabling/disabling of the vblank machinery as implemented in drm_irq.c is racy. Which means we shouldn't do it all the time. In i915 we are now solid enough with vblank handling in general and also well-covered in tests that we'll attempt to kill the disabling timer as the next step. Since keeping vblanks going when we don't need them if you have a hw vblank counter seriously hampers deep sleep states residency. But given how bug-riddled our vblank code was I want to move slowly. And we need to keep the hack for all those drivers which haven't properly been audited and tested (i.e. everyone else). -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 04/12] drm: Add drm_vblank_on()
On Wed, May 21, 2014 at 01:32:35PM +0200, Thierry Reding wrote:
On Wed, May 14, 2014 at 08:51:06PM +0200, Daniel Vetter wrote:
From: Ville Syrjälä ville.syrj...@linux.intel.com
drm_vblank_off() will turn off vblank interrupts, but as long as the
refcount is elevated drm_vblank_get() will not re-enable them. This
is a problem is someone is holding a vblank reference while a modeset is
happening, and the driver requires vblank interrupt to work during that
time.
Add drm_vblank_on() as a counterpart to drm_vblank_off() which will
re-enabled vblank interrupts if the refcount is already elevated. This
will allow drivers to choose the specific places in the modeset sequence
at which vblank interrupts get disabled and enabled.
Testcase: igt/kms_flip/*-vs-suspend
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
[danvet: Add Testcase tag for the igt I've written.]
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/drm_irq.c| 72
++--
drivers/gpu/drm/i915/intel_display.c | 8
include/drm/drmP.h | 1 +
3 files changed, 62 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 13d671ed3421..dd786d84daab 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -840,6 +840,41 @@ static void drm_update_vblank_count(struct drm_device
*dev, int crtc)
}
/**
+ * drm_vblank_enable - enable the vblank interrupt on a CRTC
+ * @dev: DRM device
+ * @crtc: CRTC in question
+ */
Perhaps the kernel-doc here should contain some of what's described in
the commit message? Also a Return: section would be useful here to
specify what's an error and what isn't.
+static int drm_vblank_enable(struct drm_device *dev, int crtc)
On second thought, since this is a local function, my comments above
apply to drm_vblank_on() below rather than drm_vblank_enable().
Yeah, we don't pull this into the kerneldoc, only functions exported to
drivers. Follow-up patch should (hopefully) sufficiently pimp the
kerneldoc for drm_vblank_on. If not please complain.
+{
+ int ret = 0;
+
+ assert_spin_locked(dev-vbl_lock);
+
+ spin_lock(dev-vblank_time_lock);
+
+ if (!dev-vblank[crtc].enabled) {
+ /* Enable vblank irqs under vblank_time_lock protection.
+* All vblank count timestamp updates are held off
+* until we are done reinitializing master counter and
+* timestamps. Filtercode in drm_handle_vblank() will
+* prevent double-accounting of same vblank interval.
+*/
Coding style:
/*
* Enable...
*/
?
Yeah, will polish.
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
Perhaps split off the i915 changes into a separate patch?
Yeah, should have. But I've got fed up with the conflicts this topic
branch caused so pulled it in. I'll apply your review feedback as
follow-up patches.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH igt v2] tests/kms_mmio_vs_cs_flip: Add a test case to exercise mmio vs. CS flip races
From: Ville Syrjälä ville.syrj...@linux.intel.com
kms_mmio_vs_cs_flip has two subtests:
- setplane_vs_cs_flip tests the interaction between
fullscreen sprites and CS flips
- setcrtc_vs_cs_flip tests the interaction between
primary plane panning and CS flips
v2: Skip sprite test when there are no sprites
Reduce busy_bo to 64MB (now works on my gen2)
Handle pipe vs. port incompatibility
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
tests/Makefile.sources | 1 +
tests/kms_mmio_vs_cs_flip.c | 537
2 files changed, 538 insertions(+)
create mode 100644 tests/kms_mmio_vs_cs_flip.c
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 383a209..1f7931d 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -64,6 +64,7 @@ TESTS_progs_M = \
kms_fence_pin_leak \
kms_flip \
kms_flip_tiling \
+ kms_mmio_vs_cs_flip \
kms_pipe_crc_basic \
kms_plane \
kms_render \
diff --git a/tests/kms_mmio_vs_cs_flip.c b/tests/kms_mmio_vs_cs_flip.c
new file mode 100644
index 000..a724721
--- /dev/null
+++ b/tests/kms_mmio_vs_cs_flip.c
@@ -0,0 +1,537 @@
+/*
+ * Copyright © 2014 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the Software),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#include errno.h
+#include stdbool.h
+#include stdio.h
+#include string.h
+#include time.h
+
+#include drmtest.h
+#include igt_debugfs.h
+#include igt_kms.h
+#include intel_chipset.h
+#include ioctl_wrappers.h
+
+typedef struct {
+ int drm_fd;
+ igt_display_t display;
+ igt_pipe_crc_t *pipe_crc;
+ drm_intel_bufmgr *bufmgr;
+ drm_intel_bo *busy_bo;
+ uint32_t devid;
+ bool flip_done;
+} data_t;
+
+static void exec_nop(data_t *data, uint32_t handle, unsigned int ring)
+{
+ struct intel_batchbuffer *batch;
+ drm_intel_bo *bo;
+
+ batch = intel_batchbuffer_alloc(data-bufmgr, data-devid);
+ igt_assert(batch);
+
+ bo = gem_handle_to_libdrm_bo(data-bufmgr, data-drm_fd, , handle);
+ igt_assert(bo);
+
+ /* add relocs to make sure the kernel will think we write to dst */
+ BEGIN_BATCH(4);
+ OUT_BATCH(MI_BATCH_BUFFER_END);
+ OUT_BATCH(MI_NOOP);
+ OUT_RELOC(bo, I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER, 0);
+ OUT_BATCH(MI_NOOP);
+ ADVANCE_BATCH();
+
+ intel_batchbuffer_flush_on_ring(batch, ring);
+ intel_batchbuffer_free(batch);
+
+ drm_intel_bo_unreference(bo);
+}
+
+static void exec_blt(data_t *data)
+{
+ struct intel_batchbuffer *batch;
+ int w, h, pitch, i;
+
+ batch = intel_batchbuffer_alloc(data-bufmgr, data-devid);
+ igt_assert(batch);
+
+ w = 8192;
+ h = data-busy_bo-size / (8192 * 4);
+ pitch = w * 4;
+
+ for (i = 0; i 40; i++) {
+ BLIT_COPY_BATCH_START(data-devid, 0);
+ OUT_BATCH((3 24) | /* 32 bits */
+ (0xcc 16) | /* copy ROP */
+ pitch);
+ OUT_BATCH(0 16 | 0);
+ OUT_BATCH(h 16 | w);
+ OUT_RELOC(data-busy_bo, I915_GEM_DOMAIN_RENDER,
I915_GEM_DOMAIN_RENDER, 0);
+ BLIT_RELOC_UDW(data-devid);
+ OUT_BATCH(0 16 | 0);
+ OUT_BATCH(pitch);
+ OUT_RELOC(data-busy_bo, I915_GEM_DOMAIN_RENDER, 0, 0);
+ BLIT_RELOC_UDW(data-devid);
+ ADVANCE_BATCH();
+ }
+
+ intel_batchbuffer_flush(batch);
+ intel_batchbuffer_free(batch);
+}
+
+static void page_flip_handler(int fd, unsigned int frame, unsigned int sec,
+ unsigned int usec, void *_data)
+{
+ data_t *data = _data;
+
+ data-flip_done = true;
+}
+
+static void wait_for_flip(data_t *data, uint32_t flip_handle)
+{
+ struct timeval timeout = {
+ .tv_sec = 3,
+
Re: [Intel-gfx] [PATCH 3/3] drm/i915: Provide DPIO diagrams as docboox tables
On Wed, May 21, 2014 at 06:59:03PM +0800, Lee, Chon Ming wrote: On 04/25 20:14, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com The ascii art version of the DPIO diagram gets mangled by docbook, so we can't use it there. Insted provide another version built using table. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com When generating drm.tmpl to html, it say no structured comments found for i915_reg.h I am getting this error when displaying the link for i915_reg.h in HTML format: Document generation inconsistency. Not sure is some issue on my side because I have issue apply this patch series, so I apply manually. Anyway, the table below is similar to your previous patch. So, Worked here, dunno what's broken on your side ... Note that some docbooks moved around in 3.15-rc1 and the build system doesn't handle that. You need to run a git clean -dfx in the Documentation directory to fix that. Reviewed-by: Chon Ming Lee chon.ming@intel.com Both patches merged, thanks. -Daniel --- Documentation/DocBook/drm.tmpl | 86 ++ 1 file changed, 86 insertions(+) diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index e361ccd..bf11fe5 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -2945,6 +2945,92 @@ int num_ioctls;/synopsis sect2 titleDPIO/title !Pdrivers/gpu/drm/i915/i915_reg.h DPIO + table id=dpiox2 + titleDual channel PHY (VLV/CHV)/title + tgroup cols=8 + colspec colname=c0 / + colspec colname=c1 / + colspec colname=c2 / + colspec colname=c3 / + colspec colname=c4 / + colspec colname=c5 / + colspec colname=c6 / + colspec colname=c7 / + spanspec spanname=ch0 namest=c0 nameend=c3 / + spanspec spanname=ch1 namest=c4 nameend=c7 / + spanspec spanname=ch0pcs01 namest=c0 nameend=c1 / + spanspec spanname=ch0pcs23 namest=c2 nameend=c3 / + spanspec spanname=ch1pcs01 namest=c4 nameend=c5 / + spanspec spanname=ch1pcs23 namest=c6 nameend=c7 / + thead + row + entry spanname=ch0CH0/entry + entry spanname=ch1CH1/entry + /row + /thead + tbody valign=top align=center + row + entry spanname=ch0CMN/PLL/REF/entry + entry spanname=ch1CMN/PLL/REF/entry + /row + row + entry spanname=ch0pcs01PCS01/entry + entry spanname=ch0pcs23PCS23/entry + entry spanname=ch1pcs01PCS01/entry + entry spanname=ch1pcs23PCS23/entry + /row + row + entryTX0/entry + entryTX1/entry + entryTX2/entry + entryTX3/entry + entryTX0/entry + entryTX1/entry + entryTX2/entry + entryTX3/entry + /row + row + entry spanname=ch0DDI0/entry + entry spanname=ch1DDI1/entry + /row + /tbody + /tgroup + /table + table id=dpiox1 + titleSingle channel PHY (CHV)/title + tgroup cols=4 + colspec colname=c0 / + colspec colname=c1 / + colspec colname=c2 / + colspec colname=c3 / + spanspec spanname=ch0 namest=c0 nameend=c3 / + spanspec spanname=ch0pcs01 namest=c0 nameend=c1 / + spanspec spanname=ch0pcs23 namest=c2 nameend=c3 / + thead + row + entry spanname=ch0CH0/entry + /row + /thead + tbody valign=top align=center + row + entry spanname=ch0CMN/PLL/REF/entry + /row + row + entry spanname=ch0pcs01PCS01/entry + entry spanname=ch0pcs23PCS23/entry + /row + row + entryTX0/entry + entryTX1/entry + entryTX2/entry + entryTX3/entry + /row + row + entry spanname=ch0DDI2/entry + /row + /tbody + /tgroup + /table /sect2 /sect1 -- 1.8.3.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Add null state batch to active list
On Wed, May 21, 2014 at 03:08:51PM +0300, Mika Kuoppala wrote:
for proper refcounting to take place as we use
i915_add_request() for it.
i915_add_request() also takes the context for the request
from ring-last_context so move the null state batch
submission after the ring context has been set.
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
Cc: Chris Wilson ch...@chris-wilson.co.uk
Cc: Damien Lespiau damien.lesp...@intel.com
Signed-off-by: Mika Kuoppala mika.kuopp...@intel.com
---
drivers/gpu/drm/i915/i915_drv.h |3 +++
drivers/gpu/drm/i915/i915_gem.c |4 ++--
drivers/gpu/drm/i915/i915_gem_context.c | 16
drivers/gpu/drm/i915/i915_gem_render_state.c |7 +--
4 files changed, 18 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b90ec69..6881f70 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2204,6 +2204,9 @@ static inline void i915_gem_object_unpin_pages(struct
drm_i915_gem_object *obj)
int __must_check i915_mutex_lock_interruptible(struct drm_device *dev);
int i915_gem_object_sync(struct drm_i915_gem_object *obj,
struct intel_ring_buffer *to);
+void i915_gem_object_move_to_active(struct drm_i915_gem_object *obj,
+ struct intel_ring_buffer *ring);
+void i915_gem_object_move_to_inactive(struct drm_i915_gem_object *obj);
void i915_vma_move_to_active(struct i915_vma *vma,
struct intel_ring_buffer *ring);
int i915_gem_dumb_create(struct drm_file *file_priv,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 440979f..d795800 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2038,7 +2038,7 @@ i915_gem_object_get_pages(struct drm_i915_gem_object
*obj)
return 0;
}
-static void
+void
i915_gem_object_move_to_active(struct drm_i915_gem_object *obj,
struct intel_ring_buffer *ring)
{
@@ -2084,7 +2084,7 @@ void i915_vma_move_to_active(struct i915_vma *vma,
return i915_gem_object_move_to_active(vma-obj, ring);
}
-static void
+void
i915_gem_object_move_to_inactive(struct drm_i915_gem_object *obj)
{
struct drm_i915_private *dev_priv = obj-base.dev-dev_private;
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index f220c94..d3aad5b 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -700,21 +700,21 @@ static int do_switch(struct intel_ring_buffer *ring,
/* obj is kept alive until the next request by its active ref */
i915_gem_object_ggtt_unpin(from-obj);
i915_gem_context_unreference(from);
- } else {
- if (to-is_initialized == false) {
- ret = i915_gem_render_state_init(ring);
- if (ret)
- DRM_ERROR(init render state: %d\n, ret);
- }
}
- to-is_initialized = true;
-
done:
i915_gem_context_reference(to);
ring-last_context = to;
to-last_ring = ring;
+ if (to-is_initialized == false from == NULL) {
+ ret = i915_gem_render_state_init(ring);
+ if (ret)
+ DRM_ERROR(init render state: %d\n, ret);
+ }
I think this will end badly on !RCS. So needs a ring check, and maybe
throw a WARN into i915_gem_render_state_init() if it gets called with
the wrong ring.
+
+ to-is_initialized = true;
+
return 0;
unpin_out:
diff --git a/drivers/gpu/drm/i915/i915_gem_render_state.c
b/drivers/gpu/drm/i915/i915_gem_render_state.c
index 392aa7b..d29e6b2 100644
--- a/drivers/gpu/drm/i915/i915_gem_render_state.c
+++ b/drivers/gpu/drm/i915/i915_gem_render_state.c
@@ -164,7 +164,6 @@ int i915_gem_render_state_init(struct intel_ring_buffer
*ring)
const int gen = INTEL_INFO(ring-dev)-gen;
struct i915_render_state *so;
const struct intel_renderstate_rodata *rodata;
- u32 seqno;
int ret;
rodata = render_state_get_rodata(ring-dev, gen);
@@ -186,7 +185,11 @@ int i915_gem_render_state_init(struct intel_ring_buffer
*ring)
if (ret)
goto out;
- ret = i915_add_request(ring, seqno);
+ i915_gem_object_move_to_active(so-obj, ring);
+
+ ret = __i915_add_request(ring, NULL, so-obj, NULL);
+ if (ret)
+ i915_gem_object_move_to_inactive(so-obj);
out:
render_state_free(so);
--
1.7.9.5
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: State readout and cross-checking for dp_m2_n2
On Wed, May 21, 2014 at 04:40:04PM +0530, Vandana Kannan wrote:
Adding relevant read out comparison code, in check_crtc_state, for the new
member of crtc_config, dp_m2_n2, which was introduced to store link_m_n
values for a DP downclock mode (if available). Suggested by Daniel.
v2: Changed patch title.
Daniel's review comments incorporated.
Added relevant state readout code for M2_N2. dp_m2_n2 comparison to be done
only when high RR is not in use (This is because alternate m_n register
programming will be done only when low RR is being used).
v3: Modified call to get_m2_n2 which had dp_m_n as param by mistake.
Compare dp_m_n and dp_m2_n2 for gen 7 and below. compare the structures
based on DRRS state for gen 8 and above.
Save and restore M2 N2 registers for gen 7 and below
v4: For Gen=8, check M_N registers against dp_m_n and dp_m2_n2 as there is
only one set of M_N registers
Signed-off-by: Vandana Kannan vandana.kan...@intel.com
Cc: Daniel Vetter daniel.vet...@ffwll.ch
Some comments below.
-Daniel
---
drivers/gpu/drm/i915/i915_drv.h | 8
drivers/gpu/drm/i915/i915_ums.c | 26 +++
drivers/gpu/drm/i915/intel_ddi.c | 1 +
drivers/gpu/drm/i915/intel_display.c | 83
+---
drivers/gpu/drm/i915/intel_dp.c | 2 +
drivers/gpu/drm/i915/intel_drv.h | 2 +
6 files changed, 117 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b82f157..a06551a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -815,6 +815,14 @@ struct i915_suspend_saved_registers {
u32 savePIPEB_DATA_N1;
u32 savePIPEB_LINK_M1;
u32 savePIPEB_LINK_N1;
+ u32 savePIPEA_DATA_M2;
+ u32 savePIPEA_DATA_N2;
+ u32 savePIPEA_LINK_M2;
+ u32 savePIPEA_LINK_N2;
+ u32 savePIPEB_DATA_M2;
+ u32 savePIPEB_DATA_N2;
+ u32 savePIPEB_LINK_M2;
+ u32 savePIPEB_LINK_N2;
u32 saveMCHBAR_RENDER_STANDBY;
u32 savePCH_PORT_HOTPLUG;
};
diff --git a/drivers/gpu/drm/i915/i915_ums.c b/drivers/gpu/drm/i915/i915_ums.c
index 480da59..82fc08f 100644
--- a/drivers/gpu/drm/i915/i915_ums.c
+++ b/drivers/gpu/drm/i915/i915_ums.c
@@ -141,6 +141,21 @@ void i915_save_display_reg(struct drm_device *dev)
dev_priv-regfile.savePIPEA_LINK_M1 = I915_READ(_PIPEA_LINK_M1);
dev_priv-regfile.savePIPEA_LINK_N1 = I915_READ(_PIPEA_LINK_N1);
+ /* Saving M2_N2 registers only for Gen7 because DRRS will be
+ * used only from Gen7 and for Gen8 above there is no
+ * M2_N2 register.
+ */
+ if (INTEL_INFO(dev)-gen == 7) {
+ dev_priv-regfile.savePIPEA_DATA_M2 =
+ I915_READ(_PIPEA_DATA_M2);
+ dev_priv-regfile.savePIPEA_DATA_N2 =
+ I915_READ(_PIPEA_DATA_N2);
+ dev_priv-regfile.savePIPEA_LINK_M2 =
+ I915_READ(_PIPEA_LINK_M2);
+ dev_priv-regfile.savePIPEA_LINK_N2 =
+ I915_READ(_PIPEA_LINK_N2);
+ }
+
dev_priv-regfile.saveFDI_TXA_CTL = I915_READ(_FDI_TXA_CTL);
dev_priv-regfile.saveFDI_RXA_CTL = I915_READ(_FDI_RXA_CTL);
@@ -407,6 +422,17 @@ void i915_restore_display_reg(struct drm_device *dev)
I915_WRITE(_PIPEA_LINK_M1, dev_priv-regfile.savePIPEA_LINK_M1);
I915_WRITE(_PIPEA_LINK_N1, dev_priv-regfile.savePIPEA_LINK_N1);
+ if (INTEL_INFO(dev)-gen == 7) {
+ I915_WRITE(_PIPEA_DATA_M2,
+ dev_priv-regfile.savePIPEA_DATA_M2);
+ I915_WRITE(_PIPEA_DATA_N2,
+ dev_priv-regfile.savePIPEA_DATA_N2);
+ I915_WRITE(_PIPEA_LINK_M2,
+ dev_priv-regfile.savePIPEA_LINK_M2);
+ I915_WRITE(_PIPEA_LINK_N2,
+ dev_priv-regfile.savePIPEA_LINK_N2);
+ }
These three hunks shouldn't be required since we restore the full mode in
the crtc_enable callback. We should _never_ add new register restore code
to this since maintaining such code is a major pain.
+
I915_WRITE(_FDI_RXA_CTL, dev_priv-regfile.saveFDI_RXA_CTL);
I915_WRITE(_FDI_TXA_CTL, dev_priv-regfile.saveFDI_TXA_CTL);
diff --git a/drivers/gpu/drm/i915/intel_ddi.c
b/drivers/gpu/drm/i915/intel_ddi.c
index 0ad4e96..6784f0b 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1587,6 +1587,7 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
case TRANS_DDI_MODE_SELECT_DP_MST:
pipe_config-has_dp_encoder = true;
Re: [Intel-gfx] [PATCH 09/12] drm/i915: rip our vblank reset hacks for runtime PM
On Tue, May 20, 2014 at 03:03:41PM +0300, Ville Syrjälä wrote: On Wed, May 14, 2014 at 08:51:11PM +0200, Daniel Vetter wrote: Now that we unconditionally dtrt when disabling/enabling crtcs we don't need any hacks any longer to keep the vblank logic sane when all the registers go poof. So let's rip it all out. Hmm. drm_update_vblank_count() will now see some kind of diff between the last and current value when the registers got cloberred. So the vblank counter reported to userspace will jump. But I guess that's fine as long as userspace realizes that the counter is not at all reliable across modesets. I think that's a fair assumption. Modeset is kind of a heavy operation and I wouldn't expect applications to perform one all that often. That should be even more true of applications that rely on the counters. At least that would be my expectation. Thierry pgpvD_34flmTp.pgp Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 09/12] drm/irq: Lack of interrupt support in drm_vblank_on|off
On Wed, May 14, 2014 at 08:51:12PM +0200, Daniel Vetter wrote: If we want to use this functionality in generic helpers to make sure that all drivers have somewhat sane vblank handling across modesets/dpms, we need to make it work for all drivers. But some don't support interrupts and hence also not vblank waits. Just return early on such drivers. Note that with pageflips drivers are free to implement them however they wish to. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/drm_irq.c | 6 ++ 1 file changed, 6 insertions(+) Reviewed-by: Thierry Reding tred...@nvidia.com pgp5AsNdo5y_y.pgp Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 11/12] [RFC] drm/irq: More robustness in drm_vblank_on|off
On Wed, May 14, 2014 at 08:51:16PM +0200, Daniel Vetter wrote: If we want to use this functionality in generic helpers to make sure that all drivers have somewhat sane vblank handling across modesets/dpms, we need to make it work for all drivers. But some don't support interrupts and hence also not vblank waits. Just return early on such drivers. Note that with pageflips drivers are free to implement them however they wish to. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/drm_irq.c | 6 ++ 1 file changed, 6 insertions(+) I'm confused. This seems to be the very same patch as 09/12. But since you've already merged this I guess it must have resolved itself somehow... Thierry pgpN23RN71eTD.pgp Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 00/12] irq vblank handling rework
On Wed, May 21, 2014 at 10:35:59AM +0200, Daniel Vetter wrote: On Wed, May 21, 2014 at 11:26:55AM +0300, Ville Syrjälä wrote: For everything but the reset_vblank_counter() thing: Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com And another caveat: I only glanced at the crtc_helper stuff. It looks sane but I didn't go reading through the drivers to figure out if they would fall over or work. Oh, the rfc was really just that. I don't have any plans to burn my hands trying to merge those patches ;-) Especially since interest from non-i915 hackers seems to be low. I think that's mostly because nobody except i915 has a testsuite that would even remotely be able to uncover any of these issues. =) Thierry pgpvfQATH_eUX.pgp Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2] drm/i915: Add null state batch to active list
for proper refcounting to take place as we use
i915_add_request() for it.
i915_add_request() also takes the context for the request
from ring-last_context so move the null state batch
submission after the ring context has been set.
v2: we need to check for correct ring now (Ville Syrjälä)
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
Cc: Chris Wilson ch...@chris-wilson.co.uk
Cc: Damien Lespiau damien.lesp...@intel.com
Signed-off-by: Mika Kuoppala mika.kuopp...@intel.com
---
drivers/gpu/drm/i915/i915_drv.h |3 +++
drivers/gpu/drm/i915/i915_gem.c |4 ++--
drivers/gpu/drm/i915/i915_gem_context.c | 16
drivers/gpu/drm/i915/i915_gem_render_state.c | 10 --
4 files changed, 21 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b90ec69..6881f70 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2204,6 +2204,9 @@ static inline void i915_gem_object_unpin_pages(struct
drm_i915_gem_object *obj)
int __must_check i915_mutex_lock_interruptible(struct drm_device *dev);
int i915_gem_object_sync(struct drm_i915_gem_object *obj,
struct intel_ring_buffer *to);
+void i915_gem_object_move_to_active(struct drm_i915_gem_object *obj,
+ struct intel_ring_buffer *ring);
+void i915_gem_object_move_to_inactive(struct drm_i915_gem_object *obj);
void i915_vma_move_to_active(struct i915_vma *vma,
struct intel_ring_buffer *ring);
int i915_gem_dumb_create(struct drm_file *file_priv,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 440979f..d795800 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2038,7 +2038,7 @@ i915_gem_object_get_pages(struct drm_i915_gem_object *obj)
return 0;
}
-static void
+void
i915_gem_object_move_to_active(struct drm_i915_gem_object *obj,
struct intel_ring_buffer *ring)
{
@@ -2084,7 +2084,7 @@ void i915_vma_move_to_active(struct i915_vma *vma,
return i915_gem_object_move_to_active(vma-obj, ring);
}
-static void
+void
i915_gem_object_move_to_inactive(struct drm_i915_gem_object *obj)
{
struct drm_i915_private *dev_priv = obj-base.dev-dev_private;
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index f220c94..6a2d847a 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -700,21 +700,21 @@ static int do_switch(struct intel_ring_buffer *ring,
/* obj is kept alive until the next request by its active ref */
i915_gem_object_ggtt_unpin(from-obj);
i915_gem_context_unreference(from);
- } else {
- if (to-is_initialized == false) {
- ret = i915_gem_render_state_init(ring);
- if (ret)
- DRM_ERROR(init render state: %d\n, ret);
- }
}
- to-is_initialized = true;
-
done:
i915_gem_context_reference(to);
ring-last_context = to;
to-last_ring = ring;
+ if (ring-id == RCS !to-is_initialized from == NULL) {
+ ret = i915_gem_render_state_init(ring);
+ if (ret)
+ DRM_ERROR(init render state: %d\n, ret);
+ }
+
+ to-is_initialized = true;
+
return 0;
unpin_out:
diff --git a/drivers/gpu/drm/i915/i915_gem_render_state.c
b/drivers/gpu/drm/i915/i915_gem_render_state.c
index 392aa7b..463fff1 100644
--- a/drivers/gpu/drm/i915/i915_gem_render_state.c
+++ b/drivers/gpu/drm/i915/i915_gem_render_state.c
@@ -164,9 +164,11 @@ int i915_gem_render_state_init(struct intel_ring_buffer
*ring)
const int gen = INTEL_INFO(ring-dev)-gen;
struct i915_render_state *so;
const struct intel_renderstate_rodata *rodata;
- u32 seqno;
int ret;
+ if (WARN_ON(ring-id != RCS))
+ return -ENOENT;
+
rodata = render_state_get_rodata(ring-dev, gen);
if (rodata == NULL)
return 0;
@@ -186,7 +188,11 @@ int i915_gem_render_state_init(struct intel_ring_buffer
*ring)
if (ret)
goto out;
- ret = i915_add_request(ring, seqno);
+ i915_gem_object_move_to_active(so-obj, ring);
+
+ ret = __i915_add_request(ring, NULL, so-obj, NULL);
+ if (ret)
+ i915_gem_object_move_to_inactive(so-obj);
out:
render_state_free(so);
--
1.7.9.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Add null state batch to active list
On Wed, May 21, 2014 at 03:08:51PM +0300, Mika Kuoppala wrote: diff --git a/drivers/gpu/drm/i915/i915_gem_render_state.c b/drivers/gpu/drm/i915/i915_gem_render_state.c index 392aa7b..d29e6b2 100644 --- a/drivers/gpu/drm/i915/i915_gem_render_state.c +++ b/drivers/gpu/drm/i915/i915_gem_render_state.c @@ -164,7 +164,6 @@ int i915_gem_render_state_init(struct intel_ring_buffer *ring) const int gen = INTEL_INFO(ring-dev)-gen; struct i915_render_state *so; const struct intel_renderstate_rodata *rodata; - u32 seqno; int ret; rodata = render_state_get_rodata(ring-dev, gen); @@ -186,7 +185,11 @@ int i915_gem_render_state_init(struct intel_ring_buffer *ring) if (ret) goto out; - ret = i915_add_request(ring, seqno); + i915_gem_object_move_to_active(so-obj, ring); You have the vma, use it rather than exposing this function. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 11/12] [RFC] drm/irq: More robustness in drm_vblank_on|off
On Wed, May 21, 2014 at 02:53:00PM +0200, Thierry Reding wrote: On Wed, May 14, 2014 at 08:51:16PM +0200, Daniel Vetter wrote: If we want to use this functionality in generic helpers to make sure that all drivers have somewhat sane vblank handling across modesets/dpms, we need to make it work for all drivers. But some don't support interrupts and hence also not vblank waits. Just return early on such drivers. Note that with pageflips drivers are free to implement them however they wish to. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/drm_irq.c | 6 ++ 1 file changed, 6 insertions(+) I'm confused. This seems to be the very same patch as 09/12. But since you've already merged this I guess it must have resolved itself somehow... Screwed up the patch sending and submitted two patch 09/12. This one really is just rfc and I didn't pull it in yet. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3] drm/i915: Add null state batch to active list
for proper refcounting to take place as we use
i915_add_request() for it.
i915_add_request() also takes the context for the request
from ring-last_context so move the null state batch
submission after the ring context has been set.
v2: we need to check for correct ring now (Ville Syrjälä)
v3: no need to expose i915_gem_move_object_to_active (Chris Wilson)
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
Cc: Chris Wilson ch...@chris-wilson.co.uk
Cc: Damien Lespiau damien.lesp...@intel.com
Signed-off-by: Mika Kuoppala mika.kuopp...@intel.com
---
drivers/gpu/drm/i915/i915_drv.h |1 +
drivers/gpu/drm/i915/i915_gem.c |2 +-
drivers/gpu/drm/i915/i915_gem_context.c | 16
drivers/gpu/drm/i915/i915_gem_render_state.c | 17 +++--
4 files changed, 25 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b90ec69..0bf7bfb 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2206,6 +2206,7 @@ int i915_gem_object_sync(struct drm_i915_gem_object *obj,
struct intel_ring_buffer *to);
void i915_vma_move_to_active(struct i915_vma *vma,
struct intel_ring_buffer *ring);
+void i915_gem_object_move_to_inactive(struct drm_i915_gem_object *obj);
int i915_gem_dumb_create(struct drm_file *file_priv,
struct drm_device *dev,
struct drm_mode_create_dumb *args);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 440979f..d9bf694 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2084,7 +2084,7 @@ void i915_vma_move_to_active(struct i915_vma *vma,
return i915_gem_object_move_to_active(vma-obj, ring);
}
-static void
+void
i915_gem_object_move_to_inactive(struct drm_i915_gem_object *obj)
{
struct drm_i915_private *dev_priv = obj-base.dev-dev_private;
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index f220c94..6a2d847a 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -700,21 +700,21 @@ static int do_switch(struct intel_ring_buffer *ring,
/* obj is kept alive until the next request by its active ref */
i915_gem_object_ggtt_unpin(from-obj);
i915_gem_context_unreference(from);
- } else {
- if (to-is_initialized == false) {
- ret = i915_gem_render_state_init(ring);
- if (ret)
- DRM_ERROR(init render state: %d\n, ret);
- }
}
- to-is_initialized = true;
-
done:
i915_gem_context_reference(to);
ring-last_context = to;
to-last_ring = ring;
+ if (ring-id == RCS !to-is_initialized from == NULL) {
+ ret = i915_gem_render_state_init(ring);
+ if (ret)
+ DRM_ERROR(init render state: %d\n, ret);
+ }
+
+ to-is_initialized = true;
+
return 0;
unpin_out:
diff --git a/drivers/gpu/drm/i915/i915_gem_render_state.c
b/drivers/gpu/drm/i915/i915_gem_render_state.c
index 392aa7b..82abe1e 100644
--- a/drivers/gpu/drm/i915/i915_gem_render_state.c
+++ b/drivers/gpu/drm/i915/i915_gem_render_state.c
@@ -164,9 +164,12 @@ int i915_gem_render_state_init(struct intel_ring_buffer
*ring)
const int gen = INTEL_INFO(ring-dev)-gen;
struct i915_render_state *so;
const struct intel_renderstate_rodata *rodata;
- u32 seqno;
+ struct i915_vma *vma;
int ret;
+ if (WARN_ON(ring-id != RCS))
+ return -ENOENT;
+
rodata = render_state_get_rodata(ring-dev, gen);
if (rodata == NULL)
return 0;
@@ -186,7 +189,17 @@ int i915_gem_render_state_init(struct intel_ring_buffer
*ring)
if (ret)
goto out;
- ret = i915_add_request(ring, seqno);
+ vma = i915_gem_obj_to_ggtt(so-obj);
+ if (vma == NULL) {
+ ret = -ENOSPC;
+ goto out;
+ }
+
+ i915_vma_move_to_active(vma, ring);
+
+ ret = __i915_add_request(ring, NULL, so-obj, NULL);
+ if (ret)
+ i915_gem_object_move_to_inactive(so-obj);
out:
render_state_free(so);
--
1.7.9.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/5] drm/i915: Check for FIFO underuns when disabling reporting on gmch platforms
On 16 May 2014 17:40, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä ville.syrj...@linux.intel.com
FIFO underruns don't generate an interrupt on gmch platforms, so we
should check whether there were any that we failed to notice when
we're disabling FIFO underrun reporting.
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
Reviewed-by: Thomas Wood thomas.w...@intel.com
---
drivers/gpu/drm/i915/i915_irq.c | 18 --
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index b10fbde..8bb564b 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -266,16 +266,22 @@ static bool cpt_can_enable_serr_int(struct drm_device
*dev)
return true;
}
-static void i9xx_clear_fifo_underrun(struct drm_device *dev, enum pipe pipe)
+static void i9xx_set_fifo_underrun_reporting(struct drm_device *dev,
+enum pipe pipe, bool enable)
{
struct drm_i915_private *dev_priv = dev-dev_private;
u32 reg = PIPESTAT(pipe);
- u32 pipestat = I915_READ(reg) 0x7fff;
+ u32 pipestat = I915_READ(reg) 0x;
assert_spin_locked(dev_priv-irq_lock);
- I915_WRITE(reg, pipestat | PIPE_FIFO_UNDERRUN_STATUS);
- POSTING_READ(reg);
+ if (enable) {
+ I915_WRITE(reg, pipestat | PIPE_FIFO_UNDERRUN_STATUS);
+ POSTING_READ(reg);
+ } else {
+ if (pipestat PIPE_FIFO_UNDERRUN_STATUS)
+ DRM_ERROR(pipe %c underrun\n, pipe_name(pipe));
+ }
}
static void ironlake_set_fifo_underrun_reporting(struct drm_device *dev,
@@ -432,8 +438,8 @@ bool __intel_set_cpu_fifo_underrun_reporting(struct
drm_device *dev,
intel_crtc-cpu_fifo_underrun_disabled = !enable;
- if (enable (INTEL_INFO(dev)-gen 5 || IS_VALLEYVIEW(dev)))
- i9xx_clear_fifo_underrun(dev, pipe);
+ if (INTEL_INFO(dev)-gen 5 || IS_VALLEYVIEW(dev))
+ i9xx_set_fifo_underrun_reporting(dev, pipe, enable);
else if (IS_GEN5(dev) || IS_GEN6(dev))
ironlake_set_fifo_underrun_reporting(dev, pipe, enable);
else if (IS_GEN7(dev))
--
1.8.5.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/5] drm/i915: Simplify the uncleared FIFO underrun detection
On 16 May 2014 17:40, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä ville.syrj...@linux.intel.com
Checking whether the error interrupt was enabled or not isn't really
necessary when we check for uncleared FIFO underruns. If it was enabled
we'll race with the interrupt handler a bit, but that seems OK as we
still claim the interrupt.
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
checkpatch.pl complains that there are lines over 80 characters, but otherwise:
Reviewed-by: Thomas Wood thomas.w...@intel.com
---
drivers/gpu/drm/i915/i915_irq.c | 13 ++---
1 file changed, 2 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 862964f..dd6e359 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -337,13 +337,9 @@ static void ivybridge_set_fifo_underrun_reporting(struct
drm_device *dev,
ironlake_enable_display_irq(dev_priv, DE_ERR_INT_IVB);
} else {
- bool was_enabled = !(I915_READ(DEIMR) DE_ERR_INT_IVB);
-
- /* Change the state _after_ we've read out the current one. */
ironlake_disable_display_irq(dev_priv, DE_ERR_INT_IVB);
- if (!was_enabled
- (I915_READ(GEN7_ERR_INT) ERR_INT_FIFO_UNDERRUN(pipe))) {
+ if (I915_READ(GEN7_ERR_INT) ERR_INT_FIFO_UNDERRUN(pipe)) {
DRM_ERROR(uncleared fifo underrun on pipe %c\n,
pipe_name(pipe));
}
@@ -421,14 +417,9 @@ static void cpt_set_fifo_underrun_reporting(struct
drm_device *dev,
ibx_enable_display_interrupt(dev_priv, SDE_ERROR_CPT);
} else {
- uint32_t tmp = I915_READ(SERR_INT);
- bool was_enabled = !(I915_READ(SDEIMR) SDE_ERROR_CPT);
-
- /* Change the state _after_ we've read out the current one. */
ibx_disable_display_interrupt(dev_priv, SDE_ERROR_CPT);
- if (!was_enabled
- (tmp SERR_INT_TRANS_FIFO_UNDERRUN(pch_transcoder))) {
+ if (I915_READ(SERR_INT)
SERR_INT_TRANS_FIFO_UNDERRUN(pch_transcoder)) {
DRM_ERROR(uncleared pch fifo underrun on pch
transcoder %c\n,
transcoder_name(pch_transcoder));
}
--
1.8.5.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/5] drm/i915: Check for FIFO underruns at the end of modeset on gmch
On 16 May 2014 17:40, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä ville.syrj...@linux.intel.com
FIFO underruns don't generate interrupts on gmch platforms, so
if we want to know whether a modeset triggered FIFO underruns we
need to explicitly check for them.
As a modeset on one pipe could cause underruns on other pipes,
check for underruns on all pipes.
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
Reviewed-by: Thomas Wood thomas.w...@intel.com
---
drivers/gpu/drm/i915/i915_irq.c | 28
drivers/gpu/drm/i915/intel_display.c | 6 ++
drivers/gpu/drm/i915/intel_drv.h | 1 +
3 files changed, 35 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 8bb564b..fdce260 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -266,6 +266,34 @@ static bool cpt_can_enable_serr_int(struct drm_device
*dev)
return true;
}
+void i9xx_check_fifo_underruns(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ struct intel_crtc *crtc;
+ unsigned long flags;
+
+ spin_lock_irqsave(dev_priv-irq_lock, flags);
+
+ for_each_intel_crtc(dev, crtc) {
+ u32 reg = PIPESTAT(crtc-pipe);
+ u32 pipestat;
+
+ if (crtc-cpu_fifo_underrun_disabled)
+ continue;
+
+ pipestat = I915_READ(reg) 0x;
+ if ((pipestat PIPE_FIFO_UNDERRUN_STATUS) == 0)
+ continue;
+
+ I915_WRITE(reg, pipestat | PIPE_FIFO_UNDERRUN_STATUS);
+ POSTING_READ(reg);
+
+ DRM_ERROR(pipe %c underrun\n, pipe_name(crtc-pipe));
+ }
+
+ spin_unlock_irqrestore(dev_priv-irq_lock, flags);
+}
+
static void i9xx_set_fifo_underrun_reporting(struct drm_device *dev,
enum pipe pipe, bool enable)
{
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 0f8f9bc..1b5164c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4545,6 +4545,9 @@ static void valleyview_crtc_enable(struct drm_crtc
*crtc)
intel_crtc_enable_planes(crtc);
drm_vblank_on(dev, pipe);
+
+ /* Underruns don't raise interrupts, so check manually. */
+ i9xx_check_fifo_underruns(dev);
}
static void i9xx_crtc_enable(struct drm_crtc *crtc)
@@ -4581,6 +4584,9 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
intel_crtc_enable_planes(crtc);
drm_vblank_on(dev, pipe);
+
+ /* Underruns don't raise interrupts, so check manually. */
+ i9xx_check_fifo_underruns(dev);
}
static void i9xx_pfit_disable(struct intel_crtc *crtc)
diff --git a/drivers/gpu/drm/i915/intel_drv.h
b/drivers/gpu/drm/i915/intel_drv.h
index 32a74e1..db0a74d 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -671,6 +671,7 @@ void snb_disable_pm_irq(struct drm_i915_private
*dev_priv, uint32_t mask);
void intel_runtime_pm_disable_interrupts(struct drm_device *dev);
void intel_runtime_pm_restore_interrupts(struct drm_device *dev);
int intel_get_crtc_scanline(struct intel_crtc *crtc);
+void i9xx_check_fifo_underruns(struct drm_device *dev);
/* intel_crt.c */
--
1.8.5.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/5] drm/i915: Shuffle fifo underrun disable/enable points for gmch platforms
On 16 May 2014 17:40, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com Gen2 reports FIFO underruns whenever no planes are enabled on the pipe. So in order to avoid false positives we must enable the FIFO underrun reporting only when at least one plane is enabled on the pipe. For now just move the underrun reporting enable/disable points to the other side of the plane enable/disable point. That doesn't cover cases when we turn off all the planes for the pipe but leave the pipe running on purpose, but it's better than the current situation. On gen4+ we can actually move the underrun reporting enable/disable to the opposite ends of the crtc enable/disable hooks. I suppose in theory we could leave the underrun reporting enabled all the time, except on VLV where PIPESTAT stops working when the display power well is down. If we ever get around to unifying the PIPESTAT irq handling for all gmch platforms, we should still follow the VLV route for other platforms. It would also micro-optimize the irq handler a bit since we could then skip the PIPESTAT reads for all disabled pipes. Gen3 is still a mystery, but for now I'm going to assume it behaves like gen4+. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com This doesn't apply to drm-intel-nightly, but looks fine in principle: Reviewed-by: Thomas Wood thomas.w...@intel.com --- drivers/gpu/drm/i915/intel_display.c | 30 +++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 1b5164c..7f61047 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4514,6 +4514,8 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc) intel_crtc-active = true; + intel_set_cpu_fifo_underrun_reporting(dev, pipe, true); + for_each_encoder_on_crtc(dev, crtc, encoder) if (encoder-pre_pll_enable) encoder-pre_pll_enable(encoder); @@ -4537,7 +4539,6 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc) intel_update_watermarks(crtc); intel_enable_pipe(intel_crtc); - intel_set_cpu_fifo_underrun_reporting(dev, pipe, true); for_each_encoder_on_crtc(dev, crtc, encoder) encoder-enable(encoder); @@ -4564,6 +4565,9 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc) intel_crtc-active = true; + if (!IS_GEN2(dev)) + intel_set_cpu_fifo_underrun_reporting(dev, pipe, true); + for_each_encoder_on_crtc(dev, crtc, encoder) if (encoder-pre_enable) encoder-pre_enable(encoder); @@ -4576,13 +4580,22 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc) intel_update_watermarks(crtc); intel_enable_pipe(intel_crtc); - intel_set_cpu_fifo_underrun_reporting(dev, pipe, true); for_each_encoder_on_crtc(dev, crtc, encoder) encoder-enable(encoder); intel_crtc_enable_planes(crtc); + /* +* Gen2 reports pipe underruns whenever all planes are disabled. +* So don't enable underrun reporting before at least some planes +* are enabled. +* FIXME: Need to fix the logic to work when we turn off all planes +* but leave the pipe running. +*/ + if (IS_GEN2(dev)) + intel_set_cpu_fifo_underrun_reporting(dev, pipe, true); + drm_vblank_on(dev, pipe); /* Underruns don't raise interrupts, so check manually. */ @@ -4615,12 +4628,20 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc) if (!intel_crtc-active) return; + /* +* Gen2 reports pipe underruns whenever all planes are disabled. +* So diasble underrun reporting before all the planes get disabled. +* FIXME: Need to fix the logic to work when we turn off all planes +* but leave the pipe running. +*/ + if (IS_GEN2(dev)) + intel_set_cpu_fifo_underrun_reporting(dev, pipe, false); + intel_crtc_disable_planes(crtc); for_each_encoder_on_crtc(dev, crtc, encoder) encoder-disable(encoder); - intel_set_cpu_fifo_underrun_reporting(dev, pipe, false); intel_disable_pipe(dev_priv, pipe); i9xx_pfit_disable(intel_crtc); @@ -4638,6 +4659,9 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc) i9xx_disable_pll(dev_priv, pipe); } + if (!IS_GEN2(dev)) + intel_set_cpu_fifo_underrun_reporting(dev, pipe, false); + intel_crtc-active = false; intel_update_watermarks(crtc); -- 1.8.5.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org
Re: [Intel-gfx] [PATCH 5/5] drm/i915: Shuffle fifo underrun disable/enable points for gmch platforms
On Fri, May 16, 2014 at 07:40:25PM +0300, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com Gen2 reports FIFO underruns whenever no planes are enabled on the pipe. So in order to avoid false positives we must enable the FIFO underrun reporting only when at least one plane is enabled on the pipe. For now just move the underrun reporting enable/disable points to the other side of the plane enable/disable point. That doesn't cover cases when we turn off all the planes for the pipe but leave the pipe running on purpose, but it's better than the current situation. On gen4+ we can actually move the underrun reporting enable/disable to the opposite ends of the crtc enable/disable hooks. I suppose in theory we could leave the underrun reporting enabled all the time, except on VLV where PIPESTAT stops working when the display power well is down. If we ever get around to unifying the PIPESTAT irq handling for all gmch platforms, we should still follow the VLV route for other platforms. It would also micro-optimize the irq handler a bit since we could then skip the PIPESTAT reads for all disabled pipes. Gen3 is still a mystery, but for now I'm going to assume it behaves like gen4+. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_display.c | 30 +++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 1b5164c..7f61047 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4514,6 +4514,8 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc) intel_crtc-active = true; + intel_set_cpu_fifo_underrun_reporting(dev, pipe, true); + for_each_encoder_on_crtc(dev, crtc, encoder) if (encoder-pre_pll_enable) encoder-pre_pll_enable(encoder); @@ -4537,7 +4539,6 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc) intel_update_watermarks(crtc); intel_enable_pipe(intel_crtc); - intel_set_cpu_fifo_underrun_reporting(dev, pipe, true); for_each_encoder_on_crtc(dev, crtc, encoder) encoder-enable(encoder); @@ -4564,6 +4565,9 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc) intel_crtc-active = true; + if (!IS_GEN2(dev)) + intel_set_cpu_fifo_underrun_reporting(dev, pipe, true); + for_each_encoder_on_crtc(dev, crtc, encoder) if (encoder-pre_enable) encoder-pre_enable(encoder); @@ -4576,13 +4580,22 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc) intel_update_watermarks(crtc); intel_enable_pipe(intel_crtc); - intel_set_cpu_fifo_underrun_reporting(dev, pipe, true); for_each_encoder_on_crtc(dev, crtc, encoder) encoder-enable(encoder); intel_crtc_enable_planes(crtc); + /* + * Gen2 reports pipe underruns whenever all planes are disabled. + * So don't enable underrun reporting before at least some planes + * are enabled. + * FIXME: Need to fix the logic to work when we turn off all planes + * but leave the pipe running. + */ + if (IS_GEN2(dev)) + intel_set_cpu_fifo_underrun_reporting(dev, pipe, true); I guess we should do this as part of the nuclear pageflip code iff all planes are off. But for now this looks good enough imo. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/5] drm/i915: Shuffle fifo underrun disable/enable points for gmch platforms
On Wed, May 21, 2014 at 04:52:31PM +0200, Daniel Vetter wrote: On Fri, May 16, 2014 at 07:40:25PM +0300, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com Gen2 reports FIFO underruns whenever no planes are enabled on the pipe. So in order to avoid false positives we must enable the FIFO underrun reporting only when at least one plane is enabled on the pipe. For now just move the underrun reporting enable/disable points to the other side of the plane enable/disable point. That doesn't cover cases when we turn off all the planes for the pipe but leave the pipe running on purpose, but it's better than the current situation. On gen4+ we can actually move the underrun reporting enable/disable to the opposite ends of the crtc enable/disable hooks. I suppose in theory we could leave the underrun reporting enabled all the time, except on VLV where PIPESTAT stops working when the display power well is down. If we ever get around to unifying the PIPESTAT irq handling for all gmch platforms, we should still follow the VLV route for other platforms. It would also micro-optimize the irq handler a bit since we could then skip the PIPESTAT reads for all disabled pipes. Gen3 is still a mystery, but for now I'm going to assume it behaves like gen4+. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_display.c | 30 +++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 1b5164c..7f61047 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4514,6 +4514,8 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc) intel_crtc-active = true; + intel_set_cpu_fifo_underrun_reporting(dev, pipe, true); + for_each_encoder_on_crtc(dev, crtc, encoder) if (encoder-pre_pll_enable) encoder-pre_pll_enable(encoder); @@ -4537,7 +4539,6 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc) intel_update_watermarks(crtc); intel_enable_pipe(intel_crtc); - intel_set_cpu_fifo_underrun_reporting(dev, pipe, true); for_each_encoder_on_crtc(dev, crtc, encoder) encoder-enable(encoder); @@ -4564,6 +4565,9 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc) intel_crtc-active = true; + if (!IS_GEN2(dev)) + intel_set_cpu_fifo_underrun_reporting(dev, pipe, true); + for_each_encoder_on_crtc(dev, crtc, encoder) if (encoder-pre_enable) encoder-pre_enable(encoder); @@ -4576,13 +4580,22 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc) intel_update_watermarks(crtc); intel_enable_pipe(intel_crtc); - intel_set_cpu_fifo_underrun_reporting(dev, pipe, true); for_each_encoder_on_crtc(dev, crtc, encoder) encoder-enable(encoder); intel_crtc_enable_planes(crtc); + /* +* Gen2 reports pipe underruns whenever all planes are disabled. +* So don't enable underrun reporting before at least some planes +* are enabled. +* FIXME: Need to fix the logic to work when we turn off all planes +* but leave the pipe running. +*/ + if (IS_GEN2(dev)) + intel_set_cpu_fifo_underrun_reporting(dev, pipe, true); I guess we should do this as part of the nuclear pageflip code iff all planes are off. But for now this looks good enough imo. Forgotten to mention: All patches merged now, thanks. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3] drm/i915: Add null state batch to active list
On Wed, May 21, 2014 at 05:02:56PM +0300, Mika Kuoppala wrote:
for proper refcounting to take place as we use
i915_add_request() for it.
i915_add_request() also takes the context for the request
from ring-last_context so move the null state batch
submission after the ring context has been set.
v2: we need to check for correct ring now (Ville Syrjälä)
v3: no need to expose i915_gem_move_object_to_active (Chris Wilson)
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
Cc: Chris Wilson ch...@chris-wilson.co.uk
Cc: Damien Lespiau damien.lesp...@intel.com
Signed-off-by: Mika Kuoppala mika.kuopp...@intel.com
Merged with Ville's irc r-b, thanks for the quick fix.
-Daniel
---
drivers/gpu/drm/i915/i915_drv.h |1 +
drivers/gpu/drm/i915/i915_gem.c |2 +-
drivers/gpu/drm/i915/i915_gem_context.c | 16
drivers/gpu/drm/i915/i915_gem_render_state.c | 17 +++--
4 files changed, 25 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b90ec69..0bf7bfb 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2206,6 +2206,7 @@ int i915_gem_object_sync(struct drm_i915_gem_object
*obj,
struct intel_ring_buffer *to);
void i915_vma_move_to_active(struct i915_vma *vma,
struct intel_ring_buffer *ring);
+void i915_gem_object_move_to_inactive(struct drm_i915_gem_object *obj);
int i915_gem_dumb_create(struct drm_file *file_priv,
struct drm_device *dev,
struct drm_mode_create_dumb *args);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 440979f..d9bf694 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2084,7 +2084,7 @@ void i915_vma_move_to_active(struct i915_vma *vma,
return i915_gem_object_move_to_active(vma-obj, ring);
}
-static void
+void
i915_gem_object_move_to_inactive(struct drm_i915_gem_object *obj)
{
struct drm_i915_private *dev_priv = obj-base.dev-dev_private;
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index f220c94..6a2d847a 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -700,21 +700,21 @@ static int do_switch(struct intel_ring_buffer *ring,
/* obj is kept alive until the next request by its active ref */
i915_gem_object_ggtt_unpin(from-obj);
i915_gem_context_unreference(from);
- } else {
- if (to-is_initialized == false) {
- ret = i915_gem_render_state_init(ring);
- if (ret)
- DRM_ERROR(init render state: %d\n, ret);
- }
}
- to-is_initialized = true;
-
done:
i915_gem_context_reference(to);
ring-last_context = to;
to-last_ring = ring;
+ if (ring-id == RCS !to-is_initialized from == NULL) {
+ ret = i915_gem_render_state_init(ring);
+ if (ret)
+ DRM_ERROR(init render state: %d\n, ret);
+ }
+
+ to-is_initialized = true;
+
return 0;
unpin_out:
diff --git a/drivers/gpu/drm/i915/i915_gem_render_state.c
b/drivers/gpu/drm/i915/i915_gem_render_state.c
index 392aa7b..82abe1e 100644
--- a/drivers/gpu/drm/i915/i915_gem_render_state.c
+++ b/drivers/gpu/drm/i915/i915_gem_render_state.c
@@ -164,9 +164,12 @@ int i915_gem_render_state_init(struct intel_ring_buffer
*ring)
const int gen = INTEL_INFO(ring-dev)-gen;
struct i915_render_state *so;
const struct intel_renderstate_rodata *rodata;
- u32 seqno;
+ struct i915_vma *vma;
int ret;
+ if (WARN_ON(ring-id != RCS))
+ return -ENOENT;
+
rodata = render_state_get_rodata(ring-dev, gen);
if (rodata == NULL)
return 0;
@@ -186,7 +189,17 @@ int i915_gem_render_state_init(struct intel_ring_buffer
*ring)
if (ret)
goto out;
- ret = i915_add_request(ring, seqno);
+ vma = i915_gem_obj_to_ggtt(so-obj);
+ if (vma == NULL) {
+ ret = -ENOSPC;
+ goto out;
+ }
+
+ i915_vma_move_to_active(vma, ring);
+
+ ret = __i915_add_request(ring, NULL, so-obj, NULL);
+ if (ret)
+ i915_gem_object_move_to_inactive(so-obj);
out:
render_state_free(so);
--
1.7.9.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
Re: [Intel-gfx] [PATCH v3] drm/i915: Add null state batch to active list
On Wed, May 21, 2014 at 04:54:55PM +0200, Daniel Vetter wrote:
On Wed, May 21, 2014 at 05:02:56PM +0300, Mika Kuoppala wrote:
for proper refcounting to take place as we use
i915_add_request() for it.
i915_add_request() also takes the context for the request
from ring-last_context so move the null state batch
submission after the ring context has been set.
v2: we need to check for correct ring now (Ville Syrjälä)
v3: no need to expose i915_gem_move_object_to_active (Chris Wilson)
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
Cc: Chris Wilson ch...@chris-wilson.co.uk
Cc: Damien Lespiau damien.lesp...@intel.com
Signed-off-by: Mika Kuoppala mika.kuopp...@intel.com
Merged with Ville's irc r-b, thanks for the quick fix.
Pity, it still contains some code that I'd rather not start
cargo-culting.
- ret = i915_add_request(ring, seqno);
+ vma = i915_gem_obj_to_ggtt(so-obj);
+ if (vma == NULL) {
+ ret = -ENOSPC;
+ goto out;
+ }
We use the GGTT vma much earlier, so this is suspect.
+
+ i915_vma_move_to_active(vma, ring);
+
+ ret = __i915_add_request(ring, NULL, so-obj, NULL);
+ if (ret)
+ i915_gem_object_move_to_inactive(so-obj);
As is move-to-inactive here. The only way add-request can fail is via an
EIO, and that will have triggered the move-to-inactive already - which
should nicely catch a BUG or two.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/3] drm/i915: use async hpd_irq_event function on resume
On Wed, 21 May 2014 08:52:34 +0200 Daniel Vetter dan...@ffwll.ch wrote: On Tue, May 20, 2014 at 03:25:35PM -0700, Jesse Barnes wrote: Gets the detect code (which may take awhile) out of the resume path, speeding things up a bit. Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org --- drivers/gpu/drm/i915/i915_drv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 302495f..571f688 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -606,7 +606,7 @@ static int __i915_drm_thaw(struct drm_device *dev, bool restore_gtt_mappings) intel_hpd_init(dev); dev_priv-enable_hotplug_processing = true; /* Config may have changed between suspend and resume */ - drm_helper_hpd_irq_event(dev); + async_schedule(drm_helper_hpd_irq_event_async, dev); Does that really help all that much? I've thought the driver core sychnronizes all the async workers again once resume is done. I'm better to schedule this as a fully async work with e.g. a 1s delay, like we do with the rps resume work. That might be better, I'll check on the synchronization. I thought async_schedule was the new hotness we were supposed to use everywhere... -- Jesse Barnes, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/3] drm/i915: use async hpd_irq_event function on resume
On Wed, May 21, 2014 at 08:00:22AM -0700, Jesse Barnes wrote: On Wed, 21 May 2014 08:52:34 +0200 Daniel Vetter dan...@ffwll.ch wrote: On Tue, May 20, 2014 at 03:25:35PM -0700, Jesse Barnes wrote: Gets the detect code (which may take awhile) out of the resume path, speeding things up a bit. Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org --- drivers/gpu/drm/i915/i915_drv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 302495f..571f688 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -606,7 +606,7 @@ static int __i915_drm_thaw(struct drm_device *dev, bool restore_gtt_mappings) intel_hpd_init(dev); dev_priv-enable_hotplug_processing = true; /* Config may have changed between suspend and resume */ - drm_helper_hpd_irq_event(dev); + async_schedule(drm_helper_hpd_irq_event_async, dev); Does that really help all that much? I've thought the driver core sychnronizes all the async workers again once resume is done. I'm better to schedule this as a fully async work with e.g. a 1s delay, like we do with the rps resume work. That might be better, I'll check on the synchronization. I thought async_schedule was the new hotness we were supposed to use everywhere... It's pretty cool for easy async in driver load/resume since it autosyncs. You can even create more async domains if you need finer-grained sync points. But if we know that we can be more lenient than full sync before our driver load/resume completes we need to use classical work queues. But for stuff like doing paralle modeset restore async domains look like the right thing. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3] drm/i915: Add null state batch to active list
On Wed, May 21, 2014 at 04:00:40PM +0100, Chris Wilson wrote:
On Wed, May 21, 2014 at 04:54:55PM +0200, Daniel Vetter wrote:
On Wed, May 21, 2014 at 05:02:56PM +0300, Mika Kuoppala wrote:
for proper refcounting to take place as we use
i915_add_request() for it.
i915_add_request() also takes the context for the request
from ring-last_context so move the null state batch
submission after the ring context has been set.
v2: we need to check for correct ring now (Ville Syrjälä)
v3: no need to expose i915_gem_move_object_to_active (Chris Wilson)
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
Cc: Chris Wilson ch...@chris-wilson.co.uk
Cc: Damien Lespiau damien.lesp...@intel.com
Signed-off-by: Mika Kuoppala mika.kuopp...@intel.com
Merged with Ville's irc r-b, thanks for the quick fix.
Pity, it still contains some code that I'd rather not start
cargo-culting.
Using vmas more as first-class objects I support. Mika can you please
resend? I'll keep v3 for now to avoid blocking people.
-Daniel
- ret = i915_add_request(ring, seqno);
+ vma = i915_gem_obj_to_ggtt(so-obj);
+ if (vma == NULL) {
+ ret = -ENOSPC;
+ goto out;
+ }
We use the GGTT vma much earlier, so this is suspect.
+
+ i915_vma_move_to_active(vma, ring);
+
+ ret = __i915_add_request(ring, NULL, so-obj, NULL);
+ if (ret)
+ i915_gem_object_move_to_inactive(so-obj);
As is move-to-inactive here. The only way add-request can fail is via an
EIO, and that will have triggered the move-to-inactive already - which
should nicely catch a BUG or two.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm/i915: Change Mipi register definitions
Re-define MIPI register definitions in such a way that most of
the existing DSI code can be re-used for future platforms. Register
definitions are re-written using MMIO offset variable, so that without
changing the existing sequence, same code can be generically applied.
V2: Addressing review comments by Damien, added follwing changes:
1. Re-defined MIPI_DSI_FUNC_PRG using _PIPE macro, to remove
branching.
2. Re-written _MIPIB_DSI_FUNC_PRG and _MIPIA_DSI_FUNC_PRG
in single line.
Signed-off-by: Shashank Sharma shashank.sha...@intel.com
---
drivers/gpu/drm/i915/i915_reg.h | 689 +++
1 file changed, 416 insertions(+), 273 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index c12a858..50d5e89 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5655,20 +5655,23 @@ enum punit_power_well {
#define PIPE_CSC_POSTOFF_ME(pipe) _PIPE(pipe, _PIPE_A_CSC_POSTOFF_ME,
_PIPE_B_CSC_POSTOFF_ME)
#define PIPE_CSC_POSTOFF_LO(pipe) _PIPE(pipe, _PIPE_A_CSC_POSTOFF_LO,
_PIPE_B_CSC_POSTOFF_LO)
-/* VLV MIPI registers */
+/* MIPI registers */
+
+/* VLV port control */
#define _MIPIA_PORT_CTRL (VLV_DISPLAY_BASE + 0x61190)
#define _MIPIB_PORT_CTRL (VLV_DISPLAY_BASE + 0x61700)
#define MIPI_PORT_CTRL(pipe) _PIPE(pipe, _MIPIA_PORT_CTRL,
_MIPIB_PORT_CTRL)
-#define DPI_ENABLE(1 31) /* A + B */
+
+#define DPI_ENABLE(1 31)
#define MIPIA_MIPI4DPHY_DELAY_COUNT_SHIFT 27
#define MIPIA_MIPI4DPHY_DELAY_COUNT_MASK (0xf 27)
#define DUAL_LINK_MODE_MASK (1 26)
#define DUAL_LINK_MODE_FRONT_BACK (0 26)
#define DUAL_LINK_MODE_PIXEL_ALTERNATIVE (1 26)
-#define DITHERING_ENABLE (1 25) /* A + B */
+#define DITHERING_ENABLE (1 25)
#define FLOPPED_HSTX (1 23)
-#define DE_INVERT (1 19) /* XXX */
+#define DE_INVERT (1 19)
#define MIPIA_FLISDSI_DELAY_COUNT_SHIFT 18
#define MIPIA_FLISDSI_DELAY_COUNT_MASK(0xf 18)
#define AFE_LATCHOUT (1 17)
@@ -5699,33 +5702,46 @@ enum punit_power_well {
#define LANE_CONFIGURATION_DUAL_LINK_A(1 0)
#define LANE_CONFIGURATION_DUAL_LINK_B(2 0)
+/* VLV tearing effect control */
#define _MIPIA_TEARING_CTRL(VLV_DISPLAY_BASE + 0x61194)
#define _MIPIB_TEARING_CTRL(VLV_DISPLAY_BASE + 0x61704)
#define MIPI_TEARING_CTRL(pipe)_PIPE(pipe,
_MIPIA_TEARING_CTRL, _MIPIB_TEARING_CTRL)
-#define TEARING_EFFECT_DELAY_SHIFT0
-#define TEARING_EFFECT_DELAY_MASK (0x 0)
+#define TEARING_EFFECT_DELAY_SHIFT 0
+#define TEARING_EFFECT_DELAY_MASK (0x 0)
-/* XXX: all bits reserved */
-#define _MIPIA_AUTOPWG (VLV_DISPLAY_BASE + 0x611a0)
+/* VLV: all bits reserved */
+#define _MIPIA_AUTOPWG (dev_priv-mipi_mmio_base + \
+ 0x611a0)
/* MIPI DSI Controller and D-PHY registers */
+#define _MIPIA_DEVICE_READY(dev_priv-mipi_mmio_base + \
+ 0xb000)
+#define _MIPIB_DEVICE_READY(dev_priv-mipi_mmio_base + \
+ 0xb800)
+#define MIPI_DEVICE_READY(check) (!check ? \
+ _MIPIA_DEVICE_READY : _MIPIB_DEVICE_READY)
+
+#define BUS_POSSESSION(1 3) /* set to give bus to receiver
*/
+#define ULPS_STATE_MASK (3 1)
+#define ULPS_STATE_ENTER (2 1)
+#define ULPS_STATE_EXIT (1 1)
+#define ULPS_STATE_NORMAL_OPERATION (0 1)
+#define DEVICE_READY (1 0)
+
+#define _MIPIA_INTR_STAT (dev_priv-mipi_mmio_base + \
+ 0xb004)
+#define _MIPIB_INTR_STAT (dev_priv-mipi_mmio_base + \
+ 0xb804)
+#define MIPI_INTR_STAT(check) (!check ? \
+ _MIPIA_INTR_STAT : _MIPIB_INTR_STAT)
+
+#define _MIPIA_INTR_EN (dev_priv-mipi_mmio_base + \
+ 0xb008)
+#define _MIPIB_INTR_EN (dev_priv-mipi_mmio_base + \
+ 0xb808)
+#define MIPI_INTR_EN(check)(!check ? \
+ _MIPIA_INTR_EN : _MIPIB_INTR_EN)
-#define _MIPIA_DEVICE_READY(VLV_DISPLAY_BASE + 0xb000)
-#define _MIPIB_DEVICE_READY
Re: [Intel-gfx] [PATCH v3] drm/i915: Add null state batch to active list
On Wed, May 21, 2014 at 07:02:56AM -0700, Mika Kuoppala wrote:
+ if (ring-id == RCS !to-is_initialized from == NULL) {
+ ret = i915_gem_render_state_init(ring);
+ if (ret)
+ DRM_ERROR(init render state: %d\n, ret);
+ }
Apologies if this has already been discussed, but why do we have the
'from == NULL' check? Shouldn't we initialize all uninitialized RCS
contexts? Otherwise I thought we'll inherit whatever state 'from' left
behind.
The hw state should be valid in either case (and so I expect would fix
the rc6 issue either way), it's just the difference between initializing
every context to a specific valid state or initializing every context to
_some_ valid state. The commit message on the first render state patch
seemed to indicate the former while the implementation looks like the
latter. Just want to understand which we intended.
Thanks,
Brad
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Change Mipi register definitions
On Wed, May 21, 2014 at 08:56:59PM +0530, Shashank Sharma wrote:
Re-define MIPI register definitions in such a way that most of
the existing DSI code can be re-used for future platforms. Register
definitions are re-written using MMIO offset variable, so that without
changing the existing sequence, same code can be generically applied.
V2: Addressing review comments by Damien, added follwing changes:
1. Re-defined MIPI_DSI_FUNC_PRG using _PIPE macro, to remove
branching.
2. Re-written _MIPIB_DSI_FUNC_PRG and _MIPIA_DSI_FUNC_PRG
in single line.
Signed-off-by: Shashank Sharma shashank.sha...@intel.com
Sorry but this patch is a mess. Way too many pointless formatting
changes in there.
---
drivers/gpu/drm/i915/i915_reg.h | 689
+++
1 file changed, 416 insertions(+), 273 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index c12a858..50d5e89 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5655,20 +5655,23 @@ enum punit_power_well {
#define PIPE_CSC_POSTOFF_ME(pipe) _PIPE(pipe, _PIPE_A_CSC_POSTOFF_ME,
_PIPE_B_CSC_POSTOFF_ME)
#define PIPE_CSC_POSTOFF_LO(pipe) _PIPE(pipe, _PIPE_A_CSC_POSTOFF_LO,
_PIPE_B_CSC_POSTOFF_LO)
-/* VLV MIPI registers */
+/* MIPI registers */
This change is not needed.
+
+/* VLV port control */
#define _MIPIA_PORT_CTRL (VLV_DISPLAY_BASE + 0x61190)
#define _MIPIB_PORT_CTRL (VLV_DISPLAY_BASE + 0x61700)
#define MIPI_PORT_CTRL(pipe) _PIPE(pipe, _MIPIA_PORT_CTRL,
_MIPIB_PORT_CTRL)
Why isn't mipi_mmio_base used here? Does the register not need the new
offset? I htink it would still be cleaner to use the mipi_mmio_offset
for all the MIPI registers.
-#define DPI_ENABLE (1 31) /* A + B */
+
+#define DPI_ENABLE (1 31)
Not needed.
#define MIPIA_MIPI4DPHY_DELAY_COUNT_SHIFT 27
#define MIPIA_MIPI4DPHY_DELAY_COUNT_MASK(0xf 27)
#define DUAL_LINK_MODE_MASK (1 26)
#define DUAL_LINK_MODE_FRONT_BACK (0 26)
#define DUAL_LINK_MODE_PIXEL_ALTERNATIVE(1 26)
-#define DITHERING_ENABLE(1 25) /* A + B */
+#define DITHERING_ENABLE(1 25)
#define FLOPPED_HSTX(1 23)
-#define DE_INVERT (1 19) /* XXX */
+#define DE_INVERT (1 19)
More unneeded comment changes.
#define MIPIA_FLISDSI_DELAY_COUNT_SHIFT 18
#define MIPIA_FLISDSI_DELAY_COUNT_MASK (0xf 18)
#define AFE_LATCHOUT(1 17)
@@ -5699,33 +5702,46 @@ enum punit_power_well {
#define LANE_CONFIGURATION_DUAL_LINK_A (1 0)
#define LANE_CONFIGURATION_DUAL_LINK_B (2 0)
+/* VLV tearing effect control */
#define _MIPIA_TEARING_CTRL (VLV_DISPLAY_BASE + 0x61194)
#define _MIPIB_TEARING_CTRL (VLV_DISPLAY_BASE + 0x61704)
#define MIPI_TEARING_CTRL(pipe) _PIPE(pipe,
_MIPIA_TEARING_CTRL, _MIPIB_TEARING_CTRL)
-#define TEARING_EFFECT_DELAY_SHIFT 0
-#define TEARING_EFFECT_DELAY_MASK (0x 0)
+#define TEARING_EFFECT_DELAY_SHIFT 0
+#define TEARING_EFFECT_DELAY_MASK(0x 0)
Bad formatting change.
etc. Did you run this through some autoformatting tool or something?
Please don't do that. A simple sed job should be all that's needed
for mipi_mmio_offset.
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] tests/drv_hangman: Convert test from shell script to c
-Original Message- From: Mika Kuoppala [mailto:mika.kuopp...@linux.intel.com] Sent: Tuesday, May 20, 2014 12:56 PM To: intel-gfx@lists.freedesktop.org Cc: Mateo Lozano, Oscar Subject: [PATCH 1/2] tests/drv_hangman: Convert test from shell script to c Mixing script and standlone tests didn't mix well with the strict i915_ring_stop flags handling. Also squash drv_missed_irq_hang to the new test. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=78322 Suggested-by: Daniel Vetter daniel.vet...@ffwll.ch Signed-off-by: Mika Kuoppala mika.kuopp...@intel.com --- tests/Makefile.sources|3 +- tests/drv_hangman | 70 - tests/drv_hangman.c | 357 + tests/drv_missed_irq_hang | 70 - I am missing a tests/.gitignore entry Other than that: Reviewed-by: Oscar Mateo oscar.ma...@intel.com ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: move bsd dispatch index somewhere better
Adding stuff at the bottom is really no how this should be done, since
that's the place for ums/dri dungeons.
This was added in
commit a8ebba75b358f9c912cbcba0c14a2072e7280b2f
Author: Zhao Yakui yakui.z...@intel.com
Date: Thu Apr 17 10:37:40 2014 +0800
drm/i915: Use the coarse ping-pong mechanism based on drm fd to dispatch
the BSD command on BDW GT3
Also add a note to prevent this from happening again - people really
should be less lazy and take more time to look for a good home of
their new driver-global state.
Cc: Imre Deak imre.d...@intel.com
Cc: Zhao Yakui yakui.z...@intel.com
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/i915_dma.c| 1 -
drivers/gpu/drm/i915/i915_drv.h| 10 --
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 6 +++---
3 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index f14c47a715a4..ead67c0c4109 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1571,7 +1571,6 @@ int i915_driver_load(struct drm_device *dev, unsigned
long flags)
spin_lock_init(dev_priv-backlight_lock);
spin_lock_init(dev_priv-uncore.lock);
spin_lock_init(dev_priv-mm.object_stat_lock);
- dev_priv-ring_index = 0;
mutex_init(dev_priv-dpio_lock);
mutex_init(dev_priv-modeset_restore_lock);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 0bf7bfb6566c..d2da390b6b9f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1098,6 +1098,9 @@ struct i915_gem_mm {
*/
bool busy;
+ /* the indicator for dispatch video commands on two BSD rings */
+ int bsd_ring_dispatch_index;
+
/** Bit 6 swizzling required for X tiling */
uint32_t bit_6_swizzle_x;
/** Bit 6 swizzling required for Y tiling */
@@ -1553,8 +1556,11 @@ struct drm_i915_private {
struct i915_dri1_state dri1;
/* Old ums support infrastructure, same warning applies. */
struct i915_ums_state ums;
- /* the indicator for dispatch video commands on two BSD rings */
- int ring_index;
+
+ /*
+* NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
+* will be rejected. Instead look for a better place.
+*/
};
static inline struct drm_i915_private *to_i915(const struct drm_device *dev)
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 94e53a0fcdc3..de2fd90bdd0f 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1027,12 +1027,12 @@ static int gen8_dispatch_bsd_ring(struct drm_device
*dev,
int ring_id;
mutex_lock(dev-struct_mutex);
- if (dev_priv-ring_index == 0) {
+ if (dev_priv-mm.bsd_ring_dispatch_index == 0) {
ring_id = VCS;
- dev_priv-ring_index = 1;
+ dev_priv-mm.bsd_ring_dispatch_index = 1;
} else {
ring_id = VCS2;
- dev_priv-ring_index = 0;
+ dev_priv-mm.bsd_ring_dispatch_index = 0;
}
file_priv-bsd_ring = dev_priv-ring[ring_id];
mutex_unlock(dev-struct_mutex);
--
1.9.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Change Mipi register definitions
Thanks for pointing this out.
I will correct all formatting stuff and re-send patch.
Regards
Shashank
-Original Message-
From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
Sent: Wednesday, May 21, 2014 9:05 PM
To: Sharma, Shashank
Cc: Lespiau, Damien; Vetter, Daniel; intel-gfx@lists.freedesktop.org; Nikula,
Jani
Subject: Re: [Intel-gfx] [PATCH 2/2] drm/i915: Change Mipi register definitions
On Wed, May 21, 2014 at 08:56:59PM +0530, Shashank Sharma wrote:
Re-define MIPI register definitions in such a way that most of the
existing DSI code can be re-used for future platforms. Register
definitions are re-written using MMIO offset variable, so that without
changing the existing sequence, same code can be generically applied.
V2: Addressing review comments by Damien, added follwing changes:
1. Re-defined MIPI_DSI_FUNC_PRG using _PIPE macro, to remove
branching.
2. Re-written _MIPIB_DSI_FUNC_PRG and _MIPIA_DSI_FUNC_PRG
in single line.
Signed-off-by: Shashank Sharma shashank.sha...@intel.com
Sorry but this patch is a mess. Way too many pointless formatting changes in
there.
---
drivers/gpu/drm/i915/i915_reg.h | 689
+++
1 file changed, 416 insertions(+), 273 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h
b/drivers/gpu/drm/i915/i915_reg.h index c12a858..50d5e89 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5655,20 +5655,23 @@ enum punit_power_well { #define
PIPE_CSC_POSTOFF_ME(pipe) _PIPE(pipe, _PIPE_A_CSC_POSTOFF_ME,
_PIPE_B_CSC_POSTOFF_ME) #define PIPE_CSC_POSTOFF_LO(pipe) _PIPE(pipe,
_PIPE_A_CSC_POSTOFF_LO, _PIPE_B_CSC_POSTOFF_LO)
-/* VLV MIPI registers */
+/* MIPI registers */
This change is not needed.
+
+/* VLV port control */
#define _MIPIA_PORT_CTRL (VLV_DISPLAY_BASE + 0x61190)
#define _MIPIB_PORT_CTRL (VLV_DISPLAY_BASE + 0x61700)
#define MIPI_PORT_CTRL(pipe) _PIPE(pipe, _MIPIA_PORT_CTRL,
_MIPIB_PORT_CTRL)
Why isn't mipi_mmio_base used here? Does the register not need the new offset?
I htink it would still be cleaner to use the mipi_mmio_offset for all the MIPI
registers.
-#define DPI_ENABLE (1 31) /* A + B */
+
+#define DPI_ENABLE (1 31)
Not needed.
#define MIPIA_MIPI4DPHY_DELAY_COUNT_SHIFT 27
#define MIPIA_MIPI4DPHY_DELAY_COUNT_MASK(0xf 27)
#define DUAL_LINK_MODE_MASK (1 26)
#define DUAL_LINK_MODE_FRONT_BACK (0 26)
#define DUAL_LINK_MODE_PIXEL_ALTERNATIVE(1 26)
-#define DITHERING_ENABLE(1 25) /* A + B */
+#define DITHERING_ENABLE(1 25)
#define FLOPPED_HSTX(1 23)
-#define DE_INVERT (1 19) /* XXX */
+#define DE_INVERT (1 19)
More unneeded comment changes.
#define MIPIA_FLISDSI_DELAY_COUNT_SHIFT 18
#define MIPIA_FLISDSI_DELAY_COUNT_MASK (0xf 18)
#define AFE_LATCHOUT(1 17)
@@ -5699,33 +5702,46 @@ enum punit_power_well {
#define LANE_CONFIGURATION_DUAL_LINK_A (1 0)
#define LANE_CONFIGURATION_DUAL_LINK_B (2 0)
+/* VLV tearing effect control */
#define _MIPIA_TEARING_CTRL (VLV_DISPLAY_BASE + 0x61194)
#define _MIPIB_TEARING_CTRL (VLV_DISPLAY_BASE + 0x61704)
#define MIPI_TEARING_CTRL(pipe) _PIPE(pipe,
_MIPIA_TEARING_CTRL, _MIPIB_TEARING_CTRL)
-#define TEARING_EFFECT_DELAY_SHIFT 0
-#define TEARING_EFFECT_DELAY_MASK (0x 0)
+#define TEARING_EFFECT_DELAY_SHIFT 0
+#define TEARING_EFFECT_DELAY_MASK(0x 0)
Bad formatting change.
etc. Did you run this through some autoformatting tool or something?
Please don't do that. A simple sed job should be all that's needed for
mipi_mmio_offset.
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] tests/drv_hangman: Add subtest for error state capture/dump
-Original Message- From: Mika Kuoppala [mailto:mika.kuopp...@linux.intel.com] Sent: Tuesday, May 20, 2014 12:56 PM To: intel-gfx@lists.freedesktop.org Cc: Mateo Lozano, Oscar Subject: [PATCH 2/2] tests/drv_hangman: Add subtest for error state capture/dump Guarantees that error capture works at a very basic level. v2: Also check that the ring object contains a reloc with MI_BB_START for the presumed batch object's address. v3: Chris review comments: - Move variables to local scope. - Do not assume there is only one request. - Some gen encode flags into the BB start address. Also, use igt_set/get_stop_rings as suggested by Mika Kuoppala. v4: Make as a subtest of drv_hangman. Signed-off-by: Oscar Mateo oscar.ma...@intel.com (v3) Signed-off-by: Mika Kuoppala mika.kuopp...@intel.com Reviewed-by: Oscar Mateo oscar.ma...@intel.com (v4) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2] UXA: Wait until a pageflip actually completes to report it.
UXA was reporting page-flip completion as soon as the flip was scheduled
with the kernel, instead of waiting for the kernel to indicate that the
flip had actually completed.
Moving the DRI2SwapComplete call to the right place fixes all of our
Piglit tests for OML_sync_control when run on xf86-video-intel/UXA,
aside from a bit of difficult-to-reproduce flakiness when using a
divisor 1.
This also eliminates a compile-time and run-time warning when built
against an xserver with Warn on DRI2SwapComplete with constant UST/MSC
applied.
v2: The drawable may have disappeared by the time the flip completes.
Don't try to report swap completion in that case.
Signed-off-by: Jamey Sharp ja...@minilop.net
Cc: Theo Hill theo0...@gmail.com
Cc: Eric Anholt e...@anholt.net
Cc: Chris Wilson ch...@chris-wilson.co.uk
---
I can experimentally confirm Chris' claim that this patch causes
SwapBuffers to block once one swap is already outstanding, giving
double-buffering behavior rather than the desired triple-buffering.
However, it only has an effect for full-screen windows, and only when
not running under a compositor.
- If the window is not full-screen, UXA is already only double-buffered.
- If full-screen, UXA is usually triple-buffered, but not reliably.
- If run under a compositor, either the compositor crashes during my
test, or it still appears to be triple-buffered even with this patch.
If you want triple-buffering, NAK'ing this patch is clearly not the way
to get it, since the driver already doesn't do it reliably.
Please merge this patch, which fixes two spec violations that make
OML_sync_control unusable; and if you're concerned about uncomposited
triple-buffering in UXA, please find a less awful way to get it.
Thanks,
Jamey
src/uxa/intel_dri.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/src/uxa/intel_dri.c b/src/uxa/intel_dri.c
index ca58052..d4a242e 100644
--- a/src/uxa/intel_dri.c
+++ b/src/uxa/intel_dri.c
@@ -932,10 +932,6 @@ I830DRI2ScheduleFlip(struct intel_screen_private *intel,
/* Then flip DRI2 pointers and update the screen pixmap */
I830DRI2ExchangeBuffers(intel, info-front, info-back);
- DRI2SwapComplete(info-client, draw, 0, 0, 0,
-DRI2_EXCHANGE_COMPLETE,
-info-event_complete,
-info-event_data);
return TRUE;
}
@@ -1090,6 +1086,14 @@ void I830DRI2FlipEventHandler(unsigned int frame,
unsigned int tv_sec,
assert(intel-pending_flip[flip_info-pipe] == flip_info);
intel-pending_flip[flip_info-pipe] = NULL;
+ /* Assuming the drawable's still around, complete the swap. */
+ if (drawable)
+ DRI2SwapComplete(flip_info-client, drawable,
+frame, tv_sec, tv_usec,
+DRI2_EXCHANGE_COMPLETE,
+flip_info-event_complete,
+flip_info-event_data);
+
chain = flip_info-chain;
if (chain) {
DrawablePtr chain_drawable = NULL;
--
1.9.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Change Mipi register definitions
On Wed, May 21, 2014 at 06:35:12PM +0300, Ville Syrjälä wrote: + +/* VLV port control */ #define _MIPIA_PORT_CTRL (VLV_DISPLAY_BASE + 0x61190) #define _MIPIB_PORT_CTRL (VLV_DISPLAY_BASE + 0x61700) #define MIPI_PORT_CTRL(pipe) _PIPE(pipe, _MIPIA_PORT_CTRL, _MIPIB_PORT_CTRL) Why isn't mipi_mmio_base used here? Does the register not need the new offset? I htink it would still be cleaner to use the mipi_mmio_offset for all the MIPI registers. On VLV, this register (a at least one other) isn't part of the MIPI IP block address space (base + 0xbxxx) so shouldn't be defined as mipi_mmio_base + offset. -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC] set up an sync channel between audio and display driver (i.e. ALSA and DRM)
On Tue, May 20, 2014 at 05:29:07PM +0300, Imre Deak wrote: On Tue, 2014-05-20 at 05:52 +0300, Lin, Mengdong wrote: This RFC is based on previous discussion to set up a generic communication channel between display and audio driver and an internal design of Intel MCG/VPG HDMI audio driver. It's still an initial draft and your advice would be appreciated to improve the design. The basic idea is to create a new avsink module and let both drm and alsa depend on it. This new module provides a framework and APIs for synchronization between the display and audio driver. 1. Display/Audio Client The avsink core provides APIs to create, register and lookup a display/audio client. A specific display driver (eg. i915) or audio driver (eg. HD-Audio driver) can create a client, add some resources objects (shared power wells, display outputs, and audio inputs, register ops) to the client, and then register this client to avisink core. The peer driver can look up a registered client by a name or type, or both. If a client gives a valid peer client name on registration, avsink core will bind the two clients as peer for each other. And we expect a display client and an audio client to be peers for each other in a system. One problem we have at the moment is the order of calling the system suspend/resume handlers of the display driver wrt. that of the audio driver. Since the power well control is part of the display HW block, we need to run the display driver's resume handler first, initialize the HW, and only then let the audio driver's resume handler run. For similar reasons we have to call the audio suspend handler first and only then the display driver resume handler. Currently we solve this using the display driver's late/early suspend/resume hooks, but we'd need a more robust solution. This seems to be a similar issue to the load time ordering problem that you describe later. Having a real device for avsync that would be a child of the display device would solve the ordering issue in both cases. I admit I haven't looked into it if this is feasible, but I would like to see some solution to this as part of the plan. If we are able create and mandate that HDMI display controller is parent and audio is child device, then this wouldn't be an issue and PM frameowrk will ensure parent is suspended last. If there is a scenario where HDMI audio has to active but display has to go to low power, then parent-child device is not optimal. There needs to be a mechanism to turn on/off individual hw blocks within the controller. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC] set up an sync channel between audio and display driver (i.e. ALSA and DRM)
On Wed, May 21, 2014 at 5:56 PM, Babu, Ramesh ramesh.b...@intel.com wrote: On Tue, May 20, 2014 at 05:29:07PM +0300, Imre Deak wrote: On Tue, 2014-05-20 at 05:52 +0300, Lin, Mengdong wrote: This RFC is based on previous discussion to set up a generic communication channel between display and audio driver and an internal design of Intel MCG/VPG HDMI audio driver. It's still an initial draft and your advice would be appreciated to improve the design. The basic idea is to create a new avsink module and let both drm and alsa depend on it. This new module provides a framework and APIs for synchronization between the display and audio driver. 1. Display/Audio Client The avsink core provides APIs to create, register and lookup a display/audio client. A specific display driver (eg. i915) or audio driver (eg. HD-Audio driver) can create a client, add some resources objects (shared power wells, display outputs, and audio inputs, register ops) to the client, and then register this client to avisink core. The peer driver can look up a registered client by a name or type, or both. If a client gives a valid peer client name on registration, avsink core will bind the two clients as peer for each other. And we expect a display client and an audio client to be peers for each other in a system. One problem we have at the moment is the order of calling the system suspend/resume handlers of the display driver wrt. that of the audio driver. Since the power well control is part of the display HW block, we need to run the display driver's resume handler first, initialize the HW, and only then let the audio driver's resume handler run. For similar reasons we have to call the audio suspend handler first and only then the display driver resume handler. Currently we solve this using the display driver's late/early suspend/resume hooks, but we'd need a more robust solution. This seems to be a similar issue to the load time ordering problem that you describe later. Having a real device for avsync that would be a child of the display device would solve the ordering issue in both cases. I admit I haven't looked into it if this is feasible, but I would like to see some solution to this as part of the plan. If we are able create and mandate that HDMI display controller is parent and audio is child device, then this wouldn't be an issue and PM frameowrk will ensure parent is suspended last. If there is a scenario where HDMI audio has to active but display has to go to low power, then parent-child device is not optimal. There needs to be a mechanism to turn on/off individual hw blocks within the controller. Our gfx runtime pm code is a _lot_ better than that. We track each power domain individually and enable/disable them only when need. armsoc drivers could do the same or make sure that the avsink device is a child of the right block. Of course if your driver only has binary runtime pm and fires up everything then we have a problem. But imo that's a problem with that driver, not with making avsink real devices as children of something. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v4] drm/i915: Add null state batch to active list
From: Mika Kuoppala mika.kuopp...@linux.intel.com
for proper refcounting to take place as we use
i915_add_request() for it.
i915_add_request() also takes the context for the request
from ring-last_context so move the null state batch
submission after the ring context has been set.
v2: we need to check for correct ring now (Ville Syrjälä)
v3: no need to expose i915_gem_move_object_to_active (Chris Wilson)
v4: cargoculted vma/active/inactive error handling removed (Chris Wilson)
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
Cc: Chris Wilson ch...@chris-wilson.co.uk
Cc: Damien Lespiau damien.lesp...@intel.com
Signed-off-by: Mika Kuoppala mika.kuopp...@intel.com
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/i915_gem_context.c | 16
drivers/gpu/drm/i915/i915_gem_render_state.c |8 ++--
2 files changed, 14 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index f220c94..6a2d847a 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -700,21 +700,21 @@ static int do_switch(struct intel_ring_buffer *ring,
/* obj is kept alive until the next request by its active ref */
i915_gem_object_ggtt_unpin(from-obj);
i915_gem_context_unreference(from);
- } else {
- if (to-is_initialized == false) {
- ret = i915_gem_render_state_init(ring);
- if (ret)
- DRM_ERROR(init render state: %d\n, ret);
- }
}
- to-is_initialized = true;
-
done:
i915_gem_context_reference(to);
ring-last_context = to;
to-last_ring = ring;
+ if (ring-id == RCS !to-is_initialized from == NULL) {
+ ret = i915_gem_render_state_init(ring);
+ if (ret)
+ DRM_ERROR(init render state: %d\n, ret);
+ }
+
+ to-is_initialized = true;
+
return 0;
unpin_out:
diff --git a/drivers/gpu/drm/i915/i915_gem_render_state.c
b/drivers/gpu/drm/i915/i915_gem_render_state.c
index 392aa7b..cfbf6fc 100644
--- a/drivers/gpu/drm/i915/i915_gem_render_state.c
+++ b/drivers/gpu/drm/i915/i915_gem_render_state.c
@@ -164,9 +164,11 @@ int i915_gem_render_state_init(struct intel_ring_buffer
*ring)
const int gen = INTEL_INFO(ring-dev)-gen;
struct i915_render_state *so;
const struct intel_renderstate_rodata *rodata;
- u32 seqno;
int ret;
+ if (WARN_ON(ring-id != RCS))
+ return -ENOENT;
+
rodata = render_state_get_rodata(ring-dev, gen);
if (rodata == NULL)
return 0;
@@ -186,8 +188,10 @@ int i915_gem_render_state_init(struct intel_ring_buffer
*ring)
if (ret)
goto out;
- ret = i915_add_request(ring, seqno);
+ i915_vma_move_to_active(i915_gem_obj_to_ggtt(so-obj), ring);
+ ret = __i915_add_request(ring, NULL, so-obj, NULL);
+ /* __i915_add_request moves object to inactive if it fails */
out:
render_state_free(so);
return ret;
--
1.7.9.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] UXA: Wait until a pageflip actually completes to report it.
On Wed, May 21, 2014 at 08:53:18AM -0700, Jamey Sharp wrote: UXA was reporting page-flip completion as soon as the flip was scheduled with the kernel, instead of waiting for the kernel to indicate that the flip had actually completed. Moving the DRI2SwapComplete call to the right place fixes all of our Piglit tests for OML_sync_control when run on xf86-video-intel/UXA, aside from a bit of difficult-to-reproduce flakiness when using a divisor 1. This also eliminates a compile-time and run-time warning when built against an xserver with Warn on DRI2SwapComplete with constant UST/MSC applied. v2: The drawable may have disappeared by the time the flip completes. Don't try to report swap completion in that case. Signed-off-by: Jamey Sharp ja...@minilop.net Cc: Theo Hill theo0...@gmail.com Cc: Eric Anholt e...@anholt.net Cc: Chris Wilson ch...@chris-wilson.co.uk --- I can experimentally confirm Chris' claim that this patch causes SwapBuffers to block once one swap is already outstanding, giving double-buffering behavior rather than the desired triple-buffering. However, it only has an effect for full-screen windows, and only when not running under a compositor. - If the window is not full-screen, UXA is already only double-buffered. - If full-screen, UXA is usually triple-buffered, but not reliably. - If run under a compositor, either the compositor crashes during my test, or it still appears to be triple-buffered even with this patch. If you want triple-buffering, NAK'ing this patch is clearly not the way to get it, since the driver already doesn't do it reliably. Please merge this patch, which fixes two spec violations that make OML_sync_control unusable; and if you're concerned about uncomposited triple-buffering in UXA, please find a less awful way to get it. Why not just change the default to double buffering? Or fix it correctly? -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] UXA: Wait until a pageflip actually completes to report it.
On Wed, May 21, 2014 at 05:24:18PM +0100, Chris Wilson wrote: On Wed, May 21, 2014 at 08:53:18AM -0700, Jamey Sharp wrote: Please merge this patch, which fixes two spec violations that make OML_sync_control unusable; and if you're concerned about uncomposited triple-buffering in UXA, please find a less awful way to get it. Why not just change the default to double buffering? Or fix it correctly? This patch *is* fixing it correctly. I don't understand the buffer management well enough to implement triple buffering in your driver though, sorry; all I know is that what's there now isn't it. Jamey signature.asc Description: Digital signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC] set up an sync channel between audio and display driver (i.e. ALSA and DRM)
On Wed, 2014-05-21 at 18:05 +0200, Daniel Vetter wrote: On Wed, May 21, 2014 at 5:56 PM, Babu, Ramesh ramesh.b...@intel.com wrote: On Tue, May 20, 2014 at 05:29:07PM +0300, Imre Deak wrote: On Tue, 2014-05-20 at 05:52 +0300, Lin, Mengdong wrote: This RFC is based on previous discussion to set up a generic communication channel between display and audio driver and an internal design of Intel MCG/VPG HDMI audio driver. It's still an initial draft and your advice would be appreciated to improve the design. The basic idea is to create a new avsink module and let both drm and alsa depend on it. This new module provides a framework and APIs for synchronization between the display and audio driver. 1. Display/Audio Client The avsink core provides APIs to create, register and lookup a display/audio client. A specific display driver (eg. i915) or audio driver (eg. HD-Audio driver) can create a client, add some resources objects (shared power wells, display outputs, and audio inputs, register ops) to the client, and then register this client to avisink core. The peer driver can look up a registered client by a name or type, or both. If a client gives a valid peer client name on registration, avsink core will bind the two clients as peer for each other. And we expect a display client and an audio client to be peers for each other in a system. One problem we have at the moment is the order of calling the system suspend/resume handlers of the display driver wrt. that of the audio driver. Since the power well control is part of the display HW block, we need to run the display driver's resume handler first, initialize the HW, and only then let the audio driver's resume handler run. For similar reasons we have to call the audio suspend handler first and only then the display driver resume handler. Currently we solve this using the display driver's late/early suspend/resume hooks, but we'd need a more robust solution. This seems to be a similar issue to the load time ordering problem that you describe later. Having a real device for avsync that would be a child of the display device would solve the ordering issue in both cases. I admit I haven't looked into it if this is feasible, but I would like to see some solution to this as part of the plan. If we are able create and mandate that HDMI display controller is parent and audio is child device, then this wouldn't be an issue and PM frameowrk will ensure parent is suspended last. If there is a scenario where HDMI audio has to active but display has to go to low power, then parent-child device is not optimal. There needs to be a mechanism to turn on/off individual hw blocks within the controller. Our gfx runtime pm code is a _lot_ better than that. We track each power domain individually and enable/disable them only when need. armsoc drivers could do the same or make sure that the avsink device is a child of the right block. Of course if your driver only has binary runtime pm and fires up everything then we have a problem. But imo that's a problem with that driver, not with making avsink real devices as children of something. I would also add that at least in case of Haswell, there is really a hard dependency between the display device and the HDMI audio functionality: The power well required by HDMI is controlled via the PWR_WELL_CTL2 register which is in turn part of the display power domain. This domain is turned off when the display device is in D3 state, so to turn on audio we really have to first put the display device into D0 state. Since the PM framework doesn't provide any way to reorder the initialization of devices, we can only depend on the device parent - child relationship to achieve the above correct init order. --Imre signature.asc Description: This is a digitally signed message part ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/vlv: assert and de-assert sideband reset at boot and resume v3
On Tue, May 20, 2014 at 11:09:03AM -0700, Jesse Barnes wrote:
This is a bit like the CMN reset de-assert we do in DPIO_CTL, except
that it resets the whole common lane section of the PHY. This is
required on machines where the BIOS doesn't do this for us on boot or
resume to properly re-calibrate and get the PHY ready to transmit data.
Without this patch, such machines won't resume correctly much of the time,
with the symptom being a 'port ready' timeout and/or a link training
failure.
v2: extract simpler set_power_well function for use in reset_dpio (Imre)
move to reset_dpio (Daniel Ville)
v3: don't reset if DPIO reset is already de-asserted (Imre)
Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
---
drivers/gpu/drm/i915/intel_display.c | 19 +++
drivers/gpu/drm/i915/intel_drv.h | 3 ++-
drivers/gpu/drm/i915/intel_pm.c | 13 ++---
3 files changed, 31 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index df58afc..bdb4624 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1509,6 +1509,25 @@ static void intel_reset_dpio(struct drm_device *dev)
} else {
/*
+ * If DPIO has already been reset, e.g. by BIOS, just skip all
+ * this.
+ */
+ if (I915_READ(DPIO_CTL) DPIO_CMNRST)
+ return;
+
+ /*
+ * From VLV2A0_DP_eDP_HDMI_DPIO_driver_vbios_notes_11.docx:
+ * Need to assert and de-assert PHY SB reset by gating the
+ * common lane power, then un-gating it.
+ * Simply ungating isn't enough to reset the PHY enough to get
+ * ports and lanes running.
+ */
+ __vlv_set_power_well(dev_priv, PUNIT_POWER_WELL_DPIO_CMN_BC,
+ false);
+ __vlv_set_power_well(dev_priv, PUNIT_POWER_WELL_DPIO_CMN_BC,
+ true);
Hmm. I wonder how the power well hack in intel_uncore_sanitize() ties in
with this. We should definitely rip that out regardless.
Another thing I'm wodering is did the BIOS/hw really power on the
common lane, or did we do that outselves? If the latter, then I wonder
if we simply do that too early. Or more precisely do we need to make
sure the cri clock and/or refclock are enabled before we power on the
common lane?
And third is that do we need to enable the TX wells before the CMN
well? Currently we do the opposite which could also explain why this
CMN well toggle fixes things.
And finally we should make sure the cmnreset assert/deassert happens
corecctly wrt. to the power well transitions.
So I'd really like to see the following measures taken:
- kill power well hack from intel_uncore_sanitize()
- move the DPLL register setup from intel_reset_dpio() into
vlv_set_power_well() like so:
vlv_set_power_well()
{
+ if (power_well_id == PUNIT_POWER_WELL_DPIO_CMN_BC enable) {
+ I915_WRITE(DPLL, ...);
+ POSTING_READ(DPLL);
+ udelay(1); /* 10ns for cmnreset, 0ns for sidereset */
+ }
really enable the power well
}
- reorder vlv_power_wells[] to bring up TX wells before CMN.
Although maybe the DPLL stuff should then be done already before
the TX wells are brought up? But maybe not.
- move the cmnreset assert/deassert into the power well code like so:
vlv_set_power_well()
{
+ if (power_well_id == PUNIT_POWER_WELL_DPIO_CMN_BC !enable)
+ assert cmnreset
do stuff
+ if (power_well_id == PUNIT_POWER_WELL_DPIO_CMN_BC enable)
+ deassert cmnreset
}
If those measures aren't enough, then I think this patch would be OK.
But right now I have this nagging feeling that we may have shot
ourselves in the foot and we're about to pile workarounds on top of our
own bug.
+
+ /*
* From VLV2A0_DP_eDP_DPIO_driver_vbios_notes_10.docx -
* 6. De-assert cmn_reset/side_reset. Same as VLV X0.
* a. GUnit 0x2110 bit[0] set to 1 (def 0)
diff --git a/drivers/gpu/drm/i915/intel_drv.h
b/drivers/gpu/drm/i915/intel_drv.h
index 0ef2777..feb6165 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -966,7 +966,8 @@ void intel_runtime_pm_put(struct drm_i915_private
*dev_priv);
void intel_init_runtime_pm(struct drm_i915_private *dev_priv);
void intel_fini_runtime_pm(struct drm_i915_private *dev_priv);
void ilk_wm_get_hw_state(struct drm_device *dev);
-
+void __vlv_set_power_well(struct drm_i915_private *dev_priv,
+ enum punit_power_well power_well_id, bool enable);
/* intel_sdvo.c */
bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg, bool
is_sdvob);
diff --git a/drivers/gpu/drm/i915/intel_pm.c
Re: [Intel-gfx] [PATCH] drm/i915/vlv: assert and de-assert sideband reset at boot and resume v3
On Wed, 21 May 2014 20:54:03 +0300
Ville Syrjälä ville.syrj...@linux.intel.com wrote:
On Tue, May 20, 2014 at 11:09:03AM -0700, Jesse Barnes wrote:
This is a bit like the CMN reset de-assert we do in DPIO_CTL, except
that it resets the whole common lane section of the PHY. This is
required on machines where the BIOS doesn't do this for us on boot or
resume to properly re-calibrate and get the PHY ready to transmit data.
Without this patch, such machines won't resume correctly much of the time,
with the symptom being a 'port ready' timeout and/or a link training
failure.
v2: extract simpler set_power_well function for use in reset_dpio (Imre)
move to reset_dpio (Daniel Ville)
v3: don't reset if DPIO reset is already de-asserted (Imre)
Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
---
drivers/gpu/drm/i915/intel_display.c | 19 +++
drivers/gpu/drm/i915/intel_drv.h | 3 ++-
drivers/gpu/drm/i915/intel_pm.c | 13 ++---
3 files changed, 31 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index df58afc..bdb4624 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1509,6 +1509,25 @@ static void intel_reset_dpio(struct drm_device *dev)
} else {
/*
+* If DPIO has already been reset, e.g. by BIOS, just skip all
+* this.
+*/
+ if (I915_READ(DPIO_CTL) DPIO_CMNRST)
+ return;
+
+ /*
+* From VLV2A0_DP_eDP_HDMI_DPIO_driver_vbios_notes_11.docx:
+* Need to assert and de-assert PHY SB reset by gating the
+* common lane power, then un-gating it.
+* Simply ungating isn't enough to reset the PHY enough to get
+* ports and lanes running.
+*/
+ __vlv_set_power_well(dev_priv, PUNIT_POWER_WELL_DPIO_CMN_BC,
+false);
+ __vlv_set_power_well(dev_priv, PUNIT_POWER_WELL_DPIO_CMN_BC,
+true);
Hmm. I wonder how the power well hack in intel_uncore_sanitize() ties in
with this. We should definitely rip that out regardless.
Another thing I'm wodering is did the BIOS/hw really power on the
common lane, or did we do that outselves? If the latter, then I wonder
if we simply do that too early. Or more precisely do we need to make
sure the cri clock and/or refclock are enabled before we power on the
common lane?
And third is that do we need to enable the TX wells before the CMN
well? Currently we do the opposite which could also explain why this
CMN well toggle fixes things.
And finally we should make sure the cmnreset assert/deassert happens
corecctly wrt. to the power well transitions.
So I'd really like to see the following measures taken:
- kill power well hack from intel_uncore_sanitize()
- move the DPLL register setup from intel_reset_dpio() into
vlv_set_power_well() like so:
vlv_set_power_well()
{
+ if (power_well_id == PUNIT_POWER_WELL_DPIO_CMN_BC enable) {
+ I915_WRITE(DPLL, ...);
+ POSTING_READ(DPLL);
+ udelay(1); /* 10ns for cmnreset, 0ns for sidereset */
+ }
really enable the power well
}
- reorder vlv_power_wells[] to bring up TX wells before CMN.
Although maybe the DPLL stuff should then be done already before
the TX wells are brought up? But maybe not.
- move the cmnreset assert/deassert into the power well code like so:
vlv_set_power_well()
{
+ if (power_well_id == PUNIT_POWER_WELL_DPIO_CMN_BC !enable)
+ assert cmnreset
do stuff
+ if (power_well_id == PUNIT_POWER_WELL_DPIO_CMN_BC enable)
+ deassert cmnreset
}
If those measures aren't enough, then I think this patch would be OK.
But right now I have this nagging feeling that we may have shot
ourselves in the foot and we're about to pile workarounds on top of our
own bug.
I worry about this too, but on the flip side we only have the
incomplete doc to look at, and these bugs can be really hard to find
and test fixes for (this reset one in particular bit only some
machines on only some boots for example).
So going with something untested like the above may make more sense,
but may also give us a false sense of security wrt this issue.
Can you ping the PHY guys with the above and see what they have to
say? Maybe in the context of the updated programming doc stuff I sent
out? Ideally we could get firm answers, update the docs, then update
the code.
In the meantime, upstream will be broken without a tested fix on these
machines.
Thanks,
--
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
Re: [Intel-gfx] Why the memcpy from a mapped GPU memory is so slow on Intel Bay Trail?
三月！ sunnyma...@qq.com writes: Hello! I'm developing some openCL application with Beignet in Ubuntu 14.04 x64 Desktop upon Bay Trail E3825. And I found that reading data from GPU memory through whatever drm_intel gem_bo_map or drm_intel_gem_bo_get subdata cost about 0.002 ~ 0.003 second to fetch a 7MiB array, which is not quite satisfing. Could anybody help solve this problem? GPUs (except in the case of SNB/IVB/HSW where the CPU is coherent with the GPU other than the GPU's L1/2 caches) are extremely slow to read From because write-combining memory is effectively uncached performance for reads. You can get better streaming read performance using the movntdqa instruction, and you can see an example of code using it in streaming-load-memcpy.c in mesa (though it looks like that code is missing an mfence, which iirc is required). pgp_LYfPOMM9S.pgp Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm/i915: use SNB A step FDI values on A step CPUs
More of an example of where to use the stepping macro than anything
else. I think these early steppings never went outside of Intel.
Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
---
drivers/gpu/drm/i915/intel_display.c | 17 +++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index bdb4624..828bac5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2883,6 +2883,13 @@ static void ironlake_fdi_link_train(struct drm_crtc
*crtc)
}
+static const int snb_a_fdi_train_param[] = {
+ FDI_LINK_TRAIN_400MV_0DB_SNB_A,
+ FDI_LINK_TRAIN_400MV_6DB_SNB_A,
+ FDI_LINK_TRAIN_600MV_3_5DB_SNB_A,
+ FDI_LINK_TRAIN_800MV_0DB_SNB_A,
+};
+
static const int snb_b_fdi_train_param[] = {
FDI_LINK_TRAIN_400MV_0DB_SNB_B,
FDI_LINK_TRAIN_400MV_6DB_SNB_B,
@@ -2898,6 +2905,12 @@ static void gen6_fdi_link_train(struct drm_crtc *crtc)
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
int pipe = intel_crtc-pipe;
u32 reg, temp, i, retry;
+ const int *train_param;
+
+ if (INTEL_STEPPING(dev) == 0)
+ train_param = snb_a_fdi_train_param;
+ else
+ train_param = snb_b_fdi_train_param;
/* Train 1: umask FDI RX Interrupt symbol_lock and bit_lock bit
for train result */
@@ -2943,7 +2956,7 @@ static void gen6_fdi_link_train(struct drm_crtc *crtc)
reg = FDI_TX_CTL(pipe);
temp = I915_READ(reg);
temp = ~FDI_LINK_TRAIN_VOL_EMP_MASK;
- temp |= snb_b_fdi_train_param[i];
+ temp |= train_param[i];
I915_WRITE(reg, temp);
POSTING_READ(reg);
@@ -2996,7 +3009,7 @@ static void gen6_fdi_link_train(struct drm_crtc *crtc)
reg = FDI_TX_CTL(pipe);
temp = I915_READ(reg);
temp = ~FDI_LINK_TRAIN_VOL_EMP_MASK;
- temp |= snb_b_fdi_train_param[i];
+ temp |= train_param[i];
I915_WRITE(reg, temp);
POSTING_READ(reg);
--
1.8.4.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/2] drm/i915: add stepping macro
In some cases to enable or disable features workarounds, we may need
to check the GPU stepping. Add a macro to do that based on caching the
PCI revision ID reg.
Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
---
drivers/gpu/drm/i915/i915_dma.c | 2 ++
drivers/gpu/drm/i915/i915_drv.h | 2 ++
2 files changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index dea455b..9bcbbf2 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1565,6 +1565,8 @@ int i915_driver_load(struct drm_device *dev, unsigned
long flags)
device_info = (struct intel_device_info *)dev_priv-info;
*device_info = *info;
+ pci_read_config_byte(dev-pdev, PCI_REVISION_ID, dev_priv-stepping);
+
spin_lock_init(dev_priv-irq_lock);
spin_lock_init(dev_priv-gpu_error.lock);
spin_lock_init(dev_priv-backlight_lock);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 5d5e57d..22d57b8 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1341,6 +1341,7 @@ struct drm_i915_private {
struct kmem_cache *slab;
const struct intel_device_info info;
+ u8 stepping;
int relative_constants_mode;
@@ -1867,6 +1868,7 @@ struct drm_i915_cmd_table {
};
#define INTEL_INFO(dev)(to_i915(dev)-info)
+#define INTEL_STEPPING(dev) (to_i915(dev)-stepping)
#define IS_I830(dev) ((dev)-pdev-device == 0x3577)
#define IS_845G(dev) ((dev)-pdev-device == 0x2562)
--
1.8.4.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/vlv: assert and de-assert sideband reset at boot and resume v3
On Wed, May 21, 2014 at 11:11:15AM -0700, Jesse Barnes wrote: And to answer more specifically... On Wed, 21 May 2014 20:54:03 +0300 Ville Syrjälä ville.syrj...@linux.intel.com wrote: + __vlv_set_power_well(dev_priv, PUNIT_POWER_WELL_DPIO_CMN_BC, + false); + __vlv_set_power_well(dev_priv, PUNIT_POWER_WELL_DPIO_CMN_BC, + true); Hmm. I wonder how the power well hack in intel_uncore_sanitize() ties in with this. We should definitely rip that out regardless. Yeah we can rip that out. That's just an ungate, and it assumes the BIOS has already done the reset toggle for us. Well I'm assumign the system may boot/resume with the wells powered down. And we definitely don't have the cri/ref clocks set up at this point. So if they're needed to complete the resets the PHY may get stuck or something. Also it just ungates everything at once, if the wells need some ordering in bringup we have just messed things up here. Another thing I'm wodering is did the BIOS/hw really power on the common lane, or did we do that outselves? If the latter, then I wonder if we simply do that too early. Or more precisely do we need to make sure the cri clock and/or refclock are enabled before we power on the common lane? Depends on the platform. It looks like the right thing happens at boot time on most machines (i.e. the BIOS does a full toggle which causes a reset), but on suspend/resume it's up to us. And of course on machines without video init at boot time, we need to do it ourselves as well. I don't think the cri or refclk is required at this point, at least not if the docs we have are correct (the PHY reset is supposed to be the very first thing). The timing diagrams do show some kind of relationship between the cri/ref clocks and the reset signals getting deasserted. And third is that do we need to enable the TX wells before the CMN well? Currently we do the opposite which could also explain why this CMN well toggle fixes things. I don't think that matters, but we should ask the PHY guys. The lack of symmetry between the gate and ungate bothers me too. My theory here is that the cmn and/or side reset needs to propagate to the data lanes to propery initialize them. If the lanes are powered down when the reset occurs things go bad. And looks like the Dynamic Power Gating section in the phy cspec pretty much says as much. That is you need to fully reset the entire phy if you want to ungate even one of the blocks. So based on that I think pretty much everything I said is needed to make stuff work correctly. Also I think currently we power gate the TX blocks whenever the port gets disabled. If we want to keep doing that, we need to adjust valleyview_modeset_global_resources() to fully reset the phy whenever previously power down wells need to be brought up. The alternative would be to model the entire PHY as a single power well, which would definitely be the simpler option. -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915: add stepping macro
On Wed, May 21, 2014 at 11:42:25AM -0700, Jesse Barnes wrote: In some cases to enable or disable features workarounds, we may need to check the GPU stepping. Add a macro to do that based on caching the PCI revision ID reg. What about just using dev-pdev-revision like we already do? -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/3] drm/i915: don't read HSW_AUD_PIN_ELD_CP_VLD when the power well is off
From: Paulo Zanoni paulo.r.zan...@intel.com
Because this will trigger Unclaimed register messages. All I need to
reproduce this problem is to boot my HSW machine with eDP+HDMI
connected.
Regression introduced by:
commit 9ed109a7b445e3f073d8ea72f888ec80c0532465
Author: Daniel Vetter daniel.vet...@ffwll.ch
Date: Thu Apr 24 23:54:52 2014 +0200
drm/i915: Track has_audio in the pipe config
Cc: Daniel Vetter daniel.vet...@ffwll.ch
Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
---
drivers/gpu/drm/i915/intel_ddi.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 271ce19..355f569 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1560,9 +1560,11 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
break;
}
- temp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
- if (temp (AUDIO_OUTPUT_ENABLE_A (intel_crtc-pipe * 4)))
- pipe_config-has_audio = true;
+ if (intel_display_power_enabled(dev_priv, POWER_DOMAIN_AUDIO)) {
+ temp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
+ if (temp (AUDIO_OUTPUT_ENABLE_A (intel_crtc-pipe * 4)))
+ pipe_config-has_audio = true;
+ }
if (encoder-type == INTEL_OUTPUT_EDP dev_priv-vbt.edp_bpp
pipe_config-pipe_bpp dev_priv-vbt.edp_bpp) {
--
1.9.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/3] drm/i915: don't read HSW_AUD_PIN_ELD_CP_VLD wehn the powe rwell is off (2)
From: Paulo Zanoni paulo.r.zan...@intel.com
Because this will trigger Unclaimed register messages. This can be
reproduced by running the pm_rpm tests of the test suite. It can
probably be reproduced by any of the tests that do modesets too.
Regression introduced by:
commit acfa75b02e72bad7c93564ac379712e29c001432
Author: Daniel Vetter daniel.vet...@ffwll.ch
Date: Thu Apr 24 23:54:51 2014 +0200
drm/i915: Simplify audio handling on DDI ports
Cc: Daniel Vetter daniel.vet...@ffwll.ch
Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
---
drivers/gpu/drm/i915/intel_ddi.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 355f569..54a3bb7 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1372,10 +1372,12 @@ static void intel_disable_ddi(struct intel_encoder
*intel_encoder)
struct drm_i915_private *dev_priv = dev-dev_private;
uint32_t tmp;
- tmp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
- tmp = ~((AUDIO_OUTPUT_ENABLE_A | AUDIO_ELD_VALID_A)
-(pipe * 4));
- I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, tmp);
+ if (intel_display_power_enabled(dev_priv, POWER_DOMAIN_AUDIO)) {
+ tmp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
+ tmp = ~((AUDIO_OUTPUT_ENABLE_A | AUDIO_ELD_VALID_A)
+(pipe * 4));
+ I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, tmp);
+ }
if (type == INTEL_OUTPUT_EDP) {
struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
--
1.9.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/3] drm/i915: reorganize the unclaimed register detection code
From: Paulo Zanoni paulo.r.zan...@intel.com
The current code only runs when we do an I915_WRITE operation. It
checks if the unclaimed register flag is set before we do the
operation, and then it checks it again after we do the operation. This
double check allows us to find out if the I915_WRITE operation in
question is the bad one, or if some previous code is the bad one. When
it finds a problem, our code uses DRM_ERROR to signal it.
The good thing about the current code is that it detects the problem,
so at least we can know we did something wrong. The problem is that
even though we find the problem, we don't really have much information
to actually debug it. So whenever I see one of these DRM_ERROR
messages on my systems, the first thing I do is apply a patch to
change the DRM_ERROR to a WARN and also check for unclaimed registers
on I915_READ operations. This local patch makes things even slower,
but it usually helps a lot in finding the bad code.
The first point here is that since the current code is only useful to
detect whether we have a problem or not, but it is not really good to
find the cause of the problem, I don't think we should be checking
both before and after every I915_WRITE operation: just doing the check
once should be enough for us to quickly detect problems. With this
change, the code that runs by default for every single user will only
do 1 read operation for every single I915_WRITE, instead of 2. This
patch does this change.
The second point is that the local patch I have should be upstream,
but since it makes things slower it should be disabled by default. So
I added the i915.mmio_debug option to enable it.
So after this patch, this is what will happen:
- By default, we will try to detect unclaimed registers once after
every I915_WRITE operation. Previously we tried twice for every
I915_WRITE.
- When we find an unclaimed register we will still print a DRM_ERROR
message, but we will now tell the user to try again with
i915.mmio_debug=1.
- When we use i915.mmio_debug=1 we will try to find unclaimed
registers both before and after every I915_READ and I915_WRITE
operation, and we will print stack traces in case we find them.
This should really help locating the exact point of the bad code
(or at least finding out that i915.ko is not the problem).
This commit also opens space for really-slow register debugging
operations on other platforms. In theory we can now add lots and lots
of debug code behind i915.mmio_debug, enable this option on our tests,
and catch more problems.
Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_params.c | 6
drivers/gpu/drm/i915/intel_uncore.c | 69 +
3 files changed, 69 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8daf4f7..60cf78f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2045,6 +2045,7 @@ struct i915_params {
bool reset;
bool disable_display;
bool disable_vtd_wa;
+ bool mmio_debug;
};
extern struct i915_params i915 __read_mostly;
diff --git a/drivers/gpu/drm/i915/i915_params.c
b/drivers/gpu/drm/i915/i915_params.c
index d05a2af..d9fa00f 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -48,6 +48,7 @@ struct i915_params i915 __read_mostly = {
.disable_display = 0,
.enable_cmd_parser = 1,
.disable_vtd_wa = 0,
+ .mmio_debug = 0,
};
module_param_named(modeset, i915.modeset, int, 0400);
@@ -156,3 +157,8 @@ MODULE_PARM_DESC(disable_vtd_wa, Disable all VT-d
workarounds (default: false)
module_param_named(enable_cmd_parser, i915.enable_cmd_parser, int, 0600);
MODULE_PARM_DESC(enable_cmd_parser,
Enable command parsing (1=enabled [default], 0=disabled));
+
+module_param_named(mmio_debug, i915.mmio_debug, bool, 0600);
+MODULE_PARM_DESC(disable_vtd_wa,
+ Enable the MMIO debug code (default: false). This may negatively
+ affect performance.);
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index cd0d6e2..b72f5f5 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -506,21 +506,73 @@ ilk_dummy_write(struct drm_i915_private *dev_priv)
__raw_i915_write32(dev_priv, MI_MODE, 0);
}
+/**
+ * hsw_unclaimed_reg_debug - tries to find unclaimed registers
+ * @dev_priv: device private data
+ * @reg: register we're about to touch or just have touched
+ * @read: are we reading or writing a register now?
+ * @before: are we running this before or after we touch the register?
+ *
+ * This function tries to detect unclaimed registers and provide as much
+ * information as possible. It will only do something if the i915.mmio_debug
+ * option is enabled.
+ *
+ * If we detect an unclaimed
Re: [Intel-gfx] [PATCH 1/3] drm/i915: don't read HSW_AUD_PIN_ELD_CP_VLD when the power well is off
On Wed, May 21, 2014 at 04:23:20PM -0300, Paulo Zanoni wrote:
From: Paulo Zanoni paulo.r.zan...@intel.com
Because this will trigger Unclaimed register messages. All I need to
reproduce this problem is to boot my HSW machine with eDP+HDMI
connected.
Regression introduced by:
commit 9ed109a7b445e3f073d8ea72f888ec80c0532465
Author: Daniel Vetter daniel.vet...@ffwll.ch
Date: Thu Apr 24 23:54:52 2014 +0200
drm/i915: Track has_audio in the pipe config
Cc: Daniel Vetter daniel.vet...@ffwll.ch
Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
Argh, I always forget about these. Thanks for tracking this down, patch
merged.
-Daniel
---
drivers/gpu/drm/i915/intel_ddi.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c
b/drivers/gpu/drm/i915/intel_ddi.c
index 271ce19..355f569 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1560,9 +1560,11 @@ void intel_ddi_get_config(struct intel_encoder
*encoder,
break;
}
- temp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
- if (temp (AUDIO_OUTPUT_ENABLE_A (intel_crtc-pipe * 4)))
- pipe_config-has_audio = true;
+ if (intel_display_power_enabled(dev_priv, POWER_DOMAIN_AUDIO)) {
+ temp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
+ if (temp (AUDIO_OUTPUT_ENABLE_A (intel_crtc-pipe * 4)))
+ pipe_config-has_audio = true;
+ }
if (encoder-type == INTEL_OUTPUT_EDP dev_priv-vbt.edp_bpp
pipe_config-pipe_bpp dev_priv-vbt.edp_bpp) {
--
1.9.0
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915: add stepping macro
On Wed, 21 May 2014 19:50:53 +0100 Chris Wilson ch...@chris-wilson.co.uk wrote: On Wed, May 21, 2014 at 11:42:25AM -0700, Jesse Barnes wrote: In some cases to enable or disable features workarounds, we may need to check the GPU stepping. Add a macro to do that based on caching the PCI revision ID reg. What about just using dev-pdev-revision like we already do? Oh missed that, was looking for stepping comments and didn't see it. Yeah that's fine. -- Jesse Barnes, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/3] drm/i915: grab the audio power domain when enabling audio on HSW+
From: Paulo Zanoni paulo.r.zan...@intel.com
With the current code, we unconditionally touch
HSW_AUD_PIN_ELD_CP_VLD, which means we can touch it when the power
well is off, and that will trigger an Unclaimed register message.
Just adding the intel_crtc-config.has_audio should already avoid the
unclaimed register messsages, but since we actually need the power
well to make the Audio code work, it makes sense to also grab the
audio power domain reference, and release it when it's not needed
anymore.
I used IGT's pm_rpm to reproduce this bug, but it can probably be
reproduced on other tests that do modesets. I'm using a machine with
eDP+HDMI connected.
Regression introduced by:
commit acfa75b02e72bad7c93564ac379712e29c001432
Author: Daniel Vetter daniel.vet...@ffwll.ch
Date: Thu Apr 24 23:54:51 2014 +0200
drm/i915: Simplify audio handling on DDI ports
Credits to Daniel for suggesting this implementation.
Cc: Daniel Vetter daniel.vet...@ffwll.ch
Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
---
drivers/gpu/drm/i915/intel_ddi.c | 14 ++
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 355f569..b17b9c7 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1355,6 +1355,7 @@ static void intel_enable_ddi(struct intel_encoder
*intel_encoder)
}
if (intel_crtc-config.has_audio) {
+ intel_display_power_get(dev_priv, POWER_DOMAIN_AUDIO);
tmp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
tmp |= ((AUDIO_OUTPUT_ENABLE_A | AUDIO_ELD_VALID_A) (pipe *
4));
I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, tmp);
@@ -1372,10 +1373,15 @@ static void intel_disable_ddi(struct intel_encoder
*intel_encoder)
struct drm_i915_private *dev_priv = dev-dev_private;
uint32_t tmp;
- tmp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
- tmp = ~((AUDIO_OUTPUT_ENABLE_A | AUDIO_ELD_VALID_A)
-(pipe * 4));
- I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, tmp);
+ /* We can't touch HSW_AUD_PIN_ELD_CP_VLD uncionditionally because this
+* register is part of the power well on Haswell. */
+ if (intel_crtc-config.has_audio) {
+ tmp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
+ tmp = ~((AUDIO_OUTPUT_ENABLE_A | AUDIO_ELD_VALID_A)
+(pipe * 4));
+ I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, tmp);
+ intel_display_power_put(dev_priv, POWER_DOMAIN_AUDIO);
+ }
if (type == INTEL_OUTPUT_EDP) {
struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
--
1.9.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/3] drm/i915: grab the audio power domain when enabling audio on HSW+
On Wed, May 21, 2014 at 05:29:31PM -0300, Paulo Zanoni wrote:
From: Paulo Zanoni paulo.r.zan...@intel.com
With the current code, we unconditionally touch
HSW_AUD_PIN_ELD_CP_VLD, which means we can touch it when the power
well is off, and that will trigger an Unclaimed register message.
Just adding the intel_crtc-config.has_audio should already avoid the
unclaimed register messsages, but since we actually need the power
well to make the Audio code work, it makes sense to also grab the
audio power domain reference, and release it when it's not needed
anymore.
I used IGT's pm_rpm to reproduce this bug, but it can probably be
reproduced on other tests that do modesets. I'm using a machine with
eDP+HDMI connected.
Regression introduced by:
commit acfa75b02e72bad7c93564ac379712e29c001432
Author: Daniel Vetter daniel.vet...@ffwll.ch
Date: Thu Apr 24 23:54:51 2014 +0200
drm/i915: Simplify audio handling on DDI ports
Credits to Daniel for suggesting this implementation.
Cc: Daniel Vetter daniel.vet...@ffwll.ch
Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
Queued for -next, thanks for the patch.
-Daniel
---
drivers/gpu/drm/i915/intel_ddi.c | 14 ++
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c
b/drivers/gpu/drm/i915/intel_ddi.c
index 355f569..b17b9c7 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1355,6 +1355,7 @@ static void intel_enable_ddi(struct intel_encoder
*intel_encoder)
}
if (intel_crtc-config.has_audio) {
+ intel_display_power_get(dev_priv, POWER_DOMAIN_AUDIO);
tmp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
tmp |= ((AUDIO_OUTPUT_ENABLE_A | AUDIO_ELD_VALID_A) (pipe *
4));
I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, tmp);
@@ -1372,10 +1373,15 @@ static void intel_disable_ddi(struct intel_encoder
*intel_encoder)
struct drm_i915_private *dev_priv = dev-dev_private;
uint32_t tmp;
- tmp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
- tmp = ~((AUDIO_OUTPUT_ENABLE_A | AUDIO_ELD_VALID_A)
- (pipe * 4));
- I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, tmp);
+ /* We can't touch HSW_AUD_PIN_ELD_CP_VLD uncionditionally because this
+ * register is part of the power well on Haswell. */
+ if (intel_crtc-config.has_audio) {
+ tmp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
+ tmp = ~((AUDIO_OUTPUT_ENABLE_A | AUDIO_ELD_VALID_A)
+ (pipe * 4));
+ I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, tmp);
+ intel_display_power_put(dev_priv, POWER_DOMAIN_AUDIO);
+ }
if (type == INTEL_OUTPUT_EDP) {
struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
--
1.9.0
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 4/5] drm/i915: Wait for pending page flips before enabling/disabling the primary plane
Reviewed-by Rodrigo Vivi rodrigo.v...@gmail.com
On Wed, May 21, 2014 at 4:04 AM, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä ville.syrj...@linux.intel.com
We have to write to the primary plane base address registrer when we
enable/disable the primary plane in response to sprite coverage. Those
writes will cause the flip counter to increment which could interfere
with the detection of CS flip completion. We could end up completing
CS flips before the CS has even executed the commands from the ring.
To avoid such issues, wait for CS flips to finish before we toggle the
primary plane on/off.
v2: Rebased due to atomic sprite update changes
Testcase: igt/kms_mmio_vs_cs_flip/setplane_vs_cs_flip
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/intel_display.c | 2 +-
drivers/gpu/drm/i915/intel_drv.h | 2 ++
drivers/gpu/drm/i915/intel_sprite.c | 3 +++
3 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 8ca8b2f..849bc71 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3291,7 +3291,7 @@ bool intel_has_pending_fb_unpin(struct drm_device
*dev)
return false;
}
-static void intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc)
+void intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc)
{
struct drm_device *dev = crtc-dev;
struct drm_i915_private *dev_priv = dev-dev_private;
diff --git a/drivers/gpu/drm/i915/intel_drv.h
b/drivers/gpu/drm/i915/intel_drv.h
index a78eb75..b9b13eb 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -801,6 +801,8 @@ int valleyview_get_vco(struct drm_i915_private
*dev_priv);
void intel_mode_from_pipe_config(struct drm_display_mode *mode,
struct intel_crtc_config *pipe_config);
int intel_format_to_fourcc(int format);
+void intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc);
+
/* intel_dp.c */
void intel_dp_init(struct drm_device *dev, int output_reg, enum port
port);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c
b/drivers/gpu/drm/i915/intel_sprite.c
index 7780f6c..d6acd6b 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1018,6 +1018,9 @@ intel_update_plane(struct drm_plane *plane, struct
drm_crtc *crtc,
intel_crtc-primary_enabled = primary_enabled;
+ if (primary_was_enabled != primary_enabled)
+ intel_crtc_wait_for_pending_flips(crtc);
+
if (primary_was_enabled !primary_enabled)
intel_pre_disable_primary(crtc);
--
1.8.5.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [alsa-devel] [RFC] set up an sync channel between audio and display driver (i.e. ALSA and DRM)
This RFC is based on previous discussion to set up a generic communication channel between display and audio driver and an internal design of Intel MCG/VPG HDMI audio driver. It's still an initial draft and your advice would be appreciated to improve the design. The basic idea is to create a new avsink module and let both drm and alsa depend on it. 1. Display/Audio Client The avsink core provides APIs to create, register and lookup a display/audio client. For HD-audio HDMI, both controller and codec drivers would need the avsink access. So, both drivers will register the own client? http://nvidia.custhelp.com/app/answers/detail/a_id/2544/~/my-nvidia-graphics-card-came-with-an-internal-spdif-pass-through-audio-cable-to http://www.intel.com/support/motherboards/desktop/sb/CS-032871.htm Does it mean that those grpahic card HDMI which use motherboard internal spdif connector will not be supported any more when graphic card have no way to communicate with audio driver ? https://git.kernel.org/cgit/linux/kernel/git/tiwai/sound.git/commit/sound/pci/hda/hda_auto_parser.c?id=3f25dcf691ebf45924a34b9aaedec78e5a255798 Should alsa regard these kind of digital device as HDMI or SPDIF ? ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] linux-next: manual merge of the drm-intel tree with the drm-intel-fixes tree
Hi all,
Today's linux-next merge of the drm-intel tree got a conflict in
drivers/gpu/drm/i915/i915_drv.h between commit f93e94efebbe (drm/i915:
Fix dynamic allocation of physical handles) from the drm-intel-fixes
tree and commit 5cc9ed4b9a7a (drm/i915: Introduce mapping of user
pages into video memory (userptr) ioctl) from the drm-intel tree.
I fixed it up (see below) and can carry the fix as necessary (no action
is required).
--
Cheers,
Stephen Rothwells...@canb.auug.org.au
diff --cc drivers/gpu/drm/i915/i915_drv.h
index d6dc54aa123a,d2da390b6b9f..
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@@ -1609,9 -1712,21 +1697,21 @@@ struct drm_i915_gem_object
struct drm_file *pin_filp;
/** for phy allocated objects */
- struct drm_i915_gem_phys_object *phys_obj;
+ drm_dma_handle_t *phys_handle;
- };
+ union {
+ struct i915_gem_userptr {
+ uintptr_t ptr;
+ unsigned read_only :1;
+ unsigned workers :4;
+ #define I915_GEM_USERPTR_MAX_WORKERS 15
+
+ struct mm_struct *mm;
+ struct i915_mmu_object *mn;
+ struct work_struct *work;
+ } userptr;
+ };
+ };
#define to_intel_bo(x) container_of(x, struct drm_i915_gem_object, base)
/**
signature.asc
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
