subject:"Re\: \[Intel\-gfx\] \[PATCH v9\] drm\/i915\/icl\: Check for fused\-off VDBOX and VEBOX instances"

Re: [Intel-gfx] [PATCH v9] drm/i915/icl: Check for fused-off VDBOX and VEBOX instances

2018-02-28 Thread Sagar Arun Kamble




On 2/28/2018 11:29 PM, Oscar Mateo wrote:



On 2/26/2018 9:49 PM, Sagar Arun Kamble wrote:



On 2/27/2018 4:34 AM, Oscar Mateo wrote:



On 2/25/2018 9:22 PM, Sagar Arun Kamble wrote:



On 2/23/2018 4:35 AM, Oscar Mateo wrote:





+ * We might have detected that some engines are fused off after 
we initialized
+ * the forcewake domains. Prune them, to make sure they only 
reference existing

+ * engines.
+ */
+void intel_uncore_prune(struct drm_i915_private *dev_priv)
+{
+    if (INTEL_GEN(dev_priv) >= 11) {
+    enum forcewake_domains fw_domains = 
dev_priv->uncore.fw_domains;

+    enum forcewake_domain_id domain_id;
+    int i;
+
+    for (i = 0; i < I915_MAX_VCS; i++) {
+    domain_id = FW_DOMAIN_ID_MEDIA_VDBOX0 + i;
+
+    if (HAS_ENGINE(dev_priv, _VCS(i)))
+    continue;
+
+    if (fw_domains & BIT(domain_id))
fw_domains check seems redundant as it is initialized based on 
HAS_ENGINE.

we can just have
if (!HAS_ENGINE(dev_priv, _VCS(i)))
    fw_domain_fini(dev_priv, domain_id);


I don't want to call fw_domain_fini on something we never 
initialized in the first place (e.g. VCS1/3 and VECS1/2/3 on an 
ICL-LP).



Right. Makes sense.
for loop iterating over engines based on ring_mask can help us rely 
on only HAS_ENGINE condition and then we can have complete pruning 
in single for loop.

what do you think?


Hmmm.. I'm not sure I follow: intel_device_info_init_mmio modifies 
ring_mask, so if I loop over engines based on ring_mask I am going 
to miss those I want to prune, right?



Oops ... you are right ...
My suggestion about skipping fw_domains check will not work 
currently. In future may be if we create default ring_mask and 
runtime ring_mask it can be reworked.


Other suggestion to use single for loop (for_each_engine()) can be 
done I think.
It will make it generic for all engine types.  Below is what I am 
thinking of as part of intel_uncore_prune:


for (i = 0; i < ARRAY_SIZE(intel_engines); i++) {
    if (HAS_ENGINE(dev_priv, i))
        continue;
    if (fw_domains & BIT(i))
        fw_domain_fini(dev_priv, i);
}


This won't work either, because the index for engines and forcewake 
domains is different. If you think it is important to make the prune 
function more generic, I can translate between the two (but it will be 
for naught if, as you mentioned, we are working to create separate 
default ring_mask and runtime ring_mask in the future).


Yes. Translation will help (I thought of FW_D_ID_MEDIA to be reused for 
FW_D_ID_MEDIA_VDB0).
I think this patch can go in current shape and will pursue other changes 
as follow up based on inputs.


I feel making it generic will allow pruning to scale across engine types 
(if that is needed in future).
I am not sure if we want to pursue the default/runtime ring_mask change 
(another use case of this that i think is if user wants to know default 
config vs fused)

Tvrtko, Chris - what do you think?

+ fw_domain_fini(dev_priv, domain_id);
+    }
+
+    for (i = 0; i < I915_MAX_VECS; i++) {
+    domain_id = FW_DOMAIN_ID_MEDIA_VEBOX0 + i;
+
+    if (HAS_ENGINE(dev_priv, _VECS(i)))
+    continue;
+
+    if (fw_domains & BIT(domain_id))
+    fw_domain_fini(dev_priv, domain_id);
+    }
+    }
+}
+
  void intel_uncore_fini(struct drm_i915_private *dev_priv)
  {
  /* Paranoia: make sure we have disabled everything before 
we exit. */
diff --git a/drivers/gpu/drm/i915/intel_uncore.h 
b/drivers/gpu/drm/i915/intel_uncore.h

index 53ef77d..28feabf 100644
--- a/drivers/gpu/drm/i915/intel_uncore.h
+++ b/drivers/gpu/drm/i915/intel_uncore.h
@@ -129,6 +129,7 @@ struct intel_uncore {
    void intel_uncore_sanitize(struct drm_i915_private *dev_priv);
  void intel_uncore_init(struct drm_i915_private *dev_priv);
+void intel_uncore_prune(struct drm_i915_private *dev_priv);
  bool intel_uncore_unclaimed_mmio(struct drm_i915_private 
*dev_priv);
  bool intel_uncore_arm_unclaimed_mmio_detection(struct 
drm_i915_private *dev_priv);

  void intel_uncore_fini(struct drm_i915_private *dev_priv);














--
Thanks,
Sagar

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

Re: [Intel-gfx] [PATCH v9] drm/i915/icl: Check for fused-off VDBOX and VEBOX instances

2018-02-28 Thread Oscar Mateo




On 2/26/2018 9:49 PM, Sagar Arun Kamble wrote:



On 2/27/2018 4:34 AM, Oscar Mateo wrote:



On 2/25/2018 9:22 PM, Sagar Arun Kamble wrote:



On 2/23/2018 4:35 AM, Oscar Mateo wrote:





+ * We might have detected that some engines are fused off after 
we initialized
+ * the forcewake domains. Prune them, to make sure they only 
reference existing

+ * engines.
+ */
+void intel_uncore_prune(struct drm_i915_private *dev_priv)
+{
+    if (INTEL_GEN(dev_priv) >= 11) {
+    enum forcewake_domains fw_domains = 
dev_priv->uncore.fw_domains;

+    enum forcewake_domain_id domain_id;
+    int i;
+
+    for (i = 0; i < I915_MAX_VCS; i++) {
+    domain_id = FW_DOMAIN_ID_MEDIA_VDBOX0 + i;
+
+    if (HAS_ENGINE(dev_priv, _VCS(i)))
+    continue;
+
+    if (fw_domains & BIT(domain_id))
fw_domains check seems redundant as it is initialized based on 
HAS_ENGINE.

we can just have
if (!HAS_ENGINE(dev_priv, _VCS(i)))
    fw_domain_fini(dev_priv, domain_id);


I don't want to call fw_domain_fini on something we never 
initialized in the first place (e.g. VCS1/3 and VECS1/2/3 on an 
ICL-LP).



Right. Makes sense.
for loop iterating over engines based on ring_mask can help us rely 
on only HAS_ENGINE condition and then we can have complete pruning 
in single for loop.

what do you think?


Hmmm.. I'm not sure I follow: intel_device_info_init_mmio modifies 
ring_mask, so if I loop over engines based on ring_mask I am going to 
miss those I want to prune, right?



Oops ... you are right ...
My suggestion about skipping fw_domains check will not work currently. 
In future may be if we create default ring_mask and runtime ring_mask 
it can be reworked.


Other suggestion to use single for loop (for_each_engine()) can be 
done I think.
It will make it generic for all engine types.  Below is what I am 
thinking of as part of intel_uncore_prune:


for (i = 0; i < ARRAY_SIZE(intel_engines); i++) {
    if (HAS_ENGINE(dev_priv, i))
        continue;
    if (fw_domains & BIT(i))
        fw_domain_fini(dev_priv, i);
}


This won't work either, because the index for engines and forcewake 
domains is different. If you think it is important to make the prune 
function more generic, I can translate between the two (but it will be 
for naught if, as you mentioned, we are working to create separate 
default ring_mask and runtime ring_mask in the future).



+ fw_domain_fini(dev_priv, domain_id);
+    }
+
+    for (i = 0; i < I915_MAX_VECS; i++) {
+    domain_id = FW_DOMAIN_ID_MEDIA_VEBOX0 + i;
+
+    if (HAS_ENGINE(dev_priv, _VECS(i)))
+    continue;
+
+    if (fw_domains & BIT(domain_id))
+    fw_domain_fini(dev_priv, domain_id);
+    }
+    }
+}
+
  void intel_uncore_fini(struct drm_i915_private *dev_priv)
  {
  /* Paranoia: make sure we have disabled everything before 
we exit. */
diff --git a/drivers/gpu/drm/i915/intel_uncore.h 
b/drivers/gpu/drm/i915/intel_uncore.h

index 53ef77d..28feabf 100644
--- a/drivers/gpu/drm/i915/intel_uncore.h
+++ b/drivers/gpu/drm/i915/intel_uncore.h
@@ -129,6 +129,7 @@ struct intel_uncore {
    void intel_uncore_sanitize(struct drm_i915_private *dev_priv);
  void intel_uncore_init(struct drm_i915_private *dev_priv);
+void intel_uncore_prune(struct drm_i915_private *dev_priv);
  bool intel_uncore_unclaimed_mmio(struct drm_i915_private 
*dev_priv);
  bool intel_uncore_arm_unclaimed_mmio_detection(struct 
drm_i915_private *dev_priv);

  void intel_uncore_fini(struct drm_i915_private *dev_priv);












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

Re: [Intel-gfx] [PATCH v9] drm/i915/icl: Check for fused-off VDBOX and VEBOX instances

2018-02-26 Thread Sagar Arun Kamble




On 2/27/2018 4:34 AM, Oscar Mateo wrote:



On 2/25/2018 9:22 PM, Sagar Arun Kamble wrote:



On 2/23/2018 4:35 AM, Oscar Mateo wrote:





+ * We might have detected that some engines are fused off after 
we initialized
+ * the forcewake domains. Prune them, to make sure they only 
reference existing

+ * engines.
+ */
+void intel_uncore_prune(struct drm_i915_private *dev_priv)
+{
+    if (INTEL_GEN(dev_priv) >= 11) {
+    enum forcewake_domains fw_domains = 
dev_priv->uncore.fw_domains;

+    enum forcewake_domain_id domain_id;
+    int i;
+
+    for (i = 0; i < I915_MAX_VCS; i++) {
+    domain_id = FW_DOMAIN_ID_MEDIA_VDBOX0 + i;
+
+    if (HAS_ENGINE(dev_priv, _VCS(i)))
+    continue;
+
+    if (fw_domains & BIT(domain_id))
fw_domains check seems redundant as it is initialized based on 
HAS_ENGINE.

we can just have
if (!HAS_ENGINE(dev_priv, _VCS(i)))
    fw_domain_fini(dev_priv, domain_id);


I don't want to call fw_domain_fini on something we never 
initialized in the first place (e.g. VCS1/3 and VECS1/2/3 on an 
ICL-LP).



Right. Makes sense.
for loop iterating over engines based on ring_mask can help us rely 
on only HAS_ENGINE condition and then we can have complete pruning in 
single for loop.

what do you think?


Hmmm.. I'm not sure I follow: intel_device_info_init_mmio modifies 
ring_mask, so if I loop over engines based on ring_mask I am going to 
miss those I want to prune, right?



Oops ... you are right ...
My suggestion about skipping fw_domains check will not work currently. 
In future may be if we create default ring_mask and runtime ring_mask it 
can be reworked.


Other suggestion to use single for loop (for_each_engine()) can be done 
I think.
It will make it generic for all engine types.  Below is what I am 
thinking of as part of intel_uncore_prune:


for (i = 0; i < ARRAY_SIZE(intel_engines); i++) {
    if (HAS_ENGINE(dev_priv, i))
        continue;
    if (fw_domains & BIT(i))
        fw_domain_fini(dev_priv, i);
}

+ fw_domain_fini(dev_priv, domain_id);
+    }
+
+    for (i = 0; i < I915_MAX_VECS; i++) {
+    domain_id = FW_DOMAIN_ID_MEDIA_VEBOX0 + i;
+
+    if (HAS_ENGINE(dev_priv, _VECS(i)))
+    continue;
+
+    if (fw_domains & BIT(domain_id))
+    fw_domain_fini(dev_priv, domain_id);
+    }
+    }
+}
+
  void intel_uncore_fini(struct drm_i915_private *dev_priv)
  {
  /* Paranoia: make sure we have disabled everything before we 
exit. */
diff --git a/drivers/gpu/drm/i915/intel_uncore.h 
b/drivers/gpu/drm/i915/intel_uncore.h

index 53ef77d..28feabf 100644
--- a/drivers/gpu/drm/i915/intel_uncore.h
+++ b/drivers/gpu/drm/i915/intel_uncore.h
@@ -129,6 +129,7 @@ struct intel_uncore {
    void intel_uncore_sanitize(struct drm_i915_private *dev_priv);
  void intel_uncore_init(struct drm_i915_private *dev_priv);
+void intel_uncore_prune(struct drm_i915_private *dev_priv);
  bool intel_uncore_unclaimed_mmio(struct drm_i915_private 
*dev_priv);
  bool intel_uncore_arm_unclaimed_mmio_detection(struct 
drm_i915_private *dev_priv);

  void intel_uncore_fini(struct drm_i915_private *dev_priv);










--
Thanks,
Sagar

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

Re: [Intel-gfx] [PATCH v9] drm/i915/icl: Check for fused-off VDBOX and VEBOX instances

2018-02-26 Thread Oscar Mateo




On 2/25/2018 9:22 PM, Sagar Arun Kamble wrote:



On 2/23/2018 4:35 AM, Oscar Mateo wrote:





+ * We might have detected that some engines are fused off after we 
initialized
+ * the forcewake domains. Prune them, to make sure they only 
reference existing

+ * engines.
+ */
+void intel_uncore_prune(struct drm_i915_private *dev_priv)
+{
+    if (INTEL_GEN(dev_priv) >= 11) {
+    enum forcewake_domains fw_domains = 
dev_priv->uncore.fw_domains;

+    enum forcewake_domain_id domain_id;
+    int i;
+
+    for (i = 0; i < I915_MAX_VCS; i++) {
+    domain_id = FW_DOMAIN_ID_MEDIA_VDBOX0 + i;
+
+    if (HAS_ENGINE(dev_priv, _VCS(i)))
+    continue;
+
+    if (fw_domains & BIT(domain_id))
fw_domains check seems redundant as it is initialized based on 
HAS_ENGINE.

we can just have
if (!HAS_ENGINE(dev_priv, _VCS(i)))
    fw_domain_fini(dev_priv, domain_id);


I don't want to call fw_domain_fini on something we never initialized 
in the first place (e.g. VCS1/3 and VECS1/2/3 on an ICL-LP).



Right. Makes sense.
for loop iterating over engines based on ring_mask can help us rely on 
only HAS_ENGINE condition and then we can have complete pruning in 
single for loop.

what do you think?


Hmmm.. I'm not sure I follow: intel_device_info_init_mmio modifies 
ring_mask, so if I loop over engines based on ring_mask I am going to 
miss those I want to prune, right?



+ fw_domain_fini(dev_priv, domain_id);
+    }
+
+    for (i = 0; i < I915_MAX_VECS; i++) {
+    domain_id = FW_DOMAIN_ID_MEDIA_VEBOX0 + i;
+
+    if (HAS_ENGINE(dev_priv, _VECS(i)))
+    continue;
+
+    if (fw_domains & BIT(domain_id))
+    fw_domain_fini(dev_priv, domain_id);
+    }
+    }
+}
+
  void intel_uncore_fini(struct drm_i915_private *dev_priv)
  {
  /* Paranoia: make sure we have disabled everything before we 
exit. */
diff --git a/drivers/gpu/drm/i915/intel_uncore.h 
b/drivers/gpu/drm/i915/intel_uncore.h

index 53ef77d..28feabf 100644
--- a/drivers/gpu/drm/i915/intel_uncore.h
+++ b/drivers/gpu/drm/i915/intel_uncore.h
@@ -129,6 +129,7 @@ struct intel_uncore {
    void intel_uncore_sanitize(struct drm_i915_private *dev_priv);
  void intel_uncore_init(struct drm_i915_private *dev_priv);
+void intel_uncore_prune(struct drm_i915_private *dev_priv);
  bool intel_uncore_unclaimed_mmio(struct drm_i915_private *dev_priv);
  bool intel_uncore_arm_unclaimed_mmio_detection(struct 
drm_i915_private *dev_priv);

  void intel_uncore_fini(struct drm_i915_private *dev_priv);








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

Re: [Intel-gfx] [PATCH v9] drm/i915/icl: Check for fused-off VDBOX and VEBOX instances

2018-02-25 Thread Sagar Arun Kamble




On 2/23/2018 4:35 AM, Oscar Mateo wrote:





+ * We might have detected that some engines are fused off after we 
initialized
+ * the forcewake domains. Prune them, to make sure they only 
reference existing

+ * engines.
+ */
+void intel_uncore_prune(struct drm_i915_private *dev_priv)
+{
+    if (INTEL_GEN(dev_priv) >= 11) {
+    enum forcewake_domains fw_domains = 
dev_priv->uncore.fw_domains;

+    enum forcewake_domain_id domain_id;
+    int i;
+
+    for (i = 0; i < I915_MAX_VCS; i++) {
+    domain_id = FW_DOMAIN_ID_MEDIA_VDBOX0 + i;
+
+    if (HAS_ENGINE(dev_priv, _VCS(i)))
+    continue;
+
+    if (fw_domains & BIT(domain_id))
fw_domains check seems redundant as it is initialized based on 
HAS_ENGINE.

we can just have
if (!HAS_ENGINE(dev_priv, _VCS(i)))
    fw_domain_fini(dev_priv, domain_id);


I don't want to call fw_domain_fini on something we never initialized 
in the first place (e.g. VCS1/3 and VECS1/2/3 on an ICL-LP).



Right. Makes sense.
for loop iterating over engines based on ring_mask can help us rely on 
only HAS_ENGINE condition and then we can have complete pruning in 
single for loop.

what do you think?

+ fw_domain_fini(dev_priv, domain_id);
+    }
+
+    for (i = 0; i < I915_MAX_VECS; i++) {
+    domain_id = FW_DOMAIN_ID_MEDIA_VEBOX0 + i;
+
+    if (HAS_ENGINE(dev_priv, _VECS(i)))
+    continue;
+
+    if (fw_domains & BIT(domain_id))
+    fw_domain_fini(dev_priv, domain_id);
+    }
+    }
+}
+
  void intel_uncore_fini(struct drm_i915_private *dev_priv)
  {
  /* Paranoia: make sure we have disabled everything before we 
exit. */
diff --git a/drivers/gpu/drm/i915/intel_uncore.h 
b/drivers/gpu/drm/i915/intel_uncore.h

index 53ef77d..28feabf 100644
--- a/drivers/gpu/drm/i915/intel_uncore.h
+++ b/drivers/gpu/drm/i915/intel_uncore.h
@@ -129,6 +129,7 @@ struct intel_uncore {
    void intel_uncore_sanitize(struct drm_i915_private *dev_priv);
  void intel_uncore_init(struct drm_i915_private *dev_priv);
+void intel_uncore_prune(struct drm_i915_private *dev_priv);
  bool intel_uncore_unclaimed_mmio(struct drm_i915_private *dev_priv);
  bool intel_uncore_arm_unclaimed_mmio_detection(struct 
drm_i915_private *dev_priv);

  void intel_uncore_fini(struct drm_i915_private *dev_priv);






--
Thanks,
Sagar

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

Re: [Intel-gfx] [PATCH v9] drm/i915/icl: Check for fused-off VDBOX and VEBOX instances

2018-02-22 Thread kbuild test robot

Hi Oscar,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on next-20180222]
[cannot apply to v4.16-rc2]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Oscar-Mateo/drm-i915-icl-Check-for-fused-off-VDBOX-and-VEBOX-instances/20180223-095336
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: i386-randconfig-a1-201807 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/gpu/drm/i915/intel_device_info.c: In function 
'intel_device_info_init_mmio':
>> drivers/gpu/drm/i915/intel_device_info.c:619:18: error: 'I915_MAX_VCS' 
>> undeclared (first use in this function)
 for (i = 0; i < I915_MAX_VCS; i++) {
 ^
   drivers/gpu/drm/i915/intel_device_info.c:619:18: note: each undeclared 
identifier is reported only once for each function it appears in
>> drivers/gpu/drm/i915/intel_device_info.c:631:18: error: 'I915_MAX_VECS' 
>> undeclared (first use in this function)
 for (i = 0; i < I915_MAX_VECS; i++) {
 ^
>> drivers/gpu/drm/i915/intel_device_info.c:632:3: error: implicit declaration 
>> of function '_VECS' [-Werror=implicit-function-declaration]
  if (!HAS_ENGINE(dev_priv, _VECS(i)))
  ^
   cc1: some warnings being treated as errors
--
   drivers/gpu/drm/i915/intel_uncore.c: In function 'intel_uncore_prune':
   drivers/gpu/drm/i915/intel_uncore.c:1468:19: error: 'I915_MAX_VCS' 
undeclared (first use in this function)
  for (i = 0; i < I915_MAX_VCS; i++) {
  ^
   drivers/gpu/drm/i915/intel_uncore.c:1468:19: note: each undeclared 
identifier is reported only once for each function it appears in
>> drivers/gpu/drm/i915/intel_uncore.c:1469:16: error: 
>> 'FW_DOMAIN_ID_MEDIA_VDBOX0' undeclared (first use in this function)
   domain_id = FW_DOMAIN_ID_MEDIA_VDBOX0 + i;
   ^
   drivers/gpu/drm/i915/intel_uncore.c:1478:19: error: 'I915_MAX_VECS' 
undeclared (first use in this function)
  for (i = 0; i < I915_MAX_VECS; i++) {
  ^
>> drivers/gpu/drm/i915/intel_uncore.c:1479:16: error: 
>> 'FW_DOMAIN_ID_MEDIA_VEBOX0' undeclared (first use in this function)
   domain_id = FW_DOMAIN_ID_MEDIA_VEBOX0 + i;
   ^
   drivers/gpu/drm/i915/intel_uncore.c:1481:4: error: implicit declaration of 
function '_VECS' [-Werror=implicit-function-declaration]
   if (HAS_ENGINE(dev_priv, _VECS(i)))
   ^
   cc1: some warnings being treated as errors

vim +/I915_MAX_VCS +619 drivers/gpu/drm/i915/intel_device_info.c

   595  
   596  /*
   597   * Determine which engines are fused off in our particular hardware. 
Since the
   598   * fuse register is in the blitter powerwell, we need forcewake to be 
ready at
   599   * this point (but later we need to prune the forcewake domains for 
engines that
   600   * are indeed fused off).
   601   */
   602  void intel_device_info_init_mmio(struct drm_i915_private *dev_priv)
   603  {
   604  struct intel_device_info *info = mkwrite_device_info(dev_priv);
   605  u8 vdbox_disable, vebox_disable;
   606  u32 media_fuse;
   607  int i;
   608  
   609  if (INTEL_GEN(dev_priv) < 11)
   610  return;
   611  
   612  media_fuse = I915_READ(GEN11_GT_VEBOX_VDBOX_DISABLE);
   613  
   614  vdbox_disable = media_fuse & GEN11_GT_VDBOX_DISABLE_MASK;
   615  vebox_disable = (media_fuse & GEN11_GT_VEBOX_DISABLE_MASK) >>
   616  GEN11_GT_VEBOX_DISABLE_SHIFT;
   617  
   618  DRM_DEBUG_DRIVER("vdbox disable: %04x\n", vdbox_disable);
 > 619  for (i = 0; i < I915_MAX_VCS; i++) {
   620  if (!HAS_ENGINE(dev_priv, _VCS(i)))
   621  continue;
   622  
   623  if (!(BIT(i) & vdbox_disable))
   624  continue;
   625  
   626  info->ring_mask &= ~ENGINE_MASK(_VCS(i));
   627  DRM_DEBUG_DRIVER("vcs%u fused off\n", i);
   628  }
   629  
   630  DRM_DEBUG_DRIVER("vebox disable: %04x\n", vebox_disable);
 > 631  for (i = 0; i < I915_MAX_VECS; i++) {
 > 632  if (!HAS_ENGINE(dev_priv, _VECS(i)))

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v9] drm/i915/icl: Check for fused-off VDBOX and VEBOX instances

2018-02-22 Thread kbuild test robot

Hi Oscar,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on next-20180222]
[cannot apply to v4.16-rc2]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Oscar-Mateo/drm-i915-icl-Check-for-fused-off-VDBOX-and-VEBOX-instances/20180223-095336
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: i386-randconfig-x004-201807 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All error/warnings (new ones prefixed by >>):

   drivers/gpu/drm/i915/intel_device_info.c: In function 
'intel_device_info_init_mmio':
>> drivers/gpu/drm/i915/intel_device_info.c:619:18: error: 'I915_MAX_VCS' 
>> undeclared (first use in this function); did you mean 'I915_MAP_WC'?
 for (i = 0; i < I915_MAX_VCS; i++) {
 ^~~~
 I915_MAP_WC
   drivers/gpu/drm/i915/intel_device_info.c:619:18: note: each undeclared 
identifier is reported only once for each function it appears in
>> drivers/gpu/drm/i915/intel_device_info.c:631:18: error: 'I915_MAX_VECS' 
>> undeclared (first use in this function); did you mean 'I915_MAX_VCS'?
 for (i = 0; i < I915_MAX_VECS; i++) {
 ^
 I915_MAX_VCS
   In file included from include/linux/kernel.h:11:0,
from include/asm-generic/bug.h:18,
from arch/x86/include/asm/bug.h:82,
from include/linux/bug.h:5,
from include/linux/seq_file.h:7,
from include/drm/drm_print.h:31,
from drivers/gpu/drm/i915/intel_device_info.c:25:
>> drivers/gpu/drm/i915/intel_device_info.c:632:29: error: implicit declaration 
>> of function '_VECS'; did you mean '_VCS'? 
>> [-Werror=implicit-function-declaration]
  if (!HAS_ENGINE(dev_priv, _VECS(i)))
^
   include/linux/bitops.h:7:28: note: in definition of macro 'BIT'
#define BIT(nr)   (1UL << (nr))
   ^~
   drivers/gpu/drm/i915/i915_drv.h:2752:35: note: in expansion of macro 
'ENGINE_MASK'
 (!!((dev_priv)->info.ring_mask & ENGINE_MASK(id)))
  ^~~
>> drivers/gpu/drm/i915/intel_device_info.c:632:8: note: in expansion of macro 
>> 'HAS_ENGINE'
  if (!HAS_ENGINE(dev_priv, _VECS(i)))
   ^~
   cc1: some warnings being treated as errors
--
   drivers/gpu/drm/i915/intel_uncore.c: In function 'intel_uncore_prune':
>> drivers/gpu/drm/i915/intel_uncore.c:1468:19: error: 'I915_MAX_VCS' 
>> undeclared (first use in this function); did you mean 'I915_MAP_WC'?
  for (i = 0; i < I915_MAX_VCS; i++) {
  ^~~~
  I915_MAP_WC
   drivers/gpu/drm/i915/intel_uncore.c:1468:19: note: each undeclared 
identifier is reported only once for each function it appears in
>> drivers/gpu/drm/i915/intel_uncore.c:1469:16: error: 
>> 'FW_DOMAIN_ID_MEDIA_VDBOX0' undeclared (first use in this function); did you 
>> mean 'FW_DOMAIN_ID_MEDIA'?
   domain_id = FW_DOMAIN_ID_MEDIA_VDBOX0 + i;
   ^
   FW_DOMAIN_ID_MEDIA
>> drivers/gpu/drm/i915/intel_uncore.c:1478:19: error: 'I915_MAX_VECS' 
>> undeclared (first use in this function); did you mean 'I915_MAX_VCS'?
  for (i = 0; i < I915_MAX_VECS; i++) {
  ^
  I915_MAX_VCS
>> drivers/gpu/drm/i915/intel_uncore.c:1479:16: error: 
>> 'FW_DOMAIN_ID_MEDIA_VEBOX0' undeclared (first use in this function); did you 
>> mean 'FW_DOMAIN_ID_MEDIA_VDBOX0'?
   domain_id = FW_DOMAIN_ID_MEDIA_VEBOX0 + i;
   ^
   FW_DOMAIN_ID_MEDIA_VDBOX0
   In file included from include/linux/kernel.h:11:0,
from include/asm-generic/bug.h:18,
from arch/x86/include/asm/bug.h:82,
from include/linux/bug.h:5,
from include/linux/mmdebug.h:5,
from include/linux/gfp.h:5,
from include/linux/slab.h:15,
from include/linux/io-mapping.h:22,
from drivers/gpu/drm/i915/i915_drv.h:36,
from drivers/gpu/drm/i915/intel_uncore.c:24:
>> drivers/gpu/drm/i915/intel_uncore.c:1481:29: error: implicit declaration of 
>> function '_VECS'; did you mean '_VCS'? 
>> [-Werror=implicit-function-declaration]
   if (HAS_ENGINE(dev_priv, _VECS(i)))
^
   include/linux/bitops.h:7:28: note: in definition of macro 'BIT'
#define BIT(nr)   (1UL << (nr))
   ^~
   drivers/gpu/drm/i915/i915_drv.h:2752:35: note: in expansion of macro 
'ENGINE_MASK'

Re: [Intel-gfx] [PATCH v9] drm/i915/icl: Check for fused-off VDBOX and VEBOX instances

2018-02-22 Thread Oscar Mateo




On 2/21/2018 10:17 PM, Sagar Arun Kamble wrote:
Looks good to me. Few cosmetic changes suggested below. With those 
addressed:

Reviewed-by: Sagar Arun Kamble 

On 2/22/2018 5:05 AM, Oscar Mateo wrote:

In Gen11, the Video Decode engines (aka VDBOX, aka VCS, aka BSD) and the
Video Enhancement engines (aka VEBOX, aka VECS) could be fused off. 
Also,
each VDBOX and VEBOX has its own power well, which only exist if the 
related

engine exists in the HW.

Unfortunately, we have a Catch-22 situation going on: we need the 
blitter
forcewake to read the register with the fuse info, but we cannot 
initialize
the forcewake domains without knowin about the engines present in the 
HW.

*knowing
We workaround this problem by pruning the forcewake domains after 
reading

the fuse information.

This line can be re-framed like:
"We workaround this problem by allowing initialization of all 
forcewake domains and then pruning the fused off forcewake domains

based on fuse info which can be read acquiring blitter forcewake"


Can do.



Bspec: 20680

v2: We were shifting incorrectly for vebox disable (Vinay)

v3: Assert mmio is ready and warn if we have attempted to initialize
 forcewake for fused-off engines (Paulo)

v4:
   - Use INTEL_GEN in new code (Tvrtko)
   - Shorter local variable (Tvrtko, Michal)
   - Keep "if (!...) continue" style (Tvrtko)
   - No unnecessary BUG_ON (Tvrtko)
   - WARN_ON and cleanup if wrong mask (Tvrtko, Michal)
   - Use I915_READ_FW (Michal)
   - Use I915_MAX_VCS/VECS macros (Michal)

v5: Rebased by Rodrigo fixing conflicts on top of:
 commit 33def1ff7b0 ("drm/i915: Simplify intel_engines_init")

v6: Fix v5. Remove info->num_rings. (by Oscar)

v7: Rebase (Rodrigo).

v8:
   - 
s/intel_device_info_fused_off_engines/intel_device_info_init_mmio 
(Chris)

   - Make vdbox_disable & vebox_disable local variables (Chris)

v9:
   - Move function declaration to intel_device_info.h (Michal)
   - Missing indent in bit fields definitions (Michal)
   - When RC6 is enabled by BIOS, the fuse register cannot be read until
 the blitter powerwell is awake. Shuffle where the fuse is read, 
prune

 the forcewake domains after the fact and change the commit message
 accordingly (Vinay, Sagar, Chris).

Cc: Paulo Zanoni 
Cc: Vinay Belgaumkar 
Cc: Tvrtko Ursulin 
Cc: Michal Wajdeczko 
Cc: Chris Wilson 
Cc: Daniele Ceraolo Spurio 
Cc: Sagar Arun Kamble 
Signed-off-by: Rodrigo Vivi 
Signed-off-by: Oscar Mateo 
---
  drivers/gpu/drm/i915/i915_drv.c  |  4 +++
  drivers/gpu/drm/i915/i915_reg.h  |  5 +++
  drivers/gpu/drm/i915/intel_device_info.c | 47 
+++

  drivers/gpu/drm/i915/intel_device_info.h |  1 +
  drivers/gpu/drm/i915/intel_uncore.c  | 55 


  drivers/gpu/drm/i915/intel_uncore.h  |  1 +
  6 files changed, 113 insertions(+)

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

index d09f8e6..2269b56 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1031,6 +1031,10 @@ static int i915_driver_init_mmio(struct 
drm_i915_private *dev_priv)

    intel_uncore_init(dev_priv);
  +    intel_device_info_init_mmio(dev_priv);
+
+    intel_uncore_prune(dev_priv);
+
  intel_uc_init_mmio(dev_priv);
    ret = intel_engines_init_mmio(dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_reg.h 
b/drivers/gpu/drm/i915/i915_reg.h

index 784d79c..e6a0d84 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2854,6 +2854,11 @@ enum i915_power_well_id {
  #define GEN10_EU_DISABLE3    _MMIO(0x9140)
  #define   GEN10_EU_DIS_SS_MASK    0xff
  +#define GEN11_GT_VEBOX_VDBOX_DISABLE    _MMIO(0x9140)
+#define   GEN11_GT_VDBOX_DISABLE_MASK    0xff
+#define   GEN11_GT_VEBOX_DISABLE_SHIFT    16
+#define   GEN11_GT_VEBOX_DISABLE_MASK    (0xff << 
GEN11_GT_VEBOX_DISABLE_SHIFT)

+
  #define GEN6_BSD_SLEEP_PSMI_CONTROL    _MMIO(0x12050)
  #define   GEN6_BSD_SLEEP_MSG_DISABLE    (1 << 0)
  #define   GEN6_BSD_SLEEP_FLUSH_DISABLE    (1 << 2)
diff --git a/drivers/gpu/drm/i915/intel_device_info.c 
b/drivers/gpu/drm/i915/intel_device_info.c

index 9352f34..70ea654 100644
--- a/drivers/gpu/drm/i915/intel_device_info.c
+++ b/drivers/gpu/drm/i915/intel_device_info.c
@@ -595,3 +595,50 @@ void intel_driver_caps_print(const struct 
intel_driver_caps *caps,

  {
  drm_printf(p, "scheduler: %x\n", caps->scheduler);
  }
+
+/*
+ * Determine which engines are fused off in our particular hardware. 
Since the
+ * fuse register is in the blitter powerwell, we need forcewake to 
be ready at
+ * this point (but later we need to prune the forcewake domains for 
engines that

+ * are indeed fused

Re: [Intel-gfx] [PATCH v9] drm/i915/icl: Check for fused-off VDBOX and VEBOX instances

2018-02-21 Thread Sagar Arun Kamble

Looks good to me. Few cosmetic changes suggested below. With those 
addressed:

Reviewed-by: Sagar Arun Kamble 

On 2/22/2018 5:05 AM, Oscar Mateo wrote:

In Gen11, the Video Decode engines (aka VDBOX, aka VCS, aka BSD) and the
Video Enhancement engines (aka VEBOX, aka VECS) could be fused off. Also,
each VDBOX and VEBOX has its own power well, which only exist if the related
engine exists in the HW.

Unfortunately, we have a Catch-22 situation going on: we need the blitter
forcewake to read the register with the fuse info, but we cannot initialize
the forcewake domains without knowin about the engines present in the HW.

*knowing

We workaround this problem by pruning the forcewake domains after reading
the fuse information.

This line can be re-framed like:
"We workaround this problem by allowing initialization of all forcewake 
domains and then pruning the fused off forcewake domains

based on fuse info which can be read acquiring blitter forcewake"


Bspec: 20680

v2: We were shifting incorrectly for vebox disable (Vinay)

v3: Assert mmio is ready and warn if we have attempted to initialize
 forcewake for fused-off engines (Paulo)

v4:
   - Use INTEL_GEN in new code (Tvrtko)
   - Shorter local variable (Tvrtko, Michal)
   - Keep "if (!...) continue" style (Tvrtko)
   - No unnecessary BUG_ON (Tvrtko)
   - WARN_ON and cleanup if wrong mask (Tvrtko, Michal)
   - Use I915_READ_FW (Michal)
   - Use I915_MAX_VCS/VECS macros (Michal)

v5: Rebased by Rodrigo fixing conflicts on top of:
 commit 33def1ff7b0 ("drm/i915: Simplify intel_engines_init")

v6: Fix v5. Remove info->num_rings. (by Oscar)

v7: Rebase (Rodrigo).

v8:
   - s/intel_device_info_fused_off_engines/intel_device_info_init_mmio (Chris)
   - Make vdbox_disable & vebox_disable local variables (Chris)

v9:
   - Move function declaration to intel_device_info.h (Michal)
   - Missing indent in bit fields definitions (Michal)
   - When RC6 is enabled by BIOS, the fuse register cannot be read until
 the blitter powerwell is awake. Shuffle where the fuse is read, prune
 the forcewake domains after the fact and change the commit message
 accordingly (Vinay, Sagar, Chris).

Cc: Paulo Zanoni 
Cc: Vinay Belgaumkar 
Cc: Tvrtko Ursulin 
Cc: Michal Wajdeczko 
Cc: Chris Wilson 
Cc: Daniele Ceraolo Spurio 
Cc: Sagar Arun Kamble 
Signed-off-by: Rodrigo Vivi 
Signed-off-by: Oscar Mateo 
---
  drivers/gpu/drm/i915/i915_drv.c  |  4 +++
  drivers/gpu/drm/i915/i915_reg.h  |  5 +++
  drivers/gpu/drm/i915/intel_device_info.c | 47 +++
  drivers/gpu/drm/i915/intel_device_info.h |  1 +
  drivers/gpu/drm/i915/intel_uncore.c  | 55 
  drivers/gpu/drm/i915/intel_uncore.h  |  1 +
  6 files changed, 113 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index d09f8e6..2269b56 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1031,6 +1031,10 @@ static int i915_driver_init_mmio(struct drm_i915_private 
*dev_priv)
  
  	intel_uncore_init(dev_priv);
  
+	intel_device_info_init_mmio(dev_priv);

+
+   intel_uncore_prune(dev_priv);
+
intel_uc_init_mmio(dev_priv);
  
  	ret = intel_engines_init_mmio(dev_priv);

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 784d79c..e6a0d84 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2854,6 +2854,11 @@ enum i915_power_well_id {
  #define GEN10_EU_DISABLE3 _MMIO(0x9140)
  #define   GEN10_EU_DIS_SS_MASK0xff
  
+#define GEN11_GT_VEBOX_VDBOX_DISABLE	_MMIO(0x9140)

+#define   GEN11_GT_VDBOX_DISABLE_MASK  0xff
+#define   GEN11_GT_VEBOX_DISABLE_SHIFT 16
+#define   GEN11_GT_VEBOX_DISABLE_MASK  (0xff << GEN11_GT_VEBOX_DISABLE_SHIFT)
+
  #define GEN6_BSD_SLEEP_PSMI_CONTROL   _MMIO(0x12050)
  #define   GEN6_BSD_SLEEP_MSG_DISABLE  (1 << 0)
  #define   GEN6_BSD_SLEEP_FLUSH_DISABLE(1 << 2)
diff --git a/drivers/gpu/drm/i915/intel_device_info.c 
b/drivers/gpu/drm/i915/intel_device_info.c
index 9352f34..70ea654 100644
--- a/drivers/gpu/drm/i915/intel_device_info.c
+++ b/drivers/gpu/drm/i915/intel_device_info.c
@@ -595,3 +595,50 @@ void intel_driver_caps_print(const struct 
intel_driver_caps *caps,
  {
drm_printf(p, "scheduler: %x\n", caps->scheduler);
  }
+
+/*
+ * Determine which engines are fused off in our particular hardware. Since the
+ * fuse register is in the blitter powerwell, we need forcewake to be ready at
+ * this point (but later we need to prune the forcewake domains for engines 
that
+ * are indeed fused off).
+ */
+void intel_device_info_init_mmio(struct drm_i915_private *dev_priv)

Re: [Intel-gfx] [PATCH v9] drm/i915/icl: Check for fused-off VDBOX and VEBOX instances

Re: [Intel-gfx] [PATCH v9] drm/i915/icl: Check for fused-off VDBOX and VEBOX instances

Re: [Intel-gfx] [PATCH v9] drm/i915/icl: Check for fused-off VDBOX and VEBOX instances

Re: [Intel-gfx] [PATCH v9] drm/i915/icl: Check for fused-off VDBOX and VEBOX instances

Re: [Intel-gfx] [PATCH v9] drm/i915/icl: Check for fused-off VDBOX and VEBOX instances

Re: [Intel-gfx] [PATCH v9] drm/i915/icl: Check for fused-off VDBOX and VEBOX instances

Re: [Intel-gfx] [PATCH v9] drm/i915/icl: Check for fused-off VDBOX and VEBOX instances

Re: [Intel-gfx] [PATCH v9] drm/i915/icl: Check for fused-off VDBOX and VEBOX instances

Re: [Intel-gfx] [PATCH v9] drm/i915/icl: Check for fused-off VDBOX and VEBOX instances

9 matches

Site Navigation

Mail list logo

Footer information