Fixes that failed to backport to v4.12-rc1

Fixes that failed to backport to v4.12-rc1

[Jani Nikula]

Continuing [1] for v4.12-rc1

The following commits have been marked as Cc: stable or fixing something
in v4.12-rc1 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.

BR,
Jani.

e6ba9992de6c ("drm/i915: Differentiate between sw write location into ring and last hw read")
73cc0b9aa9af ("drm/i915: Do not sync RCU during shrinking")



[1] http://mid.mail-archive.com/87lgs9dugv.fsf@intel.com

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

Re: Fixes that failed to backport to v4.12-rc1

[Jani Nikula]

On Mon, 15 May 2017, Jani Nikula <jani.nikula@intel.com> wrote:
> The following commits have been marked as Cc: stable or fixing something
> in v4.12-rc1 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.

Update:

d96a7d2adb04 ("drm/i915: Fix scaling check for 90/270 degree plane rotation")
fce5adf568ab ("drm/i915: Fix SKL+ watermarks for 90/270 rotation")

I am about to pick up 73714c05df97 ("drm/i915: Fix 90/270 rotated
coordinates for FBC") which does apply cleanly - does it make sense
without the other two?

BR,
Jani.


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

Re: Fixes that failed to backport to v4.12-rc1

[Ville Syrjälä]

On Wed, Jun 07, 2017 at 04:45:07PM +0300, Jani Nikula wrote:
> On Mon, 15 May 2017, Jani Nikula <jani.nikula@intel.com> wrote:
> > The following commits have been marked as Cc: stable or fixing something
> > in v4.12-rc1 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.
> 
> Update:
> 
> d96a7d2adb04 ("drm/i915: Fix scaling check for 90/270 degree plane rotation")
> fce5adf568ab ("drm/i915: Fix SKL+ watermarks for 90/270 rotation")
> 
> I am about to pick up 73714c05df97 ("drm/i915: Fix 90/270 rotated
> coordinates for FBC") which does apply cleanly - does it make sense
> without the other two?

Probably not. Shouldn't be too hard for me to fix up the other two
since I already resolved the same conflicts in the other direction
when applying the patches ;)

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH fixes 1/2] drm/i915: Fix scaling check for 90/270 degree plane rotation

[ville.syrjala]

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

Starting from commit b63a16f6cd89 ("drm/i915: Compute display surface
offset in the plane check hook for SKL+") we've already rotated the src
coordinates by 270 degrees by the time we check if a scaler is needed
or not, so we must not account for the rotation a second time.
Previously we did these steps in the opposite order and hence the
scaler check had to deal with rotation itself. The double rotation
handling causes us to enable a scaler pretty much every time 90/270
degree plane rotation is requested, leading to fuzzier fonts and whatnot.

v2: s/unsigned/unsigned int/ to appease checkpatch
v3: s/DRM_ROTATE_0/DRM_MODE_ROTATE_0/

Cc: stable@vger.kernel.org
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reported-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Tested-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Fixes: b63a16f6cd89 ("drm/i915: Compute display surface offset in the plane check hook for SKL+")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/20170331180056.14086-2-ville.syrjala@linux.intel.com
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
(cherry picked from commit d96a7d2adb040a67e163a82dad6316f9f572498a)
---
 drivers/gpu/drm/i915/intel_display.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3cabe52a4e3b..c3785d404ba5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4598,7 +4598,7 @@ static void cpt_verify_modeset(struct drm_device *dev, int pipe)
 
 static int
 skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach,
-		  unsigned scaler_user, int *scaler_id, unsigned int rotation,
+		  unsigned int scaler_user, int *scaler_id,
 		  int src_w, int src_h, int dst_w, int dst_h)
 {
 	struct intel_crtc_scaler_state *scaler_state =
@@ -4607,9 +4607,12 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach,
 		to_intel_crtc(crtc_state->base.crtc);
 	int need_scaling;
 
-	need_scaling = drm_rotation_90_or_270(rotation) ?
-		(src_h != dst_w || src_w != dst_h):
-		(src_w != dst_w || src_h != dst_h);
+	/*
+	 * Src coordinates are already rotated by 270 degrees for
+	 * the 90/270 degree plane rotation cases (to match the
+	 * GTT mapping), hence no need to account for rotation here.
+	 */
+	need_scaling = src_w != dst_w || src_h != dst_h;
 
 	/*
 	 * if plane is being disabled or scaler is no more required or force detach
@@ -4671,7 +4674,7 @@ int skl_update_scaler_crtc(struct intel_crtc_state *state)
 	const struct drm_display_mode *adjusted_mode = &state->base.adjusted_mode;
 
 	return skl_update_scaler(state, !state->base.active, SKL_CRTC_INDEX,
-		&state->scaler_state.scaler_id, DRM_ROTATE_0,
+		&state->scaler_state.scaler_id,
 		state->pipe_src_w, state->pipe_src_h,
 		adjusted_mode->crtc_hdisplay, adjusted_mode->crtc_vdisplay);
 }
@@ -4700,7 +4703,6 @@ static int skl_update_scaler_plane(struct intel_crtc_state *crtc_state,
 	ret = skl_update_scaler(crtc_state, force_detach,
 				drm_plane_index(&intel_plane->base),
 				&plane_state->scaler_id,
-				plane_state->base.rotation,
 				drm_rect_width(&plane_state->base.src) >> 16,
 				drm_rect_height(&plane_state->base.src) >> 16,
 				drm_rect_width(&plane_state->base.dst),
-- 
2.13.0

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

[PATCH fixes 2/2] drm/i915: Fix SKL+ watermarks for 90/270 rotation

[ville.syrjala]

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

skl_check_plane_surface() already rotates the clipped plane source
coordinates to match the scanout direction because that's the way
the GTT mapping is set up. Thus we no longer need to rotate the
coordinates in the watermark code.

For cursors we use the non-clipped coordinates which are not rotated
appropriately, but that doesn't actually matter since cursors don't
even support 90/270 degree rotation.

v2: Resolve conflicts from SKL+ wm rework

Cc: stable@vger.kernel.org
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Fixes: b63a16f6cd89 ("drm/i915: Compute display surface offset in the plane check hook for SKL+")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/20170331180056.14086-3-ville.syrjala@linux.intel.com
Tested-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
(cherry picked from commit fce5adf568abb1e8264d677156e2e0deb529194d)
---
 drivers/gpu/drm/i915/intel_pm.c | 36 ++++++++++++++++++++++++------------
 1 file changed, 24 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 570bd603f401..6b1caf9ed3ca 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3373,20 +3373,26 @@ skl_plane_downscale_amount(const struct intel_crtc_state *cstate,
 
 	/* n.b., src is 16.16 fixed point, dst is whole integer */
 	if (plane->id == PLANE_CURSOR) {
+		/*
+		 * Cursors only support 0/180 degree rotation,
+		 * hence no need to account for rotation here.
+		 */
 		src_w = pstate->base.src_w;
 		src_h = pstate->base.src_h;
 		dst_w = pstate->base.crtc_w;
 		dst_h = pstate->base.crtc_h;
 	} else {
+		/*
+		 * Src coordinates are already rotated by 270 degrees for
+		 * the 90/270 degree plane rotation cases (to match the
+		 * GTT mapping), hence no need to account for rotation here.
+		 */
 		src_w = drm_rect_width(&pstate->base.src);
 		src_h = drm_rect_height(&pstate->base.src);
 		dst_w = drm_rect_width(&pstate->base.dst);
 		dst_h = drm_rect_height(&pstate->base.dst);
 	}
 
-	if (drm_rotation_90_or_270(pstate->base.rotation))
-		swap(dst_w, dst_h);
-
 	downscale_h = max(src_h / dst_h, (uint32_t)DRM_PLANE_HELPER_NO_SCALING);
 	downscale_w = max(src_w / dst_w, (uint32_t)DRM_PLANE_HELPER_NO_SCALING);
 
@@ -3417,12 +3423,14 @@ skl_plane_relative_data_rate(const struct intel_crtc_state *cstate,
 	if (y && format != DRM_FORMAT_NV12)
 		return 0;
 
+	/*
+	 * Src coordinates are already rotated by 270 degrees for
+	 * the 90/270 degree plane rotation cases (to match the
+	 * GTT mapping), hence no need to account for rotation here.
+	 */
 	width = drm_rect_width(&intel_pstate->base.src) >> 16;
 	height = drm_rect_height(&intel_pstate->base.src) >> 16;
 
-	if (drm_rotation_90_or_270(pstate->rotation))
-		swap(width, height);
-
 	/* for planar format */
 	if (format == DRM_FORMAT_NV12) {
 		if (y)  /* y-plane data rate */
@@ -3505,12 +3513,14 @@ skl_ddb_min_alloc(const struct drm_plane_state *pstate,
 	    fb->modifier != I915_FORMAT_MOD_Yf_TILED)
 		return 8;
 
+	/*
+	 * Src coordinates are already rotated by 270 degrees for
+	 * the 90/270 degree plane rotation cases (to match the
+	 * GTT mapping), hence no need to account for rotation here.
+	 */
 	src_w = drm_rect_width(&intel_pstate->base.src) >> 16;
 	src_h = drm_rect_height(&intel_pstate->base.src) >> 16;
 
-	if (drm_rotation_90_or_270(pstate->rotation))
-		swap(src_w, src_h);
-
 	/* Halve UV plane width and height for NV12 */
 	if (fb->format->format == DRM_FORMAT_NV12 && !y) {
 		src_w /= 2;
@@ -3794,13 +3804,15 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
 		width = intel_pstate->base.crtc_w;
 		height = intel_pstate->base.crtc_h;
 	} else {
+		/*
+		 * Src coordinates are already rotated by 270 degrees for
+		 * the 90/270 degree plane rotation cases (to match the
+		 * GTT mapping), hence no need to account for rotation here.
+		 */
 		width = drm_rect_width(&intel_pstate->base.src) >> 16;
 		height = drm_rect_height(&intel_pstate->base.src) >> 16;
 	}
 
-	if (drm_rotation_90_or_270(pstate->rotation))
-		swap(width, height);
-
 	cpp = fb->format->cpp[0];
 	plane_pixel_rate = skl_adjusted_plane_pixel_rate(cstate, intel_pstate);
 
-- 
2.13.0

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

Re: Fixes that failed to backport to v4.12-rc1

[Jani Nikula]

On Thu, 08 Jun 2017, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Wed, Jun 07, 2017 at 04:45:07PM +0300, Jani Nikula wrote:
>> On Mon, 15 May 2017, Jani Nikula <jani.nikula@intel.com> wrote:
>> > The following commits have been marked as Cc: stable or fixing something
>> > in v4.12-rc1 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.
>> 
>> Update:
>> 
>> d96a7d2adb04 ("drm/i915: Fix scaling check for 90/270 degree plane rotation")
>> fce5adf568ab ("drm/i915: Fix SKL+ watermarks for 90/270 rotation")
>> 
>> I am about to pick up 73714c05df97 ("drm/i915: Fix 90/270 rotated
>> coordinates for FBC") which does apply cleanly - does it make sense
>> without the other two?
>
> Probably not. Shouldn't be too hard for me to fix up the other two
> since I already resolved the same conflicts in the other direction
> when applying the patches ;)

Whoops, I already sent the pull request without the two. Anything bad in
me queuing the backports you provided in the next -rc? Or should I
recall the pull request to include these two at the same time?

BR,
Jani.

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

Re: Fixes that failed to backport to v4.12-rc1

[Ville Syrjälä]

On Thu, Jun 08, 2017 at 05:47:23PM +0300, Jani Nikula wrote:
> On Thu, 08 Jun 2017, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > On Wed, Jun 07, 2017 at 04:45:07PM +0300, Jani Nikula wrote:
> >> On Mon, 15 May 2017, Jani Nikula <jani.nikula@intel.com> wrote:
> >> > The following commits have been marked as Cc: stable or fixing something
> >> > in v4.12-rc1 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.
> >> 
> >> Update:
> >> 
> >> d96a7d2adb04 ("drm/i915: Fix scaling check for 90/270 degree plane rotation")
> >> fce5adf568ab ("drm/i915: Fix SKL+ watermarks for 90/270 rotation")
> >> 
> >> I am about to pick up 73714c05df97 ("drm/i915: Fix 90/270 rotated
> >> coordinates for FBC") which does apply cleanly - does it make sense
> >> without the other two?
> >
> > Probably not. Shouldn't be too hard for me to fix up the other two
> > since I already resolved the same conflicts in the other direction
> > when applying the patches ;)
> 
> Whoops, I already sent the pull request without the two. Anything bad in
> me queuing the backports you provided in the next -rc? Or should I
> recall the pull request to include these two at the same time?

Next -rc is fine. No need to rush.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: Fixes that failed to backport to v4.12-rc1

[Jani Nikula]

On Mon, 15 May 2017, Jani Nikula <jani.nikula@intel.com> wrote:
> Continuing [1] for v4.12-rc1
>
> The following commits have been marked as Cc: stable or fixing something
> in v4.12-rc1 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.

Update:

c4a8952612f6 ("drm/i915/guc: Clear enable_guc_loading in case of init failure")

BR,
Jani.


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

Re: Fixes that failed to backport to v4.12-rc1

[Jani Nikula]

On Thu, 08 Jun 2017, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Thu, Jun 08, 2017 at 05:47:23PM +0300, Jani Nikula wrote:
>> On Thu, 08 Jun 2017, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
>> > On Wed, Jun 07, 2017 at 04:45:07PM +0300, Jani Nikula wrote:
>> >> On Mon, 15 May 2017, Jani Nikula <jani.nikula@intel.com> wrote:
>> >> > The following commits have been marked as Cc: stable or fixing something
>> >> > in v4.12-rc1 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.
>> >> 
>> >> Update:
>> >> 
>> >> d96a7d2adb04 ("drm/i915: Fix scaling check for 90/270 degree plane rotation")
>> >> fce5adf568ab ("drm/i915: Fix SKL+ watermarks for 90/270 rotation")
>> >> 
>> >> I am about to pick up 73714c05df97 ("drm/i915: Fix 90/270 rotated
>> >> coordinates for FBC") which does apply cleanly - does it make sense
>> >> without the other two?
>> >
>> > Probably not. Shouldn't be too hard for me to fix up the other two
>> > since I already resolved the same conflicts in the other direction
>> > when applying the patches ;)
>> 
>> Whoops, I already sent the pull request without the two. Anything bad in
>> me queuing the backports you provided in the next -rc? Or should I
>> recall the pull request to include these two at the same time?
>
> Next -rc is fine. No need to rush.

Thanks for the backports, pushed them.

BR,
Jani.

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

Re: Fixes that failed to backport to v4.12-rc1

[Michel Thierry]

On 13/06/17 01:27, Jani Nikula wrote:
> On Mon, 15 May 2017, Jani Nikula <jani.nikula@intel.com> wrote:
>> Continuing [1] for v4.12-rc1
>>
>> The following commits have been marked as Cc: stable or fixing something
>> in v4.12-rc1 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.
>
Hi Jani,

> Update:
>
> c4a8952612f6 ("drm/i915/guc: Clear enable_guc_loading in case of init failure")
>

This would only be needed if this other commit is also present:

04f7b24eccdf ("drm/i915/guc: Assert that we switch between known
ggtt->invalidate functions")

Which it shouldn't be the case.

Thanks,
-Michel

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

[PATCH] drm/i915: Differentiate between sw write location into ring and last hw read

[Chris Wilson]

We need to keep track of the last location we ask the hw to read up to
(RING_TAIL) separately from our last write location into the ring, so
that in the event of a GPU reset we do not tell the HW to proceed into
a partially written request (which can happen if that request is waiting
for an external signal before being executed).

v2: Refactor intel_ring_reset() (Mika)

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100144
Testcase: igt/gem_exec_fence/await-hang
Fixes: 821ed7df6e2a ("drm/i915: Update reset path to fix incomplete requests")
Fixes: d55ac5bf97c6 ("drm/i915: Defer transfer onto execution timeline to actual hw submission")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/20170425130049.26147-1-chris@chris-wilson.co.uk
Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
(cherry picked from commit e6ba9992de6c63fe86c028b4876338e1cb7dac34)
---
 drivers/gpu/drm/i915/i915_gem_request.c    |  2 +-
 drivers/gpu/drm/i915/i915_guc_submission.c |  4 +--
 drivers/gpu/drm/i915/intel_lrc.c           |  6 ++---
 drivers/gpu/drm/i915/intel_ringbuffer.c    | 41 ++++++++++++++++++++----------
 drivers/gpu/drm/i915/intel_ringbuffer.h    | 19 ++++++++++++--
 5 files changed, 48 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
index 5ddbc9499775..a74d0ac737cb 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.c
+++ b/drivers/gpu/drm/i915/i915_gem_request.c
@@ -623,7 +623,7 @@ i915_gem_request_alloc(struct intel_engine_cs *engine,
 	 * GPU processing the request, we never over-estimate the
 	 * position of the head.
 	 */
-	req->head = req->ring->tail;
+	req->head = req->ring->emit;
 
 	/* Check that we didn't interrupt ourselves with a new request */
 	GEM_BUG_ON(req->timeline->seqno != req->fence.seqno);
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 1642fff9cf13..ab5140ba108d 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -480,9 +480,7 @@ static void guc_wq_item_append(struct i915_guc_client *client,
 	GEM_BUG_ON(freespace < wqi_size);
 
 	/* The GuC firmware wants the tail index in QWords, not bytes */
-	tail = rq->tail;
-	assert_ring_tail_valid(rq->ring, rq->tail);
-	tail >>= 3;
+	tail = intel_ring_set_tail(rq->ring, rq->tail) >> 3;
 	GEM_BUG_ON(tail > WQ_RING_TAIL_MAX);
 
 	/* For now workqueue item is 4 DWs; workqueue buffer is 2 pages. So we
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index c8f7c631fc1f..10c63dbd617c 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -326,8 +326,7 @@ static u64 execlists_update_context(struct drm_i915_gem_request *rq)
 		rq->ctx->ppgtt ?: rq->i915->mm.aliasing_ppgtt;
 	u32 *reg_state = ce->lrc_reg_state;
 
-	assert_ring_tail_valid(rq->ring, rq->tail);
-	reg_state[CTX_RING_TAIL+1] = rq->tail;
+	reg_state[CTX_RING_TAIL+1] = intel_ring_set_tail(rq->ring, rq->tail);
 
 	/* True 32b PPGTT with dynamic page allocation: update PDP
 	 * registers and point the unallocated PDPs to scratch page.
@@ -2036,8 +2035,7 @@ void intel_lr_context_resume(struct drm_i915_private *dev_priv)
 			ce->state->obj->mm.dirty = true;
 			i915_gem_object_unpin_map(ce->state->obj);
 
-			ce->ring->head = ce->ring->tail = 0;
-			intel_ring_update_space(ce->ring);
+			intel_ring_reset(ce->ring, 0);
 		}
 	}
 }
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 66a2b8b83972..513a0f4b469b 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -49,7 +49,7 @@ static int __intel_ring_space(int head, int tail, int size)
 
 void intel_ring_update_space(struct intel_ring *ring)
 {
-	ring->space = __intel_ring_space(ring->head, ring->tail, ring->size);
+	ring->space = __intel_ring_space(ring->head, ring->emit, ring->size);
 }
 
 static int
@@ -774,8 +774,8 @@ static void i9xx_submit_request(struct drm_i915_gem_request *request)
 
 	i915_gem_request_submit(request);
 
-	assert_ring_tail_valid(request->ring, request->tail);
-	I915_WRITE_TAIL(request->engine, request->tail);
+	I915_WRITE_TAIL(request->engine,
+			intel_ring_set_tail(request->ring, request->tail));
 }
 
 static void i9xx_emit_breadcrumb(struct drm_i915_gem_request *req, u32 *cs)
@@ -1316,11 +1316,23 @@ int intel_ring_pin(struct intel_ring *ring, unsigned int offset_bias)
 	return PTR_ERR(addr);
 }
 
+void intel_ring_reset(struct intel_ring *ring, u32 tail)
+{
+	GEM_BUG_ON(!list_empty(&ring->request_list));
+	ring->tail = tail;
+	ring->head = tail;
+	ring->emit = tail;
+	intel_ring_update_space(ring);
+}
+
 void intel_ring_unpin(struct intel_ring *ring)
 {
 	GEM_BUG_ON(!ring->vma);
 	GEM_BUG_ON(!ring->vaddr);
 
+	/* Discard any unused bytes beyond that submitted to hw. */
+	intel_ring_reset(ring, ring->tail);
+
 	if (i915_vma_is_map_and_fenceable(ring->vma))
 		i915_vma_unpin_iomap(ring->vma);
 	else
@@ -1562,8 +1574,9 @@ void intel_legacy_submission_resume(struct drm_i915_private *dev_priv)
 	struct intel_engine_cs *engine;
 	enum intel_engine_id id;
 
+	/* Restart from the beginning of the rings for convenience */
 	for_each_engine(engine, dev_priv, id)
-		engine->buffer->head = engine->buffer->tail;
+		intel_ring_reset(engine->buffer, 0);
 }
 
 static int ring_request_alloc(struct drm_i915_gem_request *request)
@@ -1616,7 +1629,7 @@ static int wait_for_space(struct drm_i915_gem_request *req, int bytes)
 		unsigned space;
 
 		/* Would completion of this request free enough space? */
-		space = __intel_ring_space(target->postfix, ring->tail,
+		space = __intel_ring_space(target->postfix, ring->emit,
 					   ring->size);
 		if (space >= bytes)
 			break;
@@ -1641,8 +1654,8 @@ static int wait_for_space(struct drm_i915_gem_request *req, int bytes)
 u32 *intel_ring_begin(struct drm_i915_gem_request *req, int num_dwords)
 {
 	struct intel_ring *ring = req->ring;
-	int remain_actual = ring->size - ring->tail;
-	int remain_usable = ring->effective_size - ring->tail;
+	int remain_actual = ring->size - ring->emit;
+	int remain_usable = ring->effective_size - ring->emit;
 	int bytes = num_dwords * sizeof(u32);
 	int total_bytes, wait_bytes;
 	bool need_wrap = false;
@@ -1678,17 +1691,17 @@ u32 *intel_ring_begin(struct drm_i915_gem_request *req, int num_dwords)
 
 	if (unlikely(need_wrap)) {
 		GEM_BUG_ON(remain_actual > ring->space);
-		GEM_BUG_ON(ring->tail + remain_actual > ring->size);
+		GEM_BUG_ON(ring->emit + remain_actual > ring->size);
 
 		/* Fill the tail with MI_NOOP */
-		memset(ring->vaddr + ring->tail, 0, remain_actual);
-		ring->tail = 0;
+		memset(ring->vaddr + ring->emit, 0, remain_actual);
+		ring->emit = 0;
 		ring->space -= remain_actual;
 	}
 
-	GEM_BUG_ON(ring->tail > ring->size - bytes);
-	cs = ring->vaddr + ring->tail;
-	ring->tail += bytes;
+	GEM_BUG_ON(ring->emit > ring->size - bytes);
+	cs = ring->vaddr + ring->emit;
+	ring->emit += bytes;
 	ring->space -= bytes;
 	GEM_BUG_ON(ring->space < 0);
 
@@ -1699,7 +1712,7 @@ u32 *intel_ring_begin(struct drm_i915_gem_request *req, int num_dwords)
 int intel_ring_cacheline_align(struct drm_i915_gem_request *req)
 {
 	int num_dwords =
-		(req->ring->tail & (CACHELINE_BYTES - 1)) / sizeof(uint32_t);
+		(req->ring->emit & (CACHELINE_BYTES - 1)) / sizeof(uint32_t);
 	u32 *cs;
 
 	if (num_dwords == 0)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index a82a0807f64d..f7144fe09613 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -145,6 +145,7 @@ struct intel_ring {
 
 	u32 head;
 	u32 tail;
+	u32 emit;
 
 	int space;
 	int size;
@@ -488,6 +489,8 @@ intel_write_status_page(struct intel_engine_cs *engine, int reg, u32 value)
 struct intel_ring *
 intel_engine_create_ring(struct intel_engine_cs *engine, int size);
 int intel_ring_pin(struct intel_ring *ring, unsigned int offset_bias);
+void intel_ring_reset(struct intel_ring *ring, u32 tail);
+void intel_ring_update_space(struct intel_ring *ring);
 void intel_ring_unpin(struct intel_ring *ring);
 void intel_ring_free(struct intel_ring *ring);
 
@@ -511,7 +514,7 @@ intel_ring_advance(struct drm_i915_gem_request *req, u32 *cs)
 	 * reserved for the command packet (i.e. the value passed to
 	 * intel_ring_begin()).
 	 */
-	GEM_BUG_ON((req->ring->vaddr + req->ring->tail) != cs);
+	GEM_BUG_ON((req->ring->vaddr + req->ring->emit) != cs);
 }
 
 static inline u32
@@ -540,7 +543,19 @@ assert_ring_tail_valid(const struct intel_ring *ring, unsigned int tail)
 	GEM_BUG_ON(tail >= ring->size);
 }
 
-void intel_ring_update_space(struct intel_ring *ring);
+static inline unsigned int
+intel_ring_set_tail(struct intel_ring *ring, unsigned int tail)
+{
+	/* Whilst writes to the tail are strictly order, there is no
+	 * serialisation between readers and the writers. The tail may be
+	 * read by i915_gem_request_retire() just as it is being updated
+	 * by execlists, as although the breadcrumb is complete, the context
+	 * switch hasn't been seen.
+	 */
+	assert_ring_tail_valid(ring, tail);
+	ring->tail = tail;
+	return tail;
+}
 
 void intel_engine_init_global_seqno(struct intel_engine_cs *engine, u32 seqno);
 
-- 
2.11.0

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

Re: [PATCH] drm/i915: Differentiate between sw write location into ring and last hw read

[Jani Nikula]

On Thu, 15 Jun 2017, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> We need to keep track of the last location we ask the hw to read up to
> (RING_TAIL) separately from our last write location into the ring, so
> that in the event of a GPU reset we do not tell the HW to proceed into
> a partially written request (which can happen if that request is waiting
> for an external signal before being executed).
>
> v2: Refactor intel_ring_reset() (Mika)
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100144
> Testcase: igt/gem_exec_fence/await-hang
> Fixes: 821ed7df6e2a ("drm/i915: Update reset path to fix incomplete requests")
> Fixes: d55ac5bf97c6 ("drm/i915: Defer transfer onto execution timeline to actual hw submission")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> Link: http://patchwork.freedesktop.org/patch/msgid/20170425130049.26147-1-chris@chris-wilson.co.uk
> Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
> (cherry picked from commit e6ba9992de6c63fe86c028b4876338e1cb7dac34)

Thanks, picked up to drm-intel-fixes.

BR,
Jani.

> ---
>  drivers/gpu/drm/i915/i915_gem_request.c    |  2 +-
>  drivers/gpu/drm/i915/i915_guc_submission.c |  4 +--
>  drivers/gpu/drm/i915/intel_lrc.c           |  6 ++---
>  drivers/gpu/drm/i915/intel_ringbuffer.c    | 41 ++++++++++++++++++++----------
>  drivers/gpu/drm/i915/intel_ringbuffer.h    | 19 ++++++++++++--
>  5 files changed, 48 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
> index 5ddbc9499775..a74d0ac737cb 100644
> --- a/drivers/gpu/drm/i915/i915_gem_request.c
> +++ b/drivers/gpu/drm/i915/i915_gem_request.c
> @@ -623,7 +623,7 @@ i915_gem_request_alloc(struct intel_engine_cs *engine,
>  	 * GPU processing the request, we never over-estimate the
>  	 * position of the head.
>  	 */
> -	req->head = req->ring->tail;
> +	req->head = req->ring->emit;
>  
>  	/* Check that we didn't interrupt ourselves with a new request */
>  	GEM_BUG_ON(req->timeline->seqno != req->fence.seqno);
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
> index 1642fff9cf13..ab5140ba108d 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -480,9 +480,7 @@ static void guc_wq_item_append(struct i915_guc_client *client,
>  	GEM_BUG_ON(freespace < wqi_size);
>  
>  	/* The GuC firmware wants the tail index in QWords, not bytes */
> -	tail = rq->tail;
> -	assert_ring_tail_valid(rq->ring, rq->tail);
> -	tail >>= 3;
> +	tail = intel_ring_set_tail(rq->ring, rq->tail) >> 3;
>  	GEM_BUG_ON(tail > WQ_RING_TAIL_MAX);
>  
>  	/* For now workqueue item is 4 DWs; workqueue buffer is 2 pages. So we
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index c8f7c631fc1f..10c63dbd617c 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -326,8 +326,7 @@ static u64 execlists_update_context(struct drm_i915_gem_request *rq)
>  		rq->ctx->ppgtt ?: rq->i915->mm.aliasing_ppgtt;
>  	u32 *reg_state = ce->lrc_reg_state;
>  
> -	assert_ring_tail_valid(rq->ring, rq->tail);
> -	reg_state[CTX_RING_TAIL+1] = rq->tail;
> +	reg_state[CTX_RING_TAIL+1] = intel_ring_set_tail(rq->ring, rq->tail);
>  
>  	/* True 32b PPGTT with dynamic page allocation: update PDP
>  	 * registers and point the unallocated PDPs to scratch page.
> @@ -2036,8 +2035,7 @@ void intel_lr_context_resume(struct drm_i915_private *dev_priv)
>  			ce->state->obj->mm.dirty = true;
>  			i915_gem_object_unpin_map(ce->state->obj);
>  
> -			ce->ring->head = ce->ring->tail = 0;
> -			intel_ring_update_space(ce->ring);
> +			intel_ring_reset(ce->ring, 0);
>  		}
>  	}
>  }
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 66a2b8b83972..513a0f4b469b 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -49,7 +49,7 @@ static int __intel_ring_space(int head, int tail, int size)
>  
>  void intel_ring_update_space(struct intel_ring *ring)
>  {
> -	ring->space = __intel_ring_space(ring->head, ring->tail, ring->size);
> +	ring->space = __intel_ring_space(ring->head, ring->emit, ring->size);
>  }
>  
>  static int
> @@ -774,8 +774,8 @@ static void i9xx_submit_request(struct drm_i915_gem_request *request)
>  
>  	i915_gem_request_submit(request);
>  
> -	assert_ring_tail_valid(request->ring, request->tail);
> -	I915_WRITE_TAIL(request->engine, request->tail);
> +	I915_WRITE_TAIL(request->engine,
> +			intel_ring_set_tail(request->ring, request->tail));
>  }
>  
>  static void i9xx_emit_breadcrumb(struct drm_i915_gem_request *req, u32 *cs)
> @@ -1316,11 +1316,23 @@ int intel_ring_pin(struct intel_ring *ring, unsigned int offset_bias)
>  	return PTR_ERR(addr);
>  }
>  
> +void intel_ring_reset(struct intel_ring *ring, u32 tail)
> +{
> +	GEM_BUG_ON(!list_empty(&ring->request_list));
> +	ring->tail = tail;
> +	ring->head = tail;
> +	ring->emit = tail;
> +	intel_ring_update_space(ring);
> +}
> +
>  void intel_ring_unpin(struct intel_ring *ring)
>  {
>  	GEM_BUG_ON(!ring->vma);
>  	GEM_BUG_ON(!ring->vaddr);
>  
> +	/* Discard any unused bytes beyond that submitted to hw. */
> +	intel_ring_reset(ring, ring->tail);
> +
>  	if (i915_vma_is_map_and_fenceable(ring->vma))
>  		i915_vma_unpin_iomap(ring->vma);
>  	else
> @@ -1562,8 +1574,9 @@ void intel_legacy_submission_resume(struct drm_i915_private *dev_priv)
>  	struct intel_engine_cs *engine;
>  	enum intel_engine_id id;
>  
> +	/* Restart from the beginning of the rings for convenience */
>  	for_each_engine(engine, dev_priv, id)
> -		engine->buffer->head = engine->buffer->tail;
> +		intel_ring_reset(engine->buffer, 0);
>  }
>  
>  static int ring_request_alloc(struct drm_i915_gem_request *request)
> @@ -1616,7 +1629,7 @@ static int wait_for_space(struct drm_i915_gem_request *req, int bytes)
>  		unsigned space;
>  
>  		/* Would completion of this request free enough space? */
> -		space = __intel_ring_space(target->postfix, ring->tail,
> +		space = __intel_ring_space(target->postfix, ring->emit,
>  					   ring->size);
>  		if (space >= bytes)
>  			break;
> @@ -1641,8 +1654,8 @@ static int wait_for_space(struct drm_i915_gem_request *req, int bytes)
>  u32 *intel_ring_begin(struct drm_i915_gem_request *req, int num_dwords)
>  {
>  	struct intel_ring *ring = req->ring;
> -	int remain_actual = ring->size - ring->tail;
> -	int remain_usable = ring->effective_size - ring->tail;
> +	int remain_actual = ring->size - ring->emit;
> +	int remain_usable = ring->effective_size - ring->emit;
>  	int bytes = num_dwords * sizeof(u32);
>  	int total_bytes, wait_bytes;
>  	bool need_wrap = false;
> @@ -1678,17 +1691,17 @@ u32 *intel_ring_begin(struct drm_i915_gem_request *req, int num_dwords)
>  
>  	if (unlikely(need_wrap)) {
>  		GEM_BUG_ON(remain_actual > ring->space);
> -		GEM_BUG_ON(ring->tail + remain_actual > ring->size);
> +		GEM_BUG_ON(ring->emit + remain_actual > ring->size);
>  
>  		/* Fill the tail with MI_NOOP */
> -		memset(ring->vaddr + ring->tail, 0, remain_actual);
> -		ring->tail = 0;
> +		memset(ring->vaddr + ring->emit, 0, remain_actual);
> +		ring->emit = 0;
>  		ring->space -= remain_actual;
>  	}
>  
> -	GEM_BUG_ON(ring->tail > ring->size - bytes);
> -	cs = ring->vaddr + ring->tail;
> -	ring->tail += bytes;
> +	GEM_BUG_ON(ring->emit > ring->size - bytes);
> +	cs = ring->vaddr + ring->emit;
> +	ring->emit += bytes;
>  	ring->space -= bytes;
>  	GEM_BUG_ON(ring->space < 0);
>  
> @@ -1699,7 +1712,7 @@ u32 *intel_ring_begin(struct drm_i915_gem_request *req, int num_dwords)
>  int intel_ring_cacheline_align(struct drm_i915_gem_request *req)
>  {
>  	int num_dwords =
> -		(req->ring->tail & (CACHELINE_BYTES - 1)) / sizeof(uint32_t);
> +		(req->ring->emit & (CACHELINE_BYTES - 1)) / sizeof(uint32_t);
>  	u32 *cs;
>  
>  	if (num_dwords == 0)
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index a82a0807f64d..f7144fe09613 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -145,6 +145,7 @@ struct intel_ring {
>  
>  	u32 head;
>  	u32 tail;
> +	u32 emit;
>  
>  	int space;
>  	int size;
> @@ -488,6 +489,8 @@ intel_write_status_page(struct intel_engine_cs *engine, int reg, u32 value)
>  struct intel_ring *
>  intel_engine_create_ring(struct intel_engine_cs *engine, int size);
>  int intel_ring_pin(struct intel_ring *ring, unsigned int offset_bias);
> +void intel_ring_reset(struct intel_ring *ring, u32 tail);
> +void intel_ring_update_space(struct intel_ring *ring);
>  void intel_ring_unpin(struct intel_ring *ring);
>  void intel_ring_free(struct intel_ring *ring);
>  
> @@ -511,7 +514,7 @@ intel_ring_advance(struct drm_i915_gem_request *req, u32 *cs)
>  	 * reserved for the command packet (i.e. the value passed to
>  	 * intel_ring_begin()).
>  	 */
> -	GEM_BUG_ON((req->ring->vaddr + req->ring->tail) != cs);
> +	GEM_BUG_ON((req->ring->vaddr + req->ring->emit) != cs);
>  }
>  
>  static inline u32
> @@ -540,7 +543,19 @@ assert_ring_tail_valid(const struct intel_ring *ring, unsigned int tail)
>  	GEM_BUG_ON(tail >= ring->size);
>  }
>  
> -void intel_ring_update_space(struct intel_ring *ring);
> +static inline unsigned int
> +intel_ring_set_tail(struct intel_ring *ring, unsigned int tail)
> +{
> +	/* Whilst writes to the tail are strictly order, there is no
> +	 * serialisation between readers and the writers. The tail may be
> +	 * read by i915_gem_request_retire() just as it is being updated
> +	 * by execlists, as although the breadcrumb is complete, the context
> +	 * switch hasn't been seen.
> +	 */
> +	assert_ring_tail_valid(ring, tail);
> +	ring->tail = tail;
> +	return tail;
> +}
>  
>  void intel_engine_init_global_seqno(struct intel_engine_cs *engine, u32 seqno);

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

Re: Fixes that failed to backport to v4.12-rc1

[Jani Nikula]

On Mon, 15 May 2017, Jani Nikula <jani.nikula@intel.com> wrote:
> Continuing [1] for v4.12-rc1
>
> The following commits have been marked as Cc: stable or fixing something
> in v4.12-rc1 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.

Update:

f6262bda462e ("drm/i915: Don't enable backlight at setup time.")
e27ab73d17ef ("drm/i915: Mark CPU cache as dirty on every transition for CPU writes")
071750e550af ("drm/i915: Disable EXEC_OBJECT_ASYNC when doing relocations")

BR,
Jani.


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

[PATCH] drm/i915: Don't enable backlight at setup time.

[Dhinakaran Pandiyan]

Maarten and Ville noticed that we are enabling backlight via DP aux very
early in the modeset_init path via the intel_dp_aux_setup_backlight()
function, since commit e7156c833903 ("drm/i915: Add Backlight Control using
DPCD for eDP connectors (v9)"). Looks like all we need to do during
_setup_backlight() is read the current brightness state instead of
modifying it.

v2: Rewrote commit message.

Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Yetunde Adebisi <yetundex.adebisi@intel.com>
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Acked-by: Jani Nikula <jani.nikula@intel.com>
Tested-by: Puthikorn Voravootivat <puthik@chromium.org>
Fixes: e7156c833903 ("drm/i915: Add Backlight Control using DPCD for eDP connectors (v9)")
Link: http://patchwork.freedesktop.org/patch/msgid/1497384239-2965-1-git-send-email-dhinakaran.pandiyan@intel.com
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
(cherry picked from commit f6262bda462e81e959b80a96dac799bd9df27f73)
---
 drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
index 6532e22..40ba313 100644
--- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
+++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
@@ -119,8 +119,6 @@ static int intel_dp_aux_setup_backlight(struct intel_connector *connector,
 	struct intel_dp *intel_dp = enc_to_intel_dp(&connector->encoder->base);
 	struct intel_panel *panel = &connector->panel;
 
-	intel_dp_aux_enable_backlight(connector);
-
 	if (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_BYTE_COUNT)
 		panel->backlight.max = 0xFFFF;
 	else
-- 
2.7.4

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

Re: [PATCH] drm/i915: Don't enable backlight at setup time.

[Pandiyan, Dhinakaran]

On Mon, 2017-06-19 at 11:08 -0700, Dhinakaran Pandiyan wrote:
> Maarten and Ville noticed that we are enabling backlight via DP aux very
> early in the modeset_init path via the intel_dp_aux_setup_backlight()
> function, since commit e7156c833903 ("drm/i915: Add Backlight Control using
> DPCD for eDP connectors (v9)"). Looks like all we need to do during
> _setup_backlight() is read the current brightness state instead of
> modifying it.
> 
> v2: Rewrote commit message.
> 
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Yetunde Adebisi <yetundex.adebisi@intel.com>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Acked-by: Jani Nikula <jani.nikula@intel.com>
> Tested-by: Puthikorn Voravootivat <puthik@chromium.org>
> Fixes: e7156c833903 ("drm/i915: Add Backlight Control using DPCD for eDP connectors (v9)")
> Link: http://patchwork.freedesktop.org/patch/msgid/1497384239-2965-1-git-send-email-dhinakaran.pandiyan@intel.com
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> (cherry picked from commit f6262bda462e81e959b80a96dac799bd9df27f73)


I am sending a backport for the first time, please let me know if I
messed up something. 



> ---
>  drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> index 6532e22..40ba313 100644
> --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> @@ -119,8 +119,6 @@ static int intel_dp_aux_setup_backlight(struct intel_connector *connector,
>  	struct intel_dp *intel_dp = enc_to_intel_dp(&connector->encoder->base);
>  	struct intel_panel *panel = &connector->panel;
>  
> -	intel_dp_aux_enable_backlight(connector);
> -
>  	if (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_BYTE_COUNT)
>  		panel->backlight.max = 0xFFFF;
>  	else

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

Re: [PATCH] drm/i915: Don't enable backlight at setup time.

[Jani Nikula]

On Mon, 19 Jun 2017, Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> wrote:
> Maarten and Ville noticed that we are enabling backlight via DP aux very
> early in the modeset_init path via the intel_dp_aux_setup_backlight()
> function, since commit e7156c833903 ("drm/i915: Add Backlight Control using
> DPCD for eDP connectors (v9)"). Looks like all we need to do during
> _setup_backlight() is read the current brightness state instead of
> modifying it.
>
> v2: Rewrote commit message.
>
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Yetunde Adebisi <yetundex.adebisi@intel.com>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Acked-by: Jani Nikula <jani.nikula@intel.com>
> Tested-by: Puthikorn Voravootivat <puthik@chromium.org>
> Fixes: e7156c833903 ("drm/i915: Add Backlight Control using DPCD for eDP connectors (v9)")
> Link: http://patchwork.freedesktop.org/patch/msgid/1497384239-2965-1-git-send-email-dhinakaran.pandiyan@intel.com
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> (cherry picked from commit f6262bda462e81e959b80a96dac799bd9df27f73)

Pushed to drm-intel-fixes, thanks for the backport.

BR,
Jani.

> ---
>  drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> index 6532e22..40ba313 100644
> --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> @@ -119,8 +119,6 @@ static int intel_dp_aux_setup_backlight(struct intel_connector *connector,
>  	struct intel_dp *intel_dp = enc_to_intel_dp(&connector->encoder->base);
>  	struct intel_panel *panel = &connector->panel;
>  
> -	intel_dp_aux_enable_backlight(connector);
> -
>  	if (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_BYTE_COUNT)
>  		panel->backlight.max = 0xFFFF;
>  	else

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