[Intel-gfx] [PATCH v3 0/2] Enable GVT suspend/resume

2020-09-08 Thread Colin Xu
This patchset enables GVT suspend/resume so that GVT enabled VM can
continue running after host resuming from suspend state.

V2:
- Change kzalloc/kfree to vzalloc/vfree since the space allocated
from kmalloc may not enough for all saved GGTT entries.
- Keep gvt suspend/resume wrapper in intel_gvt.h/intel_gvt.c and
move the actual implementation to gvt.h/gvt.c. (zhenyu)
- Check gvt config on and active with intel_gvt_active(). (zhenyu)

V3: (zhenyu)
- Incorrect copy length. Should be num entries * entry size.
- Use memcpy_toio()/memcpy_fromio() instead of memcpy for iomem.
- Add F_PM_SAVE flags to indicate which MMIOs to save/restore for PM.

Colin Xu (2):
  drm/i915/gvt: Save/restore HW status for GVT during suspend/resume
  drm/i915/gvt: Add GVT suspend/resume routine to i915

 drivers/gpu/drm/i915/gvt/gtt.c  | 75 +
 drivers/gpu/drm/i915/gvt/gtt.h  |  2 +
 drivers/gpu/drm/i915/gvt/gvt.c  | 15 ++
 drivers/gpu/drm/i915/gvt/gvt.h  |  8 +++
 drivers/gpu/drm/i915/gvt/handlers.c | 39 +--
 drivers/gpu/drm/i915/gvt/mmio.h |  3 ++
 drivers/gpu/drm/i915/gvt/vgpu.c |  1 +
 drivers/gpu/drm/i915/i915_drv.c |  4 ++
 drivers/gpu/drm/i915/intel_gvt.c| 29 +++
 drivers/gpu/drm/i915/intel_gvt.h| 10 
 10 files changed, 183 insertions(+), 3 deletions(-)

-- 
2.28.0

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


[Intel-gfx] [PATCH v3 1/2] drm/i915/gvt: Save/restore HW status for GVT during suspend/resume

2020-09-08 Thread Colin Xu
This patch save/restore necessary GVT info during i915 suspend/resume so
that GVT enabled QEMU VM can continue running.

Only GGTT and fence regs are saved/restored now. GVT will save GGTT
entries into GVT in suspend routine, and restore the saved entries
and re-init fence regs in resume routine.

V2:
- Change kzalloc/kfree to vzalloc/vfree since the space allocated
from kmalloc may not enough for all saved GGTT entries.
- Keep gvt suspend/resume wrapper in intel_gvt.h/intel_gvt.c and
move the actual implementation to gvt.h/gvt.c. (zhenyu)
- Check gvt config on and active with intel_gvt_active(). (zhenyu)

V3: (zhenyu)
- Incorrect copy length. Should be num entries * entry size.
- Use memcpy_toio()/memcpy_fromio() instead of memcpy for iomem.
- Add F_PM_SAVE flags to indicate which MMIOs to save/restore for PM.

Signed-off-by: Hang Yuan 
Signed-off-by: Colin Xu 
---
 drivers/gpu/drm/i915/gvt/gtt.c  | 75 +
 drivers/gpu/drm/i915/gvt/gtt.h  |  2 +
 drivers/gpu/drm/i915/gvt/gvt.c  | 15 ++
 drivers/gpu/drm/i915/gvt/gvt.h  |  8 +++
 drivers/gpu/drm/i915/gvt/handlers.c | 39 +--
 drivers/gpu/drm/i915/gvt/mmio.h |  3 ++
 drivers/gpu/drm/i915/gvt/vgpu.c |  1 +
 drivers/gpu/drm/i915/intel_gvt.c| 29 +++
 drivers/gpu/drm/i915/intel_gvt.h| 10 
 9 files changed, 179 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
index 04bf018ecc34..817095dd221b 100644
--- a/drivers/gpu/drm/i915/gvt/gtt.c
+++ b/drivers/gpu/drm/i915/gvt/gtt.c
@@ -2533,6 +2533,11 @@ static void intel_vgpu_destroy_ggtt_mm(struct intel_vgpu 
*vgpu)
}
intel_vgpu_destroy_mm(vgpu->gtt.ggtt_mm);
vgpu->gtt.ggtt_mm = NULL;
+
+   if (vgpu->ggtt_entries) {
+   vfree(vgpu->ggtt_entries);
+   vgpu->ggtt_entries = NULL;
+   }
 }
 
 /**
@@ -2834,3 +2839,73 @@ void intel_vgpu_reset_ggtt(struct intel_vgpu *vgpu, bool 
invalidate_old)
 
ggtt_invalidate(gvt->gt);
 }
+
+/**
+ * intel_gvt_save_ggtt - save all vGPU's ggtt entries
+ * @gvt: intel gvt device
+ *
+ * This function is called at driver suspend stage to save
+ * GGTT entries of every active vGPU.
+ *
+ */
+void intel_gvt_save_ggtt(struct intel_gvt *gvt)
+{
+   struct intel_vgpu *vgpu;
+   int id;
+   u32 index, num_low, num_hi;
+   void __iomem *addr;
+
+   for_each_active_vgpu(gvt, vgpu, id) {
+   num_low = vgpu_aperture_sz(vgpu) >> PAGE_SHIFT;
+   num_hi = vgpu_hidden_sz(vgpu) >> PAGE_SHIFT;
+   vgpu->ggtt_entries = vzalloc((num_low + num_hi) * sizeof(u64));
+   if (!vgpu->ggtt_entries)
+   continue;
+
+   index = vgpu_aperture_gmadr_base(vgpu) >> PAGE_SHIFT;
+   addr = (gen8_pte_t __iomem *)gvt->gt->i915->ggtt.gsm + index;
+   memcpy_fromio(vgpu->ggtt_entries, addr, num_low * sizeof(u64));
+
+   index = vgpu_hidden_gmadr_base(vgpu) >> PAGE_SHIFT;
+   addr = (gen8_pte_t __iomem *)gvt->gt->i915->ggtt.gsm + index;
+   memcpy_fromio(vgpu->ggtt_entries + num_low,
+ addr, num_hi * sizeof(u64));
+   }
+}
+
+/**
+ * intel_gvt_restore_ggtt - restore all vGPU's ggtt entries
+ * @gvt: intel gvt device
+ *
+ * This function is called at driver resume stage to restore
+ * GGTT entries of every active vGPU.
+ *
+ */
+void intel_gvt_restore_ggtt(struct intel_gvt *gvt)
+{
+   struct intel_vgpu *vgpu;
+   int id;
+   u32 index, num_low, num_hi;
+   void __iomem *addr;
+
+   for_each_active_vgpu(gvt, vgpu, id) {
+   if (!vgpu->ggtt_entries) {
+   gvt_vgpu_err("fail to get saved ggtt\n");
+   continue;
+   }
+
+   num_low = vgpu_aperture_sz(vgpu) >> PAGE_SHIFT;
+   num_hi = vgpu_hidden_sz(vgpu) >> PAGE_SHIFT;
+
+   index = vgpu_aperture_gmadr_base(vgpu) >> PAGE_SHIFT;
+   addr = (gen8_pte_t __iomem *)gvt->gt->i915->ggtt.gsm + index;
+   memcpy_toio(addr, vgpu->ggtt_entries, num_low * sizeof(u64));
+   index = vgpu_hidden_gmadr_base(vgpu) >> PAGE_SHIFT;
+   addr = (gen8_pte_t __iomem *)gvt->gt->i915->ggtt.gsm + index;
+   memcpy_toio(addr, vgpu->ggtt_entries + num_low,
+   num_hi * sizeof(u64));
+
+   vfree(vgpu->ggtt_entries);
+   vgpu->ggtt_entries = NULL;
+   }
+}
diff --git a/drivers/gpu/drm/i915/gvt/gtt.h b/drivers/gpu/drm/i915/gvt/gtt.h
index b76a262dd9bc..0d2fb2714852 100644
--- a/drivers/gpu/drm/i915/gvt/gtt.h
+++ b/drivers/gpu/drm/i915/gvt/gtt.h
@@ -279,5 +279,7 @@ int intel_vgpu_emulate_ggtt_mmio_write(struct intel_vgpu 
*vgpu,
unsigned int off, void *p_data, unsigned int bytes);
 
 void intel_vgpu_destroy_all_ppgtt_mm(struct intel_vgpu *vgpu);
+void intel_gvt_save_ggtt(struct intel_gvt *gvt

[Intel-gfx] [PATCH v3 2/2] drm/i915/gvt: Add GVT suspend/resume routine to i915

2020-09-08 Thread Colin Xu
This patch add gvt suspend/resume wrapper into i915: i915_drm_suspend()
and i915_drm_resume(). GVT relies on i915 so suspend gvt ahead of other
i915 sub-routine and resume gvt at last.

V2:
- Direct call into gvt suspend/resume wrapper in intel_gvt.h/intel_gvt.c.
The wrapper and implementation will check and call gvt routine. (zhenyu)

V3:
Refresh.

Signed-off-by: Hang Yuan 
Signed-off-by: Colin Xu 
---
 drivers/gpu/drm/i915/i915_drv.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index d66fe09d337e..28055dc65ecd 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1090,6 +1090,8 @@ static int i915_drm_suspend(struct drm_device *dev)
 
drm_kms_helper_poll_disable(dev);
 
+   intel_gvt_suspend(dev_priv);
+
pci_save_state(pdev);
 
intel_display_suspend(dev);
@@ -1267,6 +1269,8 @@ static int i915_drm_resume(struct drm_device *dev)
 
intel_power_domains_enable(dev_priv);
 
+   intel_gvt_resume(dev_priv);
+
enable_rpm_wakeref_asserts(&dev_priv->runtime_pm);
 
return 0;
-- 
2.28.0

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


Re: [Intel-gfx] linux-next: manual merge of the drm-intel tree with Linus' tree

2020-09-08 Thread Hans de Goede

Hi Stephen,

On 9/8/20 6:00 AM, Stephen Rothwell wrote:

Hi all,

Today's linux-next merge of the drm-intel tree got a conflict in:

   drivers/gpu/drm/i915/display/intel_panel.c

between commit:

   f8bd54d21904 ("drm/i915: panel: Use atomic PWM API for devs with an external PWM 
controller")

from Linus' tree and commit:

   6b51e7d23aa8 ("drm/i915: panel: Honor the VBT PWM frequency for devs with an 
external PWM controller")


That doesn't sound correct, those are both commits from the drm-intel tree.


from the drm-intel tree.

I fixed it up (I just used the latter)


Just taking the drivers/gpu/drm/i915/display/intel_panel.c contents of:

f8bd54d21904 ("drm/i915: panel: Use atomic PWM API for devs with an external PWM 
controller")

Is the right thing to do, the problem is a difference in a line which gets
removed in that commit.

Regards,

Hans

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


[Intel-gfx] ✗ Fi.CI.IGT: failure for series starting with [1/3] drm/atomic-helper: Extract drm_atomic_helper_calc_timestamping_constants()

2020-09-08 Thread Patchwork
== Series Details ==

Series: series starting with [1/3] drm/atomic-helper: Extract 
drm_atomic_helper_calc_timestamping_constants()
URL   : https://patchwork.freedesktop.org/series/81419/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_8971_full -> Patchwork_18448_full


Summary
---

  **FAILURE**

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

  

Possible new issues
---

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

### IGT changes ###

 Possible regressions 

  * igt@perf_pmu@rc6-runtime-pm-long:
- shard-hsw:  [PASS][1] -> [INCOMPLETE][2]
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8971/shard-hsw1/igt@perf_...@rc6-runtime-pm-long.html
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18448/shard-hsw7/igt@perf_...@rc6-runtime-pm-long.html

  
 Suppressed 

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * {igt@kms_writeback@writeback-invalid-parameters}:
- shard-tglb: NOTRUN -> [SKIP][3] +3 similar issues
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18448/shard-tglb2/igt@kms_writeb...@writeback-invalid-parameters.html

  
Known issues


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

### IGT changes ###

 Issues hit 

  * igt@gem_exec_reloc@basic-many-active@vecs0:
- shard-glk:  [PASS][4] -> [FAIL][5] ([i915#2389]) +1 similar issue
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8971/shard-glk1/igt@gem_exec_reloc@basic-many-act...@vecs0.html
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18448/shard-glk7/igt@gem_exec_reloc@basic-many-act...@vecs0.html

  * igt@gem_exec_whisper@basic-fds-priority-all:
- shard-glk:  [PASS][6] -> [DMESG-WARN][7] ([i915#118] / [i915#95])
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8971/shard-glk4/igt@gem_exec_whis...@basic-fds-priority-all.html
   [7]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18448/shard-glk6/igt@gem_exec_whis...@basic-fds-priority-all.html

  * igt@gem_userptr_blits@sync-unmap-cycles:
- shard-skl:  [PASS][8] -> [TIMEOUT][9] ([i915#1958] / [i915#2424])
   [8]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8971/shard-skl4/igt@gem_userptr_bl...@sync-unmap-cycles.html
   [9]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18448/shard-skl6/igt@gem_userptr_bl...@sync-unmap-cycles.html

  * 
igt@kms_atomic_transition@plane-all-transition-nonblocking-fencing@edp-1-pipe-a:
- shard-skl:  [PASS][10] -> [DMESG-WARN][11] ([i915#1982]) +3 
similar issues
   [10]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8971/shard-skl8/igt@kms_atomic_transition@plane-all-transition-nonblocking-fenc...@edp-1-pipe-a.html
   [11]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18448/shard-skl5/igt@kms_atomic_transition@plane-all-transition-nonblocking-fenc...@edp-1-pipe-a.html

  * igt@kms_cursor_crc@pipe-a-cursor-128x42-sliding:
- shard-glk:  [PASS][12] -> [FAIL][13] ([i915#54])
   [12]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8971/shard-glk1/igt@kms_cursor_...@pipe-a-cursor-128x42-sliding.html
   [13]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18448/shard-glk7/igt@kms_cursor_...@pipe-a-cursor-128x42-sliding.html

  * igt@kms_cursor_crc@pipe-b-cursor-suspend:
- shard-skl:  [PASS][14] -> [INCOMPLETE][15] ([i915#300])
   [14]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8971/shard-skl3/igt@kms_cursor_...@pipe-b-cursor-suspend.html
   [15]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18448/shard-skl7/igt@kms_cursor_...@pipe-b-cursor-suspend.html

  * igt@kms_flip@flip-vs-expired-vblank-interruptible@a-hdmi-a2:
- shard-glk:  [PASS][16] -> [FAIL][17] ([i915#79])
   [16]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8971/shard-glk1/igt@kms_flip@flip-vs-expired-vblank-interrupti...@a-hdmi-a2.html
   [17]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18448/shard-glk9/igt@kms_flip@flip-vs-expired-vblank-interrupti...@a-hdmi-a2.html

  * igt@kms_frontbuffer_tracking@fbc-badstride:
- shard-tglb: [PASS][18] -> [DMESG-WARN][19] ([i915#1982]) +1 
similar issue
   [18]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8971/shard-tglb3/igt@kms_frontbuffer_track...@fbc-badstride.html
   [19]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18448/shard-tglb6/igt@kms_frontbuffer_track...@fbc-badstride.html

  * igt@kms_psr@psr2_cursor_plane_move:
- shard-iclb: [PASS][20] -> [SK

Re: [Intel-gfx] [PATCH 4/5] drm_dp_cec: add plumbing in preparation for MST support

2020-09-08 Thread Hans Verkuil
On 01/09/2020 08:22, Sam McNally wrote:
> From: Hans Verkuil 
> 
> Signed-off-by: Hans Verkuil 
> [sa...@chromium.org:
>  - rebased
>  - removed polling-related changes
>  - moved the calls to drm_dp_cec_(un)set_edid() into the next patch
> ]
> Signed-off-by: Sam McNally 
> ---
> 
>  .../display/amdgpu_dm/amdgpu_dm_mst_types.c   |  2 +-
>  drivers/gpu/drm/drm_dp_cec.c  | 22 ++-
>  drivers/gpu/drm/i915/display/intel_dp.c   |  2 +-
>  drivers/gpu/drm/nouveau/nouveau_connector.c   |  2 +-
>  include/drm/drm_dp_helper.h   |  6 +++--
>  5 files changed, 19 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> index 461fa4da0a34..6e7075893ec9 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> @@ -419,7 +419,7 @@ void amdgpu_dm_initialize_dp_connector(struct 
> amdgpu_display_manager *dm,
>  
>   drm_dp_aux_init(&aconnector->dm_dp_aux.aux);
>   drm_dp_cec_register_connector(&aconnector->dm_dp_aux.aux,
> -   &aconnector->base);
> +   &aconnector->base, false);
>  
>   if (aconnector->base.connector_type == DRM_MODE_CONNECTOR_eDP)
>   return;
> diff --git a/drivers/gpu/drm/drm_dp_cec.c b/drivers/gpu/drm/drm_dp_cec.c
> index 3ab2609f9ec7..04ab7b88055c 100644
> --- a/drivers/gpu/drm/drm_dp_cec.c
> +++ b/drivers/gpu/drm/drm_dp_cec.c
> @@ -14,6 +14,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  /*
>   * Unfortunately it turns out that we have a chicken-and-egg situation
> @@ -338,8 +339,6 @@ void drm_dp_cec_set_edid(struct drm_dp_aux *aux, const 
> struct edid *edid)
>   if (aux->cec.adap) {
>   if (aux->cec.adap->capabilities == cec_caps &&
>   aux->cec.adap->available_log_addrs == num_las) {
> - /* Unchanged, so just set the phys addr */
> - cec_s_phys_addr_from_edid(aux->cec.adap, edid);
>   goto unlock;
>   }
>   /*
> @@ -364,15 +363,16 @@ void drm_dp_cec_set_edid(struct drm_dp_aux *aux, const 
> struct edid *edid)
>   if (cec_register_adapter(aux->cec.adap, connector->dev->dev)) {
>   cec_delete_adapter(aux->cec.adap);
>   aux->cec.adap = NULL;
> - } else {
> - /*
> -  * Update the phys addr for the new CEC adapter. When called
> -  * from drm_dp_cec_register_connector() edid == NULL, so in
> -  * that case the phys addr is just invalidated.
> -  */
> - cec_s_phys_addr_from_edid(aux->cec.adap, edid);
>   }
>  unlock:
> + /*
> +  * Update the phys addr for the new CEC adapter. When called
> +  * from drm_dp_cec_register_connector() edid == NULL, so in
> +  * that case the phys addr is just invalidated.
> +  */

The comment is no longer in sync with the code: if EDID == NULL, then
nothing is done due to the edid check in the 'if' below.

> + if (aux->cec.adap && edid) {

I think this should just be: if (aux->cec.adap)

Also, the {} aren't necessary here.

> + cec_s_phys_addr_from_edid(aux->cec.adap, edid);
> + }
>   mutex_unlock(&aux->cec.lock);
>  }
>  EXPORT_SYMBOL(drm_dp_cec_set_edid);

Frankly, the changes to this function should be dropped completely, from
what I can see they are not necessary. It was done in my original patch
because of the way I handled mst, but you did it differently (and I think
better), so these changes are no longer needed.

I know I am actually commenting on my old patch, but that patch was from a
work-in-progress git branch and was never meant as a 'proper' patch.

However, what complicates matters is that after digging a bit more I discovered
that commit 732300154980 ("drm: Do not call drm_dp_cec_set_edid() while 
registering
DP connectors") changed drm_dp_cec_register_connector() so that it no longer
calls drm_dp_cec_set_edid(), but the comments there and in this function were
not updated. It would be nice if you can add a patch fixing these outdated
comments.

Regardless of that change in commit 732300154980, the edid pointer can still be
NULL and the existing behavior should be kept (i.e. create a CEC device, but 
with
an invalid physical address since there is no EDID for some reason).

Regards,

Hans

> @@ -418,6 +418,7 @@ EXPORT_SYMBOL(drm_dp_cec_unset_edid);
>   * drm_dp_cec_register_connector() - register a new connector
>   * @aux: DisplayPort AUX channel
>   * @connector: drm connector
> + * @is_mst: set to true if this is an MST branch
>   *
>   * A new connector was registered with associated CEC adapter name and
>   * CEC adapter parent device. After registering the name and parent
> @@ -425,12 +426,13 @@ EXPORT_SYMBOL(drm_dp_cec_unset_

Re: [Intel-gfx] [PATCH V2 5/5] DO NOT MERGE: iommu: disable list appending in dma-iommu

2020-09-08 Thread Lu Baolu

On 2020/9/8 14:23, Christoph Hellwig wrote:

On Tue, Sep 08, 2020 at 02:04:53PM +0800, Lu Baolu wrote:

Do you mind telling where can I find Marek's series?


[PATCH v10 00/30] DRM: fix struct sg_table nents vs. orig_nents misuse

on various lists including the iommu one.



Get it. Thank you!

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


Re: [Intel-gfx] [PATCH] drm/i915/gvt: Introduce per object locking in GVT scheduler.

2020-09-08 Thread Zhenyu Wang
On 2020.09.07 23:02:03 +0300, Zhi Wang wrote:
> To support ww locking and per-object implemented in i915, GVT scheduler needs
> to be refined. Most of the changes are located in shadow batch buffer, shadow
> wa context in GVT-g, where use quite a lot of i915 gem object APIs.
> 
> Cc: Maarten Lankhorst 
> Cc: Joonas Lahtinen 
> Cc: Zhenyu Wang 
> Signed-off-by: Zhi Wang 
> ---
>  drivers/gpu/drm/i915/gvt/scheduler.c | 68 
> ++--
>  1 file changed, 57 insertions(+), 11 deletions(-)

Looks ok to me.

Reviewed-by: Zhenyu Wang 

Pls verify against latest gvt-staging then create pull against gt-next
for 5.10.

Thanks

> 
> diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c 
> b/drivers/gpu/drm/i915/gvt/scheduler.c
> index 1570eb8..fe7ee10 100644
> --- a/drivers/gpu/drm/i915/gvt/scheduler.c
> +++ b/drivers/gpu/drm/i915/gvt/scheduler.c
> @@ -396,7 +396,9 @@ static void release_shadow_wa_ctx(struct 
> intel_shadow_wa_ctx *wa_ctx)
>   if (!wa_ctx->indirect_ctx.obj)
>   return;
>  
> + i915_gem_object_lock(wa_ctx->indirect_ctx.obj, NULL);
>   i915_gem_object_unpin_map(wa_ctx->indirect_ctx.obj);
> + i915_gem_object_unlock(wa_ctx->indirect_ctx.obj);
>   i915_gem_object_put(wa_ctx->indirect_ctx.obj);
>  
>   wa_ctx->indirect_ctx.obj = NULL;
> @@ -504,6 +506,7 @@ static int prepare_shadow_batch_buffer(struct 
> intel_vgpu_workload *workload)
>   struct intel_gvt *gvt = workload->vgpu->gvt;
>   const int gmadr_bytes = gvt->device_info.gmadr_bytes_in_cmd;
>   struct intel_vgpu_shadow_bb *bb;
> + struct i915_gem_ww_ctx ww;
>   int ret;
>  
>   list_for_each_entry(bb, &workload->shadow_bb, list) {
> @@ -528,10 +531,19 @@ static int prepare_shadow_batch_buffer(struct 
> intel_vgpu_workload *workload)
>* directly
>*/
>   if (!bb->ppgtt) {
> - bb->vma = i915_gem_object_ggtt_pin(bb->obj,
> -NULL, 0, 0, 0);
> + i915_gem_ww_ctx_init(&ww, false);
> +retry:
> + i915_gem_object_lock(bb->obj, &ww);
> +
> + bb->vma = i915_gem_object_ggtt_pin_ww(bb->obj, &ww,
> +   NULL, 0, 0, 0);
>   if (IS_ERR(bb->vma)) {
>   ret = PTR_ERR(bb->vma);
> + if (ret == -EDEADLK) {
> + ret = i915_gem_ww_ctx_backoff(&ww);
> + if (!ret)
> + goto retry;
> + }
>   goto err;
>   }
>  
> @@ -545,13 +557,18 @@ static int prepare_shadow_batch_buffer(struct 
> intel_vgpu_workload *workload)
> 0);
>   if (ret)
>   goto err;
> +
> + /* No one is going to touch shadow bb from now on. */
> + i915_gem_object_flush_map(bb->obj);
> +
> + i915_gem_object_unlock(bb->obj);
> + i915_gem_ww_ctx_fini(&ww);
>   }
>  
> - /* No one is going to touch shadow bb from now on. */
> - i915_gem_object_flush_map(bb->obj);
>   }
>   return 0;
>  err:
> + i915_gem_ww_ctx_fini(&ww);
>   release_shadow_batch_buffer(workload);
>   return ret;
>  }
> @@ -578,14 +595,30 @@ static int prepare_shadow_wa_ctx(struct 
> intel_shadow_wa_ctx *wa_ctx)
>   unsigned char *per_ctx_va =
>   (unsigned char *)wa_ctx->indirect_ctx.shadow_va +
>   wa_ctx->indirect_ctx.size;
> + struct i915_gem_ww_ctx ww;
> + int ret;
>  
>   if (wa_ctx->indirect_ctx.size == 0)
>   return 0;
>  
> - vma = i915_gem_object_ggtt_pin(wa_ctx->indirect_ctx.obj, NULL,
> -0, CACHELINE_BYTES, 0);
> - if (IS_ERR(vma))
> - return PTR_ERR(vma);
> + i915_gem_ww_ctx_init(&ww, false);
> +retry:
> + i915_gem_object_lock(wa_ctx->indirect_ctx.obj, &ww);
> +
> + vma = i915_gem_object_ggtt_pin_ww(wa_ctx->indirect_ctx.obj, &ww, NULL,
> +   0, CACHELINE_BYTES, 0);
> + if (IS_ERR(vma)) {
> + ret = PTR_ERR(vma);
> + if (ret == -EDEADLK) {
> + ret = i915_gem_ww_ctx_backoff(&ww);
> + if (!ret)
> + goto retry;
> + }
> + return ret;
> + }
> +
> + i915_gem_object_unlock(wa_ctx->indirect_ctx.obj);
> + i915_gem_ww_ctx_fini(&ww);
>  
>   /* FIXME: we are not tracking our pinned VMA leaving it
>* up to the core to fix up the stray pin_count upon
> @@ -619,12 +652,14 @@ static void release_shadow_batch_buffer(struct 
> intel_vgpu_workload *workload)
>  

Re: [Intel-gfx] linux-next: manual merge of the drm-intel tree with Linus' tree

2020-09-08 Thread Stephen Rothwell
Hi Hans,

On Tue, 8 Sep 2020 10:22:06 +0200 Hans de Goede  wrote:
>
> On 9/8/20 6:00 AM, Stephen Rothwell wrote:
> > 
> > Today's linux-next merge of the drm-intel tree got a conflict in:
> > 
> >drivers/gpu/drm/i915/display/intel_panel.c
> > 
> > between commit:
> > 
> >f8bd54d21904 ("drm/i915: panel: Use atomic PWM API for devs with an 
> > external PWM controller")

This should have been

  899c537c25f9 ("drm/i915: Use 64-bit division macro")

> > 
> > from Linus' tree and commit:
> > 
> >6b51e7d23aa8 ("drm/i915: panel: Honor the VBT PWM frequency for devs 
> > with an external PWM controller")  
> 
> That doesn't sound correct, those are both commits from the drm-intel tree.
> 
> > from the drm-intel tree.
> > 
> > I fixed it up (I just used the latter)  
> 
> Just taking the drivers/gpu/drm/i915/display/intel_panel.c contents of:
> 
> f8bd54d21904 ("drm/i915: panel: Use atomic PWM API for devs with an external 
> PWM controller")
> 
> Is the right thing to do, the problem is a difference in a line which gets
> removed in that commit.

Which is what I actually did, I guess :-)

Sorry about that.
-- 
Cheers,
Stephen Rothwell


pgpLDwW1KT85B.pgp
Description: OpenPGP digital signature
___
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: Nuke dpio_phy_iosf_port[]

2020-09-08 Thread Patchwork
== Series Details ==

Series: drm/i915: Nuke dpio_phy_iosf_port[]
URL   : https://patchwork.freedesktop.org/series/81431/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_8973_full -> Patchwork_18449_full


Summary
---

  **SUCCESS**

  No regressions found.

  

Known issues


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

### IGT changes ###

 Issues hit 

  * igt@gem_eio@kms:
- shard-snb:  [PASS][1] -> [DMESG-WARN][2] ([i915#1982])
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8973/shard-snb2/igt@gem_...@kms.html
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18449/shard-snb4/igt@gem_...@kms.html

  * igt@gem_exec_reloc@basic-many-active@rcs0:
- shard-glk:  [PASS][3] -> [FAIL][4] ([i915#2389]) +1 similar issue
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8973/shard-glk5/igt@gem_exec_reloc@basic-many-act...@rcs0.html
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18449/shard-glk2/igt@gem_exec_reloc@basic-many-act...@rcs0.html

  * igt@i915_selftest@mock@contexts:
- shard-apl:  [PASS][5] -> [INCOMPLETE][6] ([i915#1635] / 
[i915#2278])
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8973/shard-apl4/igt@i915_selftest@m...@contexts.html
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18449/shard-apl1/igt@i915_selftest@m...@contexts.html

  * 
igt@kms_atomic_transition@plane-all-transition-nonblocking-fencing@edp-1-pipe-a:
- shard-skl:  [PASS][7] -> [DMESG-WARN][8] ([i915#1982]) +10 
similar issues
   [7]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8973/shard-skl4/igt@kms_atomic_transition@plane-all-transition-nonblocking-fenc...@edp-1-pipe-a.html
   [8]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18449/shard-skl9/igt@kms_atomic_transition@plane-all-transition-nonblocking-fenc...@edp-1-pipe-a.html

  * igt@kms_big_fb@linear-64bpp-rotate-0:
- shard-glk:  [PASS][9] -> [DMESG-WARN][10] ([i915#1982])
   [9]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8973/shard-glk6/igt@kms_big...@linear-64bpp-rotate-0.html
   [10]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18449/shard-glk6/igt@kms_big...@linear-64bpp-rotate-0.html

  * igt@kms_flip@2x-flip-vs-expired-vblank@bc-hdmi-a1-hdmi-a2:
- shard-glk:  [PASS][11] -> [FAIL][12] ([i915#79]) +1 similar issue
   [11]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8973/shard-glk1/igt@kms_flip@2x-flip-vs-expired-vbl...@bc-hdmi-a1-hdmi-a2.html
   [12]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18449/shard-glk1/igt@kms_flip@2x-flip-vs-expired-vbl...@bc-hdmi-a1-hdmi-a2.html

  * igt@kms_flip@flip-vs-expired-vblank@c-dp1:
- shard-kbl:  [PASS][13] -> [FAIL][14] ([i915#79])
   [13]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8973/shard-kbl7/igt@kms_flip@flip-vs-expired-vbl...@c-dp1.html
   [14]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18449/shard-kbl7/igt@kms_flip@flip-vs-expired-vbl...@c-dp1.html

  * igt@kms_flip@flip-vs-suspend-interruptible@c-edp1:
- shard-skl:  [PASS][15] -> [INCOMPLETE][16] ([i915#198])
   [15]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8973/shard-skl10/igt@kms_flip@flip-vs-suspend-interrupti...@c-edp1.html
   [16]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18449/shard-skl5/igt@kms_flip@flip-vs-suspend-interrupti...@c-edp1.html

  * igt@kms_flip@flip-vs-suspend@b-dp1:
- shard-kbl:  [PASS][17] -> [DMESG-WARN][18] ([i915#180])
   [17]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8973/shard-kbl1/igt@kms_flip@flip-vs-susp...@b-dp1.html
   [18]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18449/shard-kbl6/igt@kms_flip@flip-vs-susp...@b-dp1.html

  * igt@kms_flip@plain-flip-ts-check-interruptible@b-edp1:
- shard-skl:  [PASS][19] -> [FAIL][20] ([i915#2122])
   [19]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8973/shard-skl1/igt@kms_flip@plain-flip-ts-check-interrupti...@b-edp1.html
   [20]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18449/shard-skl2/igt@kms_flip@plain-flip-ts-check-interrupti...@b-edp1.html

  * igt@kms_frontbuffer_tracking@fbc-farfromfence:
- shard-kbl:  [PASS][21] -> [DMESG-WARN][22] ([i915#1982])
   [21]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8973/shard-kbl1/igt@kms_frontbuffer_track...@fbc-farfromfence.html
   [22]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18449/shard-kbl6/igt@kms_frontbuffer_track...@fbc-farfromfence.html

  * igt@kms_frontbuffer_tracking@psr-1p-offscren-pri-shrfb-draw-mmap-cpu:
- shard-tglb: [PASS][23] -> [DMESG-WARN][24] ([i915#1982]) +3 
similar issues
   [23]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8973/shard-tglb5/igt@kms_frontbuffer_track...@psr-1p-offscren-pri-shrfb-draw-mmap-cpu.html
   [24]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18449/s

Re: [Intel-gfx] [PATCH 3/3] drm/i915: Drop the drm_atomic_helper_calc_timestamping_constants() call

2020-09-08 Thread Ville Syrjälä
On Mon, Sep 07, 2020 at 08:14:38PM +0200, Daniel Vetter wrote:
> On Mon, Sep 07, 2020 at 03:00:26PM +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä 
> > 
> > We update the timestamping constants per-crtc explicitly in
> > intel_crtc_update_active_timings(). Furtermore the helper will
> > use uapi.adjusted_mode whereas we want hw.adjusted_mode. Thus
> > let's drop the helper call an rely on what we already have in
> > intel_crtc_update_active_timings(). We can now also drop the
> > hw.adjusted_mode -> uapi.adjusted_mode copy hack that was added
> > to keep the helper from deriving the timestamping constants from
> > the wrong thing.
> > 
> > Signed-off-by: Ville Syrjälä 
> 
> Does this fix some CI fail? I'd kinda expect/hope for that ...

Nah. Just trying to get rid of some of the confusing stuff before
we add even more confusing stuff for bigjoiner.

> 
> Anyway looks like a good idea to not mess with the uapi state like this.
> 
> Reviewed-by: Daniel Vetter 
> 
> > ---
> >  drivers/gpu/drm/i915/display/intel_display.c | 7 ---
> >  1 file changed, 7 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c 
> > b/drivers/gpu/drm/i915/display/intel_display.c
> > index 035840ce3825..a846f414c759 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -13472,12 +13472,6 @@ intel_modeset_pipe_config(struct intel_crtc_state 
> > *pipe_config)
> > "hw max bpp: %i, pipe bpp: %i, dithering: %i\n",
> > base_bpp, pipe_config->pipe_bpp, pipe_config->dither);
> >  
> > -   /*
> > -* Make drm_calc_timestamping_constants in
> > -* drm_atomic_helper_update_legacy_modeset_state() happy
> > -*/
> > -   pipe_config->uapi.adjusted_mode = pipe_config->hw.adjusted_mode;
> > -
> > return 0;
> >  }
> >  
> > @@ -15578,7 +15572,6 @@ static void intel_atomic_commit_tail(struct 
> > intel_atomic_state *state)
> >  
> > if (state->modeset) {
> > drm_atomic_helper_update_legacy_modeset_state(dev, 
> > &state->base);
> > -   drm_atomic_helper_calc_timestamping_constants(&state->base);
> >  
> > intel_set_cdclk_pre_plane_update(state);
> >  
> > -- 
> > 2.26.2
> > 
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Ville Syrjälä
Intel
___
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/gvt: Introduce per object locking in GVT scheduler.

2020-09-08 Thread Patchwork
== Series Details ==

Series: drm/i915/gvt: Introduce per object locking in GVT scheduler.
URL   : https://patchwork.freedesktop.org/series/81434/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_8973_full -> Patchwork_18450_full


Summary
---

  **SUCCESS**

  No regressions found.

  

Known issues


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

### IGT changes ###

 Issues hit 

  * igt@gem_ctx_isolation@preservation-s3@vcs0:
- shard-kbl:  [PASS][1] -> [DMESG-WARN][2] ([i915#180]) +1 similar 
issue
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8973/shard-kbl6/igt@gem_ctx_isolation@preservation...@vcs0.html
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18450/shard-kbl3/igt@gem_ctx_isolation@preservation...@vcs0.html

  * igt@gem_exec_reloc@basic-many-active@rcs0:
- shard-glk:  [PASS][3] -> [FAIL][4] ([i915#2389])
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8973/shard-glk5/igt@gem_exec_reloc@basic-many-act...@rcs0.html
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18450/shard-glk2/igt@gem_exec_reloc@basic-many-act...@rcs0.html

  * igt@gem_exec_whisper@basic-queues-forked-all:
- shard-glk:  [PASS][5] -> [DMESG-WARN][6] ([i915#118] / [i915#95]) 
+1 similar issue
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8973/shard-glk5/igt@gem_exec_whis...@basic-queues-forked-all.html
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18450/shard-glk2/igt@gem_exec_whis...@basic-queues-forked-all.html

  * igt@gem_huc_copy@huc-copy:
- shard-tglb: [PASS][7] -> [SKIP][8] ([i915#2190])
   [7]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8973/shard-tglb1/igt@gem_huc_c...@huc-copy.html
   [8]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18450/shard-tglb6/igt@gem_huc_c...@huc-copy.html

  * igt@gem_userptr_blits@sync-unmap-cycles:
- shard-skl:  [PASS][9] -> [TIMEOUT][10] ([i915#1958] / [i915#2424])
   [9]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8973/shard-skl1/igt@gem_userptr_bl...@sync-unmap-cycles.html
   [10]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18450/shard-skl4/igt@gem_userptr_bl...@sync-unmap-cycles.html

  * 
igt@kms_atomic_transition@plane-all-transition-nonblocking-fencing@edp-1-pipe-a:
- shard-skl:  [PASS][11] -> [DMESG-WARN][12] ([i915#1982]) +6 
similar issues
   [11]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8973/shard-skl4/igt@kms_atomic_transition@plane-all-transition-nonblocking-fenc...@edp-1-pipe-a.html
   [12]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18450/shard-skl2/igt@kms_atomic_transition@plane-all-transition-nonblocking-fenc...@edp-1-pipe-a.html

  * igt@kms_flip@flip-vs-suspend@a-edp1:
- shard-skl:  [PASS][13] -> [INCOMPLETE][14] ([i915#198])
   [13]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8973/shard-skl8/igt@kms_flip@flip-vs-susp...@a-edp1.html
   [14]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18450/shard-skl6/igt@kms_flip@flip-vs-susp...@a-edp1.html

  * igt@kms_flip@flip-vs-suspend@c-hdmi-a1:
- shard-hsw:  [PASS][15] -> [INCOMPLETE][16] ([i915#2055])
   [15]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8973/shard-hsw5/igt@kms_flip@flip-vs-susp...@c-hdmi-a1.html
   [16]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18450/shard-hsw1/igt@kms_flip@flip-vs-susp...@c-hdmi-a1.html

  * igt@kms_flip@plain-flip-fb-recreate-interruptible@c-edp1:
- shard-skl:  [PASS][17] -> [FAIL][18] ([i915#2122]) +1 similar 
issue
   [17]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8973/shard-skl7/igt@kms_flip@plain-flip-fb-recreate-interrupti...@c-edp1.html
   [18]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18450/shard-skl5/igt@kms_flip@plain-flip-fb-recreate-interrupti...@c-edp1.html

  * igt@kms_flip_tiling@flip-changes-tiling-y:
- shard-skl:  [PASS][19] -> [FAIL][20] ([i915#699])
   [19]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8973/shard-skl9/igt@kms_flip_til...@flip-changes-tiling-y.html
   [20]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18450/shard-skl7/igt@kms_flip_til...@flip-changes-tiling-y.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-shrfb-msflip-blt:
- shard-iclb: [PASS][21] -> [DMESG-WARN][22] ([i915#1982]) +1 
similar issue
   [21]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8973/shard-iclb3/igt@kms_frontbuffer_track...@fbc-1p-primscrn-shrfb-msflip-blt.html
   [22]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18450/shard-iclb3/igt@kms_frontbuffer_track...@fbc-1p-primscrn-shrfb-msflip-blt.html

  * igt@kms_frontbuffer_tracking@fbc-farfromfence:
- shard-kbl:  [PASS][23] -> [DMESG-WARN][24] ([i915#1982]) +1 
similar issue
   [23]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8973/shard-kbl1/igt@kms_frontbuffer_track...@fbc

[Intel-gfx] Patch "drm/i915: Fix sha_text population code" has been added to the 5.8-stable tree

2020-09-08 Thread gregkh


This is a note to let you know that I've just added the patch titled

drm/i915: Fix sha_text population code

to the 5.8-stable tree which can be found at:

http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
 drm-i915-fix-sha_text-population-code.patch
and it can be found in the queue-5.8 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let  know about it.


>From 9ab57658a608f879469ffa22b723c4539c05a58f Mon Sep 17 00:00:00 2001
From: Sean Paul 
Date: Tue, 18 Aug 2020 11:38:49 -0400
Subject: drm/i915: Fix sha_text population code

From: Sean Paul 

commit 9ab57658a608f879469ffa22b723c4539c05a58f upstream.

This patch fixes a few bugs:

1- We weren't taking into account sha_leftovers when adding multiple
   ksvs to sha_text. As such, we were or'ing the end of ksv[j - 1] with
   the beginning of ksv[j]

2- In the sha_leftovers == 2 and sha_leftovers == 3 case, bstatus was
   being placed on the wrong half of sha_text, overlapping the leftover
   ksv value

3- In the sha_leftovers == 2 case, we need to manually terminate the
   byte stream with 0x80 since the hardware doesn't have enough room to
   add it after writing M0

The upside is that all of the HDCP supported HDMI repeaters I could
find on Amazon just strip HDCP anyways, so it turns out to be _really_
hard to hit any of these cases without an MST hub, which is not (yet)
supported. Oh, and the sha_leftovers == 1 case works perfectly!

Fixes: ee5e5e7a5e0f ("drm/i915: Add HDCP framework + base implementation")
Cc: Chris Wilson 
Cc: Ramalingam C 
Cc: Daniel Vetter 
Cc: Sean Paul 
Cc: Jani Nikula 
Cc: Joonas Lahtinen 
Cc: Rodrigo Vivi 
Cc: intel-gfx@lists.freedesktop.org
Cc:  # v4.17+
Reviewed-by: Ramalingam C 
Signed-off-by: Sean Paul 
Signed-off-by: Ramalingam C 
Link: 
https://patchwork.freedesktop.org/patch/msgid/20200818153910.27894-2-s...@poorly.run
(cherry picked from commit 1f0882214fd0037b74f245d9be75c31516fed040)
Signed-off-by: Jani Nikula 
Signed-off-by: Greg Kroah-Hartman 

---
 drivers/gpu/drm/i915/display/intel_hdcp.c |   26 --
 include/drm/drm_hdcp.h|3 +++
 2 files changed, 23 insertions(+), 6 deletions(-)

--- a/drivers/gpu/drm/i915/display/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
@@ -336,8 +336,10 @@ int intel_hdcp_validate_v_prime(struct i
 
/* Fill up the empty slots in sha_text and write it out */
sha_empty = sizeof(sha_text) - sha_leftovers;
-   for (j = 0; j < sha_empty; j++)
-   sha_text |= ksv[j] << ((sizeof(sha_text) - j - 1) * 8);
+   for (j = 0; j < sha_empty; j++) {
+   u8 off = ((sizeof(sha_text) - j - 1 - sha_leftovers) * 
8);
+   sha_text |= ksv[j] << off;
+   }
 
ret = intel_write_sha_text(dev_priv, sha_text);
if (ret < 0)
@@ -435,7 +437,7 @@ int intel_hdcp_validate_v_prime(struct i
/* Write 32 bits of text */
intel_de_write(dev_priv, HDCP_REP_CTL,
   rep_ctl | HDCP_SHA1_TEXT_32);
-   sha_text |= bstatus[0] << 24 | bstatus[1] << 16;
+   sha_text |= bstatus[0] << 8 | bstatus[1];
ret = intel_write_sha_text(dev_priv, sha_text);
if (ret < 0)
return ret;
@@ -450,17 +452,29 @@ int intel_hdcp_validate_v_prime(struct i
return ret;
sha_idx += sizeof(sha_text);
}
+
+   /*
+* Terminate the SHA-1 stream by hand. For the other leftover
+* cases this is appended by the hardware.
+*/
+   intel_de_write(dev_priv, HDCP_REP_CTL,
+  rep_ctl | HDCP_SHA1_TEXT_32);
+   sha_text = DRM_HDCP_SHA1_TERMINATOR << 24;
+   ret = intel_write_sha_text(dev_priv, sha_text);
+   if (ret < 0)
+   return ret;
+   sha_idx += sizeof(sha_text);
} else if (sha_leftovers == 3) {
-   /* Write 32 bits of text */
+   /* Write 32 bits of text (filled from LSB) */
intel_de_write(dev_priv, HDCP_REP_CTL,
   rep_ctl | HDCP_SHA1_TEXT_32);
-   sha_text |= bstatus[0] << 24;
+   sha_text |= bstatus[0];
ret = intel_write_sha_text(dev_priv, sha_text);
if (ret < 0)
return ret;
sha_idx += sizeof(sha_text);
 
-   /* Write 8 bits of text, 24 bits of M0 */
+   /* Write 8 bits of text (filled from LSB), 24 bits of M0 */
intel_de_write(dev_priv, HDCP_REP_CTL,
   rep_ctl | HDCP_SHA1_TEXT_8);
ret = intel_write_sha_text(dev_

[Intel-gfx] ✗ Fi.CI.BUILD: failure for Patch "drm/i915: Fix sha_text population code" has been added to the 5.8-stable tree

2020-09-08 Thread Patchwork
== Series Details ==

Series: Patch "drm/i915: Fix sha_text population code" has been added to the 
5.8-stable tree
URL   : https://patchwork.freedesktop.org/series/81458/
State : failure

== Summary ==

Patch is empty.
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".


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


Re: [Intel-gfx] linux-next: manual merge of the drm-intel tree with Linus' tree

2020-09-08 Thread Hans de Goede

Hi,

On 9/8/20 1:04 PM, Stephen Rothwell wrote:

Hi Hans,

On Tue, 8 Sep 2020 10:22:06 +0200 Hans de Goede  wrote:


On 9/8/20 6:00 AM, Stephen Rothwell wrote:


Today's linux-next merge of the drm-intel tree got a conflict in:

drivers/gpu/drm/i915/display/intel_panel.c

between commit:

f8bd54d21904 ("drm/i915: panel: Use atomic PWM API for devs with an external PWM 
controller")


This should have been

   899c537c25f9 ("drm/i915: Use 64-bit division macro")


Yes that makes more sense.


from Linus' tree and commit:

6b51e7d23aa8 ("drm/i915: panel: Honor the VBT PWM frequency for devs with an 
external PWM controller")


That doesn't sound correct, those are both commits from the drm-intel tree.


from the drm-intel tree.

I fixed it up (I just used the latter)


Just taking the drivers/gpu/drm/i915/display/intel_panel.c contents of:

f8bd54d21904 ("drm/i915: panel: Use atomic PWM API for devs with an external PWM 
controller")

Is the right thing to do, the problem is a difference in a line which gets
removed in that commit.


Which is what I actually did, I guess :-)


Yes, looks good.


Sorry about that.


No problem and thank you for all the work you do on -next.

Regards,

Hans

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


[Intel-gfx] [PATCH 1/4] drm/i915: Kill unused savePCH_PORT_HOTPLUG

2020-09-08 Thread Ville Syrjala
From: Ville Syrjälä 

We don't save/restore PCH_PORT_HOTPLUG so no point in reseving
space for the value.

Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/i915_drv.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a455752221cc..2e4438e8e3eb 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -543,7 +543,6 @@ struct i915_suspend_saved_registers {
u32 saveSWF0[16];
u32 saveSWF1[16];
u32 saveSWF3[3];
-   u32 savePCH_PORT_HOTPLUG;
u16 saveGCDGMBUS;
 };
 
-- 
2.26.2

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


[Intel-gfx] [PATCH 2/4] drm/i915: Nuke the magic FBC_CONTROL save/restore

2020-09-08 Thread Ville Syrjala
From: Ville Syrjälä 

The FBC_CONTROL save restore is there just to preserve the
compression interval setting. Since commit a68ce21ba0c4
("drm/i915/fbc: Store the fbc1 compression interval in the params")
we've been explicitly setting the interval to a specific
value, so the sace/restore is now entirely pointless.

Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/i915_drv.h | 1 -
 drivers/gpu/drm/i915/i915_suspend.c | 8 
 2 files changed, 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 2e4438e8e3eb..3917bb1a6157 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -537,7 +537,6 @@ struct intel_gmbus {
 
 struct i915_suspend_saved_registers {
u32 saveDSPARB;
-   u32 saveFBC_CONTROL;
u32 saveCACHE_MODE_0;
u32 saveMI_ARB_STATE;
u32 saveSWF0[16];
diff --git a/drivers/gpu/drm/i915/i915_suspend.c 
b/drivers/gpu/drm/i915/i915_suspend.c
index ed2be3489f8e..592c230e6914 100644
--- a/drivers/gpu/drm/i915/i915_suspend.c
+++ b/drivers/gpu/drm/i915/i915_suspend.c
@@ -37,10 +37,6 @@ static void i915_save_display(struct drm_i915_private 
*dev_priv)
/* Display arbitration control */
if (INTEL_GEN(dev_priv) <= 4)
dev_priv->regfile.saveDSPARB = I915_READ(DSPARB);
-
-   /* save FBC interval */
-   if (HAS_FBC(dev_priv) && INTEL_GEN(dev_priv) <= 4 && !IS_G4X(dev_priv))
-   dev_priv->regfile.saveFBC_CONTROL = I915_READ(FBC_CONTROL);
 }
 
 static void i915_restore_display(struct drm_i915_private *dev_priv)
@@ -52,10 +48,6 @@ static void i915_restore_display(struct drm_i915_private 
*dev_priv)
/* only restore FBC info on the platform that supports FBC*/
intel_fbc_global_disable(dev_priv);
 
-   /* restore FBC interval */
-   if (HAS_FBC(dev_priv) && INTEL_GEN(dev_priv) <= 4 && !IS_G4X(dev_priv))
-   I915_WRITE(FBC_CONTROL, dev_priv->regfile.saveFBC_CONTROL);
-
intel_vga_redisable(dev_priv);
 }
 
-- 
2.26.2

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


[Intel-gfx] [PATCH 3/4] drm/i915: Nuke MI_ARB_STATE save/restore

2020-09-08 Thread Ville Syrjala
From: Ville Syrjälä 

Originally added in commit 1f84e550a870 ("drm/i915 more registers for
S3 (DSPCLK_GATE_D, CACHE_MODE_0, MI_ARB_STATE)") to fix some underruns.
I suspect that was due to the trickle feed settings getting clobbered
during suspend. We've been disabling trickle feed explicitly since
commit 20f949670f51 ("drm/i915: Disable trickle feed via MI_ARB_STATE
for the gen4") so this magic save/restore should no longer be needed.

Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/i915_drv.h | 1 -
 drivers/gpu/drm/i915/i915_suspend.c | 6 --
 2 files changed, 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 3917bb1a6157..cf51246b5402 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -538,7 +538,6 @@ struct intel_gmbus {
 struct i915_suspend_saved_registers {
u32 saveDSPARB;
u32 saveCACHE_MODE_0;
-   u32 saveMI_ARB_STATE;
u32 saveSWF0[16];
u32 saveSWF1[16];
u32 saveSWF3[3];
diff --git a/drivers/gpu/drm/i915/i915_suspend.c 
b/drivers/gpu/drm/i915/i915_suspend.c
index 592c230e6914..34c7d7bccec5 100644
--- a/drivers/gpu/drm/i915/i915_suspend.c
+++ b/drivers/gpu/drm/i915/i915_suspend.c
@@ -66,9 +66,6 @@ int i915_save_state(struct drm_i915_private *dev_priv)
if (INTEL_GEN(dev_priv) < 7)
dev_priv->regfile.saveCACHE_MODE_0 = I915_READ(CACHE_MODE_0);
 
-   /* Memory Arbitration state */
-   dev_priv->regfile.saveMI_ARB_STATE = I915_READ(MI_ARB_STATE);
-
/* Scratch space */
if (IS_GEN(dev_priv, 2) && IS_MOBILE(dev_priv)) {
for (i = 0; i < 7; i++) {
@@ -107,9 +104,6 @@ int i915_restore_state(struct drm_i915_private *dev_priv)
I915_WRITE(CACHE_MODE_0, dev_priv->regfile.saveCACHE_MODE_0 |
   0x);
 
-   /* Memory arbitration state */
-   I915_WRITE(MI_ARB_STATE, dev_priv->regfile.saveMI_ARB_STATE | 
0x);
-
/* Scratch space */
if (IS_GEN(dev_priv, 2) && IS_MOBILE(dev_priv)) {
for (i = 0; i < 7; i++) {
-- 
2.26.2

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


[Intel-gfx] [PATCH 4/4] drm/i915: Nuke CACHE_MODE_0 save/restore

2020-09-08 Thread Ville Syrjala
From: Ville Syrjälä 

The CACHE_MODE_0 save/restore was added without explanation in
commit 1f84e550a870 ("drm/i915 more registers for S3 (DSPCLK_GATE_D,
CACHE_MODE_0, MI_ARB_STATE)"). If there are any bits we care about
those should be set explicitly during some appropriate init function.
Let's assume it's all good and just nuke this magic save/restore.

Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/i915_drv.h | 1 -
 drivers/gpu/drm/i915/i915_suspend.c | 9 -
 2 files changed, 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index cf51246b5402..537c780af858 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -537,7 +537,6 @@ struct intel_gmbus {
 
 struct i915_suspend_saved_registers {
u32 saveDSPARB;
-   u32 saveCACHE_MODE_0;
u32 saveSWF0[16];
u32 saveSWF1[16];
u32 saveSWF3[3];
diff --git a/drivers/gpu/drm/i915/i915_suspend.c 
b/drivers/gpu/drm/i915/i915_suspend.c
index 34c7d7bccec5..932c7df64dd3 100644
--- a/drivers/gpu/drm/i915/i915_suspend.c
+++ b/drivers/gpu/drm/i915/i915_suspend.c
@@ -62,10 +62,6 @@ int i915_save_state(struct drm_i915_private *dev_priv)
pci_read_config_word(pdev, GCDGMBUS,
 &dev_priv->regfile.saveGCDGMBUS);
 
-   /* Cache mode state */
-   if (INTEL_GEN(dev_priv) < 7)
-   dev_priv->regfile.saveCACHE_MODE_0 = I915_READ(CACHE_MODE_0);
-
/* Scratch space */
if (IS_GEN(dev_priv, 2) && IS_MOBILE(dev_priv)) {
for (i = 0; i < 7; i++) {
@@ -99,11 +95,6 @@ int i915_restore_state(struct drm_i915_private *dev_priv)
  dev_priv->regfile.saveGCDGMBUS);
i915_restore_display(dev_priv);
 
-   /* Cache mode state */
-   if (INTEL_GEN(dev_priv) < 7)
-   I915_WRITE(CACHE_MODE_0, dev_priv->regfile.saveCACHE_MODE_0 |
-  0x);
-
/* Scratch space */
if (IS_GEN(dev_priv, 2) && IS_MOBILE(dev_priv)) {
for (i = 0; i < 7; i++) {
-- 
2.26.2

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


Re: [Intel-gfx] [PATCH] drm/managed: Cleanup of unused functions and polishing docs

2020-09-08 Thread Luben Tuikov
On 2020-09-03 10:26 a.m., Daniel Vetter wrote:
> On Wed, Sep 02, 2020 at 09:26:27AM +0200, Daniel Vetter wrote:
>> Following functions are only used internally, not by drivers:
>> - devm_drm_dev_init
>>
>> Also, now that we have a very slick and polished way to allocate a
>> drm_device with devm_drm_dev_alloc, update all the docs to reflect the
>> new reality. Mostly this consists of deleting old and misleading
>> hints. Two main ones:
>>
>> - it is no longer required that the drm_device base class is first in
>>   the structure. devm_drm_dev_alloc can cope with it being anywhere
>>
>> - obviously embedded now strongly recommends using devm_drm_dev_alloc
>>
>> v2: Fix typos (Noralf)
>>
>> v3: Split out the removal of drm_dev_init, that's blocked on some
>> discussions on how to convert vgem/vkms/i915-selftests. Adjust commit
>> message to reflect that.
>>
>> Cc: Noralf Trønnes 
>> Acked-by: Noralf Trønnes  (v2)
>> Acked-by: Sam Ravnborg 
>> Cc: Luben Tuikov 
>> Cc: amd-...@lists.freedesktop.org
>> Signed-off-by: Daniel Vetter 
> 
> Ok pushed that now to drm-misc-next. I'm also working on getting the
> remaining bits of the basic drm managed conversion resubmitted. That was
> unfortunately massively sidelined for the dma-fence discussions.
> 
> Quick heads-up:
> drmm_add_final_kfree and drm_dev_init will both disappear, please use
> devm_drm_dev_alloc.

Hi Daniel,

In drm_drv.c, in the "DOC: driver instance overview" section,
it would help a lot, if you could add/summarize/clarify,
succinctly, the two paths, device instantiation:

devm_drm_dev_alloc(); ...
drm_dev_init(); ...
drm_dev_register(); ...

And, device destruction, and where/how the "release"
method of drm_driver plays out.

The allocation part is mostly there, that's good,
but the release/destruction part is not. That is,
the platform driver callbacks are there, but not
the DRM driver part. In this patch, there is no
mention of drm_dev_init(), and the documentation
update of this patch doesn't mention it, while
it is being used by at least one driver.

If, on the other hand, you're thinking of removing
the "release" callback, a lucid explanation on
kref reaching 0--what should be done by DRM
and what should be done by drivers, would be very nice
and helpful to low-level DRM device driver maintainers/writers.

Also, consider renaming "drm_add_action()" to something
qualifying the action: either a bitmask as an argument,
or right in the name, "drm_add_action_release()", else
one begs the question "What action?"

It would be helpful, to describe the order of "release"
actions on kref reaching 0, and what is expected of
DRM and what of drivers, in the order of the expected
callbacks.

Regards,
Luben


> 
> Cheers, Daniel
> 
>> ---
>>  .../driver-api/driver-model/devres.rst|  2 +-
>>  drivers/gpu/drm/drm_drv.c | 78 +--
>>  drivers/gpu/drm/drm_managed.c |  2 +-
>>  include/drm/drm_device.h  |  2 +-
>>  include/drm/drm_drv.h | 16 ++--
>>  5 files changed, 30 insertions(+), 70 deletions(-)
>>
>> diff --git a/Documentation/driver-api/driver-model/devres.rst 
>> b/Documentation/driver-api/driver-model/devres.rst
>> index efc21134..aa4d2420f79e 100644
>> --- a/Documentation/driver-api/driver-model/devres.rst
>> +++ b/Documentation/driver-api/driver-model/devres.rst
>> @@ -263,7 +263,7 @@ DMA
>>dmam_pool_destroy()
>>  
>>  DRM
>> -  devm_drm_dev_init()
>> +  devm_drm_dev_alloc()
>>  
>>  GPIO
>>devm_gpiod_get()
>> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
>> index d4506f7a234e..7c1689842ec0 100644
>> --- a/drivers/gpu/drm/drm_drv.c
>> +++ b/drivers/gpu/drm/drm_drv.c
>> @@ -240,13 +240,13 @@ void drm_minor_release(struct drm_minor *minor)
>>   * DOC: driver instance overview
>>   *
>>   * A device instance for a drm driver is represented by &struct drm_device. 
>> This
>> - * is initialized with drm_dev_init(), usually from bus-specific ->probe()
>> - * callbacks implemented by the driver. The driver then needs to initialize 
>> all
>> - * the various subsystems for the drm device like memory management, vblank
>> - * handling, modesetting support and intial output configuration plus 
>> obviously
>> - * initialize all the corresponding hardware bits. Finally when everything 
>> is up
>> - * and running and ready for userspace the device instance can be published
>> - * using drm_dev_register().
>> + * is allocated and initialized with devm_drm_dev_alloc(), usually from
>> + * bus-specific ->probe() callbacks implemented by the driver. The driver 
>> then
>> + * needs to initialize all the various subsystems for the drm device like 
>> memory
>> + * management, vblank handling, modesetting support and initial output
>> + * configuration plus obviously initialize all the corresponding hardware 
>> bits.
>> + * Finally when everything is up and running and ready for userspace the 
>> device
>> + * i

Re: [Intel-gfx] [PATCH 0/8] Convert the intel iommu driver to the dma-iommu api

2020-09-08 Thread Tom Murphy
On Fri, 28 Aug 2020 at 00:34, Tom Murphy  wrote:
>
> On Thu, 27 Aug 2020 at 22:36, Logan Gunthorpe  wrote:
> >
> >
> >
> > On 2020-08-23 6:04 p.m., Tom Murphy wrote:
> > > I have added a check for the sg_dma_len == 0 :
> > > """
> > >  } __sgt_iter(struct scatterlist *sgl, bool dma) {
> > > struct sgt_iter s = { .sgp = sgl };
> > >
> > > +   if (sgl && sg_dma_len(sgl) == 0)
> > > +   s.sgp = NULL;
> > >
> > > if (s.sgp) {
> > > .
> > > """
> > > at location [1].
> > > but it doens't fix the problem.
> >
> > Based on my read of the code, it looks like we also need to change usage
> > of sgl->length... Something like the rough patch below, maybe?
> >
> > Also, Tom, do you have an updated version of the patchset to convert the
> > Intel IOMMU to dma-iommu available? The last one I've found doesn't
> > apply cleanly (I'm assuming parts of it have been merged in slightly
> > modified forms).
> >
>
> I'll try and post one in the next 24hours

I have just posted this now:
The subject of the cover letter is:
"[PATCH V2 0/5] Convert the intel iommu driver to the dma-iommu api"

>
> > Thanks,
> >
> > Logan
> >
> > --
> >
> > diff --git a/drivers/gpu/drm/i915/i915_scatterlist.h
> > b/drivers/gpu/drm/i915/i915
> > index b7b59328cb76..9367ac801f0c 100644
> > --- a/drivers/gpu/drm/i915/i915_scatterlist.h
> > +++ b/drivers/gpu/drm/i915/i915_scatterlist.h
> > @@ -27,13 +27,19 @@ static __always_inline struct sgt_iter {
> >  } __sgt_iter(struct scatterlist *sgl, bool dma) {
> > struct sgt_iter s = { .sgp = sgl };
> >
> > +   if (sgl && !sg_dma_len(s.sgp))
> > +   s.sgp = NULL;
> > +
> > if (s.sgp) {
> > s.max = s.curr = s.sgp->offset;
> > -   s.max += s.sgp->length;
> > -   if (dma)
> > +
> > +   if (dma) {
> > +   s.max += sg_dma_len(s.sgp);
> > s.dma = sg_dma_address(s.sgp);
> > -   else
> > +   } else {
> > +   s.max += s.sgp->length;
> > s.pfn = page_to_pfn(sg_page(s.sgp));
> > +   }
> > }
> >
> > return s;
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH V2 5/5] DO NOT MERGE: iommu: disable list appending in dma-iommu

2020-09-08 Thread Christoph Hellwig
On Thu, Sep 03, 2020 at 09:18:37PM +0100, Tom Murphy wrote:
> Disable combining sg segments in the dma-iommu api.
> Combining the sg segments exposes a bug in the intel i915 driver which
> causes visual artifacts and the screen to freeze. This is most likely
> because of how the i915 handles the returned list. It probably doesn't
> respect the returned value specifying the number of elements in the list
> and instead depends on the previous behaviour of the intel iommu driver
> which would return the same number of elements in the output list as in
> the input list.

So what is the state of addressing this properly in i915?  IF we can't
get it done ASAP I wonder if we need a runtime quirk to disable
merging instead of blocking this conversion..
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH V2 5/5] DO NOT MERGE: iommu: disable list appending in dma-iommu

2020-09-08 Thread Christoph Hellwig
On Tue, Sep 08, 2020 at 06:36:19AM +0100, Christoph Hellwig wrote:
> On Mon, Sep 07, 2020 at 09:18:50PM +0100, Tom Murphy wrote:
> > Yeah we talked about passing an attr to map_sg to disable merging at
> > the following microconfernce:
> > https://linuxplumbersconf.org/event/7/contributions/846/
> > As far as I can remember everyone seemed happy with that solution. I
> > won't be working on this though as I don't have any more time to
> > dedicate to this. It seems Lu Baolu will take over this.
> 
> I'm absolutely again passing a flag.  Tha just invites further
> abuse.  We need a PCI ID based quirk or something else that can't
> be as easily abused.

Also, I looked at i915 and there are just three dma_map_sg callers.
The two dmabuf related ones are fixed by Marek in his series, leaving
just the one in i915_gem_gtt_prepare_pages, which does indeed look
very fishy.  But if that one is so hard to fix it can just be replaced
by an open coded for_each_sg loop that contains manual dma_map_page
calls.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH V2 5/5] DO NOT MERGE: iommu: disable list appending in dma-iommu

2020-09-08 Thread Christoph Hellwig
On Mon, Sep 07, 2020 at 09:18:50PM +0100, Tom Murphy wrote:
> Yeah we talked about passing an attr to map_sg to disable merging at
> the following microconfernce:
> https://linuxplumbersconf.org/event/7/contributions/846/
> As far as I can remember everyone seemed happy with that solution. I
> won't be working on this though as I don't have any more time to
> dedicate to this. It seems Lu Baolu will take over this.

I'm absolutely again passing a flag.  Tha just invites further
abuse.  We need a PCI ID based quirk or something else that can't
be as easily abused.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH V2 5/5] DO NOT MERGE: iommu: disable list appending in dma-iommu

2020-09-08 Thread Tom Murphy
On Mon, 7 Sep 2020 at 08:00, Christoph Hellwig  wrote:
>
> On Thu, Sep 03, 2020 at 09:18:37PM +0100, Tom Murphy wrote:
> > Disable combining sg segments in the dma-iommu api.
> > Combining the sg segments exposes a bug in the intel i915 driver which
> > causes visual artifacts and the screen to freeze. This is most likely
> > because of how the i915 handles the returned list. It probably doesn't
> > respect the returned value specifying the number of elements in the list
> > and instead depends on the previous behaviour of the intel iommu driver
> > which would return the same number of elements in the output list as in
> > the input list.
>
> So what is the state of addressing this properly in i915?  IF we can't

I think this is the latest on addressing this issue:
https://patchwork.kernel.org/cover/11306999/

tl;dr: some people seem to be looking at it but I'm not sure if it's
being actively worked on

> get it done ASAP I wonder if we need a runtime quirk to disable
> merging instead of blocking this conversion..

Yeah we talked about passing an attr to map_sg to disable merging at
the following microconfernce:
https://linuxplumbersconf.org/event/7/contributions/846/
As far as I can remember everyone seemed happy with that solution. I
won't be working on this though as I don't have any more time to
dedicate to this. It seems Lu Baolu will take over this.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH] Fixed NULL pointer dereference in handle_edid functions for GPUs that do not support EDID

2020-09-08 Thread Alejandro Sior
In the function intel_vgpu_reg_rw_edid of gvt/kvmgt.c, pos can get equal to 
NULL for GPUs that do not
properly support EDID. In those cases, when pos gets passed to the handle_edid 
functions, it
gets added a short offset then it's dereferenced in memcpy's, leading to a 
kernel
oops: null pointer dereference.

More concretely, that kernel oops renders Broadwell GPUs users
completely unable to set up virtual machines with virtual GPU passthrough 
(virtual machines
hang indefinitely when trying to make use of the virtual GPU), and make
them have huge problems when trying to remove the virtual GPUs once the
kernel oops has happened (writing 1 in the vGPU remove file just makes
the operation hang undefinitely, again, and the kernel is unable to shutdown
since the vGPU removing hangs indefinitely). More information on the
issues that this causes are described in details in this github issue post: 
https://github.com/intel/gvt-linux/issues/170#issuecomment-685806160

This patch solves this problem by checking is pos is equal to NULL, and
if it is, it sets ret to a nagative value, making the module simply indicate 
that the access to EDID region has failed, without any fatal repercussion.

When this patch is applied, Broadwell GPU users do not suffer from that
kernel oops anymore, and thus do not encounter any of
the described problems and get able to set up virtual machines
with GPU passthrough without problems.
Users of GPUs with proper EDID support, are of course, still able to
benefit from the EDID features.

Signed-off-by: Alejandro W. Sior 

---
 drivers/gpu/drm/i915/gvt/kvmgt.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index ad8a9df49f29..49163363ba4a 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -557,7 +557,9 @@ static size_t intel_vgpu_reg_rw_edid(struct intel_vgpu 
*vgpu, char *buf,
(struct vfio_edid_region *)kvmgt_vdev(vgpu)->region[i].data;
loff_t pos = *ppos & VFIO_PCI_OFFSET_MASK;
 
-   if (pos < region->vfio_edid_regs.edid_offset) {
+   if (pos == NULL) {
+   ret = -EINVAL;
+   } else if (pos < region->vfio_edid_regs.edid_offset) {
ret = handle_edid_regs(vgpu, region, buf, count, pos, iswrite);
} else {
pos -= EDID_BLOB_OFFSET;
-- 
2.28.0

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


Re: [Intel-gfx] [PATCH v3 6/7] drm: Validate encoder->possible_crtcs

2020-09-08 Thread Jan Kiszka
On 11.02.20 18:04, Daniel Vetter wrote:
> On Tue, Feb 11, 2020 at 06:22:07PM +0200, Ville Syrjala wrote:
>> From: Ville Syrjälä 
>>
>> WARN if the encoder possible_crtcs is effectively empty or contains
>> bits for non-existing crtcs.
>>
>> v2: Move to drm_mode_config_validate() (Daniel)
>> Make the docs say we WARN when this is wrong (Daniel)
>> Extract full_crtc_mask()
>>
>> Cc: Thomas Zimmermann 
>> Cc: Daniel Vetter 
>> Signed-off-by: Ville Syrjälä 
> 
> When pushing the fixup needs to be applied before the validation patch
> here, because we don't want to anger the bisect gods.
> 
> Reviewed-by: Daniel Vetter 
> 
> I think with the fixup we should be good enough with the existing nonsense
> in drivers. Fingers crossed.
> -Daniel
> 
> 
>> ---
>>  drivers/gpu/drm/drm_mode_config.c | 27 ++-
>>  include/drm/drm_encoder.h |  2 +-
>>  2 files changed, 27 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_mode_config.c 
>> b/drivers/gpu/drm/drm_mode_config.c
>> index afc91447293a..4c1b350ddb95 100644
>> --- a/drivers/gpu/drm/drm_mode_config.c
>> +++ b/drivers/gpu/drm/drm_mode_config.c
>> @@ -581,6 +581,29 @@ static void validate_encoder_possible_clones(struct 
>> drm_encoder *encoder)
>>   encoder->possible_clones, encoder_mask);
>>  }
>>  
>> +static u32 full_crtc_mask(struct drm_device *dev)
>> +{
>> +struct drm_crtc *crtc;
>> +u32 crtc_mask = 0;
>> +
>> +drm_for_each_crtc(crtc, dev)
>> +crtc_mask |= drm_crtc_mask(crtc);
>> +
>> +return crtc_mask;
>> +}
>> +
>> +static void validate_encoder_possible_crtcs(struct drm_encoder *encoder)
>> +{
>> +u32 crtc_mask = full_crtc_mask(encoder->dev);
>> +
>> +WARN((encoder->possible_crtcs & crtc_mask) == 0 ||
>> + (encoder->possible_crtcs & ~crtc_mask) != 0,
>> + "Bogus possible_crtcs: "
>> + "[ENCODER:%d:%s] possible_crtcs=0x%x (full crtc mask=0x%x)\n",
>> + encoder->base.id, encoder->name,
>> + encoder->possible_crtcs, crtc_mask);
>> +}
>> +
>>  void drm_mode_config_validate(struct drm_device *dev)
>>  {
>>  struct drm_encoder *encoder;
>> @@ -588,6 +611,8 @@ void drm_mode_config_validate(struct drm_device *dev)
>>  drm_for_each_encoder(encoder, dev)
>>  fixup_encoder_possible_clones(encoder);
>>  
>> -drm_for_each_encoder(encoder, dev)
>> +drm_for_each_encoder(encoder, dev) {
>>  validate_encoder_possible_clones(encoder);
>> +validate_encoder_possible_crtcs(encoder);
>> +}
>>  }
>> diff --git a/include/drm/drm_encoder.h b/include/drm/drm_encoder.h
>> index 3741963b9587..b236269f41ac 100644
>> --- a/include/drm/drm_encoder.h
>> +++ b/include/drm/drm_encoder.h
>> @@ -142,7 +142,7 @@ struct drm_encoder {
>>   * the bits for all &drm_crtc objects this encoder can be connected to
>>   * before calling drm_dev_register().
>>   *
>> - * In reality almost every driver gets this wrong.
>> + * You will get a WARN if you get this wrong in the driver.
>>   *
>>   * Note that since CRTC objects can't be hotplugged the assigned indices
>>   * are stable and hence known before registering all objects.
>> -- 
>> 2.24.1
>>
> 

Triggers on an Advantech AIMB-228 (R1505G, 3 DP outputs):

[   14.033246] [ cut here ]
[   14.033248] Bogus possible_crtcs: [ENCODER:65:TMDS-65] possible_crtcs=0xf 
(full crtc mask=0x7)
[   14.033279] WARNING: CPU: 0 PID: 282 at 
../drivers/gpu/drm/drm_mode_config.c:622 drm_mode_config_validate+0x17d/0x200 
[drm]
[   14.033279] Modules linked in: amdgpu(E+) mfd_core(E) 
snd_hda_codec_realtek(E) kvm_amd(E) gpu_sched(E) i2c_algo_bit(E) ttm(E) 
snd_hda_codec_generic(E) kvm(E) ledtrig_audio(E) snd_hda_codec_hdmi(E) 
drm_kms_helper(E) snd_hda_intel(E) snd_intel_dspcfg(E) snd_hda_codec(E) cec(E) 
snd_hwdep(E) drm(E) irqbypass(E) snd_hda_core(E) crc32_pclmul(E) snd_pcm(E) 
ghash_clmulni_intel(E) snd_timer(E) sg(E) aesni_intel(E) snd(E) ccp(E) 
soundcore(E) rng_core(E) glue_helper(E) libaes(E) crypto_simd(E) k10temp(E) 
efi_pstore(E) sp5100_tco(E) evdev(E) cryptd(E) pcspkr(E) efivars(E) video(E) 
button(E) acpi_cpufreq(E) w83627hf_wdt(E) watchdog(E) nct6775(E) hwmon_vid(E) 
efivarfs(E) ip_tables(E) x_tables(E) autofs4(E) ext4(E) crc32c_generic(E) 
crc16(E) mbcache(E) jbd2(E) sd_mod(E) t10_pi(E) crc_t10dif(E) 
crct10dif_generic(E) uas(E) usb_storage(E) ahci(E) libahci(E) xhci_pci(E) 
r8169(E) realtek(E) mdio_devres(E) xhci_hcd(E) libata(E) i2c_amd_mp2_pci(E) 
crct10dif_pclmul(E) crct10dif_common(E) scsi_mod(E)
[   14.033306]  crc32c_intel(E) i2c_piix4(E) usbcore(E) libphy(E)
[   14.033310] CPU: 0 PID: 282 Comm: systemd-udevd Tainted: GE 
5.9.0-rc3+ #2
[   14.033311] Hardware name: Default string Default string/Default string, 
BIOS 5.0.1.4 02/14/2020
[   14.033324] RIP: 0010:drm_mode_config_validate+0x17d/0x200 [drm]
[   14.033327] Code: 42 f0 75 e6 41 85 f8 74 09 44 89 c0 f7 d0 85 f8 74 1a 49 
8b 54 24

Re: [Intel-gfx] [PATCH V2 5/5] DO NOT MERGE: iommu: disable list appending in dma-iommu

2020-09-08 Thread Christoph Hellwig
On Tue, Sep 08, 2020 at 02:04:53PM +0800, Lu Baolu wrote:
> Do you mind telling where can I find Marek's series?

[PATCH v10 00/30] DRM: fix struct sg_table nents vs. orig_nents misuse

on various lists including the iommu one.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [1/4] drm/i915: Kill unused savePCH_PORT_HOTPLUG

2020-09-08 Thread Patchwork
== Series Details ==

Series: series starting with [1/4] drm/i915: Kill unused savePCH_PORT_HOTPLUG
URL   : https://patchwork.freedesktop.org/series/81461/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_8982 -> Patchwork_18453


Summary
---

  **FAILURE**

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

  External URL: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18453/index.html

Possible new issues
---

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

### IGT changes ###

 Possible regressions 

  * igt@gem_exec_gttfill@basic:
- fi-kbl-7500u:   [PASS][1] -> [INCOMPLETE][2]
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-kbl-7500u/igt@gem_exec_gttf...@basic.html
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18453/fi-kbl-7500u/igt@gem_exec_gttf...@basic.html
- fi-byt-j1900:   [PASS][3] -> [INCOMPLETE][4]
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-byt-j1900/igt@gem_exec_gttf...@basic.html
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18453/fi-byt-j1900/igt@gem_exec_gttf...@basic.html

  * igt@gem_exec_parallel@engines@basic:
- fi-skl-guc: NOTRUN -> [INCOMPLETE][5]
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18453/fi-skl-guc/igt@gem_exec_parallel@engi...@basic.html
- fi-skl-6600u:   NOTRUN -> [INCOMPLETE][6]
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18453/fi-skl-6600u/igt@gem_exec_parallel@engi...@basic.html
- fi-bdw-5557u:   NOTRUN -> [INCOMPLETE][7]
   [7]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18453/fi-bdw-5557u/igt@gem_exec_parallel@engi...@basic.html

  * igt@i915_selftest@live@gem_execbuf:
- fi-bsw-nick:[PASS][8] -> [INCOMPLETE][9]
   [8]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-bsw-nick/igt@i915_selftest@live@gem_execbuf.html
   [9]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18453/fi-bsw-nick/igt@i915_selftest@live@gem_execbuf.html
- fi-bsw-n3050:   [PASS][10] -> [INCOMPLETE][11]
   [10]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-bsw-n3050/igt@i915_selftest@live@gem_execbuf.html
   [11]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18453/fi-bsw-n3050/igt@i915_selftest@live@gem_execbuf.html
- fi-ilk-650: NOTRUN -> [INCOMPLETE][12]
   [12]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18453/fi-ilk-650/igt@i915_selftest@live@gem_execbuf.html

  * igt@runner@aborted:
- fi-byt-j1900:   NOTRUN -> [FAIL][13]
   [13]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18453/fi-byt-j1900/igt@run...@aborted.html

  
Known issues


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

### IGT changes ###

 Issues hit 

  * igt@gem_exec_gttfill@basic:
- fi-apl-guc: [PASS][14] -> [INCOMPLETE][15] ([i915#1635])
   [14]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-apl-guc/igt@gem_exec_gttf...@basic.html
   [15]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18453/fi-apl-guc/igt@gem_exec_gttf...@basic.html

  * igt@i915_selftest@live@gem_execbuf:
- fi-snb-2600:[PASS][16] -> [INCOMPLETE][17] ([i915#82])
   [16]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-snb-2600/igt@i915_selftest@live@gem_execbuf.html
   [17]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18453/fi-snb-2600/igt@i915_selftest@live@gem_execbuf.html

  
 Possible fixes 

  * igt@gem_exec_gttfill@basic:
- fi-bdw-5557u:   [INCOMPLETE][18] -> [PASS][19]
   [18]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-bdw-5557u/igt@gem_exec_gttf...@basic.html
   [19]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18453/fi-bdw-5557u/igt@gem_exec_gttf...@basic.html
- fi-skl-guc: [INCOMPLETE][20] -> [PASS][21]
   [20]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-skl-guc/igt@gem_exec_gttf...@basic.html
   [21]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18453/fi-skl-guc/igt@gem_exec_gttf...@basic.html
- fi-skl-6600u:   [INCOMPLETE][22] -> [PASS][23]
   [22]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-skl-6600u/igt@gem_exec_gttf...@basic.html
   [23]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18453/fi-skl-6600u/igt@gem_exec_gttf...@basic.html
- fi-ilk-650: [INCOMPLETE][24] -> [PASS][25]
   [24]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-ilk-650/igt@gem_exec_gttf...@basic.html
   [25]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18453/fi-ilk-650/igt@gem_exec_gttf...@basic.html


[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Fixed NULL pointer dereference in handle_edid functions for GPUs that do not support EDID

2020-09-08 Thread Patchwork
== Series Details ==

Series: Fixed NULL pointer dereference in handle_edid functions for GPUs that 
do not support EDID
URL   : https://patchwork.freedesktop.org/series/81463/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
24f0f91b0b97 Fixed NULL pointer dereference in handle_edid functions for GPUs 
that do not support EDID
-:7: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description 
(prefer a maximum 75 chars per line)
#7: 
In the function intel_vgpu_reg_rw_edid of gvt/kvmgt.c, pos can get equal to 
NULL for GPUs that do not

-:42: CHECK:COMPARISON_TO_NULL: Comparison to NULL could be written "!pos"
#42: FILE: drivers/gpu/drm/i915/gvt/kvmgt.c:560:
+   if (pos == NULL) {

-:47: WARNING:NO_AUTHOR_SIGN_OFF: Missing Signed-off-by: line by nominal patch 
author 'Alejandro Sior '

total: 0 errors, 2 warnings, 1 checks, 10 lines checked


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


[Intel-gfx] ✗ Fi.CI.BAT: failure for Fixed NULL pointer dereference in handle_edid functions for GPUs that do not support EDID

2020-09-08 Thread Patchwork
== Series Details ==

Series: Fixed NULL pointer dereference in handle_edid functions for GPUs that 
do not support EDID
URL   : https://patchwork.freedesktop.org/series/81463/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_8982 -> Patchwork_18454


Summary
---

  **FAILURE**

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

  External URL: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18454/index.html

Possible new issues
---

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

### IGT changes ###

 Possible regressions 

  * igt@gem_exec_gttfill@basic:
- fi-kbl-7500u:   [PASS][1] -> [INCOMPLETE][2]
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-kbl-7500u/igt@gem_exec_gttf...@basic.html
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18454/fi-kbl-7500u/igt@gem_exec_gttf...@basic.html

  * igt@gem_exec_parallel@engines@basic:
- fi-skl-6600u:   NOTRUN -> [INCOMPLETE][3]
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18454/fi-skl-6600u/igt@gem_exec_parallel@engi...@basic.html

  * igt@gem_exec_parallel@engines@contexts:
- fi-byt-j1900:   [PASS][4] -> [INCOMPLETE][5]
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-byt-j1900/igt@gem_exec_parallel@engi...@contexts.html
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18454/fi-byt-j1900/igt@gem_exec_parallel@engi...@contexts.html

  * igt@i915_selftest@live@gem_execbuf:
- fi-bsw-nick:[PASS][6] -> [INCOMPLETE][7]
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-bsw-nick/igt@i915_selftest@live@gem_execbuf.html
   [7]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18454/fi-bsw-nick/igt@i915_selftest@live@gem_execbuf.html
- fi-ilk-650: NOTRUN -> [INCOMPLETE][8]
   [8]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18454/fi-ilk-650/igt@i915_selftest@live@gem_execbuf.html

  * igt@runner@aborted:
- fi-byt-j1900:   NOTRUN -> [FAIL][9]
   [9]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18454/fi-byt-j1900/igt@run...@aborted.html

  
 Warnings 

  * igt@runner@aborted:
- fi-bdw-5557u:   [FAIL][10] ([i915#483]) -> [FAIL][11]
   [10]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-bdw-5557u/igt@run...@aborted.html
   [11]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18454/fi-bdw-5557u/igt@run...@aborted.html

  
 Suppressed 

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@gem_exec_parallel@engines@contexts:
- {fi-ehl-1}: NOTRUN -> [INCOMPLETE][12]
   [12]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18454/fi-ehl-1/igt@gem_exec_parallel@engi...@contexts.html

  
Known issues


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

### IGT changes ###

 Issues hit 

  * igt@i915_selftest@live@gem_execbuf:
- fi-snb-2600:[PASS][13] -> [INCOMPLETE][14] ([i915#82])
   [13]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-snb-2600/igt@i915_selftest@live@gem_execbuf.html
   [14]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18454/fi-snb-2600/igt@i915_selftest@live@gem_execbuf.html

  
 Possible fixes 

  * igt@gem_exec_gttfill@basic:
- fi-skl-6600u:   [INCOMPLETE][15] -> [PASS][16]
   [15]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-skl-6600u/igt@gem_exec_gttf...@basic.html
   [16]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18454/fi-skl-6600u/igt@gem_exec_gttf...@basic.html
- fi-ilk-650: [INCOMPLETE][17] -> [PASS][18]
   [17]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-ilk-650/igt@gem_exec_gttf...@basic.html
   [18]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18454/fi-ilk-650/igt@gem_exec_gttf...@basic.html

  * igt@gem_exec_parallel@engines@basic:
- {fi-ehl-1}: [INCOMPLETE][19] -> [PASS][20]
   [19]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-ehl-1/igt@gem_exec_parallel@engi...@basic.html
   [20]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18454/fi-ehl-1/igt@gem_exec_parallel@engi...@basic.html

  * igt@i915_selftest@live@gem_execbuf:
- fi-gdg-551: [INCOMPLETE][21] ([i915#172]) -> [PASS][22]
   [21]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-gdg-551/igt@i915_selftest@live@gem_execbuf.html
   [22]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18454/fi-gdg-551/igt@i915_selftest@live@gem_execbuf.html
- fi-snb-2520m:   [INCOMPLETE][23] -> [PASS][24]
   [23]: 
https:

Re: [Intel-gfx] [PATCH 0/8] Convert the intel iommu driver to the dma-iommu api

2020-09-08 Thread Tvrtko Ursulin



Hi,

On 27/08/2020 22:36, Logan Gunthorpe wrote:

On 2020-08-23 6:04 p.m., Tom Murphy wrote:

I have added a check for the sg_dma_len == 0 :
"""
  } __sgt_iter(struct scatterlist *sgl, bool dma) {
 struct sgt_iter s = { .sgp = sgl };

+   if (sgl && sg_dma_len(sgl) == 0)
+   s.sgp = NULL;

 if (s.sgp) {
 .
"""
at location [1].
but it doens't fix the problem.


Based on my read of the code, it looks like we also need to change usage
of sgl->length... Something like the rough patch below, maybe?


This thread was brought to my attention and I initially missed this 
reply. Essentially I came to the same conclusion about the need to use 
sg_dma_len. One small correction below:



Also, Tom, do you have an updated version of the patchset to convert the
Intel IOMMU to dma-iommu available? The last one I've found doesn't
apply cleanly (I'm assuming parts of it have been merged in slightly
modified forms).

Thanks,

Logan

--

diff --git a/drivers/gpu/drm/i915/i915_scatterlist.h
b/drivers/gpu/drm/i915/i915
index b7b59328cb76..9367ac801f0c 100644
--- a/drivers/gpu/drm/i915/i915_scatterlist.h
+++ b/drivers/gpu/drm/i915/i915_scatterlist.h
@@ -27,13 +27,19 @@ static __always_inline struct sgt_iter {
  } __sgt_iter(struct scatterlist *sgl, bool dma) {
 struct sgt_iter s = { .sgp = sgl };

+   if (sgl && !sg_dma_len(s.sgp))


I'd extend the condition to be, just to be safe:

if (dma && sgl && !sg_dma_len(s.sgp))


+   s.sgp = NULL;
+
 if (s.sgp) {
 s.max = s.curr = s.sgp->offset;
-   s.max += s.sgp->length;
-   if (dma)
+
+   if (dma) {
+   s.max += sg_dma_len(s.sgp);
 s.dma = sg_dma_address(s.sgp);
-   else
+   } else {
+   s.max += s.sgp->length;
 s.pfn = page_to_pfn(sg_page(s.sgp));
+   }


Otherwise has this been tested or alternatively how to test it? (How to 
repro the issue.)


Regards,

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


[Intel-gfx] [PATCH 5.8 162/186] drm/i915: Fix sha_text population code

2020-09-08 Thread Greg Kroah-Hartman
From: Sean Paul 

commit 9ab57658a608f879469ffa22b723c4539c05a58f upstream.

This patch fixes a few bugs:

1- We weren't taking into account sha_leftovers when adding multiple
   ksvs to sha_text. As such, we were or'ing the end of ksv[j - 1] with
   the beginning of ksv[j]

2- In the sha_leftovers == 2 and sha_leftovers == 3 case, bstatus was
   being placed on the wrong half of sha_text, overlapping the leftover
   ksv value

3- In the sha_leftovers == 2 case, we need to manually terminate the
   byte stream with 0x80 since the hardware doesn't have enough room to
   add it after writing M0

The upside is that all of the HDCP supported HDMI repeaters I could
find on Amazon just strip HDCP anyways, so it turns out to be _really_
hard to hit any of these cases without an MST hub, which is not (yet)
supported. Oh, and the sha_leftovers == 1 case works perfectly!

Fixes: ee5e5e7a5e0f ("drm/i915: Add HDCP framework + base implementation")
Cc: Chris Wilson 
Cc: Ramalingam C 
Cc: Daniel Vetter 
Cc: Sean Paul 
Cc: Jani Nikula 
Cc: Joonas Lahtinen 
Cc: Rodrigo Vivi 
Cc: intel-gfx@lists.freedesktop.org
Cc:  # v4.17+
Reviewed-by: Ramalingam C 
Signed-off-by: Sean Paul 
Signed-off-by: Ramalingam C 
Link: 
https://patchwork.freedesktop.org/patch/msgid/20200818153910.27894-2-s...@poorly.run
(cherry picked from commit 1f0882214fd0037b74f245d9be75c31516fed040)
Signed-off-by: Jani Nikula 
Signed-off-by: Greg Kroah-Hartman 

---
 drivers/gpu/drm/i915/display/intel_hdcp.c |   26 --
 include/drm/drm_hdcp.h|3 +++
 2 files changed, 23 insertions(+), 6 deletions(-)

--- a/drivers/gpu/drm/i915/display/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
@@ -336,8 +336,10 @@ int intel_hdcp_validate_v_prime(struct i
 
/* Fill up the empty slots in sha_text and write it out */
sha_empty = sizeof(sha_text) - sha_leftovers;
-   for (j = 0; j < sha_empty; j++)
-   sha_text |= ksv[j] << ((sizeof(sha_text) - j - 1) * 8);
+   for (j = 0; j < sha_empty; j++) {
+   u8 off = ((sizeof(sha_text) - j - 1 - sha_leftovers) * 
8);
+   sha_text |= ksv[j] << off;
+   }
 
ret = intel_write_sha_text(dev_priv, sha_text);
if (ret < 0)
@@ -435,7 +437,7 @@ int intel_hdcp_validate_v_prime(struct i
/* Write 32 bits of text */
intel_de_write(dev_priv, HDCP_REP_CTL,
   rep_ctl | HDCP_SHA1_TEXT_32);
-   sha_text |= bstatus[0] << 24 | bstatus[1] << 16;
+   sha_text |= bstatus[0] << 8 | bstatus[1];
ret = intel_write_sha_text(dev_priv, sha_text);
if (ret < 0)
return ret;
@@ -450,17 +452,29 @@ int intel_hdcp_validate_v_prime(struct i
return ret;
sha_idx += sizeof(sha_text);
}
+
+   /*
+* Terminate the SHA-1 stream by hand. For the other leftover
+* cases this is appended by the hardware.
+*/
+   intel_de_write(dev_priv, HDCP_REP_CTL,
+  rep_ctl | HDCP_SHA1_TEXT_32);
+   sha_text = DRM_HDCP_SHA1_TERMINATOR << 24;
+   ret = intel_write_sha_text(dev_priv, sha_text);
+   if (ret < 0)
+   return ret;
+   sha_idx += sizeof(sha_text);
} else if (sha_leftovers == 3) {
-   /* Write 32 bits of text */
+   /* Write 32 bits of text (filled from LSB) */
intel_de_write(dev_priv, HDCP_REP_CTL,
   rep_ctl | HDCP_SHA1_TEXT_32);
-   sha_text |= bstatus[0] << 24;
+   sha_text |= bstatus[0];
ret = intel_write_sha_text(dev_priv, sha_text);
if (ret < 0)
return ret;
sha_idx += sizeof(sha_text);
 
-   /* Write 8 bits of text, 24 bits of M0 */
+   /* Write 8 bits of text (filled from LSB), 24 bits of M0 */
intel_de_write(dev_priv, HDCP_REP_CTL,
   rep_ctl | HDCP_SHA1_TEXT_8);
ret = intel_write_sha_text(dev_priv, bstatus[1]);
--- a/include/drm/drm_hdcp.h
+++ b/include/drm/drm_hdcp.h
@@ -29,6 +29,9 @@
 /* Slave address for the HDCP registers in the receiver */
 #define DRM_HDCP_DDC_ADDR  0x3A
 
+/* Value to use at the end of the SHA-1 bytestream used for repeaters */
+#define DRM_HDCP_SHA1_TERMINATOR   0x80
+
 /* HDCP register offsets for HDMI/DVI devices */
 #define DRM_HDCP_DDC_BKSV  0x00
 #define DRM_HDCP_DDC_RI_PRIME  0x08


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

Re: [Intel-gfx] [PATCH 0/8] Convert the intel iommu driver to the dma-iommu api

2020-09-08 Thread Logan Gunthorpe


On 2020-09-08 9:28 a.m., Tvrtko Ursulin wrote:
>>
>> diff --git a/drivers/gpu/drm/i915/i915_scatterlist.h
>> b/drivers/gpu/drm/i915/i915
>> index b7b59328cb76..9367ac801f0c 100644
>> --- a/drivers/gpu/drm/i915/i915_scatterlist.h
>> +++ b/drivers/gpu/drm/i915/i915_scatterlist.h
>> @@ -27,13 +27,19 @@ static __always_inline struct sgt_iter {
>>   } __sgt_iter(struct scatterlist *sgl, bool dma) {
>>  struct sgt_iter s = { .sgp = sgl };
>>
>> +   if (sgl && !sg_dma_len(s.sgp))
> 
> I'd extend the condition to be, just to be safe:
> if (dma && sgl && !sg_dma_len(s.sgp))
>

Right, good catch, that's definitely necessary.

>> +   s.sgp = NULL;
>> +
>>  if (s.sgp) {
>>  s.max = s.curr = s.sgp->offset;
>> -   s.max += s.sgp->length;
>> -   if (dma)
>> +
>> +   if (dma) {
>> +   s.max += sg_dma_len(s.sgp);
>>  s.dma = sg_dma_address(s.sgp);
>> -   else
>> +   } else {
>> +   s.max += s.sgp->length;
>>  s.pfn = page_to_pfn(sg_page(s.sgp));
>> +   }
> 
> Otherwise has this been tested or alternatively how to test it? (How to
> repro the issue.)

It has not been tested. To test it, you need Tom's patch set without the
last "DO NOT MERGE" patch:

https://lkml.kernel.org/lkml/20200907070035.ga25...@infradead.org/T/

Thanks,

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


Re: [Intel-gfx] [PATCH 0/8] Convert the intel iommu driver to the dma-iommu api

2020-09-08 Thread Tvrtko Ursulin


On 08/09/2020 16:44, Logan Gunthorpe wrote:

On 2020-09-08 9:28 a.m., Tvrtko Ursulin wrote:


diff --git a/drivers/gpu/drm/i915/i915_scatterlist.h
b/drivers/gpu/drm/i915/i915
index b7b59328cb76..9367ac801f0c 100644
--- a/drivers/gpu/drm/i915/i915_scatterlist.h
+++ b/drivers/gpu/drm/i915/i915_scatterlist.h
@@ -27,13 +27,19 @@ static __always_inline struct sgt_iter {
   } __sgt_iter(struct scatterlist *sgl, bool dma) {
  struct sgt_iter s = { .sgp = sgl };

+   if (sgl && !sg_dma_len(s.sgp))


I'd extend the condition to be, just to be safe:
 if (dma && sgl && !sg_dma_len(s.sgp))



Right, good catch, that's definitely necessary.


+   s.sgp = NULL;
+
  if (s.sgp) {
  s.max = s.curr = s.sgp->offset;
-   s.max += s.sgp->length;
-   if (dma)
+
+   if (dma) {
+   s.max += sg_dma_len(s.sgp);
  s.dma = sg_dma_address(s.sgp);
-   else
+   } else {
+   s.max += s.sgp->length;
  s.pfn = page_to_pfn(sg_page(s.sgp));
+   }


Otherwise has this been tested or alternatively how to test it? (How to
repro the issue.)


It has not been tested. To test it, you need Tom's patch set without the
last "DO NOT MERGE" patch:

https://lkml.kernel.org/lkml/20200907070035.ga25...@infradead.org/T/


Tom, do you have a branch somewhere I could pull from? (Just being lazy 
about downloading a bunch of messages from the archives.)


What GPU is in your Lenovo x1 carbon 5th generation and what 
graphical/desktop setup I need to repro?


Regards,

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


Re: [Intel-gfx] [PATCH] drm/i915/gvt: Fix NULL pointer dereference in intel_vgpu_reg_rw_edid()

2020-09-08 Thread Markus Elfring
> In the function intel_vgpu_reg_rw_edid of gvt/kvmgt.c, pos can get equal to 
> NULL for GPUs that do not
> properly support EDID. …

* I propose to reconsider the previous patch subject once more.

* Did the script “checkpatch.pl” point any adjustment possibilities out
  for the change description?


> This patch solves this problem by …

Would an imperative wording be preferred for the commit message?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst#n151


> … to a nagative value, …

Please avoid a typo here.


I suggest to add the tag “Fixes” to the commit message.

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


Re: [Intel-gfx] [01/12] drm/i915: Add more AUX CHs to the enum

2020-09-08 Thread Souza, Jose
On Wed, 2020-07-01 at 00:55 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <
> ville.syrj...@linux.intel.com
> >
> 
> We need to go up to AUX_CH_I (aka. AUX CH USBC6) these days.
> 

Reviewed-by: José Roberto de Souza 

> Signed-off-by: Ville Syrjälä <
> ville.syrj...@linux.intel.com
> >
> ---
>  drivers/gpu/drm/i915/display/intel_display.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.h 
> b/drivers/gpu/drm/i915/display/intel_display.h
> index f68007ff8a13..5b736883cd11 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.h
> +++ b/drivers/gpu/drm/i915/display/intel_display.h
> @@ -282,6 +282,8 @@ enum aux_ch {
>   AUX_CH_E, /* ICL+ */
>   AUX_CH_F,
>   AUX_CH_G,
> + AUX_CH_H,
> + AUX_CH_I,
>  };
>  
>  #define aux_ch_name(a) ((a) + 'A')
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [02/12] drm/i915: Add PORT_{H, I} to intel_port_to_power_domain()

2020-09-08 Thread Souza, Jose
On Wed, 2020-07-01 at 00:55 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <
> ville.syrj...@linux.intel.com
> >
> 
> We need to go up to PORT_I (aka. TC6) these days.
> 

Reviewed-by: José Roberto de Souza 

> Signed-off-by: Ville Syrjälä <
> ville.syrj...@linux.intel.com
> >
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c 
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 182cef0dc2fd..665aa4283fb9 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -7289,6 +7289,10 @@ enum intel_display_power_domain 
> intel_port_to_power_domain(enum port port)
>   return POWER_DOMAIN_PORT_DDI_F_LANES;
>   case PORT_G:
>   return POWER_DOMAIN_PORT_DDI_G_LANES;
> + case PORT_H:
> + return POWER_DOMAIN_PORT_DDI_H_LANES;
> + case PORT_I:
> + return POWER_DOMAIN_PORT_DDI_I_LANES;
>   default:
>   MISSING_CASE(port);
>   return POWER_DOMAIN_PORT_OTHER;
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [03/12] drm/i915: Add AUX_CH_{H, I} power domain handling

2020-09-08 Thread Souza, Jose
On Wed, 2020-07-01 at 00:55 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <
> ville.syrj...@linux.intel.com
> >
> 
> AUX CH H/I need their power domains too.

Reviewed-by: José Roberto de Souza 

> 
> Signed-off-by: Ville Syrjälä <
> ville.syrj...@linux.intel.com
> >
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c 
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 665aa4283fb9..87831fd9e1e1 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -7318,6 +7318,10 @@ intel_aux_power_domain(struct intel_digital_port 
> *dig_port)
>   return POWER_DOMAIN_AUX_F_TBT;
>   case AUX_CH_G:
>   return POWER_DOMAIN_AUX_G_TBT;
> + case AUX_CH_H:
> + return POWER_DOMAIN_AUX_H_TBT;
> + case AUX_CH_I:
> + return POWER_DOMAIN_AUX_I_TBT;
>   default:
>   MISSING_CASE(dig_port->aux_ch);
>   return POWER_DOMAIN_AUX_C_TBT;
> @@ -7349,6 +7353,10 @@ intel_legacy_aux_to_power_domain(enum aux_ch aux_ch)
>   return POWER_DOMAIN_AUX_F;
>   case AUX_CH_G:
>   return POWER_DOMAIN_AUX_G;
> + case AUX_CH_H:
> + return POWER_DOMAIN_AUX_H;
> + case AUX_CH_I:
> + return POWER_DOMAIN_AUX_I;
>   default:
>   MISSING_CASE(aux_ch);
>   return POWER_DOMAIN_AUX_A;
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [04/12] drm/i915: Add VBT DVO ports H and I

2020-09-08 Thread Souza, Jose
On Wed, 2020-07-01 at 00:55 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <
> ville.syrj...@linux.intel.com
> >
> 
> VBT has ports H and I since version 217.
> 

Reviewed-by: José Roberto de Souza 

> Signed-off-by: Ville Syrjälä <
> ville.syrj...@linux.intel.com
> >
> ---
>  drivers/gpu/drm/i915/display/intel_bios.c | 2 ++
>  drivers/gpu/drm/i915/display/intel_vbt_defs.h | 8 ++--
>  2 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c 
> b/drivers/gpu/drm/i915/display/intel_bios.c
> index 6593e2c38043..2bf0bc0deee8 100644
> --- a/drivers/gpu/drm/i915/display/intel_bios.c
> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
> @@ -1653,6 +1653,8 @@ static enum port dvo_port_to_port(struct 
> drm_i915_private *dev_priv,
>   [PORT_E] = { DVO_PORT_HDMIE, DVO_PORT_DPE, DVO_PORT_CRT },
>   [PORT_F] = { DVO_PORT_HDMIF, DVO_PORT_DPF, -1 },
>   [PORT_G] = { DVO_PORT_HDMIG, DVO_PORT_DPG, -1 },
> + [PORT_H] = { DVO_PORT_HDMIH, DVO_PORT_DPH, -1 },
> + [PORT_I] = { DVO_PORT_HDMII, DVO_PORT_DPI, -1 },
>   };
>   /*
>* Bspec lists the ports as A, B, C, D - however internally in our
> diff --git a/drivers/gpu/drm/i915/display/intel_vbt_defs.h 
> b/drivers/gpu/drm/i915/display/intel_vbt_defs.h
> index aef7fe932d1a..e502d65300fa 100644
> --- a/drivers/gpu/drm/i915/display/intel_vbt_defs.h
> +++ b/drivers/gpu/drm/i915/display/intel_vbt_defs.h
> @@ -293,8 +293,12 @@ struct bdb_general_features {
>  #define DVO_PORT_HDMIE   12  /* 193 
> */
>  #define DVO_PORT_DPF 13  /* N/A */
>  #define DVO_PORT_HDMIF   14  /* N/A 
> */
> -#define DVO_PORT_DPG 15
> -#define DVO_PORT_HDMIG   16
> +#define DVO_PORT_DPG 15  /* 217 */
> +#define DVO_PORT_HDMIG   16  /* 217 
> */
> +#define DVO_PORT_DPH 17  /* 217 */
> +#define DVO_PORT_HDMIH   18  /* 217 
> */
> +#define DVO_PORT_DPI 19  /* 217 */
> +#define DVO_PORT_HDMII   20  /* 217 
> */
>  #define DVO_PORT_MIPIA   21  /* 171 
> */
>  #define DVO_PORT_MIPIB   22  /* 171 
> */
>  #define DVO_PORT_MIPIC   23  /* 171 
> */
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [05/12] drm/i915: Add VBT AUX CH H and I

2020-09-08 Thread Souza, Jose
On Wed, 2020-07-01 at 00:55 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <
> ville.syrj...@linux.intel.com
> >
> 
> As with everything else VBT can now specify AUX CH H or I.
> 

Reviewed-by: José Roberto de Souza 

> Signed-off-by: Ville Syrjälä <
> ville.syrj...@linux.intel.com
> >
> ---
>  drivers/gpu/drm/i915/display/intel_bios.c | 6 ++
>  drivers/gpu/drm/i915/display/intel_vbt_defs.h | 2 ++
>  2 files changed, 8 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c 
> b/drivers/gpu/drm/i915/display/intel_bios.c
> index 2bf0bc0deee8..05eb88ee73f8 100644
> --- a/drivers/gpu/drm/i915/display/intel_bios.c
> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
> @@ -2649,6 +2649,12 @@ enum aux_ch intel_bios_port_aux_ch(struct 
> drm_i915_private *dev_priv,
>   case DP_AUX_G:
>   aux_ch = AUX_CH_G;
>   break;
> + case DP_AUX_H:
> + aux_ch = AUX_CH_H;
> + break;
> + case DP_AUX_I:
> + aux_ch = AUX_CH_I;
> + break;
>   default:
>   MISSING_CASE(info->alternate_aux_channel);
>   aux_ch = AUX_CH_A;
> diff --git a/drivers/gpu/drm/i915/display/intel_vbt_defs.h 
> b/drivers/gpu/drm/i915/display/intel_vbt_defs.h
> index e502d65300fa..b5f7a52f751a 100644
> --- a/drivers/gpu/drm/i915/display/intel_vbt_defs.h
> +++ b/drivers/gpu/drm/i915/display/intel_vbt_defs.h
> @@ -334,6 +334,8 @@ enum vbt_gmbus_ddi {
>  #define DP_AUX_E 0x50
>  #define DP_AUX_F 0x60
>  #define DP_AUX_G 0x70
> +#define DP_AUX_H 0x80
> +#define DP_AUX_I 0x90
>  
>  #define VBT_DP_MAX_LINK_RATE_HBR30
>  #define VBT_DP_MAX_LINK_RATE_HBR21
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [06/12] drm/i915: Nuke the redundant TC/TBT HPD bit defines

2020-09-08 Thread Souza, Jose
On Wed, 2020-07-01 at 00:55 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <
> ville.syrj...@linux.intel.com
> >
> 
> We have nice parametrized GEN11_{TC,TBT}_HOTPLUG() so nuke
> the overlapping defines.

Reviewed-by: José Roberto de Souza 

> 
> Signed-off-by: Ville Syrjälä <
> ville.syrj...@linux.intel.com
> >
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 20 +-
>  drivers/gpu/drm/i915/i915_reg.h | 36 +++--
>  2 files changed, 22 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 562b43ed077f..ad52109c747d 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -131,19 +131,19 @@ static const u32 hpd_bxt[HPD_NUM_PINS] = {
>  };
>  
>  static const u32 hpd_gen11[HPD_NUM_PINS] = {
> - [HPD_PORT_C] = GEN11_TC1_HOTPLUG | GEN11_TBT1_HOTPLUG,
> - [HPD_PORT_D] = GEN11_TC2_HOTPLUG | GEN11_TBT2_HOTPLUG,
> - [HPD_PORT_E] = GEN11_TC3_HOTPLUG | GEN11_TBT3_HOTPLUG,
> - [HPD_PORT_F] = GEN11_TC4_HOTPLUG | GEN11_TBT4_HOTPLUG,
> + [HPD_PORT_C] = GEN11_TC_HOTPLUG(PORT_TC1) | GEN11_TBT_HOTPLUG(PORT_TC1),
> + [HPD_PORT_D] = GEN11_TC_HOTPLUG(PORT_TC2) | GEN11_TBT_HOTPLUG(PORT_TC2),
> + [HPD_PORT_E] = GEN11_TC_HOTPLUG(PORT_TC3) | GEN11_TBT_HOTPLUG(PORT_TC3),
> + [HPD_PORT_F] = GEN11_TC_HOTPLUG(PORT_TC4) | GEN11_TBT_HOTPLUG(PORT_TC4),
>  };
>  
>  static const u32 hpd_gen12[HPD_NUM_PINS] = {
> - [HPD_PORT_D] = GEN11_TC1_HOTPLUG | GEN11_TBT1_HOTPLUG,
> - [HPD_PORT_E] = GEN11_TC2_HOTPLUG | GEN11_TBT2_HOTPLUG,
> - [HPD_PORT_F] = GEN11_TC3_HOTPLUG | GEN11_TBT3_HOTPLUG,
> - [HPD_PORT_G] = GEN11_TC4_HOTPLUG | GEN11_TBT4_HOTPLUG,
> - [HPD_PORT_H] = GEN12_TC5_HOTPLUG | GEN12_TBT5_HOTPLUG,
> - [HPD_PORT_I] = GEN12_TC6_HOTPLUG | GEN12_TBT6_HOTPLUG,
> + [HPD_PORT_D] = GEN11_TC_HOTPLUG(PORT_TC1) | GEN11_TBT_HOTPLUG(PORT_TC1),
> + [HPD_PORT_E] = GEN11_TC_HOTPLUG(PORT_TC2) | GEN11_TBT_HOTPLUG(PORT_TC2),
> + [HPD_PORT_F] = GEN11_TC_HOTPLUG(PORT_TC3) | GEN11_TBT_HOTPLUG(PORT_TC3),
> + [HPD_PORT_G] = GEN11_TC_HOTPLUG(PORT_TC4) | GEN11_TBT_HOTPLUG(PORT_TC4),
> + [HPD_PORT_H] = GEN11_TC_HOTPLUG(PORT_TC5) | GEN11_TBT_HOTPLUG(PORT_TC5),
> + [HPD_PORT_I] = GEN11_TC_HOTPLUG(PORT_TC6) | GEN11_TBT_HOTPLUG(PORT_TC6),
>  };
>  
>  static const u32 hpd_icp[HPD_NUM_PINS] = {
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 2ecde5c2e357..d7359f3bbc64 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7681,32 +7681,20 @@ enum {
>  #define GEN11_DE_HPD_IMR _MMIO(0x44474)
>  #define GEN11_DE_HPD_IIR _MMIO(0x44478)
>  #define GEN11_DE_HPD_IER _MMIO(0x4447c)
> -#define  GEN12_TC6_HOTPLUG   (1 << 21)
> -#define  GEN12_TC5_HOTPLUG   (1 << 20)
> -#define  GEN11_TC4_HOTPLUG   (1 << 19)
> -#define  GEN11_TC3_HOTPLUG   (1 << 18)
> -#define  GEN11_TC2_HOTPLUG   (1 << 17)
> -#define  GEN11_TC1_HOTPLUG   (1 << 16)
>  #define  GEN11_TC_HOTPLUG(tc_port)   (1 << ((tc_port) + 16))
> -#define  GEN11_DE_TC_HOTPLUG_MASK(GEN12_TC6_HOTPLUG | \
> -  GEN12_TC5_HOTPLUG | \
> -  GEN11_TC4_HOTPLUG | \
> -  GEN11_TC3_HOTPLUG | \
> -  GEN11_TC2_HOTPLUG | \
> -  GEN11_TC1_HOTPLUG)
> -#define  GEN12_TBT6_HOTPLUG  (1 << 5)
> -#define  GEN12_TBT5_HOTPLUG  (1 << 4)
> -#define  GEN11_TBT4_HOTPLUG  (1 << 3)
> -#define  GEN11_TBT3_HOTPLUG  (1 << 2)
> -#define  GEN11_TBT2_HOTPLUG  (1 << 1)
> -#define  GEN11_TBT1_HOTPLUG  (1 << 0)
> +#define  GEN11_DE_TC_HOTPLUG_MASK(GEN11_TC_HOTPLUG(PORT_TC6) | \
> +  GEN11_TC_HOTPLUG(PORT_TC5) | \
> +  GEN11_TC_HOTPLUG(PORT_TC4) | \
> +  GEN11_TC_HOTPLUG(PORT_TC3) | \
> +  GEN11_TC_HOTPLUG(PORT_TC2) | \
> +  GEN11_TC_HOTPLUG(PORT_TC1))
>  #define  GEN11_TBT_HOTPLUG(tc_port)  (1 << (tc_port))
> -#define  GEN11_DE_TBT_HOTPLUG_MASK   (GEN12_TBT6_HOTPLUG | \
> -  GEN12_TBT5_HOTPLUG | \
> -  GEN11_TBT4_HOTPLUG | \
> -  GEN11_TBT3_HOTPLUG | \
> -  GEN11_TBT2_HOTPLUG | \
> -  GEN11_TBT1_HOTPLUG)
> +#define  GEN11_DE_TBT_HOTPLUG_MASK  

Re: [Intel-gfx] [07/12] drm/i915: Configure GEN11_{TBT, TC}_HOTPLUG_CTL for ports TC5/6

2020-09-08 Thread Souza, Jose
On Wed, 2020-07-01 at 00:55 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <
> ville.syrj...@linux.intel.com
> >
> 
> gen11_hpd_detection_setup() is missing ports TC5/6. Add them.
> 
> TODO: Might be nice to only enable the hpd detection logic
> for ports we actually have. Should be rolled out for all
> platforms if/when done...

TC5 and TC6 don't exist in ICL but this is not a reserved register, should not 
cause any harm.

Reviewed-by: José Roberto de Souza 

> 
> Signed-off-by: Ville Syrjälä <
> ville.syrj...@linux.intel.com
> >
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index ad52109c747d..839ae674bc44 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -3109,14 +3109,18 @@ static void gen11_hpd_detection_setup(struct 
> drm_i915_private *dev_priv)
>   hotplug |= GEN11_HOTPLUG_CTL_ENABLE(PORT_TC1) |
>  GEN11_HOTPLUG_CTL_ENABLE(PORT_TC2) |
>  GEN11_HOTPLUG_CTL_ENABLE(PORT_TC3) |
> -GEN11_HOTPLUG_CTL_ENABLE(PORT_TC4);
> +GEN11_HOTPLUG_CTL_ENABLE(PORT_TC4) |
> +GEN11_HOTPLUG_CTL_ENABLE(PORT_TC5) |
> +GEN11_HOTPLUG_CTL_ENABLE(PORT_TC6);
>   I915_WRITE(GEN11_TC_HOTPLUG_CTL, hotplug);
>  
>   hotplug = I915_READ(GEN11_TBT_HOTPLUG_CTL);
>   hotplug |= GEN11_HOTPLUG_CTL_ENABLE(PORT_TC1) |
>  GEN11_HOTPLUG_CTL_ENABLE(PORT_TC2) |
>  GEN11_HOTPLUG_CTL_ENABLE(PORT_TC3) |
> -GEN11_HOTPLUG_CTL_ENABLE(PORT_TC4);
> +GEN11_HOTPLUG_CTL_ENABLE(PORT_TC4) |
> +GEN11_HOTPLUG_CTL_ENABLE(PORT_TC5) |
> +GEN11_HOTPLUG_CTL_ENABLE(PORT_TC6);
>   I915_WRITE(GEN11_TBT_HOTPLUG_CTL, hotplug);
>  }
>  
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [08/12] drm/i915: Split icp_hpd_detection_setup() into ddi vs. tc parts

2020-09-08 Thread Souza, Jose
On Wed, 2020-07-01 at 00:55 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <
> ville.syrj...@linux.intel.com
> >
> 
> No reason to stuff both DDI and TC port handling into the same
> function. Split it into two.

Reviewed-by: José Roberto de Souza 

> 
> Signed-off-by: Ville Syrjälä <
> ville.syrj...@linux.intel.com
> >
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 48 ++---
>  1 file changed, 27 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 839ae674bc44..92d74448ee03 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -3045,21 +3045,24 @@ static void ibx_hpd_irq_setup(struct drm_i915_private 
> *dev_priv)
>   ibx_hpd_detection_setup(dev_priv);
>  }
>  
> -static void icp_hpd_detection_setup(struct drm_i915_private *dev_priv,
> - u32 ddi_hotplug_enable_mask,
> - u32 tc_hotplug_enable_mask)
> +static void icp_ddi_hpd_detection_setup(struct drm_i915_private *dev_priv,
> + u32 enable_mask)
>  {
>   u32 hotplug;
>  
>   hotplug = I915_READ(SHOTPLUG_CTL_DDI);
> - hotplug |= ddi_hotplug_enable_mask;
> + hotplug |= enable_mask;
>   I915_WRITE(SHOTPLUG_CTL_DDI, hotplug);
> +}
>  
> - if (tc_hotplug_enable_mask) {
> - hotplug = I915_READ(SHOTPLUG_CTL_TC);
> - hotplug |= tc_hotplug_enable_mask;
> - I915_WRITE(SHOTPLUG_CTL_TC, hotplug);
> - }
> +static void icp_tc_hpd_detection_setup(struct drm_i915_private *dev_priv,
> +u32 enable_mask)
> +{
> + u32 hotplug;
> +
> + hotplug = I915_READ(SHOTPLUG_CTL_TC);
> + hotplug |= enable_mask;
> + I915_WRITE(SHOTPLUG_CTL_TC, hotplug);
>  }
>  
>  static void icp_hpd_irq_setup(struct drm_i915_private *dev_priv,
> @@ -3075,7 +3078,9 @@ static void icp_hpd_irq_setup(struct drm_i915_private 
> *dev_priv,
>  
>   ibx_display_interrupt_update(dev_priv, hotplug_irqs, enabled_irqs);
>  
> - icp_hpd_detection_setup(dev_priv, ddi_enable_mask, tc_enable_mask);
> + icp_ddi_hpd_detection_setup(dev_priv, ddi_enable_mask);
> + if (tc_enable_mask)
> + icp_tc_hpd_detection_setup(dev_priv, tc_enable_mask);
>  }
>  
>  /*
> @@ -3493,17 +3498,18 @@ static void icp_irq_postinstall(struct 
> drm_i915_private *dev_priv)
>   gen3_assert_iir_is_zero(&dev_priv->uncore, SDEIIR);
>   I915_WRITE(SDEIMR, ~mask);
>  
> - if (HAS_PCH_TGP(dev_priv))
> - icp_hpd_detection_setup(dev_priv, TGP_DDI_HPD_ENABLE_MASK,
> - TGP_TC_HPD_ENABLE_MASK);
> - else if (HAS_PCH_JSP(dev_priv))
> - icp_hpd_detection_setup(dev_priv, TGP_DDI_HPD_ENABLE_MASK, 0);
> - else if (HAS_PCH_MCC(dev_priv))
> - icp_hpd_detection_setup(dev_priv, ICP_DDI_HPD_ENABLE_MASK,
> - ICP_TC_HPD_ENABLE(PORT_TC1));
> - else
> - icp_hpd_detection_setup(dev_priv, ICP_DDI_HPD_ENABLE_MASK,
> - ICP_TC_HPD_ENABLE_MASK);
> + if (HAS_PCH_TGP(dev_priv)) {
> + icp_ddi_hpd_detection_setup(dev_priv, TGP_DDI_HPD_ENABLE_MASK);
> + icp_tc_hpd_detection_setup(dev_priv, TGP_TC_HPD_ENABLE_MASK);
> + } else if (HAS_PCH_JSP(dev_priv)) {
> + icp_ddi_hpd_detection_setup(dev_priv, TGP_DDI_HPD_ENABLE_MASK);
> + } else if (HAS_PCH_MCC(dev_priv)) {
> + icp_ddi_hpd_detection_setup(dev_priv, ICP_DDI_HPD_ENABLE_MASK);
> + icp_tc_hpd_detection_setup(dev_priv, 
> ICP_TC_HPD_ENABLE(PORT_TC1));
> + } else {
> + icp_ddi_hpd_detection_setup(dev_priv, ICP_DDI_HPD_ENABLE_MASK);
> + icp_tc_hpd_detection_setup(dev_priv, ICP_TC_HPD_ENABLE_MASK);
> + }
>  }
>  
>  static void gen11_irq_postinstall(struct drm_i915_private *dev_priv)
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [09/12] drm/i915: Move hpd_pin setup to encoder init

2020-09-08 Thread Souza, Jose
On Wed, 2020-07-01 at 00:55 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <
> ville.syrj...@linux.intel.com
> >
> 
> Currently DP/HDMI/DDI encoders init their hpd_pin from the
> connector init. Let's move it to the encoder init so that
> we don't need to add platform specific junk to the connector
> init (which is shared by all g4x+ platforms).
> 

Reviewed-by: José Roberto de Souza 

> Signed-off-by: Ville Syrjälä <
> ville.syrj...@linux.intel.com
> >
> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c  | 1 +
>  drivers/gpu/drm/i915/display/intel_dp.c   | 2 +-
>  drivers/gpu/drm/i915/display/intel_hdmi.c | 2 +-
>  3 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c 
> b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 884b507c5f55..d024491738b3 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -4907,6 +4907,7 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, 
> enum port port)
>   encoder->port = port;
>   encoder->cloneable = 0;
>   encoder->pipe_mask = ~0;
> + encoder->hpd_pin = intel_hpd_pin_default(dev_priv, port);
>  
>   if (INTEL_GEN(dev_priv) >= 11)
>   intel_dig_port->saved_port_bits = intel_de_read(dev_priv,
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c 
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index 3df5d901dd9d..cd516cd8acb8 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -8211,7 +8211,6 @@ intel_dp_init_connector(struct intel_digital_port 
> *intel_dig_port,
>   if (INTEL_GEN(dev_priv) >= 11)
>   connector->ycbcr_420_allowed = true;
>  
> - intel_encoder->hpd_pin = intel_hpd_pin_default(dev_priv, port);
>   intel_connector->polled = DRM_CONNECTOR_POLL_HPD;
>  
>   intel_dp_aux_init(intel_dp);
> @@ -8354,6 +8353,7 @@ bool intel_dp_init(struct drm_i915_private *dev_priv,
>   }
>   intel_encoder->cloneable = 0;
>   intel_encoder->port = port;
> + intel_encoder->hpd_pin = intel_hpd_pin_default(dev_priv, port);
>  
>   intel_dig_port->hpd_pulse = intel_dp_hpd_pulse;
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c 
> b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index 864a1642e81c..f515d0fce968 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> @@ -3253,7 +3253,6 @@ void intel_hdmi_init_connector(struct 
> intel_digital_port *intel_dig_port,
>   if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
>   connector->ycbcr_420_allowed = true;
>  
> - intel_encoder->hpd_pin = intel_hpd_pin_default(dev_priv, port);
>   intel_connector->polled = DRM_CONNECTOR_POLL_HPD;
>  
>   if (HAS_DDI(dev_priv))
> @@ -3385,6 +3384,7 @@ void intel_hdmi_init(struct drm_i915_private *dev_priv,
>   intel_encoder->pipe_mask = ~0;
>   }
>   intel_encoder->cloneable = 1 << INTEL_OUTPUT_ANALOG;
> + intel_encoder->hpd_pin = intel_hpd_pin_default(dev_priv, port);
>   /*
>* BSpec is unclear about HDMI+HDMI cloning on g4x, but it seems
>* to work on real hardware. And since g4x can send infoframes to
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [12/12] drm/i915: Nuke pointless variable

2020-09-08 Thread Souza, Jose
On Wed, 2020-07-01 at 00:56 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <
> ville.syrj...@linux.intel.com
> >
> 
> No point in assigning the function return value to a local
> variable if we're just going to use it the one time.
> 

Reviewed-by: José Roberto de Souza 

> Signed-off-by: Ville Syrjälä <
> ville.syrj...@linux.intel.com
> >
> ---
>  drivers/gpu/drm/i915/display/intel_hotplug.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c 
> b/drivers/gpu/drm/i915/display/intel_hotplug.c
> index 8a8e77314a4e..938466b2c9d1 100644
> --- a/drivers/gpu/drm/i915/display/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/display/intel_hotplug.c
> @@ -480,7 +480,6 @@ void intel_hpd_irq_handler(struct drm_i915_private 
> *dev_priv,
>* only the one of them (DP) will have ->hpd_pulse().
>*/
>   for_each_intel_encoder(&dev_priv->drm, encoder) {
> - bool has_hpd_pulse = intel_encoder_has_hpd_pulse(encoder);
>   enum port port = encoder->port;
>   bool long_hpd;
>  
> @@ -488,7 +487,7 @@ void intel_hpd_irq_handler(struct drm_i915_private 
> *dev_priv,
>   if (!(BIT(pin) & pin_mask))
>   continue;
>  
> - if (!has_hpd_pulse)
> + if (!intel_encoder_has_hpd_pulse(encoder))
>   continue;
>  
>   long_hpd = long_mask & BIT(pin);
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v2 06/18] drm/dp: Add helpers to identify downstream facing port types

2020-09-08 Thread Lyude Paul
On Fri, 2020-09-04 at 14:53 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä 
> 
> Add a few helpers to let us better identify which kind of DFP
> we're dealing with.
> 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/drm_dp_helper.c | 60 +
>  include/drm/drm_dp_helper.h |  5 +++
>  2 files changed, 65 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_dp_helper.c
> b/drivers/gpu/drm/drm_dp_helper.c
> index c21bbfc3d714..0fcb94f7dbe5 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -363,6 +363,66 @@ int drm_dp_dpcd_read_link_status(struct drm_dp_aux
> *aux,
>  }
>  EXPORT_SYMBOL(drm_dp_dpcd_read_link_status);
>  
> +static bool is_edid_digital_input_dp(const struct edid *edid)
> +{
> + return edid && edid->revision >= 4 &&
> + edid->input & DRM_EDID_INPUT_DIGITAL &&
> + (edid->input & DRM_EDID_DIGITAL_TYPE_MASK) ==
> DRM_EDID_DIGITAL_TYPE_DP;
> +}
> +
> +/**
> + * drm_dp_downstream_is_type() - is the downstream facing port of certain
> type?
> + * @dpcd: DisplayPort configuration data
> + * @port_cap: port capabilities
> + *
> + * Caveat: Only works with DPCD 1.1+ port caps.
> + *
> + * Returns whether the downstream facing port matches the type.

Nitpick: s/Returns/Returns:/ for kdoc purposes

> + */
> +bool drm_dp_downstream_is_type(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
> +const u8 port_cap[4], u8 type)
> +{
> + return drm_dp_is_branch(dpcd) &&
> + dpcd[DP_DPCD_REV] >= 0x11 &&
> + (port_cap[0] & DP_DS_PORT_TYPE_MASK) == type;
> +}
> +EXPORT_SYMBOL(drm_dp_downstream_is_type);
> +
> +/**
> + * drm_dp_downstream_is_tmds() - is the downstream facing port TMDS?
> + * @dpcd: DisplayPort configuration data
> + * @port_cap: port capabilities
> + * @edid: EDID
> + *
> + * Returns whether the downstream facing port is TMDS (HDMI/DVI).

Same nitpick here

> + */
> +bool drm_dp_downstream_is_tmds(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
> +const u8 port_cap[4],
> +const struct edid *edid)
> +{
> + if (dpcd[DP_DPCD_REV] < 0x11) {
> + switch (dpcd[DP_DOWNSTREAMPORT_PRESENT] &
> DP_DWN_STRM_PORT_TYPE_MASK) {
> + case DP_DWN_STRM_PORT_TYPE_TMDS:
> + return true;
> + default:
> + return false;
> + }
> + }
> +
> + switch (port_cap[0] & DP_DS_PORT_TYPE_MASK) {
> + case DP_DS_PORT_TYPE_DP_DUALMODE:
> + if (is_edid_digital_input_dp(edid))
> + return false;
> + fallthrough;
> + case DP_DS_PORT_TYPE_DVI:
> + case DP_DS_PORT_TYPE_HDMI:
> + return true;
> + default:
> + return false;
> + }
> +}
> +EXPORT_SYMBOL(drm_dp_downstream_is_tmds);
> +
>  /**
>   * drm_dp_send_real_edid_checksum() - send back real edid checksum value
>   * @aux: DisplayPort AUX channel
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index 86461a40066b..4f946826dfce 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -1638,6 +1638,11 @@ bool drm_dp_send_real_edid_checksum(struct drm_dp_aux
> *aux,
>  int drm_dp_read_downstream_info(struct drm_dp_aux *aux,
>   const u8 dpcd[DP_RECEIVER_CAP_SIZE],
>   u8 downstream_ports[DP_MAX_DOWNSTREAM_PORTS]);
> +bool drm_dp_downstream_is_type(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
> +const u8 port_cap[4], u8 type);
> +bool drm_dp_downstream_is_tmds(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
> +const u8 port_cap[4],
> +const struct edid *edid);
>  int drm_dp_downstream_max_clock(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
>   const u8 port_cap[4]);
>  int drm_dp_downstream_max_bpc(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
-- 
Cheers,
Lyude Paul (she/her)
Software Engineer at Red Hat

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


Re: [Intel-gfx] [PATCH v2 07/18] drm/dp: Pimp drm_dp_downstream_max_bpc()

2020-09-08 Thread Lyude Paul
On Fri, 2020-09-04 at 14:53 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä 
> 
> Deal with more cases in drm_dp_downstream_max_bpc():
> - DPCD 1.0 -> assume 8bpc for non-DP
> - DPCD 1.1+ DP (or DP++ with DP sink) -> allow anything
> - DPCD 1.1+ TMDS -> check the caps, assume 8bpc if the value is crap
> - anything else -> assume 8bpc
> 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/drm_dp_helper.c   | 69 +++
>  .../drm/i915/display/intel_display_debugfs.c  |  3 +-
>  drivers/gpu/drm/i915/display/intel_dp.c   |  2 +-
>  include/drm/drm_dp_helper.h   | 10 ++-
>  4 files changed, 51 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_helper.c
> b/drivers/gpu/drm/drm_dp_helper.c
> index 0fcb94f7dbe5..ab87209c25d8 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -653,36 +653,44 @@ int drm_dp_downstream_max_clock(const u8
> dpcd[DP_RECEIVER_CAP_SIZE],
>  EXPORT_SYMBOL(drm_dp_downstream_max_clock);
>  
>  /**
> - * drm_dp_downstream_max_bpc() - extract branch device max
> - *   bits per component
> - * @dpcd: DisplayPort configuration data
> - * @port_cap: port capabilities
> - *
> - * See also:
> - * drm_dp_read_downstream_info()
> - * drm_dp_downstream_max_clock()
> - *
> - * Returns: Max bpc on success or 0 if max bpc not defined
> - */
> +  * drm_dp_downstream_max_bpc() - extract downstream facing port max
> +  *   bits per component
> +  * @dpcd: DisplayPort configuration data
> +  * @port_cap: downstream facing port capabilities
> +  * @edid: EDID
> +  *
> +  * Returns max bpc on success or 0 if max bpc not defined

s/Returns/Returns:/

> +  */
>  int drm_dp_downstream_max_bpc(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
> -   const u8 port_cap[4])
> +   const u8 port_cap[4],
> +   const struct edid *edid)
>  {
> - int type = port_cap[0] & DP_DS_PORT_TYPE_MASK;
> - bool detailed_cap_info = dpcd[DP_DOWNSTREAMPORT_PRESENT] &
> - DP_DETAILED_CAP_INFO_AVAILABLE;
> - int bpc;
> -
> - if (!detailed_cap_info)
> + if (!drm_dp_is_branch(dpcd))
>   return 0;
>  
> - switch (type) {
> - case DP_DS_PORT_TYPE_VGA:
> - case DP_DS_PORT_TYPE_DVI:
> - case DP_DS_PORT_TYPE_HDMI:
> + if (dpcd[DP_DPCD_REV] < 0x11) {
> + switch (dpcd[DP_DOWNSTREAMPORT_PRESENT] &
> DP_DWN_STRM_PORT_TYPE_MASK) {
> + case DP_DWN_STRM_PORT_TYPE_DP:
> + return 0;
> + default:
> + return 8;
> + }
> + }
> +
> + switch (port_cap[0] & DP_DS_PORT_TYPE_MASK) {
> + case DP_DS_PORT_TYPE_DP:
> + return 0;
>   case DP_DS_PORT_TYPE_DP_DUALMODE:
> - bpc = port_cap[2] & DP_DS_MAX_BPC_MASK;
> + if (is_edid_digital_input_dp(edid))
> + return 0;
> + fallthrough;
> + case DP_DS_PORT_TYPE_HDMI:
> + case DP_DS_PORT_TYPE_DVI:
> + case DP_DS_PORT_TYPE_VGA:
> + if ((dpcd[DP_DOWNSTREAMPORT_PRESENT] &
> DP_DETAILED_CAP_INFO_AVAILABLE) == 0)
> + return 8;
>  
> - switch (bpc) {
> + switch (port_cap[2] & DP_DS_MAX_BPC_MASK) {
>   case DP_DS_8BPC:
>   return 8;
>   case DP_DS_10BPC:
> @@ -691,10 +699,12 @@ int drm_dp_downstream_max_bpc(const u8
> dpcd[DP_RECEIVER_CAP_SIZE],
>   return 12;
>   case DP_DS_16BPC:
>   return 16;
> + default:
> + return 8;
>   }
> - fallthrough;
> + break;
>   default:
> - return 0;
> + return 8;
>   }
>  }
>  EXPORT_SYMBOL(drm_dp_downstream_max_bpc);
> @@ -717,12 +727,15 @@ EXPORT_SYMBOL(drm_dp_downstream_id);
>   * @m: pointer for debugfs file
>   * @dpcd: DisplayPort configuration data
>   * @port_cap: port capabilities
> + * @edid: EDID
>   * @aux: DisplayPort AUX channel
>   *
>   */
>  void drm_dp_downstream_debug(struct seq_file *m,
>const u8 dpcd[DP_RECEIVER_CAP_SIZE],
> -  const u8 port_cap[4], struct drm_dp_aux *aux)
> +  const u8 port_cap[4],
> +  const struct edid *edid,
> +  struct drm_dp_aux *aux)
>  {
>   bool detailed_cap_info = dpcd[DP_DOWNSTREAMPORT_PRESENT] &
>DP_DETAILED_CAP_INFO_AVAILABLE;
> @@ -789,7 +802,7 @@ void drm_dp_downstream_debug(struct seq_file *m,
>   seq_printf(m, "\t\tMax TMDS clock: %d kHz\n",
> clk);
>   }
>  
> - bpc = drm_dp_downstream_max_bpc(dpcd, port_cap);
> + bpc = drm_dp_downstream_max_bpc(dpcd, port_cap, edid);
>  
>   if (bpc > 0)
>

Re: [Intel-gfx] [PATCH v2 07/18] drm/dp: Pimp drm_dp_downstream_max_bpc()

2020-09-08 Thread Lyude Paul
On Fri, 2020-09-04 at 14:53 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä 
> 
> Deal with more cases in drm_dp_downstream_max_bpc():
> - DPCD 1.0 -> assume 8bpc for non-DP
> - DPCD 1.1+ DP (or DP++ with DP sink) -> allow anything
> - DPCD 1.1+ TMDS -> check the caps, assume 8bpc if the value is crap
> - anything else -> assume 8bpc
> 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/drm_dp_helper.c   | 69 +++
>  .../drm/i915/display/intel_display_debugfs.c  |  3 +-
>  drivers/gpu/drm/i915/display/intel_dp.c   |  2 +-
>  include/drm/drm_dp_helper.h   | 10 ++-
>  4 files changed, 51 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_helper.c
> b/drivers/gpu/drm/drm_dp_helper.c
> index 0fcb94f7dbe5..ab87209c25d8 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -653,36 +653,44 @@ int drm_dp_downstream_max_clock(const u8
> dpcd[DP_RECEIVER_CAP_SIZE],
>  EXPORT_SYMBOL(drm_dp_downstream_max_clock);
>  
>  /**
> - * drm_dp_downstream_max_bpc() - extract branch device max
> - *   bits per component
> - * @dpcd: DisplayPort configuration data
> - * @port_cap: port capabilities
> - *
> - * See also:
> - * drm_dp_read_downstream_info()
> - * drm_dp_downstream_max_clock()
> - *
> - * Returns: Max bpc on success or 0 if max bpc not defined
> - */
> +  * drm_dp_downstream_max_bpc() - extract downstream facing port max
> +  *   bits per component
> +  * @dpcd: DisplayPort configuration data
> +  * @port_cap: downstream facing port capabilities
> +  * @edid: EDID
> +  *
> +  * Returns max bpc on success or 0 if max bpc not defined
> +  */
>  int drm_dp_downstream_max_bpc(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
> -   const u8 port_cap[4])
> +   const u8 port_cap[4],
> +   const struct edid *edid)
>  {
> - int type = port_cap[0] & DP_DS_PORT_TYPE_MASK;
> - bool detailed_cap_info = dpcd[DP_DOWNSTREAMPORT_PRESENT] &
> - DP_DETAILED_CAP_INFO_AVAILABLE;
> - int bpc;
> -
> - if (!detailed_cap_info)

I don't think we can drop this check. There's a somewhat surprising blurb
about downstream port caps in the DP 2.0 spec (section 5.3.3.1):

   In addition, the adapter shall set the Detailed Capabilities Info registers
   (DPCD Addresses 00080h through 0008Fh) to show all the downstream types,
   including DFP 0. Either one or four bytes are used, per DFP type
   indication. Therefore, up to 16 (with 1-byte descriptor) or four (with 4-
   byte descriptor) DFP capabilities can be stored.

I've never once actually seen a sink do this, but this does mean it's
technically possible tthat if we don't check the detailed caps bit then we
might end up reading another port's DFP type instead of max_bpc info. Note
though that we can make the assumption the four byte version of the field is
used for DP 1.4+

> + if (!drm_dp_is_branch(dpcd))
>   return 0;
>  
> - switch (type) {
> - case DP_DS_PORT_TYPE_VGA:
> - case DP_DS_PORT_TYPE_DVI:
> - case DP_DS_PORT_TYPE_HDMI:
> + if (dpcd[DP_DPCD_REV] < 0x11) {
> + switch (dpcd[DP_DOWNSTREAMPORT_PRESENT] &
> DP_DWN_STRM_PORT_TYPE_MASK) {
> + case DP_DWN_STRM_PORT_TYPE_DP:
> + return 0;
> + default:
> + return 8;
> + }
> + }
> +
> + switch (port_cap[0] & DP_DS_PORT_TYPE_MASK) {
> + case DP_DS_PORT_TYPE_DP:
> + return 0;
>   case DP_DS_PORT_TYPE_DP_DUALMODE:
> - bpc = port_cap[2] & DP_DS_MAX_BPC_MASK;
> + if (is_edid_digital_input_dp(edid))
> + return 0;
> + fallthrough;
> + case DP_DS_PORT_TYPE_HDMI:
> + case DP_DS_PORT_TYPE_DVI:
> + case DP_DS_PORT_TYPE_VGA:
> + if ((dpcd[DP_DOWNSTREAMPORT_PRESENT] &
> DP_DETAILED_CAP_INFO_AVAILABLE) == 0)
> + return 8;
>  
> - switch (bpc) {
> + switch (port_cap[2] & DP_DS_MAX_BPC_MASK) {
>   case DP_DS_8BPC:
>   return 8;
>   case DP_DS_10BPC:
> @@ -691,10 +699,12 @@ int drm_dp_downstream_max_bpc(const u8
> dpcd[DP_RECEIVER_CAP_SIZE],
>   return 12;
>   case DP_DS_16BPC:
>   return 16;
> + default:
> + return 8;
>   }
> - fallthrough;
> + break;
>   default:
> - return 0;
> + return 8;
>   }
>  }
>  EXPORT_SYMBOL(drm_dp_downstream_max_bpc);
> @@ -717,12 +727,15 @@ EXPORT_SYMBOL(drm_dp_downstream_id);
>   * @m: pointer for debugfs file
>   * @dpcd: DisplayPort configuration data
>   * @port_cap: port capabilities
> + * @edid: EDID
>   * @aux: DisplayPort AUX channel
>   *
>   */
>  void drm_dp_downstream_debug(struct seq_file *m,
> 

Re: [Intel-gfx] [PATCH v2 08/18] drm/dp: Redo drm_dp_downstream_max_clock() as drm_dp_downstream_max_dotclock()

2020-09-08 Thread Lyude Paul
BTW - we started using drm_dp_downstream_max_clock() in nouveau, so you'll
need to update the function call there too.

On Fri, 2020-09-04 at 14:53 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä 
> 
> We want to differentiate between the DFP dotclock and TMDS clock
> limits. Let's convert the current thing to just give us the
> dotclock limit.
> 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/drm_dp_helper.c | 46 +
>  drivers/gpu/drm/i915/display/intel_dp.c |  4 +--
>  include/drm/drm_dp_helper.h |  4 +--
>  3 files changed, 20 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_helper.c
> b/drivers/gpu/drm/drm_dp_helper.c
> index ab87209c25d8..822a30e609ef 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -616,41 +616,32 @@ int drm_dp_read_downstream_info(struct drm_dp_aux
> *aux,
>  EXPORT_SYMBOL(drm_dp_read_downstream_info);
>  
>  /**
> - * drm_dp_downstream_max_clock() - extract branch device max
> - * pixel rate for legacy VGA
> - * converter or max TMDS clock
> - * rate for others
> + * drm_dp_downstream_max_dotclock() - extract downstream facing port max
> dot clock
>   * @dpcd: DisplayPort configuration data
>   * @port_cap: port capabilities
>   *
> - * See also:
> - * drm_dp_read_downstream_info()
> - * drm_dp_downstream_max_bpc()
> - *
> - * Returns: Max clock in kHz on success or 0 if max clock not defined
> + * Returns downstream facing port max dot clock in kHz on success,
> + * or 0 if max clock not defined
>   */
> -int drm_dp_downstream_max_clock(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
> - const u8 port_cap[4])
> +int drm_dp_downstream_max_dotclock(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
> +const u8 port_cap[4])
>  {
> - int type = port_cap[0] & DP_DS_PORT_TYPE_MASK;
> - bool detailed_cap_info = dpcd[DP_DOWNSTREAMPORT_PRESENT] &
> - DP_DETAILED_CAP_INFO_AVAILABLE;
> + if (!drm_dp_is_branch(dpcd))
> + return 0;
>  
> - if (!detailed_cap_info)
> + if (dpcd[DP_DPCD_REV] < 0x11)
>   return 0;
>  
> - switch (type) {
> + switch (port_cap[0] & DP_DS_PORT_TYPE_MASK) {
>   case DP_DS_PORT_TYPE_VGA:
> - return port_cap[1] * 8 * 1000;
> - case DP_DS_PORT_TYPE_DVI:
> - case DP_DS_PORT_TYPE_HDMI:
> - case DP_DS_PORT_TYPE_DP_DUALMODE:
> - return port_cap[1] * 2500;
> + if ((dpcd[DP_DOWNSTREAMPORT_PRESENT] &
> DP_DETAILED_CAP_INFO_AVAILABLE) == 0)
> + return 0;
> + return port_cap[1] * 8000;
>   default:
>   return 0;
>   }
>  }
> -EXPORT_SYMBOL(drm_dp_downstream_max_clock);
> +EXPORT_SYMBOL(drm_dp_downstream_max_dotclock);
>  
>  /**
>* drm_dp_downstream_max_bpc() - extract downstream facing port max
> @@ -793,14 +784,9 @@ void drm_dp_downstream_debug(struct seq_file *m,
>   seq_printf(m, "\t\tSW: %d.%d\n", rev[0], rev[1]);
>  
>   if (detailed_cap_info) {
> - clk = drm_dp_downstream_max_clock(dpcd, port_cap);
> -
> - if (clk > 0) {
> - if (type == DP_DS_PORT_TYPE_VGA)
> - seq_printf(m, "\t\tMax dot clock: %d kHz\n",
> clk);
> - else
> - seq_printf(m, "\t\tMax TMDS clock: %d kHz\n",
> clk);
> - }
> + clk = drm_dp_downstream_max_dotclock(dpcd, port_cap);
> + if (clk > 0)
> + seq_printf(m, "\t\tMax dot clock: %d kHz\n", clk);
>  
>   bpc = drm_dp_downstream_max_bpc(dpcd, port_cap, edid);
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index c73b3efd84e0..8f4aee35c203 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -261,8 +261,8 @@ intel_dp_downstream_max_dotclock(struct intel_dp
> *intel_dp)
>   if (type != DP_DS_PORT_TYPE_VGA)
>   return max_dotclk;
>  
> - ds_max_dotclk = drm_dp_downstream_max_clock(intel_dp->dpcd,
> - intel_dp-
> >downstream_ports);
> + ds_max_dotclk = drm_dp_downstream_max_dotclock(intel_dp->dpcd,
> +intel_dp-
> >downstream_ports);
>  
>   if (ds_max_dotclk != 0)
>   max_dotclk = min(max_dotclk, ds_max_dotclk);
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index 6218de1294c1..19bc04207788 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -1643,8 +1643,8 @@ bool drm_dp_downstream_is_type(const u8
> dpcd[DP_RECEIVER_CAP_SIZE],
>  bool drm_dp_downstream_is_tmds(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
>  const u8 po

Re: [Intel-gfx] [PATCH v2 10/18] drm/dp: Add drm_dp_downstream_{min, max}_tmds_clock()

2020-09-08 Thread Lyude Paul
On Fri, 2020-09-04 at 14:53 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä 
> 
> Add helpers to get the TMDS clock limits for HDMI/DVI downstream
> facing ports.
> 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/drm_dp_helper.c | 116 
>  include/drm/drm_dp_helper.h |   6 ++
>  2 files changed, 122 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_dp_helper.c
> b/drivers/gpu/drm/drm_dp_helper.c
> index 822a30e609ef..f567428f2aef 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -643,6 +643,114 @@ int drm_dp_downstream_max_dotclock(const u8
> dpcd[DP_RECEIVER_CAP_SIZE],
>  }
>  EXPORT_SYMBOL(drm_dp_downstream_max_dotclock);
>  
> +/**
> + * drm_dp_downstream_max_tmds_clock() - extract downstream facing port max
> TMDS clock
> + * @dpcd: DisplayPort configuration data
> + * @port_cap: port capabilities
> + * @edid: EDID
> + *
> + * Returns HDMI/DVI downstream facing port max TMDS clock in kHz on
> success,
> + * or 0 if max TMDS clock not defined
> + */
> +int drm_dp_downstream_max_tmds_clock(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
> +  const u8 port_cap[4],
> +  const struct edid *edid)
> +{
> + if (!drm_dp_is_branch(dpcd))
> + return 0;
> +
> + if (dpcd[DP_DPCD_REV] < 0x11) {
> + switch (dpcd[DP_DOWNSTREAMPORT_PRESENT] &
> DP_DWN_STRM_PORT_TYPE_MASK) {
> + case DP_DWN_STRM_PORT_TYPE_TMDS:
> + return 165000;
> + default:
> + return 0;
> + }
> + }
> +
> + switch (port_cap[0] & DP_DS_PORT_TYPE_MASK) {
> + case DP_DS_PORT_TYPE_DP_DUALMODE:
> + if (is_edid_digital_input_dp(edid))
> + return 0;
> + /*
> +  * It's left up to the driver to check the
> +  * DP dual mode adapter's max TMDS clock.
> +  *
> +  * Unfortunatley it looks like branch devices
> +  * may not fordward that the DP dual mode i2c
> +  * access so we just usually get i2c nak :(
> +  */
> + fallthrough;
> + case DP_DS_PORT_TYPE_HDMI:
> +  /*
> +   * We should perhaps assume 165 MHz when detailed cap
> +   * info is not available. But looks like many typical
> +   * branch devices fall into that category and so we'd
> +   * probably end up with users complaining that they can't
> +   * get high resolution modes with their favorite dongle.
> +   *
> +   * So let's limit to 300 MHz instead since DPCD 1.4
> +   * HDMI 2.0 DFPs are required to have the detailed cap
> +   * info. So it's more likely we're dealing with a HDMI 1.4
> +   * compatible* device here.
> +   */
> + if ((dpcd[DP_DOWNSTREAMPORT_PRESENT] &
> DP_DETAILED_CAP_INFO_AVAILABLE) == 0)
> + return 30;
> + return port_cap[1] * 2500;
> + case DP_DS_PORT_TYPE_DVI:
> + if ((dpcd[DP_DOWNSTREAMPORT_PRESENT] &
> DP_DETAILED_CAP_INFO_AVAILABLE) == 0)
> + return 165000;
> + /* FIXME what to do about DVI dual link? */
> + return port_cap[1] * 2500;
> + default:
> + return 0;
> + }
> +}
> +EXPORT_SYMBOL(drm_dp_downstream_max_tmds_clock);
> +
> +/**
> + * drm_dp_downstream_min_tmds_clock() - extract downstream facing port min
> TMDS clock
> + * @dpcd: DisplayPort configuration data
> + * @port_cap: port capabilities
> + * @edid: EDID
> + *
> + * Returns HDMI/DVI downstream facing port min TMDS clock in kHz on
> success,
> + * or 0 if max TMDS clock not defined

s/max/min/

Also, I would assume if callers are interested in min they're also interested
in max and vice versa, would it maybe make sense to combine the min/max
functions here?

Also, we should probably note the existence of this function in the max
dotclock functions and vice-versa
> + */
> +int drm_dp_downstream_min_tmds_clock(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
> +  const u8 port_cap[4],
> +  const struct edid *edid)
> +{
> + if (!drm_dp_is_branch(dpcd))
> + return 0;
> +
> + if (dpcd[DP_DPCD_REV] < 0x11) {
> + switch (dpcd[DP_DOWNSTREAMPORT_PRESENT] &
> DP_DWN_STRM_PORT_TYPE_MASK) {
> + case DP_DWN_STRM_PORT_TYPE_TMDS:
> + return 25000;
> + default:
> + return 0;
> + }
> + }
> +
> + switch (port_cap[0] & DP_DS_PORT_TYPE_MASK) {
> + case DP_DS_PORT_TYPE_DP_DUALMODE:
> + if (is_edid_digital_input_dp(edid))
> + return 0;
> + fallthrough;
> + case DP_DS_PORT_TYPE_DVI:
> + case DP_DS_PORT_TYPE_HDMI:
> + /*
> + 

Re: [Intel-gfx] [PATCH v2 10/18] drm/dp: Add drm_dp_downstream_{min, max}_tmds_clock()

2020-09-08 Thread Lyude Paul
On Fri, 2020-09-04 at 14:53 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä 
> 
> Add helpers to get the TMDS clock limits for HDMI/DVI downstream
> facing ports.
> 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/drm_dp_helper.c | 116 
>  include/drm/drm_dp_helper.h |   6 ++
>  2 files changed, 122 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_dp_helper.c
> b/drivers/gpu/drm/drm_dp_helper.c
> index 822a30e609ef..f567428f2aef 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -643,6 +643,114 @@ int drm_dp_downstream_max_dotclock(const u8
> dpcd[DP_RECEIVER_CAP_SIZE],
>  }
>  EXPORT_SYMBOL(drm_dp_downstream_max_dotclock);
>  
> +/**
> + * drm_dp_downstream_max_tmds_clock() - extract downstream facing port max
> TMDS clock
> + * @dpcd: DisplayPort configuration data
> + * @port_cap: port capabilities
> + * @edid: EDID
> + *
> + * Returns HDMI/DVI downstream facing port max TMDS clock in kHz on
> success,
> + * or 0 if max TMDS clock not defined
> + */
> +int drm_dp_downstream_max_tmds_clock(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
> +  const u8 port_cap[4],
> +  const struct edid *edid)
> +{
> + if (!drm_dp_is_branch(dpcd))
> + return 0;
> +
> + if (dpcd[DP_DPCD_REV] < 0x11) {
> + switch (dpcd[DP_DOWNSTREAMPORT_PRESENT] &
> DP_DWN_STRM_PORT_TYPE_MASK) {
> + case DP_DWN_STRM_PORT_TYPE_TMDS:
> + return 165000;
> + default:
> + return 0;
> + }
> + }
> +
> + switch (port_cap[0] & DP_DS_PORT_TYPE_MASK) {
> + case DP_DS_PORT_TYPE_DP_DUALMODE:
> + if (is_edid_digital_input_dp(edid))
> + return 0;
> + /*
> +  * It's left up to the driver to check the
> +  * DP dual mode adapter's max TMDS clock.
> +  *
> +  * Unfortunatley it looks like branch devices
> +  * may not fordward that the DP dual mode i2c
> +  * access so we just usually get i2c nak :(
> +  */
> + fallthrough;
> + case DP_DS_PORT_TYPE_HDMI:
> +  /*
> +   * We should perhaps assume 165 MHz when detailed cap
> +   * info is not available. But looks like many typical
> +   * branch devices fall into that category and so we'd
> +   * probably end up with users complaining that they can't
> +   * get high resolution modes with their favorite dongle.
> +   *
> +   * So let's limit to 300 MHz instead since DPCD 1.4
> +   * HDMI 2.0 DFPs are required to have the detailed cap
> +   * info. So it's more likely we're dealing with a HDMI 1.4
> +   * compatible* device here.

Forgot to mention - not directly related to this series, there's some hidden
i2c bits that I think can also be probed for this sort of information on
passive adapters, I know amdgpu actually supports this. I wonder how many of
them also apply to older active adapters...

> +   */
> + if ((dpcd[DP_DOWNSTREAMPORT_PRESENT] &
> DP_DETAILED_CAP_INFO_AVAILABLE) == 0)
> + return 30;
> + return port_cap[1] * 2500;
> + case DP_DS_PORT_TYPE_DVI:
> + if ((dpcd[DP_DOWNSTREAMPORT_PRESENT] &
> DP_DETAILED_CAP_INFO_AVAILABLE) == 0)
> + return 165000;
> + /* FIXME what to do about DVI dual link? */
> + return port_cap[1] * 2500;
> + default:
> + return 0;
> + }
> +}
> +EXPORT_SYMBOL(drm_dp_downstream_max_tmds_clock);
> +
> +/**
> + * drm_dp_downstream_min_tmds_clock() - extract downstream facing port min
> TMDS clock
> + * @dpcd: DisplayPort configuration data
> + * @port_cap: port capabilities
> + * @edid: EDID
> + *
> + * Returns HDMI/DVI downstream facing port min TMDS clock in kHz on
> success,
> + * or 0 if max TMDS clock not defined
> + */
> +int drm_dp_downstream_min_tmds_clock(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
> +  const u8 port_cap[4],
> +  const struct edid *edid)
> +{
> + if (!drm_dp_is_branch(dpcd))
> + return 0;
> +
> + if (dpcd[DP_DPCD_REV] < 0x11) {
> + switch (dpcd[DP_DOWNSTREAMPORT_PRESENT] &
> DP_DWN_STRM_PORT_TYPE_MASK) {
> + case DP_DWN_STRM_PORT_TYPE_TMDS:
> + return 25000;
> + default:
> + return 0;
> + }
> + }
> +
> + switch (port_cap[0] & DP_DS_PORT_TYPE_MASK) {
> + case DP_DS_PORT_TYPE_DP_DUALMODE:
> + if (is_edid_digital_input_dp(edid))
> + return 0;
> + fallthrough;
> + case DP_DS_PORT_TYPE_DVI:
> + case DP_DS_PORT_TYPE_HDMI:
> + /*
> +  * Unclear wh

Re: [Intel-gfx] [PATCH v2 12/18] drm/i915: Configure DP 1.3+ protocol converted HDMI mode

2020-09-08 Thread Lyude Paul
On Fri, 2020-09-04 at 14:53 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä 
> 
> DP 1.3 adds some extra control knobs for DP->HDMI protocol conversion.
> Let's use that to configure the "HDMI mode" (ie. infoframes vs. not)
> based on the capabilities of the sink.
> 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c |  1 +
>  drivers/gpu/drm/i915/display/intel_dp.c  | 23 +++
>  drivers/gpu/drm/i915/display/intel_dp.h  |  1 +
>  3 files changed, 25 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 28ff85493f25..9adba0d0b4aa 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -3470,6 +3470,7 @@ static void hsw_ddi_pre_enable_dp(struct
> intel_atomic_state *state,
>   intel_ddi_init_dp_buf_reg(encoder);
>   if (!is_mst)
>   intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
> + intel_dp_configure_protocol_converter(intel_dp);
>   intel_dp_sink_set_decompression_state(intel_dp, crtc_state,
> true);
>   intel_dp_sink_set_fec_ready(intel_dp, crtc_state);
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index a703e4659e47..047449253a54 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -3792,6 +3792,28 @@ static void intel_dp_enable_port(struct intel_dp
> *intel_dp,
>   intel_de_posting_read(dev_priv, intel_dp->output_reg);
>  }
>  
> +void intel_dp_configure_protocol_converter(struct intel_dp *intel_dp)
> +{
> + struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> + u8 tmp;
> +
> + if (intel_dp->dpcd[DP_DPCD_REV] < 0x13)
> + return;
> +
> + if (!drm_dp_is_branch(intel_dp->dpcd))
> + return;
> +
> + tmp = intel_dp->has_hdmi_sink ?
> + DP_HDMI_DVI_OUTPUT_CONFIG : 0;
> +
> + if (drm_dp_dpcd_writeb(&intel_dp->aux,
> +DP_PROTOCOL_CONVERTER_CONTROL_0, tmp) <= 0)
> + drm_dbg_kms(&i915->drm, "Failed to set protocol converter HDMI
> mode to %s\n",
> + enableddisabled(intel_dp->has_hdmi_sink));
> +
> + /* TODO: configure YCbCr 4:2:2/4:2:0 conversion */
> +}

Maybe add a DP helper for this while we're at it? Up to you

> +
>  static void intel_enable_dp(struct intel_atomic_state *state,
>   struct intel_encoder *encoder,
>   const struct intel_crtc_state *pipe_config,
> @@ -3829,6 +3851,7 @@ static void intel_enable_dp(struct intel_atomic_state
> *state,
>   }
>  
>   intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
> + intel_dp_configure_protocol_converter(intel_dp);
>   intel_dp_start_link_train(intel_dp);
>   intel_dp_stop_link_train(intel_dp);
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.h
> b/drivers/gpu/drm/i915/display/intel_dp.h
> index ec5688a21f66..08a1c0aa8b94 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.h
> +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> @@ -51,6 +51,7 @@ int intel_dp_get_link_train_fallback_values(struct
> intel_dp *intel_dp,
>  int intel_dp_retrain_link(struct intel_encoder *encoder,
> struct drm_modeset_acquire_ctx *ctx);
>  void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode);
> +void intel_dp_configure_protocol_converter(struct intel_dp *intel_dp);
>  void intel_dp_sink_set_decompression_state(struct intel_dp *intel_dp,
>  const struct intel_crtc_state
> *crtc_state,
>  bool enable);
-- 
Cheers,
Lyude Paul (she/her)
Software Engineer at Red Hat

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


[Intel-gfx] [PATCH v2] drm/i915/gvt: Prevent NULL pointer dereference in intel_vgpu_reg_rw_edid()

2020-09-08 Thread Alejandro Sior
In the function intel_vgpu_reg_rw_edid of kvmgt.c, pos can be equal
to NULL for GPUs that do not properly support EDID. In those cases, when
pos gets passed to the handle_edid functions, it gets added a short offset
then it's dereferenced in memcpy's, leading to NULL pointer
dereference kernel oops.

More concretely, that kernel oops renders some Broadwell GPUs users
unable to set up virtual machines with virtual GPU passthrough (virtual
machines hang indefinitely when trying to make use of the virtual GPU),
and make them unable to remove the virtual GPUs once the kernel oops has
happened (it hangs indefinitely, and notably too when the kernel tries to
shutdown). The issues that this causes and steps to reproduce are
discussed in more details in this github issue post:
https://github.com/intel/gvt-linux/issues/170#issuecomment-685806160

Check if pos is equal to NULL, and if it is, set ret to a negative
value, making the module simply indicate that the access to EDID region
has failed, without any fatal repercussion.

Signed-off-by: Alejandro Sior 

---
Changes in v2:
- removed middle name of author to comply with git name
- rephrased the patch description with imperative phrasing
- removed useless paragraph
- made a paragraph more concise
- fixed typos
- made individual lines shorter than 75 chars

 drivers/gpu/drm/i915/gvt/kvmgt.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index ad8a9df49f29..49163363ba4a 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -557,7 +557,9 @@ static size_t intel_vgpu_reg_rw_edid(struct intel_vgpu 
*vgpu, char *buf,
(struct vfio_edid_region *)kvmgt_vdev(vgpu)->region[i].data;
loff_t pos = *ppos & VFIO_PCI_OFFSET_MASK;
 
-   if (pos < region->vfio_edid_regs.edid_offset) {
+   if (pos == NULL) {
+   ret = -EINVAL;
+   } else if (pos < region->vfio_edid_regs.edid_offset) {
ret = handle_edid_regs(vgpu, region, buf, count, pos, iswrite);
} else {
pos -= EDID_BLOB_OFFSET;
-- 
2.28.0

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


Re: [Intel-gfx] [PATCH v2 13/18] drm/dp: Add drm_dp_downstream_mode()

2020-09-08 Thread Lyude Paul
On Fri, 2020-09-04 at 14:53 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä 
> 
> The downstream facing port caps in the DPCD can give us a hint
> as to what kind of display mode the sink can use if it doesn't
> have an EDID. Use that information to pick a suitable mode.
> 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/drm_dp_helper.c | 54 +
>  drivers/gpu/drm/drm_edid.c  | 19 
>  include/drm/drm_dp_helper.h | 12 
>  include/drm/drm_edid.h  |  4 +++
>  4 files changed, 89 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_dp_helper.c
> b/drivers/gpu/drm/drm_dp_helper.c
> index f567428f2aef..0d5e9bcf11d0 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -808,6 +808,60 @@ int drm_dp_downstream_max_bpc(const u8
> dpcd[DP_RECEIVER_CAP_SIZE],
>  }
>  EXPORT_SYMBOL(drm_dp_downstream_max_bpc);
>  
> +/**
> + * drm_dp_downstream_mode() - return a mode for downstream facing port
> + * @dpcd: DisplayPort configuration data
> + * @port_cap: port capabilities
> + *
> + * Provides a suitable mode for downstream facing ports without EDID.
> + *
> + * Returns a new drm_display_mode on success or NULL on failure
> + */
> +struct drm_display_mode *
> +drm_dp_downstream_mode(struct drm_device *dev,
> +const u8 dpcd[DP_RECEIVER_CAP_SIZE],
> +const u8 port_cap[4])
> +
> +{
> + u8 vic;
> +
> + if (!drm_dp_is_branch(dpcd))
> + return NULL;
> +
> + if (dpcd[DP_DPCD_REV] < 0x11)
> + return NULL;
> +
> + switch (port_cap[0] & DP_DS_PORT_TYPE_MASK) {
> + case DP_DS_PORT_TYPE_NON_EDID:
> + switch (port_cap[0] & DP_DS_NON_EDID_MASK) {
> + case DP_DS_NON_EDID_720x480i_60:
> + vic = 6;
> + break;
> + case DP_DS_NON_EDID_720x480i_50:
> + vic = 21;
> + break;
> + case DP_DS_NON_EDID_1920x1080i_60:
> + vic = 5;
> + break;
> + case DP_DS_NON_EDID_1920x1080i_50:
> + vic = 20;
> + break;
> + case DP_DS_NON_EDID_1280x720_60:
> + vic = 4;
> + break;
> + case DP_DS_NON_EDID_1280x720_50:
> + vic = 19;
> + break;
> + default:
> + return NULL;
> + }
> + return drm_display_mode_from_cea_vic(dev, vic);
> + default:
> + return NULL;
> + }
> +}
> +EXPORT_SYMBOL(drm_dp_downstream_mode);
> +
>  /**
>   * drm_dp_downstream_id() - identify branch device
>   * @aux: DisplayPort AUX channel
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 6840f0530a38..b9419fed6c28 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -3738,6 +3738,25 @@ drm_add_cmdb_modes(struct drm_connector *connector,
> u8 svd)
>   bitmap_set(hdmi->y420_cmdb_modes, vic, 1);
>  }
>  
> +struct drm_display_mode *
> +drm_display_mode_from_cea_vic(struct drm_device *dev,
> +   u8 video_code)
> +{
> + const struct drm_display_mode *cea_mode;
> + struct drm_display_mode *newmode;
> +
> + cea_mode = cea_mode_for_vic(video_code);
> + if (!cea_mode)
> + return NULL;
> +
> + newmode = drm_mode_duplicate(dev, cea_mode);
> + if (!newmode)
> + return NULL;
> +
> + return newmode;
> +}
> +EXPORT_SYMBOL(drm_display_mode_from_cea_vic);

Forgot the kdocs

> +
>  static int
>  do_cea_modes(struct drm_connector *connector, const u8 *db, u8 len)
>  {
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index 6812a3e0de8d..fbba4a0f7366 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -28,6 +28,8 @@
>  #include 
>  #include 
>  
> +struct drm_device;
> +
>  /*
>   * Unless otherwise noted, all values are from the DP 1.1a spec.  Note that
>   * DP and DPCD versions are independent.  Differences from 1.0 are not
> noted,
> @@ -385,6 +387,13 @@
>  # define DP_DS_PORT_TYPE_DP_DUALMODE5
>  # define DP_DS_PORT_TYPE_WIRELESS   6
>  # define DP_DS_PORT_HPD  (1 << 3)
> +# define DP_DS_NON_EDID_MASK (0xf << 4)
> +# define DP_DS_NON_EDID_720x480i_60  (1 << 4)
> +# define DP_DS_NON_EDID_720x480i_50  (2 << 4)
> +# define DP_DS_NON_EDID_1920x1080i_60(3 << 4)
> +# define DP_DS_NON_EDID_1920x1080i_50(4 << 4)
> +# define DP_DS_NON_EDID_1280x720_60  (5 << 4)
> +# define DP_DS_NON_EDID_1280x720_50  (7 << 4)
>  /* offset 1 for VGA is maximum megapixels per second / 8 */
>  /* offset 1 for DVI/HDMI is maximum TMDS clock in Mbps / 2.5 */
>  /* offset 2 for VGA/DVI/HDMI */
> @@ -1654,6 +1663,9 @@ int drm_dp_downstream_min_tmds_clock(const u8
> dpcd[DP_R

Re: [Intel-gfx] [PATCH v2 17/18] drm/dp: Add helpers for DFP YCbCr 4:2:0 handling

2020-09-08 Thread Lyude Paul
On Fri, 2020-09-04 at 14:53 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä 
> 
> Add helpers to determine whether the DFP supports
> YCbCr 4:2:0 passthrough or YCbCr 4:4:4->4:2:0 conversion.
> 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/drm_dp_helper.c | 44 +
>  include/drm/drm_dp_helper.h |  8 ++
>  2 files changed, 52 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_dp_helper.c
> b/drivers/gpu/drm/drm_dp_helper.c
> index 0d5e9bcf11d0..dc68e10aa1fd 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -808,6 +808,50 @@ int drm_dp_downstream_max_bpc(const u8
> dpcd[DP_RECEIVER_CAP_SIZE],
>  }
>  EXPORT_SYMBOL(drm_dp_downstream_max_bpc);
>  
> +bool drm_dp_downstream_420_passthrough(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
> +const u8 port_cap[4])
> +{
> + if (!drm_dp_is_branch(dpcd))
> + return false;
> +
> + if (dpcd[DP_DPCD_REV] < 0x13)
> + return false;
> +
> + switch (port_cap[0] & DP_DS_PORT_TYPE_MASK) {
> + case DP_DS_PORT_TYPE_DP:
> + return true;
> + case DP_DS_PORT_TYPE_HDMI:
> + if ((dpcd[DP_DOWNSTREAMPORT_PRESENT] &
> DP_DETAILED_CAP_INFO_AVAILABLE) == 0)
> + return false;
> +
> + return port_cap[3] & DP_DS_HDMI_YCBCR420_PASS_THROUGH;
> + default:
> + return false;
> + }
> +}
> +EXPORT_SYMBOL(drm_dp_downstream_420_passthrough);

Forgot the kdocs again

> +
> +bool drm_dp_downstream_444_to_420_conversion(const u8
> dpcd[DP_RECEIVER_CAP_SIZE],
> +  const u8 port_cap[4])
> +{
> + if (!drm_dp_is_branch(dpcd))
> + return false;
> +
> + if (dpcd[DP_DPCD_REV] < 0x13)
> + return false;
> +
> + switch (port_cap[0] & DP_DS_PORT_TYPE_MASK) {
> + case DP_DS_PORT_TYPE_HDMI:
> + if ((dpcd[DP_DOWNSTREAMPORT_PRESENT] &
> DP_DETAILED_CAP_INFO_AVAILABLE) == 0)
> + return false;
> +
> + return port_cap[3] & DP_DS_HDMI_YCBCR444_TO_420_CONV;
> + default:
> + return false;
> + }
> +}
> +EXPORT_SYMBOL(drm_dp_downstream_444_to_420_conversion);
> +
>  /**
>   * drm_dp_downstream_mode() - return a mode for downstream facing port
>   * @dpcd: DisplayPort configuration data
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index fbba4a0f7366..c9f2851904d0 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -407,6 +407,10 @@ struct drm_device;
>  # define DP_DS_DVI_HIGH_COLOR_DEPTH  (1 << 2)
>  /* offset 3 for HDMI */
>  # define DP_DS_HDMI_FRAME_SEQ_TO_FRAME_PACK (1 << 0)
> +# define DP_DS_HDMI_YCBCR422_PASS_THROUGH   (1 << 1)
> +# define DP_DS_HDMI_YCBCR420_PASS_THROUGH   (1 << 2)
> +# define DP_DS_HDMI_YCBCR444_TO_422_CONV(1 << 3)
> +# define DP_DS_HDMI_YCBCR444_TO_420_CONV(1 << 4)
>  
>  #define DP_MAX_DOWNSTREAM_PORTS  0x10
>  
> @@ -1663,6 +1667,10 @@ int drm_dp_downstream_min_tmds_clock(const u8
> dpcd[DP_RECEIVER_CAP_SIZE],
>  int drm_dp_downstream_max_bpc(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
> const u8 port_cap[4],
> const struct edid *edid);
> +bool drm_dp_downstream_420_passthrough(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
> +const u8 port_cap[4]);
> +bool drm_dp_downstream_444_to_420_conversion(const u8
> dpcd[DP_RECEIVER_CAP_SIZE],
> +  const u8 port_cap[4]);
>  struct drm_display_mode *drm_dp_downstream_mode(struct drm_device *dev,
>   const u8
> dpcd[DP_RECEIVER_CAP_SIZE],
>   const u8 port_cap[4]);
-- 
Cheers,
Lyude Paul (she/her)
Software Engineer at Red Hat

___
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: Prevent NULL pointer dereference in intel_vgpu_reg_rw_edid()

2020-09-08 Thread Patchwork
== Series Details ==

Series: drm/i915/gvt: Prevent NULL pointer dereference in 
intel_vgpu_reg_rw_edid()
URL   : https://patchwork.freedesktop.org/series/81470/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
cc442c9d2d41 drm/i915/gvt: Prevent NULL pointer dereference in 
intel_vgpu_reg_rw_edid()
-:37: CHECK:COMPARISON_TO_NULL: Comparison to NULL could be written "!pos"
#37: FILE: drivers/gpu/drm/i915/gvt/kvmgt.c:560:
+   if (pos == NULL) {

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


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


Re: [Intel-gfx] [PATCH v2 00/18] drm/i915: Pimp DP DFP handling

2020-09-08 Thread Lyude Paul
With the nitpicks addressed (note there were a couple of other spots where we
wanted to use Return: in the kdocs, but I didn't bother pointing all of them
out), all but patch 07 is:

Reviewed-by: Lyude Paul 

On Fri, 2020-09-04 at 14:53 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä 
> 
> Attempt to deal with DP downstream facing ports (DFP) more
> thoroughly. This involves reading more of the port caps
> and dealing with various clock/bpc limitations.
> 
> And we try to enable YCbCr 444->420 conversion for HDMI DFPs
> which could allow some 4k displays to actually use 4k on
> pre-icl hardware (which doesn't have native 420 output),
> assuming we don't run into some other hardware limits.
> 
> I dropped my earlier patches to also hook in the DP dual mode
> adapter probing since sadly I've not actually seen a DP->DP++
> dongle that passes through the i2c traffic for those.
> 
> Only pimped the SST side of things. Not sure what would
> be required to get it all working for MST.
> 
> Ville Syrjälä (18):
>   drm/dp: Dump downstream facing port caps
>   drm/i915/lspcon: Do not send infoframes to non-HDMI sinks
>   drm/dp: Define protocol converter DPCD registers
>   drm/dp: Define more downstream facing port caps
>   drm/i915: Reworkd DFP max bpc handling
>   drm/dp: Add helpers to identify downstream facing port types
>   drm/dp: Pimp drm_dp_downstream_max_bpc()
>   drm/dp: Redo drm_dp_downstream_max_clock() as
> drm_dp_downstream_max_dotclock()
>   drm/i915: Reworkd DP DFP clock handling
>   drm/dp: Add drm_dp_downstream_{min,max}_tmds_clock()
>   drm/i915: Deal with TMDS DFP clock limits
>   drm/i915: Configure DP 1.3+ protocol converted HDMI mode
>   drm/dp: Add drm_dp_downstream_mode()
>   drm/i915: Handle downstream facing ports w/o EDID
>   drm/i915: Extract intel_hdmi_has_audio()
>   drm/i915: DP->HDMI TMDS clock limits vs. deep color
>   drm/dp: Add helpers for DFP YCbCr 4:2:0 handling
>   drm/i915: Do YCbCr 444->420 conversion via DP protocol converters
> 
>  drivers/gpu/drm/drm_dp_helper.c   | 382 +++---
>  drivers/gpu/drm/drm_edid.c|  19 +
>  drivers/gpu/drm/i915/display/intel_ddi.c  |  11 +-
>  .../drm/i915/display/intel_display_debugfs.c  |   3 +-
>  .../drm/i915/display/intel_display_types.h|   9 +
>  drivers/gpu/drm/i915/display/intel_dp.c   | 304 +++---
>  drivers/gpu/drm/i915/display/intel_dp.h   |   1 +
>  drivers/gpu/drm/i915/display/intel_hdmi.c |  82 ++--
>  drivers/gpu/drm/i915/display/intel_hdmi.h |   2 +
>  include/drm/drm_dp_helper.h   |  63 ++-
>  include/drm/drm_edid.h|   4 +
>  11 files changed, 738 insertions(+), 142 deletions(-)
> 
-- 
Cheers,
Lyude Paul (she/her)
Software Engineer at Red Hat

___
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: Prevent NULL pointer dereference in intel_vgpu_reg_rw_edid()

2020-09-08 Thread Patchwork
== Series Details ==

Series: drm/i915/gvt: Prevent NULL pointer dereference in 
intel_vgpu_reg_rw_edid()
URL   : https://patchwork.freedesktop.org/series/81470/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_8982 -> Patchwork_18455


Summary
---

  **SUCCESS**

  No regressions found.

  External URL: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18455/index.html

Known issues


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

### IGT changes ###

 Issues hit 

  * igt@gem_exec_gttfill@basic:
- fi-kbl-7500u:   [PASS][1] -> [INCOMPLETE][2] ([i915#2439])
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-kbl-7500u/igt@gem_exec_gttf...@basic.html
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18455/fi-kbl-7500u/igt@gem_exec_gttf...@basic.html

  * igt@i915_selftest@live@gem_execbuf:
- fi-bsw-nick:[PASS][3] -> [INCOMPLETE][4] ([i915#2439])
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-bsw-nick/igt@i915_selftest@live@gem_execbuf.html
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18455/fi-bsw-nick/igt@i915_selftest@live@gem_execbuf.html

  
 Possible fixes 

  * igt@gem_exec_gttfill@basic:
- fi-cfl-8109u:   [INCOMPLETE][5] ([i915#2439]) -> [PASS][6]
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-cfl-8109u/igt@gem_exec_gttf...@basic.html
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18455/fi-cfl-8109u/igt@gem_exec_gttf...@basic.html
- fi-bdw-5557u:   [INCOMPLETE][7] ([i915#2439]) -> [PASS][8]
   [7]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-bdw-5557u/igt@gem_exec_gttf...@basic.html
   [8]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18455/fi-bdw-5557u/igt@gem_exec_gttf...@basic.html
- fi-skl-6600u:   [INCOMPLETE][9] ([i915#2439]) -> [PASS][10]
   [9]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-skl-6600u/igt@gem_exec_gttf...@basic.html
   [10]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18455/fi-skl-6600u/igt@gem_exec_gttf...@basic.html
- fi-elk-e7500:   [INCOMPLETE][11] ([i915#66]) -> [PASS][12]
   [11]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-elk-e7500/igt@gem_exec_gttf...@basic.html
   [12]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18455/fi-elk-e7500/igt@gem_exec_gttf...@basic.html
- fi-skl-lmem:[INCOMPLETE][13] ([i915#2439]) -> [PASS][14]
   [13]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-skl-lmem/igt@gem_exec_gttf...@basic.html
   [14]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18455/fi-skl-lmem/igt@gem_exec_gttf...@basic.html
- fi-ilk-650: [INCOMPLETE][15] ([i915#2439]) -> [PASS][16]
   [15]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-ilk-650/igt@gem_exec_gttf...@basic.html
   [16]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18455/fi-ilk-650/igt@gem_exec_gttf...@basic.html

  * igt@gem_exec_parallel@engines@basic:
- fi-bsw-kefka:   [INCOMPLETE][17] ([i915#2439]) -> [PASS][18]
   [17]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-bsw-kefka/igt@gem_exec_parallel@engi...@basic.html
   [18]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18455/fi-bsw-kefka/igt@gem_exec_parallel@engi...@basic.html
- {fi-ehl-1}: [INCOMPLETE][19] ([i915#2439]) -> [PASS][20]
   [19]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-ehl-1/igt@gem_exec_parallel@engi...@basic.html
   [20]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18455/fi-ehl-1/igt@gem_exec_parallel@engi...@basic.html

  * igt@i915_selftest@live@gem_execbuf:
- fi-gdg-551: [INCOMPLETE][21] ([i915#172]) -> [PASS][22]
   [21]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-gdg-551/igt@i915_selftest@live@gem_execbuf.html
   [22]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18455/fi-gdg-551/igt@i915_selftest@live@gem_execbuf.html
- fi-snb-2520m:   [INCOMPLETE][23] ([i915#2439]) -> [PASS][24]
   [23]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-snb-2520m/igt@i915_selftest@live@gem_execbuf.html
   [24]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18455/fi-snb-2520m/igt@i915_selftest@live@gem_execbuf.html

  
 Warnings 

  * igt@gem_exec_parallel@engines@basic:
- fi-bxt-dsi: [INCOMPLETE][25] ([i915#1635]) -> [INCOMPLETE][26] 
([i915#1635] / [i915#2439])
   [25]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-bxt-dsi/igt@gem_exec_parallel@engi...@basic.html
   [26]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18455/fi-bxt-dsi/igt@gem_exec_parallel@engi...@basic.html
- fi-apl-guc: [INCOMPLETE][27] ([i915#1635]) -> [INCOMPLETE][28] 
([i915#1635] / [i915#2439])
   [27]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-apl-guc/igt@gem_exec_parallel@engi...@basic.html
   [28]: 
https://intel-

[Intel-gfx] ✗ Fi.CI.IGT: failure for drm/i915/gvt: Prevent NULL pointer dereference in intel_vgpu_reg_rw_edid()

2020-09-08 Thread Patchwork
== Series Details ==

Series: drm/i915/gvt: Prevent NULL pointer dereference in 
intel_vgpu_reg_rw_edid()
URL   : https://patchwork.freedesktop.org/series/81470/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_8982_full -> Patchwork_18455_full


Summary
---

  **FAILURE**

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

  

Possible new issues
---

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

### IGT changes ###

 Possible regressions 

  * igt@perf@enable-disable:
- shard-skl:  [PASS][1] -> [FAIL][2]
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/shard-skl8/igt@p...@enable-disable.html
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18455/shard-skl10/igt@p...@enable-disable.html

  
Known issues


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

### IGT changes ###

 Issues hit 

  * igt@gem_exec_reloc@basic-cpu-active:
- shard-skl:  [PASS][3] -> [INCOMPLETE][4] ([i915#198] / 
[i915#2439])
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/shard-skl5/igt@gem_exec_re...@basic-cpu-active.html
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18455/shard-skl3/igt@gem_exec_re...@basic-cpu-active.html

  * igt@gem_exec_reloc@basic-cpu-gtt-active:
- shard-tglb: [PASS][5] -> [INCOMPLETE][6] ([i915#2439]) +2 similar 
issues
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/shard-tglb7/igt@gem_exec_re...@basic-cpu-gtt-active.html
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18455/shard-tglb7/igt@gem_exec_re...@basic-cpu-gtt-active.html
- shard-snb:  [PASS][7] -> [INCOMPLETE][8] ([i915#2439] / 
[i915#82]) +3 similar issues
   [7]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/shard-snb2/igt@gem_exec_re...@basic-cpu-gtt-active.html
   [8]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18455/shard-snb1/igt@gem_exec_re...@basic-cpu-gtt-active.html

  * igt@gem_exec_reloc@basic-wc-active:
- shard-iclb: [PASS][9] -> [INCOMPLETE][10] ([i915#2439]) +2 
similar issues
   [9]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/shard-iclb5/igt@gem_exec_re...@basic-wc-active.html
   [10]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18455/shard-iclb8/igt@gem_exec_re...@basic-wc-active.html

  * igt@gem_exec_reloc@basic-wc-cpu-active:
- shard-apl:  [PASS][11] -> [INCOMPLETE][12] ([i915#1635] / 
[i915#2439]) +3 similar issues
   [11]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/shard-apl7/igt@gem_exec_re...@basic-wc-cpu-active.html
   [12]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18455/shard-apl3/igt@gem_exec_re...@basic-wc-cpu-active.html

  * igt@gem_exec_reloc@basic-write-read-active:
- shard-glk:  [PASS][13] -> [INCOMPLETE][14] ([i915#2439]) +3 
similar issues
   [13]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/shard-glk7/igt@gem_exec_re...@basic-write-read-active.html
   [14]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18455/shard-glk1/igt@gem_exec_re...@basic-write-read-active.html

  * igt@gem_exec_schedule@deep@rcs0:
- shard-skl:  [PASS][15] -> [INCOMPLETE][16] ([i915#2439]) +2 
similar issues
   [15]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/shard-skl10/igt@gem_exec_schedule@d...@rcs0.html
   [16]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18455/shard-skl5/igt@gem_exec_schedule@d...@rcs0.html

  * igt@gem_exec_whisper@basic-contexts-all:
- shard-hsw:  [PASS][17] -> [INCOMPLETE][18] ([i915#2439])
   [17]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/shard-hsw8/igt@gem_exec_whis...@basic-contexts-all.html
   [18]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18455/shard-hsw1/igt@gem_exec_whis...@basic-contexts-all.html

  * igt@gem_exec_whisper@basic-forked:
- shard-kbl:  [PASS][19] -> [INCOMPLETE][20] ([i915#2439]) +2 
similar issues
   [19]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/shard-kbl2/igt@gem_exec_whis...@basic-forked.html
   [20]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18455/shard-kbl1/igt@gem_exec_whis...@basic-forked.html

  * igt@gem_userptr_blits@unsync-unmap-cycles:
- shard-skl:  [PASS][21] -> [TIMEOUT][22] ([i915#1958])
   [21]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/shard-skl3/igt@gem_userptr_bl...@unsync-unmap-cycles.html
   [22]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18455/shard-skl3/igt@gem_userptr_bl...@unsync-unmap-cycles.html

  * igt@i915_suspend@forcewake:
-

Re: [Intel-gfx] [PATCH] drm/i915/pll: Centralize PLL_ENABLE register lookup

2020-09-08 Thread Vivi, Rodrigo


> On Sep 3, 2020, at 10:04 AM, Srivatsa, Anusha  
> wrote:
> 
> 
> 
>> -Original Message-
>> From: Vivi, Rodrigo 
>> Sent: Wednesday, September 2, 2020 2:32 PM
>> To: Srivatsa, Anusha 
>> Cc: intel-gfx@lists.freedesktop.org
>> Subject: Re: [Intel-gfx] [PATCH] drm/i915/pll: Centralize PLL_ENABLE register
>> lookup
>> 
>> 
>> 
>>> On Sep 2, 2020, at 12:30 PM, Srivatsa, Anusha
>>  wrote:
>>> 
>>> 
>>> 
 -Original Message-
 From: Rodrigo Vivi 
 Sent: Tuesday, September 1, 2020 12:30 PM
 To: Srivatsa, Anusha 
 Cc: intel-gfx@lists.freedesktop.org
 Subject: Re: [Intel-gfx] [PATCH] drm/i915/pll: Centralize PLL_ENABLE
 register lookup
 
 On Tue, Sep 01, 2020 at 11:27:58AM -0700, Anusha Srivatsa wrote:
> We currenty check for platform at multiple parts in the driver to
> grab the correct PLL. Let us begin to centralize it through a helper
> function.
> 
> v2: s/intel_get_pll_enable_reg()/intel_combo_pll_enable_reg()
> (Ville)
> 
> Suggested-by: Matt Roper 
> Cc: Ville Syrjälä 
> Cc: Matt Roper 
> Signed-off-by: Anusha Srivatsa 
> ---
> drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 25
> +++
> 1 file changed, 15 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> index c9013f8f766f..7440836c5e44 100644
> --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> @@ -147,6 +147,18 @@ void assert_shared_dpll(struct
>> drm_i915_private
 *dev_priv,
>   pll->info->name, onoff(state), onoff(cur_state));  }
> 
> +static
> +i915_reg_t intel_combo_pll_enable_reg(struct drm_i915_private
 *dev_priv,
> + struct intel_shared_dpll *pll) {
> +
> + if (IS_ELKHARTLAKE(dev_priv) && (pll->info->id ==
 DPLL_ID_EHL_DPLL4))
> + return MG_PLL_ENABLE(0);
> +
> + return CNL_DPLL_ENABLE(pll->info->id);
> +
> +
> +}
> /**
> * intel_prepare_shared_dpll - call a dpll's prepare hook
> * @crtc_state: CRTC, and its state, which has a shared dpll @@
> -3842,12 +3854,7 @@ static bool combo_pll_get_hw_state(struct
 drm_i915_private *dev_priv,
>  struct intel_shared_dpll *pll,
>  struct intel_dpll_hw_state *hw_state)  {
> - i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
> -
> - if (IS_ELKHARTLAKE(dev_priv) &&
> - pll->info->id == DPLL_ID_EHL_DPLL4) {
> - enable_reg = MG_PLL_ENABLE(0);
> - }
> + i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll);
> 
>   return icl_pll_get_hw_state(dev_priv, pll, hw_state, enable_reg);
> } @@ -4045,11 +4052,10 @@ static void icl_pll_enable(struct
> drm_i915_private *dev_priv,  static void combo_pll_enable(struct
 drm_i915_private *dev_priv,
>struct intel_shared_dpll *pll)  {
> - i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
> + i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll);
> 
>   if (IS_ELKHARTLAKE(dev_priv) &&
>   pll->info->id == DPLL_ID_EHL_DPLL4) {
 
 there's probably something else that we can do now with the
 power_{put,get} to get rid of the, now, doubled if checks...
>>> 
>>> Don't follow you here Rodrigo.
>> 
>> me neither ;)
>> I'm just brainstorming... thinking out lout.
>> 
>>> Are you suggesting using power_{put/get} to somehow get rid of doubled
>> if?
>> 
>> after this patch, on this path we will do this if check twice.
>> not a big deal, but we can probably do something better.
>> 
>> However I don't understand why we had this get/put here at first place.
>> Only for this platform and only for this pll4. So, what I am wondering is 
>> that
>> we have something better to do with the power_well infrastructure in
>> general that would allow us to avoid the if (platform && pll4) in favor of
>> something more generic.
>> 
>> but definitely not a blocker for this patch itself.
> Ok. So maybe the power well infrastructure change can be part a later patch?

sure

> 
>> 
>>> 
> - enable_reg = MG_PLL_ENABLE(0);
> 
>   /*
>* We need to disable DC states when this DPLL is enabled.
> @@ -4157,11 +4163,10 @@ static void icl_pll_disable(struct
> drm_i915_private *dev_priv,  static void combo_pll_disable(struct
 drm_i915_private *dev_priv,
> struct intel_shared_dpll *pll)  {
> - i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
> + i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll);
> 
>   if (IS_ELKHARTLAKE(dev_priv) &&
>   pll->info->id == DPLL_ID_EHL_DPLL4) {
> - enable_reg = MG_PLL_ENABLE(0);
>

Re: [Intel-gfx] [PATCH 1/2] drm/i915/gem: Replace reloc chain with terminator on error unwind

2020-09-08 Thread Pavel Machek
Hi!

> > > > Thanks for the patches. I assume this should fix problem from
> > > > "5.9-rc1: graphics regression moved from -next to mainline" thread.
> > > > 
> > > > I have applied them over current -next, and my machine seems to be
> > > > working so far (but uptime is less than 30 minutes).
> > > > 
> > > > If the machine still works tommorow, I'll assume problem is solved.
> > > 
> > > Aye, best wait until we have to start competing with Chromium for
> > > memory... The suspicion is that it was the resource allocation failure
> > > path.
> > 
> > Yep, my machines are low on memory.
> > 
> > But ... test did not work that well. I have dead X and blinking
> > screen. Machine still works reasonably well over ssh, so I guess
> > that's an improvement.
> 
> Well my last remaining 32bit gen3 device is currently pushing up the
> daises, so could you try removing the attempt to use WC? Something like
> 
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> @@ -955,10 +955,7 @@ static u32 *__reloc_gpu_map(struct reloc_cache *cache,
>  {
> u32 *map;
> 
> -   map = i915_gem_object_pin_map(pool->obj,
> - cache->has_llc ?
> - I915_MAP_FORCE_WB :
> - I915_MAP_FORCE_WC);
> +   map = i915_gem_object_pin_map(pool->obj, I915_MAP_FORCE_WB);
> 
> on top of the previous patch. Faultinjection didn't turn up anything in
> eb_relocate_vma, so we need to dig deeper.

With this on top of other patches, it works.

Tested-by: Pavel Machek 

Best regards,
Pavel

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


Re: [Intel-gfx] [PATCH 05/24] drm/vkms: Use devm_drm_dev_alloc

2020-09-08 Thread Melissa Wen
Hi Daniel,

Thanks for this work.

This change works smoothly to me. However, there is a check in the vkms_exit
that doesn't look very good. Please, see inline comment.

On 09/04, Daniel Vetter wrote:
> This means we also need to slightly restructure the exit code, so that
> final cleanup of the drm_device is triggered by unregistering the
> platform device. Note that devres is both clean up when the driver is
> unbound (not the case for vkms, we don't bind), and also when unregistering
> the device (very much the case for vkms). Therefore we can rely on devres
> even though vkms isn't a proper platform device driver.
> 
> This also somewhat untangles the load code, since the drm and platform device
> setup are no longer interleaved, but two distinct steps.
> 
> v2: use devres_open/release_group so we can use devm without real
> hacks in the driver core or having to create an entire fake bus for
> testing drivers. Might want to extract this into helpers eventually,
> maybe as a mock_drm_dev_alloc or test_drm_dev_alloc.
> 
> Signed-off-by: Daniel Vetter 
> Cc: Rodrigo Siqueira 
> Cc: Haneen Mohammed 
> Cc: Daniel Vetter 
> ---
>  drivers/gpu/drm/vkms/vkms_drv.c | 54 -
>  1 file changed, 26 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
> index 83dd5567de8b..0d2c6dcf73c3 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.c
> +++ b/drivers/gpu/drm/vkms/vkms_drv.c
> @@ -61,9 +61,6 @@ static void vkms_release(struct drm_device *dev)
>  {
>   struct vkms_device *vkms = container_of(dev, struct vkms_device, drm);
>  
> - platform_device_unregister(vkms->platform);
> - drm_atomic_helper_shutdown(&vkms->drm);
> - drm_mode_config_cleanup(&vkms->drm);
>   destroy_workqueue(vkms->output.composer_workq);
>  }
>  
> @@ -144,30 +141,31 @@ static int vkms_modeset_init(struct vkms_device 
> *vkmsdev)
>  static int __init vkms_init(void)
>  {
>   int ret;
> + struct platform_device *pdev;
>  
> - vkms_device = kzalloc(sizeof(*vkms_device), GFP_KERNEL);
> - if (!vkms_device)
> - return -ENOMEM;
> + pdev = platform_device_register_simple(DRIVER_NAME, -1, NULL, 0);
> + if (IS_ERR(pdev))
> + return PTR_ERR(pdev);
>  
> - vkms_device->platform =
> - platform_device_register_simple(DRIVER_NAME, -1, NULL, 0);
> - if (IS_ERR(vkms_device->platform)) {
> - ret = PTR_ERR(vkms_device->platform);
> - goto out_free;
> + if (!devres_open_group(&pdev->dev, NULL, GFP_KERNEL)) {
> + ret = -ENOMEM;
> + goto out_unregister;
>   }
>  
> - ret = drm_dev_init(&vkms_device->drm, &vkms_driver,
> -&vkms_device->platform->dev);
> - if (ret)
> - goto out_unregister;
> - drmm_add_final_kfree(&vkms_device->drm, vkms_device);
> + vkms_device = devm_drm_dev_alloc(&pdev->dev, &vkms_driver,
> +  struct vkms_device, drm);
> + if (IS_ERR(vkms_device)) {
> + ret = PTR_ERR(vkms_device);
> + goto out_devres;
> + }
> + vkms_device->platform = pdev;
>  
>   ret = dma_coerce_mask_and_coherent(vkms_device->drm.dev,
>  DMA_BIT_MASK(64));
>  
>   if (ret) {
>   DRM_ERROR("Could not initialize DMA support\n");
> - goto out_put;
> + goto out_devres;
>   }
>  
>   vkms_device->drm.irq_enabled = true;
> @@ -175,39 +173,39 @@ static int __init vkms_init(void)
>   ret = drm_vblank_init(&vkms_device->drm, 1);
>   if (ret) {
>   DRM_ERROR("Failed to vblank\n");
> - goto out_put;
> + goto out_devres;
>   }
>  
>   ret = vkms_modeset_init(vkms_device);
>   if (ret)
> - goto out_put;
> + goto out_devres;
>  
>   ret = drm_dev_register(&vkms_device->drm, 0);
>   if (ret)
> - goto out_put;
> + goto out_devres;
>  
>   return 0;
>  
> -out_put:
> - drm_dev_put(&vkms_device->drm);
> - platform_device_unregister(vkms_device->platform);
> - return ret;
> +out_devres:
> + devres_release_group(&pdev->dev, NULL);
>  out_unregister:
> - platform_device_unregister(vkms_device->platform);
> -out_free:
> - kfree(vkms_device);
> + platform_device_unregister(pdev);
>   return ret;
>  }
>  
>  static void __exit vkms_exit(void)
>  {
> + struct platform_device *pdev = vkms_device->platform;
> +
>   if (!vkms_device) {
>   DRM_INFO("vkms_device is NULL.\n");
>   return;
>   }

I think it would be better to check vkms_device before assigning
vkms_device->platform to the pdev above. I don't know if the compiler
handles this, but doing the validation first seems more clear.

>  
>   drm_dev_unregister(&vkms_device->drm);
> - drm_dev_put(&vkms_device->drm);
> + d

[Intel-gfx] [PATCH] drm/i915/pll: Centralize PLL_ENABLE register lookup

2020-09-08 Thread Anusha Srivatsa
We currenty check for platform at multiple parts in the driver
to grab the correct PLL. Let us begin to centralize it through a
helper function.

v2: s/intel_get_pll_enable_reg()/intel_combo_pll_enable_reg() (Ville)

v3: Clean up combo_pll_disable() (Rodrigo)

Suggested-by: Matt Roper 
Cc: Ville Syrjälä 
Cc: Matt Roper 
Signed-off-by: Anusha Srivatsa 
Reviewed-by: Rodrigo Vivi 
---
 drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 29 +++
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c 
b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
index c9013f8f766f..441b6f52e808 100644
--- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
@@ -147,6 +147,18 @@ void assert_shared_dpll(struct drm_i915_private *dev_priv,
pll->info->name, onoff(state), onoff(cur_state));
 }
 
+static
+i915_reg_t intel_combo_pll_enable_reg(struct drm_i915_private *dev_priv,
+   struct intel_shared_dpll *pll)
+{
+
+   if (IS_ELKHARTLAKE(dev_priv) && (pll->info->id == DPLL_ID_EHL_DPLL4))
+   return MG_PLL_ENABLE(0);
+
+   return CNL_DPLL_ENABLE(pll->info->id);
+
+
+}
 /**
  * intel_prepare_shared_dpll - call a dpll's prepare hook
  * @crtc_state: CRTC, and its state, which has a shared dpll
@@ -3842,12 +3854,7 @@ static bool combo_pll_get_hw_state(struct 
drm_i915_private *dev_priv,
   struct intel_shared_dpll *pll,
   struct intel_dpll_hw_state *hw_state)
 {
-   i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
-
-   if (IS_ELKHARTLAKE(dev_priv) &&
-   pll->info->id == DPLL_ID_EHL_DPLL4) {
-   enable_reg = MG_PLL_ENABLE(0);
-   }
+   i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll);
 
return icl_pll_get_hw_state(dev_priv, pll, hw_state, enable_reg);
 }
@@ -4045,11 +4052,10 @@ static void icl_pll_enable(struct drm_i915_private 
*dev_priv,
 static void combo_pll_enable(struct drm_i915_private *dev_priv,
 struct intel_shared_dpll *pll)
 {
-   i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
+   i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll);
 
if (IS_ELKHARTLAKE(dev_priv) &&
pll->info->id == DPLL_ID_EHL_DPLL4) {
-   enable_reg = MG_PLL_ENABLE(0);
 
/*
 * We need to disable DC states when this DPLL is enabled.
@@ -4157,19 +4163,18 @@ static void icl_pll_disable(struct drm_i915_private 
*dev_priv,
 static void combo_pll_disable(struct drm_i915_private *dev_priv,
  struct intel_shared_dpll *pll)
 {
-   i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
+   i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll);
+
+   icl_pll_disable(dev_priv, pll, enable_reg);
 
if (IS_ELKHARTLAKE(dev_priv) &&
pll->info->id == DPLL_ID_EHL_DPLL4) {
-   enable_reg = MG_PLL_ENABLE(0);
-   icl_pll_disable(dev_priv, pll, enable_reg);
 
intel_display_power_put(dev_priv, POWER_DOMAIN_DPLL_DC_OFF,
pll->wakeref);
return;
}
 
-   icl_pll_disable(dev_priv, pll, enable_reg);
 }
 
 static void tbt_pll_disable(struct drm_i915_private *dev_priv,
-- 
2.25.0

___
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/pll: Centralize PLL_ENABLE register lookup (rev3)

2020-09-08 Thread Patchwork
== Series Details ==

Series: drm/i915/pll: Centralize PLL_ENABLE register lookup (rev3)
URL   : https://patchwork.freedesktop.org/series/81150/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
3be3651c4fb7 drm/i915/pll: Centralize PLL_ENABLE register lookup
-:33: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#33: FILE: drivers/gpu/drm/i915/display/intel_dpll_mgr.c:152:
+i915_reg_t intel_combo_pll_enable_reg(struct drm_i915_private *dev_priv,
+   struct intel_shared_dpll *pll)

-:35: CHECK:BRACES: Blank lines aren't necessary after an open brace '{'
#35: FILE: drivers/gpu/drm/i915/display/intel_dpll_mgr.c:154:
+{
+

-:36: WARNING:SUSPECT_CODE_INDENT: suspect code indent for conditional 
statements (8, 24)
#36: FILE: drivers/gpu/drm/i915/display/intel_dpll_mgr.c:155:
+   if (IS_ELKHARTLAKE(dev_priv) && (pll->info->id == DPLL_ID_EHL_DPLL4))
+   return MG_PLL_ENABLE(0);

-:36: CHECK:UNNECESSARY_PARENTHESES: Unnecessary parentheses around 
'pll->info->id == DPLL_ID_EHL_DPLL4'
#36: FILE: drivers/gpu/drm/i915/display/intel_dpll_mgr.c:155:
+   if (IS_ELKHARTLAKE(dev_priv) && (pll->info->id == DPLL_ID_EHL_DPLL4))

-:41: CHECK:LINE_SPACING: Please don't use multiple blank lines
#41: FILE: drivers/gpu/drm/i915/display/intel_dpll_mgr.c:160:
+
+

-:42: CHECK:BRACES: Blank lines aren't necessary before a close brace '}'
#42: FILE: drivers/gpu/drm/i915/display/intel_dpll_mgr.c:161:
+
+}

total: 0 errors, 1 warnings, 5 checks, 65 lines checked


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


[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/pll: Centralize PLL_ENABLE register lookup (rev3)

2020-09-08 Thread Patchwork
== Series Details ==

Series: drm/i915/pll: Centralize PLL_ENABLE register lookup (rev3)
URL   : https://patchwork.freedesktop.org/series/81150/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_8985 -> Patchwork_18456


Summary
---

  **FAILURE**

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

  External URL: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18456/index.html

Possible new issues
---

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

### IGT changes ###

 Possible regressions 

  * igt@gem_ctx_create@basic-files:
- fi-tgl-u2:  [PASS][1] -> [INCOMPLETE][2]
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8985/fi-tgl-u2/igt@gem_ctx_cre...@basic-files.html
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18456/fi-tgl-u2/igt@gem_ctx_cre...@basic-files.html

  
Known issues


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

### IGT changes ###

 Issues hit 

  * igt@gem_exec_gttfill@basic:
- fi-kbl-7500u:   [PASS][3] -> [INCOMPLETE][4] ([i915#2439])
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8985/fi-kbl-7500u/igt@gem_exec_gttf...@basic.html
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18456/fi-kbl-7500u/igt@gem_exec_gttf...@basic.html
- fi-elk-e7500:   [PASS][5] -> [INCOMPLETE][6] ([i915#2439] / [i915#66])
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8985/fi-elk-e7500/igt@gem_exec_gttf...@basic.html
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18456/fi-elk-e7500/igt@gem_exec_gttf...@basic.html
- fi-ilk-650: [PASS][7] -> [INCOMPLETE][8] ([i915#2439])
   [7]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8985/fi-ilk-650/igt@gem_exec_gttf...@basic.html
   [8]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18456/fi-ilk-650/igt@gem_exec_gttf...@basic.html

  * igt@i915_selftest@live@gem_execbuf:
- fi-gdg-551: [PASS][9] -> [INCOMPLETE][10] ([i915#172] / 
[i915#2440])
   [9]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8985/fi-gdg-551/igt@i915_selftest@live@gem_execbuf.html
   [10]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18456/fi-gdg-551/igt@i915_selftest@live@gem_execbuf.html
- fi-snb-2600:[PASS][11] -> [INCOMPLETE][12] ([i915#2440] / 
[i915#82])
   [11]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8985/fi-snb-2600/igt@i915_selftest@live@gem_execbuf.html
   [12]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18456/fi-snb-2600/igt@i915_selftest@live@gem_execbuf.html
- fi-bsw-n3050:   [PASS][13] -> [INCOMPLETE][14] ([i915#2439])
   [13]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8985/fi-bsw-n3050/igt@i915_selftest@live@gem_execbuf.html
   [14]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18456/fi-bsw-n3050/igt@i915_selftest@live@gem_execbuf.html
- fi-snb-2520m:   [PASS][15] -> [INCOMPLETE][16] ([i915#2439])
   [15]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8985/fi-snb-2520m/igt@i915_selftest@live@gem_execbuf.html
   [16]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18456/fi-snb-2520m/igt@i915_selftest@live@gem_execbuf.html

  
 Possible fixes 

  * igt@gem_exec_gttfill@basic:
- fi-kbl-r:   [INCOMPLETE][17] ([i915#2439]) -> [PASS][18]
   [17]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8985/fi-kbl-r/igt@gem_exec_gttf...@basic.html
   [18]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18456/fi-kbl-r/igt@gem_exec_gttf...@basic.html
- fi-bdw-5557u:   [INCOMPLETE][19] ([i915#2439]) -> [PASS][20]
   [19]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8985/fi-bdw-5557u/igt@gem_exec_gttf...@basic.html
   [20]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18456/fi-bdw-5557u/igt@gem_exec_gttf...@basic.html

  * igt@gem_tiled_fence_blits@basic:
- fi-pnv-d510:[INCOMPLETE][21] ([i915#2439] / [i915#299]) -> 
[PASS][22]
   [21]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8985/fi-pnv-d510/igt@gem_tiled_fence_bl...@basic.html
   [22]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18456/fi-pnv-d510/igt@gem_tiled_fence_bl...@basic.html

  
 Warnings 

  * igt@runner@aborted:
- fi-kbl-r:   [FAIL][23] ([i915#1186] / [i915#1784] / [i915#2439]) 
-> [FAIL][24] ([i915#2398] / [i915#2439])
   [23]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8985/fi-kbl-r/igt@run...@aborted.html
   [24]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18456/fi-kbl-r/igt@run...@aborted.html
- fi-kbl-7500u:   [FAIL][25] ([i915#1784] / [i915#2398] / [i915#2439]) 
-> [FA

Re: [Intel-gfx] [11/12] drm/i915: Introduce intel_hpd_hotplug_irqs()

2020-09-08 Thread Souza, Jose
On Wed, 2020-07-01 at 00:56 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <
> ville.syrj...@linux.intel.com
> >
> 
> Introduce intel_hpd_hotplug_irqs() as a partner to
> intel_hpd_enabled_irqs(). There's no need to care about the
> encoders which we're not exposing, so we can avoid hardocoding

hard-coding

> the masks in various places.

Pretty nice patch, you only missed to do this change in the irq_handlers so we 
could nuke the SDE_DDI_MASKs, or are you planning to do this in a
follow up patch? If later consider this

Reviewed-by: José Roberto de Souza 

> 
> Signed-off-by: Ville Syrjälä <
> ville.syrj...@linux.intel.com
> >
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 50 +++--
>  1 file changed, 23 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 95ab4432a87d..b8a6a21f4c54 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -2943,6 +2943,18 @@ static u32 intel_hpd_enabled_irqs(struct 
> drm_i915_private *dev_priv,
>   return enabled_irqs;
>  }
>  
> +static u32 intel_hpd_hotplug_irqs(struct drm_i915_private *dev_priv,
> +   const u32 hpd[HPD_NUM_PINS])
> +{
> + struct intel_encoder *encoder;
> + u32 hotplug_irqs = 0;
> +
> + for_each_intel_encoder(&dev_priv->drm, encoder)
> + hotplug_irqs |= hpd[encoder->hpd_pin];
> +
> + return hotplug_irqs;
> +}
> +
>  static void ibx_hpd_detection_setup(struct drm_i915_private *dev_priv)
>  {
>   u32 hotplug;
> @@ -2972,12 +2984,8 @@ static void ibx_hpd_irq_setup(struct drm_i915_private 
> *dev_priv)
>  {
>   u32 hotplug_irqs, enabled_irqs;
>  
> - if (HAS_PCH_IBX(dev_priv))
> - hotplug_irqs = SDE_HOTPLUG_MASK;
> - else
> - hotplug_irqs = SDE_HOTPLUG_MASK_CPT;
> -
>   enabled_irqs = intel_hpd_enabled_irqs(dev_priv, 
> dev_priv->hotplug.pch_hpd);
> + hotplug_irqs = intel_hpd_hotplug_irqs(dev_priv, 
> dev_priv->hotplug.pch_hpd);
>  
>   ibx_display_interrupt_update(dev_priv, hotplug_irqs, enabled_irqs);
>  
> @@ -3005,13 +3013,12 @@ static void icp_tc_hpd_detection_setup(struct 
> drm_i915_private *dev_priv,
>  }
>  
>  static void icp_hpd_irq_setup(struct drm_i915_private *dev_priv,
> -   u32 sde_ddi_mask, u32 sde_tc_mask,
> u32 ddi_enable_mask, u32 tc_enable_mask)
>  {
>   u32 hotplug_irqs, enabled_irqs;
>  
> - hotplug_irqs = sde_ddi_mask | sde_tc_mask;
>   enabled_irqs = intel_hpd_enabled_irqs(dev_priv, 
> dev_priv->hotplug.pch_hpd);
> + hotplug_irqs = intel_hpd_hotplug_irqs(dev_priv, 
> dev_priv->hotplug.pch_hpd);
>  
>   I915_WRITE(SHPD_FILTER_CNT, SHPD_FILTER_CNT_500_ADJ);
>  
> @@ -3029,7 +3036,6 @@ static void icp_hpd_irq_setup(struct drm_i915_private 
> *dev_priv,
>  static void mcc_hpd_irq_setup(struct drm_i915_private *dev_priv)
>  {
>   icp_hpd_irq_setup(dev_priv,
> -   SDE_DDI_MASK_ICP, SDE_TC_HOTPLUG_ICP(PORT_TC1),
> ICP_DDI_HPD_ENABLE_MASK, ICP_TC_HPD_ENABLE(PORT_TC1));
>  }
>  
> @@ -3041,7 +3047,6 @@ static void mcc_hpd_irq_setup(struct drm_i915_private 
> *dev_priv)
>  static void jsp_hpd_irq_setup(struct drm_i915_private *dev_priv)
>  {
>   icp_hpd_irq_setup(dev_priv,
> -   SDE_DDI_MASK_TGP, 0,
> TGP_DDI_HPD_ENABLE_MASK, 0);
>  }
>  
> @@ -3074,7 +3079,7 @@ static void gen11_hpd_irq_setup(struct drm_i915_private 
> *dev_priv)
>   u32 val;
>  
>   enabled_irqs = intel_hpd_enabled_irqs(dev_priv, dev_priv->hotplug.hpd);
> - hotplug_irqs = GEN11_DE_TC_HOTPLUG_MASK | GEN11_DE_TBT_HOTPLUG_MASK;
> + hotplug_irqs = intel_hpd_hotplug_irqs(dev_priv, dev_priv->hotplug.hpd);
>  
>   val = I915_READ(GEN11_DE_HPD_IMR);
>   val &= ~hotplug_irqs;
> @@ -3085,10 +3090,10 @@ static void gen11_hpd_irq_setup(struct 
> drm_i915_private *dev_priv)
>   gen11_hpd_detection_setup(dev_priv);
>  
>   if (INTEL_PCH_TYPE(dev_priv) >= PCH_TGP)
> - icp_hpd_irq_setup(dev_priv, SDE_DDI_MASK_TGP, SDE_TC_MASK_TGP,
> + icp_hpd_irq_setup(dev_priv,
> TGP_DDI_HPD_ENABLE_MASK, 
> TGP_TC_HPD_ENABLE_MASK);
>   else if (INTEL_PCH_TYPE(dev_priv) >= PCH_ICP)
> - icp_hpd_irq_setup(dev_priv, SDE_DDI_MASK_ICP, SDE_TC_MASK_ICP,
> + icp_hpd_irq_setup(dev_priv,
> ICP_DDI_HPD_ENABLE_MASK, 
> ICP_TC_HPD_ENABLE_MASK);
>  }
>  
> @@ -3124,8 +3129,8 @@ static void spt_hpd_irq_setup(struct drm_i915_private 
> *dev_priv)
>   if (INTEL_PCH_TYPE(dev_priv) >= PCH_CNP)
>   I915_WRITE(SHPD_FILTER_CNT, SHPD_FILTER_CNT_500_ADJ);
>  
> - hotplug_irqs = SDE_HOTPLUG_MASK_SPT;
>   enabled_irqs = intel_hpd_enabled_irqs(dev_priv, 
> dev_priv->hotplug.pch_hpd);
> + hotplug_irqs = intel_hpd_hotplug_irqs(dev_priv, 
> dev_priv->hotpl

Re: [Intel-gfx] [PATCH] drm/i915/gvt: Introduce per object locking in GVT scheduler.

2020-09-08 Thread Colin Xu
I tested this patch on the suspend/resume case with vGPU created (no 
need really activate), can still observer the system freeze issue as 
mentioned in another patch I sent. So I suppose we still need decouple 
context pin/unpin with submission setup/clean, but move to workload 
create/destroy?


After made similar changes based on this one, plus the suspend/resume 
support patch, below tests can pass:


- Create vGPU then suspend/resume.

- Run VM w/ vGPU then suspend/resume.

https://lists.freedesktop.org/archives/intel-gvt-dev/2020-September/007061.html

On 2020-09-08 04:02, Zhi Wang wrote:

To support ww locking and per-object implemented in i915, GVT scheduler needs
to be refined. Most of the changes are located in shadow batch buffer, shadow
wa context in GVT-g, where use quite a lot of i915 gem object APIs.

Cc: Maarten Lankhorst 
Cc: Joonas Lahtinen 
Cc: Zhenyu Wang 
Signed-off-by: Zhi Wang 
---
  drivers/gpu/drm/i915/gvt/scheduler.c | 68 ++--
  1 file changed, 57 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c 
b/drivers/gpu/drm/i915/gvt/scheduler.c
index 1570eb8..fe7ee10 100644
--- a/drivers/gpu/drm/i915/gvt/scheduler.c
+++ b/drivers/gpu/drm/i915/gvt/scheduler.c
@@ -396,7 +396,9 @@ static void release_shadow_wa_ctx(struct 
intel_shadow_wa_ctx *wa_ctx)
if (!wa_ctx->indirect_ctx.obj)
return;
  
+	i915_gem_object_lock(wa_ctx->indirect_ctx.obj, NULL);

i915_gem_object_unpin_map(wa_ctx->indirect_ctx.obj);
+   i915_gem_object_unlock(wa_ctx->indirect_ctx.obj);
i915_gem_object_put(wa_ctx->indirect_ctx.obj);
  
  	wa_ctx->indirect_ctx.obj = NULL;

@@ -504,6 +506,7 @@ static int prepare_shadow_batch_buffer(struct 
intel_vgpu_workload *workload)
struct intel_gvt *gvt = workload->vgpu->gvt;
const int gmadr_bytes = gvt->device_info.gmadr_bytes_in_cmd;
struct intel_vgpu_shadow_bb *bb;
+   struct i915_gem_ww_ctx ww;
int ret;
  
  	list_for_each_entry(bb, &workload->shadow_bb, list) {

@@ -528,10 +531,19 @@ static int prepare_shadow_batch_buffer(struct 
intel_vgpu_workload *workload)
 * directly
 */
if (!bb->ppgtt) {
-   bb->vma = i915_gem_object_ggtt_pin(bb->obj,
-  NULL, 0, 0, 0);
+   i915_gem_ww_ctx_init(&ww, false);
+retry:
+   i915_gem_object_lock(bb->obj, &ww);
+
+   bb->vma = i915_gem_object_ggtt_pin_ww(bb->obj, &ww,
+ NULL, 0, 0, 0);
if (IS_ERR(bb->vma)) {
ret = PTR_ERR(bb->vma);
+   if (ret == -EDEADLK) {
+   ret = i915_gem_ww_ctx_backoff(&ww);
+   if (!ret)
+   goto retry;
+   }
goto err;
}
  
@@ -545,13 +557,18 @@ static int prepare_shadow_batch_buffer(struct intel_vgpu_workload *workload)

  0);
if (ret)
goto err;
+
+   /* No one is going to touch shadow bb from now on. */
+   i915_gem_object_flush_map(bb->obj);
+
+   i915_gem_object_unlock(bb->obj);
+   i915_gem_ww_ctx_fini(&ww);
}
  
-		/* No one is going to touch shadow bb from now on. */

-   i915_gem_object_flush_map(bb->obj);
}
return 0;
  err:
+   i915_gem_ww_ctx_fini(&ww);
release_shadow_batch_buffer(workload);
return ret;
  }
@@ -578,14 +595,30 @@ static int prepare_shadow_wa_ctx(struct 
intel_shadow_wa_ctx *wa_ctx)
unsigned char *per_ctx_va =
(unsigned char *)wa_ctx->indirect_ctx.shadow_va +
wa_ctx->indirect_ctx.size;
+   struct i915_gem_ww_ctx ww;
+   int ret;
  
  	if (wa_ctx->indirect_ctx.size == 0)

return 0;
  
-	vma = i915_gem_object_ggtt_pin(wa_ctx->indirect_ctx.obj, NULL,

-  0, CACHELINE_BYTES, 0);
-   if (IS_ERR(vma))
-   return PTR_ERR(vma);
+   i915_gem_ww_ctx_init(&ww, false);
+retry:
+   i915_gem_object_lock(wa_ctx->indirect_ctx.obj, &ww);
+
+   vma = i915_gem_object_ggtt_pin_ww(wa_ctx->indirect_ctx.obj, &ww, NULL,
+ 0, CACHELINE_BYTES, 0);
+   if (IS_ERR(vma)) {
+   ret = PTR_ERR(vma);
+   if (ret == -EDEADLK) {
+   ret = i915_gem_ww_ctx_backoff(&ww);
+   if (!ret)
+   goto retry;
+   }
+   return ret;
+   }
+
+   i915_gem_obj

Re: [Intel-gfx] [PATCH V2 5/5] DO NOT MERGE: iommu: disable list appending in dma-iommu

2020-09-08 Thread Lu Baolu

Hi Christoph,

On 9/8/20 2:23 PM, Christoph Hellwig wrote:

On Tue, Sep 08, 2020 at 02:04:53PM +0800, Lu Baolu wrote:

Do you mind telling where can I find Marek's series?


[PATCH v10 00/30] DRM: fix struct sg_table nents vs. orig_nents misuse

on various lists including the iommu one.



It seems that more work is needed in i915 driver. I will added below
quirk as you suggested.

--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -851,6 +851,31 @@ static int __finalise_sg(struct device *dev, struct 
scatterlist *sg, int nents,

unsigned int cur_len = 0, max_len = dma_get_max_seg_size(dev);
int i, count = 0;

+   /*
+* The Intel graphic device driver is used to assume that the 
returned
+* sg list is not combound. This blocks the efforts of 
converting the

+* Intel IOMMU driver to dma-iommu api's. Add this quirk to make the
+* device driver work and should be removed once it's fixed in i915
+* driver.
+*/
+   if (dev_is_pci(dev) &&
+   to_pci_dev(dev)->vendor == PCI_VENDOR_ID_INTEL &&
+   (to_pci_dev(dev)->class >> 16) == PCI_BASE_CLASS_DISPLAY) {
+   for_each_sg(sg, s, nents, i) {
+   unsigned int s_iova_off = sg_dma_address(s);
+   unsigned int s_length = sg_dma_len(s);
+   unsigned int s_iova_len = s->length;
+
+   s->offset += s_iova_off;
+   s->length = s_length;
+   sg_dma_address(s) = dma_addr + s_iova_off;
+   sg_dma_len(s) = s_length;
+   dma_addr += s_iova_len;
+   }
+
+   return nents;
+   }
+

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


Re: [Intel-gfx] [PATCH] drm/i915/gvt: Introduce per object locking in GVT scheduler.

2020-09-08 Thread Zhenyu Wang
On 2020.09.09 09:43:21 +0800, Colin Xu wrote:
> I tested this patch on the suspend/resume case with vGPU created (no need
> really activate), can still observer the system freeze issue as mentioned in
> another patch I sent. So I suppose we still need decouple context pin/unpin
> with submission setup/clean, but move to workload create/destroy?
>

yeah, I observed that conflict too. How about merging Zhi's ww lock fix first
then next for left suspend/resume fixes? And better if you could pack all fixes
in one series.

Thanks

> After made similar changes based on this one, plus the suspend/resume
> support patch, below tests can pass:
> 
> - Create vGPU then suspend/resume.
> 
> - Run VM w/ vGPU then suspend/resume.
> 
> https://lists.freedesktop.org/archives/intel-gvt-dev/2020-September/007061.html
> 
> On 2020-09-08 04:02, Zhi Wang wrote:
> > To support ww locking and per-object implemented in i915, GVT scheduler 
> > needs
> > to be refined. Most of the changes are located in shadow batch buffer, 
> > shadow
> > wa context in GVT-g, where use quite a lot of i915 gem object APIs.
> > 
> > Cc: Maarten Lankhorst 
> > Cc: Joonas Lahtinen 
> > Cc: Zhenyu Wang 
> > Signed-off-by: Zhi Wang 
> > ---
> >   drivers/gpu/drm/i915/gvt/scheduler.c | 68 
> > ++--
> >   1 file changed, 57 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c 
> > b/drivers/gpu/drm/i915/gvt/scheduler.c
> > index 1570eb8..fe7ee10 100644
> > --- a/drivers/gpu/drm/i915/gvt/scheduler.c
> > +++ b/drivers/gpu/drm/i915/gvt/scheduler.c
> > @@ -396,7 +396,9 @@ static void release_shadow_wa_ctx(struct 
> > intel_shadow_wa_ctx *wa_ctx)
> > if (!wa_ctx->indirect_ctx.obj)
> > return;
> > +   i915_gem_object_lock(wa_ctx->indirect_ctx.obj, NULL);
> > i915_gem_object_unpin_map(wa_ctx->indirect_ctx.obj);
> > +   i915_gem_object_unlock(wa_ctx->indirect_ctx.obj);
> > i915_gem_object_put(wa_ctx->indirect_ctx.obj);
> > wa_ctx->indirect_ctx.obj = NULL;
> > @@ -504,6 +506,7 @@ static int prepare_shadow_batch_buffer(struct 
> > intel_vgpu_workload *workload)
> > struct intel_gvt *gvt = workload->vgpu->gvt;
> > const int gmadr_bytes = gvt->device_info.gmadr_bytes_in_cmd;
> > struct intel_vgpu_shadow_bb *bb;
> > +   struct i915_gem_ww_ctx ww;
> > int ret;
> > list_for_each_entry(bb, &workload->shadow_bb, list) {
> > @@ -528,10 +531,19 @@ static int prepare_shadow_batch_buffer(struct 
> > intel_vgpu_workload *workload)
> >  * directly
> >  */
> > if (!bb->ppgtt) {
> > -   bb->vma = i915_gem_object_ggtt_pin(bb->obj,
> > -  NULL, 0, 0, 0);
> > +   i915_gem_ww_ctx_init(&ww, false);
> > +retry:
> > +   i915_gem_object_lock(bb->obj, &ww);
> > +
> > +   bb->vma = i915_gem_object_ggtt_pin_ww(bb->obj, &ww,
> > + NULL, 0, 0, 0);
> > if (IS_ERR(bb->vma)) {
> > ret = PTR_ERR(bb->vma);
> > +   if (ret == -EDEADLK) {
> > +   ret = i915_gem_ww_ctx_backoff(&ww);
> > +   if (!ret)
> > +   goto retry;
> > +   }
> > goto err;
> > }
> > @@ -545,13 +557,18 @@ static int prepare_shadow_batch_buffer(struct 
> > intel_vgpu_workload *workload)
> >   0);
> > if (ret)
> > goto err;
> > +
> > +   /* No one is going to touch shadow bb from now on. */
> > +   i915_gem_object_flush_map(bb->obj);
> > +
> > +   i915_gem_object_unlock(bb->obj);
> > +   i915_gem_ww_ctx_fini(&ww);
> > }
> > -   /* No one is going to touch shadow bb from now on. */
> > -   i915_gem_object_flush_map(bb->obj);
> > }
> > return 0;
> >   err:
> > +   i915_gem_ww_ctx_fini(&ww);
> > release_shadow_batch_buffer(workload);
> > return ret;
> >   }
> > @@ -578,14 +595,30 @@ static int prepare_shadow_wa_ctx(struct 
> > intel_shadow_wa_ctx *wa_ctx)
> > unsigned char *per_ctx_va =
> > (unsigned char *)wa_ctx->indirect_ctx.shadow_va +
> > wa_ctx->indirect_ctx.size;
> > +   struct i915_gem_ww_ctx ww;
> > +   int ret;
> > if (wa_ctx->indirect_ctx.size == 0)
> > return 0;
> > -   vma = i915_gem_object_ggtt_pin(wa_ctx->indirect_ctx.obj, NULL,
> > -  0, CACHELINE_BYTES, 0);
> > -   if (IS_ERR(vma))
> > -   return PTR_ERR(vma);
> > +   i915_gem_ww_ctx_init(&ww, false);
> > +retry:
> > +   i915_gem_object_lock(wa_ctx->indirect_ctx.obj, &ww);
> > +
> > +   vma = i915_gem_object_ggtt_pin_ww(wa_

Re: [Intel-gfx] [PATCH] drm/i915/gvt: Introduce per object locking in GVT scheduler.

2020-09-08 Thread Colin Xu



On 2020-09-09 10:06, Zhenyu Wang wrote:

On 2020.09.09 09:43:21 +0800, Colin Xu wrote:

I tested this patch on the suspend/resume case with vGPU created (no need
really activate), can still observer the system freeze issue as mentioned in
another patch I sent. So I suppose we still need decouple context pin/unpin
with submission setup/clean, but move to workload create/destroy?


yeah, I observed that conflict too. How about merging Zhi's ww lock fix first
then next for left suspend/resume fixes? And better if you could pack all fixes
in one series.

Thanks
No problem at all. And I would like to hear more comments if moving the 
context pin/unpin to workload lifecycle is reasonable.



After made similar changes based on this one, plus the suspend/resume
support patch, below tests can pass:

- Create vGPU then suspend/resume.

- Run VM w/ vGPU then suspend/resume.

https://lists.freedesktop.org/archives/intel-gvt-dev/2020-September/007061.html

On 2020-09-08 04:02, Zhi Wang wrote:

To support ww locking and per-object implemented in i915, GVT scheduler needs
to be refined. Most of the changes are located in shadow batch buffer, shadow
wa context in GVT-g, where use quite a lot of i915 gem object APIs.

Cc: Maarten Lankhorst 
Cc: Joonas Lahtinen 
Cc: Zhenyu Wang 
Signed-off-by: Zhi Wang 
---
   drivers/gpu/drm/i915/gvt/scheduler.c | 68 
++--
   1 file changed, 57 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c 
b/drivers/gpu/drm/i915/gvt/scheduler.c
index 1570eb8..fe7ee10 100644
--- a/drivers/gpu/drm/i915/gvt/scheduler.c
+++ b/drivers/gpu/drm/i915/gvt/scheduler.c
@@ -396,7 +396,9 @@ static void release_shadow_wa_ctx(struct 
intel_shadow_wa_ctx *wa_ctx)
if (!wa_ctx->indirect_ctx.obj)
return;
+   i915_gem_object_lock(wa_ctx->indirect_ctx.obj, NULL);
i915_gem_object_unpin_map(wa_ctx->indirect_ctx.obj);
+   i915_gem_object_unlock(wa_ctx->indirect_ctx.obj);
i915_gem_object_put(wa_ctx->indirect_ctx.obj);
wa_ctx->indirect_ctx.obj = NULL;
@@ -504,6 +506,7 @@ static int prepare_shadow_batch_buffer(struct 
intel_vgpu_workload *workload)
struct intel_gvt *gvt = workload->vgpu->gvt;
const int gmadr_bytes = gvt->device_info.gmadr_bytes_in_cmd;
struct intel_vgpu_shadow_bb *bb;
+   struct i915_gem_ww_ctx ww;
int ret;
list_for_each_entry(bb, &workload->shadow_bb, list) {
@@ -528,10 +531,19 @@ static int prepare_shadow_batch_buffer(struct 
intel_vgpu_workload *workload)
 * directly
 */
if (!bb->ppgtt) {
-   bb->vma = i915_gem_object_ggtt_pin(bb->obj,
-  NULL, 0, 0, 0);
+   i915_gem_ww_ctx_init(&ww, false);
+retry:
+   i915_gem_object_lock(bb->obj, &ww);
+
+   bb->vma = i915_gem_object_ggtt_pin_ww(bb->obj, &ww,
+ NULL, 0, 0, 0);
if (IS_ERR(bb->vma)) {
ret = PTR_ERR(bb->vma);
+   if (ret == -EDEADLK) {
+   ret = i915_gem_ww_ctx_backoff(&ww);
+   if (!ret)
+   goto retry;
+   }
goto err;
}
@@ -545,13 +557,18 @@ static int prepare_shadow_batch_buffer(struct 
intel_vgpu_workload *workload)
  0);
if (ret)
goto err;
+
+   /* No one is going to touch shadow bb from now on. */
+   i915_gem_object_flush_map(bb->obj);
+
+   i915_gem_object_unlock(bb->obj);
+   i915_gem_ww_ctx_fini(&ww);
}
-   /* No one is going to touch shadow bb from now on. */
-   i915_gem_object_flush_map(bb->obj);
}
return 0;
   err:
+   i915_gem_ww_ctx_fini(&ww);
release_shadow_batch_buffer(workload);
return ret;
   }
@@ -578,14 +595,30 @@ static int prepare_shadow_wa_ctx(struct 
intel_shadow_wa_ctx *wa_ctx)
unsigned char *per_ctx_va =
(unsigned char *)wa_ctx->indirect_ctx.shadow_va +
wa_ctx->indirect_ctx.size;
+   struct i915_gem_ww_ctx ww;
+   int ret;
if (wa_ctx->indirect_ctx.size == 0)
return 0;
-   vma = i915_gem_object_ggtt_pin(wa_ctx->indirect_ctx.obj, NULL,
-  0, CACHELINE_BYTES, 0);
-   if (IS_ERR(vma))
-   return PTR_ERR(vma);
+   i915_gem_ww_ctx_init(&ww, false);
+retry:
+   i915_gem_object_lock(wa_ctx->indirect_ctx.obj, &ww);
+
+   vma = i915_gem_object_ggtt_pin_ww(wa_ctx