[Intel-gfx] [PULL] topic/core-stuff
Hi Dave, Just flushing out my pile of random drm patches for the merge window, nothing big. And it all hung around in drm-intel trees for a while (only just rebased now). Cheers, Daniel The following changes since commit 182407a6ed5333fc37dd980a8de91a8f826a94f6: drm: add DP MST encoder type (2014-05-30 11:59:51 +1000) are available in the git repository at: git://anongit.freedesktop.org/drm-intel tags/topic/core-stuff-2014-06-02 for you to fetch changes up to 46340642d7c314e4d718ebcdbb00bd55ed7d9d9f: imx-drm: imx-tve: remove unused variable (2014-06-02 09:57:32 +0200) Daniel Vetter (1): drm/dp-helper: Deprecate old i2c-over-dp_aux heleprs Ross Zwisler (1): drm: Missed clflushopt in drm_clflush_virt_range Takashi Iwai (2): drm/ast: Fix double lock at PM resume drm/exynos: Fix double locks at PM resume Thierry Reding (2): drm/plane: Fix sparse warnings drm/plane: Fix a couple of checkpatch warnings Vincent Stehlé (1): imx-drm: imx-tve: remove unused variable drivers/gpu/drm/ast/ast_drv.c | 2 -- drivers/gpu/drm/drm_cache.c | 2 +- drivers/gpu/drm/drm_dp_helper.c | 4 drivers/gpu/drm/drm_plane_helper.c | 7 --- drivers/gpu/drm/exynos/exynos_drm_drv.c | 2 +- drivers/staging/imx-drm/imx-tve.c | 1 - include/drm/drm_plane_helper.h | 2 +- 7 files changed, 11 insertions(+), 9 deletions(-) -- 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 14/9] drm: Kick start vblank interrupts at drm_vblank_on()
From: Ville Syrjälä If the user is interested in getting accurate vblank sequence numbers all the time they may disable the vblank disable timer entirely. In that case it seems appropriate to kick start the vblank interrupts already from drm_vblank_on(). Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/drm_irq.c | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 82a039a..6376d96 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -1126,9 +1126,12 @@ void drm_vblank_on(struct drm_device *dev, int crtc) vblank->last = (dev->driver->get_vblank_counter(dev, crtc) - 1) & dev->max_vblank_count; - - /* re-enable interrupts if there's are users left */ - if (atomic_read(&vblank->refcount) != 0) + /* +* re-enable interrupts if there are users left, or the +* user wishes vblank interrupts to be enabled all the time. +*/ + if (atomic_read(&vblank->refcount) != 0 || + (!dev->vblank_disable_immediate && drm_vblank_offdelay < 0)) WARN_ON(drm_vblank_enable(dev, crtc)); spin_unlock_irqrestore(&dev->vbl_lock, irqflags); } -- 1.8.5.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 13/9] drm: Fix race between drm_vblank_off() and drm_queue_vblank_event()
From: Ville Syrjälä
Currently it's possible that the following will happen:
1. drm_wait_vblank() calls drm_vblank_get()
2. drm_vblank_off() gets called
3. drm_wait_vblank() calls drm_queue_vblank_event() which
adds the event to the queue event though vblank interrupts
are currently disabled (and may not be re-enabled ever again).
To fix the problem, add another vblank->enabled check into
drm_queue_vblank_event().
drm_vblank_off() holds event_lock around the vblank disable,
so no further locking needs to be added to drm_queue_vblank_event().
vblank disable from another source is not possible since
drm_wait_vblank() already holds a vblank reference.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/drm_irq.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 2b97059..82a039a 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -1273,6 +1273,7 @@ static int drm_queue_vblank_event(struct drm_device *dev,
int pipe,
union drm_wait_vblank *vblwait,
struct drm_file *file_priv)
{
+ struct drm_vblank_crtc *vblank = &dev->vblank[pipe];
struct drm_pending_vblank_event *e;
struct timeval now;
unsigned long flags;
@@ -1296,6 +1297,18 @@ static int drm_queue_vblank_event(struct drm_device
*dev, int pipe,
spin_lock_irqsave(&dev->event_lock, flags);
+ /*
+* drm_vblank_off() might have been called after we called
+* drm_vblank_get(). drm_vblank_off() holds event_lock
+* around the vblank disable, so no need for further locking.
+* The reference from drm_vblank_get() protects against
+* vblank disable from another source.
+*/
+ if (!vblank->enabled) {
+ ret = -EINVAL;
+ goto err_unlock;
+ }
+
if (file_priv->event_space < sizeof e->event) {
ret = -EBUSY;
goto err_unlock;
--
1.8.5.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 12/9] drm: Fix deadlock between event_lock and vbl_lock/vblank_time_lock
From: Ville Syrjälä
Currently both drm_irq.c and several drivers call drm_vblank_put()
while holding event_lock. Now that drm_vblank_put() can disable the
vblank interrupt directly it may need to grab vbl_lock and
vblank_time_lock. That causes deadlocks since we take the locks
in the opposite order in two places in drm_irq.c. So let's make
sure the locking order is always event_lock->vbl_lock->vblank_time_lock.
In drm_vblank_off() pull up event_lock from underneath vbl_lock. Hold
the event_lock across the whole operation to make sure we only send
out the events that were on the queue when we disabled the interrupt,
and not ones that got added just after (assuming drm_vblank_on() already
managed to get called somewhere between).
To sort the other deadlock pull the event_lock out from
drm_handle_vblank_events() into drm_handle_vblank() to be taken outside
vblank_time_lock. Add the appropriate assert_spin_locked() to
drm_handle_vblank_events().
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/drm_irq.c | 47 +--
1 file changed, 25 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index b008803..2b97059 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -1040,14 +1040,25 @@ void drm_vblank_off(struct drm_device *dev, int crtc)
unsigned long irqflags;
unsigned int seq;
- spin_lock_irqsave(&dev->vbl_lock, irqflags);
+ spin_lock_irqsave(&dev->event_lock, irqflags);
+
+ spin_lock(&dev->vbl_lock);
vblank_disable_and_save(dev, crtc);
wake_up(&vblank->queue);
+ /*
+* Prevent subsequent drm_vblank_get() from re-enabling
+* the vblank interrupt by bumping the refcount.
+*/
+ if (!vblank->inmodeset) {
+ atomic_inc(&vblank->refcount);
+ vblank->inmodeset = 1;
+ }
+ spin_unlock(&dev->vbl_lock);
+
/* Send any queued vblank events, lest the natives grow disquiet */
seq = drm_vblank_count_and_time(dev, crtc, &now);
- spin_lock(&dev->event_lock);
list_for_each_entry_safe(e, t, &dev->vblank_event_list, base.link) {
if (e->pipe != crtc)
continue;
@@ -1058,18 +1069,7 @@ void drm_vblank_off(struct drm_device *dev, int crtc)
drm_vblank_put(dev, e->pipe);
send_vblank_event(dev, e, seq, &now);
}
- spin_unlock(&dev->event_lock);
-
- /*
-* Prevent subsequent drm_vblank_get() from re-enabling
-* the vblank interrupt by bumping the refcount.
-*/
- if (!vblank->inmodeset) {
- atomic_inc(&vblank->refcount);
- vblank->inmodeset = 1;
- }
-
- spin_unlock_irqrestore(&dev->vbl_lock, irqflags);
+ spin_unlock_irqrestore(&dev->event_lock, irqflags);
}
EXPORT_SYMBOL(drm_vblank_off);
@@ -1449,12 +1449,11 @@ static void drm_handle_vblank_events(struct drm_device
*dev, int crtc)
{
struct drm_pending_vblank_event *e, *t;
struct timeval now;
- unsigned long flags;
unsigned int seq;
- seq = drm_vblank_count_and_time(dev, crtc, &now);
+ assert_spin_locked(&dev->event_lock);
- spin_lock_irqsave(&dev->event_lock, flags);
+ seq = drm_vblank_count_and_time(dev, crtc, &now);
list_for_each_entry_safe(e, t, &dev->vblank_event_list, base.link) {
if (e->pipe != crtc)
@@ -1470,8 +1469,6 @@ static void drm_handle_vblank_events(struct drm_device
*dev, int crtc)
send_vblank_event(dev, e, seq, &now);
}
- spin_unlock_irqrestore(&dev->event_lock, flags);
-
trace_drm_vblank_event(crtc, seq);
}
@@ -1494,15 +1491,18 @@ bool drm_handle_vblank(struct drm_device *dev, int crtc)
if (!dev->num_crtcs)
return false;
+ spin_lock_irqsave(&dev->event_lock, irqflags);
+
/* Need timestamp lock to prevent concurrent execution with
* vblank enable/disable, as this would cause inconsistent
* or corrupted timestamps and vblank counts.
*/
- spin_lock_irqsave(&dev->vblank_time_lock, irqflags);
+ spin_lock(&dev->vblank_time_lock);
/* Vblank irq handling disabled. Nothing to do. */
if (!vblank->enabled) {
- spin_unlock_irqrestore(&dev->vblank_time_lock, irqflags);
+ spin_unlock(&dev->vblank_time_lock);
+ spin_unlock_irqrestore(&dev->event_lock, irqflags);
return false;
}
@@ -1542,10 +1542,13 @@ bool drm_handle_vblank(struct drm_device *dev, int crtc)
crtc, (int) diff_ns);
}
+ spin_unlock(&dev->vblank_time_lock);
+
wake_up(&vblank->queue);
drm_handle_vblank_events(dev, crtc);
- spin_unlock_irqrestore(&dev->vblank_time_lock, irqflags);
+ spin_unlock_irqrestore(&dev->event_lock, irqflags
Re: [Intel-gfx] [PATCH v3] drm/i915: Added write-enable pte bit support
On Sun, Jun 01, 2014 at 12:12:50PM +0530, Akash Goel wrote:
> On Mon, 2014-05-19 at 13:03 +0530, Akash Goel wrote:
> > On Mon, 2014-05-19 at 08:56 +0200, Daniel Vetter wrote:
> > > On Sun, May 18, 2014 at 11:27:00AM +0530, Akash Goel wrote:
> > > > On Wed, 2014-05-14 at 10:14 +0200, Daniel Vetter wrote:
> > > > > On Tue, May 13, 2014 at 03:43:12PM -0700, Jesse Barnes wrote:
> > > > > > On Wed, 14 May 2014 00:30:34 +0200
> > > > > > Daniel Vetter wrote:
> > > > > >
> > > > > > > On Tue, May 13, 2014 at 03:05:24PM -0700, Jesse Barnes wrote:
> > > > > > > > On Tue, 11 Feb 2014 14:19:03 +0530
> > > > > > > > akash.g...@intel.com wrote:
> > > > > > > >
> > > > > > > > > @@ -810,6 +815,7 @@ static void
> > > > > > > > > gen6_ppgtt_insert_entries(struct i915_address_space *vm,
> > > > > > > > > pt_vaddr[act_pte] =
> > > > > > > > >
> > > > > > > > > vm->pte_encode(sg_page_iter_dma_address(&sg_iter),
> > > > > > > > > cache_level, true);
> > > > > > > > > +
> > > > > > > > > if (++act_pte == I915_PPGTT_PT_ENTRIES) {
> > > > > > > > > kunmap_atomic(pt_vaddr);
> > > > > > > > > pt_vaddr = NULL;
> > > > > > > >
> > > > > > > > Some extra whitespace here.
> > > >
> > > > Thanks, will remove this.
> > > >
> > > > > > > >
> > > > > > > > Otherwise:
> > > > > > > > Reviewed-by: Jesse Barnes
> > > > > > >
> > > > > > > Hm, looking at the patch again encoding this into the cache_level
> > > > > > > enum is
> > > > > > > fraught with fun. And due to IS_VLV aliasing chv this will blow
> > > > > > > up on chv
> > > > > > > very likely. My old idea was to eventually add a pte_flags param
> > > > > > > all over
> > > > > > > for this stuff with additional bits.
> > > > > >
> > > > > > That works too; and yeah CHV is all different here isn't it. But
> > > > > > won't
> > > > > > it go through the gen8 paths anyway?
> > > > >
> > > > > Yes, but we have a switch on the cache_level in the gen8 pte encode
> > > > > function. Since the bit31 gets or'ed in for VLV, it'll hit also chv
> > > > > and
> > > > > wreak havoc in that specific switch - we'll hit the default case when
> > > > > we
> > > > > don't expect to.
> > > > >
> > > > > cache_level = functional behaviour relevant for the kernel's
> > > > > clflushing
> > > > > code
> > > > >
> > > >
> > > > As the 'IS_VALLEYVIEW' check will alias with CHV also, can I just update
> > > > the condition here, to include 'GEN7' also (IS_GEN7) in the check.
> > >
> > > Adding new random bits to an enum which is used all over the place (and
> > > not just in the pte encoding functions) makes the code much harder to
> > > read. Also the code that deals with enum cache_level is all about cache
> > > coherency, which has rather tricky logic.
> > >
> > > Hence I expect this choice to cause further bugs down the road, which is
> > > why I don't really like it. My apologies for not spotting this earlier.
> > > -Daniel
> >
> > Hi Daniel,
> >
> > I understand your concern, but actually (as of now) this bit is only VLV
> > specific. As per an earlier suggestion from Chris, I decided to
> > overload the cache_level enum itself, in lieu of adding a new parameter
> > to all the respective 'xxx_insert_entries' & 'xxx_pte_encode' functions.
> >
> > And this is being done just before calling the 'xxx_insert_entries'
> > function, which simply passes the flag as it is to 'xxx_pte_encode'
> > function. So there may not be any risk here, if we use the appropriate
> > VLV check.
>
> Please can you provide your inputs here.
I guess you've run into a case where Chris&I disagree. I still think
wiring up a flags parameter to all the pte encode functions is the sane
option to pick here.
-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 1/6] drm/i915: use VBT to determine whether to enumerate the VGA port
On Thu, May 29, 2014 at 02:47:56PM -0700, Ben Widawsky wrote:
> On Fri, Apr 04, 2014 at 04:12:07PM -0700, Jesse Barnes wrote:
> > Some platforms may not have it, and enumerating it is both confusing and
> > time consuming due to the hotplug and DDC probing.
> >
> > Signed-off-by: Jesse Barnes
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 3697433..6a6406f 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -10678,7 +10678,7 @@ static void intel_setup_outputs(struct drm_device
> > *dev)
> >
> > intel_lvds_init(dev);
> >
> > - if (!IS_ULT(dev))
> > + if (!IS_ULT(dev) && dev_priv->vbt.int_crt_support)
> > intel_crt_init(dev);
> >
> > if (HAS_DDI(dev)) {
>
> For the love of god, can we please rename "bits X" in the
> struct bdb_general_features? That caused me endless pain. It should be
> "byte 0"
>
> Also, I guess there is a problem when the VBT is missing, and we have a
> dont have a CRT on non-ULT (since the default seems to be true). This
> however is no worse than the current situation AFAICT.
>
> Reviewed-by: Ben Widawsky
Queued for -next, thanks for the patch.
-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] drm/i915/bdw: Use timeout mode for RC6 on bdw
On Fri, May 30, 2014 at 11:30:18PM +, O'Rourke, Tom wrote: > >On Wed, Apr 30, 2014 at 02:14:02PM -0700, Kristen Carlson Accardi wrote: > >> On Thu, 01 May 2014 00:03:15 +0300 > >> Imre Deak wrote: > >> > >> > On Wed, 2014-04-30 at 13:41 -0700, Ben Widawsky wrote: > >> > > On Wed, Apr 30, 2014 at 01:34:36PM -0700, Kristen Carlson Accardi > >> > > wrote: > >> > > > On Tue, 29 Apr 2014 22:31:49 -0700 Ben Widawsky > >> > > > wrote: > >> > > > > >> > > > > On Wed, Apr 09, 2014 at 11:44:06AM -0700, Tom O'Rourke wrote: > >> > > > > > Higher RC6 residency is observed using timeout mode instead > >> > > > > > of EI mode. This applies to Broadwell only. > >> > > > > > The difference is particularly noticeable with video > >> > > > > > playback. > >> > > > > > > >> > > > > > Issue: VIZ-3778 > >> > > > > > Change-Id: I62bb12e21caf19651034826b45cde7f73a80938d > >> > > > > > Signed-off-by: Tom O'Rourke > >> > > > > > >> > > > > I've merged this one to my bdw-rc6 branch, and therefore my > >> > > > > broadwell branch. Hopefully Kristen will see some improvement. > >> > > > > >> > > > Unfortunately, I built your bdw-rc6 branch along with the revert > >> > > > I need to get my panel to work, and I get zero rc6 residency. > >> > > > Do I have to explicitly enable it? > >> > > > >> > > I'm not actually sure. You can try it and let me know. I haven't > >> > > had any time to verify the rebase. We can check my hack. > >> > > >> > Note that in -nightly you also have to update sanitize_rc6_option() > >> > along with intel_enable_gt_powersave() and > >> > intel_disable_gt_powersave() since atm these keep RC6 disabled on BDW. > >> > > >> > --Imre > >> > > >> > >> > >> Yes, I reverted fb5ed3b201fe5670c9ffeec3b5f6ff044d543c9e and was able > >> to see some rc6 residency. With the idle workload, residency appears > >> to be similar to before, so no regression. > > > >Thanks. I'll squash this in where appropriate. > > > >-- > >Ben Widawsky, Intel Open Source Technology Center > > [TOR:] Can we get this patch merged now that RC6 is working on > drm-intel-nightly? Needs some review from bdw people. Also some relative residency improvement date should be added to the commit message (yes, we're allowed to do that now officially). -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] Breaking suspend/resume by the Pipe A quirk
On Thu, May 29, 2014 at 11:19:47PM +0200, Thomas Richter wrote: > Hi Daniel, hi folks, > > according to my knowledge, the pipe A quirk is unconditionally enabled on > the 830 to allow resume to work properly. Unfortunately, it does quite the > opposite on the S6010, it breaks resume completely. > > If the pipe A quirk is disabled, then the boot console works correctly. > Resume does not, the display is dead, but it is possible to remotely connect > to the machine, from there POST the video card (via vbetool post), stop X, > then restart X, then the display is back. > > If the pipe A quirk is enabled, then try to resume from suspend, then the > machine is dead completely. You can ping it, but not log in. I currently > have not yet tried to figure out where it hangs, but I would suspect the > problem is somewhere in the i915 kernel module. The display > just stays black. > > Thus, in addition to the watermark fix I proposed, please *disable* the > unconditional pipe A quirk for the 830GM since it really breaks things, not > only the boot console. Can you go right ahead and please submit this as a patch? 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 2/3] drm/i915: Change Mipi register definitions
On Sun, Jun 01, 2014 at 05:41:54AM +, Sharma, Shashank wrote: > Hi Damien, > Please correct me if I am missing something, but the only reason we are > seeing those extra formatting changes is, almost all of the old MIPI > register definitions were beyond 80 char, and was checked in like that > with warnings (How ?) So when I was checking for checkpatch errors, I > saw that, and I tried to fix that in the previous patch. Checkpatch et al. isn't a hard rule here, and occasionally checkpatch is just wrong. We have lots more exceptions in i915_reg.h since it makes it easier to read in general. -Daniel > > Anyways, I will send another patch, as per your suggestions, which will > include only the dev_priv->mmio_offset change, and rebase the other one on > top of it. > > Regards > Shashank > -Original Message- > From: Lespiau, Damien > Sent: Saturday, May 31, 2014 3:19 PM > To: Sharma, Shashank > Cc: Kumar, Shobhit; intel-gfx@lists.freedesktop.org; Vetter, Daniel; > ville.syrj...@linux.intel.com > Subject: Re: [PATCH 2/3] drm/i915: Change Mipi register definitions > > On Sat, May 31, 2014 at 01:32:42PM +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. > > > > V4: Addressing review comments by Ville This patch removes all the > > un-necessary formatting changes. > > V5: Addressing review comments by Damien Changed input variable name > > from tc to pipe > > > > Signed-off-by: Shashank Sharma > > I'm sorry if we haven't been clear enough, but in a patch that changes > VLV_DISPLAY_BASE + 0xf00 to dev_priv->mipi_mmio_base + 0xf00, we can't > have: > > > -#define MIPI_PORT_CTRL(pipe) _PIPE(pipe, _MIPIA_PORT_CTRL, > > _MIPIB_PORT_CTRL) > > +#define MIPI_PORT_CTRL(pipe) _PIPE(pipe, > > _MIPIA_PORT_CTRL, \ > > + _MIPIB_PORT_CTRL) > > That's the un-necessary formatting changes that Ville was talking about, and > the "change only one thing per patch" I was talking about. In this case the > change is "make VLV_DISPLAY_BASE + 0xfoo" to dev_priv->mipi_mmio_base + > 0xf00", so the diff should only show that kind of changes. > > Please bear with me for this one, let's get it "correct" and I'm sure the > next ones will be easier. > > -- > Damien > ___ > 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 2/4] drm/i915: leave rc6 enabled at suspend time
On Fri, May 30, 2014 at 08:32:20AM -0700, Jesse Barnes wrote: > On Fri, 30 May 2014 15:54:37 +0300 > Imre Deak wrote: > > > On Thu, 2014-05-29 at 14:11 -0700, Jesse Barnes wrote: > > > From: Kristen Carlson Accardi > > > > > > This allows the system to enter the lowest power mode during system > > > freeze. > > > > > > Signed-off-by: Jesse Barnes > > > --- > > > drivers/gpu/drm/i915/i915_drv.c | 3 --- > > > drivers/gpu/drm/i915/intel_drv.h | 1 + > > > drivers/gpu/drm/i915/intel_pm.c | 16 +++- > > > 3 files changed, 12 insertions(+), 8 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c > > > b/drivers/gpu/drm/i915/i915_drv.c > > > index 66c6ffb..433bdfa 100644 > > > --- a/drivers/gpu/drm/i915/i915_drv.c > > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > > @@ -521,8 +521,6 @@ static int i915_drm_freeze(struct drm_device *dev) > > > drm_irq_uninstall(dev); > > > dev_priv->enable_hotplug_processing = false; > > > > > > - intel_disable_gt_powersave(dev); > > > - > > > > I wonder what was the reason for this call. One possibility is that > > i915_save_state() depends on it to save the correct registers, but it > > would be good to clarify this. save_state needs to die. Pretty much because it's fragile like you've just pointed out. > > It also cancels some deferred works which we do need here. But we could > > also add that to intel_enable_gt_powersave_sync() in this patch. > > Yeah I was worried about that too, but then we do the reset on resume > anyway, and I didn't see anything in my logs in testing... > > But I can split that out if there's a reason to. Seems like we do a > bit too much teardown at suspend these days (like tearing down opregion > state), I'd like to trim it back if possible and share between runtime > and system suspend/freeze. Yeah, that's the direction I'm pushing towards, too - we should only stop timers, work, interrupts and stuff like that, but never tear down structures. So if you can use this opportunity to fix a few of the offenders (like opregion) I'd be very happy. -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 4/4] drm/i915: make sure PC8 is enabled on suspend and disabled on resume
On Thu, May 29, 2014 at 02:11:37PM -0700, Jesse Barnes wrote:
> From: Kristen Carlson Accardi
>
> This matches the runtime suspend paths and allows the system to enter
> the lowest power mode at freeze time.
>
> Signed-off-by: Kristen Carlson Accardi
> Signed-off-by: Jesse Barnes
pc8 is fully subsumed into runtime pm by now. Do we _really_ still need
this?
-Daniel
> ---
> drivers/gpu/drm/i915/i915_drv.c | 6 ++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index b6211d7..24dc856 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -558,6 +558,9 @@ static int i915_drm_freeze(struct drm_device *dev)
>
> intel_display_set_init_power(dev_priv, false);
>
> + if (IS_HASWELL(dev) || IS_BROADWELL(dev))
> + hsw_enable_pc8(dev_priv);
> +
> return 0;
> }
>
> @@ -618,6 +621,9 @@ static int __i915_drm_thaw(struct drm_device *dev, bool
> restore_gtt_mappings)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
>
> + if (IS_HASWELL(dev) || IS_BROADWELL(dev))
> + hsw_disable_pc8(dev_priv);
> +
> if (drm_core_check_feature(dev, DRIVER_MODESET) &&
> restore_gtt_mappings) {
> mutex_lock(&dev->struct_mutex);
> --
> 1.9.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
Re: [Intel-gfx] [PATCH] drm/i915: enable PPGTT on VLV
On Thu, May 29, 2014 at 02:33:21PM -0700, Jesse Barnes wrote:
> Working for real this time. i915_ppgtt_info has all sorts of good stuff
> in it and X is running nicely on top.
>
> Signed-off-by: Jesse Barnes
Maintainer-nitpick: Please don't forget the patch changelog ...
-Daniel
> ---
> drivers/gpu/drm/i915/i915_drv.h | 6 ++
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index bea9ab40..8631fb3 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1936,10 +1936,8 @@ struct drm_i915_cmd_table {
> #define I915_NEED_GFX_HWS(dev) (INTEL_INFO(dev)->need_gfx_hws)
>
> #define HAS_HW_CONTEXTS(dev) (INTEL_INFO(dev)->gen >= 6)
> -#define HAS_ALIASING_PPGTT(dev) (INTEL_INFO(dev)->gen >= 6 && \
> - (!IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)))
> -#define HAS_PPGTT(dev) (INTEL_INFO(dev)->gen >= 7 \
> - && !IS_GEN8(dev))
> +#define HAS_ALIASING_PPGTT(dev) (INTEL_INFO(dev)->gen >= 6)
> +#define HAS_PPGTT(dev) (INTEL_INFO(dev)->gen >= 7 &&
> !IS_GEN8(dev))
> #define USES_PPGTT(dev) intel_enable_ppgtt(dev, false)
> #define USES_FULL_PPGTT(dev) intel_enable_ppgtt(dev, true)
>
> --
> 1.9.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
Re: [Intel-gfx] [PATCH] drm/i915: vlv: fix stuck primary plane due to SR watermarks
On Fri, May 30, 2014 at 09:07:19AM +0300, Imre Deak wrote:
> Blanking/unblanking the console in a loop on an Asus T100 sometimes
> leaves the console blank. After some digging I found that applying
>
> commit 61bc95c1fbbb6a08b55bbe161fdf1ea5493fc595
> Author: Egbert Eich
> Date: Mon Mar 4 09:24:38 2013 -0500
>
> DRM/i915: On G45 enable cursor plane briefly after enabling the display
> plane.
>
> fixed VLV too. At least in my case the problem seems to happen already
> during the previous crtc disabling, and it goes away if we disable SR
> watermarks before disabling the primary plane.
>
> I also noticed that the problem only happens if the C7S CPU idle state
> is entered, disabling that also gets rid of the problem. As an
> alternative I also tried to increase the plane SR watermark close to its
> maximum but that didn't solve the issue.
>
> Signed-off-by: Imre Deak
Can we please escalate this a bit with the hw guys? Duct-tape for g4x is
kinda ok, but imo for byt we should try harder ...
-Daniel
> ---
> drivers/gpu/drm/i915/i915_drv.h | 1 +
> drivers/gpu/drm/i915/intel_display.c | 9 +
> drivers/gpu/drm/i915/intel_pm.c | 7 +++
> 3 files changed, 17 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index bea9ab40..afee677 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2591,6 +2591,7 @@ extern void gen6_set_rps(struct drm_device *dev, u8
> val);
> extern void valleyview_set_rps(struct drm_device *dev, u8 val);
> extern int valleyview_rps_max_freq(struct drm_i915_private *dev_priv);
> extern int valleyview_rps_min_freq(struct drm_i915_private *dev_priv);
> +extern void valleyview_disable_sr_watermarks(struct drm_i915_private
> *dev_priv);
> extern void intel_detect_pch(struct drm_device *dev);
> extern int intel_trans_dp_port_sel(struct drm_crtc *crtc);
> extern int intel_enable_rc6(const struct drm_device *dev);
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 54095d4..11611e1 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4767,6 +4767,15 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
> if (IS_GEN2(dev))
> intel_set_cpu_fifo_underrun_reporting(dev, pipe, false);
>
> + /*
> + * Having the SR WMs enabled when disabling the primary plane may
> + * leave the primary plane in a stuck state, where it wouldn't
> + * start fetching pixels after a subsequent crtc enable.
> + * The WMs will be set to their proper values again when calling
> + * intel_update_watermarks() next.
> + */
> + if (IS_VALLEYVIEW(dev))
> + valleyview_disable_sr_watermarks(dev_priv);
> intel_crtc_disable_planes(crtc);
>
> for_each_encoder_on_crtc(dev, crtc, encoder)
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 1840d15..01962aa 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -1306,6 +1306,13 @@ static void vlv_update_drain_latency(struct drm_device
> *dev)
> }
> }
>
> +void valleyview_disable_sr_watermarks(struct drm_i915_private *dev_priv)
> +{
> + I915_WRITE(FW_BLC_SELF_VLV,
> +I915_READ(FW_BLC_SELF_VLV) & ~FW_CSPWRDWNEN);
> + POSTING_READ(FW_BLC_SELF_VLV);
> +}
> +
> #define single_plane_enabled(mask) is_power_of_2(mask)
>
> static void valleyview_update_wm(struct drm_crtc *crtc)
> --
> 1.8.4
>
> ___
> 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 v2] drm/i915: Always apply cursor width changes
On Fri, May 30, 2014 at 04:35:26PM +0300, Chris Wilson wrote:
> It is possible for userspace to create a big object large enough for a
> 256x256, and then switch over to using it as a 64x64 cursor. This
> requires the cursor update routines to check for a change in width on
> every update, rather than just when the cursor is originally enabled.
>
> This also fixes an issue with 845g/865g which cannot change the base
> address of the cursor whilst it is active.
>
> Signed-off-by: Chris Wilson
> [Antti:rebased, adjusted macro names and moved some lines, no functional
> changes]
> Reviewed-by: Antti Koskipaa
> Tested-by: Antti Koskipaa
Still missing the igt testcase. Is Antti still working on that one? And I
guess now we also need an Cc: sta...@vger.kernel.org on this one here.
-Daniel
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 2 +-
> drivers/gpu/drm/i915/intel_display.c | 106
> +--
> drivers/gpu/drm/i915/intel_drv.h | 3 +-
> 3 files changed, 53 insertions(+), 58 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index 2e5f76a..04d0b7c 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2353,7 +2353,7 @@ static int i915_display_info(struct seq_file *m, void
> *unused)
>
> active = cursor_position(dev, crtc->pipe, &x, &y);
> seq_printf(m, "\tcursor visible? %s, position (%d, %d),
> addr 0x%08x, active? %s\n",
> -yesno(crtc->cursor_visible),
> +yesno(crtc->cursor_base),
> x, y, crtc->cursor_addr,
> yesno(active));
> }
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 731cd01..955f92d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7872,29 +7872,33 @@ static void i845_update_cursor(struct drm_crtc *crtc,
> u32 base)
> struct drm_device *dev = crtc->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> - bool visible = base != 0;
> - u32 cntl;
> + uint32_t cntl;
>
> - if (intel_crtc->cursor_visible == visible)
> - return;
> -
> - cntl = I915_READ(_CURACNTR);
> - if (visible) {
> + if (base != intel_crtc->cursor_base) {
> /* On these chipsets we can only modify the base whilst
>* the cursor is disabled.
>*/
> + if (intel_crtc->cursor_cntl) {
> + I915_WRITE(_CURACNTR, 0);
> + POSTING_READ(_CURACNTR);
> + intel_crtc->cursor_cntl = 0;
> + }
> +
> I915_WRITE(_CURABASE, base);
> + POSTING_READ(_CURABASE);
> + }
>
> - cntl &= ~(CURSOR_FORMAT_MASK);
> - /* XXX width must be 64, stride 256 => 0x00 << 28 */
> - cntl |= CURSOR_ENABLE |
> + /* XXX width must be 64, stride 256 => 0x00 << 28 */
> + cntl = 0;
> + if (base)
> + cntl = (CURSOR_ENABLE |
> CURSOR_GAMMA_ENABLE |
> - CURSOR_FORMAT_ARGB;
> - } else
> - cntl &= ~(CURSOR_ENABLE | CURSOR_GAMMA_ENABLE);
> - I915_WRITE(_CURACNTR, cntl);
> -
> - intel_crtc->cursor_visible = visible;
> + CURSOR_FORMAT_ARGB);
> + if (intel_crtc->cursor_cntl != cntl) {
> + I915_WRITE(_CURACNTR, cntl);
> + POSTING_READ(_CURACNTR);
> + intel_crtc->cursor_cntl = cntl;
> + }
> }
>
> static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base)
> @@ -7903,16 +7907,12 @@ static void i9xx_update_cursor(struct drm_crtc *crtc,
> u32 base)
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> int pipe = intel_crtc->pipe;
> - bool visible = base != 0;
> -
> - if (intel_crtc->cursor_visible != visible) {
> - int16_t width = intel_crtc->cursor_width;
> - uint32_t cntl = I915_READ(CURCNTR(pipe));
> - if (base) {
> - cntl &= ~(CURSOR_MODE | MCURSOR_PIPE_SELECT);
> - cntl |= MCURSOR_GAMMA_ENABLE;
> + uint32_t cntl;
>
> - switch (width) {
> + cntl = 0;
> + if (base) {
> + cntl = MCURSOR_GAMMA_ENABLE;
> + switch (intel_crtc->cursor_width) {
> case 64:
> cntl |= CURSOR_MODE_64_ARGB_AX;
> break;
> @@ -7925,18 +7925,16 @@ static void i9xx_update_cursor(struct drm_crtc *crtc,
> u32 base)
> default:
> WARN_ON(1);
>
Re: [Intel-gfx] [PATCH 3/3] drm/i915: Boost GPU frequency if we detect outstanding pageflips
On Fri, May 30, 2014 at 05:16:36PM +0100, Chris Wilson wrote:
> If we hit a vblank and see that have a pageflip queue but not yet
> processed, ensure that the GPU is running at maximum in order to clear
> the backlog. Pageflips are only queued for the following vblank, if we
> miss it, there will be a visible stutter. Boosting the GPU frequency
> doesn't prevent us from missing the target vblank, but it should help
> the subsequent frames hitting theirs.
>
> v2: Reorder vblank vs flip-complete so that we only check for a missed
> flip after processing the completion events, and avoid spurious boosts.
>
> Signed-off-by: Chris Wilson
> ---
> drivers/gpu/drm/i915/i915_drv.h | 1 +
> drivers/gpu/drm/i915/intel_display.c | 6 ++
> drivers/gpu/drm/i915/intel_drv.h | 1 +
> drivers/gpu/drm/i915/intel_pm.c | 15 +++
> 4 files changed, 23 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index f9450045a532..af050e439168 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -910,6 +910,7 @@ struct intel_gen6_power_mgmt {
>
> bool enabled;
> struct delayed_work delayed_resume_work;
> + struct work_struct boost_work;
>
> /*
>* Protects RPS/RC6 register access and PCU communication.
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 54b69838e2de..3ad1529e74df 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9258,6 +9258,7 @@ void intel_check_page_flip(struct drm_device *dev, int
> pipe)
> struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe];
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> unsigned long flags;
> + bool outstanding;
>
> if (crtc == NULL)
> return;
> @@ -9270,7 +9271,12 @@ void intel_check_page_flip(struct drm_device *dev, int
> pipe)
> page_flip_completed(dev_priv, intel_crtc,
> intel_crtc->unpin_work);
> intel_crtc->unpin_work = NULL;
> }
> + outstanding = (intel_crtc->unpin_work != NULL &&
> +crtc_sbc(intel_crtc) - intel_crtc->unpin_work->sbc > 1);
s/outstanding/still_pending/? Since the situation is very much not
oustanding at all ...
-Daniel
> spin_unlock_irqrestore(&dev->event_lock, flags);
> +
> + if (outstanding)
> + intel_queue_rps_boost(dev);
> }
>
> static int intel_crtc_page_flip(struct drm_crtc *crtc,
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index b17c295fe967..a77f104db366 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -964,6 +964,7 @@ void ironlake_teardown_rc6(struct drm_device *dev);
> void gen6_update_ring_freq(struct drm_device *dev);
> void gen6_rps_idle(struct drm_i915_private *dev_priv);
> void gen6_rps_boost(struct drm_i915_private *dev_priv);
> +void intel_queue_rps_boost(struct drm_device *dev);
> void intel_aux_display_runtime_get(struct drm_i915_private *dev_priv);
> void intel_aux_display_runtime_put(struct drm_i915_private *dev_priv);
> void intel_runtime_pm_get(struct drm_i915_private *dev_priv);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 00ab3c7a282d..dce2c268851f 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -6707,6 +6707,19 @@ int vlv_freq_opcode(struct drm_i915_private *dev_priv,
> int val)
> return DIV_ROUND_CLOSEST(4 * mul * val, dev_priv->mem_freq) + 0xbd - 6;
> }
>
> +static void __intel_rps_boost_work(struct work_struct *work)
> +{
> + gen6_rps_boost(container_of(work, struct drm_i915_private,
> rps.boost_work));
> +}
> +
> +void intel_queue_rps_boost(struct drm_device *dev)
> +{
> + struct drm_i915_private *dev_priv = to_i915(dev);
> +
> + if (INTEL_INFO(dev)->gen >= 6)
> + queue_work(dev_priv->wq, &dev_priv->rps.boost_work);
> +}
> +
> void intel_pm_setup(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -6715,6 +6728,8 @@ void intel_pm_setup(struct drm_device *dev)
>
> INIT_DELAYED_WORK(&dev_priv->rps.delayed_resume_work,
> intel_gen6_powersave_work);
> + INIT_WORK(&dev_priv->rps.boost_work,
> + __intel_rps_boost_work);
>
> dev_priv->pm.suspended = false;
> dev_priv->pm.irqs_disabled = false;
> --
> 2.0.0.rc4
>
> ___
> 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
Re: [Intel-gfx] 00:02.0 VGA compatible controller: Intel Corporation ValleyView Gen7 (rev 0a)
On Wed, 28 May 2014, ANDRIAMILAMINA wrote: > Hello there, > > I am not very good in linux stuff but, i would like to know if is there > any possibility to get this device work. What did you try? Should work with recent upstream. BR, Jani. > here is a result of lspci: > > 00:02.0 VGA compatible controller: Intel Corporation ValleyView Gen7 > (rev 0a) > > Or it is even planned? > > Thanks > > -- > ANDRIAMILAMINA Monge > sYSaDMIN > mo...@gulfsat.mg > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, 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] Align i830 watermark to cache lines
Hi folks,
as by discussion, the problem with the i830 watermark problems is likely
that the 830 requires the number of entries in the buffer to be a
multiple of the cache line size. I provide hereby a small patch
against intel_pm.c that performs the alignment for GEN2 chips.
Tested on the Fujitsu S6010 and R31, seems to work fine here and
generates reasonable watermarks that do not flicker.
What is a bit unsatisfactory is that, due to the nature of the patch,
the number of entries in the buffer is always rounded up (necessarily,
to be conservative), even though for all practical configurations, the
rounded up size is too large to fit into the buffer, and thus the
rounding direction is "round down" instead of "round up" for all
realistic settings.
Anyhow, the stuff works.
Greetings,
Thomas
>From ee1210a1f49abaddc2c6c46cfb521db6ab08c261 Mon Sep 17 00:00:00 2001
From: thor
Date: Sun, 1 Jun 2014 18:33:20 +0200
Subject: [PATCH] Align i830 watermark to cache lines.
Signed-off-by: thor
---
drivers/gpu/drm/i915/intel_pm.c | 38 +-
1 file changed, 37 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 1840d15..fbfd57c 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1489,6 +1489,22 @@ static void i965_update_wm(struct drm_crtc *unused_crtc)
I915_WRITE(DSPFW3, (cursor_sr << DSPFW_CURSOR_SR_SHIFT));
}
+static int round_to_lines(int watermark, int fifo_size, int line_size)
+{
+ int entries = fifo_size - watermark;
+
+ if (entries < 0)
+ entries = 0;
+
+ entries = DIV_ROUND_UP(entries, line_size);
+ while (entries > fifo_size)
+ entries -= line_size;
+
+ watermark = fifo_size - line_size;
+
+ return watermark;
+}
+
static void i9xx_update_wm(struct drm_crtc *unused_crtc)
{
struct drm_device *dev = unused_crtc->dev;
@@ -1520,6 +1536,12 @@ static void i9xx_update_wm(struct drm_crtc *unused_crtc)
planea_wm = intel_calculate_wm(adjusted_mode->crtc_clock,
wm_info, fifo_size, cpp,
latency_ns);
+
+ if (IS_GEN2(dev))
+ planea_wm = round_to_lines(planea_wm,
+ fifo_size,
+ I830_FIFO_LINE_SIZE);
+
enabled = crtc;
} else
planea_wm = fifo_size - wm_info->guard_size;
@@ -1536,6 +1558,12 @@ static void i9xx_update_wm(struct drm_crtc *unused_crtc)
planeb_wm = intel_calculate_wm(adjusted_mode->crtc_clock,
wm_info, fifo_size, cpp,
latency_ns);
+
+ if (IS_GEN2(dev))
+ planeb_wm = round_to_lines(planeb_wm,
+ fifo_size,
+ I830_FIFO_LINE_SIZE);
+
if (enabled == NULL)
enabled = crtc;
else
@@ -1631,16 +1659,24 @@ static void i845_update_wm(struct drm_crtc *unused_crtc)
const struct drm_display_mode *adjusted_mode;
uint32_t fwater_lo;
int planea_wm;
+ int fifo_size;
crtc = single_enabled_crtc(dev);
if (crtc == NULL)
return;
adjusted_mode = &to_intel_crtc(crtc)->config.adjusted_mode;
+ fifo_size = dev_priv->display.get_fifo_size(dev, 0);
planea_wm = intel_calculate_wm(adjusted_mode->crtc_clock,
&i845_wm_info,
- dev_priv->display.get_fifo_size(dev, 0),
+ fifo_size,
4, latency_ns);
+
+ planea_wm = round_to_lines(planea_wm,
+ fifo_size,
+ I830_FIFO_LINE_SIZE);
+
+
fwater_lo = I915_READ(FW_BLC) & ~0xfff;
fwater_lo |= (3<<8) | planea_wm;
--
1.7.10.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v10] drm/i915: Replaced Blitter ring based flips with MMIO flips
On Mon, 2014-06-02 at 06:56 +, Chris Wilson wrote:
> On Sun, Jun 01, 2014 at 04:43:13PM +0530, sourab.gu...@intel.com wrote:
> > From: Sourab Gupta
> >
> > This patch enables the framework for using MMIO based flip calls,
> > in contrast with the CS based flip calls which are being used currently.
> >
> > MMIO based flip calls can be enabled on architectures where
> > Render and Blitter engines reside in different power wells. The
> > decision to use MMIO flips can be made based on workloads to give
> > 100% residency for Media power well.
> >
> > v2: The MMIO flips now use the interrupt driven mechanism for issuing the
> > flips when target seqno is reached. (Incorporating Ville's idea)
> >
> > v3: Rebasing on latest code. Code restructuring after incorporating
> > Damien's comments
> >
> > v4: Addressing Ville's review comments
> > -general cleanup
> > -updating only base addr instead of calling update_primary_plane
> > -extending patch for gen5+ platforms
> >
> > v5: Addressed Ville's review comments
> > -Making mmio flip vs cs flip selection based on module parameter
> > -Adding check for DRIVER_MODESET feature in notify_ring before calling
> > notify mmio flip.
> > -Other changes mostly in function arguments
> >
> > v6: -Having a seperate function to check condition for using mmio flips
> > (Ville)
> > -propogating error code from i915_gem_check_olr (Ville)
> >
> > v7: -Adding __must_check with i915_gem_check_olr (Chris)
> > -Renaming mmio_flip_data to mmio_flip (Chris)
> > -Rebasing on latest nightly
> >
> > v8: -Rebasing on latest code
> > -squash 3rd patch in series(mmio setbase vs page flip race) with this
> > patch
> > -Added new tiling mode update in intel_do_mmio_flip (Chris)
> >
> > v9: -check for obj->last_write_seqno being 0 instead of obj->ring being
> > NULL in
> > intel_postpone_flip, as this is a more restrictive condition (Chris)
> >
> > v10: -Applied Chris's suggestions for squashing patches 2,3 into this patch.
> > These patches make the selection of CS vs MMIO flip at the page flip time,
> > and
> > make the module parameter for using mmio flips as tristate, the states being
> > 'force CS flips', 'force mmio flips', 'driver discretion'.
> > Changed the logic for driver discretion (Chris)
> >
> > Reviewed-by: Chris Wilson
> > Signed-off-by: Sourab Gupta
> > Signed-off-by: Akash Goel
> Tested-by: Chris Wilson # snb, ivb
>
> Really happy with this now, just a few irrelevant bikesheds.
>
> > -static int
> > +__must_check int
> > i915_gem_check_olr(struct intel_engine_cs *ring, u32 seqno)
> > {
>
> Only the declaration requires the __must_check attribute, we don't need
> it here as well.
>
> > int ret;
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c
> > b/drivers/gpu/drm/i915/i915_irq.c
> > index 4ef6423..e0edb1f 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -1218,6 +1218,9 @@ static void notify_ring(struct drm_device *dev,
> >
> > trace_i915_gem_request_complete(ring);
> >
> > + if (drm_core_check_feature(dev, DRIVER_MODESET))
> > + intel_notify_mmio_flip(ring);
> > +
>
> I wish Ville had suggested making UMS do the extra work of setting up
> the spinlock instead.
>
> > +static bool use_mmio_flip(struct intel_engine_cs *ring,
> > + struct drm_i915_gem_object *obj)
> > +{
> > +/* This is not being used for older platforms, because
> > +* non-availability of flip done interrupt forces us to use
> > +* CS flips. Older platforms derive flip done using some clever
> > +* tricks involving the flip_pending status bits and vblank irqs.
> > +* So using MMIO flips there would disrupt this mechanism.
> > +*/
> > +
> > + if (INTEL_INFO(ring->dev)->gen < 5)
> > + return false;
> > +
> > + if (i915.use_mmio_flip < 0)
> > + return false;
> > + else if (i915.use_mmio_flip > 0)
> > + return true;
> > + else
> > + return ring != obj->ring;
> > +}
>
> Check whitespace.
>
Hi Chris,
Couldn't get the whitespace error here, and at other places. Also,
checkpatch.pl doesn't show any. Can you please point out the error.
Thanks,
Sourab
> > +
> > +static void intel_do_mmio_flip(struct intel_crtc *intel_crtc)
> > +{
> > + struct drm_device *dev = intel_crtc->base.dev;
> > + struct drm_i915_private *dev_priv = dev->dev_private;
> > + struct intel_framebuffer *intel_fb =
> > + to_intel_framebuffer(intel_crtc->base.primary->fb);
> > + struct drm_i915_gem_object *obj = intel_fb->obj;
> > + u32 dspcntr;
> > + u32 reg;
> > +
> > + intel_mark_page_flip_active(intel_crtc);
> > +
> > + reg = DSPCNTR(intel_crtc->plane);
> > + dspcntr = I915_READ(reg);
> > +
> > + if (INTEL_INFO(dev)->gen >= 4) {
> > + if (obj->tiling_mode != I915_TILING_NONE)
> > + dspcntr |= DISPPLANE_TILED;
> > + else
> > +
Re: [Intel-gfx] Breaking suspend/resume by the Pipe A quirk
Am 02.06.2014 10:27, schrieb Daniel Vetter: Can you go right ahead and please submit this as a patch? Certainly, but I would prefer to get more information on this. Even though the R31 *also* works without the pipe A quirk, I am not sure it does work on all other hardware configurations. There is, however, an important difference between the R31 and the S6010: The R31 uses two independent display pipes for the generating the display, LVDS for the internal and VGA for the external display. As a result, frame rates and resolutions can be different between the two outputs. The S6010, however, seems to use a single pipe design, with the internal display connected via DVI (not LVDS!) and the external by VGA. This has the unfortunate side effect that I cannot set the resolutions of internal and external display independently. Any attempt to modify the external resolution while using the internal screen results in an "no crtc found for output VGA1" when using xrandr. (Not quite sure what this means, but I believe that the VGA output is simply a duplicate of the DVI output, and the two are probably connected through a bios-switchable bridge chip). Thus, I would *prefer* to be conservative and only disable the pipe_A quirk only in situations where there is a single display pipe (as in the S6010) and, just to be on the safe side, keep it enabled in dual-pipe (as in R31) configurations. Now I wonder how I could possibly distinguish between the two. Could you please provide some pointers? Greetings, Thomas ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t] kms_cursor_crc: Test cursor size change ioctl
Now that we support cursor changes other than 64x64, a bug was found
where the size change was only applied at cursor enable time, rather
than at every update. Add a testcase for that.
Signed-off-by: Antti Koskipaa
---
tests/kms_cursor_crc.c | 55 ++
1 file changed, 55 insertions(+)
diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
index 06625ee..1b8da26 100644
--- a/tests/kms_cursor_crc.c
+++ b/tests/kms_cursor_crc.c
@@ -54,6 +54,7 @@ typedef struct {
int left, right, top, bottom;
int screenw, screenh;
int curw, curh; /* cursor size */
+ int cursor_max_size;
igt_pipe_crc_t *pipe_crc;
} test_data_t;
@@ -272,6 +273,7 @@ static bool prepare_crtc(test_data_t *test_data,
igt_output_t *output,
test_data->screenh = mode->vdisplay;
test_data->curw = cursor_w;
test_data->curh = cursor_h;
+ test_data->cursor_max_size = cursor_w;
/* make sure cursor is disabled */
cursor_disable(test_data);
@@ -354,6 +356,56 @@ static void create_cursor_fb(data_t *data, int cur_w, int
cur_h)
igt_assert(cairo_status(cr) == 0);
}
+static void test_cursor_size(test_data_t *test_data)
+{
+ data_t *data = test_data->data;
+ igt_display_t *display = &data->display;
+ igt_pipe_crc_t *pipe_crc = test_data->pipe_crc;
+ igt_crc_t crc[10], ref_crc;
+ igt_plane_t *cursor;
+ cairo_t *cr;
+ uint32_t fb_id;
+ int i, size, cursor_max_size = test_data->cursor_max_size;
+
+ /* Create a maximum size cursor, then change the size in flight to
+* smaller ones to see that the size is applied correctly
+*/
+ fb_id = igt_create_fb(data->drm_fd, cursor_max_size, cursor_max_size,
+ DRM_FORMAT_ARGB, false, &data->fb);
+ igt_assert(fb_id);
+
+ /* Use a solid white rectangle as the cursor */
+ cr = igt_get_cairo_ctx(data->drm_fd, &data->fb);
+ igt_paint_color_alpha(cr, 0, 0, cursor_max_size, cursor_max_size, 1.0,
1.0, 1.0, 1.0);
+
+ /* Hardware test loop */
+ cursor_enable(test_data);
+ cursor = igt_output_get_plane(test_data->output, IGT_PLANE_CURSOR);
+ igt_plane_set_position(cursor, 0, 0);
+ for (i = 0, size = cursor_max_size; size >= 64; size /= 2, i++) {
+ /* Change size in flight: */
+ int ret = drmModeSetCursor(data->drm_fd,
test_data->output->config.crtc->crtc_id,
+ data->fb.gem_handle, size, size);
+ igt_assert(ret == 0);
+ igt_wait_for_vblank(data->drm_fd, test_data->pipe);
+ igt_pipe_crc_collect_crc(pipe_crc, &crc[i]);
+ }
+ cursor_disable(test_data);
+ /* Software test loop */
+ cr = igt_get_cairo_ctx(data->drm_fd, &data->primary_fb);
+ for (i = 0, size = cursor_max_size; size >= 64; size /= 2, i++) {
+ /* Now render the same in software and collect crc */
+ igt_paint_color_alpha(cr, 0, 0, size, size, 1.0, 1.0, 1.0, 1.0);
+ igt_display_commit(display);
+ igt_wait_for_vblank(data->drm_fd, test_data->pipe);
+ igt_pipe_crc_collect_crc(pipe_crc, &ref_crc);
+ /* Clear screen afterwards */
+ igt_paint_color(cr, 0, 0, test_data->screenw,
test_data->screenh,
+ 0.0, 0.0, 0.0);
+ igt_assert(igt_crc_equal(&crc[i], &ref_crc));
+ }
+}
+
static void run_test_generic(data_t *data, int cursor_max_size)
{
int cursor_size;
@@ -407,6 +459,9 @@ igt_main
igt_display_init(&data.display, data.drm_fd);
}
+ igt_subtest_f("cursor-size-change")
+ run_test(&data, test_cursor_size, cursor_width, cursor_height);
+
run_test_generic(&data, cursor_width);
igt_fixture {
--
1.8.3.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915: Always apply cursor width changes
On 06/02/2014 11:49 AM, Daniel Vetter wrote:
> On Fri, May 30, 2014 at 04:35:26PM +0300, Chris Wilson wrote:
>> It is possible for userspace to create a big object large enough for a
>> 256x256, and then switch over to using it as a 64x64 cursor. This
>> requires the cursor update routines to check for a change in width on
>> every update, rather than just when the cursor is originally enabled.
>>
>> This also fixes an issue with 845g/865g which cannot change the base
>> address of the cursor whilst it is active.
>>
>> Signed-off-by: Chris Wilson
>> [Antti:rebased, adjusted macro names and moved some lines, no functional
>> changes]
>> Reviewed-by: Antti Koskipaa
>> Tested-by: Antti Koskipaa
>
> Still missing the igt testcase. Is Antti still working on that one?
The testcase was just posted. It just needed some cleanup.
> And I
> guess now we also need an Cc: sta...@vger.kernel.org on this one here.
> -Daniel
>
>> ---
>> drivers/gpu/drm/i915/i915_debugfs.c | 2 +-
>> drivers/gpu/drm/i915/intel_display.c | 106
>> +--
>> drivers/gpu/drm/i915/intel_drv.h | 3 +-
>> 3 files changed, 53 insertions(+), 58 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
>> b/drivers/gpu/drm/i915/i915_debugfs.c
>> index 2e5f76a..04d0b7c 100644
>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>> @@ -2353,7 +2353,7 @@ static int i915_display_info(struct seq_file *m, void
>> *unused)
>>
>> active = cursor_position(dev, crtc->pipe, &x, &y);
>> seq_printf(m, "\tcursor visible? %s, position (%d, %d),
>> addr 0x%08x, active? %s\n",
>> - yesno(crtc->cursor_visible),
>> + yesno(crtc->cursor_base),
>> x, y, crtc->cursor_addr,
>> yesno(active));
>> }
>> diff --git a/drivers/gpu/drm/i915/intel_display.c
>> b/drivers/gpu/drm/i915/intel_display.c
>> index 731cd01..955f92d 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -7872,29 +7872,33 @@ static void i845_update_cursor(struct drm_crtc
>> *crtc, u32 base)
>> struct drm_device *dev = crtc->dev;
>> struct drm_i915_private *dev_priv = dev->dev_private;
>> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>> -bool visible = base != 0;
>> -u32 cntl;
>> +uint32_t cntl;
>>
>> -if (intel_crtc->cursor_visible == visible)
>> -return;
>> -
>> -cntl = I915_READ(_CURACNTR);
>> -if (visible) {
>> +if (base != intel_crtc->cursor_base) {
>> /* On these chipsets we can only modify the base whilst
>> * the cursor is disabled.
>> */
>> +if (intel_crtc->cursor_cntl) {
>> +I915_WRITE(_CURACNTR, 0);
>> +POSTING_READ(_CURACNTR);
>> +intel_crtc->cursor_cntl = 0;
>> +}
>> +
>> I915_WRITE(_CURABASE, base);
>> +POSTING_READ(_CURABASE);
>> +}
>>
>> -cntl &= ~(CURSOR_FORMAT_MASK);
>> -/* XXX width must be 64, stride 256 => 0x00 << 28 */
>> -cntl |= CURSOR_ENABLE |
>> +/* XXX width must be 64, stride 256 => 0x00 << 28 */
>> +cntl = 0;
>> +if (base)
>> +cntl = (CURSOR_ENABLE |
>> CURSOR_GAMMA_ENABLE |
>> -CURSOR_FORMAT_ARGB;
>> -} else
>> -cntl &= ~(CURSOR_ENABLE | CURSOR_GAMMA_ENABLE);
>> -I915_WRITE(_CURACNTR, cntl);
>> -
>> -intel_crtc->cursor_visible = visible;
>> +CURSOR_FORMAT_ARGB);
>> +if (intel_crtc->cursor_cntl != cntl) {
>> +I915_WRITE(_CURACNTR, cntl);
>> +POSTING_READ(_CURACNTR);
>> +intel_crtc->cursor_cntl = cntl;
>> +}
>> }
>>
>> static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base)
>> @@ -7903,16 +7907,12 @@ static void i9xx_update_cursor(struct drm_crtc
>> *crtc, u32 base)
>> struct drm_i915_private *dev_priv = dev->dev_private;
>> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>> int pipe = intel_crtc->pipe;
>> -bool visible = base != 0;
>> -
>> -if (intel_crtc->cursor_visible != visible) {
>> -int16_t width = intel_crtc->cursor_width;
>> -uint32_t cntl = I915_READ(CURCNTR(pipe));
>> -if (base) {
>> -cntl &= ~(CURSOR_MODE | MCURSOR_PIPE_SELECT);
>> -cntl |= MCURSOR_GAMMA_ENABLE;
>> +uint32_t cntl;
>>
>> -switch (width) {
>> +cntl = 0;
>> +if (base) {
>> +cntl = MCURSOR_GAMMA_ENABLE;
>> +switch (intel_crtc->cursor_width) {
>> case 64:
>> cntl |= CURSOR_MODE_64_ARGB_AX;
>> break;
>> @@ -7925,18 +7925,16 @@ stati
Re: [Intel-gfx] [PATCH v10] drm/i915: Replaced Blitter ring based flips with MMIO flips
On Mon, Jun 02, 2014 at 10:38:13AM +, Gupta, Sourab wrote:
> On Mon, 2014-06-02 at 06:56 +, Chris Wilson wrote:
> > On Sun, Jun 01, 2014 at 04:43:13PM +0530, sourab.gu...@intel.com wrote:
> > > +static bool use_mmio_flip(struct intel_engine_cs *ring,
> > > + struct drm_i915_gem_object *obj)
> > > +{
> > > + /* This is not being used for older platforms, because
> > > + * non-availability of flip done interrupt forces us to use
> > > + * CS flips. Older platforms derive flip done using some clever
> > > + * tricks involving the flip_pending status bits and vblank irqs.
> > > + * So using MMIO flips there would disrupt this mechanism.
> > > + */
> > > +
> > > + if (INTEL_INFO(ring->dev)->gen < 5)
> > > + return false;
> > > +
> > > + if (i915.use_mmio_flip < 0)
> > > + return false;
> > > + else if (i915.use_mmio_flip > 0)
> > > + return true;
> > > + else
> > > + return ring != obj->ring;
> > > +}
> >
> > Check whitespace.
> >
> Hi Chris,
> Couldn't get the whitespace error here, and at other places. Also,
> checkpatch.pl doesn't show any. Can you please point out the error.
It's the alignment of
/*
*
*/
that needs to be checked, and later the if (ret == 0) looks to be using
a mixture of tabs and spaces.
-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] drm/i915: enable PPGTT on VLV
On Thu, May 29, 2014 at 02:33:21PM -0700, Jesse Barnes wrote:
> Working for real this time. i915_ppgtt_info has all sorts of good stuff
> in it and X is running nicely on top.
>
> Signed-off-by: Jesse Barnes
So it wasn't just my vlv where it appears to work. That's nice.
Reviewed-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/i915_drv.h | 6 ++
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index bea9ab40..8631fb3 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1936,10 +1936,8 @@ struct drm_i915_cmd_table {
> #define I915_NEED_GFX_HWS(dev) (INTEL_INFO(dev)->need_gfx_hws)
>
> #define HAS_HW_CONTEXTS(dev) (INTEL_INFO(dev)->gen >= 6)
> -#define HAS_ALIASING_PPGTT(dev) (INTEL_INFO(dev)->gen >= 6 && \
> - (!IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)))
> -#define HAS_PPGTT(dev) (INTEL_INFO(dev)->gen >= 7 \
> - && !IS_GEN8(dev))
> +#define HAS_ALIASING_PPGTT(dev) (INTEL_INFO(dev)->gen >= 6)
> +#define HAS_PPGTT(dev) (INTEL_INFO(dev)->gen >= 7 &&
> !IS_GEN8(dev))
> #define USES_PPGTT(dev) intel_enable_ppgtt(dev, false)
> #define USES_FULL_PPGTT(dev) intel_enable_ppgtt(dev, true)
>
> --
> 1.9.1
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
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/3] drm/i915: Change Mipi register definitions
On Sun, Jun 01, 2014 at 06:41:54AM +0100, Sharma, Shashank wrote: > Hi Damien, > Please correct me if I am missing something, but the only reason we > are seeing those extra formatting changes is, almost all of the old > MIPI register definitions were beyond 80 char, and was checked in like > that with warnings (How ?) So when I was checking for checkpatch > errors, I saw that, and I tried to fix that in the previous patch. You're absolutely correct. However that 80 chars limit is a soft one and I think, in that case, being able to review the patch wins. Would someone think it's a good idea to have those definitions under 80 chars, one could always send a patch *just* changing that. I wouln't bother. -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v11] drm/i915: Replaced Blitter ring based flips with MMIO flips
From: Sourab Gupta
This patch enables the framework for using MMIO based flip calls,
in contrast with the CS based flip calls which are being used currently.
MMIO based flip calls can be enabled on architectures where
Render and Blitter engines reside in different power wells. The
decision to use MMIO flips can be made based on workloads to give
100% residency for Media power well.
v2: The MMIO flips now use the interrupt driven mechanism for issuing the
flips when target seqno is reached. (Incorporating Ville's idea)
v3: Rebasing on latest code. Code restructuring after incorporating
Damien's comments
v4: Addressing Ville's review comments
-general cleanup
-updating only base addr instead of calling update_primary_plane
-extending patch for gen5+ platforms
v5: Addressed Ville's review comments
-Making mmio flip vs cs flip selection based on module parameter
-Adding check for DRIVER_MODESET feature in notify_ring before calling
notify mmio flip.
-Other changes mostly in function arguments
v6: -Having a seperate function to check condition for using mmio flips (Ville)
-propogating error code from i915_gem_check_olr (Ville)
v7: -Adding __must_check with i915_gem_check_olr (Chris)
-Renaming mmio_flip_data to mmio_flip (Chris)
-Rebasing on latest nightly
v8: -Rebasing on latest code
-squash 3rd patch in series(mmio setbase vs page flip race) with this patch
-Added new tiling mode update in intel_do_mmio_flip (Chris)
v9: -check for obj->last_write_seqno being 0 instead of obj->ring being NULL in
intel_postpone_flip, as this is a more restrictive condition (Chris)
v10: -Applied Chris's suggestions for squashing patches 2,3 into this patch.
These patches make the selection of CS vs MMIO flip at the page flip time, and
make the module parameter for using mmio flips as tristate, the states being
'force CS flips', 'force mmio flips', 'driver discretion'.
Changed the logic for driver discretion (Chris)
v11: Minor code cleanup(better readability, fixing whitespace errors, using
lockdep to check mutex locked status in postpone_flip, removal of __must_check
in function definition) (Chris)
Reviewed-by: Chris Wilson
Signed-off-by: Sourab Gupta
Signed-off-by: Akash Goel
Tested-by: Chris Wilson # snb, ivb
---
drivers/gpu/drm/i915/i915_dma.c | 1 +
drivers/gpu/drm/i915/i915_drv.h | 8 ++
drivers/gpu/drm/i915/i915_gem.c | 2 +-
drivers/gpu/drm/i915/i915_irq.c | 3 +
drivers/gpu/drm/i915/i915_params.c | 5 ++
drivers/gpu/drm/i915/intel_display.c | 148 ++-
drivers/gpu/drm/i915/intel_drv.h | 6 ++
7 files changed, 171 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index b9159ad..532733a 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1572,6 +1572,7 @@ 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);
+ spin_lock_init(&dev_priv->mmio_flip_lock);
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 bea9ab40..4d5dbec 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1367,6 +1367,9 @@ struct drm_i915_private {
/* protects the irq masks */
spinlock_t irq_lock;
+ /* protects the mmio flip data */
+ spinlock_t mmio_flip_lock;
+
bool display_irqs_enabled;
/* To control wakeup latency, e.g. for irq-driven dp aux transfers. */
@@ -2036,6 +2039,7 @@ struct i915_params {
bool reset;
bool disable_display;
bool disable_vtd_wa;
+ int use_mmio_flip;
};
extern struct i915_params i915 __read_mostly;
@@ -2231,6 +2235,8 @@ bool i915_gem_retire_requests(struct drm_device *dev);
void i915_gem_retire_requests_ring(struct intel_engine_cs *ring);
int __must_check i915_gem_check_wedge(struct i915_gpu_error *error,
bool interruptible);
+int __must_check i915_gem_check_olr(struct intel_engine_cs *ring, u32 seqno);
+
static inline bool i915_reset_in_progress(struct i915_gpu_error *error)
{
return unlikely(atomic_read(&error->reset_counter)
@@ -2601,6 +2607,8 @@ int i915_reg_read_ioctl(struct drm_device *dev, void
*data,
int i915_get_reset_stats_ioctl(struct drm_device *dev, void *data,
struct drm_file *file);
+void intel_notify_mmio_flip(struct intel_engine_cs *ring);
+
/* overlay */
extern struct intel_overlay_error_state
*intel_overlay_capture_error_state(struct drm_device *dev);
extern void intel_overlay_print_error_state(struct drm_i915_error_state_buf *e,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/g
[Intel-gfx] [PATCH] drm/i915: fix display power sw state reporting
Atm, we refcount both power domains and power wells and
intel_display_power_enabled_sw() returns the power domain refcount. What
the callers are really interested in though is the sw state of the
underlying power wells. Due to this we will report incorrectly that a
given power domain is off if its power wells were enabled via another
power domain, for example POWER_DOMAIN_INIT which enables all power
wells.
As a fix return instead the state based on the refcount of all power
wells included in the passed in power domain.
References: https://bugs.freedesktop.org/show_bug.cgi?id=79505
References: https://bugs.freedesktop.org/show_bug.cgi?id=79038
Reported-by: Chris Wilson
Signed-off-by: Imre Deak
---
drivers/gpu/drm/i915/intel_pm.c | 17 -
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 1840d15..ee27d74 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5805,10 +5805,25 @@ bool intel_display_power_enabled_sw(struct
drm_i915_private *dev_priv,
enum intel_display_power_domain domain)
{
struct i915_power_domains *power_domains;
+ struct i915_power_well *power_well;
+ bool is_enabled;
+ int i;
+
+ if (dev_priv->pm.suspended)
+ return false;
power_domains = &dev_priv->power_domains;
+ is_enabled = true;
+ for_each_power_well_rev(i, power_well, BIT(domain), power_domains) {
+ if (power_well->always_on)
+ continue;
- return power_domains->domain_use_count[domain];
+ if (!power_well->count) {
+ is_enabled = false;
+ break;
+ }
+ }
+ return is_enabled;
}
bool intel_display_power_enabled(struct drm_i915_private *dev_priv,
--
1.8.4
___
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: Change Mipi register definitions
On Sun, Jun 01, 2014 at 07:24:47PM +0530, Shashank Sharma wrote: > > /* XXX: all bits reserved */ > -#define _MIPIA_AUTOPWG (VLV_DISPLAY_BASE + > 0x611a0) > +#define _MIPIA_AUTOPWG (dev_priv->mipi_mmio_base + > 0x611a0) This one isn't part of the MIPI block address space IIUC, so we shouldn't express it as mipi_mmio_base + offset. Let's leave VLV_DISPLAY_BASE there. > -#define _MIPIB_READ_DATA_VALID (VLV_DISPLAY_BASE + > 0xb938) > +#define _MIPIA_READ_DATA_VALID (dev_priv->mipi_mmio_base + > 0xb138) > +#define _MIPIB_READ_DATA_VALID (dev_priv->mipi_mmio_base + > 0xb938) > #define MIPI_READ_DATA_VALID(pipe) _PIPE(pipe, _MIPIA_READ_DATA_VALID, > _MIPIB_READ_DATA_VALID) > #define READ_DATA_VALID(n) (1 << (n)) > > + > /* For UMS only (deprecated): */ That's an extra line here. Details, sure, but that's the rigour expected from submissions. HTH, -- Damien ___ 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: Boost GPU frequency if we detect outstanding pageflips
On Mon, Jun 02, 2014 at 10:53:57AM +0200, Daniel Vetter wrote: > On Fri, May 30, 2014 at 05:16:36PM +0100, Chris Wilson wrote: > > + outstanding = (intel_crtc->unpin_work != NULL && > > + crtc_sbc(intel_crtc) - intel_crtc->unpin_work->sbc > 1); > > s/outstanding/still_pending/? Since the situation is very much not > oustanding at all ... I changed it to missed_vblank = blah; if (missed_vblank) queue_boost(). -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 4/4] drm/i915: make sure PC8 is enabled on suspend and disabled on resume
On Mon, 2014-06-02 at 10:45 +0200, Daniel Vetter wrote:
> On Thu, May 29, 2014 at 02:11:37PM -0700, Jesse Barnes wrote:
> > From: Kristen Carlson Accardi
> >
> > This matches the runtime suspend paths and allows the system to enter
> > the lowest power mode at freeze time.
> >
> > Signed-off-by: Kristen Carlson Accardi
> > Signed-off-by: Jesse Barnes
>
> pc8 is fully subsumed into runtime pm by now. Do we _really_ still need
> this?
Yes, since the system suspend/resume handlers are called with an RPM ref
held and thus PC8 disabled.
--Imre
> -Daniel
>
> > ---
> > drivers/gpu/drm/i915/i915_drv.c | 6 ++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c
> > b/drivers/gpu/drm/i915/i915_drv.c
> > index b6211d7..24dc856 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -558,6 +558,9 @@ static int i915_drm_freeze(struct drm_device *dev)
> >
> > intel_display_set_init_power(dev_priv, false);
> >
> > + if (IS_HASWELL(dev) || IS_BROADWELL(dev))
> > + hsw_enable_pc8(dev_priv);
> > +
> > return 0;
> > }
> >
> > @@ -618,6 +621,9 @@ static int __i915_drm_thaw(struct drm_device *dev, bool
> > restore_gtt_mappings)
> > {
> > struct drm_i915_private *dev_priv = dev->dev_private;
> >
> > + if (IS_HASWELL(dev) || IS_BROADWELL(dev))
> > + hsw_disable_pc8(dev_priv);
> > +
> > if (drm_core_check_feature(dev, DRIVER_MODESET) &&
> > restore_gtt_mappings) {
> > mutex_lock(&dev->struct_mutex);
> > --
> > 1.9.1
> >
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
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
[Intel-gfx] [PATCH 2/3] drm/i915: Change Mipi register definitions
From: Shashank Sharma
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.
V4: Addressing review comments by Damien and Ville, splitting into two patches
This patch removes all the un-necessary formatting changes from previous patch.
V5: Removed 80 char limit formatting for existing MIPI regs
V6: Removed extra space, change one definition
Signed-off-by: Shashank Sharma
---
drivers/gpu/drm/i915/i915_reg.h | 183
1 file changed, 93 insertions(+), 90 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index c12a858..dd2ce82 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5706,12 +5706,12 @@ enum punit_power_well {
#define TEARING_EFFECT_DELAY_MASK (0x << 0)
/* XXX: all bits reserved */
-#define _MIPIA_AUTOPWG (VLV_DISPLAY_BASE + 0x611a0)
+#define _MIPIA_AUTOPWG (VLV_DISPLAY_BASE + 0x611a0)
/* MIPI DSI Controller and D-PHY registers */
-#define _MIPIA_DEVICE_READY(VLV_DISPLAY_BASE + 0xb000)
-#define _MIPIB_DEVICE_READY(VLV_DISPLAY_BASE + 0xb800)
+#define _MIPIA_DEVICE_READY(dev_priv->mipi_mmio_base + 0xb000)
+#define _MIPIB_DEVICE_READY(dev_priv->mipi_mmio_base + 0xb800)
#define MIPI_DEVICE_READY(pipe)_PIPE(pipe,
_MIPIA_DEVICE_READY, _MIPIB_DEVICE_READY)
#define BUS_POSSESSION(1 << 3) /* set
to give bus to receiver */
#define ULPS_STATE_MASK (3 << 1)
@@ -5720,11 +5720,11 @@ enum punit_power_well {
#define ULPS_STATE_NORMAL_OPERATION (0 << 1)
#define DEVICE_READY (1 << 0)
-#define _MIPIA_INTR_STAT (VLV_DISPLAY_BASE + 0xb004)
-#define _MIPIB_INTR_STAT (VLV_DISPLAY_BASE + 0xb804)
+#define _MIPIA_INTR_STAT (dev_priv->mipi_mmio_base + 0xb004)
+#define _MIPIB_INTR_STAT (dev_priv->mipi_mmio_base + 0xb804)
#define MIPI_INTR_STAT(pipe) _PIPE(pipe, _MIPIA_INTR_STAT,
_MIPIB_INTR_STAT)
-#define _MIPIA_INTR_EN (VLV_DISPLAY_BASE + 0xb008)
-#define _MIPIB_INTR_EN (VLV_DISPLAY_BASE + 0xb808)
+#define _MIPIA_INTR_EN (dev_priv->mipi_mmio_base + 0xb008)
+#define _MIPIB_INTR_EN (dev_priv->mipi_mmio_base + 0xb808)
#define MIPI_INTR_EN(pipe) _PIPE(pipe, _MIPIA_INTR_EN,
_MIPIB_INTR_EN)
#define TEARING_EFFECT(1 << 31)
#define SPL_PKT_SENT_INTERRUPT(1 << 30)
@@ -5759,8 +5759,8 @@ enum punit_power_well {
#define RXSOT_SYNC_ERROR (1 << 1)
#define RXSOT_ERROR (1 << 0)
-#define _MIPIA_DSI_FUNC_PRG(VLV_DISPLAY_BASE + 0xb00c)
-#define _MIPIB_DSI_FUNC_PRG(VLV_DISPLAY_BASE + 0xb80c)
+#define _MIPIA_DSI_FUNC_PRG(dev_priv->mipi_mmio_base + 0xb00c)
+#define _MIPIB_DSI_FUNC_PRG(dev_priv->mipi_mmio_base + 0xb80c)
#define MIPI_DSI_FUNC_PRG(pipe)_PIPE(pipe,
_MIPIA_DSI_FUNC_PRG, _MIPIB_DSI_FUNC_PRG)
#define CMD_MODE_DATA_WIDTH_MASK (7 << 13)
#define CMD_MODE_NOT_SUPPORTED(0 << 13)
@@ -5782,77 +5782,78 @@ enum punit_power_well {
#define DATA_LANES_PRG_REG_SHIFT 0
#define DATA_LANES_PRG_REG_MASK (7 << 0)
-#define _MIPIA_HS_TX_TIMEOUT (VLV_DISPLAY_BASE + 0xb010)
-#define _MIPIB_HS_TX_TIMEOUT (VLV_DISPLAY_BASE + 0xb810)
+#define _MIPIA_HS_TX_TIMEOUT (dev_priv->mipi_mmio_base + 0xb010)
+#define _MIPIB_HS_TX_TIMEOUT (dev_priv->mipi_mmio_base + 0xb810)
#define MIPI_HS_TX_TIMEOUT(pipe) _PIPE(pipe, _MIPIA_HS_TX_TIMEOUT,
_MIPIB_HS_TX_TIMEOUT)
#define HIGH_SPEED_TX_TIMEOUT_COUNTER_MASK0xff
-#define _MIPIA_LP_RX_TIMEOUT (VLV_DISPLAY_BASE + 0xb014)
-#define _MIPIB_LP_RX_TIMEOUT (VLV_DISPLAY_BASE + 0xb814)
+#define _MIPIA_LP_RX_TIMEOUT (dev_priv->mipi_mmio_base + 0xb014)
+#define _MIPIB_LP_RX_TIMEOUT (dev_priv->mipi_mmio_base + 0xb814)
#define MIPI_LP_RX_TIMEOUT(pipe) _PIPE(pipe, _MIPIA_LP_RX_TIMEOUT,
_MIPIB_LP_RX_TIMEOUT)
#define LOW_POWER_RX_TIMEOUT_COUNTER_MASK 0xff
-#define _MIPIA_TURN_AROUND_TIMEOUT (VLV_DISPLAY_BASE + 0xb018)
-#define _MIPIB_TURN_AROUND_TIMEOUT (VLV_DISPLAY_BASE + 0xb818)
+#define _MIPIA_TURN_AROUND_TIMEOUT (dev_priv->mipi_mmi
[Intel-gfx] [PATCH 3/3] drm/i915: Use transcoder as index to MIPI regs
From: Shashank Sharma
Conceptually, the MIPI registers are addressed by the MIPI transcoder
index, not the pipe. It doesn't matter right now, because there's a
1:1 relationship between pipes and MIPI transcoders, but that change
allows us to break that link in the future
V1: Created new patch to address Damien's review comment.
Replacing _PIPE calls to _TRANSCODER calls
V2: Re-basing on patch 2
V3: Re-basing on patch 2
V4: Re-basing on patch 2
Signed-off-by: Shashank Sharma
---
drivers/gpu/drm/i915/i915_reg.h | 140 ++--
1 file changed, 93 insertions(+), 47 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index dd2ce82..e449195 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5659,7 +5659,8 @@ enum punit_power_well {
#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 MIPI_PORT_CTRL(tc) _TRANSCODER(tc, _MIPIA_PORT_CTRL, \
+ _MIPIB_PORT_CTRL)
#define DPI_ENABLE(1 << 31) /* A + B */
#define MIPIA_MIPI4DPHY_DELAY_COUNT_SHIFT 27
#define MIPIA_MIPI4DPHY_DELAY_COUNT_MASK (0xf << 27)
@@ -5701,7 +5702,8 @@ enum punit_power_well {
#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 MIPI_TEARING_CTRL(tc) _TRANSCODER(tc, \
+ _MIPIA_TEARING_CTRL, _MIPIB_TEARING_CTRL)
#define TEARING_EFFECT_DELAY_SHIFT0
#define TEARING_EFFECT_DELAY_MASK (0x << 0)
@@ -5712,7 +5714,8 @@ enum punit_power_well {
#define _MIPIA_DEVICE_READY(dev_priv->mipi_mmio_base + 0xb000)
#define _MIPIB_DEVICE_READY(dev_priv->mipi_mmio_base + 0xb800)
-#define MIPI_DEVICE_READY(pipe)_PIPE(pipe,
_MIPIA_DEVICE_READY, _MIPIB_DEVICE_READY)
+#define MIPI_DEVICE_READY(tc) _TRANSCODER(tc, _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)
@@ -5722,10 +5725,12 @@ enum punit_power_well {
#define _MIPIA_INTR_STAT (dev_priv->mipi_mmio_base + 0xb004)
#define _MIPIB_INTR_STAT (dev_priv->mipi_mmio_base + 0xb804)
-#define MIPI_INTR_STAT(pipe) _PIPE(pipe, _MIPIA_INTR_STAT,
_MIPIB_INTR_STAT)
+#define MIPI_INTR_STAT(tc) _TRANSCODER(tc, _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(pipe) _PIPE(pipe, _MIPIA_INTR_EN,
_MIPIB_INTR_EN)
+#define MIPI_INTR_EN(tc) _TRANSCODER(tc, _MIPIA_INTR_EN, \
+ _MIPIB_INTR_EN)
#define TEARING_EFFECT(1 << 31)
#define SPL_PKT_SENT_INTERRUPT(1 << 30)
#define GEN_READ_DATA_AVAIL (1 << 29)
@@ -5761,7 +5766,8 @@ enum punit_power_well {
#define _MIPIA_DSI_FUNC_PRG(dev_priv->mipi_mmio_base + 0xb00c)
#define _MIPIB_DSI_FUNC_PRG(dev_priv->mipi_mmio_base + 0xb80c)
-#define MIPI_DSI_FUNC_PRG(pipe)_PIPE(pipe,
_MIPIA_DSI_FUNC_PRG, _MIPIB_DSI_FUNC_PRG)
+#define MIPI_DSI_FUNC_PRG(tc) _TRANSCODER(tc, _MIPIA_DSI_FUNC_PRG, \
+ _MIPIB_DSI_FUNC_PRG)
#define CMD_MODE_DATA_WIDTH_MASK (7 << 13)
#define CMD_MODE_NOT_SUPPORTED(0 << 13)
#define CMD_MODE_DATA_WIDTH_16_BIT(1 << 13)
@@ -5784,27 +5790,32 @@ enum punit_power_well {
#define _MIPIA_HS_TX_TIMEOUT (dev_priv->mipi_mmio_base + 0xb010)
#define _MIPIB_HS_TX_TIMEOUT (dev_priv->mipi_mmio_base + 0xb810)
-#define MIPI_HS_TX_TIMEOUT(pipe) _PIPE(pipe, _MIPIA_HS_TX_TIMEOUT,
_MIPIB_HS_TX_TIMEOUT)
+#define MIPI_HS_TX_TIMEOUT(tc) _TRANSCODER(tc, _MIPIA_HS_TX_TIMEOUT, \
+ _MIPIB_HS_TX_TIMEOUT)
#define HIGH_SPEED_TX_TIMEOUT_COUNTER_MASK0xff
#define _MIPIA_LP_RX_TIMEOUT (dev_priv->mipi_mmio_base + 0xb014)
#define _MIPIB_LP_RX_TIMEOUT (dev_priv->mi
Re: [Intel-gfx] [PATCH 2/3] drm/i915: Change Mipi register definitions
On Mon, Jun 02, 2014 at 06:07:47PM +0530, shashank.sha...@intel.com wrote:
> From: Shashank Sharma
>
> 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.
>
> V4: Addressing review comments by Damien and Ville, splitting into two patches
> This patch removes all the un-necessary formatting changes from previous
> patch.
> V5: Removed 80 char limit formatting for existing MIPI regs
> V6: Removed extra space, change one definition
>
> Signed-off-by: Shashank Sharma
Reviewed-by: Damien Lespiau
--
Damien
> ---
> drivers/gpu/drm/i915/i915_reg.h | 183
>
> 1 file changed, 93 insertions(+), 90 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index c12a858..dd2ce82 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5706,12 +5706,12 @@ enum punit_power_well {
> #define TEARING_EFFECT_DELAY_MASK (0x << 0)
>
> /* XXX: all bits reserved */
> -#define _MIPIA_AUTOPWG (VLV_DISPLAY_BASE +
> 0x611a0)
> +#define _MIPIA_AUTOPWG (VLV_DISPLAY_BASE + 0x611a0)
>
> /* MIPI DSI Controller and D-PHY registers */
>
> -#define _MIPIA_DEVICE_READY (VLV_DISPLAY_BASE + 0xb000)
> -#define _MIPIB_DEVICE_READY (VLV_DISPLAY_BASE + 0xb800)
> +#define _MIPIA_DEVICE_READY (dev_priv->mipi_mmio_base + 0xb000)
> +#define _MIPIB_DEVICE_READY (dev_priv->mipi_mmio_base + 0xb800)
> #define MIPI_DEVICE_READY(pipe) _PIPE(pipe,
> _MIPIA_DEVICE_READY, _MIPIB_DEVICE_READY)
> #define BUS_POSSESSION (1 << 3) /* set
> to give bus to receiver */
> #define ULPS_STATE_MASK (3 << 1)
> @@ -5720,11 +5720,11 @@ enum punit_power_well {
> #define ULPS_STATE_NORMAL_OPERATION (0 << 1)
> #define DEVICE_READY(1 << 0)
>
> -#define _MIPIA_INTR_STAT (VLV_DISPLAY_BASE + 0xb004)
> -#define _MIPIB_INTR_STAT (VLV_DISPLAY_BASE + 0xb804)
> +#define _MIPIA_INTR_STAT (dev_priv->mipi_mmio_base + 0xb004)
> +#define _MIPIB_INTR_STAT (dev_priv->mipi_mmio_base + 0xb804)
> #define MIPI_INTR_STAT(pipe) _PIPE(pipe, _MIPIA_INTR_STAT,
> _MIPIB_INTR_STAT)
> -#define _MIPIA_INTR_EN (VLV_DISPLAY_BASE +
> 0xb008)
> -#define _MIPIB_INTR_EN (VLV_DISPLAY_BASE +
> 0xb808)
> +#define _MIPIA_INTR_EN (dev_priv->mipi_mmio_base +
> 0xb008)
> +#define _MIPIB_INTR_EN (dev_priv->mipi_mmio_base +
> 0xb808)
> #define MIPI_INTR_EN(pipe) _PIPE(pipe, _MIPIA_INTR_EN,
> _MIPIB_INTR_EN)
> #define TEARING_EFFECT (1 << 31)
> #define SPL_PKT_SENT_INTERRUPT (1 << 30)
> @@ -5759,8 +5759,8 @@ enum punit_power_well {
> #define RXSOT_SYNC_ERROR(1 << 1)
> #define RXSOT_ERROR (1 << 0)
>
> -#define _MIPIA_DSI_FUNC_PRG (VLV_DISPLAY_BASE + 0xb00c)
> -#define _MIPIB_DSI_FUNC_PRG (VLV_DISPLAY_BASE + 0xb80c)
> +#define _MIPIA_DSI_FUNC_PRG (dev_priv->mipi_mmio_base + 0xb00c)
> +#define _MIPIB_DSI_FUNC_PRG (dev_priv->mipi_mmio_base + 0xb80c)
> #define MIPI_DSI_FUNC_PRG(pipe) _PIPE(pipe,
> _MIPIA_DSI_FUNC_PRG, _MIPIB_DSI_FUNC_PRG)
> #define CMD_MODE_DATA_WIDTH_MASK(7 << 13)
> #define CMD_MODE_NOT_SUPPORTED (0 << 13)
> @@ -5782,77 +5782,78 @@ enum punit_power_well {
> #define DATA_LANES_PRG_REG_SHIFT0
> #define DATA_LANES_PRG_REG_MASK (7 << 0)
>
> -#define _MIPIA_HS_TX_TIMEOUT (VLV_DISPLAY_BASE + 0xb010)
> -#define _MIPIB_HS_TX_TIMEOUT (VLV_DISPLAY_BASE + 0xb810)
> +#define _MIPIA_HS_TX_TIMEOUT (dev_priv->mipi_mmio_base + 0xb010)
> +#define _MIPIB_HS_TX_TIMEOUT (dev_priv->mipi_mmio_base + 0xb810)
> #define MIPI_HS_TX_TIMEOUT(pipe) _PIPE(pipe, _MIPIA_HS_TX_TIMEOUT,
> _MIPIB_HS_TX_TIMEOUT)
> #define HIGH_SPEED_TX_TIMEOUT_COUNTER_MASK 0xff
>
> -#define _MIPIA_LP_RX_TIMEOUT (VLV_DISPLAY_BASE + 0xb014)
> -#define _MIPIB_LP_RX_TIMEOUT (VLV_DISPLAY_BASE + 0xb814)
> +#define _MIPIA_LP_RX_TIMEOUT (dev_priv->mipi_mmio_base + 0xb014)
> +#define _MIPIB_LP_RX_TIMEOUT (dev_priv->mipi_mmio_base + 0xb814)
> #define MIPI_LP_RX_TIMEOUT(pipe) _PIPE(pipe, _MIPIA_LP_RX_TIMEOUT,
> _MIPIB_LP_RX_TIMEOUT)
>
Re: [Intel-gfx] [PATCH 3/3] drm/i915: Use transcoder as index to MIPI regs
On Mon, Jun 02, 2014 at 06:07:48PM +0530, shashank.sha...@intel.com wrote: > From: Shashank Sharma > > Conceptually, the MIPI registers are addressed by the MIPI transcoder > index, not the pipe. It doesn't matter right now, because there's a > 1:1 relationship between pipes and MIPI transcoders, but that change > allows us to break that link in the future > > V1: Created new patch to address Damien's review comment. > Replacing _PIPE calls to _TRANSCODER calls > V2: Re-basing on patch 2 > V3: Re-basing on patch 2 > V4: Re-basing on patch 2 > > Signed-off-by: Shashank Sharma You still have a few inconsistencies here and there because you tried to get everything under 80 chars in a previous version. Oh well. I guess we can have a pass on top if someone's OCD is uncontrollable. Reviewed-by: Damien Lespiau -- Damien ___ 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 transcoder as index to MIPI regs
Hi Damien, Can you please point out these, as this patch is re-based on latest 2/3, I was expecting this to be without any inconsistency. I personally checked for any <80 char formatting, which is not required. But if I missed any, I can again fix this, please let me know. Regards Shashank -Original Message- From: Lespiau, Damien Sent: Monday, June 02, 2014 6:22 PM To: Sharma, Shashank Cc: intel-gfx@lists.freedesktop.org; ville.syrj...@linux.intel.com; Vetter, Daniel; Kumar, Shobhit Subject: Re: [PATCH 3/3] drm/i915: Use transcoder as index to MIPI regs On Mon, Jun 02, 2014 at 06:07:48PM +0530, shashank.sha...@intel.com wrote: > From: Shashank Sharma > > Conceptually, the MIPI registers are addressed by the MIPI transcoder > index, not the pipe. It doesn't matter right now, because there's a > 1:1 relationship between pipes and MIPI transcoders, but that change > allows us to break that link in the future > > V1: Created new patch to address Damien's review comment. > Replacing _PIPE calls to _TRANSCODER calls > V2: Re-basing on patch 2 > V3: Re-basing on patch 2 > V4: Re-basing on patch 2 > > Signed-off-by: Shashank Sharma You still have a few inconsistencies here and there because you tried to get everything under 80 chars in a previous version. Oh well. I guess we can have a pass on top if someone's OCD is uncontrollable. Reviewed-by: Damien Lespiau -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: fix display power sw state reporting
On Mon, Jun 02, 2014 at 02:21:10PM +0300, Imre Deak wrote:
> Atm, we refcount both power domains and power wells and
> intel_display_power_enabled_sw() returns the power domain refcount. What
> the callers are really interested in though is the sw state of the
> underlying power wells. Due to this we will report incorrectly that a
> given power domain is off if its power wells were enabled via another
> power domain, for example POWER_DOMAIN_INIT which enables all power
> wells.
>
> As a fix return instead the state based on the refcount of all power
> wells included in the passed in power domain.
>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=79505
> References: https://bugs.freedesktop.org/show_bug.cgi?id=79038
> Reported-by: Chris Wilson
> Signed-off-by: Imre Deak
Reviewed-by: Damien Lespiau
--
Damien
> ---
> drivers/gpu/drm/i915/intel_pm.c | 17 -
> 1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 1840d15..ee27d74 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5805,10 +5805,25 @@ bool intel_display_power_enabled_sw(struct
> drm_i915_private *dev_priv,
> enum intel_display_power_domain domain)
> {
> struct i915_power_domains *power_domains;
> + struct i915_power_well *power_well;
> + bool is_enabled;
> + int i;
> +
> + if (dev_priv->pm.suspended)
> + return false;
>
> power_domains = &dev_priv->power_domains;
> + is_enabled = true;
> + for_each_power_well_rev(i, power_well, BIT(domain), power_domains) {
> + if (power_well->always_on)
> + continue;
>
> - return power_domains->domain_use_count[domain];
> + if (!power_well->count) {
> + is_enabled = false;
> + break;
> + }
> + }
> + return is_enabled;
> }
>
> bool intel_display_power_enabled(struct drm_i915_private *dev_priv,
> --
> 1.8.4
>
> ___
> 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
Re: [Intel-gfx] [PATCH 3/3] drm/i915: Use transcoder as index to MIPI regs
On Mon, Jun 02, 2014 at 01:55:13PM +0100, Sharma, Shashank wrote: > Hi Damien, > > Can you please point out these, as this patch is re-based on latest > 2/3, I was expecting this to be without any inconsistency. > I personally checked for any <80 char formatting, which is not > required. But if I missed any, I can again fix this, please let me > know. At this point, there's no "rule". As Daniel said earlier the 80 chars limit is a soft one, esp. in headers declaring list of registers. For the inconsistencies, it's just a personal preference, I would try to make all defines look alike, right now you have: #define MIPI_DPI_CONTROL(tc)_TRANSCODER(tc, _MIPIA_DPI_CONTROL, \ _MIPIB_DPI_CONTROL) #define MIPI_GEN_FIFO_STAT(tc) _TRANSCODER(tc, _MIPIA_GEN_FIFO_STAT, \ _MIPIB_GEN_FIFO_STAT) #define MIPI_READ_DATA_VALID(tc)_TRANSCODER(tc, \ _MIPIA_READ_DATA_VALID, _MIPIB_READ_DATA_VALID) All different alignments. Not something I would ever do, but there's no rule against it per se, hence the r-b. You have a couple more of debatable splits: #define _MIPIA_CLK_LANE_SWITCH_TIME_CNT (dev_priv->mipi_mmio_base \ + 0xb088) #define MIPI_READ_DATA_RETURN(tc, n) \ (_TRANSCODER(tc, _MIPIA_READ_DATA_RETURN0, _MIPIB_READ_DATA_RETURN0) \ + 4 * (n)) /* n: 0...7 */ Esp. for the first one, these are cases where the "< 80 chars" split goes against readibility. Someone may ask you to fix those "bad" splits, not me this time though. -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: avoid unclaimed registers when capturing the error state
On Fri, Dec 20, 2013 at 03:09:41PM -0200, Paulo Zanoni wrote: > From: Paulo Zanoni > > We're iterating over the CPU transcoders, so check for the correct > power domain. > > This fixes many "unclaimed register" error messages. > > This can be reproduced by the IGT test mentioned below, but we still > get a FAIL when we run it. > > Testcase: igt/kms_lip/flip-vs-panning-vs-hang > Signed-off-by: Paulo Zanoni This of course breaks error capture on every platform that doesn't use rpm, and even then is spectacularly useless on those that 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
Re: [Intel-gfx] [PATCH 2/2] drm/i915: vlv/chv: fix DSI sideband register accessing
On 5/26/2014 6:22 PM, Jani Nikula wrote:
On Mon, 19 May 2014, Imre Deak wrote:
So far we used the wrong opcodes to access the DSI registers, so the
register writes during DSI programming didn't actually succeed and left
the registers unchanged. This wasn't a problem for the initial modeset,
where the BIOS-programmed values happened to work, but after resuming
from s0ix these registers are reset and failing to program them results
in a blank screen.
Shobhit, this was already merged, did you have a chance to test it?
Sorry just saw this today. Yeah I have pulled this in my test tree and
suspend/resume was working perfect, but then I was not doing display
powergate. In the internal code though S0ix was implemented. I will
double check if the internal code had correct opcodes and somehow wrong
opcodes trickled into intel-gfx. As of now looks okay and works fine for me.
Regards
Shobhit
Jani.
Signed-off-by: Imre Deak
---
drivers/gpu/drm/i915/intel_sideband.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_sideband.c
b/drivers/gpu/drm/i915/intel_sideband.c
index f3909d5..01d841e 100644
--- a/drivers/gpu/drm/i915/intel_sideband.c
+++ b/drivers/gpu/drm/i915/intel_sideband.c
@@ -270,13 +270,13 @@ void intel_sbi_write(struct drm_i915_private *dev_priv,
u16 reg, u32 value,
u32 vlv_flisdsi_read(struct drm_i915_private *dev_priv, u32 reg)
{
u32 val = 0;
- vlv_sideband_rw(dev_priv, DPIO_DEVFN, IOSF_PORT_FLISDSI, SB_MRD_NP,
+ vlv_sideband_rw(dev_priv, DPIO_DEVFN, IOSF_PORT_FLISDSI, SB_CRRDDA_NP,
reg, &val);
return val;
}
void vlv_flisdsi_write(struct drm_i915_private *dev_priv, u32 reg, u32 val)
{
- vlv_sideband_rw(dev_priv, DPIO_DEVFN, IOSF_PORT_FLISDSI, SB_MWR_NP,
+ vlv_sideband_rw(dev_priv, DPIO_DEVFN, IOSF_PORT_FLISDSI, SB_CRWRDA_NP,
reg, &val);
}
--
1.8.4
___
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] drm/i915: tell the user if both KMS and UMS are disabled
If both KMS is disabled (by i915.modeset=0 or nomodeset parameters) and
UMS is disabled (by CONFIG_DRM_I915_UMS=n, the default), the user might
not be aware his setup is not supported. Inform the users (and, by
extension, the poor i915 developers having to read their dmesgs in bug
reports) why their graphics experience might be lacking.
A similar message was added on the UMS path in
commit e147accbd19f55489dabdcc4dc3551cc3e3f2553
Author: Jani Nikula
Date: Thu Oct 10 15:25:37 2013 +0300
drm/i915: tell the user KMS is required for gen6+
but it won't be reached if CONFIG_DRM_I915_UMS=n since
commit b30324adaf8d2e5950a602bde63030d15a61826f
Author: Daniel Vetter
Date: Wed Nov 13 22:11:25 2013 +0100
drm/i915: Deprecated UMS support
v2: Use DRM_DEBUG_DRIVER.
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/i915_drv.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index b6d2a8f96278..8e58083ffb11 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1576,6 +1576,7 @@ static int __init i915_init(void)
driver.get_vblank_timestamp = NULL;
#ifndef CONFIG_DRM_I915_UMS
/* Silently fail loading to not upset userspace. */
+ DRM_DEBUG_DRIVER("KMS and UMS disabled.\n");
return 0;
#endif
}
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: avoid unclaimed registers when capturing the error state
On Mon, 2014-06-02 at 14:35 +0100, Chris Wilson wrote: > On Fri, Dec 20, 2013 at 03:09:41PM -0200, Paulo Zanoni wrote: > > From: Paulo Zanoni > > > > We're iterating over the CPU transcoders, so check for the correct > > power domain. > > > > This fixes many "unclaimed register" error messages. > > > > This can be reproduced by the IGT test mentioned below, but we still > > get a FAIL when we run it. > > > > Testcase: igt/kms_lip/flip-vs-panning-vs-hang > > Signed-off-by: Paulo Zanoni > > This of course breaks error capture on every platform that doesn't use > rpm, and even then is spectacularly useless on those that do. It is broken atm, because of the bug in intel_display_power_enabled_sw(). With that fixed these checks should work everywhere, since POWER_DOMAIN_INIT is always held on platforms w/o RPM. --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: Provide a parameter to disable loading
On Thu, 16 Jan 2014, Damien Lespiau wrote: > On Thu, Jan 16, 2014 at 07:07:09PM +0200, Ville Syrjälä wrote: >> On Thu, Jan 16, 2014 at 04:33:26PM +, Damien Lespiau wrote: >> > It can be handy at times to forbid i915 from loading at startup to >> > modprobe it later. >> >> modprobe.blacklist=i915 > > Ah, yeah, that works. Except when it doesn't, e.g. when a distro explicitly modprobes at boot. Damien, care to resurrect the patch please? BR, Jani. > > -- > Damien > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, 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] drivers/gpu/drm/i915/dma: style fixes
Fixed several double space pointer notations, and added one newline
Signed-off-by: Robin Schroer
---
drivers/gpu/drm/i915/i915_dma.c | 30 --
1 file changed, 16 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index b9159ad..f3ae664 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -136,7 +136,7 @@ static void i915_free_hws(struct drm_device *dev)
I915_WRITE(HWS_PGA, 0x1000);
}
-void i915_kernel_lost_context(struct drm_device * dev)
+void i915_kernel_lost_context(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_i915_master_private *master_priv;
@@ -164,7 +164,7 @@ void i915_kernel_lost_context(struct drm_device * dev)
master_priv->sarea_priv->perf_boxes |= I915_BOX_RING_EMPTY;
}
-static int i915_dma_cleanup(struct drm_device * dev)
+static int i915_dma_cleanup(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
int i;
@@ -188,7 +188,7 @@ static int i915_dma_cleanup(struct drm_device * dev)
return 0;
}
-static int i915_initialize(struct drm_device * dev, drm_i915_init_t * init)
+static int i915_initialize(struct drm_device *dev, drm_i915_init_t *init)
{
struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_i915_master_private *master_priv =
dev->primary->master->driver_priv;
@@ -233,7 +233,7 @@ static int i915_initialize(struct drm_device * dev,
drm_i915_init_t * init)
return 0;
}
-static int i915_dma_resume(struct drm_device * dev)
+static int i915_dma_resume(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_engine_cs *ring = LP_RING(dev_priv);
@@ -357,7 +357,7 @@ static int validate_cmd(int cmd)
return 0;
}
-static int i915_emit_cmds(struct drm_device * dev, int *buffer, int dwords)
+static int i915_emit_cmds(struct drm_device *dev, int *buffer, int dwords)
{
struct drm_i915_private *dev_priv = dev->dev_private;
int i, ret;
@@ -367,6 +367,7 @@ static int i915_emit_cmds(struct drm_device * dev, int
*buffer, int dwords)
for (i = 0; i < dwords;) {
int sz = validate_cmd(buffer[i]);
+
if (sz == 0 || i + sz > dwords)
return -EINVAL;
i += sz;
@@ -451,7 +452,7 @@ static void i915_emit_breadcrumb(struct drm_device *dev)
}
}
-static int i915_dispatch_cmdbuffer(struct drm_device * dev,
+static int i915_dispatch_cmdbuffer(struct drm_device *dev,
drm_i915_cmdbuffer_t *cmd,
struct drm_clip_rect *cliprects,
void *cmdbuf)
@@ -485,8 +486,8 @@ static int i915_dispatch_cmdbuffer(struct drm_device * dev,
return 0;
}
-static int i915_dispatch_batchbuffer(struct drm_device * dev,
-drm_i915_batchbuffer_t * batch,
+static int i915_dispatch_batchbuffer(struct drm_device *dev,
+drm_i915_batchbuffer_t *batch,
struct drm_clip_rect *cliprects)
{
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -547,7 +548,7 @@ static int i915_dispatch_batchbuffer(struct drm_device *
dev,
return 0;
}
-static int i915_dispatch_flip(struct drm_device * dev)
+static int i915_dispatch_flip(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_i915_master_private *master_priv =
@@ -753,7 +754,7 @@ fail_batch_free:
return ret;
}
-static int i915_emit_irq(struct drm_device * dev)
+static int i915_emit_irq(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_i915_master_private *master_priv =
dev->primary->master->driver_priv;
@@ -779,7 +780,7 @@ static int i915_emit_irq(struct drm_device * dev)
return dev_priv->dri1.counter;
}
-static int i915_wait_irq(struct drm_device * dev, int irq_nr)
+static int i915_wait_irq(struct drm_device *dev, int irq_nr)
{
struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_i915_master_private *master_priv =
dev->primary->master->driver_priv;
@@ -1264,6 +1265,7 @@ static void i915_switcheroo_set_state(struct pci_dev
*pdev, enum vga_switcheroo_
{
struct drm_device *dev = pci_get_drvdata(pdev);
pm_message_t pmm = { .event = PM_EVENT_SUSPEND };
+
if (state == VGA_SWITCHEROO_ON) {
pr_info("switched on\n");
dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
@@ -1895,7 +1897,7 @@ int i915_driver_open(struct drm_device *dev, struct
drm_file *file)
* and DMA structures, since the kernel won't be using them, and clea
* up any GEM state.
*/
-void i915_driver_lastclose(struc
Re: [Intel-gfx] [PATCH 3/3] drm/i915: Use transcoder as index to MIPI regs
Hi Damien, Thanks for providing the pointers. In my first patch I tried to aligned all the registers definitions, and I got my first review comment for not required formatting changes. Since then, I just replaced _PIPE with _TRANSCODER, so there are no changes at all. So I have just maintained the alignment as it is from the previous MIPI reg definitions and there is no extra/unnecessary tab or space inserted. This line: #define MIPI_READ_DATA_VALID(tc)_TRANSCODER(tc, \ >_MIPIA_READ_DATA_VALID, _MIPIB_READ_DATA_VALID) has a different alignment, just to keep the second line < 80 char. If you insert one more tab in front of _MIPIA_READ_DATA_VALID, its going beyond 80 char, so I had to pull it up. Similarly, #define _MIPIA_CLK_LANE_SWITCH_TIME_CNT (dev_priv->mipi_mmio_base \ >+ 0xb088) > There were only two options, either a checkpatch warning, or push to next line. > #define MIPI_READ_DATA_RETURN(tc, n) \ >(_TRANSCODER(tc, _MIPIA_READ_DATA_RETURN0, _MIPIB_READ_DATA_RETURN0) \ >+ 4 * (n)) /* n: 0...7 */ > This line was maintained as original alignment, just replacing PIPE with TRANSCODER, no tabs/space inserted. So, you have to agree that, I might have symptoms of OCD, but definitely not uncontrollable :). Going forward I will keep this mind that we can play around checkpatch rules it it gives good readability. Thanks for your time and patience for the review, and thanks a lot for R-B. Regards Shashank On 6/2/2014 6:56 PM, Damien Lespiau wrote: On Mon, Jun 02, 2014 at 01:55:13PM +0100, Sharma, Shashank wrote: Hi Damien, Can you please point out these, as this patch is re-based on latest 2/3, I was expecting this to be without any inconsistency. I personally checked for any <80 char formatting, which is not required. But if I missed any, I can again fix this, please let me know. At this point, there's no "rule". As Daniel said earlier the 80 chars limit is a soft one, esp. in headers declaring list of registers. For the inconsistencies, it's just a personal preference, I would try to make all defines look alike, right now you have: #define MIPI_DPI_CONTROL(tc)_TRANSCODER(tc, _MIPIA_DPI_CONTROL, \ _MIPIB_DPI_CONTROL) #define MIPI_GEN_FIFO_STAT(tc) _TRANSCODER(tc, _MIPIA_GEN_FIFO_STAT, \ _MIPIB_GEN_FIFO_STAT) #define MIPI_READ_DATA_VALID(tc)_TRANSCODER(tc, \ _MIPIA_READ_DATA_VALID, _MIPIB_READ_DATA_VALID) All different alignments. Not something I would ever do, but there's no rule against it per se, hence the r-b. You have a couple more of debatable splits: #define _MIPIA_CLK_LANE_SWITCH_TIME_CNT (dev_priv->mipi_mmio_base \ + 0xb088) #define MIPI_READ_DATA_RETURN(tc, n) \ (_TRANSCODER(tc, _MIPIA_READ_DATA_RETURN0, _MIPIB_READ_DATA_RETURN0) \ + 4 * (n)) /* n: 0...7 */ Esp. for the first one, these are cases where the "< 80 chars" split goes against readibility. Someone may ask you to fix those "bad" splits, not me this time though. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drivers/gpu/drm/i915/dma: style fixes
On Mon, Jun 02, 2014 at 04:59:39PM +0200, Robin Schroer wrote: > Fixed several double space pointer notations, and added one newline > > Signed-off-by: Robin Schroer Reviewed-by: Damien Lespiau -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drivers/gpu/drm/i915/dma: style fixes
On Mon, Jun 02, 2014 at 04:10:08PM +0100, Damien Lespiau wrote: > On Mon, Jun 02, 2014 at 04:59:39PM +0200, Robin Schroer wrote: > > Fixed several double space pointer notations, and added one newline > > > > Signed-off-by: Robin Schroer > > Reviewed-by: Damien Lespiau Queued for -next, thanks for the patch. Aside: If you want to do some janitorial tasks in drm/i915, here's a list: http://www.x.org/wiki/DRMJanitors/ Imo pure coding-style fixes are fairly pointless, at least if their free-standing. -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] Breaking suspend/resume by the Pipe A quirk
On Mon, Jun 02, 2014 at 12:41:36PM +0200, Thomas Richter wrote: > Am 02.06.2014 10:27, schrieb Daniel Vetter: > > > > > >Can you go right ahead and please submit this as a patch? > > Certainly, but I would prefer to get more information on this. Even though > the R31 *also* works without the pipe A quirk, I am not sure it does work on > all other hardware configurations. > > There is, however, an important difference between the R31 and the S6010: > The R31 uses two independent display pipes for the generating the display, > LVDS for the internal and VGA for the external display. As a result, frame > rates and resolutions can be different between the two outputs. > > The S6010, however, seems to use a single pipe design, with the internal > display connected via DVI (not LVDS!) and the external by VGA. This has the > unfortunate side effect that I cannot set the resolutions of internal and > external display independently. Any attempt to modify the external > resolution while using the internal screen results in an "no crtc found for > output VGA1" when using xrandr. (Not quite sure what this means, but I > believe that the VGA output is simply a duplicate of the DVI output, and the > two are probably connected through a bios-switchable bridge chip). > > Thus, I would *prefer* to be conservative and only disable the pipe_A quirk > only in situations where there is a single display pipe (as in the S6010) > and, just to be on the safe side, keep it enabled in dual-pipe (as in R31) > configurations. We've put a crtc restriction on VGA (it needs to be crtc 0) to work around some issues. DVI/LVDS should work on crtc 1. You can set this with the --crtc knob for xrandr. > Now I wonder how I could possibly distinguish between the two. Could you > please provide some pointers? You're probably the last real user of this hw left. You're needs win, especially if you know that it fixes stuff on other platforms, too. So holesale removal of the pipe quirk for i830M seems like the right thing to do here. Especially since Chris also complained that it makes stuff worse for his i845. -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 2/3] drm/i915: Change Mipi register definitions
On Mon, Jun 02, 2014 at 01:42:41PM +0100, Damien Lespiau wrote:
> On Mon, Jun 02, 2014 at 06:07:47PM +0530, shashank.sha...@intel.com wrote:
> > From: Shashank Sharma
> >
> > 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.
> >
> > V4: Addressing review comments by Damien and Ville, splitting into two
> > patches
> > This patch removes all the un-necessary formatting changes from previous
> > patch.
> > V5: Removed 80 char limit formatting for existing MIPI regs
> > V6: Removed extra space, change one definition
> >
> > Signed-off-by: Shashank Sharma
>
> Reviewed-by: Damien Lespiau
Both patches merged, thanks.
-Daniel
>
> --
> Damien
>
> > ---
> > drivers/gpu/drm/i915/i915_reg.h | 183
> >
> > 1 file changed, 93 insertions(+), 90 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index c12a858..dd2ce82 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -5706,12 +5706,12 @@ enum punit_power_well {
> > #define TEARING_EFFECT_DELAY_MASK (0x << 0)
> >
> > /* XXX: all bits reserved */
> > -#define _MIPIA_AUTOPWG (VLV_DISPLAY_BASE +
> > 0x611a0)
> > +#define _MIPIA_AUTOPWG (VLV_DISPLAY_BASE + 0x611a0)
> >
> > /* MIPI DSI Controller and D-PHY registers */
> >
> > -#define _MIPIA_DEVICE_READY(VLV_DISPLAY_BASE +
> > 0xb000)
> > -#define _MIPIB_DEVICE_READY(VLV_DISPLAY_BASE +
> > 0xb800)
> > +#define _MIPIA_DEVICE_READY(dev_priv->mipi_mmio_base +
> > 0xb000)
> > +#define _MIPIB_DEVICE_READY(dev_priv->mipi_mmio_base +
> > 0xb800)
> > #define MIPI_DEVICE_READY(pipe)_PIPE(pipe,
> > _MIPIA_DEVICE_READY, _MIPIB_DEVICE_READY)
> > #define BUS_POSSESSION(1 << 3) /* set
> > to give bus to receiver */
> > #define ULPS_STATE_MASK (3 << 1)
> > @@ -5720,11 +5720,11 @@ enum punit_power_well {
> > #define ULPS_STATE_NORMAL_OPERATION (0 << 1)
> > #define DEVICE_READY (1 << 0)
> >
> > -#define _MIPIA_INTR_STAT (VLV_DISPLAY_BASE + 0xb004)
> > -#define _MIPIB_INTR_STAT (VLV_DISPLAY_BASE + 0xb804)
> > +#define _MIPIA_INTR_STAT (dev_priv->mipi_mmio_base + 0xb004)
> > +#define _MIPIB_INTR_STAT (dev_priv->mipi_mmio_base + 0xb804)
> > #define MIPI_INTR_STAT(pipe) _PIPE(pipe, _MIPIA_INTR_STAT,
> > _MIPIB_INTR_STAT)
> > -#define _MIPIA_INTR_EN (VLV_DISPLAY_BASE +
> > 0xb008)
> > -#define _MIPIB_INTR_EN (VLV_DISPLAY_BASE +
> > 0xb808)
> > +#define _MIPIA_INTR_EN (dev_priv->mipi_mmio_base +
> > 0xb008)
> > +#define _MIPIB_INTR_EN (dev_priv->mipi_mmio_base +
> > 0xb808)
> > #define MIPI_INTR_EN(pipe) _PIPE(pipe, _MIPIA_INTR_EN,
> > _MIPIB_INTR_EN)
> > #define TEARING_EFFECT(1 << 31)
> > #define SPL_PKT_SENT_INTERRUPT(1 << 30)
> > @@ -5759,8 +5759,8 @@ enum punit_power_well {
> > #define RXSOT_SYNC_ERROR (1 << 1)
> > #define RXSOT_ERROR (1 << 0)
> >
> > -#define _MIPIA_DSI_FUNC_PRG(VLV_DISPLAY_BASE +
> > 0xb00c)
> > -#define _MIPIB_DSI_FUNC_PRG(VLV_DISPLAY_BASE +
> > 0xb80c)
> > +#define _MIPIA_DSI_FUNC_PRG(dev_priv->mipi_mmio_base +
> > 0xb00c)
> > +#define _MIPIB_DSI_FUNC_PRG(dev_priv->mipi_mmio_base +
> > 0xb80c)
> > #define MIPI_DSI_FUNC_PRG(pipe)_PIPE(pipe,
> > _MIPIA_DSI_FUNC_PRG, _MIPIB_DSI_FUNC_PRG)
> > #define CMD_MODE_DATA_WIDTH_MASK (7 << 13)
> > #define CMD_MODE_NOT_SUPPORTED(0 << 13)
> > @@ -5782,77 +5782,78 @@ enum punit_power_well {
> > #define DATA_LANES_PRG_REG_SHIFT 0
> > #define DATA_LANES_PRG_REG_MASK (7 << 0)
> >
> > -#define _MIPIA_HS_TX_TIMEOUT (VLV_DISPLAY_BASE +
> > 0xb010)
> > -#define _MIPIB_HS_TX_TIMEOUT (VLV_DISPLAY_BASE +
> > 0xb810)
> > +#define _MIPIA_HS_TX_TIMEOUT (dev_priv->mipi_mmio_base +
> > 0xb010)
> > +#define _MIPIB_HS_TX_TIMEOUT (dev_priv->mipi_mmio_base +
> > 0xb810)
> > #define MIPI_HS_TX_TIMEOUT(pipe) _PIPE(pipe, _MIPIA_HS_TX_TIMEOUT,
> > _MIPIB_HS_TX_TIMEOUT)
> > #define HIGH_SPEED_TX_TIMEOUT_COUNTER_MASK0xff
>
Re: [Intel-gfx] [PATCH v2] drm/i915: tell the user if both KMS and UMS are disabled
On Mon, Jun 02, 2014 at 04:58:30PM +0300, Jani Nikula wrote:
> If both KMS is disabled (by i915.modeset=0 or nomodeset parameters) and
> UMS is disabled (by CONFIG_DRM_I915_UMS=n, the default), the user might
> not be aware his setup is not supported. Inform the users (and, by
> extension, the poor i915 developers having to read their dmesgs in bug
> reports) why their graphics experience might be lacking.
>
> A similar message was added on the UMS path in
> commit e147accbd19f55489dabdcc4dc3551cc3e3f2553
> Author: Jani Nikula
> Date: Thu Oct 10 15:25:37 2013 +0300
>
> drm/i915: tell the user KMS is required for gen6+
>
> but it won't be reached if CONFIG_DRM_I915_UMS=n since
> commit b30324adaf8d2e5950a602bde63030d15a61826f
> Author: Daniel Vetter
> Date: Wed Nov 13 22:11:25 2013 +0100
>
> drm/i915: Deprecated UMS support
>
> v2: Use DRM_DEBUG_DRIVER.
>
> Signed-off-by: Jani Nikula
Queued for -next, thanks for the patch. I'll shuffle it into the 3.16 pile
when I get around to that.
-Daniel
> ---
> drivers/gpu/drm/i915/i915_drv.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index b6d2a8f96278..8e58083ffb11 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1576,6 +1576,7 @@ static int __init i915_init(void)
> driver.get_vblank_timestamp = NULL;
> #ifndef CONFIG_DRM_I915_UMS
> /* Silently fail loading to not upset userspace. */
> + DRM_DEBUG_DRIVER("KMS and UMS disabled.\n");
> return 0;
> #endif
> }
> --
> 1.9.1
>
--
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 4/4] drm/i915: make sure PC8 is enabled on suspend and disabled on resume
On Mon, Jun 02, 2014 at 02:37:35PM +0300, Imre Deak wrote: > On Mon, 2014-06-02 at 10:45 +0200, Daniel Vetter wrote: > > On Thu, May 29, 2014 at 02:11:37PM -0700, Jesse Barnes wrote: > > > From: Kristen Carlson Accardi > > > > > > This matches the runtime suspend paths and allows the system to enter > > > the lowest power mode at freeze time. > > > > > > Signed-off-by: Kristen Carlson Accardi > > > Signed-off-by: Jesse Barnes > > > > pc8 is fully subsumed into runtime pm by now. Do we _really_ still need > > this? > > Yes, since the system suspend/resume handlers are called with an RPM ref > held and thus PC8 disabled. But doesn't patch 1 try to fix that? Imo we should have this here but instead go through highl-level the runtime pm functions to shut off the chip on all platforms. -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] drm/i915: enable PPGTT on VLV
On Mon, Jun 02, 2014 at 02:09:06PM +0300, Ville Syrjälä wrote: > On Thu, May 29, 2014 at 02:33:21PM -0700, Jesse Barnes wrote: > > Working for real this time. i915_ppgtt_info has all sorts of good stuff > > in it and X is running nicely on top. > > > > Signed-off-by: Jesse Barnes > > So it wasn't just my vlv where it appears to work. That's nice. > > Reviewed-by: Ville Syrjälä Queued for -next, thanks for the patch. -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 v2] drm/i915: Always apply cursor width changes
On Mon, Jun 02, 2014 at 01:46:11PM +0300, Antti Koskipää wrote:
> On 06/02/2014 11:49 AM, Daniel Vetter wrote:
> > On Fri, May 30, 2014 at 04:35:26PM +0300, Chris Wilson wrote:
> >> It is possible for userspace to create a big object large enough for a
> >> 256x256, and then switch over to using it as a 64x64 cursor. This
> >> requires the cursor update routines to check for a change in width on
> >> every update, rather than just when the cursor is originally enabled.
> >>
> >> This also fixes an issue with 845g/865g which cannot change the base
> >> address of the cursor whilst it is active.
> >>
> >> Signed-off-by: Chris Wilson
> >> [Antti:rebased, adjusted macro names and moved some lines, no functional
> >> changes]
> >> Reviewed-by: Antti Koskipaa
> >> Tested-by: Antti Koskipaa
> >
> > Still missing the igt testcase. Is Antti still working on that one?
>
> The testcase was just posted. It just needed some cleanup.
Cool. Patch is queued for -next, thanks for the patch.
-Daniel
>
> > And I
> > guess now we also need an Cc: sta...@vger.kernel.org on this one here.
> > -Daniel
> >
> >> ---
> >> drivers/gpu/drm/i915/i915_debugfs.c | 2 +-
> >> drivers/gpu/drm/i915/intel_display.c | 106
> >> +--
> >> drivers/gpu/drm/i915/intel_drv.h | 3 +-
> >> 3 files changed, 53 insertions(+), 58 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> >> b/drivers/gpu/drm/i915/i915_debugfs.c
> >> index 2e5f76a..04d0b7c 100644
> >> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> >> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> >> @@ -2353,7 +2353,7 @@ static int i915_display_info(struct seq_file *m,
> >> void *unused)
> >>
> >>active = cursor_position(dev, crtc->pipe, &x, &y);
> >>seq_printf(m, "\tcursor visible? %s, position (%d, %d),
> >> addr 0x%08x, active? %s\n",
> >> - yesno(crtc->cursor_visible),
> >> + yesno(crtc->cursor_base),
> >> x, y, crtc->cursor_addr,
> >> yesno(active));
> >>}
> >> diff --git a/drivers/gpu/drm/i915/intel_display.c
> >> b/drivers/gpu/drm/i915/intel_display.c
> >> index 731cd01..955f92d 100644
> >> --- a/drivers/gpu/drm/i915/intel_display.c
> >> +++ b/drivers/gpu/drm/i915/intel_display.c
> >> @@ -7872,29 +7872,33 @@ static void i845_update_cursor(struct drm_crtc
> >> *crtc, u32 base)
> >>struct drm_device *dev = crtc->dev;
> >>struct drm_i915_private *dev_priv = dev->dev_private;
> >>struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> >> - bool visible = base != 0;
> >> - u32 cntl;
> >> + uint32_t cntl;
> >>
> >> - if (intel_crtc->cursor_visible == visible)
> >> - return;
> >> -
> >> - cntl = I915_READ(_CURACNTR);
> >> - if (visible) {
> >> + if (base != intel_crtc->cursor_base) {
> >>/* On these chipsets we can only modify the base whilst
> >> * the cursor is disabled.
> >> */
> >> + if (intel_crtc->cursor_cntl) {
> >> + I915_WRITE(_CURACNTR, 0);
> >> + POSTING_READ(_CURACNTR);
> >> + intel_crtc->cursor_cntl = 0;
> >> + }
> >> +
> >>I915_WRITE(_CURABASE, base);
> >> + POSTING_READ(_CURABASE);
> >> + }
> >>
> >> - cntl &= ~(CURSOR_FORMAT_MASK);
> >> - /* XXX width must be 64, stride 256 => 0x00 << 28 */
> >> - cntl |= CURSOR_ENABLE |
> >> + /* XXX width must be 64, stride 256 => 0x00 << 28 */
> >> + cntl = 0;
> >> + if (base)
> >> + cntl = (CURSOR_ENABLE |
> >>CURSOR_GAMMA_ENABLE |
> >> - CURSOR_FORMAT_ARGB;
> >> - } else
> >> - cntl &= ~(CURSOR_ENABLE | CURSOR_GAMMA_ENABLE);
> >> - I915_WRITE(_CURACNTR, cntl);
> >> -
> >> - intel_crtc->cursor_visible = visible;
> >> + CURSOR_FORMAT_ARGB);
> >> + if (intel_crtc->cursor_cntl != cntl) {
> >> + I915_WRITE(_CURACNTR, cntl);
> >> + POSTING_READ(_CURACNTR);
> >> + intel_crtc->cursor_cntl = cntl;
> >> + }
> >> }
> >>
> >> static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base)
> >> @@ -7903,16 +7907,12 @@ static void i9xx_update_cursor(struct drm_crtc
> >> *crtc, u32 base)
> >>struct drm_i915_private *dev_priv = dev->dev_private;
> >>struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> >>int pipe = intel_crtc->pipe;
> >> - bool visible = base != 0;
> >> -
> >> - if (intel_crtc->cursor_visible != visible) {
> >> - int16_t width = intel_crtc->cursor_width;
> >> - uint32_t cntl = I915_READ(CURCNTR(pipe));
> >> - if (base) {
> >> - cntl &= ~(CURSOR_MODE | MCURSOR_PIPE_SELECT);
> >> - cntl |= MCURSOR_GAMMA_ENABLE;
> >> + uint32_t cntl;
> >>
> >> - switch (width) {
> >> + cntl = 0;
> >> + if (base) {
> >> + cn
[Intel-gfx] [Patch] Disabling the pipe A quirk for the Fujitsu S6010
Hi Daniel, hi others,
please find a patch attached that disables the pipe A quirk for the
Fujitsu S6010. I will probably add a line for the R31 later, I only
need to add the model number.
How is the watermark-alignment patch for the 830 doing, btw?
Greetings,
Thomas
>From 2006abcd850f8c0995153ffb491efd590103f17f Mon Sep 17 00:00:00 2001
From: thor
Date: Mon, 2 Jun 2014 17:32:55 +0200
Subject: [PATCH 2/2] Disabling the pipe A quirk for the Fujitsu S6010.
Signed-off-by: thor
---
drivers/gpu/drm/i915/intel_display.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 54095d4..02b6525 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11535,6 +11535,18 @@ static void quirk_pipea_force(struct drm_device *dev)
}
/*
+ * Some 830 based systems do not work with the pipe A quirk
+ * correctly since they do not use pipe A in first place
+ */
+static void quirk_disable_pipea_force(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+
+ dev_priv->quirks &= ~QUIRK_PIPEA_FORCE;
+ DRM_INFO("removing the pipe a force quirk for this hardware\n");
+}
+
+/*
* Some machines (Lenovo U160) do not work with SSC on LVDS for some reason
*/
static void quirk_ssc_force_disable(struct drm_device *dev)
@@ -11603,6 +11615,9 @@ static struct intel_quirk intel_quirks[] = {
/* 830 needs to leave pipe A & dpll A up */
{ 0x3577, PCI_ANY_ID, PCI_ANY_ID, quirk_pipea_force },
+ /* However, do not enable the quirk on S6010 */
+ { 0x3577, 0x10cf, 0x113c, quirk_disable_pipea_force },
+
/* Lenovo U160 cannot use SSC on LVDS */
{ 0x0046, 0x17aa, 0x3920, quirk_ssc_force_disable },
--
1.7.10.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] kms_cursor_crc: Test cursor size change ioctl
On Mon, Jun 02, 2014 at 01:43:18PM +0300, Antti Koskipaa wrote:
> Now that we support cursor changes other than 64x64, a bug was found
> where the size change was only applied at cursor enable time, rather
> than at every update. Add a testcase for that.
>
> Signed-off-by: Antti Koskipaa
Merged, thanks.
-Daniel
> ---
> tests/kms_cursor_crc.c | 55
> ++
> 1 file changed, 55 insertions(+)
>
> diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
> index 06625ee..1b8da26 100644
> --- a/tests/kms_cursor_crc.c
> +++ b/tests/kms_cursor_crc.c
> @@ -54,6 +54,7 @@ typedef struct {
> int left, right, top, bottom;
> int screenw, screenh;
> int curw, curh; /* cursor size */
> + int cursor_max_size;
> igt_pipe_crc_t *pipe_crc;
> } test_data_t;
>
> @@ -272,6 +273,7 @@ static bool prepare_crtc(test_data_t *test_data,
> igt_output_t *output,
> test_data->screenh = mode->vdisplay;
> test_data->curw = cursor_w;
> test_data->curh = cursor_h;
> + test_data->cursor_max_size = cursor_w;
>
> /* make sure cursor is disabled */
> cursor_disable(test_data);
> @@ -354,6 +356,56 @@ static void create_cursor_fb(data_t *data, int cur_w,
> int cur_h)
> igt_assert(cairo_status(cr) == 0);
> }
>
> +static void test_cursor_size(test_data_t *test_data)
> +{
> + data_t *data = test_data->data;
> + igt_display_t *display = &data->display;
> + igt_pipe_crc_t *pipe_crc = test_data->pipe_crc;
> + igt_crc_t crc[10], ref_crc;
> + igt_plane_t *cursor;
> + cairo_t *cr;
> + uint32_t fb_id;
> + int i, size, cursor_max_size = test_data->cursor_max_size;
> +
> + /* Create a maximum size cursor, then change the size in flight to
> + * smaller ones to see that the size is applied correctly
> + */
> + fb_id = igt_create_fb(data->drm_fd, cursor_max_size, cursor_max_size,
> + DRM_FORMAT_ARGB, false, &data->fb);
> + igt_assert(fb_id);
> +
> + /* Use a solid white rectangle as the cursor */
> + cr = igt_get_cairo_ctx(data->drm_fd, &data->fb);
> + igt_paint_color_alpha(cr, 0, 0, cursor_max_size, cursor_max_size, 1.0,
> 1.0, 1.0, 1.0);
> +
> + /* Hardware test loop */
> + cursor_enable(test_data);
> + cursor = igt_output_get_plane(test_data->output, IGT_PLANE_CURSOR);
> + igt_plane_set_position(cursor, 0, 0);
> + for (i = 0, size = cursor_max_size; size >= 64; size /= 2, i++) {
> + /* Change size in flight: */
> + int ret = drmModeSetCursor(data->drm_fd,
> test_data->output->config.crtc->crtc_id,
> +data->fb.gem_handle, size, size);
> + igt_assert(ret == 0);
> + igt_wait_for_vblank(data->drm_fd, test_data->pipe);
> + igt_pipe_crc_collect_crc(pipe_crc, &crc[i]);
> + }
> + cursor_disable(test_data);
> + /* Software test loop */
> + cr = igt_get_cairo_ctx(data->drm_fd, &data->primary_fb);
> + for (i = 0, size = cursor_max_size; size >= 64; size /= 2, i++) {
> + /* Now render the same in software and collect crc */
> + igt_paint_color_alpha(cr, 0, 0, size, size, 1.0, 1.0, 1.0, 1.0);
> + igt_display_commit(display);
> + igt_wait_for_vblank(data->drm_fd, test_data->pipe);
> + igt_pipe_crc_collect_crc(pipe_crc, &ref_crc);
> + /* Clear screen afterwards */
> + igt_paint_color(cr, 0, 0, test_data->screenw,
> test_data->screenh,
> + 0.0, 0.0, 0.0);
> + igt_assert(igt_crc_equal(&crc[i], &ref_crc));
> + }
> +}
> +
> static void run_test_generic(data_t *data, int cursor_max_size)
> {
> int cursor_size;
> @@ -407,6 +459,9 @@ igt_main
> igt_display_init(&data.display, data.drm_fd);
> }
>
> + igt_subtest_f("cursor-size-change")
> + run_test(&data, test_cursor_size, cursor_width, cursor_height);
> +
> run_test_generic(&data, cursor_width);
>
> igt_fixture {
> --
> 1.8.3.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
Re: [Intel-gfx] [Patch] Disabling the pipe A quirk for the Fujitsu S6010
On Mon, Jun 02, 2014 at 05:38:13PM +0200, Thomas Richter wrote:
> Hi Daniel, hi others,
>
> please find a patch attached that disables the pipe A quirk for the Fujitsu
> S6010. I will probably add a line for the R31 later, I only
> need to add the model number.
>
> How is the watermark-alignment patch for the 830 doing, btw?
>
> Greetings,
> Thomas
>
> From 2006abcd850f8c0995153ffb491efd590103f17f Mon Sep 17 00:00:00 2001
> From: thor
> Date: Mon, 2 Jun 2014 17:32:55 +0200
> Subject: [PATCH 2/2] Disabling the pipe A quirk for the Fujitsu S6010.
>
> Signed-off-by: thor
Like I've explained, this is nacked. I'll merge the patch I've wanted now.
-Daniel
> ---
> drivers/gpu/drm/i915/intel_display.c | 15 +++
> 1 file changed, 15 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 54095d4..02b6525 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11535,6 +11535,18 @@ static void quirk_pipea_force(struct drm_device *dev)
> }
>
> /*
> + * Some 830 based systems do not work with the pipe A quirk
> + * correctly since they do not use pipe A in first place
> + */
> +static void quirk_disable_pipea_force(struct drm_device *dev)
> +{
> + struct drm_i915_private *dev_priv = dev->dev_private;
> +
> + dev_priv->quirks &= ~QUIRK_PIPEA_FORCE;
> + DRM_INFO("removing the pipe a force quirk for this hardware\n");
> +}
> +
> +/*
> * Some machines (Lenovo U160) do not work with SSC on LVDS for some reason
> */
> static void quirk_ssc_force_disable(struct drm_device *dev)
> @@ -11603,6 +11615,9 @@ static struct intel_quirk intel_quirks[] = {
> /* 830 needs to leave pipe A & dpll A up */
> { 0x3577, PCI_ANY_ID, PCI_ANY_ID, quirk_pipea_force },
>
> + /* However, do not enable the quirk on S6010 */
> + { 0x3577, 0x10cf, 0x113c, quirk_disable_pipea_force },
> +
> /* Lenovo U160 cannot use SSC on LVDS */
> { 0x0046, 0x17aa, 0x3920, quirk_ssc_force_disable },
>
> --
> 1.7.10.4
>
--
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 4/4] drm/i915: make sure PC8 is enabled on suspend and disabled on resume
On Mon, 2014-06-02 at 17:32 +0200, Daniel Vetter wrote: > On Mon, Jun 02, 2014 at 02:37:35PM +0300, Imre Deak wrote: > > On Mon, 2014-06-02 at 10:45 +0200, Daniel Vetter wrote: > > > On Thu, May 29, 2014 at 02:11:37PM -0700, Jesse Barnes wrote: > > > > From: Kristen Carlson Accardi > > > > > > > > This matches the runtime suspend paths and allows the system to enter > > > > the lowest power mode at freeze time. > > > > > > > > Signed-off-by: Kristen Carlson Accardi > > > > Signed-off-by: Jesse Barnes > > > > > > pc8 is fully subsumed into runtime pm by now. Do we _really_ still need > > > this? > > > > Yes, since the system suspend/resume handlers are called with an RPM ref > > held and thus PC8 disabled. > > But doesn't patch 1 try to fix that? That only disables the display side, but we won't disable PC8 until the RPM suspend handler is called. And that won't happen because the last RPM ref is held by the DPM framework for the duration of system suspend/resume handlers. > Imo we should have this here but instead go through highl-level the runtime > pm functions to shut off the chip on all platforms. After the planned refactoring we could have a low-level function that we can call both from here and the runtime PM path, but until that happens we need to do this here directly. --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
[Intel-gfx] [PATCH] drm/i915: Nuke pipe A quirk on i830M
Apparently it does more harm than good. Thomas Richter reports that
it helps his machine (Thinkpad X31) and there's another report from a
Fujitsu S6010. Also, we've nuked it on i845G already to make Chris'
machine happy.
Cc: Thomas Richter
References: 538C54E0.8090507@rus.uni-stuttgart.de">http://mid.mail-archive.com/538C54E0.8090507@rus.uni-stuttgart.de
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/i915/intel_display.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 0c90bcc6fa7a..174385b9 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11844,9 +11844,6 @@ static struct intel_quirk intel_quirks[] = {
/* ThinkPad T60 needs pipe A force quirk (bug #16494) */
{ 0x2782, 0x17aa, 0x201a, quirk_pipea_force },
- /* 830 needs to leave pipe A & dpll A up */
- { 0x3577, PCI_ANY_ID, PCI_ANY_ID, quirk_pipea_force },
-
/* Lenovo U160 cannot use SSC on LVDS */
{ 0x0046, 0x17aa, 0x3920, quirk_ssc_force_disable },
--
1.9.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/4] drm/i915: make sure PC8 is enabled on suspend and disabled on resume
On Mon, Jun 02, 2014 at 06:57:18PM +0300, Imre Deak wrote: > On Mon, 2014-06-02 at 17:32 +0200, Daniel Vetter wrote: > > On Mon, Jun 02, 2014 at 02:37:35PM +0300, Imre Deak wrote: > > > On Mon, 2014-06-02 at 10:45 +0200, Daniel Vetter wrote: > > > > On Thu, May 29, 2014 at 02:11:37PM -0700, Jesse Barnes wrote: > > > > > From: Kristen Carlson Accardi > > > > > > > > > > This matches the runtime suspend paths and allows the system to enter > > > > > the lowest power mode at freeze time. > > > > > > > > > > Signed-off-by: Kristen Carlson Accardi > > > > > Signed-off-by: Jesse Barnes > > > > > > > > pc8 is fully subsumed into runtime pm by now. Do we _really_ still need > > > > this? > > > > > > Yes, since the system suspend/resume handlers are called with an RPM ref > > > held and thus PC8 disabled. > > > > But doesn't patch 1 try to fix that? > > That only disables the display side, but we won't disable PC8 until the > RPM suspend handler is called. And that won't happen because the last > RPM ref is held by the DPM framework for the duration of system > suspend/resume handlers. Yeah, there's discussion going on to make system suspend be optionally based upon runtime pm in the core. Atm that's not possible since the system wakes up everyone to suspend them. > > Imo we should have this here but instead go through highl-level the runtime > > pm functions to shut off the chip on all platforms. > > After the planned refactoring we could have a low-level function that we > can call both from here and the runtime PM path, but until that happens > we need to do this here directly. Yeah, that's what I'm actually aiming for - we should be able to call the runtime pm suspend code from the system suspend code and share pretty much all the code. The sequence I'm thinking of would be for system suspend: 1. Stop everything we need to stop (gpu, display, rps, ...) to be able to enter runtime pm. 2. Check for leaked references. This might be tricky because audio. 3. Call runtime pm suspend hooks. Resume would be the same, but inverted. -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 66/66] drm/i915: runtime PM support for DPMS
On Thu, Apr 24, 2014 at 11:55:42PM +0200, Daniel Vetter wrote:
> Keeping track of the power domains is a bit messy since crtc->active
> is currently updated by the platform hooks, but we need to be aware of
> which state transition exactly is going on. Maybe we simply need to
> shovel all the power domain handling down into platform code to
> simplify this. But doing that requires some more auditing since
> currently the ->mode_set callbacks still read some random registers
> (to e.g. figure out the reference clocks).
>
> Also note that intel_crtc_update_dpms is always call first/last even
> for encoders which have their own dpms functions. Hence we really only
> need to update this place here.
>
> Being a quick "does it blow up?" run not really tested yet.
>
> Signed-off-by: Daniel Vetter
Ok, I've simply gone ahead and merged this with a !HAS_DDI check so that I
can unblock runtime pm for dpms.
-Daniel
> ---
> drivers/gpu/drm/i915/intel_display.c | 26 ++
> 1 file changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index e0bd0f94e43e..1b5d6b099b37 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4478,16 +4478,34 @@ void intel_crtc_update_dpms(struct drm_crtc *crtc)
> {
> struct drm_device *dev = crtc->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> + struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> struct intel_encoder *intel_encoder;
> + enum intel_display_power_domain domain;
> + unsigned long domains;
> bool enable = false;
>
> for_each_encoder_on_crtc(dev, crtc, intel_encoder)
> enable |= intel_encoder->connectors_active;
>
> - if (enable)
> - dev_priv->display.crtc_enable(crtc);
> - else
> - dev_priv->display.crtc_disable(crtc);
> + if (enable) {
> + if (!intel_crtc->active) {
> + domains = get_crtc_power_domains(crtc);
> + for_each_power_domain(domain, domains)
> + intel_display_power_get(dev_priv, domain);
> + intel_crtc->enabled_power_domains = domains;
> +
> + dev_priv->display.crtc_enable(crtc);
> + }
> + } else {
> + if (intel_crtc->active) {
> + dev_priv->display.crtc_disable(crtc);
> +
> + domains = intel_crtc->enabled_power_domains;
> + for_each_power_domain(domain, domains)
> + intel_display_power_put(dev_priv, domain);
> + intel_crtc->enabled_power_domains = 0;
> + }
> + }
>
> intel_crtc_update_sarea(crtc, enable);
> }
> --
> 1.8.1.4
>
--
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] Disabling the pipe A quirk for the Fujitsu S6010
Hi Daniel, From 2006abcd850f8c0995153ffb491efd590103f17f Mon Sep 17 00:00:00 2001 From: thor Date: Mon, 2 Jun 2014 17:32:55 +0200 Subject: [PATCH 2/2] Disabling the pipe A quirk for the Fujitsu S6010. Signed-off-by: thor Like I've explained, this is nacked. I'll merge the patch I've wanted now. Excuse my ignorance, but what do you mean by "naked"? Do you need anything else (for the watermark patch) to get it going? It is signed-off? Is this not done correctly? Sorry for my ignorance. Greetings, Thomas ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Drop unused lut tables from intel_plane
Signed-off-by: Matt Roper
---
drivers/gpu/drm/i915/intel_drv.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 76de420..fea8e05 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -428,7 +428,6 @@ struct intel_plane {
struct drm_i915_gem_object *obj;
bool can_scale;
int max_downscale;
- u32 lut_r[1024], lut_g[1024], lut_b[1024];
int crtc_x, crtc_y;
unsigned int crtc_w, crtc_h;
uint32_t src_x, src_y;
--
1.8.5.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drivers/gpu/drm/i915/dma: style fixes
On Mon, Jun 02, 2014 at 05:21:32PM +0200, Daniel Vetter wrote: > On Mon, Jun 02, 2014 at 04:10:08PM +0100, Damien Lespiau wrote: > > On Mon, Jun 02, 2014 at 04:59:39PM +0200, Robin Schroer wrote: > > > Fixed several double space pointer notations, and added one newline > > > > > > Signed-off-by: Robin Schroer > > > > Reviewed-by: Damien Lespiau > > Queued for -next, thanks for the patch. Aside: If you want to do some > janitorial tasks in drm/i915, here's a list: > > http://www.x.org/wiki/DRMJanitors/ > > Imo pure coding-style fixes are fairly pointless, at least if their > free-standing. > -Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch Thanks, I'll look into it. -- Robin Schroer ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [Patch] Disabling the pipe A quirk for the Fujitsu S6010
On Mon, Jun 02, 2014 at 06:52:13PM +0200, Thomas Richter wrote: > Hi Daniel, > > > > >> From 2006abcd850f8c0995153ffb491efd590103f17f Mon Sep 17 00:00:00 2001 > >>From: thor > >>Date: Mon, 2 Jun 2014 17:32:55 +0200 > >>Subject: [PATCH 2/2] Disabling the pipe A quirk for the Fujitsu S6010. > >> > >>Signed-off-by: thor > > > >Like I've explained, this is nacked. I'll merge the patch I've wanted now. > > Excuse my ignorance, but what do you mean by "naked"? Do you need anything > else (for the watermark patch) to get it going? It is signed-off? Is this > not done correctly? nack = not acknowledged, i.e. rejected. Comes from tcp. I've applied the patch instead to just remove the quirk on all i830M. -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] drm/i915: Drop unused lut tables from intel_plane
On Mon, Jun 02, 2014 at 10:12:06AM -0700, Matt Roper wrote:
> Signed-off-by: Matt Roper
The commit message is a bit terse and could do with some digging.
Something like:
Those LUT where defined in the original sprite patch introducing intel_plane,
but were never used.
commit b840d907fcf6d5d5ef91af4518b3dab3a5da0f75
Author: Jesse Barnes
Date: Tue Dec 13 13:19:38 2011 -0800
drm/i915: add SNB and IVB video sprite support v6
Reviewed-by: Damien Lespiau
> ---
> drivers/gpu/drm/i915/intel_drv.h | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 76de420..fea8e05 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -428,7 +428,6 @@ struct intel_plane {
> struct drm_i915_gem_object *obj;
> bool can_scale;
> int max_downscale;
> - u32 lut_r[1024], lut_g[1024], lut_b[1024];
> int crtc_x, crtc_y;
> unsigned int crtc_w, crtc_h;
> uint32_t src_x, src_y;
> --
> 1.8.5.1
>
> ___
> 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
Re: [Intel-gfx] [PATCH 10/11] drm/i915: Improve PSR debugfs status.
On 5/16/2014 5:43 AM, Rodrigo Vivi wrote: Now we have the active/inactive state for exit and this actually changes the HW enable bit the status was a bit confusing for users. So let's provide more info. Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/i915/i915_debugfs.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 6636ca2..0ca9376 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -1975,10 +1975,12 @@ static int i915_edp_psr_status(struct seq_file *m, void *data) seq_printf(m, "Sink_Support: %s\n", yesno(dev_priv->psr.sink_support)); seq_printf(m, "Source_OK: %s\n", yesno(dev_priv->psr.source_ok)); + seq_printf(m, "Enabled: %s\n", yesno(dev_priv->psr.enabled)); + seq_printf(m, "Active: %s\n", yesno(dev_priv->psr.active)); enabled = HAS_PSR(dev) && I915_READ(EDP_PSR_CTL(dev)) & EDP_PSR_ENABLE; - seq_printf(m, "Enabled: %s\n", yesno(enabled)); + seq_printf(m, "HW Enabled & Active bit: %s\n", yesno(enabled)); if (HAS_PSR(dev)) psrperf = I915_READ(EDP_PSR_PERF_CNT(dev)) & Please remove all references to PSR performance counter. This register is primarily meant as a debug register and its implementation is broken in the h/w. Whenever the cdclk is gated to save power, the performance counter is stopped. But when the clk is re-enabled it doesn't reset the counter. This unnecessarily confuses the end users.. When the system goes through suspend / resume cycle the performance counter most likely will transition from a non-zero value to zero.. I already received few queries from our customers related to this performance customer and they refuse to believe me when i tell them PSR is still functional when the performance counter reports 0 :-) AFAIK this register definition is missing in open source HSW B spec as well.. Thanks, Vijay ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/vlv: T12 eDP panel timing enforcement during reboot.
From: Clint Taylor
The panel power sequencer on vlv doesn't appear to accept changes to its
T12 power down duration during warm reboots. This change forces a delay
for warm reboots to the T12 panel timing as defined in the VBT table for
the connected panel.
Ver2: removed redundant pr_crit(), commented magic value for pp_div_reg
Ver3: moved SYS_RESTART check earlier, new name for pp_div.
Signed-off-by: Clint Taylor
---
drivers/gpu/drm/i915/intel_dp.c | 42 ++
drivers/gpu/drm/i915/intel_drv.h |2 ++
2 files changed, 44 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 5ca68aa9..fb7725a 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -28,6 +28,8 @@
#include
#include
#include
+#include
+#include
#include
#include
#include
@@ -302,6 +304,38 @@ static u32 _pp_stat_reg(struct intel_dp *intel_dp)
return VLV_PIPE_PP_STATUS(vlv_power_sequencer_pipe(intel_dp));
}
+/* Reboot notifier handler to shutdown panel power to guarantee T12 timing */
+static int edp_notify_handler(struct notifier_block *this, unsigned long code,
+ void *unused)
+{
+ struct intel_dp *intel_dp = container_of(this, typeof(* intel_dp),
+edp_notifier);
+ struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+ struct drm_device *dev = intel_dig_port->base.base.dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ u32 pp_div;
+ u32 pp_ctrl_reg, pp_div_reg;
+ enum pipe pipe = vlv_power_sequencer_pipe(intel_dp);
+
+ if ((!is_edp(intel_dp)) &&
+ (code != SYS_RESTART ))
+ return 0;
+
+ if (IS_VALLEYVIEW(dev)) {
+ pp_ctrl_reg = VLV_PIPE_PP_CONTROL(pipe);
+ pp_div_reg = VLV_PIPE_PP_DIVISOR(pipe);
+ pp_div = I915_READ(VLV_PIPE_PP_DIVISOR(pipe));
+ pp_div &= PP_REFERENCE_DIVIDER_MASK;
+
+ /* 0x1F write to PP_DIV_REG sets max cycle delay */
+ I915_WRITE(pp_div_reg , pp_div | 0x1F);
+ I915_WRITE(pp_ctrl_reg,
+ PANEL_UNLOCK_REGS | PANEL_POWER_OFF);
+ msleep(intel_dp->panel_power_cycle_delay);
+ }
+ return 0;
+}
+
static bool edp_have_panel_power(struct intel_dp *intel_dp)
{
struct drm_device *dev = intel_dp_to_dev(intel_dp);
@@ -3344,6 +3378,10 @@ void intel_dp_encoder_destroy(struct drm_encoder
*encoder)
mutex_lock(&dev->mode_config.mutex);
edp_panel_vdd_off_sync(intel_dp);
mutex_unlock(&dev->mode_config.mutex);
+ if (intel_dp->edp_notifier.notifier_call) {
+ unregister_reboot_notifier(&intel_dp->edp_notifier);
+ intel_dp->edp_notifier.notifier_call = NULL;
+ }
}
kfree(intel_dig_port);
}
@@ -3782,6 +3820,10 @@ intel_dp_init_connector(struct intel_digital_port
*intel_dig_port,
if (is_edp(intel_dp)) {
intel_dp_init_panel_power_timestamps(intel_dp);
intel_dp_init_panel_power_sequencer(dev, intel_dp, &power_seq);
+ if (IS_VALLEYVIEW(dev)) {
+ intel_dp->edp_notifier.notifier_call =
edp_notify_handler;
+ register_reboot_notifier(&intel_dp->edp_notifier);
+ }
}
intel_dp_aux_init(intel_dp, intel_connector);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 328b1a7..ea2cc07 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -510,6 +510,8 @@ struct intel_dp {
unsigned long last_power_on;
unsigned long last_backlight_off;
bool psr_setup_done;
+ struct notifier_block edp_notifier;
+
bool use_tps3;
struct intel_connector *attached_connector;
--
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] Disabling the pipe A quirk for the Fujitsu S6010
Am 02.06.2014 19:39, schrieb Daniel Vetter: nack = not acknowledged, i.e. rejected. Comes from tcp. I've applied the patch instead to just remove the quirk on all i830M. Ok, thanks, I'm fine with that. Greetings, Thomas ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] WARNING on i915 - intel_panel
On 27 May 2014 08:15, Daniel Vetter wrote: > On Mon, May 26, 2014 at 9:44 PM, Pedro Ribeiro wrote: >> Kern.log is attached, but as you can see it does not contain the same >> verbose drm debug information as dmesg... Should I just keep piping >> dmesg to a file and then cat it all together? >> I never really understood why there are so many logs: kern, messages, >> syslog, instead of a single central log. > > Indeed, that one isn't useful either :( Next idea: Increase the > in-kernel dmesg buffer size and hope it all fits with log_buf_size=4M > (on the kernel cmdline). Maybe you can go even higher, not sure. > -Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch Daniel, doesn't seem like that is working. I'll leave it be and try to test new kernels and see if it just goes away. I'll report back if it doesn't. Regards, Pedro ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/bdw: Use timeout mode for RC6 on bdw
>From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter >Sent: Monday, June 02, 2014 1:26 AM >To: O'Rourke, Tom >Cc: intel-gfx@lists.freedesktop.org; Ben Widawsky; Kristen Carlson Accardi >Subject: Re: [Intel-gfx] [PATCH] drm/i915/bdw: Use timeout mode for RC6 on >bdw > >On Fri, May 30, 2014 at 11:30:18PM +, O'Rourke, Tom wrote: >> >On Wed, Apr 30, 2014 at 02:14:02PM -0700, Kristen Carlson Accardi wrote: >> >> On Thu, 01 May 2014 00:03:15 +0300 >> >> Imre Deak wrote: >> >> >> >> > On Wed, 2014-04-30 at 13:41 -0700, Ben Widawsky wrote: >> >> > > On Wed, Apr 30, 2014 at 01:34:36PM -0700, Kristen Carlson Accardi >wrote: >> >> > > > On Tue, 29 Apr 2014 22:31:49 -0700 Ben Widawsky >> >> > > > wrote: >> >> > > > >> >> > > > > On Wed, Apr 09, 2014 at 11:44:06AM -0700, Tom O'Rourke wrote: >> >> > > > > > Higher RC6 residency is observed using timeout mode >> >> > > > > > instead of EI mode. This applies to Broadwell only. >> >> > > > > > The difference is particularly noticeable with video >> >> > > > > > playback. >> >> > > > > > >> >> > > > > > Issue: VIZ-3778 >> >> > > > > > Change-Id: I62bb12e21caf19651034826b45cde7f73a80938d >> >> > > > > > Signed-off-by: Tom O'Rourke >> >> > > > > >> >> > > > > I've merged this one to my bdw-rc6 branch, and therefore my >> >> > > > > broadwell branch. Hopefully Kristen will see some improvement. >> >> > > > >> >> > > > Unfortunately, I built your bdw-rc6 branch along with the >> >> > > > revert I need to get my panel to work, and I get zero rc6 residency. >> >> > > > Do I have to explicitly enable it? >> >> > > >> >> > > I'm not actually sure. You can try it and let me know. I >> >> > > haven't had any time to verify the rebase. We can check my hack. >> >> > >> >> > Note that in -nightly you also have to update >> >> > sanitize_rc6_option() along with intel_enable_gt_powersave() and >> >> > intel_disable_gt_powersave() since atm these keep RC6 disabled on BDW. >> >> > >> >> > --Imre >> >> > >> >> >> >> >> >> Yes, I reverted fb5ed3b201fe5670c9ffeec3b5f6ff044d543c9e and was >> >> able to see some rc6 residency. With the idle workload, residency >> >> appears to be similar to before, so no regression. >> > >> >Thanks. I'll squash this in where appropriate. >> > >> >-- >> >Ben Widawsky, Intel Open Source Technology Center >> >> [TOR:] Can we get this patch merged now that RC6 is working on drm-intel- >nightly? > >Needs some review from bdw people. Also some relative residency >improvement date should be added to the commit message (yes, we're allowed >to do that now officially). >-Daniel >-- [TOR:] Hello bdw people, please review this patch. Is relative performance data now required in the commit message? A week ago this would have been prohibited. Thanks, Tom ___ 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)
> -Original Message- > From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] > > Hi Daniel, > > > > Would you please share more info about your idea? > > > > - What would be an avsink device represent here? > > E.g. on Intel platforms, will the whole display device have a child > > avsink device or multiple avsink devices for each DDI port? > > My idea would be to have one for each output pipe (i.e. the link between > audio and gfx), not one per ddi. Gfx driver would then let audio know > when a screen is connected and which one (e.g. exact model serial from > edid). > This is somewhat important for dp mst where there's no longer a fixed > relationship between audio pin and screen Thanks. But if we use avsink device, I prefer to have an avsink device per DDI or several avsink devices per DDI, It's because 1. Without DP MST, there is a fixed mapping between each audio codec pin and DDI; 2. With DP MST, the above pin: DDI mapping is still valid (at least on Intel platforms), and there is also a fixed mapping between each device (screen) connected to a pin/DDI. 3. HD-Audio driver creates a PCM (audio stream) devices for each pin. Keeping this behavior can make audio driver works on platforms without implementing the sound/gfx sync channel. And I guess in the future the audio driver will creates more than one PCM devices for a DP MST-capable pin, according how many devices a DDI can support. 4. Display mode change can change the pipe connected to a DDI even if the monitor stays on the same DDI, If we have an avsink device per pipe, the audio driver will have to check another avsink device for this case. It seems not convenient. > > - And for the relationship between audio driver and the avsink device, > > which would be the master and which would be the component? > > 1:1 for avsink:alsa pin (iirc it's called a pin, not sure about the name). > That way the audio driver has a clear point for getting at the eld and > similar information. Since the audio driver usually already binds to some device (PCI or platform device), I think the audio driver cannot bind to the new avsink devices created by display driver, and we need a new driver to handle these device and communication. While the display driver creates the new endpoint "avsink" devices, the audio driver can also create the same number of audio endpoint devices. And we could let the audio endpoint device be the master and its peer display endpoint device be the component. Thus the master/component framework can help us to bind/unbind each pair of display/audio endpoint devices. Is it doable? If okay, I'll modify the RFC and see if there are other gaps. > > In addition, the component framework does not touch PM now. > > And introducing PM to the component framework seems not easy since > > there can be potential conflict caused by parent-child relationship of > > the involved devices. > > Yeah, the entire PM situation seems to be a bit bad. It also looks like on > resume/suspend we still have problems, at least on the audio side since > we need to coordinate between 2 completel different underlying devices. > But at least with the parent->child relationship we have a guranatee that > the avsink won't be suspended after the gfx device is already off. > -Daniel Yes. You're right. And we could find a way to hide the Intel-specific display "power well" from the audio driver by using runtime PM API on these devices. Thanks Mengdong ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v1 0/3] Adding support for plane constant alpha drm property.
Gentle reminder for reviewing the patches. Thanks, Sagar On Mon, 2014-05-05 at 23:44 +0530, sagar.a.kam...@intel.com wrote: > From: Sagar Kamble > > This patch series introduces drm property for plane level alpha. > These patches are based on following patches which are already under > review/reviewed: > > Documentation: drm: describing drm properties exposed by various drivers > Propagate the error from intel_update_plane() up through > intel_plane_restore() to the caller. > > Sagar Kamble (3): > drm/i915: Add set_property function for planes > drm/i915: Enabling constant alpha drm property > Documentation: drm: describing plane constant alpha property > > Documentation/DocBook/drm.tmpl | 10 - > drivers/gpu/drm/i915/i915_dma.c | 11 ++ > drivers/gpu/drm/i915/i915_drv.h | 1 + > drivers/gpu/drm/i915/i915_reg.h | 1 + > drivers/gpu/drm/i915/intel_drv.h| 1 + > drivers/gpu/drm/i915/intel_sprite.c | 43 > - > 6 files changed, 65 insertions(+), 2 deletions(-) > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] [v5] drm/i915/bdw: Only use 2g GGTT for 32b platforms
Tested-by: "Yang, Guang A" With this patch, the 32 bit system can be able to boot normally. Best Regards~~ Open Source Technology Center (OTC) Terence Yang（杨光） Tel: 86-021-61167360 iNet: 8821-7360 > -Original Message- > From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of > Ben Widawsky > Sent: Wednesday, May 28, 2014 7:53 AM > To: Intel GFX > Cc: Ben Widawsky; sta...@vger.kernel.org; Widawsky, Benjamin > Subject: [Intel-gfx] [PATCH] [v5] drm/i915/bdw: Only use 2g GGTT for 32b > platforms > > Daniel requested in the bug that I use a 3GB fallback size. Since this is not > in > the spec as a valid size, I decided against it. We could potentially add a > patch to > bump it to 3GB on top of this one. > > This probably should be CC: stable - but I'll let the powers that be decide > that > one. > > Regression from a revert of the revert: > commit 7907f45bf9f67a1c5e5d4ae05bab428d7c2f43b2 > Author: Ben Widawsky > Date: Wed Feb 19 22:05:46 2014 -0800 > > Revert "drm/i915/bdw: Limit GTT to 2GB" > > v2: Change ifdef to 32b, instead of ifndef update comment > > v3. Update comment to not wrap (Daniel). > Update commit message > > v4: s/CONFIG_32/CONFIG_X86_32 (Jani). > > v5: s/CONFIG_x86_32BIT/CONFIG_x86_32, as meant in v4 s/32B/32b (chris) > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=76619 > Cc: sta...@vger.kernel.org > Reviewed-by: Daniel Vetter > Signed-off-by: Rodrigo Vivi > Signed-off-by: Ben Widawsky > --- > drivers/gpu/drm/i915/i915_gem_gtt.c | 7 +++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c > b/drivers/gpu/drm/i915/i915_gem_gtt.c > index 931b906..eec820a 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > @@ -1775,6 +1775,13 @@ static inline unsigned int > gen8_get_total_gtt_size(u16 bdw_gmch_ctl) > bdw_gmch_ctl &= BDW_GMCH_GGMS_MASK; > if (bdw_gmch_ctl) > bdw_gmch_ctl = 1 << bdw_gmch_ctl; > + > +#ifdef CONFIG_X86_32 > + /* Limit 32b platforms to a 2GB GGTT: 4 << 20 / pte size * PAGE_SIZE */ > + if (bdw_gmch_ctl > 4) > + bdw_gmch_ctl = 4; > +#endif > + > return bdw_gmch_ctl << 20; > } > > -- > 1.9.3 > > ___ > 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
