Re: [Intel-gfx] [PATCH] Lower threshold for pixel doubling.
Any comments?
Without this, plugging one of the older Chromebook models into a Dell U3011
monitor produces a garbled display at the default 2048x1280 resolution.
The original threshold was apparently fairly arbitrary:
http://cgit.freedesktop.org/~anholt/xf86-video-intel/commit/?id=8fcf9a81179ee8577ddab5e904c58fbfd14cf59c
.
Stuart
On Mon, May 20, 2013 at 11:15 AM, Stuart Abercrombie <
sabercrom...@chromium.org> wrote:
> 90% of core speed (=180MHz dot clock) is too high for 2048x1280 to get
> pixel doubling on Pineview, which it needs to avoid underruns, so
> lower this to 85%.
>
> Signed-off-by: Stuart Abercrombie
> ---
> drivers/gpu/drm/i915/intel_display.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index efe8299..9c924e9 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4564,14 +4564,14 @@ static void i9xx_set_pipeconf(struct intel_crtc
> *intel_crtc)
> pipeconf = I915_READ(PIPECONF(intel_crtc->pipe));
>
> if (intel_crtc->pipe == 0 && INTEL_INFO(dev)->gen < 4) {
> - /* Enable pixel doubling when the dot clock is > 90% of
> the (display)
> + /* Enable pixel doubling when the dot clock is > 85% of
> the (display)
> * core speed.
> *
> * XXX: No double-wide on 915GM pipe B. Is that the only
> reason for the
> * pipe == 0 check?
> */
> if (intel_crtc->config.requested_mode.clock >
> - dev_priv->display.get_display_clock_speed(dev) * 9 /
> 10)
> + dev_priv->display.get_display_clock_speed(dev) * 17 /
> 20)
> pipeconf |= PIPECONF_DOUBLE_WIDE;
> else
> pipeconf &= ~PIPECONF_DOUBLE_WIDE;
> --
> 1.8.2.1
>
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] Lower threshold for pixel doubling.
Without this change I saw PIPE_FIFO_UNDERRUN_STATUS set in PIPEASTAT, which
I took to indicate an underrun problem.
Here's what I found with other modes on this monitor:
1920x1200 works with pixel doubling enabled. Pixel clock 193.2 MHz.
2048x1152 works with pixel doubling enabled. Pixel clock 198.0 MHz.
1600x1200 works with pixel doubling disabled. Pixel clock 162.0 MHz.
2048x1280 fails with pixel doubling disabled. PIxel clock 174.2 MHz..
Based on this, it seems the threshold needs to be be between 162.0MHz and
174.2MHz, whereas currently it's at 180MHz. The change puts it at 170MHz.
Stuart
On Wed, May 29, 2013 at 8:22 AM, Daniel Vetter wrote:
> On Tue, May 28, 2013 at 10:39:07AM -0700, Stuart Abercrombie wrote:
> > Any comments?
> >
> > Without this, plugging one of the older Chromebook models into a Dell
> U3011
> > monitor produces a garbled display at the default 2048x1280 resolution.
> >
> > The original threshold was apparently fairly arbitrary:
> >
> >
> http://cgit.freedesktop.org/~anholt/xf86-video-intel/commit/?id=8fcf9a81179ee8577ddab5e904c58fbfd14cf59c
>
> Do you see any pipe underruns without this patch? There are some not-yet
> implemented tricks we should be pulling around re-splitting DSP_ARB fifo
> entries, which tend to totally kill high-res modes.
> -Daniel
>
> > .
> >
> > Stuart
> >
> >
> > On Mon, May 20, 2013 at 11:15 AM, Stuart Abercrombie <
> > sabercrom...@chromium.org> wrote:
> >
> > > 90% of core speed (=180MHz dot clock) is too high for 2048x1280 to get
> > > pixel doubling on Pineview, which it needs to avoid underruns, so
> > > lower this to 85%.
> > >
> > > Signed-off-by: Stuart Abercrombie
> > > ---
> > > drivers/gpu/drm/i915/intel_display.c | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > > b/drivers/gpu/drm/i915/intel_display.c
> > > index efe8299..9c924e9 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -4564,14 +4564,14 @@ static void i9xx_set_pipeconf(struct intel_crtc
> > > *intel_crtc)
> > > pipeconf = I915_READ(PIPECONF(intel_crtc->pipe));
> > >
> > > if (intel_crtc->pipe == 0 && INTEL_INFO(dev)->gen < 4) {
> > > - /* Enable pixel doubling when the dot clock is > 90% of
> > > the (display)
> > > + /* Enable pixel doubling when the dot clock is > 85% of
> > > the (display)
> > > * core speed.
> > > *
> > > * XXX: No double-wide on 915GM pipe B. Is that the
> only
> > > reason for the
> > > * pipe == 0 check?
> > > */
> > > if (intel_crtc->config.requested_mode.clock >
> > > - dev_priv->display.get_display_clock_speed(dev) * 9
> /
> > > 10)
> > > + dev_priv->display.get_display_clock_speed(dev) *
> 17 /
> > > 20)
> > > pipeconf |= PIPECONF_DOUBLE_WIDE;
> > > else
> > > pipeconf &= ~PIPECONF_DOUBLE_WIDE;
> > > --
> > > 1.8.2.1
> > >
> > >
>
> > ___
> > 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] [PATCH] Avoid i915 flip unpin/HPD event handler deadlock.
Both of these were taking the mode_config mutex but executed from the
same work queue. If intel_crtc_page_flip happened to flush a work queue
containing an HPD event handler work item, deadlock resulted, since the
mutex required by the HPD handler was taken before the flush. Instead
use a separate work queue for the flip unpin work.
Signed-off-by: sabercrom...@chromium.org
Reviewed-by: marc...@chromium.org
---
drivers/gpu/drm/i915/i915_dma.c | 21 -
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/intel_display.c | 4 ++--
3 files changed, 23 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 4f129bb..9215360 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1558,6 +1558,22 @@ int i915_driver_load(struct drm_device *dev, unsigned
long flags)
goto out_mtrrfree;
}
+ /* intel_crtc_page_flip runs with the mode_config mutex having been
+* taken in the DRM layer. It synchronously waits for pending unpin
+* work items while holding this mutex. Therefore this queue cannot
+* contain work items that take this mutex, such as HPD event
+* handling, or we deadlock. There is also no reason for flipping to
+* wait on such events. Therefore put flip unpinning in its own
+* work queue.
+*/
+ dev_priv->flip_unpin_wq = alloc_ordered_workqueue("i915", 0);
+ if (dev_priv->flip_unpin_wq == NULL) {
+ DRM_ERROR("Failed to create flip unpin workqueue.\n");
+ destroy_workqueue(dev_priv->wq);
+ ret = -ENOMEM;
+ goto out_mtrrfree;
+ }
+
/* This must be called before any calls to HAS_PCH_* */
intel_detect_pch(dev);
@@ -1628,6 +1644,7 @@ out_gem_unload:
intel_teardown_gmbus(dev);
intel_teardown_mchbar(dev);
destroy_workqueue(dev_priv->wq);
+ destroy_workqueue(dev_priv->flip_unpin_wq);
out_mtrrfree:
if (dev_priv->mm.gtt_mtrr >= 0) {
mtrr_del(dev_priv->mm.gtt_mtrr,
@@ -1709,7 +1726,8 @@ int i915_driver_unload(struct drm_device *dev)
intel_opregion_fini(dev);
if (drm_core_check_feature(dev, DRIVER_MODESET)) {
- /* Flush any outstanding unpin_work. */
+ /* Flush any outstanding unpin, HPD, etc. work. */
+ flush_workqueue(dev_priv->flip_unpin_wq);
flush_workqueue(dev_priv->wq);
mutex_lock(&dev->struct_mutex);
@@ -1734,6 +1752,7 @@ int i915_driver_unload(struct drm_device *dev)
intel_teardown_mchbar(dev);
destroy_workqueue(dev_priv->wq);
+ destroy_workqueue(dev_priv->flip_unpin_wq);
pci_dev_put(dev_priv->bridge_dev);
kfree(dev->dev_private);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 4130e6d..7eb4619 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -766,6 +766,7 @@ typedef struct drm_i915_private {
struct work_struct error_work;
struct completion error_completion;
struct workqueue_struct *wq;
+ struct workqueue_struct *flip_unpin_wq;
/* Display functions */
struct drm_i915_display_funcs display;
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index e204913..acd07dd 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6937,7 +6937,7 @@ static void do_intel_finish_page_flip(struct drm_device
*dev,
&obj->pending_flip.counter);
wake_up(&dev_priv->pending_flip_queue);
- queue_work(dev_priv->wq, &work->work);
+ queue_work(dev_priv->flip_unpin_wq, &work->work);
trace_i915_flip_complete(intel_crtc->plane, work->pending_flip_obj);
}
@@ -7305,7 +7305,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
spin_unlock_irqrestore(&dev->event_lock, flags);
if (atomic_read(&intel_crtc->unpin_work_count) >= 2)
- flush_workqueue(dev_priv->wq);
+ flush_workqueue(dev_priv->flip_unpin_wq);
ret = i915_mutex_lock_interruptible(dev);
if (ret)
--
1.8.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] Lower threshold for pixel doubling.
90% of core speed (=180MHz dot clock) is too high for 2048x1280 to get
pixel doubling on Pineview, which it needs to avoid underruns, so
lower this to 85%.
Signed-off-by: Stuart Abercrombie
---
drivers/gpu/drm/i915/intel_display.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index efe8299..9c924e9 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4564,14 +4564,14 @@ static void i9xx_set_pipeconf(struct intel_crtc
*intel_crtc)
pipeconf = I915_READ(PIPECONF(intel_crtc->pipe));
if (intel_crtc->pipe == 0 && INTEL_INFO(dev)->gen < 4) {
- /* Enable pixel doubling when the dot clock is > 90% of the
(display)
+ /* Enable pixel doubling when the dot clock is > 85% of the
(display)
* core speed.
*
* XXX: No double-wide on 915GM pipe B. Is that the only reason
for the
* pipe == 0 check?
*/
if (intel_crtc->config.requested_mode.clock >
- dev_priv->display.get_display_clock_speed(dev) * 9 / 10)
+ dev_priv->display.get_display_clock_speed(dev) * 17 / 20)
pipeconf |= PIPECONF_DOUBLE_WIDE;
else
pipeconf &= ~PIPECONF_DOUBLE_WIDE;
--
1.8.2.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] Poll for HDMI disconnects.
Following a hotplug interrupt the driver uses a successful EDID read to
indicate HDMI sink presence.
This leads to missing HDMI cable unplug events because the DDC lines can remain
up, allowing an EDID read to complete, well after the HPD line goes down during
unplugging
Since it is only the disconnect case that suffers from unplug ordering problems
with the DDC lines, restrict polling to that. Otherwise we waste power.
---
drivers/gpu/drm/drm_crtc_helper.c |8 ++--
drivers/gpu/drm/i915/intel_hdmi.c |2 +-
include/drm/drm_crtc.h|2 ++
3 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc_helper.c
b/drivers/gpu/drm/drm_crtc_helper.c
index 3252e70..b38ea4f 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -938,8 +938,12 @@ static void output_poll_execute(struct work_struct *work)
if (!connector->polled)
continue;
- else if (connector->polled & (DRM_CONNECTOR_POLL_CONNECT |
DRM_CONNECTOR_POLL_DISCONNECT))
- repoll = true;
+ if (connector->polled & (DRM_CONNECTOR_POLL_CONNECT |
DRM_CONNECTOR_POLL_DISCONNECT)) {
+ if (connector->polled &
DRM_CONNECTOR_POLL_DISCONNECT_ONLY)
+ repoll = (connector->status ==
connector_status_connected);
+ else
+ repoll = true;
+ }
old_status = connector->status;
/* if we are connected and don't want to poll for disconnect
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
b/drivers/gpu/drm/i915/intel_hdmi.c
index 229897f..246e8f4 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -964,7 +964,7 @@ void intel_hdmi_init(struct drm_device *dev, int sdvox_reg,
enum port port)
intel_encoder->type = INTEL_OUTPUT_HDMI;
- connector->polled = DRM_CONNECTOR_POLL_HPD;
+ connector->polled = DRM_CONNECTOR_POLL_DISCONNECT |
DRM_CONNECTOR_POLL_DISCONNECT_ONLY;
connector->interlace_allowed = 1;
connector->doublescan_allowed = 0;
intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 316ce64..a60abb5 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -503,6 +503,8 @@ enum drm_connector_force {
/* can cleanly poll for disconnections without flickering the screen */
/* DACs should rarely do this without a lot of testing */
#define DRM_CONNECTOR_POLL_DISCONNECT (1 << 2)
+/* Only poll for disconnections. */
+#define DRM_CONNECTOR_POLL_DISCONNECT_ONLY (1 << 3)
#define MAX_ELD_BYTES 128
--
1.7.7.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] Poll for HDMI disconnects.
> Nope, the real fix is to simply check the status of the hpd line before > trying the edid read. We already have that for g4x class chips, check > g4x_hdmi_connected. We'd need to add similar checks for all other > platforms. > -Daniel That would be preferable. Unfortunately we did not find a way to check the HPD line status. See https://bugs.freedesktop.org/show_bug.cgi?id=55372. Where is this status available to be read? Stuart ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] Poll for HDMI disconnects.
> In the hotplug register, like on g4x. But that moved to to PCH_IIR on pch > platforms. I plan to rework the entire hotplug handling for 3.8, hence why > I haven't bothered to wire this up yet. You are saying that the HPD line state is available in the register called SDEIIR in the code? The documentation only describes pulse detection bits in this register, not anything directly reflecting the line state. The same is true of PCH_PORT_HOTPLUG/SHOTPLUG_CTL. Testing did not show these registers, or others in the same range, reflecting the HPD line state. Hence my question. Stuart ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] Poll for HDMI disconnects.
> My docs here say that the SDE_ISR reg contains what we want - high > level irq bit when the hpd line is enabled. I admit, I haven't tested > this ... I'm looking at 2.1.1 in this http://intellinuxgraphics.org/documentation/SNB/IHD_OS_Vol3_Part3.pdf. All it has relating to hotplug in SDEIIR are D, C and B versions of this: "The ISR is an active high level representing the DIgital Port B hotplug line when the Digital Port B hotplug detect input is enabled. The unmasked IIR is set on either a short or long pulse detection status in the Digital Port Hot Plug Control Register." The SHOTPLUG_CTL description in 2.1.6 says DP_B_HPD_Status "reflects the hot plug detect status on the digital port B... When either a long or short pulse is detected, one of these bits will set. These bits are ORed together to go in the main ISR hotplug register bit." Based on this description, and observed behavior, it looks as if these registers only tell you about pulses/interrupt sources, not the line state. Am I missing something? Stuart ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] Poll for HDMI disconnects.
> I've tried to be slightly lazy than in my previous mail and quickly > tested this on my snb here: Bit 23 in SDEISR (0xc4000) is set when the > cable is plugged in, and cleared when nothing is plugged in. Afaict it > works as advertised. Note tha the SDE irq definitions nicely > differentiates between "active high pulse" type interrupts and "active > high level" interrupts. The hpd irq sources are all of the level type. OK, so this is more complicated than I realized. For whatever reason this bit does not work with all monitor configurations. Thanks for clarifying the expected behavior. Stuart ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
