[Intel-gfx] Fixes that failed to apply to v5.11-rc4

[Intel-gfx] Fixes that failed to apply to v5.11-rc4

[Jani Nikula]

The following commits have been marked as Cc: stable or fixing something
in v5.11-rc4 or earlier, but failed to cherry-pick to
drm-intel-fixes. Please see if they are worth backporting, and please do
so if they are.

Conflicts:
dbe13ae1d6ab ("drm/i915/pmu: Don't grab wakeref when enabling events")
9bb36cf66091 ("drm/i915: Check for rq->hwsp validity after acquiring RCU lock")
5b4dc95cf7f5 ("drm/i915/gt: Prevent use of engine->wa_ctx after error")
6a3daee1b38e ("drm/i915/selftests: Fix some error codes")
67fba3f1c73b ("drm/i915/dp: Fix LTTPR vswing/pre-emp setting in non-transparent mode")

Fails to build:
3170a21f7059 ("drm/i915: Only enable DFP 4:4:4->4:2:0 conversion when outputting YCbCr 4:4:4")

BR,
Jani.

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

[PATCH] drm/i915/gt: Prevent use of engine->wa_ctx after error

[Chris Wilson]

On error we unpin and free the wa_ctx.vma, but do not clear any of the
derived flags. During lrc_init, we look at the flags and attempt to
dereference the wa_ctx.vma if they are set. To protect the error path
where we try to limp along without the wa_ctx, make sure we clear those
flags!

Reported-by: Matt Roper <matthew.d.roper@intel.com>
Fixes: 604a8f6f1e33 ("drm/i915/lrc: Only enable per-context and per-bb buffers if set")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Matt Roper <matthew.d.roper@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: <stable@vger.kernel.org> # v4.15+
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20210108204026.20682-1-chris@chris-wilson.co.uk
(cherry-picked from 5b4dc95cf7f573e927fbbd406ebe54225d41b9b2)
---
 drivers/gpu/drm/i915/gt/intel_lrc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index 7614a3d24fca..26c7d0a50585 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -3988,6 +3988,9 @@ static int lrc_setup_wa_ctx(struct intel_engine_cs *engine)
 static void lrc_destroy_wa_ctx(struct intel_engine_cs *engine)
 {
 	i915_vma_unpin_and_release(&engine->wa_ctx.vma, 0);
+
+	/* Called on error unwind, clear all flags to prevent further use */
+	memset(&engine->wa_ctx, 0, sizeof(engine->wa_ctx));
 }
 
 typedef u32 *(*wa_bb_func_t)(struct intel_engine_cs *engine, u32 *batch);
-- 
2.30.0

[Intel-gfx] [PATCH] drm/i915/gt: Prevent use of engine->wa_ctx after error

[Chris Wilson]

On error we unpin and free the wa_ctx.vma, but do not clear any of the
derived flags. During lrc_init, we look at the flags and attempt to
dereference the wa_ctx.vma if they are set. To protect the error path
where we try to limp along without the wa_ctx, make sure we clear those
flags!

Reported-by: Matt Roper <matthew.d.roper@intel.com>
Fixes: 604a8f6f1e33 ("drm/i915/lrc: Only enable per-context and per-bb buffers if set")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Matt Roper <matthew.d.roper@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: <stable@vger.kernel.org> # v4.15+
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20210108204026.20682-1-chris@chris-wilson.co.uk
(cherry-picked from 5b4dc95cf7f573e927fbbd406ebe54225d41b9b2)
---
 drivers/gpu/drm/i915/gt/intel_lrc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index 7614a3d24fca..26c7d0a50585 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -3988,6 +3988,9 @@ static int lrc_setup_wa_ctx(struct intel_engine_cs *engine)
 static void lrc_destroy_wa_ctx(struct intel_engine_cs *engine)
 {
 	i915_vma_unpin_and_release(&engine->wa_ctx.vma, 0);
+
+	/* Called on error unwind, clear all flags to prevent further use */
+	memset(&engine->wa_ctx, 0, sizeof(engine->wa_ctx));
 }
 
 typedef u32 *(*wa_bb_func_t)(struct intel_engine_cs *engine, u32 *batch);
-- 
2.30.0

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

[Intel-gfx] [PATCH] drm/i915/pmu: Don't grab wakeref when enabling events

[Chris Wilson]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Chris found a CI report which points out calling intel_runtime_pm_get from
inside i915_pmu_enable hook is not allowed since it can be invoked from
hard irq context. This is something we knew but forgot, so lets fix it
once again.

We do this by syncing the internal book keeping with hardware rc6 counter
on driver load.

v2:
 * Always sync on parking and fully sync on init.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Fixes: f4e9894b6952 ("drm/i915/pmu: Correct the rc6 offset upon enabling")
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: https://patchwork.freedesktop.org/patch/msgid/20201214094349.3563876-1-tvrtko.ursulin@linux.intel.com
(cherry picked from commit dbe13ae1d6abaab417edf3c37601c6a56594a4cd)
---
 drivers/gpu/drm/i915/i915_pmu.c | 30 ++++++++++++++++--------------
 1 file changed, 16 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
index d76685ce0399..9856479b56d8 100644
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -184,13 +184,24 @@ static u64 get_rc6(struct intel_gt *gt)
 	return val;
 }
 
-static void park_rc6(struct drm_i915_private *i915)
+static void init_rc6(struct i915_pmu *pmu)
 {
-	struct i915_pmu *pmu = &i915->pmu;
+	struct drm_i915_private *i915 = container_of(pmu, typeof(*i915), pmu);
+	intel_wakeref_t wakeref;
 
-	if (pmu->enable & config_enabled_mask(I915_PMU_RC6_RESIDENCY))
+	with_intel_runtime_pm(i915->gt.uncore->rpm, wakeref) {
 		pmu->sample[__I915_SAMPLE_RC6].cur = __get_rc6(&i915->gt);
+		pmu->sample[__I915_SAMPLE_RC6_LAST_REPORTED].cur =
+					pmu->sample[__I915_SAMPLE_RC6].cur;
+		pmu->sleep_last = ktime_get();
+	}
+}
 
+static void park_rc6(struct drm_i915_private *i915)
+{
+	struct i915_pmu *pmu = &i915->pmu;
+
+	pmu->sample[__I915_SAMPLE_RC6].cur = __get_rc6(&i915->gt);
 	pmu->sleep_last = ktime_get();
 }
 
@@ -201,6 +212,7 @@ static u64 get_rc6(struct intel_gt *gt)
 	return __get_rc6(gt);
 }
 
+static void init_rc6(struct i915_pmu *pmu) { }
 static void park_rc6(struct drm_i915_private *i915) {}
 
 #endif
@@ -612,10 +624,8 @@ static void i915_pmu_enable(struct perf_event *event)
 		container_of(event->pmu, typeof(*i915), pmu.base);
 	unsigned int bit = event_enabled_bit(event);
 	struct i915_pmu *pmu = &i915->pmu;
-	intel_wakeref_t wakeref;
 	unsigned long flags;
 
-	wakeref = intel_runtime_pm_get(&i915->runtime_pm);
 	spin_lock_irqsave(&pmu->lock, flags);
 
 	/*
@@ -626,13 +636,6 @@ static void i915_pmu_enable(struct perf_event *event)
 	GEM_BUG_ON(bit >= ARRAY_SIZE(pmu->enable_count));
 	GEM_BUG_ON(pmu->enable_count[bit] == ~0);
 
-	if (pmu->enable_count[bit] == 0 &&
-	    config_enabled_mask(I915_PMU_RC6_RESIDENCY) & BIT_ULL(bit)) {
-		pmu->sample[__I915_SAMPLE_RC6_LAST_REPORTED].cur = 0;
-		pmu->sample[__I915_SAMPLE_RC6].cur = __get_rc6(&i915->gt);
-		pmu->sleep_last = ktime_get();
-	}
-
 	pmu->enable |= BIT_ULL(bit);
 	pmu->enable_count[bit]++;
 
@@ -673,8 +676,6 @@ static void i915_pmu_enable(struct perf_event *event)
 	 * an existing non-zero value.
 	 */
 	local64_set(&event->hw.prev_count, __i915_pmu_event_read(event));
-
-	intel_runtime_pm_put(&i915->runtime_pm, wakeref);
 }
 
 static void i915_pmu_disable(struct perf_event *event)
@@ -1130,6 +1131,7 @@ void i915_pmu_register(struct drm_i915_private *i915)
 	hrtimer_init(&pmu->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
 	pmu->timer.function = i915_sample;
 	pmu->cpuhp.cpu = -1;
+	init_rc6(pmu);
 
 	if (!is_igp(i915)) {
 		pmu->name = kasprintf(GFP_KERNEL,
-- 
2.30.0

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

Re: [Intel-gfx] [PATCH] drm/i915/pmu: Don't grab wakeref when enabling events

[Tvrtko Ursulin]

On 18/01/2021 10:07, Chris Wilson wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Chris found a CI report which points out calling intel_runtime_pm_get from
> inside i915_pmu_enable hook is not allowed since it can be invoked from
> hard irq context. This is something we knew but forgot, so lets fix it
> once again.
> 
> We do this by syncing the internal book keeping with hardware rc6 counter
> on driver load.
> 
> v2:
>   * Always sync on parking and fully sync on init.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Fixes: f4e9894b6952 ("drm/i915/pmu: Correct the rc6 offset upon enabling")
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> Link: https://patchwork.freedesktop.org/patch/msgid/20201214094349.3563876-1-tvrtko.ursulin@linux.intel.com
> (cherry picked from commit dbe13ae1d6abaab417edf3c37601c6a56594a4cd)
> ---
>   drivers/gpu/drm/i915/i915_pmu.c | 30 ++++++++++++++++--------------
>   1 file changed, 16 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
> index d76685ce0399..9856479b56d8 100644
> --- a/drivers/gpu/drm/i915/i915_pmu.c
> +++ b/drivers/gpu/drm/i915/i915_pmu.c
> @@ -184,13 +184,24 @@ static u64 get_rc6(struct intel_gt *gt)
>   	return val;
>   }
>   
> -static void park_rc6(struct drm_i915_private *i915)
> +static void init_rc6(struct i915_pmu *pmu)
>   {
> -	struct i915_pmu *pmu = &i915->pmu;
> +	struct drm_i915_private *i915 = container_of(pmu, typeof(*i915), pmu);
> +	intel_wakeref_t wakeref;
>   
> -	if (pmu->enable & config_enabled_mask(I915_PMU_RC6_RESIDENCY))
> +	with_intel_runtime_pm(i915->gt.uncore->rpm, wakeref) {
>   		pmu->sample[__I915_SAMPLE_RC6].cur = __get_rc6(&i915->gt);
> +		pmu->sample[__I915_SAMPLE_RC6_LAST_REPORTED].cur =
> +					pmu->sample[__I915_SAMPLE_RC6].cur;
> +		pmu->sleep_last = ktime_get();
> +	}
> +}
>   
> +static void park_rc6(struct drm_i915_private *i915)
> +{
> +	struct i915_pmu *pmu = &i915->pmu;
> +
> +	pmu->sample[__I915_SAMPLE_RC6].cur = __get_rc6(&i915->gt);
>   	pmu->sleep_last = ktime_get();
>   }
>   
> @@ -201,6 +212,7 @@ static u64 get_rc6(struct intel_gt *gt)
>   	return __get_rc6(gt);
>   }
>   
> +static void init_rc6(struct i915_pmu *pmu) { }
>   static void park_rc6(struct drm_i915_private *i915) {}
>   
>   #endif
> @@ -612,10 +624,8 @@ static void i915_pmu_enable(struct perf_event *event)
>   		container_of(event->pmu, typeof(*i915), pmu.base);
>   	unsigned int bit = event_enabled_bit(event);
>   	struct i915_pmu *pmu = &i915->pmu;
> -	intel_wakeref_t wakeref;
>   	unsigned long flags;
>   
> -	wakeref = intel_runtime_pm_get(&i915->runtime_pm);
>   	spin_lock_irqsave(&pmu->lock, flags);
>   
>   	/*
> @@ -626,13 +636,6 @@ static void i915_pmu_enable(struct perf_event *event)
>   	GEM_BUG_ON(bit >= ARRAY_SIZE(pmu->enable_count));
>   	GEM_BUG_ON(pmu->enable_count[bit] == ~0);
>   
> -	if (pmu->enable_count[bit] == 0 &&
> -	    config_enabled_mask(I915_PMU_RC6_RESIDENCY) & BIT_ULL(bit)) {
> -		pmu->sample[__I915_SAMPLE_RC6_LAST_REPORTED].cur = 0;
> -		pmu->sample[__I915_SAMPLE_RC6].cur = __get_rc6(&i915->gt);
> -		pmu->sleep_last = ktime_get();
> -	}
> -
>   	pmu->enable |= BIT_ULL(bit);
>   	pmu->enable_count[bit]++;
>   
> @@ -673,8 +676,6 @@ static void i915_pmu_enable(struct perf_event *event)
>   	 * an existing non-zero value.
>   	 */
>   	local64_set(&event->hw.prev_count, __i915_pmu_event_read(event));
> -
> -	intel_runtime_pm_put(&i915->runtime_pm, wakeref);
>   }
>   
>   static void i915_pmu_disable(struct perf_event *event)
> @@ -1130,6 +1131,7 @@ void i915_pmu_register(struct drm_i915_private *i915)
>   	hrtimer_init(&pmu->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
>   	pmu->timer.function = i915_sample;
>   	pmu->cpuhp.cpu = -1;
> +	init_rc6(pmu);
>   
>   	if (!is_igp(i915)) {
>   		pmu->name = kasprintf(GFP_KERNEL,
> 

Looks 100% as the conflict resolution I was about to send out so thanks 
for doing the work.

Regards,

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

[PATCH] drm/i915: Check for rq->hwsp validity after acquiring RCU lock

[Chris Wilson]

Since we allow removing the timeline map at runtime, there is a risk
that rq->hwsp points into a stale page. To control that risk, we hold
the RCU read lock while reading *rq->hwsp, but we missed a couple of
important barriers. First, the unpinning / removal of the timeline map
must be after all RCU readers into that map are complete, i.e. after an
rcu barrier (in this case courtesy of call_rcu()). Secondly, we must
make sure that the rq->hwsp we are about to dereference under the RCU
lock is valid. In this case, we make the rq->hwsp pointer safe during
i915_request_retire() and so we know that rq->hwsp may become invalid
only after the request has been signaled. Therefore is the request is
not yet signaled when we acquire rq->hwsp under the RCU, we know that
rq->hwsp will remain valid for the duration of the RCU read lock.

This is a very small window that may lead to either considering the
request not completed (causing a delay until the request is checked
again, any wait for the request is not affected) or dereferencing an
invalid pointer.

Fixes: 3adac4689f58 ("drm/i915: Introduce concept of per-timeline (context) HWSP")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: <stable@vger.kernel.org> # v5.1+
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20201218122421.18344-1-chris@chris-wilson.co.uk
(cherry picked from commit 9bb36cf66091ddf2d8840e5aa705ad3c93a6279b)
---
 drivers/gpu/drm/i915/gt/intel_breadcrumbs.c |  9 ++---
 drivers/gpu/drm/i915/gt/intel_timeline.c    | 10 +++---
 drivers/gpu/drm/i915/i915_request.h         | 37 ++++++++++++++++++---
 3 files changed, 38 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
index a24cc1ff08a0..0625cbb3b431 100644
--- a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
+++ b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
@@ -134,11 +134,6 @@ static bool remove_signaling_context(struct intel_breadcrumbs *b,
 	return true;
 }
 
-static inline bool __request_completed(const struct i915_request *rq)
-{
-	return i915_seqno_passed(__hwsp_seqno(rq), rq->fence.seqno);
-}
-
 __maybe_unused static bool
 check_signal_order(struct intel_context *ce, struct i915_request *rq)
 {
@@ -257,7 +252,7 @@ static void signal_irq_work(struct irq_work *work)
 		list_for_each_entry_rcu(rq, &ce->signals, signal_link) {
 			bool release;
 
-			if (!__request_completed(rq))
+			if (!__i915_request_is_complete(rq))
 				break;
 
 			if (!test_and_clear_bit(I915_FENCE_FLAG_SIGNAL,
@@ -379,7 +374,7 @@ static void insert_breadcrumb(struct i915_request *rq)
 	 * straight onto a signaled list, and queue the irq worker for
 	 * its signal completion.
 	 */
-	if (__request_completed(rq)) {
+	if (__i915_request_is_complete(rq)) {
 		if (__signal_request(rq) &&
 		    llist_add(&rq->signal_node, &b->signaled_requests))
 			irq_work_queue(&b->irq_work);
diff --git a/drivers/gpu/drm/i915/gt/intel_timeline.c b/drivers/gpu/drm/i915/gt/intel_timeline.c
index 7ea94d201fe6..8015964043eb 100644
--- a/drivers/gpu/drm/i915/gt/intel_timeline.c
+++ b/drivers/gpu/drm/i915/gt/intel_timeline.c
@@ -126,6 +126,10 @@ static void __rcu_cacheline_free(struct rcu_head *rcu)
 	struct intel_timeline_cacheline *cl =
 		container_of(rcu, typeof(*cl), rcu);
 
+	/* Must wait until after all *rq->hwsp are complete before removing */
+	i915_gem_object_unpin_map(cl->hwsp->vma->obj);
+	__idle_hwsp_free(cl->hwsp, ptr_unmask_bits(cl->vaddr, CACHELINE_BITS));
+
 	i915_active_fini(&cl->active);
 	kfree(cl);
 }
@@ -133,11 +137,6 @@ static void __rcu_cacheline_free(struct rcu_head *rcu)
 static void __idle_cacheline_free(struct intel_timeline_cacheline *cl)
 {
 	GEM_BUG_ON(!i915_active_is_idle(&cl->active));
-
-	i915_gem_object_unpin_map(cl->hwsp->vma->obj);
-	i915_vma_put(cl->hwsp->vma);
-	__idle_hwsp_free(cl->hwsp, ptr_unmask_bits(cl->vaddr, CACHELINE_BITS));
-
 	call_rcu(&cl->rcu, __rcu_cacheline_free);
 }
 
@@ -179,7 +178,6 @@ cacheline_alloc(struct intel_timeline_hwsp *hwsp, unsigned int cacheline)
 		return ERR_CAST(vaddr);
 	}
 
-	i915_vma_get(hwsp->vma);
 	cl->hwsp = hwsp;
 	cl->vaddr = page_pack_bits(vaddr, cacheline);
 
diff --git a/drivers/gpu/drm/i915/i915_request.h b/drivers/gpu/drm/i915/i915_request.h
index 620b6fab2c5c..92adfee30c7c 100644
--- a/drivers/gpu/drm/i915/i915_request.h
+++ b/drivers/gpu/drm/i915/i915_request.h
@@ -434,7 +434,7 @@ static inline u32 hwsp_seqno(const struct i915_request *rq)
 
 static inline bool __i915_request_has_started(const struct i915_request *rq)
 {
-	return i915_seqno_passed(hwsp_seqno(rq), rq->fence.seqno - 1);
+	return i915_seqno_passed(__hwsp_seqno(rq), rq->fence.seqno - 1);
 }
 
 /**
@@ -465,11 +465,19 @@ static inline bool __i915_request_has_started(const struct i915_request *rq)
  */
 static inline bool i915_request_started(const struct i915_request *rq)
 {
+	bool result;
+
 	if (i915_request_signaled(rq))
 		return true;
 
-	/* Remember: started but may have since been preempted! */
-	return __i915_request_has_started(rq);
+	result = true;
+	rcu_read_lock(); /* the HWSP may be freed at runtime */
+	if (likely(!i915_request_signaled(rq)))
+		/* Remember: started but may have since been preempted! */
+		result = __i915_request_has_started(rq);
+	rcu_read_unlock();
+
+	return result;
 }
 
 /**
@@ -482,10 +490,16 @@ static inline bool i915_request_started(const struct i915_request *rq)
  */
 static inline bool i915_request_is_running(const struct i915_request *rq)
 {
+	bool result;
+
 	if (!i915_request_is_active(rq))
 		return false;
 
-	return __i915_request_has_started(rq);
+	rcu_read_lock();
+	result = __i915_request_has_started(rq) && i915_request_is_active(rq);
+	rcu_read_unlock();
+
+	return result;
 }
 
 /**
@@ -509,12 +523,25 @@ static inline bool i915_request_is_ready(const struct i915_request *rq)
 	return !list_empty(&rq->sched.link);
 }
 
+static inline bool __i915_request_is_complete(const struct i915_request *rq)
+{
+	return i915_seqno_passed(__hwsp_seqno(rq), rq->fence.seqno);
+}
+
 static inline bool i915_request_completed(const struct i915_request *rq)
 {
+	bool result;
+
 	if (i915_request_signaled(rq))
 		return true;
 
-	return i915_seqno_passed(hwsp_seqno(rq), rq->fence.seqno);
+	result = true;
+	rcu_read_lock(); /* the HWSP may be freed at runtime */
+	if (likely(!i915_request_signaled(rq)))
+		result = __i915_request_is_complete(rq);
+	rcu_read_unlock();
+
+	return result;
 }
 
 static inline void i915_request_mark_complete(struct i915_request *rq)
-- 
2.30.0

[Intel-gfx] [PATCH] drm/i915: Check for rq->hwsp validity after acquiring RCU lock

[Chris Wilson]

Since we allow removing the timeline map at runtime, there is a risk
that rq->hwsp points into a stale page. To control that risk, we hold
the RCU read lock while reading *rq->hwsp, but we missed a couple of
important barriers. First, the unpinning / removal of the timeline map
must be after all RCU readers into that map are complete, i.e. after an
rcu barrier (in this case courtesy of call_rcu()). Secondly, we must
make sure that the rq->hwsp we are about to dereference under the RCU
lock is valid. In this case, we make the rq->hwsp pointer safe during
i915_request_retire() and so we know that rq->hwsp may become invalid
only after the request has been signaled. Therefore is the request is
not yet signaled when we acquire rq->hwsp under the RCU, we know that
rq->hwsp will remain valid for the duration of the RCU read lock.

This is a very small window that may lead to either considering the
request not completed (causing a delay until the request is checked
again, any wait for the request is not affected) or dereferencing an
invalid pointer.

Fixes: 3adac4689f58 ("drm/i915: Introduce concept of per-timeline (context) HWSP")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: <stable@vger.kernel.org> # v5.1+
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20201218122421.18344-1-chris@chris-wilson.co.uk
(cherry picked from commit 9bb36cf66091ddf2d8840e5aa705ad3c93a6279b)
---
 drivers/gpu/drm/i915/gt/intel_breadcrumbs.c |  9 ++---
 drivers/gpu/drm/i915/gt/intel_timeline.c    | 10 +++---
 drivers/gpu/drm/i915/i915_request.h         | 37 ++++++++++++++++++---
 3 files changed, 38 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
index a24cc1ff08a0..0625cbb3b431 100644
--- a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
+++ b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
@@ -134,11 +134,6 @@ static bool remove_signaling_context(struct intel_breadcrumbs *b,
 	return true;
 }
 
-static inline bool __request_completed(const struct i915_request *rq)
-{
-	return i915_seqno_passed(__hwsp_seqno(rq), rq->fence.seqno);
-}
-
 __maybe_unused static bool
 check_signal_order(struct intel_context *ce, struct i915_request *rq)
 {
@@ -257,7 +252,7 @@ static void signal_irq_work(struct irq_work *work)
 		list_for_each_entry_rcu(rq, &ce->signals, signal_link) {
 			bool release;
 
-			if (!__request_completed(rq))
+			if (!__i915_request_is_complete(rq))
 				break;
 
 			if (!test_and_clear_bit(I915_FENCE_FLAG_SIGNAL,
@@ -379,7 +374,7 @@ static void insert_breadcrumb(struct i915_request *rq)
 	 * straight onto a signaled list, and queue the irq worker for
 	 * its signal completion.
 	 */
-	if (__request_completed(rq)) {
+	if (__i915_request_is_complete(rq)) {
 		if (__signal_request(rq) &&
 		    llist_add(&rq->signal_node, &b->signaled_requests))
 			irq_work_queue(&b->irq_work);
diff --git a/drivers/gpu/drm/i915/gt/intel_timeline.c b/drivers/gpu/drm/i915/gt/intel_timeline.c
index 7ea94d201fe6..8015964043eb 100644
--- a/drivers/gpu/drm/i915/gt/intel_timeline.c
+++ b/drivers/gpu/drm/i915/gt/intel_timeline.c
@@ -126,6 +126,10 @@ static void __rcu_cacheline_free(struct rcu_head *rcu)
 	struct intel_timeline_cacheline *cl =
 		container_of(rcu, typeof(*cl), rcu);
 
+	/* Must wait until after all *rq->hwsp are complete before removing */
+	i915_gem_object_unpin_map(cl->hwsp->vma->obj);
+	__idle_hwsp_free(cl->hwsp, ptr_unmask_bits(cl->vaddr, CACHELINE_BITS));
+
 	i915_active_fini(&cl->active);
 	kfree(cl);
 }
@@ -133,11 +137,6 @@ static void __rcu_cacheline_free(struct rcu_head *rcu)
 static void __idle_cacheline_free(struct intel_timeline_cacheline *cl)
 {
 	GEM_BUG_ON(!i915_active_is_idle(&cl->active));
-
-	i915_gem_object_unpin_map(cl->hwsp->vma->obj);
-	i915_vma_put(cl->hwsp->vma);
-	__idle_hwsp_free(cl->hwsp, ptr_unmask_bits(cl->vaddr, CACHELINE_BITS));
-
 	call_rcu(&cl->rcu, __rcu_cacheline_free);
 }
 
@@ -179,7 +178,6 @@ cacheline_alloc(struct intel_timeline_hwsp *hwsp, unsigned int cacheline)
 		return ERR_CAST(vaddr);
 	}
 
-	i915_vma_get(hwsp->vma);
 	cl->hwsp = hwsp;
 	cl->vaddr = page_pack_bits(vaddr, cacheline);
 
diff --git a/drivers/gpu/drm/i915/i915_request.h b/drivers/gpu/drm/i915/i915_request.h
index 620b6fab2c5c..92adfee30c7c 100644
--- a/drivers/gpu/drm/i915/i915_request.h
+++ b/drivers/gpu/drm/i915/i915_request.h
@@ -434,7 +434,7 @@ static inline u32 hwsp_seqno(const struct i915_request *rq)
 
 static inline bool __i915_request_has_started(const struct i915_request *rq)
 {
-	return i915_seqno_passed(hwsp_seqno(rq), rq->fence.seqno - 1);
+	return i915_seqno_passed(__hwsp_seqno(rq), rq->fence.seqno - 1);
 }
 
 /**
@@ -465,11 +465,19 @@ static inline bool __i915_request_has_started(const struct i915_request *rq)
  */
 static inline bool i915_request_started(const struct i915_request *rq)
 {
+	bool result;
+
 	if (i915_request_signaled(rq))
 		return true;
 
-	/* Remember: started but may have since been preempted! */
-	return __i915_request_has_started(rq);
+	result = true;
+	rcu_read_lock(); /* the HWSP may be freed at runtime */
+	if (likely(!i915_request_signaled(rq)))
+		/* Remember: started but may have since been preempted! */
+		result = __i915_request_has_started(rq);
+	rcu_read_unlock();
+
+	return result;
 }
 
 /**
@@ -482,10 +490,16 @@ static inline bool i915_request_started(const struct i915_request *rq)
  */
 static inline bool i915_request_is_running(const struct i915_request *rq)
 {
+	bool result;
+
 	if (!i915_request_is_active(rq))
 		return false;
 
-	return __i915_request_has_started(rq);
+	rcu_read_lock();
+	result = __i915_request_has_started(rq) && i915_request_is_active(rq);
+	rcu_read_unlock();
+
+	return result;
 }
 
 /**
@@ -509,12 +523,25 @@ static inline bool i915_request_is_ready(const struct i915_request *rq)
 	return !list_empty(&rq->sched.link);
 }
 
+static inline bool __i915_request_is_complete(const struct i915_request *rq)
+{
+	return i915_seqno_passed(__hwsp_seqno(rq), rq->fence.seqno);
+}
+
 static inline bool i915_request_completed(const struct i915_request *rq)
 {
+	bool result;
+
 	if (i915_request_signaled(rq))
 		return true;
 
-	return i915_seqno_passed(hwsp_seqno(rq), rq->fence.seqno);
+	result = true;
+	rcu_read_lock(); /* the HWSP may be freed at runtime */
+	if (likely(!i915_request_signaled(rq)))
+		result = __i915_request_is_complete(rq);
+	rcu_read_unlock();
+
+	return result;
 }
 
 static inline void i915_request_mark_complete(struct i915_request *rq)
-- 
2.30.0

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

Re: [Intel-gfx] [PATCH] drm/i915: Check for rq->hwsp validity after acquiring RCU lock

[Jani Nikula]

On Mon, 18 Jan 2021, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Since we allow removing the timeline map at runtime, there is a risk
> that rq->hwsp points into a stale page. To control that risk, we hold
> the RCU read lock while reading *rq->hwsp, but we missed a couple of
> important barriers. First, the unpinning / removal of the timeline map
> must be after all RCU readers into that map are complete, i.e. after an
> rcu barrier (in this case courtesy of call_rcu()). Secondly, we must
> make sure that the rq->hwsp we are about to dereference under the RCU
> lock is valid. In this case, we make the rq->hwsp pointer safe during
> i915_request_retire() and so we know that rq->hwsp may become invalid
> only after the request has been signaled. Therefore is the request is
> not yet signaled when we acquire rq->hwsp under the RCU, we know that
> rq->hwsp will remain valid for the duration of the RCU read lock.
>
> This is a very small window that may lead to either considering the
> request not completed (causing a delay until the request is checked
> again, any wait for the request is not affected) or dereferencing an
> invalid pointer.
>
> Fixes: 3adac4689f58 ("drm/i915: Introduce concept of per-timeline (context) HWSP")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: <stable@vger.kernel.org> # v5.1+
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Link: https://patchwork.freedesktop.org/patch/msgid/20201218122421.18344-1-chris@chris-wilson.co.uk
> (cherry picked from commit 9bb36cf66091ddf2d8840e5aa705ad3c93a6279b)

Thanks for the backports, all three pushed to drm-intel-fixes.

BR,
Jani.


> ---
>  drivers/gpu/drm/i915/gt/intel_breadcrumbs.c |  9 ++---
>  drivers/gpu/drm/i915/gt/intel_timeline.c    | 10 +++---
>  drivers/gpu/drm/i915/i915_request.h         | 37 ++++++++++++++++++---
>  3 files changed, 38 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
> index a24cc1ff08a0..0625cbb3b431 100644
> --- a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
> @@ -134,11 +134,6 @@ static bool remove_signaling_context(struct intel_breadcrumbs *b,
>  	return true;
>  }
>  
> -static inline bool __request_completed(const struct i915_request *rq)
> -{
> -	return i915_seqno_passed(__hwsp_seqno(rq), rq->fence.seqno);
> -}
> -
>  __maybe_unused static bool
>  check_signal_order(struct intel_context *ce, struct i915_request *rq)
>  {
> @@ -257,7 +252,7 @@ static void signal_irq_work(struct irq_work *work)
>  		list_for_each_entry_rcu(rq, &ce->signals, signal_link) {
>  			bool release;
>  
> -			if (!__request_completed(rq))
> +			if (!__i915_request_is_complete(rq))
>  				break;
>  
>  			if (!test_and_clear_bit(I915_FENCE_FLAG_SIGNAL,
> @@ -379,7 +374,7 @@ static void insert_breadcrumb(struct i915_request *rq)
>  	 * straight onto a signaled list, and queue the irq worker for
>  	 * its signal completion.
>  	 */
> -	if (__request_completed(rq)) {
> +	if (__i915_request_is_complete(rq)) {
>  		if (__signal_request(rq) &&
>  		    llist_add(&rq->signal_node, &b->signaled_requests))
>  			irq_work_queue(&b->irq_work);
> diff --git a/drivers/gpu/drm/i915/gt/intel_timeline.c b/drivers/gpu/drm/i915/gt/intel_timeline.c
> index 7ea94d201fe6..8015964043eb 100644
> --- a/drivers/gpu/drm/i915/gt/intel_timeline.c
> +++ b/drivers/gpu/drm/i915/gt/intel_timeline.c
> @@ -126,6 +126,10 @@ static void __rcu_cacheline_free(struct rcu_head *rcu)
>  	struct intel_timeline_cacheline *cl =
>  		container_of(rcu, typeof(*cl), rcu);
>  
> +	/* Must wait until after all *rq->hwsp are complete before removing */
> +	i915_gem_object_unpin_map(cl->hwsp->vma->obj);
> +	__idle_hwsp_free(cl->hwsp, ptr_unmask_bits(cl->vaddr, CACHELINE_BITS));
> +
>  	i915_active_fini(&cl->active);
>  	kfree(cl);
>  }
> @@ -133,11 +137,6 @@ static void __rcu_cacheline_free(struct rcu_head *rcu)
>  static void __idle_cacheline_free(struct intel_timeline_cacheline *cl)
>  {
>  	GEM_BUG_ON(!i915_active_is_idle(&cl->active));
> -
> -	i915_gem_object_unpin_map(cl->hwsp->vma->obj);
> -	i915_vma_put(cl->hwsp->vma);
> -	__idle_hwsp_free(cl->hwsp, ptr_unmask_bits(cl->vaddr, CACHELINE_BITS));
> -
>  	call_rcu(&cl->rcu, __rcu_cacheline_free);
>  }
>  
> @@ -179,7 +178,6 @@ cacheline_alloc(struct intel_timeline_hwsp *hwsp, unsigned int cacheline)
>  		return ERR_CAST(vaddr);
>  	}
>  
> -	i915_vma_get(hwsp->vma);
>  	cl->hwsp = hwsp;
>  	cl->vaddr = page_pack_bits(vaddr, cacheline);
>  
> diff --git a/drivers/gpu/drm/i915/i915_request.h b/drivers/gpu/drm/i915/i915_request.h
> index 620b6fab2c5c..92adfee30c7c 100644
> --- a/drivers/gpu/drm/i915/i915_request.h
> +++ b/drivers/gpu/drm/i915/i915_request.h
> @@ -434,7 +434,7 @@ static inline u32 hwsp_seqno(const struct i915_request *rq)
>  
>  static inline bool __i915_request_has_started(const struct i915_request *rq)
>  {
> -	return i915_seqno_passed(hwsp_seqno(rq), rq->fence.seqno - 1);
> +	return i915_seqno_passed(__hwsp_seqno(rq), rq->fence.seqno - 1);
>  }
>  
>  /**
> @@ -465,11 +465,19 @@ static inline bool __i915_request_has_started(const struct i915_request *rq)
>   */
>  static inline bool i915_request_started(const struct i915_request *rq)
>  {
> +	bool result;
> +
>  	if (i915_request_signaled(rq))
>  		return true;
>  
> -	/* Remember: started but may have since been preempted! */
> -	return __i915_request_has_started(rq);
> +	result = true;
> +	rcu_read_lock(); /* the HWSP may be freed at runtime */
> +	if (likely(!i915_request_signaled(rq)))
> +		/* Remember: started but may have since been preempted! */
> +		result = __i915_request_has_started(rq);
> +	rcu_read_unlock();
> +
> +	return result;
>  }
>  
>  /**
> @@ -482,10 +490,16 @@ static inline bool i915_request_started(const struct i915_request *rq)
>   */
>  static inline bool i915_request_is_running(const struct i915_request *rq)
>  {
> +	bool result;
> +
>  	if (!i915_request_is_active(rq))
>  		return false;
>  
> -	return __i915_request_has_started(rq);
> +	rcu_read_lock();
> +	result = __i915_request_has_started(rq) && i915_request_is_active(rq);
> +	rcu_read_unlock();
> +
> +	return result;
>  }
>  
>  /**
> @@ -509,12 +523,25 @@ static inline bool i915_request_is_ready(const struct i915_request *rq)
>  	return !list_empty(&rq->sched.link);
>  }
>  
> +static inline bool __i915_request_is_complete(const struct i915_request *rq)
> +{
> +	return i915_seqno_passed(__hwsp_seqno(rq), rq->fence.seqno);
> +}
> +
>  static inline bool i915_request_completed(const struct i915_request *rq)
>  {
> +	bool result;
> +
>  	if (i915_request_signaled(rq))
>  		return true;
>  
> -	return i915_seqno_passed(hwsp_seqno(rq), rq->fence.seqno);
> +	result = true;
> +	rcu_read_lock(); /* the HWSP may be freed at runtime */
> +	if (likely(!i915_request_signaled(rq)))
> +		result = __i915_request_is_complete(rq);
> +	rcu_read_unlock();
> +
> +	return result;
>  }
>  
>  static inline void i915_request_mark_complete(struct i915_request *rq)

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [Intel-gfx] [PATCH] drm/i915: Check for rq->hwsp validity after acquiring RCU lock

[Jani Nikula]

On Mon, 18 Jan 2021, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Since we allow removing the timeline map at runtime, there is a risk
> that rq->hwsp points into a stale page. To control that risk, we hold
> the RCU read lock while reading *rq->hwsp, but we missed a couple of
> important barriers. First, the unpinning / removal of the timeline map
> must be after all RCU readers into that map are complete, i.e. after an
> rcu barrier (in this case courtesy of call_rcu()). Secondly, we must
> make sure that the rq->hwsp we are about to dereference under the RCU
> lock is valid. In this case, we make the rq->hwsp pointer safe during
> i915_request_retire() and so we know that rq->hwsp may become invalid
> only after the request has been signaled. Therefore is the request is
> not yet signaled when we acquire rq->hwsp under the RCU, we know that
> rq->hwsp will remain valid for the duration of the RCU read lock.
>
> This is a very small window that may lead to either considering the
> request not completed (causing a delay until the request is checked
> again, any wait for the request is not affected) or dereferencing an
> invalid pointer.
>
> Fixes: 3adac4689f58 ("drm/i915: Introduce concept of per-timeline (context) HWSP")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: <stable@vger.kernel.org> # v5.1+
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Link: https://patchwork.freedesktop.org/patch/msgid/20201218122421.18344-1-chris@chris-wilson.co.uk
> (cherry picked from commit 9bb36cf66091ddf2d8840e5aa705ad3c93a6279b)

Thanks for the backports, all three pushed to drm-intel-fixes.

BR,
Jani.


> ---
>  drivers/gpu/drm/i915/gt/intel_breadcrumbs.c |  9 ++---
>  drivers/gpu/drm/i915/gt/intel_timeline.c    | 10 +++---
>  drivers/gpu/drm/i915/i915_request.h         | 37 ++++++++++++++++++---
>  3 files changed, 38 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
> index a24cc1ff08a0..0625cbb3b431 100644
> --- a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
> @@ -134,11 +134,6 @@ static bool remove_signaling_context(struct intel_breadcrumbs *b,
>  	return true;
>  }
>  
> -static inline bool __request_completed(const struct i915_request *rq)
> -{
> -	return i915_seqno_passed(__hwsp_seqno(rq), rq->fence.seqno);
> -}
> -
>  __maybe_unused static bool
>  check_signal_order(struct intel_context *ce, struct i915_request *rq)
>  {
> @@ -257,7 +252,7 @@ static void signal_irq_work(struct irq_work *work)
>  		list_for_each_entry_rcu(rq, &ce->signals, signal_link) {
>  			bool release;
>  
> -			if (!__request_completed(rq))
> +			if (!__i915_request_is_complete(rq))
>  				break;
>  
>  			if (!test_and_clear_bit(I915_FENCE_FLAG_SIGNAL,
> @@ -379,7 +374,7 @@ static void insert_breadcrumb(struct i915_request *rq)
>  	 * straight onto a signaled list, and queue the irq worker for
>  	 * its signal completion.
>  	 */
> -	if (__request_completed(rq)) {
> +	if (__i915_request_is_complete(rq)) {
>  		if (__signal_request(rq) &&
>  		    llist_add(&rq->signal_node, &b->signaled_requests))
>  			irq_work_queue(&b->irq_work);
> diff --git a/drivers/gpu/drm/i915/gt/intel_timeline.c b/drivers/gpu/drm/i915/gt/intel_timeline.c
> index 7ea94d201fe6..8015964043eb 100644
> --- a/drivers/gpu/drm/i915/gt/intel_timeline.c
> +++ b/drivers/gpu/drm/i915/gt/intel_timeline.c
> @@ -126,6 +126,10 @@ static void __rcu_cacheline_free(struct rcu_head *rcu)
>  	struct intel_timeline_cacheline *cl =
>  		container_of(rcu, typeof(*cl), rcu);
>  
> +	/* Must wait until after all *rq->hwsp are complete before removing */
> +	i915_gem_object_unpin_map(cl->hwsp->vma->obj);
> +	__idle_hwsp_free(cl->hwsp, ptr_unmask_bits(cl->vaddr, CACHELINE_BITS));
> +
>  	i915_active_fini(&cl->active);
>  	kfree(cl);
>  }
> @@ -133,11 +137,6 @@ static void __rcu_cacheline_free(struct rcu_head *rcu)
>  static void __idle_cacheline_free(struct intel_timeline_cacheline *cl)
>  {
>  	GEM_BUG_ON(!i915_active_is_idle(&cl->active));
> -
> -	i915_gem_object_unpin_map(cl->hwsp->vma->obj);
> -	i915_vma_put(cl->hwsp->vma);
> -	__idle_hwsp_free(cl->hwsp, ptr_unmask_bits(cl->vaddr, CACHELINE_BITS));
> -
>  	call_rcu(&cl->rcu, __rcu_cacheline_free);
>  }
>  
> @@ -179,7 +178,6 @@ cacheline_alloc(struct intel_timeline_hwsp *hwsp, unsigned int cacheline)
>  		return ERR_CAST(vaddr);
>  	}
>  
> -	i915_vma_get(hwsp->vma);
>  	cl->hwsp = hwsp;
>  	cl->vaddr = page_pack_bits(vaddr, cacheline);
>  
> diff --git a/drivers/gpu/drm/i915/i915_request.h b/drivers/gpu/drm/i915/i915_request.h
> index 620b6fab2c5c..92adfee30c7c 100644
> --- a/drivers/gpu/drm/i915/i915_request.h
> +++ b/drivers/gpu/drm/i915/i915_request.h
> @@ -434,7 +434,7 @@ static inline u32 hwsp_seqno(const struct i915_request *rq)
>  
>  static inline bool __i915_request_has_started(const struct i915_request *rq)
>  {
> -	return i915_seqno_passed(hwsp_seqno(rq), rq->fence.seqno - 1);
> +	return i915_seqno_passed(__hwsp_seqno(rq), rq->fence.seqno - 1);
>  }
>  
>  /**
> @@ -465,11 +465,19 @@ static inline bool __i915_request_has_started(const struct i915_request *rq)
>   */
>  static inline bool i915_request_started(const struct i915_request *rq)
>  {
> +	bool result;
> +
>  	if (i915_request_signaled(rq))
>  		return true;
>  
> -	/* Remember: started but may have since been preempted! */
> -	return __i915_request_has_started(rq);
> +	result = true;
> +	rcu_read_lock(); /* the HWSP may be freed at runtime */
> +	if (likely(!i915_request_signaled(rq)))
> +		/* Remember: started but may have since been preempted! */
> +		result = __i915_request_has_started(rq);
> +	rcu_read_unlock();
> +
> +	return result;
>  }
>  
>  /**
> @@ -482,10 +490,16 @@ static inline bool i915_request_started(const struct i915_request *rq)
>   */
>  static inline bool i915_request_is_running(const struct i915_request *rq)
>  {
> +	bool result;
> +
>  	if (!i915_request_is_active(rq))
>  		return false;
>  
> -	return __i915_request_has_started(rq);
> +	rcu_read_lock();
> +	result = __i915_request_has_started(rq) && i915_request_is_active(rq);
> +	rcu_read_unlock();
> +
> +	return result;
>  }
>  
>  /**
> @@ -509,12 +523,25 @@ static inline bool i915_request_is_ready(const struct i915_request *rq)
>  	return !list_empty(&rq->sched.link);
>  }
>  
> +static inline bool __i915_request_is_complete(const struct i915_request *rq)
> +{
> +	return i915_seqno_passed(__hwsp_seqno(rq), rq->fence.seqno);
> +}
> +
>  static inline bool i915_request_completed(const struct i915_request *rq)
>  {
> +	bool result;
> +
>  	if (i915_request_signaled(rq))
>  		return true;
>  
> -	return i915_seqno_passed(hwsp_seqno(rq), rq->fence.seqno);
> +	result = true;
> +	rcu_read_lock(); /* the HWSP may be freed at runtime */
> +	if (likely(!i915_request_signaled(rq)))
> +		result = __i915_request_is_complete(rq);
> +	rcu_read_unlock();
> +
> +	return result;
>  }
>  
>  static inline void i915_request_mark_complete(struct i915_request *rq)

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

Re: [Intel-gfx] Fixes that failed to apply to v5.11-rc4

[Chris Wilson]

Quoting Jani Nikula (2021-01-18 09:07:02)
> 
> The following commits have been marked as Cc: stable or fixing something
> in v5.11-rc4 or earlier, but failed to cherry-pick to
> drm-intel-fixes. Please see if they are worth backporting, and please do
> so if they are.
> 
> Conflicts:
> 6a3daee1b38e ("drm/i915/selftests: Fix some error codes")

Selftest bad error codes, that should not impact users so not worth the
backport.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✗ Fi.CI.BUILD: failure for drm/i915: Check for rq->hwsp validity after acquiring RCU lock (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: Check for rq->hwsp validity after acquiring RCU lock (rev2)
URL   : https://patchwork.freedesktop.org/series/85618/
State : failure

== Summary ==

Applying: drm/i915/gt: Prevent use of engine->wa_ctx after error
Using index info to reconstruct a base tree...
M	drivers/gpu/drm/i915/gt/intel_lrc.c
Falling back to patching base and 3-way merge...
Auto-merging drivers/gpu/drm/i915/gt/intel_lrc.c
CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/gt/intel_lrc.c
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 drm/i915/gt: Prevent use of engine->wa_ctx after error
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

[PATCH -fixes] drm/i915: Only enable DFP 4:4:4->4:2:0 conversion when outputting YCbCr 4:4:4

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Let's not enable the 4:4:4->4:2:0 conversion bit in the DFP unless we're
actually outputting YCbCr 4:4:4. It would appear some protocol
converters blindy consult this bit even when the source is outputting
RGB, resulting in a visual mess.

Cc: stable@vger.kernel.org
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/2914
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20210111164111.13302-1-ville.syrjala@linux.intel.com
Fixes: 181567aa9f0d ("drm/i915: Do YCbCr 444->420 conversion via DP protocol converters")
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
(cherry picked from commit 3170a21f7059c4660c469f59bf529f372a57da5f)
---
Unfortunately the crtc_state plumbing to
intel_dp_configure_protocol_converter() was part of the 
HDMI 2.1 PCON stuff, so couldn't just cherry-pick it alone.

 drivers/gpu/drm/i915/display/intel_ddi.c | 2 +-
 drivers/gpu/drm/i915/display/intel_dp.c  | 9 +++++----
 drivers/gpu/drm/i915/display/intel_dp.h  | 3 ++-
 3 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 92940a0c5ef8..d5ace48b1ace 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -3725,7 +3725,7 @@ static void hsw_ddi_pre_enable_dp(struct intel_atomic_state *state,
 	intel_ddi_init_dp_buf_reg(encoder, crtc_state);
 	if (!is_mst)
 		intel_dp_set_power(intel_dp, DP_SET_POWER_D0);
-	intel_dp_configure_protocol_converter(intel_dp);
+	intel_dp_configure_protocol_converter(intel_dp, crtc_state);
 	intel_dp_sink_set_decompression_state(intel_dp, crtc_state,
 					      true);
 	intel_dp_sink_set_fec_ready(intel_dp, crtc_state);
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 37f1a10fd021..09123e8625c4 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -4014,7 +4014,8 @@ static void intel_dp_enable_port(struct intel_dp *intel_dp,
 	intel_de_posting_read(dev_priv, intel_dp->output_reg);
 }
 
-void intel_dp_configure_protocol_converter(struct intel_dp *intel_dp)
+void intel_dp_configure_protocol_converter(struct intel_dp *intel_dp,
+					   const struct intel_crtc_state *crtc_state)
 {
 	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
 	u8 tmp;
@@ -4033,8 +4034,8 @@ void intel_dp_configure_protocol_converter(struct intel_dp *intel_dp)
 		drm_dbg_kms(&i915->drm, "Failed to set protocol converter HDMI mode to %s\n",
 			    enableddisabled(intel_dp->has_hdmi_sink));
 
-	tmp = intel_dp->dfp.ycbcr_444_to_420 ?
-		DP_CONVERSION_TO_YCBCR420_ENABLE : 0;
+	tmp = crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR444 &&
+		intel_dp->dfp.ycbcr_444_to_420 ? DP_CONVERSION_TO_YCBCR420_ENABLE : 0;
 
 	if (drm_dp_dpcd_writeb(&intel_dp->aux,
 			       DP_PROTOCOL_CONVERTER_CONTROL_1, tmp) != 1)
@@ -4088,7 +4089,7 @@ static void intel_enable_dp(struct intel_atomic_state *state,
 	}
 
 	intel_dp_set_power(intel_dp, DP_SET_POWER_D0);
-	intel_dp_configure_protocol_converter(intel_dp);
+	intel_dp_configure_protocol_converter(intel_dp, pipe_config);
 	intel_dp_start_link_train(intel_dp, pipe_config);
 	intel_dp_stop_link_train(intel_dp, pipe_config);
 
diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
index b871a09b6901..05f7ddf7a795 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.h
+++ b/drivers/gpu/drm/i915/display/intel_dp.h
@@ -51,7 +51,8 @@ int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
 int intel_dp_retrain_link(struct intel_encoder *encoder,
 			  struct drm_modeset_acquire_ctx *ctx);
 void intel_dp_set_power(struct intel_dp *intel_dp, u8 mode);
-void intel_dp_configure_protocol_converter(struct intel_dp *intel_dp);
+void intel_dp_configure_protocol_converter(struct intel_dp *intel_dp,
+					   const struct intel_crtc_state *crtc_state);
 void intel_dp_sink_set_decompression_state(struct intel_dp *intel_dp,
 					   const struct intel_crtc_state *crtc_state,
 					   bool enable);
-- 
2.26.2

[Intel-gfx] [PATCH -fixes] drm/i915: Only enable DFP 4:4:4->4:2:0 conversion when outputting YCbCr 4:4:4

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Let's not enable the 4:4:4->4:2:0 conversion bit in the DFP unless we're
actually outputting YCbCr 4:4:4. It would appear some protocol
converters blindy consult this bit even when the source is outputting
RGB, resulting in a visual mess.

Cc: stable@vger.kernel.org
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/2914
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20210111164111.13302-1-ville.syrjala@linux.intel.com
Fixes: 181567aa9f0d ("drm/i915: Do YCbCr 444->420 conversion via DP protocol converters")
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
(cherry picked from commit 3170a21f7059c4660c469f59bf529f372a57da5f)
---
Unfortunately the crtc_state plumbing to
intel_dp_configure_protocol_converter() was part of the 
HDMI 2.1 PCON stuff, so couldn't just cherry-pick it alone.

 drivers/gpu/drm/i915/display/intel_ddi.c | 2 +-
 drivers/gpu/drm/i915/display/intel_dp.c  | 9 +++++----
 drivers/gpu/drm/i915/display/intel_dp.h  | 3 ++-
 3 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 92940a0c5ef8..d5ace48b1ace 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -3725,7 +3725,7 @@ static void hsw_ddi_pre_enable_dp(struct intel_atomic_state *state,
 	intel_ddi_init_dp_buf_reg(encoder, crtc_state);
 	if (!is_mst)
 		intel_dp_set_power(intel_dp, DP_SET_POWER_D0);
-	intel_dp_configure_protocol_converter(intel_dp);
+	intel_dp_configure_protocol_converter(intel_dp, crtc_state);
 	intel_dp_sink_set_decompression_state(intel_dp, crtc_state,
 					      true);
 	intel_dp_sink_set_fec_ready(intel_dp, crtc_state);
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 37f1a10fd021..09123e8625c4 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -4014,7 +4014,8 @@ static void intel_dp_enable_port(struct intel_dp *intel_dp,
 	intel_de_posting_read(dev_priv, intel_dp->output_reg);
 }
 
-void intel_dp_configure_protocol_converter(struct intel_dp *intel_dp)
+void intel_dp_configure_protocol_converter(struct intel_dp *intel_dp,
+					   const struct intel_crtc_state *crtc_state)
 {
 	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
 	u8 tmp;
@@ -4033,8 +4034,8 @@ void intel_dp_configure_protocol_converter(struct intel_dp *intel_dp)
 		drm_dbg_kms(&i915->drm, "Failed to set protocol converter HDMI mode to %s\n",
 			    enableddisabled(intel_dp->has_hdmi_sink));
 
-	tmp = intel_dp->dfp.ycbcr_444_to_420 ?
-		DP_CONVERSION_TO_YCBCR420_ENABLE : 0;
+	tmp = crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR444 &&
+		intel_dp->dfp.ycbcr_444_to_420 ? DP_CONVERSION_TO_YCBCR420_ENABLE : 0;
 
 	if (drm_dp_dpcd_writeb(&intel_dp->aux,
 			       DP_PROTOCOL_CONVERTER_CONTROL_1, tmp) != 1)
@@ -4088,7 +4089,7 @@ static void intel_enable_dp(struct intel_atomic_state *state,
 	}
 
 	intel_dp_set_power(intel_dp, DP_SET_POWER_D0);
-	intel_dp_configure_protocol_converter(intel_dp);
+	intel_dp_configure_protocol_converter(intel_dp, pipe_config);
 	intel_dp_start_link_train(intel_dp, pipe_config);
 	intel_dp_stop_link_train(intel_dp, pipe_config);
 
diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
index b871a09b6901..05f7ddf7a795 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.h
+++ b/drivers/gpu/drm/i915/display/intel_dp.h
@@ -51,7 +51,8 @@ int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
 int intel_dp_retrain_link(struct intel_encoder *encoder,
 			  struct drm_modeset_acquire_ctx *ctx);
 void intel_dp_set_power(struct intel_dp *intel_dp, u8 mode);
-void intel_dp_configure_protocol_converter(struct intel_dp *intel_dp);
+void intel_dp_configure_protocol_converter(struct intel_dp *intel_dp,
+					   const struct intel_crtc_state *crtc_state);
 void intel_dp_sink_set_decompression_state(struct intel_dp *intel_dp,
 					   const struct intel_crtc_state *crtc_state,
 					   bool enable);
-- 
2.26.2

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

Re: [PATCH -fixes] drm/i915: Only enable DFP 4:4:4->4:2:0 conversion when outputting YCbCr 4:4:4

[Jani Nikula]

On Mon, 18 Jan 2021, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Let's not enable the 4:4:4->4:2:0 conversion bit in the DFP unless we're
> actually outputting YCbCr 4:4:4. It would appear some protocol
> converters blindy consult this bit even when the source is outputting
> RGB, resulting in a visual mess.
>
> Cc: stable@vger.kernel.org
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/2914
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Link: https://patchwork.freedesktop.org/patch/msgid/20210111164111.13302-1-ville.syrjala@linux.intel.com
> Fixes: 181567aa9f0d ("drm/i915: Do YCbCr 444->420 conversion via DP protocol converters")
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> (cherry picked from commit 3170a21f7059c4660c469f59bf529f372a57da5f)
> ---
> Unfortunately the crtc_state plumbing to
> intel_dp_configure_protocol_converter() was part of the 
> HDMI 2.1 PCON stuff, so couldn't just cherry-pick it alone.

Thanks, pushed to fixes.

BR,
Jani.

>
>  drivers/gpu/drm/i915/display/intel_ddi.c | 2 +-
>  drivers/gpu/drm/i915/display/intel_dp.c  | 9 +++++----
>  drivers/gpu/drm/i915/display/intel_dp.h  | 3 ++-
>  3 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 92940a0c5ef8..d5ace48b1ace 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -3725,7 +3725,7 @@ static void hsw_ddi_pre_enable_dp(struct intel_atomic_state *state,
>  	intel_ddi_init_dp_buf_reg(encoder, crtc_state);
>  	if (!is_mst)
>  		intel_dp_set_power(intel_dp, DP_SET_POWER_D0);
> -	intel_dp_configure_protocol_converter(intel_dp);
> +	intel_dp_configure_protocol_converter(intel_dp, crtc_state);
>  	intel_dp_sink_set_decompression_state(intel_dp, crtc_state,
>  					      true);
>  	intel_dp_sink_set_fec_ready(intel_dp, crtc_state);
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 37f1a10fd021..09123e8625c4 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -4014,7 +4014,8 @@ static void intel_dp_enable_port(struct intel_dp *intel_dp,
>  	intel_de_posting_read(dev_priv, intel_dp->output_reg);
>  }
>  
> -void intel_dp_configure_protocol_converter(struct intel_dp *intel_dp)
> +void intel_dp_configure_protocol_converter(struct intel_dp *intel_dp,
> +					   const struct intel_crtc_state *crtc_state)
>  {
>  	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
>  	u8 tmp;
> @@ -4033,8 +4034,8 @@ void intel_dp_configure_protocol_converter(struct intel_dp *intel_dp)
>  		drm_dbg_kms(&i915->drm, "Failed to set protocol converter HDMI mode to %s\n",
>  			    enableddisabled(intel_dp->has_hdmi_sink));
>  
> -	tmp = intel_dp->dfp.ycbcr_444_to_420 ?
> -		DP_CONVERSION_TO_YCBCR420_ENABLE : 0;
> +	tmp = crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR444 &&
> +		intel_dp->dfp.ycbcr_444_to_420 ? DP_CONVERSION_TO_YCBCR420_ENABLE : 0;
>  
>  	if (drm_dp_dpcd_writeb(&intel_dp->aux,
>  			       DP_PROTOCOL_CONVERTER_CONTROL_1, tmp) != 1)
> @@ -4088,7 +4089,7 @@ static void intel_enable_dp(struct intel_atomic_state *state,
>  	}
>  
>  	intel_dp_set_power(intel_dp, DP_SET_POWER_D0);
> -	intel_dp_configure_protocol_converter(intel_dp);
> +	intel_dp_configure_protocol_converter(intel_dp, pipe_config);
>  	intel_dp_start_link_train(intel_dp, pipe_config);
>  	intel_dp_stop_link_train(intel_dp, pipe_config);
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
> index b871a09b6901..05f7ddf7a795 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.h
> +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> @@ -51,7 +51,8 @@ int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
>  int intel_dp_retrain_link(struct intel_encoder *encoder,
>  			  struct drm_modeset_acquire_ctx *ctx);
>  void intel_dp_set_power(struct intel_dp *intel_dp, u8 mode);
> -void intel_dp_configure_protocol_converter(struct intel_dp *intel_dp);
> +void intel_dp_configure_protocol_converter(struct intel_dp *intel_dp,
> +					   const struct intel_crtc_state *crtc_state);
>  void intel_dp_sink_set_decompression_state(struct intel_dp *intel_dp,
>  					   const struct intel_crtc_state *crtc_state,
>  					   bool enable);

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [Intel-gfx] [PATCH -fixes] drm/i915: Only enable DFP 4:4:4->4:2:0 conversion when outputting YCbCr 4:4:4

[Jani Nikula]

On Mon, 18 Jan 2021, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Let's not enable the 4:4:4->4:2:0 conversion bit in the DFP unless we're
> actually outputting YCbCr 4:4:4. It would appear some protocol
> converters blindy consult this bit even when the source is outputting
> RGB, resulting in a visual mess.
>
> Cc: stable@vger.kernel.org
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/2914
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Link: https://patchwork.freedesktop.org/patch/msgid/20210111164111.13302-1-ville.syrjala@linux.intel.com
> Fixes: 181567aa9f0d ("drm/i915: Do YCbCr 444->420 conversion via DP protocol converters")
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> (cherry picked from commit 3170a21f7059c4660c469f59bf529f372a57da5f)
> ---
> Unfortunately the crtc_state plumbing to
> intel_dp_configure_protocol_converter() was part of the 
> HDMI 2.1 PCON stuff, so couldn't just cherry-pick it alone.

Thanks, pushed to fixes.

BR,
Jani.

>
>  drivers/gpu/drm/i915/display/intel_ddi.c | 2 +-
>  drivers/gpu/drm/i915/display/intel_dp.c  | 9 +++++----
>  drivers/gpu/drm/i915/display/intel_dp.h  | 3 ++-
>  3 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 92940a0c5ef8..d5ace48b1ace 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -3725,7 +3725,7 @@ static void hsw_ddi_pre_enable_dp(struct intel_atomic_state *state,
>  	intel_ddi_init_dp_buf_reg(encoder, crtc_state);
>  	if (!is_mst)
>  		intel_dp_set_power(intel_dp, DP_SET_POWER_D0);
> -	intel_dp_configure_protocol_converter(intel_dp);
> +	intel_dp_configure_protocol_converter(intel_dp, crtc_state);
>  	intel_dp_sink_set_decompression_state(intel_dp, crtc_state,
>  					      true);
>  	intel_dp_sink_set_fec_ready(intel_dp, crtc_state);
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 37f1a10fd021..09123e8625c4 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -4014,7 +4014,8 @@ static void intel_dp_enable_port(struct intel_dp *intel_dp,
>  	intel_de_posting_read(dev_priv, intel_dp->output_reg);
>  }
>  
> -void intel_dp_configure_protocol_converter(struct intel_dp *intel_dp)
> +void intel_dp_configure_protocol_converter(struct intel_dp *intel_dp,
> +					   const struct intel_crtc_state *crtc_state)
>  {
>  	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
>  	u8 tmp;
> @@ -4033,8 +4034,8 @@ void intel_dp_configure_protocol_converter(struct intel_dp *intel_dp)
>  		drm_dbg_kms(&i915->drm, "Failed to set protocol converter HDMI mode to %s\n",
>  			    enableddisabled(intel_dp->has_hdmi_sink));
>  
> -	tmp = intel_dp->dfp.ycbcr_444_to_420 ?
> -		DP_CONVERSION_TO_YCBCR420_ENABLE : 0;
> +	tmp = crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR444 &&
> +		intel_dp->dfp.ycbcr_444_to_420 ? DP_CONVERSION_TO_YCBCR420_ENABLE : 0;
>  
>  	if (drm_dp_dpcd_writeb(&intel_dp->aux,
>  			       DP_PROTOCOL_CONVERTER_CONTROL_1, tmp) != 1)
> @@ -4088,7 +4089,7 @@ static void intel_enable_dp(struct intel_atomic_state *state,
>  	}
>  
>  	intel_dp_set_power(intel_dp, DP_SET_POWER_D0);
> -	intel_dp_configure_protocol_converter(intel_dp);
> +	intel_dp_configure_protocol_converter(intel_dp, pipe_config);
>  	intel_dp_start_link_train(intel_dp, pipe_config);
>  	intel_dp_stop_link_train(intel_dp, pipe_config);
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
> index b871a09b6901..05f7ddf7a795 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.h
> +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> @@ -51,7 +51,8 @@ int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
>  int intel_dp_retrain_link(struct intel_encoder *encoder,
>  			  struct drm_modeset_acquire_ctx *ctx);
>  void intel_dp_set_power(struct intel_dp *intel_dp, u8 mode);
> -void intel_dp_configure_protocol_converter(struct intel_dp *intel_dp);
> +void intel_dp_configure_protocol_converter(struct intel_dp *intel_dp,
> +					   const struct intel_crtc_state *crtc_state);
>  void intel_dp_sink_set_decompression_state(struct intel_dp *intel_dp,
>  					   const struct intel_crtc_state *crtc_state,
>  					   bool enable);

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

Re: [Intel-gfx] Fixes that failed to apply to v5.11-rc4

[Jani Nikula]

On Mon, 18 Jan 2021, Jani Nikula <jani.nikula@intel.com> wrote:
> The following commits have been marked as Cc: stable or fixing something
> in v5.11-rc4 or earlier, but failed to cherry-pick to
> drm-intel-fixes. Please see if they are worth backporting, and please do
> so if they are.
>
> Conflicts:
> dbe13ae1d6ab ("drm/i915/pmu: Don't grab wakeref when enabling events")
> 9bb36cf66091 ("drm/i915: Check for rq->hwsp validity after acquiring RCU lock")
> 5b4dc95cf7f5 ("drm/i915/gt: Prevent use of engine->wa_ctx after error")
> 6a3daee1b38e ("drm/i915/selftests: Fix some error codes")
> 67fba3f1c73b ("drm/i915/dp: Fix LTTPR vswing/pre-emp setting in non-transparent mode")
>
> Fails to build:
> 3170a21f7059 ("drm/i915: Only enable DFP 4:4:4->4:2:0 conversion when outputting YCbCr 4:4:4")
>
> BR,
> Jani.

Update.

Conflicts:
5b4dc95cf7f5 ("drm/i915/gt: Prevent use of engine->wa_ctx after error")
6a3daee1b38e ("drm/i915/selftests: Fix some error codes")
67fba3f1c73b ("drm/i915/dp: Fix LTTPR vswing/pre-emp setting in non-transparent mode")
699390f7f026 ("drm/i915: Fix the PHY compliance test vs. hotplug mishap")
e7004ea4f5f5 ("drm/i915/gt: Close race between enable_breadcrumbs and cancel_breadcrumbs")
fed387572040 ("drm/i915/display: Prevent double YUV range correction on HDR planes")

Fails to build:
0713eb979d2c ("drm/i915: Disable atomics in L3 for gen9")
f8abfda84841 ("drm/i915: Fix the MST PBN divider calculation")

BR,
Jani.

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

Re: [Intel-gfx] Fixes that failed to apply to v5.11-rc4

[Chris Wilson]

Quoting Jani Nikula (2021-02-02 07:15:18)
> On Mon, 18 Jan 2021, Jani Nikula <jani.nikula@intel.com> wrote:
> > The following commits have been marked as Cc: stable or fixing something
> > in v5.11-rc4 or earlier, but failed to cherry-pick to
> > drm-intel-fixes. Please see if they are worth backporting, and please do
> > so if they are.
> >
> > Conflicts:
> > dbe13ae1d6ab ("drm/i915/pmu: Don't grab wakeref when enabling events")
> > 9bb36cf66091 ("drm/i915: Check for rq->hwsp validity after acquiring RCU lock")
> > 5b4dc95cf7f5 ("drm/i915/gt: Prevent use of engine->wa_ctx after error")
> > 6a3daee1b38e ("drm/i915/selftests: Fix some error codes")
> > 67fba3f1c73b ("drm/i915/dp: Fix LTTPR vswing/pre-emp setting in non-transparent mode")
> >
> > Fails to build:
> > 3170a21f7059 ("drm/i915: Only enable DFP 4:4:4->4:2:0 conversion when outputting YCbCr 4:4:4")
> >
> > BR,
> > Jani.
> 
> Update.
> 
> Conflicts:
> 5b4dc95cf7f5 ("drm/i915/gt: Prevent use of engine->wa_ctx after error")

Already in 488751a0ef9b ("drm/i915/gt: Prevent use of engine->wa_ctx after error")

> 6a3daee1b38e ("drm/i915/selftests: Fix some error codes")

No user or even likely CI impact, not worth backporting [unless it turns
up later as a prerequisite].

> 67fba3f1c73b ("drm/i915/dp: Fix LTTPR vswing/pre-emp setting in non-transparent mode")
> 699390f7f026 ("drm/i915: Fix the PHY compliance test vs. hotplug mishap")
> e7004ea4f5f5 ("drm/i915/gt: Close race between enable_breadcrumbs and cancel_breadcrumbs")

Required at least one other friend.

There's another patch that we need in fixes for v5.10, so I'll include
that: drm/i915/gem: Drop lru bumping on display unpinning

I've put the 3 patches on fdo,
https://cgit.freedesktop.org/~ickle/linux-2.6/log/?h=dif

Hopefully they are a happy bunch.

p.s. 5.11-rc6 kills CI.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] Fixes that failed to apply to v5.11-rc4

[Jani Nikula]

On Tue, 02 Feb 2021, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Quoting Jani Nikula (2021-02-02 07:15:18)
>> On Mon, 18 Jan 2021, Jani Nikula <jani.nikula@intel.com> wrote:
>> > The following commits have been marked as Cc: stable or fixing something
>> > in v5.11-rc4 or earlier, but failed to cherry-pick to
>> > drm-intel-fixes. Please see if they are worth backporting, and please do
>> > so if they are.
>> >
>> > Conflicts:
>> > dbe13ae1d6ab ("drm/i915/pmu: Don't grab wakeref when enabling events")
>> > 9bb36cf66091 ("drm/i915: Check for rq->hwsp validity after acquiring RCU lock")
>> > 5b4dc95cf7f5 ("drm/i915/gt: Prevent use of engine->wa_ctx after error")
>> > 6a3daee1b38e ("drm/i915/selftests: Fix some error codes")
>> > 67fba3f1c73b ("drm/i915/dp: Fix LTTPR vswing/pre-emp setting in non-transparent mode")
>> >
>> > Fails to build:
>> > 3170a21f7059 ("drm/i915: Only enable DFP 4:4:4->4:2:0 conversion when outputting YCbCr 4:4:4")
>> >
>> > BR,
>> > Jani.
>> 
>> Update.
>> 
>> Conflicts:
>> 5b4dc95cf7f5 ("drm/i915/gt: Prevent use of engine->wa_ctx after error")
>
> Already in 488751a0ef9b ("drm/i915/gt: Prevent use of engine->wa_ctx after error")
>
>> 6a3daee1b38e ("drm/i915/selftests: Fix some error codes")
>
> No user or even likely CI impact, not worth backporting [unless it turns
> up later as a prerequisite].
>
>> 67fba3f1c73b ("drm/i915/dp: Fix LTTPR vswing/pre-emp setting in non-transparent mode")
>> 699390f7f026 ("drm/i915: Fix the PHY compliance test vs. hotplug mishap")
>> e7004ea4f5f5 ("drm/i915/gt: Close race between enable_breadcrumbs and cancel_breadcrumbs")
>
> Required at least one other friend.
>
> There's another patch that we need in fixes for v5.10, so I'll include
> that: drm/i915/gem: Drop lru bumping on display unpinning
>
> I've put the 3 patches on fdo,
> https://cgit.freedesktop.org/~ickle/linux-2.6/log/?h=dif
>
> Hopefully they are a happy bunch.

Thanks, picked the 3 up.

> p.s. 5.11-rc6 kills CI.

Yeah, looks like a fix is in the works.

BR,
Jani.


[1] http://lore.kernel.org/r/8735yfd2q4.fsf@nanos.tec.linutronix.de

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

[Intel-gfx] [PATCH -fixes] drm/i915/display: Prevent double YUV range correction on HDR planes

[Ville Syrjala]

From: Andres Calderon Jaramillo <andrescj@chromium.org>

Prevent the ICL HDR plane pipeline from performing YUV color range
correction twice when the input is in limited range. This is done by
removing the limited-range code from icl_program_input_csc().

Before this patch the following could happen: user space gives us a YUV
buffer in limited range; per the pipeline in [1], the plane would first
go through a "YUV Range correct" stage that expands the range; the plane
would then go through the "Input CSC" stage which would also expand the
range because icl_program_input_csc() would use a matrix and an offset
that assume limited-range input; this would ultimately cause dark and
light colors to appear darker and lighter than they should respectively.

This is an issue because if a buffer switches between being scanned out
and being composited with the GPU, the user will see a color difference.
If this switching happens quickly and frequently, the user will perceive
this as a flickering.

[1] https://01.org/sites/default/files/documentation/intel-gfx-prm-osrc-icllp-vol12-displayengine_0.pdf#page=281

Cc: stable@vger.kernel.org
Signed-off-by: Andres Calderon Jaramillo <andrescj@chromium.org>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20201215224219.3896256-1-andrescj@google.com
(cherry picked from commit fed387572040e84ead53852a7820e30a30e515d0)
---
 drivers/gpu/drm/i915/display/intel_display.c |  2 +
 drivers/gpu/drm/i915/display/intel_sprite.c  | 65 +++-----------------
 2 files changed, 12 insertions(+), 55 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 53a00cf3fa32..39396248f388 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -4807,6 +4807,8 @@ u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state,
 			plane_color_ctl |= PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE;
 	} else if (fb->format->is_yuv) {
 		plane_color_ctl |= PLANE_COLOR_INPUT_CSC_ENABLE;
+		if (plane_state->hw.color_range == DRM_COLOR_YCBCR_FULL_RANGE)
+			plane_color_ctl |= PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE;
 	}
 
 	return plane_color_ctl;
diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c b/drivers/gpu/drm/i915/display/intel_sprite.c
index 019a2d6d807a..3da2544fa1c0 100644
--- a/drivers/gpu/drm/i915/display/intel_sprite.c
+++ b/drivers/gpu/drm/i915/display/intel_sprite.c
@@ -618,13 +618,19 @@ skl_program_scaler(struct intel_plane *plane,
 
 /* Preoffset values for YUV to RGB Conversion */
 #define PREOFF_YUV_TO_RGB_HI		0x1800
-#define PREOFF_YUV_TO_RGB_ME		0x1F00
+#define PREOFF_YUV_TO_RGB_ME		0x0000
 #define PREOFF_YUV_TO_RGB_LO		0x1800
 
 #define  ROFF(x)          (((x) & 0xffff) << 16)
 #define  GOFF(x)          (((x) & 0xffff) << 0)
 #define  BOFF(x)          (((x) & 0xffff) << 16)
 
+/*
+ * Programs the input color space conversion stage for ICL HDR planes.
+ * Note that it is assumed that this stage always happens after YUV
+ * range correction. Thus, the input to this stage is assumed to be
+ * in full-range YCbCr.
+ */
 static void
 icl_program_input_csc(struct intel_plane *plane,
 		      const struct intel_crtc_state *crtc_state,
@@ -672,52 +678,7 @@ icl_program_input_csc(struct intel_plane *plane,
 			0x0, 0x7800, 0x7F10,
 		},
 	};
-
-	/* Matrix for Limited Range to Full Range Conversion */
-	static const u16 input_csc_matrix_lr[][9] = {
-		/*
-		 * BT.601 Limted range YCbCr -> full range RGB
-		 * The matrix required is :
-		 * [1.164384, 0.000, 1.596027,
-		 *  1.164384, -0.39175, -0.812813,
-		 *  1.164384, 2.017232, 0.0000]
-		 */
-		[DRM_COLOR_YCBCR_BT601] = {
-			0x7CC8, 0x7950, 0x0,
-			0x8D00, 0x7950, 0x9C88,
-			0x0, 0x7950, 0x6810,
-		},
-		/*
-		 * BT.709 Limited range YCbCr -> full range RGB
-		 * The matrix required is :
-		 * [1.164384, 0.000, 1.792741,
-		 *  1.164384, -0.213249, -0.532909,
-		 *  1.164384, 2.112402, 0.0000]
-		 */
-		[DRM_COLOR_YCBCR_BT709] = {
-			0x7E58, 0x7950, 0x0,
-			0x8888, 0x7950, 0xADA8,
-			0x0, 0x7950,  0x6870,
-		},
-		/*
-		 * BT.2020 Limited range YCbCr -> full range RGB
-		 * The matrix required is :
-		 * [1.164, 0.000, 1.678,
-		 *  1.164, -0.1873, -0.6504,
-		 *  1.164, 2.1417, 0.0000]
-		 */
-		[DRM_COLOR_YCBCR_BT2020] = {
-			0x7D70, 0x7950, 0x0,
-			0x8A68, 0x7950, 0xAC00,
-			0x0, 0x7950, 0x6890,
-		},
-	};
-	const u16 *csc;
-
-	if (plane_state->hw.color_range == DRM_COLOR_YCBCR_FULL_RANGE)
-		csc = input_csc_matrix[plane_state->hw.color_encoding];
-	else
-		csc = input_csc_matrix_lr[plane_state->hw.color_encoding];
+	const u16 *csc = input_csc_matrix[plane_state->hw.color_encoding];
 
 	intel_de_write_fw(dev_priv, PLANE_INPUT_CSC_COEFF(pipe, plane_id, 0),
 			  ROFF(csc[0]) | GOFF(csc[1]));
@@ -734,14 +695,8 @@ icl_program_input_csc(struct intel_plane *plane,
 
 	intel_de_write_fw(dev_priv, PLANE_INPUT_CSC_PREOFF(pipe, plane_id, 0),
 			  PREOFF_YUV_TO_RGB_HI);
-	if (plane_state->hw.color_range == DRM_COLOR_YCBCR_FULL_RANGE)
-		intel_de_write_fw(dev_priv,
-				  PLANE_INPUT_CSC_PREOFF(pipe, plane_id, 1),
-				  0);
-	else
-		intel_de_write_fw(dev_priv,
-				  PLANE_INPUT_CSC_PREOFF(pipe, plane_id, 1),
-				  PREOFF_YUV_TO_RGB_ME);
+	intel_de_write_fw(dev_priv, PLANE_INPUT_CSC_PREOFF(pipe, plane_id, 1),
+			  PREOFF_YUV_TO_RGB_ME);
 	intel_de_write_fw(dev_priv, PLANE_INPUT_CSC_PREOFF(pipe, plane_id, 2),
 			  PREOFF_YUV_TO_RGB_LO);
 	intel_de_write_fw(dev_priv,
-- 
2.26.2

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

Re: [Intel-gfx] [PATCH -fixes] drm/i915/display: Prevent double YUV range correction on HDR planes

[Jani Nikula]

On Tue, 02 Feb 2021, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Andres Calderon Jaramillo <andrescj@chromium.org>
>
> Prevent the ICL HDR plane pipeline from performing YUV color range
> correction twice when the input is in limited range. This is done by
> removing the limited-range code from icl_program_input_csc().
>
> Before this patch the following could happen: user space gives us a YUV
> buffer in limited range; per the pipeline in [1], the plane would first
> go through a "YUV Range correct" stage that expands the range; the plane
> would then go through the "Input CSC" stage which would also expand the
> range because icl_program_input_csc() would use a matrix and an offset
> that assume limited-range input; this would ultimately cause dark and
> light colors to appear darker and lighter than they should respectively.
>
> This is an issue because if a buffer switches between being scanned out
> and being composited with the GPU, the user will see a color difference.
> If this switching happens quickly and frequently, the user will perceive
> this as a flickering.
>
> [1] https://01.org/sites/default/files/documentation/intel-gfx-prm-osrc-icllp-vol12-displayengine_0.pdf#page=281
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Andres Calderon Jaramillo <andrescj@chromium.org>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Link: https://patchwork.freedesktop.org/patch/msgid/20201215224219.3896256-1-andrescj@google.com
> (cherry picked from commit fed387572040e84ead53852a7820e30a30e515d0)

Thanks, pushed to drm-intel-fixes.

BR,
Jani.

> ---
>  drivers/gpu/drm/i915/display/intel_display.c |  2 +
>  drivers/gpu/drm/i915/display/intel_sprite.c  | 65 +++-----------------
>  2 files changed, 12 insertions(+), 55 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 53a00cf3fa32..39396248f388 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -4807,6 +4807,8 @@ u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state,
>  			plane_color_ctl |= PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE;
>  	} else if (fb->format->is_yuv) {
>  		plane_color_ctl |= PLANE_COLOR_INPUT_CSC_ENABLE;
> +		if (plane_state->hw.color_range == DRM_COLOR_YCBCR_FULL_RANGE)
> +			plane_color_ctl |= PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE;
>  	}
>  
>  	return plane_color_ctl;
> diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c b/drivers/gpu/drm/i915/display/intel_sprite.c
> index 019a2d6d807a..3da2544fa1c0 100644
> --- a/drivers/gpu/drm/i915/display/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/display/intel_sprite.c
> @@ -618,13 +618,19 @@ skl_program_scaler(struct intel_plane *plane,
>  
>  /* Preoffset values for YUV to RGB Conversion */
>  #define PREOFF_YUV_TO_RGB_HI		0x1800
> -#define PREOFF_YUV_TO_RGB_ME		0x1F00
> +#define PREOFF_YUV_TO_RGB_ME		0x0000
>  #define PREOFF_YUV_TO_RGB_LO		0x1800
>  
>  #define  ROFF(x)          (((x) & 0xffff) << 16)
>  #define  GOFF(x)          (((x) & 0xffff) << 0)
>  #define  BOFF(x)          (((x) & 0xffff) << 16)
>  
> +/*
> + * Programs the input color space conversion stage for ICL HDR planes.
> + * Note that it is assumed that this stage always happens after YUV
> + * range correction. Thus, the input to this stage is assumed to be
> + * in full-range YCbCr.
> + */
>  static void
>  icl_program_input_csc(struct intel_plane *plane,
>  		      const struct intel_crtc_state *crtc_state,
> @@ -672,52 +678,7 @@ icl_program_input_csc(struct intel_plane *plane,
>  			0x0, 0x7800, 0x7F10,
>  		},
>  	};
> -
> -	/* Matrix for Limited Range to Full Range Conversion */
> -	static const u16 input_csc_matrix_lr[][9] = {
> -		/*
> -		 * BT.601 Limted range YCbCr -> full range RGB
> -		 * The matrix required is :
> -		 * [1.164384, 0.000, 1.596027,
> -		 *  1.164384, -0.39175, -0.812813,
> -		 *  1.164384, 2.017232, 0.0000]
> -		 */
> -		[DRM_COLOR_YCBCR_BT601] = {
> -			0x7CC8, 0x7950, 0x0,
> -			0x8D00, 0x7950, 0x9C88,
> -			0x0, 0x7950, 0x6810,
> -		},
> -		/*
> -		 * BT.709 Limited range YCbCr -> full range RGB
> -		 * The matrix required is :
> -		 * [1.164384, 0.000, 1.792741,
> -		 *  1.164384, -0.213249, -0.532909,
> -		 *  1.164384, 2.112402, 0.0000]
> -		 */
> -		[DRM_COLOR_YCBCR_BT709] = {
> -			0x7E58, 0x7950, 0x0,
> -			0x8888, 0x7950, 0xADA8,
> -			0x0, 0x7950,  0x6870,
> -		},
> -		/*
> -		 * BT.2020 Limited range YCbCr -> full range RGB
> -		 * The matrix required is :
> -		 * [1.164, 0.000, 1.678,
> -		 *  1.164, -0.1873, -0.6504,
> -		 *  1.164, 2.1417, 0.0000]
> -		 */
> -		[DRM_COLOR_YCBCR_BT2020] = {
> -			0x7D70, 0x7950, 0x0,
> -			0x8A68, 0x7950, 0xAC00,
> -			0x0, 0x7950, 0x6890,
> -		},
> -	};
> -	const u16 *csc;
> -
> -	if (plane_state->hw.color_range == DRM_COLOR_YCBCR_FULL_RANGE)
> -		csc = input_csc_matrix[plane_state->hw.color_encoding];
> -	else
> -		csc = input_csc_matrix_lr[plane_state->hw.color_encoding];
> +	const u16 *csc = input_csc_matrix[plane_state->hw.color_encoding];
>  
>  	intel_de_write_fw(dev_priv, PLANE_INPUT_CSC_COEFF(pipe, plane_id, 0),
>  			  ROFF(csc[0]) | GOFF(csc[1]));
> @@ -734,14 +695,8 @@ icl_program_input_csc(struct intel_plane *plane,
>  
>  	intel_de_write_fw(dev_priv, PLANE_INPUT_CSC_PREOFF(pipe, plane_id, 0),
>  			  PREOFF_YUV_TO_RGB_HI);
> -	if (plane_state->hw.color_range == DRM_COLOR_YCBCR_FULL_RANGE)
> -		intel_de_write_fw(dev_priv,
> -				  PLANE_INPUT_CSC_PREOFF(pipe, plane_id, 1),
> -				  0);
> -	else
> -		intel_de_write_fw(dev_priv,
> -				  PLANE_INPUT_CSC_PREOFF(pipe, plane_id, 1),
> -				  PREOFF_YUV_TO_RGB_ME);
> +	intel_de_write_fw(dev_priv, PLANE_INPUT_CSC_PREOFF(pipe, plane_id, 1),
> +			  PREOFF_YUV_TO_RGB_ME);
>  	intel_de_write_fw(dev_priv, PLANE_INPUT_CSC_PREOFF(pipe, plane_id, 2),
>  			  PREOFF_YUV_TO_RGB_LO);
>  	intel_de_write_fw(dev_priv,

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

[Intel-gfx] ✗ Fi.CI.BUILD: failure for drm/i915/display: Prevent double YUV range correction on HDR planes (rev3)

[Patchwork]

== Series Details ==

Series: drm/i915/display: Prevent double YUV range correction on HDR planes (rev3)
URL   : https://patchwork.freedesktop.org/series/84966/
State : failure

== Summary ==

Applying: drm/i915/display: Prevent double YUV range correction on HDR planes
Using index info to reconstruct a base tree...
M	drivers/gpu/drm/i915/display/intel_display.c
M	drivers/gpu/drm/i915/display/intel_sprite.c
Falling back to patching base and 3-way merge...
Auto-merging drivers/gpu/drm/i915/display/intel_sprite.c
Auto-merging drivers/gpu/drm/i915/display/intel_display.c
CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/display/intel_display.c
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 drm/i915/display: Prevent double YUV range correction on HDR planes
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".


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

Re: [Intel-gfx] Fixes that failed to apply to v5.11-rc4

[Imre Deak]

Hi,

On Tue, Feb 02, 2021 at 09:15:18AM +0200, Jani Nikula wrote:
> On Mon, 18 Jan 2021, Jani Nikula <jani.nikula@intel.com> wrote:
> > The following commits have been marked as Cc: stable or fixing something
> > in v5.11-rc4 or earlier, but failed to cherry-pick to
> > drm-intel-fixes. Please see if they are worth backporting, and please do
> > so if they are.
> >
> > Conflicts:
> > dbe13ae1d6ab ("drm/i915/pmu: Don't grab wakeref when enabling events")
> > 9bb36cf66091 ("drm/i915: Check for rq->hwsp validity after acquiring RCU lock")
> > 5b4dc95cf7f5 ("drm/i915/gt: Prevent use of engine->wa_ctx after error")
> > 6a3daee1b38e ("drm/i915/selftests: Fix some error codes")
> > 67fba3f1c73b ("drm/i915/dp: Fix LTTPR vswing/pre-emp setting in non-transparent mode")
> >
> > Fails to build:
> > 3170a21f7059 ("drm/i915: Only enable DFP 4:4:4->4:2:0 conversion when outputting YCbCr 4:4:4")
> >
> > BR,
> > Jani.
> 
> Update.
> 
> Conflicts:
> 5b4dc95cf7f5 ("drm/i915/gt: Prevent use of engine->wa_ctx after error")
> 6a3daee1b38e ("drm/i915/selftests: Fix some error codes")
> 67fba3f1c73b ("drm/i915/dp: Fix LTTPR vswing/pre-emp setting in non-transparent mode")

This depends on
  1c6e527d6947 ("rm/i915/dp: Move intel_dp_set_signal_levels() to intel_dp_link_training.c")

> 699390f7f026 ("drm/i915: Fix the PHY compliance test vs. hotplug mishap")
> e7004ea4f5f5 ("drm/i915/gt: Close race between enable_breadcrumbs and cancel_breadcrumbs")
> fed387572040 ("drm/i915/display: Prevent double YUV range correction on HDR planes")
> 
> Fails to build:
> 0713eb979d2c ("drm/i915: Disable atomics in L3 for gen9")
> f8abfda84841 ("drm/i915: Fix the MST PBN divider calculation")

and this one depends on
  a321fc2b4e60 ("rm/dp/mst: Export drm_dp_get_vc_payload_bw()")

> 
> BR,
> Jani.
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] Fixes that failed to apply to v5.11-rc4

[Jani Nikula]

On Tue, 02 Feb 2021, Imre Deak <imre.deak@intel.com> wrote:
> Hi,
>
> On Tue, Feb 02, 2021 at 09:15:18AM +0200, Jani Nikula wrote:
>> On Mon, 18 Jan 2021, Jani Nikula <jani.nikula@intel.com> wrote:
>> > The following commits have been marked as Cc: stable or fixing something
>> > in v5.11-rc4 or earlier, but failed to cherry-pick to
>> > drm-intel-fixes. Please see if they are worth backporting, and please do
>> > so if they are.
>> >
>> > Conflicts:
>> > dbe13ae1d6ab ("drm/i915/pmu: Don't grab wakeref when enabling events")
>> > 9bb36cf66091 ("drm/i915: Check for rq->hwsp validity after acquiring RCU lock")
>> > 5b4dc95cf7f5 ("drm/i915/gt: Prevent use of engine->wa_ctx after error")
>> > 6a3daee1b38e ("drm/i915/selftests: Fix some error codes")
>> > 67fba3f1c73b ("drm/i915/dp: Fix LTTPR vswing/pre-emp setting in non-transparent mode")
>> >
>> > Fails to build:
>> > 3170a21f7059 ("drm/i915: Only enable DFP 4:4:4->4:2:0 conversion when outputting YCbCr 4:4:4")
>> >
>> > BR,
>> > Jani.
>> 
>> Update.
>> 
>> Conflicts:
>> 5b4dc95cf7f5 ("drm/i915/gt: Prevent use of engine->wa_ctx after error")
>> 6a3daee1b38e ("drm/i915/selftests: Fix some error codes")
>> 67fba3f1c73b ("drm/i915/dp: Fix LTTPR vswing/pre-emp setting in non-transparent mode")
>
> This depends on
>   1c6e527d6947 ("rm/i915/dp: Move intel_dp_set_signal_levels() to intel_dp_link_training.c")
>
>> 699390f7f026 ("drm/i915: Fix the PHY compliance test vs. hotplug mishap")
>> e7004ea4f5f5 ("drm/i915/gt: Close race between enable_breadcrumbs and cancel_breadcrumbs")
>> fed387572040 ("drm/i915/display: Prevent double YUV range correction on HDR planes")
>> 
>> Fails to build:
>> 0713eb979d2c ("drm/i915: Disable atomics in L3 for gen9")
>> f8abfda84841 ("drm/i915: Fix the MST PBN divider calculation")
>
> and this one depends on
>   a321fc2b4e60 ("rm/dp/mst: Export drm_dp_get_vc_payload_bw()")

Thanks, picked the 2+2 commits up.

BR,
Jani.


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