Re: [Intel-gfx] [PATCH v10 10/11] drm: Add aspect ratio parsing in DRM layer

[Nautiyal, Ankit K]

On 4/6/2018 11:14 PM, Ville Syrjälä wrote:

On Fri, Apr 06, 2018 at 10:55:14PM +0530, Nautiyal, Ankit K wrote:

This patch is causing failure of IGT test kms_3d. The kms_3d test
expects the no. of 3d modes to be 13.

(The test has hard-coded value for expected no. of 3d modes as 13)

But due to the addition of "matching aspect_ratio" in drm_mode_equal in
this patch, the total no. of

modes in the connector modelist is increased by 2, resulting in failure
of assertion 'mode_count==13'.

If kms_3d isn't setting the aspect ratio cap how is it affected by these
changes?

In drm_mode.c, the drm_mode_connector_list_update() uses drm_mode_equal,
to remove duplicate modes in connector_modes from the 
connector->probe_modes.
Earlier, it wasn't matching for aspect-ratio and was discarding two of 
the modes with aspect ratio,

as duplicates of other modes in the list.

Later, when we are pruning the modes in drm_mode_get_connector, the 
logic there assumes,
that the modes are in a sorted order so that we just match with the last 
valid mode for uniqueness.

This isn't the case with the spoofed edid in kms_3d.
Earlier, I was thinking if we should change the no. of expected modes in 
kms_3d,

but that's not correct approach.

So finally, The pruning logic needs to be changed, to do away with any 
assumption and check
all the modes in the list for duplicates. This however will take more 
time to remove duplicates.


Any other suggestions on this?

Regards
-Ankit




Perhaps this need to be handled in the test.

-Regards,

Ankit


On 4/6/2018 10:34 PM, Nautiyal, Ankit K wrote:

From: "Sharma, Shashank" 

Current DRM layer functions don't parse aspect ratio information
while converting a user mode->kernel mode or vice versa. This
causes modeset to pick mode with wrong aspect ratio, eventually
causing failures in HDMI compliance test cases, due to wrong VIC.

This patch adds aspect ratio information in DRM's mode conversion
and mode comparision functions, to make sure kernel picks mode
with right aspect ratio (as per the VIC).

Background:
This patch was once reviewed and merged, and later reverted due to
lack of DRM cap protection. This is a re-spin of this patch, this
time with DRM cap protection, to avoid aspect ratio information, when
the client doesn't request for it.

Review link: https://pw-emeril.freedesktop.org/patch/104068/
Background discussion: https://patchwork.kernel.org/patch/9379057/

Signed-off-by: Shashank Sharma 
Signed-off-by: Lin, Jia 
Signed-off-by: Akashdeep Sharma 
Reviewed-by: Jim Bride  (V2)
Reviewed-by: Jose Abreu  (V4)

Cc: Ville Syrjala 
Cc: Jim Bride 
Cc: Jose Abreu 
Cc: Ankit Nautiyal 

V3: modified the aspect-ratio check in drm_mode_equal as per new flags
  provided by Ville. https://patchwork.freedesktop.org/patch/188043/
V4: rebase
V5: rebase
V6: As recommended by Ville, avoided matching of aspect-ratio in
  drm_fb_helper, while trying to find a common mode among connectors
  for the target clone mode.
V7: rebase
V8: rebase
V9: rebase
V10: rebase
---
   drivers/gpu/drm/drm_fb_helper.c | 12 ++--
   drivers/gpu/drm/drm_modes.c | 35 ++-
   2 files changed, 44 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 0646b10..2ee1eaa 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -2183,7 +2183,11 @@ static bool drm_target_cloned(struct drm_fb_helper 
*fb_helper,
for (j = 0; j < i; j++) {
if (!enabled[j])
continue;
-   if (!drm_mode_equal(modes[j], modes[i]))
+   if (!drm_mode_match(modes[j], modes[i],
+   DRM_MODE_MATCH_TIMINGS |
+   DRM_MODE_MATCH_CLOCK |
+   DRM_MODE_MATCH_FLAGS |
+   DRM_MODE_MATCH_3D_FLAGS))
can_clone = false;
}
}
@@ -2203,7 +2207,11 @@ static bool drm_target_cloned(struct drm_fb_helper 
*fb_helper,
   
   		fb_helper_conn = fb_helper->connector_info[i];

list_for_each_entry(mode, _helper_conn->connector->modes, 
head) {
-   if (drm_mode_equal(mode, dmt_mode))
+   if (drm_mode_match(mode, dmt_mode,
+  DRM_MODE_MATCH_TIMINGS |
+  DRM_MODE_MATCH_CLOCK |
+  DRM_MODE_MATCH_FLAGS |
+  DRM_MODE_MATCH_3D_FLAGS))
 

Re: [Intel-gfx] [PATCH v4] drm/i915: Enable edp psr error interrupts on hsw

[Paulo Zanoni]

Em Qui, 2018-04-05 às 15:00 -0700, Dhinakaran Pandiyan escreveu:
> From: Daniel Vetter 
> 
> The definitions for the error register should be valid on bdw/skl
> too,
> but there we haven't even enabled DE_MISC handling yet.
> 
> Somewhat confusing the the moved register offset on bdw is only for
> the _CTL/_AUX register, and that _IIR/IMR stayed where they have been
> on bdw.
> 
> v2: Fixes from Ville.
> 
> v3: From DK
>  * Rebased on drm-tip
>  * Removed BDW IIR bit definition, looks like an unintentional change
> that
> should be in the following patch.
> 
> v4: From DK
>  * Don't mask REG_WRITE.
> 
> References: bspec/11974 [SRD Interrupt Bit Definition DevHSW]
> Cc: Ville Syrjälä 
> Cc: Rodrigo Vivi 
> Cc: Daniel Vetter 
> Signed-off-by: Daniel Vetter 
> Signed-off-by: Dhinakaran Pandiyan 
> Reviewed-by: Jose Roberto de Souza 
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 34
> ++
>  drivers/gpu/drm/i915/i915_reg.h |  8 
>  2 files changed, 42 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c
> b/drivers/gpu/drm/i915/i915_irq.c
> index 27aee25429b7..c2d3f30778ee 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -2391,6 +2391,26 @@ static void ilk_display_irq_handler(struct
> drm_i915_private *dev_priv,
>   ironlake_rps_change_irq_handler(dev_priv);
>  }
>  
> +static void hsw_edp_psr_irq_handler(struct drm_i915_private
> *dev_priv)
> +{
> + u32 edp_psr_iir = I915_READ(EDP_PSR_IIR);
> +
> + if (edp_psr_iir & EDP_PSR_ERROR)
> + DRM_DEBUG_KMS("PSR error\n");
> +
> + if (edp_psr_iir & EDP_PSR_PRE_ENTRY) {
> + DRM_DEBUG_KMS("PSR prepare entry in 2 vblanks\n");
> + I915_WRITE(EDP_PSR_IMR, EDP_PSR_PRE_ENTRY);

Why are we masking it here? During these 2 vblanks it's possible that
something will happen (e.g., frontbuffer writing, cursor moving, page
flipping). Are we guaranteed to get a POST_EXIT interrupt even if we
give up entering PSR before it is actually entered?


> + }
> +
> + if (edp_psr_iir & EDP_PSR_POST_EXIT) {
> + DRM_DEBUG_KMS("PSR exit completed\n");
> + I915_WRITE(EDP_PSR_IMR, 0);
> + }
> +
> + I915_WRITE(EDP_PSR_IIR, edp_psr_iir);
> +}
> +
>  static void ivb_display_irq_handler(struct drm_i915_private
> *dev_priv,
>   u32 de_iir)
>  {
> @@ -2403,6 +2423,9 @@ static void ivb_display_irq_handler(struct
> drm_i915_private *dev_priv,
>   if (de_iir & DE_ERR_INT_IVB)
>   ivb_err_int_handler(dev_priv);
>  
> + if (de_iir & DE_EDP_PSR_INT_HSW)
> + hsw_edp_psr_irq_handler(dev_priv);
> +
>   if (de_iir & DE_AUX_CHANNEL_A_IVB)
>   dp_aux_irq_handler(dev_priv);
>  
> @@ -3260,6 +3283,11 @@ static void ironlake_irq_reset(struct
> drm_device *dev)
>   if (IS_GEN7(dev_priv))
>   I915_WRITE(GEN7_ERR_INT, 0x);
>  
> + if (IS_HASWELL(dev_priv)) {
> + I915_WRITE(EDP_PSR_IMR, 0x);
> + I915_WRITE(EDP_PSR_IIR, 0x);
> + }

We need another IIR write we do for the other platforms. This is not
cargo cult (as mentioned in previous review emails), this is required
since our hardware is able to store more than one IIR interrupt. Please
do it like we do for the other interrupts.

Do git-blame in the relevant lines and read the commit messages for
more information on why stuff is the way it is. If you still think this
is unnecessary, feel free to write a patch removing the unnecessary
stuff for every interrupt instead of of making just one register be
not-cargo-culted.

> +
>   gen5_gt_irq_reset(dev_priv);
>  
>   ibx_irq_reset(dev_priv);
> @@ -3671,6 +3699,12 @@ static int ironlake_irq_postinstall(struct
> drm_device *dev)
> DE_DP_A_HOTPLUG);
>   }
>  
> + if (IS_HASWELL(dev_priv)) {
> + gen3_assert_iir_is_zero(dev_priv, EDP_PSR_IIR);
> + I915_WRITE(EDP_PSR_IMR, 0);
> + display_mask |= DE_EDP_PSR_INT_HSW;
> + }
> +
>   dev_priv->irq_mask = ~display_mask;
>  
>   ibx_irq_pre_postinstall(dev);
> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h
> index 176dca6554f4..f5783d6db614 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4011,6 +4011,13 @@ enum {
>  #define   EDP_PSR_TP1_TIME_0us   (3<<4)
>  #define   EDP_PSR_IDLE_FRAME_SHIFT   0
>  
> +/* Bspec claims those aren't shifted but stay at 0x64800 */
> +#define EDP_PSR_IMR  _MMIO(0x64834)
> +#define EDP_PSR_IIR  _MMIO(0x64838)
> +#define   EDP_PSR_ERROR  (1<<2)
> +#define   EDP_PSR_POST_EXIT  

Re: [Intel-gfx] [PATCH] drm/i915: Fix LSPCON TMDS output buffer enabling from low-power state

[Clint Taylor]

On 04/16/2018 08:53 AM, Imre Deak wrote:

LSPCON adapters in low-power state may ignore the first I2C write during
TMDS output buffer enabling, resulting in a blank screen even with an
otherwise enabled pipe. Fix this by reading back and validating the
written value a few times.

The problem was noticed on GLK machines with an onboard LSPCON adapter
after entering/exiting DC5 power state. Doing an I2C read of the adapter
ID as the first transaction - instead of the I2C write to enable the
TMDS buffers - returns the correct value. Based on this we assume that
the transaction itself is sent properly, it's only the adapter that is
not ready for some reason to accept this first write after waking from
low-power state. In my case the second I2C write attempt always
succeeded.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105854
Cc: Clinton Taylor 
Cc: Ville Syrjälä 
Signed-off-by: Imre Deak 
---
  drivers/gpu/drm/drm_dp_dual_mode_helper.c | 39 +--
  1 file changed, 32 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_dual_mode_helper.c 
b/drivers/gpu/drm/drm_dp_dual_mode_helper.c
index 02a50929af67..e7f4fe2848a5 100644
--- a/drivers/gpu/drm/drm_dp_dual_mode_helper.c
+++ b/drivers/gpu/drm/drm_dp_dual_mode_helper.c
@@ -350,19 +350,44 @@ int drm_dp_dual_mode_set_tmds_output(enum 
drm_dp_dual_mode_type type,
  {
uint8_t tmds_oen = enable ? 0 : DP_DUAL_MODE_TMDS_DISABLE;
ssize_t ret;
+   int retry;
  
  	if (type < DRM_DP_DUAL_MODE_TYPE2_DVI)

return 0;
  
-	ret = drm_dp_dual_mode_write(adapter, DP_DUAL_MODE_TMDS_OEN,

-_oen, sizeof(tmds_oen));
-   if (ret) {
-   DRM_DEBUG_KMS("Failed to %s TMDS output buffers\n",
- enable ? "enable" : "disable");
-   return ret;
+   /*
+* LSPCON adapters in low-power state may ignore the first write, so
+* read back and verify the written value a few times.
+*/
+   for (retry = 0; retry < 3; retry++) {
+   uint8_t tmp;
+
+   ret = drm_dp_dual_mode_write(adapter, DP_DUAL_MODE_TMDS_OEN,
+_oen, sizeof(tmds_oen));
+   if (ret) {
+   DRM_DEBUG_KMS("Failed to %s TMDS output buffers (%d 
attempts)\n",
+ enable ? "enable" : "disable",
+ retry + 1);
+   return ret;
+   }
+
+   ret = drm_dp_dual_mode_read(adapter, DP_DUAL_MODE_TMDS_OEN,
+   , sizeof(tmp));
+   if (ret) {
+   DRM_DEBUG_KMS("I2C read failed during TMDS output buffer %s 
(%d attempts)\n",
+ enable ? "enabling" : "disabling",
+ retry + 1);
+   return ret;
+   }
+
+   if (tmp == tmds_oen)
+   return 0;
}
  
-	return 0;

+   DRM_DEBUG_KMS("I2C write value mismatch during TMDS output buffer %s\n",
+ enable ? "enabling" : "disabling");
+
+   return -EIO;
  }
  EXPORT_SYMBOL(drm_dp_dual_mode_set_tmds_output);
  


Appears to fix the issue seen on GLK with LSPCON and DMC firmware 
loaded. Customer was concerned about the fix being in DRM instead of 
i915. However, there are no other SOCs that use this DRM function.


Reviewed-by: Clint Taylor 
Tested-by: Clint Taylor 

-Clint


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

[Intel-gfx] ✗ Fi.CI.IGT: failure for drm/i915/selftests: Handle a potential failure of intel_ring_begin

[Patchwork]

== Series Details ==

Series: drm/i915/selftests: Handle a potential failure of intel_ring_begin
URL   : https://patchwork.freedesktop.org/series/41773/
State : failure

== Summary ==

= CI Bug Log - changes from CI_DRM_4058_full -> Patchwork_8700_full =

== Summary - FAILURE ==

  Serious unknown changes coming with Patchwork_8700_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_8700_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: 
https://patchwork.freedesktop.org/api/1.0/series/41773/revisions/1/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in 
Patchwork_8700_full:

  === IGT changes ===

 Possible regressions 

igt@gem_wait@wait-vebox:
  shard-apl:  PASS -> FAIL


 Warnings 

igt@gem_mocs_settings@mocs-rc6-ctx-render:
  shard-kbl:  SKIP -> PASS


== Known issues ==

  Here are the changes found in Patchwork_8700_full that come from known issues:

  === IGT changes ===

 Issues hit 

igt@kms_color@pipe-c-ctm-blue-to-red:
  shard-kbl:  PASS -> DMESG-WARN (fdo#105602, fdo#103558, 
fdo#103313) +13

igt@kms_flip@flip-vs-wf_vblank-interruptible:
  shard-kbl:  PASS -> DMESG-WARN (fdo#105602, fdo#103558) +14

igt@kms_setmode@basic:
  shard-apl:  PASS -> FAIL (fdo#99912)

igt@perf@polling:
  shard-hsw:  PASS -> FAIL (fdo#102252)

igt@pm_rpm@debugfs-read:
  shard-kbl:  PASS -> DMESG-WARN (fdo#103558, fdo#103313)


 Possible fixes 

igt@kms_flip@2x-flip-vs-expired-vblank-interruptible:
  shard-hsw:  FAIL (fdo#105189) -> PASS

igt@kms_flip@blocking-wf_vblank:
  shard-hsw:  FAIL (fdo#103928) -> PASS


  fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
  fdo#103313 https://bugs.freedesktop.org/show_bug.cgi?id=103313
  fdo#103558 https://bugs.freedesktop.org/show_bug.cgi?id=103558
  fdo#103928 https://bugs.freedesktop.org/show_bug.cgi?id=103928
  fdo#105189 https://bugs.freedesktop.org/show_bug.cgi?id=105189
  fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


== Participating hosts (6 -> 4) ==

  Missing(2): shard-glk shard-glkb 


== Build changes ==

* Linux: CI_DRM_4058 -> Patchwork_8700

  CI_DRM_4058: 241d827c86078c4709c00251d22ea8f7554e3e36 @ 
git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4432: 8b77704db49167f7ebfd1c470d9c129d3b862cb6 @ 
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_8700: 76ea91dce11329f20e7d1a4285a546b6825c7404 @ 
git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4432: 93b35926a150e318439d2505901288594b3548f5 @ 
git://anongit.freedesktop.org/piglit

== Logs ==

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

[Intel-gfx] ✓ Fi.CI.IGT: success for series starting with [v7,1/2] drm/i915/cnl: Implement WaProgramMgsrForCorrectSliceSpecificMmioReads (rev9)

[Patchwork]

== Series Details ==

Series: series starting with [v7,1/2] drm/i915/cnl: Implement 
WaProgramMgsrForCorrectSliceSpecificMmioReads (rev9)
URL   : https://patchwork.freedesktop.org/series/40503/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4058_full -> Patchwork_8699_full =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_8699_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_8699_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: 
https://patchwork.freedesktop.org/api/1.0/series/40503/revisions/9/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in 
Patchwork_8699_full:

  === IGT changes ===

 Warnings 

igt@gem_mocs_settings@mocs-rc6-ctx-dirty-render:
  shard-kbl:  PASS -> SKIP

igt@gem_mocs_settings@mocs-rc6-ctx-render:
  shard-kbl:  SKIP -> PASS


== Known issues ==

  Here are the changes found in Patchwork_8699_full that come from known issues:

  === IGT changes ===

 Issues hit 

igt@kms_flip@basic-flip-vs-wf_vblank:
  shard-hsw:  PASS -> FAIL (fdo#103928)

igt@kms_flip_tiling@flip-to-x-tiled:
  shard-hsw:  PASS -> DMESG-WARN (fdo#102614) +1

igt@kms_setmode@basic:
  shard-apl:  PASS -> FAIL (fdo#99912)


 Possible fixes 

igt@kms_flip@2x-flip-vs-expired-vblank-interruptible:
  shard-hsw:  FAIL (fdo#105189) -> PASS

igt@kms_flip@blocking-wf_vblank:
  shard-hsw:  FAIL (fdo#103928) -> PASS


  fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
  fdo#103928 https://bugs.freedesktop.org/show_bug.cgi?id=103928
  fdo#105189 https://bugs.freedesktop.org/show_bug.cgi?id=105189
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


== Participating hosts (6 -> 4) ==

  Missing(2): shard-glk shard-glkb 


== Build changes ==

* Linux: CI_DRM_4058 -> Patchwork_8699

  CI_DRM_4058: 241d827c86078c4709c00251d22ea8f7554e3e36 @ 
git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4432: 8b77704db49167f7ebfd1c470d9c129d3b862cb6 @ 
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_8699: 5de6aa2fd60b7d1fd7a67cb846e4352ca40e7473 @ 
git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4432: 93b35926a150e318439d2505901288594b3548f5 @ 
git://anongit.freedesktop.org/piglit

== Logs ==

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

Re: [Intel-gfx] [RESEND PATCH 1/1] drm/i915/glk: Add MODULE_FIRMWARE for Geminilake

[Srivatsa, Anusha]

>-Original Message-
>From: Jani Nikula [mailto:jani.nik...@linux.intel.com]
>Sent: Wednesday, April 11, 2018 5:27 AM
>To: Ian W MORRISON 
>Cc: Vivi, Rodrigo ; Srivatsa, Anusha
>; Wajdeczko, Michal
>; Greg KH ;
>airl...@linux.ie; joonas.lahti...@linux.intel.com; 
>linux-ker...@vger.kernel.org;
>sta...@vger.kernel.org; intel-gfx@lists.freedesktop.org; dri-
>de...@lists.freedesktop.org
>Subject: Re: [RESEND PATCH 1/1] drm/i915/glk: Add MODULE_FIRMWARE for
>Geminilake
>
>On Wed, 11 Apr 2018, Ian W MORRISON  wrote:
>> 
>>
>>>
>>> NAK on indiscriminate Cc: stable. There are zero guarantees that
>>> older kernels will work with whatever firmware you throw at them.
>>>
>>
>> I included 'Cc: stable' so the patch would get added to the v4.16 and
>> v4.15 kernels which I have tested with the patch. I found that earlier
>> kernels didn't support the 'linux-firmware' package required to get
>> wifi working on Intel's new Gemini Lake NUC.
>
>You realize that this patch should have nothing to do with wifi?
>
>Rodrigo, Anusha, if you think Cc: stable is appropriate, please indicate the 
>specific
>versions of stable it is appropriate for.

Hi Jani,

The stable kernel version is 4.12 and beyond.
It is appropriate to add the CC: stable in my opinion

Anusha
>BR,
>Jani.
>
>>
>>>
>>> PS. How is this a "RESEND"? I haven't seen this before.
>>>
>>
>> It is a 'RESEND' for that very reason. I initially sent the patch to
>> the same people as a similar patch
>> (https://patchwork.kernel.org/patch/10143637/) however after realising
>> this omitted required addresses I added them and resent the patch.
>>
>> Best regards,
>> Ian
>
>--
>Jani Nikula, Intel Open Source Technology Center
___
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/selftests: Handle a potential failure of intel_ring_begin

[Patchwork]

== Series Details ==

Series: drm/i915/selftests: Handle a potential failure of intel_ring_begin
URL   : https://patchwork.freedesktop.org/series/41773/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4058 -> Patchwork_8700 =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_8700 need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_8700, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: 
https://patchwork.freedesktop.org/api/1.0/series/41773/revisions/1/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_8700:

  === IGT changes ===

 Warnings 

igt@prime_vgem@basic-fence-flip:
  fi-cnl-y3:  SKIP -> PASS


== Known issues ==

  Here are the changes found in Patchwork_8700 that come from known issues:

  === IGT changes ===

 Possible fixes 

igt@gem_mmap_gtt@basic-small-bo-tiledx:
  fi-gdg-551: FAIL (fdo#102575) -> PASS

igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
  fi-ivb-3520m:   DMESG-WARN (fdo#106084) -> PASS +1


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


== Participating hosts (35 -> 33) ==

  Missing(2): fi-ilk-m540 fi-skl-6700hq 


== Build changes ==

* Linux: CI_DRM_4058 -> Patchwork_8700

  CI_DRM_4058: 241d827c86078c4709c00251d22ea8f7554e3e36 @ 
git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4432: 8b77704db49167f7ebfd1c470d9c129d3b862cb6 @ 
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_8700: 76ea91dce11329f20e7d1a4285a546b6825c7404 @ 
git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4432: 93b35926a150e318439d2505901288594b3548f5 @ 
git://anongit.freedesktop.org/piglit


== Linux commits ==

76ea91dce113 drm/i915/selftests: Handle a potential failure of intel_ring_begin

== Logs ==

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

[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [v7,1/2] drm/i915/cnl: Implement WaProgramMgsrForCorrectSliceSpecificMmioReads (rev9)

[Patchwork]

== Series Details ==

Series: series starting with [v7,1/2] drm/i915/cnl: Implement 
WaProgramMgsrForCorrectSliceSpecificMmioReads (rev9)
URL   : https://patchwork.freedesktop.org/series/40503/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4058 -> Patchwork_8699 =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_8699 need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_8699, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: 
https://patchwork.freedesktop.org/api/1.0/series/40503/revisions/9/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_8699:

  === IGT changes ===

 Warnings 

igt@prime_vgem@basic-fence-flip:
  fi-cnl-y3:  SKIP -> PASS


== Known issues ==

  Here are the changes found in Patchwork_8699 that come from known issues:

  === IGT changes ===

 Possible fixes 

igt@debugfs_test@read_all_entries:
  fi-snb-2520m:   INCOMPLETE (fdo#103713) -> PASS

igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
  fi-bxt-dsi: INCOMPLETE (fdo#103927) -> PASS

igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
  fi-ivb-3520m:   DMESG-WARN (fdo#106084) -> PASS


  fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
  fdo#106084 https://bugs.freedesktop.org/show_bug.cgi?id=106084


== Participating hosts (35 -> 33) ==

  Missing(2): fi-ilk-m540 fi-skl-6700hq 


== Build changes ==

* Linux: CI_DRM_4058 -> Patchwork_8699

  CI_DRM_4058: 241d827c86078c4709c00251d22ea8f7554e3e36 @ 
git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4432: 8b77704db49167f7ebfd1c470d9c129d3b862cb6 @ 
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_8699: 5de6aa2fd60b7d1fd7a67cb846e4352ca40e7473 @ 
git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4432: 93b35926a150e318439d2505901288594b3548f5 @ 
git://anongit.freedesktop.org/piglit


== Linux commits ==

5de6aa2fd60b drm/i915: Implement WaProgramMgsrForL3BankSpecificMmioReads
d443c598258a drm/i915/cnl: Implement 
WaProgramMgsrForCorrectSliceSpecificMmioReads

== Logs ==

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

Re: [Intel-gfx] [PATCH v7 2/2] drm/i915: Implement WaProgramMgsrForL3BankSpecificMmioReads

[Oscar Mateo]

On 04/16/2018 02:24 PM, Yunwei Zhang wrote:

L3Bank could be fused off in hardware for debug purpose, and it
is possible that subslice is enabled while its corresponding L3Bank pairs
are disabled. In such case, if MCR packet control register(0xFDC) is
programed to point to a disabled bank pair, a MMIO read into L3Bank range
will return 0 instead of correct values.

However, this is not going to be the case in any production silicon.
Therefore, we only check at initialization and issue a warning should
this really happen.

References: HSDES#1405586840

v2:
  - use fls instead of find_last_bit (Chris)
  - use is_power_of_2() instead of counting bit set (Chris)
v3:
  - rebase on latest tip
v5:
  - Added references (Mika)
  - Move local variable into scope where they are used (Ursulin)
  - use a new local variable to reduce long line of code (Ursulin)
v6:
  - Some coding style and use more local variables for clearer
logic (Ursulin)
v7:
  - Rebased.

Cc: Oscar Mateo 
Cc: Michel Thierry 
Cc: Joonas Lahtinen 
Cc: Chris Wilson 
Cc: Mika Kuoppala 
Cc: Tvrtko Ursulin 
Signed-off-by: Yunwei Zhang 
---
  drivers/gpu/drm/i915/i915_reg.h  |  4 
  drivers/gpu/drm/i915/intel_workarounds.c | 25 +
  2 files changed, 29 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index fb10602..6c9c01b 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2709,6 +2709,10 @@ enum i915_power_well_id {
  #define   GEN10_F2_SS_DIS_SHIFT   18
  #define   GEN10_F2_SS_DIS_MASK(0xf << GEN10_F2_SS_DIS_SHIFT)
  
+#define	GEN10_MIRROR_FUSE3		_MMIO(0x9118)

+#define GEN10_L3BANK_PAIR_COUNT 4
+#define GEN10_L3BANK_MASK   0x0F
+
  #define GEN8_EU_DISABLE0  _MMIO(0x9134)
  #define   GEN8_EU_DIS0_S0_MASK0xff
  #define   GEN8_EU_DIS0_S1_SHIFT   24
diff --git a/drivers/gpu/drm/i915/intel_workarounds.c 
b/drivers/gpu/drm/i915/intel_workarounds.c
index 8a2354e..fe1c908 100644
--- a/drivers/gpu/drm/i915/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/intel_workarounds.c
@@ -647,8 +647,33 @@ static void cfl_gt_workarounds_apply(struct 
drm_i915_private *dev_priv)
  
  static void wa_init_mcr(struct drm_i915_private *dev_priv)

  {
+   const struct sseu_dev_info *sseu = &(INTEL_INFO(dev_priv)->sseu);
u32 mcr;
  
+	/*

+* L3Banks could be fused off in single slice scenario, however, if
+* more than one slice is enabled, this should not happen.
+*/
+   if (is_power_of_2(sseu->slice_mask)) {
+   /*
+* WaProgramMgsrForL3BankSpecificMmioReads:
+* read FUSE3 for enabled L3 Bank IDs, if L3 Bank matches
+* enabled subslice, no need to redirect MCR packet
+*/
+   u32 slice = fls(sseu->slice_mask);
+   u32 fuse3 = I915_READ(GEN10_MIRROR_FUSE3);
+   u8 ss_mask = sseu->subslice_mask[slice];
+
+   u8 enabled_mask = (ss_mask | ss_mask >> 4) & 0xf;
+   u8 disabled_mask = fuse3 & 0xf;
+
+   /*
+* Production silicon should have matched L3Bank and
+* subslice enabled
+*/
+   WARN_ON((enabled_mask & disabled_mask) != enabled_mask);
+   }
+


Reviewed-by: Oscar Mateo 

And this warning is also required for Icelake.


mcr = I915_READ(GEN8_MCR_SELECTOR);
mcr = calculate_mcr(dev_priv, mcr);
I915_WRITE(GEN8_MCR_SELECTOR, mcr);


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

Re: [Intel-gfx] [PATCH v7 1/2] drm/i915/cnl: Implement WaProgramMgsrForCorrectSliceSpecificMmioReads

[Oscar Mateo]

On 04/16/2018 02:22 PM, Yunwei Zhang wrote:

WaProgramMgsrForCorrectSliceSpecificMmioReads dictate that before any MMIO
read into Slice/Subslice specific registers, MCR packet control
register(0xFDC) needs to be programmed to point to any enabled
slice/subslice pair. Otherwise, incorrect value will be returned.

However, that means each subsequent MMIO read will be forwarded to a
specific slice/subslice combination as read is unicast. This is OK since
slice/subslice specific register values are consistent in almost all cases
across slice/subslice. There are rare occasions such as INSTDONE that this
value will be dependent on slice/subslice combo, in such cases, we need to
program 0xFDC and recover this after. This is already covered by
read_subslice_reg.

Also, 0xFDC will lose its information after TDR/engine reset/power state
change.

References: HSD#1405586840, BSID#0575

v2:
  - use fls() instead of find_last_bit() (Chris)
  - added INTEL_SSEU to extract sseu from device info. (Chris)
v3:
  - rebase on latest tip
v5:
  - Added references (Mika)
  - Change the ordered of passing arguments and etc. (Ursulin)
v7:
  - Rebased.

Cc: Oscar Mateo 
Cc: Michel Thierry 
Cc: Joonas Lahtinen 
Cc: Chris Wilson 
Cc: Mika Kuoppala 
Cc: Tvrtko Ursulin 
Signed-off-by: Yunwei Zhang 
---
  drivers/gpu/drm/i915/i915_drv.h  |  2 ++
  drivers/gpu/drm/i915/intel_engine_cs.c   | 30 +++---
  drivers/gpu/drm/i915/intel_workarounds.c | 12 
  3 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8e8667d..43498a47 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2725,6 +2725,8 @@ int vlv_force_gfx_clock(struct drm_i915_private 
*dev_priv, bool on);
  int intel_engines_init_mmio(struct drm_i915_private *dev_priv);
  int intel_engines_init(struct drm_i915_private *dev_priv);
  
+u32 calculate_mcr(struct drm_i915_private *dev_priv, u32 mcr);

+


As a global function, this could use a better prefix (intel_something_)

Or, alternatively, make it local and store the calculation somewhere.


  /* intel_hotplug.c */
  void intel_hpd_irq_handler(struct drm_i915_private *dev_priv,
   u32 pin_mask, u32 long_mask);
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c 
b/drivers/gpu/drm/i915/intel_engine_cs.c
index 1a83707..3b6bc5e 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -799,6 +799,18 @@ const char *i915_cache_level_str(struct drm_i915_private 
*i915, int type)
}
  }
  
+u32 calculate_mcr(struct drm_i915_private *dev_priv, u32 mcr)

+{
+   const struct sseu_dev_info *sseu = &(INTEL_INFO(dev_priv)->sseu);
+   u32 slice = fls(sseu->slice_mask);
+   u32 subslice = fls(sseu->subslice_mask[slice]);
+
+   mcr &= ~(GEN8_MCR_SLICE_MASK | GEN8_MCR_SUBSLICE_MASK);
+   mcr |= GEN8_MCR_SLICE(slice) | GEN8_MCR_SUBSLICE(subslice);
+
+   return mcr;
+}
+
  static inline uint32_t
  read_subslice_reg(struct drm_i915_private *dev_priv, int slice,
  int subslice, i915_reg_t reg)
@@ -831,18 +843,30 @@ read_subslice_reg(struct drm_i915_private *dev_priv, int 
slice,
intel_uncore_forcewake_get__locked(dev_priv, fw_domains);
  
  	mcr = I915_READ_FW(GEN8_MCR_SELECTOR);

+
/*
 * The HW expects the slice and sublice selectors to be reset to 0
-* after reading out the registers.
+* before GEN10 or to a enabled s/ss post GEN10 after reading out the
+* registers.
 */
-   WARN_ON_ONCE(mcr & mcr_slice_subslice_mask);
+   WARN_ON_ONCE(INTEL_GEN(dev_priv) < 10 &&
+(mcr & mcr_slice_subslice_mask));


Advantage of storing the calculation: you can assert here for the 
expected value, independently of the platform.



mcr &= ~mcr_slice_subslice_mask;
mcr |= mcr_slice_subslice_select;
I915_WRITE_FW(GEN8_MCR_SELECTOR, mcr);
  
  	ret = I915_READ_FW(reg);
  
-	mcr &= ~mcr_slice_subslice_mask;

+   /*
+* WaProgramMgsrForCorrectSliceSpecificMmioReads:cnl
+* expects mcr to be programed to a enabled slice/subslice pair
+* before any MMIO read into slice/subslice register
+*/
+   if (INTEL_GEN(dev_priv) < 10)
+   mcr &= ~mcr_slice_subslice_mask;
+   else
+   mcr = calculate_mcr(dev_priv, mcr);


Another advantage: no branching here either.


+
I915_WRITE_FW(GEN8_MCR_SELECTOR, mcr);
  
  	intel_uncore_forcewake_put__locked(dev_priv, fw_domains);

diff --git a/drivers/gpu/drm/i915/intel_workarounds.c 
b/drivers/gpu/drm/i915/intel_workarounds.c
index ec9d340..8a2354e 100644
--- a/drivers/gpu/drm/i915/intel_workarounds.c
+++ 

[Intel-gfx] [PATCH] drm/i915/selftests: Handle a potential failure of intel_ring_begin

[Oscar Mateo]

Silence smatch over:

drivers/gpu/drm/i915/selftests/intel_workarounds.c:58 read_nonprivs() error: 
'cs' dereferencing possible ERR_PTR()

by handling a potential (but unlikely) failure of intel_ring_begin.

Fixes: f4ecfbfc32ed ("drm/i915: Check whitelist registers across resets")
Signed-off-by: Oscar Mateo 
Cc: Chris Wilson 
---
 drivers/gpu/drm/i915/selftests/intel_workarounds.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/i915/selftests/intel_workarounds.c 
b/drivers/gpu/drm/i915/selftests/intel_workarounds.c
index fe7deca..5455b26 100644
--- a/drivers/gpu/drm/i915/selftests/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/selftests/intel_workarounds.c
@@ -54,6 +54,11 @@
srm++;
 
cs = intel_ring_begin(rq, 4 * RING_MAX_NONPRIV_SLOTS);
+   if (IS_ERR(cs)) {
+   err = PTR_ERR(cs);
+   goto err_req;
+   }
+
for (i = 0; i < RING_MAX_NONPRIV_SLOTS; i++) {
*cs++ = srm;
*cs++ = i915_mmio_reg_offset(RING_FORCE_TO_NONPRIV(base, i));
@@ -75,6 +80,8 @@
 
return result;
 
+err_req:
+   i915_request_add(rq);
 err_pin:
i915_vma_unpin(vma);
 err_obj:
-- 
1.9.1

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

[Intel-gfx] ✗ Fi.CI.SPARSE: warning for series starting with [v7,1/2] drm/i915/cnl: Implement WaProgramMgsrForCorrectSliceSpecificMmioReads (rev9)

[Patchwork]

== Series Details ==

Series: series starting with [v7,1/2] drm/i915/cnl: Implement 
WaProgramMgsrForCorrectSliceSpecificMmioReads (rev9)
URL   : https://patchwork.freedesktop.org/series/40503/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Commit: drm/i915/cnl: Implement WaProgramMgsrForCorrectSliceSpecificMmioReads
-drivers/gpu/drm/i915/selftests/../i915_drv.h:3655:16: warning: expression 
using sizeof(void)
+drivers/gpu/drm/i915/selftests/../i915_drv.h:3657:16: warning: expression 
using sizeof(void)

Commit: drm/i915: Implement WaProgramMgsrForL3BankSpecificMmioReads
Okay!

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

[Intel-gfx] [PATCH v7 2/2] drm/i915: Implement WaProgramMgsrForL3BankSpecificMmioReads

[Yunwei Zhang]

L3Bank could be fused off in hardware for debug purpose, and it
is possible that subslice is enabled while its corresponding L3Bank pairs
are disabled. In such case, if MCR packet control register(0xFDC) is
programed to point to a disabled bank pair, a MMIO read into L3Bank range
will return 0 instead of correct values.

However, this is not going to be the case in any production silicon.
Therefore, we only check at initialization and issue a warning should
this really happen.

References: HSDES#1405586840

v2:
 - use fls instead of find_last_bit (Chris)
 - use is_power_of_2() instead of counting bit set (Chris)
v3:
 - rebase on latest tip
v5:
 - Added references (Mika)
 - Move local variable into scope where they are used (Ursulin)
 - use a new local variable to reduce long line of code (Ursulin)
v6:
 - Some coding style and use more local variables for clearer
   logic (Ursulin)
v7:
 - Rebased.

Cc: Oscar Mateo 
Cc: Michel Thierry 
Cc: Joonas Lahtinen 
Cc: Chris Wilson 
Cc: Mika Kuoppala 
Cc: Tvrtko Ursulin 
Signed-off-by: Yunwei Zhang 
---
 drivers/gpu/drm/i915/i915_reg.h  |  4 
 drivers/gpu/drm/i915/intel_workarounds.c | 25 +
 2 files changed, 29 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index fb10602..6c9c01b 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2709,6 +2709,10 @@ enum i915_power_well_id {
 #define   GEN10_F2_SS_DIS_SHIFT18
 #define   GEN10_F2_SS_DIS_MASK (0xf << GEN10_F2_SS_DIS_SHIFT)
 
+#defineGEN10_MIRROR_FUSE3  _MMIO(0x9118)
+#define GEN10_L3BANK_PAIR_COUNT 4
+#define GEN10_L3BANK_MASK   0x0F
+
 #define GEN8_EU_DISABLE0   _MMIO(0x9134)
 #define   GEN8_EU_DIS0_S0_MASK 0xff
 #define   GEN8_EU_DIS0_S1_SHIFT24
diff --git a/drivers/gpu/drm/i915/intel_workarounds.c 
b/drivers/gpu/drm/i915/intel_workarounds.c
index 8a2354e..fe1c908 100644
--- a/drivers/gpu/drm/i915/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/intel_workarounds.c
@@ -647,8 +647,33 @@ static void cfl_gt_workarounds_apply(struct 
drm_i915_private *dev_priv)
 
 static void wa_init_mcr(struct drm_i915_private *dev_priv)
 {
+   const struct sseu_dev_info *sseu = &(INTEL_INFO(dev_priv)->sseu);
u32 mcr;
 
+   /*
+* L3Banks could be fused off in single slice scenario, however, if
+* more than one slice is enabled, this should not happen.
+*/
+   if (is_power_of_2(sseu->slice_mask)) {
+   /*
+* WaProgramMgsrForL3BankSpecificMmioReads:
+* read FUSE3 for enabled L3 Bank IDs, if L3 Bank matches
+* enabled subslice, no need to redirect MCR packet
+*/
+   u32 slice = fls(sseu->slice_mask);
+   u32 fuse3 = I915_READ(GEN10_MIRROR_FUSE3);
+   u8 ss_mask = sseu->subslice_mask[slice];
+
+   u8 enabled_mask = (ss_mask | ss_mask >> 4) & 0xf;
+   u8 disabled_mask = fuse3 & 0xf;
+
+   /*
+* Production silicon should have matched L3Bank and
+* subslice enabled
+*/
+   WARN_ON((enabled_mask & disabled_mask) != enabled_mask);
+   }
+
mcr = I915_READ(GEN8_MCR_SELECTOR);
mcr = calculate_mcr(dev_priv, mcr);
I915_WRITE(GEN8_MCR_SELECTOR, mcr);
-- 
2.7.4

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

[Intel-gfx] [PATCH v7 1/2] drm/i915/cnl: Implement WaProgramMgsrForCorrectSliceSpecificMmioReads

[Yunwei Zhang]

WaProgramMgsrForCorrectSliceSpecificMmioReads dictate that before any MMIO
read into Slice/Subslice specific registers, MCR packet control
register(0xFDC) needs to be programmed to point to any enabled
slice/subslice pair. Otherwise, incorrect value will be returned.

However, that means each subsequent MMIO read will be forwarded to a
specific slice/subslice combination as read is unicast. This is OK since
slice/subslice specific register values are consistent in almost all cases
across slice/subslice. There are rare occasions such as INSTDONE that this
value will be dependent on slice/subslice combo, in such cases, we need to
program 0xFDC and recover this after. This is already covered by
read_subslice_reg.

Also, 0xFDC will lose its information after TDR/engine reset/power state
change.

References: HSD#1405586840, BSID#0575

v2:
 - use fls() instead of find_last_bit() (Chris)
 - added INTEL_SSEU to extract sseu from device info. (Chris)
v3:
 - rebase on latest tip
v5:
 - Added references (Mika)
 - Change the ordered of passing arguments and etc. (Ursulin)
v7:
 - Rebased.

Cc: Oscar Mateo 
Cc: Michel Thierry 
Cc: Joonas Lahtinen 
Cc: Chris Wilson 
Cc: Mika Kuoppala 
Cc: Tvrtko Ursulin 
Signed-off-by: Yunwei Zhang 
---
 drivers/gpu/drm/i915/i915_drv.h  |  2 ++
 drivers/gpu/drm/i915/intel_engine_cs.c   | 30 +++---
 drivers/gpu/drm/i915/intel_workarounds.c | 12 
 3 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8e8667d..43498a47 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2725,6 +2725,8 @@ int vlv_force_gfx_clock(struct drm_i915_private 
*dev_priv, bool on);
 int intel_engines_init_mmio(struct drm_i915_private *dev_priv);
 int intel_engines_init(struct drm_i915_private *dev_priv);
 
+u32 calculate_mcr(struct drm_i915_private *dev_priv, u32 mcr);
+
 /* intel_hotplug.c */
 void intel_hpd_irq_handler(struct drm_i915_private *dev_priv,
   u32 pin_mask, u32 long_mask);
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c 
b/drivers/gpu/drm/i915/intel_engine_cs.c
index 1a83707..3b6bc5e 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -799,6 +799,18 @@ const char *i915_cache_level_str(struct drm_i915_private 
*i915, int type)
}
 }
 
+u32 calculate_mcr(struct drm_i915_private *dev_priv, u32 mcr)
+{
+   const struct sseu_dev_info *sseu = &(INTEL_INFO(dev_priv)->sseu);
+   u32 slice = fls(sseu->slice_mask);
+   u32 subslice = fls(sseu->subslice_mask[slice]);
+
+   mcr &= ~(GEN8_MCR_SLICE_MASK | GEN8_MCR_SUBSLICE_MASK);
+   mcr |= GEN8_MCR_SLICE(slice) | GEN8_MCR_SUBSLICE(subslice);
+
+   return mcr;
+}
+
 static inline uint32_t
 read_subslice_reg(struct drm_i915_private *dev_priv, int slice,
  int subslice, i915_reg_t reg)
@@ -831,18 +843,30 @@ read_subslice_reg(struct drm_i915_private *dev_priv, int 
slice,
intel_uncore_forcewake_get__locked(dev_priv, fw_domains);
 
mcr = I915_READ_FW(GEN8_MCR_SELECTOR);
+
/*
 * The HW expects the slice and sublice selectors to be reset to 0
-* after reading out the registers.
+* before GEN10 or to a enabled s/ss post GEN10 after reading out the
+* registers.
 */
-   WARN_ON_ONCE(mcr & mcr_slice_subslice_mask);
+   WARN_ON_ONCE(INTEL_GEN(dev_priv) < 10 &&
+(mcr & mcr_slice_subslice_mask));
mcr &= ~mcr_slice_subslice_mask;
mcr |= mcr_slice_subslice_select;
I915_WRITE_FW(GEN8_MCR_SELECTOR, mcr);
 
ret = I915_READ_FW(reg);
 
-   mcr &= ~mcr_slice_subslice_mask;
+   /*
+* WaProgramMgsrForCorrectSliceSpecificMmioReads:cnl
+* expects mcr to be programed to a enabled slice/subslice pair
+* before any MMIO read into slice/subslice register
+*/
+   if (INTEL_GEN(dev_priv) < 10)
+   mcr &= ~mcr_slice_subslice_mask;
+   else
+   mcr = calculate_mcr(dev_priv, mcr);
+
I915_WRITE_FW(GEN8_MCR_SELECTOR, mcr);
 
intel_uncore_forcewake_put__locked(dev_priv, fw_domains);
diff --git a/drivers/gpu/drm/i915/intel_workarounds.c 
b/drivers/gpu/drm/i915/intel_workarounds.c
index ec9d340..8a2354e 100644
--- a/drivers/gpu/drm/i915/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/intel_workarounds.c
@@ -645,8 +645,20 @@ static void cfl_gt_workarounds_apply(struct 
drm_i915_private *dev_priv)
   GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
 }
 
+static void wa_init_mcr(struct drm_i915_private *dev_priv)
+{
+   u32 mcr;
+
+   mcr = I915_READ(GEN8_MCR_SELECTOR);
+   mcr = calculate_mcr(dev_priv, mcr);

[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915: Add LSPCON's information in i915_display_info

[Patchwork]

== Series Details ==

Series: drm/i915: Add LSPCON's information in i915_display_info
URL   : https://patchwork.freedesktop.org/series/41763/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4057_full -> Patchwork_8698_full =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_8698_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_8698_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: 
https://patchwork.freedesktop.org/api/1.0/series/41763/revisions/1/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in 
Patchwork_8698_full:

  === IGT changes ===

 Warnings 

igt@gem_mocs_settings@mocs-rc6-ctx-dirty-render:
  shard-kbl:  PASS -> SKIP

igt@kms_draw_crc@draw-method-xrgb-mmap-gtt-untiled:
  shard-snb:  PASS -> SKIP +3

igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-mmap-cpu:
  shard-snb:  SKIP -> PASS +1


== Known issues ==

  Here are the changes found in Patchwork_8698_full that come from known issues:

  === IGT changes ===

 Issues hit 

igt@kms_color@pipe-c-ctm-red-to-blue:
  shard-kbl:  PASS -> FAIL (fdo#103558)

igt@kms_flip@flip-vs-modeset-vs-hang-interruptible:
  shard-kbl:  PASS -> DMESG-WARN (fdo#103558, fdo#103313, 
fdo#105602) +2

igt@kms_flip@modeset-vs-vblank-race:
  shard-apl:  PASS -> FAIL (fdo#103060)

igt@kms_frontbuffer_tracking@fbc-suspend:
  shard-kbl:  PASS -> DMESG-WARN (fdo#103558, fdo#103841, 
fdo#105602)

igt@kms_pipe_crc_basic@read-crc-pipe-c:
  shard-kbl:  PASS -> DMESG-WARN (fdo#103558, fdo#105602) +45


 Possible fixes 

igt@kms_mmio_vs_cs_flip@setcrtc_vs_cs_flip:
  shard-kbl:  DMESG-WARN (fdo#103558, fdo#105602) -> PASS +10

igt@perf@blocking:
  shard-hsw:  FAIL (fdo#102252) -> PASS


 Warnings 

igt@gem_exec_schedule@pi-ringfull-bsd1:
  shard-kbl:  FAIL (fdo#103158) -> DMESG-FAIL (fdo#103558)


  fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
  fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
  fdo#103158 https://bugs.freedesktop.org/show_bug.cgi?id=103158
  fdo#103313 https://bugs.freedesktop.org/show_bug.cgi?id=103313
  fdo#103558 https://bugs.freedesktop.org/show_bug.cgi?id=103558
  fdo#103841 https://bugs.freedesktop.org/show_bug.cgi?id=103841
  fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602


== Participating hosts (6 -> 4) ==

  Missing(2): shard-glk shard-glkb 


== Build changes ==

* Linux: CI_DRM_4057 -> Patchwork_8698

  CI_DRM_4057: ac35105865ed2d4d9676fdb4752798b047fb126a @ 
git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4432: 8b77704db49167f7ebfd1c470d9c129d3b862cb6 @ 
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_8698: 27030e8a20d6e8027db0bab301ab8563c48f9b41 @ 
git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4432: 93b35926a150e318439d2505901288594b3548f5 @ 
git://anongit.freedesktop.org/piglit

== Logs ==

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

[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915: Fix LSPCON TMDS output buffer enabling from low-power state

[Patchwork]

== Series Details ==

Series: drm/i915: Fix LSPCON TMDS output buffer enabling from low-power state
URL   : https://patchwork.freedesktop.org/series/41759/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4057_full -> Patchwork_8697_full =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_8697_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_8697_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: 
https://patchwork.freedesktop.org/api/1.0/series/41759/revisions/1/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in 
Patchwork_8697_full:

  === IGT changes ===

 Warnings 

igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-mmap-cpu:
  shard-snb:  SKIP -> PASS +1


== Known issues ==

  Here are the changes found in Patchwork_8697_full that come from known issues:

  === IGT changes ===

 Issues hit 

igt@kms_flip@2x-blocking-wf_vblank:
  shard-hsw:  PASS -> FAIL (fdo#103928)

igt@kms_flip@2x-plain-flip-ts-check-interruptible:
  shard-hsw:  PASS -> FAIL (fdo#100368)

igt@kms_flip@modeset-vs-vblank-race-interruptible:
  shard-hsw:  PASS -> FAIL (fdo#103060)

igt@kms_plane_lowres@pipe-c-tiling-yf:
  shard-apl:  PASS -> FAIL (fdo#103166)

igt@perf_pmu@rc6-runtime-pm-long:
  shard-apl:  PASS -> FAIL (fdo#105010)


 Possible fixes 

igt@kms_mmio_vs_cs_flip@setcrtc_vs_cs_flip:
  shard-kbl:  DMESG-WARN (fdo#105602, fdo#103558) -> PASS +10

igt@perf@blocking:
  shard-hsw:  FAIL (fdo#102252) -> PASS


  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
  fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
  fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166
  fdo#103558 https://bugs.freedesktop.org/show_bug.cgi?id=103558
  fdo#103928 https://bugs.freedesktop.org/show_bug.cgi?id=103928
  fdo#105010 https://bugs.freedesktop.org/show_bug.cgi?id=105010
  fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602


== Participating hosts (6 -> 4) ==

  Missing(2): shard-glk shard-glkb 


== Build changes ==

* Linux: CI_DRM_4057 -> Patchwork_8697

  CI_DRM_4057: ac35105865ed2d4d9676fdb4752798b047fb126a @ 
git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4432: 8b77704db49167f7ebfd1c470d9c129d3b862cb6 @ 
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_8697: 5c78b9dc05bc38b3d767abc50a33b889e7cfd29d @ 
git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4432: 93b35926a150e318439d2505901288594b3548f5 @ 
git://anongit.freedesktop.org/piglit

== Logs ==

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

Re: [Intel-gfx] [RFC v4 00/25] drm: Add generic fbdev emulation

[Noralf Trønnes]

Den 16.04.2018 10.21, skrev Daniel Vetter:

On Sat, Apr 14, 2018 at 01:52:53PM +0200, Noralf Trønnes wrote:

This patchset explores the possibility of having generic fbdev emulation
in DRM for drivers that supports dumb buffers which they can export. An
API is added to support in-kernel clients in general.

In this version I was able to reuse the modesetting code from
drm_fb_helper in the client API. This avoids code duplication, carries
over lessons learned and the modesetting code is bisectable. The
downside is that it takes +10 patches to rip drm_fb_helper in two, so
maybe it's not worth it wrt possible breakage and a challenging review.

So my idea wasn't to rip the fbdev helper in  half first (that's indeed a
lot of work). But start out right away with using every piece of the
drm_client infrastructure you're adding in the existing fbdev code.

That way there's not a huge patch series which just adds code, with no
users, but every step of the way and every addition is tested almost right
away. That makes more gradual merging also easier. The things I have in
mind here is the generic fb_probe, or the drm_client block/unblock masters
and all that stuff.

Then, once we've demonstrated all these auxiliary pieces necessary for
drm_client.c work, we can cut the fb-helper in half and move the modeset
code into the drm_client library.


I agree. I wished for a way to cut this patchset in half, but I just
couldn't see how. I was tired of working on this, so I just put it out
hoping that you would provide some clarity. Which you did, thanks :-)

So, I think I'll strip this down to just the buffer part of the client API
and use that in the generic fbdev emulation, and start converting some
drivers.

I'll pick up the rest of the client API when I'm done with moving tinydrm
over to vmalloc buffers and have added support for device unplug.

Noralf.



I still prefer an even more gradual path like this compared to what you
have in your patch series, but I understand that's yet another huge
shuffle. And the current series seems like a good enough approach to get
to essentially the same place.


Does the Intel CI test the fbdev emulation?

We have fbdev emulation enabled, and iirc there's even a few tests for
fbdev. Just booting it on 20+ machines is a lot of testing itsefl already.


Daniel had this concern with the previous version:

 The register/unregister model needs more thought. Allowing both clients
 to register whenever they want to, and drm_device instances to come and
 go is what fbcon has done, and the resulting locking is a horror show.

 I think if we require that all in-kernel drm_clients are registers when
 loading drm.ko (and enabled/disabled only per module options and
 Kconfig), then we can throw out all the locking. That avoids a lot of
 the headaches.

I have solved this by adding a notifier that fires when a new DRM device
is registered (I've removed the new() callback). Currently only
bootsplash uses this. The fbdev client needs to be setup from the driver
since it can't know on device registration if the driver will setup it's
own fbdev emulation later and the vtcon client hooks up to a user
provided device id.

Ugh, notifier is exactly what fbcon also uses. It just hides the locking
horror show slightly, but it's equally bad. I'm working on a multi-year
plan to rip out the fbcon notifier, please don't start another one. See

commit 6104c37094e729f3d4ce65797002112735d49cd1
Author: Daniel Vetter 
Date:   Tue Aug 1 17:32:07 2017 +0200

 fbcon: Make fbcon a built-time depency for fbdev

for full details.


Since fbcon can't handle fb_open failing, the buffer has to be
pre-allocated. Exporting a GEM buffer pins the driver module making it
impossible to unload it.
I have included 2 solutions to the problem:
- sysfs file to remove/close clients: remove_internal_clients

This is the same thing that defacto happens already with fbcon: You have
to remove fbcon first (which holds a full ref on the fbdev, which prevents
the drm driver from unloading). I think explicitly asking for that
reference to disappear is ok.

It does mean everyone has to update their unload scripts, but oh well.


- Change drm_gem_prime_export() so it doesn't pin on client buffers

The double-loop in that patch definitely doesn't cut it, but worst case I
think something like that could be made to work.


If a dumb buffer is exported from a kernel thread (worker) context, the
file descriptor isn't closed and I leak a reference so the buffer isn't
freed. Please look at drm_client_buffer_create() in patch
'drm/client: Finish the in-kernel client API'.
This is a blocker that needs a solution.

Hm, missed that in my first cursory read of the series, I'l take another
look.
-Daniel



Noralf.

Changes since version 3:
Client API changes:
- Drop drm_client_register_funcs() which attached clients indirectly.
   Let clients attach directly using drm_client_new{_from_id}(). Clients
   

Re: [Intel-gfx] [PATCH v3 3/4] drm/i915: Add code to accept valid locked WOPCM register values

[Yaodong Li]

On 04/13/2018 09:20 PM, Michal Wajdeczko wrote:
On Tue, 10 Apr 2018 02:42:19 +0200, Jackie Li  
wrote:


In current code, we only compare the locked WOPCM register values 
with the
calculated values. However, we can continue loading GuC/HuC firmware 
if the
locked (or partially locked) values were valid for current GuC/HuC 
firmware

sizes.

This patch added a new code path to verify whether the locked register
values can be used for GuC/HuC firmware loading, it will recalculate the
verify the new values if these registers were partially locked, so 
that we
won't fail the GuC/HuC firmware loading even if the locked register 
values

are different from the calculated ones.

v2:
 - Update WOPCM register only if it's not locked

Signed-off-by: Jackie Li 
Cc: Michal Wajdeczko 
Cc: Sagar Arun Kamble 
Cc: Michal Winiarski 
Cc: John Spotswood 
Cc: Joonas Lahtinen 
---
 drivers/gpu/drm/i915/intel_wopcm.c | 217 
+++--

 1 file changed, 185 insertions(+), 32 deletions(-)

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

index b1c08ca..fa8d2be 100644
--- a/drivers/gpu/drm/i915/intel_wopcm.c
+++ b/drivers/gpu/drm/i915/intel_wopcm.c
@@ -140,6 +140,53 @@ static inline int check_hw_restriction(struct 
drm_i915_private *i915,

 return err;
 }
+static inline u32 calculate_min_guc_wopcm_base(u32 huc_fw_size)
+{
+    return ALIGN(huc_fw_size + WOPCM_RESERVED_SIZE,
+ GUC_WOPCM_OFFSET_ALIGNMENT);
+}
+
+static inline u32 calculate_min_guc_wopcm_size(u32 guc_fw_size)
+{
+    return guc_fw_size + GUC_WOPCM_RESERVED + GUC_WOPCM_STACK_RESERVED;
+}
+
+static inline int calculate_max_guc_wopcm_size(struct intel_wopcm 
*wopcm,

+   u32 guc_wopcm_base,
+   u32 *guc_wopcm_size)


Can't we just return this size as positive value?
I guess wopcm size will never be larger than MAX_INT.
We can add GEM_BUG_ON to be sure.


+{
+    struct drm_i915_private *i915 = wopcm_to_i915(wopcm);
+    u32 ctx_rsvd = context_reserved_size(i915);
+
+    if (guc_wopcm_base >= wopcm->size ||
+    (guc_wopcm_base + ctx_rsvd) >= wopcm->size) {
+    DRM_ERROR("GuC WOPCM base (%uKiB) is too big.\n",
+  guc_wopcm_base / 1024);
+    return -E2BIG;


You are mixing calculations with verifications here.
Focus on calculations as you have separate function that verifies values.


+    }
+
+    *guc_wopcm_size =
+    (wopcm->size - guc_wopcm_base - ctx_rsvd) & 
GUC_WOPCM_SIZE_MASK;

+
+    return 0;
+}
+
+static inline int verify_calculated_values(struct drm_i915_private


hmm, maybe we can unify somehow this verification to be able to work with
both calculated and locked values?
I actually thought about that. However, since the verification based on 
the assumption
that the unlocked reg value could be any numbers so it puts more checks 
on these values
which are unnecessary during the calculation. I've made the responding 
check common

enough to be reused by this verification code and the calculation code.



*i915,
+   u32 guc_fw_size, u32 huc_fw_size,
+   u32 guc_wopcm_base,
+   u32 guc_wopcm_size)
+{
+    if (guc_wopcm_size < calculate_min_guc_wopcm_size(guc_fw_size)) {
+    DRM_ERROR("Need %uKiB WOPCM for GuC FW, %uKiB available.\n",
+  calculate_min_guc_wopcm_size(guc_fw_size),


you are calling calculate_min_guc_wopcm_size() twice

Will update it.



+  guc_wopcm_size / 1024);
+    return -E2BIG;
+    }
+
+    return check_hw_restriction(i915, guc_wopcm_base, guc_wopcm_size,
+    huc_fw_size);
+}
+
 /**
  * intel_wopcm_init() - Initialize the WOPCM structure.
  * @wopcm: pointer to intel_wopcm.
@@ -157,10 +204,8 @@ int intel_wopcm_init(struct intel_wopcm *wopcm)
 struct drm_i915_private *i915 = wopcm_to_i915(wopcm);
 u32 guc_fw_size = intel_uc_fw_get_upload_size(>guc.fw);
 u32 huc_fw_size = intel_uc_fw_get_upload_size(>huc.fw);
-    u32 ctx_rsvd = context_reserved_size(i915);
 u32 guc_wopcm_base;
 u32 guc_wopcm_size;
-    u32 guc_wopcm_rsvd;
 int err;
GEM_BUG_ON(!wopcm->size);
@@ -177,35 +222,121 @@ int intel_wopcm_init(struct intel_wopcm *wopcm)
 return -E2BIG;
 }
-    guc_wopcm_base = ALIGN(huc_fw_size + WOPCM_RESERVED_SIZE,
-   GUC_WOPCM_OFFSET_ALIGNMENT);
-    if ((guc_wopcm_base + ctx_rsvd) >= wopcm->size) {
-    DRM_ERROR("GuC WOPCM base (%uKiB) is too big.\n",
-  guc_wopcm_base / 1024);
+    guc_wopcm_base = calculate_min_guc_wopcm_base(huc_fw_size);
+    err = calculate_max_guc_wopcm_size(wopcm, guc_wopcm_base,
+   _wopcm_size);
+    if (err)
+    return err;
+
+    DRM_DEBUG_DRIVER("Calculated GuC WOPCM 

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Add LSPCON's information in i915_display_info

[Patchwork]

== Series Details ==

Series: drm/i915: Add LSPCON's information in i915_display_info
URL   : https://patchwork.freedesktop.org/series/41763/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4057 -> Patchwork_8698 =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_8698 need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_8698, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: 
https://patchwork.freedesktop.org/api/1.0/series/41763/revisions/1/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_8698:

  === IGT changes ===

 Warnings 

igt@gem_exec_gttfill@basic:
  fi-pnv-d510:SKIP -> PASS


== Known issues ==

  Here are the changes found in Patchwork_8698 that come from known issues:

  === IGT changes ===

 Issues hit 

igt@gem_mmap_gtt@basic-small-bo-tiledx:
  fi-gdg-551: PASS -> FAIL (fdo#102575)

igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
  fi-ivb-3520m:   PASS -> DMESG-WARN (fdo#106084)


 Possible fixes 

igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
  fi-ivb-3520m:   DMESG-WARN (fdo#106084) -> PASS


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


== Participating hosts (36 -> 33) ==

  Missing(3): fi-ctg-p8600 fi-ilk-m540 fi-skl-6700hq 


== Build changes ==

* Linux: CI_DRM_4057 -> Patchwork_8698

  CI_DRM_4057: ac35105865ed2d4d9676fdb4752798b047fb126a @ 
git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4432: 8b77704db49167f7ebfd1c470d9c129d3b862cb6 @ 
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_8698: 27030e8a20d6e8027db0bab301ab8563c48f9b41 @ 
git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4432: 93b35926a150e318439d2505901288594b3548f5 @ 
git://anongit.freedesktop.org/piglit


== Linux commits ==

27030e8a20d6 drm/i915: Add LSPCON's information in i915_display_info

== Logs ==

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

Re: [Intel-gfx] [Freedreno] [PATCH 01/10] include: Move ascii85 functions from i915 to linux/ascii85.h

[Eric Anholt]

Chris Wilson  writes:

> Quoting Jordan Crouse (2018-04-05 23:06:53)
>> On Thu, Apr 05, 2018 at 04:00:47PM -0600, Jordan Crouse wrote:
>> > The i915 DRM driver very cleverly used ascii85 encoding for their
>> > GPU state file. Move the encode functions to a general header file to
>> > support other drivers that might be interested in the same
>> > functionality.
>> 
>> In a previous version of this patch, Chris asked what tree I wanted this 
>> applied
>> to, and the answer is: I'm not sure?  I'm hoping that Rob will be cool with
>> picking the rest up for msm-next for 4.18 but I'm okay with putting this
>> particular patch wherever it is easiest for the maintainers.
>
> We are a bit late to sneak it into the 4.17 drm base via i915. I don't
> anticipate any problems (for i915) with this patch going in through
> msm-next, so happy to have it land there and percolate back to i915 3
> months later.

I'd love to have it in drm-misc-next, so I can build a similar hang dump
interface for vc5.  But most importantly, I'd like to have it somewhere
soon.


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

Re: [Intel-gfx] [PATCH v3 2/4] drm/i915: Always set HUC_LOADING_AGENT_GUC bit in WOPCM offset register

[Yaodong Li]

On 04/13/2018 07:26 PM, Michal Wajdeczko wrote:
On Tue, 10 Apr 2018 02:42:18 +0200, Jackie Li  
wrote:



The enable_guc modparam is used to enable/disable GuC/HuC FW uploading
dynamcially during i915 module loading. If WOPCM offset register was

  
typo


D'oh! I really need a tool for this! Thanks, will fix it.

locked
without having HUC_LOADING_AGENT_GUC bit set to 1, the module reloading
with both GuC and HuC FW will fail since we need to set this bit to 1 
for

HuC FW uploading.

Since HUC_LOADING_AGENT_GUC bit has no impact on GuC FW uploading, this
patch updates the register updating code to make sure the WOPCM offset
register is always locked with HUC_LOADING_AGENT_GUC bit set to 1 which
will guarantee successful uploading of both GuC and HuC FW. We will 
further

take care of the locked values in the following enhancement patch.

Signed-off-by: Jackie Li 
Cc: Michal Wajdeczko 
Cc: Sagar Arun Kamble 
Cc: Michal Winiarski 
Cc: John Spotswood 
Cc: Joonas Lahtinen 
---
 drivers/gpu/drm/i915/intel_wopcm.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

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

index 74bf76f..b1c08ca 100644
--- a/drivers/gpu/drm/i915/intel_wopcm.c
+++ b/drivers/gpu/drm/i915/intel_wopcm.c
@@ -238,8 +238,6 @@ static inline int write_and_verify(struct 
drm_i915_private *dev_priv,

 int intel_wopcm_init_hw(struct intel_wopcm *wopcm)
 {
 struct drm_i915_private *dev_priv = wopcm_to_i915(wopcm);
-    u32 huc_agent;
-    u32 mask;
 int err;
if (!USES_GUC(dev_priv))
@@ -255,10 +253,10 @@ int intel_wopcm_init_hw(struct intel_wopcm *wopcm)
 if (err)
 goto err_out;
-    huc_agent = USES_HUC(dev_priv) ? HUC_LOADING_AGENT_GUC : 0;
-    mask = GUC_WOPCM_OFFSET_MASK | GUC_WOPCM_OFFSET_VALID | huc_agent;
 err = write_and_verify(dev_priv, DMA_GUC_WOPCM_OFFSET,
-   wopcm->guc.base | huc_agent, mask,
+   wopcm->guc.base | HUC_LOADING_AGENT_GUC,
+   GUC_WOPCM_OFFSET_MASK | HUC_LOADING_AGENT_GUC |
+   GUC_WOPCM_OFFSET_VALID,


while we can unconditionally set HUC_AGENT bit, there is no need to 
verify

it unless we are using HuC, so we can consider leaving old mask intact.
The idea is to verify the written values are exactly we want, so I think 
it better

to keep doing it in this way.

Thanks,
Jackie
___
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: Fix LSPCON TMDS output buffer enabling from low-power state

[Patchwork]

== Series Details ==

Series: drm/i915: Fix LSPCON TMDS output buffer enabling from low-power state
URL   : https://patchwork.freedesktop.org/series/41759/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4057 -> Patchwork_8697 =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_8697 need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_8697, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: 
https://patchwork.freedesktop.org/api/1.0/series/41759/revisions/1/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_8697:

  === IGT changes ===

 Warnings 

igt@gem_exec_gttfill@basic:
  fi-pnv-d510:SKIP -> PASS


== Known issues ==

  Here are the changes found in Patchwork_8697 that come from known issues:

  === IGT changes ===

 Issues hit 

igt@debugfs_test@read_all_entries:
  fi-snb-2520m:   PASS -> INCOMPLETE (fdo#103713)

igt@gem_mmap_gtt@basic-small-bo-tiledx:
  fi-gdg-551: PASS -> FAIL (fdo#102575)


 Possible fixes 

igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
  fi-ivb-3520m:   DMESG-WARN (fdo#106084) -> PASS +1


  fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575
  fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
  fdo#106084 https://bugs.freedesktop.org/show_bug.cgi?id=106084


== Participating hosts (36 -> 33) ==

  Missing(3): fi-ctg-p8600 fi-ilk-m540 fi-skl-6700hq 


== Build changes ==

* Linux: CI_DRM_4057 -> Patchwork_8697

  CI_DRM_4057: ac35105865ed2d4d9676fdb4752798b047fb126a @ 
git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4432: 8b77704db49167f7ebfd1c470d9c129d3b862cb6 @ 
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_8697: 5c78b9dc05bc38b3d767abc50a33b889e7cfd29d @ 
git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4432: 93b35926a150e318439d2505901288594b3548f5 @ 
git://anongit.freedesktop.org/piglit


== Linux commits ==

5c78b9dc05bc drm/i915: Fix LSPCON TMDS output buffer enabling from low-power 
state

== Logs ==

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

Re: [Intel-gfx] [PATCH v3 1/4] drm/i915: Always do WOPCM partitioning based on real firmware sizes

[Yaodong Li]

On 04/13/2018 07:15 PM, Michal Wajdeczko wrote:
On Tue, 10 Apr 2018 02:42:17 +0200, Jackie Li  
wrote:



After enabled the WOPCM write-once registers locking status checking,
reloading of the i915 module will fail with modparam enable_guc set to 3
(enable GuC and HuC firmware loading) if the module was originally 
loaded

with enable_guc set to 1 (only enable GuC firmware loading).


Is this frequent and required scenario ? or just for debug/development ?

My understanding is this should be a nice to have feature and mainly for 
debugging.

This is
because WOPCM registers were updated and locked without considering 
the HuC

FW size. Since we need both GuC and HuC FW sizes to determine the final
layout of WOPCM, we should always calculate the WOPCM layout based on 
the
actual sizes of the GuC and HuC firmware available for a specific 
platform

if we need continue to support enable/disable HuC FW loading dynamically
with enable_guc modparam.

This patch splits uC firmware fetching into two stages. First stage 
is to
fetch the firmware image and verify the firmware header. uC firmware 
will

be marked as verified and this will make FW info available for following
WOPCM layout calculation. The second stage is to create a GEM object and
copy the FW data into the created GEM object which will only be 
available
when GuC/HuC loading is enabled by enable_guc modparam. This will 
guarantee
that the WOPCM layout will be always be calculated correctly without 
making

any assumptions to the GuC and HuC firmware sizes.


You are also assuming that on reload exactly the same GuC/HuC firmwares
will bee used as in initial run. This will make this useless for debug/
development scenarios, where custom fw are likely to be specified.

This patch is mainly for providing a real fix to support 
enable_guc=1->3->1 use case.
It based on the fact that it is inevitable that sometimes we need to 
reboot the system

if the status of the fw was changed on the file system.
I am not sure how often we switch between different HuC FW with 
different sizes?

If we want to support enable_guc=1->3->1 scenarios for debug/dev then
maybe more flexible will be other approach that makes allocations from
the other end as proposed in [1]

[1] https://patchwork.freedesktop.org/patch/212471/
Actually, I do think this might be one of the options, and I've also put 
some comments on this
series. The main concern I have is it still make assumption on the GuC 
FW size and may

run into the same issue if the GuC FW failed to meet the requirement.
and for debugging purpose it would have the same possible for different 
GuC FW debugging.




v3:
 - Rebase

Signed-off-by: Jackie Li 
Cc: Michal Wajdeczko 
Cc: Sagar Arun Kamble 
Cc: Michal Winiarski 
Cc: John Spotswood 
Cc: Joonas Lahtinen 
---
 drivers/gpu/drm/i915/intel_uc.c    | 14 --
 drivers/gpu/drm/i915/intel_uc_fw.c | 31 ---
 drivers/gpu/drm/i915/intel_uc_fw.h |  7 +--
 3 files changed, 29 insertions(+), 23 deletions(-)

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

index 1cffaf7..73b8f6c 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -172,11 +172,8 @@ void intel_uc_init_early(struct drm_i915_private 
*i915)

sanitize_options_early(i915);
-    if (USES_GUC(i915))
-    intel_uc_fw_fetch(i915, >fw);
-
-    if (USES_HUC(i915))
-    intel_uc_fw_fetch(i915, >fw);
+    intel_uc_fw_fetch(i915, >fw, USES_GUC(i915));
+    intel_uc_fw_fetch(i915, >fw, USES_HUC(i915));


Hmm, side effect of those unconditional fetches might be unwanted 
warnings
about missing firmwares (on configs with disabled guc) as well as 
extended

driver load time.
Hmm, if HAS_GUC is false then fw path would be NULL. The fetch will 
return directly.


Do we really need to support this corner case enable_guc=1->3 at all 
costs?
I think this is the real solution for this issue (with no assumption). 
However, we do
need to decide whether we should support such a corner case which is 
mainly for

debugging.


/michal


 }
void intel_uc_cleanup_early(struct drm_i915_private *i915)
@@ -184,11 +181,8 @@ void intel_uc_cleanup_early(struct 
drm_i915_private *i915)

 struct intel_guc *guc = >guc;
 struct intel_huc *huc = >huc;
-    if (USES_HUC(i915))
-    intel_uc_fw_fini(>fw);
-
-    if (USES_GUC(i915))
-    intel_uc_fw_fini(>fw);
+    intel_uc_fw_fini(>fw);
+    intel_uc_fw_fini(>fw);
guc_free_load_err_log(guc);
 }
diff --git a/drivers/gpu/drm/i915/intel_uc_fw.c 
b/drivers/gpu/drm/i915/intel_uc_fw.c

index 6e8e0b5..a9cb900 100644
--- a/drivers/gpu/drm/i915/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/intel_uc_fw.c
@@ -33,11 +33,13 @@
  *
  * @dev_priv: device private
  * @uc_fw: uC firmware
+ * @copy_to_obj: 

Re: [Intel-gfx] [PATCH 0/9] GPU-bound energy efficiency improvements for the intel_pstate driver.

[Srinivas Pandruvada]

On Mon, 2018-04-16 at 17:04 +0300, Eero Tamminen wrote:
> Hi,
> 
> On 14.04.2018 07:01, Srinivas Pandruvada wrote:
> > Hi Francisco,
> > 
> > [...]
> > 
> > > Are you no longer interested in improving those aspects of the
> > > non-
> > > HWP
> > > governor?  Is it that you're planning to delete it and move back
> > > to a
> > > generic cpufreq governor for non-HWP platforms in the near
> > > future?
> > 
> > Yes that is the plan for Atom platforms, which are only non HWP
> > platforms till now. You have to show good gain for performance and
> > performance/watt to carry and maintain such big change. So we have
> > to
> > see your performance and power numbers.
> 
> For the active cases, you can look at the links at the beginning / 
> bottom of this mail thread.  Francisco provided performance results
> for 
>  >100 benchmarks.
Looks like you didn't test the idle cases, which are more important.
Systems will tend to be more idle (increased +50% by the patches). Once
you fix the idle, you have to retest and then results will be
interesting.

Once you fix this, then it is pure algorithm, whether it is done in
intel-pstate or sched-util governor is not a big different. It is
better to do in sched-util as this will benefit all architectures and
will get better test coverage and maintained.

Thanks,
Srinivas



> 
> 
> At this side of Atlantic, we've been testing different versions of
> the 
> patchset in past few months for >50 Linux 3D benchmarks on 6
> different 
> platforms.
> 
> On Geminilake and few BXT configurations (where 3D benchmarks are
> TDP 
> limited), many tests' performance improves by 5-15%, also complex
> ones. 
> And more importantly, there were no regressions.
> 
> (You can see details + links to more info in Jira ticket VIZ-12078.)
> 
> *On (fully) TDP limited cases, power usage (obviously) keeps the
> same, 
> so performance/watt improvements can be derived from the measured 
> performance improvements.*
> 
> 
> We have data also for earlier platforms from slightly older versions
> of 
> the patchset, but on those it didn't have any significant impact on 
> performance.
> 
> I think the main reason for this is that BYT & BSW NUCs that we
> have, 
> have space only for single memory module.  Without dual-memory
> channel 
> configuration, benchmarks are too memory-bottlenecked to utilized
> GPU 
> enough to make things TDP limited on those platforms.
> 
> However, now that I look at the old BYT & BSW data (for few
> benchmarks 
> which improved most on BXT & GLK), I see that there's a reduction in
> the 
> CPU power utilization according to RAPL, at least on BSW.
> 
> 
>   - Eero
> 
> 
> > > > This will benefit all architectures including x86 + non i915.
> > > > 
> > > 
> > > The current design encourages re-use of the IO utilization
> > > statistic
> > > (see PATCH 1) by other governors as a mechanism driving the
> > > trade-off
> > > between energy efficiency and responsiveness based on whether the
> > > system
> > > is close to CPU-bound, in whatever way is applicable to each
> > > governor
> > > (e.g. it would make sense for it to be hooked up to the EPP
> > > preference
> > > knob in the case of the intel_pstate HWP governor, which would
> > > allow
> > > it
> > > to achieve better energy efficiency in IO-bound situations just
> > > like
> > > this series does for non-HWP parts).  There's nothing really x86-
> > > nor
> > > i915-specific about it.
> > > 
> > > > BTW intel-pstate can be driven by sched-util governor (passive
> > > > mode),
> > > > so if your prove benefits to Broxton, this can be a default.
> > > > As before:
> > > > - No regression to idle power at all. This is more important
> > > > than
> > > > benchmarks
> > > > - Not just score, performance/watt is important
> > > > 
> > > 
> > > Is schedutil actually on par with the intel_pstate non-HWP
> > > governor
> > > as
> > > of today, according to these metrics and the overall benchmark
> > > numbers?
> > 
> > Yes, except for few cases. I have not tested recently, so may be
> > better.
> > 
> > Thanks,
> > Srinivas
> > 
> > 
> > > > Thanks,
> > > > Srinivas
> > > > 
> > > > 
> > > > > > controller does, even though the frequent IO waits may
> > > > > > actually
> > > > > > be
> > > > > > an
> > > > > > indication that the system is IO-bound (which means that
> > > > > > the
> > > > > > large
> > > > > > energy usage increase may not be translated in any
> > > > > > performance
> > > > > > benefit
> > > > > > in practice, not to speak of performance being impacted
> > > > > > negatively
> > > > > > in
> > > > > > TDP-bound scenarios like GPU rendering).
> > > > > > 
> > > > > > Regarding run-time complexity, I haven't observed this
> > > > > > governor
> > > > > > to
> > > > > > be
> > > > > > measurably more computationally intensive than the present
> > > > > > one.  It's a
> > > > > > bunch more instructions indeed, but still within the same
> > > > > > ballpark
> > > > > > as
> > > > > > the current governor.  The 

Re: [Intel-gfx] [PATCH v1 2/2] i915: content-type property for HDMI connector

[Sean Paul]

On Tue, Apr 17, 2018 at 02:16:59AM +0300, StanLis wrote:
> From: Stanislav Lisovskiy 
> 
> Added encoding of drm content_type property from
> drm_connector_state within AVI infoframe in order to properly handle
> external HDMI TV content-type setting.
> 
> Signed-off-by: Stanislav Lisovskiy 

Hi Stanislav,
Thank you for your patch.

> ---
>  drivers/gpu/drm/i915/intel_atomic.c |  1 +
>  drivers/gpu/drm/i915/intel_drv.h|  1 +
>  drivers/gpu/drm/i915/intel_hdmi.c   |  4 
>  drivers/gpu/drm/i915/intel_modes.c  | 10 ++
>  4 files changed, 16 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_atomic.c 
> b/drivers/gpu/drm/i915/intel_atomic.c
> index 40285d1b91b7..61ddb5871d8a 100644
> --- a/drivers/gpu/drm/i915/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> @@ -124,6 +124,7 @@ int intel_digital_connector_atomic_check(struct 
> drm_connector *conn,
>   if (new_conn_state->force_audio != old_conn_state->force_audio ||
>   new_conn_state->broadcast_rgb != old_conn_state->broadcast_rgb ||
>   new_conn_state->base.picture_aspect_ratio != 
> old_conn_state->base.picture_aspect_ratio ||
> + new_conn_state->base.content_type != 
> old_conn_state->base.content_type ||
>   new_conn_state->base.scaling_mode != 
> old_conn_state->base.scaling_mode)
>   crtc_state->mode_changed = true;
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h 
> b/drivers/gpu/drm/i915/intel_drv.h
> index 5bd2263407b2..07fd7ba21f38 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1818,6 +1818,7 @@ int intel_ddc_get_modes(struct drm_connector *c, struct 
> i2c_adapter *adapter);
>  void intel_attach_force_audio_property(struct drm_connector *connector);
>  void intel_attach_broadcast_rgb_property(struct drm_connector *connector);
>  void intel_attach_aspect_ratio_property(struct drm_connector *connector);
> +void intel_attach_content_type_property(struct drm_connector *connector);
>  
>  
>  /* intel_overlay.c */
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
> b/drivers/gpu/drm/i915/intel_hdmi.c
> index ee929f31f7db..cd484276e9b0 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -491,6 +491,8 @@ static void intel_hdmi_set_avi_infoframe(struct 
> drm_encoder *encoder,
>  
> intel_hdmi->rgb_quant_range_selectable,
>  is_hdmi2_sink);
>  
> + frame.avi.content_type = connector->state->content_type;
> +
>   /* TODO: handle pixel repetition for YCBCR420 outputs */
>   intel_write_infoframe(encoder, crtc_state, );
>  }
> @@ -2065,7 +2067,9 @@ intel_hdmi_add_properties(struct intel_hdmi 
> *intel_hdmi, struct drm_connector *c
>   intel_attach_force_audio_property(connector);
>   intel_attach_broadcast_rgb_property(connector);
>   intel_attach_aspect_ratio_property(connector);
> + intel_attach_content_type_property(connector);
>   connector->state->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
> + connector->state->content_type = HDMI_CONTENT_TYPE_GRAPHICS;

This is redudant, the attach function already sets this.

>  }
>  
>  /*
> diff --git a/drivers/gpu/drm/i915/intel_modes.c 
> b/drivers/gpu/drm/i915/intel_modes.c
> index b39846613e3c..232811ab71a3 100644
> --- a/drivers/gpu/drm/i915/intel_modes.c
> +++ b/drivers/gpu/drm/i915/intel_modes.c
> @@ -133,3 +133,13 @@ intel_attach_aspect_ratio_property(struct drm_connector 
> *connector)
>   connector->dev->mode_config.aspect_ratio_property,
>   DRM_MODE_PICTURE_ASPECT_NONE);
>  }
> +
> +void
> +intel_attach_content_type_property(struct drm_connector *connector)
> +{
> + if (!drm_mode_create_content_type_property(connector->dev))
> + drm_object_attach_property(>base,
> + connector->dev->mode_config.content_type_property,
> + DRM_MODE_CONTENT_TYPE_GRAPHICS);
> +}

I think the "in" thing to do is to add this helper to the core, since this is a
core property.

Sean

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

-- 
Sean Paul, Software Engineer, Google / Chromium OS
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915: Add LSPCON's information in i915_display_info

[Shashank Sharma]

Sometimes it gets difficult for an end-user to know presence
of a LSPCON device, as the encoder is enumerated as DP device.
This patch adds a string "LSPCON present: " in the display_info
so that a user can easily know if there is a active LSPCON device
on this encoder.

Cc: Ville Syrjala 
Signed-off-by: Shashank Sharma 
---
 drivers/gpu/drm/i915/i915_debugfs.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index 785b710..4582757 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2970,12 +2970,17 @@ static void intel_dp_info(struct seq_file *m,
  struct intel_connector *intel_connector)
 {
struct intel_encoder *intel_encoder = intel_connector->encoder;
-   struct intel_dp *intel_dp = enc_to_intel_dp(_encoder->base);
+   struct intel_digital_port *intel_dig_port =
+   enc_to_dig_port(_encoder->base);
+   struct intel_dp *intel_dp = _dig_port->dp;
+   struct intel_lspcon *lspcon = _dig_port->lspcon;
 
seq_printf(m, "\tDPCD rev: %x\n", intel_dp->dpcd[DP_DPCD_REV]);
seq_printf(m, "\taudio support: %s\n", yesno(intel_dp->has_audio));
if (intel_connector->base.connector_type == DRM_MODE_CONNECTOR_eDP)
intel_panel_info(m, _connector->panel);
+   else
+   seq_printf(m, "\tLSPCON present: %s\n", yesno(lspcon->active));
 
drm_dp_downstream_debug(m, intel_dp->dpcd, intel_dp->downstream_ports,
_dp->aux);
-- 
2.7.4

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

Re: [Intel-gfx] [PATCH v5] drm: Fix HDCP downstream dev count read

[Sean Paul]

On Thu, Apr 5, 2018 at 8:09 AM Ramalingam C  wrote:

> In both HDMI and DP, device count is represented by 6:0 bits of a
> register(BInfo/Bstatus)

> So macro for bitmasking the device_count is fixed(0x3F->0x7F).

> v3:
>Retained the Rb-ed.
> v4:
>%s/drm\/i915/drm [rodrigo]
> v5:
>Added "Fixes:" and HDCP keyword in subject [Rodrigo, Sean Paul]

> Signed-off-by: Ramalingam C 


Thank you for your patch, I've pushed it to -misc-fixes

Sean


> Fixes: 495eb7f877ab drm: Add some HDCP related #defines
> cc: Sean Paul 
> Reviewed-by: Sean Paul 
> ---
>   include/drm/drm_hdcp.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

> diff --git a/include/drm/drm_hdcp.h b/include/drm/drm_hdcp.h
> index 562fa7df2637..98e63d870139 100644
> --- a/include/drm/drm_hdcp.h
> +++ b/include/drm/drm_hdcp.h
> @@ -19,7 +19,7 @@
>   #define DRM_HDCP_RI_LEN2
>   #define DRM_HDCP_V_PRIME_PART_LEN  4
>   #define DRM_HDCP_V_PRIME_NUM_PARTS 5
> -#define DRM_HDCP_NUM_DOWNSTREAM(x) (x & 0x3f)
> +#define DRM_HDCP_NUM_DOWNSTREAM(x) (x & 0x7f)
>   #define DRM_HDCP_MAX_CASCADE_EXCEEDED(x)   (x & BIT(3))
>   #define DRM_HDCP_MAX_DEVICE_EXCEEDED(x)(x & BIT(7))

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

Re: [Intel-gfx] [RFC v4 21/25] drm/fb-helper: Add drm_fb_helper_fb_open/release()

[Noralf Trønnes]

Den 16.04.2018 10.46, skrev Daniel Vetter:

On Sat, Apr 14, 2018 at 01:53:14PM +0200, Noralf Trønnes wrote:

These helpers keep track of fbdev users and drm_driver.last_close will
only restore fbdev when actually in use. Additionally the display is
turned off when the last user is closing. fbcon is a user in this context.

If struct fb_ops is defined in a library, fb_open() takes a ref on the
library (fb_ops.owner) instead of the driver module. Fix that by ensuring
that the driver module is pinned.

The functions are not added to the DRM_FB_HELPER_DEFAULT_OPS() macro,
because some of its users do set fb_open/release themselves.

Signed-off-by: Noralf Trønnes 
---
  drivers/gpu/drm/drm_fb_helper.c | 54 -
  include/drm/drm_fb_helper.h | 29 ++
  2 files changed, 82 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 98e5bc92c9f2..b1124c08b1ed 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -177,7 +177,7 @@ int drm_fb_helper_restore_fbdev_mode_unlocked(struct 
drm_fb_helper *fb_helper)
if (!drm_fbdev_emulation || !fb_helper)
return -ENODEV;
  
-	if (READ_ONCE(fb_helper->deferred_setup))

+   if (READ_ONCE(fb_helper->deferred_setup) || 
!READ_ONCE(fb_helper->open_count))
return 0;
  
  	mutex_lock(_helper->lock);

@@ -368,6 +368,7 @@ void drm_fb_helper_prepare(struct drm_device *dev, struct 
drm_fb_helper *helper,
INIT_WORK(>dirty_work, drm_fb_helper_dirty_work);
helper->dirty_clip.x1 = helper->dirty_clip.y1 = ~0;
mutex_init(>lock);
+   helper->open_count = 1;
helper->funcs = funcs;
helper->dev = dev;
  }
@@ -620,6 +621,53 @@ int drm_fb_helper_defio_init(struct drm_fb_helper 
*fb_helper)
  }
  EXPORT_SYMBOL(drm_fb_helper_defio_init);
  
+/**

+ * drm_fb_helper_fb_open - implementation for _ops.fb_open
+ * @info: fbdev registered by the helper
+ * @user: 1=userspace, 0=fbcon
+ *
+ * Increase fbdev use count.
+ * If _ops is wrapped in a library, pin the driver module.
+ */
+int drm_fb_helper_fb_open(struct fb_info *info, int user)
+{
+   struct drm_fb_helper *fb_helper = info->par;
+   struct drm_device *dev = fb_helper->dev;
+
+   if (info->fbops->owner != dev->driver->fops->owner) {
+   if (!try_module_get(dev->driver->fops->owner))
+   return -ENODEV;
+   }
+
+   fb_helper->open_count++;
+
+   return 0;
+}
+EXPORT_SYMBOL(drm_fb_helper_fb_open);
+
+/**
+ * drm_fb_helper_fb_release - implementation for _ops.fb_release
+ * @info: fbdev registered by the helper
+ * @user: 1=userspace, 0=fbcon
+ *
+ * Decrease fbdev use count and turn off if there are no users left.
+ * If _ops is wrapped in a library, unpin the driver module.
+ */
+int drm_fb_helper_fb_release(struct fb_info *info, int user)
+{
+   struct drm_fb_helper *fb_helper = info->par;
+   struct drm_device *dev = fb_helper->dev;
+
+   if (!(--fb_helper->open_count))
+   drm_fb_helper_dpms(info, DRM_MODE_DPMS_OFF);
+
+   if (info->fbops->owner != dev->driver->fops->owner)
+   module_put(dev->driver->fops->owner);
+
+   return 0;
+}
+EXPORT_SYMBOL(drm_fb_helper_fb_release);
+
  /**
   * drm_fb_helper_sys_read - wrapper around fb_sys_read
   * @info: fb_info struct pointer
@@ -1436,6 +1484,10 @@ static int drm_fb_helper_single_fb_probe(struct 
drm_fb_helper *fb_helper,
if (ret < 0)
return ret;
  
+	/* Block restore without users if we do track it */

+   if (fb_helper->fbdev->fbops->fb_open == drm_fb_helper_fb_open)
+   fb_helper->open_count = 0;

Since we kzcalloc, isn't this entirely redundant? Looks confusing to me at
least.


I have to keep the existing restore behaviour for those that don't use
drm_fb_helper_fb_open(). I do this by setting fb_helper->open_count = 1
in drm_fb_helper_prepare(). I have tried to give a describtion in the
@open_count docs.

Ofc I could have changed all drivers to use drm_fb_helper_fb_open(), but
I think that time is better spent moving drivers over to generic fbdev
instead.

Noralf.


Otherwise looks all reasonable.
-Daniel



+
strcpy(fb_helper->fb->comm, "[fbcon]");
return 0;
  }
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index 5f66f253a97b..330983975d5e 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -184,6 +184,22 @@ struct drm_fb_helper {
 * See also: @deferred_setup
 */
int preferred_bpp;
+
+   /**
+* @open_count:
+*
+* Keeps track of fbdev use to know when to not restore fbdev and to
+* disable the pipeline when the last user is gone.
+*
+* Drivers that use drm_fb_helper_fb_open() as their \.fb_open
+* callback will get an initial value of 0 and get 

Re: [Intel-gfx] [RFC v4 19/25] drm/client: Finish the in-kernel client API

[Noralf Trønnes]

Den 16.04.2018 10.27, skrev Daniel Vetter:

On Sat, Apr 14, 2018 at 01:53:12PM +0200, Noralf Trønnes wrote:

The modesetting code is already present, this adds the rest of the API.

Mentioning the TODO in the commit message would be good. Helps readers
like me who have an attention span measured in seconds :-)

Just commenting on the create_buffer leak here


+static struct drm_client_buffer *
+drm_client_buffer_create(struct drm_client_dev *client, u32 width, u32 height, 
u32 format)
+{
+   struct drm_mode_create_dumb dumb_args = { };
+   struct drm_prime_handle prime_args = { };
+   struct drm_client_buffer *buffer;
+   struct dma_buf *dma_buf;
+   void *vaddr;
+   int ret;
+
+   buffer = kzalloc(sizeof(*buffer), GFP_KERNEL);
+   if (!buffer)
+   return ERR_PTR(-ENOMEM);
+
+   buffer->client = client;
+   buffer->width = width;
+   buffer->height = height;
+   buffer->format = format;
+
+   dumb_args.width = buffer->width;
+   dumb_args.height = buffer->height;
+   dumb_args.bpp = drm_format_plane_cpp(format, 0) * 8;
+   ret = drm_mode_create_dumb(client->dev, _args, client->file);
+   if (ret)
+   goto err_free;
+
+   buffer->handle = dumb_args.handle;
+   buffer->pitch = dumb_args.pitch;
+   buffer->size = dumb_args.size;
+
+   prime_args.handle = dumb_args.handle;
+   ret = drm_prime_handle_to_fd(client->dev, _args, client->file);
+   if (ret)
+   goto err_delete;
+
+   dma_buf = dma_buf_get(prime_args.fd);
+   if (IS_ERR(dma_buf)) {
+   ret = PTR_ERR(dma_buf);
+   goto err_delete;
+   }
+
+   /*
+* If called from a worker the dmabuf fd isn't closed and the ref
+* doesn't drop to zero on free.
+* If I use __close_fd() it's all fine, but that function is not 
exported.
+*
+* How do I get rid of this fd when in a worker/kernel thread?
+* The fd isn't used beyond this function.
+*/
+// WARN_ON(__close_fd(current->files, prime_args.fd));

Hm, this isn't 100% what I had in mind as the sequence for generic buffer
creation. Pseudo-code:

ret = drm_mode_create_dumb(client->dev, _args, client->file);
if (ret)
goto err_free;

gem_bo = drm_gem_object_lookup(client->file, dumb_args.handle);

gives you _really_ directly the underlying gem_bo. Of course this doesn't
work for non-gem based driver, but reality is that (almost) all of them
are. And we will not accept any new drivers which aren't gem based. So
ignoring vmwgfx for this drm_client work is imo perfectly fine. We should
ofc keep the option in the fb helpers to use non-gem buffers (so that
vmwgfx could switch over from their own in-driver fbdev helpers). All we
need for that is to keep the fb_probe callback.

Was there any other reason than vmwgfx for using prime buffers instead of
just directly using gem?


The reason for using a prime buffer is that it gives me easy access to a
dma_buf which I use to get the virtual address (dma_buf_vmap) and for
mmap (dma_buf_mmap).

Would this stripped down version of drm_gem_prime_handle_to_fd() work?

struct dma_buf *drm_gem_to_dmabuf(struct drm_gem_object *obj)
{
    struct dma_buf *dmabuf;

    mutex_lock(>dev->object_name_lock);
    /* re-export the original imported object */
    if (obj->import_attach) {
        dmabuf = obj->import_attach->dmabuf;
        get_dma_buf(dmabuf);
        goto out;
    }

    if (obj->dma_buf) {
        dmabuf = obj->dma_buf;
        get_dma_buf(dmabuf);
        goto out;
    }

    dmabuf = export_and_register_object(obj->dev, obj, 0);
out:
    mutex_unlock(>dev->object_name_lock);

    return dmabuf;
}

Now I could do this:

    ret = drm_mode_create_dumb(dev, _args, file);

    obj = drm_gem_object_lookup(file, dumb_args.handle);

    dmabuf = drm_gem_to_dmabuf(obj);

    vaddr = dma_buf_vmap(dmabuf);


Noralf.

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

[Intel-gfx] [PATCH] drm/i915: Fix LSPCON TMDS output buffer enabling from low-power state

[Imre Deak]

LSPCON adapters in low-power state may ignore the first I2C write during
TMDS output buffer enabling, resulting in a blank screen even with an
otherwise enabled pipe. Fix this by reading back and validating the
written value a few times.

The problem was noticed on GLK machines with an onboard LSPCON adapter
after entering/exiting DC5 power state. Doing an I2C read of the adapter
ID as the first transaction - instead of the I2C write to enable the
TMDS buffers - returns the correct value. Based on this we assume that
the transaction itself is sent properly, it's only the adapter that is
not ready for some reason to accept this first write after waking from
low-power state. In my case the second I2C write attempt always
succeeded.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105854
Cc: Clinton Taylor 
Cc: Ville Syrjälä 
Signed-off-by: Imre Deak 
---
 drivers/gpu/drm/drm_dp_dual_mode_helper.c | 39 +--
 1 file changed, 32 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_dual_mode_helper.c 
b/drivers/gpu/drm/drm_dp_dual_mode_helper.c
index 02a50929af67..e7f4fe2848a5 100644
--- a/drivers/gpu/drm/drm_dp_dual_mode_helper.c
+++ b/drivers/gpu/drm/drm_dp_dual_mode_helper.c
@@ -350,19 +350,44 @@ int drm_dp_dual_mode_set_tmds_output(enum 
drm_dp_dual_mode_type type,
 {
uint8_t tmds_oen = enable ? 0 : DP_DUAL_MODE_TMDS_DISABLE;
ssize_t ret;
+   int retry;
 
if (type < DRM_DP_DUAL_MODE_TYPE2_DVI)
return 0;
 
-   ret = drm_dp_dual_mode_write(adapter, DP_DUAL_MODE_TMDS_OEN,
-_oen, sizeof(tmds_oen));
-   if (ret) {
-   DRM_DEBUG_KMS("Failed to %s TMDS output buffers\n",
- enable ? "enable" : "disable");
-   return ret;
+   /*
+* LSPCON adapters in low-power state may ignore the first write, so
+* read back and verify the written value a few times.
+*/
+   for (retry = 0; retry < 3; retry++) {
+   uint8_t tmp;
+
+   ret = drm_dp_dual_mode_write(adapter, DP_DUAL_MODE_TMDS_OEN,
+_oen, sizeof(tmds_oen));
+   if (ret) {
+   DRM_DEBUG_KMS("Failed to %s TMDS output buffers (%d 
attempts)\n",
+ enable ? "enable" : "disable",
+ retry + 1);
+   return ret;
+   }
+
+   ret = drm_dp_dual_mode_read(adapter, DP_DUAL_MODE_TMDS_OEN,
+   , sizeof(tmp));
+   if (ret) {
+   DRM_DEBUG_KMS("I2C read failed during TMDS output 
buffer %s (%d attempts)\n",
+ enable ? "enabling" : "disabling",
+ retry + 1);
+   return ret;
+   }
+
+   if (tmp == tmds_oen)
+   return 0;
}
 
-   return 0;
+   DRM_DEBUG_KMS("I2C write value mismatch during TMDS output buffer %s\n",
+ enable ? "enabling" : "disabling");
+
+   return -EIO;
 }
 EXPORT_SYMBOL(drm_dp_dual_mode_set_tmds_output);
 
-- 
2.13.2

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

[Intel-gfx] ✗ Fi.CI.IGT: failure for Enabling content-type setting for external HDMI displays. (rev2)

[Patchwork]

== Series Details ==

Series: Enabling content-type setting for external HDMI displays. (rev2)
URL   : https://patchwork.freedesktop.org/series/41744/
State : failure

== Summary ==

= CI Bug Log - changes from CI_DRM_4054_full -> Patchwork_8696_full =

== Summary - FAILURE ==

  Serious unknown changes coming with Patchwork_8696_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_8696_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: 
https://patchwork.freedesktop.org/api/1.0/series/41744/revisions/2/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in 
Patchwork_8696_full:

  === IGT changes ===

 Possible regressions 

igt@kms_cursor_crc@cursor-256x256-suspend:
  shard-snb:  PASS -> DMESG-WARN


 Warnings 

igt@gem_mocs_settings@mocs-rc6-ctx-dirty-render:
  shard-kbl:  SKIP -> PASS +1


== Known issues ==

  Here are the changes found in Patchwork_8696_full that come from known issues:

  === IGT changes ===

 Issues hit 

igt@drv_suspend@fence-restore-untiled:
  shard-kbl:  PASS -> INCOMPLETE (fdo#103665)

igt@kms_rotation_crc@sprite-rotation-180:
  shard-snb:  PASS -> FAIL (fdo#103925)

igt@kms_setmode@basic:
  shard-apl:  PASS -> FAIL (fdo#99912)

igt@kms_sysfs_edid_timing:
  shard-apl:  PASS -> WARN (fdo#100047)


 Possible fixes 

igt@kms_cursor_legacy@flip-vs-cursor-toggle:
  shard-hsw:  FAIL (fdo#102670) -> PASS

igt@kms_flip@2x-plain-flip-ts-check:
  shard-hsw:  FAIL (fdo#100368) -> PASS

igt@kms_flip@wf_vblank-ts-check:
  shard-hsw:  FAIL (fdo#103928) -> PASS


  fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#102670 https://bugs.freedesktop.org/show_bug.cgi?id=102670
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#103925 https://bugs.freedesktop.org/show_bug.cgi?id=103925
  fdo#103928 https://bugs.freedesktop.org/show_bug.cgi?id=103928
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


== Participating hosts (6 -> 4) ==

  Missing(2): shard-glk shard-glkb 


== Build changes ==

* Linux: CI_DRM_4054 -> Patchwork_8696

  CI_DRM_4054: a0e39233b88795009de32094efa8a2135f34338f @ 
git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4430: 8b77704db49167f7ebfd1c470d9c129d3b862cb6 @ 
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_8696: a21e5056a6f99d399a7f76fa8d02094cd8224538 @ 
git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4430: 93b35926a150e318439d2505901288594b3548f5 @ 
git://anongit.freedesktop.org/piglit

== Logs ==

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

[Intel-gfx] [PATCH i-g-t] tests/gem_exec_latency: New subtests for checking submission from RT tasks

[Tvrtko Ursulin]

From: Tvrtko Ursulin 

We want to make sure RT tasks which use a lot of CPU times can submit
batch buffers with roughly the same latency (and certainly not worse)
compared to normal tasks.

Signed-off-by: Tvrtko Ursulin 
Cc: Chris Wilson 
---
 tests/gem_exec_latency.c | 176 +++
 1 file changed, 176 insertions(+)

diff --git a/tests/gem_exec_latency.c b/tests/gem_exec_latency.c
index 9498c0921e60..420ede0f83a0 100644
--- a/tests/gem_exec_latency.c
+++ b/tests/gem_exec_latency.c
@@ -36,11 +36,15 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "drm.h"
 
 #include "igt_sysfs.h"
 #include "igt_vgem.h"
+#include "igt_dummyload.h"
+#include "igt_stats.h"
+
 #include "i915/gem_ring.h"
 
 #define LOCAL_I915_EXEC_NO_RELOC (1<<11)
@@ -351,6 +355,172 @@ static void latency_from_ring(int fd,
}
 }
 
+static void __rearm_spin_batch(igt_spin_t *spin)
+{
+   const uint32_t mi_arb_chk = 0x5 << 23;
+
+   *spin->batch = mi_arb_chk;
+   *spin->running = 0;
+   __sync_synchronize();
+}
+
+static void
+__submit_spin_batch(int fd, igt_spin_t *spin, unsigned int flags)
+{
+   struct drm_i915_gem_execbuffer2 eb = spin->execbuf;
+
+   eb.flags &= ~(0x3f | I915_EXEC_BSD_MASK);
+   eb.flags |= flags | I915_EXEC_NO_RELOC;
+
+   gem_execbuf(fd, );
+}
+
+struct rt_pkt
+{
+#define RT_OK (0)
+#define RT_FAIL (1)
+#define RT_TIMEOUT (2)
+   int status;
+   struct igt_mean mean;
+   double max;
+};
+
+static void __spin_wait(struct rt_pkt *msg, igt_spin_t *spin, double *t_wait)
+{
+   struct timespec ts = { };
+   uint64_t t_last = 0;
+
+   igt_nsec_elapsed();
+
+   while (!READ_ONCE(*spin->running)) {
+   uint64_t t = igt_nsec_elapsed();
+
+   if ((t - t_last) > 5UL * NSEC_PER_SEC) {
+/* Absolute timeout to save time. */
+   msg->status = RT_TIMEOUT;
+   } else if ((t - t_last) > NSEC_PER_SEC / 10) {
+   /* Backoff every 100ms to give it chance to complete. */
+   t_last = t;
+   usleep(1);
+   }
+   }
+
+   *t_wait = igt_nsec_elapsed() / 1e9;
+
+   msg->status = RT_OK;
+}
+
+/*
+ * Test whether RT thread which hogs the CPU a lot can submit work with
+ * reasonable latency.
+ */
+static void
+rthog_latency_on_ring(int fd, unsigned int ring, const char *name)
+{
+   const char *passname[] = { "warmup", "normal", "rt" };
+   struct rt_pkt res[3];
+   unsigned int i;
+   int link[2];
+   int ret;
+
+   igt_require(gem_can_store_dword(fd, ring));
+
+   igt_assert(pipe(link) == 0);
+
+   memset(res, 0, sizeof(res));
+
+igt_fork(child, 1) {
+   unsigned int pass = 0; /* Three passes: warmup, normal, rt. */
+
+   do {
+   struct rt_pkt msg = { };
+   igt_spin_t *spin;
+
+   igt_mean_init();
+
+   if (pass == 2) {
+   struct sched_param rt =
+   { .sched_priority = 99 };
+
+   ret = sched_setscheduler(0,
+   SCHED_FIFO | 
SCHED_RESET_ON_FORK,
+   );
+   if (ret) {
+   igt_warn("Failed to set scheduling 
policy!\n");
+   msg.status = RT_FAIL;
+   write(link[1], , sizeof(msg));
+   exit(1);
+   }
+   }
+
+   spin = __igt_spin_batch_new_poll(fd, 0, ring);
+   if (!spin) {
+   igt_warn("Failed to create spinner! (%s)\n",
+passname[pass]);
+   msg.status = RT_FAIL;
+   write(link[1], , sizeof(msg));
+   exit(1);
+   }
+   igt_spin_busywait_until_running(spin);
+
+   igt_until_timeout(pass > 0 ? 5 : 2) {
+   double t;
+
+   igt_spin_batch_end(spin);
+   gem_sync(fd, spin->handle);
+
+   __rearm_spin_batch(spin);
+   __submit_spin_batch(fd, spin, ring);
+
+   __spin_wait(, spin, );
+   if (msg.status != RT_OK) {
+   igt_warn("Wait timeout! (%s)\n",
+passname[pass]);
+   

[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915: Print error state times relative to capture

[Patchwork]

== Series Details ==

Series: drm/i915: Print error state times relative to capture
URL   : https://patchwork.freedesktop.org/series/41749/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4054_full -> Patchwork_8695_full =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_8695_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_8695_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: 
https://patchwork.freedesktop.org/api/1.0/series/41749/revisions/1/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in 
Patchwork_8695_full:

  === IGT changes ===

 Warnings 

igt@gem_mocs_settings@mocs-rc6-ctx-dirty-render:
  shard-kbl:  SKIP -> PASS +1

igt@kms_draw_crc@draw-method-xrgb-mmap-cpu-untiled:
  shard-snb:  PASS -> SKIP


== Known issues ==

  Here are the changes found in Patchwork_8695_full that come from known issues:

  === IGT changes ===

 Issues hit 

igt@drv_suspend@debugfs-reader:
  shard-snb:  PASS -> DMESG-WARN (fdo#102365)

igt@gem_render_copy_redux@normal:
  shard-kbl:  PASS -> INCOMPLETE (fdo#103665)

igt@kms_rotation_crc@sprite-rotation-180:
  shard-snb:  PASS -> FAIL (fdo#103925)

igt@kms_setmode@basic:
  shard-apl:  PASS -> FAIL (fdo#99912)

igt@kms_sysfs_edid_timing:
  shard-apl:  PASS -> WARN (fdo#100047)


 Possible fixes 

igt@kms_cursor_legacy@flip-vs-cursor-toggle:
  shard-hsw:  FAIL (fdo#102670) -> PASS

igt@kms_flip@2x-plain-flip-ts-check:
  shard-hsw:  FAIL (fdo#100368) -> PASS

igt@kms_flip@wf_vblank-ts-check:
  shard-hsw:  FAIL (fdo#103928) -> PASS


  fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#102365 https://bugs.freedesktop.org/show_bug.cgi?id=102365
  fdo#102670 https://bugs.freedesktop.org/show_bug.cgi?id=102670
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#103925 https://bugs.freedesktop.org/show_bug.cgi?id=103925
  fdo#103928 https://bugs.freedesktop.org/show_bug.cgi?id=103928
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


== Participating hosts (6 -> 4) ==

  Missing(2): shard-glk shard-glkb 


== Build changes ==

* Linux: CI_DRM_4054 -> Patchwork_8695

  CI_DRM_4054: a0e39233b88795009de32094efa8a2135f34338f @ 
git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4430: 8b77704db49167f7ebfd1c470d9c129d3b862cb6 @ 
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_8695: 927090130dd55839f57c9b76a6dd87cf53e119c9 @ 
git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4430: 93b35926a150e318439d2505901288594b3548f5 @ 
git://anongit.freedesktop.org/piglit

== Logs ==

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

Re: [Intel-gfx] [RFC v2] drm/i915: Engine discovery query

[Lionel Landwerlin]

On 16/04/18 03:11, Tvrtko Ursulin wrote:


On 11/04/2018 17:32, Lionel Landwerlin wrote:

Looks good to me, a few nits below.
I'll try to find some time to display this information in gputop.

Reviewed-by: Lionel Landwerlin 


Thanks!


On 11/04/18 02:46, Tvrtko Ursulin wrote:

From: Tvrtko Ursulin 

Engine discovery query allows userspace to enumerate engines, probe 
their
configuration features, all without needing to maintain the internal 
PCI

ID based database.

A new query for the generic i915 query ioctl is added named
DRM_I915_QUERY_ENGINE_INFO, together with accompanying structure
drm_i915_query_engine_info. The address of latter should be passed 
to the

kernel in the query.data_ptr field, and should be large enough for the
kernel to fill out all known engines as struct drm_i915_engine_info
elements trailing the query.

As with other queries, setting the item query length to zero allows
userspace to query minimum required buffer size.

Enumerated engines have common type mask which can be used to query all
hardware engines, versus engines userspace can submit to using the 
execbuf

uAPI.

Engines also have capabilities which are per engine class namespace of
bits describing features not present on all engine instances.

v2:
  * Fixed HEVC assignment.
  * Reorder some fields, rename type to flags, increase width. (Lionel)
  * No need to allocate temporary storage if we do it engine by engine.
    (Lionel)

Signed-off-by: Tvrtko Ursulin 
Cc: Chris Wilson 
Cc: Jon Bloomfield 
Cc: Dmitry Rogozhkin 
Cc: Lionel Landwerlin 
Cc: Joonas Lahtinen 
---
This is RFC for now since it is not very usable before the new 
execbuf API

or virtual engine queue submission gets closer.

In this version I have added capability of distinguishing between 
hardware
engines and ABI engines. This is to account for probable future use 
cases
like virtualization, where guest might only see one engine instance, 
but

will want to know overall hardware capabilities in order to tune its
behaviour.

In general I think we will have to wait a bit before defining the final
look of the uAPI, but in the meantime I thought it useful to share 
as RFC.

---
  drivers/gpu/drm/i915/i915_query.c   | 63 
+

  drivers/gpu/drm/i915/intel_engine_cs.c  |  2 ++
  drivers/gpu/drm/i915/intel_ringbuffer.h |  3 ++
  include/uapi/drm/i915_drm.h | 46 
  4 files changed, 114 insertions(+)

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

index 3ace929dd90f..a1c0e3acc882 100644
--- a/drivers/gpu/drm/i915/i915_query.c
+++ b/drivers/gpu/drm/i915/i915_query.c
@@ -82,9 +82,72 @@ static int query_topology_info(struct 
drm_i915_private *dev_priv,

  return total_length;
  }
+static int
+query_engine_info(struct drm_i915_private *i915,
+  struct drm_i915_query_item *query_item)
+{
+    struct drm_i915_query_engine_info __user *query_ptr =
+    u64_to_user_ptr(query_item->data_ptr);
+    struct drm_i915_engine_info __user *info_ptr = 
_ptr->engines[0];

+    struct drm_i915_query_engine_info query;
+    struct intel_engine_cs *engine;
+    enum intel_engine_id id;
+    int len;
+
+    if (query_item->flags)
+    return -EINVAL;
+
+    len = sizeof(struct drm_i915_query_engine_info) +
+  I915_NUM_ENGINES * sizeof(struct drm_i915_engine_info);
+
+    if (!query_item->length)
+    return len;
+    else if (query_item->length < len)
+    return -EINVAL;
+
+    if (copy_from_user(, query_ptr, sizeof(query)))
+    return -EFAULT;
+
+    if (query.num_engines || query.rsvd[0] || query.rsvd[1] ||
+    query.rsvd[2])
+    return -EINVAL;
+
+    if (!access_ok(VERIFY_WRITE, query_ptr, query_item->length))


You could limit this to len ?


I could, I am just wondering if you are leading somewhere with this 
suggestion like something I missed?


Check against query_item->length has the semantics of "you told me you 
are giving me this big of a buffer, and I'll check if you were lying 
even if I won't use it all", versus check against len would be "I'll 
check only the part I'll write into, even if you told me buffer is 
bigger".


The current check is a protecting userspace against more mistakes, but 
perhaps you are thinking about some tricks which would be possible by 
just checking len?


Cool, looks like you thought about everything :)

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

[Intel-gfx] ✓ Fi.CI.BAT: success for Enabling content-type setting for external HDMI displays. (rev2)

[Patchwork]

== Series Details ==

Series: Enabling content-type setting for external HDMI displays. (rev2)
URL   : https://patchwork.freedesktop.org/series/41744/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4054 -> Patchwork_8696 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: 
https://patchwork.freedesktop.org/api/1.0/series/41744/revisions/2/mbox/

== Known issues ==

  Here are the changes found in Patchwork_8696 that come from known issues:

  === IGT changes ===

 Issues hit 

igt@drv_module_reload@basic-no-display:
  fi-cnl-psr: NOTRUN -> DMESG-WARN (fdo#105395) +2

igt@kms_pipe_crc_basic@read-crc-pipe-c-frame-sequence:
  fi-cnl-psr: NOTRUN -> FAIL (fdo#103481)


 Possible fixes 

igt@prime_vgem@basic-fence-flip:
  fi-ilk-650: FAIL (fdo#104008) -> PASS


  fdo#103481 https://bugs.freedesktop.org/show_bug.cgi?id=103481
  fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
  fdo#105395 https://bugs.freedesktop.org/show_bug.cgi?id=105395


== Participating hosts (35 -> 21) ==

  Additional (1): fi-cnl-psr 
  Missing(15): fi-ilk-m540 fi-bxt-dsi fi-skl-gvtdvm fi-bsw-n3050 
fi-byt-j1900 fi-skl-6770hq fi-bdw-gvtdvm fi-skl-6260u fi-bwr-2160 fi-cfl-s3 
fi-bxt-j4205 fi-gdg-551 fi-pnv-d510 fi-skl-6700hq fi-skl-6600u 


== Build changes ==

* Linux: CI_DRM_4054 -> Patchwork_8696

  CI_DRM_4054: a0e39233b88795009de32094efa8a2135f34338f @ 
git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4430: 8b77704db49167f7ebfd1c470d9c129d3b862cb6 @ 
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_8696: a21e5056a6f99d399a7f76fa8d02094cd8224538 @ 
git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4430: 93b35926a150e318439d2505901288594b3548f5 @ 
git://anongit.freedesktop.org/piglit


== Linux commits ==

a21e5056a6f9 i915: content-type property for HDMI connector
ff3a198613ab drm: content-type property for HDMI connector

== Logs ==

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

[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Enabling content-type setting for external HDMI displays. (rev2)

[Patchwork]

== Series Details ==

Series: Enabling content-type setting for external HDMI displays. (rev2)
URL   : https://patchwork.freedesktop.org/series/41744/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
ff3a198613ab drm: content-type property for HDMI connector
-:84: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#84: FILE: drivers/gpu/drm/drm_connector.c:1287:
+   drm_property_create_enum(dev, 0, "content type",
+   drm_content_type_enum_list,

-:87: CHECK:COMPARISON_TO_NULL: Comparison to NULL could be written 
"!dev->mode_config.content_type_property"
#87: FILE: drivers/gpu/drm/drm_connector.c:1290:
+   if (dev->mode_config.content_type_property == NULL)

total: 0 errors, 0 warnings, 2 checks, 120 lines checked
a21e5056a6f9 i915: content-type property for HDMI connector
-:73: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#73: FILE: drivers/gpu/drm/i915/intel_modes.c:142:
+   drm_object_attach_property(>base,
+   connector->dev->mode_config.content_type_property,

total: 0 errors, 0 warnings, 1 checks, 44 lines checked

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

[Intel-gfx] [PATCH v1 2/2] i915: content-type property for HDMI connector

[StanLis]

From: Stanislav Lisovskiy 

Added encoding of drm content_type property from
drm_connector_state within AVI infoframe in order to properly handle
external HDMI TV content-type setting.

Signed-off-by: Stanislav Lisovskiy 
---
 drivers/gpu/drm/i915/intel_atomic.c |  1 +
 drivers/gpu/drm/i915/intel_drv.h|  1 +
 drivers/gpu/drm/i915/intel_hdmi.c   |  4 
 drivers/gpu/drm/i915/intel_modes.c  | 10 ++
 4 files changed, 16 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_atomic.c 
b/drivers/gpu/drm/i915/intel_atomic.c
index 40285d1b91b7..61ddb5871d8a 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -124,6 +124,7 @@ int intel_digital_connector_atomic_check(struct 
drm_connector *conn,
if (new_conn_state->force_audio != old_conn_state->force_audio ||
new_conn_state->broadcast_rgb != old_conn_state->broadcast_rgb ||
new_conn_state->base.picture_aspect_ratio != 
old_conn_state->base.picture_aspect_ratio ||
+   new_conn_state->base.content_type != 
old_conn_state->base.content_type ||
new_conn_state->base.scaling_mode != 
old_conn_state->base.scaling_mode)
crtc_state->mode_changed = true;
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 5bd2263407b2..07fd7ba21f38 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1818,6 +1818,7 @@ int intel_ddc_get_modes(struct drm_connector *c, struct 
i2c_adapter *adapter);
 void intel_attach_force_audio_property(struct drm_connector *connector);
 void intel_attach_broadcast_rgb_property(struct drm_connector *connector);
 void intel_attach_aspect_ratio_property(struct drm_connector *connector);
+void intel_attach_content_type_property(struct drm_connector *connector);
 
 
 /* intel_overlay.c */
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
b/drivers/gpu/drm/i915/intel_hdmi.c
index ee929f31f7db..cd484276e9b0 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -491,6 +491,8 @@ static void intel_hdmi_set_avi_infoframe(struct drm_encoder 
*encoder,
   
intel_hdmi->rgb_quant_range_selectable,
   is_hdmi2_sink);
 
+   frame.avi.content_type = connector->state->content_type;
+
/* TODO: handle pixel repetition for YCBCR420 outputs */
intel_write_infoframe(encoder, crtc_state, );
 }
@@ -2065,7 +2067,9 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, 
struct drm_connector *c
intel_attach_force_audio_property(connector);
intel_attach_broadcast_rgb_property(connector);
intel_attach_aspect_ratio_property(connector);
+   intel_attach_content_type_property(connector);
connector->state->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
+   connector->state->content_type = HDMI_CONTENT_TYPE_GRAPHICS;
 }
 
 /*
diff --git a/drivers/gpu/drm/i915/intel_modes.c 
b/drivers/gpu/drm/i915/intel_modes.c
index b39846613e3c..232811ab71a3 100644
--- a/drivers/gpu/drm/i915/intel_modes.c
+++ b/drivers/gpu/drm/i915/intel_modes.c
@@ -133,3 +133,13 @@ intel_attach_aspect_ratio_property(struct drm_connector 
*connector)
connector->dev->mode_config.aspect_ratio_property,
DRM_MODE_PICTURE_ASPECT_NONE);
 }
+
+void
+intel_attach_content_type_property(struct drm_connector *connector)
+{
+   if (!drm_mode_create_content_type_property(connector->dev))
+   drm_object_attach_property(>base,
+   connector->dev->mode_config.content_type_property,
+   DRM_MODE_CONTENT_TYPE_GRAPHICS);
+}
+
-- 
2.17.0

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

[Intel-gfx] [PATCH v1 0/2] Enabling content-type setting for external HDMI displays.

[StanLis]

From: Stanislav Lisovskiy 

Added content type setting property to drm_connector(part 1)
and enabled transmitting it with HDMI AVI infoframes
for i915(part 2).

Stanislav Lisovskiy (2):
  drm: content-type property for HDMI connector
  i915: content-type property for HDMI connector

 Documentation/gpu/kms-properties.csv |  1 +
 drivers/gpu/drm/drm_atomic.c |  4 
 drivers/gpu/drm/drm_connector.c  | 34 
 drivers/gpu/drm/drm_edid.c   |  1 +
 drivers/gpu/drm/i915/intel_atomic.c  |  1 +
 drivers/gpu/drm/i915/intel_drv.h |  1 +
 drivers/gpu/drm/i915/intel_hdmi.c|  4 
 drivers/gpu/drm/i915/intel_modes.c   | 10 
 include/drm/drm_connector.h  | 10 
 include/drm/drm_mode_config.h|  5 
 include/uapi/drm/drm_mode.h  |  5 
 11 files changed, 76 insertions(+)

-- 
2.17.0

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

[Intel-gfx] [PATCH v1 1/2] drm: content-type property for HDMI connector

[StanLis]

From: Stanislav Lisovskiy 

Added content_type property to
drm_connector_state in order to properly handle
external HDMI TV content-type setting.

Signed-off-by: Stanislav Lisovskiy 
---
 Documentation/gpu/kms-properties.csv |  1 +
 drivers/gpu/drm/drm_atomic.c |  4 
 drivers/gpu/drm/drm_connector.c  | 34 
 drivers/gpu/drm/drm_edid.c   |  1 +
 include/drm/drm_connector.h  | 10 
 include/drm/drm_mode_config.h|  5 
 include/uapi/drm/drm_mode.h  |  5 
 7 files changed, 60 insertions(+)

diff --git a/Documentation/gpu/kms-properties.csv 
b/Documentation/gpu/kms-properties.csv
index 6b28b014cb7d..7a02b2782f33 100644
--- a/Documentation/gpu/kms-properties.csv
+++ b/Documentation/gpu/kms-properties.csv
@@ -17,6 +17,7 @@ Owner Module/Drivers,Group,Property Name,Type,Property 
Values,Object attached,De
 ,Virtual GPU,“suggested X”,RANGE,"Min=0, Max=0x",Connector,property to 
suggest an X offset for a connector
 ,,“suggested Y”,RANGE,"Min=0, Max=0x",Connector,property to suggest an 
Y offset for a connector
 ,Optional,"""aspect ratio""",ENUM,"{ ""None"", ""4:3"", ""16:9"" 
}",Connector,TDB
+,Optional,"""content type""",ENUM,"{ ""Graphics"", ""Photo"", ""Cinema"", 
""Game"" }",Connector,TDB
 i915,Generic,"""Broadcast RGB""",ENUM,"{ ""Automatic"", ""Full"", ""Limited 
16:235"" }",Connector,"When this property is set to Limited 16:235 and CTM is 
set, the hardware will be programmed with the result of the multiplication of 
CTM by the limited range matrix to ensure the pixels normaly in the range 
0..1.0 are remapped to the range 16/255..235/255."
 ,,“audio”,ENUM,"{ ""force-dvi"", ""off"", ""auto"", ""on"" }",Connector,TBD
 ,SDVO-TV,“mode”,ENUM,"{ ""NTSC_M"", ""NTSC_J"", ""NTSC_443"", ""PAL_B"" } 
etc.",Connector,TBD
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 7d25c42f22db..72fd2a1c801f 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1266,6 +1266,8 @@ static int drm_atomic_connector_set_property(struct 
drm_connector *connector,
state->link_status = val;
} else if (property == config->aspect_ratio_property) {
state->picture_aspect_ratio = val;
+   } else if (property == config->content_type_property) {
+   state->content_type = val;
} else if (property == connector->scaling_mode_property) {
state->scaling_mode = val;
} else if (property == connector->content_protection_property) {
@@ -1351,6 +1353,8 @@ drm_atomic_connector_get_property(struct drm_connector 
*connector,
*val = state->link_status;
} else if (property == config->aspect_ratio_property) {
*val = state->picture_aspect_ratio;
+   } else if (property == config->content_type_property) {
+   *val = state->content_type;
} else if (property == connector->scaling_mode_property) {
*val = state->scaling_mode;
} else if (property == connector->content_protection_property) {
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index b3cde897cd80..d03586edd483 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -720,6 +720,13 @@ static const struct drm_prop_enum_list 
drm_aspect_ratio_enum_list[] = {
{ DRM_MODE_PICTURE_ASPECT_16_9, "16:9" },
 };
 
+static const struct drm_prop_enum_list drm_content_type_enum_list[] = {
+   { DRM_MODE_CONTENT_TYPE_GRAPHICS, "GRAPHICS" },
+   { DRM_MODE_CONTENT_TYPE_PHOTO, "PHOTO" },
+   { DRM_MODE_CONTENT_TYPE_CINEMA, "CINEMA" },
+   { DRM_MODE_CONTENT_TYPE_GAME, "GAME" },
+};
+
 static const struct drm_prop_enum_list drm_panel_orientation_enum_list[] = {
{ DRM_MODE_PANEL_ORIENTATION_NORMAL,"Normal"},
{ DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP, "Upside Down"   },
@@ -1260,6 +1267,33 @@ int drm_mode_create_aspect_ratio_property(struct 
drm_device *dev)
 }
 EXPORT_SYMBOL(drm_mode_create_aspect_ratio_property);
 
+/**
+ * drm_mode_create_content_type_property - create content type property
+ * @dev: DRM device
+ *
+ * Called by a driver the first time it's needed, must be attached to desired
+ * connectors.
+ *
+ * Returns:
+ * Zero on success, negative errno on failure.
+ */
+int drm_mode_create_content_type_property(struct drm_device *dev)
+{
+   if (dev->mode_config.content_type_property)
+   return 0;
+
+   dev->mode_config.content_type_property =
+   drm_property_create_enum(dev, 0, "content type",
+   drm_content_type_enum_list,
+   ARRAY_SIZE(drm_content_type_enum_list));
+
+   if (dev->mode_config.content_type_property == NULL)
+   return -ENOMEM;
+
+   return 0;
+}

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Print error state times relative to capture

[Patchwork]

== Series Details ==

Series: drm/i915: Print error state times relative to capture
URL   : https://patchwork.freedesktop.org/series/41749/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4054 -> Patchwork_8695 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: 
https://patchwork.freedesktop.org/api/1.0/series/41749/revisions/1/mbox/

== Known issues ==

  Here are the changes found in Patchwork_8695 that come from known issues:

  === IGT changes ===

 Issues hit 

igt@drv_module_reload@basic-no-display:
  fi-cnl-psr: NOTRUN -> DMESG-WARN (fdo#105395) +2

igt@kms_chamelium@dp-crc-fast:
  fi-kbl-7500u:   PASS -> DMESG-FAIL (fdo#103841)


 Possible fixes 

igt@prime_vgem@basic-fence-flip:
  fi-ilk-650: FAIL (fdo#104008) -> PASS


  fdo#103841 https://bugs.freedesktop.org/show_bug.cgi?id=103841
  fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
  fdo#105395 https://bugs.freedesktop.org/show_bug.cgi?id=105395


== Participating hosts (35 -> 21) ==

  Additional (1): fi-cnl-psr 
  Missing(15): fi-ilk-m540 fi-bxt-dsi fi-skl-gvtdvm fi-bsw-n3050 
fi-byt-j1900 fi-skl-6770hq fi-bdw-gvtdvm fi-skl-6260u fi-bwr-2160 fi-cfl-s3 
fi-bxt-j4205 fi-gdg-551 fi-pnv-d510 fi-skl-6700hq fi-skl-6600u 


== Build changes ==

* Linux: CI_DRM_4054 -> Patchwork_8695

  CI_DRM_4054: a0e39233b88795009de32094efa8a2135f34338f @ 
git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4430: 8b77704db49167f7ebfd1c470d9c129d3b862cb6 @ 
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_8695: 927090130dd55839f57c9b76a6dd87cf53e119c9 @ 
git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4430: 93b35926a150e318439d2505901288594b3548f5 @ 
git://anongit.freedesktop.org/piglit


== Linux commits ==

927090130dd5 drm/i915: Print error state times relative to capture

== Logs ==

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

Re: [Intel-gfx] [PATCH 0/9] GPU-bound energy efficiency improvements for the intel_pstate driver.

[Eero Tamminen]

Hi,

On 14.04.2018 07:01, Srinivas Pandruvada wrote:

Hi Francisco,

[...]


Are you no longer interested in improving those aspects of the non-
HWP
governor?  Is it that you're planning to delete it and move back to a
generic cpufreq governor for non-HWP platforms in the near future?


Yes that is the plan for Atom platforms, which are only non HWP
platforms till now. You have to show good gain for performance and
performance/watt to carry and maintain such big change. So we have to
see your performance and power numbers.


For the active cases, you can look at the links at the beginning / 
bottom of this mail thread.  Francisco provided performance results for 
>100 benchmarks.



At this side of Atlantic, we've been testing different versions of the 
patchset in past few months for >50 Linux 3D benchmarks on 6 different 
platforms.


On Geminilake and few BXT configurations (where 3D benchmarks are TDP 
limited), many tests' performance improves by 5-15%, also complex ones. 
And more importantly, there were no regressions.


(You can see details + links to more info in Jira ticket VIZ-12078.)

*On (fully) TDP limited cases, power usage (obviously) keeps the same, 
so performance/watt improvements can be derived from the measured 
performance improvements.*



We have data also for earlier platforms from slightly older versions of 
the patchset, but on those it didn't have any significant impact on 
performance.


I think the main reason for this is that BYT & BSW NUCs that we have, 
have space only for single memory module.  Without dual-memory channel 
configuration, benchmarks are too memory-bottlenecked to utilized GPU 
enough to make things TDP limited on those platforms.


However, now that I look at the old BYT & BSW data (for few benchmarks 
which improved most on BXT & GLK), I see that there's a reduction in the 
CPU power utilization according to RAPL, at least on BSW.



- Eero



This will benefit all architectures including x86 + non i915.



The current design encourages re-use of the IO utilization statistic
(see PATCH 1) by other governors as a mechanism driving the trade-off
between energy efficiency and responsiveness based on whether the
system
is close to CPU-bound, in whatever way is applicable to each governor
(e.g. it would make sense for it to be hooked up to the EPP
preference
knob in the case of the intel_pstate HWP governor, which would allow
it
to achieve better energy efficiency in IO-bound situations just like
this series does for non-HWP parts).  There's nothing really x86- nor
i915-specific about it.


BTW intel-pstate can be driven by sched-util governor (passive
mode),
so if your prove benefits to Broxton, this can be a default.
As before:
- No regression to idle power at all. This is more important than
benchmarks
- Not just score, performance/watt is important



Is schedutil actually on par with the intel_pstate non-HWP governor
as
of today, according to these metrics and the overall benchmark
numbers?

Yes, except for few cases. I have not tested recently, so may be
better.

Thanks,
Srinivas



Thanks,
Srinivas



controller does, even though the frequent IO waits may actually
be
an
indication that the system is IO-bound (which means that the
large
energy usage increase may not be translated in any performance
benefit
in practice, not to speak of performance being impacted
negatively
in
TDP-bound scenarios like GPU rendering).

Regarding run-time complexity, I haven't observed this governor
to
be
measurably more computationally intensive than the present
one.  It's a
bunch more instructions indeed, but still within the same
ballpark
as
the current governor.  The average increase in CPU utilization
on
my BXT
with this series is less than 0.03% (sampled via ftrace for v1,
I
can
repeat the measurement for the v2 I have in the works, though I
don't
expect the result to be substantially different).  If this is a
problem
for you there are several optimization opportunities that would
cut
down
the number of CPU cycles get_target_pstate_lp() takes to
execute by
a
large percent (most of the optimization ideas I can think of
right
now
though would come at some
accuracy/maintainability/debuggability
cost,
but may still be worth pursuing), but the computational
overhead is
low
enough at this point that the impact on any benchmark or real
workload
would be orders of magnitude lower than its variance, which
makes
it
kind of difficult to keep the discussion data-driven [as
possibly
any
performance optimization discussion should ever be ;)].



Thanks,
Srinivas






[Absolute benchmark results are unfortunately omitted
from
this
letter
due to company policies, but the percent change and
Student's
T
p-value are included above and in the referenced
benchmark
results]

The most obvious impact of this series will likely be the
overall
improvement in graphics performance on systems with an
IGP
integrated
into the processor package (though for the moment this is
only
enabled
on 

Re: [Intel-gfx] [PATCH] drm/i915: Print error state times relative to capture

[Chris Wilson]

Quoting Mika Kuoppala (2018-04-16 14:40:52)
> Using plain jiffies in error state output makes the output
> time differences relative to the current system time. This
> is wrong as it makes output time differences dependent
> of when the error state is printed rather than when it is
> captured.
> 
> Store capture jiffies into error state and use it
> when outputting the state to fix time differences output.

Good point. It was always confusing to see "several hours ago", when what
we wanted to know was how long before the hanging request. Hmm, do we want
to use that as our reference point rather than the capture timestamp? Do 
we even need to see the jiffies? (Yes, we probably do want the raw value
to check, but I think it's secondary.)

> Cc: Chris Wilson 
> Signed-off-by: Mika Kuoppala 

Reviewed-by: Chris Wilson 

Please do consider using the (earliest) guilty timestamp as time 0 and
print before/after. If no guilty then use capture?
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 11/13] drm/omapdrm: Nuke omap_framebuffer_get_next_connector()

[Tomi Valkeinen]

On 09/04/18 11:41, Daniel Vetter wrote:
> On Fri, Apr 06, 2018 at 09:10:43AM +0300, Tomi Valkeinen wrote:
>> On 05/04/18 19:50, Daniel Vetter wrote:
>>> On Thu, Apr 05, 2018 at 06:13:58PM +0300, Ville Syrjala wrote:
 From: Ville Syrjälä 

 omap_framebuffer_get_next_connector() uses plane->fb which we want to
 deprecate for atomic drivers. As omap_framebuffer_get_next_connector()
 is unused just nuke the entire function.

 Cc: Tomi Valkeinen 
 Signed-off-by: Ville Syrjälä 
>>>
>>> Yeah was slightly worried how to fix up this one, but we're lucky!
>>>
>>> Reviewed-by: Daniel Vetter 
>>
>> I tried to remove it just a week ago, but Sebastian said that it's used
>> by a unmerged series about DSI command mode displays, so I dropped the
>> patch.
>>
>> In the unmerged series, it's used by omap_framebuffer_dirty() ([PATCHv3
>> 3/8] drm/omap: add support for manually updated displays). So we have a
>> framebuffer, and we want to know which crtcs need to be flushed.
> 
> You can't do that in atomic drivers.
> 
> You need to take appropriate locks and do the full ->state->fb deref.
> Also, there's a gazillion of copies for generic framebuffer_dirty
> implementations floating around, pleas try to coordinate with those.

Ok. In that case we need to refresh the manual update series to do
things differently. For this patch:

Reviewed-by: Tomi Valkeinen 

 Tomi

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915: Print error state times relative to capture

[Mika Kuoppala]

Using plain jiffies in error state output makes the output
time differences relative to the current system time. This
is wrong as it makes output time differences dependent
of when the error state is printed rather than when it is
captured.

Store capture jiffies into error state and use it
when outputting the state to fix time differences output.

Cc: Chris Wilson 
Signed-off-by: Mika Kuoppala 
---
 drivers/gpu/drm/i915/i915_gpu_error.c | 20 +---
 drivers/gpu/drm/i915/i915_gpu_error.h |  1 +
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c 
b/drivers/gpu/drm/i915/i915_gpu_error.c
index effaf982b19b..fc7e76f2 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -404,7 +404,8 @@ static const char *bannable(const struct 
drm_i915_error_context *ctx)
 
 static void error_print_request(struct drm_i915_error_state_buf *m,
const char *prefix,
-   const struct drm_i915_error_request *erq)
+   const struct drm_i915_error_request *erq,
+   const unsigned long timestamp)
 {
if (!erq->seqno)
return;
@@ -412,7 +413,7 @@ static void error_print_request(struct 
drm_i915_error_state_buf *m,
err_printf(m, "%s pid %d, ban score %d, seqno %8x:%08x, prio %d, 
emitted %dms ago, head %08x, tail %08x\n",
   prefix, erq->pid, erq->ban_score,
   erq->context, erq->seqno, erq->priority,
-  jiffies_to_msecs(jiffies - erq->jiffies),
+  jiffies_to_msecs(timestamp - erq->jiffies),
   erq->head, erq->tail);
 }
 
@@ -427,7 +428,8 @@ static void error_print_context(struct 
drm_i915_error_state_buf *m,
 }
 
 static void error_print_engine(struct drm_i915_error_state_buf *m,
-  const struct drm_i915_error_engine *ee)
+  const struct drm_i915_error_engine *ee,
+  const unsigned long timestamp)
 {
int n;
 
@@ -499,12 +501,12 @@ static void error_print_engine(struct 
drm_i915_error_state_buf *m,
   hangcheck_action_to_str(ee->hangcheck_action));
err_printf(m, "  hangcheck action timestamp: %lu, %u ms ago\n",
   ee->hangcheck_timestamp,
-  jiffies_to_msecs(jiffies - ee->hangcheck_timestamp));
+  jiffies_to_msecs(timestamp - ee->hangcheck_timestamp));
err_printf(m, "  engine reset count: %u\n", ee->reset_count);
 
for (n = 0; n < ee->num_ports; n++) {
err_printf(m, "  ELSP[%d]:", n);
-   error_print_request(m, " ", >execlist[n]);
+   error_print_request(m, " ", >execlist[n], timestamp);
}
 
error_print_context(m, "  Active context: ", >context);
@@ -650,6 +652,7 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf 
*m,
ts = ktime_to_timespec64(error->uptime);
err_printf(m, "Uptime: %lld s %ld us\n",
   (s64)ts.tv_sec, ts.tv_nsec / NSEC_PER_USEC);
+   err_printf(m, "Jiffies: %lu (now %lu)\n", error->timestamp, jiffies);
 
for (i = 0; i < ARRAY_SIZE(error->engine); i++) {
if (error->engine[i].hangcheck_stalled &&
@@ -710,7 +713,8 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf 
*m,
 
for (i = 0; i < ARRAY_SIZE(error->engine); i++) {
if (error->engine[i].engine_id != -1)
-   error_print_engine(m, >engine[i]);
+   error_print_engine(m, >engine[i],
+  error->timestamp);
}
 
for (i = 0; i < ARRAY_SIZE(error->active_vm); i++) {
@@ -769,7 +773,8 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf 
*m,
   dev_priv->engine[i]->name,
   ee->num_requests);
for (j = 0; j < ee->num_requests; j++)
-   error_print_request(m, " ", >requests[j]);
+   error_print_request(m, " ", >requests[j],
+   error->timestamp);
}
 
if (IS_ERR(ee->waiters)) {
@@ -1743,6 +1748,7 @@ static int capture(void *data)
error->boottime = ktime_get_boottime();
error->uptime = ktime_sub(ktime_get(),
  error->i915->gt.last_init_time);
+   error->timestamp = jiffies;
 
capture_params(error);
capture_gen_state(error);
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.h 
b/drivers/gpu/drm/i915/i915_gpu_error.h
index c05b6034d718..1f36ad526bc5 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.h
+++ b/drivers/gpu/drm/i915/i915_gpu_error.h
@@ -30,6 +30,7 @@ struct 

[Intel-gfx] ✗ Fi.CI.IGT: failure for drm/i915/gvt: Deliver guest cursor hotspot info (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915/gvt: Deliver guest cursor hotspot info (rev2)
URL   : https://patchwork.freedesktop.org/series/41727/
State : failure

== Summary ==

= CI Bug Log - changes from CI_DRM_4054_full -> Patchwork_8694_full =

== Summary - FAILURE ==

  Serious unknown changes coming with Patchwork_8694_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_8694_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: 
https://patchwork.freedesktop.org/api/1.0/series/41727/revisions/2/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in 
Patchwork_8694_full:

  === IGT changes ===

 Possible regressions 

igt@gem_exec_store@pages-vebox:
  shard-hsw:  PASS -> FAIL


 Warnings 

igt@gem_mocs_settings@mocs-rc6-ctx-dirty-render:
  shard-kbl:  SKIP -> PASS +1


== Known issues ==

  Here are the changes found in Patchwork_8694_full that come from known issues:

  === IGT changes ===

 Issues hit 

igt@kms_cursor_crc@cursor-256x256-rapid-movement:
  shard-kbl:  PASS -> INCOMPLETE (fdo#103665)

igt@kms_flip@2x-dpms-vs-vblank-race:
  shard-hsw:  PASS -> FAIL (fdo#103060)

igt@kms_flip@plain-flip-fb-recreate:
  shard-hsw:  PASS -> FAIL (fdo#100368)

igt@kms_rotation_crc@sprite-rotation-180:
  shard-snb:  PASS -> FAIL (fdo#103925)

igt@kms_setmode@basic:
  shard-apl:  PASS -> FAIL (fdo#99912)

igt@kms_sysfs_edid_timing:
  shard-apl:  PASS -> WARN (fdo#100047)


 Possible fixes 

igt@kms_cursor_legacy@flip-vs-cursor-toggle:
  shard-hsw:  FAIL (fdo#102670) -> PASS

igt@kms_flip@wf_vblank-ts-check:
  shard-hsw:  FAIL (fdo#103928) -> PASS


  fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#102670 https://bugs.freedesktop.org/show_bug.cgi?id=102670
  fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#103925 https://bugs.freedesktop.org/show_bug.cgi?id=103925
  fdo#103928 https://bugs.freedesktop.org/show_bug.cgi?id=103928
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


== Participating hosts (6 -> 4) ==

  Missing(2): shard-glk shard-glkb 


== Build changes ==

* Linux: CI_DRM_4054 -> Patchwork_8694

  CI_DRM_4054: a0e39233b88795009de32094efa8a2135f34338f @ 
git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4430: 8b77704db49167f7ebfd1c470d9c129d3b862cb6 @ 
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_8694: a117fc1be218228651a8f966f1106912287f3135 @ 
git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4430: 93b35926a150e318439d2505901288594b3548f5 @ 
git://anongit.freedesktop.org/piglit

== Logs ==

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

[Intel-gfx] [PATCH v1 0/2] Enabling content-type setting for external HDMI displays.

[StanLis]

From: Stanislav Lisovskiy 

Added content type setting property to drm_connector(part 1)
and enabled transmitting it with HDMI AVI infoframes
for i915(part 2).

Stanislav Lisovskiy (2):
  drm: content-type property for HDMI connector
  i915: content-type property for HDMI connector

 Documentation/gpu/kms-properties.csv |  1 +
 drivers/gpu/drm/drm_atomic.c |  4 
 drivers/gpu/drm/drm_connector.c  | 34 
 drivers/gpu/drm/drm_edid.c   |  1 +
 drivers/gpu/drm/i915/intel_atomic.c  |  1 +
 drivers/gpu/drm/i915/intel_drv.h |  1 +
 drivers/gpu/drm/i915/intel_hdmi.c|  4 
 drivers/gpu/drm/i915/intel_modes.c   | 10 
 include/drm/drm_connector.h  | 10 
 include/drm/drm_mode_config.h|  5 
 include/uapi/drm/drm_mode.h  |  5 
 11 files changed, 76 insertions(+)

-- 
2.17.0

-
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

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

Re: [Intel-gfx] [PATCH igt] igt/prime_mmap: Test for userptr support first

[Tvrtko Ursulin]

On 12/04/2018 19:08, Chris Wilson wrote:

Before we start trying to use userptr to test interoperability with
PRIME, we first need to check that the device in question has userptr
support.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106013
Signed-off-by: Chris Wilson 
---
  tests/prime_mmap.c | 15 +++
  1 file changed, 15 insertions(+)

diff --git a/tests/prime_mmap.c b/tests/prime_mmap.c
index 0da0aa68..67a6a232 100644
--- a/tests/prime_mmap.c
+++ b/tests/prime_mmap.c
@@ -307,6 +307,19 @@ static int prime_handle_to_fd_no_assert(uint32_t handle, 
int flags, int *fd_out)
return ret;
  }
  
+static bool has_userptr(void)

+{
+   uint32_t handle = 0;
+   void *ptr;
+
+   igt_assert(posix_memalign(, 4096, 4096) == 0);
+   if ( __gem_userptr(fd, ptr, 4096, 0, 0, ) == 0)
+   gem_close(fd, handle);
+   free(ptr);
+
+   return handle;
+}
+
  /* test for mmap(dma_buf_export(userptr)) */
  static void
  test_userptr(void)
@@ -315,6 +328,8 @@ test_userptr(void)
void *ptr;
uint32_t handle;
  
+	igt_require(has_userptr());

+
/* create userptr bo */
ret = posix_memalign(, 4096, BO_SIZE);
igt_assert_eq(ret, 0);



Reviewed-by: Tvrtko Ursulin 

Regards,

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

Re: [Intel-gfx] [RFC v2] drm/i915: Engine discovery query

[Tvrtko Ursulin]

On 11/04/2018 17:32, Lionel Landwerlin wrote:

Looks good to me, a few nits below.
I'll try to find some time to display this information in gputop.

Reviewed-by: Lionel Landwerlin 


Thanks!


On 11/04/18 02:46, Tvrtko Ursulin wrote:

From: Tvrtko Ursulin 

Engine discovery query allows userspace to enumerate engines, probe their
configuration features, all without needing to maintain the internal PCI
ID based database.

A new query for the generic i915 query ioctl is added named
DRM_I915_QUERY_ENGINE_INFO, together with accompanying structure
drm_i915_query_engine_info. The address of latter should be passed to the
kernel in the query.data_ptr field, and should be large enough for the
kernel to fill out all known engines as struct drm_i915_engine_info
elements trailing the query.

As with other queries, setting the item query length to zero allows
userspace to query minimum required buffer size.

Enumerated engines have common type mask which can be used to query all
hardware engines, versus engines userspace can submit to using the 
execbuf

uAPI.

Engines also have capabilities which are per engine class namespace of
bits describing features not present on all engine instances.

v2:
  * Fixed HEVC assignment.
  * Reorder some fields, rename type to flags, increase width. (Lionel)
  * No need to allocate temporary storage if we do it engine by engine.
    (Lionel)

Signed-off-by: Tvrtko Ursulin 
Cc: Chris Wilson 
Cc: Jon Bloomfield 
Cc: Dmitry Rogozhkin 
Cc: Lionel Landwerlin 
Cc: Joonas Lahtinen 
---
This is RFC for now since it is not very usable before the new execbuf 
API

or virtual engine queue submission gets closer.

In this version I have added capability of distinguishing between 
hardware

engines and ABI engines. This is to account for probable future use cases
like virtualization, where guest might only see one engine instance, but
will want to know overall hardware capabilities in order to tune its
behaviour.

In general I think we will have to wait a bit before defining the final
look of the uAPI, but in the meantime I thought it useful to share as 
RFC.

---
  drivers/gpu/drm/i915/i915_query.c   | 63 
+

  drivers/gpu/drm/i915/intel_engine_cs.c  |  2 ++
  drivers/gpu/drm/i915/intel_ringbuffer.h |  3 ++
  include/uapi/drm/i915_drm.h | 46 
  4 files changed, 114 insertions(+)

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

index 3ace929dd90f..a1c0e3acc882 100644
--- a/drivers/gpu/drm/i915/i915_query.c
+++ b/drivers/gpu/drm/i915/i915_query.c
@@ -82,9 +82,72 @@ static int query_topology_info(struct 
drm_i915_private *dev_priv,

  return total_length;
  }
+static int
+query_engine_info(struct drm_i915_private *i915,
+  struct drm_i915_query_item *query_item)
+{
+    struct drm_i915_query_engine_info __user *query_ptr =
+    u64_to_user_ptr(query_item->data_ptr);
+    struct drm_i915_engine_info __user *info_ptr = 
_ptr->engines[0];

+    struct drm_i915_query_engine_info query;
+    struct intel_engine_cs *engine;
+    enum intel_engine_id id;
+    int len;
+
+    if (query_item->flags)
+    return -EINVAL;
+
+    len = sizeof(struct drm_i915_query_engine_info) +
+  I915_NUM_ENGINES * sizeof(struct drm_i915_engine_info);
+
+    if (!query_item->length)
+    return len;
+    else if (query_item->length < len)
+    return -EINVAL;
+
+    if (copy_from_user(, query_ptr, sizeof(query)))
+    return -EFAULT;
+
+    if (query.num_engines || query.rsvd[0] || query.rsvd[1] ||
+    query.rsvd[2])
+    return -EINVAL;
+
+    if (!access_ok(VERIFY_WRITE, query_ptr, query_item->length))


You could limit this to len ?


I could, I am just wondering if you are leading somewhere with this 
suggestion like something I missed?


Check against query_item->length has the semantics of "you told me you 
are giving me this big of a buffer, and I'll check if you were lying 
even if I won't use it all", versus check against len would be "I'll 
check only the part I'll write into, even if you told me buffer is bigger".


The current check is a protecting userspace against more mistakes, but 
perhaps you are thinking about some tricks which would be possible by 
just checking len?





+    return -EFAULT;
+
+    for_each_engine(engine, i915, id) {
+    struct drm_i915_engine_info info;
+
+    if (__copy_from_user(, info_ptr, sizeof(info)))
+    return -EFAULT;
+
+    if (info.flags || info.class || info.instance ||
+    info.capabilities || info.rsvd0 || info.rsvd1[0] ||
+    info.rsvd1[1])
+    return -EINVAL;
+
+    info.class = engine->uabi_class;
+    info.instance = 

Re: [Intel-gfx] [PATCH i-g-t v4] tests/perf_pmu: Avoid RT thread for accuracy test

[Chris Wilson]

Quoting Tvrtko Ursulin (2018-04-16 10:55:29)
> 
> On 14/04/2018 12:35, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2018-04-11 14:52:36)
> >>
> >> On 11/04/2018 14:23, Chris Wilson wrote:
> >>> Quoting Tvrtko Ursulin (2018-04-04 10:51:52)
>  From: Tvrtko Ursulin 
> 
>  Realtime scheduling interferes with execlists submission (tasklet) so try
>  to simplify the PWM loop in a few ways:
> 
> * Drop RT.
> * Longer batches for smaller systematic error.
> * More truthful test duration calculation.
> * Less clock queries.
> * No self-adjust - instead just report the achieved cycle and let the
>   parent check against it.
> * Report absolute cycle error.
> 
>  v2:
> * Bring back self-adjust. (Chris Wilson)
>   (But slightly fixed version with no overflow.)
> 
>  v3:
> * Log average and mean calibration for each pass.
> 
>  v4:
> * Eliminate development leftovers.
> * Fix variance logging.
> 
>  Signed-off-by: Tvrtko Ursulin 
> >>>
> >>>   From a pragmatic point of view, there's no point waiting for me to be
> >>> happy with the convergence if CI is, and the variance will definitely be
> >>> interesting (although you could have used igt_mean to compute the
> >>> iterative variance), so
> >>>
> >>> Reviewed-by: Chris Wilson 
> >>
> >> Thanks, I've pushed it and so we'll see.
> > 
> > We should resurrect the RT variant in the near future. It's definitely
> > an issue in our driver that random userspace can impact execution of
> > unconnected others. (Handling RT starvation of workers is something we
> > have to be aware of elsewhere, commonly hits oom if we don't have an
> > escape clause.) Lots of words just to say, we should add a test for RT
> > to exercise the bad behaviour. Hmm, doesn't need to be pmu, just we need
> > an assertion that execution latency is bounded and no RT hog will delay
> > it.
> 
> Agreed, I can add a simple test to gem_exec_latency.
> 
> But with regards on how to fix this - re-enabling direct submission 
> sounds simplest (not only indirect via tasklet) in theory although I do 
> remember you were raising some issues with this route last time I 
> mentioned it. It does sound like a conceptually correct thing to do.

The problem comes down to that we want direct submission from the irq
handler, which the tasklet solves very nicely for us (most of the time).
Finding an alternative hook other than irq_exit() is the challenge,
irq_work might be acceptable.
 
> As an alternative we could explore conversion effort and resulting 
> latencies from conversion to threaded irq handler.

* shivers

Then we have at least consistently bad latency ;) And the sysadmin can
decide how to prioritise, boo.
 
> You also had a patch to improve tasklet scheduling in some cases now I 
> remember. We can try that after I write the test as well. Although I 
> have no idea how hard of a sell that would be.

I think the next plan for upstream tasklets is to try and avoid having
one vector influence the ksoftirqd latency of another. However, that
doesn't solve it for us, where it's likely we've consumed the tasklet
timeslice and so will still be deferred onto ksoftirqd. (It just solves
the case of netdev forcing us to ksoftirqd along with itself.) The hack
I use on top of that to always do at least one immediate execution of
HISOFTIRQ boils down to why just allow that special case, to which there
is no good answer.

Hmm, irq_work, my only concern is if it is run with irqs disabled. We
could live without, but that's an alarmingly big chunk of code.
-Chris
___
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/gvt: Deliver guest cursor hotspot info (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915/gvt: Deliver guest cursor hotspot info (rev2)
URL   : https://patchwork.freedesktop.org/series/41727/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4054 -> Patchwork_8694 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: 
https://patchwork.freedesktop.org/api/1.0/series/41727/revisions/2/mbox/

== Known issues ==

  Here are the changes found in Patchwork_8694 that come from known issues:

  === IGT changes ===

 Issues hit 

igt@drv_module_reload@basic-no-display:
  fi-cnl-psr: NOTRUN -> DMESG-WARN (fdo#105395) +2


 Possible fixes 

igt@prime_vgem@basic-fence-flip:
  fi-ilk-650: FAIL (fdo#104008) -> PASS


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


== Participating hosts (41 -> 21) ==

  Additional (1): fi-cnl-psr 
  Missing(21): fi-ilk-m540 fi-bxt-dsi fi-skl-gvtdvm fi-bsw-n3050 
fi-byt-j1900 fi-skl-6770hq fi-bdw-gvtdvm fi-skl-6260u fi-bwr-2160 fi-cfl-s3 
fi-bxt-j4205 fi-gdg-551 shard-apl fi-pnv-d510 shard-glk shard-hsw shard-kbl 
shard-snb shard-glkb fi-skl-6700hq fi-skl-6600u 


== Build changes ==

* Linux: CI_DRM_4054 -> Patchwork_8694

  CI_DRM_4054: a0e39233b88795009de32094efa8a2135f34338f @ 
git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4430: 8b77704db49167f7ebfd1c470d9c129d3b862cb6 @ 
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_8694: a117fc1be218228651a8f966f1106912287f3135 @ 
git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4430: 93b35926a150e318439d2505901288594b3548f5 @ 
git://anongit.freedesktop.org/piglit


== Linux commits ==

a117fc1be218 drm/i915/gvt: Deliver guest cursor hotspot info

== Logs ==

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

Re: [Intel-gfx] [PATCH i-g-t v4] tests/perf_pmu: Avoid RT thread for accuracy test

[Tvrtko Ursulin]

On 14/04/2018 12:35, Chris Wilson wrote:

Quoting Tvrtko Ursulin (2018-04-11 14:52:36)


On 11/04/2018 14:23, Chris Wilson wrote:

Quoting Tvrtko Ursulin (2018-04-04 10:51:52)

From: Tvrtko Ursulin 

Realtime scheduling interferes with execlists submission (tasklet) so try
to simplify the PWM loop in a few ways:

   * Drop RT.
   * Longer batches for smaller systematic error.
   * More truthful test duration calculation.
   * Less clock queries.
   * No self-adjust - instead just report the achieved cycle and let the
 parent check against it.
   * Report absolute cycle error.

v2:
   * Bring back self-adjust. (Chris Wilson)
 (But slightly fixed version with no overflow.)

v3:
   * Log average and mean calibration for each pass.

v4:
   * Eliminate development leftovers.
   * Fix variance logging.

Signed-off-by: Tvrtko Ursulin 


  From a pragmatic point of view, there's no point waiting for me to be
happy with the convergence if CI is, and the variance will definitely be
interesting (although you could have used igt_mean to compute the
iterative variance), so

Reviewed-by: Chris Wilson 


Thanks, I've pushed it and so we'll see.


We should resurrect the RT variant in the near future. It's definitely
an issue in our driver that random userspace can impact execution of
unconnected others. (Handling RT starvation of workers is something we
have to be aware of elsewhere, commonly hits oom if we don't have an
escape clause.) Lots of words just to say, we should add a test for RT
to exercise the bad behaviour. Hmm, doesn't need to be pmu, just we need
an assertion that execution latency is bounded and no RT hog will delay
it.


Agreed, I can add a simple test to gem_exec_latency.

But with regards on how to fix this - re-enabling direct submission 
sounds simplest (not only indirect via tasklet) in theory although I do 
remember you were raising some issues with this route last time I 
mentioned it. It does sound like a conceptually correct thing to do.


As an alternative we could explore conversion effort and resulting 
latencies from conversion to threaded irq handler.


You also had a patch to improve tasklet scheduling in some cases now I 
remember. We can try that after I write the test as well. Although I 
have no idea how hard of a sell that would be.


Regards,

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

[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/gvt: Deliver guest cursor hotspot info (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915/gvt: Deliver guest cursor hotspot info (rev2)
URL   : https://patchwork.freedesktop.org/series/41727/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
a117fc1be218 drm/i915/gvt: Deliver guest cursor hotspot info
-:36: CHECK:UNNECESSARY_PARENTHESES: Unnecessary parentheses around 'c->x_hot 
<= c->width'
#36: FILE: drivers/gpu/drm/i915/gvt/dmabuf.c:197:
+   if (c && (c->x_hot <= c->width) && (c->y_hot <= c->height))

-:36: CHECK:UNNECESSARY_PARENTHESES: Unnecessary parentheses around 'c->y_hot 
<= c->height'
#36: FILE: drivers/gpu/drm/i915/gvt/dmabuf.c:197:
+   if (c && (c->x_hot <= c->width) && (c->y_hot <= c->height))

total: 0 errors, 0 warnings, 2 checks, 79 lines checked

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

Re: [Intel-gfx] [RFC v4 16/25] drm: Make ioctls available for in-kernel clients

[Daniel Vetter]

On Sat, Apr 14, 2018 at 01:53:09PM +0200, Noralf Trønnes wrote:
> Make ioctl wrappers for functions that will be used by the in-kernel API.
> The following functions are touched:
> - drm_mode_create_dumb_ioctl()
> - drm_mode_destroy_dumb_ioctl()
> - drm_mode_addfb2()
> - drm_mode_rmfb()
> - drm_prime_handle_to_fd_ioctl()
> 
> drm_mode_addfb2() also gets the ability to override the debug name.
> 
> Signed-off-by: Noralf Trønnes 
> ---
>  drivers/gpu/drm/drm_crtc_internal.h | 18 ++---
>  drivers/gpu/drm/drm_dumb_buffers.c  | 33 
>  drivers/gpu/drm/drm_framebuffer.c   | 50 
> -
>  drivers/gpu/drm/drm_internal.h  |  3 +++
>  drivers/gpu/drm/drm_ioc32.c |  2 +-
>  drivers/gpu/drm/drm_ioctl.c |  4 +--
>  drivers/gpu/drm/drm_prime.c | 13 +++---
>  7 files changed, 84 insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_crtc_internal.h 
> b/drivers/gpu/drm/drm_crtc_internal.h
> index 3c2b82865ad2..8f8886ac0e4d 100644
> --- a/drivers/gpu/drm/drm_crtc_internal.h
> +++ b/drivers/gpu/drm/drm_crtc_internal.h
> @@ -62,6 +62,12 @@ int drm_mode_getresources(struct drm_device *dev,
>  
>  
>  /* drm_dumb_buffers.c */
> +int drm_mode_create_dumb(struct drm_device *dev,
> +  struct drm_mode_create_dumb *args,
> +  struct drm_file *file_priv);
> +int drm_mode_destroy_dumb(struct drm_device *dev, u32 handle,
> +   struct drm_file *file_priv);
> +
>  /* IOCTLs */
>  int drm_mode_create_dumb_ioctl(struct drm_device *dev,
>  void *data, struct drm_file *file_priv);
> @@ -163,14 +169,18 @@ int drm_framebuffer_check_src_coords(uint32_t src_x, 
> uint32_t src_y,
>const struct drm_framebuffer *fb);
>  void drm_fb_release(struct drm_file *file_priv);
>  
> +int drm_mode_addfb2(struct drm_device *dev, struct drm_mode_fb_cmd2 *r,
> + struct drm_file *file_priv, const char *comm);
> +int drm_mode_rmfb(struct drm_device *dev, u32 fb_id,
> +   struct drm_file *file_priv);
>  
>  /* IOCTL */
>  int drm_mode_addfb(struct drm_device *dev,
>  void *data, struct drm_file *file_priv);
> -int drm_mode_addfb2(struct drm_device *dev,
> - void *data, struct drm_file *file_priv);
> -int drm_mode_rmfb(struct drm_device *dev,
> -   void *data, struct drm_file *file_priv);
> +int drm_mode_addfb2_ioctl(struct drm_device *dev,
> +   void *data, struct drm_file *file_priv);
> +int drm_mode_rmfb_ioctl(struct drm_device *dev,
> + void *data, struct drm_file *file_priv);
>  int drm_mode_getfb(struct drm_device *dev,
>  void *data, struct drm_file *file_priv);
>  int drm_mode_dirtyfb_ioctl(struct drm_device *dev,
> diff --git a/drivers/gpu/drm/drm_dumb_buffers.c 
> b/drivers/gpu/drm/drm_dumb_buffers.c
> index 39ac15ce4702..eed9687b8698 100644
> --- a/drivers/gpu/drm/drm_dumb_buffers.c
> +++ b/drivers/gpu/drm/drm_dumb_buffers.c
> @@ -53,10 +53,10 @@
>   * a hardware-specific ioctl to allocate suitable buffer objects.
>   */
>  
> -int drm_mode_create_dumb_ioctl(struct drm_device *dev,
> -void *data, struct drm_file *file_priv)
> +int drm_mode_create_dumb(struct drm_device *dev,
> +  struct drm_mode_create_dumb *args,
> +  struct drm_file *file_priv)
>  {
> - struct drm_mode_create_dumb *args = data;
>   u32 cpp, stride, size;
>  
>   if (!dev->driver->dumb_create)
> @@ -91,6 +91,12 @@ int drm_mode_create_dumb_ioctl(struct drm_device *dev,
>   return dev->driver->dumb_create(file_priv, dev, args);
>  }
>  
> +int drm_mode_create_dumb_ioctl(struct drm_device *dev,
> +void *data, struct drm_file *file_priv)
> +{
> + return drm_mode_create_dumb(dev, data, file_priv);
> +}
> +
>  /**
>   * drm_mode_mmap_dumb_ioctl - create an mmap offset for a dumb backing 
> storage buffer
>   * @dev: DRM device
> @@ -122,17 +128,22 @@ int drm_mode_mmap_dumb_ioctl(struct drm_device *dev,
>  >offset);
>  }
>  
> +int drm_mode_destroy_dumb(struct drm_device *dev, u32 handle,
> +   struct drm_file *file_priv)
> +{
> + if (!dev->driver->dumb_create)
> + return -ENOSYS;
> +
> + if (dev->driver->dumb_destroy)
> + return dev->driver->dumb_destroy(file_priv, dev, handle);
> + else
> + return drm_gem_dumb_destroy(file_priv, dev, handle);
> +}
> +
>  int drm_mode_destroy_dumb_ioctl(struct drm_device *dev,
>   void *data, struct drm_file *file_priv)
>  {
>   struct drm_mode_destroy_dumb *args = data;
>  
> - if (!dev->driver->dumb_create)
> - return -ENOSYS;
> -
> - if (dev->driver->dumb_destroy)
> - return 

Re: [Intel-gfx] [RFC v4 22/25] drm/fb-helper: Add generic fbdev emulation

[Daniel Vetter]

On Sat, Apr 14, 2018 at 01:53:15PM +0200, Noralf Trønnes wrote:
> This adds generic fbdev emulation for drivers that supports
> dumb buffers which they can export.
> 
> All the driver has to do is call drm_fbdev_generic_setup().
> 
> Signed-off-by: Noralf Trønnes 
> ---
>  drivers/gpu/drm/drm_fb_helper.c | 255 
> 
>  include/drm/drm_fb_helper.h |  20 
>  2 files changed, 275 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index b1124c08b1ed..1954de5b13e0 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -30,6 +30,7 @@
>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>  
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -1995,6 +1996,260 @@ void drm_fb_helper_output_poll_changed(struct 
> drm_device *dev)
>  }
>  EXPORT_SYMBOL(drm_fb_helper_output_poll_changed);
>  
> +static int drm_fbdev_fb_mmap(struct fb_info *info, struct vm_area_struct 
> *vma)
> +{
> + struct drm_fb_helper *fb_helper = info->par;
> +
> + return dma_buf_mmap(fb_helper->buffer->dma_buf, vma, 0);
> +}
> +
> +/*
> + * fb_ops.fb_destroy is called by the last put_fb_info() call at the end of
> + * unregister_framebuffer() or fb_release().
> + */
> +static void drm_fbdev_fb_destroy(struct fb_info *info)
> +{
> + struct drm_fb_helper *fb_helper = info->par;
> + struct fb_ops *fbops = NULL;
> +
> + DRM_DEBUG("\n");
> +
> + if (fb_helper->fbdev->fbdefio)
> + fbops = fb_helper->fbdev->fbops;
> +
> + drm_fb_helper_fini(fb_helper);
> + drm_client_framebuffer_delete(fb_helper->buffer);
> + drm_client_free(fb_helper->client);
> + kfree(fb_helper);
> + kfree(fbops);
> +}
> +
> +static struct fb_ops drm_fbdev_fb_ops = {
> + /*
> +  * No need to set owner, this module is already pinned by the driver.
> +  * A reference is taken on the driver module in drm_fb_helper_fb_open()
> +  * to prevent the driver going away with open fd's.
> +  */
> + DRM_FB_HELPER_DEFAULT_OPS,
> + .fb_open= drm_fb_helper_fb_open,
> + .fb_release = drm_fb_helper_fb_release,
> + .fb_destroy = drm_fbdev_fb_destroy,
> + .fb_mmap= drm_fbdev_fb_mmap,
> + .fb_read= drm_fb_helper_sys_read,
> + .fb_write   = drm_fb_helper_sys_write,
> + .fb_fillrect= drm_fb_helper_sys_fillrect,
> + .fb_copyarea= drm_fb_helper_sys_copyarea,
> + .fb_imageblit   = drm_fb_helper_sys_imageblit,
> +};
> +
> +static struct fb_deferred_io drm_fbdev_defio = {
> + .delay  = HZ / 20,
> + .deferred_io= drm_fb_helper_deferred_io,
> +};
> +
> +/* Hack to test tinydrm before converting to vmalloc buffers */
> +static int drm_fbdev_cma_deferred_io_mmap(struct fb_info *info,
> +   struct vm_area_struct *vma)
> +{
> + fb_deferred_io_mmap(info, vma);
> + vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot);
> +
> + return 0;
> +}
> +
> +static int drm_fb_helper_generic_probe(struct drm_fb_helper *fb_helper,
> +struct drm_fb_helper_surface_size *sizes)
> +{
> + struct drm_client_dev *client = fb_helper->client;
> + struct drm_display_mode sizes_mode = {
> + .hdisplay = sizes->surface_width,
> + .vdisplay = sizes->surface_height,
> + };
> + struct drm_client_buffer *buffer;
> + struct drm_framebuffer *fb;
> + struct fb_info *fbi;
> + u32 format;
> + int ret;
> +
> + DRM_DEBUG_KMS("surface width(%d), height(%d) and bpp(%d)\n",
> +   sizes->surface_width, sizes->surface_height,
> +   sizes->surface_bpp);
> +
> + format = drm_mode_legacy_fb_format(sizes->surface_bpp, 
> sizes->surface_depth);
> + buffer = drm_client_framebuffer_create(client, _mode, format);
> + if (IS_ERR(buffer))
> + return PTR_ERR(buffer);
> +
> + fb_helper->buffer = buffer;
> + fb_helper->fb = buffer->fb;
> + fb = buffer->fb;
> +
> + fbi = drm_fb_helper_alloc_fbi(fb_helper);
> + if (IS_ERR(fbi)) {
> + ret = PTR_ERR(fbi);
> + goto err_free_buffer;
> + }
> +
> + fbi->par = fb_helper;
> + fbi->fbops = _fbdev_fb_ops;
> + fbi->screen_size = fb->height * fb->pitches[0];
> + fbi->fix.smem_len = fbi->screen_size;
> + fbi->screen_buffer = buffer->vaddr;
> + strcpy(fbi->fix.id, "DRM emulated");
> +
> + drm_fb_helper_fill_fix(fbi, fb->pitches[0], fb->format->depth);
> + drm_fb_helper_fill_var(fbi, fb_helper, sizes->fb_width, 
> sizes->fb_height);
> +
> + /*
> +  * Drivers that set the dirty callback:
> +  * - Doesn't use defio:
> +  *   i915, virtio, rockchip
> +  * - defio with vmalloc buffer blitted on the real one:
> +  *   vmwgfx
> +  * - defio is disabled because it doesn't work with shmem:
> +  

Re: [Intel-gfx] [RFC v4 21/25] drm/fb-helper: Add drm_fb_helper_fb_open/release()

[Daniel Vetter]

On Sat, Apr 14, 2018 at 01:53:14PM +0200, Noralf Trønnes wrote:
> These helpers keep track of fbdev users and drm_driver.last_close will
> only restore fbdev when actually in use. Additionally the display is
> turned off when the last user is closing. fbcon is a user in this context.
> 
> If struct fb_ops is defined in a library, fb_open() takes a ref on the
> library (fb_ops.owner) instead of the driver module. Fix that by ensuring
> that the driver module is pinned.
> 
> The functions are not added to the DRM_FB_HELPER_DEFAULT_OPS() macro,
> because some of its users do set fb_open/release themselves.
> 
> Signed-off-by: Noralf Trønnes 
> ---
>  drivers/gpu/drm/drm_fb_helper.c | 54 
> -
>  include/drm/drm_fb_helper.h | 29 ++
>  2 files changed, 82 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 98e5bc92c9f2..b1124c08b1ed 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -177,7 +177,7 @@ int drm_fb_helper_restore_fbdev_mode_unlocked(struct 
> drm_fb_helper *fb_helper)
>   if (!drm_fbdev_emulation || !fb_helper)
>   return -ENODEV;
>  
> - if (READ_ONCE(fb_helper->deferred_setup))
> + if (READ_ONCE(fb_helper->deferred_setup) || 
> !READ_ONCE(fb_helper->open_count))
>   return 0;
>  
>   mutex_lock(_helper->lock);
> @@ -368,6 +368,7 @@ void drm_fb_helper_prepare(struct drm_device *dev, struct 
> drm_fb_helper *helper,
>   INIT_WORK(>dirty_work, drm_fb_helper_dirty_work);
>   helper->dirty_clip.x1 = helper->dirty_clip.y1 = ~0;
>   mutex_init(>lock);
> + helper->open_count = 1;
>   helper->funcs = funcs;
>   helper->dev = dev;
>  }
> @@ -620,6 +621,53 @@ int drm_fb_helper_defio_init(struct drm_fb_helper 
> *fb_helper)
>  }
>  EXPORT_SYMBOL(drm_fb_helper_defio_init);
>  
> +/**
> + * drm_fb_helper_fb_open - implementation for _ops.fb_open
> + * @info: fbdev registered by the helper
> + * @user: 1=userspace, 0=fbcon
> + *
> + * Increase fbdev use count.
> + * If _ops is wrapped in a library, pin the driver module.
> + */
> +int drm_fb_helper_fb_open(struct fb_info *info, int user)
> +{
> + struct drm_fb_helper *fb_helper = info->par;
> + struct drm_device *dev = fb_helper->dev;
> +
> + if (info->fbops->owner != dev->driver->fops->owner) {
> + if (!try_module_get(dev->driver->fops->owner))
> + return -ENODEV;
> + }
> +
> + fb_helper->open_count++;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_fb_helper_fb_open);
> +
> +/**
> + * drm_fb_helper_fb_release - implementation for _ops.fb_release
> + * @info: fbdev registered by the helper
> + * @user: 1=userspace, 0=fbcon
> + *
> + * Decrease fbdev use count and turn off if there are no users left.
> + * If _ops is wrapped in a library, unpin the driver module.
> + */
> +int drm_fb_helper_fb_release(struct fb_info *info, int user)
> +{
> + struct drm_fb_helper *fb_helper = info->par;
> + struct drm_device *dev = fb_helper->dev;
> +
> + if (!(--fb_helper->open_count))
> + drm_fb_helper_dpms(info, DRM_MODE_DPMS_OFF);
> +
> + if (info->fbops->owner != dev->driver->fops->owner)
> + module_put(dev->driver->fops->owner);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_fb_helper_fb_release);
> +
>  /**
>   * drm_fb_helper_sys_read - wrapper around fb_sys_read
>   * @info: fb_info struct pointer
> @@ -1436,6 +1484,10 @@ static int drm_fb_helper_single_fb_probe(struct 
> drm_fb_helper *fb_helper,
>   if (ret < 0)
>   return ret;
>  
> + /* Block restore without users if we do track it */
> + if (fb_helper->fbdev->fbops->fb_open == drm_fb_helper_fb_open)
> + fb_helper->open_count = 0;

Since we kzcalloc, isn't this entirely redundant? Looks confusing to me at
least.

Otherwise looks all reasonable.
-Daniel


> +
>   strcpy(fb_helper->fb->comm, "[fbcon]");
>   return 0;
>  }
> diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
> index 5f66f253a97b..330983975d5e 100644
> --- a/include/drm/drm_fb_helper.h
> +++ b/include/drm/drm_fb_helper.h
> @@ -184,6 +184,22 @@ struct drm_fb_helper {
>* See also: @deferred_setup
>*/
>   int preferred_bpp;
> +
> + /**
> +  * @open_count:
> +  *
> +  * Keeps track of fbdev use to know when to not restore fbdev and to
> +  * disable the pipeline when the last user is gone.
> +  *
> +  * Drivers that use drm_fb_helper_fb_open() as their \.fb_open
> +  * callback will get an initial value of 0 and get restore based on
> +  * actual use. Others will get an initial value of 1 which means that
> +  * fbdev will always be restored. Drivers that call
> +  * drm_fb_helper_fb_open() in their \.fb_open, thus needs to set the
> +  * initial value to 0 themselves in their 

Re: [Intel-gfx] [RFC v4 17/25] drm/client: Bail out if there's a DRM master

[Daniel Vetter]

On Sat, Apr 14, 2018 at 01:53:10PM +0200, Noralf Trønnes wrote:
> If there's a DRM master, return -EBUSY.
> Block userspace from becoming master by taking the master lock while
> the client is setting the mode.
> 
> Suggested-by: Daniel Vetter 
> Signed-off-by: Noralf Trønnes 
> ---
>  drivers/gpu/drm/drm_auth.c  | 33 +
>  drivers/gpu/drm/drm_client.c| 34 +-
>  drivers/gpu/drm/drm_fb_helper.c |  8 
>  drivers/gpu/drm/drm_internal.h  |  2 ++
>  include/drm/drm_client.h|  4 ++--
>  5 files changed, 70 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
> index d9c0f7573905..d656d0d93da3 100644
> --- a/drivers/gpu/drm/drm_auth.c
> +++ b/drivers/gpu/drm/drm_auth.c
> @@ -366,3 +366,36 @@ void drm_master_put(struct drm_master **master)
>   *master = NULL;
>  }
>  EXPORT_SYMBOL(drm_master_put);
> +
> +/**
> + * drm_master_block - Block DRM master operations
> + * @dev: DRM device
> + *
> + * This function checks if there is a master on @dev. If there is no master 
> it
> + * blocks anyone from becoming master. In-kernel clients can use this to know
> + * when they can act as master. Use drm_master_unblock() to unblock.
> + *
> + * Returns:
> + * True if there is no master, false otherwise.
> + */
> +bool drm_master_block(struct drm_device *dev)
> +{
> + mutex_lock(>master_mutex);
> + if (dev->master) {
> + mutex_unlock(>master_mutex);
> + return false;
> + }
> +
> + return true;
> +}
> +
> +/**
> + * drm_master_unblock - Unblock DRM master operations
> + * @dev: DRM device
> + *
> + * Unblock and allow userspace to become master.
> + */
> +void drm_master_unblock(struct drm_device *dev)
> +{
> + mutex_unlock(>master_mutex);
> +}
> diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
> index 27818a467b09..764c556630b8 100644
> --- a/drivers/gpu/drm/drm_client.c
> +++ b/drivers/gpu/drm/drm_client.c
> @@ -18,6 +18,8 @@
>  #include 
>  #include 
>  
> +#include "drm_internal.h"
> +
>  struct drm_client_display_offset {
>   int x, y;
>  };
> @@ -313,18 +315,30 @@ static int drm_client_display_restore_legacy(struct 
> drm_client_display *display)
>  /**
>   * drm_client_display_restore() - Restore client display
>   * @display: Client display
> + * @force: If true, restore even if there's a DRM master

This smells a bit like bad interface design. Essentially this is a "should
I lock or not" flag, and if locking should/needs to be done by at least
some callers, then all of them should do that.

To make sure no one screws up, you can add a

drm_client_masters_are_blocked()
{
lockdep_assert_held(client->dev->master_lock);
}

helper and call it at all the places you really want to have all other
masters excluded.

And locking at the code, we really do want to pull the
master_block/unblock critical section all the way out, to make sure that
an fbdev action only ever works or doesn't do any kind of damage at all.
We really don't want another master to take over while fbdev emulation is
doing stuff - drm_client_master_block/unblock is meant to fix these races.
-Daniel

>   *
>   * Restore client display using the current modeset configuration.
>   *
>   * Return:
>   * Zero on succes or negative error code on failure.
>   */
> -int drm_client_display_restore(struct drm_client_display *display)
> +int drm_client_display_restore(struct drm_client_display *display, bool 
> force)
>  {
> - if (drm_drv_uses_atomic_modeset(display->dev))
> - return drm_client_display_restore_atomic(display, true);
> + struct drm_device *dev = display->dev;
> + int ret;
> +
> + if (!force && !drm_master_block(dev))
> + return -EBUSY;
> +
> + if (drm_drv_uses_atomic_modeset(dev))
> + ret = drm_client_display_restore_atomic(display, true);
>   else
> - return drm_client_display_restore_legacy(display);
> + ret = drm_client_display_restore_legacy(display);
> +
> + if (!force)
> + drm_master_unblock(dev);
> +
> + return ret;
>  }
>  EXPORT_SYMBOL(drm_client_display_restore);
>  
> @@ -354,13 +368,23 @@ static void drm_client_display_dpms_legacy(struct 
> drm_client_display *display, i
>   * drm_client_display_dpms() - Set display DPMS mode
>   * @display: Client display
>   * @mode: DPMS mode
> + *
> + * Returns:
> + * Zero on success, -EBUSY if there's a DRM master.
>   */
> -void drm_client_display_dpms(struct drm_client_display *display, int mode)
> +int drm_client_display_dpms(struct drm_client_display *display, int mode)
>  {
> + if (!drm_master_block(display->dev))
> + return -EBUSY;
> +
>   if (drm_drv_uses_atomic_modeset(display->dev))
>   drm_client_display_restore_atomic(display, mode == 
> DRM_MODE_DPMS_ON);
>   else
>   

Re: [Intel-gfx] [RFC v4 12/25] drm/i915: Add drm_driver->initial_client_display callback

[Daniel Vetter]

On Sat, Apr 14, 2018 at 01:53:05PM +0200, Noralf Trønnes wrote:
> As part of moving the modesetting code out of drm_fb_helper and into
> drm_client, the drm_fb_helper_funcs->initial_config callback needs to go.
> Replace it with a drm_driver->initial_client_display callback that can
> work for all in-kernel clients.
> 
> TODO:
> - Add a patch that moves the function out of intel_fbdev.c since it's not
>   fbdev specific anymore.
> 
> Signed-off-by: Noralf Trønnes 

So the reason we originally added this callback for i915 fast boot was
that there wasn't any atomic around yet. And it was all an experiment to
figure out how to best go about designing fastboot.

But now we have fbdev, and fastboot design is also pretty clear:

1. driver loads
2. driver reads out current hw state, reconstructs a full atomic state for
everything and stuffs it into connector/crtc/plane->state pointers.
3. fbdev and any other client read out current state (with some caveats)
and just take it over.

What non-fastboot drivers do:
1. drivers load
2. reset both hw and sw state to everything off.

Now the intel_fb_initial_config is all generic code really, and it will
neatly fall back to the default config if everything is off. This means we
could:
1. Move the intel_fb_initial_config into the fbdev helpers.
2. Nuke the ->initial_config callback.

And pronto! every driver which implements hw state readout will get fbdev
fastboot for free.

And since you've already rewritting the intel code to use drm_client, it's
practically done already. Just need to s/intel_/drm_fbdev_helper_ or
something like that :-)
-Daniel

> ---
>  drivers/gpu/drm/drm_fb_helper.c|  19 +--
>  drivers/gpu/drm/i915/i915_drv.c|   1 +
>  drivers/gpu/drm/i915/intel_drv.h   |  11 
>  drivers/gpu/drm/i915/intel_fbdev.c | 113 
> ++---
>  include/drm/drm_drv.h  |  21 +++
>  5 files changed, 104 insertions(+), 61 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index b992f59dad30..5407bf6dc8c0 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -2103,6 +2103,20 @@ static void drm_setup_crtcs(struct drm_fb_helper 
> *fb_helper,
>   /* prevent concurrent modification of connector_count by hotplug */
>   lockdep_assert_held(_helper->lock);
>  
> + mutex_lock(>mode_config.mutex);
> + if (drm_fb_helper_probe_connector_modes(fb_helper, width, height) == 0)
> + DRM_DEBUG_KMS("No connectors reported connected with modes\n");
> +
> + if (dev->driver->initial_client_display) {
> + display = dev->driver->initial_client_display(dev, width, 
> height);
> + if (display) {
> + drm_client_display_free(fb_helper->display);
> + fb_helper->display = display;
> + mutex_unlock(>mode_config.mutex);
> + return;
> + }
> + }
> +
>   crtcs = kcalloc(fb_helper->connector_count,
>   sizeof(struct drm_fb_helper_crtc *), GFP_KERNEL);
>   modes = kcalloc(fb_helper->connector_count,
> @@ -2120,9 +2134,6 @@ static void drm_setup_crtcs(struct drm_fb_helper 
> *fb_helper,
>   if (IS_ERR(display))
>   goto out;
>  
> - mutex_lock(_helper->dev->mode_config.mutex);
> - if (drm_fb_helper_probe_connector_modes(fb_helper, width, height) == 0)
> - DRM_DEBUG_KMS("No connectors reported connected with modes\n");
>   drm_enable_connectors(fb_helper, enabled);
>  
>   if (!(fb_helper->funcs->initial_config &&
> @@ -2144,7 +2155,6 @@ static void drm_setup_crtcs(struct drm_fb_helper 
> *fb_helper,
>  
>   drm_pick_crtcs(fb_helper, crtcs, modes, 0, width, height);
>   }
> - mutex_unlock(_helper->dev->mode_config.mutex);
>  
>   /* need to set the modesets up here for use later */
>   /* fill out the connector<->crtc mappings into the modesets */
> @@ -2182,6 +2192,7 @@ static void drm_setup_crtcs(struct drm_fb_helper 
> *fb_helper,
>   drm_client_display_free(fb_helper->display);
>   fb_helper->display = display;
>  out:
> + mutex_unlock(>mode_config.mutex);
>   kfree(crtcs);
>   kfree(modes);
>   kfree(offsets);
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 07c07d55398b..b746c0cbaa4b 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -2857,6 +2857,7 @@ static struct drm_driver driver = {
>  
>   .dumb_create = i915_gem_dumb_create,
>   .dumb_map_offset = i915_gem_mmap_gtt,
> + .initial_client_display = i915_initial_client_display,
>   .ioctls = i915_ioctls,
>   .num_ioctls = ARRAY_SIZE(i915_ioctls),
>   .fops = _driver_fops,
> diff --git a/drivers/gpu/drm/i915/intel_drv.h 
> b/drivers/gpu/drm/i915/intel_drv.h
> index d4368589b355..f77f510617c5 100644
> --- 

Re: [Intel-gfx] [RFC v4 11/25] drm/connector: Add connector array functions

[Daniel Vetter]

On Sat, Apr 14, 2018 at 01:53:04PM +0200, Noralf Trønnes wrote:
> Add functions to deal with the registred connectors as an array:
> - drm_connector_get_all()
> - drm_connector_put_all()
> 
> And to get the enabled status of those connectors:
> drm_connector_get_enabled_status()
> 
> This is prep work to remove struct drm_fb_helper_connector.
> 
> Signed-off-by: Noralf Trønnes 
> ---
>  drivers/gpu/drm/drm_connector.c | 105 
> 
>  include/drm/drm_connector.h |   5 ++
>  2 files changed, 110 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index b9eb143d70fc..25c333c05a4e 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -1854,3 +1854,108 @@ drm_connector_pick_cmdline_mode(struct drm_connector 
> *connector)
>   return mode;
>  }
>  EXPORT_SYMBOL(drm_connector_pick_cmdline_mode);
> +
> +/**
> + * drm_connector_get_all - Get all connectors into an array
> + * @dev: DRM device
> + * @connectors: Returned connector array
> + *
> + * This function iterates through all registered connectors and adds them to 
> an
> + * array allocated by this function. A ref is taken on the connectors.
> + *
> + * Use drm_connector_put_all() to drop refs and free the array.
> + *
> + * Returns:
> + * Number of connectors or -ENOMEM on failure.
> + */
> +int drm_connector_get_all(struct drm_device *dev, struct drm_connector 
> ***connectors)

I honestly don't like the fbdev pattern of having connector arrays all
that much. I think in a world of hotplug we should have as much code as
possible iterating over the life connector list using the special
functions.

Given that these functions here set a bit a bad example imo I'd drop them
and just live with the code duplication.
-Daniel

> +{
> + struct drm_connector *connector, **temp, **conns = NULL;
> + struct drm_connector_list_iter conn_iter;
> + int connector_count = 0;
> +
> + drm_connector_list_iter_begin(dev, _iter);
> + drm_for_each_connector_iter(connector, _iter) {
> + temp = krealloc(conns, (connector_count + 1) * sizeof(*conns), 
> GFP_KERNEL);
> + if (!temp)
> + goto err_put_free;
> +
> + conns = temp;
> + conns[connector_count++] = connector;
> + drm_connector_get(connector);
> + }
> + drm_connector_list_iter_end(_iter);
> +
> + *connectors = conns;
> +
> + return connector_count;
> +
> +err_put_free:
> + drm_connector_list_iter_end(_iter);
> + drm_connector_put_all(conns, connector_count);
> +
> + return -ENOMEM;
> +}
> +EXPORT_SYMBOL(drm_connector_get_all);
> +
> +/**
> + * drm_connector_put_all - Put and free connector array
> + * @connectors: Array of connectors
> + * @connector_count: Number of connectors in the array (can be negative or 
> zero)
> + *
> + * This function drops the ref on the connectors an frees the array.
> + */
> +void drm_connector_put_all(struct drm_connector **connectors, int 
> connector_count)
> +{
> + int i;
> +
> + if (connector_count < 1)
> + return;
> +
> + for (i = 0; i < connector_count; i++)
> + drm_connector_put(connectors[i]);
> + kfree(connectors);
> +}
> +EXPORT_SYMBOL(drm_connector_put_all);
> +
> +/**
> + * drm_connector_get_enabled_status - Get enabled status on connectors
> + * @connectors: Array of connectors
> + * @connector_count: Number of connectors in the array
> + *
> + * This loops over the connector array and sets enabled if connector status 
> is
> + * _connected_. If no connectors are connected, a new pass is done and
> + * connectors that are not _disconnected_ are set enabled.
> + *
> + * The caller is responsible for freeing the array using kfree().
> + *
> + * Returns:
> + * A boolean array of connector enabled statuses or NULL on allocation 
> failure.
> + */
> +bool *drm_connector_get_enabled_status(struct drm_connector **connectors,
> +unsigned int connector_count)
> +{
> + bool *enabled, any_enabled = false;
> + unsigned int i;
> +
> + enabled = kcalloc(connector_count, sizeof(*enabled), GFP_KERNEL);
> + if (!enabled)
> + return NULL;
> +
> + for (i = 0; i < connector_count; i++) {
> + if (connectors[i]->status == connector_status_connected) {
> + enabled[i] = true;
> + any_enabled = true;
> + }
> + }
> +
> + if (any_enabled)
> + return enabled;
> +
> + for (i = 0; i < connector_count; i++)
> + if (connectors[i]->status != connector_status_disconnected)
> + enabled[i] = true;
> +
> + return enabled;
> +}
> +EXPORT_SYMBOL(drm_connector_get_enabled_status);
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 9cb4ca42373c..c3556a5f40c9 100644
> --- 

Re: [Intel-gfx] [RFC v4 06/25] drm/atomic: Move __drm_atomic_helper_disable_plane/set_config()

[Daniel Vetter]

On Sat, Apr 14, 2018 at 01:52:59PM +0200, Noralf Trønnes wrote:
> Prepare for moving drm_fb_helper modesetting code to drm_client.
> drm_client will be linked to drm.ko, so move
> __drm_atomic_helper_disable_plane() and __drm_atomic_helper_set_config()
> out of drm_kms_helper.ko.
> 
> Signed-off-by: Noralf Trønnes 
> ---
>  drivers/gpu/drm/drm_atomic.c| 168 
> 
>  drivers/gpu/drm/drm_atomic_helper.c | 168 
> +---
>  drivers/gpu/drm/drm_fb_helper.c |   8 +-
>  include/drm/drm_atomic.h|   5 ++
>  include/drm/drm_atomic_helper.h |   4 -
>  5 files changed, 179 insertions(+), 174 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 7d25c42f22db..1fb602b6c8b1 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -2060,6 +2060,174 @@ void drm_atomic_clean_old_fb(struct drm_device *dev,
>  }
>  EXPORT_SYMBOL(drm_atomic_clean_old_fb);
>  
> +/* just used from drm_client and atomic-helper: */

In that case please put the prototype for these into the internal header
drm_crtc_internal.h. Just to avoid drivers abusing this (they really
shouldn't).
-Daniel

> +int drm_atomic_disable_plane(struct drm_plane *plane,
> +  struct drm_plane_state *plane_state)
> +{
> + int ret;
> +
> + ret = drm_atomic_set_crtc_for_plane(plane_state, NULL);
> + if (ret != 0)
> + return ret;
> +
> + drm_atomic_set_fb_for_plane(plane_state, NULL);
> + plane_state->crtc_x = 0;
> + plane_state->crtc_y = 0;
> + plane_state->crtc_w = 0;
> + plane_state->crtc_h = 0;
> + plane_state->src_x = 0;
> + plane_state->src_y = 0;
> + plane_state->src_w = 0;
> + plane_state->src_h = 0;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_atomic_disable_plane);
> +
> +static int update_output_state(struct drm_atomic_state *state,
> +struct drm_mode_set *set)
> +{
> + struct drm_device *dev = set->crtc->dev;
> + struct drm_crtc *crtc;
> + struct drm_crtc_state *new_crtc_state;
> + struct drm_connector *connector;
> + struct drm_connector_state *new_conn_state;
> + int ret, i;
> +
> + ret = drm_modeset_lock(>mode_config.connection_mutex,
> +state->acquire_ctx);
> + if (ret)
> + return ret;
> +
> + /* First disable all connectors on the target crtc. */
> + ret = drm_atomic_add_affected_connectors(state, set->crtc);
> + if (ret)
> + return ret;
> +
> + for_each_new_connector_in_state(state, connector, new_conn_state, i) {
> + if (new_conn_state->crtc == set->crtc) {
> + ret = drm_atomic_set_crtc_for_connector(new_conn_state,
> + NULL);
> + if (ret)
> + return ret;
> +
> + /* Make sure legacy setCrtc always re-trains */
> + new_conn_state->link_status = DRM_LINK_STATUS_GOOD;
> + }
> + }
> +
> + /* Then set all connectors from set->connectors on the target crtc */
> + for (i = 0; i < set->num_connectors; i++) {
> + new_conn_state = drm_atomic_get_connector_state(state,
> + set->connectors[i]);
> + if (IS_ERR(new_conn_state))
> + return PTR_ERR(new_conn_state);
> +
> + ret = drm_atomic_set_crtc_for_connector(new_conn_state,
> + set->crtc);
> + if (ret)
> + return ret;
> + }
> +
> + for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
> + /*
> +  * Don't update ->enable for the CRTC in the set_config request,
> +  * since a mismatch would indicate a bug in the upper layers.
> +  * The actual modeset code later on will catch any
> +  * inconsistencies here.
> +  */
> + if (crtc == set->crtc)
> + continue;
> +
> + if (!new_crtc_state->connector_mask) {
> + ret = drm_atomic_set_mode_prop_for_crtc(new_crtc_state,
> + NULL);
> + if (ret < 0)
> + return ret;
> +
> + new_crtc_state->active = false;
> + }
> + }
> +
> + return 0;
> +}
> +
> +/* just used from drm_client and atomic-helper: */
> +int drm_atomic_set_config(struct drm_mode_set *set,
> +   struct drm_atomic_state *state)
> +{
> + struct drm_crtc_state *crtc_state;
> + struct drm_plane_state *primary_state;
> + struct drm_crtc *crtc = set->crtc;
> + int hdisplay, vdisplay;
> + int ret;
> +
> +   

Re: [Intel-gfx] [RFC v4 19/25] drm/client: Finish the in-kernel client API

[Daniel Vetter]

On Sat, Apr 14, 2018 at 01:53:12PM +0200, Noralf Trønnes wrote:
> The modesetting code is already present, this adds the rest of the API.

Mentioning the TODO in the commit message would be good. Helps readers
like me who have an attention span measured in seconds :-)

Just commenting on the create_buffer leak here

> +static struct drm_client_buffer *
> +drm_client_buffer_create(struct drm_client_dev *client, u32 width, u32 
> height, u32 format)
> +{
> + struct drm_mode_create_dumb dumb_args = { };
> + struct drm_prime_handle prime_args = { };
> + struct drm_client_buffer *buffer;
> + struct dma_buf *dma_buf;
> + void *vaddr;
> + int ret;
> +
> + buffer = kzalloc(sizeof(*buffer), GFP_KERNEL);
> + if (!buffer)
> + return ERR_PTR(-ENOMEM);
> +
> + buffer->client = client;
> + buffer->width = width;
> + buffer->height = height;
> + buffer->format = format;
> +
> + dumb_args.width = buffer->width;
> + dumb_args.height = buffer->height;
> + dumb_args.bpp = drm_format_plane_cpp(format, 0) * 8;
> + ret = drm_mode_create_dumb(client->dev, _args, client->file);
> + if (ret)
> + goto err_free;
> +
> + buffer->handle = dumb_args.handle;
> + buffer->pitch = dumb_args.pitch;
> + buffer->size = dumb_args.size;
> +
> + prime_args.handle = dumb_args.handle;
> + ret = drm_prime_handle_to_fd(client->dev, _args, client->file);
> + if (ret)
> + goto err_delete;
> +
> + dma_buf = dma_buf_get(prime_args.fd);
> + if (IS_ERR(dma_buf)) {
> + ret = PTR_ERR(dma_buf);
> + goto err_delete;
> + }
> +
> + /*
> +  * If called from a worker the dmabuf fd isn't closed and the ref
> +  * doesn't drop to zero on free.
> +  * If I use __close_fd() it's all fine, but that function is not 
> exported.
> +  *
> +  * How do I get rid of this fd when in a worker/kernel thread?
> +  * The fd isn't used beyond this function.
> +  */
> +//   WARN_ON(__close_fd(current->files, prime_args.fd));

Hm, this isn't 100% what I had in mind as the sequence for generic buffer
creation. Pseudo-code:

ret = drm_mode_create_dumb(client->dev, _args, client->file);
if (ret)
goto err_free;

gem_bo = drm_gem_object_lookup(client->file, dumb_args.handle);

gives you _really_ directly the underlying gem_bo. Of course this doesn't
work for non-gem based driver, but reality is that (almost) all of them
are. And we will not accept any new drivers which aren't gem based. So
ignoring vmwgfx for this drm_client work is imo perfectly fine. We should
ofc keep the option in the fb helpers to use non-gem buffers (so that
vmwgfx could switch over from their own in-driver fbdev helpers). All we
need for that is to keep the fb_probe callback.

Was there any other reason than vmwgfx for using prime buffers instead of
just directly using gem?

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [RFC v4 00/25] drm: Add generic fbdev emulation

[Daniel Vetter]

On Sat, Apr 14, 2018 at 01:52:53PM +0200, Noralf Trønnes wrote:
> This patchset explores the possibility of having generic fbdev emulation
> in DRM for drivers that supports dumb buffers which they can export. An
> API is added to support in-kernel clients in general.
> 
> In this version I was able to reuse the modesetting code from
> drm_fb_helper in the client API. This avoids code duplication, carries
> over lessons learned and the modesetting code is bisectable. The
> downside is that it takes +10 patches to rip drm_fb_helper in two, so
> maybe it's not worth it wrt possible breakage and a challenging review.

So my idea wasn't to rip the fbdev helper in  half first (that's indeed a
lot of work). But start out right away with using every piece of the
drm_client infrastructure you're adding in the existing fbdev code.

That way there's not a huge patch series which just adds code, with no
users, but every step of the way and every addition is tested almost right
away. That makes more gradual merging also easier. The things I have in
mind here is the generic fb_probe, or the drm_client block/unblock masters
and all that stuff.

Then, once we've demonstrated all these auxiliary pieces necessary for
drm_client.c work, we can cut the fb-helper in half and move the modeset
code into the drm_client library.

I still prefer an even more gradual path like this compared to what you
have in your patch series, but I understand that's yet another huge
shuffle. And the current series seems like a good enough approach to get
to essentially the same place.

> Does the Intel CI test the fbdev emulation?

We have fbdev emulation enabled, and iirc there's even a few tests for
fbdev. Just booting it on 20+ machines is a lot of testing itsefl already.

> 
> Daniel had this concern with the previous version:
> 
> The register/unregister model needs more thought. Allowing both clients
> to register whenever they want to, and drm_device instances to come and
> go is what fbcon has done, and the resulting locking is a horror show.
> 
> I think if we require that all in-kernel drm_clients are registers when
> loading drm.ko (and enabled/disabled only per module options and
> Kconfig), then we can throw out all the locking. That avoids a lot of
> the headaches.
> 
> I have solved this by adding a notifier that fires when a new DRM device
> is registered (I've removed the new() callback). Currently only
> bootsplash uses this. The fbdev client needs to be setup from the driver
> since it can't know on device registration if the driver will setup it's
> own fbdev emulation later and the vtcon client hooks up to a user
> provided device id.

Ugh, notifier is exactly what fbcon also uses. It just hides the locking
horror show slightly, but it's equally bad. I'm working on a multi-year
plan to rip out the fbcon notifier, please don't start another one. See

commit 6104c37094e729f3d4ce65797002112735d49cd1
Author: Daniel Vetter 
Date:   Tue Aug 1 17:32:07 2017 +0200

fbcon: Make fbcon a built-time depency for fbdev

for full details.

> Since fbcon can't handle fb_open failing, the buffer has to be
> pre-allocated. Exporting a GEM buffer pins the driver module making it
> impossible to unload it.
> I have included 2 solutions to the problem:
> - sysfs file to remove/close clients: remove_internal_clients

This is the same thing that defacto happens already with fbcon: You have
to remove fbcon first (which holds a full ref on the fbdev, which prevents
the drm driver from unloading). I think explicitly asking for that
reference to disappear is ok.

It does mean everyone has to update their unload scripts, but oh well.

> - Change drm_gem_prime_export() so it doesn't pin on client buffers

The double-loop in that patch definitely doesn't cut it, but worst case I
think something like that could be made to work.

> If a dumb buffer is exported from a kernel thread (worker) context, the
> file descriptor isn't closed and I leak a reference so the buffer isn't
> freed. Please look at drm_client_buffer_create() in patch
> 'drm/client: Finish the in-kernel client API'.
> This is a blocker that needs a solution.

Hm, missed that in my first cursory read of the series, I'l take another
look.
-Daniel

> 
> 
> Noralf.
> 
> Changes since version 3:
> Client API changes:
> - Drop drm_client_register_funcs() which attached clients indirectly.
>   Let clients attach directly using drm_client_new{_from_id}(). Clients
>   that wants to attach to all devices must be linked into drm.ko and use
>   the DRM device notifier. This is done to avoid the lock/race
>   register/unregister hell we have with fbcon. (Daniel Vetter)
> - drm_client_display_restore() checks if there is a master and if so
>   returns -EBUSY. (Daniel Vetter)
> - Allocate drm_file up front instead of on-demand. Since fbdev can't do
>   on demand buffer allocation because of fbcon, there's no need for this.
> - Add sysfs file to 

Re: [Intel-gfx] [RFC v4 00/25] drm: Add generic fbdev emulation

[Daniel Vetter]

On Thu, Apr 12, 2018 at 06:34:46PM +0200, Noralf Trønnes wrote:
> I hit a 'Sending rate exceeded' error with this patchset, so it didn't go
> out as it should.
> I will resend the patchset when I find out how to avoid this problem.

That's generally an issue with your ISP. gmail works ime for mail bombs,
even big ones.
-Daniel

> 
> Noralf.
> 
> 
> Den 12.04.2018 15.17, skrev Noralf Trønnes:
> > This patchset explores the possibility of having generic fbdev emulation
> > in DRM for drivers that supports dumb buffers which they can export. An
> > API is added to support in-kernel clients in general.
> > 
> > In this version I was able to reuse the modesetting code from
> > drm_fb_helper in the client API. This avoids code duplication, carries
> > over lessons learned and the modesetting code is bisectable. The
> > downside is that it takes +10 patches to rip drm_fb_helper in two, so
> > maybe it's not worth it wrt possible breakage and a challenging review.
> > 
> > Does the Intel CI test the fbdev emulation?
> > 
> > Daniel had this concern with the previous version:
> > 
> >  The register/unregister model needs more thought. Allowing both clients
> >  to register whenever they want to, and drm_device instances to come and
> >  go is what fbcon has done, and the resulting locking is a horror show.
> > 
> >  I think if we require that all in-kernel drm_clients are registers when
> >  loading drm.ko (and enabled/disabled only per module options and
> >  Kconfig), then we can throw out all the locking. That avoids a lot of
> >  the headaches.
> > 
> > I have solved this by adding a notifier that fires when a new DRM device
> > is registered (I've removed the new() callback). Currently only
> > bootsplash uses this. The fbdev client needs to be setup from the driver
> > since it can't know on device registration if the driver will setup it's
> > own fbdev emulation later and the vtcon client hooks up to a user
> > provided device id.
> > 
> > Since fbcon can't handle fb_open failing, the buffer has to be
> > pre-allocated. Exporting a GEM buffer pins the driver module making it
> > impossible to unload it.
> > I have included 2 solutions to the problem:
> > - sysfs file to remove/close clients: remove_internal_clients
> > - Change drm_gem_prime_export() so it doesn't pin on client buffers
> > 
> > If a dumb buffer is exported from a kernel thread (worker) context, the
> > file descriptor isn't closed and I leak a reference so the buffer isn't
> > freed. Please look at drm_client_buffer_create() in patch
> > 'drm/client: Finish the in-kernel client API'.
> > This is a blocker that needs a solution.
> > 
> > 
> > Noralf.
> > 
> > Changes since version 3:
> > Client API changes:
> > - Drop drm_client_register_funcs() which attached clients indirectly.
> >Let clients attach directly using drm_client_new{_from_id}(). Clients
> >that wants to attach to all devices must be linked into drm.ko and use
> >the DRM device notifier. This is done to avoid the lock/race
> >register/unregister hell we have with fbcon. (Daniel Vetter)
> > - drm_client_display_restore() checks if there is a master and if so
> >returns -EBUSY. (Daniel Vetter)
> > - Allocate drm_file up front instead of on-demand. Since fbdev can't do
> >on demand buffer allocation because of fbcon, there's no need for this.
> > - Add sysfs file to remove clients
> > - Don't pin driver module when exporting gem client buffers
> > - Dropped page flip support since drm_fb_helper is now used for fbdev
> >emulation.
> > 
> > - The bootsplash client now switches over to fbdev on keypress.
> > 
> > Changes since version 2:
> > - Don't set drm master for in-kernel clients. (Daniel Vetter)
> > - Add in-kernel client API
> > 
> > Changes since version 1:
> > - Don't add drm_fb_helper_fb_open() and drm_fb_helper_fb_release() to
> >DRM_FB_HELPER_DEFAULT_OPS(). (Fi.CI.STATIC)
> >The following uses that macro and sets fb_open/close: udlfb_ops,
> >amdgpufb_ops, drm_fb_helper_generic_fbdev_ops, nouveau_fbcon_ops,
> >nouveau_fbcon_sw_ops, radeonfb_ops.
> >This results in: warning: Initializer entry defined twice
> > - Support CONFIG_DRM_KMS_HELPER=m (kbuild test robot)
> >ERROR:  [drivers/gpu/drm/drm_kms_helper.ko] undefined!
> > - Drop buggy patch: (Chris Wilson)
> >drm/prime: Clear drm_gem_object->dma_buf on release
> > - Defer buffer creation until fb_open.
> > 
> > 
> > David Herrmann (1):
> >drm: provide management functions for drm_file
> > 
> > Noralf Trønnes (24):
> >drm/file: Don't set master on in-kernel clients
> >drm/fb-helper: No need to cache rotation and sw_rotations
> >drm/fb-helper: Remove drm_fb_helper_debug_enter/leave()
> >drm/fb-helper: dpms_legacy(): Only set on connectors in use
> >drm/atomic: Move __drm_atomic_helper_disable_plane/set_config()
> >drm: Begin an API for in-kernel clients
> >drm/fb-helper: Use struct drm_client_display
> >

Re: [Intel-gfx] [PATCH v4] drm/i915/gvt: Deliver guest cursor hotspot info

[Zhang, Tina]

> -Original Message-
> From: Zhenyu Wang [mailto:zhen...@linux.intel.com]
> Sent: Monday, April 16, 2018 1:52 PM
> To: Zhang, Tina 
> Cc: intel-gvt-...@lists.freedesktop.org; kra...@redhat.com; intel-
> g...@lists.freedesktop.org; Wang, Zhi A 
> Subject: Re: [PATCH v4] drm/i915/gvt: Deliver guest cursor hotspot info
> 
> On 2018.04.16 13:51:38 +0800, Tina Zhang wrote:
> > Guest OS driver uses PV info registers to deliver cursor hotspot info
> > to host. This patch is used to get cursor hotspot info from virtual
> > registers and deliver it to host userspace.
> >
> > v3->v4:
> > - return UINT_MAX when x_hot/y_hot is invalid. (Zhenyu)
> > - correct version.
> >
> > v2->v3:
> > - add validate_hotspot(). (Zhenyu)
> >
> > v1->v2:
> > - name as cursor_x_hot/cursor_y_hot. (Zhenyu)
> > - use i915_reg_t definition instead of magic numbers. (Zhenyu)
> >
> > Signed-off-by: Tina Zhang 
> > Cc: Zhenyu Wang 
> > Cc: Zhi Wang 
> > Cc: Gerd Hoffmann 
> > ---
> >  drivers/gpu/drm/i915/gvt/dmabuf.c | 22 --
> >  drivers/gpu/drm/i915/gvt/fb_decoder.c |  3 +++
> >  drivers/gpu/drm/i915/gvt/handlers.c   |  4 ++--
> >  drivers/gpu/drm/i915/gvt/vgpu.c   |  3 +++
> >  drivers/gpu/drm/i915/i915_pvinfo.h|  5 -
> >  5 files changed, 28 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gvt/dmabuf.c
> > b/drivers/gpu/drm/i915/gvt/dmabuf.c
> > index 6f4f8e9..a7c7082 100644
> > --- a/drivers/gpu/drm/i915/gvt/dmabuf.c
> > +++ b/drivers/gpu/drm/i915/gvt/dmabuf.c
> > @@ -192,6 +192,14 @@ static struct drm_i915_gem_object
> *vgpu_create_gem(struct drm_device *dev,
> > return obj;
> >  }
> >
> > +static bool validate_hotspot(struct intel_vgpu_cursor_plane_format
> > +*c) {
> > +   if (c && (c->x_hot <= c->width) && (c->y_hot <= c->height))
> > +   return true;
> 
> No way c could be NULL here.
See, we check c first.

BR,
Tina
> 
> > +   else
> > +   return false;
> > +}
> > +
> >  static int vgpu_get_plane_info(struct drm_device *dev,
> > struct intel_vgpu *vgpu,
> > struct intel_vgpu_fb_info *info,
> > @@ -229,12 +237,14 @@ static int vgpu_get_plane_info(struct drm_device
> *dev,
> > info->x_pos = c.x_pos;
> > info->y_pos = c.y_pos;
> >
> > -   /* The invalid cursor hotspot value is delivered to host
> > -* until we find a way to get the cursor hotspot info of
> > -* guest OS.
> > -*/
> > -   info->x_hot = UINT_MAX;
> > -   info->y_hot = UINT_MAX;
> > +   if (validate_hotspot()) {
> > +   info->x_hot = c.x_hot;
> > +   info->y_hot = c.y_hot;
> > +   } else {
> > +   info->x_hot = UINT_MAX;
> > +   info->y_hot = UINT_MAX;
> > +   }
> > +
> > info->size = (((info->stride * c.height * c.bpp) / 8)
> > + (PAGE_SIZE - 1)) >> PAGE_SHIFT;
> > } else {
> > diff --git a/drivers/gpu/drm/i915/gvt/fb_decoder.c
> > b/drivers/gpu/drm/i915/gvt/fb_decoder.c
> > index 1c12068..5e7468b 100644
> > --- a/drivers/gpu/drm/i915/gvt/fb_decoder.c
> > +++ b/drivers/gpu/drm/i915/gvt/fb_decoder.c
> > @@ -36,6 +36,7 @@
> >  #include 
> >  #include "i915_drv.h"
> >  #include "gvt.h"
> > +#include "i915_pvinfo.h"
> >
> >  #define PRIMARY_FORMAT_NUM 16
> >  struct pixel_format {
> > @@ -384,6 +385,8 @@ int intel_vgpu_decode_cursor_plane(struct
> intel_vgpu *vgpu,
> > plane->y_pos = (val & _CURSOR_POS_Y_MASK) >>
> _CURSOR_POS_Y_SHIFT;
> > plane->y_sign = (val & _CURSOR_SIGN_Y_MASK) >>
> _CURSOR_SIGN_Y_SHIFT;
> >
> > +   plane->x_hot = vgpu_vreg_t(vgpu, vgtif_reg(cursor_x_hot));
> > +   plane->y_hot = vgpu_vreg_t(vgpu, vgtif_reg(cursor_y_hot));
> > return 0;
> >  }
> >
> > diff --git a/drivers/gpu/drm/i915/gvt/handlers.c
> > b/drivers/gpu/drm/i915/gvt/handlers.c
> > index a33c1c3e..2c824a9 100644
> > --- a/drivers/gpu/drm/i915/gvt/handlers.c
> > +++ b/drivers/gpu/drm/i915/gvt/handlers.c
> > @@ -1201,8 +1201,8 @@ static int pvinfo_mmio_write(struct intel_vgpu
> *vgpu, unsigned int offset,
> > ret = handle_g2v_notification(vgpu, data);
> > break;
> > /* add xhot and yhot to handled list to avoid error log */
> > -   case 0x78830:
> > -   case 0x78834:
> > +   case _vgtif_reg(cursor_x_hot):
> > +   case _vgtif_reg(cursor_y_hot):
> > case _vgtif_reg(pdp[0].lo):
> > case _vgtif_reg(pdp[0].hi):
> > case _vgtif_reg(pdp[1].lo):
> > diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c
> > b/drivers/gpu/drm/i915/gvt/vgpu.c index 2e0a02a..bf75300 100644
> > --- a/drivers/gpu/drm/i915/gvt/vgpu.c
> > +++ b/drivers/gpu/drm/i915/gvt/vgpu.c
> > @@ -58,6 +58,9 @@ void populate_pvinfo_page(struct intel_vgpu *vgpu)
> >
> > vgpu_vreg_t(vgpu, vgtif_reg(avail_rs.fence_num)) =
> > 

[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/gvt: Deliver guest cursor hotspot info (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915/gvt: Deliver guest cursor hotspot info (rev2)
URL   : https://patchwork.freedesktop.org/series/41727/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
28bd5550e843 drm/i915/gvt: Deliver guest cursor hotspot info
-:36: CHECK:UNNECESSARY_PARENTHESES: Unnecessary parentheses around 'c->x_hot 
<= c->width'
#36: FILE: drivers/gpu/drm/i915/gvt/dmabuf.c:197:
+   if (c && (c->x_hot <= c->width) && (c->y_hot <= c->height))

-:36: CHECK:UNNECESSARY_PARENTHESES: Unnecessary parentheses around 'c->y_hot 
<= c->height'
#36: FILE: drivers/gpu/drm/i915/gvt/dmabuf.c:197:
+   if (c && (c->x_hot <= c->width) && (c->y_hot <= c->height))

total: 0 errors, 0 warnings, 2 checks, 79 lines checked

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

Re: [Intel-gfx] [PATCH v4] drm/i915/gvt: Deliver guest cursor hotspot info

[Zhenyu Wang]

On 2018.04.16 13:51:38 +0800, Tina Zhang wrote:
> Guest OS driver uses PV info registers to deliver cursor hotspot info
> to host. This patch is used to get cursor hotspot info from virtual
> registers and deliver it to host userspace.
> 
> v3->v4:
> - return UINT_MAX when x_hot/y_hot is invalid. (Zhenyu)
> - correct version.
> 
> v2->v3:
> - add validate_hotspot(). (Zhenyu)
> 
> v1->v2:
> - name as cursor_x_hot/cursor_y_hot. (Zhenyu)
> - use i915_reg_t definition instead of magic numbers. (Zhenyu)
> 
> Signed-off-by: Tina Zhang 
> Cc: Zhenyu Wang 
> Cc: Zhi Wang 
> Cc: Gerd Hoffmann 
> ---
>  drivers/gpu/drm/i915/gvt/dmabuf.c | 22 --
>  drivers/gpu/drm/i915/gvt/fb_decoder.c |  3 +++
>  drivers/gpu/drm/i915/gvt/handlers.c   |  4 ++--
>  drivers/gpu/drm/i915/gvt/vgpu.c   |  3 +++
>  drivers/gpu/drm/i915/i915_pvinfo.h|  5 -
>  5 files changed, 28 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/dmabuf.c 
> b/drivers/gpu/drm/i915/gvt/dmabuf.c
> index 6f4f8e9..a7c7082 100644
> --- a/drivers/gpu/drm/i915/gvt/dmabuf.c
> +++ b/drivers/gpu/drm/i915/gvt/dmabuf.c
> @@ -192,6 +192,14 @@ static struct drm_i915_gem_object 
> *vgpu_create_gem(struct drm_device *dev,
>   return obj;
>  }
>  
> +static bool validate_hotspot(struct intel_vgpu_cursor_plane_format *c)
> +{
> + if (c && (c->x_hot <= c->width) && (c->y_hot <= c->height))
> + return true;

No way c could be NULL here.

> + else
> + return false;
> +}
> +
>  static int vgpu_get_plane_info(struct drm_device *dev,
>   struct intel_vgpu *vgpu,
>   struct intel_vgpu_fb_info *info,
> @@ -229,12 +237,14 @@ static int vgpu_get_plane_info(struct drm_device *dev,
>   info->x_pos = c.x_pos;
>   info->y_pos = c.y_pos;
>  
> - /* The invalid cursor hotspot value is delivered to host
> -  * until we find a way to get the cursor hotspot info of
> -  * guest OS.
> -  */
> - info->x_hot = UINT_MAX;
> - info->y_hot = UINT_MAX;
> + if (validate_hotspot()) {
> + info->x_hot = c.x_hot;
> + info->y_hot = c.y_hot;
> + } else {
> + info->x_hot = UINT_MAX;
> + info->y_hot = UINT_MAX;
> + }
> +
>   info->size = (((info->stride * c.height * c.bpp) / 8)
>   + (PAGE_SIZE - 1)) >> PAGE_SHIFT;
>   } else {
> diff --git a/drivers/gpu/drm/i915/gvt/fb_decoder.c 
> b/drivers/gpu/drm/i915/gvt/fb_decoder.c
> index 1c12068..5e7468b 100644
> --- a/drivers/gpu/drm/i915/gvt/fb_decoder.c
> +++ b/drivers/gpu/drm/i915/gvt/fb_decoder.c
> @@ -36,6 +36,7 @@
>  #include 
>  #include "i915_drv.h"
>  #include "gvt.h"
> +#include "i915_pvinfo.h"
>  
>  #define PRIMARY_FORMAT_NUM   16
>  struct pixel_format {
> @@ -384,6 +385,8 @@ int intel_vgpu_decode_cursor_plane(struct intel_vgpu 
> *vgpu,
>   plane->y_pos = (val & _CURSOR_POS_Y_MASK) >> _CURSOR_POS_Y_SHIFT;
>   plane->y_sign = (val & _CURSOR_SIGN_Y_MASK) >> _CURSOR_SIGN_Y_SHIFT;
>  
> + plane->x_hot = vgpu_vreg_t(vgpu, vgtif_reg(cursor_x_hot));
> + plane->y_hot = vgpu_vreg_t(vgpu, vgtif_reg(cursor_y_hot));
>   return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/gvt/handlers.c 
> b/drivers/gpu/drm/i915/gvt/handlers.c
> index a33c1c3e..2c824a9 100644
> --- a/drivers/gpu/drm/i915/gvt/handlers.c
> +++ b/drivers/gpu/drm/i915/gvt/handlers.c
> @@ -1201,8 +1201,8 @@ static int pvinfo_mmio_write(struct intel_vgpu *vgpu, 
> unsigned int offset,
>   ret = handle_g2v_notification(vgpu, data);
>   break;
>   /* add xhot and yhot to handled list to avoid error log */
> - case 0x78830:
> - case 0x78834:
> + case _vgtif_reg(cursor_x_hot):
> + case _vgtif_reg(cursor_y_hot):
>   case _vgtif_reg(pdp[0].lo):
>   case _vgtif_reg(pdp[0].hi):
>   case _vgtif_reg(pdp[1].lo):
> diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c b/drivers/gpu/drm/i915/gvt/vgpu.c
> index 2e0a02a..bf75300 100644
> --- a/drivers/gpu/drm/i915/gvt/vgpu.c
> +++ b/drivers/gpu/drm/i915/gvt/vgpu.c
> @@ -58,6 +58,9 @@ void populate_pvinfo_page(struct intel_vgpu *vgpu)
>  
>   vgpu_vreg_t(vgpu, vgtif_reg(avail_rs.fence_num)) = vgpu_fence_sz(vgpu);
>  
> + vgpu_vreg_t(vgpu, vgtif_reg(cursor_x_hot)) = UINT_MAX;
> + vgpu_vreg_t(vgpu, vgtif_reg(cursor_y_hot)) = UINT_MAX;
> +
>   gvt_dbg_core("Populate PVINFO PAGE for vGPU %d\n", vgpu->id);
>   gvt_dbg_core("aperture base [GMADR] 0x%llx size 0x%llx\n",
>   vgpu_aperture_gmadr_base(vgpu), vgpu_aperture_sz(vgpu));
> diff --git a/drivers/gpu/drm/i915/i915_pvinfo.h 
> b/drivers/gpu/drm/i915/i915_pvinfo.h
> index 195203f..d61914a 100644
> --- a/drivers/gpu/drm/i915/i915_pvinfo.h
> +++