On Tue, 19 Apr 2011 22:46:00 +0200, Daniel Vetter
wrote:
> If it's a simple gem accounting error that won't lead to immediate harm
> (like a NULL-deref) or is a simple violation of a required invariant
> that the caller should always check/ensure, downgrade the BUG_ON to
> a WARN_ON and hope the
One of the original chores for intel_get_load_detect_pipe() was to steal
a CRTC from an active output for the transient purpose of load detection.
Understandably this code was removed some time ago, but its legacy remains.
In order to fix a real bug, where we failed to attach a fb, I first had to
r
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/intel_display.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 0d8d5e2..61fa02f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++
We need to ensure that we feed valid memory into the display plane
attached to the pipe when switching the pipe on. Otherwise, the display
engine may read through an invalid PTE and so throw an PGTBL_ER
exception.
For bonus amusement value, we perform the first load detect before even
establishing
As we only allow the use of a disabled CRTC, we don't need to handle the
case we are reusing an already enabled pipe.
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/intel_display.c | 28 ++--
1 files changed, 10 insertions(+), 18 deletions(-)
diff --git a/drivers
... and the no longer relevant comment. The code ceased stealing a pipe
for load detection a long time ago.
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/intel_display.c |7 ++-
1 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/
Keep all the state required for undoing and restoring the previous pipe
configuration together in a single struct passed from
intel_get_load_detect_pipe() to intel_release_load_detect_pipe() rather
than stuffing them inside the common encoder structure.
Signed-off-by: Chris Wilson
---
drivers/gp
... and so remove the confusion as to whether to use the returned crtc
or intel_encoder->base.crtc with the subsequent load-detection. Even
though they were the same, the two instances of load-detection code
disagreed over which was the more correct.
Signed-off-by: Chris Wilson
---
drivers/gpu/d
On Tue, 19 Apr 2011 14:26:04 -0700, Keith Packard wrote:
> On Tue, 19 Apr 2011 21:32:02 +0100, Chris Wilson
> wrote:
>
> > We need to ensure that we feed valid memory into the display plane
> > attached to the pipe when switching the pipe on. Otherwise, the display
> > engine may read through a
On Wed, Apr 20, 2011 at 09:18:03AM +0100, Chris Wilson wrote:
> > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > index 316603e..8cac87c 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > +++ b/drivers/gpu/drm/i915/i915_gem
On Wed, 20 Apr 2011 07:27:24 -0700, Ben Widawsky wrote:
> On Wed, Apr 20, 2011 at 09:18:03AM +0100, Chris Wilson wrote:
>
> > > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > > b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > > index 316603e..8cac87c 100644
> > > --- a/drivers/gpu/d
On Thu, Apr 07, 2011 at 02:32:58PM -0700, Jesse Barnes wrote:
> Rather than branching in ironlake_pch_enable, add a new train_fdi
> function to the display function pointer struct and use it instead.
>
> Signed-off-by: Jesse Barnes
> ---
> drivers/gpu/drm/i915/i915_drv.h |1 +
> drivers
On Wed, 20 Apr 2011 07:45:08 -0700
Ben Widawsky wrote:
> > @@ -7270,6 +7267,7 @@ static void intel_init_display(struct drm_device *dev)
> > "Disable CxSR\n");
> > dev_priv->display.update_wm = NULL;
> > }
> >
Given that the hardware may be left in a random condition by the BIOS,
it is conceivable that we then attempt to clear the DP_PIPEB_SELECT bit
without us ever enabling/attaching the DP encoder to a pipe. Thus
causing a NULL deference when we attempt to wait for a vblank on that
crtc.
Reported-and-
On Wed, 20 Apr 2011 16:42:08 +0100, Chris Wilson
wrote:
> Given that the hardware may be left in a random condition by the BIOS,
> it is conceivable that we then attempt to clear the DP_PIPEB_SELECT bit
> without us ever enabling/attaching the DP encoder to a pipe. Thus
> causing a NULL deference
On Wed, 20 Apr 2011 10:22:17 +0100, Chris Wilson
wrote:
> +
> + if (!drm_crtc_helper_set_mode(crtc, mode, 0, 0, crtc->fb)) {
> + DRM_DEBUG_KMS("failed to set mode on load-detect
> pipe\n");
> + return false;
> + }
This seems like
On Wed, 20 Apr 2011 10:22:18 +0100, Chris Wilson
wrote:
> Keep all the state required for undoing and restoring the previous pipe
> configuration together in a single struct passed from
> intel_get_load_detect_pipe() to intel_release_load_detect_pipe() rather
> than stuffing them inside the commo
On Wed, 20 Apr 2011 10:22:19 +0100, Chris Wilson
wrote:
> ... and the no longer relevant comment. The code ceased stealing a pipe
> for load detection a long time ago.
Reviewed-by: Keith Packard
--
keith.pack...@intel.com
pgpo4Jr0qCdEM.pgp
Description: PGP signature
On Wed, 20 Apr 2011 10:22:20 +0100, Chris Wilson
wrote:
> Signed-off-by: Chris Wilson
Reviewed-by: Keith Packard
(this might be separately tested to see if it might fix some TV load
detection issues?)
--
keith.pack...@intel.com
pgpnWymRFWXkU.pgp
Description: PGP signature
... and so remove the confusion as to whether to use the returned crtc
or intel_encoder->base.crtc with the subsequent load-detection. Even
though they were the same, the two instances of load-detection code
disagreed over which was the more correct.
Signed-off-by: Chris Wilson
---
drivers/gpu/d
Check the return value from drm_crtc_set_mode(), report the failure
via a debug message and propagate the error back to the caller. This
prevents us from blissfully continuing to do the load detection on a
disabled pipe. Fortunately actual failure for modesetting is very rare,
and reported failures
On Wed, 20 Apr 2011 10:22:21 +0100, Chris Wilson
wrote:
> As we only allow the use of a disabled CRTC, we don't need to handle the
> case we are reusing an already enabled pipe.
Reviewed-by: Keith Packard
--
keith.pack...@intel.com
pgpYSoJipl2cu.pgp
Description: PGP signature
_
On Wed, 20 Apr 2011 10:36:29 -0700, Keith Packard wrote:
> On Wed, 20 Apr 2011 16:42:08 +0100, Chris Wilson
> wrote:
> > Given that the hardware may be left in a random condition by the BIOS,
> > it is conceivable that we then attempt to clear the DP_PIPEB_SELECT bit
> > without us ever enabling
On Wed, 20 Apr 2011 10:22:22 +0100, Chris Wilson
wrote:
> For bonus amusement value, we perform the first load detect before even
> establishing our fbdev.
It seems like we need to be able to perform load detection in this
case to create a suitable frame buffer at boot time.
I also don't see a
On Wed, 20 Apr 2011 11:43:38 -0700, Keith Packard wrote:
> On Wed, 20 Apr 2011 10:22:22 +0100, Chris Wilson
> wrote:
>
> > For bonus amusement value, we perform the first load detect before even
> > establishing our fbdev.
>
> It seems like we need to be able to perform load detection in this
On Tue, 27 Jul 2010 11:03:56 -0400
Adam Jackson wrote:
> On Fri, 2010-07-23 at 16:29 -0400, Andrew Lutomirski wrote:
>
> > Does that include not breaking DirectColor? If we program the gamma
> > ramp to 129 slots, old userspace submits 256 entries that are not
> > monotonic, and we decimate the
On Wed, Apr 20, 2011 at 3:05 PM, Jesse Barnes wrote:
> On Tue, 27 Jul 2010 11:03:56 -0400
> Adam Jackson wrote:
>
>> On Fri, 2010-07-23 at 16:29 -0400, Andrew Lutomirski wrote:
>>
>> > Does that include not breaking DirectColor? If we program the gamma
>> > ramp to 129 slots, old userspace submi
> > Andrew, do you have anything hacked together for this yet?
>
> Nope. I gave up because I couldn't even get the mode to set. :)
Ok well you should be able to now. :) Using the patchset I posted
earlier along with the two attached patches, the testdisplay program in
intel-gpu-tools will set a
On Wed, 20 Apr 2011 11:23:01 -0700, Keith Packard wrote:
> On Wed, 20 Apr 2011 10:22:20 +0100, Chris Wilson
> wrote:
> > Signed-off-by: Chris Wilson
>
> Reviewed-by: Keith Packard
>
> (this might be separately tested to see if it might fix some TV load
> detection issues?)
It is an old bug
On Wed, Apr 20, 2011 at 3:38 PM, Jesse Barnes wrote:
>> > Andrew, do you have anything hacked together for this yet?
>>
>> Nope. I gave up because I couldn't even get the mode to set. :)
>
> Ok well you should be able to now. :) Using the patchset I posted
> earlier along with the two attached p
Chris:
I've tested this patch and it doesn't help us here. Even if I add
lvds_fixed_mode=, the display still comes up with the messed
up configuration from the motherboard's completely ignorant BIOS. It
appears that lvds_fixed_mode is being ignored by the driver.
I think there's still a mis
On Wed, 20 Apr 2011 14:48:36 -0500 (CDT), Mike Isely wrote:
>
> Chris:
>
> I've tested this patch and it doesn't help us here. Even if I add
> lvds_fixed_mode=, the display still comes up with the messed
> up configuration from the motherboard's completely ignorant BIOS. It
> appears that l
On Wed, 20 Apr 2011, Chris Wilson wrote:
> On Wed, 20 Apr 2011 14:48:36 -0500 (CDT), Mike Isely wrote:
> >
> > Chris:
> >
> > I've tested this patch and it doesn't help us here. Even if I add
> > lvds_fixed_mode=, the display still comes up with the messed
> > up configuration from the mothe
We need to ensure that we feed valid memory into the display plane
attached to the pipe when switching the pipe on. Otherwise, the display
engine may read through an invalid PTE and so throw an PGTBL_ER
exception.
As we need to perform load detection before even the first object is
allocated for t
On Wed, 20 Apr 2011 20:38:54 +0100, Chris Wilson
wrote:
> On Wed, 20 Apr 2011 11:23:01 -0700, Keith Packard wrote:
> > On Wed, 20 Apr 2011 10:22:20 +0100, Chris Wilson
> > wrote:
> > > Signed-off-by: Chris Wilson
> >
> > Reviewed-by: Keith Packard
> >
> > (this might be separately tested t
On Wed, 20 Apr 2011 19:55:10 +0100, Chris Wilson
wrote:
> I'll spin up a patch for a temporary buffer and see what you think.
Ok. I'd love to be able to use that code path all of the time, but I
don't like the thought of the cost of a temporary allocation every time
you do load detection.
--
We need to ensure that we feed valid memory into the display plane
attached to the pipe when switching the pipe on. Otherwise, the display
engine may read through an invalid PTE and so throw an PGTBL_ER
exception.
As we need to perform load detection before even the first object is
allocated for t
Provide a reference count to track the forcewake state of the GPU.
Which exports a mechanism to allow userspace to wake the GT. This also
potentially saves a UC read if the GT is known to be awake already.
The reference count is atomic, but the register access and hardware wake
sequence is protect
Signed-off-by: Ben Widawsky
---
drivers/gpu/drm/i915/i915_debugfs.c |2 ++
drivers/gpu/drm/i915/intel_display.c | 12
2 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 3cb0722..94b3
Moved the macros around to properly do reads and writes for the given
GPU. This is to address special requirements for gen6 (SNB) reads and
writes.
Registers in the range 0-0x4 on gen6 platforms require special
handling. Instead of relying on the callers to pick the registers
correctly, move t
The render P-state handling code requires reading from a GT register.
This means that FORCEWAKE must be written to, a resource which is shared
and should be protected by struct_mutex.
Signed-off-by: Ben Widawsky
---
drivers/gpu/drm/i915/i915_dma.c |1 +
drivers/gpu/drm/i915/i915_drv.h |4
forcewake is controlled by the open and close of the debugfs file. This
assures that buggy applications cannot cause the GT to stay on forever.
Signed-off-by: Ben Widawsky
---
drivers/gpu/drm/i915/i915_debugfs.c | 80 +++
1 files changed, 80 insertions(+), 0 del
Once you're done laughing that I got up to v5 (I think I skipped a v3,
and v1 was the IOCTls, so it's really more like a v3) for patches meant
to simply allow register reads and wites...
Back to struct_mutex since it turned out to be my mistake regarding the
IIR problem. The only issue in the int
On Sun, 17 Apr 2011 17:37:32 +0100, Chris Wilson
wrote:
> The 965G (Broadwater) and 965GM (Crestline) chipsets had many errata in
> handling pages allocation above 4GiB. So we should be careful never to
> allocate and attempt to pass such through to the GPU and so limit
> ourselves to GFP_DMA32 o
On Wed, 20 Apr 2011 17:21:03 -0700, Eric Anholt wrote:
> On Sun, 17 Apr 2011 17:37:32 +0100, Chris Wilson
> wrote:
> > The 965G (Broadwater) and 965GM (Crestline) chipsets had many errata in
> > handling pages allocation above 4GiB. So we should be careful never to
> > allocate and attempt to pa
On Wed, 20 Apr 2011 16:53:17 -0700, Ben Widawsky wrote:
>
> Signed-off-by: Ben Widawsky
Just to annoy you, this needs to be split up into the various categories
of fixes. Because...
> static void ironlake_crtc_dpms(struct drm_crtc *crtc, int mode)
> @@ -3067,9 +3074,12 @@ static void i9xx_crt
On Wed, 20 Apr 2011 16:53:18 -0700, Ben Widawsky wrote:
> The render P-state handling code requires reading from a GT register.
> This means that FORCEWAKE must be written to, a resource which is shared
> and should be protected by struct_mutex.
>
> Signed-off-by: Ben Widawsky
> ---
> drivers/g
On Wed, 20 Apr 2011 16:53:14 -0700, Ben Widawsky wrote:
>
> Once you're done laughing that I got up to v5 (I think I skipped a v3,
> and v1 was the IOCTls, so it's really more like a v3) for patches meant
> to simply allow register reads and wites...
>
> Back to struct_mutex since it turned out
48 matches
Mail list logo
