date:20160528

Re: [Intel-gfx] 4.7-rc0: redshift stopped working on intel display

2016-05-28 Thread Pavel Machek

On Sat 2016-05-28 12:12:06, Pavel Machek wrote:
> Hi!
> 
> It looks like redshift stopped working. Even pretty crazy settings
> have no visible effect:
> 
> pavel@amd:~$ redshift -O 1500 -g 6.6:6.6:6.6
> Using method `randr'.
> pavel@amd:~$ redshift -x
> Using method `randr'.
> pavel@amd:~$ uname -a
> Linux amd 4.6.0 #47 SMP Fri May 27 12:07:10 CEST 2016 x86_64 GNU/Linux
> pavel@amd:~$ redshift -O 5500 -g 6.6:6.6:6.6
> Using method `randr'.
> pavel@amd:~$ redshift -O 5500 -g 6.6:6.6:6.6 -b .3
> Using method `randr'.
> pavel@amd:~$
> pavel@amd:~$ lspci | grep VGA
> 00:02.0 VGA compatible controller: Intel Corporation 4 Series Chipset
> Integrated Graphics Controller (rev 03)
> pavel@amd:~$

suspend-to-ram + resume cycle updates the display to match the
settings. Not convenient, but...

Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [RFC 03/12] drm/i915: Support for GuC interrupts

2016-05-28 Thread Goel, Akash




On 5/28/2016 8:05 PM, Chris Wilson wrote:

On Sat, May 28, 2016 at 07:15:52PM +0530, Goel, Akash wrote:



On 5/28/2016 5:43 PM, Chris Wilson wrote:

On Sat, May 28, 2016 at 02:52:16PM +0530, Goel, Akash wrote:



On 5/28/2016 1:26 AM, Chris Wilson wrote:

On Sat, May 28, 2016 at 01:12:54AM +0530, akash.g...@intel.com wrote:

+void gen8_reset_guc_interrupts(struct drm_device *dev)
+{
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   i915_reg_t reg = gen6_pm_iir(dev_priv);



>From the looks of this we have multiple shadows for the same register.

That's very bad.

Now the platforms might be mutually exclusive, but it is still a mistake
that will catch us out.


Will check how it is in newer platforms.


+   spin_lock_irq(_priv->irq_lock);
+   I915_WRITE(reg, dev_priv->guc_events);
+   I915_WRITE(reg, dev_priv->guc_events);


What? Not even the tiniest of comments to explain?


Sorry actually just copied these steps as is from the
gen6_reset_rps_interrupts(), considering that the same set of
registers (IIR, IER, IMR) are involved here.
So the double clearing of IIR followed by posting read could be
needed here also.


Move it all to i915_irq.c and export routines to manipulate pm_iir such
that multiple users do not conflict.


Sorry but all interrupt related stuff for rps & GuC is already
inside i915_irq.c file.


Didn't notice, because this code didn't match my expectations for an
interface exported from i915_irq.c


Also the IER, IMR, IIR registers are being updated in a non
conflicting manner, no overlap between the PM bits & GuC events
bits.


They share a register, that mandates arbitration.



I think the arbitration (& serialization) is already being provided by 
irq_lock.



You mean to say need to have single set of routines only for interrupt
reset/enable/disable operations for rps & GuC.


Yes.



Fine will make them to use a single set of low level routines.


+   POSTING_READ(reg);


Again. Not even the tiniest of comments to explain?


+   spin_unlock_irq(_priv->irq_lock);
+}
+
+void gen8_enable_guc_interrupts(struct drm_device *dev)
+{
+   struct drm_i915_private *dev_priv = dev->dev_private;
+
+   spin_lock_irq(_priv->irq_lock);
+   if (!dev_priv->guc.interrupts_enabled) {
+   WARN_ON(I915_READ(gen6_pm_iir(dev_priv)) &
+   dev_priv->guc_events);
+   dev_priv->guc.interrupts_enabled = true;
+   I915_WRITE(gen6_pm_ier(dev_priv),
+ I915_READ(gen6_pm_ier(dev_priv)) | dev_priv->guc_events);


ier should be known, rmw on the reg should not be required.


Sorry same as above, copy paste from gen6_enable_rps_interrupts().
Without rmw, would this be fine ?

if (dev_priv->rps.interrupts_enabled)
I915_WRITE(gen6_pm_ier(dev_priv),
dev_priv->pm_rps_events | dev_priv->guc_events);
else
I915_WRITE(gen6_pm_ier(dev_priv), dev_priv->guc_events);


Still has the presumption of owning a register that is ostensibly used
by others.


Since pm_ier is a shared register and being used by others also, rmw
seem to be more suited here. Otherwise need to be aware of who all is
sharing it so as to update it without disturbing the bits owned by
others.


Exactly, see above. The best interfaces from i915_irq.c do not use rmw
on the register values.


Fine will try to do away with use rmw operation for pm_ier by
maintaining a bit mask of enabled interrupts (just like pm_irq_mask).




+static void gen8_guc2host_events_work(struct work_struct *work)
+{
+   struct drm_i915_private *dev_priv =
+   container_of(work, struct drm_i915_private, guc.events_work);
+
+   spin_lock_irq(_priv->irq_lock);
+   /* Speed up work cancelation during disabling guc interrupts. */
+   if (!dev_priv->guc.interrupts_enabled) {
+   spin_unlock_irq(_priv->irq_lock);
+   return;
+   }
+
+   DISABLE_RPM_WAKEREF_ASSERTS(dev_priv);


This just shouts that the code is broken.


You mean to say that ideally the wakeref_count (& power.usage_count)
should already be non zero here.


Yes. If it is not under your control, then you have a bug in your code.
Existing DISABLE_RPM_WAKEREF_ASSERTS tell us where we know we have a bug
(and hacks in place whilst we wait for patch review).



This work item can also execute in a window where wakeref_count (&
power.usage_count) have become zero but runtime suspend has not yet
kicked in (due to auto-suspend delay), so "RPM wakelock ref not held
during HW access" warning would come.


i.e. your code is buggy, as DISABLE_RPM_WAKEREF_ASSERTS implied.



But isn't this applicable to rps work item also ?.
If there is a way found to circumvent this, then same can be applied to 
GuC work item also. DISABLE_RPM_WAKEREF_ASSERTS is a stopgap solution.



void intel_crt_init(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_guc.h

Re: [Intel-gfx] [RFC 03/12] drm/i915: Support for GuC interrupts

2016-05-28 Thread Chris Wilson

On Sat, May 28, 2016 at 07:15:52PM +0530, Goel, Akash wrote:
> 
> 
> On 5/28/2016 5:43 PM, Chris Wilson wrote:
> >On Sat, May 28, 2016 at 02:52:16PM +0530, Goel, Akash wrote:
> >>
> >>
> >>On 5/28/2016 1:26 AM, Chris Wilson wrote:
> >>>On Sat, May 28, 2016 at 01:12:54AM +0530, akash.g...@intel.com wrote:
> From: Sagar Arun Kamble 
> 
> There are certain types of interrupts which Host can recieve from GuC.
> GuC ukernel sends an interrupt to Host for certain events, like for
> example retrieve/consume the logs generated by ukernel.
> This patch adds support to receive interrupts from GuC but currently
> enables & partially handles only the interrupt sent by GuC ukernel.
> Future patches will add support for handling other interrupt types.
> 
> Signed-off-by: Sagar Arun Kamble 
> Signed-off-by: Akash Goel 
> ---
> drivers/gpu/drm/i915/i915_drv.h|   1 +
> drivers/gpu/drm/i915/i915_guc_submission.c |   2 +
> drivers/gpu/drm/i915/i915_irq.c| 100 
> -
> drivers/gpu/drm/i915/i915_reg.h|  11 
> drivers/gpu/drm/i915/intel_drv.h   |   3 +
> drivers/gpu/drm/i915/intel_guc.h   |   5 ++
> drivers/gpu/drm/i915/intel_guc_loader.c|   1 +
> 7 files changed, 120 insertions(+), 3 deletions(-)
> 
> static void gen6_rps_irq_handler(struct drm_i915_private *dev_priv, u32 
> pm_iir);
> +static void gen8_guc_irq_handler(struct drm_i915_private *dev_priv, u32 
> pm_iir);
> 
> /* For display hotplug interrupt */
> static inline void
> @@ -400,6 +401,55 @@ void gen6_disable_rps_interrupts(struct 
> drm_i915_private *dev_priv)
>   synchronize_irq(dev_priv->dev->irq);
> }
> 
> +void gen8_reset_guc_interrupts(struct drm_device *dev)
> +{
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + i915_reg_t reg = gen6_pm_iir(dev_priv);
> >>>
> >>>From the looks of this we have multiple shadows for the same register.
> >>>That's very bad.
> >>>
> >>>Now the platforms might be mutually exclusive, but it is still a mistake
> >>>that will catch us out.
> >>>
> >>Will check how it is in newer platforms.
> >>
> + spin_lock_irq(_priv->irq_lock);
> + I915_WRITE(reg, dev_priv->guc_events);
> + I915_WRITE(reg, dev_priv->guc_events);
> >>>
> >>>What? Not even the tiniest of comments to explain?
> >>>
> >>Sorry actually just copied these steps as is from the
> >>gen6_reset_rps_interrupts(), considering that the same set of
> >>registers (IIR, IER, IMR) are involved here.
> >>So the double clearing of IIR followed by posting read could be
> >>needed here also.
> >
> >Move it all to i915_irq.c and export routines to manipulate pm_iir such
> >that multiple users do not conflict.
> >
> Sorry but all interrupt related stuff for rps & GuC is already
> inside i915_irq.c file.

Didn't notice, because this code didn't match my expectations for an
interface exported from i915_irq.c

> Also the IER, IMR, IIR registers are being updated in a non
> conflicting manner, no overlap between the PM bits & GuC events
> bits.

They share a register, that mandates arbitration.
 
> You mean to say need to have single set of routines only for interrupt
> reset/enable/disable operations for rps & GuC.

Yes.
 
> + POSTING_READ(reg);
> >>>
> >>>Again. Not even the tiniest of comments to explain?
> >>>
> + spin_unlock_irq(_priv->irq_lock);
> +}
> +
> +void gen8_enable_guc_interrupts(struct drm_device *dev)
> +{
> + struct drm_i915_private *dev_priv = dev->dev_private;
> +
> + spin_lock_irq(_priv->irq_lock);
> + if (!dev_priv->guc.interrupts_enabled) {
> + WARN_ON(I915_READ(gen6_pm_iir(dev_priv)) &
> + dev_priv->guc_events);
> + dev_priv->guc.interrupts_enabled = true;
> + I915_WRITE(gen6_pm_ier(dev_priv),
> +   I915_READ(gen6_pm_ier(dev_priv)) | dev_priv->guc_events);
> >>>
> >>>ier should be known, rmw on the reg should not be required.
> >>>
> >>Sorry same as above, copy paste from gen6_enable_rps_interrupts().
> >>Without rmw, would this be fine ?
> >>
> >>if (dev_priv->rps.interrupts_enabled)
> >>I915_WRITE(gen6_pm_ier(dev_priv),
> >>dev_priv->pm_rps_events | dev_priv->guc_events);
> >>else
> >>I915_WRITE(gen6_pm_ier(dev_priv), dev_priv->guc_events);
> >
> >Still has the presumption of owning a register that is ostensibly used
> >by others.
> >
> Since pm_ier is a shared register and being used by others also, rmw
> seem to be more suited here. Otherwise need to be aware of who all is
> sharing it so as to update it without disturbing the bits owned by
> others.

Exactly, see above. The best interfaces from i915_irq.c do not use rmw
on the register values.
 
>

Re: [Intel-gfx] [RFC 03/12] drm/i915: Support for GuC interrupts

2016-05-28 Thread Goel, Akash




On 5/28/2016 5:43 PM, Chris Wilson wrote:

On Sat, May 28, 2016 at 02:52:16PM +0530, Goel, Akash wrote:



On 5/28/2016 1:26 AM, Chris Wilson wrote:

On Sat, May 28, 2016 at 01:12:54AM +0530, akash.g...@intel.com wrote:

From: Sagar Arun Kamble 

There are certain types of interrupts which Host can recieve from GuC.
GuC ukernel sends an interrupt to Host for certain events, like for
example retrieve/consume the logs generated by ukernel.
This patch adds support to receive interrupts from GuC but currently
enables & partially handles only the interrupt sent by GuC ukernel.
Future patches will add support for handling other interrupt types.

Signed-off-by: Sagar Arun Kamble 
Signed-off-by: Akash Goel 
---
drivers/gpu/drm/i915/i915_drv.h|   1 +
drivers/gpu/drm/i915/i915_guc_submission.c |   2 +
drivers/gpu/drm/i915/i915_irq.c| 100 -
drivers/gpu/drm/i915/i915_reg.h|  11 
drivers/gpu/drm/i915/intel_drv.h   |   3 +
drivers/gpu/drm/i915/intel_guc.h   |   5 ++
drivers/gpu/drm/i915/intel_guc_loader.c|   1 +
7 files changed, 120 insertions(+), 3 deletions(-)



static void gen6_rps_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir);
+static void gen8_guc_irq_handler(struct drm_i915_private *dev_priv, u32 
pm_iir);

/* For display hotplug interrupt */
static inline void
@@ -400,6 +401,55 @@ void gen6_disable_rps_interrupts(struct drm_i915_private 
*dev_priv)
synchronize_irq(dev_priv->dev->irq);
}

+void gen8_reset_guc_interrupts(struct drm_device *dev)
+{
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   i915_reg_t reg = gen6_pm_iir(dev_priv);



>From the looks of this we have multiple shadows for the same register.

That's very bad.

Now the platforms might be mutually exclusive, but it is still a mistake
that will catch us out.


Will check how it is in newer platforms.


+   spin_lock_irq(_priv->irq_lock);
+   I915_WRITE(reg, dev_priv->guc_events);
+   I915_WRITE(reg, dev_priv->guc_events);


What? Not even the tiniest of comments to explain?


Sorry actually just copied these steps as is from the
gen6_reset_rps_interrupts(), considering that the same set of
registers (IIR, IER, IMR) are involved here.
So the double clearing of IIR followed by posting read could be
needed here also.


Move it all to i915_irq.c and export routines to manipulate pm_iir such
that multiple users do not conflict.

Sorry but all interrupt related stuff for rps & GuC is already inside 
i915_irq.c file.
Also the IER, IMR, IIR registers are being updated in a non conflicting 
manner, no overlap between the PM bits & GuC events bits.


You mean to say need to have single set of routines only for interrupt
reset/enable/disable operations for rps & GuC.


+   POSTING_READ(reg);


Again. Not even the tiniest of comments to explain?


+   spin_unlock_irq(_priv->irq_lock);
+}
+
+void gen8_enable_guc_interrupts(struct drm_device *dev)
+{
+   struct drm_i915_private *dev_priv = dev->dev_private;
+
+   spin_lock_irq(_priv->irq_lock);
+   if (!dev_priv->guc.interrupts_enabled) {
+   WARN_ON(I915_READ(gen6_pm_iir(dev_priv)) &
+   dev_priv->guc_events);
+   dev_priv->guc.interrupts_enabled = true;
+   I915_WRITE(gen6_pm_ier(dev_priv),
+ I915_READ(gen6_pm_ier(dev_priv)) | dev_priv->guc_events);


ier should be known, rmw on the reg should not be required.


Sorry same as above, copy paste from gen6_enable_rps_interrupts().
Without rmw, would this be fine ?

if (dev_priv->rps.interrupts_enabled)
I915_WRITE(gen6_pm_ier(dev_priv),
dev_priv->pm_rps_events | dev_priv->guc_events);
else
I915_WRITE(gen6_pm_ier(dev_priv), dev_priv->guc_events);


Still has the presumption of owning a register that is ostensibly used
by others.


Since pm_ier is a shared register and being used by others also, rmw
seem to be more suited here. Otherwise need to be aware of who all is
sharing it so as to update it without disturbing the bits owned by
others.


+static void gen8_guc2host_events_work(struct work_struct *work)
+{
+   struct drm_i915_private *dev_priv =
+   container_of(work, struct drm_i915_private, guc.events_work);
+
+   spin_lock_irq(_priv->irq_lock);
+   /* Speed up work cancelation during disabling guc interrupts. */
+   if (!dev_priv->guc.interrupts_enabled) {
+   spin_unlock_irq(_priv->irq_lock);
+   return;
+   }
+
+   DISABLE_RPM_WAKEREF_ASSERTS(dev_priv);


This just shouts that the code is broken.


You mean to say that ideally the wakeref_count (& power.usage_count)
should already be non zero here.


Yes. If it is not under your control, then you have a bug in your code.
Existing

Re: [Intel-gfx] [RFC 03/12] drm/i915: Support for GuC interrupts

2016-05-28 Thread Chris Wilson

On Sat, May 28, 2016 at 02:52:16PM +0530, Goel, Akash wrote:
> 
> 
> On 5/28/2016 1:26 AM, Chris Wilson wrote:
> >On Sat, May 28, 2016 at 01:12:54AM +0530, akash.g...@intel.com wrote:
> >>From: Sagar Arun Kamble 
> >>
> >>There are certain types of interrupts which Host can recieve from GuC.
> >>GuC ukernel sends an interrupt to Host for certain events, like for
> >>example retrieve/consume the logs generated by ukernel.
> >>This patch adds support to receive interrupts from GuC but currently
> >>enables & partially handles only the interrupt sent by GuC ukernel.
> >>Future patches will add support for handling other interrupt types.
> >>
> >>Signed-off-by: Sagar Arun Kamble 
> >>Signed-off-by: Akash Goel 
> >>---
> >> drivers/gpu/drm/i915/i915_drv.h|   1 +
> >> drivers/gpu/drm/i915/i915_guc_submission.c |   2 +
> >> drivers/gpu/drm/i915/i915_irq.c| 100 
> >> -
> >> drivers/gpu/drm/i915/i915_reg.h|  11 
> >> drivers/gpu/drm/i915/intel_drv.h   |   3 +
> >> drivers/gpu/drm/i915/intel_guc.h   |   5 ++
> >> drivers/gpu/drm/i915/intel_guc_loader.c|   1 +
> >> 7 files changed, 120 insertions(+), 3 deletions(-)
> >>
> >>diff --git a/drivers/gpu/drm/i915/i915_drv.h 
> >>b/drivers/gpu/drm/i915/i915_drv.h
> >>index e4c8e34..7aae033 100644
> >>--- a/drivers/gpu/drm/i915/i915_drv.h
> >>+++ b/drivers/gpu/drm/i915/i915_drv.h
> >>@@ -1790,6 +1790,7 @@ struct drm_i915_private {
> >>u32 gt_irq_mask;
> >>u32 pm_irq_mask;
> >>u32 pm_rps_events;
> >>+   u32 guc_events;
> >>u32 pipestat_irq_mask[I915_MAX_PIPES];
> >>
> >>struct i915_hotplug hotplug;
> >>diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c 
> >>b/drivers/gpu/drm/i915/i915_guc_submission.c
> >>index da7c242..c2f3a67 100644
> >>--- a/drivers/gpu/drm/i915/i915_guc_submission.c
> >>+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> >>@@ -990,6 +990,8 @@ int intel_guc_suspend(struct drm_device *dev)
> >>if (guc->guc_fw.guc_fw_load_status != GUC_FIRMWARE_SUCCESS)
> >>return 0;
> >>
> >>+   gen8_disable_guc_interrupts(dev);
> >>+
> >>ctx = dev_priv->kernel_context;
> >>
> >>data[0] = HOST2GUC_ACTION_ENTER_S_STATE;
> >>diff --git a/drivers/gpu/drm/i915/i915_irq.c 
> >>b/drivers/gpu/drm/i915/i915_irq.c
> >>index caaf1e2..b4294a8 100644
> >>--- a/drivers/gpu/drm/i915/i915_irq.c
> >>+++ b/drivers/gpu/drm/i915/i915_irq.c
> >>@@ -170,6 +170,7 @@ static void gen5_assert_iir_is_zero(struct 
> >>drm_i915_private *dev_priv,
> >> } while (0)
> >>
> >> static void gen6_rps_irq_handler(struct drm_i915_private *dev_priv, u32 
> >> pm_iir);
> >>+static void gen8_guc_irq_handler(struct drm_i915_private *dev_priv, u32 
> >>pm_iir);
> >>
> >> /* For display hotplug interrupt */
> >> static inline void
> >>@@ -400,6 +401,55 @@ void gen6_disable_rps_interrupts(struct 
> >>drm_i915_private *dev_priv)
> >>synchronize_irq(dev_priv->dev->irq);
> >> }
> >>
> >>+void gen8_reset_guc_interrupts(struct drm_device *dev)
> >>+{
> >>+   struct drm_i915_private *dev_priv = dev->dev_private;
> >>+   i915_reg_t reg = gen6_pm_iir(dev_priv);
> >
> >From the looks of this we have multiple shadows for the same register.
> >That's very bad.
> >
> >Now the platforms might be mutually exclusive, but it is still a mistake
> >that will catch us out.
> >
> Will check how it is in newer platforms.
> 
> >>+   spin_lock_irq(_priv->irq_lock);
> >>+   I915_WRITE(reg, dev_priv->guc_events);
> >>+   I915_WRITE(reg, dev_priv->guc_events);
> >
> >What? Not even the tiniest of comments to explain?
> >
> Sorry actually just copied these steps as is from the
> gen6_reset_rps_interrupts(), considering that the same set of
> registers (IIR, IER, IMR) are involved here.
> So the double clearing of IIR followed by posting read could be
> needed here also.

Move it all to i915_irq.c and export routines to manipulate pm_iir such
that multiple users do not conflict.
 
> >>+   POSTING_READ(reg);
> >
> >Again. Not even the tiniest of comments to explain?
> >
> >>+   spin_unlock_irq(_priv->irq_lock);
> >>+}
> >>+
> >>+void gen8_enable_guc_interrupts(struct drm_device *dev)
> >>+{
> >>+   struct drm_i915_private *dev_priv = dev->dev_private;
> >>+
> >>+   spin_lock_irq(_priv->irq_lock);
> >>+   if (!dev_priv->guc.interrupts_enabled) {
> >>+   WARN_ON(I915_READ(gen6_pm_iir(dev_priv)) &
> >>+   dev_priv->guc_events);
> >>+   dev_priv->guc.interrupts_enabled = true;
> >>+   I915_WRITE(gen6_pm_ier(dev_priv),
> >>+ I915_READ(gen6_pm_ier(dev_priv)) | dev_priv->guc_events);
> >
> >ier should be known, rmw on the reg should not be required.
> >
> Sorry same as above, copy paste from gen6_enable_rps_interrupts().
> Without rmw, would this be fine ?
> 
>   if (dev_priv->rps.interrupts_enabled)
>   I915_WRITE(gen6_pm_ier(dev_priv),
>

[Intel-gfx] 4.7-rc0: redshift stopped working on intel display

2016-05-28 Thread Pavel Machek

Hi!

It looks like redshift stopped working. Even pretty crazy settings
have no visible effect:

pavel@amd:~$ redshift -O 1500 -g 6.6:6.6:6.6
Using method `randr'.
pavel@amd:~$ redshift -x
Using method `randr'.
pavel@amd:~$ uname -a
Linux amd 4.6.0 #47 SMP Fri May 27 12:07:10 CEST 2016 x86_64 GNU/Linux
pavel@amd:~$ redshift -O 5500 -g 6.6:6.6:6.6
Using method `randr'.
pavel@amd:~$ redshift -O 5500 -g 6.6:6.6:6.6 -b .3
Using method `randr'.
pavel@amd:~$
pavel@amd:~$ lspci | grep VGA
00:02.0 VGA compatible controller: Intel Corporation 4 Series Chipset
Integrated Graphics Controller (rev 03)
pavel@amd:~$

Any ideas? Thanks,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [RFC 03/12] drm/i915: Support for GuC interrupts

2016-05-28 Thread Goel, Akash




On 5/28/2016 1:26 AM, Chris Wilson wrote:

On Sat, May 28, 2016 at 01:12:54AM +0530, akash.g...@intel.com wrote:

From: Sagar Arun Kamble 

There are certain types of interrupts which Host can recieve from GuC.
GuC ukernel sends an interrupt to Host for certain events, like for
example retrieve/consume the logs generated by ukernel.
This patch adds support to receive interrupts from GuC but currently
enables & partially handles only the interrupt sent by GuC ukernel.
Future patches will add support for handling other interrupt types.

Signed-off-by: Sagar Arun Kamble 
Signed-off-by: Akash Goel 
---
 drivers/gpu/drm/i915/i915_drv.h|   1 +
 drivers/gpu/drm/i915/i915_guc_submission.c |   2 +
 drivers/gpu/drm/i915/i915_irq.c| 100 -
 drivers/gpu/drm/i915/i915_reg.h|  11 
 drivers/gpu/drm/i915/intel_drv.h   |   3 +
 drivers/gpu/drm/i915/intel_guc.h   |   5 ++
 drivers/gpu/drm/i915/intel_guc_loader.c|   1 +
 7 files changed, 120 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e4c8e34..7aae033 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1790,6 +1790,7 @@ struct drm_i915_private {
u32 gt_irq_mask;
u32 pm_irq_mask;
u32 pm_rps_events;
+   u32 guc_events;
u32 pipestat_irq_mask[I915_MAX_PIPES];

struct i915_hotplug hotplug;
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c 
b/drivers/gpu/drm/i915/i915_guc_submission.c
index da7c242..c2f3a67 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -990,6 +990,8 @@ int intel_guc_suspend(struct drm_device *dev)
if (guc->guc_fw.guc_fw_load_status != GUC_FIRMWARE_SUCCESS)
return 0;

+   gen8_disable_guc_interrupts(dev);
+
ctx = dev_priv->kernel_context;

data[0] = HOST2GUC_ACTION_ENTER_S_STATE;
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index caaf1e2..b4294a8 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -170,6 +170,7 @@ static void gen5_assert_iir_is_zero(struct drm_i915_private 
*dev_priv,
 } while (0)

 static void gen6_rps_irq_handler(struct drm_i915_private *dev_priv, u32 
pm_iir);
+static void gen8_guc_irq_handler(struct drm_i915_private *dev_priv, u32 
pm_iir);

 /* For display hotplug interrupt */
 static inline void
@@ -400,6 +401,55 @@ void gen6_disable_rps_interrupts(struct drm_i915_private 
*dev_priv)
synchronize_irq(dev_priv->dev->irq);
 }

+void gen8_reset_guc_interrupts(struct drm_device *dev)
+{
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   i915_reg_t reg = gen6_pm_iir(dev_priv);


From the looks of this we have multiple shadows for the same register.
That's very bad.

Now the platforms might be mutually exclusive, but it is still a mistake
that will catch us out.


Will check how it is in newer platforms.


+   spin_lock_irq(_priv->irq_lock);
+   I915_WRITE(reg, dev_priv->guc_events);
+   I915_WRITE(reg, dev_priv->guc_events);


What? Not even the tiniest of comments to explain?

Sorry actually just copied these steps as is from the 
gen6_reset_rps_interrupts(), considering that the same set of registers 
(IIR, IER, IMR) are involved here.
So the double clearing of IIR followed by posting read could be needed 
here also.



+   POSTING_READ(reg);


Again. Not even the tiniest of comments to explain?


+   spin_unlock_irq(_priv->irq_lock);
+}
+
+void gen8_enable_guc_interrupts(struct drm_device *dev)
+{
+   struct drm_i915_private *dev_priv = dev->dev_private;
+
+   spin_lock_irq(_priv->irq_lock);
+   if (!dev_priv->guc.interrupts_enabled) {
+   WARN_ON(I915_READ(gen6_pm_iir(dev_priv)) &
+   dev_priv->guc_events);
+   dev_priv->guc.interrupts_enabled = true;
+   I915_WRITE(gen6_pm_ier(dev_priv),
+ I915_READ(gen6_pm_ier(dev_priv)) | dev_priv->guc_events);


ier should be known, rmw on the reg should not be required.


Sorry same as above, copy paste from gen6_enable_rps_interrupts().
Without rmw, would this be fine ?

if (dev_priv->rps.interrupts_enabled)
I915_WRITE(gen6_pm_ier(dev_priv),
dev_priv->pm_rps_events | dev_priv->guc_events);
else
I915_WRITE(gen6_pm_ier(dev_priv), dev_priv->guc_events);



+   gen6_enable_pm_irq(dev_priv, dev_priv->guc_events);
+   }
+   spin_unlock_irq(_priv->irq_lock);
+}
+
+void gen8_disable_guc_interrupts(struct drm_device *dev)
+{
+   struct drm_i915_private *dev_priv = dev->dev_private;
+
+   spin_lock_irq(_priv->irq_lock);
+   dev_priv->guc.interrupts_enabled = false;
+

[Intel-gfx] [PATCH] Add more Intel GPUs IDs to quirks.c

2016-05-28 Thread Nikolai Zhubr


Hello all,

Please someone have a look at this trivial and annoying issue.

Why users have to "fix" this problem for newer Intel PCI ids on a 1-by-1 
basis themselves yet again and again, after lots of time-wasting and 
hair-pulling.


+++ drivers/pci/quirks.c2016-05-23 00:34:16.001122309 +0300
@@ -3052,6 +3052,10 @@
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x0102, disable_igfx_irq);
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x010a, disable_igfx_irq);
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x0152, disable_igfx_irq);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x0402, disable_igfx_irq);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x0412, disable_igfx_irq);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x0C02, disable_igfx_irq);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x0C12, disable_igfx_irq);

(Not sure about all of them, but 402 is definitely required)

(Please CC me if necessary, I'm not subscribed @vger)

Thank you.
Nikolai
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [RFC 09/12] drm/i915: Add a char device file interface to capture GuC ukernel logs

2016-05-28 Thread Goel, Akash




On 5/28/2016 1:18 AM, Chris Wilson wrote:

On Sat, May 28, 2016 at 01:13:00AM +0530, akash.g...@intel.com wrote:

From: Akash Goel 

This patch provides a new character device file interface '/dev/dri/guc_log'
for the User to capture the GuC ukernel logs.


Do not make the device conditional on !CONFIG_DEBUGFS.
With a stable interface like the device, you do not need a debugfs
entry.


Sorry, actually I was unsure, don't know which interface would be most 
appropriate here.
Thought if CONFIG_DEBUG_FS is defined, then relay backed debugfs file 
might be the most preferred interface, in which case device file 
interface may not be required.


If device file interface is most suitable, then may not have a relay 
backed debugfs interface.


Please suggest.

Best regards
Akash

-Chris


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [RFC 03/12] drm/i915: Support for GuC interrupts

2016-05-28 Thread Goel, Akash




On 5/28/2016 1:13 AM, Chris Wilson wrote:

On Sat, May 28, 2016 at 01:12:54AM +0530, akash.g...@intel.com wrote:

From: Sagar Arun Kamble 

There are certain types of interrupts which Host can recieve from GuC.
GuC ukernel sends an interrupt to Host for certain events, like for
example retrieve/consume the logs generated by ukernel.
This patch adds support to receive interrupts from GuC but currently
enables & partially handles only the interrupt sent by GuC ukernel.
Future patches will add support for handling other interrupt types.


gen8 doesn't have a GuC. How can these functions be applicable to gen8?


Sorry I missed that in the Driver GuC is used only from Gen9 onwards.
But Gen8 (both BDW & CHV) does have a GuC.
I will rename the functions.

Best regards
Akash


-Chris


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] What's using all my DRI memory?

2016-05-28 Thread Felix Miata


Adam Nielsen composed on 2016-05-28 16:59 (UTC+1000):


Any suggestions with this?  My system just crashed again because the
Intel driver used up all 16GB of my system memory.


Which Intel driver version?

Which X server version?

Which kernel version?

Which window manager/DE?

Have you tried running Memtest overnight or longer?

How did you decide it's the Intel driver exhausting RAM?

If using X server > 16.999, have you tried removing the Intel driver, using 
instead the modeset driver built into the server?

--
"The wise are known for their understanding, and pleasant
words are persuasive." Proverbs 16:21 (New Living Translation)

 Team OS/2 ** Reg. Linux User #211409 ** a11y rocks!

Felix Miata  ***  http://fm.no-ip.com/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [v4.6-10530-g28165ec7a99b] i915: ERROR "CPU pipe/PCH transcoder" A FIFO underrun

2016-05-28 Thread Sedat Dilek

On 5/27/16, Chris Bainbridge  wrote:
> On 25 May 2016 at 08:31, Sedat Dilek  wrote:
>> Hi Daniel,
>>
>> with latest Linus Git I see this with my Intel SandyBridge GPU...
>>
>> [   17.629014] [drm:intel_cpu_fifo_underrun_irq_handler [i915]]
>> *ERROR* CPU pipe A FIFO underrun
>> [   17.630652] [drm:intel_set_pch_fifo_underrun_reporting [i915]]
>> *ERROR* uncleared pch fifo underrun on pch transcoder A
>> [   17.630685] [drm:intel_pch_fifo_underrun_irq_handler [i915]]
>> *ERROR* PCH transcoder A FIFO underrun
>
> Guessing this is https://bugs.freedesktop.org/show_bug.cgi?id=95736
>

Thanks for the hint!

Did you try with reverting this "first bad" commit mentionned in your
BR (or a "series of commits")?

- Sedat -
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] What's using all my DRI memory?

2016-05-28 Thread Adam Nielsen

Hi all,

Any suggestions with this?  My system just crashed again because the
Intel driver used up all 16GB of my system memory.

Thanks,
Adam.

On Mon, 9 May 2016 22:06:49 +1000
Adam Nielsen  wrote:

> Hi all,
> 
> I'm trying to track down an annoying bug which is making my system
> crash every two weeks, and in between those two weeks I get various
> programs, typically Firefox, killed every day or two due to out of
> memory errors.
> 
> Apparently all this memory is used by the Intel video driver, which
> doesn't leave enough memory on my 16GB system for applications to run.
> I'm not sure how to work out why this is the case.  Is it a bug in the
> Intel driver, not releasing the memory?  Is it a buggy program
> allocating too much display memory?  How can I figure out what's using
> up all the memory?
> 
> According to this, it looks like 8GB of memory is in use:
> 
> =
> $ cat /sys/kernel/debug/dri/0/i915_gem_objects 
> 854 objects, 8128716800 bytes
> 446 [17] objects, 657387520 [105037824] bytes in gtt
>   11 [1] active objects, 1662976 [1048576] bytes
>   435 [16] inactive objects, 655724544 [103989248] bytes
> 0 unbound objects, 0 bytes
> 28 purgeable objects, 23433216 bytes
> 5 pinned mappable objects, 103358464 bytes
> 4 fault mappable objects, 55271424 bytes
> 2147483648 [268435456] gtt total
> 
> systemd-logind: 801 objects, 7467040768 bytes (1593344 active, 599011328 
> inactive, 600604672 global, 6698557440 shared, 6850830336 unbound)
> systemd-logind: 40 objects, 693960704 bytes (0 active, 89206784 inactive, 
> 89206784 global, 520888320 shared, 599760896 unbound)
> =
> 
> This seems excessively high, so is there any way to figure out what
> it's being used for?
> 
> There must be a leak somewhere, because the system will run fine for a
> week, then programs start getting killed by the kernel OOM handler more
> and more frequently until I can't load any programs any more (they get
> killed during launch) and have to reboot the machine.
> 
> Many thanks,
> Adam.
> 
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx



___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✗ Ro.CI.BAT: failure for Support for sustained capturing of GuC firmware logs

2016-05-28 Thread Patchwork

== Series Details ==

Series: Support for sustained capturing of GuC firmware logs
URL   : https://patchwork.freedesktop.org/series/7910/
State : failure

== Summary ==

Series 7910v1 Support for sustained capturing of GuC firmware logs
http://patchwork.freedesktop.org/api/1.0/series/7910/revisions/1/mbox

Test drv_hangman:
Subgroup error-state-basic:
pass   -> DMESG-WARN (ro-bdw-i7-5557U)
pass   -> DMESG-WARN (ro-ilk1-i5-650)
pass   -> DMESG-WARN (ro-hsw-i3-4010u)
pass   -> DMESG-WARN (ro-bdw-i7-5600u)
pass   -> DMESG-WARN (ro-snb-i7-2620M)
pass   -> DMESG-WARN (ro-hsw-i7-4770r)
pass   -> DMESG-WARN (ro-ivb-i7-3770)
pass   -> DMESG-WARN (ro-byt-n2820)
Test gem_exec_flush:
Subgroup basic-batch-kernel-default-cmd:
pass   -> FAIL   (ro-byt-n2820)
Test gem_ringfill:
Subgroup basic-default-hang:
pass   -> DMESG-WARN (ro-bdw-i7-5557U)
pass   -> DMESG-WARN (ro-ilk1-i5-650)
pass   -> DMESG-WARN (ro-hsw-i3-4010u)
pass   -> DMESG-WARN (ro-bdw-i7-5600u)
pass   -> DMESG-WARN (ro-snb-i7-2620M)
pass   -> DMESG-WARN (ro-hsw-i7-4770r)
pass   -> DMESG-WARN (ro-ivb-i7-3770)
pass   -> DMESG-WARN (ro-byt-n2820)
Test kms_flip:
Subgroup basic-flip-vs-dpms:
dmesg-warn -> PASS   (ro-bdw-i7-5600u)
Subgroup basic-flip-vs-wf_vblank:
fail   -> PASS   (ro-bdw-i7-5600u)
Test kms_pipe_crc_basic:
Subgroup hang-read-crc-pipe-a:
pass   -> DMESG-WARN (ro-bdw-i7-5557U)
pass   -> DMESG-WARN (ro-ilk1-i5-650)
pass   -> DMESG-WARN (ro-hsw-i3-4010u)
pass   -> DMESG-WARN (ro-bdw-i7-5600u)
pass   -> DMESG-WARN (ro-snb-i7-2620M)
pass   -> DMESG-WARN (ro-hsw-i7-4770r)
pass   -> DMESG-WARN (ro-ivb-i7-3770)
pass   -> DMESG-WARN (ro-byt-n2820)
Subgroup hang-read-crc-pipe-b:
pass   -> DMESG-WARN (ro-bdw-i7-5557U)
pass   -> DMESG-WARN (ro-ilk1-i5-650)
pass   -> DMESG-WARN (ro-hsw-i3-4010u)
pass   -> DMESG-WARN (ro-bdw-i7-5600u)
pass   -> DMESG-WARN (ro-snb-i7-2620M)
pass   -> DMESG-WARN (ro-hsw-i7-4770r)
pass   -> DMESG-WARN (ro-ivb-i7-3770)
pass   -> DMESG-WARN (ro-byt-n2820)
Subgroup hang-read-crc-pipe-c:
pass   -> DMESG-WARN (ro-ivb-i7-3770)
pass   -> DMESG-WARN (ro-hsw-i7-4770r)
pass   -> DMESG-WARN (ro-bdw-i7-5600u)
pass   -> DMESG-WARN (ro-bdw-i7-5557U)
pass   -> DMESG-WARN (ro-hsw-i3-4010u)

fi-bdw-i7-5557u  total:209  pass:192  dwarn:5   dfail:0   fail:0   skip:12 
fi-hsw-i7-4770k  total:102  pass:85   dwarn:1   dfail:0   fail:0   skip:15 
fi-hsw-i7-4770r  total:209  pass:179  dwarn:5   dfail:0   fail:0   skip:25 
fi-skl-i7-6700k  total:209  pass:180  dwarn:4   dfail:0   fail:0   skip:25 
fi-snb-i7-2600   total:209  pass:166  dwarn:4   dfail:0   fail:0   skip:39 
ro-bdw-i7-5557U  total:209  pass:188  dwarn:6   dfail:0   fail:0   skip:15 
ro-bdw-i7-5600u  total:209  pass:176  dwarn:5   dfail:0   fail:0   skip:28 
ro-byt-n2820 total:209  pass:165  dwarn:4   dfail:0   fail:3   skip:37 
ro-hsw-i3-4010u  total:209  pass:181  dwarn:5   dfail:0   fail:0   skip:23 
ro-hsw-i7-4770r  total:209  pass:181  dwarn:5   dfail:0   fail:0   skip:23 
ro-ilk-i7-620lm  total:1pass:0dwarn:0   dfail:0   fail:0   skip:0  
ro-ilk1-i5-650   total:204  pass:142  dwarn:4   dfail:0   fail:1   skip:57 
ro-ivb-i7-3770   total:209  pass:172  dwarn:5   dfail:0   fail:0   skip:32 
ro-ivb2-i7-3770  total:209  pass:176  dwarn:5   dfail:0   fail:0   skip:28 
ro-snb-i7-2620M  total:209  pass:166  dwarn:4   dfail:0   fail:1   skip:38 
fi-bsw-n3050 failed to connect after reboot
fi-byt-n2820 failed to connect after reboot
ro-bsw-n3050 failed to connect after reboot

Results at /archive/results/CI_IGT_test/RO_Patchwork_1044/

5c0712f drm-intel-nightly: 2016y-05m-27d-12h-32m-45s UTC integration manifest
f00d84c drm/i915: Forcefully flush GuC log buffer on reset
956106a drm/i915: Add sysfs interface to capture the GuC ukernel logs
c7d48f8 drm/i915: Support to capture GuC logs by multiple clients via device 
file iface
c5e7b4f drm/i915: Add a char device file interface to capture GuC ukernel logs
e6f28b2 drm/i915: Store GuC ukernel logs in the local buffer
f9e5b8f drm/i915: Allocate local buffer to store GuC ukernel logs
5d5304c drm/i915: Store GuC ukernel logs in the relay buffer
79528fb drm/i915: Add a relay backed

Re: [Intel-gfx] 4.7-rc0: redshift stopped working on intel display

Re: [Intel-gfx] [RFC 03/12] drm/i915: Support for GuC interrupts

Re: [Intel-gfx] [RFC 03/12] drm/i915: Support for GuC interrupts

Re: [Intel-gfx] [RFC 03/12] drm/i915: Support for GuC interrupts

Re: [Intel-gfx] [RFC 03/12] drm/i915: Support for GuC interrupts

[Intel-gfx] 4.7-rc0: redshift stopped working on intel display

Re: [Intel-gfx] [RFC 03/12] drm/i915: Support for GuC interrupts

[Intel-gfx] [PATCH] Add more Intel GPUs IDs to quirks.c

Re: [Intel-gfx] [RFC 09/12] drm/i915: Add a char device file interface to capture GuC ukernel logs

Re: [Intel-gfx] [RFC 03/12] drm/i915: Support for GuC interrupts

Re: [Intel-gfx] What's using all my DRI memory?

Re: [Intel-gfx] [v4.6-10530-g28165ec7a99b] i915: ERROR "CPU pipe/PCH transcoder" A FIFO underrun

Re: [Intel-gfx] What's using all my DRI memory?

[Intel-gfx] ✗ Ro.CI.BAT: failure for Support for sustained capturing of GuC firmware logs

14 matches

Site Navigation

Mail list logo

Footer information