Re: [Intel-gfx] [PATCH] drm/i915/dsc: Fix the deadlock in dsc debugfs node
Quoting Lyude Paul (2018-12-19 23:50:58)
> Reviewed-by: Lyude Paul
>
> On Wed, 2018-12-19 at 15:51 -0800, Manasi Navare wrote:
> > The DSC debugfs node causes a possible deadlock situation. This patch
> > resets the try_again at the beginning of loop to fix this.
> >
> > Fixes: e845f099f1c6 ('drm/i915/dsc: Add Per connector debugfs node for DSC
> > support/enable')
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109097
> > Cc: Lyude Paul
> > Cc: Chris Wilson
> > Signed-off-by: Manasi Navare
Thanks for the quick fix and review, pushed.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Fix i915_gem_wait_for_idle oops due to active_requests check
i915_gem_wait_for_idle() waits for all requests being completed and
calls i915_retire_requests() to retire them. It assumes the
active_requests should be zero finally.
In i915_retire_requests(), it will retire all requests on the active
rings. Unfortunately, active_requests is increased in
i915_request_alloc() and reduced in i915_request_retire(), but the
request is added into active rings in i915_request_add().
If i915_gem_wait_for_idle() is called between i915_request_alloc()
and i915_request_add(), this request will not be retired. Then, the
active_requests will not be zero in the end.
Normally, i915_request_alloc() and i915_request_add() will be called
in sequence with drm.struct_mutex locked. But in
intel_vgpu_create_workload(), it will pre-allocate the request and
call i915_request_add() in the workload thread for performance
optimization. The above issue will be triggered.
This patch introduced a new counter named reserved_requests for
request allocation. The active_requests will be increased when
the request is really added into the active rings.
8<- below is the oops when above issue is hitted.
[2018-11-28 23:17:54] [12278.310417] kernel BUG at
drivers/gpu/drm/i915/i915_gem.c:4702!
[2018-11-28 23:17:54] [12278.310802] invalid opcode: [#1] PREEMPT SMP
[2018-11-28 23:17:54] [12278.311012] CPU: 0 PID: 61 Comm: kswapd0 Tainted: G
U WC4.19.0-26.iot-lts2018-sos #1
[2018-11-28 23:17:54] [12278.311393] RIP:
0010:i915_gem_wait_for_idle.part.78.cold.114+0x45/0x47
[2018-11-28 23:17:54] [12278.311675] Code: 7b 8b ae ff 48 8b 35 e6 92 3c 01 49
c7 c0 af 48 55 a9 b9 5e 12 00 00 48 c7 c2 50 7a 0b a9 48 c7 c7 f4 e6 60 a8 e8
37 38 b6 ff <0f> 0b 48 c7 c1 a8 59 55 a9 ba b8 12 00 00 48 c7 c6 20 7a 0b a9 48
[2018-11-28 23:17:55] [12278.312447] RSP: 0018:8e31acd8bbb8 EFLAGS: 00010246
[2018-11-28 23:17:55] [12278.312673] RAX: 000e RBX:
000a RCX:
[2018-11-28 23:17:55] [12278.312971] RDX: 0001 RSI:
0008 RDI: 8e31ae841400
[2018-11-28 23:17:55] [12278.313268] RBP: 8e31acea8340 R08:
01416578 R09: 8e31aea15000
[2018-11-28 23:17:55] [12278.313566] R10: 8e31ae807100 R11:
8e31ae841400 R12: 8e31acea
[2018-11-28 23:17:55] [12278.313863] R13: 0b2ab1d38ed0 R14:
R15: 8e31acd8bd70
[2018-11-28 23:17:55] [12278.314162] FS: ()
GS:8e31afa0() knlGS:
[2018-11-28 23:17:55] [12278.314499] CS: 0010 DS: ES: CR0:
80050033
[2018-11-28 23:17:55] [12278.314741] CR2: 7ff94948f000 CR3:
000226813000 CR4: 003406f0
[2018-11-28 23:17:55] [12278.315039] Call Trace:
[2018-11-28 23:17:55] [12278.315162] i915_gem_shrink+0x3b7/0x4b0
[2018-11-28 23:17:55] [12278.315340] i915_gem_shrinker_scan+0x104/0x130
[2018-11-28 23:17:55] [12278.315537] do_shrink_slab+0x12c/0x2c0
[2018-11-28 23:17:55] [12278.315706] shrink_slab+0x225/0x2c0
[2018-11-28 23:17:55] [12278.315864] shrink_node+0xe4/0x430
[2018-11-28 23:17:55] [12278.316018] kswapd+0x3ce/0x730
[2018-11-28 23:17:55] [12278.316161] ? mem_cgroup_shrink_node+0x1a0/0x1a0
[2018-11-28 23:17:55] [12278.316365] kthread+0x11e/0x140
[2018-11-28 23:17:55] [12278.316508] ? kthread_create_worker_on_cpu+0x70/0x70
[2018-11-28 23:17:55] [12278.316727] ret_from_fork+0x3a/0x50
[2018-11-28 23:17:55] [12278.316884] Modules linked in: igb_avb(C) xhci_pci
xhci_hcd dca ici_isys_mod ipu4_acpi intel_ipu4_isys_csslib intel_ipu4_psys
intel_ipu4_psys_csslib intel_ipu4_mmu intel_ipu4 iova crlmodule_lite
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109005
Signed-off-by: Bin Yang
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_gem.c | 2 +-
drivers/gpu/drm/i915/i915_request.c | 10 +++---
3 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 815db160b966..7a757f0f504f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1948,6 +1948,7 @@ struct drm_i915_private {
struct list_head active_rings;
struct list_head closed_vma;
u32 active_requests;
+ u32 reserved_requests;
u32 request_serial;
/**
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index d92147ab4489..1873e21c84c1 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -200,7 +200,7 @@ void i915_gem_unpark(struct drm_i915_private *i915)
GEM_TRACE("\n");
lockdep_assert_held(&i915->drm.struct_mutex);
- GEM_BUG_ON(!i915->gt.active_requests);
+ GEM_BUG_ON(!i915->gt.reserved_requests);
if (i915->gt.awake)
return;
diff --git a/drivers/gpu/drm/i915/i915_request.c
b/drivers/gpu/drm/i915/i915_request.c
index 637909c59f1f..394283799ee9 100644
--- a/drivers/gpu/drm/i91
Re: [Intel-gfx] [RFC v3 1/3] PM/runtime: Add a new interface to get accounted time
On Wed, 19 Dec 2018 at 17:52, Vincent Guittot
wrote:
>
> On Wed, 19 Dec 2018 at 17:36, Ulf Hansson wrote:
> >
> > On Wed, 19 Dec 2018 at 14:26, Vincent Guittot
> > wrote:
> > >
> > > On Wed, 19 Dec 2018 at 11:43, Ulf Hansson wrote:
> > > >
> > > > On Wed, 19 Dec 2018 at 11:34, Vincent Guittot
> > > > wrote:
> > > > >
> > > > > On Wed, 19 Dec 2018 at 11:21, Ulf Hansson
> > > > > wrote:
> > > > > >
> > >
> > > > > > > > >
> > > > > > > > > diff --git a/drivers/base/power/runtime.c
> > > > > > > > > b/drivers/base/power/runtime.c
> > > > > > > > > index 7062469..6461469 100644
> > > > > > > > > --- a/drivers/base/power/runtime.c
> > > > > > > > > +++ b/drivers/base/power/runtime.c
> > > > > > > > > @@ -88,6 +88,32 @@ static void __update_runtime_status(struct
> > > > > > > > > device *dev, enum rpm_status status)
> > > > > > > > > dev->power.runtime_status = status;
> > > > > > > > > }
> > > > > > > > >
> > > > > > > > > +u64 pm_runtime_accounted_time_get(struct device *dev, enum
> > > > > > > > > rpm_status status, bool update)
> > > > > > > > > +{
> > > > > > > > > + u64 now = ktime_to_ns(ktime_get());
> > > > > > > >
> > > > > > > > I think you should stay on jiffies here - and then switch to
> > > > > > > > ktime in patch 3.
> > > > > > > >
> > > > > > > > > + u64 delta = 0, time = 0;
> > > > > > > > > + unsigned long flags;
> > > > > > > > > +
> > > > > > > > > + spin_lock_irqsave(&dev->power.lock, flags);
> > > > > > > > > +
> > > > > > > > > + if (dev->power.disable_depth > 0)
> > > > > > > > > + goto unlock;
> > > > > > > > > +
> > > > > > > > > + /* Add ongoing state if requested */
> > > > > > > > > + if (update && dev->power.runtime_status == status)
> > > > > > > > > + delta = now - dev->power.accounting_timestamp;
> > > > > > > > > +
> > > > > > > >
> > > > > > > > Hmm. Do we really need to update the accounting timestamp? I
> > > > > > > > would
> > > > > > > > rather avoid it if possible.
> > > > > > >
> > > > > > > i915/drm uses this to track ongoing suspended state. In fact they
> > > > > > > are
> > > > > > > mainly interested by this part
> > > > > >
> > > > > > Again, sorry I don't follow.
> > > > >
> > > > > In fact we don't update dev->power.accounting_timestamp but only use
> > > > > it to get how much time has elapsed in the current state.
> > > > >
> > > > > >
> > > > > > My suggested changes below, would do exactly that; track the ongoing
> > > > > > suspended state.
> > > > > >
> > > > > > The user can call the function several times while the device
> > > > > > remains
> > > > > > RPM_SUSPENDED, and if needed the user could then compute the delta
> > > > > > in-between the calls, for whatever reason that may be needed.
> > > > >
> > > > > So I'm not sure to catch your question:
> > > > > Is your problem linked to status != RPM_SUSPENDED or the update
> > > > > parameter that compute delta ?
> > > >
> > > > My intent was to keep things simple.
> > > >
> > > > 1. Only expose last suspended time, which means tracking the ongoing
> > > > suspended state. In other words, you can also remove "enum rpm_status
> > > > status" as the in-parameter to pm_runtime_accounted_time_get().
> > >
> > > Ok for this point if Rafael doesn't see any benefit of keeping the
> > > generic interface
> > >
> > > > 2. Don't allow the user of pm_runtime_accounted_time_get() to update
> > > > the current timestamp, in "dev->power.accounting_timestamp".
> > >
> > > But pm_runtime_accounted_time_get doesn't update
> > > dev->power.accounting_timestamp, it only reads it to know when when
> > > the last state transition happened
> >
> > I understand, sorry for not being clear enough.
> >
> > My point is, you must not update dev->power.suspended_time, without
> > also setting a new value for dev->power.accounting_timestamp.
> > Otherwise, the next time the core calls
> > update_pm_runtime_accounting(), it will end up adding a wrong delta to
> > dev->power.suspended_time.
>
> I fully agree on that and that's why dev->power.accounting_timestamp
> is not and has never been modified
Huh, I have miss-read your patch. What a mess, my apologies.
>
> >
> > BTW, it seems like you have based this on top of some patch converting
> > from jiffies to ktime, as I can't fine dev->power.suspended_time, but
> > instead I have dev->power.suspended_jiffies.
> >
> > On Wed, 19 Dec 2018 at 14:26, Vincent Guittot
> > wrote:
> > >
> > > On Wed, 19 Dec 2018 at 11:43, Ulf Hansson wrote:
> > > >
> > > > On Wed, 19 Dec 2018 at 11:34, Vincent Guittot
> > > > wrote:
> > > > >
> > > > > On Wed, 19 Dec 2018 at 11:21, Ulf Hansson
> > > > > wrote:
> > > > > >
> > >
> > > > > > > > >
> > > > > > > > > diff --git a/drivers/base/power/runtime.c
> > > > > > > > > b/drivers/base/power/runtime.c
> > > > > > > > > index 7062469..6461469 100644
> > > > > > > > > --- a/drivers/base/power/runtime.c
> > > > > > > > > +++ b/drivers/base/power/runtime.c
> > > >
Re: [Intel-gfx] [PATCH] drm/i915: Fix i915_gem_wait_for_idle oops due to active_requests check
Quoting Bin Yang (2018-12-20 08:01:35) > Normally, i915_request_alloc() and i915_request_add() will be called > in sequence with drm.struct_mutex locked. But in > intel_vgpu_create_workload(), it will pre-allocate the request and > call i915_request_add() in the workload thread for performance > optimization. The above issue will be triggered. That's your bug. It's not normally, it's a strict requirement that the struct_mutex (request generation mutex) be held over the course of generating the request. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC v3 1/3] PM/runtime: Add a new interface to get accounted time
On Thu, 20 Dec 2018 at 09:16, Ulf Hansson wrote:
>
> On Wed, 19 Dec 2018 at 17:52, Vincent Guittot
> wrote:
> >
> > On Wed, 19 Dec 2018 at 17:36, Ulf Hansson wrote:
> > >
> > > On Wed, 19 Dec 2018 at 14:26, Vincent Guittot
> > > wrote:
> > > >
> > > > On Wed, 19 Dec 2018 at 11:43, Ulf Hansson
> > > > wrote:
> > > > >
> > > > > On Wed, 19 Dec 2018 at 11:34, Vincent Guittot
> > > > > wrote:
> > > > > >
> > > > > > On Wed, 19 Dec 2018 at 11:21, Ulf Hansson
> > > > > > wrote:
> > > > > > >
> > > >
> > > > > > > > > >
> > > > > > > > > > diff --git a/drivers/base/power/runtime.c
> > > > > > > > > > b/drivers/base/power/runtime.c
> > > > > > > > > > index 7062469..6461469 100644
> > > > > > > > > > --- a/drivers/base/power/runtime.c
> > > > > > > > > > +++ b/drivers/base/power/runtime.c
> > > > > > > > > > @@ -88,6 +88,32 @@ static void
> > > > > > > > > > __update_runtime_status(struct device *dev, enum rpm_status
> > > > > > > > > > status)
> > > > > > > > > > dev->power.runtime_status = status;
> > > > > > > > > > }
> > > > > > > > > >
> > > > > > > > > > +u64 pm_runtime_accounted_time_get(struct device *dev, enum
> > > > > > > > > > rpm_status status, bool update)
> > > > > > > > > > +{
> > > > > > > > > > + u64 now = ktime_to_ns(ktime_get());
> > > > > > > > >
> > > > > > > > > I think you should stay on jiffies here - and then switch to
> > > > > > > > > ktime in patch 3.
> > > > > > > > >
> > > > > > > > > > + u64 delta = 0, time = 0;
> > > > > > > > > > + unsigned long flags;
> > > > > > > > > > +
> > > > > > > > > > + spin_lock_irqsave(&dev->power.lock, flags);
> > > > > > > > > > +
> > > > > > > > > > + if (dev->power.disable_depth > 0)
> > > > > > > > > > + goto unlock;
> > > > > > > > > > +
> > > > > > > > > > + /* Add ongoing state if requested */
> > > > > > > > > > + if (update && dev->power.runtime_status == status)
> > > > > > > > > > + delta = now -
> > > > > > > > > > dev->power.accounting_timestamp;
> > > > > > > > > > +
> > > > > > > > >
> > > > > > > > > Hmm. Do we really need to update the accounting timestamp? I
> > > > > > > > > would
> > > > > > > > > rather avoid it if possible.
> > > > > > > >
> > > > > > > > i915/drm uses this to track ongoing suspended state. In fact
> > > > > > > > they are
> > > > > > > > mainly interested by this part
> > > > > > >
> > > > > > > Again, sorry I don't follow.
> > > > > >
> > > > > > In fact we don't update dev->power.accounting_timestamp but only use
> > > > > > it to get how much time has elapsed in the current state.
> > > > > >
> > > > > > >
> > > > > > > My suggested changes below, would do exactly that; track the
> > > > > > > ongoing
> > > > > > > suspended state.
> > > > > > >
> > > > > > > The user can call the function several times while the device
> > > > > > > remains
> > > > > > > RPM_SUSPENDED, and if needed the user could then compute the delta
> > > > > > > in-between the calls, for whatever reason that may be needed.
> > > > > >
> > > > > > So I'm not sure to catch your question:
> > > > > > Is your problem linked to status != RPM_SUSPENDED or the update
> > > > > > parameter that compute delta ?
> > > > >
> > > > > My intent was to keep things simple.
> > > > >
> > > > > 1. Only expose last suspended time, which means tracking the ongoing
> > > > > suspended state. In other words, you can also remove "enum rpm_status
> > > > > status" as the in-parameter to pm_runtime_accounted_time_get().
> > > >
> > > > Ok for this point if Rafael doesn't see any benefit of keeping the
> > > > generic interface
> > > >
> > > > > 2. Don't allow the user of pm_runtime_accounted_time_get() to update
> > > > > the current timestamp, in "dev->power.accounting_timestamp".
> > > >
> > > > But pm_runtime_accounted_time_get doesn't update
> > > > dev->power.accounting_timestamp, it only reads it to know when when
> > > > the last state transition happened
> > >
> > > I understand, sorry for not being clear enough.
> > >
> > > My point is, you must not update dev->power.suspended_time, without
> > > also setting a new value for dev->power.accounting_timestamp.
> > > Otherwise, the next time the core calls
> > > update_pm_runtime_accounting(), it will end up adding a wrong delta to
> > > dev->power.suspended_time.
> >
> > I fully agree on that and that's why dev->power.accounting_timestamp
> > is not and has never been modified
>
> Huh, I have miss-read your patch. What a mess, my apologies.
>
> >
> > >
> > > BTW, it seems like you have based this on top of some patch converting
> > > from jiffies to ktime, as I can't fine dev->power.suspended_time, but
> > > instead I have dev->power.suspended_jiffies.
> > >
> > > On Wed, 19 Dec 2018 at 14:26, Vincent Guittot
> > > wrote:
> > > >
> > > > On Wed, 19 Dec 2018 at 11:43, Ulf Hansson
> > > > wrote:
> > > > >
> > > > > On Wed, 19 Dec 2018 at 11:34, Vincent Guittot
> > > > > wrote:
> > > > > >
> > > > > > On We
Re: [Intel-gfx] [PATCH] drm/i915: Fix i915_gem_wait_for_idle oops due to active_requests check
On Thu, 2018-12-20 at 08:35 +, Chris Wilson wrote: > Quoting Bin Yang (2018-12-20 08:01:35) > > Normally, i915_request_alloc() and i915_request_add() will be called > > in sequence with drm.struct_mutex locked. But in > > intel_vgpu_create_workload(), it will pre-allocate the request and > > call i915_request_add() in the workload thread for performance > > optimization. The above issue will be triggered. > > That's your bug. It's not normally, it's a strict requirement that the > struct_mutex (request generation mutex) be held over the course of > generating the request. > -Chris This code is introduced by below patch. Add original patch owners to discuss this issue. commit d0302e74003bf1f0fc41c06948b745204c4704ea Author: Ping Gao Date: Thu Jun 29 12:22:43 2017 +0800 drm/i915/gvt: Audit and shadow workload during ELSP writing Let the workload audit and shadow ahead of vGPU scheduling, that will eliminate GPU idle time and improve performance for multi-VM. The performance of Heaven running simultaneously in 3VMs has improved 20% after this patch. v2:Remove condition current->vgpu==vgpu when shadow during ELSP writing. Signed-off-by: Ping Gao Reviewed-by: Zhi Wang Signed-off-by: Zhenyu Wang ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 01/16] drm/dp_mst: Rename drm_dp_mst_get_validated_(port|mstb)_ref and friends
On Wed, Dec 19, 2018 at 07:19:45PM -0500, Lyude Paul wrote:
> s/drm_dp_get_validated_port_ref/drm_dp_mst_topology_get_port_validated/
> s/drm_dp_put_port/drm_dp_mst_topology_put_port/
> s/drm_dp_get_validated_mstb_ref/drm_dp_mst_topology_get_mstb_validated/
> s/drm_dp_put_mst_branch_device/drm_dp_mst_topology_put_mstb/
>
> This is a much more consistent naming scheme, and will make even more
> sense once we redesign how the current refcounting scheme here works.
>
> Signed-off-by: Lyude Paul
> Cc: Daniel Vetter
> Cc: David Airlie
> Cc: Jerry Zuo
> Cc: Harry Wentland
> Cc: Juston Li
Reviewed-by: Daniel Vetter
> ---
> drivers/gpu/drm/drm_dp_mst_topology.c | 114 ++
> 1 file changed, 62 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 2ab16c9e6243..6f9b211069a7 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -46,7 +46,7 @@ static bool dump_dp_payload_table(struct
> drm_dp_mst_topology_mgr *mgr,
> char *buf);
> static int test_calc_pbn_mode(void);
>
> -static void drm_dp_put_port(struct drm_dp_mst_port *port);
> +static void drm_dp_mst_topology_put_port(struct drm_dp_mst_port *port);
>
> static int drm_dp_dpcd_write_payload(struct drm_dp_mst_topology_mgr *mgr,
>int id,
> @@ -888,7 +888,7 @@ static void drm_dp_destroy_mst_branch_device(struct kref
> *kref)
>*/
> list_for_each_entry_safe(port, tmp, &mstb->ports, next) {
> list_del(&port->next);
> - drm_dp_put_port(port);
> + drm_dp_mst_topology_put_port(port);
> }
>
> /* drop any tx slots msg */
> @@ -911,7 +911,7 @@ static void drm_dp_destroy_mst_branch_device(struct kref
> *kref)
> kref_put(kref, drm_dp_free_mst_branch_device);
> }
>
> -static void drm_dp_put_mst_branch_device(struct drm_dp_mst_branch *mstb)
> +static void drm_dp_mst_topology_put_mstb(struct drm_dp_mst_branch *mstb)
> {
> kref_put(&mstb->kref, drm_dp_destroy_mst_branch_device);
> }
> @@ -930,7 +930,7 @@ static void drm_dp_port_teardown_pdt(struct
> drm_dp_mst_port *port, int old_pdt)
> case DP_PEER_DEVICE_MST_BRANCHING:
> mstb = port->mstb;
> port->mstb = NULL;
> - drm_dp_put_mst_branch_device(mstb);
> + drm_dp_mst_topology_put_mstb(mstb);
> break;
> }
> }
> @@ -970,12 +970,14 @@ static void drm_dp_destroy_port(struct kref *kref)
> kfree(port);
> }
>
> -static void drm_dp_put_port(struct drm_dp_mst_port *port)
> +static void drm_dp_mst_topology_put_port(struct drm_dp_mst_port *port)
> {
> kref_put(&port->kref, drm_dp_destroy_port);
> }
>
> -static struct drm_dp_mst_branch
> *drm_dp_mst_get_validated_mstb_ref_locked(struct drm_dp_mst_branch *mstb,
> struct drm_dp_mst_branch *to_find)
> +static struct drm_dp_mst_branch *
> +drm_dp_mst_topology_get_mstb_validated_locked(struct drm_dp_mst_branch *mstb,
> + struct drm_dp_mst_branch *to_find)
> {
> struct drm_dp_mst_port *port;
> struct drm_dp_mst_branch *rmstb;
> @@ -985,7 +987,8 @@ static struct drm_dp_mst_branch
> *drm_dp_mst_get_validated_mstb_ref_locked(struct
> }
> list_for_each_entry(port, &mstb->ports, next) {
> if (port->mstb) {
> - rmstb =
> drm_dp_mst_get_validated_mstb_ref_locked(port->mstb, to_find);
> + rmstb = drm_dp_mst_topology_get_mstb_validated_locked(
> + port->mstb, to_find);
> if (rmstb)
> return rmstb;
> }
> @@ -993,12 +996,15 @@ static struct drm_dp_mst_branch
> *drm_dp_mst_get_validated_mstb_ref_locked(struct
> return NULL;
> }
>
> -static struct drm_dp_mst_branch *drm_dp_get_validated_mstb_ref(struct
> drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_branch *mstb)
> +static struct drm_dp_mst_branch *
> +drm_dp_mst_topology_get_mstb_validated(struct drm_dp_mst_topology_mgr *mgr,
> +struct drm_dp_mst_branch *mstb)
> {
> struct drm_dp_mst_branch *rmstb = NULL;
> mutex_lock(&mgr->lock);
> if (mgr->mst_primary)
> - rmstb =
> drm_dp_mst_get_validated_mstb_ref_locked(mgr->mst_primary, mstb);
> + rmstb = drm_dp_mst_topology_get_mstb_validated_locked(
> + mgr->mst_primary, mstb);
> mutex_unlock(&mgr->lock);
> return rmstb;
> }
> @@ -1021,7 +1027,9 @@ static struct drm_dp_mst_port
> *drm_dp_mst_get_port_ref_locked(struct drm_dp_mst_
> return NULL;
> }
>
> -static struct drm_dp_mst_port *drm_dp_get_validated_port_ref(struct
> drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port)
> +static struct drm_dp_mst_port *
> +drm_dp_mst_topology_get_port_validated(stru
Re: [Intel-gfx] [PATCH v2 06/16] drm/i915: Keep malloc references to MST ports
On Wed, Dec 19, 2018 at 07:19:50PM -0500, Lyude Paul wrote: > So that the ports stay around until we've destroyed the connectors, in > order to ensure that we don't pass an invalid pointer to any MST helpers > once we introduce the new MST VCPI helpers. > > Changes since v1: > * Move drm_dp_mst_get_port_malloc() to where we assign > intel_connector->port - danvet > > Signed-off-by: Lyude Paul > Cc: Daniel Vetter > Cc: David Airlie > Cc: Jerry Zuo > Cc: Harry Wentland > Cc: Juston Li Reviewed-by: Daniel Vetter > --- > drivers/gpu/drm/i915/intel_connector.c | 4 > drivers/gpu/drm/i915/intel_dp_mst.c| 1 + > 2 files changed, 5 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_connector.c > b/drivers/gpu/drm/i915/intel_connector.c > index 18e370f607bc..37d2c644f4b8 100644 > --- a/drivers/gpu/drm/i915/intel_connector.c > +++ b/drivers/gpu/drm/i915/intel_connector.c > @@ -95,6 +95,10 @@ void intel_connector_destroy(struct drm_connector > *connector) > intel_panel_fini(&intel_connector->panel); > > drm_connector_cleanup(connector); > + > + if (intel_connector->port) > + drm_dp_mst_put_port_malloc(intel_connector->port); > + > kfree(connector); > } > > diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c > b/drivers/gpu/drm/i915/intel_dp_mst.c > index f05427b74e34..631fd1537252 100644 > --- a/drivers/gpu/drm/i915/intel_dp_mst.c > +++ b/drivers/gpu/drm/i915/intel_dp_mst.c > @@ -457,6 +457,7 @@ static struct drm_connector > *intel_dp_add_mst_connector(struct drm_dp_mst_topolo > intel_connector->get_hw_state = intel_dp_mst_get_hw_state; > intel_connector->mst_port = intel_dp; > intel_connector->port = port; > + drm_dp_mst_get_port_malloc(port); > > connector = &intel_connector->base; > ret = drm_connector_init(dev, connector, &intel_dp_mst_connector_funcs, > -- > 2.19.2 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC v3 1/3] PM/runtime: Add a new interface to get accounted time
[...]
> > > > Re-thinking this a bit from my earlier comments - and by following the
> > > > above reasoning, it sounds like this better belongs in the
> > > > driver/subsystem, without requiring any data from the core.
> > > >
> > > > The driver/subsystem could just store a timestamp in it's
> > > > ->runtime_suspend() callback and whenever needed, it could compute a
> > > > delta towards it. That should work, right?
> > >
> > > I don't know i915/drm enough to know all that details
> >
> > Okay, so let me re-summarize the main issue I see with your approach
> > in $subject patch.
> >
> > dev->power.accounting_timestamp can't be used to know when last
> > transition was made. If I understand correctly, that is how you use
> > it. No?
>
> Yes. At least that how I have interpreted the current code
>
> >
> > Anyway, as stated, that's because the timestamp becomes updated, if
> > update_pm_runtime_accounting() is called via the sysfs nobs, which
> > means there is no state transition happening, but only accounting data
> > is updated.
>
> Yes I have not realized that the update also happens there which makes
> me think that i have
> may be over interpreted the code and the initialization of
> i915->pmu.suspended_jiffies_last
>
> >
> > So, what I think we can do from the core perspective, if it helps
> > (which I am not sure of):
> > 1. Export a function, which returns the value of
> > dev->power.suspended_jiffies.
> > 2. Export a wrapper function (to deal with locking) which calls
> > update_pm_runtime_accounting(). This wrapper function allows the user
> > the update the total suspended time, also taking into account the time
> > spent in the current state.
>
> Having now in mind that suspended_jiffies can be updated outside state
> transition like via sysfs call,
> we can maybe just implements 2 and return dev->power.suspended_jiffies
>
> something like below
> unsigned long pm_runtime_get_suspended_time(struct device *dev)
"pm_runtime_suspended_time()" should be sufficient I think. The "get"
part would become confusing due to the existing get/put functions that
are part of the runtime PM interface.
> {
> unsigned long time;
> unsigned long flags;
>
> spin_lock_irqsave(&dev->power.lock, flags);
>
> update_pm_runtime_accounting(dev);
>
> time = dev->power.suspended_time;
dev->power.suspended_jiffies
...at least until you converts to ktime :-)
>
> spin_unlock_irqrestore(&dev->power.lock, flags);
>
> return time;
> }
Yes, this looks fine to me!
Kind regards
Uffe
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC PATCH v2 1/2] drm/i915: Export current required functions for GVT
Quoting Zhenyu Wang (2018-11-26 08:05:53)
> This trys to export all required i915 functions for GVT.
We have couple of __ prefixed functions proposed here. That'd mostly
mean whoever wrote the function intended it to be used only at file
scope for its nature (or with special caution outside of file).
This often means no error checking and other unintended behavior, so
they wouldn't be great candidates for exporting.
If we could pull just enough code from where they are used in GVT to
to see if we can form a more well-defined interface that wouldn't need
__ protection. Or there could already exist one.
We should also make sure that each EXPORT_SYMBOL_GPL function has
up-to-date kerneldoc comment for it explaining what it does *and*
that it matches what the GVT module expects from the function.
With that said, looks small enough that we'll probably get this
hammered down shortly enough :)
Just a reminder, whatever API we declare here, is completely internal to
i915 and GVT, so we're free to change it going forward. I expect there
to be a few changes after we've established the baseline (what we
currently have), but then it should settle down.
Regards, Joonas
>
> Signed-off-by: Zhenyu Wang
> ---
> drivers/gpu/drm/i915/i915_gem.c | 11 +++
> drivers/gpu/drm/i915/i915_gem_context.c | 2 ++
> drivers/gpu/drm/i915/i915_gem_dmabuf.c| 1 +
> drivers/gpu/drm/i915/i915_gem_fence_reg.c | 2 ++
> drivers/gpu/drm/i915/i915_gem_gtt.c | 1 +
> drivers/gpu/drm/i915/i915_request.c | 3 +++
> drivers/gpu/drm/i915/i915_vma.c | 2 ++
> drivers/gpu/drm/i915/intel_ringbuffer.c | 1 +
> drivers/gpu/drm/i915/intel_runtime_pm.c | 2 ++
> drivers/gpu/drm/i915/intel_uncore.c | 3 +++
> 10 files changed, 28 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index c55b1f75c980..9af6e9810f85 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -701,6 +701,7 @@ void *i915_gem_object_alloc(struct drm_i915_private
> *dev_priv)
> {
> return kmem_cache_zalloc(dev_priv->objects, GFP_KERNEL);
> }
> +EXPORT_SYMBOL_GPL(i915_gem_object_alloc);
>
> void i915_gem_object_free(struct drm_i915_gem_object *obj)
> {
> @@ -1029,6 +1030,7 @@ int i915_gem_obj_prepare_shmem_write(struct
> drm_i915_gem_object *obj,
> i915_gem_object_unpin_pages(obj);
> return ret;
> }
> +EXPORT_SYMBOL_GPL(i915_gem_obj_prepare_shmem_write);
>
> static void
> shmem_clflush_swizzled_range(char *addr, unsigned long length,
> @@ -2764,6 +2766,7 @@ void __i915_gem_object_set_pages(struct
> drm_i915_gem_object *obj,
> list_add(&obj->mm.link, &i915->mm.unbound_list);
> spin_unlock(&i915->mm.obj_lock);
> }
> +EXPORT_SYMBOL_GPL(__i915_gem_object_set_pages);
>
> static int i915_gem_object_get_pages(struct drm_i915_gem_object *obj)
> {
> @@ -2930,6 +2933,7 @@ void *i915_gem_object_pin_map(struct
> drm_i915_gem_object *obj,
> ptr = ERR_PTR(ret);
> goto out_unlock;
> }
> +EXPORT_SYMBOL_GPL(i915_gem_object_pin_map);
>
> static int
> i915_gem_object_pwrite_gtt(struct drm_i915_gem_object *obj,
> @@ -4041,6 +4045,7 @@ i915_gem_object_set_to_gtt_domain(struct
> drm_i915_gem_object *obj, bool write)
> i915_gem_object_unpin_pages(obj);
> return 0;
> }
> +EXPORT_SYMBOL_GPL(i915_gem_object_set_to_gtt_domain);
>
> /**
> * Changes the cache-level of an object across all VMA.
> @@ -4406,6 +4411,7 @@ i915_gem_object_set_to_cpu_domain(struct
> drm_i915_gem_object *obj, bool write)
>
> return 0;
> }
> +EXPORT_SYMBOL_GPL(i915_gem_object_set_to_cpu_domain);
>
> /* Throttle our rendering by waiting until the ring has completed our
> requests
> * emitted over 20 msec ago.
> @@ -4535,6 +4541,7 @@ i915_gem_object_ggtt_pin(struct drm_i915_gem_object
> *obj,
>
> return vma;
> }
> +EXPORT_SYMBOL_GPL(i915_gem_object_ggtt_pin);
>
> static __always_inline unsigned int __busy_read_flag(unsigned int id)
> {
> @@ -4758,6 +4765,7 @@ void i915_gem_object_init(struct drm_i915_gem_object
> *obj,
>
> i915_gem_info_add_obj(to_i915(obj->base.dev), obj->base.size);
> }
> +EXPORT_SYMBOL_GPL(i915_gem_object_init);
>
> static const struct drm_i915_gem_object_ops i915_gem_object_ops = {
> .flags = I915_GEM_OBJECT_HAS_STRUCT_PAGE |
> @@ -4864,6 +4872,7 @@ i915_gem_object_create(struct drm_i915_private
> *dev_priv, u64 size)
> i915_gem_object_free(obj);
> return ERR_PTR(ret);
> }
> +EXPORT_SYMBOL_GPL(i915_gem_object_create);
>
> static bool discard_backing_storage(struct drm_i915_gem_object *obj)
> {
> @@ -5061,6 +5070,7 @@ void __i915_gem_object_release_unless_active(struct
> drm_i915_gem_object *obj)
> else
> i915_gem_object_put(obj);
> }
> +EXPORT_SYMBOL_GPL(__i915_gem_object_release_unless_active);
>
> void i915_gem_sanitize(struct drm_i915_private *i915)
Re: [Intel-gfx] [RFC PATCH v2 2/2] drm/i915: Move GVT device model into separate module
Quoting Zhenyu Wang (2018-11-26 08:05:54)
> This makes GVT device model into a stand alone module named as
> "i915_gvt", which depends on i915 but load after i915 that do
> all left GVT host initialization setup.
>
> Currently i915_gvt module would still load "kvmgt" module to
> initialize for VFIO/mdev device, but this might be changed in future.
> As will make hypervisor module load to initialize GVT host instead.
>
> One problem by this split is that GVT requires HW initial MMIO state
> as base initial state for each vGPU after create, which should be
> taken in very early stage of i915 load to reflect HW state. Just
> reading all MMIO won't work, as it could cause unknown side effect e.g
> HW hang. So current choice is to read back MMIO that GVT actually
> track. But instead of reference any GVT files, this one just copied
> register list. So GVT will use initial dump instead when load.
As discussed in IRC, lets try to get the golden MMIO concept going, so
we could completely get rid of intel_gvt.c.
Regards, Joonas
>
> Cc: Joonas Lahtinen
> Signed-off-by: Zhenyu Wang
> ---
> drivers/gpu/drm/i915/Kconfig|2 +-
> drivers/gpu/drm/i915/Makefile |4 +-
> drivers/gpu/drm/i915/gvt/Makefile |8 +-
> drivers/gpu/drm/i915/gvt/firmware.c |2 +-
> drivers/gpu/drm/i915/gvt/gvt.c | 52 +-
> drivers/gpu/drm/i915/gvt/gvt.h |3 +
> drivers/gpu/drm/i915/i915_drv.c | 43 +-
> drivers/gpu/drm/i915/i915_drv.h | 10 +-
> drivers/gpu/drm/i915/i915_params.c |5 -
> drivers/gpu/drm/i915/i915_params.h |3 +-
> drivers/gpu/drm/i915/intel_gvt.c| 1519 +--
> drivers/gpu/drm/i915/intel_gvt.h| 29 +-
> 12 files changed, 1547 insertions(+), 133 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
> index 33a458b7f1fc..a05261cabf53 100644
> --- a/drivers/gpu/drm/i915/Kconfig
> +++ b/drivers/gpu/drm/i915/Kconfig
> @@ -96,7 +96,7 @@ config DRM_I915_USERPTR
> If in doubt, say "Y".
>
> config DRM_I915_GVT
> -bool "Enable Intel GVT-g graphics virtualization host support"
> +tristate "Enable Intel GVT-g graphics virtualization host support"
> depends on DRM_I915
> depends on 64BIT
> default n
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 0ff878c994e2..3c23d0a5552a 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -186,12 +186,12 @@ i915-y += i915_perf.o \
> i915_oa_cnl.o \
> i915_oa_icl.o
>
> -ifeq ($(CONFIG_DRM_I915_GVT),y)
> +ifneq ($(CONFIG_DRM_I915_GVT),n)
> i915-y += intel_gvt.o
> -include $(src)/gvt/Makefile
> endif
>
> # LPE Audio for VLV and CHT
> i915-y += intel_lpe_audio.o
>
> obj-$(CONFIG_DRM_I915) += i915.o
> +obj-$(CONFIG_DRM_I915_GVT) += gvt/
> diff --git a/drivers/gpu/drm/i915/gvt/Makefile
> b/drivers/gpu/drm/i915/gvt/Makefile
> index b016dc753db9..f41cfc7625b4 100644
> --- a/drivers/gpu/drm/i915/gvt/Makefile
> +++ b/drivers/gpu/drm/i915/gvt/Makefile
> @@ -1,10 +1,10 @@
> # SPDX-License-Identifier: GPL-2.0
> -GVT_DIR := gvt
> GVT_SOURCE := gvt.o aperture_gm.o handlers.o vgpu.o trace_points.o
> firmware.o \
> interrupt.o gtt.o cfg_space.o opregion.o mmio.o display.o edid.o \
> execlist.o scheduler.o sched_policy.o mmio_context.o cmd_parser.o
> debugfs.o \
> fb_decoder.o dmabuf.o page_track.o
>
> -ccflags-y += -I$(src) -I$(src)/$(GVT_DIR)
> -i915-y += $(addprefix $(GVT_DIR)/,
> $(GVT_SOURCE))
> -obj-$(CONFIG_DRM_I915_GVT_KVMGT) += $(GVT_DIR)/kvmgt.o
> +ccflags-y += -I$(src) -I$(src)/../
> +i915_gvt-y := $(GVT_SOURCE)
> +obj-$(CONFIG_DRM_I915_GVT_KVMGT) += kvmgt.o
> +obj-$(CONFIG_DRM_I915_GVT) += i915_gvt.o
> diff --git a/drivers/gpu/drm/i915/gvt/firmware.c
> b/drivers/gpu/drm/i915/gvt/firmware.c
> index 4ac18b447247..7a3441fb9f70 100644
> --- a/drivers/gpu/drm/i915/gvt/firmware.c
> +++ b/drivers/gpu/drm/i915/gvt/firmware.c
> @@ -70,7 +70,7 @@ static int mmio_snapshot_handler(struct intel_gvt *gvt, u32
> offset, void *data)
> {
> struct drm_i915_private *dev_priv = gvt->dev_priv;
>
> - *(u32 *)(data + offset) = I915_READ_NOTRACE(_MMIO(offset));
> + *(u32 *)(data + offset) = dev_priv->hw_init_mmio[offset >> 2];
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/i915/gvt/gvt.c b/drivers/gpu/drm/i915/gvt/gvt.c
> index 733a2a0d0c30..bde20d86acf5 100644
> --- a/drivers/gpu/drm/i915/gvt/gvt.c
> +++ b/drivers/gpu/drm/i915/gvt/gvt.c
> @@ -30,10 +30,11 @@
> *
> */
>
> +#include
> #include
> #include
> #include
> -
> +#include
> #include "i915_drv.h"
> #include "gvt.h"
> #include
> @@ -216,8 +217,8 @@ int intel_gvt_init_host(void)
> } else {
> #if IS_ENABLED(CONFIG_DRM_I915_GVT_KVMGT)
>
Re: [Intel-gfx] [PATCH v6 3/3] drm/i915/gvt: Change KVMGT as self load module
Quoting Zhenyu Wang (2018-12-07 10:16:53) > This trys to make 'kvmgt' module as self loadable instead of loading > by i915/gvt device model. So hypervisor specific module could be > stand-alone, e.g only after loading hypervisor specific module, GVT > feature could be enabled via specific hypervisor interface, e.g VFIO/mdev. > > So this trys to use hypervisor module register/unregister interface > for that. Hypervisor module needs to take care of module reference > itself when working for hypervisor interface, e.g for VFIO/mdev, > hypervisor module would reference counting mdev when open and release. > > This makes 'kvmgt' module really split from GVT device model. User > needs to load 'kvmgt' to enable VFIO/mdev interface. > > v6: > - remove unused variable > > v5: > - put module reference in register error path > > v4: > - fix checkpatch warning > > v3: > - Fix module reference handling for device open and release. Unused > mdev devices would be cleaned up in device unregister when module unload. > > v2: > - Fix kvmgt order after i915 for built-in case > > Cc: "Yuan, Hang" > Cc: Alex Williamson > Cc: "He, Min" > Reviewed-by: Yuan, Hang > Signed-off-by: Zhenyu Wang For what i915 is concerned, this is the desireable direction. When combined with the other series about making GVT build independantly, and golden MMIO state (instead of capturing at boot). With those This should both expedite the i915 driver loading on DOM0, when GVT can be loaded as secondary module. And the golden MMIO state instead of capturing from host would avoid leaking any changes in host configuration (say, due to BIOS update) to the guests. So as an intermediary step to that direction, for i915 portion (not that much in this case) this is: Acked-by: Joonas Lahtinen I'm expecting somebody with more inisght to review the code under gvt/. Regards, Joonas ___ 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: Fix i915_gem_wait_for_idle oops due to active_requests check
== Series Details == Series: drm/i915: Fix i915_gem_wait_for_idle oops due to active_requests check URL : https://patchwork.freedesktop.org/series/54330/ State : warning == Summary == $ dim checkpatch origin/drm-tip bba6cf804980 drm/i915: Fix i915_gem_wait_for_idle oops due to active_requests check -:32: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line) #32: [2018-11-28 23:17:54] [12278.310417] kernel BUG at drivers/gpu/drm/i915/i915_gem.c:4702! total: 0 errors, 1 warnings, 0 checks, 49 lines checked ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/i915: Fix i915_gem_wait_for_idle oops due to active_requests check
== Series Details == Series: drm/i915: Fix i915_gem_wait_for_idle oops due to active_requests check URL : https://patchwork.freedesktop.org/series/54330/ State : warning == Summary == $ dim sparse origin/drm-tip Sparse version: v0.5.2 Commit: drm/i915: Fix i915_gem_wait_for_idle oops due to active_requests check -drivers/gpu/drm/i915/selftests/../i915_drv.h:3550:16: warning: expression using sizeof(void) +drivers/gpu/drm/i915/selftests/../i915_drv.h:3551:16: warning: expression using sizeof(void) ___ 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: Fix i915_gem_wait_for_idle oops due to active_requests check
== Series Details ==
Series: drm/i915: Fix i915_gem_wait_for_idle oops due to active_requests check
URL : https://patchwork.freedesktop.org/series/54330/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_5335 -> Patchwork_11133
Summary
---
**FAILURE**
Serious unknown changes coming with Patchwork_11133 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_11133, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL:
https://patchwork.freedesktop.org/api/1.0/series/54330/revisions/1/mbox/
Possible new issues
---
Here are the unknown changes that may have been introduced in Patchwork_11133:
### IGT changes ###
Possible regressions
* igt@i915_selftest@live_objects:
- fi-whl-u: PASS -> INCOMPLETE
- fi-kbl-8809g: PASS -> INCOMPLETE
- fi-kbl-7500u: PASS -> INCOMPLETE
- fi-kbl-x1275: PASS -> INCOMPLETE
- fi-hsw-peppy: PASS -> INCOMPLETE
- fi-skl-6600u: PASS -> INCOMPLETE
- fi-pnv-d510:PASS -> INCOMPLETE
- fi-hsw-4770r: PASS -> INCOMPLETE
- fi-skl-guc: PASS -> INCOMPLETE
- fi-blb-e6850: NOTRUN -> INCOMPLETE
- fi-skl-6700k2: PASS -> INCOMPLETE
- fi-skl-6260u: PASS -> INCOMPLETE
- fi-icl-u3: PASS -> INCOMPLETE
- fi-skl-6770hq: PASS -> INCOMPLETE
- fi-kbl-r: PASS -> INCOMPLETE
- fi-gdg-551: PASS -> INCOMPLETE
- fi-bwr-2160:PASS -> INCOMPLETE
- fi-bsw-kefka: PASS -> INCOMPLETE
- fi-kbl-guc: PASS -> INCOMPLETE
- fi-icl-u2: PASS -> INCOMPLETE
- fi-snb-2520m: PASS -> INCOMPLETE
- fi-kbl-7567u: PASS -> INCOMPLETE
- fi-skl-iommu: PASS -> INCOMPLETE
- fi-skl-6700hq: PASS -> INCOMPLETE
- fi-ivb-3520m: PASS -> INCOMPLETE
- fi-hsw-4770:PASS -> INCOMPLETE
- fi-ilk-650: PASS -> INCOMPLETE
- fi-bsw-n3050: PASS -> INCOMPLETE
* {igt@runner@aborted}:
- fi-ilk-650: NOTRUN -> FAIL
- fi-pnv-d510:NOTRUN -> FAIL
- fi-bdw-gvtdvm: NOTRUN -> FAIL
- fi-cfl-8109u: NOTRUN -> FAIL
- fi-hsw-peppy: NOTRUN -> FAIL
- fi-icl-u2: NOTRUN -> FAIL
- fi-gdg-551: NOTRUN -> FAIL
- fi-glk-dsi: NOTRUN -> FAIL
- fi-snb-2520m: NOTRUN -> FAIL
- fi-hsw-4770:NOTRUN -> FAIL
- fi-kbl-7500u: NOTRUN -> FAIL
- fi-bxt-j4205: NOTRUN -> FAIL
- fi-skl-6700hq: NOTRUN -> FAIL
- fi-whl-u: NOTRUN -> FAIL
- fi-icl-u3: NOTRUN -> FAIL
- fi-kbl-7560u: NOTRUN -> FAIL
- fi-bxt-dsi: NOTRUN -> FAIL
- fi-byt-j1900: NOTRUN -> FAIL
- fi-skl-iommu: NOTRUN -> FAIL
- fi-cfl-guc: NOTRUN -> FAIL
- fi-kbl-7567u: NOTRUN -> FAIL
- fi-skl-guc: NOTRUN -> FAIL
- fi-skl-6700k2: NOTRUN -> FAIL
- fi-bsw-n3050: NOTRUN -> FAIL
- fi-blb-e6850: NOTRUN -> FAIL
- fi-kbl-x1275: NOTRUN -> FAIL
- fi-bsw-kefka: NOTRUN -> FAIL
- fi-cfl-8700k: NOTRUN -> FAIL
- fi-hsw-4770r: NOTRUN -> FAIL
- fi-skl-6600u: NOTRUN -> FAIL
- fi-kbl-8809g: NOTRUN -> FAIL
- fi-byt-clapper: NOTRUN -> FAIL
- fi-apl-guc: NOTRUN -> FAIL
- fi-kbl-r: NOTRUN -> FAIL
- fi-bdw-5557u: NOTRUN -> FAIL
- fi-bwr-2160:NOTRUN -> FAIL
- fi-byt-n2820: NOTRUN -> FAIL
- fi-skl-6770hq: NOTRUN -> FAIL
- fi-kbl-guc: NOTRUN -> FAIL
- fi-skl-gvtdvm: NOTRUN -> FAIL
- fi-ivb-3520m: NOTRUN -> FAIL
- fi-skl-6260u: NOTRUN -> FAIL
- fi-elk-e7500: NOTRUN -> FAIL
Known issues
Here are the changes found in Patchwork_11133 that come from known issues:
### IGT changes ###
Issues hit
* igt@i915_selftest@live_objects:
- fi-skl-gvtdvm: PASS -> INCOMPLETE [fdo#105600]
- fi-bxt-dsi: PASS -> INCOMPLETE [fdo#103927]
- fi-bdw-5557u: PASS -> INCOMPLETE [fdo#107513]
- fi-cfl-8700k: PASS -> INCOMPLETE [fdo#108474]
- fi-glk-dsi: PASS -> INCOMPLETE [fdo#103359] / [k.org#198133]
- fi-apl-guc: PASS -> INCOMPLETE [fdo#103927]
- fi-cfl-guc: PASS -> INCOMPLETE [fdo#108474]
- fi-elk-e7500: PASS -> INCOMPLETE [fdo#103989]
- fi-bxt-j4205: PASS -> INCOMPLETE [fdo#103927]
- fi-kbl-7560u: PASS -> INCOMPLETE [fdo#108808]
- fi-byt-j1900: PASS -> INCOMPLETE [fdo#102657]
- fi-byt-clapper: PASS -> INCOMPLETE [fdo#102657]
- fi-cfl-8109u: PASS -> INCOMPLETE [fdo#108474]
- fi-byt-n2820: P
Re: [Intel-gfx] [PATCH i-g-t] i915/gem_workarounds: adding 0xe420 register to WO list
Talha Nassar writes:
> From: talha nassar
>
> HW team confirmed that this register is write-only.
It could be that it is that bit only. We don't need
to care about that now,
Reviewed-by: Mika Kuoppala
>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=107338
> Cc: Chris Wilson
> Cc: Mika Kuoppala
> Signed-off-by: talha nassar
> ---
> tests/i915/gem_workarounds.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/tests/i915/gem_workarounds.c b/tests/i915/gem_workarounds.c
> index 78478ad..b7d7d9c 100644
> --- a/tests/i915/gem_workarounds.c
> +++ b/tests/i915/gem_workarounds.c
> @@ -51,7 +51,8 @@ static struct write_only_list {
> unsigned int gen;
> uint32_t addr;
> } wo_list[] = {
> - { 10, 0xE5F0 } /* WaForceContextSaveRestoreNonCoherent:cnl */
> + { 10, 0xE5F0 }, /* WaForceContextSaveRestoreNonCoherent:cnl */
> + { 11, 0xE420 } /* WaEnableFloatBlendOptimization:icl */
>
> /*
>* FIXME: If you are contemplating adding stuff here
> --
> 2.7.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] i915/gem_workarounds: adding 0xe420 register to WO list
Quoting Mika Kuoppala (2018-12-20 10:54:33) > Talha Nassar writes: > > > From: talha nassar > > > > HW team confirmed that this register is write-only. > > It could be that it is that bit only. We don't need > to care about that now, > > Reviewed-by: Mika Kuoppala Please follow my suggestion to put this information into the w/a itself so that it also works in selftests/ -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Fix ILK-IVB primary plane enable delays
Primary and sprite plane enable on ILK-IVB may take two frames to complete Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103925 Signed-off-by: Juha-Pekka Heikkila --- drivers/gpu/drm/i915/intel_display.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 3b70948..b46ab48 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -10813,8 +10813,11 @@ int intel_plane_atomic_calc_changes(const struct intel_crtc_state *old_crtc_stat * Despite the w/a only being listed for IVB we assume that * the ILK/SNB note has similar ramifications, hence we apply * the w/a on all three platforms. +* +* With experimental results seems this is needed also for primary +* plane, not only sprite plane. */ - if (plane->id == PLANE_SPRITE0 && + if (plane->id != PLANE_CURSOR && (IS_GEN_RANGE(dev_priv, 5, 6) || IS_IVYBRIDGE(dev_priv)) && (turn_on || (!needs_scaling(old_plane_state) && -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [v4,1/3] ACPI / PMIC: Add support for executing PMIC MIPI sequence elements
== Series Details ==
Series: series starting with [v4,1/3] ACPI / PMIC: Add support for executing
PMIC MIPI sequence elements
URL : https://patchwork.freedesktop.org/series/54003/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_5311 -> Patchwork_11089
Summary
---
**WARNING**
Minor unknown changes coming with Patchwork_11089 need to be verified
manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_11089, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL:
https://patchwork.freedesktop.org/api/1.0/series/54003/revisions/1/mbox/
Possible new issues
---
Here are the unknown changes that may have been introduced in Patchwork_11089:
### IGT changes ###
Warnings
* igt@kms_flip@basic-flip-vs-dpms:
- fi-skl-6770hq: PASS -> SKIP +36
* igt@kms_pipe_crc_basic@nonblocking-crc-pipe-c:
- fi-kbl-7567u: PASS -> SKIP +33
* igt@pm_rpm@basic-pci-d3-state:
- fi-byt-n2820: PASS -> SKIP
Known issues
Here are the changes found in Patchwork_11089 that come from known issues:
### IGT changes ###
Issues hit
* igt@i915_module_load@reload-with-fault-injection:
- fi-kbl-7567u: PASS -> DMESG-WARN [fdo#105602] / [fdo#108529] +1
* igt@kms_flip@basic-flip-vs-dpms:
- fi-icl-u3: NOTRUN -> DMESG-WARN [fdo#108924] / [fdo#109044]
* igt@kms_pipe_crc_basic@read-crc-pipe-b:
- fi-byt-clapper: PASS -> FAIL [fdo#107362] +1
* igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
- fi-blb-e6850: PASS -> INCOMPLETE [fdo#107718]
* igt@pm_rpm@basic-rte:
- fi-byt-n2820: PASS -> FAIL [fdo#108800]
* igt@pm_rpm@module-reload:
- fi-kbl-7567u: PASS -> DMESG-WARN [fdo#108529]
* {igt@runner@aborted}:
- fi-icl-u3: NOTRUN -> FAIL [fdo#108924]
- fi-icl-y: NOTRUN -> FAIL [fdo#108070]
Possible fixes
* igt@gem_ctx_create@basic-files:
- fi-kbl-7560u: INCOMPLETE [fdo#103665] -> PASS
Warnings
* igt@kms_chamelium@common-hpd-after-suspend:
- fi-kbl-7567u: DMESG-WARN [fdo#108473] -> DMESG-FAIL [fdo#105079]
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
[fdo#105079]: https://bugs.freedesktop.org/show_bug.cgi?id=105079
[fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
[fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
[fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
[fdo#108070]: https://bugs.freedesktop.org/show_bug.cgi?id=108070
[fdo#108473]: https://bugs.freedesktop.org/show_bug.cgi?id=108473
[fdo#108529]: https://bugs.freedesktop.org/show_bug.cgi?id=108529
[fdo#108800]: https://bugs.freedesktop.org/show_bug.cgi?id=108800
[fdo#108924]: https://bugs.freedesktop.org/show_bug.cgi?id=108924
[fdo#109044]: https://bugs.freedesktop.org/show_bug.cgi?id=109044
Participating hosts (47 -> 45)
--
Additional (2): fi-icl-y fi-icl-u3
Missing(4): fi-kbl-soraka fi-ctg-p8600 fi-byt-squawks fi-ilk-m540
Build changes
-
* Linux: CI_DRM_5311 -> Patchwork_11089
CI_DRM_5311: a42fd8bf199784ee4ff1cdb5ee03eedd9a535d4a @
git://anongit.freedesktop.org/gfx-ci/linux
IGT_4746: 2c793666d8c8328733f5769b16ae5858fee97f3f @
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_11089: 715d38e2ddb4a7ba0a6df7f0be439a510a6df035 @
git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
715d38e2ddb4 drm/i915/intel_dsi_vbt: Add support for PMIC MIPI sequences
f8ff55af50c5 ACPI / PMIC: Implement exec_mipi_pmic_seq_element for CHT Whiskey
Cove PMIC
13a3d26bad7a ACPI / PMIC: Add support for executing PMIC MIPI sequence elements
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_11089/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 05/18] video/hdmi: Add an enum for HDMI packet types
Regards
Shashank
On 9/21/2018 12:21 AM, Ville Syrjala wrote:
From: Ville Syrjälä
We'll be wanting to send more than just infoframes over HDMI. So add an
enum for other packet types.
TODO: Maybe just include the infoframe types in the packet type enum
and get rid of the infoframe type enum?
Cc: Thierry Reding
Cc: Hans Verkuil
Cc: linux-me...@vger.kernel.org
Signed-off-by: Ville Syrjälä
---
include/linux/hdmi.h | 15 +++
1 file changed, 15 insertions(+)
diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
index c76b50a48e48..80521d9591a1 100644
--- a/include/linux/hdmi.h
+++ b/include/linux/hdmi.h
@@ -27,6 +27,21 @@
#include
#include
+enum hdmi_packet_type {
+ HDMI_PACKET_TYPE_NULL = 0x00,
+ HDMI_PACKET_TYPE_AUDIO_CLOCK_REGEN = 0x01,
+ HDMI_PACKET_TYPE_AUDIO_SAMPLE = 0x02,
+ HDMI_PACKET_TYPE_GENERAL_CONTROL = 0x03,
+ HDMI_PACKET_TYPE_AUDIO_CP = 0x04,
Call it TYPE_ACP instead of AUDIO_CP just to be inline with H14b spec
description ?
+ HDMI_PACKET_TYPE_ISRC1 = 0x05,
+ HDMI_PACKET_TYPE_ISRC2 = 0x06,
+ HDMI_PACKET_TYPE_ONE_BIT_AUDIO_SAMPLE = 0x07,
+ HDMI_PACKET_TYPE_DST_AUDIO = 0x08,
+ HDMI_PACKET_TYPE_HBR_AUDIO_STREAM = 0x09,
+ HDMI_PACKET_TYPE_GAMUT_METADATA = 0x0a,
+ /* + enum hdmi_infoframe_type */
+};
+
With or without the change suggested above:
Reviewed-by: Shashank Sharma
enum hdmi_infoframe_type {
HDMI_INFOFRAME_TYPE_VENDOR = 0x81,
HDMI_INFOFRAME_TYPE_AVI = 0x82,
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v9 08/39] drm/i915: hdcp1.4 CP_IRQ handling and SW encryption tracking
On 12/19/2018 9:18 PM, Daniel Vetter wrote:
On Thu, Dec 13, 2018 at 09:31:10AM +0530, Ramalingam C wrote:
"hdcp_encrypted" flag is defined to denote the HDCP1.4 encryption status.
This SW tracking is used to determine the need for real hdcp1.4 disable
and hdcp_check_link upon CP_IRQ.
On CP_IRQ we filter the CP_IRQ related to the states like Link failure
and reauthentication req etc and handle them in hdcp_check_link.
CP_IRQ corresponding to the authentication msg availability are ignored.
WARN_ON is added for the abrupt stop of HDCP encryption of a port.
Signed-off-by: Ramalingam C
---
drivers/gpu/drm/i915/intel_dp.c | 2 +-
drivers/gpu/drm/i915/intel_drv.h | 5 ++-
drivers/gpu/drm/i915/intel_hdcp.c | 89 ++-
3 files changed, 74 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index aba884c64879..89315e15fb34 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4758,7 +4758,7 @@ static void intel_dp_check_service_irq(struct intel_dp
*intel_dp)
intel_dp_handle_test_request(intel_dp);
if (val & DP_CP_IRQ)
- intel_hdcp_check_link(intel_dp->attached_connector);
+ intel_hdcp_handle_cp_irq(intel_dp->attached_connector);
if (val & DP_SINK_SPECIFIC_IRQ)
DRM_DEBUG_DRIVER("Sink specific irq unhandled\n");
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 191b6e0f086c..decd0346c6a7 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -393,6 +393,9 @@ struct intel_hdcp {
struct delayed_work check_work;
struct work_struct prop_work;
+ /* HDCP1.4 Encryption status */
+ u8 hdcp_encrypted;
Another bool I guess? Or unsigned : 1;
as per #intel-gfx discussion I will fallback to bool.
+
/* HDCP2.2 related definitions */
/* Flag indicates whether this connector supports HDCP2.2 or not. */
u8 hdcp2_supported;
@@ -2058,10 +2061,10 @@ int intel_hdcp_init(struct intel_connector *connector,
bool hdcp2_supported);
int intel_hdcp_enable(struct intel_connector *connector);
int intel_hdcp_disable(struct intel_connector *connector);
-int intel_hdcp_check_link(struct intel_connector *connector);
bool is_hdcp_supported(struct drm_i915_private *dev_priv, enum port port);
bool intel_hdcp_capable(struct intel_connector *connector);
bool is_hdcp2_supported(struct drm_i915_private *dev_priv);
+void intel_hdcp_handle_cp_irq(struct intel_connector *connector);
/* intel_psr.c */
#define CAN_PSR(dev_priv) (HAS_PSR(dev_priv) && dev_priv->psr.sink_support)
diff --git a/drivers/gpu/drm/i915/intel_hdcp.c
b/drivers/gpu/drm/i915/intel_hdcp.c
index 9405fce07b93..2b7814a6f12b 100644
--- a/drivers/gpu/drm/i915/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/intel_hdcp.c
@@ -75,6 +75,16 @@ bool intel_hdcp_capable(struct intel_connector *connector)
return capable;
}
+static inline bool intel_hdcp_in_use(struct intel_connector *connector)
+{
+ struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
+ enum port port = connector->encoder->port;
+ u32 reg;
+
+ reg = I915_READ(PORT_HDCP_STATUS(port));
+ return reg & HDCP_STATUS_ENC;
+}
+
static int intel_hdcp_poll_ksv_fifo(struct intel_digital_port *intel_dig_port,
const struct intel_hdcp_shim *shim)
{
@@ -669,6 +679,7 @@ static int _intel_hdcp_disable(struct intel_connector
*connector)
DRM_DEBUG_KMS("[%s:%d] HDCP is being disabled...\n",
connector->base.name, connector->base.base.id);
+ hdcp->hdcp_encrypted = 0;
I915_WRITE(PORT_HDCP_CONF(port), 0);
if (intel_wait_for_register(dev_priv, PORT_HDCP_STATUS(port), ~0, 0,
ENCRYPT_STATUS_CHANGE_TIMEOUT_MS)) {
@@ -714,8 +725,10 @@ static int _intel_hdcp_enable(struct intel_connector
*connector)
/* Incase of authentication failures, HDCP spec expects reauth. */
for (i = 0; i < tries; i++) {
ret = intel_hdcp_auth(conn_to_dig_port(connector), hdcp->shim);
- if (!ret)
+ if (!ret) {
+ hdcp->hdcp_encrypted = 1;
return 0;
+ }
DRM_DEBUG_KMS("HDCP Auth failure (%d)\n", ret);
@@ -742,16 +755,17 @@ int intel_hdcp_check_link(struct intel_connector *connector)
enum port port = intel_dig_port->base.port;
int ret = 0;
- if (!hdcp->shim)
- return -ENOENT;
-
mutex_lock(&hdcp->mutex);
- if (hdcp->value == DRM_MODE_CONTENT_PROTECTION_UNDESIRED)
+ /* Check_link valid only when HDCP1.4 is enabled */
+ if (hdcp->value != DRM_MODE_CONTENT_PROTECTION_ENABLED ||
+ !hdcp->hdcp_encrypted) {
+ ret = -EINVAL;
goto out;
Re: [Intel-gfx] [v5 1/2] drm: Add colorspace connector property
Regards
Shashank
On 12/11/2018 11:44 PM, Uma Shankar wrote:
This patch adds a colorspace connector property, enabling
userspace to switch to various supported colorspaces.
This will help enable BT2020 along with other colorspaces.
v2: Addressed Maarten and Ville's review comments. Enhanced
the colorspace enum to incorporate both HDMI and DP supported
colorspaces. Also, added a default option for colorspace.
v3: Removed Adobe references from enum definitions as per
Ville, Hans Verkuil and Jonas Karlman suggestions. Changed
Default to an unset state where driver will assign the colorspace
is not chosen by user, suggested by Ville and Maarten. Addressed
other misc review comments from Maarten. Split the changes to
have separate colorspace property for DP and HDMI.
v4: Addressed Chris and Ville's review comments, and created a
common colorspace property for DP and HDMI, filtered the list
based on the colorspaces supported by the respective protocol
standard.
v5: Made the property creation helper accept enum list based on
platform capabilties as suggested by Shashank. Consolidated HDMI
and DP property creation in the common helper.
Signed-off-by: Uma Shankar
---
drivers/gpu/drm/drm_atomic_uapi.c | 4 ++
drivers/gpu/drm/drm_connector.c | 82 +++
include/drm/drm_connector.h | 17
include/uapi/drm/drm_mode.h | 33
4 files changed, 136 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
b/drivers/gpu/drm/drm_atomic_uapi.c
index 86ac339..9df7520 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -729,6 +729,8 @@ static int drm_atomic_connector_set_property(struct
drm_connector *connector,
return -EINVAL;
}
state->content_protection = val;
+ } else if (property == connector->colorspace_property) {
+ state->colorspace = val;
} else if (property == config->writeback_fb_id_property) {
struct drm_framebuffer *fb = drm_framebuffer_lookup(dev, NULL,
val);
int ret = drm_atomic_set_writeback_fb_for_connector(state, fb);
@@ -797,6 +799,8 @@ static int drm_atomic_connector_set_property(struct
drm_connector *connector,
*val = state->picture_aspect_ratio;
} else if (property == config->content_type_property) {
*val = state->content_type;
+ } else if (property == connector->colorspace_property) {
+ *val = state->colorspace;
} else if (property == connector->scaling_mode_property) {
*val = state->scaling_mode;
} else if (property == connector->content_protection_property) {
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index fa9baac..46928f7 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -826,6 +826,54 @@ int drm_display_info_set_bus_formats(struct
drm_display_info *info,
};
DRM_ENUM_NAME_FN(drm_get_content_protection_name, drm_cp_enum_list)
+/* List of HDMI Colorspaces supported */
+static const struct drm_prop_enum_list hdmi_colorspace[] = {
Should we call this list hdmi_colorspaces :) ?
+ /* For Default case, driver will set the colorspace */
+ { COLORIMETRY_DEFAULT, "Default" },
+ /* Standard Definition Colorimetry based on CEA 861 */
+ { COLORIMETRY_ITU_601, "ITU_601" },
+ { COLORIMETRY_ITU_709, "ITU_709" },
+ /* Standard Definition Colorimetry based on IEC 61966-2-4 */
+ { COLORIMETRY_XV_YCC_601, "XV_YCC_601" },
+ /* High Definition Colorimetry based on IEC 61966-2-4 */
+ { COLORIMETRY_XV_YCC_709, "XV_YCC_709" },
+ /* Colorimetry based on IEC 61966-2-1/Amendment 1 */
+ { COLORIMETRY_S_YCC_601, "S_YCC_601" },
+ /* Colorimetry based on IEC 61966-2-5 [33] */
+ { COLORIMETRY_OPYCC_601, "opYCC_601" },
+ /* Colorimetry based on IEC 61966-2-5 */
+ { COLORIMETRY_OPRGB, "opRGB" },
+ /* Colorimetry based on ITU-R BT.2020 */
+ { COLORIMETRY_BT2020_RGB, "BT2020_RGB" },
+ /* Colorimetry based on ITU-R BT.2020 */
+ { COLORIMETRY_BT2020_YCC, "BT2020_YCC" },
+ /* Colorimetry based on ITU-R BT.2020 */
+ { COLORIMETRY_BT2020_CYCC, "BT2020_CYCC" },
+};
+
+/* List of DP Colorspaces supported */
Same here for dp_colorspaces ?
+static const struct drm_prop_enum_list dp_colorspace[] = {
+ /* For Default case, driver will set the colorspace */
+ { COLORIMETRY_DEFAULT, "Default" },
+ /* Standard Definition Colorimetry based on CEA 861 */
+ { COLORIMETRY_ITU_601, "ITU_601" },
+ { COLORIMETRY_ITU_709, "ITU_709" },
+ /* Standard Definition Colorimetry based on IEC 61966-2-4 */
+ { COLORIMETRY_XV_YCC_601, "XV_YCC_601" },
+ /* High Definition Colorimetry based on IEC 61966-2-4 */
+ { COLORIMETRY_XV_YCC_709, "XV_YCC_709" },
+ /* Colorimetry
[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Fix ILK-IVB primary plane enable delays
== Series Details ==
Series: drm/i915: Fix ILK-IVB primary plane enable delays
URL : https://patchwork.freedesktop.org/series/54336/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_5335 -> Patchwork_11134
Summary
---
**FAILURE**
Serious unknown changes coming with Patchwork_11134 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_11134, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL:
https://patchwork.freedesktop.org/api/1.0/series/54336/revisions/1/mbox/
Possible new issues
---
Here are the unknown changes that may have been introduced in Patchwork_11134:
### IGT changes ###
Possible regressions
* igt@i915_selftest@live_hangcheck:
- fi-apl-guc: PASS -> DMESG-FAIL
Warnings
* igt@pm_rpm@basic-pci-d3-state:
- fi-bsw-kefka: SKIP -> PASS
Known issues
Here are the changes found in Patchwork_11134 that come from known issues:
### IGT changes ###
Issues hit
* igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
- fi-byt-clapper: PASS -> FAIL [fdo#103191] / [fdo#107362]
* {igt@runner@aborted}:
- fi-icl-u2: NOTRUN -> FAIL [fdo#108866]
- fi-bsw-n3050: NOTRUN -> FAIL [fdo#108656]
Possible fixes
* igt@i915_module_load@reload:
- fi-blb-e6850: INCOMPLETE [fdo#107718] -> PASS
* igt@kms_chamelium@hdmi-hpd-fast:
- fi-kbl-7500u: FAIL [fdo#108767] -> PASS
* igt@kms_frontbuffer_tracking@basic:
- fi-hsw-peppy: DMESG-WARN [fdo#102614] -> PASS
* igt@kms_pipe_crc_basic@read-crc-pipe-b-frame-sequence:
- fi-byt-clapper: FAIL [fdo#103191] / [fdo#107362] -> PASS
* igt@pm_rpm@basic-rte:
- fi-bsw-kefka: FAIL [fdo#108800] -> PASS
Warnings
* igt@gem_ctx_create@basic-files:
- fi-bsw-n3050: FAIL [fdo#108656] -> DMESG-FAIL [fdo#108656]
* igt@i915_selftest@live_contexts:
- fi-icl-u2: DMESG-FAIL [fdo#108569] -> INCOMPLETE [fdo#108315]
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
[fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
[fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
[fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
[fdo#108315]: https://bugs.freedesktop.org/show_bug.cgi?id=108315
[fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
[fdo#108656]: https://bugs.freedesktop.org/show_bug.cgi?id=108656
[fdo#108767]: https://bugs.freedesktop.org/show_bug.cgi?id=108767
[fdo#108800]: https://bugs.freedesktop.org/show_bug.cgi?id=108800
[fdo#108866]: https://bugs.freedesktop.org/show_bug.cgi?id=108866
Participating hosts (49 -> 45)
--
Missing(4): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan
Build changes
-
* Linux: CI_DRM_5335 -> Patchwork_11134
CI_DRM_5335: d1bb3d7e01b06108d5f326706a1cb5d305a847c7 @
git://anongit.freedesktop.org/gfx-ci/linux
IGT_4751: 47f5a57a81b66c21d06695e263a22b87f5a33009 @
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_11134: 1aca3e8e043e437ca29224b347c1ddc8e2952727 @
git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
1aca3e8e043e drm/i915: Fix ILK-IVB primary plane enable delays
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_11134/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] uABI / Removing DRM_I915_LOW_LEVEL_TRACEPOINTS Kconfig
Quoting Steven Rostedt (2018-12-19 21:22:57)
> On Wed, 19 Dec 2018 12:08:18 +0200
> Joonas Lahtinen wrote:
>
> > To me, it seems almost as if folks are too preoccupied with thinking if
> > we somehow can do this through tracepoints, to stop and actually think
> > if we should.
>
> Regardless of whether it should or shouldn't, one solution to this is
> to make the trace event in question record basically nothing but a
> pointer.
>
> DECLARE_EVENT_CLASS(i915_hw_request,
> TP_PROTO(struct i915_request *rq),
> TP_ARGS(rq),
>
> TP_STRUCT__entry(
> __field(void *, rq)
> ),
>
> TP_fast_assign(
> __entry->rq = rq;
>),
>
> TP_printk("rq=%p", __entry->rq)
> );
>
> Define the events from that:
>
> DEFINE_EVENT(i915_hw_request, i915_request_submit,
> TP_PROTO(struct i915_request *rq),
> TP_ARGS(rq)
> );
>
>
> No tool can use that information. But for those that want to, you make a
> separate module that you can load that has:
That's a nifty idea.
If the module is built out-of-tree from gpuvis package, you'd
still lose the functionality on kernel update, though. But I
guess it'd then be a problem of the distro that chooses to
package gpuvis/gputop/whatever profiler in stable manner.
On the other hand, if we would have eBPF based (kudos to Chris for
mentioning) module in-tree for userspace to hook to, it would
make things more easily fixable from userspace
You could for example automate the download a matching piece
of eBPF to execute on the module for a kernel version.
So the lines definitely get blurred here, about what is breaking
userspace and what is userspace deliberately setting itself up for
breaking.
*But* before we seek for a judgement on that. We're still having
the peculiarity that the tracepoints would shortly have to be
generated based from hardware event log.
Do you have an idea how to solve that elegantly? Or do you rule such
thing to be outside of the scope for tracepoints?
So if we still end up with a hybrid solution where another uAPI is
needed for hardware events, I'm not sure if above is worthy the hassle
when we can just add to the other uAPI the same events.
Regards, Joonas
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v9 07/39] drm/i915: MEI interface definition
On 12/19/2018 8:51 PM, Daniel Vetter wrote: Indeed, I overlooked that. Maybe highlight it a bit more with a separate if (!CONFIG_ENABLED(MEI_HDCP)) return false; so it stick out more in the previous patch. Currently it's a bit burried. With that + the init ordering fixed: Reviewed-by: Daniel Vetter Sure. Thank you. There's no need to split the patch out anymore, since without the mei patches you can't enable that Kconfig option and hence no bisect issues. Still Kconfig and makefile is added at 22nd patch but component support in mei_hdcp is added at 35th patch. So even now this chunk needs to be kept after the component support addition. -Ram Cheers, Daniel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 1/3] drm/i915: Add an update_pipe callback to intel_encoder and call this on fastsets (v2)
When we are doing a fastset (needs_modeset=false, update_pipe=true) we
may need to update some encoder-level things such as checking that PSR
is enabled.
This commit adds an update_pipe callback to intel_encoder and a new
intel_encoders_update_pipe helper which calls this for all encoders
connected to a crtc. The new intel_encoders_update_pipe helper is called
from intel_update_crtc when doing a fastset.
Changes in v2:
-Name the new encoder callback update_pipe instead of just update
Reviewed-by: Maarten Lankhorst
Signed-off-by: Hans de Goede
---
drivers/gpu/drm/i915/intel_display.c | 23 +++
drivers/gpu/drm/i915/intel_drv.h | 3 +++
2 files changed, 26 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index a858238972a2..2876a3752079 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5578,6 +5578,26 @@ static void intel_encoders_post_pll_disable(struct
drm_crtc *crtc,
}
}
+static void intel_encoders_update_pipe(struct drm_crtc *crtc,
+ struct intel_crtc_state *crtc_state,
+ struct drm_atomic_state *old_state)
+{
+ struct drm_connector_state *conn_state;
+ struct drm_connector *conn;
+ int i;
+
+ for_each_new_connector_in_state(old_state, conn, conn_state, i) {
+ struct intel_encoder *encoder =
+ to_intel_encoder(conn_state->best_encoder);
+
+ if (conn_state->crtc != crtc)
+ continue;
+
+ if (encoder->update_pipe)
+ encoder->update_pipe(encoder, crtc_state, conn_state);
+ }
+}
+
static void ironlake_crtc_enable(struct intel_crtc_state *pipe_config,
struct drm_atomic_state *old_state)
{
@@ -12750,6 +12770,9 @@ static void intel_update_crtc(struct drm_crtc *crtc,
} else {
intel_pre_plane_update(to_intel_crtc_state(old_crtc_state),
pipe_config);
+
+ if (pipe_config->update_pipe)
+ intel_encoders_update_pipe(crtc, pipe_config, state);
}
if (new_plane_state)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index ef1315d7c4ae..8717b873cd53 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -243,6 +243,9 @@ struct intel_encoder {
void (*post_pll_disable)(struct intel_encoder *,
const struct intel_crtc_state *,
const struct drm_connector_state *);
+ void (*update_pipe)(struct intel_encoder *,
+ const struct intel_crtc_state *,
+ const struct drm_connector_state *);
/* Read out the current hw state of this connector, returning true if
* the encoder is active. If the encoder is enabled it also set the pipe
* it is connected to in the pipe parameter. */
--
2.20.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 3/3] drm/i915: DDI: call intel_psr_ and _edp_drrs_enable() on pipe updates (v2)
Call intel_psr_enable() and intel_edp_drrs_enable() on pipe updates to make
sure that we enable PSR / DRRS (when applicable) on fastsets.
Note calling these functions when PSR / DRRS has already been enabled is a
no-op, so it is safe to do this on every encoder->update_pipe callback.
Changes in v2:
-Merge the patches adding the intel_psr_enable() and intel_edp_drrs_enable()
calls into a single patch
Reviewed-by: Maarten Lankhorst
Signed-off-by: Hans de Goede
---
drivers/gpu/drm/i915/intel_ddi.c | 19 +++
1 file changed, 19 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index e3cc19e19199..fdf57f451b72 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -3537,6 +3537,24 @@ static void intel_disable_ddi(struct intel_encoder
*encoder,
intel_disable_ddi_dp(encoder, old_crtc_state, old_conn_state);
}
+static void intel_ddi_update_pipe_dp(struct intel_encoder *encoder,
+const struct intel_crtc_state *crtc_state,
+const struct drm_connector_state
*conn_state)
+{
+ struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
+
+ intel_psr_enable(intel_dp, crtc_state);
+ intel_edp_drrs_enable(intel_dp, crtc_state);
+}
+
+static void intel_ddi_update_pipe(struct intel_encoder *encoder,
+ const struct intel_crtc_state *crtc_state,
+ const struct drm_connector_state *conn_state)
+{
+ if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
+ intel_ddi_update_pipe_dp(encoder, crtc_state, conn_state);
+}
+
static void intel_ddi_set_fia_lane_count(struct intel_encoder *encoder,
const struct intel_crtc_state
*pipe_config,
enum port port)
@@ -4169,6 +4187,7 @@ void intel_ddi_init(struct drm_i915_private *dev_priv,
enum port port)
intel_encoder->pre_enable = intel_ddi_pre_enable;
intel_encoder->disable = intel_disable_ddi;
intel_encoder->post_disable = intel_ddi_post_disable;
+ intel_encoder->update_pipe = intel_ddi_update_pipe;
intel_encoder->get_hw_state = intel_ddi_get_hw_state;
intel_encoder->get_config = intel_ddi_get_config;
intel_encoder->suspend = intel_ddi_encoder_suspend;
--
2.20.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 2/3] drm/i915: Allow calling intel_edp_drrs_enable twice
Do not make it an error to call intel_edp_drrs_enable while drrs has
already been enabled, instead exit silently in this case.
This is a preparation patch for ensuring that DRRS is enabled on fastsets.
Note that the removed WARN_ON could also be triggered from userspace
through the i915_drrs_ctl debugfs entry which was added by
commit 35954e88bc50 ("drm/i915: Runtime disable for eDP DRRS")
Reviewed-by: Maarten Lankhorst
Signed-off-by: Hans de Goede
---
drivers/gpu/drm/i915/intel_dp.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 5b601b754707..62fd11540942 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -6432,8 +6432,8 @@ void intel_edp_drrs_enable(struct intel_dp *intel_dp,
}
mutex_lock(&dev_priv->drrs.mutex);
- if (WARN_ON(dev_priv->drrs.dp)) {
- DRM_ERROR("DRRS already enabled\n");
+ if (dev_priv->drrs.dp) {
+ DRM_DEBUG_KMS("DRRS already enabled\n");
goto unlock;
}
--
2.20.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/3] drm/i915/icl: Work around broken VBTs for port F detection
VBT may include incorrect information about the presence of port F. Work around this on SKUs where we know the port is not present. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108915 Cc: Mika Kahola Cc: Jani Nikula Signed-off-by: Imre Deak --- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/intel_display.c | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 815db160b966..13f94c4cc062 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2317,6 +2317,7 @@ intel_info(const struct drm_i915_private *dev_priv) (dev_priv)->info.gt == 3) #define IS_CNL_WITH_PORT_F(dev_priv) (IS_CANNONLAKE(dev_priv) && \ (INTEL_DEVID(dev_priv) & 0x0004) == 0x0004) +#define IS_ICL_WITH_PORT_F(dev_priv) (INTEL_DEVID(dev_priv) != 0x8A51) #define IS_ALPHA_SUPPORT(intel_info) ((intel_info)->is_alpha_support) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 2b81da068010..bd7fdaf7e093 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -14276,8 +14276,10 @@ static void intel_setup_outputs(struct drm_i915_private *dev_priv) /* * On some ICL SKUs port F is not present. No strap bits for * this, so rely on VBT. +* Work around broken VBTs on SKUs known to have no port F. */ - if (intel_bios_is_port_present(dev_priv, PORT_F)) + if (IS_ICL_WITH_PORT_F(dev_priv) && + intel_bios_is_port_present(dev_priv, PORT_F)) intel_ddi_init(dev_priv, PORT_F); icl_dsi_init(dev_priv); -- 2.13.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/3] drm/i915/ddi: Move DDI port detection to the corresponding helper
We have already a function to detect DDI ports using VBT, so instead of
opencoding the DDI specific version of this, move the opencoded part to
the existing helper.
Cc: Jani Nikula
Cc: Mika Kahola
Signed-off-by: Imre Deak
---
drivers/gpu/drm/i915/intel_bios.c| 9 +
drivers/gpu/drm/i915/intel_display.c | 4 +---
2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_bios.c
b/drivers/gpu/drm/i915/intel_bios.c
index 764d84d4109b..fa4091c0768b 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -1947,6 +1947,15 @@ bool intel_bios_is_port_present(struct drm_i915_private
*dev_priv, enum port por
};
int i;
+ if (HAS_DDI(dev_priv)) {
+ const struct ddi_vbt_port_info *port_info =
+ &dev_priv->vbt.ddi_port_info[port];
+
+ return port_info->supports_dp ||
+ port_info->supports_dvi ||
+ port_info->supports_hdmi;
+ }
+
/* FIXME maybe deal with port A as well? */
if (WARN_ON(port == PORT_A) || port >= ARRAY_SIZE(port_mapping))
return false;
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 3b7094822aa9..a2f8aaf61c61 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14315,9 +14315,7 @@ static void intel_setup_outputs(struct drm_i915_private
*dev_priv)
* On SKL we don't have a way to detect DDI-E so we rely on VBT.
*/
if (IS_GEN9_BC(dev_priv) &&
- (dev_priv->vbt.ddi_port_info[PORT_E].supports_dp ||
-dev_priv->vbt.ddi_port_info[PORT_E].supports_dvi ||
-dev_priv->vbt.ddi_port_info[PORT_E].supports_hdmi))
+ intel_bios_is_port_present(dev_priv, PORT_E))
intel_ddi_init(dev_priv, PORT_E);
} else if (HAS_PCH_SPLIT(dev_priv)) {
--
2.13.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/3] drm/i915/icl: Detect port F presence via VBT
Registering an output for a non-existent port (on a given SKU) can lead
to problems when trying to use the port, for instance timeouts during
power well enabling. Since there are no strap bits for port detection we
have to rely on VBT for this, so do that here.
There are no known SKUs where any of the A-E ports are non-existent, so
to reduce the likelihood of breakage due to incorrect VBT information,
do this detection only for port F (which is known to be missing on some
ICL SKUs).
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108915
Cc: Mika Kahola
Cc: Jani Nikula
Signed-off-by: Imre Deak
---
drivers/gpu/drm/i915/intel_display.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index a2f8aaf61c61..2b81da068010 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14273,7 +14273,13 @@ static void intel_setup_outputs(struct
drm_i915_private *dev_priv)
intel_ddi_init(dev_priv, PORT_C);
intel_ddi_init(dev_priv, PORT_D);
intel_ddi_init(dev_priv, PORT_E);
- intel_ddi_init(dev_priv, PORT_F);
+ /*
+* On some ICL SKUs port F is not present. No strap bits for
+* this, so rely on VBT.
+*/
+ if (intel_bios_is_port_present(dev_priv, PORT_F))
+ intel_ddi_init(dev_priv, PORT_F);
+
icl_dsi_init(dev_priv);
} else if (IS_GEN9_LP(dev_priv)) {
/*
--
2.13.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] drm/i915: Disable FBC on fastset if necessary
Hi,
On 18-12-18 16:27, Maarten Lankhorst wrote:
Without this, we will get a dmesg-warn when enable_fbc is cleared on a fastset:
WARN_ON(!crtc_state->enable_fbc)
WARNING: CPU: 0 PID: 1090 at drivers/gpu/drm/i915/intel_fbc.c:1091
intel_fbc_enable+0x2ce/0x580 [i915]
RIP: 0010:intel_fbc_enable+0x2ce/0x580 [i915]
Call Trace:
? __mutex_unlock_slowpath+0x46/0x2b0
intel_update_crtc+0x6f/0x2b0 [i915]
skl_update_crtcs+0x1d1/0x2b0 [i915]
intel_atomic_commit_tail+0x1ea/0xdb0 [i915]
intel_atomic_commit+0x244/0x330 [i915]
drm_mode_atomic_ioctl+0x85d/0x950
? drm_atomic_set_property+0x970/0x970
drm_ioctl_kernel+0x81/0xf0
drm_ioctl+0x2de/0x390
? drm_atomic_set_property+0x970/0x970
? __handle_mm_fault+0x81b/0xfc0
do_vfs_ioctl+0xa0/0x6e0
? __do_page_fault+0x2a5/0x550
ksys_ioctl+0x35/0x60
__x64_sys_ioctl+0x11/0x20
do_syscall_64+0x55/0x190
entry_SYSCALL_64_after_hwframe+0x49/0xbe
Signed-off-by: Maarten Lankhorst
---
drivers/gpu/drm/i915/intel_display.c | 3 +++
drivers/gpu/drm/i915/intel_fbc.c | 2 --
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 825d9b9e7f28..a0fae61028d6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5355,6 +5355,9 @@ static void intel_pre_plane_update(struct
intel_crtc_state *old_crtc_state,
if (hsw_pre_update_disable_ips(old_crtc_state, pipe_config))
hsw_disable_ips(old_crtc_state);
+ if (pipe_config->update_pipe && !pipe_config->enable_fbc)
+ intel_fbc_disable(crtc);
+
if (old_primary_state) {
struct intel_plane_state *new_primary_state =
intel_atomic_get_new_plane_state(old_intel_state,
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 1d3ff026d1bc..ccd5e110a19c 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -1129,8 +1129,6 @@ void intel_fbc_disable(struct intel_crtc *crtc)
if (!fbc_supported(dev_priv))
return;
- WARN_ON(crtc->active);
-
mutex_lock(&fbc->lock);
if (fbc->crtc == crtc)
__intel_fbc_disable(dev_priv);
Hmm, normally we call intel_fbc_disable() from the first
for_each_oldnew_crtc_in_state() loop in intel_atomic_commit_tail()
that has a:
if (!needs_modeset(new_crtc_state))
continue;
Causing it to be skipped on fastsets. But on a full modeset
that first loop also calls intel_pre_plane_update() directly
after the needs_modeset() call and before it will call
intel_fbc_disable() itself.
So withn a full modeset your patch is causing disable_fbc to be
called earlier (when moving to a new state without fbc) then before.
Before your patch on a full modeset we would do:
intel_pre_plane_update()
intel_crtc_disable_planes()
dev_priv->display.crtc_disable()
intel_fbc_disable(intel_crtc);
On a full modeset, but now we are doing:
intel_pre_plane_update() ->
intel_fbc_disable(intel_crtc);
intel_crtc_disable_planes()
dev_priv->display.crtc_disable()
intel_fbc_disable(intel_crtc); /* Second call is a no-op */
Which seems like an undesirable side-effect of this patch.
I think it would be better to instead add the:
if (pipe_config->update_pipe && !pipe_config->enable_fbc)
intel_fbc_disable(crtc);
In intel_update_crtc() in the else block of the if (modeset) {} else {}
in that function, after the intel_pre_plane_update() call, so that we
do the pre_plane_update() vs fbc_disable() in the same order as
in the full modeset path and so that we don't change the call
order of the full modeset path.
This will also nicely group it together with the
intel_encoders_update_pipe() call which my fastset PSR / DRRS
patches add (*).
I'm still learning the i915 code so I may be wrong here,
but the approach I'm suggesting seems better to me.
Regards,
Hans
*) Which will cause a conflict when merging both, but fixing that
is easy.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Fix ILK-IVB primary plane enable delays (rev2)
== Series Details == Series: drm/i915: Fix ILK-IVB primary plane enable delays (rev2) URL : https://patchwork.freedesktop.org/series/54336/ State : success == Summary == CI Bug Log - changes from CI_DRM_5336 -> Patchwork_11135 Summary --- **WARNING** Minor unknown changes coming with Patchwork_11135 need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_11135, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. External URL: https://patchwork.freedesktop.org/api/1.0/series/54336/revisions/2/mbox/ Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_11135: ### IGT changes ### Warnings * igt@amdgpu/amd_basic@cs-sdma: - fi-kbl-8809g: PASS -> NOTRUN +27 * igt@amdgpu/amd_basic@query-info: - fi-bsw-kefka: NOTRUN -> SKIP +56 * igt@gem_mmap_gtt@basic-write-gtt: - fi-blb-e6850: NOTRUN -> PASS +162 * igt@kms_addfb_basic@invalid-set-prop-any: - fi-glk-dsi: NOTRUN -> PASS +155 * igt@kms_busy@basic-flip-a: - fi-kbl-7567u: SKIP -> PASS +2 * igt@kms_force_connector_basic@force-edid: - fi-glk-dsi: NOTRUN -> SKIP +36 * igt@pm_rpm@module-reload: - fi-blb-e6850: NOTRUN -> SKIP +49 * igt@prime_vgem@basic-fence-read: - fi-bsw-kefka: NOTRUN -> PASS +230 Known issues Here are the changes found in Patchwork_11135 that come from known issues: ### IGT changes ### Issues hit * igt@amdgpu/amd_basic@userptr: - fi-kbl-8809g: PASS -> DMESG-WARN [fdo#108965] * igt@gem_ctx_create@basic-files: - fi-bsw-n3050: PASS -> FAIL [fdo#108656] * igt@kms_busy@basic-flip-b: - fi-ilk-650: PASS -> DMESG-WARN [fdo#106387] * igt@kms_chamelium@hdmi-hpd-fast: - fi-kbl-7500u: PASS -> FAIL [fdo#108767] * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b: - fi-byt-clapper: PASS -> FAIL [fdo#103191] / [fdo#107362] * igt@pm_rpm@basic-rte: - fi-bsw-kefka: NOTRUN -> FAIL [fdo#108800] Possible fixes * igt@amdgpu/amd_basic@cs-compute: - fi-kbl-8809g: FAIL [fdo#108094] -> NOTRUN * igt@amdgpu/amd_prime@amd-to-i915: - fi-kbl-8809g: FAIL [fdo#107341] -> NOTRUN * igt@gem_ctx_create@basic-files: - fi-bsw-kefka: INCOMPLETE [fdo#105876] / [fdo#108714] -> PASS * igt@gem_exec_suspend@basic-s4-devices: - fi-blb-e6850: INCOMPLETE [fdo#107718] -> PASS * igt@gem_mmap_gtt@basic-small-copy-xy: - fi-glk-dsi: INCOMPLETE [fdo#103359] / [k.org#198133] -> PASS * igt@prime_vgem@basic-fence-flip: - fi-gdg-551: FAIL [fdo#103182] -> PASS [fdo#103182]: https://bugs.freedesktop.org/show_bug.cgi?id=103182 [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191 [fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359 [fdo#105876]: https://bugs.freedesktop.org/show_bug.cgi?id=105876 [fdo#106387]: https://bugs.freedesktop.org/show_bug.cgi?id=106387 [fdo#107341]: https://bugs.freedesktop.org/show_bug.cgi?id=107341 [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362 [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718 [fdo#108094]: https://bugs.freedesktop.org/show_bug.cgi?id=108094 [fdo#108656]: https://bugs.freedesktop.org/show_bug.cgi?id=108656 [fdo#108714]: https://bugs.freedesktop.org/show_bug.cgi?id=108714 [fdo#108767]: https://bugs.freedesktop.org/show_bug.cgi?id=108767 [fdo#108800]: https://bugs.freedesktop.org/show_bug.cgi?id=108800 [fdo#108965]: https://bugs.freedesktop.org/show_bug.cgi?id=108965 [k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133 Participating hosts (49 -> 44) -- Additional (1): fi-skl-6700hq Missing(6): fi-kbl-soraka fi-hsw-4770r fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 Build changes - * Linux: CI_DRM_5336 -> Patchwork_11135 CI_DRM_5336: 9fd194c1f8bd36cb623d842f5ed52b2ca2c83d18 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4752: 321fe77d32fef32ef820f53924045fe6ef0cf6ed @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_11135: 6bc271d5db131059da7b77bbe00bd2d0165e3b01 @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == 6bc271d5db13 drm/i915: Fix ILK-IVB primary plane enable delays == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_11135/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,1/3] drm/i915: Add an update_pipe callback to intel_encoder and call this on fastsets (v2)
== Series Details == Series: series starting with [v2,1/3] drm/i915: Add an update_pipe callback to intel_encoder and call this on fastsets (v2) URL : https://patchwork.freedesktop.org/series/54339/ State : warning == Summary == $ dim checkpatch origin/drm-tip 79612d0a1354 drm/i915: Add an update_pipe callback to intel_encoder and call this on fastsets (v2) -:71: WARNING:FUNCTION_ARGUMENTS: function definition argument 'struct intel_encoder *' should also have an identifier name #71: FILE: drivers/gpu/drm/i915/intel_drv.h:246: + void (*update_pipe)(struct intel_encoder *, -:71: WARNING:FUNCTION_ARGUMENTS: function definition argument 'const struct intel_crtc_state *' should also have an identifier name #71: FILE: drivers/gpu/drm/i915/intel_drv.h:246: + void (*update_pipe)(struct intel_encoder *, -:71: WARNING:FUNCTION_ARGUMENTS: function definition argument 'const struct drm_connector_state *' should also have an identifier name #71: FILE: drivers/gpu/drm/i915/intel_drv.h:246: + void (*update_pipe)(struct intel_encoder *, total: 0 errors, 3 warnings, 0 checks, 44 lines checked 8b9d52c1738e drm/i915: Allow calling intel_edp_drrs_enable twice d15d2e96832a drm/i915: DDI: call intel_psr_ and _edp_drrs_enable() on pipe updates (v2) -:14: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line) #14: -Merge the patches adding the intel_psr_enable() and intel_edp_drrs_enable() total: 0 errors, 1 warnings, 0 checks, 31 lines checked ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] drm/i915: Disable FBC on fastset if necessary
Hi,
On 20-12-18 14:43, Hans de Goede wrote:
Hi,
On 18-12-18 16:27, Maarten Lankhorst wrote:
Without this, we will get a dmesg-warn when enable_fbc is cleared on a fastset:
WARN_ON(!crtc_state->enable_fbc)
WARNING: CPU: 0 PID: 1090 at drivers/gpu/drm/i915/intel_fbc.c:1091
intel_fbc_enable+0x2ce/0x580 [i915]
RIP: 0010:intel_fbc_enable+0x2ce/0x580 [i915]
Call Trace:
? __mutex_unlock_slowpath+0x46/0x2b0
intel_update_crtc+0x6f/0x2b0 [i915]
skl_update_crtcs+0x1d1/0x2b0 [i915]
intel_atomic_commit_tail+0x1ea/0xdb0 [i915]
intel_atomic_commit+0x244/0x330 [i915]
drm_mode_atomic_ioctl+0x85d/0x950
? drm_atomic_set_property+0x970/0x970
drm_ioctl_kernel+0x81/0xf0
drm_ioctl+0x2de/0x390
? drm_atomic_set_property+0x970/0x970
? __handle_mm_fault+0x81b/0xfc0
do_vfs_ioctl+0xa0/0x6e0
? __do_page_fault+0x2a5/0x550
ksys_ioctl+0x35/0x60
__x64_sys_ioctl+0x11/0x20
do_syscall_64+0x55/0x190
entry_SYSCALL_64_after_hwframe+0x49/0xbe
Signed-off-by: Maarten Lankhorst
---
drivers/gpu/drm/i915/intel_display.c | 3 +++
drivers/gpu/drm/i915/intel_fbc.c | 2 --
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 825d9b9e7f28..a0fae61028d6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5355,6 +5355,9 @@ static void intel_pre_plane_update(struct
intel_crtc_state *old_crtc_state,
if (hsw_pre_update_disable_ips(old_crtc_state, pipe_config))
hsw_disable_ips(old_crtc_state);
+ if (pipe_config->update_pipe && !pipe_config->enable_fbc)
+ intel_fbc_disable(crtc);
+
if (old_primary_state) {
struct intel_plane_state *new_primary_state =
intel_atomic_get_new_plane_state(old_intel_state,
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 1d3ff026d1bc..ccd5e110a19c 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -1129,8 +1129,6 @@ void intel_fbc_disable(struct intel_crtc *crtc)
if (!fbc_supported(dev_priv))
return;
- WARN_ON(crtc->active);
-
mutex_lock(&fbc->lock);
if (fbc->crtc == crtc)
__intel_fbc_disable(dev_priv);
Hmm, normally we call intel_fbc_disable() from the first
for_each_oldnew_crtc_in_state() loop in intel_atomic_commit_tail()
that has a:
if (!needs_modeset(new_crtc_state))
continue;
Causing it to be skipped on fastsets. But on a full modeset
that first loop also calls intel_pre_plane_update() directly
after the needs_modeset() call and before it will call
intel_fbc_disable() itself.
So withn a full modeset your patch is causing disable_fbc to be
called earlier (when moving to a new state without fbc) then before.
Before your patch on a full modeset we would do:
intel_pre_plane_update()
intel_crtc_disable_planes()
dev_priv->display.crtc_disable()
intel_fbc_disable(intel_crtc);
On a full modeset, but now we are doing:
intel_pre_plane_update() ->
intel_fbc_disable(intel_crtc);
intel_crtc_disable_planes()
dev_priv->display.crtc_disable()
intel_fbc_disable(intel_crtc); /* Second call is a no-op */
Which seems like an undesirable side-effect of this patch.
I think it would be better to instead add the:
if (pipe_config->update_pipe && !pipe_config->enable_fbc)
intel_fbc_disable(crtc);
In intel_update_crtc() in the else block of the if (modeset) {} else {}
in that function, after the intel_pre_plane_update() call, so that we
do the pre_plane_update() vs fbc_disable() in the same order as
in the full modeset path and so that we don't change the call
order of the full modeset path.
This will also nicely group it together with the
intel_encoders_update_pipe() call which my fastset PSR / DRRS
patches add (*).
p.s.
And this will also put it just before the only place where we
call intel_fbc_enable(), so also grouping it with that call,
which feels like the right thing to do to me.
I'm still learning the i915 code so I may be wrong here,
but the approach I'm suggesting seems better to me.
Regards,
Hans
*) Which will cause a conflict when merging both, but fixing that
is easy.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [v2,1/3] drm/i915: Add an update_pipe callback to intel_encoder and call this on fastsets (v2)
== Series Details == Series: series starting with [v2,1/3] drm/i915: Add an update_pipe callback to intel_encoder and call this on fastsets (v2) URL : https://patchwork.freedesktop.org/series/54339/ State : success == Summary == CI Bug Log - changes from CI_DRM_5336 -> Patchwork_11136 Summary --- **WARNING** Minor unknown changes coming with Patchwork_11136 need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_11136, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. External URL: https://patchwork.freedesktop.org/api/1.0/series/54339/revisions/1/mbox/ Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_11136: ### IGT changes ### Warnings * igt@amdgpu/amd_basic@query-info: - fi-bsw-kefka: NOTRUN -> SKIP +55 * igt@gem_mmap_gtt@basic-write-gtt: - fi-blb-e6850: NOTRUN -> PASS +111 * igt@kms_addfb_basic@invalid-set-prop-any: - fi-glk-dsi: NOTRUN -> PASS +155 * igt@kms_busy@basic-flip-a: - fi-kbl-7567u: SKIP -> PASS +2 * igt@kms_force_connector_basic@force-edid: - fi-glk-dsi: NOTRUN -> SKIP +36 * igt@kms_pipe_crc_basic@hang-read-crc-pipe-c: - fi-blb-e6850: NOTRUN -> SKIP +21 * igt@prime_vgem@basic-fence-read: - fi-bsw-kefka: NOTRUN -> PASS +232 Known issues Here are the changes found in Patchwork_11136 that come from known issues: ### IGT changes ### Issues hit * igt@kms_frontbuffer_tracking@basic: - fi-byt-clapper: PASS -> FAIL [fdo#103167] * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b: - fi-byt-clapper: PASS -> FAIL [fdo#107362] * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b: - fi-blb-e6850: NOTRUN -> INCOMPLETE [fdo#107718] Possible fixes * igt@gem_ctx_create@basic-files: - fi-bsw-kefka: INCOMPLETE [fdo#105876] / [fdo#108714] -> PASS * igt@gem_exec_suspend@basic-s4-devices: - fi-blb-e6850: INCOMPLETE [fdo#107718] -> PASS * igt@gem_mmap_gtt@basic-small-copy-xy: - fi-glk-dsi: INCOMPLETE [fdo#103359] / [k.org#198133] -> PASS * igt@prime_vgem@basic-fence-flip: - fi-gdg-551: FAIL [fdo#103182] -> PASS [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167 [fdo#103182]: https://bugs.freedesktop.org/show_bug.cgi?id=103182 [fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359 [fdo#105876]: https://bugs.freedesktop.org/show_bug.cgi?id=105876 [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362 [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718 [fdo#108714]: https://bugs.freedesktop.org/show_bug.cgi?id=108714 [k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133 Participating hosts (49 -> 43) -- Additional (1): fi-skl-6700hq Missing(7): fi-kbl-soraka fi-hsw-4770r fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-icl-y Build changes - * Linux: CI_DRM_5336 -> Patchwork_11136 CI_DRM_5336: 9fd194c1f8bd36cb623d842f5ed52b2ca2c83d18 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4752: 321fe77d32fef32ef820f53924045fe6ef0cf6ed @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_11136: d15d2e96832ae55868d3dbf5c9b33eaaccfa6597 @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == d15d2e96832a drm/i915: DDI: call intel_psr_ and _edp_drrs_enable() on pipe updates (v2) 8b9d52c1738e drm/i915: Allow calling intel_edp_drrs_enable twice 79612d0a1354 drm/i915: Add an update_pipe callback to intel_encoder and call this on fastsets (v2) == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_11136/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v4 1/3] PM/runtime: Add a new interface to get accounted time
Some drivers (like i915/drm) needs to get the accounted suspended time.
pm_runtime_suspended_time() will return the suspended accounted time
in ns unit.
Signed-off-by: Vincent Guittot
---
drivers/base/power/runtime.c | 16
include/linux/pm_runtime.h | 2 ++
2 files changed, 18 insertions(+)
diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
index beb85c3..e695544 100644
--- a/drivers/base/power/runtime.c
+++ b/drivers/base/power/runtime.c
@@ -86,6 +86,22 @@ static void __update_runtime_status(struct device *dev, enum
rpm_status status)
dev->power.runtime_status = status;
}
+u64 pm_runtime_suspended_time(struct device *dev)
+{
+ unsigned long flags, time;
+
+ spin_lock_irqsave(&dev->power.lock, flags);
+
+ update_pm_runtime_accounting(dev);
+
+ time = dev->power.suspended_jiffies;
+
+ spin_unlock_irqrestore(&dev->power.lock, flags);
+
+ return jiffies_to_nsecs(time);
+}
+EXPORT_SYMBOL_GPL(pm_runtime_suspended_time);
+
/**
* pm_runtime_deactivate_timer - Deactivate given device's suspend timer.
* @dev: Device to handle.
diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h
index f0fc470..d479707 100644
--- a/include/linux/pm_runtime.h
+++ b/include/linux/pm_runtime.h
@@ -113,6 +113,8 @@ static inline bool pm_runtime_is_irq_safe(struct device
*dev)
return dev->power.irq_safe;
}
+extern u64 pm_runtime_suspended_time(struct device *dev);
+
#else /* !CONFIG_PM */
static inline bool queue_pm_work(struct work_struct *work) { return false; }
--
2.7.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v4 0/3] Move pm_runtime accounted time to raw nsec
Move pm_runtime accounted time to raw nsec. The subject of the patchset has changed as the 1st patch of the previous version has been queued by Rafael. Patch 1 adds a new pm_runtime interface to get accounted suspended time Patch 2 moves drm/i915 driver on the new interface and removes access to internal fields. Patch 3 moves time accounting on raw ns. This patch initially used ktime instead of raw ns but it was easier to move i915 driver on raw ns than on ktime. Changes since v3: - Rebase on v4.20-rc7 without patch that has been queued by Rafael - Simplify the new interface pm_runtime_suspended_time() Changes since v2: - remove patch1 that has been queued by rafael - add new interface in pm_runtime to get accounted time - reorder patchset to prevent compilation error Changes since v1: - updated commit message of patch 1 - Added patches 2 & 3 to move runtime_pm accounting on raw ns Thara Gopinath (1): PM/runtime:Replace jiffies based accounting with ktime based accounting Vincent Guittot (2): PM/runtime: Add a new interface to get accounted time drm/i915: Move on the new pm runtime interface drivers/base/power/runtime.c| 27 ++- drivers/base/power/sysfs.c | 11 --- drivers/gpu/drm/i915/i915_pmu.c | 16 ++-- drivers/gpu/drm/i915/i915_pmu.h | 4 ++-- include/linux/pm.h | 6 +++--- include/linux/pm_runtime.h | 2 ++ 6 files changed, 43 insertions(+), 23 deletions(-) -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v4 3/3] PM/runtime:Replace jiffies based accounting with ktime based accounting
From: Thara Gopinath
This patch replaces jiffies based accounting for runtime_active_time
and runtime_suspended_time with ktime base accounting. This makes the
runtime debug counters inline with genpd and other pm subsytems which
uses ktime based accounting.
Signed-off-by: Thara Gopinath
[move from ktime to raw nsec]
Signed-off-by: Vincent Guittot
---
drivers/base/power/runtime.c | 17 +
drivers/base/power/sysfs.c | 11 ---
include/linux/pm.h | 6 +++---
3 files changed, 20 insertions(+), 14 deletions(-)
diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
index e695544..f700524 100644
--- a/drivers/base/power/runtime.c
+++ b/drivers/base/power/runtime.c
@@ -64,8 +64,8 @@ static int rpm_suspend(struct device *dev, int rpmflags);
*/
void update_pm_runtime_accounting(struct device *dev)
{
- unsigned long now = jiffies;
- unsigned long delta;
+ u64 now = ktime_to_ns(ktime_get());
+ u64 delta;
delta = now - dev->power.accounting_timestamp;
@@ -75,9 +75,9 @@ void update_pm_runtime_accounting(struct device *dev)
return;
if (dev->power.runtime_status == RPM_SUSPENDED)
- dev->power.suspended_jiffies += delta;
+ dev->power.suspended_time += delta;
else
- dev->power.active_jiffies += delta;
+ dev->power.active_time += delta;
}
static void __update_runtime_status(struct device *dev, enum rpm_status status)
@@ -88,17 +88,18 @@ static void __update_runtime_status(struct device *dev,
enum rpm_status status)
u64 pm_runtime_suspended_time(struct device *dev)
{
- unsigned long flags, time;
+ u64 time;
+ unsigned long flags;
spin_lock_irqsave(&dev->power.lock, flags);
update_pm_runtime_accounting(dev);
- time = dev->power.suspended_jiffies;
+ time = dev->power.suspended_time;
spin_unlock_irqrestore(&dev->power.lock, flags);
- return jiffies_to_nsecs(time);
+ return time;
}
EXPORT_SYMBOL_GPL(pm_runtime_suspended_time);
@@ -1503,7 +1504,7 @@ void pm_runtime_init(struct device *dev)
dev->power.request_pending = false;
dev->power.request = RPM_REQ_NONE;
dev->power.deferred_resume = false;
- dev->power.accounting_timestamp = jiffies;
+ dev->power.accounting_timestamp = ktime_to_ns(ktime_get());
INIT_WORK(&dev->power.work, pm_runtime_work);
dev->power.timer_expires = 0;
diff --git a/drivers/base/power/sysfs.c b/drivers/base/power/sysfs.c
index d713738..96c8a22 100644
--- a/drivers/base/power/sysfs.c
+++ b/drivers/base/power/sysfs.c
@@ -125,9 +125,12 @@ static ssize_t runtime_active_time_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
int ret;
+ u64 tmp;
spin_lock_irq(&dev->power.lock);
update_pm_runtime_accounting(dev);
- ret = sprintf(buf, "%i\n", jiffies_to_msecs(dev->power.active_jiffies));
+ tmp = dev->power.active_time;
+ do_div(tmp, NSEC_PER_MSEC);
+ ret = sprintf(buf, "%llu\n", tmp);
spin_unlock_irq(&dev->power.lock);
return ret;
}
@@ -138,10 +141,12 @@ static ssize_t runtime_suspended_time_show(struct device
*dev,
struct device_attribute *attr, char *buf)
{
int ret;
+ u64 tmp;
spin_lock_irq(&dev->power.lock);
update_pm_runtime_accounting(dev);
- ret = sprintf(buf, "%i\n",
- jiffies_to_msecs(dev->power.suspended_jiffies));
+ tmp = dev->power.suspended_time;
+ do_div(tmp, NSEC_PER_MSEC);
+ ret = sprintf(buf, "%llu\n", tmp);
spin_unlock_irq(&dev->power.lock);
return ret;
}
diff --git a/include/linux/pm.h b/include/linux/pm.h
index e723b78..e5a34e2 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -632,9 +632,9 @@ struct dev_pm_info {
int runtime_error;
int autosuspend_delay;
unsigned long last_busy;
- unsigned long active_jiffies;
- unsigned long suspended_jiffies;
- unsigned long accounting_timestamp;
+ u64 active_time;
+ u64 suspended_time;
+ u64 accounting_timestamp;
#endif
struct pm_subsys_data *subsys_data; /* Owned by the subsystem. */
void (*set_latency_tolerance)(struct device *, s32);
--
2.7.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v4 2/3] drm/i915: Move on the new pm runtime interface
Use the new pm runtime interface to get the accounted suspended time:
pm_runtime_accounted_time_get()
Signed-off-by: Vincent Guittot
---
drivers/gpu/drm/i915/i915_pmu.c | 16 ++--
drivers/gpu/drm/i915/i915_pmu.h | 4 ++--
2 files changed, 8 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
index d6c8f8f..3f76f60 100644
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -5,6 +5,7 @@
*/
#include
+#include
#include "i915_pmu.h"
#include "intel_ringbuffer.h"
#include "i915_drv.h"
@@ -478,7 +479,6 @@ static u64 get_rc6(struct drm_i915_private *i915)
* counter value.
*/
spin_lock_irqsave(&i915->pmu.lock, flags);
- spin_lock(&kdev->power.lock);
/*
* After the above branch intel_runtime_pm_get_if_in_use failed
@@ -491,16 +491,13 @@ static u64 get_rc6(struct drm_i915_private *i915)
* suspended and if not we cannot do better than report the last
* known RC6 value.
*/
- if (kdev->power.runtime_status == RPM_SUSPENDED) {
- if (!i915->pmu.sample[__I915_SAMPLE_RC6_ESTIMATED].cur)
- i915->pmu.suspended_jiffies_last =
- kdev->power.suspended_jiffies;
+ if (pm_runtime_status_suspended(kdev)) {
+ val = pm_runtime_suspended_time(kdev);
- val = kdev->power.suspended_jiffies -
- i915->pmu.suspended_jiffies_last;
- val += jiffies - kdev->power.accounting_timestamp;
+ if (!i915->pmu.sample[__I915_SAMPLE_RC6_ESTIMATED].cur)
+ i915->pmu.suspended_time_last = val;
- val = jiffies_to_nsecs(val);
+ val -= i915->pmu.suspended_time_last;
val += i915->pmu.sample[__I915_SAMPLE_RC6].cur;
i915->pmu.sample[__I915_SAMPLE_RC6_ESTIMATED].cur = val;
@@ -510,7 +507,6 @@ static u64 get_rc6(struct drm_i915_private *i915)
val = i915->pmu.sample[__I915_SAMPLE_RC6].cur;
}
- spin_unlock(&kdev->power.lock);
spin_unlock_irqrestore(&i915->pmu.lock, flags);
}
diff --git a/drivers/gpu/drm/i915/i915_pmu.h b/drivers/gpu/drm/i915/i915_pmu.h
index 7f164ca..3dc2a30 100644
--- a/drivers/gpu/drm/i915/i915_pmu.h
+++ b/drivers/gpu/drm/i915/i915_pmu.h
@@ -95,9 +95,9 @@ struct i915_pmu {
*/
struct i915_pmu_sample sample[__I915_NUM_PMU_SAMPLERS];
/**
-* @suspended_jiffies_last: Cached suspend time from PM core.
+* @suspended_time_last: Cached suspend time from PM core.
*/
- unsigned long suspended_jiffies_last;
+ u64 suspended_time_last;
/**
* @i915_attr: Memory block holding device attributes.
*/
--
2.7.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: add a helper to make a copy of i915_params
Abstract the one user in anticipation of more.
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/i915_gpu_error.c | 11 +--
drivers/gpu/drm/i915/i915_params.c| 14 ++
drivers/gpu/drm/i915/i915_params.h| 1 +
3 files changed, 16 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c
b/drivers/gpu/drm/i915/i915_gpu_error.c
index 63df41d93379..8c04479c1586 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -1834,18 +1834,9 @@ static void capture_gen_state(struct i915_gpu_state
*error)
error->driver_caps = i915->caps;
}
-static __always_inline void dup_param(const char *type, void *x)
-{
- if (!__builtin_strcmp(type, "char *"))
- *(void **)x = kstrdup(*(void **)x, GFP_ATOMIC);
-}
-
static void capture_params(struct i915_gpu_state *error)
{
- error->params = i915_modparams;
-#define DUP(T, x, ...) dup_param(#T, &error->params.x);
- I915_PARAMS_FOR_EACH(DUP);
-#undef DUP
+ i915_params_copy(&error->params, &i915_modparams);
}
static unsigned long capture_find_epoch(const struct i915_gpu_state *error)
diff --git a/drivers/gpu/drm/i915/i915_params.c
b/drivers/gpu/drm/i915/i915_params.c
index 2e0356561839..ae3ece4ec7ab 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -203,3 +203,17 @@ void i915_params_dump(const struct i915_params *params,
struct drm_printer *p)
I915_PARAMS_FOR_EACH(PRINT);
#undef PRINT
}
+
+static __always_inline void dup_param(const char *type, void *x)
+{
+ if (!__builtin_strcmp(type, "char *"))
+ *(void **)x = kstrdup(*(void **)x, GFP_ATOMIC);
+}
+
+void i915_params_copy(struct i915_params *dest, const struct i915_params *src)
+{
+ *dest = *src;
+#define DUP(T, x, ...) dup_param(#T, &dest->x);
+ I915_PARAMS_FOR_EACH(DUP);
+#undef DUP
+}
diff --git a/drivers/gpu/drm/i915/i915_params.h
b/drivers/gpu/drm/i915/i915_params.h
index 7e56c516c815..fd1cf9415e60 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -78,6 +78,7 @@ struct i915_params {
extern struct i915_params i915_modparams __read_mostly;
void i915_params_dump(const struct i915_params *params, struct drm_printer *p);
+void i915_params_copy(struct i915_params *dest, const struct i915_params *src);
#endif
--
2.11.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v9 10/39] drm/i915: Implement HDCP2.2 receiver authentication
> On Wed, 19 Dec 2018, "Winkler, Tomas" wrote:
> >>
> >> On Wed, 19 Dec 2018, "C, Ramalingam" wrote:
> >> > On 12/19/2018 8:05 PM, Daniel Vetter wrote:
> >> >> On Thu, Dec 13, 2018 at 09:31:12AM +0530, Ramalingam C wrote:
> >> >>> struct intel_hdcp {
> >> >>> @@ -414,6 +430,24 @@ struct intel_hdcp {
> >> >>> */
> >> >>>u8 content_type;
> >> >>>struct hdcp_port_data port_data;
> >> >>> +
> >> >>> + u8 is_paired;
> >> >>> + u8 is_repeater;
> >> >> Make these two bool, will simplify the code a bunch.
> >> >
> >> > Seems there is a movement for not to use the bool in structures.
> >>
> >> No. Please use bools in structs when it makes sense. Avoid bools in
> >> structs when you need to care about memory footprint or alignment or
> >> packing or the like. This is not one of those cases.
> >>
> >> > Thats why I have changed these from bool to u8 from v8 onwards.
> >> > Checkpatch also complains on this
> >>
> >> Sorry to say, checkpatch is not the authority although we do send out
> >> automated checkpatch results.
> >
> > I believe it was Linus' call to not use bool in structs at all
> > https://lkml.org/lkml/2017/11/21/384
>
> I don't care. That's a valid judgement in the context referenced, but the
> conclusion "no bools in structs at all" isn't. In this case, I think bools
> are the
> better option, and anything else makes the code worse.
The solution was to use bit fields,
unsinged int is_paired:1;
unsinged int is_repeter:1
There is a strong point in consistency so there are no mistakes.
But frankly I don't really have strong feelings about it.
Thanks
Tomas
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: add a helper to make a copy of i915_params
Quoting Jani Nikula (2018-12-20 14:27:50) > Abstract the one user in anticipation of more. > > Signed-off-by: Jani Nikula No harm done, Reviewed-by: Chris Wilson -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v9 07/39] drm/i915: MEI interface definition
On Thu, Dec 20, 2018 at 06:48:08PM +0530, C, Ramalingam wrote: > > On 12/19/2018 8:51 PM, Daniel Vetter wrote: > > Indeed, I overlooked that. Maybe highlight it a bit more with a separate > > > > if (!CONFIG_ENABLED(MEI_HDCP)) > > return false; > > > > so it stick out more in the previous patch. Currently it's a bit burried. > > > > With that + the init ordering fixed: > > > > Reviewed-by: Daniel Vetter > > Sure. Thank you. > > > > > There's no need to split the patch out anymore, since without the mei > > patches you can't enable that Kconfig option and hence no bisect issues. > > Still Kconfig and makefile is added at 22nd patch but component support in > mei_hdcp is added at 35th patch. > So even now this chunk needs to be kept after the component support addition. IS_ENABLED(CONFIG_FOO) will just always evaluate to false if the Kconfig symbol isn't defined yet. Kconfig works by injection additional #define contstants into the build. So current ordering is safe. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [v5 1/2] drm: Add colorspace connector property
>-Original Message-
>From: dri-devel [mailto:dri-devel-boun...@lists.freedesktop.org] On Behalf Of
>Sharma, Shashank
>Sent: Thursday, December 20, 2018 5:28 PM
>To: Shankar, Uma ; intel-gfx@lists.freedesktop.org;
>dri-de...@lists.freedesktop.org
>Cc: hansv...@cisco.com; Syrjala, Ville ; Lankhorst,
>Maarten ; jo...@kwiboo.se
>Subject: Re: [v5 1/2] drm: Add colorspace connector property
>
>Regards
>
>Shashank
>
>
>On 12/11/2018 11:44 PM, Uma Shankar wrote:
>> This patch adds a colorspace connector property, enabling userspace to
>> switch to various supported colorspaces.
>> This will help enable BT2020 along with other colorspaces.
>>
>> v2: Addressed Maarten and Ville's review comments. Enhanced the
>> colorspace enum to incorporate both HDMI and DP supported colorspaces.
>> Also, added a default option for colorspace.
>>
>> v3: Removed Adobe references from enum definitions as per Ville, Hans
>> Verkuil and Jonas Karlman suggestions. Changed Default to an unset
>> state where driver will assign the colorspace is not chosen by user,
>> suggested by Ville and Maarten. Addressed other misc review comments
>> from Maarten. Split the changes to have separate colorspace property
>> for DP and HDMI.
>>
>> v4: Addressed Chris and Ville's review comments, and created a common
>> colorspace property for DP and HDMI, filtered the list based on the
>> colorspaces supported by the respective protocol standard.
>>
>> v5: Made the property creation helper accept enum list based on
>> platform capabilties as suggested by Shashank. Consolidated HDMI and
>> DP property creation in the common helper.
>>
>> Signed-off-by: Uma Shankar
>> ---
>> drivers/gpu/drm/drm_atomic_uapi.c | 4 ++
>> drivers/gpu/drm/drm_connector.c | 82
>+++
>> include/drm/drm_connector.h | 17
>> include/uapi/drm/drm_mode.h | 33
>> 4 files changed, 136 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
>> b/drivers/gpu/drm/drm_atomic_uapi.c
>> index 86ac339..9df7520 100644
>> --- a/drivers/gpu/drm/drm_atomic_uapi.c
>> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
>> @@ -729,6 +729,8 @@ static int drm_atomic_connector_set_property(struct
>drm_connector *connector,
>> return -EINVAL;
>> }
>> state->content_protection = val;
>> +} else if (property == connector->colorspace_property) {
>> +state->colorspace = val;
>> } else if (property == config->writeback_fb_id_property) {
>> struct drm_framebuffer *fb = drm_framebuffer_lookup(dev,
>NULL, val);
>> int ret = drm_atomic_set_writeback_fb_for_connector(state,
>fb); @@
>> -797,6 +799,8 @@ static int drm_atomic_connector_set_property(struct
>drm_connector *connector,
>> *val = state->picture_aspect_ratio;
>> } else if (property == config->content_type_property) {
>> *val = state->content_type;
>> +} else if (property == connector->colorspace_property) {
>> +*val = state->colorspace;
>> } else if (property == connector->scaling_mode_property) {
>> *val = state->scaling_mode;
>> } else if (property == connector->content_protection_property) {
>> diff --git a/drivers/gpu/drm/drm_connector.c
>> b/drivers/gpu/drm/drm_connector.c index fa9baac..46928f7 100644
>> --- a/drivers/gpu/drm/drm_connector.c
>> +++ b/drivers/gpu/drm/drm_connector.c
>> @@ -826,6 +826,54 @@ int drm_display_info_set_bus_formats(struct
>drm_display_info *info,
>> };
>> DRM_ENUM_NAME_FN(drm_get_content_protection_name,
>drm_cp_enum_list)
>>
>> +/* List of HDMI Colorspaces supported */ static const struct
>> +drm_prop_enum_list hdmi_colorspace[] = {
>Should we call this list hdmi_colorspaces :) ?
Ok, will update this.
>> +/* For Default case, driver will set the colorspace */
>> +{ COLORIMETRY_DEFAULT, "Default" },
>> +/* Standard Definition Colorimetry based on CEA 861 */
>> +{ COLORIMETRY_ITU_601, "ITU_601" },
>> +{ COLORIMETRY_ITU_709, "ITU_709" },
>> +/* Standard Definition Colorimetry based on IEC 61966-2-4 */
>> +{ COLORIMETRY_XV_YCC_601, "XV_YCC_601" },
>> +/* High Definition Colorimetry based on IEC 61966-2-4 */
>> +{ COLORIMETRY_XV_YCC_709, "XV_YCC_709" },
>> +/* Colorimetry based on IEC 61966-2-1/Amendment 1 */
>> +{ COLORIMETRY_S_YCC_601, "S_YCC_601" },
>> +/* Colorimetry based on IEC 61966-2-5 [33] */
>> +{ COLORIMETRY_OPYCC_601, "opYCC_601" },
>> +/* Colorimetry based on IEC 61966-2-5 */
>> +{ COLORIMETRY_OPRGB, "opRGB" },
>> +/* Colorimetry based on ITU-R BT.2020 */
>> +{ COLORIMETRY_BT2020_RGB, "BT2020_RGB" },
>> +/* Colorimetry based on ITU-R BT.2020 */
>> +{ COLORIMETRY_BT2020_YCC, "BT2020_YCC" },
>> +/* Colorimetry based on ITU-R BT.2020 */
>> +{ COLORIMETRY_BT2020_CYCC, "BT2020_CYCC" }, };
>> +
>> +/* List of DP Colorspaces supported */
>Same here for dp_colorspaces ?
Sure.
Re: [Intel-gfx] [PATCH v9 08/39] drm/i915: hdcp1.4 CP_IRQ handling and SW encryption tracking
On Thu, Dec 20, 2018 at 04:59:13PM +0530, C, Ramalingam wrote:
>
> On 12/19/2018 9:18 PM, Daniel Vetter wrote:
> > On Thu, Dec 13, 2018 at 09:31:10AM +0530, Ramalingam C wrote:
> > > "hdcp_encrypted" flag is defined to denote the HDCP1.4 encryption status.
> > > This SW tracking is used to determine the need for real hdcp1.4 disable
> > > and hdcp_check_link upon CP_IRQ.
> > >
> > > On CP_IRQ we filter the CP_IRQ related to the states like Link failure
> > > and reauthentication req etc and handle them in hdcp_check_link.
> > > CP_IRQ corresponding to the authentication msg availability are ignored.
> > >
> > > WARN_ON is added for the abrupt stop of HDCP encryption of a port.
> > >
> > > Signed-off-by: Ramalingam C
> > > ---
> > > drivers/gpu/drm/i915/intel_dp.c | 2 +-
> > > drivers/gpu/drm/i915/intel_drv.h | 5 ++-
> > > drivers/gpu/drm/i915/intel_hdcp.c | 89
> > > ++-
> > > 3 files changed, 74 insertions(+), 22 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > > b/drivers/gpu/drm/i915/intel_dp.c
> > > index aba884c64879..89315e15fb34 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -4758,7 +4758,7 @@ static void intel_dp_check_service_irq(struct
> > > intel_dp *intel_dp)
> > > intel_dp_handle_test_request(intel_dp);
> > > if (val & DP_CP_IRQ)
> > > - intel_hdcp_check_link(intel_dp->attached_connector);
> > > + intel_hdcp_handle_cp_irq(intel_dp->attached_connector);
> > > if (val & DP_SINK_SPECIFIC_IRQ)
> > > DRM_DEBUG_DRIVER("Sink specific irq unhandled\n");
> > > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > > b/drivers/gpu/drm/i915/intel_drv.h
> > > index 191b6e0f086c..decd0346c6a7 100644
> > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > @@ -393,6 +393,9 @@ struct intel_hdcp {
> > > struct delayed_work check_work;
> > > struct work_struct prop_work;
> > > + /* HDCP1.4 Encryption status */
> > > + u8 hdcp_encrypted;
> > Another bool I guess? Or unsigned : 1;
> as per #intel-gfx discussion I will fallback to bool.
> >
> > > +
> > > /* HDCP2.2 related definitions */
> > > /* Flag indicates whether this connector supports HDCP2.2 or
> > > not. */
> > > u8 hdcp2_supported;
> > > @@ -2058,10 +2061,10 @@ int intel_hdcp_init(struct intel_connector
> > > *connector,
> > > bool hdcp2_supported);
> > > int intel_hdcp_enable(struct intel_connector *connector);
> > > int intel_hdcp_disable(struct intel_connector *connector);
> > > -int intel_hdcp_check_link(struct intel_connector *connector);
> > > bool is_hdcp_supported(struct drm_i915_private *dev_priv, enum port
> > > port);
> > > bool intel_hdcp_capable(struct intel_connector *connector);
> > > bool is_hdcp2_supported(struct drm_i915_private *dev_priv);
> > > +void intel_hdcp_handle_cp_irq(struct intel_connector *connector);
> > > /* intel_psr.c */
> > > #define CAN_PSR(dev_priv) (HAS_PSR(dev_priv) &&
> > > dev_priv->psr.sink_support)
> > > diff --git a/drivers/gpu/drm/i915/intel_hdcp.c
> > > b/drivers/gpu/drm/i915/intel_hdcp.c
> > > index 9405fce07b93..2b7814a6f12b 100644
> > > --- a/drivers/gpu/drm/i915/intel_hdcp.c
> > > +++ b/drivers/gpu/drm/i915/intel_hdcp.c
> > > @@ -75,6 +75,16 @@ bool intel_hdcp_capable(struct intel_connector
> > > *connector)
> > > return capable;
> > > }
> > > +static inline bool intel_hdcp_in_use(struct intel_connector *connector)
> > > +{
> > > + struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> > > + enum port port = connector->encoder->port;
> > > + u32 reg;
> > > +
> > > + reg = I915_READ(PORT_HDCP_STATUS(port));
> > > + return reg & HDCP_STATUS_ENC;
> > > +}
> > > +
> > > static int intel_hdcp_poll_ksv_fifo(struct intel_digital_port
> > > *intel_dig_port,
> > > const struct intel_hdcp_shim *shim)
> > > {
> > > @@ -669,6 +679,7 @@ static int _intel_hdcp_disable(struct intel_connector
> > > *connector)
> > > DRM_DEBUG_KMS("[%s:%d] HDCP is being disabled...\n",
> > > connector->base.name, connector->base.base.id);
> > > + hdcp->hdcp_encrypted = 0;
> > > I915_WRITE(PORT_HDCP_CONF(port), 0);
> > > if (intel_wait_for_register(dev_priv, PORT_HDCP_STATUS(port),
> > > ~0, 0,
> > > ENCRYPT_STATUS_CHANGE_TIMEOUT_MS)) {
> > > @@ -714,8 +725,10 @@ static int _intel_hdcp_enable(struct intel_connector
> > > *connector)
> > > /* Incase of authentication failures, HDCP spec expects reauth.
> > > */
> > > for (i = 0; i < tries; i++) {
> > > ret = intel_hdcp_auth(conn_to_dig_port(connector),
> > > hdcp->shim);
> > > - if (!ret)
> > > + if (!ret) {
> > > + hdcp->hdcp
Re: [Intel-gfx] [PATCH v9 10/39] drm/i915: Implement HDCP2.2 receiver authentication
On Thu, Dec 20, 2018 at 02:28:55PM +, Winkler, Tomas wrote:
>
>
> > On Wed, 19 Dec 2018, "Winkler, Tomas" wrote:
> > >>
> > >> On Wed, 19 Dec 2018, "C, Ramalingam" wrote:
> > >> > On 12/19/2018 8:05 PM, Daniel Vetter wrote:
> > >> >> On Thu, Dec 13, 2018 at 09:31:12AM +0530, Ramalingam C wrote:
> > >> >>> struct intel_hdcp {
> > >> >>> @@ -414,6 +430,24 @@ struct intel_hdcp {
> > >> >>> */
> > >> >>> u8 content_type;
> > >> >>> struct hdcp_port_data port_data;
> > >> >>> +
> > >> >>> +u8 is_paired;
> > >> >>> +u8 is_repeater;
> > >> >> Make these two bool, will simplify the code a bunch.
> > >> >
> > >> > Seems there is a movement for not to use the bool in structures.
> > >>
> > >> No. Please use bools in structs when it makes sense. Avoid bools in
> > >> structs when you need to care about memory footprint or alignment or
> > >> packing or the like. This is not one of those cases.
> > >>
> > >> > Thats why I have changed these from bool to u8 from v8 onwards.
> > >> > Checkpatch also complains on this
> > >>
> > >> Sorry to say, checkpatch is not the authority although we do send out
> > >> automated checkpatch results.
> > >
> > > I believe it was Linus' call to not use bool in structs at all
> > > https://lkml.org/lkml/2017/11/21/384
> >
> > I don't care. That's a valid judgement in the context referenced, but the
> > conclusion "no bools in structs at all" isn't. In this case, I think bools
> > are the
> > better option, and anything else makes the code worse.
>
> The solution was to use bit fields,
> unsinged int is_paired:1;
> unsinged int is_repeter:1
This doesn't work with READ_ONCE/WRITE_ONCE, and it generates terrible
assembly (at least gcc is well known for struggling with these, compared
to open-coded bitops). So depending upon what you want to do, and where
youre space/performance tradeoff lies, doing this unconditionally is just
wrong.
It was the right thing for the patch Linus commented on though.
-Daniel
> There is a strong point in consistency so there are no mistakes.
>
> But frankly I don't really have strong feelings about it.
>
> Thanks
> Tomas
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.SPARSE: warning for series starting with [1/3] drm/i915/ddi: Move DDI port detection to the corresponding helper
== Series Details == Series: series starting with [1/3] drm/i915/ddi: Move DDI port detection to the corresponding helper URL : https://patchwork.freedesktop.org/series/54341/ State : warning == Summary == $ dim sparse origin/drm-tip Sparse version: v0.5.2 Commit: drm/i915/ddi: Move DDI port detection to the corresponding helper Okay! Commit: drm/i915/icl: Detect port F presence via VBT Okay! Commit: drm/i915/icl: Work around broken VBTs for port F detection -drivers/gpu/drm/i915/selftests/../i915_drv.h:3550:16: warning: expression using sizeof(void) +drivers/gpu/drm/i915/selftests/../i915_drv.h:3551:16: warning: expression using sizeof(void) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Disable FBC on fastset if necessary, v2.
Without this, we will get a dmesg-warn when enable_fbc is cleared on a fastset: WARN_ON(!crtc_state->enable_fbc) WARNING: CPU: 0 PID: 1090 at drivers/gpu/drm/i915/intel_fbc.c:1091 intel_fbc_enable+0x2ce/0x580 [i915] RIP: 0010:intel_fbc_enable+0x2ce/0x580 [i915] Call Trace: ? __mutex_unlock_slowpath+0x46/0x2b0 intel_update_crtc+0x6f/0x2b0 [i915] skl_update_crtcs+0x1d1/0x2b0 [i915] intel_atomic_commit_tail+0x1ea/0xdb0 [i915] intel_atomic_commit+0x244/0x330 [i915] drm_mode_atomic_ioctl+0x85d/0x950 ? drm_atomic_set_property+0x970/0x970 drm_ioctl_kernel+0x81/0xf0 drm_ioctl+0x2de/0x390 ? drm_atomic_set_property+0x970/0x970 ? __handle_mm_fault+0x81b/0xfc0 do_vfs_ioctl+0xa0/0x6e0 ? __do_page_fault+0x2a5/0x550 ksys_ioctl+0x35/0x60 __x64_sys_ioctl+0x11/0x20 do_syscall_64+0x55/0x190 entry_SYSCALL_64_after_hwframe+0x49/0xbe Changes since v1: - Move intel_fbc_disable to intel_update_crtc() (Hans) Cc: Hans de Goede Signed-off-by: Maarten Lankhorst --- drivers/gpu/drm/i915/intel_display.c | 4 +++- drivers/gpu/drm/i915/intel_fbc.c | 2 -- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index eb399c9bdf56..3a4cb752e000 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -12757,7 +12757,9 @@ static void intel_update_crtc(struct drm_crtc *crtc, pipe_config); } - if (new_plane_state) + if (pipe_config->update_pipe && !pipe_config->enable_fbc) + intel_fbc_disable(intel_crtc); + else if (new_plane_state) intel_fbc_enable(intel_crtc, pipe_config, new_plane_state); intel_begin_crtc_commit(crtc, old_crtc_state); diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c index 1d3ff026d1bc..ccd5e110a19c 100644 --- a/drivers/gpu/drm/i915/intel_fbc.c +++ b/drivers/gpu/drm/i915/intel_fbc.c @@ -1129,8 +1129,6 @@ void intel_fbc_disable(struct intel_crtc *crtc) if (!fbc_supported(dev_priv)) return; - WARN_ON(crtc->active); - mutex_lock(&fbc->lock); if (fbc->crtc == crtc) __intel_fbc_disable(dev_priv); -- 2.19.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/debugfs_crc: Degrade ERROR when overflowing to INFO
Signed-off-by: Maarten Lankhorst
---
drivers/gpu/drm/drm_debugfs_crc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_debugfs_crc.c
b/drivers/gpu/drm/drm_debugfs_crc.c
index 00e743153e94..80f473ce5c67 100644
--- a/drivers/gpu/drm/drm_debugfs_crc.c
+++ b/drivers/gpu/drm/drm_debugfs_crc.c
@@ -408,7 +408,7 @@ int drm_crtc_add_crc_entry(struct drm_crtc *crtc, bool
has_frame,
spin_unlock(&crc->lock);
if (!was_overflow)
- DRM_ERROR("Overflow of CRC buffer, userspace reads too
slow.\n");
+ DRM_INFO("Overflow of CRC buffer, userspace reads too
slow.\n");
return -ENOBUFS;
}
--
2.19.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] drm/i915: Disable FBC on fastset if necessary
Op 20-12-2018 om 14:56 schreef Hans de Goede:
> Hi,
>
> On 20-12-18 14:43, Hans de Goede wrote:
>> Hi,
>>
>> On 18-12-18 16:27, Maarten Lankhorst wrote:
>>> Without this, we will get a dmesg-warn when enable_fbc is cleared on a
>>> fastset:
>>> WARN_ON(!crtc_state->enable_fbc)
>>> WARNING: CPU: 0 PID: 1090 at drivers/gpu/drm/i915/intel_fbc.c:1091
>>> intel_fbc_enable+0x2ce/0x580 [i915]
>>> RIP: 0010:intel_fbc_enable+0x2ce/0x580 [i915]
>>> Call Trace:
>>> ? __mutex_unlock_slowpath+0x46/0x2b0
>>> intel_update_crtc+0x6f/0x2b0 [i915]
>>> skl_update_crtcs+0x1d1/0x2b0 [i915]
>>> intel_atomic_commit_tail+0x1ea/0xdb0 [i915]
>>> intel_atomic_commit+0x244/0x330 [i915]
>>> drm_mode_atomic_ioctl+0x85d/0x950
>>> ? drm_atomic_set_property+0x970/0x970
>>> drm_ioctl_kernel+0x81/0xf0
>>> drm_ioctl+0x2de/0x390
>>> ? drm_atomic_set_property+0x970/0x970
>>> ? __handle_mm_fault+0x81b/0xfc0
>>> do_vfs_ioctl+0xa0/0x6e0
>>> ? __do_page_fault+0x2a5/0x550
>>> ksys_ioctl+0x35/0x60
>>> __x64_sys_ioctl+0x11/0x20
>>> do_syscall_64+0x55/0x190
>>> entry_SYSCALL_64_after_hwframe+0x49/0xbe
>>>
>>> Signed-off-by: Maarten Lankhorst
>>> ---
>>> drivers/gpu/drm/i915/intel_display.c | 3 +++
>>> drivers/gpu/drm/i915/intel_fbc.c | 2 --
>>> 2 files changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_display.c
>>> b/drivers/gpu/drm/i915/intel_display.c
>>> index 825d9b9e7f28..a0fae61028d6 100644
>>> --- a/drivers/gpu/drm/i915/intel_display.c
>>> +++ b/drivers/gpu/drm/i915/intel_display.c
>>> @@ -5355,6 +5355,9 @@ static void intel_pre_plane_update(struct
>>> intel_crtc_state *old_crtc_state,
>>> if (hsw_pre_update_disable_ips(old_crtc_state, pipe_config))
>>> hsw_disable_ips(old_crtc_state);
>>> + if (pipe_config->update_pipe && !pipe_config->enable_fbc)
>>> + intel_fbc_disable(crtc);
>>> +
>>> if (old_primary_state) {
>>> struct intel_plane_state *new_primary_state =
>>> intel_atomic_get_new_plane_state(old_intel_state,
>>> diff --git a/drivers/gpu/drm/i915/intel_fbc.c
>>> b/drivers/gpu/drm/i915/intel_fbc.c
>>> index 1d3ff026d1bc..ccd5e110a19c 100644
>>> --- a/drivers/gpu/drm/i915/intel_fbc.c
>>> +++ b/drivers/gpu/drm/i915/intel_fbc.c
>>> @@ -1129,8 +1129,6 @@ void intel_fbc_disable(struct intel_crtc *crtc)
>>> if (!fbc_supported(dev_priv))
>>> return;
>>> - WARN_ON(crtc->active);
>>> -
>>> mutex_lock(&fbc->lock);
>>> if (fbc->crtc == crtc)
>>> __intel_fbc_disable(dev_priv);
>>
>> Hmm, normally we call intel_fbc_disable() from the first
>> for_each_oldnew_crtc_in_state() loop in intel_atomic_commit_tail()
>> that has a:
>>
>> if (!needs_modeset(new_crtc_state))
>> continue;
>>
>> Causing it to be skipped on fastsets. But on a full modeset
>> that first loop also calls intel_pre_plane_update() directly
>> after the needs_modeset() call and before it will call
>> intel_fbc_disable() itself.
>>
>> So withn a full modeset your patch is causing disable_fbc to be
>> called earlier (when moving to a new state without fbc) then before.
>>
>> Before your patch on a full modeset we would do:
>>
>> intel_pre_plane_update()
>> intel_crtc_disable_planes()
>> dev_priv->display.crtc_disable()
>> intel_fbc_disable(intel_crtc);
>>
>> On a full modeset, but now we are doing:
>>
>> intel_pre_plane_update() ->
>> intel_fbc_disable(intel_crtc);
>> intel_crtc_disable_planes()
>> dev_priv->display.crtc_disable()
>> intel_fbc_disable(intel_crtc); /* Second call is a no-op */
>>
>> Which seems like an undesirable side-effect of this patch.
>>
>> I think it would be better to instead add the:
>>
>> if (pipe_config->update_pipe && !pipe_config->enable_fbc)
>> intel_fbc_disable(crtc);
>>
>> In intel_update_crtc() in the else block of the if (modeset) {} else {}
>> in that function, after the intel_pre_plane_update() call, so that we
>> do the pre_plane_update() vs fbc_disable() in the same order as
>> in the full modeset path and so that we don't change the call
>> order of the full modeset path.
>>
>> This will also nicely group it together with the
>> intel_encoders_update_pipe() call which my fastset PSR / DRRS
>> patches add (*).
>
> p.s.
>
> And this will also put it just before the only place where we
> call intel_fbc_enable(), so also grouping it with that call,
> which feels like the right thing to do to me.
>
>> I'm still learning the i915 code so I may be wrong here,
>> but the approach I'm suggesting seems better to me.
Hey,
Correctly observed. I don't think it's harmful in general though. But just in
case I posted a v2. :)
~Maarten
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915/ddi: Move DDI port detection to the corresponding helper
== Series Details ==
Series: series starting with [1/3] drm/i915/ddi: Move DDI port detection to the
corresponding helper
URL : https://patchwork.freedesktop.org/series/54341/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_5336 -> Patchwork_11137
Summary
---
**WARNING**
Minor unknown changes coming with Patchwork_11137 need to be verified
manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_11137, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL:
https://patchwork.freedesktop.org/api/1.0/series/54341/revisions/1/mbox/
Possible new issues
---
Here are the unknown changes that may have been introduced in Patchwork_11137:
### IGT changes ###
Warnings
* igt@kms_busy@basic-flip-a:
- fi-kbl-7567u: SKIP -> PASS +2
Known issues
Here are the changes found in Patchwork_11137 that come from known issues:
### IGT changes ###
Issues hit
* igt@kms_chamelium@hdmi-hpd-fast:
- fi-kbl-7500u: PASS -> FAIL [fdo#108767]
* igt@kms_pipe_crc_basic@read-crc-pipe-a:
- fi-byt-clapper: PASS -> FAIL [fdo#107362]
* igt@pm_rpm@basic-rte:
- fi-bsw-kefka: NOTRUN -> FAIL [fdo#108800]
* {igt@runner@aborted}:
- fi-icl-u3: NOTRUN -> FAIL [fdo#108866]
Possible fixes
* igt@gem_ctx_create@basic-files:
- fi-bsw-kefka: INCOMPLETE [fdo#105876] / [fdo#108714] -> PASS
* igt@gem_exec_suspend@basic-s4-devices:
- fi-blb-e6850: INCOMPLETE [fdo#107718] -> PASS
* igt@gem_mmap_gtt@basic-small-copy-xy:
- fi-glk-dsi: INCOMPLETE [fdo#103359] / [k.org#198133] -> PASS
* igt@prime_vgem@basic-fence-flip:
- fi-gdg-551: FAIL [fdo#103182] -> PASS
Warnings
* igt@i915_selftest@live_contexts:
- fi-icl-u3: DMESG-FAIL [fdo#108569] -> INCOMPLETE [fdo#108315]
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#103182]: https://bugs.freedesktop.org/show_bug.cgi?id=103182
[fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
[fdo#105876]: https://bugs.freedesktop.org/show_bug.cgi?id=105876
[fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
[fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
[fdo#108315]: https://bugs.freedesktop.org/show_bug.cgi?id=108315
[fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
[fdo#108714]: https://bugs.freedesktop.org/show_bug.cgi?id=108714
[fdo#108767]: https://bugs.freedesktop.org/show_bug.cgi?id=108767
[fdo#108800]: https://bugs.freedesktop.org/show_bug.cgi?id=108800
[fdo#108866]: https://bugs.freedesktop.org/show_bug.cgi?id=108866
[k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133
Participating hosts (49 -> 43)
--
Additional (1): fi-skl-6700hq
Missing(7): fi-kbl-soraka fi-hsw-4770r fi-ilk-m540 fi-byt-squawks
fi-bsw-cyan fi-ctg-p8600 fi-icl-y
Build changes
-
* Linux: CI_DRM_5336 -> Patchwork_11137
CI_DRM_5336: 9fd194c1f8bd36cb623d842f5ed52b2ca2c83d18 @
git://anongit.freedesktop.org/gfx-ci/linux
IGT_4752: 321fe77d32fef32ef820f53924045fe6ef0cf6ed @
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_11137: b77346cbb6c5f94d4b2a364ed1270df788b454e5 @
git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
b77346cbb6c5 drm/i915/icl: Work around broken VBTs for port F detection
524b5d6a2e13 drm/i915/icl: Detect port F presence via VBT
1f8b8a045206 drm/i915/ddi: Move DDI port detection to the corresponding helper
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_11137/
___
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: add a helper to make a copy of i915_params
== Series Details == Series: drm/i915: add a helper to make a copy of i915_params URL : https://patchwork.freedesktop.org/series/54353/ State : warning == Summary == $ dim checkpatch origin/drm-tip 6179c542522e drm/i915: add a helper to make a copy of i915_params -:53: CHECK:MACRO_ARG_PRECEDENCE: Macro argument 'x' may be better as '(x)' to avoid precedence issues #53: FILE: drivers/gpu/drm/i915/i915_params.c:216: +#define DUP(T, x, ...) dup_param(#T, &dest->x); -:53: WARNING:TRAILING_SEMICOLON: macros should not use a trailing semicolon #53: FILE: drivers/gpu/drm/i915/i915_params.c:216: +#define DUP(T, x, ...) dup_param(#T, &dest->x); total: 0 errors, 1 warnings, 1 checks, 43 lines checked ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 3/3] drm/i915/icl: Work around broken VBTs for port F detection
VBT may include incorrect information about the presence of port F. Work around this on SKUs where we know the port is not present. v2: - Fix IS_ICL_WITH_PORT_F, so it's useable from any context. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108915 Cc: Mika Kahola Cc: Jani Nikula Signed-off-by: Imre Deak --- drivers/gpu/drm/i915/i915_drv.h | 2 ++ drivers/gpu/drm/i915/intel_display.c | 4 +++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 815db160b966..e45cda9d9555 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2317,6 +2317,8 @@ intel_info(const struct drm_i915_private *dev_priv) (dev_priv)->info.gt == 3) #define IS_CNL_WITH_PORT_F(dev_priv) (IS_CANNONLAKE(dev_priv) && \ (INTEL_DEVID(dev_priv) & 0x0004) == 0x0004) +#define IS_ICL_WITH_PORT_F(dev_priv) (IS_ICELAKE(dev_priv) && \ + INTEL_DEVID(dev_priv) != 0x8A51) #define IS_ALPHA_SUPPORT(intel_info) ((intel_info)->is_alpha_support) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 2b81da068010..bd7fdaf7e093 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -14276,8 +14276,10 @@ static void intel_setup_outputs(struct drm_i915_private *dev_priv) /* * On some ICL SKUs port F is not present. No strap bits for * this, so rely on VBT. +* Work around broken VBTs on SKUs known to have no port F. */ - if (intel_bios_is_port_present(dev_priv, PORT_F)) + if (IS_ICL_WITH_PORT_F(dev_priv) && + intel_bios_is_port_present(dev_priv, PORT_F)) intel_ddi_init(dev_priv, PORT_F); icl_dsi_init(dev_priv); -- 2.13.2 ___ 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: add a helper to make a copy of i915_params
== Series Details ==
Series: drm/i915: add a helper to make a copy of i915_params
URL : https://patchwork.freedesktop.org/series/54353/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_5336 -> Patchwork_11138
Summary
---
**WARNING**
Minor unknown changes coming with Patchwork_11138 need to be verified
manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_11138, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL:
https://patchwork.freedesktop.org/api/1.0/series/54353/revisions/1/mbox/
Possible new issues
---
Here are the unknown changes that may have been introduced in Patchwork_11138:
### IGT changes ###
Warnings
* igt@kms_busy@basic-flip-a:
- fi-kbl-7567u: SKIP -> PASS +2
Known issues
Here are the changes found in Patchwork_11138 that come from known issues:
### IGT changes ###
Issues hit
* igt@i915_module_load@reload:
- fi-blb-e6850: NOTRUN -> INCOMPLETE [fdo#107718]
* igt@i915_selftest@live_hangcheck:
- fi-bwr-2160:PASS -> DMESG-FAIL [fdo#108735]
* igt@kms_chamelium@hdmi-hpd-fast:
- fi-kbl-7500u: PASS -> FAIL [fdo#108767]
* igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
- fi-byt-clapper: PASS -> FAIL [fdo#103191] / [fdo#107362]
* igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b:
- fi-byt-clapper: PASS -> FAIL [fdo#107362]
* {igt@runner@aborted}:
- fi-icl-u3: NOTRUN -> FAIL [fdo#108866]
Possible fixes
* igt@gem_ctx_create@basic-files:
- fi-bsw-kefka: INCOMPLETE [fdo#105876] / [fdo#108714] -> PASS
* igt@gem_exec_suspend@basic-s4-devices:
- fi-blb-e6850: INCOMPLETE [fdo#107718] -> PASS
* igt@gem_mmap_gtt@basic-small-copy-xy:
- fi-glk-dsi: INCOMPLETE [fdo#103359] / [k.org#198133] -> PASS
* igt@prime_vgem@basic-fence-flip:
- fi-gdg-551: FAIL [fdo#103182] -> PASS
Warnings
* igt@i915_selftest@live_contexts:
- fi-icl-u3: DMESG-FAIL [fdo#108569] -> INCOMPLETE [fdo#108315]
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#103182]: https://bugs.freedesktop.org/show_bug.cgi?id=103182
[fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
[fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
[fdo#105876]: https://bugs.freedesktop.org/show_bug.cgi?id=105876
[fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
[fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
[fdo#108315]: https://bugs.freedesktop.org/show_bug.cgi?id=108315
[fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
[fdo#108714]: https://bugs.freedesktop.org/show_bug.cgi?id=108714
[fdo#108735]: https://bugs.freedesktop.org/show_bug.cgi?id=108735
[fdo#108767]: https://bugs.freedesktop.org/show_bug.cgi?id=108767
[fdo#108866]: https://bugs.freedesktop.org/show_bug.cgi?id=108866
[k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133
Participating hosts (49 -> 44)
--
Additional (1): fi-skl-6700hq
Missing(6): fi-kbl-soraka fi-hsw-4770r fi-ilk-m540 fi-byt-squawks
fi-bsw-cyan fi-ctg-p8600
Build changes
-
* Linux: CI_DRM_5336 -> Patchwork_11138
CI_DRM_5336: 9fd194c1f8bd36cb623d842f5ed52b2ca2c83d18 @
git://anongit.freedesktop.org/gfx-ci/linux
IGT_4752: 321fe77d32fef32ef820f53924045fe6ef0cf6ed @
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_11138: 6179c542522edc36ad60e9cc9cfbb7b8e4c11811 @
git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
6179c542522e drm/i915: add a helper to make a copy of i915_params
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_11138/
___
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: Disable FBC on fastset if necessary, v2.
== Series Details == Series: drm/i915: Disable FBC on fastset if necessary, v2. URL : https://patchwork.freedesktop.org/series/54363/ State : warning == Summary == $ dim checkpatch origin/drm-tip 59cca50e5e76 drm/i915: Disable FBC on fastset if necessary, v2. -:6: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line) #6: Without this, we will get a dmesg-warn when enable_fbc is cleared on a fastset: total: 0 errors, 1 warnings, 0 checks, 18 lines checked ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v9 35/39] misc/mei/hdcp: Component framework for I915 Interface
On 12/19/2018 12:15 PM, C, Ramalingam wrote:
Tomas and Daniel,
From the discussion on this thread, I infer following understanding:
* At present(v9) I915 wants to be hard binded to mei_hdcp
device-driver binding status through components
o This means I915 driver load will get complete only when the
mei_hdcp's device and driver are bound.
o if mei_hdcp device reset I915 will unregister itself from
userspace, and wait for the mei_hdcp device-deriver rebinding.
+ Could be due to FW error or any unexpected failures those
are rare occurances.
o when mei_hdcp module is removed i915 will unregister itself.
o Becasue of this, Ideally I915 dont expect the device reset
from mei for suspend and resume.
* At present Mei bus is designed as below:
o Device will disappear on FW failures, FW upgrade, suspend of
the system etc.
o And when the errors are handled or on system resume mei device
will reappear, hence binding with corresponding driver.
* Mei doesn't plan to avoid the device reset(disappearance and
reappearance) for suspend and resume in near future.
Based on above understanding, I propose the below approach. Please correct or
approve it.
* At present(v9) component_add from mei_hdcp indicates the
mei_hdcp's device-driver binded state.
* Instead lets use component to indicate the mei_hdcp's module
availability,
o by adding the component at module_init and removing it from
module_exit.
* This way I915 will not be impacted due to the mei device reset at
suspend.
* In such scenario I915 will have no idea about the device-driver
bind status of mei_hdcp.
o So incase of device is not available, mei_hdcp is responsible
to prune such calls with -EIO error.
* This approach avoid any future impact to I915, incase mei intended
to support suspend and resume.
I am aware this is not the ideal solution we want. But I feel this is the best
at present we could do for this I915-mei interface.
Best regards,
Ram
something like (un compiled code)
diff --git a/drivers/misc/mei/hdcp/mei_hdcp.c b/drivers/misc/mei/hdcp/mei_hdcp.c
index b22a71e8c5d7..b5b57a883e3b 100644
--- a/drivers/misc/mei/hdcp/mei_hdcp.c
+++ b/drivers/misc/mei/hdcp/mei_hdcp.c
@@ -23,11 +23,15 @@
#include
#include
#include
+#include
#include
#include
#include "mei_hdcp.h"
+struct i915_component_master *i915_master_comp;
+static bool mei_hdcp_component_registered;
+
/**
* mei_initiate_hdcp2_session() - Initiate a Wired HDCP2.2 Tx Session in ME FW
* @dev: device corresponding to the mei_cl_device
@@ -691,8 +695,7 @@ mei_close_hdcp_session(struct device *dev, struct
hdcp_port_data *data)
return 0;
}
-static __attribute__((unused))
-struct i915_hdcp_component_ops mei_hdcp_ops = {
+static struct i915_hdcp_component_ops mei_hdcp_ops = {
.owner = THIS_MODULE,
.initiate_hdcp2_session = mei_initiate_hdcp2_session,
.verify_receiver_cert_prepare_km = mei_verify_receiver_cert_prepare_km,
@@ -707,20 +710,84 @@ struct i915_hdcp_component_ops mei_hdcp_ops = {
.close_hdcp_session = mei_close_hdcp_session,
};
+static int mei_hdcp_component_bind(struct device *mei_kdev,
+ struct device *i915_kdev, void *data)
+{
+ struct i915_component_master *master_comp = data;
+
+ dev_info(mei_kdev, "MEI HDCP comp bind\n");
+ WARN_ON(master_comp->hdcp_ops);
+ master_comp->hdcp_ops = &mei_hdcp_ops;
+ master_comp->mei_dev = mei_kdev;
+
+ i915_master_comp = master_comp;
+
+ return 0;
+}
+
+static void mei_hdcp_component_unbind(struct device *mei_kdev,
+ struct device *i915_kdev, void *data)
+{
+ struct i915_component_master *master_comp = data;
+
+ dev_info(mei_kdev, "MEI HDCP comp unbind\n");
+ master_comp->hdcp_ops = NULL;
+ master_comp->mei_dev = NULL;
+ i915_master_comp = NULL;
+}
+
+static const struct component_ops mei_hdcp_component_bind_ops = {
+ .bind = mei_hdcp_component_bind,
+ .unbind = mei_hdcp_component_unbind,
+};
+
+static void mei_hdcp_component_init(struct device *dev)
+{
+ int ret;
+
+ if (mei_hdcp_component_registered && i915_master_comp) {
+ i915_master_comp->mei_dev = dev;
+ return;
+ }
+
+ dev_info(dev, "MEI HDCP comp init\n");
+ ret = component_add(dev, &mei_hdcp_component_bind_ops);
+ if (ret < 0) {
+ dev_err(dev, "Failed to add MEI HDCP comp (%d)\n", ret);
+ return;
+ }
+
+ mei_hdcp_component_registered = true;
+}
+
+static void mei_hdcp_component_cleanup(struct device *dev)
+{
+ if (!mei_hdcp_component_registered)
+ return;
+
+ dev_info(dev, "MEI HDCP comp cleanup\n");
+ component_del(dev, &mei_hdcp_component_bind_ops);
+ mei_hdcp_component_re
Re: [Intel-gfx] [PATCH v9 35/39] misc/mei/hdcp: Component framework for I915 Interface
From: C, Ramalingam
Sent: Thursday, December 20, 2018 18:00
To: Daniel Vetter ; Winkler, Tomas
Cc: Greg KH ; Rafael J. Wysocki
; intel-gfx ; dri-devel
; Sean Paul ; Shankar,
Uma ; Syrjala, Ville ;
Chris Wilson
Subject: Re: [PATCH v9 35/39] misc/mei/hdcp: Component framework for I915
Interface
On 12/19/2018 12:15 PM, C, Ramalingam wrote:
Tomas and Daniel,
From the discussion on this thread, I infer following understanding:
* At present(v9) I915 wants to be hard binded to mei_hdcp device-driver
binding status through components
* This means I915 driver load will get complete only when the mei_hdcp's
device and driver are bound.
* if mei_hdcp device reset I915 will unregister itself from userspace,
and wait for the mei_hdcp device-deriver rebinding.
* Could be due to FW error or any unexpected failures those are rare
occurances.
* when mei_hdcp module is removed i915 will unregister itself.
* Becasue of this, Ideally I915 dont expect the device reset from mei
for suspend and resume.
* At present Mei bus is designed as below:
* Device will disappear on FW failures, FW upgrade, suspend of the
system etc.
* And when the errors are handled or on system resume mei device will
reappear, hence binding with corresponding driver.
* Mei doesn't plan to avoid the device reset(disappearance and
reappearance) for suspend and resume in near future.
Based on above understanding, I propose the below approach. Please correct or
approve it.
* At present(v9) component_add from mei_hdcp indicates the mei_hdcp's
device-driver binded state.
* Instead lets use component to indicate the mei_hdcp's module availability,
* by adding the component at module_init and removing it from
module_exit.
* This way I915 will not be impacted due to the mei device reset at suspend.
* In such scenario I915 will have no idea about the device-driver bind
status of mei_hdcp.
* So incase of device is not available, mei_hdcp is responsible to prune
such calls with -EIO error.
* This approach avoid any future impact to I915, incase mei intended to
support suspend and resume.
I am aware this is not the ideal solution we want. But I feel this is the best
at present we could do for this I915-mei interface.
Best regards,
Ram
something like (un compiled code)
diff --git a/drivers/misc/mei/hdcp/mei_hdcp.c b/drivers/misc/mei/hdcp/mei_hdcp.c
index b22a71e8c5d7..b5b57a883e3b 100644
--- a/drivers/misc/mei/hdcp/mei_hdcp.c
+++ b/drivers/misc/mei/hdcp/mei_hdcp.c
@@ -23,11 +23,15 @@
#include
#include
#include
+#include
#include
#include
#include "mei_hdcp.h"
+struct i915_component_master *i915_master_comp;
+static bool mei_hdcp_component_registered;
+
/**
* mei_initiate_hdcp2_session() - Initiate a Wired HDCP2.2 Tx Session in ME FW
* @dev: device corresponding to the mei_cl_device
@@ -691,8 +695,7 @@ mei_close_hdcp_session(struct device *dev, struct
hdcp_port_data *data)
return 0;
}
-static __attribute__((unused))
-struct i915_hdcp_component_ops mei_hdcp_ops = {
+static struct i915_hdcp_component_ops mei_hdcp_ops = {
.owner = THIS_MODULE,
.initiate_hdcp2_session = mei_initiate_hdcp2_session,
.verify_receiver_cert_prepare_km = mei_verify_receiver_cert_prepare_km,
@@ -707,20 +710,84 @@ struct i915_hdcp_component_ops mei_hdcp_ops = {
.close_hdcp_session = mei_close_hdcp_session,
};
+static int mei_hdcp_component_bind(struct device *mei_kdev,
+ struct device *i915_kdev, void *data)
+{
+ struct i915_component_master *master_comp = data;
+
+ dev_info(mei_kdev, "MEI HDCP comp bind\n");
+ WARN_ON(master_comp->hdcp_ops);
+ master_comp->hdcp_ops = &mei_hdcp_ops;
+ master_comp->mei_dev = mei_kdev;
+
+ i915_master_comp = master_comp;
+
+ return 0;
+}
+
+static void mei_hdcp_component_unbind(struct device *mei_kdev,
+ struct device *i915_kdev, void *data)
+{
+ struct i915_component_master *master_comp = data;
+
+ dev_info(mei_kdev, "MEI HDCP comp unbind\n");
+ master_comp->hdcp_ops = NULL;
+ master_comp->mei_dev = NULL;
+ i915_master_comp = NULL;
+}
+
+static const struct component_ops mei_hdcp_component_bind_ops = {
+ .bind = mei_hdcp_component_bind,
+ .unbind = mei_hdcp_component_unbind,
+};
+
+static void mei_hdcp_component_init(struct device *dev)
+{
+ int ret;
+
+ if (mei_hdcp_component_registered && i915_master_comp) {
+ i915_master_comp->mei_dev = dev;
+ return;
+ }
+
+ dev_info(dev, "MEI HDCP comp init\n");
+ ret = component_add(dev, &mei_hdcp_component_bind_ops);
+ if (ret < 0) {
+ dev_err(dev, "Failed to add MEI HDCP comp (%d)\n", ret);
+ retu
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Disable FBC on fastset if necessary, v2.
== Series Details ==
Series: drm/i915: Disable FBC on fastset if necessary, v2.
URL : https://patchwork.freedesktop.org/series/54363/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_5336 -> Patchwork_11139
Summary
---
**WARNING**
Minor unknown changes coming with Patchwork_11139 need to be verified
manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_11139, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL:
https://patchwork.freedesktop.org/api/1.0/series/54363/revisions/1/mbox/
Possible new issues
---
Here are the unknown changes that may have been introduced in Patchwork_11139:
### IGT changes ###
Warnings
* igt@kms_busy@basic-flip-a:
- fi-kbl-7567u: SKIP -> PASS +2
Known issues
Here are the changes found in Patchwork_11139 that come from known issues:
### IGT changes ###
Issues hit
* igt@i915_selftest@live_hangcheck:
- fi-bwr-2160:PASS -> DMESG-FAIL [fdo#108735]
- fi-kbl-7560u: PASS -> INCOMPLETE [fdo#108044]
* igt@kms_flip@basic-flip-vs-modeset:
- fi-skl-6700hq: NOTRUN -> DMESG-WARN [fdo#105998]
* igt@kms_frontbuffer_tracking@basic:
- fi-hsw-peppy: PASS -> DMESG-WARN [fdo#102614]
* igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b:
- fi-byt-clapper: PASS -> FAIL [fdo#107362]
* igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b-frame-sequence:
- fi-byt-clapper: PASS -> FAIL [fdo#103191] / [fdo#107362]
* igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
- fi-skl-6700k2: PASS -> FAIL [fdo#103375] +1
* {igt@runner@aborted}:
- fi-bsw-kefka: NOTRUN -> FAIL [fdo#108656]
Possible fixes
* igt@gem_exec_suspend@basic-s4-devices:
- fi-blb-e6850: INCOMPLETE [fdo#107718] -> PASS
* igt@gem_mmap_gtt@basic-small-copy-xy:
- fi-glk-dsi: INCOMPLETE [fdo#103359] / [k.org#198133] -> PASS
* igt@prime_vgem@basic-fence-flip:
- fi-gdg-551: FAIL [fdo#103182] -> PASS
Warnings
* igt@gem_ctx_create@basic-files:
- fi-bsw-kefka: INCOMPLETE [fdo#105876] / [fdo#108714] -> DMESG-FAIL
[fdo#108656]
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
[fdo#103182]: https://bugs.freedesktop.org/show_bug.cgi?id=103182
[fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
[fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
[fdo#103375]: https://bugs.freedesktop.org/show_bug.cgi?id=103375
[fdo#105876]: https://bugs.freedesktop.org/show_bug.cgi?id=105876
[fdo#105998]: https://bugs.freedesktop.org/show_bug.cgi?id=105998
[fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
[fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
[fdo#108044]: https://bugs.freedesktop.org/show_bug.cgi?id=108044
[fdo#108656]: https://bugs.freedesktop.org/show_bug.cgi?id=108656
[fdo#108714]: https://bugs.freedesktop.org/show_bug.cgi?id=108714
[fdo#108735]: https://bugs.freedesktop.org/show_bug.cgi?id=108735
[k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133
Participating hosts (49 -> 43)
--
Additional (1): fi-skl-6700hq
Missing(7): fi-kbl-soraka fi-hsw-4770r fi-ilk-m540 fi-byt-squawks
fi-bsw-cyan fi-ctg-p8600 fi-pnv-d510
Build changes
-
* Linux: CI_DRM_5336 -> Patchwork_11139
CI_DRM_5336: 9fd194c1f8bd36cb623d842f5ed52b2ca2c83d18 @
git://anongit.freedesktop.org/gfx-ci/linux
IGT_4752: 321fe77d32fef32ef820f53924045fe6ef0cf6ed @
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_11139: 59cca50e5e76b68acfe6d35585e69c218c8fc530 @
git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
59cca50e5e76 drm/i915: Disable FBC on fastset if necessary, v2.
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_11139/
___
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/debugfs_crc: Degrade ERROR when overflowing to INFO
== Series Details == Series: drm/debugfs_crc: Degrade ERROR when overflowing to INFO URL : https://patchwork.freedesktop.org/series/54364/ State : warning == Summary == $ dim checkpatch origin/drm-tip ae751d55dd4f drm/debugfs_crc: Degrade ERROR when overflowing to INFO -:7: WARNING:COMMIT_MESSAGE: Missing commit description - Add an appropriate one total: 0 errors, 1 warnings, 0 checks, 8 lines checked ___ 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/debugfs_crc: Degrade ERROR when overflowing to INFO
== Series Details == Series: drm/debugfs_crc: Degrade ERROR when overflowing to INFO URL : https://patchwork.freedesktop.org/series/54364/ State : success == Summary == CI Bug Log - changes from CI_DRM_5336 -> Patchwork_11140 Summary --- **WARNING** Minor unknown changes coming with Patchwork_11140 need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_11140, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. External URL: https://patchwork.freedesktop.org/api/1.0/series/54364/revisions/1/mbox/ Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_11140: ### IGT changes ### Warnings * igt@kms_busy@basic-flip-a: - fi-kbl-7567u: SKIP -> PASS +2 Known issues Here are the changes found in Patchwork_11140 that come from known issues: ### IGT changes ### Issues hit * igt@kms_chamelium@hdmi-hpd-fast: - fi-kbl-7500u: PASS -> FAIL [fdo#108767] Possible fixes * igt@gem_ctx_create@basic-files: - fi-bsw-kefka: INCOMPLETE [fdo#105876] / [fdo#108714] -> PASS * igt@gem_exec_suspend@basic-s4-devices: - fi-blb-e6850: INCOMPLETE [fdo#107718] -> PASS * igt@gem_mmap_gtt@basic-small-copy-xy: - fi-glk-dsi: INCOMPLETE [fdo#103359] / [k.org#198133] -> PASS * igt@prime_vgem@basic-fence-flip: - fi-gdg-551: FAIL [fdo#103182] -> PASS [fdo#103182]: https://bugs.freedesktop.org/show_bug.cgi?id=103182 [fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359 [fdo#105876]: https://bugs.freedesktop.org/show_bug.cgi?id=105876 [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718 [fdo#108714]: https://bugs.freedesktop.org/show_bug.cgi?id=108714 [fdo#108767]: https://bugs.freedesktop.org/show_bug.cgi?id=108767 [k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133 Participating hosts (49 -> 43) -- Additional (1): fi-skl-6700hq Missing(7): fi-kbl-soraka fi-hsw-4770r fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-icl-y Build changes - * Linux: CI_DRM_5336 -> Patchwork_11140 CI_DRM_5336: 9fd194c1f8bd36cb623d842f5ed52b2ca2c83d18 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4752: 321fe77d32fef32ef820f53924045fe6ef0cf6ed @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_11140: ae751d55dd4feddc27f1dded781e5225f203c35e @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == ae751d55dd4f drm/debugfs_crc: Degrade ERROR when overflowing to INFO == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_11140/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/3] drm/i915/ddi: Move DDI port detection to the corresponding helper (rev2)
== Series Details == Series: series starting with [1/3] drm/i915/ddi: Move DDI port detection to the corresponding helper (rev2) URL : https://patchwork.freedesktop.org/series/54341/ State : warning == Summary == $ dim checkpatch origin/drm-tip 0f71660394ec drm/i915/ddi: Move DDI port detection to the corresponding helper 51299ac215a8 drm/i915/icl: Detect port F presence via VBT 2e4395c3050c drm/i915/icl: Work around broken VBTs for port F detection -:25: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'dev_priv' - possible side-effects? #25: FILE: drivers/gpu/drm/i915/i915_drv.h:2320: +#define IS_ICL_WITH_PORT_F(dev_priv) (IS_ICELAKE(dev_priv) && \ + INTEL_DEVID(dev_priv) != 0x8A51) total: 0 errors, 0 warnings, 1 checks, 19 lines checked ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.SPARSE: warning for series starting with [1/3] drm/i915/ddi: Move DDI port detection to the corresponding helper (rev2)
== Series Details == Series: series starting with [1/3] drm/i915/ddi: Move DDI port detection to the corresponding helper (rev2) URL : https://patchwork.freedesktop.org/series/54341/ State : warning == Summary == $ dim sparse origin/drm-tip Sparse version: v0.5.2 Commit: drm/i915/ddi: Move DDI port detection to the corresponding helper Okay! Commit: drm/i915/icl: Detect port F presence via VBT Okay! Commit: drm/i915/icl: Work around broken VBTs for port F detection -drivers/gpu/drm/i915/selftests/../i915_drv.h:3550:16: warning: expression using sizeof(void) +drivers/gpu/drm/i915/selftests/../i915_drv.h:3552:16: warning: expression using sizeof(void) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v9 35/39] misc/mei/hdcp: Component framework for I915 Interface
On 12/20/2018 9:36 PM, Winkler, Tomas wrote:
+static void __exit mei_hdcp_exit(void)
+{
+ mei_hdcp_component_cleanup(&cldev->dev);
Don’t think you can do that, no guarantees this will be valid pointer
As we discussed offline, we have the below line at cleanup.
So valid pointer is made sure. I will protect init and cleanup with mutex too.
+static void mei_hdcp_component_cleanup(struct device *dev)
+{
+ if (!mei_hdcp_component_registered)
+ return;
-Ram
+ mei_cldev_driver_unregister(mei_hdcp_driver);
+}
+
+module_init(mei_hdcp_init);
+module_exit(mei_hdcp_exit);
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PULL] drm-misc-fixes
Hi Dave and Daniel, A late spectre fix for you to pull for 4.20 final. drm-misc-fixes-2018-12-20: Fix spectre v1 vuln in drm_ioctl Cheers, Sean The following changes since commit 7566ec393f4161572ba6f11ad5171fd5d59b0fbd: Linux 4.20-rc7 (2018-12-16 15:46:55 -0800) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-fixes-2018-12-20 for you to fetch changes up to 505b5240329b922f21f91d5b5d1e535c805eca6d: drm/ioctl: Fix Spectre v1 vulnerabilities (2018-12-20 08:13:29 +0100) Fix spectre v1 vuln in drm_ioctl Gustavo A. R. Silva (1): drm/ioctl: Fix Spectre v1 vulnerabilities drivers/gpu/drm/drm_ioctl.c | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) -- Sean Paul, Software Engineer, Google / Chromium OS ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Implement HDCP2.2 (rev12)
== Series Details == Series: drm/i915: Implement HDCP2.2 (rev12) URL : https://patchwork.freedesktop.org/series/38254/ State : failure == Summary == Applying: drm/i915: Gathering the HDCP1.4 routines together Applying: drm: header for i915 - MEI_HDCP interface Applying: drivers/base: use a worker for sysfs unbind Applying: component: alloc component_match without any comp to match Applying: drm/i915: component master at i915 driver load Using index info to reconstruct a base tree... M drivers/gpu/drm/i915/i915_drv.c M drivers/gpu/drm/i915/i915_drv.h Falling back to patching base and 3-way merge... Auto-merging drivers/gpu/drm/i915/i915_drv.h Auto-merging drivers/gpu/drm/i915/i915_drv.c Applying: drm/i915: Initialize HDCP2.2 Applying: drm/i915: MEI interface definition Applying: drm/i915: hdcp1.4 CP_IRQ handling and SW encryption tracking Applying: drm/i915: Enable and Disable of HDCP2.2 Applying: drm/i915: Implement HDCP2.2 receiver authentication Applying: drm: helper functions for hdcp2 seq_num to from u32 Applying: drm/i915: Implement HDCP2.2 repeater authentication Applying: drm: HDCP2.2 link check related constants Applying: drm/i915: Implement HDCP2.2 link integrity check Applying: drm/i915: Handle HDCP2.2 downstream topology change Applying: drm/i915: Implement the HDCP2.2 support for DP Applying: drm/i915: Implement the HDCP2.2 support for HDMI Applying: drm/i915: Add HDCP2.2 support for DP connectors Applying: drm/i915: Add HDCP2.2 support for HDMI connectors Applying: mei: bus: whitelist hdcp client Applying: mei: bus: export to_mei_cl_device for mei client device drivers Applying: misc/mei/hdcp: Client driver for HDCP application Applying: misc/mei/hdcp: Define ME FW interface for HDCP2.2 Applying: misc/mei/hdcp: Initiate Wired HDCP2.2 Tx Session Applying: misc/mei/hdcp: Verify Receiver Cert and prepare km Applying: misc/mei/hdcp: Verify H_prime Applying: misc/mei/hdcp: Store the HDCP Pairing info Applying: misc/mei/hdcp: Initiate Locality check Applying: misc/mei/hdcp: Verify L_prime Applying: misc/mei/hdcp: Prepare Session Key Applying: misc/mei/hdcp: Repeater topology verification and ack Applying: misc/mei/hdcp: Verify M_prime Applying: misc/mei/hdcp: Enabling the HDCP authentication Applying: misc/mei/hdcp: Closing wired HDCP2.2 Tx Session Applying: misc/mei/hdcp: Component framework for I915 Interface error: patch failed: drivers/misc/mei/hdcp/mei_hdcp.c:23 error: drivers/misc/mei/hdcp/mei_hdcp.c: patch does not apply error: Did you hand edit your patch? It does not apply to blobs recorded in its index. hint: Use 'git am --show-current-patch' to see the failed patch Using index info to reconstruct a base tree... Patch failed at 0035 misc/mei/hdcp: Component framework for I915 Interface 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
[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915/ddi: Move DDI port detection to the corresponding helper (rev2)
== Series Details == Series: series starting with [1/3] drm/i915/ddi: Move DDI port detection to the corresponding helper (rev2) URL : https://patchwork.freedesktop.org/series/54341/ State : success == Summary == CI Bug Log - changes from CI_DRM_5336 -> Patchwork_11141 Summary --- **WARNING** Minor unknown changes coming with Patchwork_11141 need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_11141, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. External URL: https://patchwork.freedesktop.org/api/1.0/series/54341/revisions/2/mbox/ Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_11141: ### IGT changes ### Warnings * igt@kms_busy@basic-flip-a: - fi-kbl-7567u: SKIP -> PASS +2 Known issues Here are the changes found in Patchwork_11141 that come from known issues: ### IGT changes ### Issues hit * igt@kms_chamelium@hdmi-hpd-fast: - fi-kbl-7500u: PASS -> FAIL [fdo#108767] * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a: - fi-byt-clapper: PASS -> FAIL [fdo#103191] / [fdo#107362] Possible fixes * igt@gem_ctx_create@basic-files: - fi-bsw-kefka: INCOMPLETE [fdo#105876] / [fdo#108714] -> PASS * igt@gem_exec_suspend@basic-s4-devices: - fi-blb-e6850: INCOMPLETE [fdo#107718] -> PASS * igt@gem_mmap_gtt@basic-small-copy-xy: - fi-glk-dsi: INCOMPLETE [fdo#103359] / [k.org#198133] -> PASS * igt@prime_vgem@basic-fence-flip: - fi-gdg-551: FAIL [fdo#103182] -> PASS [fdo#103182]: https://bugs.freedesktop.org/show_bug.cgi?id=103182 [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191 [fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359 [fdo#105876]: https://bugs.freedesktop.org/show_bug.cgi?id=105876 [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362 [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718 [fdo#108714]: https://bugs.freedesktop.org/show_bug.cgi?id=108714 [fdo#108767]: https://bugs.freedesktop.org/show_bug.cgi?id=108767 [k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133 Participating hosts (49 -> 44) -- Additional (1): fi-skl-6700hq Missing(6): fi-kbl-soraka fi-hsw-4770r fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 Build changes - * Linux: CI_DRM_5336 -> Patchwork_11141 CI_DRM_5336: 9fd194c1f8bd36cb623d842f5ed52b2ca2c83d18 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4752: 321fe77d32fef32ef820f53924045fe6ef0cf6ed @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_11141: 2e4395c3050c99e51227b146a7048fd7c8498760 @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == 2e4395c3050c drm/i915/icl: Work around broken VBTs for port F detection 51299ac215a8 drm/i915/icl: Detect port F presence via VBT 0f71660394ec drm/i915/ddi: Move DDI port detection to the corresponding helper == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_11141/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 3/3] drm/i915: DDI: call intel_psr_ and _edp_drrs_enable() on pipe updates (v2)
On Thu, Dec 20, 2018 at 02:21:20PM +0100, Hans de Goede wrote:
> Call intel_psr_enable() and intel_edp_drrs_enable() on pipe updates to make
> sure that we enable PSR / DRRS (when applicable) on fastsets.
>
> Note calling these functions when PSR / DRRS has already been enabled is a
> no-op, so it is safe to do this on every encoder->update_pipe callback.
>
> Changes in v2:
> -Merge the patches adding the intel_psr_enable() and intel_edp_drrs_enable()
> calls into a single patch
>
> Reviewed-by: Maarten Lankhorst
> Signed-off-by: Hans de Goede
Cc: Dhinakaran Pandiyan
Cc: José Roberto de Souza
Acked-by: Rodrigo Vivi
> ---
> drivers/gpu/drm/i915/intel_ddi.c | 19 +++
> 1 file changed, 19 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> b/drivers/gpu/drm/i915/intel_ddi.c
> index e3cc19e19199..fdf57f451b72 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -3537,6 +3537,24 @@ static void intel_disable_ddi(struct intel_encoder
> *encoder,
> intel_disable_ddi_dp(encoder, old_crtc_state, old_conn_state);
> }
>
> +static void intel_ddi_update_pipe_dp(struct intel_encoder *encoder,
> + const struct intel_crtc_state *crtc_state,
> + const struct drm_connector_state
> *conn_state)
> +{
> + struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> +
> + intel_psr_enable(intel_dp, crtc_state);
> + intel_edp_drrs_enable(intel_dp, crtc_state);
> +}
> +
> +static void intel_ddi_update_pipe(struct intel_encoder *encoder,
> + const struct intel_crtc_state *crtc_state,
> + const struct drm_connector_state *conn_state)
> +{
> + if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
> + intel_ddi_update_pipe_dp(encoder, crtc_state, conn_state);
> +}
> +
> static void intel_ddi_set_fia_lane_count(struct intel_encoder *encoder,
>const struct intel_crtc_state
> *pipe_config,
>enum port port)
> @@ -4169,6 +4187,7 @@ void intel_ddi_init(struct drm_i915_private *dev_priv,
> enum port port)
> intel_encoder->pre_enable = intel_ddi_pre_enable;
> intel_encoder->disable = intel_disable_ddi;
> intel_encoder->post_disable = intel_ddi_post_disable;
> + intel_encoder->update_pipe = intel_ddi_update_pipe;
> intel_encoder->get_hw_state = intel_ddi_get_hw_state;
> intel_encoder->get_config = intel_ddi_get_config;
> intel_encoder->suspend = intel_ddi_encoder_suspend;
> --
> 2.20.1
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PULL] drm-misc-fixes
On Thu, Dec 20, 2018 at 11:57:40AM -0500, Sean Paul wrote: > > Hi Dave and Daniel, > A late spectre fix for you to pull for 4.20 final. > > drm-misc-fixes-2018-12-20: > Fix spectre v1 vuln in drm_ioctl > > Cheers, Sean Thanks, applied. -Daniel > > > The following changes since commit 7566ec393f4161572ba6f11ad5171fd5d59b0fbd: > > Linux 4.20-rc7 (2018-12-16 15:46:55 -0800) > > are available in the Git repository at: > > git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-fixes-2018-12-20 > > for you to fetch changes up to 505b5240329b922f21f91d5b5d1e535c805eca6d: > > drm/ioctl: Fix Spectre v1 vulnerabilities (2018-12-20 08:13:29 +0100) > > > Fix spectre v1 vuln in drm_ioctl > > > Gustavo A. R. Silva (1): > drm/ioctl: Fix Spectre v1 vulnerabilities > > drivers/gpu/drm/drm_ioctl.c | 10 -- > 1 file changed, 8 insertions(+), 2 deletions(-) > > -- > Sean Paul, Software Engineer, Google / Chromium OS -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [v5 2/2] drm/i915: Attach colorspace property and enable modeset
Regards
Shashank
On 12/11/2018 11:44 PM, Uma Shankar wrote:
This patch attaches the colorspace connector property to the
hdmi connector. Based on colorspace change, modeset will be
triggered to switch to new colorspace.
Based on colorspace property value create an infoframe
with appropriate colorspace. This can be used to send an
infoframe packet with proper colorspace value set which
will help to enable wider color gamut like BT2020 on sink.
This patch attaches and enables HDMI colorspace, DP will be
taken care separately.
v2: Merged the changes of creating infoframe as well to this
patch as per Maarten's suggestion.
v3: Addressed review comments from Shashank. Separated HDMI
and DP colorspaces as suggested by Ville and Maarten.
v4: Addressed Chris and Ville's review comments, and created a
common colorspace property for DP and HDMI, filtered the list
based on the colorspaces supported by the respective protocol
standard. Handle the default case properly.
v5: Added Platform specific colorspace enums and called the
property creation helper using the same.
Signed-off-by: Uma Shankar
---
drivers/gpu/drm/i915/intel_atomic.c| 1 +
drivers/gpu/drm/i915/intel_connector.c | 63 ++
drivers/gpu/drm/i915/intel_drv.h | 1 +
drivers/gpu/drm/i915/intel_hdmi.c | 18 ++
4 files changed, 83 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_atomic.c
b/drivers/gpu/drm/i915/intel_atomic.c
index a5a2c8f..35ef70a 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -125,6 +125,7 @@ int intel_digital_connector_atomic_check(struct
drm_connector *conn,
*/
if (new_conn_state->force_audio != old_conn_state->force_audio ||
new_conn_state->broadcast_rgb != old_conn_state->broadcast_rgb ||
+ new_state->colorspace != old_state->colorspace ||
new_conn_state->base.picture_aspect_ratio !=
old_conn_state->base.picture_aspect_ratio ||
new_conn_state->base.content_type !=
old_conn_state->base.content_type ||
new_conn_state->base.scaling_mode !=
old_conn_state->base.scaling_mode)
diff --git a/drivers/gpu/drm/i915/intel_connector.c
b/drivers/gpu/drm/i915/intel_connector.c
index 18e370f..59fa420 100644
--- a/drivers/gpu/drm/i915/intel_connector.c
+++ b/drivers/gpu/drm/i915/intel_connector.c
@@ -31,6 +31,48 @@
#include "intel_drv.h"
#include "i915_drv.h"
+static const struct drm_prop_enum_list gen10_hdmi_colorspace[] = {
+ /* For Default case, driver will set the colorspace */
+ { COLORIMETRY_DEFAULT, "Default" },
+ /* Standard Definition Colorimetry based on CEA 861 */
+ { COLORIMETRY_ITU_601, "ITU_601" },
+ { COLORIMETRY_ITU_709, "ITU_709" },
+ /* Standard Definition Colorimetry based on IEC 61966-2-4 */
+ { COLORIMETRY_XV_YCC_601, "XV_YCC_601" },
+ /* High Definition Colorimetry based on IEC 61966-2-4 */
+ { COLORIMETRY_XV_YCC_709, "XV_YCC_709" },
+ /* Colorimetry based on IEC 61966-2-1/Amendment 1 */
+ { COLORIMETRY_S_YCC_601, "S_YCC_601" },
+ /* Colorimetry based on IEC 61966-2-5 [33] */
+ { COLORIMETRY_OPYCC_601, "opYCC_601" },
+ /* Colorimetry based on IEC 61966-2-5 */
+ { COLORIMETRY_OPRGB, "opRGB" },
+ /* Colorimetry based on ITU-R BT.2020 */
+ { COLORIMETRY_BT2020_RGB, "BT2020_RGB" },
+ /* Colorimetry based on ITU-R BT.2020 */
+ { COLORIMETRY_BT2020_YCC, "BT2020_YCC" },
+ /* Colorimetry based on ITU-R BT.2020 */
+ { COLORIMETRY_BT2020_CYCC, "BT2020_CYCC" },
+};
+
+static const struct drm_prop_enum_list legacy_hdmi_colorspace[] = {
+ /* For Default case, driver will set the colorspace */
+ { COLORIMETRY_DEFAULT, "Default" },
+ /* Standard Definition Colorimetry based on CEA 861 */
+ { COLORIMETRY_ITU_601, "ITU_601" },
+ { COLORIMETRY_ITU_709, "ITU_709" },
+ /* Standard Definition Colorimetry based on IEC 61966-2-4 */
+ { COLORIMETRY_XV_YCC_601, "XV_YCC_601" },
+ /* High Definition Colorimetry based on IEC 61966-2-4 */
+ { COLORIMETRY_XV_YCC_709, "XV_YCC_709" },
+ /* Colorimetry based on IEC 61966-2-1/Amendment 1 */
+ { COLORIMETRY_S_YCC_601, "S_YCC_601" },
+ /* Colorimetry based on IEC 61966-2-5 [33] */
+ { COLORIMETRY_OPYCC_601, "opYCC_601" },
+ /* Colorimetry based on IEC 61966-2-5 */
+ { COLORIMETRY_OPRGB, "opRGB" },
+};
+
int intel_connector_init(struct intel_connector *connector)
{
struct intel_digital_connector_state *conn_state;
@@ -262,3 +304,24 @@ int intel_ddc_get_modes(struct drm_connector *connector,
connector->dev->mode_config.aspect_ratio_property,
DRM_MODE_PICTURE_ASPECT_NONE);
}
+
+void
+intel_attach_colorspace_property(struct drm_connector *connector)
+{
+ struct drm_device *dev = connector->dev;
+ struct drm_i915_private *dev_p
Re: [Intel-gfx] [v2 01/14] drm: Add HDR source metadata property
Regards
Shashank
On 12/12/2018 2:08 AM, Uma Shankar wrote:
This patch adds a blob property to get HDR metadata
information from userspace. This will be send as part
of AVI Infoframe to panel.
v2: Rebase and modified the metadata structure elements
as per Ville's POC changes.
Signed-off-by: Uma Shankar
---
drivers/gpu/drm/drm_connector.c | 6 ++
include/drm/drm_connector.h | 10 ++
include/drm/drm_mode_config.h | 6 ++
include/linux/hdmi.h| 10 ++
include/uapi/drm/drm_mode.h | 16
5 files changed, 48 insertions(+)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index da8ae80..361fcda 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1027,6 +1027,12 @@ int drm_connector_create_standard_properties(struct
drm_device *dev)
return -ENOMEM;
dev->mode_config.non_desktop_property = prop;
+ prop = drm_property_create(dev, DRM_MODE_PROP_BLOB,
+ "HDR_SOURCE_METADATA", 0);
I see that the compositor would be using this blob to setup the output
HDR curve post blending, which would be in most of cases, sink HDR. So
we should ideally call it HDR_OUTPUT_METADATA or output_hdr_metadata.
+ if (!prop)
+ return -ENOMEM;
+ dev->mode_config.hdr_source_metadata_property = prop;
+
return 0;
}
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 9be2181..2ee45dc 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -520,6 +520,13 @@ struct drm_connector_state {
* and the connector bpc limitations obtained from edid.
*/
u8 max_bpc;
+
+ /**
+* @metadata_blob_ptr:
+* DRM blob property for HDR metadata
+*/
+ struct drm_property_blob *hdr_source_metadata_blob_ptr;
Same goes again here, output_hdr_metadata_blob ?
+ u8 hdr_metadata_changed : 1;
};
/**
@@ -1154,6 +1161,9 @@ struct drm_connector {
* &drm_mode_config.connector_free_work.
*/
struct llist_node free_node;
+
+ /* HDR metdata */
+ struct hdr_static_metadata hdr_metadata;
I think while designing this framework, we should leave the scope for
dynamic metadata, which would be following up soon. So we should have a
union inside struct hdr_metedata, which will have option for both static
and dynamic type of metadata. I will add details to the place where you
are adding definition of hdr_static_metadata().
};
#define obj_to_connector(x) container_of(x, struct drm_connector, base)
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index 69ccd27..4b3211b 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -836,6 +836,12 @@ struct drm_mode_config {
*/
struct drm_property *writeback_out_fence_ptr_property;
+ /*
+* hdr_metadata_property: Connector property containing hdr metatda
+* This will be provided by userspace compositors based on HDR content
+*/
+ struct drm_property *hdr_source_metadata_property;
Again, source->output
+
/* dumb ioctl parameters */
uint32_t preferred_depth, prefer_shadow;
diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
index d2bacf5..bed3e08 100644
--- a/include/linux/hdmi.h
+++ b/include/linux/hdmi.h
@@ -137,6 +137,16 @@ enum hdmi_content_type {
HDMI_CONTENT_TYPE_GAME,
};
+enum hdmi_metadata_type {
+ HDMI_STATIC_METADATA_TYPE1 = 1,
+};
+
+enum hdmi_eotf {
+ HDMI_EOTF_TRADITIONAL_GAMMA_SDR,
+ HDMI_EOTF_TRADITIONAL_GAMMA_HDR,
+ HDMI_EOTF_SMPTE_ST2084,
I guess we are not bothering about HLG now, which is fine actually, less
complicated for now.
+};
+
struct hdmi_avi_infoframe {
enum hdmi_infoframe_type type;
unsigned char version;
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index a439c2e..5012af2 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -630,6 +630,22 @@ struct drm_color_lut {
__u16 reserved;
};
+/* HDR Metadata */
+struct hdr_static_metadata {
+ uint8_t eotf;
+ uint8_t metadata_type;
+ struct {
+ uint16_t x, y;
+ } display_primaries[3];
+ struct {
+ uint16_t x, y;
+ } white_point;
+ uint16_t max_mastering_display_luminance;
+ uint16_t min_mastering_display_luminance;
+ uint16_t max_fall;
+ uint16_t max_cll;
+};
How about defining struct hdr_metadata {
uint8_t size;
union {
struct hdr_static_metadata *static;
struct hdr_metadata_dynamic *dynamic;
} metadata;
};
So that its easier when we are extending support for dynamic HDR ?
- Shashank
+
#define DRM_MODE_PAGE_FLIP_EVENT 0x01
#define DRM_MODE_PAGE_FLIP_ASYNC 0x02
#define DRM_MODE_PAGE_FLIP_TA
Re: [Intel-gfx] [v2 02/14] drm: Add CEA extended tag blocks and HDR bitfield macros
Regards Shashank On 12/12/2018 2:08 AM, Uma Shankar wrote: Add bit field and macro for extended tag in CEA block. Also, declare macros for HDR metadata block. This should have been a part of patch, where these macros are being used, so that we can see it being used properly. While re-basing can you please merge ? v2: Rebase Signed-off-by: Uma Shankar --- drivers/gpu/drm/drm_edid.c | 16 1 file changed, 16 insertions(+) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index b506e36..106fd38 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2843,6 +2843,22 @@ static int drm_cvt_modes(struct drm_connector *connector, #define EDID_CEA_YCRCB422 (1 << 4) #define EDID_CEA_VCDB_QS (1 << 6) +#define DATA_BLOCK_EXTENDED_TAG 0x07 Alignment should be one tab back, also I think we already have added extended tag macro (for parsing YCBCR420 blocks) +#define VIDEO_CAPABILITY_DATA_BLOCK0x0 +#define VSVD_DATA_BLOCK0x1 Alignment one tab back +#define COLORIMETRY_DATA_BLOCK 0x5 +#define HDR_STATIC_METADATA_BLOCK 0x6 + +/* HDR Metadata Block: Bit fields */ +#define SUPPORTED_EOTF_MASK0x3f +#define TRADITIONAL_GAMMA_SDR (0x1 << 0) +#define TRADITIONAL_GAMMA_HDR (0x1 << 1) +#define SMPTE_ST2084 (0x1 << 2) +#define FUTURE_EOTF(0x1 << 3) why not properly call it HLG if we are adding a bit for it already ? +#define RESERVED_EOTF (0x3 << 4) + +#define STATIC_METADATA_TYPE1 (0x1 << 0) + /* * Search EDID for CEA extension block. */ - Shashank ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [v2 03/14] drm: Parse HDR metadata info from EDID
Regards
Shashank
On 12/12/2018 2:08 AM, Uma Shankar wrote:
HDR metadata block is introduced in CEA-861.3 spec.
Parsing the same to get the panel's HDR metadata.
v2: Rebase and added Ville's POC changes to the patch.
Signed-off-by: Uma Shankar
---
drivers/gpu/drm/drm_edid.c | 45 +
1 file changed, 45 insertions(+)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 106fd38..d12b74e 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3818,6 +3818,49 @@ static void fixup_detailed_cea_mode_clock(struct
drm_display_mode *mode)
mode->clock = clock;
}
I guess the previous patch (or a art of previous patch) should be merged
in this patch.
+static bool cea_db_is_hdmi_hdr_metadata_block(const u8 *db)
+{
+ if (cea_db_tag(db) != DATA_BLOCK_EXTENDED_TAG)
+ return false;
+
+ if (db[1] != HDR_STATIC_METADATA_BLOCK)
+ return false;
+
+ return true;
+}
+
+static uint16_t eotf_supported(const u8 *edid_ext)
Why uint16 ? why not uint8_t ? this is clearly one byte.
+{
+
+ return edid_ext[2] &
+ (BIT(HDMI_EOTF_TRADITIONAL_GAMMA_SDR) |
Should we bother about SDR bit at all ? Why not just check HDR bits ?
+BIT(HDMI_EOTF_TRADITIONAL_GAMMA_HDR) |
+BIT(HDMI_EOTF_SMPTE_ST2084));
+
+}
+
+static uint16_t hdr_metadata_type(const u8 *edid_ext)
+{
Same uint8_t stuff
+
+ return edid_ext[3] &
+ BIT(HDMI_STATIC_METADATA_TYPE1);
+}
+
+static void
+drm_parse_hdr_metadata_block(struct drm_connector *connector, const u8 *db)
+{
+ uint16_t len;
+
+ len = cea_db_payload_len(db);
Length is incorrect here for extended tag blocks, as this function
doesn't account the extended tag byte's size. So the payload length is
looking for is len - 1. May be a good time to add
cea_extended_db_payload_len() ?
+ connector->hdr_metadata.eotf = eotf_supported(db);
+ connector->hdr_metadata.metadata_type = hdr_metadata_type(db);
+
+ if (len >= 5)
+ connector->hdr_metadata.max_fall = db[5];
+ if (len >= 4)
+ connector->hdr_metadata.max_cll = db[4];
+}
+
static void
drm_parse_hdmi_vsdb_audio(struct drm_connector *connector, const u8 *db)
{
@@ -4468,6 +4511,8 @@ static void drm_parse_cea_ext(struct drm_connector
*connector,
drm_parse_hdmi_forum_vsdb(connector, db);
if (cea_db_is_y420cmdb(db))
drm_parse_y420cmdb_bitmap(connector, db);
+ if (cea_db_is_hdmi_hdr_metadata_block(db))
+ drm_parse_hdr_metadata_block(connector, db);
}
}
- Shashank
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [v2 04/14] drm: Parse Colorimetry data block from EDID
Regards
Shashank
On 12/12/2018 2:08 AM, Uma Shankar wrote:
CEA 861.3 spec adds colorimetry data block for HDMI.
Parsing the block to get the colorimetry data from
panel.
v2: Rebase
Signed-off-by: Uma Shankar
---
drivers/gpu/drm/drm_edid.c | 24
include/drm/drm_connector.h | 2 ++
2 files changed, 26 insertions(+)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index d12b74e..344d8c1 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3818,6 +3818,28 @@ static void fixup_detailed_cea_mode_clock(struct
drm_display_mode *mode)
mode->clock = clock;
}
+static bool cea_db_is_hdmi_colorimetry_data_block(const u8 *db)
+{
+ if (cea_db_tag(db) != DATA_BLOCK_EXTENDED_TAG)
+ return false;
+
+ if (db[1] != COLORIMETRY_DATA_BLOCK)
+
Again, the COLORIMETRY_DATA_BLOCK definition should be added into this
patch.
return false;
+
+ return true;
+}
+
+static void
+drm_parse_colorimetry_data_block(struct drm_connector *connector, const u8 *db)
+{
+ struct drm_hdmi_info *info = &connector->display_info.hdmi;
+ uint16_t len;
+
+ len = cea_db_payload_len(db);
Again, the return value of this function doesn't account for extended db
tag byte, so what we are looking for is len -1
+ info->colorimetry = db[2];
colorimetry block is actually db[2] and db[3].bit7 (which represents
DCI-P3 space), so we probably want to make info->colorimetryuint16_t
+}
+
+
static bool cea_db_is_hdmi_hdr_metadata_block(const u8 *db)
{
if (cea_db_tag(db) != DATA_BLOCK_EXTENDED_TAG)
@@ -4513,6 +4535,8 @@ static void drm_parse_cea_ext(struct drm_connector
*connector,
drm_parse_y420cmdb_bitmap(connector, db);
if (cea_db_is_hdmi_hdr_metadata_block(db))
drm_parse_hdr_metadata_block(connector, db);
+ if (cea_db_is_hdmi_colorimetry_data_block(db))
+ drm_parse_colorimetry_data_block(connector, db);
}
}
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 2ee45dc..90ce364 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -206,6 +206,8 @@ struct drm_hdmi_info {
/** @y420_dc_modes: bitmap of deep color support index */
u8 y420_dc_modes;
+
Please add a one line description about this variable.
+ u8 colorimetry;
};
/**
Shashank
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [v2 05/14] drm/i915: Attach HDR metadata property to connector
Regards Shashank On 12/12/2018 2:08 AM, Uma Shankar wrote: Attach HDR metadata property to connector object. v2: Rebase Signed-off-by: Uma Shankar --- drivers/gpu/drm/i915/intel_hdmi.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index 07e803a..8a1e5cb 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -2154,6 +2154,8 @@ static void intel_hdmi_destroy(struct drm_connector *connector) intel_attach_aspect_ratio_property(connector); drm_connector_attach_content_type_property(connector); connector->state->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE; + drm_object_attach_property(&connector->base, + connector->dev->mode_config.hdr_source_metadata_property, 0); Alignment with line above missing. - Shashank if (!HAS_GMCH_DISPLAY(dev_priv)) drm_connector_attach_max_bpc_property(connector, 8, 12); ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] uABI / Removing DRM_I915_LOW_LEVEL_TRACEPOINTS Kconfig
On Wed, Dec 19, 2018, at 12:22 PM, Steven Rostedt wrote: > On Wed, 19 Dec 2018 12:08:18 +0200 > Joonas Lahtinen wrote: >· > > To me, it seems almost as if folks are too preoccupied with thinking if > > we somehow can do this through tracepoints, to stop and actually think > > if we should. >· > Regardless of whether it should or shouldn't, one solution to this is > to make the trace event in question record basically nothing but a > pointer. Right now, these are the events I'm capturing w/ an AMD gpu: amdgpu_cs:0-1150 [002] 630662.649417: amdgpu_cs_ioctl: sched_job=3490671, timeline=gfx, context=105, seqno=3081096, ring_name=91cb1ab1bdd0, num_ibs=3 gfx-190 [000] 630662.649451: amdgpu_sched_run_job: sched_job=3490671, timeline=gfx, context=105, seqno=3081096, ring_name=91cb1ab1bdd0, num_ibs=3 gfx-190 [000] 630662.649454: dma_fence_signaled: driver=amd_sched timeline=gfx context=104 seqno=3081096 With Intel gpu (and rebuilt kernel w/ CONFIG_DRM_I915_LOW_LEVEL_TRACEPOINTS): -0 [002] 821.717208: intel_engine_notify: dev=0, engine=0:0, seqno=38896, waiters=1 RenderThread-1024 [002] 825.722358: i915_request_queue: dev=0, engine=0:0, hw_id=9, ctx=30, seqno=38896, flags=0x0 RenderThread-1024 [002] 825.722371: i915_request_add: dev=0, engine=0:0, hw_id=9, ctx=30, seqno=38896, global=0 RenderThread-1024 [002] 825.722372: i915_request_submit: dev=0, engine=0:0, hw_id=9, ctx=30, seqno=38896, global=0 RenderThread-1024 [002] 825.745964: i915_request_execute: dev=0, engine=0:0, hw_id=9, ctx=30, seqno=38896, global=42199 RenderThread-1024 [002] 825.745964: i915_request_in: dev=0, engine=0:0, hw_id=9, ctx=30, seqno=38896, prio=0, global=42199, port=1 -0 [002] 825.755943: intel_engine_notify: dev=0, engine=0:0, seqno=42199, waiters=1 It's quite obvious that just because gpuvis sees those amdgpu tracepoints when running with the AMD card and parses and displays those, it does *not* get those same tracepoints when I run with an Intel gpu. And with my Iris Pro Graphics 580 Gen9, I can reasonably expect to get the above i915 tracepoints. But if I install a new Intel Xe Gen11, why should I expect to see those Gen9 i915 tracepoint events? Is it because we are tying tracepoints and the created uABI to *kernel modules* and not the hardware? I'm asking, because personally I would expect the hardware to drive these tracepoint events, much like I check cpu flags to see whether I can run AVX code, or perf has intel_pt recording on one machine, but not another. Right now gpuvis graphs the above events in an easy to understand view. Occasionally, it's really nice to use trace-cmd to get textual representation for grepping, etc. Storing pointers would obviously break that. I guess if it's what we need to do to avoid the uABI problem, then it's what we do - still better than using an entirely new tracing system if we can avoid that. Thanks much. -Mike ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [v2 07/14] drm: Implement HDR source metadata set and get property handling
Regards
Shashank
On 12/12/2018 2:08 AM, Uma Shankar wrote:
HDR source metadata set and get property implemented in this
patch.
Again, HDR output metadata ? How about re-arranging the line like "This
patch implements get() and set() functions for HDR output metadata
property" just to make it more clear ?
The blob data is received from userspace and saved in
connector state, the same is returned as blob in get property
call to userspace.
v2: Rebase and added Ville's POC changes
Signed-off-by: Uma Shankar
---
drivers/gpu/drm/drm_atomic.c | 2 ++
drivers/gpu/drm/drm_atomic_uapi.c | 13 +
2 files changed, 15 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 5eb4013..4e71c6b 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -881,6 +881,8 @@ static void drm_atomic_connector_print_state(struct
drm_printer *p,
drm_printf(p, "connector[%u]: %s\n", connector->base.id, connector->name);
drm_printf(p, "\tcrtc=%s\n", state->crtc ? state->crtc->name :
"(null)");
+ drm_printf(p, "\thdr_metadata_changed=%d\n",
+ state->hdr_metadata_changed);
Alignment
if (connector->connector_type == DRM_MODE_CONNECTOR_WRITEBACK)
if (state->writeback_job && state->writeback_job->fb)
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
b/drivers/gpu/drm/drm_atomic_uapi.c
index c408898..b721b12 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -686,6 +686,8 @@ static int drm_atomic_connector_set_property(struct
drm_connector *connector,
{
struct drm_device *dev = connector->dev;
struct drm_mode_config *config = &dev->mode_config;
+ bool replaced = false;
+ int ret;
if (property == config->prop_crtc_id) {
struct drm_crtc *crtc = drm_crtc_find(dev, NULL, val);
@@ -734,6 +736,14 @@ static int drm_atomic_connector_set_property(struct
drm_connector *connector,
*/
if (state->link_status != DRM_LINK_STATUS_GOOD)
state->link_status = val;
+ } else if (property == config->hdr_source_metadata_property) {
+ ret = drm_atomic_replace_property_blob_from_id(dev,
+ &state->hdr_source_metadata_blob_ptr,
+ val,
+ -1, sizeof(struct hdr_static_metadata),
+ &replaced);
Alignment, but I know it might be difficult to contain the params within
80 chars.
+ state->hdr_metadata_changed |= replaced;
+ return ret;
} else if (property == config->aspect_ratio_property) {
state->picture_aspect_ratio = val;
} else if (property == config->content_type_property) {
@@ -816,6 +826,9 @@ static int drm_atomic_connector_set_property(struct
drm_connector *connector,
*val = state->content_type;
} else if (property == connector->scaling_mode_property) {
*val = state->scaling_mode;
+ } else if (property == config->hdr_source_metadata_property) {
+ *val = (state->hdr_source_metadata_blob_ptr) ?
+ state->hdr_source_metadata_blob_ptr->base.id : 0;
} else if (property == connector->content_protection_property) {
*val = state->content_protection;
} else if (property == config->writeback_fb_id_property) {
- Shashank
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Disable FBC on fastset if necessary, v2.
Hi, On 20-12-18 16:17, Maarten Lankhorst wrote: Without this, we will get a dmesg-warn when enable_fbc is cleared on a fastset: WARN_ON(!crtc_state->enable_fbc) WARNING: CPU: 0 PID: 1090 at drivers/gpu/drm/i915/intel_fbc.c:1091 intel_fbc_enable+0x2ce/0x580 [i915] RIP: 0010:intel_fbc_enable+0x2ce/0x580 [i915] Call Trace: ? __mutex_unlock_slowpath+0x46/0x2b0 intel_update_crtc+0x6f/0x2b0 [i915] skl_update_crtcs+0x1d1/0x2b0 [i915] intel_atomic_commit_tail+0x1ea/0xdb0 [i915] intel_atomic_commit+0x244/0x330 [i915] drm_mode_atomic_ioctl+0x85d/0x950 ? drm_atomic_set_property+0x970/0x970 drm_ioctl_kernel+0x81/0xf0 drm_ioctl+0x2de/0x390 ? drm_atomic_set_property+0x970/0x970 ? __handle_mm_fault+0x81b/0xfc0 do_vfs_ioctl+0xa0/0x6e0 ? __do_page_fault+0x2a5/0x550 ksys_ioctl+0x35/0x60 __x64_sys_ioctl+0x11/0x20 do_syscall_64+0x55/0x190 entry_SYSCALL_64_after_hwframe+0x49/0xbe Changes since v1: - Move intel_fbc_disable to intel_update_crtc() (Hans) Cc: Hans de Goede Signed-off-by: Maarten Lankhorst Patch looks good to me: Reviewed-by: Hans de Goede Regards, Hans --- drivers/gpu/drm/i915/intel_display.c | 4 +++- drivers/gpu/drm/i915/intel_fbc.c | 2 -- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index eb399c9bdf56..3a4cb752e000 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -12757,7 +12757,9 @@ static void intel_update_crtc(struct drm_crtc *crtc, pipe_config); } - if (new_plane_state) + if (pipe_config->update_pipe && !pipe_config->enable_fbc) + intel_fbc_disable(intel_crtc); + else if (new_plane_state) intel_fbc_enable(intel_crtc, pipe_config, new_plane_state); intel_begin_crtc_commit(crtc, old_crtc_state); diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c index 1d3ff026d1bc..ccd5e110a19c 100644 --- a/drivers/gpu/drm/i915/intel_fbc.c +++ b/drivers/gpu/drm/i915/intel_fbc.c @@ -1129,8 +1129,6 @@ void intel_fbc_disable(struct intel_crtc *crtc) if (!fbc_supported(dev_priv)) return; - WARN_ON(crtc->active); - mutex_lock(&fbc->lock); if (fbc->crtc == crtc) __intel_fbc_disable(dev_priv); ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [v4 4/4] drm/i915/icl: Add degamma and gamma lut size to gen11 caps
Add the degamma and gamma lut sizes to gen11 capability
structure.
Note: Currently this doesn't account for the extended range gamma
entries and this will be addressed with new segmented gamma ABI
in a future patch.
v2: Reorder the patch as per Maarten's suggestion.
v3: Rebase
v4: Updated commit message with a note as per Matt's suggestion.
Signed-off-by: Uma Shankar
---
drivers/gpu/drm/i915/i915_pci.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index 6350db5..add5642 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -635,7 +635,8 @@
}, \
GEN(11), \
.ddb_size = 2048, \
- .has_logical_ring_elsq = 1
+ .has_logical_ring_elsq = 1, \
+ .color = { .degamma_lut_size = 33, .gamma_lut_size = 1024 }
static const struct intel_device_info intel_icelake_11_info = {
GEN11_FEATURES,
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [v4 0/4] Add support for Gen 11 pipe color features
This patch series adds support for Gen11 pipe degamma, CSC and gamma hardware blocks. CRC checks are not working for 10bit gamma but works for 8bit pallete modes which seems to be due to some rounding errors in pipe. ToDo: Support for Multi Segmented Gamma will be added later. v2: Addressed Maarten's review comments and re-ordered the patch series. v3: Addressed Matt's review comments. Removed rmw patterns as suggested by Matt. v4: Addressed Matt's review comments. Uma Shankar (4): drm/i915: Remove gamma_mode state variable drm/i915/icl: Add icl pipe degamma and gamma support drm/i915/icl: Enable ICL Pipe CSC block drm/i915/icl: Add degamma and gamma lut size to gen11 caps drivers/gpu/drm/i915/i915_pci.c | 3 +- drivers/gpu/drm/i915/i915_reg.h | 7 ++- drivers/gpu/drm/i915/intel_color.c | 84 +--- drivers/gpu/drm/i915/intel_display.c | 3 -- drivers/gpu/drm/i915/intel_drv.h | 3 -- 5 files changed, 86 insertions(+), 14 deletions(-) -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [v4 1/4] drm/i915: Remove gamma_mode state variable
Removed crtc state variable for gamma mode as it's redundant
since currently we have fixed modes on respective hardware
platforms. This was making this state variable irrelevant.
Credits-to: Matt Roper
Signed-off-by: Uma Shankar
---
drivers/gpu/drm/i915/intel_color.c | 5 +
drivers/gpu/drm/i915/intel_display.c | 3 ---
drivers/gpu/drm/i915/intel_drv.h | 3 ---
3 files changed, 1 insertion(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_color.c
b/drivers/gpu/drm/i915/intel_color.c
index 37fd9dd..f32e4a7 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -370,12 +370,11 @@ static void haswell_load_luts(struct intel_crtc_state
*crtc_state)
* GAMMA_MODE is configured for split gamma and IPS_CTL has IPS enabled.
*/
if (IS_HASWELL(dev_priv) && crtc_state->ips_enabled &&
- (crtc_state->gamma_mode == GAMMA_MODE_MODE_SPLIT)) {
+ (I915_READ(GAMMA_MODE(crtc->pipe)) & GAMMA_MODE_MODE_SPLIT)) {
hsw_disable_ips(crtc_state);
reenable_ips = true;
}
- crtc_state->gamma_mode = GAMMA_MODE_MODE_8BIT;
I915_WRITE(GAMMA_MODE(crtc->pipe), GAMMA_MODE_MODE_8BIT);
i9xx_load_luts(crtc_state);
@@ -476,7 +475,6 @@ static void broadwell_load_luts(struct intel_crtc_state
*crtc_state)
bdw_load_gamma_lut(crtc_state,
INTEL_INFO(dev_priv)->color.degamma_lut_size);
- crtc_state->gamma_mode = GAMMA_MODE_MODE_SPLIT;
I915_WRITE(GAMMA_MODE(pipe), GAMMA_MODE_MODE_SPLIT);
POSTING_READ(GAMMA_MODE(pipe));
@@ -532,7 +530,6 @@ static void glk_load_luts(struct intel_crtc_state
*crtc_state)
bdw_load_gamma_lut(crtc_state, 0);
- crtc_state->gamma_mode = GAMMA_MODE_MODE_10BIT;
I915_WRITE(GAMMA_MODE(pipe), GAMMA_MODE_MODE_10BIT);
POSTING_READ(GAMMA_MODE(pipe));
}
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 3b70948..704d9d3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9679,9 +9679,6 @@ static bool haswell_get_pipe_config(struct intel_crtc
*crtc,
intel_get_pipe_src_size(crtc, pipe_config);
intel_get_crtc_ycbcr_config(crtc, pipe_config);
- pipe_config->gamma_mode =
- I915_READ(GAMMA_MODE(crtc->pipe)) & GAMMA_MODE_MODE_MASK;
-
power_domain = POWER_DOMAIN_PIPE_PANEL_FITTER(crtc->pipe);
if (intel_display_power_get_if_enabled(dev_priv, power_domain)) {
power_domain_mask |= BIT_ULL(power_domain);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 1028af8..7427a36 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -921,9 +921,6 @@ struct intel_crtc_state {
struct intel_crtc_wm_state wm;
- /* Gamma mode programmed on the pipe */
- uint32_t gamma_mode;
-
/* bitmask of visible planes (enum plane_id) */
u8 active_planes;
u8 nv12_planes;
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [v4 2/4] drm/i915/icl: Add icl pipe degamma and gamma support
Add support for icl pipe degamma and gamma.
v2: Removed a POSTING_READ and corrected the Bit
Definition as per Maarten's comments.
v3: Addressed Matt's review comments. Removed rmw patterns
as suggested by Matt.
v4: Fixed Matt's review comments.
Signed-off-by: Uma Shankar
---
drivers/gpu/drm/i915/i915_reg.h| 3 ++
drivers/gpu/drm/i915/intel_color.c | 67 ++
2 files changed, 70 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 02af9b5..1852c33 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7092,6 +7092,9 @@ enum {
#define GAMMA_MODE_MODE_12BIT (2 << 0)
#define GAMMA_MODE_MODE_SPLIT (3 << 0)
+#definePRE_CSC_GAMMA_ENABLE(1 << 31)
+#definePOST_CSC_GAMMA_ENABLE (1 << 30)
+
/* DMC/CSR */
#define CSR_PROGRAM(i) _MMIO(0x8 + (i) * 4)
#define CSR_SSP_BASE_ADDR_GEN9 0x2FC0
diff --git a/drivers/gpu/drm/i915/intel_color.c
b/drivers/gpu/drm/i915/intel_color.c
index f32e4a7..e72d8d6 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -534,6 +534,71 @@ static void glk_load_luts(struct intel_crtc_state
*crtc_state)
POSTING_READ(GAMMA_MODE(pipe));
}
+static void icl_load_degamma_lut(struct intel_crtc_state *crtc_state)
+{
+ struct drm_device *dev = crtc_state->base.crtc->dev;
+ struct drm_i915_private *dev_priv = to_i915(dev);
+ enum pipe pipe = to_intel_crtc(crtc_state->base.crtc)->pipe;
+ const uint32_t lut_size = INTEL_INFO(dev_priv)->color.degamma_lut_size;
+ uint32_t i;
+
+ /*
+* When setting the auto-increment bit, the hardware seems to
+* ignore the index bits, so we need to reset it to index 0
+* separately.
+*/
+ I915_WRITE(PRE_CSC_GAMC_INDEX(pipe), 0);
+ I915_WRITE(PRE_CSC_GAMC_INDEX(pipe), PRE_CSC_GAMC_AUTO_INCREMENT);
+
+ if (crtc_state->base.degamma_lut) {
+ struct drm_color_lut *lut = crtc_state->base.degamma_lut->data;
+
+ for (i = 0; i < lut_size; i++) {
+ /*
+* First 33 entries represent range from 0 to 1.0
+* 34th and 35th entry will represent extended range
+* inputs 3.0 and 7.0 respectively, currently clamped
+* at 1.0. Since the precision is 16bit, the user value
+* can be directly filled to register.
+* ToDo: Extend to max 7.0.
+*/
+ I915_WRITE(PRE_CSC_GAMC_DATA(pipe), lut[i].red);
+ }
+ } else {
+ /* load a linear table. */
+ for (i = 0; i < lut_size; i++) {
+ uint32_t v = (i * (1 << 16)) / (lut_size - 1);
+
+ I915_WRITE(PRE_CSC_GAMC_DATA(pipe), v);
+ }
+ }
+
+ /* Clamp values > 1.0. */
+ while (i++ < 35)
+ I915_WRITE(PRE_CSC_GAMC_DATA(pipe), (1 << 16));
+}
+
+static void icl_load_luts(struct intel_crtc_state *crtc_state)
+{
+ struct drm_crtc *crtc = crtc_state->base.crtc;
+ struct drm_device *dev = crtc_state->base.crtc->dev;
+ struct drm_i915_private *dev_priv = to_i915(dev);
+ enum pipe pipe = to_intel_crtc(crtc)->pipe;
+ u32 gamma_mode = 0;
+
+ if (crtc_state_is_legacy_gamma(crtc_state)) {
+ haswell_load_luts(crtc_state);
+ return;
+ }
+
+ icl_load_degamma_lut(crtc_state);
+ bdw_load_gamma_lut(crtc_state, 0);
+
+ gamma_mode = GAMMA_MODE_MODE_10BIT | PRE_CSC_GAMMA_ENABLE
+ | POST_CSC_GAMMA_ENABLE;
+ I915_WRITE(GAMMA_MODE(pipe), gamma_mode);
+}
+
/* Loads the palette/gamma unit for the CRTC on CherryView. */
static void cherryview_load_luts(struct intel_crtc_state *crtc_state)
{
@@ -649,6 +714,8 @@ void intel_color_init(struct intel_crtc *crtc)
} else if (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) {
dev_priv->display.load_csc_matrix = ilk_load_csc_matrix;
dev_priv->display.load_luts = glk_load_luts;
+ } else if (IS_ICELAKE(dev_priv)) {
+ dev_priv->display.load_luts = icl_load_luts;
} else {
dev_priv->display.load_luts = i9xx_load_luts;
}
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [v4 3/4] drm/i915/icl: Enable ICL Pipe CSC block
Enable ICL pipe csc hardware. CSC block is enabled
in CSC_MODE register instead of PLANE_COLOR_CTL.
ToDO: Extend the ABI to accept 32 bit coefficient values
instead of 16bit for future platforms.
v2: Addressed Maarten's review comments.
v3: Addressed Matt's review comments. Removed rmw patterns
as suggested by Matt.
v4: Addressed Matt's review comments.
Signed-off-by: Uma Shankar
---
drivers/gpu/drm/i915/i915_reg.h| 4 +++-
drivers/gpu/drm/i915/intel_color.c | 12 ++--
2 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 1852c33..565ef6a 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -9861,7 +9861,9 @@ enum skl_power_gate {
#define _PIPE_A_CSC_COEFF_RV_GV0x49020
#define _PIPE_A_CSC_COEFF_BV 0x49024
#define _PIPE_A_CSC_MODE 0x49028
-#define CSC_BLACK_SCREEN_OFFSET (1 << 2)
+#define CSC_ENABLE(1 << 31)
+#define OUTPUT_CSC_ENABLE (1 << 30)
+#define CSC_BLACK_SCREEN_OFFSET (1 << 2)
#define CSC_POSITION_BEFORE_GAMMA(1 << 1)
#define CSC_MODE_YUV_TO_RGB (1 << 0)
#define _PIPE_A_CSC_PREOFF_HI 0x49030
diff --git a/drivers/gpu/drm/i915/intel_color.c
b/drivers/gpu/drm/i915/intel_color.c
index e72d8d6..d5b240c 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -134,7 +134,11 @@ static void ilk_load_ycbcr_conversion_matrix(struct
intel_crtc *crtc)
I915_WRITE(PIPE_CSC_POSTOFF_HI(pipe), POSTOFF_RGB_TO_YUV_HI);
I915_WRITE(PIPE_CSC_POSTOFF_ME(pipe), POSTOFF_RGB_TO_YUV_ME);
I915_WRITE(PIPE_CSC_POSTOFF_LO(pipe), POSTOFF_RGB_TO_YUV_LO);
- I915_WRITE(PIPE_CSC_MODE(pipe), 0);
+
+ if (INTEL_GEN(dev_priv) >= 10)
+ I915_WRITE(PIPE_CSC_MODE(pipe), OUTPUT_CSC_ENABLE);
+ else
+ I915_WRITE(PIPE_CSC_MODE(pipe), 0);
}
static void ilk_load_csc_matrix(struct intel_crtc_state *crtc_state)
@@ -242,7 +246,10 @@ static void ilk_load_csc_matrix(struct intel_crtc_state
*crtc_state)
I915_WRITE(PIPE_CSC_POSTOFF_ME(pipe), postoff);
I915_WRITE(PIPE_CSC_POSTOFF_LO(pipe), postoff);
- I915_WRITE(PIPE_CSC_MODE(pipe), 0);
+ if (INTEL_GEN(dev_priv) >= 10)
+ I915_WRITE(PIPE_CSC_MODE(pipe), CSC_ENABLE);
+ else
+ I915_WRITE(PIPE_CSC_MODE(pipe), 0);
} else {
uint32_t mode = CSC_MODE_YUV_TO_RGB;
@@ -715,6 +722,7 @@ void intel_color_init(struct intel_crtc *crtc)
dev_priv->display.load_csc_matrix = ilk_load_csc_matrix;
dev_priv->display.load_luts = glk_load_luts;
} else if (IS_ICELAKE(dev_priv)) {
+ dev_priv->display.load_csc_matrix = ilk_load_csc_matrix;
dev_priv->display.load_luts = icl_load_luts;
} else {
dev_priv->display.load_luts = i9xx_load_luts;
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for Add support for Gen 11 pipe color features (rev4)
== Series Details ==
Series: Add support for Gen 11 pipe color features (rev4)
URL : https://patchwork.freedesktop.org/series/51408/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_5337 -> Patchwork_11143
Summary
---
**SUCCESS**
No regressions found.
External URL:
https://patchwork.freedesktop.org/api/1.0/series/51408/revisions/4/mbox/
Known issues
Here are the changes found in Patchwork_11143 that come from known issues:
### IGT changes ###
Issues hit
* igt@gem_ctx_create@basic-files:
- fi-bsw-n3050: PASS -> INCOMPLETE [fdo#105876] / [fdo#108714]
* igt@gem_exec_suspend@basic-s4-devices:
- fi-blb-e6850: PASS -> INCOMPLETE [fdo#107718]
* igt@kms_chamelium@hdmi-hpd-fast:
- fi-kbl-7500u: PASS -> FAIL [fdo#108767]
* igt@kms_pipe_crc_basic@read-crc-pipe-b-frame-sequence:
- fi-byt-clapper: PASS -> FAIL [fdo#103191] / [fdo#107362]
* {igt@runner@aborted}:
- fi-icl-y: NOTRUN -> FAIL [fdo#108915]
Possible fixes
* igt@kms_flip@basic-flip-vs-dpms:
- fi-skl-6700hq: DMESG-WARN [fdo#105998] -> PASS
* igt@kms_frontbuffer_tracking@basic:
- fi-icl-u2: FAIL [fdo#103167] -> PASS
Warnings
* igt@i915_selftest@live_contexts:
- fi-icl-u3: DMESG-FAIL [fdo#108569] -> INCOMPLETE [fdo#108315]
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
[fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
[fdo#105876]: https://bugs.freedesktop.org/show_bug.cgi?id=105876
[fdo#105998]: https://bugs.freedesktop.org/show_bug.cgi?id=105998
[fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
[fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
[fdo#108315]: https://bugs.freedesktop.org/show_bug.cgi?id=108315
[fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
[fdo#108714]: https://bugs.freedesktop.org/show_bug.cgi?id=108714
[fdo#108767]: https://bugs.freedesktop.org/show_bug.cgi?id=108767
[fdo#108915]: https://bugs.freedesktop.org/show_bug.cgi?id=108915
Participating hosts (48 -> 41)
--
Additional (1): fi-icl-y
Missing(8): fi-kbl-soraka fi-ilk-m540 fi-byt-j1900 fi-byt-squawks
fi-bsw-cyan fi-ctg-p8600 fi-gdg-551 fi-pnv-d510
Build changes
-
* Linux: CI_DRM_5337 -> Patchwork_11143
CI_DRM_5337: 3ac901085a9fae8699716ac44579dab1dec546c3 @
git://anongit.freedesktop.org/gfx-ci/linux
IGT_4752: 321fe77d32fef32ef820f53924045fe6ef0cf6ed @
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_11143: 1c61a958fc987d091ee67970494a3c12ab588b38 @
git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
1c61a958fc98 drm/i915/icl: Add degamma and gamma lut size to gen11 caps
01fa72a76c2d drm/i915/icl: Enable ICL Pipe CSC block
dd1779407c88 drm/i915/icl: Add icl pipe degamma and gamma support
0ae0548c84eb drm/i915: Remove gamma_mode state variable
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_11143/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4 3/3] PM/runtime:Replace jiffies based accounting with ktime based accounting
On Thu, 20 Dec 2018 at 15:17, Vincent Guittot
wrote:
>
> From: Thara Gopinath
>
> This patch replaces jiffies based accounting for runtime_active_time
> and runtime_suspended_time with ktime base accounting. This makes the
> runtime debug counters inline with genpd and other pm subsytems which
> uses ktime based accounting.
>
> Signed-off-by: Thara Gopinath
> [move from ktime to raw nsec]
> Signed-off-by: Vincent Guittot
> ---
> drivers/base/power/runtime.c | 17 +
> drivers/base/power/sysfs.c | 11 ---
> include/linux/pm.h | 6 +++---
> 3 files changed, 20 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
> index e695544..f700524 100644
> --- a/drivers/base/power/runtime.c
> +++ b/drivers/base/power/runtime.c
> @@ -64,8 +64,8 @@ static int rpm_suspend(struct device *dev, int rpmflags);
> */
> void update_pm_runtime_accounting(struct device *dev)
> {
> - unsigned long now = jiffies;
> - unsigned long delta;
> + u64 now = ktime_to_ns(ktime_get());
> + u64 delta;
>
> delta = now - dev->power.accounting_timestamp;
>
> @@ -75,9 +75,9 @@ void update_pm_runtime_accounting(struct device *dev)
> return;
>
> if (dev->power.runtime_status == RPM_SUSPENDED)
> - dev->power.suspended_jiffies += delta;
> + dev->power.suspended_time += delta;
> else
> - dev->power.active_jiffies += delta;
> + dev->power.active_time += delta;
> }
>
> static void __update_runtime_status(struct device *dev, enum rpm_status
> status)
> @@ -88,17 +88,18 @@ static void __update_runtime_status(struct device *dev,
> enum rpm_status status)
>
> u64 pm_runtime_suspended_time(struct device *dev)
> {
> - unsigned long flags, time;
> + u64 time;
> + unsigned long flags;
>
> spin_lock_irqsave(&dev->power.lock, flags);
>
> update_pm_runtime_accounting(dev);
>
> - time = dev->power.suspended_jiffies;
> + time = dev->power.suspended_time;
>
> spin_unlock_irqrestore(&dev->power.lock, flags);
>
> - return jiffies_to_nsecs(time);
> + return time;
> }
> EXPORT_SYMBOL_GPL(pm_runtime_suspended_time);
>
> @@ -1503,7 +1504,7 @@ void pm_runtime_init(struct device *dev)
> dev->power.request_pending = false;
> dev->power.request = RPM_REQ_NONE;
> dev->power.deferred_resume = false;
> - dev->power.accounting_timestamp = jiffies;
> + dev->power.accounting_timestamp = ktime_to_ns(ktime_get());
This change worries me a bit, but it may not be a problem in practice.
pm_runtime_init() is called when devices get initialized, via
device_initialize(). If timekeeping has not been initialized prior a
call to ktime_get() is done, it will fail and causing a NULL pointer
deference or something along those lines.
In other words, do we know that device_initialize() is always called
after timekeeping has been initialized during boot?
[...]
Kind regards
Uffe
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4 1/3] PM/runtime: Add a new interface to get accounted time
On Thu, 20 Dec 2018 at 15:16, Vincent Guittot
wrote:
>
> Some drivers (like i915/drm) needs to get the accounted suspended time.
> pm_runtime_suspended_time() will return the suspended accounted time
> in ns unit.
>
> Signed-off-by: Vincent Guittot
Reviewed-by: Ulf Hansson
Kind regards
Uffe
> ---
> drivers/base/power/runtime.c | 16
> include/linux/pm_runtime.h | 2 ++
> 2 files changed, 18 insertions(+)
>
> diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
> index beb85c3..e695544 100644
> --- a/drivers/base/power/runtime.c
> +++ b/drivers/base/power/runtime.c
> @@ -86,6 +86,22 @@ static void __update_runtime_status(struct device *dev,
> enum rpm_status status)
> dev->power.runtime_status = status;
> }
>
> +u64 pm_runtime_suspended_time(struct device *dev)
> +{
> + unsigned long flags, time;
> +
> + spin_lock_irqsave(&dev->power.lock, flags);
> +
> + update_pm_runtime_accounting(dev);
> +
> + time = dev->power.suspended_jiffies;
> +
> + spin_unlock_irqrestore(&dev->power.lock, flags);
> +
> + return jiffies_to_nsecs(time);
> +}
> +EXPORT_SYMBOL_GPL(pm_runtime_suspended_time);
> +
> /**
> * pm_runtime_deactivate_timer - Deactivate given device's suspend timer.
> * @dev: Device to handle.
> diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h
> index f0fc470..d479707 100644
> --- a/include/linux/pm_runtime.h
> +++ b/include/linux/pm_runtime.h
> @@ -113,6 +113,8 @@ static inline bool pm_runtime_is_irq_safe(struct device
> *dev)
> return dev->power.irq_safe;
> }
>
> +extern u64 pm_runtime_suspended_time(struct device *dev);
> +
> #else /* !CONFIG_PM */
>
> static inline bool queue_pm_work(struct work_struct *work) { return false; }
> --
> 2.7.4
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4 2/3] drm/i915: Move on the new pm runtime interface
On Thu, 20 Dec 2018 at 15:17, Vincent Guittot
wrote:
>
> Use the new pm runtime interface to get the accounted suspended time:
> pm_runtime_accounted_time_get()
pm_runtime_suspended_time()
This change also makes quite some nice cleanups to the code, which is
mostly because of converting to the new runtime PM API. I think the
changelog deserves to state that, in some simple way.
>
> Signed-off-by: Vincent Guittot
Other than the minor things above:
Reviewed-by: Ulf Hansson
Kind regards
Uffe
> ---
> drivers/gpu/drm/i915/i915_pmu.c | 16 ++--
> drivers/gpu/drm/i915/i915_pmu.h | 4 ++--
> 2 files changed, 8 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
> index d6c8f8f..3f76f60 100644
> --- a/drivers/gpu/drm/i915/i915_pmu.c
> +++ b/drivers/gpu/drm/i915/i915_pmu.c
> @@ -5,6 +5,7 @@
> */
>
> #include
> +#include
> #include "i915_pmu.h"
> #include "intel_ringbuffer.h"
> #include "i915_drv.h"
> @@ -478,7 +479,6 @@ static u64 get_rc6(struct drm_i915_private *i915)
> * counter value.
> */
> spin_lock_irqsave(&i915->pmu.lock, flags);
> - spin_lock(&kdev->power.lock);
>
> /*
> * After the above branch intel_runtime_pm_get_if_in_use
> failed
> @@ -491,16 +491,13 @@ static u64 get_rc6(struct drm_i915_private *i915)
> * suspended and if not we cannot do better than report the
> last
> * known RC6 value.
> */
> - if (kdev->power.runtime_status == RPM_SUSPENDED) {
> - if
> (!i915->pmu.sample[__I915_SAMPLE_RC6_ESTIMATED].cur)
> - i915->pmu.suspended_jiffies_last =
> -
> kdev->power.suspended_jiffies;
> + if (pm_runtime_status_suspended(kdev)) {
> + val = pm_runtime_suspended_time(kdev);
>
> - val = kdev->power.suspended_jiffies -
> - i915->pmu.suspended_jiffies_last;
> - val += jiffies - kdev->power.accounting_timestamp;
> + if
> (!i915->pmu.sample[__I915_SAMPLE_RC6_ESTIMATED].cur)
> + i915->pmu.suspended_time_last = val;
>
> - val = jiffies_to_nsecs(val);
> + val -= i915->pmu.suspended_time_last;
> val += i915->pmu.sample[__I915_SAMPLE_RC6].cur;
>
> i915->pmu.sample[__I915_SAMPLE_RC6_ESTIMATED].cur =
> val;
> @@ -510,7 +507,6 @@ static u64 get_rc6(struct drm_i915_private *i915)
> val = i915->pmu.sample[__I915_SAMPLE_RC6].cur;
> }
>
> - spin_unlock(&kdev->power.lock);
> spin_unlock_irqrestore(&i915->pmu.lock, flags);
> }
>
> diff --git a/drivers/gpu/drm/i915/i915_pmu.h b/drivers/gpu/drm/i915/i915_pmu.h
> index 7f164ca..3dc2a30 100644
> --- a/drivers/gpu/drm/i915/i915_pmu.h
> +++ b/drivers/gpu/drm/i915/i915_pmu.h
> @@ -95,9 +95,9 @@ struct i915_pmu {
> */
> struct i915_pmu_sample sample[__I915_NUM_PMU_SAMPLERS];
> /**
> -* @suspended_jiffies_last: Cached suspend time from PM core.
> +* @suspended_time_last: Cached suspend time from PM core.
> */
> - unsigned long suspended_jiffies_last;
> + u64 suspended_time_last;
> /**
> * @i915_attr: Memory block holding device attributes.
> */
> --
> 2.7.4
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 3/3] drm/i915: DDI: call intel_psr_ and _edp_drrs_enable() on pipe updates (v2)
On Thu, 2018-12-20 at 09:10 -0800, Rodrigo Vivi wrote:
> On Thu, Dec 20, 2018 at 02:21:20PM +0100, Hans de Goede wrote:
> > Call intel_psr_enable() and intel_edp_drrs_enable() on pipe updates
> > to make
> > sure that we enable PSR / DRRS (when applicable) on fastsets.
I am probably missing something, doesn't intel_pipe_config_compare()
need to check for pipe_config->has_psr? And also read the hardware PSR
state at boot?
> >
> > Note calling these functions when PSR / DRRS has already been
> > enabled is a
> > no-op, so it is safe to do this on every encoder->update_pipe
> > callback.
> >
> > Changes in v2:
> > -Merge the patches adding the intel_psr_enable() and
> > intel_edp_drrs_enable()
> > calls into a single patch
> >
> > Reviewed-by: Maarten Lankhorst
> > Signed-off-by: Hans de Goede
>
> Cc: Dhinakaran Pandiyan
> Cc: José Roberto de Souza
>
> Acked-by: Rodrigo Vivi
>
> > ---
> > drivers/gpu/drm/i915/intel_ddi.c | 19 +++
> > 1 file changed, 19 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> > b/drivers/gpu/drm/i915/intel_ddi.c
> > index e3cc19e19199..fdf57f451b72 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -3537,6 +3537,24 @@ static void intel_disable_ddi(struct
> > intel_encoder *encoder,
> > intel_disable_ddi_dp(encoder, old_crtc_state,
> > old_conn_state);
> > }
> >
> > +static void intel_ddi_update_pipe_dp(struct intel_encoder
> > *encoder,
> > +const struct intel_crtc_state
> > *crtc_state,
> > +const struct drm_connector_state
> > *conn_state)
> > +{
> > + struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> > +
> > + intel_psr_enable(intel_dp, crtc_state);
> > + intel_edp_drrs_enable(intel_dp, crtc_state);
> > +}
> > +
> > +static void intel_ddi_update_pipe(struct intel_encoder *encoder,
> > + const struct intel_crtc_state
> > *crtc_state,
> > + const struct drm_connector_state
> > *conn_state)
> > +{
> > + if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
We could restrict this to eDP outputs as both PSR and DRRS are eDP
features.
> > + intel_ddi_update_pipe_dp(encoder, crtc_state,
> > conn_state);
> > +}
> > +
> > static void intel_ddi_set_fia_lane_count(struct intel_encoder
> > *encoder,
> > const struct intel_crtc_state
> > *pipe_config,
> > enum port port)
> > @@ -4169,6 +4187,7 @@ void intel_ddi_init(struct drm_i915_private
> > *dev_priv, enum port port)
> > intel_encoder->pre_enable = intel_ddi_pre_enable;
> > intel_encoder->disable = intel_disable_ddi;
> > intel_encoder->post_disable = intel_ddi_post_disable;
> > + intel_encoder->update_pipe = intel_ddi_update_pipe;
> > intel_encoder->get_hw_state = intel_ddi_get_hw_state;
> > intel_encoder->get_config = intel_ddi_get_config;
> > intel_encoder->suspend = intel_ddi_encoder_suspend;
> > --
> > 2.20.1
> >
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [v4 1/4] drm/i915: Remove gamma_mode state variable
On Fri, Dec 21, 2018 at 01:29:38AM +0530, Uma Shankar wrote:
> Removed crtc state variable for gamma mode as it's redundant
> since currently we have fixed modes on respective hardware
> platforms. This was making this state variable irrelevant.
>
> Credits-to: Matt Roper
>
> Signed-off-by: Uma Shankar
> ---
> drivers/gpu/drm/i915/intel_color.c | 5 +
> drivers/gpu/drm/i915/intel_display.c | 3 ---
> drivers/gpu/drm/i915/intel_drv.h | 3 ---
> 3 files changed, 1 insertion(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_color.c
> b/drivers/gpu/drm/i915/intel_color.c
> index 37fd9dd..f32e4a7 100644
> --- a/drivers/gpu/drm/i915/intel_color.c
> +++ b/drivers/gpu/drm/i915/intel_color.c
> @@ -370,12 +370,11 @@ static void haswell_load_luts(struct intel_crtc_state
> *crtc_state)
>* GAMMA_MODE is configured for split gamma and IPS_CTL has IPS enabled.
>*/
> if (IS_HASWELL(dev_priv) && crtc_state->ips_enabled &&
> - (crtc_state->gamma_mode == GAMMA_MODE_MODE_SPLIT)) {
> + (I915_READ(GAMMA_MODE(crtc->pipe)) & GAMMA_MODE_MODE_SPLIT)) {
We can probably avoid this read on each LUT load if we just handle it
once during hardware readout/sanitization. Since our driver never
actually sets MODE_SPLIT on this platform, we only have to worry about
the BIOS leaving us in split gamma mode; that means we could do
if (ips)
disable ips
intel_color_set_csc()
intel_color_load_luts()
if (ips)
re-enable ips
once during intel_sanitize_crtc(), to immediately program the hardware
to a known-good state at startup (linear CTM and disabled LUT's). That
would also be a little bit more consistent overall; since we don't
actually readout the BIOS-set LUT's or CTM today, we wind up having them
remain active for a while until they go away with the first fastset or
modeset.
Otherwise, this patch looks good, so
Reviewed-by: Matt Roper
and you can deal with sanitizing the color management stuff at bootup as
a separate patch later.
Matt
> hsw_disable_ips(crtc_state);
> reenable_ips = true;
> }
>
> - crtc_state->gamma_mode = GAMMA_MODE_MODE_8BIT;
> I915_WRITE(GAMMA_MODE(crtc->pipe), GAMMA_MODE_MODE_8BIT);
>
> i9xx_load_luts(crtc_state);
> @@ -476,7 +475,6 @@ static void broadwell_load_luts(struct intel_crtc_state
> *crtc_state)
> bdw_load_gamma_lut(crtc_state,
> INTEL_INFO(dev_priv)->color.degamma_lut_size);
>
> - crtc_state->gamma_mode = GAMMA_MODE_MODE_SPLIT;
> I915_WRITE(GAMMA_MODE(pipe), GAMMA_MODE_MODE_SPLIT);
> POSTING_READ(GAMMA_MODE(pipe));
>
> @@ -532,7 +530,6 @@ static void glk_load_luts(struct intel_crtc_state
> *crtc_state)
>
> bdw_load_gamma_lut(crtc_state, 0);
>
> - crtc_state->gamma_mode = GAMMA_MODE_MODE_10BIT;
> I915_WRITE(GAMMA_MODE(pipe), GAMMA_MODE_MODE_10BIT);
> POSTING_READ(GAMMA_MODE(pipe));
> }
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 3b70948..704d9d3 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9679,9 +9679,6 @@ static bool haswell_get_pipe_config(struct intel_crtc
> *crtc,
> intel_get_pipe_src_size(crtc, pipe_config);
> intel_get_crtc_ycbcr_config(crtc, pipe_config);
>
> - pipe_config->gamma_mode =
> - I915_READ(GAMMA_MODE(crtc->pipe)) & GAMMA_MODE_MODE_MASK;
> -
> power_domain = POWER_DOMAIN_PIPE_PANEL_FITTER(crtc->pipe);
> if (intel_display_power_get_if_enabled(dev_priv, power_domain)) {
> power_domain_mask |= BIT_ULL(power_domain);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 1028af8..7427a36 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -921,9 +921,6 @@ struct intel_crtc_state {
>
> struct intel_crtc_wm_state wm;
>
> - /* Gamma mode programmed on the pipe */
> - uint32_t gamma_mode;
> -
> /* bitmask of visible planes (enum plane_id) */
> u8 active_planes;
> u8 nv12_planes;
> --
> 1.9.1
>
--
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [v4 2/4] drm/i915/icl: Add icl pipe degamma and gamma support
On Fri, Dec 21, 2018 at 01:29:39AM +0530, Uma Shankar wrote:
> Add support for icl pipe degamma and gamma.
>
> v2: Removed a POSTING_READ and corrected the Bit
> Definition as per Maarten's comments.
>
> v3: Addressed Matt's review comments. Removed rmw patterns
> as suggested by Matt.
>
> v4: Fixed Matt's review comments.
>
> Signed-off-by: Uma Shankar
> ---
> drivers/gpu/drm/i915/i915_reg.h| 3 ++
> drivers/gpu/drm/i915/intel_color.c | 67
> ++
> 2 files changed, 70 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 02af9b5..1852c33 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7092,6 +7092,9 @@ enum {
> #define GAMMA_MODE_MODE_12BIT(2 << 0)
> #define GAMMA_MODE_MODE_SPLIT(3 << 0)
>
> +#define PRE_CSC_GAMMA_ENABLE(1 << 31)
> +#define POST_CSC_GAMMA_ENABLE (1 << 30)
> +
> /* DMC/CSR */
> #define CSR_PROGRAM(i) _MMIO(0x8 + (i) * 4)
> #define CSR_SSP_BASE_ADDR_GEN9 0x2FC0
> diff --git a/drivers/gpu/drm/i915/intel_color.c
> b/drivers/gpu/drm/i915/intel_color.c
> index f32e4a7..e72d8d6 100644
> --- a/drivers/gpu/drm/i915/intel_color.c
> +++ b/drivers/gpu/drm/i915/intel_color.c
> @@ -534,6 +534,71 @@ static void glk_load_luts(struct intel_crtc_state
> *crtc_state)
> POSTING_READ(GAMMA_MODE(pipe));
> }
>
> +static void icl_load_degamma_lut(struct intel_crtc_state *crtc_state)
> +{
> + struct drm_device *dev = crtc_state->base.crtc->dev;
> + struct drm_i915_private *dev_priv = to_i915(dev);
> + enum pipe pipe = to_intel_crtc(crtc_state->base.crtc)->pipe;
> + const uint32_t lut_size = INTEL_INFO(dev_priv)->color.degamma_lut_size;
> + uint32_t i;
> +
> + /*
> + * When setting the auto-increment bit, the hardware seems to
> + * ignore the index bits, so we need to reset it to index 0
> + * separately.
> + */
> + I915_WRITE(PRE_CSC_GAMC_INDEX(pipe), 0);
> + I915_WRITE(PRE_CSC_GAMC_INDEX(pipe), PRE_CSC_GAMC_AUTO_INCREMENT);
> +
> + if (crtc_state->base.degamma_lut) {
> + struct drm_color_lut *lut = crtc_state->base.degamma_lut->data;
> +
> + for (i = 0; i < lut_size; i++) {
> + /*
> + * First 33 entries represent range from 0 to 1.0
> + * 34th and 35th entry will represent extended range
> + * inputs 3.0 and 7.0 respectively, currently clamped
> + * at 1.0. Since the precision is 16bit, the user value
> + * can be directly filled to register.
> + * ToDo: Extend to max 7.0.
> + */
> + I915_WRITE(PRE_CSC_GAMC_DATA(pipe), lut[i].red);
Since 1.0 = 0x100, should we scale this by (0x100 / 0xFF) until we have
the newer segmented gamma ABI you're working on?
> + }
> + } else {
> + /* load a linear table. */
> + for (i = 0; i < lut_size; i++) {
> + uint32_t v = (i * (1 << 16)) / (lut_size - 1);
> +
> + I915_WRITE(PRE_CSC_GAMC_DATA(pipe), v);
> + }
> + }
> +
> + /* Clamp values > 1.0. */
> + while (i++ < 35)
> + I915_WRITE(PRE_CSC_GAMC_DATA(pipe), (1 << 16));
> +}
> +
> +static void icl_load_luts(struct intel_crtc_state *crtc_state)
> +{
> + struct drm_crtc *crtc = crtc_state->base.crtc;
> + struct drm_device *dev = crtc_state->base.crtc->dev;
> + struct drm_i915_private *dev_priv = to_i915(dev);
> + enum pipe pipe = to_intel_crtc(crtc)->pipe;
> + u32 gamma_mode = 0;
> +
> + if (crtc_state_is_legacy_gamma(crtc_state)) {
> + haswell_load_luts(crtc_state);
> + return;
> + }
> +
> + icl_load_degamma_lut(crtc_state);
> + bdw_load_gamma_lut(crtc_state, 0);
> +
> + gamma_mode = GAMMA_MODE_MODE_10BIT | PRE_CSC_GAMMA_ENABLE
> + | POST_CSC_GAMMA_ENABLE;
> + I915_WRITE(GAMMA_MODE(pipe), gamma_mode);
We might as well just stick these constants right in the I915_WRITE
call; no need to create a separate local variable to hold them.
Matt
> +}
> +
> /* Loads the palette/gamma unit for the CRTC on CherryView. */
> static void cherryview_load_luts(struct intel_crtc_state *crtc_state)
> {
> @@ -649,6 +714,8 @@ void intel_color_init(struct intel_crtc *crtc)
> } else if (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) {
> dev_priv->display.load_csc_matrix = ilk_load_csc_matrix;
> dev_priv->display.load_luts = glk_load_luts;
> + } else if (IS_ICELAKE(dev_priv)) {
> + dev_priv->display.load_luts = icl_load_luts;
> } else {
> dev_priv->display.load_luts = i9xx_load_luts;
> }
> --
> 1.9.1
>
--
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel
Re: [Intel-gfx] Local Display Direct Flip Feature Discussion
+ intel-gfx and dri-devel Hi, This is the Gvt-g local display direct flip discussion. And please reference here (https://lists.freedesktop.org/archives/intel-gvt-dev/2018-December/004594.html) to see what the feature is. Mostly, with Gvt-g local display direct flip feature, vGPU can directly post its framebuffer to the assigned local HW display plane w/o host userspace involvement. We propose to use drm_framebuffer to stand for the framebuffer attached on a vGPU's plane. Since GVT-g can guarantee this drm_framebuffer is updated with the latest info of the framebuffer attached on the vGPU's plane during guest page flip, atomic async plane update mechanism is used in i915 to help updating the vGPU assigned HW planes efficiently with those special drm_framebuffers. Host userspace is in charge of the HW plane assignment. And currently we are discussing about: 1) Whether the drm_framebuffer can be generic enough to deal with the direct flip use cases? In our case, the drm_framebuffer has no gem backends, as the display memory management is handled by guest. 2) How to expose the drm_framebuffer to the host userspace? a) Through vfio/display ioctl The kernel implementation is easy. But this might turn qemu into a display manager. Qemu has to handle all the KMS mode-setting stuff things. b) Through drm/i915 ioctl This needs some code change in kernel space to deal with this ioctl between i915 and GVT-g. But the good thing is this ioctl can be used by the userspace display manager w/o vfio/display's help. Thanks. > -Original Message- > From: Gerd Hoffmann [mailto:kra...@redhat.com] > Sent: Thursday, December 20, 2018 7:34 PM > To: Zhang, Tina > Cc: Tian, Kevin ; Wang, Zhenyu Z > ; Wang, Zhi A ; He, Min > ; Yuan, Hang ; Alex Williamson > ; Lv, Zhiyuan ; Vetter, > Daniel ; intel-gvt-dev d...@lists.freedesktop.org>; Wang, Hongbo > Subject: Re: Local Display Direct Flip Feature Discussion > > On Thu, Dec 20, 2018 at 08:45:09AM +, Zhang, Tina wrote: > > > > > > > -Original Message- > > > From: intel-gvt-dev > > > [mailto:intel-gvt-dev-boun...@lists.freedesktop.org] On Behalf Of > > > Gerd Hoffmann > > > Sent: Wednesday, December 19, 2018 7:26 PM > > > To: Zhang, Tina > > > Cc: Tian, Kevin ; Wang, Zhenyu Z > > > ; Wang, Zhi A ; He, > > > Min ; Yuan, Hang ; Alex > > > Williamson ; Lv, Zhiyuan > > > ; Vetter, Daniel ; > > > intel-gvt-dev ; Wang, Hongbo > > > > > > Subject: Re: Local Display Direct Flip Feature Discussion > > > > > > Hi, > > > > > > > > Isn't a framebuffer just a gem object with metadata (fourcc, > > > > > width, height, stride, ...) attached? So I'm wondering how that > > > > > works in detail. What happens on page-flip? Do you make the > > > > > framebuffer reference another gem object then? Or do you blit > > > > > the guest display to the framebuffer? > > > > The special DRM framebuffer is transparent to the guest display driver. > > > > We just want to use this object to save the guest framebuffer info > > > > in host-side. > > > > > > Hmm, so this object isn't a normal drm framebuffer. You are just > > > masquerading it as drm framebuffer, so you can assign it to a drm > > > crtc or drm plane. > > Not so bad actually :-) > > Host just wants to use a drm framebuffer to describe the drm > > framebuffer attached on a vGPU's plane. And the only special thing is > > that this host drm frambuffer has on gem backends, which means host > > cannot manage the > > ... has no gem ... ? > > > memory of this drm framebuffer. And it's OK, as the guest does the > management. > > Besides, generic drm framebuffer was designed to be able to deal with > > this kind of situation. > > Hmm, ok. Never dealed with framebuffers not backed by gem objects so far, > seems to not be very common too. But the doc comments in > include/drm/drm_framebuffer.h suggest it is fine indeed. > > What is the plan for hardware cursor support? Support two framebuffers, for > primary and cursor? We got the hardware cursor support in our to-go list: https://lists.freedesktop.org/archives/intel-gvt-dev/2018-December/004559.html We will support both primary and cursor. > > > > It makes sense to allow mapping guest outputs to host outputs. I > > > think it is more useful to handle that at crtc level not plane > > > level, so it'll work for both primary and cursor plane. I think it > > > would be cleaner to introduce new drm (generic or i915) APIs for > > > that instead of creating special framebuffer objects which behave in non- > standard ways. > > The APIs solution is one of our options and we have patch for it. The > > userspace interface is still under discussion. And here are the three > candidates: > > 1) Through i915 ioctl > > 2) Through vfio/display ioctl > > 3) Through GVT-g sys fs or debugfs > > And option2 is considered as the preferred one, as this drm > > framebuffer is related to the vGPU. > > (3) Having this (in debugfs) would be usefu
Re: [Intel-gfx] Local Display Direct Flip Feature Discussion
On 2018.12.20 12:33:35 +0100, Gerd Hoffmann wrote: > On Thu, Dec 20, 2018 at 08:45:09AM +, Zhang, Tina wrote: > > > > > > > -Original Message- > > > From: intel-gvt-dev [mailto:intel-gvt-dev-boun...@lists.freedesktop.org] > > > On > > > Behalf Of Gerd Hoffmann > > > Sent: Wednesday, December 19, 2018 7:26 PM > > > To: Zhang, Tina > > > Cc: Tian, Kevin ; Wang, Zhenyu Z > > > ; Wang, Zhi A ; He, Min > > > ; Yuan, Hang ; Alex Williamson > > > ; Lv, Zhiyuan ; Vetter, > > > Daniel ; intel-gvt-dev > > d...@lists.freedesktop.org>; Wang, Hongbo > > > Subject: Re: Local Display Direct Flip Feature Discussion > > > > > > Hi, > > > > > > > > Isn't a framebuffer just a gem object with metadata (fourcc, width, > > > > > height, stride, ...) attached? So I'm wondering how that works in > > > > > detail. What happens on page-flip? Do you make the framebuffer > > > > > reference another gem object then? Or do you blit the guest display > > > > > to the framebuffer? > > > > The special DRM framebuffer is transparent to the guest display driver. > > > > We just want to use this object to save the guest framebuffer info in > > > > host-side. > > > > > > Hmm, so this object isn't a normal drm framebuffer. You are just > > > masquerading it as drm framebuffer, so you can assign it to a drm crtc or > > > drm > > > plane. > > Not so bad actually :-) > > Host just wants to use a drm framebuffer to describe the drm framebuffer > > attached on a vGPU's plane. And the only special thing is that this host drm > > frambuffer has on gem backends, which means host cannot manage the > > ... has no gem ... ? > > > memory of this drm framebuffer. And it's OK, as the guest does the > > management. > > Besides, generic drm framebuffer was designed to be able to deal with this > > kind > > of situation. > > Hmm, ok. Never dealed with framebuffers not backed by gem objects so > far, seems to not be very common too. But the doc comments in > include/drm/drm_framebuffer.h suggest it is fine indeed. > > What is the plan for hardware cursor support? Support two framebuffers, > for primary and cursor? > > > > It makes sense to allow mapping guest outputs to host outputs. I think > > > it is > > > more useful to handle that at crtc level not plane level, so it'll work > > > for both > > > primary and cursor plane. I think it would be cleaner to introduce new > > > drm > > > (generic or i915) APIs for that instead of creating special framebuffer > > > objects > > > which behave in non-standard ways. > > The APIs solution is one of our options and we have patch for it. The > > userspace > > interface is still under discussion. And here are the three candidates: > > 1) Through i915 ioctl > > 2) Through vfio/display ioctl > > 3) Through GVT-g sys fs or debugfs > > And option2 is considered as the preferred one, as this drm framebuffer is > > related > > to the vGPU. > > (3) Having this (in debugfs) would be useful for debugging purposes, > even in addition to (1) or (2). > > (2) Implies qemu must handle it, so support must either be implemented > in qemu directly, or in another process cooperating with qemu in > some way (extending spice protocol & spice client comes to mind). > Given that the input side (mouse and kbd events) need cooperation > with qemu anyway this might not be much of a limitation though. > > (1) Would allow to handle this without having to worry about qemu/vfio > at all. Needs some infrastructure (drm ioctl to enumerate vgpus for > example). Possibly the amdgpu guys (which are doing vgpu using > sr/iov instead of mdev) are interested in this too. > > No clear winner, but I tend to agree that (2) looks best. > yeah, that's always my idea on this, because vfio interface would be only place to align with vGPU/mdev life cycle. GVT specific sysfs should only contain kind of static configuration or things won't depend on vGPU open or close, so isn't a good place. Debugfs could be used for anything helpful to dump. Original idea is still using dmabuf object to present vGPU plane. Instead of using polling for now as in qemu drm/kms display POC code, as you sugguested, some notification method could be created for vGPU plane flip. For direct flip, some flag or attribute could be added for drm/i915 ioctl when create drm_framebuffer from dmabuf or when modesetting to say "this is special vGPU plane buffer so pls help to do atomic async update on it when guest flip". > > > Alternatively try to tackle the problem in a completely different way. > > > Right now qemu will check for display changes using a timer. We could add > > > some page flip notification mechanism (using an eventfd for example) so > > > qemu can update the host plane (or notify spice client) instantly instead > > > of > > > waiting for the next display refresh timer tick. > > We could do that. But the local display direct flip feature doesn't want to > > involve > > the host userspace, c
Re: [Intel-gfx] uABI / Removing DRM_I915_LOW_LEVEL_TRACEPOINTS Kconfig
Quoting Michael Sartain (2018-12-20 20:27:19) > On Wed, Dec 19, 2018, at 12:22 PM, Steven Rostedt wrote: > > On Wed, 19 Dec 2018 12:08:18 +0200 > > Joonas Lahtinen wrote: > >· > > > To me, it seems almost as if folks are too preoccupied with thinking if > > > we somehow can do this through tracepoints, to stop and actually think > > > if we should. > >· > > Regardless of whether it should or shouldn't, one solution to this is > > to make the trace event in question record basically nothing but a > > pointer. > > Right now, these are the events I'm capturing w/ an AMD gpu: > > amdgpu_cs:0-1150 [002] 630662.649417: amdgpu_cs_ioctl: > sched_job=3490671, timeline=gfx, context=105, seqno=3081096, > ring_name=91cb1ab1bdd0, num_ibs=3 > gfx-190 [000] 630662.649451: amdgpu_sched_run_job: > sched_job=3490671, timeline=gfx, context=105, seqno=3081096, > ring_name=91cb1ab1bdd0, num_ibs=3 > gfx-190 [000] 630662.649454: dma_fence_signaled: > driver=amd_sched timeline=gfx context=104 seqno=3081096 > > With Intel gpu (and rebuilt kernel w/ CONFIG_DRM_I915_LOW_LEVEL_TRACEPOINTS): > > -0 [002] 821.717208: intel_engine_notify: dev=0, > engine=0:0, seqno=38896, waiters=1 > RenderThread-1024 [002] 825.722358: i915_request_queue: dev=0, > engine=0:0, hw_id=9, ctx=30, seqno=38896, flags=0x0 > RenderThread-1024 [002] 825.722371: i915_request_add: dev=0, > engine=0:0, hw_id=9, ctx=30, seqno=38896, global=0 > RenderThread-1024 [002] 825.722372: i915_request_submit: dev=0, > engine=0:0, hw_id=9, ctx=30, seqno=38896, global=0 > RenderThread-1024 [002] 825.745964: i915_request_execute: dev=0, > engine=0:0, hw_id=9, ctx=30, seqno=38896, global=42199 > RenderThread-1024 [002] 825.745964: i915_request_in: dev=0, > engine=0:0, hw_id=9, ctx=30, seqno=38896, prio=0, global=42199, port=1 > -0 [002] 825.755943: intel_engine_notify: dev=0, > engine=0:0, seqno=42199, waiters=1 > > It's quite obvious that just because gpuvis sees those amdgpu tracepoints > when > running with the AMD card and parses and displays those, it does *not* get > those same tracepoints when I run with an Intel gpu. > > And with my Iris Pro Graphics 580 Gen9, I can reasonably expect to get the > above i915 tracepoints. > > But if I install a new Intel Xe Gen11, why should I expect to see those Gen9 > i915 tracepoint events? Is it because we are tying tracepoints and the > created uABI to *kernel modules* and not the hardware? You're on the correct track here. The issue is that even for Gen9, we foresee to unnecessary maintenance burden to keep the tracepoints, as we work on the scheduler or them disappearing from kernel scope when HW-assisted scheduling is enabled. And the lack of versioning on tracepoints does not make it easy on userspace to do graceful degradiation or to detect the underneath platform. Stuffing the versioning info to every tracepoint, to make sure it's in the captured ring buffer being inspected, is not too elegant, so some auxilary interface would still have to be probed. > I'm asking, because personally I would expect the hardware to drive these > tracepoint events, much like I check cpu flags to see whether I can run AVX > code, or perf has intel_pt recording on one machine, but not another. If we wanted to make sure we can keep them stable within a gen, we would have to move them closer to the point we talk to hardware and would basically just emit information that a) came from userspace (which is stable due to uABI) or b) is going to hardware (we don't expect the underlying hardware magically change). > Right now gpuvis graphs the above events in an easy to understand view. > Occasionally, it's really nice to use trace-cmd to get textual representation > for grepping, etc. Storing pointers would obviously break that. Steve's idea kind of solves that. There would be an auxilary module build out-of-tree (say, from gpuvis), that would emit a new tracepoint "b" with more information on triggering tracepoint "a". So basically you would stop looking for tracepoint "a", and load your module and just look for "b". It's bit on the grey area when it comes to breaking userspace and the philosophical question is, is it us breaking userspace or userspace setting itself a trap. But I guess it might be OK, if the distros knowingly bundle such out-of-tree module (which is not subject to kernel stability). > I guess if it's > what we need to do to avoid the uABI problem, then it's what we do - still > better than using an entirely new tracing system if we can avoid that. The bigger problem that I'd still like to hear some ideas for is before drawing conclusion is about elegantly sourcing the tracepoints from hardware events. Trying to do live conversion from hardware generated ring buffer during execution just to make sure it interleaves with the software generated ring buffer and works under same trigger, sounds not so p
