[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Add HP Convertible 360 to OpRegion quirk list

[Patchwork]

== Series Details ==

Series: drm/i915: Add HP Convertible 360 to OpRegion quirk list
URL   : https://patchwork.freedesktop.org/series/28411/
State : success

== Summary ==

Series 28411v1 drm/i915: Add HP Convertible 360 to OpRegion quirk list
https://patchwork.freedesktop.org/api/1.0/series/28411/revisions/1/mbox/

Test kms_pipe_crc_basic:
Subgroup hang-read-crc-pipe-b:
dmesg-warn -> PASS   (fi-pnv-d510) fdo#101597

fdo#101597 https://bugs.freedesktop.org/show_bug.cgi?id=101597

fi-bdw-5557u total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  
time:433s
fi-bdw-gvtdvmtotal:279  pass:265  dwarn:0   dfail:0   fail:0   skip:14  
time:421s
fi-blb-e6850 total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  
time:360s
fi-bsw-n3050 total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  
time:494s
fi-bxt-j4205 total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  
time:489s
fi-byt-j1900 total:279  pass:254  dwarn:1   dfail:0   fail:0   skip:24  
time:528s
fi-byt-n2820 total:279  pass:250  dwarn:1   dfail:0   fail:0   skip:28  
time:516s
fi-glk-2atotal:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  
time:589s
fi-hsw-4770  total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  
time:426s
fi-hsw-4770r total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  
time:407s
fi-ilk-650   total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  
time:433s
fi-ivb-3520m total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:506s
fi-ivb-3770  total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:477s
fi-kbl-7500u total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:459s
fi-kbl-7560u total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:568s
fi-kbl-r total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:585s
fi-pnv-d510  total:279  pass:223  dwarn:1   dfail:0   fail:0   skip:55  
time:583s
fi-skl-6260u total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:452s
fi-skl-6700k total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:642s
fi-skl-6770hqtotal:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:467s
fi-skl-gvtdvmtotal:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  
time:424s
fi-skl-x1585ltotal:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:483s
fi-snb-2520m total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  
time:550s
fi-snb-2600  total:279  pass:250  dwarn:0   dfail:0   fail:0   skip:29  
time:408s

92b02d6f8c3affc9e5bdcb4eff791fa33b38c6c5 drm-tip: 2017y-08m-04d-18h-09m-16s UTC 
integration manifest
69eb8ec609df drm/i915: Add HP Convertible 360 to OpRegion quirk list

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5330/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915: Add HP Convertible 360 to OpRegion quirk list

[Radhakrishna Sripada]

The quirklist for OpRegion panel type got introduced in
commit c8ebfad7a063 ("drm/i915: Ignore OpRegion panel
type except on select machines"). Adding the entry for
HP Convertible 360 to the white list.

Fixes: a05628195a0d ("drm/i915: Get panel_type from OpRegion panel details")
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=10
Cc: Nicholas Stommel 
Cc: Dhinakaran Pandiyan 
Cc: Jani Nikula 
Signed-off-by: Radhakrishna Sripada 
---
 drivers/gpu/drm/i915/intel_opregion.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_opregion.c 
b/drivers/gpu/drm/i915/intel_opregion.c
index 2bd03001cc70..a2ef4d4957d8 100644
--- a/drivers/gpu/drm/i915/intel_opregion.c
+++ b/drivers/gpu/drm/i915/intel_opregion.c
@@ -1035,6 +1035,20 @@ static const struct dmi_system_id 
intel_use_opregion_panel_type[] = {
DMI_MATCH(DMI_PRODUCT_NAME, "IX45GM2"),
},
},
+   {
+   .callback = intel_use_opregion_panel_type_callback,
+   .ident = "HP Spectre x360 Convertible",
+   .matches = {DMI_MATCH(DMI_SYS_VENDOR, "HP"),
+   DMI_MATCH(DMI_PRODUCT_NAME, "HP Spectre x360 
Convertible"),
+   },
+   },
+   {
+   .callback = intel_use_opregion_panel_type_callback,
+   .ident = "HP Spectre x360 Convertible",
+   .matches = {DMI_MATCH(DMI_SYS_VENDOR, "Hewlett-Packard"),
+   DMI_MATCH(DMI_PRODUCT_NAME, "HP Spectre x360 
Convertible 13"),
+   },
+   },
{ }
 };
 
-- 
2.9.3

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

Re: [Intel-gfx] [PATCH 07/15] drm/i915/guc: Create a GuC receive function

[Michel Thierry]

On 8/4/2017 9:27 AM, Michal Wajdeczko wrote:

From: Oscar Mateo 

This function, symmetrical to the send(), will handle Guc2Host message
interrupts (which at the moment still only covers requests to flush
the GuC logs).

Cc: Michal Wajdeczko 
Cc: Daniele Ceraolo Spurio 
Signed-off-by: Oscar Mateo 
Signed-off-by: Michal Wajdeczko 
---
  drivers/gpu/drm/i915/intel_uc.c | 18 +-
  drivers/gpu/drm/i915/intel_uc.h |  5 +
  2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index a091e83..258e0d0 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -109,6 +109,7 @@ void intel_uc_init_early(struct drm_i915_private *dev_priv)

 mutex_init(>send_mutex);
 guc->send = intel_guc_send_nop;
+   guc->recv = intel_guc_receive_nop;
 guc->notify = guc_write_irq_trigger;
  }

@@ -315,6 +316,7 @@ static int guc_enable_communication(struct intel_guc *guc)
 return intel_guc_enable_ct(guc);

 guc->send = intel_guc_send_mmio;
+   guc->recv = intel_guc_receive_mmio;
 return 0;
  }

@@ -326,6 +328,7 @@ static void guc_disable_communication(struct intel_guc *guc)
 intel_guc_disable_ct(guc);

 guc->send = intel_guc_send_nop;
+   guc->recv = intel_guc_receive_nop;
  }

  int intel_uc_init_hw(struct drm_i915_private *dev_priv)
@@ -466,6 +469,11 @@ int intel_guc_send_nop(struct intel_guc *guc, const u32 
*action, u32 len,
 return -ENODEV;
  }

+void intel_guc_receive_nop(struct intel_guc *guc)
+{
+   WARN(1, "Unexpected receive\n");
+}
+
  /*
   * This function implements the MMIO based host to GuC interface.
   */
@@ -532,7 +540,10 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 
*action, u32 len,
 return ret;
  }

-void intel_guc_notification_handler(struct intel_guc *guc)
+/*
+ * This function implements the MMIO based GuC to host interface.
+ */
+void intel_guc_receive_mmio(struct intel_guc *guc)
  {
 struct drm_i915_private *dev_priv = guc_to_i915(guc);
 u32 msg, flush;
@@ -565,6 +576,11 @@ void intel_guc_notification_handler(struct intel_guc *guc)
 }
  }

+void intel_guc_notification_handler(struct intel_guc *guc)
+{
+   guc->recv(guc);
+}
+
  int intel_guc_sample_forcewake(struct intel_guc *guc)
  {
 struct drm_i915_private *dev_priv = guc_to_i915(guc);
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 4808f47..6f20e66 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -208,6 +208,9 @@ struct intel_guc {
 /* GuC's FW specific send function */
 int (*send)(struct intel_guc *guc, const u32 *data, u32 len, u32 
*resp);

+   /* GuC's FW specific receive function */
+   void (*recv)(struct intel_guc *guc);
+


I think you already explained to some of us why returning any error code 
in recv would be pretty much useless (the error would be useful to 
whoever send the data). But feel free to say it yourself ;)



 /* GuC's FW specific notify function */
 void (*notify)(struct intel_guc *guc);
  };
@@ -230,6 +233,8 @@ void intel_guc_notification_handler(struct intel_guc *guc);
  int intel_guc_sample_forcewake(struct intel_guc *guc);
  int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len, u32 
*response);
  int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len, 
u32 *response);
+void intel_guc_receive_nop(struct intel_guc *guc);
+void intel_guc_receive_mmio(struct intel_guc *guc);

  static inline int intel_guc_send(struct intel_guc *guc, const u32 *action, 
u32 len)
  {
--
2.7.4

___
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

Re: [Intel-gfx] [PATCH 01/15] drm/i915/guc: Add support for data reporting in GuC responses

[Michal Wajdeczko]

On Fri, Aug 04, 2017 at 02:29:33PM -0700, Daniele Ceraolo Spurio wrote:
> 
> 
> On 04/08/17 13:40, Michel Thierry wrote:
> > On 8/4/2017 9:26 AM, Michal Wajdeczko wrote:
> > > GuC may return additional data in the command status response.
> > > Format and meaning of this data is action specific.
> > > We will use this non-negative data as a new success return value.
> > > 
> > > Signed-off-by: Michal Wajdeczko 
> > > Cc: Oscar Mateo 
> > > Cc: Michel Thierry 
> > > Cc: Daniele Ceraolo Spurio 
> > > ---
> > >   drivers/gpu/drm/i915/intel_guc_ct.c   | 14 +++---
> > >   drivers/gpu/drm/i915/intel_guc_fwif.h |  6 ++
> > >   drivers/gpu/drm/i915/intel_uc.c   |  5 -
> > >   3 files changed, 17 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c
> > > b/drivers/gpu/drm/i915/intel_guc_ct.c
> > > index c4cbec1..1249868 100644
> > > --- a/drivers/gpu/drm/i915/intel_guc_ct.c
> > > +++ b/drivers/gpu/drm/i915/intel_guc_ct.c
> > > @@ -387,9 +387,9 @@ static int ctch_send(struct intel_guc *guc,
> > >   err = wait_for_response(desc, fence, status);
> > >   if (unlikely(err))
> > >   return err;
> > > -if (*status != INTEL_GUC_STATUS_SUCCESS)
> > > +if (INTEL_GUC_RECV_TO_STATUS(*status) != INTEL_GUC_STATUS_SUCCESS)
> > >   return -EIO;
> > > -return 0;
> > > +return INTEL_GUC_RECV_TO_DATA(*status);
> > >   }
> > >   /*
> > > @@ -399,18 +399,18 @@ static int intel_guc_send_ct(struct intel_guc
> > > *guc, const u32 *action, u32 len)
> > >   {
> > >   struct intel_guc_ct_channel *ctch = >ct.host_channel;
> > >   u32 status = ~0; /* undefined */
> > > -int err;
> > > +int ret;
> > >   mutex_lock(>send_mutex);
> > > -err = ctch_send(guc, ctch, action, len, );
> > > -if (unlikely(err)) {
> > > +ret = ctch_send(guc, ctch, action, len, );
> > > +if (unlikely(ret < 0)) {
> > >   DRM_ERROR("CT: send action %#X failed; err=%d status=%#X\n",
> > > -  action[0], err, status);
> > > +  action[0], ret, status);
> > >   }
> > >   mutex_unlock(>send_mutex);
> > > -return err;
> > > +return ret;
> > >   }
> > >   /**
> > > diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h
> > > b/drivers/gpu/drm/i915/intel_guc_fwif.h
> > > index 5fa2860..98c0560 100644
> > > --- a/drivers/gpu/drm/i915/intel_guc_fwif.h
> > > +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
> > > @@ -567,10 +567,16 @@ enum intel_guc_action {
> > >* command in SS0. The response is distinguishable from a command
> > >* by the fact that all the MASK bits are set. The remaining bits
> > >* give more detail.
> > > + * Bits [16:27] are reserved for optional data reporting.
> 
> mmm, from the information I can find the optional data reporting bits are
> only [16:22], while [23:27] should be MBZ. Are you extending the range to
> cope with future changes on the GuC side or am I missing something? If it is
> the first case, my personal preference would be to stick with whatever is in
> the GuC API to avoid confusion. Since you're adding all the defines below it
> should be trivial to extend it if we ever need to.

It's the former case. But by looking the same information, only [15:0] bits are
declared for success/failure code, and [27:23] are MBZ for specific action.
So current proposal is still in line with that spec.

Michal

> 
> > >*/
> > >   #defineINTEL_GUC_RECV_MASK((u32)0xF000)
> > >   #defineINTEL_GUC_RECV_IS_RESPONSE(x)((u32)(x) >=
> > > INTEL_GUC_RECV_MASK)
> > >   #defineINTEL_GUC_RECV_STATUS(x)(INTEL_GUC_RECV_MASK | (x))
> > > +#define INTEL_GUC_RECV_DATA_SHIFT16
> > > +#define INTEL_GUC_RECV_DATA_MASK(0xFFF << INTEL_GUC_RECV_DATA_SHIFT)
> > > +#define INTEL_GUC_RECV_TO_STATUS(x)((x) & ~
> > > INTEL_GUC_RECV_DATA_MASK)
> > 
> > checkpatch should have complained about the blank space after '~'.
> > 
> > > +#define INTEL_GUC_RECV_TO_DATA(x)(((x) &
> > > INTEL_GUC_RECV_DATA_MASK) >> \
> > > + INTEL_GUC_RECV_DATA_SHIFT)
> > >   /* GUC will return status back to SOFT_SCRATCH_O_REG */
> > >   enum intel_guc_status {
> > > diff --git a/drivers/gpu/drm/i915/intel_uc.c
> > > b/drivers/gpu/drm/i915/intel_uc.c
> > > index 27e072c..ff25477 100644
> > > --- a/drivers/gpu/drm/i915/intel_uc.c
> > > +++ b/drivers/gpu/drm/i915/intel_uc.c
> > > @@ -502,7 +502,7 @@ int intel_guc_send_mmio(struct intel_guc *guc,
> > > const u32 *action, u32 len)
> > >  INTEL_GUC_RECV_MASK,
> > >  INTEL_GUC_RECV_MASK,
> > >  10, 10, );
> > > -if (status != INTEL_GUC_STATUS_SUCCESS) {
> > > +if (INTEL_GUC_RECV_TO_STATUS(status) != INTEL_GUC_STATUS_SUCCESS) {
> > >   /*
> > >* Either the GuC explicitly returned an error (which
> > >* we 

Re: [Intel-gfx] [PATCH 04/15] drm/i915/guc: Implement response handling in send_mmio()

[Michel Thierry]

On 8/4/2017 9:27 AM, Michal Wajdeczko wrote:

In addition to already returned small data encoded in the status MMIO,
GuC may write more additional data in remaining MMIO regs. Lets copy
all that regs into optionally provided response buffer.

Signed-off-by: Michal Wajdeczko 
Cc: Daniele Ceraolo Spurio 
Cc: Oscar Mateo 
---
  drivers/gpu/drm/i915/intel_uc.c | 5 +
  1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 77da750..c704968 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -517,6 +517,11 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 
*action, u32 len,
  " ret=%d status=0x%08X response=0x%08X\n",
  action[0], ret, status, I915_READ(SOFT_SCRATCH(15)));
 } else {
+   if (response) {
+   /* Skip reg[0] with the status/response mask */
+   for (i = 1; i < guc->send_regs.count; i++)
+   response[i] = I915_READ(guc_send_reg(guc, i));
+   }


new line here?

 /* Use data encoded in status dword as return value */
 ret = INTEL_GUC_RECV_TO_DATA(status);
 }


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

Re: [Intel-gfx] [PATCH 03/15] drm/i915/guc: Add send_and_receive() helper function

[Michel Thierry]

On 8/4/2017 9:27 AM, Michal Wajdeczko wrote:

In the previous patch we have changed signature of the send function
pointer but we didn't modify signature of the corresponding helper
function to minimize number of required changes. Let's add separate
helper to expose new functionality but still hide underlying details.

Signed-off-by: Michal Wajdeczko 
Cc: Oscar Mateo 
Cc: Michel Thierry 
Cc: Daniele Ceraolo Spurio 
---
  drivers/gpu/drm/i915/intel_uc.h | 7 +++
  1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 53ea5f1..9353ac3 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -235,6 +235,13 @@ static inline int intel_guc_send(struct intel_guc *guc, 
const u32 *action, u32 l
return guc->send(guc, action, len, NULL);
  }
  
+static inline int intel_guc_send_and_receive(struct intel_guc *guc,

+const u32 *action, u32 len,
+u32 *response)
+{
+   return guc->send(guc, action, len, response);
+}
+
  static inline void intel_guc_notify(struct intel_guc *guc)
  {
guc->notify(guc);



No users for now, anyway

Reviewed-by: Michel Thierry 

I just think this patch should come after #4 ("drm/i915/guc: Implement 
response handling in send_mmio()").

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

Re: [Intel-gfx] [PATCH 01/15] drm/i915/guc: Add support for data reporting in GuC responses

[Daniele Ceraolo Spurio]

On 04/08/17 13:40, Michel Thierry wrote:

On 8/4/2017 9:26 AM, Michal Wajdeczko wrote:

GuC may return additional data in the command status response.
Format and meaning of this data is action specific.
We will use this non-negative data as a new success return value.

Signed-off-by: Michal Wajdeczko 
Cc: Oscar Mateo 
Cc: Michel Thierry 
Cc: Daniele Ceraolo Spurio 
---
  drivers/gpu/drm/i915/intel_guc_ct.c   | 14 +++---
  drivers/gpu/drm/i915/intel_guc_fwif.h |  6 ++
  drivers/gpu/drm/i915/intel_uc.c   |  5 -
  3 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c 
b/drivers/gpu/drm/i915/intel_guc_ct.c

index c4cbec1..1249868 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/intel_guc_ct.c
@@ -387,9 +387,9 @@ static int ctch_send(struct intel_guc *guc,
  err = wait_for_response(desc, fence, status);
  if (unlikely(err))
  return err;
-if (*status != INTEL_GUC_STATUS_SUCCESS)
+if (INTEL_GUC_RECV_TO_STATUS(*status) != INTEL_GUC_STATUS_SUCCESS)
  return -EIO;
-return 0;
+return INTEL_GUC_RECV_TO_DATA(*status);
  }
  /*
@@ -399,18 +399,18 @@ static int intel_guc_send_ct(struct intel_guc 
*guc, const u32 *action, u32 len)

  {
  struct intel_guc_ct_channel *ctch = >ct.host_channel;
  u32 status = ~0; /* undefined */
-int err;
+int ret;
  mutex_lock(>send_mutex);
-err = ctch_send(guc, ctch, action, len, );
-if (unlikely(err)) {
+ret = ctch_send(guc, ctch, action, len, );
+if (unlikely(ret < 0)) {
  DRM_ERROR("CT: send action %#X failed; err=%d status=%#X\n",
-  action[0], err, status);
+  action[0], ret, status);
  }
  mutex_unlock(>send_mutex);
-return err;
+return ret;
  }
  /**
diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h 
b/drivers/gpu/drm/i915/intel_guc_fwif.h

index 5fa2860..98c0560 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -567,10 +567,16 @@ enum intel_guc_action {
   * command in SS0. The response is distinguishable from a command
   * by the fact that all the MASK bits are set. The remaining bits
   * give more detail.
+ * Bits [16:27] are reserved for optional data reporting.


mmm, from the information I can find the optional data reporting bits 
are only [16:22], while [23:27] should be MBZ. Are you extending the 
range to cope with future changes on the GuC side or am I missing 
something? If it is the first case, my personal preference would be to 
stick with whatever is in the GuC API to avoid confusion. Since you're 
adding all the defines below it should be trivial to extend it if we 
ever need to.



   */
  #defineINTEL_GUC_RECV_MASK((u32)0xF000)
  #defineINTEL_GUC_RECV_IS_RESPONSE(x)((u32)(x) >= 
INTEL_GUC_RECV_MASK)

  #defineINTEL_GUC_RECV_STATUS(x)(INTEL_GUC_RECV_MASK | (x))
+#define INTEL_GUC_RECV_DATA_SHIFT16
+#define INTEL_GUC_RECV_DATA_MASK(0xFFF << INTEL_GUC_RECV_DATA_SHIFT)
+#define INTEL_GUC_RECV_TO_STATUS(x)((x) & ~ 
INTEL_GUC_RECV_DATA_MASK)


checkpatch should have complained about the blank space after '~'.

+#define INTEL_GUC_RECV_TO_DATA(x)(((x) & 
INTEL_GUC_RECV_DATA_MASK) >> \

+ INTEL_GUC_RECV_DATA_SHIFT)
  /* GUC will return status back to SOFT_SCRATCH_O_REG */
  enum intel_guc_status {
diff --git a/drivers/gpu/drm/i915/intel_uc.c 
b/drivers/gpu/drm/i915/intel_uc.c

index 27e072c..ff25477 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -502,7 +502,7 @@ int intel_guc_send_mmio(struct intel_guc *guc, 
const u32 *action, u32 len)

 INTEL_GUC_RECV_MASK,
 INTEL_GUC_RECV_MASK,
 10, 10, );
-if (status != INTEL_GUC_STATUS_SUCCESS) {
+if (INTEL_GUC_RECV_TO_STATUS(status) != INTEL_GUC_STATUS_SUCCESS) {
  /*
   * Either the GuC explicitly returned an error (which
   * we convert to -EIO here) or no response at all was
@@ -514,6 +514,9 @@ int intel_guc_send_mmio(struct intel_guc *guc, 
const u32 *action, u32 len)

  DRM_WARN("INTEL_GUC_SEND: Action 0x%X failed;"
   " ret=%d status=0x%08X response=0x%08X\n",
   action[0], ret, status, I915_READ(SOFT_SCRATCH(15)));
+} else {
+/* Use data encoded in status dword as return value */
+ret = INTEL_GUC_RECV_TO_DATA(status);
  }
  intel_uncore_forcewake_put(dev_priv, guc->send_regs.fw_domains);



Other than the blank space after that '~', it looks good to me.

Just a note, for other people reading this; there are 3 cases in which 
intel_guc_send return value is only checked for truthiness (i.e. 
__guc_allocate_doorbell, __guc_deallocate_doorbell and 
intel_guc_sample_forcewake callers are checking 

Re: [Intel-gfx] [PATCH 02/15] drm/i915/guc: Prepare send() function to accept bigger response

[Michel Thierry]

On 8/4/2017 9:26 AM, Michal Wajdeczko wrote:

This is a preparation step for the upcoming patches.
We already can return some small data decoded from the command
status, but we will need more in the future.

Signed-off-by: Michal Wajdeczko 
Cc: Oscar Mateo 
Cc: Michel Thierry 
Cc: Daniele Ceraolo Spurio 
---
  drivers/gpu/drm/i915/intel_guc_ct.c | 7 ---
  drivers/gpu/drm/i915/intel_uc.c | 6 --
  drivers/gpu/drm/i915/intel_uc.h | 8 
  3 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c 
b/drivers/gpu/drm/i915/intel_guc_ct.c
index 1249868..c17cb42 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/intel_guc_ct.c
@@ -79,7 +79,7 @@ static int guc_action_register_ct_buffer(struct intel_guc 
*guc,
int err;
  
  	/* Can't use generic send(), CT registration must go over MMIO */

-   err = intel_guc_send_mmio(guc, action, ARRAY_SIZE(action));
+   err = intel_guc_send_mmio(guc, action, ARRAY_SIZE(action), NULL);
if (err)


another one that is only checking for diff 0 (re prev patch), I know the 
h2g register-ct cmd shouldn't be returning any positive value though.



DRM_ERROR("CT: register %s buffer failed; err=%d\n",
  guc_ct_buffer_type_to_str(type), err);
@@ -98,7 +98,7 @@ static int guc_action_deregister_ct_buffer(struct intel_guc 
*guc,
int err;
  
  	/* Can't use generic send(), CT deregistration must go over MMIO */

-   err = intel_guc_send_mmio(guc, action, ARRAY_SIZE(action));
+   err = intel_guc_send_mmio(guc, action, ARRAY_SIZE(action), NULL);
if (err)
DRM_ERROR("CT: deregister %s buffer failed; owner=%d err=%d\n",
  guc_ct_buffer_type_to_str(type), owner, err);
@@ -395,7 +395,8 @@ static int ctch_send(struct intel_guc *guc,
  /*
   * Command Transport (CT) buffer based GuC send function.
   */
-static int intel_guc_send_ct(struct intel_guc *guc, const u32 *action, u32 len)
+static int intel_guc_send_ct(struct intel_guc *guc, const u32 *action, u32 len,
+u32 *response)
  {
struct intel_guc_ct_channel *ctch = >ct.host_channel;
u32 status = ~0; /* undefined */
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index ff25477..77da750 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -459,7 +459,8 @@ void intel_uc_fini_hw(struct drm_i915_private *dev_priv)
i915_ggtt_disable_guc(dev_priv);
  }
  
-int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len)

+int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len,
+  u32 *response)
  {
WARN(1, "Unexpected send: action=%#x\n", *action);
return -ENODEV;
@@ -468,7 +469,8 @@ int intel_guc_send_nop(struct intel_guc *guc, const u32 
*action, u32 len)
  /*
   * This function implements the MMIO based host to GuC interface.
   */
-int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len)
+int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len,
+   u32 *response)
  {
struct drm_i915_private *dev_priv = guc_to_i915(guc);
u32 status;
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 22ae52b..53ea5f1 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -206,7 +206,7 @@ struct intel_guc {
struct mutex send_mutex;
  
  	/* GuC's FW specific send function */

-   int (*send)(struct intel_guc *guc, const u32 *data, u32 len);
+   int (*send)(struct intel_guc *guc, const u32 *data, u32 len, u32 *resp);
  
  	/* GuC's FW specific notify function */

void (*notify)(struct intel_guc *guc);
@@ -227,12 +227,12 @@ void intel_uc_fini_fw(struct drm_i915_private *dev_priv);
  int intel_uc_init_hw(struct drm_i915_private *dev_priv);
  void intel_uc_fini_hw(struct drm_i915_private *dev_priv);
  int intel_guc_sample_forcewake(struct intel_guc *guc);
-int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len);
-int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len);
+int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len, u32 
*response);
+int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len, u32 
*response);
  
  static inline int intel_guc_send(struct intel_guc *guc, const u32 *action, u32 len)

  {
-   return guc->send(guc, action, len);
+   return guc->send(guc, action, len, NULL);
  }
  
  static inline void intel_guc_notify(struct intel_guc *guc)




Reviewed-by: Michel Thierry 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org

Re: [Intel-gfx] [PATCH 01/15] drm/i915/guc: Add support for data reporting in GuC responses

[Michel Thierry]

On 8/4/2017 9:26 AM, Michal Wajdeczko wrote:

GuC may return additional data in the command status response.
Format and meaning of this data is action specific.
We will use this non-negative data as a new success return value.

Signed-off-by: Michal Wajdeczko 
Cc: Oscar Mateo 
Cc: Michel Thierry 
Cc: Daniele Ceraolo Spurio 
---
  drivers/gpu/drm/i915/intel_guc_ct.c   | 14 +++---
  drivers/gpu/drm/i915/intel_guc_fwif.h |  6 ++
  drivers/gpu/drm/i915/intel_uc.c   |  5 -
  3 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c 
b/drivers/gpu/drm/i915/intel_guc_ct.c
index c4cbec1..1249868 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/intel_guc_ct.c
@@ -387,9 +387,9 @@ static int ctch_send(struct intel_guc *guc,
err = wait_for_response(desc, fence, status);
if (unlikely(err))
return err;
-   if (*status != INTEL_GUC_STATUS_SUCCESS)
+   if (INTEL_GUC_RECV_TO_STATUS(*status) != INTEL_GUC_STATUS_SUCCESS)
return -EIO;
-   return 0;
+   return INTEL_GUC_RECV_TO_DATA(*status);
  }
  
  /*

@@ -399,18 +399,18 @@ static int intel_guc_send_ct(struct intel_guc *guc, const 
u32 *action, u32 len)
  {
struct intel_guc_ct_channel *ctch = >ct.host_channel;
u32 status = ~0; /* undefined */
-   int err;
+   int ret;
  
  	mutex_lock(>send_mutex);
  
-	err = ctch_send(guc, ctch, action, len, );

-   if (unlikely(err)) {
+   ret = ctch_send(guc, ctch, action, len, );
+   if (unlikely(ret < 0)) {
DRM_ERROR("CT: send action %#X failed; err=%d status=%#X\n",
- action[0], err, status);
+ action[0], ret, status);
}
  
  	mutex_unlock(>send_mutex);

-   return err;
+   return ret;
  }
  
  /**

diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h 
b/drivers/gpu/drm/i915/intel_guc_fwif.h
index 5fa2860..98c0560 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -567,10 +567,16 @@ enum intel_guc_action {
   * command in SS0. The response is distinguishable from a command
   * by the fact that all the MASK bits are set. The remaining bits
   * give more detail.
+ * Bits [16:27] are reserved for optional data reporting.
   */
  #define   INTEL_GUC_RECV_MASK ((u32)0xF000)
  #define   INTEL_GUC_RECV_IS_RESPONSE(x)   ((u32)(x) >= 
INTEL_GUC_RECV_MASK)
  #define   INTEL_GUC_RECV_STATUS(x)(INTEL_GUC_RECV_MASK | (x))
+#define INTEL_GUC_RECV_DATA_SHIFT  16
+#define INTEL_GUC_RECV_DATA_MASK   (0xFFF << INTEL_GUC_RECV_DATA_SHIFT)
+#define INTEL_GUC_RECV_TO_STATUS(x)((x) & ~ INTEL_GUC_RECV_DATA_MASK)


checkpatch should have complained about the blank space after '~'.


+#define INTEL_GUC_RECV_TO_DATA(x)  (((x) & INTEL_GUC_RECV_DATA_MASK) >> \
+INTEL_GUC_RECV_DATA_SHIFT)
  
  /* GUC will return status back to SOFT_SCRATCH_O_REG */

  enum intel_guc_status {
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 27e072c..ff25477 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -502,7 +502,7 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 
*action, u32 len)
   INTEL_GUC_RECV_MASK,
   INTEL_GUC_RECV_MASK,
   10, 10, );
-   if (status != INTEL_GUC_STATUS_SUCCESS) {
+   if (INTEL_GUC_RECV_TO_STATUS(status) != INTEL_GUC_STATUS_SUCCESS) {
/*
 * Either the GuC explicitly returned an error (which
 * we convert to -EIO here) or no response at all was
@@ -514,6 +514,9 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 
*action, u32 len)
DRM_WARN("INTEL_GUC_SEND: Action 0x%X failed;"
 " ret=%d status=0x%08X response=0x%08X\n",
 action[0], ret, status, I915_READ(SOFT_SCRATCH(15)));
+   } else {
+   /* Use data encoded in status dword as return value */
+   ret = INTEL_GUC_RECV_TO_DATA(status);
}
  
  	intel_uncore_forcewake_put(dev_priv, guc->send_regs.fw_domains);




Other than the blank space after that '~', it looks good to me.

Just a note, for other people reading this; there are 3 cases in which 
intel_guc_send return value is only checked for truthiness (i.e. 
__guc_allocate_doorbell, __guc_deallocate_doorbell and 
intel_guc_sample_forcewake callers are checking for 'if (err)').


I know none of the existing H2G commands will return any extra data, so 
intel_guc_send should be returning only negative numbers or zero (for now).


-Michel
___

Re: [Intel-gfx] [PATCH 05/15] drm/i915/guc: Move Guc notification handling to separate function

[Michal Wajdeczko]

On Fri, Aug 04, 2017 at 07:00:33PM +0100, Chris Wilson wrote:
> Quoting Michal Wajdeczko (2017-08-04 17:27:02)
> > To allow future code reuse. While here, fix comment style.
> 
> Might as well fix the alignment as well then... 
> 
> > +   /* Handle flush interrupt in bottom half */
> > +   queue_work(dev_priv->guc.log.runtime.flush_wq,
> > +   _priv->guc.log.runtime.flush_work);
> > +
> > +   dev_priv->guc.log.flush_interrupt_count++;
> > +   } else {
> > +   /*
> > +* Not clearing of unhandled event bits won't result in
> > +* re-triggering of the interrupt.
> > +*/
> > +   }
> > +}
> 
> Argh, and those are spaces not tabs! Back to the checkpatch dungeon you
> go, until you are sorry!

Hmm, that's weird. On my side there were all tabs.

$ scripts/checkpatch.pl 
../RFC/0005-drm-i915-guc-Move-Guc-notification-handling-to-separ.patch
total: 0 errors, 0 warnings, 89 lines checked

../RFC/0005-drm-i915-guc-Move-Guc-notification-handling-to-separ.patch has no 
obvious style problems and is ready for submission.

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

Re: [Intel-gfx] [PATCH 12/15] drm/i915/guc: Prepare to process incoming requests from CT

[Michal Wajdeczko]

On Fri, Aug 04, 2017 at 06:13:42PM +0100, Chris Wilson wrote:
> Quoting Michal Wajdeczko (2017-08-04 17:27:09)
> >  static inline const char *guc_ct_buffer_type_to_str(u32 type)
> > @@ -600,13 +609,76 @@ static int guc_handle_response(struct intel_guc *guc, 
> > const u32 *data)
> >  static int guc_handle_request(struct intel_guc *guc, const u32 *data)
> >  {
> > u32 header = data[0];
> > +   u32 len = ct_header_get_len(header) + 1; /* total len with header */
> > +   struct ct_incoming_request *request;
> > +   unsigned long flags;
> >  
> > GEM_BUG_ON(ct_header_is_response(header));
> > /* data layout beyond header is request specific */
> >  
> > +   request = kmalloc(sizeof(*request), GFP_ATOMIC);
> > +   if (unlikely(!request)) {
> > +   DRM_ERROR("CT: dropping request %*phn\n", 4*len, data);
> > +   return 0; /* XXX: -ENOMEM ? */
> > +   }
> > +
> > +   GEM_BUG_ON(len > GUC_CT_MSG_LEN_MASK + 1);
> 
> This is incoming from the guc, if we can validate it, do so. Keep
> GEM_BUG_ON() for programming errors and absolute catastrophe.

Sorry, this check is leftover from earlier design.

Now it will be always satisfied as len can't be encoded beyond given mask ;)
I can change it into more appropriate runtime check against our buffer size:

if (4*len > sizeof(request->data)) ...

or into compile time check (with assumption then len can't larger than mask)

BUILD_BUG_ON(sizeof(request->data) < 4*(GUC_MSG_LEN_MASK+1));

or both

-Michal


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

Re: [Intel-gfx] [PATCH i-g-t] tests/kms_frontbuffer_tracking: increase FBC wait timeout to 5s

[Paulo Zanoni]

Em Sex, 2017-08-04 às 09:47 +, Lofstedt, Marta escreveu:
> +Paolo
> 
> > -Original Message-
> > From: Lofstedt, Marta
> > Sent: Wednesday, June 28, 2017 2:17 PM
> > To: intel-gfx@lists.freedesktop.org
> > Cc: Latvala, Petri ; Lofstedt, Marta
> > 
> > Subject: [PATCH i-g-t] tests/kms_frontbuffer_tracking: increase FBC
> > wait
> > timeout to 5s
> > 
> > The subtests: igt@kms_frontbuffer_tracking@fbc-*draw*
> > has non-consistent results, pending between fail and pass.
> > The fails are always due to "FBC disabled".
> > With this increase in timeout the flip-flop behavior is no longer
> > reproducible.

This is a partial revert of:

64590c7b768dc8d8dd962f812d5ff5a39e7e8b54
kms_frontbuffer_tracking: reduce the FBC wait timeout to 2s

(but there's no need to make it a full revert if you don't need)

It would be nice to investigate why we're needing 5 seconds instead of
2 now, the document it in the commit message. Also document that this
is a partial revert.

Acked-by: Paulo Zanoni 

> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101623
> > Signed-off-by: Marta Lofstedt 
> > ---
> >  tests/kms_frontbuffer_tracking.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/tests/kms_frontbuffer_tracking.c
> > b/tests/kms_frontbuffer_tracking.c
> > index c24e4a81..8bec5d5a 100644
> > --- a/tests/kms_frontbuffer_tracking.c
> > +++ b/tests/kms_frontbuffer_tracking.c
> > @@ -923,7 +923,7 @@ static bool fbc_stride_not_supported(void)
> > 
> >  static bool fbc_wait_until_enabled(void)  {
> > -   return igt_wait(fbc_is_enabled(), 2000, 1);
> > +   return igt_wait(fbc_is_enabled(), 5000, 1);
> >  }
> > 
> >  static bool psr_wait_until_enabled(void)
> > --
> > 2.11.0
> 
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v4 2/4] drm/i915/psr: Account for sink CRC raciness on some panels

[Pandiyan, Dhinakaran]

On Thu, 2017-08-03 at 11:07 -0700, Rodrigo Vivi wrote:
> On Tue, Jul 18, 2017 at 2:34 PM, Jim Bride  wrote:
> > According to the eDP spec, when the count field in TEST_SINK_MISC
> > increments then the six bytes of sink CRC information in the DPCD
> > should be valid.  Unfortunately, this doesn't seem to be the case
> > on some panels, and as a result we get some incorrect and inconsistent
> > values from the sink CRC DPCD locations at times.  This problem exhibits
> > itself more on faster processors (relative failure rates HSW < SKL < KBL.)
> > In order to try and account for this, we try a lot harder to read the sink
> > CRC until we get consistent values twice in a row before returning what we
> > read and delay for a time before trying to read.  We still see some
> > occasional failures, but reading the sink CRC is much more reliable,
> > particularly on SKL and KBL, with these changes than without.

I'm curious if we get the correct crc if we waited a full second.

> 
> Is DK now ok with this description?
> I believe he requested more info here.
> 
> >
> > v2: * Reduce number of retries when reading the sink CRC (Jani)
> > * Refactor to minimize changes to the code (Jani)
> > * Rebase
> > v3: * Rebase
> > v4: * Switch from do-while to for loop when reading CRC values (Jani)
> > * Rebase
> > Cc: Rodrigo Vivi 
> > Cc: Paulo Zanoni 
> > Cc: Jani Nikula 
> > Signed-off-by: Jim Bride 
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c | 33 ++---
> >  1 file changed, 30 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c 
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index 2d42d09..c90ca1c 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -3906,6 +3906,11 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 
> > *crc)
> > u8 buf;
> > int count, ret;
> > int attempts = 6;
> > +   u8 old_crc[6];
> > +
> > +   if (crc == NULL) {
> > +   return -ENOMEM;
> > +   }
> 
> wouldn't we drop this check per DK and Jani request?
> I believe we don't need it, but even if there are cases that we need
> we could remove the braces..
> 

Yeah, crc is allocated on the stack. If that is null, we'll have bigger
problems to deal with. And I think it's reasonable to assume the caller
is sending a valid array to fill data.

> >
> > ret = intel_dp_sink_crc_start(intel_dp);
> > if (ret)
> > @@ -3929,11 +3934,33 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 
> > *crc)
> > goto stop;
> > }
> >
> > -   if (drm_dp_dpcd_read(_dp->aux, DP_TEST_CRC_R_CR, crc, 6) < 0) 
> > {
> > -   ret = -EIO;
> > -   goto stop;
> > +   /*
> > +* Sometimes it takes a while for the "real" CRC values to land in
> > +* the DPCD, so try several times until we get two reads in a row
> > +* that are the same.  If we're an eDP panel, delay between reads
> > +* for a while since the values take a bit longer to propagate.
> > +*/
> > +   for (attempts = 0; attempts < 6; attempts++) {
> > +   intel_wait_for_vblank(dev_priv, intel_crtc->pipe);
> 
> DK, we need vblank wait because the crc calculation also may take one vblank.
> usually 2 actually... one to make sure you have the full screen
> updated and one for the calculation.
> In the past when we didn't used the count we were waiting 2 vblanks...
> 


Thanks for the clarification. My reasoning was, after the first two
vblank_waits for the sink to calculate crc, the ones in the retry path
were unnecessary. We just need some delay before reading the dpcd again
without having to enable vblank interrupts. Anyway, the number of
retries is low enough that it shouldn't matter.

On the other hand, since the only consumers of dp sink crc are tests,
why can't the kernel just dump what it reads to debugfs and let the test
deal with erroneous results?

> > +
> > +   if (drm_dp_dpcd_read(_dp->aux, DP_TEST_CRC_R_CR,
> > +crc, 6) < 0) {
> > +   ret = -EIO;
> > +   break;
> > +   }
> > +
> > +   if (attempts && memcmp(old_crc, crc, 6) == 0)
> > +   break;
> > +   memcpy(old_crc, crc, 6);
> 
> little bikeshed: too many hardcoded "6" around... a sizeof would be better...
> but whatever...
> 
> > +
> > +   if (is_edp(intel_dp))
> > +   usleep_range(2, 25000);
> > }
> >
> > +   if (attempts == 6) {
> > +   DRM_DEBUG_KMS("Failed to get CRC after 6 attempts.\n");
> > +   ret = -ETIMEDOUT;
> > +   }
> >  stop:
> > intel_dp_sink_crc_stop(intel_dp);
> > return ret;
> > --
> > 2.7.4
> >
> 

Re: [Intel-gfx] [PATCH i-g-t] tests/kms_frontbuffer_tracking: convert macros to functions

[Paulo Zanoni]

Em Sex, 2017-08-04 às 18:21 +0200, Daniel Vetter escreveu:
> I guess this was done to have a better indication of which testcase
> and function failed, but igt nowadays dumps an entire stacktrace.

But we may have multiple do_assertions() calls in a single function.

>  And
> macros of this magnitude mean the line number is entirely
> meaningless,
> since it doesn't point at a specific check.

False. It always points to a do_assertions() call, which is what
matters.

> 
> Reason I've started to looking into this is that in our full igt CI
> runs we have a serious problem with the fbc testcases randomly
> failing with
> 
> (kms_frontbuffer_tracking:1609) CRITICAL: Test assertion failure
> function enable_prim_screen_and_wait, file
> kms_frontbuffer_tracking.c:1771:

https://cgit.freedesktop.org/xorg/app/intel-gpu-tools/tree/tests/kms_fr
ontbuffer_tracking.c#n1771

See?


> (kms_frontbuffer_tracking:1609) CRITICAL: Failed assertion: false
> (kms_frontbuffer_tracking:1609) CRITICAL: FBC disabled
> 
> And that's not entirely helpful. Also, macros of this magnitude are
> just horrible to read.

NAK. Being a macro instead of a function is extremely helpful and the
line number always points me to the correct do_assertions() call, at
least when I run this locally.

If the line number in the CI system doesn't match what you see in your
file, then it's a problem either on your side or on the CI side. But I
don't think that's your problem. I think your problem is that we print
two different line numbers (1609 and 1771), and you're looking at the
wrong one. I would totally ACK a patch removing the 1609 one... But I
don't think that would require patching kms_frontbuffuer_tracking.c.

If you really really want to change this to a function, can't you try
to find a way to pass a __LINE__ argument that corresponds to the exact
line of the do_assertions() call and print it somewhere? Maybe another
wrapper macro could auto-include __LINE__? But seriously, patch IGT to
not print those bogus numbers, so you won't be confused next time.

> 
> Cc: Paulo Zanoni 
> Signed-off-by: Daniel Vetter 
> ---
>  tests/kms_frontbuffer_tracking.c | 166 -
> --
>  1 file changed, 84 insertions(+), 82 deletions(-)
> 
> diff --git a/tests/kms_frontbuffer_tracking.c
> b/tests/kms_frontbuffer_tracking.c
> index e03524f1c45b..8d11dc065623 100644
> --- a/tests/kms_frontbuffer_tracking.c
> +++ b/tests/kms_frontbuffer_tracking.c
> @@ -1677,88 +1677,90 @@ static int adjust_assertion_flags(const
> struct test_mode *t, int flags)
>   return flags;
>  }
>  
> -#define do_crc_assertions(flags, mandatory_sink_crc) do {
> \
> - int flags__ = (flags);  
>   \
> - struct both_crcs crc_;  
>   \
> - 
> \
> - if (!opt.check_crc || (flags__ & DONT_ASSERT_CRC))  
> \
> - break;  
>   \
> - 
> \
> - collect_crcs(_, mandatory_sink_crc);
> \
> - print_crc("Calculated CRC:", _);
> \
> - 
> \
> - igt_assert(wanted_crc); 
>   \
> - igt_assert_crc_equal(_.pipe, _crc->pipe);
> \
> - if (mandatory_sink_crc) 
>   \
> - assert_sink_crc_equal(_.sink, _crc-
> >sink);   \
> - else if (sink_crc.reliable &&   
>   \
> -  !sink_crc_equal(_.sink, _crc->sink))
> \
> - igt_info("Sink CRC differ, but not required\n");    
> \
> -} while (0)
> -
> -#define do_status_assertions(flags_) do {
> \
> - if (!opt.check_status) {
> \
> - /* Make sure we settle before continuing. */
> \
> - sleep(1);   
> \
> - break;  
>   \
> - }   
> \
> - 
> \
> - if (flags_ & ASSERT_FBC_ENABLED) {  
> \
> - igt_require(!fbc_not_enough_stolen());  
>   \
> - igt_require(!fbc_stride_not_supported());   
> \
> - if (!fbc_wait_until_enabled()) {
> \
> - fbc_print_status(); 
> \
> - igt_assert_f(false, "FBC disabled\n");  
>   \
> - }   
> \
> 

Re: [Intel-gfx] [PATCH 15/15] drm/i915/guc: Trace messages from CT while in debug

[Chris Wilson]

Quoting Michal Wajdeczko (2017-08-04 17:27:12)
> During debug we may want to investigate all communication
> from the Guc. Add proper tracing macros in debug config.
> 
> Signed-off-by: Michal Wajdeczko 
> ---
>  drivers/gpu/drm/i915/intel_guc_ct.c | 19 +++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c 
> b/drivers/gpu/drm/i915/intel_guc_ct.c
> index 9f7fc5e..71daad3 100644
> --- a/drivers/gpu/drm/i915/intel_guc_ct.c
> +++ b/drivers/gpu/drm/i915/intel_guc_ct.c
> @@ -24,6 +24,12 @@
>  #include "i915_drv.h"
>  #include "intel_guc_ct.h"
>  
> +#ifdef CONFIG_DRM_I915_DEBUG
> +#define CT_DEBUG_DRIVER(...)   DRM_DEBUG_DRIVER(__VA_ARGS__)
> +#else
> +#define CT_DEBUG_DRIVER(...)
> +#endif

+1. If we only use a single Kconfig, or enabled them for CI anyway, we
still run the risk of drowning in the noise.  Flooding CI dmesg has many
other unfortunate repercussions as well.

Oh well, back to the ddebug / trace_debug wishlist. (Being able to dump
the pertinent buffer on demand/error without flooding the test runner.)
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH i-g-t] tests/kms: increase max threshold time for edid read

[clinton . a . taylor]

From: Clint Taylor 

Current 50ms max threshold timing for an EDID read is very close to the
actual time for a 2 block HDMI EDID read of 48ms. Any delay like a clock
stretch by the EDID eeprom will cause this test to fail. A 4 block HDMI
EDID read takes approximately 88ms under nominal conditions, making the max
threshold 95ms will allow this test to pass regardless of monitor attached.

Signed-off-by: Clint Taylor 
---
 tests/kms_sysfs_edid_timing.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/kms_sysfs_edid_timing.c b/tests/kms_sysfs_edid_timing.c
index 1201388..b45e080 100644
--- a/tests/kms_sysfs_edid_timing.c
+++ b/tests/kms_sysfs_edid_timing.c
@@ -27,14 +27,14 @@
 #include 
 
 #define THRESHOLD_PER_CONNECTOR10
-#define THRESHOLD_TOTAL50
+#define THRESHOLD_TOTAL95
 #define CHECK_TIMES15
 
 IGT_TEST_DESCRIPTION("This check the time we take to read the content of all "
 "the possible connectors. Without the edid -ENXIO patch "
 
"(http://permalink.gmane.org/gmane.comp.video.dri.devel/62083), "
-"we sometimes take a *really* long time. "
-"So let's just check for some reasonable timing here");
+"we sometimes take a *really* long time. So let's just "
+"check an approximate HDMI 4 block edid read timing here");
 
 
 igt_simple_main
-- 
1.9.1

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

Re: [Intel-gfx] [PATCH 05/15] drm/i915/guc: Move Guc notification handling to separate function

[Chris Wilson]

Quoting Michal Wajdeczko (2017-08-04 17:27:02)
> To allow future code reuse. While here, fix comment style.

Might as well fix the alignment as well then... 

> +   /* Handle flush interrupt in bottom half */
> +   queue_work(dev_priv->guc.log.runtime.flush_wq,
> +   _priv->guc.log.runtime.flush_work);
> +
> +   dev_priv->guc.log.flush_interrupt_count++;
> +   } else {
> +   /*
> +* Not clearing of unhandled event bits won't result in
> +* re-triggering of the interrupt.
> +*/
> +   }
> +}

Argh, and those are spaces not tabs! Back to the checkpatch dungeon you
go, until you are sorry!
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 12/15] drm/i915/guc: Prepare to process incoming requests from CT

[Chris Wilson]

Quoting Michal Wajdeczko (2017-08-04 17:27:09)
>  static inline const char *guc_ct_buffer_type_to_str(u32 type)
> @@ -600,13 +609,76 @@ static int guc_handle_response(struct intel_guc *guc, 
> const u32 *data)
>  static int guc_handle_request(struct intel_guc *guc, const u32 *data)
>  {
> u32 header = data[0];
> +   u32 len = ct_header_get_len(header) + 1; /* total len with header */
> +   struct ct_incoming_request *request;
> +   unsigned long flags;
>  
> GEM_BUG_ON(ct_header_is_response(header));
> /* data layout beyond header is request specific */
>  
> +   request = kmalloc(sizeof(*request), GFP_ATOMIC);
> +   if (unlikely(!request)) {
> +   DRM_ERROR("CT: dropping request %*phn\n", 4*len, data);
> +   return 0; /* XXX: -ENOMEM ? */
> +   }
> +
> +   GEM_BUG_ON(len > GUC_CT_MSG_LEN_MASK + 1);

This is incoming from the guc, if we can validate it, do so. Keep
GEM_BUG_ON() for programming errors and absolute catastrophe.
-Chris

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

Re: [Intel-gfx] [PATCH i-g-t] lib: don't hang on blt on snb

[Chris Wilson]

Quoting Daniel Vetter (2017-08-04 17:07:22)
> We now have full (or a lot at least) igt running in beta CI, and snb
> blt hangs are really unhappy:
> 
> - drv_hangman@error-state-capture-blt and gem_exec_capture@capture-blt
>   reliably result in insta-machine death when we try to reset the gpu,
>   both on the CI snb and the one I have here.
> 
> - Other testcases also randomly (and sometimes rather rarely) die on
>   snb.
> 
> We can't use the endless batch because that results in a reset failure
> and wedged gpu, so also not really better.

It shouldn't be the recursion, but the invalid instruction we use to try
and trigger the hang quicker (otherwise hangcheck may see the advancing
ACTHD and give us longer to escape the loop).

In gem_exec_capture we shouldn't even need that invalid instruction, we
just need the busy batch as we pull the trigger ourselves, and if that
fails to reset a simple recursive batch we have some issues to resolve.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH igt] tests/kms_ccs: Don't overallocate CCS surface

[Jason Ekstrand]

Reviewed-by: Jason Ekstrand 

On Fri, Aug 4, 2017 at 9:37 AM, Daniel Stone  wrote:

> Due to a mix-up in kernel branches being used, I'd mangled Jason's
> original CCS test to hopelessly overallocate the CCS surface size.
> Restore it back to its original.
>
> Signed-off-by: Daniel Stone 
> Cc: Jason Ekstrand 
> ---
>  tests/kms_ccs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tests/kms_ccs.c b/tests/kms_ccs.c
> index 0524a43e..a40d6c10 100644
> --- a/tests/kms_ccs.c
> +++ b/tests/kms_ccs.c
> @@ -188,7 +188,7 @@ static void display_fb(data_t *data, int compressed)
> f.pitches[1] = ALIGN(width * 1, 128);
> f.modifier[1] = modifier;
> f.offsets[1] = size[0];
> -   size[1] = f.pitches[1] * ALIGN(f.height, 32);
> +   size[1] = f.pitches[1] * ALIGN(height, 32);
>
> f.handles[0] = gem_create(data->drm_fd, size[0] + size[1]);
> f.handles[1] = f.handles[0];
> --
> 2.13.4
>
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✗ Fi.CI.BAT: warning for drm/i915/guc: Support for Guc responses and requests

[Patchwork]

== Series Details ==

Series: drm/i915/guc: Support for Guc responses and requests
URL   : https://patchwork.freedesktop.org/series/28393/
State : warning

== Summary ==

Series 28393v1 drm/i915/guc: Support for Guc responses and requests
https://patchwork.freedesktop.org/api/1.0/series/28393/revisions/1/mbox/

Test kms_pipe_crc_basic:
Subgroup hang-read-crc-pipe-a:
dmesg-warn -> PASS   (fi-pnv-d510) fdo#101597
Test prime_vgem:
Subgroup basic-fence-flip:
pass   -> DMESG-WARN (fi-byt-j1900)

fdo#101597 https://bugs.freedesktop.org/show_bug.cgi?id=101597

fi-bdw-5557u total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  
time:437s
fi-bdw-gvtdvmtotal:279  pass:265  dwarn:0   dfail:0   fail:0   skip:14  
time:418s
fi-blb-e6850 total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  
time:361s
fi-bsw-n3050 total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  
time:501s
fi-bxt-j4205 total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  
time:491s
fi-byt-j1900 total:279  pass:253  dwarn:2   dfail:0   fail:0   skip:24  
time:529s
fi-byt-n2820 total:279  pass:250  dwarn:1   dfail:0   fail:0   skip:28  
time:514s
fi-glk-2atotal:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  
time:591s
fi-hsw-4770  total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  
time:431s
fi-hsw-4770r total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  
time:409s
fi-ilk-650   total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  
time:418s
fi-ivb-3520m total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:511s
fi-ivb-3770  total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:479s
fi-kbl-7500u total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:464s
fi-kbl-7560u total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:566s
fi-kbl-r total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:577s
fi-pnv-d510  total:279  pass:223  dwarn:1   dfail:0   fail:0   skip:55  
time:576s
fi-skl-6260u total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:446s
fi-skl-6700k total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:643s
fi-skl-6770hqtotal:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:462s
fi-skl-gvtdvmtotal:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  
time:425s
fi-skl-x1585ltotal:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:482s
fi-snb-2520m total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  
time:543s
fi-snb-2600  total:279  pass:250  dwarn:0   dfail:0   fail:0   skip:29  
time:407s

2a33b48692c55d9145a60527716b7d066815d377 drm-tip: 2017y-08m-04d-13h-34m-53s UTC 
integration manifest
dd8c5fe51790 drm/i915/guc: Trace messages from CT while in debug
a487efe9d170 drm/i915/guc: Enable GuC interrupts when using CT
7f3acdf7b8c5 drm/i915/guc: Handle default action received over CT
e310d9d5208d drm/i915/guc: Prepare to process incoming requests from CT
0fa1bd92541e drm/i915/guc: Implement response handling in send_ct()
b6e8db3b5d2f drm/i915/guc: Use better name for helper wait function
8b64a6bbed37 drm/i915/guc: Prepare to handle messages from CT RECV buffer
ff55ebe894d4 drm/i915/guc: Update CT message header definition
a4a815e059ef drm/i915/guc: Create a GuC receive function
936b57e8eba1 drm/i915/guc: Move flushing the GuC logs outside notification 
handler
e95ea974c745 drm/i915/guc: Move Guc notification handling to separate function
70a495bf2b4a drm/i915/guc: Implement response handling in send_mmio()
53b95a141d1f drm/i915/guc: Add send_and_receive() helper function
0dc31d444f83 drm/i915/guc: Prepare send() function to accept bigger response
e40541ac904e drm/i915/guc: Add support for data reporting in GuC responses

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5329/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH igt] tests/kms_ccs: Don't overallocate CCS surface

[Daniel Stone]

Due to a mix-up in kernel branches being used, I'd mangled Jason's
original CCS test to hopelessly overallocate the CCS surface size.
Restore it back to its original.

Signed-off-by: Daniel Stone 
Cc: Jason Ekstrand 
---
 tests/kms_ccs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/kms_ccs.c b/tests/kms_ccs.c
index 0524a43e..a40d6c10 100644
--- a/tests/kms_ccs.c
+++ b/tests/kms_ccs.c
@@ -188,7 +188,7 @@ static void display_fb(data_t *data, int compressed)
f.pitches[1] = ALIGN(width * 1, 128);
f.modifier[1] = modifier;
f.offsets[1] = size[0];
-   size[1] = f.pitches[1] * ALIGN(f.height, 32);
+   size[1] = f.pitches[1] * ALIGN(height, 32);
 
f.handles[0] = gem_create(data->drm_fd, size[0] + size[1]);
f.handles[1] = f.handles[0];
-- 
2.13.4

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

Re: [Intel-gfx] [PATCH i-g-t] tests/perf: follow up build fix

[Matthew Auld]

On 4 August 2017 at 17:23, Lionel Landwerlin
 wrote:
> Signed-off-by: Lionel Landwerlin 
> Fixes: adcde8ac ("tests/perf: fix build where system headers don't have Gen8 
> formats")
Tested-by: Matthew Auld 

> ---
>  tests/perf.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tests/perf.c b/tests/perf.c
> index 9cfc4bb9..a7fa33a1 100644
> --- a/tests/perf.c
> +++ b/tests/perf.c
> @@ -146,7 +146,7 @@ enum drm_i915_perf_record_type {
>  /* There is no ifdef we can use for those formats :( */
>  enum {
> local_I915_OA_FORMAT_A12 = I915_OA_FORMAT_C4_B8 + 1,
> -   local_I915_OA_FORMAT_A12_B8_C8 = I915_OA_FORMAT_A12 + 2,
> +   local_I915_OA_FORMAT_A12_B8_C8 = I915_OA_FORMAT_C4_B8 + 2,
> local_I915_OA_FORMAT_A32u40_A4u32_B8_C8 = I915_OA_FORMAT_C4_B8 + 3,
>  };
>
> --
> 2.13.3
>
> ___
> 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] [PATCH 02/15] drm/i915/guc: Prepare send() function to accept bigger response

[Michal Wajdeczko]

This is a preparation step for the upcoming patches.
We already can return some small data decoded from the command
status, but we will need more in the future.

Signed-off-by: Michal Wajdeczko 
Cc: Oscar Mateo 
Cc: Michel Thierry 
Cc: Daniele Ceraolo Spurio 
---
 drivers/gpu/drm/i915/intel_guc_ct.c | 7 ---
 drivers/gpu/drm/i915/intel_uc.c | 6 --
 drivers/gpu/drm/i915/intel_uc.h | 8 
 3 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c 
b/drivers/gpu/drm/i915/intel_guc_ct.c
index 1249868..c17cb42 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/intel_guc_ct.c
@@ -79,7 +79,7 @@ static int guc_action_register_ct_buffer(struct intel_guc 
*guc,
int err;
 
/* Can't use generic send(), CT registration must go over MMIO */
-   err = intel_guc_send_mmio(guc, action, ARRAY_SIZE(action));
+   err = intel_guc_send_mmio(guc, action, ARRAY_SIZE(action), NULL);
if (err)
DRM_ERROR("CT: register %s buffer failed; err=%d\n",
  guc_ct_buffer_type_to_str(type), err);
@@ -98,7 +98,7 @@ static int guc_action_deregister_ct_buffer(struct intel_guc 
*guc,
int err;
 
/* Can't use generic send(), CT deregistration must go over MMIO */
-   err = intel_guc_send_mmio(guc, action, ARRAY_SIZE(action));
+   err = intel_guc_send_mmio(guc, action, ARRAY_SIZE(action), NULL);
if (err)
DRM_ERROR("CT: deregister %s buffer failed; owner=%d err=%d\n",
  guc_ct_buffer_type_to_str(type), owner, err);
@@ -395,7 +395,8 @@ static int ctch_send(struct intel_guc *guc,
 /*
  * Command Transport (CT) buffer based GuC send function.
  */
-static int intel_guc_send_ct(struct intel_guc *guc, const u32 *action, u32 len)
+static int intel_guc_send_ct(struct intel_guc *guc, const u32 *action, u32 len,
+u32 *response)
 {
struct intel_guc_ct_channel *ctch = >ct.host_channel;
u32 status = ~0; /* undefined */
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index ff25477..77da750 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -459,7 +459,8 @@ void intel_uc_fini_hw(struct drm_i915_private *dev_priv)
i915_ggtt_disable_guc(dev_priv);
 }
 
-int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len)
+int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len,
+  u32 *response)
 {
WARN(1, "Unexpected send: action=%#x\n", *action);
return -ENODEV;
@@ -468,7 +469,8 @@ int intel_guc_send_nop(struct intel_guc *guc, const u32 
*action, u32 len)
 /*
  * This function implements the MMIO based host to GuC interface.
  */
-int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len)
+int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len,
+   u32 *response)
 {
struct drm_i915_private *dev_priv = guc_to_i915(guc);
u32 status;
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 22ae52b..53ea5f1 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -206,7 +206,7 @@ struct intel_guc {
struct mutex send_mutex;
 
/* GuC's FW specific send function */
-   int (*send)(struct intel_guc *guc, const u32 *data, u32 len);
+   int (*send)(struct intel_guc *guc, const u32 *data, u32 len, u32 *resp);
 
/* GuC's FW specific notify function */
void (*notify)(struct intel_guc *guc);
@@ -227,12 +227,12 @@ void intel_uc_fini_fw(struct drm_i915_private *dev_priv);
 int intel_uc_init_hw(struct drm_i915_private *dev_priv);
 void intel_uc_fini_hw(struct drm_i915_private *dev_priv);
 int intel_guc_sample_forcewake(struct intel_guc *guc);
-int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len);
-int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len);
+int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len, u32 
*response);
+int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len, u32 
*response);
 
 static inline int intel_guc_send(struct intel_guc *guc, const u32 *action, u32 
len)
 {
-   return guc->send(guc, action, len);
+   return guc->send(guc, action, len, NULL);
 }
 
 static inline void intel_guc_notify(struct intel_guc *guc)
-- 
2.7.4

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

[Intel-gfx] [PATCH 15/15] drm/i915/guc: Trace messages from CT while in debug

[Michal Wajdeczko]

During debug we may want to investigate all communication
from the Guc. Add proper tracing macros in debug config.

Signed-off-by: Michal Wajdeczko 
---
 drivers/gpu/drm/i915/intel_guc_ct.c | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c 
b/drivers/gpu/drm/i915/intel_guc_ct.c
index 9f7fc5e..71daad3 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/intel_guc_ct.c
@@ -24,6 +24,12 @@
 #include "i915_drv.h"
 #include "intel_guc_ct.h"
 
+#ifdef CONFIG_DRM_I915_DEBUG
+#define CT_DEBUG_DRIVER(...)   DRM_DEBUG_DRIVER(__VA_ARGS__)
+#else
+#define CT_DEBUG_DRIVER(...)
+#endif
+
 struct ct_request {
struct list_head link;
u32 fence;
@@ -325,6 +331,10 @@ static int ctb_write(struct intel_guc_ct_buffer *ctb,
 (send_response ? GUC_CT_MSG_SEND_STATUS : 0) |
 (action[0] << GUC_CT_MSG_ACTION_SHIFT);
 
+   CT_DEBUG_DRIVER("CT: writing %*phn %*phn %*phn\n",
+   4, , 4, ,
+   4*(len - 1), [1]);
+
cmds[tail] = header;
tail = (tail + 1) % size;
 
@@ -496,6 +506,9 @@ static int intel_guc_send_ct(struct intel_guc *guc, const 
u32 *action, u32 len,
if (unlikely(ret < 0)) {
DRM_ERROR("CT: send action %#X failed; err=%d status=%#X\n",
  action[0], ret, status);
+   } else if (unlikely(ret)) {
+   CT_DEBUG_DRIVER("CT: send action %#x returned %d (%#x)\n",
+   action[0], ret, ret);
}
 
mutex_unlock(>send_mutex);
@@ -542,10 +555,12 @@ static int ctb_read(struct intel_guc_ct_buffer *ctb, u32 
*data)
/* beware of buffer wrap case */
if (available < 0)
available += size;
+   CT_DEBUG_DRIVER("CT: available %d (%u:%u)\n", available, head, tail);
GEM_BUG_ON(available < 0);
 
data[0] = cmds[head];
head = (head + 1) % size;
+   CT_DEBUG_DRIVER("CT: header %#x\n", data[0]);
 
/* message len with header */
len = ct_header_get_len(data[0]) + 1;
@@ -563,6 +578,7 @@ static int ctb_read(struct intel_guc_ct_buffer *ctb, u32 
*data)
data[i] = cmds[head];
head = (head + 1) % size;
}
+   CT_DEBUG_DRIVER("CT: received %*phn\n", 4*len, data);
 
desc->head = head * 4;
return 0;
@@ -584,6 +600,7 @@ static int guc_handle_response(struct intel_guc *guc, const 
u32 *data)
DRM_ERROR("CT: corrupted response %*phn\n", 4*len, data);
return -EPROTO;
}
+   CT_DEBUG_DRIVER("CT: response fence %u status %#x\n", fence, status);
 
spin_lock_irqsave(>ct.lock, flags);
list_for_each_entry(req, >ct.pending_requests, link) {
@@ -615,6 +632,7 @@ static int guc_handle_request(struct intel_guc *guc, const 
u32 *data)
 
GEM_BUG_ON(ct_header_is_response(header));
/* data layout beyond header is request specific */
+   CT_DEBUG_DRIVER("CT: request %#x\n", ct_header_get_action(header));
 
request = kmalloc(sizeof(*request), GFP_ATOMIC);
if (unlikely(!request)) {
@@ -656,6 +674,7 @@ static bool guc_process_incoming_requests(struct intel_guc 
*guc)
header = request->data[0];
action = ct_header_get_action(header);
len = ct_header_get_len(header) + 1; /* also count header dw */
+   CT_DEBUG_DRIVER("CT: processing request %*phn\n", 4*len, request->data);
 
switch (action) {
case INTEL_GUC_ACTION_DEFAULT:
-- 
2.7.4

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

[Intel-gfx] [PATCH 12/15] drm/i915/guc: Prepare to process incoming requests from CT

[Michal Wajdeczko]

Requests are read from CT in the irq handler, but actual processing
will be done in the work thread. Processing of specific actions will
be added in the upcoming patches.

Signed-off-by: Michal Wajdeczko 
Cc: Oscar Mateo 
Cc: Michel Thierry 
Cc: Daniele Ceraolo Spurio 
---
 drivers/gpu/drm/i915/intel_guc_ct.c | 72 +
 drivers/gpu/drm/i915/intel_guc_ct.h |  4 +++
 2 files changed, 76 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c 
b/drivers/gpu/drm/i915/intel_guc_ct.c
index 4dfa0b9..75cd7af 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/intel_guc_ct.c
@@ -32,10 +32,17 @@ struct ct_request {
u32 *response_buf;
 };
 
+struct ct_incoming_request {
+   struct list_head link;
+   u32 data[GUC_CT_MSG_LEN_MASK+1];
+};
+
 enum { CTB_SEND = 0, CTB_RECV = 1 };
 
 enum { CTB_OWNER_HOST = 0 };
 
+static void ct_worker_func(struct work_struct *w);
+
 void intel_guc_ct_init_early(struct intel_guc_ct *ct)
 {
/* we're using static channel owners */
@@ -43,6 +50,8 @@ void intel_guc_ct_init_early(struct intel_guc_ct *ct)
 
spin_lock_init(>lock);
INIT_LIST_HEAD(>pending_requests);
+   INIT_LIST_HEAD(>incoming_requests);
+   INIT_WORK(>worker, ct_worker_func);
 }
 
 static inline const char *guc_ct_buffer_type_to_str(u32 type)
@@ -600,13 +609,76 @@ static int guc_handle_response(struct intel_guc *guc, 
const u32 *data)
 static int guc_handle_request(struct intel_guc *guc, const u32 *data)
 {
u32 header = data[0];
+   u32 len = ct_header_get_len(header) + 1; /* total len with header */
+   struct ct_incoming_request *request;
+   unsigned long flags;
 
GEM_BUG_ON(ct_header_is_response(header));
/* data layout beyond header is request specific */
 
+   request = kmalloc(sizeof(*request), GFP_ATOMIC);
+   if (unlikely(!request)) {
+   DRM_ERROR("CT: dropping request %*phn\n", 4*len, data);
+   return 0; /* XXX: -ENOMEM ? */
+   }
+
+   GEM_BUG_ON(len > GUC_CT_MSG_LEN_MASK + 1);
+   memcpy(request->data, data, 4*len);
+
+   spin_lock_irqsave(>ct.lock, flags);
+   list_add_tail(>link, >ct.incoming_requests);
+   spin_unlock_irqrestore(>ct.lock, flags);
+
+   queue_work(system_unbound_wq, >ct.worker);
return 0;
 }
 
+static bool guc_process_incoming_requests(struct intel_guc *guc)
+{
+   unsigned long flags;
+   struct ct_incoming_request *request;
+   bool done;
+   u32 header;
+   u32 action;
+   u32 len;
+
+   spin_lock_irqsave(>ct.lock, flags);
+   request = list_first_entry_or_null(>ct.incoming_requests,
+  struct ct_incoming_request, link);
+   if (request)
+   list_del(>link);
+   done = !!list_empty(>ct.incoming_requests);
+   spin_unlock_irqrestore(>ct.lock, flags);
+
+   if (!request)
+   return true;
+
+   header = request->data[0];
+   action = ct_header_get_action(header);
+   len = ct_header_get_len(header) + 1; /* also count header dw */
+
+   switch (action) {
+   default:
+   DRM_ERROR("CT: unexpected request %*phn\n",
+ 4*len, request->data);
+   break;
+   }
+
+   kfree(request);
+   return done;
+}
+
+static void ct_worker_func(struct work_struct *w)
+{
+   struct intel_guc_ct *ct = container_of(w, struct intel_guc_ct, worker);
+   struct intel_guc *guc = container_of(ct, struct intel_guc, ct);
+   bool done;
+
+   done = guc_process_incoming_requests(guc);
+   if (!done)
+   queue_work(system_unbound_wq, >worker);
+}
+
 static void intel_guc_receive_ct(struct intel_guc *guc)
 {
struct intel_guc_ct_channel *ctch = >ct.host_channel;
diff --git a/drivers/gpu/drm/i915/intel_guc_ct.h 
b/drivers/gpu/drm/i915/intel_guc_ct.h
index 557c1e8..125e004 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.h
+++ b/drivers/gpu/drm/i915/intel_guc_ct.h
@@ -73,6 +73,8 @@ struct intel_guc_ct_channel {
  * @host_channel: main channel used by the host
  * @lock: spin lock for pending requests list
  * @pending_requests: list of pending requests
+ * @incoming_requests: list of incoming requests
+ * @tasklet: tasklet for handling incoming requests
  */
 struct intel_guc_ct {
struct intel_guc_ct_channel host_channel;
@@ -80,6 +82,8 @@ struct intel_guc_ct {
 
spinlock_t lock;
struct list_head pending_requests;
+   struct list_head incoming_requests;
+   struct work_struct worker;
 };
 
 void intel_guc_ct_init_early(struct intel_guc_ct *ct);
-- 
2.7.4

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

[Intel-gfx] [PATCH 13/15] drm/i915/guc: Handle default action received over CT

[Michal Wajdeczko]

With enabled CT, instead of programming SCRATCH 15 register with the
Guc to host message, Guc will send us CT request. Content of the data[1]
of this message follows format of the data in scratch register.

Signed-off-by: Michal Wajdeczko 
Cc: Oscar Mateo 
---
 drivers/gpu/drm/i915/intel_guc_ct.c | 3 +++
 drivers/gpu/drm/i915/intel_uc.c | 7 +++
 drivers/gpu/drm/i915/intel_uc.h | 1 +
 3 files changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c 
b/drivers/gpu/drm/i915/intel_guc_ct.c
index 75cd7af..9f7fc5e 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/intel_guc_ct.c
@@ -658,6 +658,9 @@ static bool guc_process_incoming_requests(struct intel_guc 
*guc)
len = ct_header_get_len(header) + 1; /* also count header dw */
 
switch (action) {
+   case INTEL_GUC_ACTION_DEFAULT:
+   intel_guc_process_default_action(guc, request->data[1]);
+   break;
default:
DRM_ERROR("CT: unexpected request %*phn\n",
  4*len, request->data);
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 258e0d0..27758ce 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -596,3 +596,10 @@ int intel_guc_sample_forcewake(struct intel_guc *guc)
 
return intel_guc_send(guc, action, ARRAY_SIZE(action));
 }
+
+void intel_guc_process_default_action(struct intel_guc *guc, u32 msg)
+{
+   if (msg & (INTEL_GUC_RECV_MSG_CRASH_DUMP_POSTED |
+  INTEL_GUC_RECV_MSG_FLUSH_LOG_BUFFER))
+   intel_guc_log_flush(guc);
+}
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 6f20e66..2a8394b 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -230,6 +230,7 @@ void intel_uc_fini_fw(struct drm_i915_private *dev_priv);
 int intel_uc_init_hw(struct drm_i915_private *dev_priv);
 void intel_uc_fini_hw(struct drm_i915_private *dev_priv);
 void intel_guc_notification_handler(struct intel_guc *guc);
+void intel_guc_process_default_action(struct intel_guc *guc, u32 msg);
 int intel_guc_sample_forcewake(struct intel_guc *guc);
 int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len, u32 
*response);
 int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len, u32 
*response);
-- 
2.7.4

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

[Intel-gfx] [PATCH 14/15] drm/i915/guc: Enable GuC interrupts when using CT

[Michal Wajdeczko]

We will need them in G2H communication to properly handle
responses and requests from the Guc.

Signed-off-by: Michal Wajdeczko 
Cc: Oscar Mateo 
Cc: Daniele Ceraolo Spurio 
Cc: Michel Thierry 
---
 drivers/gpu/drm/i915/i915_guc_submission.c | 2 +-
 drivers/gpu/drm/i915/intel_uc.c| 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c 
b/drivers/gpu/drm/i915/i915_guc_submission.c
index 48a1e93..509497e 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -1328,7 +1328,7 @@ int intel_guc_resume(struct drm_i915_private *dev_priv)
if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
return 0;
 
-   if (i915.guc_log_level >= 0)
+   if (HAS_GUC_CT(dev_priv) || i915.guc_log_level >= 0)
gen9_enable_guc_interrupts(dev_priv);
 
ctx = dev_priv->kernel_context;
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 27758ce..0402e32 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -395,7 +395,7 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
 
intel_guc_auth_huc(dev_priv);
if (i915.enable_guc_submission) {
-   if (i915.guc_log_level >= 0)
+   if (HAS_GUC_CT(dev_priv) || i915.guc_log_level >= 0)
gen9_enable_guc_interrupts(dev_priv);
 
ret = i915_guc_submission_enable(dev_priv);
-- 
2.7.4

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

[Intel-gfx] [PATCH 06/15] drm/i915/guc: Move flushing the GuC logs outside notification handler

[Michal Wajdeczko]

From: Oscar Mateo 

To allow future code reuse.

Cc: Sagar Arun Kamble 
Cc: Daniele Ceraolo Spurio 
Signed-off-by: Oscar Mateo 
Signed-off-by: Michal Wajdeczko 
---
 drivers/gpu/drm/i915/intel_guc_log.c | 8 
 drivers/gpu/drm/i915/intel_uc.c  | 6 +-
 drivers/gpu/drm/i915/intel_uc.h  | 1 +
 3 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_log.c 
b/drivers/gpu/drm/i915/intel_guc_log.c
index 16d3b87..acd9a3f 100644
--- a/drivers/gpu/drm/i915/intel_guc_log.c
+++ b/drivers/gpu/drm/i915/intel_guc_log.c
@@ -520,6 +520,14 @@ static void guc_flush_logs(struct intel_guc *guc)
guc_log_capture_logs(guc);
 }
 
+void intel_guc_log_flush(struct intel_guc *guc)
+{
+   /* Handle flush interrupt in bottom half */
+   queue_work(guc->log.runtime.flush_wq, >log.runtime.flush_work);
+
+   guc->log.flush_interrupt_count++;
+}
+
 int intel_guc_log_create(struct intel_guc *guc)
 {
struct i915_vma *vma;
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 1e6390e..a091e83 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -556,11 +556,7 @@ void intel_guc_notification_handler(struct intel_guc *guc)
/* Clear the message bits that are handled */
I915_WRITE(SOFT_SCRATCH(15), msg & ~flush);
 
-   /* Handle flush interrupt in bottom half */
-   queue_work(dev_priv->guc.log.runtime.flush_wq,
-   _priv->guc.log.runtime.flush_work);
-
-   dev_priv->guc.log.flush_interrupt_count++;
+   intel_guc_log_flush(_priv->guc);
} else {
/*
 * Not clearing of unhandled event bits won't result in
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 2789179..4808f47 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -267,6 +267,7 @@ struct i915_vma *intel_guc_allocate_vma(struct intel_guc 
*guc, u32 size);
 /* intel_guc_log.c */
 int intel_guc_log_create(struct intel_guc *guc);
 void intel_guc_log_destroy(struct intel_guc *guc);
+void intel_guc_log_flush(struct intel_guc *guc);
 int i915_guc_log_control(struct drm_i915_private *dev_priv, u64 control_val);
 void i915_guc_log_register(struct drm_i915_private *dev_priv);
 void i915_guc_log_unregister(struct drm_i915_private *dev_priv);
-- 
2.7.4

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

[Intel-gfx] [PATCH 11/15] drm/i915/guc: Implement response handling in send_ct()

[Michal Wajdeczko]

GuC may return not only small data encoded in the status dword,
but can also append additional data into the response message.
We will copy this extra data into provided buffer, and use
number of received data dwords as new success return value.

Signed-off-by: Michal Wajdeczko 
Cc: Oscar Mateo 
Cc: Michel Thierry 
Cc: Daniele Ceraolo Spurio 
---
 drivers/gpu/drm/i915/intel_guc_ct.c | 120 +---
 drivers/gpu/drm/i915/intel_guc_ct.h |   5 ++
 2 files changed, 115 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c 
b/drivers/gpu/drm/i915/intel_guc_ct.c
index 4fabea17..4dfa0b9 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/intel_guc_ct.c
@@ -24,6 +24,14 @@
 #include "i915_drv.h"
 #include "intel_guc_ct.h"
 
+struct ct_request {
+   struct list_head link;
+   u32 fence;
+   u32 status;
+   u32 response_len;
+   u32 *response_buf;
+};
+
 enum { CTB_SEND = 0, CTB_RECV = 1 };
 
 enum { CTB_OWNER_HOST = 0 };
@@ -32,6 +40,9 @@ void intel_guc_ct_init_early(struct intel_guc_ct *ct)
 {
/* we're using static channel owners */
ct->host_channel.owner = CTB_OWNER_HOST;
+
+   spin_lock_init(>lock);
+   INIT_LIST_HEAD(>pending_requests);
 }
 
 static inline const char *guc_ct_buffer_type_to_str(u32 type)
@@ -265,7 +276,8 @@ static u32 ctch_get_next_fence(struct intel_guc_ct_channel 
*ctch)
 static int ctb_write(struct intel_guc_ct_buffer *ctb,
 const u32 *action,
 u32 len /* in dwords */,
-u32 fence)
+u32 fence,
+bool send_response)
 {
struct guc_ct_buffer_desc *desc = ctb->desc;
u32 head = desc->head / 4;  /* in dwords */
@@ -301,6 +313,7 @@ static int ctb_write(struct intel_guc_ct_buffer *ctb,
 */
header = (len << GUC_CT_MSG_LEN_SHIFT) |
 (GUC_CT_MSG_WRITE_FENCE_TO_DESC) |
+(send_response ? GUC_CT_MSG_SEND_STATUS : 0) |
 (action[0] << GUC_CT_MSG_ACTION_SHIFT);
 
cmds[tail] = header;
@@ -364,14 +377,48 @@ static int wait_for_desc_update(struct guc_ct_buffer_desc 
*desc,
return err;
 }
 
+/* Wait for the Guc response.
+ * We will update request status from the response message handler.
+ *
+ * @req:   pointer to pending request
+ * @status:placeholder for status
+ * return: 0 response received (status is valid)
+ * -ETIMEDOUT no response within hardcoded timeout
+ * \see guc_handle_response()
+ */
+static int wait_for_response_msg(struct ct_request *req, u32 *status)
+{
+   int err;
+
+   /*
+* Fast commands should complete in less than 10us, so sample quickly
+* up to that length of time, then switch to a slower sleep-wait loop.
+* No GuC command should ever take longer than 10ms.
+*/
+#define done INTEL_GUC_RECV_IS_RESPONSE(READ_ONCE(req->status))
+   err = wait_for_us(done, 10);
+   if (err)
+   err = wait_for(done, 1000);
+#undef done
+
+   if (unlikely(err))
+   DRM_ERROR("CT: fence %u err %d\n", req->fence, err);
+
+   *status = req->status;
+   return err;
+}
+
 static int ctch_send(struct intel_guc *guc,
 struct intel_guc_ct_channel *ctch,
 const u32 *action,
 u32 len,
-u32 *status)
+u32 *status,
+u32 *response)
 {
struct intel_guc_ct_buffer *ctb = >ctbs[CTB_SEND];
struct guc_ct_buffer_desc *desc = ctb->desc;
+   struct ct_request request;
+   unsigned long flags;
u32 fence;
int err;
 
@@ -380,18 +427,48 @@ static int ctch_send(struct intel_guc *guc,
GEM_BUG_ON(len & ~GUC_CT_MSG_LEN_MASK);
 
fence = ctch_get_next_fence(ctch);
-   err = ctb_write(ctb, action, len, fence);
+   request.fence = fence;
+   request.status = 0;
+   request.response_len = 0;
+   request.response_buf = response;
+
+   spin_lock_irqsave(>ct.lock, flags);
+   list_add_tail(, >ct.pending_requests);
+   spin_unlock_irqrestore(>ct.lock, flags);
+
+   err = ctb_write(ctb, action, len, fence, !!response);
if (unlikely(err))
-   return err;
+   goto unlink;
 
intel_guc_notify(guc);
 
-   err = wait_for_desc_update(desc, fence, status);
+   if (response)
+   err = wait_for_response_msg(, status);
+   else
+   err = wait_for_desc_update(desc, fence, status);
if (unlikely(err))
-   return err;
-   if (INTEL_GUC_RECV_TO_STATUS(*status) != INTEL_GUC_STATUS_SUCCESS)
-   return -EIO;
-   return INTEL_GUC_RECV_TO_DATA(*status);
+   goto unlink;
+
+   if 

[Intel-gfx] [PATCH 09/15] drm/i915/guc: Prepare to handle messages from CT RECV buffer

[Michal Wajdeczko]

GuC can respond to our commands not only by updating SEND buffer
descriptor, but can send us message over RECV buffer. Additionally
Guc can also send us unsolicited requests over RECV buffer.
Lets start reading those messages and make placeholders for actual
response/request handlers.

Signed-off-by: Michal Wajdeczko 
Cc: Oscar Mateo 
Cc: Michel Thierry 
Cc: Daniele Ceraolo Spurio 
---
 drivers/gpu/drm/i915/intel_guc_ct.c | 120 
 1 file changed, 120 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c 
b/drivers/gpu/drm/i915/intel_guc_ct.c
index c17cb42..dd30c83 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/intel_guc_ct.c
@@ -414,6 +414,124 @@ static int intel_guc_send_ct(struct intel_guc *guc, const 
u32 *action, u32 len,
return ret;
 }
 
+static inline unsigned int ct_header_get_len(u32 header)
+{
+   return (header >> GUC_CT_MSG_LEN_SHIFT) & GUC_CT_MSG_LEN_MASK;
+}
+
+static inline unsigned int ct_header_get_action(u32 header)
+{
+   return (header >> GUC_CT_MSG_ACTION_SHIFT) & GUC_CT_MSG_ACTION_MASK;
+}
+
+static inline bool ct_header_is_response(u32 header)
+{
+   return !!(header & GUC_CT_MSG_IS_RESPONSE);
+}
+
+static int ctb_read(struct intel_guc_ct_buffer *ctb, u32 *data)
+{
+   struct guc_ct_buffer_desc *desc = ctb->desc;
+   u32 head = desc->head / 4;  /* in dwords */
+   u32 tail = desc->tail / 4;  /* in dwords */
+   u32 size = desc->size / 4;  /* in dwords */
+   u32 *cmds = ctb->cmds;
+   s32 available;  /* in dwords */
+   unsigned int len;
+   unsigned int i;
+
+   GEM_BUG_ON(desc->size % 4);
+   GEM_BUG_ON(desc->head % 4);
+   GEM_BUG_ON(desc->tail % 4);
+   GEM_BUG_ON(tail >= size);
+   GEM_BUG_ON(head >= size);
+
+   /* tail == head condition indicates empty */
+   available = tail - head;
+   if (available == 0)
+   return -ENODATA;
+
+   /* beware of buffer wrap case */
+   if (available < 0)
+   available += size;
+   GEM_BUG_ON(available < 0);
+
+   data[0] = cmds[head];
+   head = (head + 1) % size;
+
+   /* message len with header */
+   len = ct_header_get_len(data[0]) + 1;
+   if (unlikely(len > (u32)available)) {
+   DRM_ERROR("CT: incomplete message %*phn %*phn %*phn\n",
+ 4, data,
+ 4 * (head + available - 1 > size ?
+  size - head : available - 1), [head],
+ 4 * (head + available - 1 > size ?
+  available - 1 - size + head : 0), [0]);
+   return -EPROTO;
+   }
+
+   for (i = 1; i < len; i++) {
+   data[i] = cmds[head];
+   head = (head + 1) % size;
+   }
+
+   desc->head = head * 4;
+   return 0;
+}
+
+static int guc_handle_response(struct intel_guc *guc, const u32 *data)
+{
+   u32 header = data[0];
+   u32 len = ct_header_get_len(header) + 1; /* total len with header */
+
+   GEM_BUG_ON(!ct_header_is_response(header));
+   /* beyond header, data shall at least include fence and status */
+   if (unlikely(len < 3)) {
+   DRM_ERROR("CT: corrupted response %*phn\n", 4*len, data);
+   return -EPROTO;
+   }
+
+   return 0;
+}
+
+static int guc_handle_request(struct intel_guc *guc, const u32 *data)
+{
+   u32 header = data[0];
+
+   GEM_BUG_ON(ct_header_is_response(header));
+   /* data layout beyond header is request specific */
+
+   return 0;
+}
+
+static void intel_guc_receive_ct(struct intel_guc *guc)
+{
+   struct intel_guc_ct_channel *ctch = >ct.host_channel;
+   struct intel_guc_ct_buffer *ctb = >ctbs[CTB_RECV];
+   u32 msg[GUC_CT_MSG_LEN_MASK+1];
+   int err = 0;
+
+   if (!ctch_is_open(ctch))
+   return;
+
+   do {
+   err = ctb_read(ctb, msg);
+   if (err)
+   break;
+
+   if (ct_header_is_response(msg[0]))
+   err = guc_handle_response(guc, msg);
+   else
+   err = guc_handle_request(guc, msg);
+   } while (!err);
+
+   if (GEM_WARN_ON(err == -EPROTO)) {
+   DRM_ERROR("CT: corrupted message detected!\n");
+   ctb->desc->is_in_error = 1;
+   }
+}
+
 /**
  * Enable buffer based command transport
  * Shall only be called for platforms with HAS_GUC_CT.
@@ -435,6 +553,7 @@ int intel_guc_enable_ct(struct intel_guc *guc)
 
/* Switch into cmd transport buffer based send() */
guc->send = intel_guc_send_ct;
+   guc->recv = intel_guc_receive_ct;
DRM_INFO("CT: %s\n", enableddisabled(true));
return 0;
 }
@@ -458,5 +577,6 @@ void intel_guc_disable_ct(struct 

[Intel-gfx] [PATCH 05/15] drm/i915/guc: Move Guc notification handling to separate function

[Michal Wajdeczko]

To allow future code reuse. While here, fix comment style.

Suggested-by: Oscar Mateo 
Signed-off-by: Michal Wajdeczko 
Cc: Oscar Mateo 
Cc: Sagar Arun Kamble 
Cc: Daniele Ceraolo Spurio 
---
 drivers/gpu/drm/i915/i915_irq.c | 33 ++---
 drivers/gpu/drm/i915/intel_uc.c | 37 +
 drivers/gpu/drm/i915/intel_uc.h |  1 +
 3 files changed, 40 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 196caa4..ac69534 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1675,37 +1675,8 @@ static void gen6_rps_irq_handler(struct drm_i915_private 
*dev_priv, u32 pm_iir)
 
 static void gen9_guc_irq_handler(struct drm_i915_private *dev_priv, u32 gt_iir)
 {
-   if (gt_iir & GEN9_GUC_TO_HOST_INT_EVENT) {
-   /* Sample the log buffer flush related bits & clear them out now
-* itself from the message identity register to minimize the
-* probability of losing a flush interrupt, when there are back
-* to back flush interrupts.
-* There can be a new flush interrupt, for different log buffer
-* type (like for ISR), whilst Host is handling one (for DPC).
-* Since same bit is used in message register for ISR & DPC, it
-* could happen that GuC sets the bit for 2nd interrupt but Host
-* clears out the bit on handling the 1st interrupt.
-*/
-   u32 msg, flush;
-
-   msg = I915_READ(SOFT_SCRATCH(15));
-   flush = msg & (INTEL_GUC_RECV_MSG_CRASH_DUMP_POSTED |
-  INTEL_GUC_RECV_MSG_FLUSH_LOG_BUFFER);
-   if (flush) {
-   /* Clear the message bits that are handled */
-   I915_WRITE(SOFT_SCRATCH(15), msg & ~flush);
-
-   /* Handle flush interrupt in bottom half */
-   queue_work(dev_priv->guc.log.runtime.flush_wq,
-  _priv->guc.log.runtime.flush_work);
-
-   dev_priv->guc.log.flush_interrupt_count++;
-   } else {
-   /* Not clearing of unhandled event bits won't result in
-* re-triggering of the interrupt.
-*/
-   }
-   }
+   if (gt_iir & GEN9_GUC_TO_HOST_INT_EVENT)
+   intel_guc_notification_handler(_priv->guc);
 }
 
 static void valleyview_pipestat_irq_ack(struct drm_i915_private *dev_priv,
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index c704968..1e6390e 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -532,6 +532,43 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 
*action, u32 len,
return ret;
 }
 
+void intel_guc_notification_handler(struct intel_guc *guc)
+{
+   struct drm_i915_private *dev_priv = guc_to_i915(guc);
+   u32 msg, flush;
+
+   /*
+* Sample the log buffer flush related bits & clear them out now
+* itself from the message identity register to minimize the
+* probability of losing a flush interrupt, when there are back
+* to back flush interrupts.
+* There can be a new flush interrupt, for different log buffer
+* type (like for ISR), whilst Host is handling one (for DPC).
+* Since same bit is used in message register for ISR & DPC, it
+* could happen that GuC sets the bit for 2nd interrupt but Host
+* clears out the bit on handling the 1st interrupt.
+*/
+
+   msg = I915_READ(SOFT_SCRATCH(15));
+   flush = msg & (INTEL_GUC_RECV_MSG_CRASH_DUMP_POSTED |
+  INTEL_GUC_RECV_MSG_FLUSH_LOG_BUFFER);
+   if (flush) {
+   /* Clear the message bits that are handled */
+   I915_WRITE(SOFT_SCRATCH(15), msg & ~flush);
+
+   /* Handle flush interrupt in bottom half */
+   queue_work(dev_priv->guc.log.runtime.flush_wq,
+   _priv->guc.log.runtime.flush_work);
+
+   dev_priv->guc.log.flush_interrupt_count++;
+   } else {
+   /*
+* Not clearing of unhandled event bits won't result in
+* re-triggering of the interrupt.
+*/
+   }
+}
+
 int intel_guc_sample_forcewake(struct intel_guc *guc)
 {
struct drm_i915_private *dev_priv = guc_to_i915(guc);
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 9353ac3..2789179 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -226,6 +226,7 @@ void intel_uc_init_fw(struct drm_i915_private *dev_priv);
 void 

[Intel-gfx] [PATCH 04/15] drm/i915/guc: Implement response handling in send_mmio()

[Michal Wajdeczko]

In addition to already returned small data encoded in the status MMIO,
GuC may write more additional data in remaining MMIO regs. Lets copy
all that regs into optionally provided response buffer.

Signed-off-by: Michal Wajdeczko 
Cc: Daniele Ceraolo Spurio 
Cc: Oscar Mateo 
---
 drivers/gpu/drm/i915/intel_uc.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 77da750..c704968 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -517,6 +517,11 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 
*action, u32 len,
 " ret=%d status=0x%08X response=0x%08X\n",
 action[0], ret, status, I915_READ(SOFT_SCRATCH(15)));
} else {
+   if (response) {
+   /* Skip reg[0] with the status/response mask */
+   for (i = 1; i < guc->send_regs.count; i++)
+   response[i] = I915_READ(guc_send_reg(guc, i));
+   }
/* Use data encoded in status dword as return value */
ret = INTEL_GUC_RECV_TO_DATA(status);
}
-- 
2.7.4

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

[Intel-gfx] [PATCH 10/15] drm/i915/guc: Use better name for helper wait function

[Michal Wajdeczko]

In next patch we will introduce another way of waiting for the response
that will use RECV buffer. To avoid mismatched names, rename old wait
function to reflect the fact that it is based on descriptor update.

Signed-off-by: Michal Wajdeczko 
---
 drivers/gpu/drm/i915/intel_guc_ct.c | 12 +++-
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c 
b/drivers/gpu/drm/i915/intel_guc_ct.c
index dd30c83..4fabea17 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/intel_guc_ct.c
@@ -321,16 +321,18 @@ static int ctb_write(struct intel_guc_ct_buffer *ctb,
return 0;
 }
 
-/* Wait for the response from the GuC.
+/* Wait for the descriptor update.
+ * Guc will update this descriptor with new fence and status.
+ * @desc:  buffer descriptor
  * @fence: response fence
  * @status:placeholder for status
  * return: 0 response received (status is valid)
  * -ETIMEDOUT no response within hardcoded timeout
  * -EPROTO no response, ct buffer was in error
  */
-static int wait_for_response(struct guc_ct_buffer_desc *desc,
-u32 fence,
-u32 *status)
+static int wait_for_desc_update(struct guc_ct_buffer_desc *desc,
+   u32 fence,
+   u32 *status)
 {
int err;
 
@@ -384,7 +386,7 @@ static int ctch_send(struct intel_guc *guc,
 
intel_guc_notify(guc);
 
-   err = wait_for_response(desc, fence, status);
+   err = wait_for_desc_update(desc, fence, status);
if (unlikely(err))
return err;
if (INTEL_GUC_RECV_TO_STATUS(*status) != INTEL_GUC_STATUS_SUCCESS)
-- 
2.7.4

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

[Intel-gfx] [PATCH 07/15] drm/i915/guc: Create a GuC receive function

[Michal Wajdeczko]

From: Oscar Mateo 

This function, symmetrical to the send(), will handle Guc2Host message
interrupts (which at the moment still only covers requests to flush
the GuC logs).

Cc: Michal Wajdeczko 
Cc: Daniele Ceraolo Spurio 
Signed-off-by: Oscar Mateo 
Signed-off-by: Michal Wajdeczko 
---
 drivers/gpu/drm/i915/intel_uc.c | 18 +-
 drivers/gpu/drm/i915/intel_uc.h |  5 +
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index a091e83..258e0d0 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -109,6 +109,7 @@ void intel_uc_init_early(struct drm_i915_private *dev_priv)
 
mutex_init(>send_mutex);
guc->send = intel_guc_send_nop;
+   guc->recv = intel_guc_receive_nop;
guc->notify = guc_write_irq_trigger;
 }
 
@@ -315,6 +316,7 @@ static int guc_enable_communication(struct intel_guc *guc)
return intel_guc_enable_ct(guc);
 
guc->send = intel_guc_send_mmio;
+   guc->recv = intel_guc_receive_mmio;
return 0;
 }
 
@@ -326,6 +328,7 @@ static void guc_disable_communication(struct intel_guc *guc)
intel_guc_disable_ct(guc);
 
guc->send = intel_guc_send_nop;
+   guc->recv = intel_guc_receive_nop;
 }
 
 int intel_uc_init_hw(struct drm_i915_private *dev_priv)
@@ -466,6 +469,11 @@ int intel_guc_send_nop(struct intel_guc *guc, const u32 
*action, u32 len,
return -ENODEV;
 }
 
+void intel_guc_receive_nop(struct intel_guc *guc)
+{
+   WARN(1, "Unexpected receive\n");
+}
+
 /*
  * This function implements the MMIO based host to GuC interface.
  */
@@ -532,7 +540,10 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 
*action, u32 len,
return ret;
 }
 
-void intel_guc_notification_handler(struct intel_guc *guc)
+/*
+ * This function implements the MMIO based GuC to host interface.
+ */
+void intel_guc_receive_mmio(struct intel_guc *guc)
 {
struct drm_i915_private *dev_priv = guc_to_i915(guc);
u32 msg, flush;
@@ -565,6 +576,11 @@ void intel_guc_notification_handler(struct intel_guc *guc)
}
 }
 
+void intel_guc_notification_handler(struct intel_guc *guc)
+{
+   guc->recv(guc);
+}
+
 int intel_guc_sample_forcewake(struct intel_guc *guc)
 {
struct drm_i915_private *dev_priv = guc_to_i915(guc);
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 4808f47..6f20e66 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -208,6 +208,9 @@ struct intel_guc {
/* GuC's FW specific send function */
int (*send)(struct intel_guc *guc, const u32 *data, u32 len, u32 *resp);
 
+   /* GuC's FW specific receive function */
+   void (*recv)(struct intel_guc *guc);
+
/* GuC's FW specific notify function */
void (*notify)(struct intel_guc *guc);
 };
@@ -230,6 +233,8 @@ void intel_guc_notification_handler(struct intel_guc *guc);
 int intel_guc_sample_forcewake(struct intel_guc *guc);
 int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len, u32 
*response);
 int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len, u32 
*response);
+void intel_guc_receive_nop(struct intel_guc *guc);
+void intel_guc_receive_mmio(struct intel_guc *guc);
 
 static inline int intel_guc_send(struct intel_guc *guc, const u32 *action, u32 
len)
 {
-- 
2.7.4

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

[Intel-gfx] [PATCH 08/15] drm/i915/guc: Update CT message header definition

[Michal Wajdeczko]

Flags bits are different in G2H message.

Signed-off-by: Michal Wajdeczko 
Cc: Daniele Ceraolo Spurio 
Cc: Oscar Mateo 
Cc: Kelvin Gardiner 
---
 drivers/gpu/drm/i915/intel_guc_fwif.h | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h 
b/drivers/gpu/drm/i915/intel_guc_fwif.h
index 98c0560..9bc3067 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -358,17 +358,24 @@ struct guc_ct_buffer_desc {
  *
  * bit[4..0]   message len (in dwords)
  * bit[7..5]   reserved
+ * bit[10..8]  flags
+ * bit[15..11] reserved
+ * bit[31..16] action code
+ *
+ * H2G flags:
  * bit[8]  write fence to desc
  * bit[9]  write status to H2G buff
  * bit[10] send status (via G2H)
- * bit[15..11] reserved
- * bit[31..16] action code
+ *
+ * G2H flags:
+ * bit[8]  is response
  */
 #define GUC_CT_MSG_LEN_SHIFT   0
 #define GUC_CT_MSG_LEN_MASK0x1F
 #define GUC_CT_MSG_WRITE_FENCE_TO_DESC (1 << 8)
 #define GUC_CT_MSG_WRITE_STATUS_TO_BUFF(1 << 9)
 #define GUC_CT_MSG_SEND_STATUS (1 << 10)
+#define GUC_CT_MSG_IS_RESPONSE (1 << 8)
 #define GUC_CT_MSG_ACTION_SHIFT16
 #define GUC_CT_MSG_ACTION_MASK 0x
 
-- 
2.7.4

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

[Intel-gfx] [PATCH 00/15] drm/i915/guc: Support for Guc responses and requests

[Michal Wajdeczko]

With this series we will be able to receive more data from the Guc.
New Guc firmware will be required to actually use that feature.

Michal Wajdeczko (13):
  drm/i915/guc: Add support for data reporting in GuC responses
  drm/i915/guc: Prepare send() function to accept bigger response
  drm/i915/guc: Add send_and_receive() helper function
  drm/i915/guc: Implement response handling in send_mmio()
  drm/i915/guc: Move Guc notification handling to separate function
  drm/i915/guc: Update CT message header definition
  drm/i915/guc: Prepare to handle messages from CT RECV buffer
  drm/i915/guc: Use better name for helper wait function
  drm/i915/guc: Implement response handling in send_ct()
  drm/i915/guc: Prepare to process incoming requests from CT
  drm/i915/guc: Handle default action received over CT
  drm/i915/guc: Enable GuC interrupts when using CT
  drm/i915/guc: Trace messages from CT while in debug

Oscar Mateo (2):
  drm/i915/guc: Move flushing the GuC logs outside notification handler
  drm/i915/guc: Create a GuC receive function

 drivers/gpu/drm/i915/i915_guc_submission.c |   2 +-
 drivers/gpu/drm/i915/i915_irq.c|  33 +--
 drivers/gpu/drm/i915/intel_guc_ct.c| 359 +++--
 drivers/gpu/drm/i915/intel_guc_ct.h|   9 +
 drivers/gpu/drm/i915/intel_guc_fwif.h  |  17 +-
 drivers/gpu/drm/i915/intel_guc_log.c   |   8 +
 drivers/gpu/drm/i915/intel_uc.c|  74 +-
 drivers/gpu/drm/i915/intel_uc.h|  23 +-
 8 files changed, 462 insertions(+), 63 deletions(-)

-- 
2.7.4

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

[Intel-gfx] [PATCH 03/15] drm/i915/guc: Add send_and_receive() helper function

[Michal Wajdeczko]

In the previous patch we have changed signature of the send function
pointer but we didn't modify signature of the corresponding helper
function to minimize number of required changes. Let's add separate
helper to expose new functionality but still hide underlying details.

Signed-off-by: Michal Wajdeczko 
Cc: Oscar Mateo 
Cc: Michel Thierry 
Cc: Daniele Ceraolo Spurio 
---
 drivers/gpu/drm/i915/intel_uc.h | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 53ea5f1..9353ac3 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -235,6 +235,13 @@ static inline int intel_guc_send(struct intel_guc *guc, 
const u32 *action, u32 l
return guc->send(guc, action, len, NULL);
 }
 
+static inline int intel_guc_send_and_receive(struct intel_guc *guc,
+const u32 *action, u32 len,
+u32 *response)
+{
+   return guc->send(guc, action, len, response);
+}
+
 static inline void intel_guc_notify(struct intel_guc *guc)
 {
guc->notify(guc);
-- 
2.7.4

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

[Intel-gfx] [PATCH 01/15] drm/i915/guc: Add support for data reporting in GuC responses

[Michal Wajdeczko]

GuC may return additional data in the command status response.
Format and meaning of this data is action specific.
We will use this non-negative data as a new success return value.

Signed-off-by: Michal Wajdeczko 
Cc: Oscar Mateo 
Cc: Michel Thierry 
Cc: Daniele Ceraolo Spurio 
---
 drivers/gpu/drm/i915/intel_guc_ct.c   | 14 +++---
 drivers/gpu/drm/i915/intel_guc_fwif.h |  6 ++
 drivers/gpu/drm/i915/intel_uc.c   |  5 -
 3 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c 
b/drivers/gpu/drm/i915/intel_guc_ct.c
index c4cbec1..1249868 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/intel_guc_ct.c
@@ -387,9 +387,9 @@ static int ctch_send(struct intel_guc *guc,
err = wait_for_response(desc, fence, status);
if (unlikely(err))
return err;
-   if (*status != INTEL_GUC_STATUS_SUCCESS)
+   if (INTEL_GUC_RECV_TO_STATUS(*status) != INTEL_GUC_STATUS_SUCCESS)
return -EIO;
-   return 0;
+   return INTEL_GUC_RECV_TO_DATA(*status);
 }
 
 /*
@@ -399,18 +399,18 @@ static int intel_guc_send_ct(struct intel_guc *guc, const 
u32 *action, u32 len)
 {
struct intel_guc_ct_channel *ctch = >ct.host_channel;
u32 status = ~0; /* undefined */
-   int err;
+   int ret;
 
mutex_lock(>send_mutex);
 
-   err = ctch_send(guc, ctch, action, len, );
-   if (unlikely(err)) {
+   ret = ctch_send(guc, ctch, action, len, );
+   if (unlikely(ret < 0)) {
DRM_ERROR("CT: send action %#X failed; err=%d status=%#X\n",
- action[0], err, status);
+ action[0], ret, status);
}
 
mutex_unlock(>send_mutex);
-   return err;
+   return ret;
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h 
b/drivers/gpu/drm/i915/intel_guc_fwif.h
index 5fa2860..98c0560 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -567,10 +567,16 @@ enum intel_guc_action {
  * command in SS0. The response is distinguishable from a command
  * by the fact that all the MASK bits are set. The remaining bits
  * give more detail.
+ * Bits [16:27] are reserved for optional data reporting.
  */
 #defineINTEL_GUC_RECV_MASK ((u32)0xF000)
 #defineINTEL_GUC_RECV_IS_RESPONSE(x)   ((u32)(x) >= 
INTEL_GUC_RECV_MASK)
 #defineINTEL_GUC_RECV_STATUS(x)(INTEL_GUC_RECV_MASK | (x))
+#define INTEL_GUC_RECV_DATA_SHIFT  16
+#define INTEL_GUC_RECV_DATA_MASK   (0xFFF << INTEL_GUC_RECV_DATA_SHIFT)
+#define INTEL_GUC_RECV_TO_STATUS(x)((x) & ~ INTEL_GUC_RECV_DATA_MASK)
+#define INTEL_GUC_RECV_TO_DATA(x)  (((x) & INTEL_GUC_RECV_DATA_MASK) >> \
+INTEL_GUC_RECV_DATA_SHIFT)
 
 /* GUC will return status back to SOFT_SCRATCH_O_REG */
 enum intel_guc_status {
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 27e072c..ff25477 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -502,7 +502,7 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 
*action, u32 len)
   INTEL_GUC_RECV_MASK,
   INTEL_GUC_RECV_MASK,
   10, 10, );
-   if (status != INTEL_GUC_STATUS_SUCCESS) {
+   if (INTEL_GUC_RECV_TO_STATUS(status) != INTEL_GUC_STATUS_SUCCESS) {
/*
 * Either the GuC explicitly returned an error (which
 * we convert to -EIO here) or no response at all was
@@ -514,6 +514,9 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 
*action, u32 len)
DRM_WARN("INTEL_GUC_SEND: Action 0x%X failed;"
 " ret=%d status=0x%08X response=0x%08X\n",
 action[0], ret, status, I915_READ(SOFT_SCRATCH(15)));
+   } else {
+   /* Use data encoded in status dword as return value */
+   ret = INTEL_GUC_RECV_TO_DATA(status);
}
 
intel_uncore_forcewake_put(dev_priv, guc->send_regs.fw_domains);
-- 
2.7.4

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

[Intel-gfx] [PATCH i-g-t] tests/perf: follow up build fix

[Lionel Landwerlin]

Signed-off-by: Lionel Landwerlin 
Fixes: adcde8ac ("tests/perf: fix build where system headers don't have Gen8 
formats")
---
 tests/perf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/perf.c b/tests/perf.c
index 9cfc4bb9..a7fa33a1 100644
--- a/tests/perf.c
+++ b/tests/perf.c
@@ -146,7 +146,7 @@ enum drm_i915_perf_record_type {
 /* There is no ifdef we can use for those formats :( */
 enum {
local_I915_OA_FORMAT_A12 = I915_OA_FORMAT_C4_B8 + 1,
-   local_I915_OA_FORMAT_A12_B8_C8 = I915_OA_FORMAT_A12 + 2,
+   local_I915_OA_FORMAT_A12_B8_C8 = I915_OA_FORMAT_C4_B8 + 2,
local_I915_OA_FORMAT_A32u40_A4u32_B8_C8 = I915_OA_FORMAT_C4_B8 + 3,
 };
 
-- 
2.13.3

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

[Intel-gfx] [PATCH i-g-t] tests/kms_frontbuffer_tracking: convert macros to functions

[Daniel Vetter]

I guess this was done to have a better indication of which testcase
and function failed, but igt nowadays dumps an entire stacktrace. And
macros of this magnitude mean the line number is entirely meaningless,
since it doesn't point at a specific check.

Reason I've started to looking into this is that in our full igt CI
runs we have a serious problem with the fbc testcases randomly
failing with

(kms_frontbuffer_tracking:1609) CRITICAL: Test assertion failure function 
enable_prim_screen_and_wait, file kms_frontbuffer_tracking.c:1771:
(kms_frontbuffer_tracking:1609) CRITICAL: Failed assertion: false
(kms_frontbuffer_tracking:1609) CRITICAL: FBC disabled

And that's not entirely helpful. Also, macros of this magnitude are
just horrible to read.

Cc: Paulo Zanoni 
Signed-off-by: Daniel Vetter 
---
 tests/kms_frontbuffer_tracking.c | 166 ---
 1 file changed, 84 insertions(+), 82 deletions(-)

diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c
index e03524f1c45b..8d11dc065623 100644
--- a/tests/kms_frontbuffer_tracking.c
+++ b/tests/kms_frontbuffer_tracking.c
@@ -1677,88 +1677,90 @@ static int adjust_assertion_flags(const struct 
test_mode *t, int flags)
return flags;
 }
 
-#define do_crc_assertions(flags, mandatory_sink_crc) do {  \
-   int flags__ = (flags);  \
-   struct both_crcs crc_;  \
-   \
-   if (!opt.check_crc || (flags__ & DONT_ASSERT_CRC))  \
-   break;  \
-   \
-   collect_crcs(_, mandatory_sink_crc);\
-   print_crc("Calculated CRC:", _);\
-   \
-   igt_assert(wanted_crc); \
-   igt_assert_crc_equal(_.pipe, _crc->pipe);\
-   if (mandatory_sink_crc) \
-   assert_sink_crc_equal(_.sink, _crc->sink);   \
-   else if (sink_crc.reliable &&   \
-!sink_crc_equal(_.sink, _crc->sink))\
-   igt_info("Sink CRC differ, but not required\n");\
-} while (0)
-
-#define do_status_assertions(flags_) do {  \
-   if (!opt.check_status) {\
-   /* Make sure we settle before continuing. */\
-   sleep(1);   \
-   break;  \
-   }   \
-   \
-   if (flags_ & ASSERT_FBC_ENABLED) {  \
-   igt_require(!fbc_not_enough_stolen());  \
-   igt_require(!fbc_stride_not_supported());   \
-   if (!fbc_wait_until_enabled()) {\
-   fbc_print_status(); \
-   igt_assert_f(false, "FBC disabled\n");  \
-   }   \
-   \
-   if (opt.fbc_check_compression)  \
-   igt_assert(fbc_wait_for_compression()); \
-   } else if (flags_ & ASSERT_FBC_DISABLED) {  \
-   igt_assert(!fbc_wait_until_enabled());  \
-   }   \
-   \
-   if (flags_ & ASSERT_PSR_ENABLED) {  \
-   if (!psr_wait_until_enabled()) {\
-   psr_print_status(); \
-   igt_assert_f(false, "PSR disabled\n");  \
-   }   \
-   } else if (flags_ & ASSERT_PSR_DISABLED) {  \
-   igt_assert(!psr_wait_until_enabled());  \
-   }   \
-} while (0)
-
-#define do_assertions(flags) do {  \
-   int flags_ = adjust_assertion_flags(t, (flags));\
-   bool mandatory_sink_crc = t->feature & FEATURE_PSR; \
-  

[Intel-gfx] [PATCH i-g-t] lib: don't hang on blt on snb

[Daniel Vetter]

We now have full (or a lot at least) igt running in beta CI, and snb
blt hangs are really unhappy:

- drv_hangman@error-state-capture-blt and gem_exec_capture@capture-blt
  reliably result in insta-machine death when we try to reset the gpu,
  both on the CI snb and the one I have here.

- Other testcases also randomly (and sometimes rather rarely) die on
  snb.

We can't use the endless batch because that results in a reset failure
and wedged gpu, so also not really better.

Until this works reliably it's best to take the tests out of igt,
since machine death has massive impact in creating noise due to the
per-build sharding changing the victimized tests all the time.

Most tests use igt_hang, but gem_exec_capture needed to be switched to
the igt_require_hang_ring helper.

Cc: Tomi Sarvela 
Cc: Chris Wilson 
Signed-off-by: Daniel Vetter 
---
 lib/igt_gt.c | 4 
 tests/gem_exec_capture.c | 3 ++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/lib/igt_gt.c b/lib/igt_gt.c
index 6f7daa5ef982..99d709fe4086 100644
--- a/lib/igt_gt.c
+++ b/lib/igt_gt.c
@@ -118,6 +118,10 @@ void igt_require_hang_ring(int fd, int ring)
if (!igt_check_boolean_env_var("IGT_HANG", true))
igt_skip("hang injection disabled by user");
 
+   igt_require_f(ring != I915_EXEC_BLT ||
+ intel_gen(intel_get_drm_devid(fd)) != 6,
+ "blt hang can causes insta-death on snb.\n");
+
gem_require_ring(fd, ring);
gem_context_require_bannable(fd);
if (!igt_check_boolean_env_var("IGT_HANG_WITHOUT_RESET", false))
diff --git a/tests/gem_exec_capture.c b/tests/gem_exec_capture.c
index f8f43d2903a9..fb4a5e85f6b2 100644
--- a/tests/gem_exec_capture.c
+++ b/tests/gem_exec_capture.c
@@ -69,6 +69,8 @@ static void capture(int fd, int dir, unsigned ring)
uint32_t *batch;
int i;
 
+   igt_require_hang_ring(fd, ring);
+
memset(obj, 0, sizeof(obj));
obj[SCRATCH].handle = gem_create(fd, 4096);
obj[CAPTURE].handle = gem_create(fd, 4096);
@@ -166,7 +168,6 @@ igt_main
continue;
 
igt_subtest_f("capture-%s", e->name) {
-   gem_require_ring(fd, e->exec_id | e->flags);
capture(fd, dir, e->exec_id | e->flags);
}
}
-- 
2.5.5

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

Re: [Intel-gfx] [PATCH] tests/kms_ccs: Fix the color/ccs surface generation

[Jason Ekstrand]

On Fri, Aug 4, 2017 at 8:42 AM, Daniel Stone  wrote:

> On 4 August 2017 at 15:56, Jason Ekstrand  wrote:
> > On August 4, 2017 2:59:56 AM Daniel Stone  wrote:
> >>> +   width = ALIGN(f.width * 4, 32) / 32;
> >>> +   height = ALIGN(f.height, 16) / 16;
> >>> +   f.pitches[1] = ALIGN(width * 1, 128);
> >>> f.modifier[1] = modifier;
> >>> f.offsets[1] = size[0];
> >>> -   size[1] = f.pitches[1] * ALIGN(height, 64);
> >>> +   size[1] = f.pitches[1] * ALIGN(height, 32);
> >>
> >>
> >> I changed this to f.height rather than height, because otherwise the
> >> kernel was rejecting the aux buffer for being too small.
> >
> > Congratulations, you found a bug in the kernel branch you're running.
> The
> > downsized height is definitely what we want and it works fine with my
> kernel
> > branch.
>
> Great. Which kernel are you running then? I'm running from here:
> https://git.collabora.com/cgit/user/daniels/linux.git/refs/heads
>

I'm working from some branch I got from Ville a couple months ago.


> Currently we have hsub/vsub defined as 16/8 (Vidya inverted this, but
> I never got a clear answer on why),


Here's my comment in the IGT test:

/* From the Sky Lake PRM, Vol 12, "Color Control Surface":
 *
 *"The compression state of the cache-line pair is
 *specified by 2 bits in the CCS.  Each CCS cache-line
 *represents an area on the main surface of 16x16 sets
 *of 128 byte Y-tiled cache-line-pairs. CCS is always Y
 *tiled."
 *
 * A "cache-line-pair" for a Y-tiled surface is two
 * horizontally adjacent cache lines.  When operating in
 * bytes and rows, this gives us a scale-down factor of
 * 32x16.  Since the main surface has a 32-bit format, we
 * need to multiply width by 4 to get bytes.
 */

We have a scaling factor, in bytes, of 32x16.  However, the main surface
uses 4 byes per pixel so we need to account for that.  In the IGT test, we
multiply the width of the main surface by 4 to get bytes.  Alternatively,
you can adjust the scale factor to 8x16 so long as you align things
correctly.

tile_width as 128, and tile_height
> comes out as 32.


Yes, that's a correct Y-tile.


> Given the calculations in intel_fill_fb_info, I come
> out with the kernel demanding either 34816 bytes for CCS (using 16/8
> hsub/vsub), or 20480 bytes (8/16) for a 1920x1080 framebuffer.


Neither of those look right.  I'm getting 6 pages, or 24576B when I run the
test which should be correct.


> Which
> kernel do you have, and how are you coming out with that calculation?
> Do we need to go back and re-figure out what pitch is?
>
> FWIW, ISL seems to get it right, according to the kernel.
>

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

Re: [Intel-gfx] [PATCH 00/11] constify i915 attribute_group structures.

[Lionel Landwerlin]

On 04/08/17 11:39, Arvind Yadav wrote:




On Friday 04 August 2017 04:04 PM, Lionel Landwerlin wrote:

On 04/08/17 11:22, Arvind Yadav wrote:

Hi Lionel,


On Friday 04 August 2017 02:33 PM, Lionel Landwerlin wrote:

Hi Arwind,

These files were generated by a script maintained in this 
repository : 
https://github.com/rib/gputop/blob/master/scripts/i915-perf-kernelgen.py 

It would best to update this script first to make sure future 
platforms get the fixes too.


Some changes have just been merged, deleted most configs but the 
test ones.

You'll need to update your series.

I have done the changes. Please review it. :) Shared patch is 
0001-i915-perf-kernelgen.py-constify-attribute_group-stru.patch.


Hm... Where is it? (I can't see it on the mailing list nor attached)
The best would be to submit a PR on the github project directly.

I have push directly on github project. I have send patch to you. Is 
there  any different way to send mail.?

 Changes are looks like this.


It turns out the structs you've made const aren't in the tree anymore.

Thanks though!



---
 scripts/i915-perf-kernelgen.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/i915-perf-kernelgen.
py b/scripts/i915-perf-kernelgen.py
index 7178f47..7633624 100755
--- a/scripts/i915-perf-kernelgen.py
+++ b/scripts/i915-perf-kernelgen.py
@@ -382,7 +382,7 @@ def output_sysfs_code(sets):
 c("};")

 c("\n")
-c("static struct attribute_group group_" + perf_name_lc + " = {")
+c("static const struct attribute_group group_" + perf_name_lc 
+ " = {")

 c.indent(8)
 c(".name = \"" + metric_set['guid'] + "\",")
 c(".attrs =  attrs_" + perf_name_lc + ",")

---



Otherwise it looks like a good change.

Thanks,

-
Lionel

On 04/08/17 06:03, Arvind Yadav wrote:

attribute_group are not supposed to change at runtime. All functions
working with attribute_group provided by  work with
const attribute_group. So mark the non-const structs as const.

Arvind Yadav (11):
   [PATCH 01/11] drm: i915: i915_oa_kblgt2: constify 
attribute_group structures.
   [PATCH 02/11] drm: i915: i915_oa_bdw: constify attribute_group 
structures.
   [PATCH 03/11] drm: i915: i915_oa_bxt: constify attribute_group 
structures.
   [PATCH 04/11] drm: i915: i915_oa_chv: constify attribute_group 
structures.
   [PATCH 05/11] drm: i915: i915_oa_glk: constify attribute_group 
structures.
   [PATCH 06/11] drm: i915: i915_oa_hsw: constify attribute_group 
structures.
   [PATCH 07/11] drm: i915: i915_oa_kblgt3: constify 
attribute_group structures.
   [PATCH 08/11] drm: i915: i915_oa_sklgt2: constify 
attribute_group structures.
   [PATCH 09/11] drm: i915: i915_oa_sklgt3: constify 
attribute_group structures.
   [PATCH 10/11] drm: i915: i915_oa_sklgt4: constify 
attribute_group structures.
   [PATCH 11/11] drm: i915: i915_sysfs: constify attribute_group 
structures.


  drivers/gpu/drm/i915/i915_oa_bdw.c| 44 
+--

  drivers/gpu/drm/i915/i915_oa_bxt.c| 30 
  drivers/gpu/drm/i915/i915_oa_chv.c| 28 +++---
  drivers/gpu/drm/i915/i915_oa_glk.c| 30 
  drivers/gpu/drm/i915/i915_oa_hsw.c| 12 +-
  drivers/gpu/drm/i915/i915_oa_kblgt2.c | 36 
++--
  drivers/gpu/drm/i915/i915_oa_kblgt3.c | 36 
++--
  drivers/gpu/drm/i915/i915_oa_sklgt2.c | 36 
++--
  drivers/gpu/drm/i915/i915_oa_sklgt3.c | 36 
++--
  drivers/gpu/drm/i915/i915_oa_sklgt4.c | 36 
++--

  drivers/gpu/drm/i915/i915_sysfs.c |  6 ++---
  11 files changed, 165 insertions(+), 165 deletions(-)




~arvind







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

Re: [Intel-gfx] [PATCH] tests/kms_ccs: Fix the color/ccs surface generation

[Daniel Stone]

On 4 August 2017 at 15:56, Jason Ekstrand  wrote:
> On August 4, 2017 2:59:56 AM Daniel Stone  wrote:
>>> +   width = ALIGN(f.width * 4, 32) / 32;
>>> +   height = ALIGN(f.height, 16) / 16;
>>> +   f.pitches[1] = ALIGN(width * 1, 128);
>>> f.modifier[1] = modifier;
>>> f.offsets[1] = size[0];
>>> -   size[1] = f.pitches[1] * ALIGN(height, 64);
>>> +   size[1] = f.pitches[1] * ALIGN(height, 32);
>>
>>
>> I changed this to f.height rather than height, because otherwise the
>> kernel was rejecting the aux buffer for being too small.
>
> Congratulations, you found a bug in the kernel branch you're running.  The
> downsized height is definitely what we want and it works fine with my kernel
> branch.

Great. Which kernel are you running then? I'm running from here:
https://git.collabora.com/cgit/user/daniels/linux.git/refs/heads

Currently we have hsub/vsub defined as 16/8 (Vidya inverted this, but
I never got a clear answer on why), tile_width as 128, and tile_height
comes out as 32. Given the calculations in intel_fill_fb_info, I come
out with the kernel demanding either 34816 bytes for CCS (using 16/8
hsub/vsub), or 20480 bytes (8/16) for a 1920x1080 framebuffer. Which
kernel do you have, and how are you coming out with that calculation?
Do we need to go back and re-figure out what pitch is?

FWIW, ISL seems to get it right, according to the kernel.

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

[Intel-gfx] [PATCH v4 i-g-t] tests/perf: fix build where system headers don't have Gen8 formats

[Lionel Landwerlin]

v2: Use previous enum to define the new Gen8 enums (Petri)

v3: Duh! (Lionel)

v4: Redefine MAX oa formats value (Daniel)

Signed-off-by: Lionel Landwerlin 
---
 tests/perf.c | 21 +++--
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/tests/perf.c b/tests/perf.c
index f0ec26dd..9cfc4bb9 100644
--- a/tests/perf.c
+++ b/tests/perf.c
@@ -143,6 +143,15 @@ enum drm_i915_perf_record_type {
 };
 #endif /* !DRM_I915_PERF_OPEN */

+/* There is no ifdef we can use for those formats :( */
+enum {
+   local_I915_OA_FORMAT_A12 = I915_OA_FORMAT_C4_B8 + 1,
+   local_I915_OA_FORMAT_A12_B8_C8 = I915_OA_FORMAT_A12 + 2,
+   local_I915_OA_FORMAT_A32u40_A4u32_B8_C8 = I915_OA_FORMAT_C4_B8 + 3,
+};
+
+#define local_I915_OA_FORMAT_MAX (local_I915_OA_FORMAT_A32u40_A4u32_B8_C8 + 1)
+
 #ifndef DRM_IOCTL_I915_PERF_ADD_CONFIG

 #define DRM_I915_PERF_ADD_CONFIG   0x37
@@ -191,7 +200,7 @@ static struct {
int n_c;
int min_gen;
int max_gen;
-} oa_formats[I915_OA_FORMAT_MAX] = {
+} oa_formats[local_I915_OA_FORMAT_MAX] = {
[I915_OA_FORMAT_A13] = { /* HSW only */
"A13", .size = 64,
.a_off = 12, .n_a = 13,
@@ -230,17 +239,17 @@ static struct {

/* Gen8+ */

-   [I915_OA_FORMAT_A12] = {
+   [local_I915_OA_FORMAT_A12] = {
"A12", .size = 64,
.a_off = 12, .n_a = 12, .first_a = 7,
.min_gen = 8 },
-   [I915_OA_FORMAT_A12_B8_C8] = {
+   [local_I915_OA_FORMAT_A12_B8_C8] = {
"A12_B8_C8", .size = 128,
.a_off = 12, .n_a = 12,
.b_off = 64, .n_b = 8,
.c_off = 96, .n_c = 8, .first_a = 7,
.min_gen = 8 },
-   [I915_OA_FORMAT_A32u40_A4u32_B8_C8] = {
+   [local_I915_OA_FORMAT_A32u40_A4u32_B8_C8] = {
"A32u40_A4u32_B8_C8", .size = 256,
.a40_high_off = 160, .a40_low_off = 16, .n_a40 = 32,
.a_off = 144, .n_a = 4, .first_a = 32,
@@ -311,7 +320,7 @@ __perf_open(int fd, struct drm_i915_perf_open_param *param)
 static int
 lookup_format(int i915_perf_fmt_id)
 {
-   igt_assert(i915_perf_fmt_id < I915_OA_FORMAT_MAX);
+   igt_assert(i915_perf_fmt_id < local_I915_OA_FORMAT_MAX);
igt_assert(oa_formats[i915_perf_fmt_id].name);

return i915_perf_fmt_id;
@@ -806,7 +815,7 @@ init_sys_info(void)
drm_i915_getparam_t gp;

test_set_name = "TestOa";
-   test_oa_format = I915_OA_FORMAT_A32u40_A4u32_B8_C8;
+   test_oa_format = local_I915_OA_FORMAT_A32u40_A4u32_B8_C8;
undefined_a_counters = gen8_undefined_a_counters;
read_report_ticks = gen8_read_report_ticks;
sanity_check_reports = gen8_sanity_check_test_oa_reports;
--
2.13.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v3 i-g-t] tests/perf: fix build where system headers don't have Gen8 formats

[Lionel Landwerlin]

v2: Use previous enum to define the new Gen8 enums (Petri)

v3: Duh! (Lionel)

Signed-off-by: Lionel Landwerlin 
---
 tests/perf.c | 13 ++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/tests/perf.c b/tests/perf.c
index f0ec26dd..54eb3605 100644
--- a/tests/perf.c
+++ b/tests/perf.c
@@ -143,6 +143,13 @@ enum drm_i915_perf_record_type {
 };
 #endif /* !DRM_I915_PERF_OPEN */

+/* There is no ifdef we can use for those formats :( */
+enum {
+   local_I915_OA_FORMAT_A12 = I915_OA_FORMAT_C4_B8 + 1,
+   local_I915_OA_FORMAT_A12_B8_C8 = I915_OA_FORMAT_C4_B8 + 2,
+   local_I915_OA_FORMAT_A32u40_A4u32_B8_C8 = I915_OA_FORMAT_C4_B8 + 3,
+};
+
 #ifndef DRM_IOCTL_I915_PERF_ADD_CONFIG

 #define DRM_I915_PERF_ADD_CONFIG   0x37
@@ -230,17 +237,17 @@ static struct {

/* Gen8+ */

-   [I915_OA_FORMAT_A12] = {
+   [local_I915_OA_FORMAT_A12] = {
"A12", .size = 64,
.a_off = 12, .n_a = 12, .first_a = 7,
.min_gen = 8 },
-   [I915_OA_FORMAT_A12_B8_C8] = {
+   [local_I915_OA_FORMAT_A12_B8_C8] = {
"A12_B8_C8", .size = 128,
.a_off = 12, .n_a = 12,
.b_off = 64, .n_b = 8,
.c_off = 96, .n_c = 8, .first_a = 7,
.min_gen = 8 },
-   [I915_OA_FORMAT_A32u40_A4u32_B8_C8] = {
+   [local_I915_OA_FORMAT_A32u40_A4u32_B8_C8] = {
"A32u40_A4u32_B8_C8", .size = 256,
.a40_high_off = 160, .a40_low_off = 16, .n_a40 = 32,
.a_off = 144, .n_a = 4, .first_a = 32,
--
2.13.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] tests/kms_ccs: Fix the color/ccs surface generation

[Jason Ekstrand]

On August 4, 2017 2:59:56 AM Daniel Stone  wrote:


Hi Jason,

On 4 August 2017 at 01:52, Jason Ekstrand  wrote:

Previously, the test used the old 64x64 convention that Ville introduced
for CCS tiles and not the current 128x32 Y-tile convention.  Also, the
original scheme for generating the CCS data was over-complicated and
didn't work correctly because it assumed you could cut the main surface
at an arbitrary Y coordinate.  While you clearly *can* do this (the
hardware does), it's not a good idea for a generator in a test.  The new
scheme, introduced here, is entirely based on the relationship between
cache-lines in the main surface and the CCS that's documented in the
PRM.  By keeping everything CCS cache-line aligned, our chances of
generating correct data for an arbitrary-size surface are much higher.


Thanks for fixing this!


+* We need to cut the surface on a CCS cache-line boundary,
+* otherwise, we're going to be in trouble when we try to
+* generate CCS data for the surface.  A cache line in the
+* CCS is 16x16 cache-line-pairs in the main surface.  16
+* cache lines is 64 rows high.
+*/
+   half_height = ALIGN(height, 128) / 2;
+   half_size = half_height * stride;
+   for (i = 0; i < fb->size / 4; i++) {
+   if (i < half_size / 4)
+   ptr[i] = RED;
+   else
+   ptr[i] = COMPRESSED_RED;


I toyed with:
else if (!(i & 0xe))
ptr[i] = COMPRESSED_RED;
else
ptr[i] = BLACK;

... to make the failure mode even more obvious. But that still writes
in (far) more compressed-red pixels than we strictly need for CCS,
right? Anyway, follow-up patch.


Yeah, we can probably do quite a bit of patterning so long as we keep the 
compressed/uncompressed split simple and make sure our pattern works in 
whole cache lines.



+static unsigned int
+y_tile_y_pos(unsigned int offset, unsigned int stride)
 {
-   return ptr +
-   ((y & ~0x3f) * stride) +
-   ((x & ~0x7) * 64) +
-   ((y & 0x3f) * 8) +
-   (x & 7);
+   unsigned int y_tiles, y;
+   y_tiles = (offset / 4096) / (stride / 128);
+   y = y_tiles * 32 + ((offset & 0x1f0) >> 4);
+   return y;
 }

@@ -143,12 +168,26 @@ static void display_fb(data_t *data, int compressed)
size[0] = f.pitches[0] * ALIGN(height, 32);

if (compressed) {
-   width = ALIGN(f.width, 16) / 16;
-   height = ALIGN(f.height, 8) / 8;
-   f.pitches[1] = ALIGN(width * 1, 64);
+   /* From the Sky Lake PRM, Vol 12, "Color Control Surface":
+*
+*"The compression state of the cache-line pair is
+*specified by 2 bits in the CCS.  Each CCS cache-line
+*represents an area on the main surface of 16x16 sets
+*of 128 byte Y-tiled cache-line-pairs. CCS is always Y
+*tiled."
+*
+* A "cache-line-pair" for a Y-tiled surface is two
+* horizontally adjacent cache lines.  When operating in
+* bytes and rows, this gives us a scale-down factor of
+* 32x16.  Since the main surface has a 32-bit format, we
+* need to multiply width by 4 to get bytes.
+*/


Yeah, this code and comment match better (& helped clarify) my
understanding of how it works. But given my understanding was mostly
gleaned from other, borderline incomprehensible, comments, as well as
manually poking bits and observing the result, maybe that's not a
ringing endorsement.


+   width = ALIGN(f.width * 4, 32) / 32;
+   height = ALIGN(f.height, 16) / 16;
+   f.pitches[1] = ALIGN(width * 1, 128);
f.modifier[1] = modifier;
f.offsets[1] = size[0];
-   size[1] = f.pitches[1] * ALIGN(height, 64);
+   size[1] = f.pitches[1] * ALIGN(height, 32);


I changed this to f.height rather than height, because otherwise the
kernel was rejecting the aux buffer for being too small.


Congratulations, you found a bug in the kernel branch you're running.  The 
downsized height is definitely what we want and it works fine with my 
kernel branch.



With that, it now passes on SKL + APL for me, so I've pushed with my
review. Thanks!

Cheers,
Daniel



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

Re: [Intel-gfx] [PATCH v2 i-g-t] tests/perf: fix build where system headers don't have Gen8 formats

[Daniel Stone]

On 4 August 2017 at 15:00, Lionel Landwerlin
 wrote:
> v2: Use previous enum to define the new Gen8 enums (Petri)
>
> Signed-off-by: Lionel Landwerlin 

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

[Intel-gfx] [PATCH i-g-t] tests/core_auth: set rlimit

[Daniel Vetter]

Some distros have huge rlimits and then the test takes forever, or
worse oom, or even worse, takse down the entire machine (which is
shouldn't be able to, but oh well, oom handling in linux).

Make sure we have a consistent rlimit by adjusting it manually.

v2: Use the default of 1024 from everywhere except ubuntu.

Cc: Tomi Sarvela 
Cc: Chris Wilson 
Signed-off-by: Daniel Vetter 
---
 tests/core_auth.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/tests/core_auth.c b/tests/core_auth.c
index e08c8aed4c63..cedcff923937 100644
--- a/tests/core_auth.c
+++ b/tests/core_auth.c
@@ -39,6 +39,7 @@
 #include 
 #include 
 #include 
+#include 
 #include "drm.h"
 
 IGT_TEST_DESCRIPTION("Call drmGetMagic() and drmAuthMagic() and see if it 
behaves.");
@@ -55,6 +56,12 @@ static void test_many_magics(int master)
char path[512];
int *fds = NULL, slave;
 
+   struct rlimit fd_limit;
+
+   do_or_die(getrlimit(RLIMIT_NOFILE, _limit));
+   fd_limit.rlim_cur = 1024;
+   do_or_die(setrlimit(RLIMIT_NOFILE, _limit));
+
sprintf(path, "/proc/self/fd/%d", master);
 
for (i = 0; ; ++i) {
@@ -157,6 +164,7 @@ igt_main
igt_subtest("basic-auth")
test_basic_auth(master);
 
+   /* this must be last, we adjust the rlimit */
igt_subtest("many-magics")
test_many_magics(master);
 }
-- 
2.5.5

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

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: remove unused function declaration

[Patchwork]

== Series Details ==

Series: drm/i915: remove unused function declaration
URL   : https://patchwork.freedesktop.org/series/28381/
State : success

== Summary ==

Series 28381v1 drm/i915: remove unused function declaration
https://patchwork.freedesktop.org/api/1.0/series/28381/revisions/1/mbox/

Test kms_pipe_crc_basic:
Subgroup hang-read-crc-pipe-a:
dmesg-warn -> PASS   (fi-pnv-d510) fdo#101597

fdo#101597 https://bugs.freedesktop.org/show_bug.cgi?id=101597

fi-bdw-5557u total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  
time:447s
fi-bdw-gvtdvmtotal:279  pass:265  dwarn:0   dfail:0   fail:0   skip:14  
time:420s
fi-blb-e6850 total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  
time:358s
fi-bsw-n3050 total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  
time:501s
fi-bwr-2160  total:279  pass:184  dwarn:0   dfail:0   fail:0   skip:95  
time:257s
fi-bxt-j4205 total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  
time:496s
fi-byt-j1900 total:279  pass:254  dwarn:1   dfail:0   fail:0   skip:24  
time:528s
fi-byt-n2820 total:279  pass:250  dwarn:1   dfail:0   fail:0   skip:28  
time:515s
fi-ctg-p8600 total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  
time:597s
fi-elk-e7500 total:279  pass:230  dwarn:0   dfail:0   fail:0   skip:49  
time:433s
fi-gdg-551   total:6pass:5dwarn:0   dfail:0   fail:0   skip:0  
fi-glk-2atotal:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  
time:583s
fi-hsw-4770  total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  
time:425s
fi-hsw-4770r total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  
time:407s
fi-ilk-650   total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  
time:417s
fi-ilk-m540  total:279  pass:233  dwarn:0   dfail:0   fail:0   skip:46  
time:489s
fi-ivb-3520m total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:520s
fi-ivb-3770  total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:480s
fi-kbl-7500u total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:456s
fi-kbl-7560u total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:566s
fi-kbl-r total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:575s
fi-pnv-d510  total:279  pass:223  dwarn:1   dfail:0   fail:0   skip:55  
time:573s
fi-skl-6260u total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:450s
fi-skl-6600u total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:576s
fi-skl-6700hqtotal:279  pass:262  dwarn:0   dfail:0   fail:0   skip:17  
time:578s
fi-skl-6700k total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:643s
fi-skl-6770hqtotal:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:465s
fi-skl-gvtdvmtotal:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  
time:428s
fi-skl-x1585ltotal:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:490s
fi-snb-2520m total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  
time:549s
fi-snb-2600  total:279  pass:250  dwarn:0   dfail:0   fail:0   skip:29  
time:404s

2a33b48692c55d9145a60527716b7d066815d377 drm-tip: 2017y-08m-04d-13h-34m-53s UTC 
integration manifest
6d15be85d8aa drm/i915: remove unused function declaration

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5328/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH i-g-t] lib/igt_chamelium: Remove any special handling for the connector FSM

[Paul Kocialkowski]

No specific treatment should be required for handling the connector FSM,
since the chamelium-side daemon will automatically send an HPD event to
reset the source.

The event is sufficient to make the receiver on the chamelium consider
the input as stable after it. On the other hand, toggling DPMS was found
to sometimes confuse the receiver so that it does not consider the input
as stable at any point. Doing nothing special instead works in all cases.

This issue can be witnessed with i915 when drm debugging is enabled and
leads to DP frame-related tests failing on the test farm.

Signed-off-by: Paul Kocialkowski 
---
 lib/igt_chamelium.c | 113 
 1 file changed, 25 insertions(+), 88 deletions(-)

diff --git a/lib/igt_chamelium.c b/lib/igt_chamelium.c
index dcd8855f..4cea5fdb 100644
--- a/lib/igt_chamelium.c
+++ b/lib/igt_chamelium.c
@@ -209,57 +209,13 @@ void chamelium_destroy_frame_dump(struct 
chamelium_frame_dump *dump)
free(dump);
 }
 
-struct fsm_monitor_args {
-   struct chamelium *chamelium;
-   struct chamelium_port *port;
-   struct udev_monitor *mon;
-};
-
-/*
- * Whenever resolutions or other factors change with the display output, the
- * Chamelium's display receivers need to be fully reset in order to perform any
- * frame-capturing related tasks. This requires cutting off the display then
- * turning it back on, and is indicated by the Chamelium sending hotplug events
- */
-static void *chamelium_fsm_mon(void *data)
-{
-   struct fsm_monitor_args *args = data;
-   drmModeConnector *connector;
-   int drm_fd = args->chamelium->drm_fd;
-
-   /*
-* Wait for the chamelium to try unplugging the connector, otherwise
-* the thread calling chamelium_rpc will kill us
-*/
-   igt_hotplug_detected(args->mon, 60);
-
-   /*
-* Just in case the RPC call being executed returns before we complete
-* the FSM modesetting sequence, so we don't leave the display in a bad
-* state.
-*/
-   pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, NULL);
-
-   igt_debug("Chamelium needs FSM, handling\n");
-   connector = chamelium_port_get_connector(args->chamelium, args->port,
-false);
-   kmstest_set_connector_dpms(drm_fd, connector, DRM_MODE_DPMS_OFF);
-   kmstest_set_connector_dpms(drm_fd, connector, DRM_MODE_DPMS_ON);
-
-   drmModeFreeConnector(connector);
-   return NULL;
-}
-
 static xmlrpc_value *chamelium_rpc(struct chamelium *chamelium,
-  struct chamelium_port *fsm_port,
   const char *method_name,
   const char *format_str,
   ...)
 {
xmlrpc_value *res;
va_list va_args;
-   struct fsm_monitor_args monitor_args;
-   pthread_t fsm_thread_id;
 
/* Cleanup the last error, if any */
if (chamelium->env.fault_occurred) {
@@ -267,31 +223,12 @@ static xmlrpc_value *chamelium_rpc(struct chamelium 
*chamelium,
xmlrpc_env_init(>env);
}
 
-   /* Unfortunately xmlrpc_client's event loop helpers are rather useless
-* for implementing any sort of event loop, since they provide no way
-* to poll for events other then the RPC response. This means in order
-* to handle the chamelium attempting FSM, we have to fork into another
-* thread and have that handle hotplugging displays
-*/
-   if (fsm_port) {
-   monitor_args.chamelium = chamelium;
-   monitor_args.port = fsm_port;
-   monitor_args.mon = igt_watch_hotplug();
-   pthread_create(_thread_id, NULL, chamelium_fsm_mon,
-  _args);
-   }
-
va_start(va_args, format_str);
xmlrpc_client_call2f_va(>env, chamelium->client,
chamelium->url, method_name, format_str, ,
va_args);
va_end(va_args);
 
-   if (fsm_port) {
-   pthread_cancel(fsm_thread_id);
-   igt_cleanup_hotplug(monitor_args.mon);
-   }
-
igt_assert_f(!chamelium->env.fault_occurred,
 "Chamelium RPC call failed: %s\n",
 chamelium->env.fault_string);
@@ -310,7 +247,7 @@ static xmlrpc_value *chamelium_rpc(struct chamelium 
*chamelium,
 void chamelium_plug(struct chamelium *chamelium, struct chamelium_port *port)
 {
igt_debug("Plugging %s\n", port->name);
-   xmlrpc_DECREF(chamelium_rpc(chamelium, NULL, "Plug", "(i)", port->id));
+   xmlrpc_DECREF(chamelium_rpc(chamelium, "Plug", "(i)", port->id));
 }
 
 /**
@@ -324,7 +261,7 @@ void chamelium_plug(struct chamelium *chamelium, struct 
chamelium_port *port)
 void chamelium_unplug(struct chamelium *chamelium, struct chamelium_port 

Re: [Intel-gfx] [PATCH igt] tests/kms_properties: Don't set immutable properties

[Daniel Stone]

On 4 August 2017 at 14:38, Daniel Stone  wrote:
> If the kernel tells us it's immutable, trying to set it probably isn't
> going to succeed.
>
> Fixes a failure seen with the IN_FORMATS property.

Pushed a v2 which removed most of the list with Daniel Vetter's R-b from IRC.

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

[Intel-gfx] [PATCH] drm/i915: remove unused function declaration

[Lionel Landwerlin]

This function is not part of the driver anymore.

Signed-off-by: Lionel Landwerlin 
---
 drivers/gpu/drm/i915/intel_lrc.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
index d02a96a011cf..4ef6a6143f5d 100644
--- a/drivers/gpu/drm/i915/intel_lrc.h
+++ b/drivers/gpu/drm/i915/intel_lrc.h
@@ -63,7 +63,6 @@ enum {
 };
 
 /* Logical Rings */
-void intel_logical_ring_stop(struct intel_engine_cs *engine);
 void intel_logical_ring_cleanup(struct intel_engine_cs *engine);
 int logical_render_ring_init(struct intel_engine_cs *engine);
 int logical_xcs_ring_init(struct intel_engine_cs *engine);
-- 
2.13.3

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

[Intel-gfx] [PATCH v2 i-g-t] tests/perf: fix build where system headers don't have Gen8 formats

[Lionel Landwerlin]

v2: Use previous enum to define the new Gen8 enums (Petri)

Signed-off-by: Lionel Landwerlin 
---
 tests/perf.c | 13 ++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/tests/perf.c b/tests/perf.c
index f0ec26dd..23f7f1af 100644
--- a/tests/perf.c
+++ b/tests/perf.c
@@ -143,6 +143,13 @@ enum drm_i915_perf_record_type {
 };
 #endif /* !DRM_I915_PERF_OPEN */

+/* There is no ifdef we can use for those formats :( */
+enum {
+   local_I915_OA_FORMAT_A12 = I915_OA_FORMAT_C4_B8 + 1,
+   local_I915_OA_FORMAT_A12_B8_C8 = I915_OA_FORMAT_A12 + 2,
+   local_I915_OA_FORMAT_A32u40_A4u32_B8_C8 = I915_OA_FORMAT_C4_B8 + 3,
+};
+
 #ifndef DRM_IOCTL_I915_PERF_ADD_CONFIG

 #define DRM_I915_PERF_ADD_CONFIG   0x37
@@ -230,17 +237,17 @@ static struct {

/* Gen8+ */

-   [I915_OA_FORMAT_A12] = {
+   [local_I915_OA_FORMAT_A12] = {
"A12", .size = 64,
.a_off = 12, .n_a = 12, .first_a = 7,
.min_gen = 8 },
-   [I915_OA_FORMAT_A12_B8_C8] = {
+   [local_I915_OA_FORMAT_A12_B8_C8] = {
"A12_B8_C8", .size = 128,
.a_off = 12, .n_a = 12,
.b_off = 64, .n_b = 8,
.c_off = 96, .n_c = 8, .first_a = 7,
.min_gen = 8 },
-   [I915_OA_FORMAT_A32u40_A4u32_B8_C8] = {
+   [local_I915_OA_FORMAT_A32u40_A4u32_B8_C8] = {
"A32u40_A4u32_B8_C8", .size = 256,
.a40_high_off = 160, .a40_low_off = 16, .n_a40 = 32,
.a_off = 144, .n_a = 4, .first_a = 32,
--
2.13.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH igt] tests/kms_properties: Don't set immutable properties

[Daniel Stone]

If the kernel tells us it's immutable, trying to set it probably isn't
going to succeed.

Fixes a failure seen with the IN_FORMATS property.

Signed-off-by: Daniel Stone 
Cc: Daniel Vetter 
Cc: Ben Widawsky 
---
 tests/kms_properties.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/tests/kms_properties.c b/tests/kms_properties.c
index 276c07be..23d8a89f 100644
--- a/tests/kms_properties.c
+++ b/tests/kms_properties.c
@@ -88,9 +88,13 @@ static bool ignore_plane_property(const char *name, bool 
atomic)
return false;
 }
 
-static bool ignore_property(uint32_t type, const char *name, bool atomic)
+static bool ignore_property(uint32_t obj_type, uint32_t prop_flags,
+   const char *name, bool atomic)
 {
-   switch (type) {
+   if (prop_flags & DRM_MODE_PROP_IMMUTABLE)
+   return true;
+
+   switch (obj_type) {
case DRM_MODE_OBJECT_CRTC:
return ignore_crtc_property(name, atomic);
case DRM_MODE_OBJECT_CONNECTOR:
@@ -122,7 +126,7 @@ static void test_properties(int fd, uint32_t type, uint32_t 
id, bool atomic)
 
igt_assert(prop);
 
-   if (ignore_property(type, prop->name, atomic)) {
+   if (ignore_property(type, prop->flags, prop->name, atomic)) {
igt_debug("Ignoring property \"%s\"\n", prop->name);
 
continue;
-- 
2.13.4

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

Re: [Intel-gfx] [PATCH i-g-t 03/11] tests/perf: update max buffer size for reading reports

[Lionel Landwerlin]

On 04/08/17 12:41, Chris Wilson wrote:

Quoting Lionel Landwerlin (2017-08-04 12:20:32)

Signed-off-by: Lionel Landwerlin 
---
  tests/perf.c | 8 
  1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/perf.c b/tests/perf.c
index 279ff0c6..65a1606d 100644
--- a/tests/perf.c
+++ b/tests/perf.c
@@ -1271,9 +1271,7 @@ read_2_oa_reports(int format_id,
 /* Note: we allocate a large buffer so that each read() iteration
  * should scrape *all* pending records.
  *
-* The largest buffer the OA unit supports is 16MB and the smallest
-* OA report format is 64bytes allowing up to 262144 reports to
-* be buffered.
+* The largest buffer the OA unit supports is 16MB.

Out of curiosity, how is userspace meant to know? Or is it part of the
platform specific details that we spread around kernel/userspace?
-Chris


The current implementation always uses the largest buffer size (16Mb).
Some of our tests verify that correct behavior at the limits (like 
overflow event & correct recovery after disable/enable).


We could make that information available, but I'm not sure it's going to 
be that useful because context-switch reports will prevent estimation on 
how much the buffer gets filled over time. The behavior from userspace 
should be to use poll for monitoring when data is available and read it 
as often as it's made available.


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

Re: [Intel-gfx] [PATCH i-g-t] tests/perf: fix build where system headers don't have Gen8 formats

[Petri Latvala]

On Fri, Aug 04, 2017 at 02:21:36PM +0100, Lionel Landwerlin wrote:
> Signed-off-by: Lionel Landwerlin 
> ---
>  tests/perf.c | 13 ++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/perf.c b/tests/perf.c
> index f0ec26dd..aa6358d3 100644
> --- a/tests/perf.c
> +++ b/tests/perf.c
> @@ -143,6 +143,13 @@ enum drm_i915_perf_record_type {
>  };
>  #endif /* !DRM_I915_PERF_OPEN */
>  
> +/* There is no ifdef we can use for those formats :( */
> +enum {
> + local_I915_OA_FORMAT_A12 = I915_OA_FORMAT_A12,
> + local_I915_OA_FORMAT_A12_B8_C8 = I915_OA_FORMAT_A12_B8_C8,
> + local_I915_OA_FORMAT_A32u40_A4u32_B8_C8 = 
> I915_OA_FORMAT_A32u40_A4u32_B8_C8,
> +};
> +


The whole point is that we don't have I915_OA_FORMAT_A12, you can't
use that for the value of local_I915_OA_FORMAT_A12, etc. You need to use the 
number.


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

[Intel-gfx] [PATCH i-g-t] tests/perf: fix build where system headers don't have Gen8 formats

[Lionel Landwerlin]

Signed-off-by: Lionel Landwerlin 
---
 tests/perf.c | 13 ++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/tests/perf.c b/tests/perf.c
index f0ec26dd..aa6358d3 100644
--- a/tests/perf.c
+++ b/tests/perf.c
@@ -143,6 +143,13 @@ enum drm_i915_perf_record_type {
 };
 #endif /* !DRM_I915_PERF_OPEN */
 
+/* There is no ifdef we can use for those formats :( */
+enum {
+   local_I915_OA_FORMAT_A12 = I915_OA_FORMAT_A12,
+   local_I915_OA_FORMAT_A12_B8_C8 = I915_OA_FORMAT_A12_B8_C8,
+   local_I915_OA_FORMAT_A32u40_A4u32_B8_C8 = 
I915_OA_FORMAT_A32u40_A4u32_B8_C8,
+};
+
 #ifndef DRM_IOCTL_I915_PERF_ADD_CONFIG
 
 #define DRM_I915_PERF_ADD_CONFIG   0x37
@@ -230,17 +237,17 @@ static struct {
 
/* Gen8+ */
 
-   [I915_OA_FORMAT_A12] = {
+   [local_I915_OA_FORMAT_A12] = {
"A12", .size = 64,
.a_off = 12, .n_a = 12, .first_a = 7,
.min_gen = 8 },
-   [I915_OA_FORMAT_A12_B8_C8] = {
+   [local_I915_OA_FORMAT_A12_B8_C8] = {
"A12_B8_C8", .size = 128,
.a_off = 12, .n_a = 12,
.b_off = 64, .n_b = 8,
.c_off = 96, .n_c = 8, .first_a = 7,
.min_gen = 8 },
-   [I915_OA_FORMAT_A32u40_A4u32_B8_C8] = {
+   [local_I915_OA_FORMAT_A32u40_A4u32_B8_C8] = {
"A32u40_A4u32_B8_C8", .size = 256,
.a40_high_off = 160, .a40_low_off = 16, .n_a40 = 32,
.a_off = 144, .n_a = 4, .first_a = 32,
-- 
2.13.3

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

[Intel-gfx] [PATCH libdrm] headers: update drm_i915.h

[Lionel Landwerlin]

Generated at the following commit from drm-next :

commit dd24df657075fdf1e850612ea50634816f3c3581
Merge: 12f8030e05c6 799c7b20b260
Author: Dave Airlie 
Date:   Wed Aug 2 12:43:12 2017 +1000

Merge branch 'drm-next-4.14' of git://people.freedesktop.org/~agd5f/linux 
into drm-next

Signed-off-by: Lionel Landwerlin 
---
 include/drm/i915_drm.h | 61 ++
 1 file changed, 52 insertions(+), 9 deletions(-)

diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h
index 5ebe0462..c26bf7c1 100644
--- a/include/drm/i915_drm.h
+++ b/include/drm/i915_drm.h
@@ -412,6 +412,25 @@ typedef struct drm_i915_irq_wait {
  */
 #define I915_PARAM_HAS_EXEC_FENCE   44
 
+/* Query whether DRM_I915_GEM_EXECBUFFER2 supports the ability to capture
+ * user specified bufffers for post-mortem debugging of GPU hangs. See
+ * EXEC_OBJECT_CAPTURE.
+ */
+#define I915_PARAM_HAS_EXEC_CAPTURE 45
+
+#define I915_PARAM_SLICE_MASK   46
+
+/* Assuming it's uniform for each slice, this queries the mask of subslices
+ * per-slice for this system.
+ */
+#define I915_PARAM_SUBSLICE_MASK47
+
+/*
+ * Query whether DRM_I915_GEM_EXECBUFFER2 supports supplying the batch buffer
+ * as the first execobject as opposed to the last. See I915_EXEC_BATCH_FIRST.
+ */
+#define I915_PARAM_HAS_EXEC_BATCH_FIRST 48
+
 typedef struct drm_i915_getparam {
__s32 param;
/*
@@ -666,6 +685,8 @@ struct drm_i915_gem_relocation_entry {
 #define I915_GEM_DOMAIN_VERTEX 0x0020
 /** GTT domain - aperture and scanout */
 #define I915_GEM_DOMAIN_GTT0x0040
+/** WC domain - uncached access */
+#define I915_GEM_DOMAIN_WC 0x0080
 /** @} */
 
 struct drm_i915_gem_exec_object {
@@ -773,8 +794,15 @@ struct drm_i915_gem_exec_object2 {
  * I915_PARAM_HAS_EXEC_FENCE to order execbufs and execute them asynchronously.
  */
 #define EXEC_OBJECT_ASYNC  (1<<6)
+/* Request that the contents of this execobject be copied into the error
+ * state upon a GPU hang involving this batch for post-mortem debugging.
+ * These buffers are recorded in no particular order as "user" in
+ * /sys/class/drm/cardN/error. Query I915_PARAM_HAS_EXEC_CAPTURE to see
+ * if the kernel supports this flag.
+ */
+#define EXEC_OBJECT_CAPTURE(1<<7)
 /* All remaining bits are MBZ and RESERVED FOR FUTURE USE */
-#define __EXEC_OBJECT_UNKNOWN_FLAGS -(EXEC_OBJECT_ASYNC<<1)
+#define __EXEC_OBJECT_UNKNOWN_FLAGS -(EXEC_OBJECT_CAPTURE<<1)
__u64 flags;
 
union {
@@ -889,7 +917,17 @@ struct drm_i915_gem_execbuffer2 {
  */
 #define I915_EXEC_FENCE_OUT(1<<17)
 
-#define __I915_EXEC_UNKNOWN_FLAGS (-(I915_EXEC_FENCE_OUT<<1))
+/*
+ * Traditionally the execbuf ioctl has only considered the final element in
+ * the execobject[] to be the executable batch. Often though, the client
+ * will known the batch object prior to construction and being able to place
+ * it into the execobject[] array first can simplify the relocation tracking.
+ * Setting I915_EXEC_BATCH_FIRST tells execbuf to use element 0 of the
+ * execobject[] as the * batch instead (the default is to use the last
+ * element).
+ */
+#define I915_EXEC_BATCH_FIRST  (1<<18)
+#define __I915_EXEC_UNKNOWN_FLAGS (-(I915_EXEC_BATCH_FIRST<<1))
 
 #define I915_EXEC_CONTEXT_ID_MASK  (0x)
 #define i915_execbuffer2_set_context_id(eb2, context) \
@@ -1293,13 +1331,18 @@ struct drm_i915_gem_context_param {
 };
 
 enum drm_i915_oa_format {
-   I915_OA_FORMAT_A13 = 1,
-   I915_OA_FORMAT_A29,
-   I915_OA_FORMAT_A13_B8_C8,
-   I915_OA_FORMAT_B4_C8,
-   I915_OA_FORMAT_A45_B8_C8,
-   I915_OA_FORMAT_B4_C8_A16,
-   I915_OA_FORMAT_C4_B8,
+   I915_OA_FORMAT_A13 = 1, /* HSW only */
+   I915_OA_FORMAT_A29, /* HSW only */
+   I915_OA_FORMAT_A13_B8_C8,   /* HSW only */
+   I915_OA_FORMAT_B4_C8,   /* HSW only */
+   I915_OA_FORMAT_A45_B8_C8,   /* HSW only */
+   I915_OA_FORMAT_B4_C8_A16,   /* HSW only */
+   I915_OA_FORMAT_C4_B8,   /* HSW+ */
+
+   /* Gen8+ */
+   I915_OA_FORMAT_A12,
+   I915_OA_FORMAT_A12_B8_C8,
+   I915_OA_FORMAT_A32u40_A4u32_B8_C8,
 
I915_OA_FORMAT_MAX  /* non-ABI */
 };
-- 
2.13.3

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

Re: [Intel-gfx] [PATCH i-g-t 06/11] tests/perf: make enable-disable more reliable

[Lionel Landwerlin]

On 04/08/17 12:44, Chris Wilson wrote:

Quoting Lionel Landwerlin (2017-08-04 12:20:35)

Estimation of the amount of reports can only refer to periodic ones,
as context switch reports completely depend on what happens on the
system. Also generate some load to prevent clock frequency changes to
impact our measurement.

If clock frequency is a fundamental invariant required for the test,
set the clock frequency via sysfs:
/sys/class/drm/card0/gt_(min|max|boost)_freq_mhz
-Chris




I haven't tried setting the boost value, but settings min to the 
maximum the device can do didn't seem to make any difference.
Reading the current frequency file would sometimes report values 200Mhz 
below what was put in the min...

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

Re: [Intel-gfx] [intel-gfx][i-g-t][help]can't read the backlight

[Jani Nikula]

On Wed, 26 Jul 2017, "Gu, HailinX"  wrote:
> Hi~ all,
> I try to run the test pm_backlight. But there is an skip with the log as 
> following.
>
>
>
> 
>
> IGT-Version: 1.19-g4258cc8e (x86_64) (Linux: 4.10.0 x86_64) Test
>
> requirement not met in function __real_main154, file pm_backlight.c:163:
>
> Test requirement: !(backlight_read(, "brightness")) Last errno: 2,
>
> No such file or directory
> 
>
> By my check there is not a file in dir /sys/class/backlight/
>
> I wander is there any kconfig or boot parameters need to set?
> I have tried adding "acpi_ osi=Linux acpi_backlight=vendor" to GRUB config, 
> but it's same as before.

Please post the dmesg, with drm.debug=14, and kernel config.

BR,
Jani.

-- 
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: fix backlight invert for non-zero minimum brightness

[Jani Nikula]

On Wed, 31 May 2017, Daniel Vetter  wrote:
> On Wed, May 31, 2017 at 11:33:55AM +0300, Jani Nikula wrote:
>> When we started following the backlight minimum brightness in
>> 6dda730e55f4 ("drm/i915: respect the VBT minimum backlight brightness")
>> we overlooked the brightness invert quirk. Even if we invert the
>> brightness, we need to take the min limit into account. We probably
>> missed this because the invert has only been required on gen4 for proper
>> operation.
>> 
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101127
>> Fixes: 6dda730e55f4 ("drm/i915: respect the VBT minimum backlight 
>> brightness")
>> Cc: Daniel Vetter 
>> Cc:  # v3.17+
>
> Not sure the cc: stable is justified, but makes sense otherwise
>
> Reviewed-by: Daniel Vetter 

Had forgotten about this, pushed now, without cc: stable.

Thanks for the review.

BR,
Jani.


>
>> Signed-off-by: Jani Nikula 
>> ---
>>  drivers/gpu/drm/i915/intel_panel.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/intel_panel.c 
>> b/drivers/gpu/drm/i915/intel_panel.c
>> index cb50c527401f..bcde9f34527a 100644
>> --- a/drivers/gpu/drm/i915/intel_panel.c
>> +++ b/drivers/gpu/drm/i915/intel_panel.c
>> @@ -469,7 +469,7 @@ static u32 intel_panel_compute_brightness(struct 
>> intel_connector *connector,
>>  
>>  if (i915.invert_brightness > 0 ||
>>  dev_priv->quirks & QUIRK_INVERT_BRIGHTNESS) {
>> -return panel->backlight.max - val;
>> +return panel->backlight.max - val + panel->backlight.min;
>>  }
>>  
>>  return val;
>> -- 
>> 2.11.0
>> 

-- 
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] New " Unclaimed read from register 0x1f0034" in 4.12-rc1

[Jani Nikula]

On Thu, 18 May 2017, Hans de Goede  wrote:
> Ok, so after rebuilding everything I get more sensible results,
> the "RPM wakelock ref not held during HW access" error now is
> the same one as I reported before in the
> "New "RPM wakelock ref not held during HW access" in 4.12-rc1 ?"
> thread. Now that I can reproduce it again, I will try the fixes
> suggested there.
>
> The "Unclaimed read from register 0x1f0034" error now has this
> stacktrace:

Replying to an old mail, the bug report is
https://bugs.freedesktop.org/show_bug.cgi?id=101705

BR,
Jani.


>
> [   56.955016] PM: early resume of devices complete after 1068.631 msecs
> [   56.956396] pcieport :00:1c.0: System wakeup disabled by ACPI
> [   56.957697] rtc_cmos 00:04: System wakeup disabled by ACPI
> [   56.958261] Suspended for 0.888 seconds
> [   56.964250] Unclaimed read from register 0x1f0034
> [   56.964347] [ cut here ]
> [   56.964428] WARNING: CPU: 2 PID: 1799 at 
> drivers/gpu/drm/i915/intel_uncore.c:801 __unclaimed_reg_debug+0x4e/0x60 [i915]
> ...
> [   56.964590] Workqueue: events_unbound async_run_entry_fn
> [   56.964594] task: 89ad698b task.stack: 9ba401454000
> [   56.964662] RIP: 0010:__unclaimed_reg_debug+0x4e/0x60 [i915]
> ...
> [   56.964688] Call Trace:
> [   56.964758]  fwtable_read32+0x17a/0x1d0 [i915]
> [   56.964818]  vlv_program_watermarks+0x3c4/0x610 [i915]
> [   56.964883]  ? intel_hdmi_get_hw_state+0x27/0xd0 [i915]
> [   56.964941]  vlv_optimize_watermarks+0x95/0xb0 [i915]
> [   56.965008]  intel_atomic_commit_tail+0x2e2/0x1030 [i915]
> [   56.965016]  ? tracing_record_cmdline+0x32/0x120
> [   56.965022]  ? __schedule+0x23e/0x860
> [   56.965090]  intel_atomic_commit+0x399/0x4b0 [i915]
> [   56.965125]  ? drm_atomic_check_only+0x37f/0x540 [drm]
> [   56.965155]  drm_atomic_commit+0x4b/0x50 [drm]
> [   56.965174]  drm_atomic_helper_commit_duplicated_state+0xc2/0xd0 
> [drm_kms_helper]
> [   56.965245]  __intel_display_resume+0x85/0xc0 [i915]
> [   56.965312]  intel_display_resume+0xf7/0x120 [i915]
> [   56.965370]  i915_drm_resume+0xe1/0x180 [i915]
> [   56.965427]  i915_pm_resume+0x1e/0x30 [i915]
> [   56.965434]  pci_pm_resume+0x65/0xa0
> [   56.965440]  dpm_run_callback+0x57/0x140
> [   56.965444]  ? pci_pm_thaw+0x90/0x90
> [   56.965447]  device_resume+0xe1/0x200
> [   56.965450]  async_resume+0x1d/0x50
> [   56.965455]  async_run_entry_fn+0x39/0x170
> [   56.965460]  process_one_work+0x193/0x3c0
> ...
> [   57.062736] PM: resume of devices complete after 107.709 msecs
> [   57.063468] PM: resume devices took 0.108 seconds
> [   57.063478] PM: Finishing wakeup.
>
> Regards,
>
> Hans
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] ✗ Fi.CI.BAT: warning for drm/i915/perf: Initialise the dynamic sysfs attr

[Chris Wilson]

Quoting Patchwork (2017-08-03 23:57:29)
> == Series Details ==
> 
> Series: drm/i915/perf: Initialise the dynamic sysfs attr
> URL   : https://patchwork.freedesktop.org/series/28340/
> State : warning
> 
> == Summary ==
> 
> Series 28340v1 drm/i915/perf: Initialise the dynamic sysfs attr
> https://patchwork.freedesktop.org/api/1.0/series/28340/revisions/1/mbox/
> 
> Test gem_exec_flush:
> Subgroup basic-batch-kernel-default-uc:
> fail   -> PASS   (fi-snb-2600) fdo#17
> Test gem_exec_suspend:
> Subgroup basic-s4-devices:
> pass   -> DMESG-WARN (fi-kbl-7500u)
> Test kms_force_connector_basic:
> Subgroup prune-stale-modes:
> pass   -> SKIP   (fi-ivb-3520m) fdo#101048
> Test kms_pipe_crc_basic:
> Subgroup hang-read-crc-pipe-a:
> dmesg-warn -> PASS   (fi-pnv-d510) fdo#101597
> Subgroup nonblocking-crc-pipe-b:
> incomplete -> PASS   (fi-bxt-j4205)
> Subgroup suspend-read-crc-pipe-b:
> dmesg-warn -> PASS   (fi-byt-n2820) fdo#101705
> 
> fdo#17 https://bugs.freedesktop.org/show_bug.cgi?id=17
> fdo#101048 https://bugs.freedesktop.org/show_bug.cgi?id=101048
> fdo#101597 https://bugs.freedesktop.org/show_bug.cgi?id=101597
> fdo#101705 https://bugs.freedesktop.org/show_bug.cgi?id=101705

And pushed, thanks for the review. Ideas welcome to improve CI detection
of these errors.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/shrinker: Wrap need_resched() inside preempt-disable

[Chris Wilson]

Quoting Joonas Lahtinen (2017-08-04 12:52:51)
> On pe, 2017-08-04 at 11:41 +0100, Chris Wilson wrote:
> > In order for us to successfully detect the end of a timeslice,
> > preemption must be disabled. Otherwise, inside the loop we may be
> > preempted many times without our noticing, and each time our timeslice
> > will be reset, invalidating need_resched()
> > 
> > Reported-by: Joonas Lahtinen 
> > Reported-by: Tomi Sarvela 
> > Fixes: 290271de34f6 ("drm/i915: Spin for struct_mutex inside shrinker")
> > Signed-off-by: Chris Wilson 
> > Cc: Mika Kuoppala 
> > Cc: Joonas Lahtinen 
> > Cc:  # v4.13-rc1+
> 
> Gets rid of the hang for my BDW.
> 
> Tested-by: Joonas Lahtinen 
> Reviewed-by: Joonas Lahtinen 

Thanks for the overnight testing, pushed.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t] kms_busy: Fix basic-modeset-* name format parameters

[Lofstedt, Marta]

Reviewed-by: Marta Lofstedt 

> -Original Message-
> From: Lofstedt, Marta
> Sent: Friday, August 4, 2017 3:05 PM
> To: 'Petri Latvala' ; intel-gfx@lists.freedesktop.org
> Cc: Daniel Vetter 
> Subject: RE: [Intel-gfx] [PATCH i-g-t] kms_busy: Fix basic-modeset-* name
> format parameters
> 
> Thanks Petri, now I can run testlists again!
> 
> > -Original Message-
> > From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On
> > Behalf Of Petri Latvala
> > Sent: Friday, August 4, 2017 2:45 PM
> > To: intel-gfx@lists.freedesktop.org
> > Cc: Daniel Vetter 
> > Subject: [Intel-gfx] [PATCH i-g-t] kms_busy: Fix basic-modeset-* name
> > format parameters
> >
> > Commit 37b06eb9b526df6c23ec75f7a9ecd9547fa76695 limited the used
> > engines to only the default engine, dropping the engine name from
> > subtest names, but left over the format parameter.
> >
> > Fixes: 37b06eb9b526 ("tests/kms_busy: Only test against one engine")
> > Signed-off-by: Petri Latvala 
> > CC: Daniel Vetter 
> > ---
> >  tests/kms_busy.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tests/kms_busy.c b/tests/kms_busy.c index
> > 16ab891..dc00214
> > 100644
> > --- a/tests/kms_busy.c
> > +++ b/tests/kms_busy.c
> > @@ -342,7 +342,7 @@ igt_main
> > test_flip(, e->exec_id |
> > e->flags, n, false);
> > }
> > igt_subtest_f("basic-modeset-%s",
> > - e->name,
> > kmstest_pipe_name(n)) {
> > + kmstest_pipe_name(n)) {
> >
> > igt_require(gem_has_ring(display.drm_fd,
> >
> > e->exec_id | e->flags));
> >
> > --
> > 2.9.3
> >
> > ___
> > 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

Re: [Intel-gfx] [PATCH i-g-t] kms_busy: Fix basic-modeset-* name format parameters

[Lofstedt, Marta]

Thanks Petri, now I can run testlists again!

> -Original Message-
> From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf
> Of Petri Latvala
> Sent: Friday, August 4, 2017 2:45 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: Daniel Vetter 
> Subject: [Intel-gfx] [PATCH i-g-t] kms_busy: Fix basic-modeset-* name
> format parameters
> 
> Commit 37b06eb9b526df6c23ec75f7a9ecd9547fa76695 limited the used
> engines to only the default engine, dropping the engine name from subtest
> names, but left over the format parameter.
> 
> Fixes: 37b06eb9b526 ("tests/kms_busy: Only test against one engine")
> Signed-off-by: Petri Latvala 
> CC: Daniel Vetter 
> ---
>  tests/kms_busy.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/kms_busy.c b/tests/kms_busy.c index 16ab891..dc00214
> 100644
> --- a/tests/kms_busy.c
> +++ b/tests/kms_busy.c
> @@ -342,7 +342,7 @@ igt_main
>   test_flip(, e->exec_id |
> e->flags, n, false);
>   }
>   igt_subtest_f("basic-modeset-%s",
> -   e->name,
> kmstest_pipe_name(n)) {
> +   kmstest_pipe_name(n)) {
> 
>   igt_require(gem_has_ring(display.drm_fd,
> 
>   e->exec_id | e->flags));
> 
> --
> 2.9.3
> 
> ___
> 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

Re: [Intel-gfx] [PATCH i-g-t 00/11] Improve robustness of the i915 perf tests

[Petri Latvala]

On Fri, Aug 04, 2017 at 12:20:29PM +0100, Lionel Landwerlin wrote:
> Hi all,
> 
> This is a resend of a series that was posted a few months ago. I've
> pushed all the patches that has a Rb. This is a shorter list but
> pretty important to make the tests more stable.


perf.c:233:3: error: ‘I915_OA_FORMAT_A12’ undeclared here (not in a function)
  [I915_OA_FORMAT_A12] = {
   ^~
perf.c:233:3: error: array index in initializer not of integer type
perf.c:233:3: note: (near initialization for ‘oa_formats’)
perf.c:233:25: warning: excess elements in array initializer
  [I915_OA_FORMAT_A12] = {
   ^
perf.c:233:25: note: (near initialization for ‘oa_formats’)
perf.c:237:3: error: ‘I915_OA_FORMAT_A12_B8_C8’ undeclared here (not in a 
function)
  [I915_OA_FORMAT_A12_B8_C8] = {
   ^~~~
perf.c:237:3: error: array index in initializer not of integer type
perf.c:237:3: note: (near initialization for ‘oa_formats’)
perf.c:237:31: warning: excess elements in array initializer
  [I915_OA_FORMAT_A12_B8_C8] = {
   ^
perf.c:237:31: note: (near initialization for ‘oa_formats’)
perf.c:243:3: error: ‘I915_OA_FORMAT_A32u40_A4u32_B8_C8’ undeclared here (not 
in a function)
  [I915_OA_FORMAT_A32u40_A4u32_B8_C8] = {
   ^


Those tokens have local definitions but only if DRM_I915_PERF_OPEN is
not present. For older (iow current) libdrm with PERF_OPEN but without
those new format tokens, we get this build error. Please fix asap.


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

Re: [Intel-gfx] [PATCH] drm/i915/shrinker: Wrap need_resched() inside preempt-disable

[Joonas Lahtinen]

On pe, 2017-08-04 at 11:41 +0100, Chris Wilson wrote:
> In order for us to successfully detect the end of a timeslice,
> preemption must be disabled. Otherwise, inside the loop we may be
> preempted many times without our noticing, and each time our timeslice
> will be reset, invalidating need_resched()
> 
> Reported-by: Joonas Lahtinen 
> Reported-by: Tomi Sarvela 
> Fixes: 290271de34f6 ("drm/i915: Spin for struct_mutex inside shrinker")
> Signed-off-by: Chris Wilson 
> Cc: Mika Kuoppala 
> Cc: Joonas Lahtinen 
> Cc:  # v4.13-rc1+

Gets rid of the hang for my BDW.

Tested-by: Joonas Lahtinen 
Reviewed-by: Joonas Lahtinen 

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t 09/11] tests/perf: remove unused frequency functions

[Chris Wilson]

Quoting Lionel Landwerlin (2017-08-04 12:20:38)
> Now that we've found that frequency changes happen mostly outside of
> our control and don't seem to be following our requests through sysfs,
> let's drop a bunch code/variables.

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

[Intel-gfx] [PATCH i-g-t] kms_busy: Fix basic-modeset-* name format parameters

[Petri Latvala]

Commit 37b06eb9b526df6c23ec75f7a9ecd9547fa76695 limited the used
engines to only the default engine, dropping the engine name from
subtest names, but left over the format parameter.

Fixes: 37b06eb9b526 ("tests/kms_busy: Only test against one engine")
Signed-off-by: Petri Latvala 
CC: Daniel Vetter 
---
 tests/kms_busy.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/kms_busy.c b/tests/kms_busy.c
index 16ab891..dc00214 100644
--- a/tests/kms_busy.c
+++ b/tests/kms_busy.c
@@ -342,7 +342,7 @@ igt_main
test_flip(, e->exec_id | e->flags, n, false);
}
igt_subtest_f("basic-modeset-%s",
- e->name, kmstest_pipe_name(n)) {
+ kmstest_pipe_name(n)) {
igt_require(gem_has_ring(display.drm_fd,
e->exec_id | e->flags));
 
-- 
2.9.3

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

Re: [Intel-gfx] [PATCH i-g-t 06/11] tests/perf: make enable-disable more reliable

[Chris Wilson]

Quoting Lionel Landwerlin (2017-08-04 12:20:35)
> Estimation of the amount of reports can only refer to periodic ones,
> as context switch reports completely depend on what happens on the
> system. Also generate some load to prevent clock frequency changes to
> impact our measurement.

If clock frequency is a fundamental invariant required for the test,
set the clock frequency via sysfs:
/sys/class/drm/card0/gt_(min|max|boost)_freq_mhz
-Chris

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

Re: [Intel-gfx] [PATCH i-g-t 03/11] tests/perf: update max buffer size for reading reports

[Chris Wilson]

Quoting Lionel Landwerlin (2017-08-04 12:20:32)
> Signed-off-by: Lionel Landwerlin 
> ---
>  tests/perf.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/perf.c b/tests/perf.c
> index 279ff0c6..65a1606d 100644
> --- a/tests/perf.c
> +++ b/tests/perf.c
> @@ -1271,9 +1271,7 @@ read_2_oa_reports(int format_id,
> /* Note: we allocate a large buffer so that each read() iteration
>  * should scrape *all* pending records.
>  *
> -* The largest buffer the OA unit supports is 16MB and the smallest
> -* OA report format is 64bytes allowing up to 262144 reports to
> -* be buffered.
> +* The largest buffer the OA unit supports is 16MB.

Out of curiosity, how is userspace meant to know? Or is it part of the
platform specific details that we spread around kernel/userspace?
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t 01/11] tests/perf: make stream_fd a global variable

[Chris Wilson]

Quoting Lionel Landwerlin (2017-08-04 12:20:30)
> When debugging unstable tests on new platforms we currently we don't
> cleanup everything well in between different tests. Since only a
> single OA stream fd can be opened at a time, having the stream_fd as a
> global variable helps us cleanup the state between tests.

We don't have constructors/destructors per-se, but we do have
igt_subtest_group which can contain fixtures to alloc/dealloc resources.

A more igt-esque approach would be

igt_subtest_group {
int perf_fd = -1;

igt_fixture {
perf_fd = __perf_open();
}

igt_subtest
... How ever many you want in the one group

igt_fixture {
__perf_close(perf_fd);
}
}

Just my 2c. You can of course join the petition for more igt
infrastructure to make writing robust tests easier... :)
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH i-g-t 06/11] tests/perf: make enable-disable more reliable

[Lionel Landwerlin]

Estimation of the amount of reports can only refer to periodic ones,
as context switch reports completely depend on what happens on the
system. Also generate some load to prevent clock frequency changes to
impact our measurement.

Signed-off-by: Lionel Landwerlin 
---
 tests/perf.c | 96 
 1 file changed, 90 insertions(+), 6 deletions(-)

diff --git a/tests/perf.c b/tests/perf.c
index 5c7a2a34..f33ccffd 100644
--- a/tests/perf.c
+++ b/tests/perf.c
@@ -2841,10 +2841,18 @@ test_enable_disable(void)
int n_full_oa_reports = oa_buf_size / report_size;
uint64_t fill_duration = n_full_oa_reports * oa_period;
 
+   load_helper_init();
+   load_helper_run(HIGH);
+
stream_fd = __perf_open(drm_fd, );
 
for (int i = 0; i < 5; i++) {
int len;
+   uint32_t n_periodic_reports;
+   struct drm_i915_perf_record_header *header;
+   uint32_t first_timestamp = 0, last_timestamp = 0;
+   uint32_t last_periodic_report[64];
+   double tick_per_period;
 
/* Giving enough time for an overflow might help catch whether
 * the OA unit has been enabled even if the driver might at
@@ -2864,18 +2872,91 @@ test_enable_disable(void)
 
nanosleep(&(struct timespec){ .tv_sec = 0,
  .tv_nsec = fill_duration / 2 },
- NULL);
+   NULL);
 
-   while ((len = read(stream_fd, buf, buf_size)) == -1 && errno == 
EINTR)
-   ;
+   n_periodic_reports = 0;
 
-   igt_assert_neq(len, -1);
+   /* Because of the race condition between notification of new
+* reports and reports landing in memory, we need to rely on
+* timestamps to figure whether we've read enough of them.
+*/
+   while (((last_timestamp - first_timestamp) * 
oa_exponent_to_ns(oa_exponent)) <
+  (fill_duration / 2)) {
 
-   igt_assert(len > report_size * n_full_oa_reports * 0.45);
-   igt_assert(len < report_size * n_full_oa_reports * 0.55);
+   while ((len = read(stream_fd, buf, buf_size)) == -1 && 
errno == EINTR)
+   ;
+
+   igt_assert_neq(len, -1);
+
+   for (int offset = 0; offset < len; offset += 
header->size) {
+   uint32_t *report;
+   double previous_tick_per_period;
+
+   header = (void *) (buf + offset);
+   report = (void *) (header + 1);
+
+   switch (header->type) {
+   case DRM_I915_PERF_RECORD_OA_REPORT_LOST:
+   break;
+   case DRM_I915_PERF_RECORD_SAMPLE:
+   if (first_timestamp == 0)
+   first_timestamp = report[1];
+   last_timestamp = report[1];
+
+   previous_tick_per_period = 
tick_per_period;
+
+   if (n_periodic_reports > 0 &&
+   oa_report_is_periodic(oa_exponent, 
report)) {
+   tick_per_period =
+   
oa_reports_tick_per_period(last_periodic_report,
+   
   report);
+
+   if 
(!double_value_within(tick_per_period,
+
previous_tick_per_period, 5))
+   igt_debug("clock 
change!\n");
+
+   igt_debug(" > report ts=%u"
+ " 
ts_delta_last_periodic=%8u is_timer=%i ctx_id=%8x gpu_ticks=%u 
nb_periodic=%u\n",
+ report[1],
+ report[1] - 
last_periodic_report[1],
+ 
oa_report_is_periodic(oa_exponent, report),
+ 
oa_report_get_ctx_id(report),
+ report[3] - 
last_periodic_report[3],
+ n_periodic_reports);
+
+   memcpy(last_periodic_report, 
report,
+  
sizeof(last_periodic_report));

[Intel-gfx] [PATCH i-g-t 00/11] Improve robustness of the i915 perf tests

[Lionel Landwerlin]

Hi all,

This is a resend of a series that was posted a few months ago. I've
pushed all the patches that has a Rb. This is a shorter list but
pretty important to make the tests more stable.

Cheers,

Lionel Landwerlin (10):
  tests/perf: make stream_fd a global variable
  tests/perf: update max buffer size for reading reports
  tests/perf: rc6: try to guess when rc6 is disabled
  tests/perf: rework oa-exponent test
  tests/perf: make enable-disable more reliable
  tests/perf: make buffer-fill more reliable
  tests/perf: load gt_boost_freq_mhz as max gt frequency
  tests/perf: remove unused frequency functions
  tests/perf: add Kabylake support
  tests/perf: add Geminilake support

Robert Bragg (1):
  tests/perf: add per context filtering test for gen8+

 tests/perf.c | 1954 +++---
 1 file changed, 1610 insertions(+), 344 deletions(-)

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

[Intel-gfx] [PATCH i-g-t 10/11] tests/perf: add Kabylake support

[Lionel Landwerlin]

Signed-off-by: Lionel Landwerlin 
---
 tests/perf.c | 17 -
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/tests/perf.c b/tests/perf.c
index f5949590..1e5a5b88 100644
--- a/tests/perf.c
+++ b/tests/perf.c
@@ -1102,8 +1102,23 @@ init_sys_info(void)
} else if (IS_BROXTON(devid)) {
test_set_uuid = "5ee72f5c-092f-421e-8b70-225f7c3e9612";
timestamp_frequency = 1920;
-   } else
+   } else if (IS_KABYLAKE(devid)) {
+   switch (intel_gt(devid)) {
+   case 1:
+   test_set_uuid = 
"baa3c7e4-52b6-4b85-801e-465a94b746dd";
+   break;
+   case 2:
+   test_set_uuid = 
"f1792f32-6db2-4b50-b4b2-557128f1688d";
+   break;
+   default:
+   igt_debug("unsupported Kabylake GT size\n");
+   return false;
+   }
+   timestamp_frequency = 1200;
+   } else {
+   igt_debug("unsupported GT\n");
return false;
+   }
 
gp.param = I915_PARAM_EU_TOTAL;
gp.value = _eus;
-- 
2.13.3

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

[Intel-gfx] [PATCH i-g-t 11/11] tests/perf: add Geminilake support

[Lionel Landwerlin]

Signed-off-by: Lionel Landwerlin 
---
 tests/perf.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tests/perf.c b/tests/perf.c
index 1e5a5b88..5b27925b 100644
--- a/tests/perf.c
+++ b/tests/perf.c
@@ -1115,6 +1115,9 @@ init_sys_info(void)
return false;
}
timestamp_frequency = 1200;
+   } else if (IS_GEMINILAKE(devid)) {
+   test_set_uuid = "dd3fd789-e783-4204-8cd0-b671bbccb0cf";
+   timestamp_frequency = 1920;
} else {
igt_debug("unsupported GT\n");
return false;
-- 
2.13.3

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

[Intel-gfx] [PATCH i-g-t 07/11] tests/perf: make buffer-fill more reliable

[Lionel Landwerlin]

Filling rate of the buffer must discard context switch reports as they
do not depend upon the periodicity, instead they're a factor on the
amount of different applications concurrently running on the system.

Signed-off-by: Lionel Landwerlin 
---
 tests/perf.c | 121 ++-
 1 file changed, 104 insertions(+), 17 deletions(-)

diff --git a/tests/perf.c b/tests/perf.c
index f33ccffd..6c0af32d 100644
--- a/tests/perf.c
+++ b/tests/perf.c
@@ -2749,22 +2749,30 @@ test_buffer_fill(void)
.num_properties = sizeof(properties) / 16,
.properties_ptr = to_user_pointer(properties),
};
+   struct drm_i915_perf_record_header *header;
int buf_size = 65536 * (256 + sizeof(struct 
drm_i915_perf_record_header));
uint8_t *buf = malloc(buf_size);
+   int len;
size_t oa_buf_size = 16 * 1024 * 1024;
size_t report_size = oa_formats[test_oa_format].size;
int n_full_oa_reports = oa_buf_size / report_size;
uint64_t fill_duration = n_full_oa_reports * oa_period;
 
+   load_helper_init();
+   load_helper_run(HIGH);
+
igt_assert(fill_duration < 10);
 
stream_fd = __perf_open(drm_fd, );
 
for (int i = 0; i < 5; i++) {
-   struct drm_i915_perf_record_header *header;
bool overflow_seen;
-   int offset = 0;
-   int len;
+   uint32_t n_periodic_reports;
+   uint32_t first_timestamp = 0, last_timestamp = 0;
+   uint32_t last_periodic_report[64];
+   double tick_per_period;
+
+   do_ioctl(stream_fd, I915_PERF_IOCTL_ENABLE, 0);
 
nanosleep(&(struct timespec){ .tv_sec = 0,
  .tv_nsec = fill_duration * 1.25 },
@@ -2776,7 +2784,7 @@ test_buffer_fill(void)
igt_assert_neq(len, -1);
 
overflow_seen = false;
-   for (offset = 0; offset < len; offset += header->size) {
+   for (int offset = 0; offset < len; offset += header->size) {
header = (void *)(buf + offset);
 
if (header->type == DRM_I915_PERF_RECORD_OA_BUFFER_LOST)
@@ -2785,32 +2793,111 @@ test_buffer_fill(void)
 
igt_assert_eq(overflow_seen, true);
 
+   do_ioctl(stream_fd, I915_PERF_IOCTL_DISABLE, 0);
+
+   igt_debug("fill_duration = %luns, oa_exponent = %u\n",
+ fill_duration, oa_exponent);
+
+   do_ioctl(stream_fd, I915_PERF_IOCTL_ENABLE, 0);
+
nanosleep(&(struct timespec){ .tv_sec = 0,
- .tv_nsec = fill_duration / 2 },
- NULL);
+   .tv_nsec = fill_duration / 2 },
+   NULL);
 
-   while ((len = read(stream_fd, buf, buf_size)) == -1 && errno == 
EINTR)
-   ;
+   n_periodic_reports = 0;
 
-   igt_assert_neq(len, -1);
+   /* Because of the race condition between notification of new
+* reports and reports landing in memory, we need to rely on
+* timestamps to figure whether we've read enough of them.
+*/
+   while (((last_timestamp - first_timestamp) * 
oa_exponent_to_ns(oa_exponent)) <
+  (fill_duration / 2)) {
 
-   igt_assert(len > report_size * n_full_oa_reports * 0.45);
-   igt_assert(len < report_size * n_full_oa_reports * 0.55);
+   igt_debug("dts=%u elapsed=%lu duration=%lu\n",
+ last_timestamp - first_timestamp,
+ (last_timestamp - first_timestamp) * 
oa_exponent_to_ns(oa_exponent),
+ fill_duration / 2);
 
-   overflow_seen = false;
-   for (offset = 0; offset < len; offset += header->size) {
-   header = (void *)(buf + offset);
+   while ((len = read(stream_fd, buf, buf_size)) == -1 && 
errno == EINTR)
+   ;
 
-   if (header->type == DRM_I915_PERF_RECORD_OA_BUFFER_LOST)
-   overflow_seen = true;
+   igt_assert_neq(len, -1);
+
+   for (int offset = 0; offset < len; offset += 
header->size) {
+   uint32_t *report;
+   double previous_tick_per_period;
+
+   header = (void *) (buf + offset);
+   report = (void *) (header + 1);
+
+   switch (header->type) {
+   case DRM_I915_PERF_RECORD_OA_REPORT_LOST:
+   igt_debug("report loss, 

[Intel-gfx] [PATCH i-g-t 09/11] tests/perf: remove unused frequency functions

[Lionel Landwerlin]

Now that we've found that frequency changes happen mostly outside of
our control and don't seem to be following our requests through sysfs,
let's drop a bunch code/variables.

Signed-off-by: Lionel Landwerlin 
---
 tests/perf.c | 83 +++-
 1 file changed, 3 insertions(+), 80 deletions(-)

diff --git a/tests/perf.c b/tests/perf.c
index 6be9f552..f5949590 100644
--- a/tests/perf.c
+++ b/tests/perf.c
@@ -285,12 +285,9 @@ static int card = -1;
 static int n_eus;
 
 static uint64_t test_metric_set_id = UINT64_MAX;
-static uint64_t gt_min_freq_mhz_saved = 0;
-static uint64_t gt_max_freq_mhz_saved = 0;
-static uint64_t gt_min_freq_mhz = 0;
-static uint64_t gt_max_freq_mhz = 0;
 
 static uint64_t timestamp_frequency = 1250;
+static uint64_t gt_max_freq_mhz = 0;
 static enum drm_i915_oa_format test_oa_format;
 static bool *undefined_a_counters;
 static uint64_t oa_exp_1_millisec;
@@ -413,16 +410,6 @@ sysfs_read(const char *file)
return read_u64_file(buf);
 }
 
-static void
-sysfs_write(const char *file, uint64_t val)
-{
-   char buf[512];
-
-   snprintf(buf, sizeof(buf), "/sys/class/drm/card%d/%s", card, file);
-
-   write_u64_file(buf, val);
-}
-
 static char *
 read_debugfs_record(int device, const char *file, const char *key)
 {
@@ -1137,66 +1124,6 @@ init_sys_info(void)
return try_read_u64_file(buf, _metric_set_id);
 }
 
-static void
-gt_frequency_range_save(void)
-{
-   gt_min_freq_mhz_saved = sysfs_read("gt_min_freq_mhz");
-   gt_max_freq_mhz_saved = sysfs_read("gt_boost_freq_mhz");
-
-   gt_min_freq_mhz = gt_min_freq_mhz_saved;
-   gt_max_freq_mhz = gt_max_freq_mhz_saved;
-}
-
-static void wait_freq_settle(void)
-{
-   struct timespec ts;
-
-   /* FIXME: Lazy sleep without check. */
-   ts.tv_sec = 0;
-   ts.tv_nsec = 2;
-   clock_nanosleep(CLOCK_MONOTONIC, 0, , NULL);
-}
-
-static void
-gt_frequency_pin(int gt_freq_mhz)
-{
-   igt_debug("requesting pinned GT freq = %dmhz\n", gt_freq_mhz);
-
-   if (gt_freq_mhz > gt_max_freq_mhz) {
-   sysfs_write("gt_max_freq_mhz", gt_freq_mhz);
-   sysfs_write("gt_min_freq_mhz", gt_freq_mhz);
-   } else {
-   sysfs_write("gt_min_freq_mhz", gt_freq_mhz);
-   sysfs_write("gt_max_freq_mhz", gt_freq_mhz);
-   }
-   gt_min_freq_mhz = gt_freq_mhz;
-   gt_max_freq_mhz = gt_freq_mhz;
-
-   wait_freq_settle();
-}
-
-static void
-gt_frequency_range_restore(void)
-{
-   igt_debug("restoring GT frequency range: min = %dmhz, max =%dmhz, 
current: min=%dmhz, max=%dmhz\n",
- (int)gt_min_freq_mhz_saved,
- (int)gt_max_freq_mhz_saved,
- (int)gt_min_freq_mhz,
- (int)gt_max_freq_mhz);
-
-   /* Assume current min/max are the same */
-   if (gt_min_freq_mhz_saved > gt_max_freq_mhz) {
-   sysfs_write("gt_max_freq_mhz", gt_max_freq_mhz_saved);
-   sysfs_write("gt_min_freq_mhz", gt_min_freq_mhz_saved);
-   } else {
-   sysfs_write("gt_min_freq_mhz", gt_min_freq_mhz_saved);
-   sysfs_write("gt_max_freq_mhz", gt_max_freq_mhz_saved);
-   }
-
-   gt_min_freq_mhz = gt_min_freq_mhz_saved;
-   gt_max_freq_mhz = gt_max_freq_mhz_saved;
-}
-
 static int
 i915_read_reports_until_timestamp(enum drm_i915_oa_format oa_format,
  uint8_t *buf,
@@ -2202,8 +2129,6 @@ test_oa_exponents(void)
igt_assert(n_time_delta_matches >= 9);
}
 
-   gt_frequency_range_restore();
-
load_helper_stop();
load_helper_deinit();
 }
@@ -4538,11 +4463,11 @@ igt_main
 
igt_require(init_sys_info());
 
-   gt_frequency_range_save();
-
write_u64_file("/proc/sys/dev/i915/perf_stream_paranoid", 1);
write_u64_file("/proc/sys/dev/i915/oa_max_sample_rate", 10);
 
+   gt_max_freq_mhz = sysfs_read("gt_boost_freq_mhz");
+
render_copy = igt_get_render_copyfunc(devid);
igt_require_f(render_copy, "no render-copy function\n");
}
@@ -4637,8 +4562,6 @@ igt_main
write_u64_file("/proc/sys/dev/i915/oa_max_sample_rate", 10);
write_u64_file("/proc/sys/dev/i915/perf_stream_paranoid", 1);
 
-   gt_frequency_range_restore();
-
close(drm_fd);
}
 }
-- 
2.13.3

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

[Intel-gfx] [PATCH i-g-t 08/11] tests/perf: load gt_boost_freq_mhz as max gt frequency

[Lionel Landwerlin]

We want the absolute max the hardware can do, not the max value
set by a previous application/user.

Signed-off-by: Lionel Landwerlin 
---
 tests/perf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/perf.c b/tests/perf.c
index 6c0af32d..6be9f552 100644
--- a/tests/perf.c
+++ b/tests/perf.c
@@ -1141,7 +1141,7 @@ static void
 gt_frequency_range_save(void)
 {
gt_min_freq_mhz_saved = sysfs_read("gt_min_freq_mhz");
-   gt_max_freq_mhz_saved = sysfs_read("gt_max_freq_mhz");
+   gt_max_freq_mhz_saved = sysfs_read("gt_boost_freq_mhz");
 
gt_min_freq_mhz = gt_min_freq_mhz_saved;
gt_max_freq_mhz = gt_max_freq_mhz_saved;
-- 
2.13.3

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

[Intel-gfx] [PATCH i-g-t 02/11] tests/perf: add per context filtering test for gen8+

[Lionel Landwerlin]

From: Robert Bragg 

Signed-off-by: Robert Bragg 
Signed-off-by: Lionel Landwerlin 
---
 tests/perf.c | 806 ---
 1 file changed, 768 insertions(+), 38 deletions(-)

diff --git a/tests/perf.c b/tests/perf.c
index 26581ce4..279ff0c6 100644
--- a/tests/perf.c
+++ b/tests/perf.c
@@ -48,7 +48,9 @@ IGT_TEST_DESCRIPTION("Test the i915 perf metrics streaming 
interface");
 #define OAREPORT_REASON_MASK   0x3f
 #define OAREPORT_REASON_SHIFT  19
 #define OAREPORT_REASON_TIMER  (1<<0)
+#define OAREPORT_REASON_INTERNAL   (3<<1)
 #define OAREPORT_REASON_CTX_SWITCH (1<<3)
+#define OAREPORT_REASON_GO (1<<4)
 #define OAREPORT_REASON_CLK_RATIO  (1<<5)
 
 #define GFX_OP_PIPE_CONTROL ((3 << 29) | (3 << 27) | (2 << 24))
@@ -565,6 +567,22 @@ oa_exponent_to_ns(int exponent)
return 10ULL * (2ULL << exponent) / timestamp_frequency;
 }
 
+static bool
+oa_report_ctx_is_valid(uint32_t *report)
+{
+   if (IS_HASWELL(devid)) {
+   return false; /* TODO */
+   } else if (IS_GEN8(devid)) {
+   return report[0] & (1ul << 25);
+   } else if (IS_GEN9(devid)) {
+   return report[0] & (1ul << 16);
+   }
+
+   /* Need to update this function for newer Gen. */
+   igt_assert(!"reached");
+}
+
+
 static void
 hsw_sanity_check_render_basic_reports(uint32_t *oa_report0, uint32_t 
*oa_report1,
  enum drm_i915_oa_format fmt)
@@ -669,6 +687,100 @@ gen8_40bit_a_delta(uint64_t value0, uint64_t value1)
return value1 - value0;
 }
 
+static void
+accumulate_uint32(size_t offset,
+ uint32_t *report0,
+  uint32_t *report1,
+  uint64_t *delta)
+{
+   uint32_t value0 = *(uint32_t *)(((uint8_t *)report0) + offset);
+   uint32_t value1 = *(uint32_t *)(((uint8_t *)report1) + offset);
+
+   *delta += (uint32_t)(value1 - value0);
+}
+
+static void
+accumulate_uint40(int a_index,
+  uint32_t *report0,
+  uint32_t *report1,
+ enum drm_i915_oa_format format,
+  uint64_t *delta)
+{
+   uint64_t value0 = gen8_read_40bit_a_counter(report0, format, a_index),
+value1 = gen8_read_40bit_a_counter(report1, format, a_index);
+
+   *delta += gen8_40bit_a_delta(value0, value1);
+}
+
+static void
+accumulate_reports(struct accumulator *accumulator,
+  uint32_t *start,
+  uint32_t *end)
+{
+   enum drm_i915_oa_format format = accumulator->format;
+   uint64_t *deltas = accumulator->deltas;
+   int idx = 0;
+
+   if (intel_gen(devid) >= 8) {
+   /* timestamp */
+   accumulate_uint32(4, start, end, deltas + idx++);
+
+   /* clock cycles */
+   accumulate_uint32(12, start, end, deltas + idx++);
+   } else {
+   /* timestamp */
+   accumulate_uint32(4, start, end, deltas + idx++);
+   }
+
+   for (int i = 0; i < oa_formats[format].n_a40; i++)
+   accumulate_uint40(i, start, end, format, deltas + idx++);
+
+   for (int i = 0; i < oa_formats[format].n_a; i++) {
+   accumulate_uint32(oa_formats[format].a_off + 4 * i,
+ start, end, deltas + idx++);
+   }
+
+   for (int i = 0; i < oa_formats[format].n_b; i++) {
+   accumulate_uint32(oa_formats[format].b_off + 4 * i,
+ start, end, deltas + idx++);
+   }
+
+   for (int i = 0; i < oa_formats[format].n_c; i++) {
+   accumulate_uint32(oa_formats[format].c_off + 4 * i,
+ start, end, deltas + idx++);
+   }
+}
+
+static void
+accumulator_print(struct accumulator *accumulator, const char *title)
+{
+   enum drm_i915_oa_format format = accumulator->format;
+   uint64_t *deltas = accumulator->deltas;
+   int idx = 0;
+
+   igt_debug("%s:\n", title);
+   if (intel_gen(devid) >= 8) {
+   igt_debug("\ttime delta = %lu\n", deltas[idx++]);
+   igt_debug("\tclock cycle delta = %lu\n", deltas[idx++]);
+
+   for (int i = 0; i < oa_formats[format].n_a40; i++)
+   igt_debug("\tA%u = %lu\n", i, deltas[idx++]);
+   } else {
+   igt_debug("\ttime delta = %lu\n", deltas[idx++]);
+   }
+
+   for (int i = 0; i < oa_formats[format].n_a; i++) {
+   int a_id = oa_formats[format].first_a + i;
+   igt_debug("\tA%u = %lu\n", a_id, deltas[idx++]);
+   }
+
+   for (int i = 0; i < oa_formats[format].n_a; i++)
+   igt_debug("\tB%u = %lu\n", i, deltas[idx++]);
+
+   for (int i = 0; i < oa_formats[format].n_c; i++)
+   igt_debug("\tC%u = %lu\n", i, deltas[idx++]);
+}
+
 /* 

[Intel-gfx] [PATCH i-g-t 03/11] tests/perf: update max buffer size for reading reports

[Lionel Landwerlin]

Signed-off-by: Lionel Landwerlin 
---
 tests/perf.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/perf.c b/tests/perf.c
index 279ff0c6..65a1606d 100644
--- a/tests/perf.c
+++ b/tests/perf.c
@@ -1271,9 +1271,7 @@ read_2_oa_reports(int format_id,
/* Note: we allocate a large buffer so that each read() iteration
 * should scrape *all* pending records.
 *
-* The largest buffer the OA unit supports is 16MB and the smallest
-* OA report format is 64bytes allowing up to 262144 reports to
-* be buffered.
+* The largest buffer the OA unit supports is 16MB.
 *
 * Being sure we are fetching all buffered reports allows us to
 * potentially throw away / skip all reports whenever we see
@@ -1286,7 +1284,8 @@ read_2_oa_reports(int format_id,
 * to indicate that the OA unit may be over taxed if lots of reports
 * are being lost.
 */
-   int buf_size = 262144 * (64 + sizeof(struct 
drm_i915_perf_record_header));
+   int max_reports = (16 * 1024 * 1024) / format_size;
+   int buf_size = sample_size * max_reports * 1.5;
uint8_t *buf = malloc(buf_size);
int n = 0;
 
@@ -1298,6 +1297,7 @@ read_2_oa_reports(int format_id,
;
 
igt_assert(len > 0);
+   igt_debug("read %d bytes\n", (int)len);
 
for (size_t offset = 0; offset < len; offset += header->size) {
const uint32_t *report;
-- 
2.13.3

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

[Intel-gfx] [PATCH i-g-t 05/11] tests/perf: rework oa-exponent test

[Lionel Landwerlin]

New issues that were discovered while making the tests work on Gen8+ :

 - we need to measure timings between periodic reports and discard all
   other kind of reports

 - it seems periodicity of the reports can be affected outside of RC6
   (frequency change), we can detect this by looking at the amount of
   clock cycles per timestamp deltas

Signed-off-by: Lionel Landwerlin 
---
 tests/perf.c | 786 ---
 1 file changed, 594 insertions(+), 192 deletions(-)

diff --git a/tests/perf.c b/tests/perf.c
index 7f9bc66d..5c7a2a34 100644
--- a/tests/perf.c
+++ b/tests/perf.c
@@ -28,6 +28,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -300,6 +301,25 @@ static uint32_t (*read_report_ticks)(uint32_t *report,
 static void (*sanity_check_reports)(uint32_t *oa_report0, uint32_t *oa_report1,
enum drm_i915_oa_format format);
 
+static bool
+timestamp_delta_within(uint32_t delta,
+  uint32_t expected_delta,
+  uint32_t margin)
+{
+   return delta >= (expected_delta - margin) &&
+  delta <= (expected_delta + margin);
+}
+
+static bool
+double_value_within(double value,
+   double expected,
+   double percent_margin)
+{
+   return value >= (expected - expected * percent_margin / 100.0) &&
+  value <= (expected + expected * percent_margin / 100.0);
+
+}
+
 static void
 __perf_close(int fd)
 {
@@ -476,6 +496,20 @@ gen8_read_report_ticks(uint32_t *report, enum 
drm_i915_oa_format format)
return report[3];
 }
 
+static void
+gen8_read_report_clock_ratios(uint32_t *report,
+ uint32_t *slice_freq_mhz,
+ uint32_t *unslice_freq_mhz)
+{
+   uint32_t unslice_freq = report[0] & 0x1ff;
+   uint32_t slice_freq_low = (report[0] >> 25) & 0x7f;
+   uint32_t slice_freq_high = (report[0] >> 9) & 0x3;
+   uint32_t slice_freq = slice_freq_low | (slice_freq_high << 7);
+
+   *slice_freq_mhz = (slice_freq * 1) / 1000;
+   *unslice_freq_mhz = (unslice_freq * 1) / 1000;
+}
+
 static const char *
 gen8_read_report_reason(const uint32_t *report)
 {
@@ -498,29 +532,6 @@ gen8_read_report_reason(const uint32_t *report)
return "unknown";
 }
 
-static bool
-oa_report_is_periodic(uint32_t oa_exponent, const uint32_t *report)
-{
-   if (IS_HASWELL(devid)) {
-   /* For Haswell we don't have a documented report reason field
-* (though empirically report[0] bit 10 does seem to correlate
-* with a timer trigger reason) so we instead infer which
-* reports are timer triggered by checking if the least
-* significant bits are zero and the exponent bit is set.
-*/
-   uint32_t oa_exponent_mask = (1 << (oa_exponent + 1)) - 1;
-
-   if ((report[1] & oa_exponent_mask) != (1 << oa_exponent))
-   return true;
-   } else {
-   if ((report[0] >> OAREPORT_REASON_SHIFT) &
-   OAREPORT_REASON_TIMER)
-   return true;
-   }
-
-   return false;
-}
-
 static uint64_t
 timebase_scale(uint32_t u32_delta)
 {
@@ -568,6 +579,29 @@ oa_exponent_to_ns(int exponent)
 }
 
 static bool
+oa_report_is_periodic(uint32_t oa_exponent, const uint32_t *report)
+{
+   if (IS_HASWELL(devid)) {
+   /* For Haswell we don't have a documented report reason field
+* (though empirically report[0] bit 10 does seem to correlate
+* with a timer trigger reason) so we instead infer which
+* reports are timer triggered by checking if the least
+* significant bits are zero and the exponent bit is set.
+*/
+   uint32_t oa_exponent_mask = (1 << (oa_exponent + 1)) - 1;
+
+   if ((report[1] & oa_exponent_mask) == (1 << oa_exponent))
+   return true;
+   } else {
+   if ((report[0] >> OAREPORT_REASON_SHIFT) &
+   OAREPORT_REASON_TIMER)
+   return true;
+   }
+
+   return false;
+}
+
+static bool
 oa_report_ctx_is_valid(uint32_t *report)
 {
if (IS_HASWELL(devid)) {
@@ -582,6 +616,128 @@ oa_report_ctx_is_valid(uint32_t *report)
igt_assert(!"reached");
 }
 
+static uint32_t
+oa_report_get_ctx_id(uint32_t *report)
+{
+   if (!oa_report_ctx_is_valid(report))
+   return 0x;
+   return report[2];
+}
+
+static double
+oa_reports_tick_per_period(uint32_t *report0, uint32_t *report1)
+{
+   if (intel_gen(devid) < 8)
+   return 0.0;
+
+   /* Measure the number GPU tick delta to timestamp delta. */
+   return (double) (report1[3] - report0[3]) /
+  (double) (report1[1] - report0[1]);
+}

[Intel-gfx] [PATCH i-g-t 01/11] tests/perf: make stream_fd a global variable

[Lionel Landwerlin]

When debugging unstable tests on new platforms we currently we don't
cleanup everything well in between different tests. Since only a
single OA stream fd can be opened at a time, having the stream_fd as a
global variable helps us cleanup the state between tests.

Signed-off-by: Lionel Landwerlin 
---
 tests/perf.c | 121 ---
 1 file changed, 65 insertions(+), 56 deletions(-)

diff --git a/tests/perf.c b/tests/perf.c
index f0ec26dd..26581ce4 100644
--- a/tests/perf.c
+++ b/tests/perf.c
@@ -276,6 +276,7 @@ static bool hsw_undefined_a_counters[45] = {
 static bool gen8_undefined_a_counters[45];
 
 static int drm_fd = -1;
+static int stream_fd = -1;
 static uint32_t devid;
 static int card = -1;
 static int n_eus;
@@ -297,10 +298,22 @@ static uint32_t (*read_report_ticks)(uint32_t *report,
 static void (*sanity_check_reports)(uint32_t *oa_report0, uint32_t *oa_report1,
enum drm_i915_oa_format format);
 
+static void
+__perf_close(int fd)
+{
+   close(fd);
+   stream_fd = -1;
+}
+
 static int
 __perf_open(int fd, struct drm_i915_perf_open_param *param)
 {
-   int ret = igt_ioctl(fd, DRM_IOCTL_I915_PERF_OPEN, param);
+   int ret;
+
+   if (stream_fd >= 0)
+   __perf_close(stream_fd);
+
+   ret = igt_ioctl(fd, DRM_IOCTL_I915_PERF_OPEN, param);
 
igt_assert(ret >= 0);
errno = 0;
@@ -951,14 +964,12 @@ test_system_wide_paranoid(void)
.num_properties = sizeof(properties) / 16,
.properties_ptr = to_user_pointer(properties),
};
-   int stream_fd;
-
write_u64_file("/proc/sys/dev/i915/perf_stream_paranoid", 0);
 
igt_drop_root();
 
stream_fd = __perf_open(drm_fd, );
-   close(stream_fd);
+   __perf_close(stream_fd);
}
 
igt_waitchildren();
@@ -1006,7 +1017,6 @@ test_invalid_oa_metric_set_id(void)
.num_properties = sizeof(properties) / 16,
.properties_ptr = to_user_pointer(properties),
};
-   int stream_fd;
 
do_ioctl_err(drm_fd, DRM_IOCTL_I915_PERF_OPEN, , EINVAL);
 
@@ -1016,7 +1026,7 @@ test_invalid_oa_metric_set_id(void)
/* Check that we aren't just seeing false positives... */
properties[ARRAY_SIZE(properties) - 1] = test_metric_set_id;
stream_fd = __perf_open(drm_fd, );
-   close(stream_fd);
+   __perf_close(stream_fd);
 
/* There's no valid default OA metric set ID... */
param.num_properties--;
@@ -1041,7 +1051,6 @@ test_invalid_oa_format_id(void)
.num_properties = sizeof(properties) / 16,
.properties_ptr = to_user_pointer(properties),
};
-   int stream_fd;
 
do_ioctl_err(drm_fd, DRM_IOCTL_I915_PERF_OPEN, , EINVAL);
 
@@ -1051,7 +1060,7 @@ test_invalid_oa_format_id(void)
/* Check that we aren't just seeing false positives... */
properties[ARRAY_SIZE(properties) - 1] = test_oa_format;
stream_fd = __perf_open(drm_fd, );
-   close(stream_fd);
+   __perf_close(stream_fd);
 
/* There's no valid default OA format... */
param.num_properties--;
@@ -1079,8 +1088,7 @@ test_missing_sample_flags(void)
 }
 
 static void
-read_2_oa_reports(int stream_fd,
- int format_id,
+read_2_oa_reports(int format_id,
  int exponent,
  uint32_t *oa_report0,
  uint32_t *oa_report1,
@@ -1214,12 +1222,13 @@ open_and_read_2_oa_reports(int format_id,
.num_properties = sizeof(properties) / 16,
.properties_ptr = to_user_pointer(properties),
};
-   int stream_fd = __perf_open(drm_fd, );
 
-   read_2_oa_reports(stream_fd, format_id, exponent,
+   stream_fd = __perf_open(drm_fd, );
+
+   read_2_oa_reports(format_id, exponent,
  oa_report0, oa_report1, timer_only);
 
-   close(stream_fd);
+   __perf_close(stream_fd);
 }
 
 static void
@@ -1519,9 +1528,10 @@ test_invalid_oa_exponent(void)
.num_properties = sizeof(properties) / 16,
.properties_ptr = to_user_pointer(properties),
};
-   int stream_fd = __perf_open(drm_fd, );
 
-   close(stream_fd);
+   stream_fd = __perf_open(drm_fd, );
+
+   __perf_close(stream_fd);
 
for (int i = 32; i < 65; i++) {
properties[7] = i;
@@ -1571,12 +1581,10 @@ test_low_oa_exponent_permissions(void)
properties[7] = ok_exponent;
 
igt_fork(child, 1) {
-   int stream_fd;
-
igt_drop_root();
 
stream_fd = __perf_open(drm_fd, );
-   close(stream_fd);
+   __perf_close(stream_fd);
}
 
igt_waitchildren();
@@ -1625,7 +1633,6 @@ test_per_context_mode_unprivileged(void)

[Intel-gfx] [PATCH i-g-t 04/11] tests/perf: rc6: try to guess when rc6 is disabled

[Lionel Landwerlin]

Signed-off-by: Lionel Landwerlin 
---
 tests/perf.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/tests/perf.c b/tests/perf.c
index 65a1606d..7f9bc66d 100644
--- a/tests/perf.c
+++ b/tests/perf.c
@@ -3453,6 +3453,17 @@ gen8_test_single_ctx_render_target_writes_a_counter(void)
} while (WEXITSTATUS(child_ret) == EAGAIN);
 }
 
+static bool
+rc6_enabled(void)
+{
+   char *rc6_status = read_debugfs_record(drm_fd, "i915_drpc_info",
+  "RC6 Enabled");
+   bool enabled = strcmp(rc6_status, "yes") == 0;
+
+   free(rc6_status);
+   return enabled;
+}
+
 static void
 test_rc6_disable(void)
 {
@@ -3472,6 +3483,8 @@ test_rc6_disable(void)
};
uint64_t n_events_start, n_events_end;
 
+   igt_skip_on(!rc6_enabled());
+
stream_fd = __perf_open(drm_fd, );
 
n_events_start = read_debugfs_u64_record(drm_fd, "i915_drpc_info",
-- 
2.13.3

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

[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/shrinker: Wrap need_resched() inside preempt-disable

[Patchwork]

== Series Details ==

Series: drm/i915/shrinker: Wrap need_resched() inside preempt-disable
URL   : https://patchwork.freedesktop.org/series/28371/
State : failure

== Summary ==

Series 28371v1 drm/i915/shrinker: Wrap need_resched() inside preempt-disable
https://patchwork.freedesktop.org/api/1.0/series/28371/revisions/1/mbox/

Test gem_exec_suspend:
Subgroup basic-s3:
pass   -> INCOMPLETE (fi-skl-6260u)
Test gem_sync:
Subgroup basic-store-all:
fail   -> PASS   (fi-ivb-3520m) fdo#17
Test kms_cursor_legacy:
Subgroup basic-busy-flip-before-cursor-legacy:
pass   -> FAIL   (fi-snb-2600) fdo#100215
Test kms_flip:
Subgroup basic-flip-vs-modeset:
skip   -> PASS   (fi-skl-x1585l) fdo#101781
Test kms_pipe_crc_basic:
Subgroup hang-read-crc-pipe-b:
pass   -> DMESG-WARN (fi-pnv-d510) fdo#101597
Subgroup suspend-read-crc-pipe-b:
pass   -> DMESG-WARN (fi-byt-j1900) fdo#101705 +1

fdo#17 https://bugs.freedesktop.org/show_bug.cgi?id=17
fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215
fdo#101781 https://bugs.freedesktop.org/show_bug.cgi?id=101781
fdo#101597 https://bugs.freedesktop.org/show_bug.cgi?id=101597
fdo#101705 https://bugs.freedesktop.org/show_bug.cgi?id=101705

fi-bdw-5557u total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  
time:438s
fi-bdw-gvtdvmtotal:279  pass:265  dwarn:0   dfail:0   fail:0   skip:14  
time:421s
fi-blb-e6850 total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  
time:360s
fi-bsw-n3050 total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  
time:491s
fi-bxt-j4205 total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  
time:491s
fi-byt-j1900 total:279  pass:254  dwarn:1   dfail:0   fail:0   skip:24  
time:516s
fi-byt-n2820 total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  
time:513s
fi-glk-2atotal:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  
time:584s
fi-hsw-4770  total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  
time:439s
fi-hsw-4770r total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  
time:411s
fi-ilk-650   total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  
time:419s
fi-ivb-3520m total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:499s
fi-ivb-3770  total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:472s
fi-kbl-7500u total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:453s
fi-kbl-7560u total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:567s
fi-kbl-r total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:574s
fi-pnv-d510  total:279  pass:221  dwarn:3   dfail:0   fail:0   skip:55  
time:571s
fi-skl-6260u total:109  pass:105  dwarn:0   dfail:0   fail:0   skip:3  
fi-skl-6700k total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:638s
fi-skl-6770hqtotal:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:469s
fi-skl-gvtdvmtotal:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  
time:429s
fi-skl-x1585ltotal:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:488s
fi-snb-2520m total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  
time:548s
fi-snb-2600  total:279  pass:249  dwarn:0   dfail:0   fail:1   skip:29  
time:406s

0394eb2d550a7b53adb3e36d0b79904ab8f1ffb2 drm-tip: 2017y-08m-04d-09h-39m-16s UTC 
integration manifest
aa253b5ccdf1 drm/i915/shrinker: Wrap need_resched() inside preempt-disable

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5327/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915/shrinker: Wrap need_resched() inside preempt-disable

[Chris Wilson]

In order for us to successfully detect the end of a timeslice,
preemption must be disabled. Otherwise, inside the loop we may be
preempted many times without our noticing, and each time our timeslice
will be reset, invalidating need_resched()

Reported-by: Joonas Lahtinen 
Reported-by: Tomi Sarvela 
Fixes: 290271de34f6 ("drm/i915: Spin for struct_mutex inside shrinker")
Signed-off-by: Chris Wilson 
Cc: Mika Kuoppala 
Cc: Joonas Lahtinen 
Cc:  # v4.13-rc1+
---
 drivers/gpu/drm/i915/i915_gem_shrinker.c | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c 
b/drivers/gpu/drm/i915/i915_gem_shrinker.c
index 26697c9e317d..3a34dcf00174 100644
--- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
+++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
@@ -43,16 +43,21 @@ static bool shrinker_lock(struct drm_i915_private 
*dev_priv, bool *unlock)
return true;
 
case MUTEX_TRYLOCK_FAILED:
+   *unlock = false;
+   preempt_disable();
do {
cpu_relax();
if (mutex_trylock(_priv->drm.struct_mutex)) {
-   case MUTEX_TRYLOCK_SUCCESS:
*unlock = true;
-   return true;
+   break;
}
} while (!need_resched());
+   preempt_enable();
+   return *unlock;
 
-   return false;
+   case MUTEX_TRYLOCK_SUCCESS:
+   *unlock = true;
+   return true;
}
 
BUG();
-- 
2.13.3

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

Re: [Intel-gfx] [PATCH 00/11] constify i915 attribute_group structures.

[Arvind Yadav]

On Friday 04 August 2017 04:04 PM, Lionel Landwerlin wrote:

On 04/08/17 11:22, Arvind Yadav wrote:

Hi Lionel,


On Friday 04 August 2017 02:33 PM, Lionel Landwerlin wrote:

Hi Arwind,

These files were generated by a script maintained in this repository 
: 
https://github.com/rib/gputop/blob/master/scripts/i915-perf-kernelgen.py 

It would best to update this script first to make sure future 
platforms get the fixes too.


Some changes have just been merged, deleted most configs but the 
test ones.

You'll need to update your series.

I have done the changes. Please review it. :) Shared patch is 
0001-i915-perf-kernelgen.py-constify-attribute_group-stru.patch.


Hm... Where is it? (I can't see it on the mailing list nor attached)
The best would be to submit a PR on the github project directly.

I have push directly on github project. I have send patch to you. Is 
there  any different way to send mail.?

 Changes are looks like this.

---
 scripts/i915-perf-kernelgen.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/i915-perf-kernelgen.
py b/scripts/i915-perf-kernelgen.py
index 7178f47..7633624 100755
--- a/scripts/i915-perf-kernelgen.py
+++ b/scripts/i915-perf-kernelgen.py
@@ -382,7 +382,7 @@ def output_sysfs_code(sets):
 c("};")

 c("\n")
-c("static struct attribute_group group_" + perf_name_lc + " = {")
+c("static const struct attribute_group group_" + perf_name_lc + 
" = {")

 c.indent(8)
 c(".name = \"" + metric_set['guid'] + "\",")
 c(".attrs =  attrs_" + perf_name_lc + ",")

---



Otherwise it looks like a good change.

Thanks,

-
Lionel

On 04/08/17 06:03, Arvind Yadav wrote:

attribute_group are not supposed to change at runtime. All functions
working with attribute_group provided by  work with
const attribute_group. So mark the non-const structs as const.

Arvind Yadav (11):
   [PATCH 01/11] drm: i915: i915_oa_kblgt2: constify 
attribute_group structures.
   [PATCH 02/11] drm: i915: i915_oa_bdw: constify attribute_group 
structures.
   [PATCH 03/11] drm: i915: i915_oa_bxt: constify attribute_group 
structures.
   [PATCH 04/11] drm: i915: i915_oa_chv: constify attribute_group 
structures.
   [PATCH 05/11] drm: i915: i915_oa_glk: constify attribute_group 
structures.
   [PATCH 06/11] drm: i915: i915_oa_hsw: constify attribute_group 
structures.
   [PATCH 07/11] drm: i915: i915_oa_kblgt3: constify 
attribute_group structures.
   [PATCH 08/11] drm: i915: i915_oa_sklgt2: constify 
attribute_group structures.
   [PATCH 09/11] drm: i915: i915_oa_sklgt3: constify 
attribute_group structures.
   [PATCH 10/11] drm: i915: i915_oa_sklgt4: constify 
attribute_group structures.
   [PATCH 11/11] drm: i915: i915_sysfs: constify attribute_group 
structures.


  drivers/gpu/drm/i915/i915_oa_bdw.c| 44 
+--

  drivers/gpu/drm/i915/i915_oa_bxt.c| 30 
  drivers/gpu/drm/i915/i915_oa_chv.c| 28 +++---
  drivers/gpu/drm/i915/i915_oa_glk.c| 30 
  drivers/gpu/drm/i915/i915_oa_hsw.c| 12 +-
  drivers/gpu/drm/i915/i915_oa_kblgt2.c | 36 
++--
  drivers/gpu/drm/i915/i915_oa_kblgt3.c | 36 
++--
  drivers/gpu/drm/i915/i915_oa_sklgt2.c | 36 
++--
  drivers/gpu/drm/i915/i915_oa_sklgt3.c | 36 
++--
  drivers/gpu/drm/i915/i915_oa_sklgt4.c | 36 
++--

  drivers/gpu/drm/i915/i915_sysfs.c |  6 ++---
  11 files changed, 165 insertions(+), 165 deletions(-)




~arvind





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

Re: [Intel-gfx] [PATCH 00/11] constify i915 attribute_group structures.

[Lionel Landwerlin]

On 04/08/17 11:22, Arvind Yadav wrote:

Hi Lionel,


On Friday 04 August 2017 02:33 PM, Lionel Landwerlin wrote:

Hi Arwind,

These files were generated by a script maintained in this repository 
: 
https://github.com/rib/gputop/blob/master/scripts/i915-perf-kernelgen.py
It would best to update this script first to make sure future 
platforms get the fixes too.


Some changes have just been merged, deleted most configs but the test 
ones.

You'll need to update your series.

I have done the changes. Please review it. :) Shared patch is 
0001-i915-perf-kernelgen.py-constify-attribute_group-stru.patch.


Hm... Where is it? (I can't see it on the mailing list nor attached)
The best would be to submit a PR on the github project directly.





Otherwise it looks like a good change.

Thanks,

-
Lionel

On 04/08/17 06:03, Arvind Yadav wrote:

attribute_group are not supposed to change at runtime. All functions
working with attribute_group provided by  work with
const attribute_group. So mark the non-const structs as const.

Arvind Yadav (11):
   [PATCH 01/11] drm: i915: i915_oa_kblgt2: constify attribute_group 
structures.
   [PATCH 02/11] drm: i915: i915_oa_bdw: constify attribute_group 
structures.
   [PATCH 03/11] drm: i915: i915_oa_bxt: constify attribute_group 
structures.
   [PATCH 04/11] drm: i915: i915_oa_chv: constify attribute_group 
structures.
   [PATCH 05/11] drm: i915: i915_oa_glk: constify attribute_group 
structures.
   [PATCH 06/11] drm: i915: i915_oa_hsw: constify attribute_group 
structures.
   [PATCH 07/11] drm: i915: i915_oa_kblgt3: constify attribute_group 
structures.
   [PATCH 08/11] drm: i915: i915_oa_sklgt2: constify attribute_group 
structures.
   [PATCH 09/11] drm: i915: i915_oa_sklgt3: constify attribute_group 
structures.
   [PATCH 10/11] drm: i915: i915_oa_sklgt4: constify attribute_group 
structures.
   [PATCH 11/11] drm: i915: i915_sysfs: constify attribute_group 
structures.


  drivers/gpu/drm/i915/i915_oa_bdw.c| 44 
+--

  drivers/gpu/drm/i915/i915_oa_bxt.c| 30 
  drivers/gpu/drm/i915/i915_oa_chv.c| 28 +++---
  drivers/gpu/drm/i915/i915_oa_glk.c| 30 
  drivers/gpu/drm/i915/i915_oa_hsw.c| 12 +-
  drivers/gpu/drm/i915/i915_oa_kblgt2.c | 36 
++--
  drivers/gpu/drm/i915/i915_oa_kblgt3.c | 36 
++--
  drivers/gpu/drm/i915/i915_oa_sklgt2.c | 36 
++--
  drivers/gpu/drm/i915/i915_oa_sklgt3.c | 36 
++--
  drivers/gpu/drm/i915/i915_oa_sklgt4.c | 36 
++--

  drivers/gpu/drm/i915/i915_sysfs.c |  6 ++---
  11 files changed, 165 insertions(+), 165 deletions(-)




~arvind



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

[Intel-gfx] [PATCH] drm/i915: add register macro definition style guide

[Jani Nikula]

This is not to try to force a new style; this is my interpretation of
what the most common existing style is.

With hopes I don't need to answer so many questions about style going
forward.

Cc: Daniel Vetter 
Signed-off-by: Jani Nikula 

---

N.b. only the *interpretation* of the existing style is up for debate
here. Proposals to *change* the style going forward can be done in other
patches changing this description. However, I doubt the usefulness of
such changes, with the possible exception of promoting the use of BIT().
---
 drivers/gpu/drm/i915/i915_reg.h | 77 +
 1 file changed, 77 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index b2546ade2c45..324cf04d6bfe 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -25,6 +25,83 @@
 #ifndef _I915_REG_H_
 #define _I915_REG_H_
 
+/*
+ * The i915 register macro definition style guide.
+ *
+ * Follow the style described here for new macros, and while changing existing
+ * macros. Do not mass change existing definitions just to update the style.
+ *
+ * LAYOUT
+ *
+ * Keep helper macros near the top. For example, _PIPE() and friends.
+ *
+ * Prefix macros that generally should not be used outside of this file with
+ * underscore '_'. For example, _PIPE() and friends, single instances of
+ * registers that are defined solely for the use by function-like macros.
+ *
+ * Avoid using the underscore prefixed macros outside of this file. There are
+ * exceptions, but keep them to a minimum.
+ *
+ * There are two basic types of register definitions: Single registers and
+ * register groups. Register groups are registers which have two or more
+ * instances, for example one per pipe, port, transcoder, etc. Register groups
+ * should be defined using function-like macros.
+ *
+ * For single registers, define the register offset first, followed by register
+ * contents.
+ *
+ * For register groups, define the register instance offsets first, prefixed
+ * with underscore, followed by a function-like macro choosing the right
+ * instance based on the parameter, followed by register contents.
+ *
+ * Define the register contents from most significant to least significant
+ * bit. Indent the bit and bit field macros using two extra spaces between
+ * #define and the macro name.
+ *
+ * For bit fields, define a _MASK and a _SHIFT macro. Define bit field contents
+ * so that they are already shifted in place, and can be directly OR'd. For
+ * convenience, function-like macros may be used to define bit fields, but do
+ * note that the macros may be needed to read as well as write the register
+ * contents.
+ *
+ * Define bits using (1 << N) instead of BIT(N). We may change this in the
+ * future, but this is the prevailing style.
+ *
+ * Group the register and its contents together without blank lines, separate
+ * from other registers and their contents with one blank line.
+ *
+ * Indent macro values from macro names using TABs. Use braces in macro values
+ * as needed to avoid unintended precedence after macro substitution. Use 
spaces
+ * in macro values according to kernel coding style. Use lower case in
+ * hexadecimal values.
+ *
+ * NAMING
+ *
+ * Try to name registers according to the specs. If the register name changes 
in
+ * the specs from platform to another, stick to the original name.
+ *
+ * Try to re-use existing register macro definitions. Only add new macros for
+ * new register offsets, or when the register contents have changed enough to
+ * warrant a full redefinition.
+ *
+ * When a register or a bit (field) changes for a new platform, prefix the new
+ * macro using the platform acronym or generation. For example, SKL_ or
+ * GEN8_. The prefix signifies the start platform/generation of the register.
+ *
+ * EXAMPLE
+ *
+ * #define _FOO_A  0xf000
+ * #define _FOO_B  0xf001
+ * #define FOO(pipe)   _MMIO_PIPE(pipe, _FOO_A, _FOO_B)
+ * #define   FOO_ENABLE(1 << 31)
+ * #define   FOO_MODE_MASK (0xf << 16)
+ * #define   FOO_MODE_SHIFT16
+ * #define   FOO_MODE_BAR  (0 << 16)
+ * #define   FOO_MODE_BAZ  (1 << 16)
+ * #define   GEN6_FOO_MODE_QUX (2 << 16)
+ *
+ */
+
 typedef struct {
uint32_t reg;
 } i915_reg_t;
-- 
2.11.0

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

Re: [Intel-gfx] [PATCH 00/11] constify i915 attribute_group structures.

[Arvind Yadav]

Hi Lionel,


On Friday 04 August 2017 02:33 PM, Lionel Landwerlin wrote:

Hi Arwind,

These files were generated by a script maintained in this repository : 
https://github.com/rib/gputop/blob/master/scripts/i915-perf-kernelgen.py
It would best to update this script first to make sure future 
platforms get the fixes too.


Some changes have just been merged, deleted most configs but the test 
ones.

You'll need to update your series.

I have done the changes. Please review it. :) Shared patch is 
0001-i915-perf-kernelgen.py-constify-attribute_group-stru.patch.



Otherwise it looks like a good change.

Thanks,

-
Lionel

On 04/08/17 06:03, Arvind Yadav wrote:

attribute_group are not supposed to change at runtime. All functions
working with attribute_group provided by  work with
const attribute_group. So mark the non-const structs as const.

Arvind Yadav (11):
   [PATCH 01/11] drm: i915: i915_oa_kblgt2: constify attribute_group 
structures.
   [PATCH 02/11] drm: i915: i915_oa_bdw: constify attribute_group 
structures.
   [PATCH 03/11] drm: i915: i915_oa_bxt: constify attribute_group 
structures.
   [PATCH 04/11] drm: i915: i915_oa_chv: constify attribute_group 
structures.
   [PATCH 05/11] drm: i915: i915_oa_glk: constify attribute_group 
structures.
   [PATCH 06/11] drm: i915: i915_oa_hsw: constify attribute_group 
structures.
   [PATCH 07/11] drm: i915: i915_oa_kblgt3: constify attribute_group 
structures.
   [PATCH 08/11] drm: i915: i915_oa_sklgt2: constify attribute_group 
structures.
   [PATCH 09/11] drm: i915: i915_oa_sklgt3: constify attribute_group 
structures.
   [PATCH 10/11] drm: i915: i915_oa_sklgt4: constify attribute_group 
structures.
   [PATCH 11/11] drm: i915: i915_sysfs: constify attribute_group 
structures.


  drivers/gpu/drm/i915/i915_oa_bdw.c| 44 
+--

  drivers/gpu/drm/i915/i915_oa_bxt.c| 30 
  drivers/gpu/drm/i915/i915_oa_chv.c| 28 +++---
  drivers/gpu/drm/i915/i915_oa_glk.c| 30 
  drivers/gpu/drm/i915/i915_oa_hsw.c| 12 +-
  drivers/gpu/drm/i915/i915_oa_kblgt2.c | 36 
++--
  drivers/gpu/drm/i915/i915_oa_kblgt3.c | 36 
++--
  drivers/gpu/drm/i915/i915_oa_sklgt2.c | 36 
++--
  drivers/gpu/drm/i915/i915_oa_sklgt3.c | 36 
++--
  drivers/gpu/drm/i915/i915_oa_sklgt4.c | 36 
++--

  drivers/gpu/drm/i915/i915_sysfs.c |  6 ++---
  11 files changed, 165 insertions(+), 165 deletions(-)




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

Re: [Intel-gfx] [PATCH i-g-t] configure.ac: Try to find XMLRPC with xmlrpc-c-config if pkg-config fails

[Arkadiusz Hiler]

On Fri, Aug 04, 2017 at 12:15:18PM +0300, Arkadiusz Hiler wrote:
> On Thu, Aug 03, 2017 at 07:35:33PM +0200, Daniel Vetter wrote:
> > On Thu, Aug 03, 2017 at 04:34:45PM +0300, Petri Latvala wrote:
> > > Debian and Ubuntu have XMLRPC packages without pkg-config files. Let's
> > > do automatically what the user would anyway do manually.
> > > 
> > > Signed-off-by: Petri Latvala 
> > > CC: Daniel Vetter 
> > > CC: Paul Kocialkowski 
> > > CC: Lyude 
> > 
> > Works like I charm, I went right ahead and pushed it.
> > 
> > Lyude, I noticed that you've disabled chamelium by default, but I guess we
> > could also auto-enable (if deps are there) if you feel like.
> > 
> > Thanks, Daniel
> 
> Hey,
> 
> I do not like that "auto-enable if deps are there" and let me explain
> why ;-)
> 
> Chamelium is not very widely used, and most people don't care about it
> being build, so that's why it should not be the default.
> 
> But then if you do care and it haven't build this one time for you the
> "fun" with rootcausing starts.
> 
> I think it's better to make people used to explicitly request chamelium
> to be build and have nice configure-time errors, stating what exactely
> is missing.
> 
> Especially now, when new dependencies for chamelium are being added all
> the time.
> 
> 
> And I believe that should apply to everything - we shouldn't have things
> that may build or may not if you follow exactely the same build steps on
> different machines.
> 
> The components should be either mandatory by default or made "mandatory"
> with a switch.
> 
> -- 
> Cheers,
> Arek

CCing people who may be interested.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] tests/kms_ccs: Fix the color/ccs surface generation

[Daniel Stone]

Hi Jason,

On 4 August 2017 at 01:52, Jason Ekstrand  wrote:
> Previously, the test used the old 64x64 convention that Ville introduced
> for CCS tiles and not the current 128x32 Y-tile convention.  Also, the
> original scheme for generating the CCS data was over-complicated and
> didn't work correctly because it assumed you could cut the main surface
> at an arbitrary Y coordinate.  While you clearly *can* do this (the
> hardware does), it's not a good idea for a generator in a test.  The new
> scheme, introduced here, is entirely based on the relationship between
> cache-lines in the main surface and the CCS that's documented in the
> PRM.  By keeping everything CCS cache-line aligned, our chances of
> generating correct data for an arbitrary-size surface are much higher.

Thanks for fixing this!

> +* We need to cut the surface on a CCS cache-line boundary,
> +* otherwise, we're going to be in trouble when we try to
> +* generate CCS data for the surface.  A cache line in the
> +* CCS is 16x16 cache-line-pairs in the main surface.  16
> +* cache lines is 64 rows high.
> +*/
> +   half_height = ALIGN(height, 128) / 2;
> +   half_size = half_height * stride;
> +   for (i = 0; i < fb->size / 4; i++) {
> +   if (i < half_size / 4)
> +   ptr[i] = RED;
> +   else
> +   ptr[i] = COMPRESSED_RED;

I toyed with:
else if (!(i & 0xe))
ptr[i] = COMPRESSED_RED;
else
ptr[i] = BLACK;

... to make the failure mode even more obvious. But that still writes
in (far) more compressed-red pixels than we strictly need for CCS,
right? Anyway, follow-up patch.

> +static unsigned int
> +y_tile_y_pos(unsigned int offset, unsigned int stride)
>  {
> -   return ptr +
> -   ((y & ~0x3f) * stride) +
> -   ((x & ~0x7) * 64) +
> -   ((y & 0x3f) * 8) +
> -   (x & 7);
> +   unsigned int y_tiles, y;
> +   y_tiles = (offset / 4096) / (stride / 128);
> +   y = y_tiles * 32 + ((offset & 0x1f0) >> 4);
> +   return y;
>  }
>
> @@ -143,12 +168,26 @@ static void display_fb(data_t *data, int compressed)
> size[0] = f.pitches[0] * ALIGN(height, 32);
>
> if (compressed) {
> -   width = ALIGN(f.width, 16) / 16;
> -   height = ALIGN(f.height, 8) / 8;
> -   f.pitches[1] = ALIGN(width * 1, 64);
> +   /* From the Sky Lake PRM, Vol 12, "Color Control Surface":
> +*
> +*"The compression state of the cache-line pair is
> +*specified by 2 bits in the CCS.  Each CCS cache-line
> +*represents an area on the main surface of 16x16 sets
> +*of 128 byte Y-tiled cache-line-pairs. CCS is always Y
> +*tiled."
> +*
> +* A "cache-line-pair" for a Y-tiled surface is two
> +* horizontally adjacent cache lines.  When operating in
> +* bytes and rows, this gives us a scale-down factor of
> +* 32x16.  Since the main surface has a 32-bit format, we
> +* need to multiply width by 4 to get bytes.
> +*/

Yeah, this code and comment match better (& helped clarify) my
understanding of how it works. But given my understanding was mostly
gleaned from other, borderline incomprehensible, comments, as well as
manually poking bits and observing the result, maybe that's not a
ringing endorsement.

> +   width = ALIGN(f.width * 4, 32) / 32;
> +   height = ALIGN(f.height, 16) / 16;
> +   f.pitches[1] = ALIGN(width * 1, 128);
> f.modifier[1] = modifier;
> f.offsets[1] = size[0];
> -   size[1] = f.pitches[1] * ALIGN(height, 64);
> +   size[1] = f.pitches[1] * ALIGN(height, 32);

I changed this to f.height rather than height, because otherwise the
kernel was rejecting the aux buffer for being too small.

With that, it now passes on SKL + APL for me, so I've pushed with my
review. Thanks!

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

Re: [Intel-gfx] [PATCH i-g-t] tests/kms_frontbuffer_tracking: increase FBC wait timeout to 5s

[Lofstedt, Marta]

+Paolo

> -Original Message-
> From: Lofstedt, Marta
> Sent: Wednesday, June 28, 2017 2:17 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: Latvala, Petri ; Lofstedt, Marta
> 
> Subject: [PATCH i-g-t] tests/kms_frontbuffer_tracking: increase FBC wait
> timeout to 5s
> 
> The subtests: igt@kms_frontbuffer_tracking@fbc-*draw*
> has non-consistent results, pending between fail and pass.
> The fails are always due to "FBC disabled".
> With this increase in timeout the flip-flop behavior is no longer 
> reproducible.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101623
> Signed-off-by: Marta Lofstedt 
> ---
>  tests/kms_frontbuffer_tracking.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/kms_frontbuffer_tracking.c
> b/tests/kms_frontbuffer_tracking.c
> index c24e4a81..8bec5d5a 100644
> --- a/tests/kms_frontbuffer_tracking.c
> +++ b/tests/kms_frontbuffer_tracking.c
> @@ -923,7 +923,7 @@ static bool fbc_stride_not_supported(void)
> 
>  static bool fbc_wait_until_enabled(void)  {
> - return igt_wait(fbc_is_enabled(), 2000, 1);
> + return igt_wait(fbc_is_enabled(), 5000, 1);
>  }
> 
>  static bool psr_wait_until_enabled(void)
> --
> 2.11.0

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

[Intel-gfx] [PATCH i-g-t] intel-ci: Also remove extended.testlist from EXTRA_DIST

[Petri Latvala]

Fixes: 848fc49e22c6 ("tests: delete extended.testlist")
Signed-off-by: Petri Latvala 
---
 tests/intel-ci/Makefile.am | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tests/intel-ci/Makefile.am b/tests/intel-ci/Makefile.am
index 9fb9744..d74e7ec 100644
--- a/tests/intel-ci/Makefile.am
+++ b/tests/intel-ci/Makefile.am
@@ -1,5 +1,4 @@
 EXTRA_DIST = \
-   extended.testlist \
fast-feedback.testlist \
generic.testlist \
meta.testlist \
-- 
2.9.3

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

Re: [Intel-gfx] [PATCH 6/6] [v4] drm/i915: Add support for CCS modifiers

[Daniel Stone]

On 3 August 2017 at 18:21, Ben Widawsky  wrote:
> On 17-08-03 12:00:56, Daniel Stone wrote:
>> if (pipe >= PIPE_C || plane >= PLANE_SPRITE1)
>>
>> cf. skl_check_ccs_aux_surface() which rejects CCS on anything other
>> than PRIMARY/SPRITE0.
>>
>> I'll squash when pushing.
>
> Okay, thanks. With universal planes however, I don't think we need such a
> restriction, but whatevs

Speak to Ville about it, I guess? The atomic check path has this check:
switch (plane->id) {
case PLANE_PRIMARY:
case PLANE_SPRITE0:
break;
default:
DRM_DEBUG_KMS("RC support only on plane 1 and 2\n");
return -EINVAL;
}

so if we advertised it on SPRITE1/2 we'd be telling userspace to try a
configuration which could never work ...

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

Re: [Intel-gfx] [PATCH 3/4] drm: Only lastclose on unload for legacy drivers

[Daniel Vetter]

On Fri, Aug 4, 2017 at 6:12 AM, Michel Dänzer  wrote:
> On 03/08/17 10:54 PM, Daniel Vetter wrote:
>> On Thu, Aug 3, 2017 at 1:17 AM, Daniel Vetter  wrote:
>>> On Wed, Aug 2, 2017 at 10:50 PM, Alex Deucher  wrote:
 On Wed, Aug 2, 2017 at 7:56 AM, Daniel Vetter  
 wrote:
> The only thing modern drivers are supposed to do in lastclose is
> restore the fb emulation state. Which is entirely optional, and
> there's really no reason to do that. So restrict it to legacy drivers
> (where the driver cleanup essentially happens in lastclose).

 vga_switcheroo_process_delayed_switch() gets called in lastclose.
 Won't that need to get moved elsewhere for this to work?
>>>
>>> Hm right, I forgot the lazy way to do runtime pm by keeping the device
>>> alive as long as anyone has an open fd for it ... This shouldn't be a
>>> problem, since you need to unregister from vgaswitcheroo anyway on
>>> unload. Maybe that blows up, I'll check the code and augment the patch
>>> as needed.
>>
>> So I think there's 3 cases:
>> - Trying to unload the module. You can't do that while anyone has the
>> fd still open, so lastclose is guaranteeed to run.
>> - Forcefully unbinding the driver through sysfs. Not any better, since
>> the can_switch stuff checks for the open count, and so will delay the
>> delayed switch no matter what.
>
> Are you sure that this is working as intended?
> https://bugs.freedesktop.org/show_bug.cgi?id=100399 sounds like
> unbinding works even while Xorg is using the DRM device.

Unbinding the gpu while someone has a file open (like X) is very much
possible, it's how it's designed. That's the forced unplug case. But:
a) amdgpu doesn't implement actual hotremoval support and blows up
b) even in the unload path we still check for can_switch, which will
notice that there's still open fds and not do the delayed switch, so
the code I've removed is already dead in that case.

But yeah, unbinding while X is running is very much possible, it's
exactly what will happen if you hotremove the gpu (from the driver
model pov) physically. The only thing I tried to show is that my patch
doesn't make anything worse :-)
-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
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t] configure.ac: Try to find XMLRPC with xmlrpc-c-config if pkg-config fails

[Arkadiusz Hiler]

On Thu, Aug 03, 2017 at 07:35:33PM +0200, Daniel Vetter wrote:
> On Thu, Aug 03, 2017 at 04:34:45PM +0300, Petri Latvala wrote:
> > Debian and Ubuntu have XMLRPC packages without pkg-config files. Let's
> > do automatically what the user would anyway do manually.
> > 
> > Signed-off-by: Petri Latvala 
> > CC: Daniel Vetter 
> > CC: Paul Kocialkowski 
> > CC: Lyude 
> 
> Works like I charm, I went right ahead and pushed it.
> 
> Lyude, I noticed that you've disabled chamelium by default, but I guess we
> could also auto-enable (if deps are there) if you feel like.
> 
> Thanks, Daniel

Hey,

I do not like that "auto-enable if deps are there" and let me explain
why ;-)

Chamelium is not very widely used, and most people don't care about it
being build, so that's why it should not be the default.

But then if you do care and it haven't build this one time for you the
"fun" with rootcausing starts.

I think it's better to make people used to explicitly request chamelium
to be build and have nice configure-time errors, stating what exactely
is missing.

Especially now, when new dependencies for chamelium are being added all
the time.


And I believe that should apply to everything - we shouldn't have things
that may build or may not if you follow exactely the same build steps on
different machines.

The components should be either mandatory by default or made "mandatory"
with a switch.

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