Re: [Intel-gfx] [PATCH 3/3] drm: Use a normal idr allocation for the obj->name
On Mon, Jan 04, 2016 at 05:31:14PM +0200, Ville Syrjälä wrote:
> On Mon, Jan 04, 2016 at 10:11:01AM +, Chris Wilson wrote:
> > Unlike the handle, the name table uses a sleeping mutex rather than a
> > spinlock. The allocation is in a normal context, and we can use the
> > simpler sleeping gfp_t, rather than have to take from the atomic
> > reserves.
> >
> > Signed-off-by: Chris Wilson
>
> Reviewed-by: Ville Syrjälä
All 3 merged to drm-misc, thanks for patches&review.
-Daniel
>
> > ---
> > drivers/gpu/drm/drm_gem.c | 4 +---
> > 1 file changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> > index ad955d7c99fd..1b0c2c127072 100644
> > --- a/drivers/gpu/drm/drm_gem.c
> > +++ b/drivers/gpu/drm/drm_gem.c
> > @@ -642,7 +642,6 @@ drm_gem_flink_ioctl(struct drm_device *dev, void *data,
> > return -ENOENT;
> >
> > mutex_lock(&dev->object_name_lock);
> > - idr_preload(GFP_KERNEL);
> > /* prevent races with concurrent gem_close. */
> > if (obj->handle_count == 0) {
> > ret = -ENOENT;
> > @@ -650,7 +649,7 @@ drm_gem_flink_ioctl(struct drm_device *dev, void *data,
> > }
> >
> > if (!obj->name) {
> > - ret = idr_alloc(&dev->object_name_idr, obj, 1, 0, GFP_NOWAIT);
> > + ret = idr_alloc(&dev->object_name_idr, obj, 1, 0, GFP_KERNEL);
> > if (ret < 0)
> > goto err;
> >
> > @@ -661,7 +660,6 @@ drm_gem_flink_ioctl(struct drm_device *dev, void *data,
> > ret = 0;
> >
> > err:
> > - idr_preload_end();
> > mutex_unlock(&dev->object_name_lock);
> > drm_gem_object_unreference_unlocked(obj);
> > return ret;
> > --
> > 2.6.4
> >
> > ___
> > dri-devel mailing list
> > dri-de...@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Ville Syrjälä
> Intel OTC
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ warning: Fi.CI.BAT
== Summary == Built on 0417da5e6f56078d87d366d5f959f8290ae9d16d drm-intel-nightly: 2016y-01m-04d-14h-05m-39s UTC integration manifest Test kms_flip: Subgroup basic-flip-vs-dpms: dmesg-warn -> PASS (ilk-hp8440p) Subgroup basic-flip-vs-modeset: pass -> DMESG-WARN (skl-i5k-2) dmesg-warn -> PASS (hsw-brixbox) Subgroup basic-plain-flip: dmesg-warn -> PASS (skl-i5k-2) dmesg-warn -> PASS (bdw-ultra) Test kms_pipe_crc_basic: Subgroup read-crc-pipe-a: pass -> DMESG-WARN (snb-x220t) Subgroup read-crc-pipe-b: dmesg-warn -> PASS (snb-dellxps) Subgroup read-crc-pipe-b-frame-sequence: pass -> DMESG-WARN (bdw-ultra) Subgroup suspend-read-crc-pipe-b: dmesg-warn -> PASS (snb-x220t) bdw-nuci7total:132 pass:122 dwarn:1 dfail:0 fail:0 skip:9 bdw-ultratotal:132 pass:124 dwarn:2 dfail:0 fail:0 skip:6 bsw-nuc-2total:135 pass:114 dwarn:1 dfail:0 fail:0 skip:20 hsw-brixbox total:135 pass:127 dwarn:1 dfail:0 fail:0 skip:7 hsw-gt2 total:135 pass:130 dwarn:1 dfail:0 fail:0 skip:4 ilk-hp8440p total:135 pass:100 dwarn:0 dfail:0 fail:0 skip:35 ivb-t430stotal:135 pass:127 dwarn:2 dfail:0 fail:0 skip:6 skl-i5k-2total:135 pass:123 dwarn:4 dfail:0 fail:0 skip:8 skl-i7k-2total:135 pass:125 dwarn:2 dfail:0 fail:0 skip:8 snb-dellxps total:135 pass:122 dwarn:1 dfail:0 fail:0 skip:12 snb-x220ttotal:135 pass:121 dwarn:2 dfail:0 fail:1 skip:11 Results at /archive/results/CI_IGT_test/Patchwork_1078/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 5/6] drm/i915: Update connector_mask during readout.
On Mon, Jan 04, 2016 at 12:53:19PM +0100, Maarten Lankhorst wrote:
> Signed-off-by: Maarten Lankhorst
> ---
> drivers/gpu/drm/i915/intel_display.c | 11 ---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 1e42309ec40a..b76778d76035 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -15421,6 +15421,7 @@ static void intel_sanitize_crtc(struct intel_crtc
> *crtc)
> WARN_ON(drm_atomic_set_mode_for_crtc(crtc->base.state, NULL) <
> 0);
> crtc->base.state->active = crtc->active;
> crtc->base.enabled = crtc->active;
> + crtc->base.state->connector_mask = 0;
>
> /* Because we only establish the connector -> encoder ->
>* crtc links if something is active, this means the
> @@ -15456,20 +15457,24 @@ static void intel_sanitize_encoder(struct
> intel_encoder *encoder)
> {
> struct intel_connector *connector;
> struct drm_device *dev = encoder->base.dev;
> + struct drm_crtc *crtc = encoder->base.crtc;
> bool active = false;
>
> /* We need to check both for a crtc link (meaning that the
>* encoder is active and trying to read from a pipe) and the
>* pipe itself being active. */
> - bool has_active_crtc = encoder->base.crtc &&
> - to_intel_crtc(encoder->base.crtc)->active;
> + bool has_active_crtc = crtc && crtc->state->active;
>
> for_each_intel_connector(dev, connector) {
> if (connector->base.encoder != &encoder->base)
> continue;
>
> active = true;
> - break;
> + if (!has_active_crtc)
> + break;
> +
> + crtc->state->connector_mask |=
> + 1 << drm_connector_index(&connector->base);
I still think this is the wrong place. Imo this should be done in
intel_modeset_update_connector_atomic_state. It'd be great if we could
somehow share the logic with drm_atomic_set_crtc_for_connector even, but
that's probably over the top.
-Daniel
> }
>
> if (active && !has_active_crtc) {
> --
> 2.1.0
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 2/6] drm/atomic: Add __drm_atomic_helper_connector_reset, v2.
On Mon, Jan 04, 2016 at 12:53:16PM +0100, Maarten Lankhorst wrote:
> This is useful for drivers that subclass connector_state, like tegra.
>
> Changes since v1:
> - Docbook updates.
>
> Signed-off-by: Maarten Lankhorst
> ---
> drivers/gpu/drm/drm_atomic_helper.c | 30 ++
> include/drm/drm_atomic_helper.h | 2 ++
> 2 files changed, 28 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c
> index 63f925b75357..27dd68f946e6 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -2607,6 +2607,28 @@ void drm_atomic_helper_plane_destroy_state(struct
> drm_plane *plane,
> EXPORT_SYMBOL(drm_atomic_helper_plane_destroy_state);
>
> /**
> + * __drm_atomic_helper_connector_reset - reset state on connector
> + * @connector: drm connector
> + * @conn_state: connector state to assign
> + *
> + * Initializes the newly allocated @conn_state and assigns it to
> + * #connector ->state, usually required when initializing the drivers
> + * or when called from the ->reset hook.
> + *
> + * This is useful for drivers that subclass the connector state.
> + */
> +void
> +__drm_atomic_helper_connector_reset(struct drm_connector *connector,
> + struct drm_connector_state *conn_state)
> +{
> + if (conn_state)
> + conn_state->connector = connector;
> +
> + connector->state = conn_state;
> +}
> +EXPORT_SYMBOL(__drm_atomic_helper_connector_reset);
It's a bit strange that we don't have __*reset functions for crtc/plane
now, especially since that would uncover that probably we should move
cleaning up the old state into this helper (to share more code). That can
be done by simply calling ->atomic_state_destroy. Callers would then only
allocate a suitable struct, call this func and init any driver-private
state.
But this here won't hurt, so merged it.
-Daniel
> +
> +/**
> * drm_atomic_helper_connector_reset - default ->reset hook for connectors
> * @connector: drm connector
> *
> @@ -2616,11 +2638,11 @@ EXPORT_SYMBOL(drm_atomic_helper_plane_destroy_state);
> */
> void drm_atomic_helper_connector_reset(struct drm_connector *connector)
> {
> - kfree(connector->state);
> - connector->state = kzalloc(sizeof(*connector->state), GFP_KERNEL);
> + struct drm_connector_state *conn_state =
> + kzalloc(sizeof(*conn_state), GFP_KERNEL);
>
> - if (connector->state)
> - connector->state->connector = connector;
> + kfree(connector->state);
> + __drm_atomic_helper_connector_reset(connector, conn_state);
> }
> EXPORT_SYMBOL(drm_atomic_helper_connector_reset);
>
> diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
> index a286cce98720..89d008dc08e2 100644
> --- a/include/drm/drm_atomic_helper.h
> +++ b/include/drm/drm_atomic_helper.h
> @@ -126,6 +126,8 @@ void __drm_atomic_helper_plane_destroy_state(struct
> drm_plane *plane,
> void drm_atomic_helper_plane_destroy_state(struct drm_plane *plane,
> struct drm_plane_state *state);
>
> +void __drm_atomic_helper_connector_reset(struct drm_connector *connector,
> + struct drm_connector_state
> *conn_state);
> void drm_atomic_helper_connector_reset(struct drm_connector *connector);
> void
> __drm_atomic_helper_connector_duplicate_state(struct drm_connector
> *connector,
> --
> 2.1.0
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 4/6] drm/atomic: add connector mask to drm_crtc_state.
On Mon, Jan 04, 2016 at 12:53:18PM +0100, Maarten Lankhorst wrote:
> It can be useful to iterate over connectors without grabbing
> connection_mutex. It can also be used to see how many connectors
> are on a crtc without iterating over the list.
>
> Signed-off-by: Maarten Lankhorst
Merged up to this patch here, thanks.
-Daniel
> ---
> drivers/gpu/drm/drm_atomic.c | 11 +++
> include/drm/drm_crtc.h | 3 +++
> 2 files changed, 14 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 6a21e5c378c1..14b321580517 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -1063,10 +1063,21 @@ drm_atomic_set_crtc_for_connector(struct
> drm_connector_state *conn_state,
> {
> struct drm_crtc_state *crtc_state;
>
> + if (conn_state->crtc && conn_state->crtc != crtc) {
> + crtc_state =
> drm_atomic_get_existing_crtc_state(conn_state->state,
> +
> conn_state->crtc);
> +
> + crtc_state->connector_mask &=
> + ~(1 << drm_connector_index(conn_state->connector));
> + }
> +
> if (crtc) {
> crtc_state = drm_atomic_get_crtc_state(conn_state->state, crtc);
> if (IS_ERR(crtc_state))
> return PTR_ERR(crtc_state);
> +
> + crtc_state->connector_mask |=
> + 1 << drm_connector_index(conn_state->connector);
> }
>
> conn_state->crtc = crtc;
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index c2f98ba2bb98..dd0db4ceab26 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -301,6 +301,7 @@ struct drm_plane_helper_funcs;
> * @active_changed: crtc_state->active has been toggled.
> * @connectors_changed: connectors to this crtc have been updated
> * @plane_mask: bitmask of (1 << drm_plane_index(plane)) of attached planes
> + * @connector_mask: bitmask of (1 << drm_connector_index(connector)) of
> attached connectors
> * @last_vblank_count: for helpers and drivers to capture the vblank of the
> * update to ensure framebuffer cleanup isn't done too early
> * @adjusted_mode: for use by helpers and drivers to compute adjusted mode
> timings
> @@ -334,6 +335,8 @@ struct drm_crtc_state {
>*/
> u32 plane_mask;
>
> + u32 connector_mask;
> +
> /* last_vblank_count: for vblank waits before cleanup */
> u32 last_vblank_count;
>
> --
> 2.1.0
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 6/6] drm/atomic: Remove drm_atomic_connectors_for_crtc.
On Mon, Jan 04, 2016 at 12:53:20PM +0100, Maarten Lankhorst wrote:
> Now that connector_mask is reliable there's no need for this
> function any more.
>
> Signed-off-by: Maarten Lankhorst
Since this doesn't touch i915 I figured I can merge this one too. So
except for the previous i915 patch it's now all in drm-misc.
-Daniel
> ---
> drivers/gpu/drm/drm_atomic.c| 30 --
> drivers/gpu/drm/drm_atomic_helper.c | 10 --
> drivers/gpu/drm/vc4/vc4_crtc.c | 2 +-
> include/drm/drm_atomic.h| 4
> 4 files changed, 5 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 14b321580517..d3ed12447fbb 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -1183,36 +1183,6 @@ drm_atomic_add_affected_planes(struct drm_atomic_state
> *state,
> EXPORT_SYMBOL(drm_atomic_add_affected_planes);
>
> /**
> - * drm_atomic_connectors_for_crtc - count number of connected outputs
> - * @state: atomic state
> - * @crtc: DRM crtc
> - *
> - * This function counts all connectors which will be connected to @crtc
> - * according to @state. Useful to recompute the enable state for @crtc.
> - */
> -int
> -drm_atomic_connectors_for_crtc(struct drm_atomic_state *state,
> -struct drm_crtc *crtc)
> -{
> - struct drm_connector *connector;
> - struct drm_connector_state *conn_state;
> -
> - int i, num_connected_connectors = 0;
> -
> - for_each_connector_in_state(state, connector, conn_state, i) {
> - if (conn_state->crtc == crtc)
> - num_connected_connectors++;
> - }
> -
> - DRM_DEBUG_ATOMIC("State %p has %i connectors for [CRTC:%d:%s]\n",
> - state, num_connected_connectors,
> - crtc->base.id, crtc->name);
> -
> - return num_connected_connectors;
> -}
> -EXPORT_SYMBOL(drm_atomic_connectors_for_crtc);
> -
> -/**
> * drm_atomic_legacy_backoff - locking backoff for legacy ioctls
> * @state: atomic state
> *
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c
> index 27dd68f946e6..13b771cb0d35 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -464,7 +464,8 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
>* crtc only changed its mode but has the same set of connectors.
>*/
> for_each_crtc_in_state(state, crtc, crtc_state, i) {
> - int num_connectors;
> + bool has_connectors =
> + !!crtc_state->connector_mask;
>
> /*
>* We must set ->active_changed after walking connectors for
> @@ -493,10 +494,7 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
> if (ret != 0)
> return ret;
>
> - num_connectors = drm_atomic_connectors_for_crtc(state,
> - crtc);
> -
> - if (crtc_state->enable != !!num_connectors) {
> + if (crtc_state->enable != has_connectors) {
> DRM_DEBUG_ATOMIC("[CRTC:%d:%s] enabled/connectors
> mismatch\n",
>crtc->base.id, crtc->name);
>
> @@ -1755,7 +1753,7 @@ static int update_output_state(struct drm_atomic_state
> *state,
> if (crtc == set->crtc)
> continue;
>
> - if (!drm_atomic_connectors_for_crtc(state, crtc)) {
> + if (!crtc_state->connector_mask) {
> ret = drm_atomic_set_mode_prop_for_crtc(crtc_state,
> NULL);
> if (ret < 0)
> diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
> index 2168a99d59aa..aa7ed24a41c7 100644
> --- a/drivers/gpu/drm/vc4/vc4_crtc.c
> +++ b/drivers/gpu/drm/vc4/vc4_crtc.c
> @@ -327,7 +327,7 @@ static int vc4_crtc_atomic_check(struct drm_crtc *crtc,
> /* The pixelvalve can only feed one encoder (and encoders are
>* 1:1 with connectors.)
>*/
> - if (drm_atomic_connectors_for_crtc(state->state, crtc) > 1)
> + if (hweight32(state->connector_mask) > 1)
> return -EINVAL;
>
> drm_atomic_crtc_state_for_each_plane(plane, state) {
> diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
> index d8576ac55693..d3eaa5df187a 100644
> --- a/include/drm/drm_atomic.h
> +++ b/include/drm/drm_atomic.h
> @@ -130,10 +130,6 @@ int __must_check
> drm_atomic_add_affected_planes(struct drm_atomic_state *state,
> struct drm_crtc *crtc);
>
> -int
> -drm_atomic_connectors_for_crtc(struct drm_atomic_state *state,
> -struct drm_crtc *crtc);
> -
> void drm_atomic_legacy_backoff(struct drm_atomic_state *state);
>
> void
> --
> 2.1.0
>
Re: [Intel-gfx] [PATCH 4/5] drm/atomic-helper: Reject legacy flips on a disabled pipe
On Tue, Dec 08, 2015 at 12:01:57PM +, Daniel Stone wrote: > Hi, > > On 8 December 2015 at 08:49, Daniel Vetter wrote: > > We want this for consistency with existing page_flip semantics. > > > > Since this spurred quite a discussion on IRC also document why we > > reject even generation when the pipe is off: It's not that it's hard > > to implement, but userspace has a track recording proofing that it's > > way too easy to accidentally abuse and cause havoc. We want to make > > sure userspace doesn't get away with that. > > > > v2: Somehow thought we do reject events already, but that code only > > existed in my imagination ... Also suggestions from Thierry. > > What a beautifully-coloured shed. > > Reviewed-by: Daniel Stone Added Ilia's spelling fixes and merged to drm-misc, thanks for the review. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 5/6] drm/i915: Update connector_mask during readout.
Op 05-01-16 om 09:35 schreef Daniel Vetter:
> On Mon, Jan 04, 2016 at 12:53:19PM +0100, Maarten Lankhorst wrote:
>> Signed-off-by: Maarten Lankhorst
>> ---
>> drivers/gpu/drm/i915/intel_display.c | 11 ---
>> 1 file changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c
>> b/drivers/gpu/drm/i915/intel_display.c
>> index 1e42309ec40a..b76778d76035 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -15421,6 +15421,7 @@ static void intel_sanitize_crtc(struct intel_crtc
>> *crtc)
>> WARN_ON(drm_atomic_set_mode_for_crtc(crtc->base.state, NULL) <
>> 0);
>> crtc->base.state->active = crtc->active;
>> crtc->base.enabled = crtc->active;
>> +crtc->base.state->connector_mask = 0;
>>
>> /* Because we only establish the connector -> encoder ->
>> * crtc links if something is active, this means the
>> @@ -15456,20 +15457,24 @@ static void intel_sanitize_encoder(struct
>> intel_encoder *encoder)
>> {
>> struct intel_connector *connector;
>> struct drm_device *dev = encoder->base.dev;
>> +struct drm_crtc *crtc = encoder->base.crtc;
>> bool active = false;
>>
>> /* We need to check both for a crtc link (meaning that the
>> * encoder is active and trying to read from a pipe) and the
>> * pipe itself being active. */
>> -bool has_active_crtc = encoder->base.crtc &&
>> -to_intel_crtc(encoder->base.crtc)->active;
>> +bool has_active_crtc = crtc && crtc->state->active;
>>
>> for_each_intel_connector(dev, connector) {
>> if (connector->base.encoder != &encoder->base)
>> continue;
>>
>> active = true;
>> -break;
>> +if (!has_active_crtc)
>> +break;
>> +
>> +crtc->state->connector_mask |=
>> +1 << drm_connector_index(&connector->base);
> I still think this is the wrong place. Imo this should be done in
> intel_modeset_update_connector_atomic_state. It'd be great if we could
> somehow share the logic with drm_atomic_set_crtc_for_connector even, but
> that's probably over the top.
>
No it should be done sooner. I want to be able use it anywhere in the
.crtc_disable calls without worrying about it..
~Maarten
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 5/6] drm/i915: Update connector_mask during readout.
On Tue, Jan 05, 2016 at 10:05:21AM +0100, Maarten Lankhorst wrote:
> Op 05-01-16 om 09:35 schreef Daniel Vetter:
> > On Mon, Jan 04, 2016 at 12:53:19PM +0100, Maarten Lankhorst wrote:
> >> Signed-off-by: Maarten Lankhorst
> >> ---
> >> drivers/gpu/drm/i915/intel_display.c | 11 ---
> >> 1 file changed, 8 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_display.c
> >> b/drivers/gpu/drm/i915/intel_display.c
> >> index 1e42309ec40a..b76778d76035 100644
> >> --- a/drivers/gpu/drm/i915/intel_display.c
> >> +++ b/drivers/gpu/drm/i915/intel_display.c
> >> @@ -15421,6 +15421,7 @@ static void intel_sanitize_crtc(struct intel_crtc
> >> *crtc)
> >>WARN_ON(drm_atomic_set_mode_for_crtc(crtc->base.state, NULL) <
> >> 0);
> >>crtc->base.state->active = crtc->active;
> >>crtc->base.enabled = crtc->active;
> >> + crtc->base.state->connector_mask = 0;
> >>
> >>/* Because we only establish the connector -> encoder ->
> >> * crtc links if something is active, this means the
> >> @@ -15456,20 +15457,24 @@ static void intel_sanitize_encoder(struct
> >> intel_encoder *encoder)
> >> {
> >>struct intel_connector *connector;
> >>struct drm_device *dev = encoder->base.dev;
> >> + struct drm_crtc *crtc = encoder->base.crtc;
> >>bool active = false;
> >>
> >>/* We need to check both for a crtc link (meaning that the
> >> * encoder is active and trying to read from a pipe) and the
> >> * pipe itself being active. */
> >> - bool has_active_crtc = encoder->base.crtc &&
> >> - to_intel_crtc(encoder->base.crtc)->active;
> >> + bool has_active_crtc = crtc && crtc->state->active;
> >>
> >>for_each_intel_connector(dev, connector) {
> >>if (connector->base.encoder != &encoder->base)
> >>continue;
> >>
> >>active = true;
> >> - break;
> >> + if (!has_active_crtc)
> >> + break;
> >> +
> >> + crtc->state->connector_mask |=
> >> + 1 << drm_connector_index(&connector->base);
> > I still think this is the wrong place. Imo this should be done in
> > intel_modeset_update_connector_atomic_state. It'd be great if we could
> > somehow share the logic with drm_atomic_set_crtc_for_connector even, but
> > that's probably over the top.
> >
> No it should be done sooner. I want to be able use it anywhere in the
> .crtc_disable calls without worrying about it..
Well I don't want to split things up all over. Atm our state recover is a
complete mess, and we need to start recovering some order in it. Updating
related things at completely different places without even a comment
stating why that's required is imo a no-go.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 5/6] drm/i915: Update connector_mask during readout.
Op 05-01-16 om 10:10 schreef Daniel Vetter:
> On Tue, Jan 05, 2016 at 10:05:21AM +0100, Maarten Lankhorst wrote:
>> Op 05-01-16 om 09:35 schreef Daniel Vetter:
>>> On Mon, Jan 04, 2016 at 12:53:19PM +0100, Maarten Lankhorst wrote:
Signed-off-by: Maarten Lankhorst
---
drivers/gpu/drm/i915/intel_display.c | 11 ---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 1e42309ec40a..b76778d76035 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15421,6 +15421,7 @@ static void intel_sanitize_crtc(struct intel_crtc
*crtc)
WARN_ON(drm_atomic_set_mode_for_crtc(crtc->base.state, NULL) <
0);
crtc->base.state->active = crtc->active;
crtc->base.enabled = crtc->active;
+ crtc->base.state->connector_mask = 0;
/* Because we only establish the connector -> encoder ->
* crtc links if something is active, this means the
@@ -15456,20 +15457,24 @@ static void intel_sanitize_encoder(struct
intel_encoder *encoder)
{
struct intel_connector *connector;
struct drm_device *dev = encoder->base.dev;
+ struct drm_crtc *crtc = encoder->base.crtc;
bool active = false;
/* We need to check both for a crtc link (meaning that the
* encoder is active and trying to read from a pipe) and the
* pipe itself being active. */
- bool has_active_crtc = encoder->base.crtc &&
- to_intel_crtc(encoder->base.crtc)->active;
+ bool has_active_crtc = crtc && crtc->state->active;
for_each_intel_connector(dev, connector) {
if (connector->base.encoder != &encoder->base)
continue;
active = true;
- break;
+ if (!has_active_crtc)
+ break;
+
+ crtc->state->connector_mask |=
+ 1 << drm_connector_index(&connector->base);
>>> I still think this is the wrong place. Imo this should be done in
>>> intel_modeset_update_connector_atomic_state. It'd be great if we could
>>> somehow share the logic with drm_atomic_set_crtc_for_connector even, but
>>> that's probably over the top.
>>>
>> No it should be done sooner. I want to be able use it anywhere in the
>> .crtc_disable calls without worrying about it..
> Well I don't want to split things up all over. Atm our state recover is a
> complete mess, and we need to start recovering some order in it. Updating
> related things at completely different places without even a comment
> stating why that's required is imo a no-go.
Ok so if I resubmit with a comment in this hunk it will be acceptable?
~Maarten
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 3/3] drm/i915: DP channel EQ check for use of DP link training optimization
On Mon, 2016-01-04 at 18:44 +0200, Ville Syrjälä wrote:
> On Mon, Jan 04, 2016 at 01:21:24PM +0200, Mika Kahola wrote:
> > Don't use DP link training optimization if channel EQ is not ok. It has
> > been reported that in case of failure in channel EQ check the link training
> > optimization can be enabled and therefore may not be able to reuse the
> > previously computed drive current and pre-emphasis levels.
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91393
> > Signed-off-by: Mika Kahola
> > ---
> > drivers/gpu/drm/i915/intel_dp.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index 3137187..0cd1ccb 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -4354,6 +4354,7 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
> > (!drm_dp_channel_eq_ok(link_status, intel_dp->lane_count))) {
> > DRM_DEBUG_KMS("%s: channel EQ not ok, retraining\n",
> > intel_encoder->base.name);
> > + intel_dp->train_set_valid = false;
> > intel_dp_start_link_train(intel_dp);
> > intel_dp_stop_link_train(intel_dp);
> > }
>
> Should do the same for the MST case I suppose?
Yes, we should do the same for MST case.
Cheers,
Mika
>
> > --
> > 1.9.1
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v9] drm/i915: Avoid writing relocs with addresses in non-canonical form
On Tue, Dec 29, 2015 at 05:28:04PM +, Chris Wilson wrote: > On Tue, Dec 29, 2015 at 06:24:52PM +0100, Michał Winiarski wrote: > > According to PRM, some parts of HW require the addresses to be in > > a canonical form, where bits [63:48] == [47]. Let's convert addresses to > > canonical form prior to relocating and return converted offsets to > > userspace. We also need to make sure that userspace is using addresses > > in canonical form in case of softpin. > > > > v2: Whitespace fixup, gen8_canonical_addr description (Chris, Ville) > > v3: Rebase on top of softpin, fix a hole in relocate_entry, > > s/expect/require (Chris) > > v4: Handle softpin in validate_exec_list (Chris) > > v5: Convert back to canonical form at copy_to_user time (Chris) > > v6: Don't use struct exec_object2 in place of exec_object > > v7: Use sign_extend64 for converting to canonical form (Joonas), > > reject non-canonical and non-page-aligned offset for softpin (Chris) > > v8: Convert back to non-canonical form in a function, > > split the test for EXEC_OBJECT_PINNED (Chris) > > v9: s/canonial/canonical, drop accidental double newline (Chris) > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92699 > > Cc: Chris Wilson > > Cc: Michel Thierry > > Cc: Ville Syrjälä > > Signed-off-by: Michał Winiarski > Reviewed-by: Chris Wilson Queued for -next, with Testcase: and Cc: -fixes added, thanks for the patch. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t] tests/kms_color:Color i-g-t
From: Dhanya
This patch will verify color correction capability of a display driver.
Gamma/CSC/De-gamma for BDW/SKL/BXT and CHT supported.
Signed-off-by: Dhanya
---
tests/.gitignore |1 +
tests/Makefile.sources |1 +
tests/kms_color.c | 1014
3 files changed, 1016 insertions(+)
create mode 100644 tests/kms_color.c
diff --git a/tests/.gitignore b/tests/.gitignore
index 7f20f2b..6fc4782 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -130,6 +130,7 @@ gen7_forcewake_mt
kms_3d
kms_addfb_basic
kms_atomic
+kms_color
kms_crtc_background_color
kms_cursor_crc
kms_draw_crc
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index d594038..f2af648 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -68,6 +68,7 @@ TESTS_progs_M = \
gem_write_read_ring_switch \
kms_addfb_basic \
kms_atomic \
+ kms_color \
kms_cursor_crc \
kms_draw_crc \
kms_fbc_crc \
diff --git a/tests/kms_color.c b/tests/kms_color.c
new file mode 100644
index 000..dd3c2fb
--- /dev/null
+++ b/tests/kms_color.c
@@ -0,0 +1,1014 @@
+/*
+ * Copyright © 2015 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ * Authors:
+ * Dhanya Pillai
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include "drmtest.h"
+#include "drm.h"
+#include "igt_debugfs.h"
+#include "igt_kms.h"
+#include "igt_core.h"
+#include "intel_io.h"
+#include "intel_chipset.h"
+#include "igt_aux.h"
+
+
+IGT_TEST_DESCRIPTION("Test Color Features at Pipe level");
+/*
+
+This tool tests the following color features:
+ CTM, GAMMA(8/10/12/split/legacy) and De-gamma.
+Few negative test cases are also included
+
+Tests willl enable one primary and one sprite plane and apply
+the specific color feature and do the verification by CRC checks.
+*/
+
+#define CSC_MAX_VALS9
+#define BDW_SPLITGAMMA_MAX_VALS512
+#define BDW_8BIT_GAMMA_MAX_VALS256
+#define BDW_10BIT_GAMMA_MAX_VALS 1024
+#define BDW_12BIT_GAMMA_MAX_VALS 513
+#define BDW_MAX_GAMMA ((1 << 24) - 1)
+#define BDW_MIN_GAMMA 0
+
+#define RED_FB 0
+#define GREEN_FB 1
+#define BLUE_FB 2
+
+#ifndef drm_r32g32b32
+struct drm_r32g32b32 {
+ __u32 r32;
+ __u32 g32;
+ __u32 b32;
+ __u32 reserved;
+};
+#endif
+#ifndef drm_palette
+struct drm_palette {
+ struct drm_r32g32b32 lut[0];
+};
+#endif
+#ifndef drm_ctm
+ struct drm_ctm {
+ __s64 ctm_coeff[9];
+};
+#endif
+#ifndef drm_palette_caps
+struct drm_palette_caps {
+ __u32 version;
+ __u32 reserved;
+ __u32 num_samples_before_ctm;
+ __u32 num_samples_after_ctm;
+};
+#endif
+
+enum ctm_color {
+ RED,
+ GREEN,
+ BLUE,
+ REVERSE,
+ NEGATIVE,
+ FRACTION
+};
+
+enum blob {
+ INVALID_BLOB_ID,
+ INVALID_BLOB_DATA,
+ INVALID_BLOB_LENGTH,
+ INVALID_BLOB_ID_SMALL,
+ INVALID_BLOB_NULL
+};
+
+enum color_property {
+ ctm_property,
+ legacy_gamma,
+ gamma_property_8bit,
+ gamma_property_10bit,
+ gamma_property_12bit,
+ gamma_property_split
+};
+static const float ctm_red[9] = {1, 1, 1, 0, 0, 0, 0, 0, 0};
+static const float ctm_green[9] = {0, 0, 0, 1, 1, 1, 0, 0, 0};
+static const float ctm_blue[9] = {0, 0, 0, 0, 0, 0, 1, 1, 1};
+static const float ctm_identity[9] = {1, 0, 0, 0, 1, 0, 0, 0, 1};
+static const float ctm_reverse[9] = {0, 0, 1, 0, 1, 0, 1, 0, 0};
+static const float ctm_negative[9] = {-1, -1, -1, -1, -1, -1, -1, -1, -1};
+static const float ctm_fraction[9] = {1234567.7652, 0.12334, 0.9898989,
0.45454545, 0.12121212, 0.232323, 3.98768, 0.0, 1.22345};
+
+
+struct framebuffer_color {
+ int red;
+ int green;
+ int blue;
+};
+str
Re: [Intel-gfx] [PATCH v2 1/2] mm: Export nr_swap_pages
On Wed, Dec 23, 2015 at 02:26:11PM -0800, Andrew Morton wrote: > On Wed, 23 Dec 2015 17:04:27 -0500 Johannes Weiner wrote: > > > On Thu, Dec 10, 2015 at 10:32:42AM +0100, Daniel Vetter wrote: > > > On Fri, Dec 04, 2015 at 11:09:52AM -0500, Johannes Weiner wrote: > > > > On Fri, Dec 04, 2015 at 03:58:53PM +, Chris Wilson wrote: > > > > > Some modules, like i915.ko, use swappable objects and may try to swap > > > > > them out under memory pressure (via the shrinker). Before doing so, > > > > > they > > > > > want to check using get_nr_swap_pages() to see if any swap space is > > > > > available as otherwise they will waste time purging the object from > > > > > the > > > > > device without recovering any memory for the system. This requires the > > > > > nr_swap_pages counter to be exported to the modules. > > > > > > > > > > Signed-off-by: Chris Wilson > > > > > Cc: "Goel, Akash" > > > > > Cc: Johannes Weiner > > > > > Cc: linux...@kvack.org > > > > > > > > Acked-by: Johannes Weiner > > > > > > Ack for merging this through drm-intel trees for 4.5? I'm a bit unclear > > > who's ack I need for that for linux-mm topics ... > > > > Andrew would be the -mm maintainer. CC'd. > > yup, please go ahead and merge that via the DRM tree. > > nr_swap_pages is a crappy name. That means "number of pages in swap", > which isn't the case. Something like "swap_pages_available" would be > better. > > And your swap_available() isn't good either ;) It can mean "is any swap > online" or "what is the amount of free swap space (in unknown units!)". > I'd call it "swap_is_full()" and put a ! in the caller. But it's > hardly important for a wee little static helper. Yeah it's not super-pretty, but then the entire core mm/shrinker abstraction is more than just a bit leaky (at least i915 has plenty of code to make sure we don't bite our own tail). In case of doubt I prefer the simplest export and avoid the mistake of fake abstraction in the form of an inline helper with a pretty name. Merged to drm-intel.git as-is, but missed the 4.5 train so will only land in 4.6. Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2, 2/4] drm/i915: simplify testing for the global default context
On Mon, Jan 04, 2016 at 01:38:26PM -0800, Jesse Barnes wrote: > On 01/04/2016 11:39 AM, Chris Wilson wrote: > > This series is NAKed. > > Why? Because you want things in a different order? Or do you object to > something in Dave's reply? The series was intended as a code cleanup and in the process tried to introduce a false concept that I objected to. Since the cleanup was not predicated upon that idea, the patches would have been much tidier without it. The fundamental issue at stake here is execlists behaves badly and we have to futz around in higher level code to undo that mistake. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4] drm/i915/guc: Move GuC wq_check_space to alloc_request_extras
On Wed, Dec 23, 2015 at 03:39:04PM +, Dave Gordon wrote:
> On 16/12/15 19:45, yu@intel.com wrote:
> >From: Alex Dai
> >
> >Split GuC work queue space checking from submission and move it to
> >ring_alloc_request_extras. The reason is that failure in later
> >i915_add_request() won't be handled. In the case timeout happens,
> >driver can return early in order to handle the error.
> >
> >v1: Move wq_reserve_space to ring_reserve_space
> >v2: Move wq_reserve_space to alloc_request_extras (Chris Wilson)
> >v3: The work queue head pointer is cached by driver now. So we can
> > quickly return if space is available.
> > s/reserve/check/g (Dave Gordon)
> >v4: Update cached wq head after ring doorbell; check wq space before
> > ring doorbell in case unexpected error happens; call wq space
> > check only when GuC submission is enabled. (Dave Gordon)
> >
> >Signed-off-by: Alex Dai
>
> LGTM.
> Reviewed-by: Dave Gordon
Queued for -next, thanks for the patch.
-Daniel
>
> >diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c
> >b/drivers/gpu/drm/i915/i915_guc_submission.c
> >index ef20071..7554d16 100644
> >--- a/drivers/gpu/drm/i915/i915_guc_submission.c
> >+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> >@@ -247,6 +247,9 @@ static int guc_ring_doorbell(struct i915_guc_client *gc)
> > db_exc.cookie = 1;
> > }
> >
> >+/* Finally, update the cached copy of the GuC's WQ head */
> >+gc->wq_head = desc->head;
> >+
> > kunmap_atomic(base);
> > return ret;
> > }
> >@@ -472,28 +475,30 @@ static void guc_fini_ctx_desc(struct intel_guc *guc,
> > sizeof(desc) * client->ctx_index);
> > }
> >
> >-/* Get valid workqueue item and return it back to offset */
> >-static int guc_get_workqueue_space(struct i915_guc_client *gc, u32 *offset)
> >+int i915_guc_wq_check_space(struct i915_guc_client *gc)
> > {
> > struct guc_process_desc *desc;
> > void *base;
> > u32 size = sizeof(struct guc_wq_item);
> > int ret = -ETIMEDOUT, timeout_counter = 200;
> >
> >+if (!gc)
> >+return 0;
> >+
> >+/* Quickly return if wq space is available since last time we cache the
> >+ * head position. */
> >+if (CIRC_SPACE(gc->wq_tail, gc->wq_head, gc->wq_size) >= size)
> >+return 0;
> >+
> > base = kmap_atomic(i915_gem_object_get_page(gc->client_obj, 0));
> > desc = base + gc->proc_desc_offset;
> >
> > while (timeout_counter-- > 0) {
> >-if (CIRC_SPACE(gc->wq_tail, desc->head, gc->wq_size) >= size) {
> >-*offset = gc->wq_tail;
> >+gc->wq_head = desc->head;
> >
> >-/* advance the tail for next workqueue item */
> >-gc->wq_tail += size;
> >-gc->wq_tail &= gc->wq_size - 1;
> >-
> >-/* this will break the loop */
> >-timeout_counter = 0;
> >+if (CIRC_SPACE(gc->wq_tail, gc->wq_head, gc->wq_size) >= size) {
> > ret = 0;
> >+break;
> > }
> >
> > if (timeout_counter)
> >@@ -511,12 +516,16 @@ static int guc_add_workqueue_item(struct
> >i915_guc_client *gc,
> > enum intel_ring_id ring_id = rq->ring->id;
> > struct guc_wq_item *wqi;
> > void *base;
> >-u32 tail, wq_len, wq_off = 0;
> >-int ret;
> >+u32 tail, wq_len, wq_off, space;
> >+
> >+space = CIRC_SPACE(gc->wq_tail, gc->wq_head, gc->wq_size);
> >+if (WARN_ON(space < sizeof(struct guc_wq_item)))
> >+return -ENOSPC; /* shouldn't happen */
> >
> >-ret = guc_get_workqueue_space(gc, &wq_off);
> >-if (ret)
> >-return ret;
> >+/* postincrement WQ tail for next time */
> >+wq_off = gc->wq_tail;
> >+gc->wq_tail += sizeof(struct guc_wq_item);
> >+gc->wq_tail &= gc->wq_size - 1;
> >
> > /* For now workqueue item is 4 DWs; workqueue buffer is 2 pages. So we
> > * should not have the case where structure wqi is across page, neither
> >diff --git a/drivers/gpu/drm/i915/intel_guc.h
> >b/drivers/gpu/drm/i915/intel_guc.h
> >index 0e048bf..5cf555d 100644
> >--- a/drivers/gpu/drm/i915/intel_guc.h
> >+++ b/drivers/gpu/drm/i915/intel_guc.h
> >@@ -43,6 +43,7 @@ struct i915_guc_client {
> > uint32_t wq_offset;
> > uint32_t wq_size;
> > uint32_t wq_tail;
> >+uint32_t wq_head;
> >
> > /* GuC submission statistics & status */
> > uint64_t submissions[I915_NUM_RINGS];
> >@@ -123,5 +124,6 @@ int i915_guc_submit(struct i915_guc_client *client,
> > struct drm_i915_gem_request *rq);
> > void i915_guc_submission_disable(struct drm_device *dev);
> > void i915_guc_submission_fini(struct drm_device *dev);
> >+int i915_guc_wq_check_space(struct i915_guc_client *client);
> >
> > #endif
> >diff --git a/drivers/gpu/drm/i915/intel_lrc.c
> >b/drivers/gpu/drm/i915/intel_lrc.c
> >index 272f36f..cd232d2 100644
> >--- a/drivers/gpu/drm/i915/int
Re: [Intel-gfx] [PATCH 2/3] drm/i915: Move HAS_RUNTIME_PM definition to the platform definition.
On Tue, 05 Jan 2016, Rodrigo Vivi wrote:
> No functional changes with this patch. The idea is just to organize
> the platform features in a standard place making new platform aditions
> easily and possible to see all the present features of the platform on
> the intel info dumped information at dmesg.
>
> (I just wonder why Ivy Bridge doesn't have runtime pm.
> So, let's use this v1 to start the discussion).
>
> Signed-off-by: Rodrigo Vivi
> ---
> drivers/gpu/drm/i915/i915_drv.c | 4
> drivers/gpu/drm/i915/i915_drv.h | 6 ++
> 2 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 6fea26f..cb8adb5 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -217,6 +217,7 @@ static const struct intel_device_info
> intel_sandybridge_m_info = {
> .gen = 6, .is_mobile = 1, .num_pipes = 2,
> .need_gfx_hws = 1, .has_hotplug = 1,
> .has_fbc = 1,
> + .has_runtime_pm = 1,
Missing for sandybridge_d?
BR,
Jani.
> .ring_mask = RENDER_RING | BSD_RING | BLT_RING,
> .has_llc = 1,
> GEN_DEFAULT_PIPEOFFSETS,
> @@ -252,6 +253,7 @@ static const struct intel_device_info
> intel_ivybridge_q_info = {
> #define VLV_FEATURES \
> .gen = 7, .num_pipes = 2, \
> .has_psr = 1, \
> + .has_runtime_pm = 1, \
> .need_gfx_hws = 1, .has_hotplug = 1, \
> .ring_mask = RENDER_RING | BSD_RING | BLT_RING, \
> .display_mmio_offset = VLV_DISPLAY_BASE, \
> @@ -274,6 +276,7 @@ static const struct intel_device_info
> intel_valleyview_d_info = {
> .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING, \
> .has_ddi = 1, \
> .has_psr = 1, \
> + .has_runtime_pm = 1,\
> .has_fpga_dbg = 1
>
> static const struct intel_device_info intel_haswell_d_info = {
> @@ -315,6 +318,7 @@ static const struct intel_device_info
> intel_cherryview_info = {
> .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
> .is_cherryview = 1,
> .has_psr = 1,
> + .has_runtime_pm = 1,
> .display_mmio_offset = VLV_DISPLAY_BASE,
> GEN_CHV_PIPEOFFSETS,
> CURSOR_OFFSETS,
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 20840f0..8143a51 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -764,6 +764,7 @@ struct intel_csr {
> func(is_preliminary) sep \
> func(has_fbc) sep \
> func(has_psr) sep \
> + func(has_runtime_pm) sep \
> func(has_pipe_cxsr) sep \
> func(has_hotplug) sep \
> func(cursor_needs_physical) sep \
> @@ -2606,10 +2607,7 @@ struct drm_i915_cmd_table {
> #define HAS_DDI(dev) (INTEL_INFO(dev)->has_ddi)
> #define HAS_FPGA_DBG_UNCLAIMED(dev) (INTEL_INFO(dev)->has_fpga_dbg)
> #define HAS_PSR(dev) (INTEL_INFO(dev)->has_psr)
> -#define HAS_RUNTIME_PM(dev) (IS_GEN6(dev) || IS_HASWELL(dev) || \
> - IS_BROADWELL(dev) || IS_VALLEYVIEW(dev) || \
> - IS_CHERRYVIEW(dev) || IS_SKYLAKE(dev) || \
> - IS_KABYLAKE(dev))
> +#define HAS_RUNTIME_PM(dev) (INTEL_INFO(dev)->has_runtime_pm)
> #define HAS_RC6(dev) (INTEL_INFO(dev)->gen >= 6)
> #define HAS_RC6p(dev)(INTEL_INFO(dev)->gen == 6 ||
> IS_IVYBRIDGE(dev))
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/3] drm/i915: Move HAS_CORE_RING_FREQ definition to the platform definition.
On Tue, 05 Jan 2016, Rodrigo Vivi wrote:
> No functional changes with this patch. The idea is just to organize
> the platform features in a standard place making new platform aditions
> easily and possible to see all the present features of the platform on
> the intel info dumped information at dmesg.
>
> Also for this one it is better to put the ones that support than
> skip for every atom based platform.
>
> Signed-off-by: Rodrigo Vivi
> ---
> drivers/gpu/drm/i915/i915_drv.c | 2 ++
> drivers/gpu/drm/i915/i915_drv.h | 5 ++---
> 2 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index cb8adb5..ed34164 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -218,6 +218,7 @@ static const struct intel_device_info
> intel_sandybridge_m_info = {
> .need_gfx_hws = 1, .has_hotplug = 1,
> .has_fbc = 1,
> .has_runtime_pm = 1,
> + .has_core_ring_freq = 1,
Missing for sandybridge_d?
BR,
Jani.
> .ring_mask = RENDER_RING | BSD_RING | BLT_RING,
> .has_llc = 1,
> GEN_DEFAULT_PIPEOFFSETS,
> @@ -228,6 +229,7 @@ static const struct intel_device_info
> intel_sandybridge_m_info = {
> .gen = 7, .num_pipes = 3, \
> .need_gfx_hws = 1, .has_hotplug = 1, \
> .has_fbc = 1, \
> + .has_core_ring_freq = 1, \
> .ring_mask = RENDER_RING | BSD_RING | BLT_RING, \
> .has_llc = 1, \
> GEN_DEFAULT_PIPEOFFSETS, \
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 8143a51..cb08d7d 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -765,6 +765,7 @@ struct intel_csr {
> func(has_fbc) sep \
> func(has_psr) sep \
> func(has_runtime_pm) sep \
> + func(has_core_ring_freq) sep \
> func(has_pipe_cxsr) sep \
> func(has_hotplug) sep \
> func(cursor_needs_physical) sep \
> @@ -2619,9 +2620,7 @@ struct drm_i915_cmd_table {
> #define HAS_RESOURCE_STREAMER(dev) (IS_HASWELL(dev) || \
> INTEL_INFO(dev)->gen >= 8)
>
> -#define HAS_CORE_RING_FREQ(dev) (INTEL_INFO(dev)->gen >= 6 && \
> - !IS_VALLEYVIEW(dev) && !IS_CHERRYVIEW(dev) && \
> - !IS_BROXTON(dev))
> +#define HAS_CORE_RING_FREQ(dev) (INTEL_INFO(dev)->has_core_ring_freq)
>
> #define INTEL_PCH_DEVICE_ID_MASK 0xff00
> #define INTEL_PCH_IBX_DEVICE_ID_TYPE 0x3b00
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/skl: Increase ddb blocks to support large cursor sizes
On Mon, Jan 04, 2016 at 07:18:26PM +0530, Kumar, Shobhit wrote: > On 12/23/2015 08:22 AM, Kumar, Shobhit wrote: > >On 12/21/2015 05:39 PM, Daniel Vetter wrote: > >>On Fri, Dec 18, 2015 at 07:24:19AM -0800, Matt Roper wrote: > >>>On Fri, Dec 18, 2015 at 07:14:17AM -0800, Matt Roper wrote: > On Fri, Dec 18, 2015 at 05:10:12PM +0200, Ville Syrjälä wrote: > >On Fri, Dec 18, 2015 at 06:58:58AM -0800, Matt Roper wrote: > >>On Fri, Dec 18, 2015 at 12:35:47PM +0200, Ville Syrjälä wrote: > >>>On Wed, Dec 16, 2015 at 07:06:20PM -0800, Radhakrishna Sripada > >>>wrote: > Original value of 32 blocks is not sufficient when using cursor > size of > 256x256 causing FIFO underruns when the reworked wm > caluclations in > > commit 024c9045221fe45482863c47c4b4c47d37f97cbf > Author: Matt Roper > Date: Thu Sep 24 15:53:11 2015 -0700 > > drm/i915/skl: Eliminate usage of pipe_wm_parameters from > SKL-style WM (v4) > >>> > >>>Well that commit is obviously incorrect. It's now using the pipe src > >>>width as the plane width for all planes. > >>> > >> > >>Yeah, we already noted that bug in another email thread, but decided > >>that it was unrelated to the problems Radhakrishna is facing. > >>Radhakrishna is only using a cursor (which doesn't use that buggy > >>function) > > > >Pop quiz: what does it use then? > > All non-cursor planes (i.e., primary+sprite). Cursors use a fixed DDB > allocation (currently 32 blocks as suggested by bspec, but > Radhakrishna's testing has found this to be too small, so his patch > here > is bumping that number up. > > >>> > >>>To elaborate on this some more, we have some suspicions about why the > >>>bspec-suggested 32 blocks might not be enough. There's some new > >>>workaround information in the bspec about sometimes needing to disable > >>>the system agent (SAGV) on non-BXT gen9, depending on the configuration > >>>and that's not something that's been implemented in our driver code yet. > >>>There's also another new workaround that checks raw system memory > >>>bandwidth and adjusts watermark programming that hasn't been implemented > >>>yet either. It could be that one of those two missing workarounds is > >>>the true culprit, but Radhakrishna's patch here looks like a reasonable > >>>short-term workaround; my main worry is that if he needs to bump the > >>>hardcoded 32 up to to 52 for the single pipe case, he might also need to > >>>bump the hardcoded 8 up as well for the multi-pipe case; I don't think > >>>anyone has tested that yet (and this seems to be a SKL-specific problem, > >>>so I can't reproduce it on my BXT). > >> > >>This needs at least a very big comment that we're just hacking around > >>with > >>duct-tape. Also allocating DDB shouldn't matter for correctness, as long > >>as we compute the WM values correctly. I wonder how this fixes anything > >>really (except making it a bit harder to hit perhaps due to the enlarged > >>buffering it affords for the cursor). > > > >Agree Daniel, but in our detailed testing this is found to have fixed > >reliably all known flickers. I recommend to have a detailed comment and > >*put* the duct-tape. I can have a rerun of detailed tests again to > >ensure that its indeed fixing everything. > > > >> > >>Imo better to just implement the other workarounds first, then see what's > >>left. > > > >I know there are few known workarounds pending. Lets continue > >implementing them and more. But with Matt's patch not fixing the issue, > >lets take this as an immediate short term fix and come back to remove > >the duct-tape when we have correct fix. > > I experimented and found that reverting this patch but having arbitrated > display bandwidth based workaround to increase latency values for all levels > is fixing the flicker. Will be working on this along with Mahesh, unless > Matt, you have a patch for this already. Excellent, really happy that we can go ahead with a reasonable looking fix right away here. Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Add rpm get/put in i915_shrinker_oom
On Tue, Dec 29, 2015 at 11:58:26AM +, Chris Wilson wrote: > On Tue, Dec 29, 2015 at 12:35:39PM +0530, Praveen Paneri wrote: > > i915_gem_shrink_all() will scan the bound list only if device is not > > suspended but in OOM scenarios it becomes absolutely necessary to > > release as much memory as possible. So, adding rpm get/put in > > i915_shrinker_oom() to ensure shrinking of bound objects in OOM > > scenario. > > > > Signed-off-by: Praveen Paneri > Reviewed-by: Chris Wilson Both applied to dinq, thanks. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 21/32] drm/i915: Broadwell execlists needs exactly the same seqno w/a as legacy
On Mon, Jan 04, 2016 at 01:34:46PM -0800, Jesse Barnes wrote: > On 12/11/2015 03:33 AM, Chris Wilson wrote: > > +* Note that this effectively effectively stalls the read by the time > > +* it takes to do a memory transaction, which more or less ensures > > +* that the write from the GPU has sufficient time to invalidate > > +* the CPU cacheline. Alternatively we could delay the interrupt from > > +* the CS ring to give the write time to land, but that would incur > > +* a delay after every batch i.e. much more frequent than a delay > > +* when waiting for the interrupt (with the same net latency). > > */ > > + struct drm_i915_private *dev_priv = ring->i915; > > + POSTING_READ_FW(RING_ACTHD(ring->mmio_base)); > > + > > intel_flush_status_page(ring, I915_GEM_HWS_INDEX); > > Funnily enough, the interrupt ought to provide the same behavior as the MMIO > read, i.e. flush outstanding system memory writes ahead of it. The fact that > we need it *plus* a CPU cache flush definitely means we're still missing > something... It is purely a timing issue (aside from bxt-a requiring the w/a). A udelay() works just as well, but the question is what value to wait for, which is where the above empiricism kicks in. (Another example would be adding 32 dword writes into the ring between the seqno and MI_USER_INTERRUPT). -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/6] drm: Create Color Management DRM properties
On Wed, Dec 23, 2015 at 09:47:00AM +, Daniel Stone wrote:
> Hi,
>
> On 21 December 2015 at 12:38, Daniel Vetter wrote:
> > On Fri, Dec 18, 2015 at 04:53:28PM +, Daniel Stone wrote:
> >> > +struct drm_r32g32b32 {
> >> > + /*
> >> > +* Data is in U8.24 fixed point format.
> >> > +* All platforms support values within [0, 1.0] range,
> >> > +* for Red, Green and Blue colors.
> >> > +*/
> >> > + __u32 r32;
> >> > + __u32 g32;
> >> > + __u32 b32;
> >> > + __u32 reserved;
> >> > +};
> >>
> >> Wait, it's U8.24 (i.e. 0 -> 255.0999403953517), but the supported
> >> range is [0..1.0]? What am I missing?
> >
> > Probably means:
> > - all platforms MUST support [0.0, 1.0] range, inclusive
> > - platforms CAN support larger values (which can make sense in degamma
> > tables if your CTM will shrink things down again).
>
> Ah, makes sense.
>
> >> I also don't have a good answer on how to support non-CM-aware
> >> clients. Currently they'll just blindly carry around the correction
> >> factors from previous clients, which is _really_ bad: imagine you have
> >> Weston ported to use KMS/CM to correct perfectly, and then start
> >> Mutter/GNOME which still corrects perfectly, but using lcms and
> >> various client-side compensation rather than the CM engine. Your
> >> output will now be double-corrected, i.e. wrong, because Mutter won't
> >> know to reset the CM properties.
> >
> > Hm yeah, I think legacy entry points should remap to atomic ones.
> > Otherwise things are massively inconsistent (and we have a problem
> > figuring out when/which table applies in the driver). One problem with
> > that approach is that the legacy table has the assumption of a fixed
> > 256 entries (well we expose the size somewhere, but that's what everyone
> > uses). At least on some platforms, the full-blown table used by these
> > patches isn't even an integer multiple of that.
>
> It's not even a legacy vs. atomic thing, this can happen in
> pure-atomic as well. Same as the render-compression plane property
> that I just replied to here as well.
>
> - Weston starts and sets up palette/CTM properties
> - VT switch to Mutter, which isn't aware of new properties
> - Mutter atomically sets new plane state, containing framebuffer which
> is already colour-corrected for the target
> - result is now double-corrected as Mutter didn't know to unset the
> old properties
>
> IOW, in the current model, any user of CM has to explicitly unset it
> before handover (not always actually possible), or any generic
> userspace must unset every property it sees and doesn't know about,
> hoping that setting to 0 will do the right thing.
>
> Or we could add another client cap, which would prevent the CM
> properties from ever being exposed to userspace which doesn't know how
> to work with it. Passing the client caps through to
> plane_duplicate_state also means that we can unset the CM properties
> at this early point for unaware clients. This would mean that we
> wouldn't have to have code to explicitly unset it in, e.g., every
> legacy hook.
We don't have any support for unsetting anything really. Same argument you
make for CM here applies to rotation too. The only generic solution (if
you really care about this) would be for logind to once sample all atomic
state on boot-up, and force-restore that state when switching. Restoring
atomic state doesn't require userspace to understanding the semantics
really.
This kind of force-restoring is probably something we should implement in
the fbdev emulation, which would cover about 99% of all cases where
developers/users want to run different compositors. Or fbdev should simply
reset CM state, like it does for rotation already (that part is easy to
add, but indeed seems to be missing).
Trying to create an ad-hoc solution (using opt-in flags) to this problem
for every single feature seems pointless imo.
> >> About the only thing I can think of is to add a
> >> DRM_CLIENT_CAP_COLOR_MGMT, and modify drm_atomic_*_duplicate_state to
> >> take client caps (passed in from file_priv with the atomic ioctl, or
> >> explicitly set by other kernel code, according to its capabilities),
> >> and if the CM cap is not set, clear out the blobs when duplicating
> >> state.
>
> (As here.)
>
> >> All of this also needs to be backed up by a lot more extensive IGT,
> >> including disabling (from _every_ mode, not just 12-bit mode on BDW)
> >> via setting blob == NULL, testing the various depths (I still don't
> >> understand the difference between 8 and 10-bit on CHV), legacy gamma
> >> hooks when using CM, testing reset/dumb clients when the above
> >> duplicate_state issue is resolved, and especially the boundary cases
> >> in conversions (e.g. the sign wraparound on CHV). The documentation
> >> also _really_ needs fixing to be consistent with the code, and
> >> consistent with itself. When that's done, I think I'll be
> >> better-placed to do a secon
Re: [Intel-gfx] [PATCH v2 1/5] drm/i915/guc: Expose (intel)_lr_context_size()
On Fri, Dec 18, 2015 at 12:00:08PM -0800, yu@intel.com wrote:
> From: Dave Gordon
>
> The GuC code needs to know the size of a logical context, so we
> expose get_lr_context_size(), renaming it intel_lr_context__size()
> to fit the naming conventions for nonstatic functions.
>
> For: VIZ-2021
> Signed-off-by: Dave Gordon
> Signed-off-by: Alex Dai
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c
> b/drivers/gpu/drm/i915/intel_lrc.c
> index e5fb8ea..7a6b896 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -2399,7 +2399,7 @@ void intel_lr_context_free(struct intel_context *ctx)
> }
> }
>
> -static uint32_t get_lr_context_size(struct intel_engine_cs *ring)
> +uint32_t intel_lr_context_size(struct intel_engine_cs *ring)
As a rule of thumb, non-static functions should have kerneldoc within
drm/i915. At least in the files where we bothered with kerneldoc already.
Please do a follow-up patch to remedy this.
Thanks, Daniel
> {
> int ret = 0;
>
> @@ -2467,7 +2467,7 @@ int intel_lr_context_deferred_alloc(struct
> intel_context *ctx,
> WARN_ON(ctx->legacy_hw_ctx.rcs_state != NULL);
> WARN_ON(ctx->engine[ring->id].state);
>
> - context_size = round_up(get_lr_context_size(ring), 4096);
> + context_size = round_up(intel_lr_context_size(ring), 4096);
>
> /* One extra page as the sharing data between driver and GuC */
> context_size += PAGE_SIZE * LRC_PPHWSP_PN;
> diff --git a/drivers/gpu/drm/i915/intel_lrc.h
> b/drivers/gpu/drm/i915/intel_lrc.h
> index 0b821b9..ae90f86 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.h
> +++ b/drivers/gpu/drm/i915/intel_lrc.h
> @@ -84,6 +84,7 @@ static inline void intel_logical_ring_emit_reg(struct
> intel_ringbuffer *ringbuf,
> #define LRC_STATE_PN (LRC_PPHWSP_PN + 1)
>
> void intel_lr_context_free(struct intel_context *ctx);
> +uint32_t intel_lr_context_size(struct intel_engine_cs *ring);
> int intel_lr_context_deferred_alloc(struct intel_context *ctx,
> struct intel_engine_cs *ring);
> void intel_lr_context_unpin(struct drm_i915_gem_request *req);
> --
> 2.5.0
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Add rpm get/put in i915_shrinker_oom
On Tue, Jan 05, 2016 at 11:16:52AM +0100, Daniel Vetter wrote: > On Tue, Dec 29, 2015 at 11:58:26AM +, Chris Wilson wrote: > > On Tue, Dec 29, 2015 at 12:35:39PM +0530, Praveen Paneri wrote: > > > i915_gem_shrink_all() will scan the bound list only if device is not > > > suspended but in OOM scenarios it becomes absolutely necessary to > > > release as much memory as possible. So, adding rpm get/put in > > > i915_shrinker_oom() to ensure shrinking of bound objects in OOM > > > scenario. > > > > > > Signed-off-by: Praveen Paneri > > Reviewed-by: Chris Wilson > > Both applied to dinq, thanks. Well that didn't work since it doesn't compile. My tree lacks intel_runtime_pm_get_noidle. Please resend with prerequisite patches included in the patch series. Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 0/5] Add GuC ADS (Addition Data Structure)
On Mon, Jan 04, 2016 at 07:11:29PM +, Dave Gordon wrote: > On 18/12/15 20:00, yu@intel.com wrote: > >From: Alex Dai > > > >The GuC firmware uses this for various purposes. The ADS itself is a chunk of > >memory created by driver to share with GuC. This series creates the GuC ADS > >object and setup some basic settings for it. > > > >This version addresses some comments from Chris W. Tidy up some code; replace > >kmap_atomic by kmap etc. > > > >Alex Dai (4): > > drm/i915/guc: Add GuC ADS (Addition Data Structure) - allocation > > drm/i915/guc: Add GuC ADS - scheduler policies > > drm/i915/guc: Add GuC ADS - MMIO reg state > > drm/i915/guc: Add GuC ADS - enabling ADS > > > >Dave Gordon (1): > > drm/i915/guc: Expose (intel)_lr_context_size() > > > > drivers/gpu/drm/i915/i915_guc_reg.h| 1 + > > drivers/gpu/drm/i915/i915_guc_submission.c | 95 > > drivers/gpu/drm/i915/intel_guc.h | 2 + > > drivers/gpu/drm/i915/intel_guc_fwif.h | 113 > > - > > drivers/gpu/drm/i915/intel_guc_loader.c| 7 ++ > > drivers/gpu/drm/i915/intel_lrc.c | 4 +- > > drivers/gpu/drm/i915/intel_lrc.h | 1 + > > 7 files changed, 220 insertions(+), 3 deletions(-) > > > > For the whole series, > > Reviewed-by: Dave Gordon All 5 merged to dinq, thanks. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 02/15] drm/i915/bios: fix header define name for intel_bios.h
On Mon, Dec 21, 2015 at 03:10:53PM +0200, Jani Nikula wrote:
> Just for OCD.
>
> Signed-off-by: Jani Nikula
Reviewed-by: Daniel Vetter
> ---
> drivers/gpu/drm/i915/intel_bios.h | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_bios.h
> b/drivers/gpu/drm/i915/intel_bios.h
> index 2dc46a98c332..21c162e01189 100644
> --- a/drivers/gpu/drm/i915/intel_bios.h
> +++ b/drivers/gpu/drm/i915/intel_bios.h
> @@ -25,8 +25,8 @@
> *
> */
>
> -#ifndef _I830_BIOS_H_
> -#define _I830_BIOS_H_
> +#ifndef _INTEL_BIOS_H_
> +#define _INTEL_BIOS_H_
>
> /**
> * struct vbt_header - VBT Header structure
> @@ -983,4 +983,4 @@ enum mipi_gpio_pin_index {
> MIPI_GPIO_MAX
> };
>
> -#endif /* _I830_BIOS_H_ */
> +#endif /* _INTEL_BIOS_H_ */
> --
> 2.1.4
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 03/15] drm/i915/bios: split the MIPI DSI VBT block parsing to two
On Mon, Dec 21, 2015 at 03:10:54PM +0200, Jani Nikula wrote:
> There's two blocks to parse, have one function per block. The existing
> one cuts neatly into two.
>
> Signed-off-by: Jani Nikula
Reviewed-by: Daniel Vetter
> ---
> drivers/gpu/drm/i915/intel_bios.c | 27 +++
> 1 file changed, 19 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_bios.c
> b/drivers/gpu/drm/i915/intel_bios.c
> index d487f602a10e..91540ab15e0b 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -765,16 +765,12 @@ static u8 *goto_next_sequence(u8 *data, int *size)
> }
>
> static void
> -parse_mipi(struct drm_i915_private *dev_priv, const struct bdb_header *bdb)
> +parse_mipi_config(struct drm_i915_private *dev_priv,
> + const struct bdb_header *bdb)
> {
> const struct bdb_mipi_config *start;
> - const struct bdb_mipi_sequence *sequence;
> const struct mipi_config *config;
> const struct mipi_pps_data *pps;
> - u8 *data;
> - const u8 *seq_data;
> - int i, panel_id, seq_size;
> - u16 block_size;
>
> /* parse MIPI blocks only if LFP type is MIPI */
> if (!dev_priv->vbt.has_mipi)
> @@ -820,8 +816,22 @@ parse_mipi(struct drm_i915_private *dev_priv, const
> struct bdb_header *bdb)
>
> /* We have mandatory mipi config blocks. Initialize as generic panel */
> dev_priv->vbt.dsi.panel_id = MIPI_DSI_GENERIC_PANEL_ID;
> +}
> +
> +static void
> +parse_mipi_sequence(struct drm_i915_private *dev_priv,
> + const struct bdb_header *bdb)
> +{
> + const struct bdb_mipi_sequence *sequence;
> + const u8 *seq_data;
> + u8 *data;
> + u16 block_size;
> + int i, panel_id, seq_size;
> +
> + /* Only our generic panel driver uses the sequence block. */
> + if (dev_priv->vbt.dsi.panel_id != MIPI_DSI_GENERIC_PANEL_ID)
> + return;
>
> - /* Check if we have sequence block as well */
> sequence = find_section(bdb, BDB_MIPI_SEQUENCE);
> if (!sequence) {
> DRM_DEBUG_KMS("No MIPI Sequence found, parsing complete\n");
> @@ -1359,7 +1369,8 @@ intel_bios_init(struct drm_i915_private *dev_priv)
> parse_driver_features(dev_priv, bdb);
> parse_edp(dev_priv, bdb);
> parse_psr(dev_priv, bdb);
> - parse_mipi(dev_priv, bdb);
> + parse_mipi_config(dev_priv, bdb);
> + parse_mipi_sequence(dev_priv, bdb);
> parse_ddi_ports(dev_priv, bdb);
>
> if (bios)
> --
> 2.1.4
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 04/15] drm/i915/bios: have get_blocksize() support MIPI sequence block v3+
On Mon, Dec 21, 2015 at 03:10:55PM +0200, Jani Nikula wrote:
> Have get_blocksize() support the special case of MIPI sequence block v3+
> which has a separate field for size. Provide and use abstractions for
> getting the blocksize given a pointer to the block "envelope",
> i.e. pointer to the block id, and given a pointer to the block payload
> data.
>
> Signed-off-by: Jani Nikula
Reviewed-by: Daniel Vetter
> ---
> drivers/gpu/drm/i915/intel_bios.c | 36 ++--
> 1 file changed, 18 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_bios.c
> b/drivers/gpu/drm/i915/intel_bios.c
> index 91540ab15e0b..7393596df37d 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -58,6 +58,22 @@
>
> static int panel_type;
>
> +/* Get BDB block size given a pointer to Block ID. */
> +static u32 _get_blocksize(const u8 *block_base)
> +{
> + /* The MIPI Sequence Block v3+ has a separate size field. */
> + if (*block_base == BDB_MIPI_SEQUENCE && *(block_base + 3) >= 3)
> + return *((const u32 *)(block_base + 4));
> + else
> + return *((const u16 *)(block_base + 1));
> +}
> +
> +/* Get BDB block size give a pointer to data after Block ID and Block Size.
> */
> +static u32 get_blocksize(const void *block_data)
> +{
> + return _get_blocksize(block_data - 3);
> +}
> +
> static const void *
> find_section(const void *_bdb, int section_id)
> {
> @@ -74,14 +90,8 @@ find_section(const void *_bdb, int section_id)
> /* walk the sections looking for section_id */
> while (index + 3 < total) {
> current_id = *(base + index);
> - index++;
> -
> - current_size = *((const u16 *)(base + index));
> - index += 2;
> -
> - /* The MIPI Sequence Block v3+ has a separate size field. */
> - if (current_id == BDB_MIPI_SEQUENCE && *(base + index) >= 3)
> - current_size = *((const u32 *)(base + index + 1));
> + current_size = _get_blocksize(base + index);
> + index += 3;
>
> if (index + current_size > total)
> return NULL;
> @@ -95,16 +105,6 @@ find_section(const void *_bdb, int section_id)
> return NULL;
> }
>
> -static u16
> -get_blocksize(const void *p)
> -{
> - u16 *block_ptr, block_size;
> -
> - block_ptr = (u16 *)((char *)p - 2);
> - block_size = *block_ptr;
> - return block_size;
> -}
> -
> static void
> fill_detail_timing_data(struct drm_display_mode *panel_fixed_mode,
> const struct lvds_dvo_timing *dvo_timing)
> --
> 2.1.4
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 05/15] drm/i915/bios: abstract finding the panel sequence block
On Mon, Dec 21, 2015 at 03:10:56PM +0200, Jani Nikula wrote:
> Make the whole thing easier to read.
>
> Signed-off-by: Jani Nikula
> ---
> drivers/gpu/drm/i915/intel_bios.c | 76
> +--
> 1 file changed, 42 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_bios.c
> b/drivers/gpu/drm/i915/intel_bios.c
> index 7393596df37d..9511a5341562 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -697,7 +697,7 @@ parse_psr(struct drm_i915_private *dev_priv, const struct
> bdb_header *bdb)
> dev_priv->vbt.psr.tp2_tp3_wakeup_time = psr_table->tp2_tp3_wakeup_time;
> }
>
> -static u8 *goto_next_sequence(u8 *data, int *size)
> +static u8 *goto_next_sequence(u8 *data, u16 *size)
> {
> u16 len;
> int tmp = *size;
> @@ -818,15 +818,52 @@ parse_mipi_config(struct drm_i915_private *dev_priv,
> dev_priv->vbt.dsi.panel_id = MIPI_DSI_GENERIC_PANEL_ID;
> }
>
> +/* Find the sequence block and size for the given panel. */
> +static const u8 *
> +find_panel_sequence_block(const struct bdb_mipi_sequence *sequence,
> + u16 panel_id, u16 *seq_size)
> +{
> + u32 total = get_blocksize(sequence);
> + const u8 *data = &sequence->data[0];
> + u8 current_id;
> + u16 current_size;
> + int index = 0;
> + int i;
> +
> + for (i = 0; i < MAX_MIPI_CONFIGURATIONS && index + 3 < total; i++) {
Commit message should mention that you make the parsin more robust and
ensure we don't walk over the end of the allocated buffer.
> + current_id = *(data + index);
> + index++;
> +
> + current_size = *((const u16 *)(data + index));
> + index += 2;
> +
> + if (index + current_size > total) {
> + DRM_ERROR("Invalid sequence block\n");
> + return NULL;
> + }
> +
> + if (current_id == panel_id) {
> + *seq_size = current_size;
> + return data + index;
> + }
> +
> + index += current_size;
> + }
> +
> + DRM_ERROR("Sequence block detected but no valid configuration\n");
> +
> + return NULL;
> +}
> +
> static void
> parse_mipi_sequence(struct drm_i915_private *dev_priv,
> const struct bdb_header *bdb)
> {
> const struct bdb_mipi_sequence *sequence;
> const u8 *seq_data;
> + u16 seq_size;
> u8 *data;
> u16 block_size;
> - int i, panel_id, seq_size;
>
> /* Only our generic panel driver uses the sequence block. */
> if (dev_priv->vbt.dsi.panel_id != MIPI_DSI_GENERIC_PANEL_ID)
> @@ -853,40 +890,11 @@ parse_mipi_sequence(struct drm_i915_private *dev_priv,
>*/
> dev_priv->vbt.dsi.seq_version = sequence->version;
>
> - seq_data = &sequence->data[0];
> -
> - /*
> - * sequence block is variable length and hence we need to parse and
> - * get the sequence data for specific panel id
> - */
> - for (i = 0; i < MAX_MIPI_CONFIGURATIONS; i++) {
> - panel_id = *seq_data;
> - seq_size = *((u16 *) (seq_data + 1));
> - if (panel_id == panel_type)
> - break;
> -
> - /* skip the sequence including seq header of 3 bytes */
> - seq_data = seq_data + 3 + seq_size;
> - if ((seq_data - &sequence->data[0]) > block_size) {
> - DRM_ERROR("Sequence start is beyond sequence block
> size, corrupted sequence block\n");
> - return;
> - }
> - }
> -
> - if (i == MAX_MIPI_CONFIGURATIONS) {
> - DRM_ERROR("Sequence block detected but no valid
> configuration\n");
> + seq_data = find_panel_sequence_block(sequence, panel_type, &seq_size);
> + if (!seq_data)
> return;
> - }
> -
> - /* check if found sequence is completely within the sequence block
> - * just being paranoid */
> - if (seq_size > block_size) {
> - DRM_ERROR("Corrupted sequence/size, bailing out\n");
> - return;
> - }
>
> - /* skip the panel id(1 byte) and seq size(2 bytes) */
> - dev_priv->vbt.dsi.data = kmemdup(seq_data + 3, seq_size, GFP_KERNEL);
> + dev_priv->vbt.dsi.data = kmemdup(seq_data, seq_size, GFP_KERNEL);
Should dropping the +3 be in a separate patch?
Otherwise looks good, with the above 2 addressed
Reviewed-by: Daniel Vetter
> if (!dev_priv->vbt.dsi.data)
> return;
>
> --
> 2.1.4
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/in
Re: [Intel-gfx] [PATCH] drm/i915: edp resume/On time optimization.
On Tue, Jan 05, 2016 at 01:30:53AM +, Kumar, Abhay wrote: > Ville, > > Is this patch is coming close to what you wanted? Please don't bottom-post but not quote properly - no one will ever find your comment and assume you accidentally sent out the patch twice. If you have to use a broken mailer that can't quote, then top-post. But please just install something that does work (I personally use Thunderbird on Windows, it getst the job done). Anything else just doesn't work. Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 3/3] drm/i915: DP channel EQ check for use of DP link training optimization
On Mon, 2016-01-04 at 18:53 +0200, Ville Syrjälä wrote:
> On Mon, Jan 04, 2016 at 06:44:09PM +0200, Ville Syrjälä wrote:
> > On Mon, Jan 04, 2016 at 01:21:24PM +0200, Mika Kahola wrote:
> > > Don't use DP link training optimization if channel EQ is not ok. It has
> > > been reported that in case of failure in channel EQ check the link
> > > training
> > > optimization can be enabled and therefore may not be able to reuse the
> > > previously computed drive current and pre-emphasis levels.
> > >
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91393
> > > Signed-off-by: Mika Kahola
> > > ---
> > > drivers/gpu/drm/i915/intel_dp.c | 1 +
> > > 1 file changed, 1 insertion(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > > b/drivers/gpu/drm/i915/intel_dp.c
> > > index 3137187..0cd1ccb 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -4354,6 +4354,7 @@ intel_dp_check_link_status(struct intel_dp
> > > *intel_dp)
> > > (!drm_dp_channel_eq_ok(link_status, intel_dp->lane_count))) {
> > > DRM_DEBUG_KMS("%s: channel EQ not ok, retraining\n",
> > > intel_encoder->base.name);
> > > + intel_dp->train_set_valid = false;
> > > intel_dp_start_link_train(intel_dp);
> > > intel_dp_stop_link_train(intel_dp);
> > > }
> >
> > Should do the same for the MST case I suppose?
>
> Also I wonder if we shold maintain some sort of blacklist of unstable
> vswing/preemph settings so that we wouldn't even try to retrain with
> values we know to be of questionable quality...
I think it would be quite tricky to maintain such a list. There is quite
a bunch of source/sink combinations out there. For eDP this would be
easier if we could first learn which vsing/preemph settings are unusable
and maintain the blacklist based on that.
Cheers,
Mika
>
> >
> > > --
> > > 1.9.1
> >
> > --
> > Ville Syrjälä
> > Intel OTC
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 2/2] drm/i915: Check DP no aux transaction bit on link training
On Mon, 2016-01-04 at 18:27 +0100, Thierry Reding wrote: > On Tue, Dec 22, 2015 at 04:53:41PM +0100, Lukas Wunner wrote: > > Hi Mika, > > > > On Mon, Dec 21, 2015 at 01:39:15PM +0200, Mika Kahola wrote: > > > Check if no AUX transactions are required on DP link training. > > > If this bit is set, we can reuse the known good drive current > > > and pre-emphasis level from the last "full" link training. > > The commit message here isn't entirely accurate. You still need AUX > transactions to configure the link according to the values obtained > from the last full training sequence. > > Thierry Thanks Thierry! I need to rephrase the commit message. Cheers, Mika ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 1/3] drm/i915: Disable fast link training if DP config changes
On Mon, 2016-01-04 at 18:42 +0200, Ville Syrjälä wrote:
> On Mon, Jan 04, 2016 at 01:21:22PM +0200, Mika Kahola wrote:
> > Disable DP link training optimization if DP link configuration
> > changes. If one of the DP link parameters i.e. link rate or
> > lane count changes the link training does no longer apply the
> > previously computed drive current and pre-emphasis level.
> > Instead, the link training is started with zero values.
> >
> > v5: Commit message update. Split the original patch in two.
> > This part considers only changes on link configuration.
> > Removed unnecessary debug messages. (Ville)
> >
> > v4: Parameter and debug message naming improvements.
> > Fix for link parameter check (Ville)
> >
> > v3: Remove cached old link parameters. Instead, disable
> > fast link training feature when link parameters are
> > set (Ville)
> >
> > v2: Readout DPCD register to check if no aux handshaking is
> > required in link training (Ander)
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91393
> > Signed-off-by: Mika Kahola
> > ---
> > drivers/gpu/drm/i915/intel_dp.c | 5 +
> > drivers/gpu/drm/i915/intel_dp_link_training.c | 3 +++
> > 2 files changed, 8 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index 796e3d3..6b36d82 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -1676,6 +1676,11 @@ found:
> > void intel_dp_set_link_params(struct intel_dp *intel_dp,
> > const struct intel_crtc_state *pipe_config)
> > {
> > + if (intel_dp->link_rate != pipe_config->port_clock ||
> > + intel_dp->lane_count != pipe_config->lane_count) {
> > + intel_dp->train_set_valid = false;
> > + }
> > +
> > intel_dp->link_rate = pipe_config->port_clock;
> > intel_dp->lane_count = pipe_config->lane_count;
> > }
> > diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c
> > b/drivers/gpu/drm/i915/intel_dp_link_training.c
> > index 793..59d59be 100644
> > --- a/drivers/gpu/drm/i915/intel_dp_link_training.c
> > +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c
> > @@ -85,6 +85,9 @@ static bool
> > intel_dp_reset_link_train(struct intel_dp *intel_dp,
> > uint8_t dp_train_pat)
> > {
> > + DRM_DEBUG_KMS("link training optimization: %s\n",
> > + intel_dp->train_set_valid ? "true" : "false");
>
> yesno(intel_dp->train_set_valid)
>
> With that
> Reviewed-by: Ville Syrjälä
>
Thanks for the review!
Cheers,
Mika
> > +
> > if (!intel_dp->train_set_valid)
> > memset(intel_dp->train_set, 0, sizeof(intel_dp->train_set));
> > intel_dp_set_signal_levels(intel_dp);
> > --
> > 1.9.1
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: add kerneldoc for intel_lr_context_size()
This function was recently renamed & exposed, so now it gets documented
Signed-off-by: Dave Gordon
---
drivers/gpu/drm/i915/intel_lrc.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 8da67b3..3662d14 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -2386,6 +2386,20 @@ void intel_lr_context_free(struct intel_context *ctx)
}
}
+/**
+ * intel_lr_context_size() - return the size of the context for an engine
+ * @ring: which engine to find the context size for
+ *
+ * Each engine may require a different amount of space for a context image,
+ * so when allocating (or copying) an image, this function can be used to
+ * find the right size for the specific engine.
+ *
+ * Return: size (in bytes) of an engine-specific context image
+ *
+ * Note: this size includes the HWSP, which is part of the context image
+ * in LRC mode, but does not include the "shared data page" used with
+ * GuC submission. The caller should account for this if using the GuC.
+ */
uint32_t intel_lr_context_size(struct intel_engine_cs *ring)
{
int ret = 0;
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ failure: Fi.CI.BAT
== Summary == HEAD is now at c837c0f drm-intel-nightly: 2016y-01m-05d-10h-35m-52s UTC integration manifest Applying: drm/i915: add kerneldoc for intel_lr_context_size() Applying: drm/i915/guc: Add GuC ADS (Addition Data Structure) - allocation Repository lacks necessary blobs to fall back on 3-way merge. Cannot fall back to three-way merge. Patch failed at 0002 drm/i915/guc: Add GuC ADS (Addition Data Structure) - allocation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 05/15] drm/i915/bios: abstract finding the panel sequence block
On Tue, 05 Jan 2016, Daniel Vetter wrote:
> On Mon, Dec 21, 2015 at 03:10:56PM +0200, Jani Nikula wrote:
>> Make the whole thing easier to read.
>>
>> Signed-off-by: Jani Nikula
>> ---
>> drivers/gpu/drm/i915/intel_bios.c | 76
>> +--
>> 1 file changed, 42 insertions(+), 34 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_bios.c
>> b/drivers/gpu/drm/i915/intel_bios.c
>> index 7393596df37d..9511a5341562 100644
>> --- a/drivers/gpu/drm/i915/intel_bios.c
>> +++ b/drivers/gpu/drm/i915/intel_bios.c
>> @@ -697,7 +697,7 @@ parse_psr(struct drm_i915_private *dev_priv, const
>> struct bdb_header *bdb)
>> dev_priv->vbt.psr.tp2_tp3_wakeup_time = psr_table->tp2_tp3_wakeup_time;
>> }
>>
>> -static u8 *goto_next_sequence(u8 *data, int *size)
>> +static u8 *goto_next_sequence(u8 *data, u16 *size)
>> {
>> u16 len;
>> int tmp = *size;
>> @@ -818,15 +818,52 @@ parse_mipi_config(struct drm_i915_private *dev_priv,
>> dev_priv->vbt.dsi.panel_id = MIPI_DSI_GENERIC_PANEL_ID;
>> }
>>
>> +/* Find the sequence block and size for the given panel. */
>> +static const u8 *
>> +find_panel_sequence_block(const struct bdb_mipi_sequence *sequence,
>> + u16 panel_id, u16 *seq_size)
>> +{
>> +u32 total = get_blocksize(sequence);
>> +const u8 *data = &sequence->data[0];
>> +u8 current_id;
>> +u16 current_size;
>> +int index = 0;
>> +int i;
>> +
>> +for (i = 0; i < MAX_MIPI_CONFIGURATIONS && index + 3 < total; i++) {
>
> Commit message should mention that you make the parsin more robust and
> ensure we don't walk over the end of the allocated buffer.
Agreed. Although it's implied in the Signed-off-by line. ;)
>
>> +current_id = *(data + index);
>> +index++;
>> +
>> +current_size = *((const u16 *)(data + index));
>> +index += 2;
>> +
>> +if (index + current_size > total) {
>> +DRM_ERROR("Invalid sequence block\n");
>> +return NULL;
>> +}
>> +
>> +if (current_id == panel_id) {
>> +*seq_size = current_size;
>> +return data + index;
>> +}
>> +
>> +index += current_size;
>> +}
>> +
>> +DRM_ERROR("Sequence block detected but no valid configuration\n");
>> +
>> +return NULL;
>> +}
>> +
>> static void
>> parse_mipi_sequence(struct drm_i915_private *dev_priv,
>> const struct bdb_header *bdb)
>> {
>> const struct bdb_mipi_sequence *sequence;
>> const u8 *seq_data;
>> +u16 seq_size;
>> u8 *data;
>> u16 block_size;
>> -int i, panel_id, seq_size;
>>
>> /* Only our generic panel driver uses the sequence block. */
>> if (dev_priv->vbt.dsi.panel_id != MIPI_DSI_GENERIC_PANEL_ID)
>> @@ -853,40 +890,11 @@ parse_mipi_sequence(struct drm_i915_private *dev_priv,
>> */
>> dev_priv->vbt.dsi.seq_version = sequence->version;
>>
>> -seq_data = &sequence->data[0];
>> -
>> -/*
>> - * sequence block is variable length and hence we need to parse and
>> - * get the sequence data for specific panel id
>> - */
>> -for (i = 0; i < MAX_MIPI_CONFIGURATIONS; i++) {
>> -panel_id = *seq_data;
>> -seq_size = *((u16 *) (seq_data + 1));
>> -if (panel_id == panel_type)
>> -break;
>> -
>> -/* skip the sequence including seq header of 3 bytes */
>> -seq_data = seq_data + 3 + seq_size;
>> -if ((seq_data - &sequence->data[0]) > block_size) {
>> -DRM_ERROR("Sequence start is beyond sequence block
>> size, corrupted sequence block\n");
>> -return;
>> -}
>> -}
>> -
>> -if (i == MAX_MIPI_CONFIGURATIONS) {
>> -DRM_ERROR("Sequence block detected but no valid
>> configuration\n");
>> +seq_data = find_panel_sequence_block(sequence, panel_type, &seq_size);
>> +if (!seq_data)
>> return;
>> -}
>> -
>> -/* check if found sequence is completely within the sequence block
>> - * just being paranoid */
>> -if (seq_size > block_size) {
>> -DRM_ERROR("Corrupted sequence/size, bailing out\n");
>> -return;
>> -}
>>
>> -/* skip the panel id(1 byte) and seq size(2 bytes) */
>> -dev_priv->vbt.dsi.data = kmemdup(seq_data + 3, seq_size, GFP_KERNEL);
>> +dev_priv->vbt.dsi.data = kmemdup(seq_data, seq_size, GFP_KERNEL);
>
> Should dropping the +3 be in a separate patch?
Really I'd rather not if you don't mind. The end result is the same, but
I'd have to think the function over again just to add a throwaway
intermediate step. Unless I just replace the return statement with
"return data + index - 3" which would feel a bit silly, don't you think?
BR,
Jani.
>
> Otherwise looks good, with the above 2 addressed
>
> Reviewed-by: Daniel Vetter
>
>> if (!
Re: [Intel-gfx] [PATCH 14/32] drm/i915: Remove forcewake dance from seqno/irq barrier on legacy gen6+
On 11/12/15 11:33, Chris Wilson wrote:
In order to ensure seqno/irq coherency, we current read a ring register.
We are not sure quite how it works, only that is does. Experiments show
that e.g. doing a clflush(seqno) instead is not sufficient, but we can
remove the forcewake dance from the mmio access.
v2: Baytrail wants a clflush too.
Signed-off-by: Chris Wilson
Cc: Daniel Vetter
---
drivers/gpu/drm/i915/intel_ringbuffer.c | 15 +--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 6cecc15ec01b..69dd69e46fa9 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1490,10 +1490,21 @@ gen6_ring_get_seqno(struct intel_engine_cs *ring, bool
lazy_coherency)
{
/* Workaround to force correct ordering between irq and seqno writes on
* ivb (and maybe also on snb) by reading from a CS register (like
-* ACTHD) before reading the status page. */
+* ACTHD) before reading the status page.
+*
+* Note that this effectively effectively stalls the read by the time
+* it takes to do a memory transaction, which more or less ensures
+* that the write from the GPU has sufficient time to invalidate
+* the CPU cacheline. Alternatively we could delay the interrupt from
+* the CS ring to give the write time to land, but that would incur
+* a delay after every batch i.e. much more frequent than a delay
+* when waiting for the interrupt (with the same net latency).
+*/
if (!lazy_coherency) {
struct drm_i915_private *dev_priv = ring->dev->dev_private;
- POSTING_READ(RING_ACTHD(ring->mmio_base));
+ POSTING_READ_FW(RING_ACTHD(ring->mmio_base));
+
+ intel_flush_status_page(ring, I915_GEM_HWS_INDEX);
}
return intel_read_status_page(ring, I915_GEM_HWS_INDEX);
Hmm ... would putting the flush before the POSTING_READ be better?
Depending on how the h/w implements the cacheline invalidation, it might
allow some overlap between the cache controller's internal activities
and the MMIO cycle ...
Also, previously we only had the flush on BXT, whereas now you're doing
it on all gen6+. I think this is probably a good thing, but just
wondered whether there's any downside to it?
Also ... are we sure that no-one calls this without having a forcewake
in effect at the time, in particular debugfs? Or is it not going to end
up going through here once lazy_coherency is abolished?
.Dave.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/6] drm/i915: Cleaning up intel_dp_hpd_pulse
Current DP detection has DPCD operations split across
intel_dp_hpd_pulse and intel_dp_detect which contains
duplicates as well. Also intel_dp_detect is called
during modes enumeration as well which will result
in multiple dpcd operations. So this patch tries
to solve both these by bringing all DPCD operations
in one single function and make intel_dp_detect
use existing values instead of repeating same steps.
v2: Pulled in a hunk from last patch of the series to
this patch. (Ander)
v3: Added MST hotplug handling. (Ander)
Tested-by: Nathan D Ciobanu
Signed-off-by: Sivakumar Thulasimani
Signed-off-by: Shubhangi Shrivastava
---
drivers/gpu/drm/i915/intel_dp.c | 71 +
1 file changed, 44 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index e3b4208..137757b 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4645,8 +4645,19 @@ intel_dp_long_pulse(struct intel_connector
*intel_connector)
intel_dp_probe_oui(intel_dp);
ret = intel_dp_probe_mst(intel_dp);
- if (ret)
+ if (ret) {
+ goto out;
+ } else if (connector->status == connector_status_connected) {
+ /*
+* If display was connected already and is still connected
+* check links status, there has been known issues of
+* link loss triggerring long pulse
+*/
+ drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
+ intel_dp_check_link_status(intel_dp);
+ drm_modeset_unlock(&dev->mode_config.connection_mutex);
goto out;
+ }
/*
* Clearing NACK and defer counts to get their exact values
@@ -4677,8 +4688,21 @@ intel_dp_long_pulse(struct intel_connector
*intel_connector)
}
out:
- if (status != connector_status_connected)
+ if (status != connector_status_connected) {
intel_dp_unset_edid(intel_dp);
+ /*
+* If we were in MST mode, and device is not there,
+* get out of MST mode
+*/
+ if (intel_dp->is_mst) {
+ DRM_DEBUG_KMS("MST device may have disappeared %d vs
%d\n",
+ intel_dp->is_mst, intel_dp->mst_mgr.mst_state);
+ intel_dp->is_mst = false;
+ drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr,
+ intel_dp->is_mst);
+ }
+ }
+
intel_display_power_put(to_i915(dev), power_domain);
return;
}
@@ -4701,7 +4725,8 @@ intel_dp_detect(struct drm_connector *connector, bool
force)
return connector_status_disconnected;
}
- intel_dp_long_pulse(intel_dp->attached_connector);
+ if (force)
+ intel_dp_long_pulse(intel_dp->attached_connector);
if (intel_connector->detect_edid)
return connector_status_connected;
@@ -5034,25 +5059,25 @@ intel_dp_hpd_pulse(struct intel_digital_port
*intel_dig_port, bool long_hpd)
/* indicate that we need to restart link training */
intel_dp->train_set_valid = false;
- if (!intel_digital_port_connected(dev_priv, intel_dig_port))
- goto mst_fail;
-
- if (!intel_dp_get_dpcd(intel_dp)) {
- goto mst_fail;
- }
-
- intel_dp_probe_oui(intel_dp);
+ intel_dp_long_pulse(intel_dp->attached_connector);
+ if (intel_dp->is_mst)
+ ret = IRQ_HANDLED;
+ goto put_power;
- if (!intel_dp_probe_mst(intel_dp)) {
- drm_modeset_lock(&dev->mode_config.connection_mutex,
NULL);
- intel_dp_check_link_status(intel_dp);
- drm_modeset_unlock(&dev->mode_config.connection_mutex);
- goto mst_fail;
- }
} else {
if (intel_dp->is_mst) {
- if (intel_dp_check_mst_status(intel_dp) == -EINVAL)
- goto mst_fail;
+ if (intel_dp_check_mst_status(intel_dp) == -EINVAL) {
+ /*
+* If we were in MST mode, and device is not
+* there, get out of MST mode
+*/
+ DRM_DEBUG_KMS("MST device may have disappeared
%d vs %d\n",
+ intel_dp->is_mst,
intel_dp->mst_mgr.mst_state);
+ intel_dp->is_mst = false;
+
drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr,
+
i
[Intel-gfx] [PATCH 3/6] drm/i915: Splitting intel_dp_check_link_status
When created originally intel_dp_check_link_status()
was supposed to handle only link training for short
pulse but has grown into handler for short pulse itself.
This patch cleans up this function by splitting it into
two halves. First intel_dp_short_pulse() is called,
which will be entry point and handle all logic for
short pulse handling while intel_dp_check_link_status()
will retain its original purpose of only doing link
status related work.
The link retraining part when EQ is not correct is
retained to intel_dp_check_link_status whereas other
operations are handled as part of intel_dp_short_pulse.
This change is required to avoid performing all DPCD
related operations on performing link retraining.
Tested-by: Nathan D Ciobanu
Signed-off-by: Sivakumar Thulasimani
Signed-off-by: Shubhangi Shrivastava
---
drivers/gpu/drm/i915/intel_dp.c | 56 -
1 file changed, 33 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 137757b..842790e 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4289,6 +4289,33 @@ go_again:
return -EINVAL;
}
+static void
+intel_dp_check_link_status(struct intel_dp *intel_dp)
+{
+ struct intel_encoder *intel_encoder = &dp_to_dig_port(intel_dp)->base;
+ u8 link_status[DP_LINK_STATUS_SIZE];
+
+ if (!intel_dp_get_link_status(intel_dp, link_status)) {
+ DRM_ERROR("Failed to get link status\n");
+ return;
+ }
+
+ if (!intel_encoder->base.crtc)
+ return;
+
+ if (!to_intel_crtc(intel_encoder->base.crtc)->active)
+ return;
+
+ /* if link training is requested we should perform it always */
+ if ((intel_dp->compliance_test_type == DP_TEST_LINK_TRAINING) ||
+ (!drm_dp_channel_eq_ok(link_status, intel_dp->lane_count))) {
+ DRM_DEBUG_KMS("%s: channel EQ not ok, retraining\n",
+ intel_encoder->base.name);
+ intel_dp_start_link_train(intel_dp);
+ intel_dp_stop_link_train(intel_dp);
+ }
+}
+
/*
* According to DP spec
* 5.1.2:
@@ -4298,15 +4325,12 @@ go_again:
* 4. Check link status on receipt of hot-plug interrupt
*/
static void
-intel_dp_check_link_status(struct intel_dp *intel_dp)
+intel_dp_short_pulse(struct intel_dp *intel_dp)
{
struct drm_device *dev = intel_dp_to_dev(intel_dp);
- struct intel_encoder *intel_encoder = &dp_to_dig_port(intel_dp)->base;
u8 sink_irq_vector;
u8 link_status[DP_LINK_STATUS_SIZE];
- WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
-
/*
* Clearing compliance test variables to allow capturing
* of values for next automated test request.
@@ -4315,12 +4339,6 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
intel_dp->compliance_test_type = 0;
intel_dp->compliance_test_data = 0;
- if (!intel_encoder->base.crtc)
- return;
-
- if (!to_intel_crtc(intel_encoder->base.crtc)->active)
- return;
-
/* Try to read receiver status if the link appears to be up */
if (!intel_dp_get_link_status(intel_dp, link_status)) {
return;
@@ -4345,14 +4363,9 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
DRM_DEBUG_DRIVER("CP or sink specific irq unhandled\n");
}
- /* if link training is requested we should perform it always */
- if ((intel_dp->compliance_test_type == DP_TEST_LINK_TRAINING) ||
- (!drm_dp_channel_eq_ok(link_status, intel_dp->lane_count))) {
- DRM_DEBUG_KMS("%s: channel EQ not ok, retraining\n",
- intel_encoder->base.name);
- intel_dp_start_link_train(intel_dp);
- intel_dp_stop_link_train(intel_dp);
- }
+ drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
+ intel_dp_check_link_status(intel_dp);
+ drm_modeset_unlock(&dev->mode_config.connection_mutex);
}
/* XXX this is probably wrong for multiple downstream ports */
@@ -5080,11 +5093,8 @@ intel_dp_hpd_pulse(struct intel_digital_port
*intel_dig_port, bool long_hpd)
}
}
- if (!intel_dp->is_mst) {
- drm_modeset_lock(&dev->mode_config.connection_mutex,
NULL);
- intel_dp_check_link_status(intel_dp);
- drm_modeset_unlock(&dev->mode_config.connection_mutex);
- }
+ if (!intel_dp->is_mst)
+ intel_dp_short_pulse(intel_dp);
}
ret = IRQ_HANDLED;
--
2.6.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/6] drm/i915: Save sink_count for tracking changes to it
Sink count can change between short pulse hpd hence this patch
adds a member variable to intel_dp so we can track any changes
between short pulse interrupts.
Tested-by: Nathan D Ciobanu
Signed-off-by: Sivakumar Thulasimani
Signed-off-by: Shubhangi Shrivastava
---
drivers/gpu/drm/i915/intel_dp.c | 7 +++
drivers/gpu/drm/i915/intel_drv.h | 1 +
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 842790e..c2e8516 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4385,14 +4385,13 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp)
/* If we're HPD-aware, SINK_COUNT changes dynamically */
if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 &&
intel_dp->downstream_ports[0] & DP_DS_PORT_HPD) {
- uint8_t reg;
if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_SINK_COUNT,
- ®, 1) < 0)
+ &intel_dp->sink_count, 1) < 0)
return connector_status_unknown;
- return DP_GET_SINK_COUNT(reg) ? connector_status_connected
- : connector_status_disconnected;
+ return DP_GET_SINK_COUNT(intel_dp->sink_count) ?
+ connector_status_connected : connector_status_disconnected;
}
/* If no HPD, poke DDC gently */
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 0438b57..88b05ba 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -757,6 +757,7 @@ struct intel_dp {
uint32_t DP;
int link_rate;
uint8_t lane_count;
+ uint8_t sink_count;
bool has_audio;
enum hdmi_force_audio force_audio;
bool limited_color_range;
--
2.6.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 0/6] Fixing sink count related detection over
This patch set cleans up DP detection logic to bring all DPCD operations at one place and to create a clear demarcation between handling of long and short pulses. This simplifies fixing of sink count related detection for DP panels. Patches: 1. First two patches clean up intel_dp_detect and form a new function which will include all DPCD related operations. 2. Third patch splits up intel_dp_check_link_status to form a new function which will handle short pulse requests. 3. Last three patches fixes the detection logic related to sink count i.e detect changes in sink count and handle them appropriately. Shubhangi Shrivastava (6): drm/i915: Splitting intel_dp_detect drm/i915: Cleaning up intel_dp_hpd_pulse drm/i915: Splitting intel_dp_check_link_status drm/i915: Save sink_count for tracking changes to it drm/i915: read sink_count dpcd always drm/i915: force full detect on sink count change drivers/gpu/drm/i915/intel_dp.c | 170 +-- drivers/gpu/drm/i915/intel_drv.h | 1 + 2 files changed, 110 insertions(+), 61 deletions(-) -- 2.6.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/6] drm/i915: Splitting intel_dp_detect
intel_dp_detect() is called for not just detection but
during modes enumeration as well. Repeating the whole
sequence during each of these calls is wasteful and
time consuming.
This patch moves probing for panel, DPCD read etc done in
intel_dp_detect() to a new function intel_dp_long_pulse().
Note that the behavior of intel_dp_detect() is changed to
report connected or disconnected depending on whether the
EDID is available or not.
This change will be required by further patches in the series
to avoid performing duplicated DPCD operations on hotplug.
v2: Moved a hunk to next patch of the series.
Moved intel_dp_unset_edid to out. (Ander)
v3: Rephrased commit message and intel_dp_unset_dp() is called
within intel_dp_set_dp() to free the previous EDID. (Ander)
Tested-by: Nathan D Ciobanu
Signed-off-by: Sivakumar Thulasimani
Signed-off-by: Shubhangi Shrivastava
---
drivers/gpu/drm/i915/intel_dp.c | 56 +
1 file changed, 35 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 796e3d3..e3b4208 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -129,6 +129,7 @@ static void edp_panel_vdd_off(struct intel_dp *intel_dp,
bool sync);
static void vlv_init_panel_power_sequencer(struct intel_dp *intel_dp);
static void vlv_steal_power_sequencer(struct drm_device *dev,
enum pipe pipe);
+static void intel_dp_unset_edid(struct intel_dp *intel_dp);
static unsigned int intel_dp_unused_lane_mask(int lane_count)
{
@@ -4587,6 +4588,7 @@ intel_dp_set_edid(struct intel_dp *intel_dp)
struct intel_connector *intel_connector = intel_dp->attached_connector;
struct edid *edid;
+ intel_dp_unset_edid(intel_dp);
edid = intel_dp_get_edid(intel_dp);
intel_connector->detect_edid = edid;
@@ -4607,9 +4609,10 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
intel_dp->has_audio = false;
}
-static enum drm_connector_status
-intel_dp_detect(struct drm_connector *connector, bool force)
+static void
+intel_dp_long_pulse(struct intel_connector *intel_connector)
{
+ struct drm_connector *connector = &intel_connector->base;
struct intel_dp *intel_dp = intel_attached_dp(connector);
struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
struct intel_encoder *intel_encoder = &intel_dig_port->base;
@@ -4619,17 +4622,6 @@ intel_dp_detect(struct drm_connector *connector, bool
force)
bool ret;
u8 sink_irq_vector;
- DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
- connector->base.id, connector->name);
- intel_dp_unset_edid(intel_dp);
-
- if (intel_dp->is_mst) {
- /* MST devices are disconnected from a monitor POV */
- if (intel_encoder->type != INTEL_OUTPUT_EDP)
- intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
- return connector_status_disconnected;
- }
-
power_domain = intel_display_port_aux_power_domain(intel_encoder);
intel_display_power_get(to_i915(dev), power_domain);
@@ -4653,14 +4645,8 @@ intel_dp_detect(struct drm_connector *connector, bool
force)
intel_dp_probe_oui(intel_dp);
ret = intel_dp_probe_mst(intel_dp);
- if (ret) {
- /* if we are in MST mode then this connector
- won't appear connected or have anything with EDID on it */
- if (intel_encoder->type != INTEL_OUTPUT_EDP)
- intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
- status = connector_status_disconnected;
+ if (ret)
goto out;
- }
/*
* Clearing NACK and defer counts to get their exact values
@@ -4691,8 +4677,36 @@ intel_dp_detect(struct drm_connector *connector, bool
force)
}
out:
+ if (status != connector_status_connected)
+ intel_dp_unset_edid(intel_dp);
intel_display_power_put(to_i915(dev), power_domain);
- return status;
+ return;
+}
+
+static enum drm_connector_status
+intel_dp_detect(struct drm_connector *connector, bool force)
+{
+ struct intel_dp *intel_dp = intel_attached_dp(connector);
+ struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+ struct intel_encoder *intel_encoder = &intel_dig_port->base;
+ struct intel_connector *intel_connector = to_intel_connector(connector);
+
+ DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
+connector->base.id, connector->name);
+
+ if (intel_dp->is_mst) {
+ /* MST devices are disconnected from a monitor POV */
+ if (intel_encoder->type != INTEL_OUTPUT_EDP)
+ intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
+ return connector_status_disconnected;
+ }
+
+ intel_dp_long_pulse(intel_dp->at
[Intel-gfx] [PATCH 5/6] drm/i915: read sink_count dpcd always
This patch reads sink_count dpcd always and removes its
read operation based on values in downstream port dpcd.
SINK_COUNT dpcd is not dependent on DOWNSTREAM_PORT_PRESENT dpcd.
SINK_COUNT denotes if a display is attached, while
DOWNSTREAM_PORT_PRESET indicates how many ports are available
in the dongle where display can be attached. so it is possible
for sink count to change irrespective of value in downstream
port dpcd.
Here is a table of possible values and scenarios
sink_count downstream_port
present
0 0 no display is attached
0 1 dongle is connected without display
1 0 display connected directly
1 1 display connected through dongle
Tested-by: Nathan D Ciobanu
Signed-off-by: Sivakumar Thulasimani
Signed-off-by: Shubhangi Shrivastava
---
drivers/gpu/drm/i915/intel_dp.c | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index c2e8516..0d58bfd 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3865,6 +3865,13 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
if (intel_dp->dpcd[DP_DPCD_REV] == 0)
return false; /* DPCD not present */
+ if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_SINK_COUNT,
+ &intel_dp->sink_count, 1) < 0)
+ return false;
+
+ if (!DP_GET_SINK_COUNT(intel_dp->sink_count))
+ return false;
+
/* Check if the panel supports PSR */
memset(intel_dp->psr_dpcd, 0, sizeof(intel_dp->psr_dpcd));
if (is_edp(intel_dp)) {
@@ -4386,10 +4393,6 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp)
if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 &&
intel_dp->downstream_ports[0] & DP_DS_PORT_HPD) {
- if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_SINK_COUNT,
- &intel_dp->sink_count, 1) < 0)
- return connector_status_unknown;
-
return DP_GET_SINK_COUNT(intel_dp->sink_count) ?
connector_status_connected : connector_status_disconnected;
}
--
2.6.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 6/6] drm/i915: force full detect on sink count change
This patch checks for changes in sink count between short pulse
hpds and forces full detect when there is a change.
This will allow both detection of hotplug and unplug of panels
through dongles that give only short pulse for such events.
v2: changed variable type from u8 to bool (Jani)
return immediately if perform_full_detect is set(Siva)
v3: changed method of determining full detection from using
pointer to return code (Siva)
Tested-by: Nathan D Ciobanu
Signed-off-by: Sivakumar Thulasimani
Signed-off-by: Shubhangi Shrivastava
---
drivers/gpu/drm/i915/intel_dp.c | 30 +++---
1 file changed, 23 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 0d58bfd..8a659ee 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4331,12 +4331,14 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
* 3. Use Link Training from 2.5.3.3 and 3.5.1.3
* 4. Check link status on receipt of hot-plug interrupt
*/
-static void
+static bool
intel_dp_short_pulse(struct intel_dp *intel_dp)
{
struct drm_device *dev = intel_dp_to_dev(intel_dp);
u8 sink_irq_vector;
u8 link_status[DP_LINK_STATUS_SIZE];
+ u8 old_sink_count = intel_dp->sink_count;
+ bool ret;
/*
* Clearing compliance test variables to allow capturing
@@ -4348,12 +4350,20 @@ intel_dp_short_pulse(struct intel_dp *intel_dp)
/* Try to read receiver status if the link appears to be up */
if (!intel_dp_get_link_status(intel_dp, link_status)) {
- return;
+ return false;
}
- /* Now read the DPCD to see if it's actually running */
- if (!intel_dp_get_dpcd(intel_dp)) {
- return;
+ /*
+* Now read the DPCD to see if it's actually running
+* Don't return immediately if dpcd read failed,
+* if sink count was 1 and dpcd read failed we need
+* to do full detection
+*/
+ ret = intel_dp_get_dpcd(intel_dp);
+
+ if ((old_sink_count != intel_dp->sink_count) || !ret) {
+ /* No need to proceed if we are going to do full detect */
+ return false;
}
/* Try to read the source of the interrupt */
@@ -4373,6 +4383,8 @@ intel_dp_short_pulse(struct intel_dp *intel_dp)
drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
intel_dp_check_link_status(intel_dp);
drm_modeset_unlock(&dev->mode_config.connection_mutex);
+
+ return true;
}
/* XXX this is probably wrong for multiple downstream ports */
@@ -5095,8 +5107,12 @@ intel_dp_hpd_pulse(struct intel_digital_port
*intel_dig_port, bool long_hpd)
}
}
- if (!intel_dp->is_mst)
- intel_dp_short_pulse(intel_dp);
+ if (!intel_dp->is_mst) {
+ if (!intel_dp_short_pulse(intel_dp)) {
+
intel_dp_long_pulse(intel_dp->attached_connector);
+ goto put_power;
+ }
+ }
}
ret = IRQ_HANDLED;
--
2.6.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/2] DPCD Backlight Control
Hi, Can someone please help review this patch series from December. https://patchwork.freedesktop.org/series/1864/ Thank you. Yetunde > -Original Message- > From: Adebisi, YetundeX > Sent: Wednesday, December 16, 2015 12:07 PM > To: intel-gfx@lists.freedesktop.org > Cc: Adebisi, YetundeX > Subject: [PATCH 0/2] DPCD Backlight Control > > These patches add support for Backlight Control using DPCD registers on eDP > displays. > > Yetunde Adebisi (2): > drm/dp: Add definition for Display Control DPCD Registers capability > size > drm/i915: Add Backlight Control using DPCD for eDP connectors (v4) > > drivers/gpu/drm/i915/Makefile | 1 + > drivers/gpu/drm/i915/intel_dp.c | 17 ++- > drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 181 > ++ > drivers/gpu/drm/i915/intel_drv.h | 6 + > drivers/gpu/drm/i915/intel_panel.c| 4 + > include/drm/drm_dp_helper.h | 1 + > 6 files changed, 204 insertions(+), 6 deletions(-) create mode 100644 > drivers/gpu/drm/i915/intel_dp_aux_backlight.c > > -- > 1.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 04/15] drm/i915/bios: have get_blocksize() support MIPI sequence block v3+
On Tue, 05 Jan 2016, Daniel Vetter wrote:
> On Mon, Dec 21, 2015 at 03:10:55PM +0200, Jani Nikula wrote:
>> Have get_blocksize() support the special case of MIPI sequence block v3+
>> which has a separate field for size. Provide and use abstractions for
>> getting the blocksize given a pointer to the block "envelope",
>> i.e. pointer to the block id, and given a pointer to the block payload
>> data.
>>
>> Signed-off-by: Jani Nikula
>
> Reviewed-by: Daniel Vetter
Patches 2-4 pushed to drm-intel-next-queued, thanks for the review.
BR,
Jani.
>
>> ---
>> drivers/gpu/drm/i915/intel_bios.c | 36 ++--
>> 1 file changed, 18 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_bios.c
>> b/drivers/gpu/drm/i915/intel_bios.c
>> index 91540ab15e0b..7393596df37d 100644
>> --- a/drivers/gpu/drm/i915/intel_bios.c
>> +++ b/drivers/gpu/drm/i915/intel_bios.c
>> @@ -58,6 +58,22 @@
>>
>> static int panel_type;
>>
>> +/* Get BDB block size given a pointer to Block ID. */
>> +static u32 _get_blocksize(const u8 *block_base)
>> +{
>> +/* The MIPI Sequence Block v3+ has a separate size field. */
>> +if (*block_base == BDB_MIPI_SEQUENCE && *(block_base + 3) >= 3)
>> +return *((const u32 *)(block_base + 4));
>> +else
>> +return *((const u16 *)(block_base + 1));
>> +}
>> +
>> +/* Get BDB block size give a pointer to data after Block ID and Block Size.
>> */
>> +static u32 get_blocksize(const void *block_data)
>> +{
>> +return _get_blocksize(block_data - 3);
>> +}
>> +
>> static const void *
>> find_section(const void *_bdb, int section_id)
>> {
>> @@ -74,14 +90,8 @@ find_section(const void *_bdb, int section_id)
>> /* walk the sections looking for section_id */
>> while (index + 3 < total) {
>> current_id = *(base + index);
>> -index++;
>> -
>> -current_size = *((const u16 *)(base + index));
>> -index += 2;
>> -
>> -/* The MIPI Sequence Block v3+ has a separate size field. */
>> -if (current_id == BDB_MIPI_SEQUENCE && *(base + index) >= 3)
>> -current_size = *((const u32 *)(base + index + 1));
>> +current_size = _get_blocksize(base + index);
>> +index += 3;
>>
>> if (index + current_size > total)
>> return NULL;
>> @@ -95,16 +105,6 @@ find_section(const void *_bdb, int section_id)
>> return NULL;
>> }
>>
>> -static u16
>> -get_blocksize(const void *p)
>> -{
>> -u16 *block_ptr, block_size;
>> -
>> -block_ptr = (u16 *)((char *)p - 2);
>> -block_size = *block_ptr;
>> -return block_size;
>> -}
>> -
>> static void
>> fill_detail_timing_data(struct drm_display_mode *panel_fixed_mode,
>> const struct lvds_dvo_timing *dvo_timing)
>> --
>> 2.1.4
>>
>> ___
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2, 2/4] drm/i915: simplify testing for the global default context
On 05/01/16 10:06, Chris Wilson wrote: On Mon, Jan 04, 2016 at 01:38:26PM -0800, Jesse Barnes wrote: On 01/04/2016 11:39 AM, Chris Wilson wrote: This series is NAKed. Why? Because you want things in a different order? Or do you object to something in Dave's reply? The series was intended as a code cleanup and in the process tried to introduce a false concept that I objected to. Since the cleanup was not predicated upon that idea, the patches would have been much tidier without it. I don't think it's a false concept; there very evidently *IS* a global default context, so why not flag it as such by name, rather than by implication. and the subsequent cleanup *does* require it. The fundamental issue at stake here is execlists behaves badly and we have to futz around in higher level code to undo that mistake. -Chris Now I agree that the execlist->default_context is a bad idea, and that's exactly what we will get rid of in patch 4, but we can't do it until these precursor patches have separated the various purposes for which it is used into distinct categories, each of which is then replaced by a cleaner alternative. So it *is* a step towards rewriting execlists, even if it doesn't (at this stage) fix everything you might eventually want. .Dave. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] Flicker caused by "drm/i915: Eliminate usage of plane_wm_parameters from ILK-style WM code (v2)"
Hi,
On Thu, Sep 24, 2015 at 03:53:07PM -0700, Matt Roper wrote:
> Just pull the info out of the plane state structure rather than staging
> it in an additional structure.
>
> v2: Add 'visible' condition to sprites_scaled so that we don't limit the
> WM level when the sprite isn't enabled. (Ville)
It looks like this patch - specifically the visible condition in
ilk_compute_cur_wm - causes screen flicker when moving the cursor from
one screen to another one in a multi-screen setup.
My hardware is a Thinkpad x201s
(http://www.thinkwiki.org/wiki/Category:X201s)
00:02.0 VGA compatible controller [0300]: Intel Corporation Core Processor
Integrated Graphics Controller [8086:0046] (rev 02),
CPU is an i7-620LM.
The screen flickering is the one the coursor is leaving. In most cases,
parts of the screen just blank for a very short moment. In some rare
cases, the screen even stays blanked until I move the cursor back to
that screen. No stability issues were observed, this seems to be a
purely cosmetic issue.
I bisected the issue to 43d59eda1f69631c267e06ab6b94ed3c14f1f6d1. As I
knew the flickering was cursor-related, I tried the following minimal
patch, which actually fixed the symptom, when applied to 4.4-rc8:
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index f091ad1..1ef0c54 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1791,7 +1791,7 @@ static uint32_t ilk_compute_cur_wm(const struct
intel_crtc_state *cstate,
{
int bpp = pstate->base.fb ? pstate->base.fb->bits_per_pixel / 8 : 0;
- if (!cstate->base.active || !pstate->visible)
+ if (!cstate->base.active)
return 0;
return ilk_wm_method2(ilk_pipe_pixel_rate(cstate),
I have no idea at all if this is actually fixing anything or if it's
just hiding the real bug. All I can say is that the flickering doesn't
occur any longer.
Jan
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v6 3/3] drm/i915: DP channel EQ check for use of DP link training optimization
Don't use DP link training optimization if channel EQ is not ok. It has
been reported that in case of failure in channel EQ check the link training
optimization can be enabled and therefore may not be able to reuse the
previously computed drive current and pre-emphasis levels.
v2: Added MST case (Ville)
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91393
Signed-off-by: Mika Kahola
---
drivers/gpu/drm/i915/intel_dp.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 3137187..c995dbd 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4254,6 +4254,7 @@ go_again:
if (intel_dp->active_mst_links &&
!drm_dp_channel_eq_ok(&esi[10],
intel_dp->lane_count)) {
DRM_DEBUG_KMS("channel EQ not ok,
retraining\n");
+ intel_dp->train_set_valid = false;
intel_dp_start_link_train(intel_dp);
intel_dp_stop_link_train(intel_dp);
}
@@ -4354,6 +4355,7 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
(!drm_dp_channel_eq_ok(link_status, intel_dp->lane_count))) {
DRM_DEBUG_KMS("%s: channel EQ not ok, retraining\n",
intel_encoder->base.name);
+ intel_dp->train_set_valid = false;
intel_dp_start_link_train(intel_dp);
intel_dp_stop_link_train(intel_dp);
}
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v6 0/3] drm/i915: Disable link training optimization if DP config has changed
These three patches are fixes for DP link trainging failures and flickering issues reported by https://bugs.freedesktop.org/show_bug.cgi?id=91393 Mika Kahola (3): drm/i915: Disable fast link training if DP config changes drm/i915: Check DP no aux transaction bit on link training drm/i915: DP channel EQ check for use of DP link training optimization drivers/gpu/drm/i915/intel_dp.c | 9 - drivers/gpu/drm/i915/intel_dp_link_training.c | 22 ++ drivers/gpu/drm/i915/intel_drv.h | 1 + 3 files changed, 31 insertions(+), 1 deletion(-) -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v6 1/3] drm/i915: Disable fast link training if DP config changes
Disable DP link training optimization if DP link configuration
changes. If one of the DP link parameters i.e. link rate or
lane count changes the link training does no longer apply the
previously computed drive current and pre-emphasis level.
Instead, the link training is started with zero values.
v6: Debug message update to use yesno() routine (Ville)
v5: Commit message update. Split the original patch in two.
This part considers only changes on link configuration.
Removed unnecessary debug messages. (Ville)
v4: Parameter and debug message naming improvements.
Fix for link parameter check (Ville)
v3: Remove cached old link parameters. Instead, disable
fast link training feature when link parameters are
set (Ville)
v2: Readout DPCD register to check if no aux handshaking is
required in link training (Ander)
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91393
Reviewed-by: Ville Syrjälä
Signed-off-by: Mika Kahola
---
drivers/gpu/drm/i915/intel_dp.c | 5 +
drivers/gpu/drm/i915/intel_dp_link_training.c | 3 +++
2 files changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 796e3d3..6b36d82 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1676,6 +1676,11 @@ found:
void intel_dp_set_link_params(struct intel_dp *intel_dp,
const struct intel_crtc_state *pipe_config)
{
+ if (intel_dp->link_rate != pipe_config->port_clock ||
+ intel_dp->lane_count != pipe_config->lane_count) {
+ intel_dp->train_set_valid = false;
+ }
+
intel_dp->link_rate = pipe_config->port_clock;
intel_dp->lane_count = pipe_config->lane_count;
}
diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c
b/drivers/gpu/drm/i915/intel_dp_link_training.c
index 793..8f22c92 100644
--- a/drivers/gpu/drm/i915/intel_dp_link_training.c
+++ b/drivers/gpu/drm/i915/intel_dp_link_training.c
@@ -85,6 +85,9 @@ static bool
intel_dp_reset_link_train(struct intel_dp *intel_dp,
uint8_t dp_train_pat)
{
+ DRM_DEBUG_KMS("link training optimization: %s\n",
+ yesno(intel_dp->train_set_valid));
+
if (!intel_dp->train_set_valid)
memset(intel_dp->train_set, 0, sizeof(intel_dp->train_set));
intel_dp_set_signal_levels(intel_dp);
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v6 2/3] drm/i915: Check DP no aux transaction bit on link training
Check if *NO_AUX_TRANSACTIONS_LINK_TRAINING* bit is set on
DPCD registers. If this bit is set, we can reuse the known
good drive current and pre-emphasis level from the last
"full" link training.
v2: Commit message update (Thierry)
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91393
Signed-off-by: Mika Kahola
---
drivers/gpu/drm/i915/intel_dp.c | 2 +-
drivers/gpu/drm/i915/intel_dp_link_training.c | 19 +++
drivers/gpu/drm/i915/intel_drv.h | 1 +
3 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 6b36d82..3137187 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3852,7 +3852,7 @@ intel_dp_link_down(struct intel_dp *intel_dp)
intel_dp->DP = DP;
}
-static bool
+bool
intel_dp_get_dpcd(struct intel_dp *intel_dp)
{
struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c
b/drivers/gpu/drm/i915/intel_dp_link_training.c
index 8f22c92..bcf2801 100644
--- a/drivers/gpu/drm/i915/intel_dp_link_training.c
+++ b/drivers/gpu/drm/i915/intel_dp_link_training.c
@@ -85,6 +85,25 @@ static bool
intel_dp_reset_link_train(struct intel_dp *intel_dp,
uint8_t dp_train_pat)
{
+ bool has_dpcd;
+ bool no_aux_handshake = false;
+
+ has_dpcd = intel_dp_get_dpcd(intel_dp);
+
+ /*
+* Source device can try to use drive current and pre-emphasis
+* parameters computed by the last "full" link training if the
+* DP_NO_AUX_HANDSHAKE_LINK_TRAINING bit is set to 1.
+*/
+ if (has_dpcd) {
+ if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11) {
+ no_aux_handshake = (intel_dp->dpcd[DP_MAX_DOWNSPREAD] &
+ DP_NO_AUX_HANDSHAKE_LINK_TRAINING);
+ }
+ }
+
+ intel_dp->train_set_valid &= no_aux_handshake;
+
DRM_DEBUG_KMS("link training optimization: %s\n",
yesno(intel_dp->train_set_valid));
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 0438b57..918bdf1 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1250,6 +1250,7 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc);
bool intel_dp_compute_config(struct intel_encoder *encoder,
struct intel_crtc_state *pipe_config);
bool intel_dp_is_edp(struct drm_device *dev, enum port port);
+bool intel_dp_get_dpcd(struct intel_dp *intel_dp);
enum irqreturn intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port,
bool long_hpd);
void intel_edp_backlight_on(struct intel_dp *intel_dp);
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ warning: Fi.CI.BAT
== Summary == Built on 05ade905f2fda5416476677509e016ef830d181a drm-intel-nightly: 2016y-01m-05d-13h-00m-24s UTC integration manifest Test gem_storedw_loop: Subgroup basic-render: dmesg-warn -> PASS (bdw-nuci7) dmesg-warn -> PASS (skl-i7k-2) Test kms_flip: Subgroup basic-flip-vs-modeset: dmesg-warn -> PASS (bsw-nuc-2) UNSTABLE pass -> DMESG-WARN (hsw-gt2) UNSTABLE Subgroup basic-plain-flip: dmesg-warn -> PASS (bdw-ultra) Test kms_pipe_crc_basic: Subgroup read-crc-pipe-a: pass -> DMESG-WARN (snb-x220t) UNSTABLE dmesg-warn -> PASS (skl-i7k-2) UNSTABLE Subgroup read-crc-pipe-b: dmesg-warn -> PASS (snb-dellxps) Subgroup suspend-read-crc-pipe-a: dmesg-warn -> PASS (snb-x220t) Test kms_setmode: Subgroup basic-clone-single-crtc: pass -> DMESG-WARN (snb-dellxps) Test pm_rpm: Subgroup basic-pci-d3-state: pass -> DMESG-WARN (bdw-ultra) bdw-nuci7total:132 pass:122 dwarn:1 dfail:0 fail:0 skip:9 bdw-ultratotal:132 pass:124 dwarn:2 dfail:0 fail:0 skip:6 bsw-nuc-2total:135 pass:114 dwarn:1 dfail:0 fail:0 skip:20 hsw-brixbox total:135 pass:126 dwarn:2 dfail:0 fail:0 skip:7 hsw-gt2 total:135 pass:129 dwarn:2 dfail:0 fail:0 skip:4 hsw-xps12total:132 pass:125 dwarn:3 dfail:0 fail:0 skip:4 ilk-hp8440p total:135 pass:100 dwarn:0 dfail:0 fail:0 skip:35 ivb-t430stotal:135 pass:127 dwarn:2 dfail:0 fail:0 skip:6 skl-i5k-2total:135 pass:124 dwarn:3 dfail:0 fail:0 skip:8 skl-i7k-2total:135 pass:125 dwarn:2 dfail:0 fail:0 skip:8 snb-dellxps total:135 pass:121 dwarn:2 dfail:0 fail:0 skip:12 snb-x220ttotal:135 pass:121 dwarn:2 dfail:0 fail:1 skip:11 Results at /archive/results/CI_IGT_test/Patchwork_1083/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2] drm/i915/bios: abstract finding the panel sequence block
Make the whole thing easier to read. While at it, make the parsing more
robust, and ensure we don't read past buffer being parsed.
v2: improve commit message (Daniel)
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/intel_bios.c | 76 +--
1 file changed, 42 insertions(+), 34 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_bios.c
b/drivers/gpu/drm/i915/intel_bios.c
index 961ae7f6d075..422ba76700e3 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -697,7 +697,7 @@ parse_psr(struct drm_i915_private *dev_priv, const struct
bdb_header *bdb)
dev_priv->vbt.psr.tp2_tp3_wakeup_time = psr_table->tp2_tp3_wakeup_time;
}
-static u8 *goto_next_sequence(u8 *data, int *size)
+static u8 *goto_next_sequence(u8 *data, u16 *size)
{
u16 len;
int tmp = *size;
@@ -818,15 +818,52 @@ parse_mipi_config(struct drm_i915_private *dev_priv,
dev_priv->vbt.dsi.panel_id = MIPI_DSI_GENERIC_PANEL_ID;
}
+/* Find the sequence block and size for the given panel. */
+static const u8 *
+find_panel_sequence_block(const struct bdb_mipi_sequence *sequence,
+ u16 panel_id, u16 *seq_size)
+{
+ u32 total = get_blocksize(sequence);
+ const u8 *data = &sequence->data[0];
+ u8 current_id;
+ u16 current_size;
+ int index = 0;
+ int i;
+
+ for (i = 0; i < MAX_MIPI_CONFIGURATIONS && index + 3 < total; i++) {
+ current_id = *(data + index);
+ index++;
+
+ current_size = *((const u16 *)(data + index));
+ index += 2;
+
+ if (index + current_size > total) {
+ DRM_ERROR("Invalid sequence block\n");
+ return NULL;
+ }
+
+ if (current_id == panel_id) {
+ *seq_size = current_size;
+ return data + index;
+ }
+
+ index += current_size;
+ }
+
+ DRM_ERROR("Sequence block detected but no valid configuration\n");
+
+ return NULL;
+}
+
static void
parse_mipi_sequence(struct drm_i915_private *dev_priv,
const struct bdb_header *bdb)
{
const struct bdb_mipi_sequence *sequence;
const u8 *seq_data;
+ u16 seq_size;
u8 *data;
u16 block_size;
- int i, panel_id, seq_size;
/* Only our generic panel driver uses the sequence block. */
if (dev_priv->vbt.dsi.panel_id != MIPI_DSI_GENERIC_PANEL_ID)
@@ -853,40 +890,11 @@ parse_mipi_sequence(struct drm_i915_private *dev_priv,
*/
dev_priv->vbt.dsi.seq_version = sequence->version;
- seq_data = &sequence->data[0];
-
- /*
-* sequence block is variable length and hence we need to parse and
-* get the sequence data for specific panel id
-*/
- for (i = 0; i < MAX_MIPI_CONFIGURATIONS; i++) {
- panel_id = *seq_data;
- seq_size = *((u16 *) (seq_data + 1));
- if (panel_id == panel_type)
- break;
-
- /* skip the sequence including seq header of 3 bytes */
- seq_data = seq_data + 3 + seq_size;
- if ((seq_data - &sequence->data[0]) > block_size) {
- DRM_ERROR("Sequence start is beyond sequence block
size, corrupted sequence block\n");
- return;
- }
- }
-
- if (i == MAX_MIPI_CONFIGURATIONS) {
- DRM_ERROR("Sequence block detected but no valid
configuration\n");
+ seq_data = find_panel_sequence_block(sequence, panel_type, &seq_size);
+ if (!seq_data)
return;
- }
-
- /* check if found sequence is completely within the sequence block
-* just being paranoid */
- if (seq_size > block_size) {
- DRM_ERROR("Corrupted sequence/size, bailing out\n");
- return;
- }
- /* skip the panel id(1 byte) and seq size(2 bytes) */
- dev_priv->vbt.dsi.data = kmemdup(seq_data + 3, seq_size, GFP_KERNEL);
+ dev_priv->vbt.dsi.data = kmemdup(seq_data, seq_size, GFP_KERNEL);
if (!dev_priv->vbt.dsi.data)
return;
--
2.1.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915: Add RPM references in the *_get_hw_state functions
On Mon, Jan 4, 2016 at 4:05 PM, Ville Syrjälä wrote: > I think what we really need here is some kind of > intel_display_power_get_unless_zero() thing. We need to make sure not > only that the rpm reference is held when reading out the state, but also > that the relevant power well(s) remain enabled while we're reading out > the state. Yeah, we need to check whether a given piece of hw is enabled or not. All the get_hw_state funcs do that, using intel_display_power_is_enabled. But it looks like intel_ddi_get_hw_state partially gets this wrong. That would also address the same backtrace in module unload. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: add kerneldoc for intel_lr_context_size()
On Tue, Jan 05, 2016 at 12:21:33PM +, Dave Gordon wrote:
> This function was recently renamed & exposed, so now it gets documented
>
> Signed-off-by: Dave Gordon
Queued for -next, thanks for the patch.
-Daniel
> ---
> drivers/gpu/drm/i915/intel_lrc.c | 14 ++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c
> b/drivers/gpu/drm/i915/intel_lrc.c
> index 8da67b3..3662d14 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -2386,6 +2386,20 @@ void intel_lr_context_free(struct intel_context *ctx)
> }
> }
>
> +/**
> + * intel_lr_context_size() - return the size of the context for an engine
> + * @ring: which engine to find the context size for
> + *
> + * Each engine may require a different amount of space for a context image,
> + * so when allocating (or copying) an image, this function can be used to
> + * find the right size for the specific engine.
> + *
> + * Return: size (in bytes) of an engine-specific context image
> + *
> + * Note: this size includes the HWSP, which is part of the context image
> + * in LRC mode, but does not include the "shared data page" used with
> + * GuC submission. The caller should account for this if using the GuC.
> + */
> uint32_t intel_lr_context_size(struct intel_engine_cs *ring)
> {
> int ret = 0;
> --
> 1.9.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 05/15] drm/i915/bios: abstract finding the panel sequence block
On Tue, Jan 05, 2016 at 02:45:00PM +0200, Jani Nikula wrote:
> On Tue, 05 Jan 2016, Daniel Vetter wrote:
> > On Mon, Dec 21, 2015 at 03:10:56PM +0200, Jani Nikula wrote:
> >> Make the whole thing easier to read.
> >>
> >> Signed-off-by: Jani Nikula
> >> ---
> >> drivers/gpu/drm/i915/intel_bios.c | 76
> >> +--
> >> 1 file changed, 42 insertions(+), 34 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_bios.c
> >> b/drivers/gpu/drm/i915/intel_bios.c
> >> index 7393596df37d..9511a5341562 100644
> >> --- a/drivers/gpu/drm/i915/intel_bios.c
> >> +++ b/drivers/gpu/drm/i915/intel_bios.c
> >> @@ -697,7 +697,7 @@ parse_psr(struct drm_i915_private *dev_priv, const
> >> struct bdb_header *bdb)
> >>dev_priv->vbt.psr.tp2_tp3_wakeup_time = psr_table->tp2_tp3_wakeup_time;
> >> }
> >>
> >> -static u8 *goto_next_sequence(u8 *data, int *size)
> >> +static u8 *goto_next_sequence(u8 *data, u16 *size)
> >> {
> >>u16 len;
> >>int tmp = *size;
> >> @@ -818,15 +818,52 @@ parse_mipi_config(struct drm_i915_private *dev_priv,
> >>dev_priv->vbt.dsi.panel_id = MIPI_DSI_GENERIC_PANEL_ID;
> >> }
> >>
> >> +/* Find the sequence block and size for the given panel. */
> >> +static const u8 *
> >> +find_panel_sequence_block(const struct bdb_mipi_sequence *sequence,
> >> +u16 panel_id, u16 *seq_size)
> >> +{
> >> + u32 total = get_blocksize(sequence);
> >> + const u8 *data = &sequence->data[0];
> >> + u8 current_id;
> >> + u16 current_size;
> >> + int index = 0;
> >> + int i;
> >> +
> >> + for (i = 0; i < MAX_MIPI_CONFIGURATIONS && index + 3 < total; i++) {
> >
> > Commit message should mention that you make the parsin more robust and
> > ensure we don't walk over the end of the allocated buffer.
>
> Agreed. Although it's implied in the Signed-off-by line. ;)
>
> >
> >> + current_id = *(data + index);
> >> + index++;
> >> +
> >> + current_size = *((const u16 *)(data + index));
> >> + index += 2;
> >> +
> >> + if (index + current_size > total) {
> >> + DRM_ERROR("Invalid sequence block\n");
> >> + return NULL;
> >> + }
> >> +
> >> + if (current_id == panel_id) {
> >> + *seq_size = current_size;
> >> + return data + index;
> >> + }
> >> +
> >> + index += current_size;
> >> + }
> >> +
> >> + DRM_ERROR("Sequence block detected but no valid configuration\n");
> >> +
> >> + return NULL;
> >> +}
> >> +
> >> static void
> >> parse_mipi_sequence(struct drm_i915_private *dev_priv,
> >>const struct bdb_header *bdb)
> >> {
> >>const struct bdb_mipi_sequence *sequence;
> >>const u8 *seq_data;
> >> + u16 seq_size;
> >>u8 *data;
> >>u16 block_size;
> >> - int i, panel_id, seq_size;
> >>
> >>/* Only our generic panel driver uses the sequence block. */
> >>if (dev_priv->vbt.dsi.panel_id != MIPI_DSI_GENERIC_PANEL_ID)
> >> @@ -853,40 +890,11 @@ parse_mipi_sequence(struct drm_i915_private
> >> *dev_priv,
> >> */
> >>dev_priv->vbt.dsi.seq_version = sequence->version;
> >>
> >> - seq_data = &sequence->data[0];
> >> -
> >> - /*
> >> - * sequence block is variable length and hence we need to parse and
> >> - * get the sequence data for specific panel id
> >> - */
> >> - for (i = 0; i < MAX_MIPI_CONFIGURATIONS; i++) {
> >> - panel_id = *seq_data;
> >> - seq_size = *((u16 *) (seq_data + 1));
> >> - if (panel_id == panel_type)
> >> - break;
> >> -
> >> - /* skip the sequence including seq header of 3 bytes */
> >> - seq_data = seq_data + 3 + seq_size;
> >> - if ((seq_data - &sequence->data[0]) > block_size) {
> >> - DRM_ERROR("Sequence start is beyond sequence block
> >> size, corrupted sequence block\n");
> >> - return;
> >> - }
> >> - }
> >> -
> >> - if (i == MAX_MIPI_CONFIGURATIONS) {
> >> - DRM_ERROR("Sequence block detected but no valid
> >> configuration\n");
> >> + seq_data = find_panel_sequence_block(sequence, panel_type, &seq_size);
> >> + if (!seq_data)
> >>return;
> >> - }
> >> -
> >> - /* check if found sequence is completely within the sequence block
> >> - * just being paranoid */
> >> - if (seq_size > block_size) {
> >> - DRM_ERROR("Corrupted sequence/size, bailing out\n");
> >> - return;
> >> - }
> >>
> >> - /* skip the panel id(1 byte) and seq size(2 bytes) */
> >> - dev_priv->vbt.dsi.data = kmemdup(seq_data + 3, seq_size, GFP_KERNEL);
> >> + dev_priv->vbt.dsi.data = kmemdup(seq_data, seq_size, GFP_KERNEL);
> >
> > Should dropping the +3 be in a separate patch?
>
> Really I'd rather not if you don't mind. The end result is the same, but
> I'd have to think the function over again just to add a throwaway
> intermediate step. Unless I just replace the return state
Re: [Intel-gfx] [PATCH v2] drm/i915: Add RPM references in the *_get_hw_state functions
On Tue, Jan 05, 2016 at 02:54:31PM +0100, Daniel Vetter wrote: > On Mon, Jan 4, 2016 at 4:05 PM, Ville Syrjälä > wrote: > > I think what we really need here is some kind of > > intel_display_power_get_unless_zero() thing. We need to make sure not > > only that the rpm reference is held when reading out the state, but also > > that the relevant power well(s) remain enabled while we're reading out > > the state. > > Yeah, we need to check whether a given piece of hw is enabled or not. > All the get_hw_state funcs do that, using > intel_display_power_is_enabled. But it looks like > intel_ddi_get_hw_state partially gets this wrong. That would also > address the same backtrace in module unload. Tried __only__ this patch but it doesn't fix the trace in driver unload, had the same hunch > -Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx signature.asc Description: Digital signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 06/15] drm/i915/bios: rewrite sequence block parsing
On Mon, Dec 21, 2015 at 03:10:57PM +0200, Jani Nikula wrote:
> Make everything a bit more readable and clear.
>
> Signed-off-by: Jani Nikula
A real pain to check, but I think I convinced myself that this is
equivalent code.
Reviewed-by: Daniel Vetter
> ---
> drivers/gpu/drm/i915/i915_drv.h | 2 +-
> drivers/gpu/drm/i915/intel_bios.c | 158
> +-
> drivers/gpu/drm/i915/intel_bios.h | 4 +-
> 3 files changed, 57 insertions(+), 107 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 9b82c4532893..07c4fc539680 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1487,7 +1487,7 @@ struct intel_vbt_data {
> u8 seq_version;
> u32 size;
> u8 *data;
> - u8 *sequence[MIPI_SEQ_MAX];
> + const u8 *sequence[MIPI_SEQ_MAX];
> } dsi;
>
> int crt_ddc_pin;
> diff --git a/drivers/gpu/drm/i915/intel_bios.c
> b/drivers/gpu/drm/i915/intel_bios.c
> index 9511a5341562..d6eaf32f33e5 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -697,73 +697,6 @@ parse_psr(struct drm_i915_private *dev_priv, const
> struct bdb_header *bdb)
> dev_priv->vbt.psr.tp2_tp3_wakeup_time = psr_table->tp2_tp3_wakeup_time;
> }
>
> -static u8 *goto_next_sequence(u8 *data, u16 *size)
> -{
> - u16 len;
> - int tmp = *size;
> -
> - if (--tmp < 0)
> - return NULL;
> -
> - /* goto first element */
> - data++;
> - while (1) {
> - switch (*data) {
> - case MIPI_SEQ_ELEM_SEND_PKT:
> - /*
> - * skip by this element payload size
> - * skip elem id, command flag and data type
> - */
> - tmp -= 5;
> - if (tmp < 0)
> - return NULL;
> -
> - data += 3;
> - len = *((u16 *)data);
> -
> - tmp -= len;
> - if (tmp < 0)
> - return NULL;
> -
> - /* skip by len */
> - data = data + 2 + len;
> - break;
> - case MIPI_SEQ_ELEM_DELAY:
> - /* skip by elem id, and delay is 4 bytes */
> - tmp -= 5;
> - if (tmp < 0)
> - return NULL;
> -
> - data += 5;
> - break;
> - case MIPI_SEQ_ELEM_GPIO:
> - tmp -= 3;
> - if (tmp < 0)
> - return NULL;
> -
> - data += 3;
> - break;
> - default:
> - DRM_ERROR("Unknown element\n");
> - return NULL;
> - }
> -
> - /* end of sequence ? */
> - if (*data == 0)
> - break;
> - }
> -
> - /* goto next sequence or end of block byte */
> - if (--tmp < 0)
> - return NULL;
> -
> - data++;
> -
> - /* update amount of data left for the sequence block to be parsed */
> - *size = tmp;
> - return data;
> -}
> -
> static void
> parse_mipi_config(struct drm_i915_private *dev_priv,
> const struct bdb_header *bdb)
> @@ -855,6 +788,39 @@ find_panel_sequence_block(const struct bdb_mipi_sequence
> *sequence,
> return NULL;
> }
>
> +static int goto_next_sequence(const u8 *data, int index, int total)
> +{
> + u16 len;
> +
> + /* Skip Sequence Byte. */
> + for (index = index + 1; index < total; index += len) {
> + u8 operation_byte = *(data + index);
> + index++;
> +
> + switch (operation_byte) {
> + case MIPI_SEQ_ELEM_END:
> + return index;
> + case MIPI_SEQ_ELEM_SEND_PKT:
> + if (index + 4 > total)
> + return 0;
> +
> + len = *((const u16 *)(data + index + 2)) + 4;
> + break;
> + case MIPI_SEQ_ELEM_DELAY:
> + len = 4;
> + break;
> + case MIPI_SEQ_ELEM_GPIO:
> + len = 2;
> + break;
> + default:
> + DRM_ERROR("Unknown operation byte\n");
> + return 0;
> + }
> + }
> +
> + return 0;
> +}
> +
> static void
> parse_mipi_sequence(struct drm_i915_private *dev_priv,
> const struct bdb_header *bdb)
> @@ -863,7 +829,7 @@ parse_mipi_sequence(struct drm_i915_private *dev_priv,
> const u8 *seq_data;
> u16 seq_size;
> u8 *data;
> - u16 block_size;
> + int index = 0;
>
> /* Only our generic panel driver uses the sequenc
Re: [Intel-gfx] [PATCH 07/15] drm/i915/dsi: be defensive about out of bounds sequence id
On Mon, Dec 21, 2015 at 03:10:58PM +0200, Jani Nikula wrote:
> Untie the VBT based generic panel driver from the VBT parsing, so that
> the two don't have to be updated in lockstep.
>
> Signed-off-by: Jani Nikula
Reviewed-by: Daniel Vetter
> ---
> drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 22 +++---
> 1 file changed, 15 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> index a5e99ac305da..45512e0df57a 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> @@ -246,14 +246,21 @@ static const fn_mipi_elem_exec exec_elem[] = {
> */
>
> static const char * const seq_name[] = {
> - "UNDEFINED",
> - "MIPI_SEQ_ASSERT_RESET",
> - "MIPI_SEQ_INIT_OTP",
> - "MIPI_SEQ_DISPLAY_ON",
> - "MIPI_SEQ_DISPLAY_OFF",
> - "MIPI_SEQ_DEASSERT_RESET"
> + [MIPI_SEQ_ASSERT_RESET] = "MIPI_SEQ_ASSERT_RESET",
> + [MIPI_SEQ_INIT_OTP] = "MIPI_SEQ_INIT_OTP",
> + [MIPI_SEQ_DISPLAY_ON] = "MIPI_SEQ_DISPLAY_ON",
> + [MIPI_SEQ_DISPLAY_OFF] = "MIPI_SEQ_DISPLAY_OFF",
> + [MIPI_SEQ_DEASSERT_RESET] = "MIPI_SEQ_DEASSERT_RESET",
> };
>
> +static const char *sequence_name(enum mipi_seq seq_id)
> +{
> + if (seq_id < ARRAY_SIZE(seq_name) && seq_name[seq_id])
> + return seq_name[seq_id];
> + else
> + return "(unknown)";
> +}
> +
> static void generic_exec_sequence(struct intel_dsi *intel_dsi, const u8
> *data)
> {
> fn_mipi_elem_exec mipi_elem_exec;
> @@ -262,7 +269,8 @@ static void generic_exec_sequence(struct intel_dsi
> *intel_dsi, const u8 *data)
> if (!data)
> return;
>
> - DRM_DEBUG_DRIVER("Starting MIPI sequence - %s\n", seq_name[*data]);
> + DRM_DEBUG_DRIVER("Starting MIPI sequence %u - %s\n",
> + *data, sequence_name(*data));
>
> /* go to the first element of the sequence */
> data++;
> --
> 2.1.4
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 08/15] drm/i915/dsi: be defensive about out of bounds operation byte
On Mon, Dec 21, 2015 at 03:10:59PM +0200, Jani Nikula wrote:
> Untie the VBT based generic panel driver from the VBT parsing, so that
> the two don't have to be updated in lockstep.
>
> Signed-off-by: Jani Nikula
> ---
> drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 22 +-
> 1 file changed, 9 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> index 45512e0df57a..ba5355506590 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> @@ -232,11 +232,9 @@ static const u8 *mipi_exec_gpio(struct intel_dsi
> *intel_dsi, const u8 *data)
> typedef const u8 * (*fn_mipi_elem_exec)(struct intel_dsi *intel_dsi,
> const u8 *data);
> static const fn_mipi_elem_exec exec_elem[] = {
> - NULL, /* reserved */
> - mipi_exec_send_packet,
> - mipi_exec_delay,
> - mipi_exec_gpio,
> - NULL, /* status read; later */
> + [MIPI_SEQ_ELEM_SEND_PKT] = mipi_exec_send_packet,
> + [MIPI_SEQ_ELEM_DELAY] = mipi_exec_delay,
> + [MIPI_SEQ_ELEM_GPIO] = mipi_exec_gpio,
> };
>
> /*
> @@ -264,7 +262,6 @@ static const char *sequence_name(enum mipi_seq seq_id)
> static void generic_exec_sequence(struct intel_dsi *intel_dsi, const u8
> *data)
> {
> fn_mipi_elem_exec mipi_elem_exec;
> - int index;
>
> if (!data)
> return;
> @@ -277,15 +274,14 @@ static void generic_exec_sequence(struct intel_dsi
> *intel_dsi, const u8 *data)
>
> /* parse each byte till we reach end of sequence byte - 0x00 */
> while (1) {
> - index = *data;
> - mipi_elem_exec = exec_elem[index];
> - if (!mipi_elem_exec) {
> - DRM_ERROR("Unsupported MIPI element, skipping sequence
> execution\n");
> + u8 operation_byte = *data++;
> + if (operation_byte >= ARRAY_SIZE(exec_elem) ||
> + !exec_elem[operation_byte]) {
> + DRM_ERROR("Unsupported MIPI operation byte %u\n",
Maybe DRM_ERROR in the previous patch too? Just for ocd consistency.
Reviewed-by: Daniel Vetter
> + operation_byte);
> return;
> }
> -
> - /* goto element payload */
> - data++;
> + mipi_elem_exec = exec_elem[operation_byte];
>
> /* execute the element specific rotines */
> data = mipi_elem_exec(intel_dsi, data);
> --
> 2.1.4
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 14/15] drm/i915/dsi: skip unknown elements for sequence block v3+
On Mon, Dec 21, 2015 at 03:11:05PM +0200, Jani Nikula wrote:
> The sequence block has sizes of elements after the operation byte since
> sequence block v3. Use it to skip elements we don't support yet.
>
> Signed-off-by: Jani Nikula
> ---
> drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 43
> +-
> 1 file changed, 24 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> index eabfd9eb9cc0..1f9c80d21904 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> @@ -335,31 +335,36 @@ static void generic_exec_sequence(struct intel_dsi
> *intel_dsi, const u8 *data)
> if (dev_priv->vbt.dsi.seq_version >= 3)
> data += 4;
>
> - /* parse each byte till we reach end of sequence byte - 0x00 */
> while (1) {
> u8 operation_byte = *data++;
> - if (operation_byte >= ARRAY_SIZE(exec_elem) ||
> - !exec_elem[operation_byte]) {
> + u8 operation_size = 0;
> +
> + if (operation_byte == MIPI_SEQ_ELEM_END)
> + break;
> +
> + if (operation_byte < ARRAY_SIZE(exec_elem) &&
> + exec_elem[operation_byte])
&& exec_elem[operation_byte] is redundant since you assing it to NULL
anyway in the else clause. While I bikeshed: Might look prettier with ?:
> + mipi_elem_exec = exec_elem[operation_byte];
> + else
> + mipi_elem_exec = NULL;
> +
> + /* Size of Operation. */
> + if (dev_priv->vbt.dsi.seq_version >= 3)
> + operation_size = *data++;
> +
> + if (mipi_elem_exec) {
> + data = mipi_elem_exec(intel_dsi, data);
> + } else if (operation_size) {
> + /* We have size, skip. */
> + DRM_DEBUG_KMS("Unsupported MIPI operation byte %u\n",
> + operation_byte);
DRM_ERROR, like in the other cases we fail to parse the sequence fully?
Either way on both bikesheds: Reviewed-by: Daniel Vetter
> + data += operation_size;
> + } else {
> + /* No size, can't skip without parsing. */
> DRM_ERROR("Unsupported MIPI operation byte %u\n",
> operation_byte);
> return;
> }
> - mipi_elem_exec = exec_elem[operation_byte];
> -
> - /* Skip Size of Operation. */
> - if (dev_priv->vbt.dsi.seq_version >= 3)
> - data++;
> -
> - /* execute the element specific rotines */
> - data = mipi_elem_exec(intel_dsi, data);
> -
> - /*
> - * After processing the element, data should point to
> - * next element or end of sequence
> - * check if have we reached end of sequence
> - */
> - if (*data == 0x00)
> - break;
> }
> }
>
> --
> 2.1.4
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 15/15] drm/i915/dsi: reduce tedious repetition
On Mon, Dec 21, 2015 at 03:11:06PM +0200, Jani Nikula wrote:
> Make it a bit tidier.
Also more save.
> Signed-off-by: Jani Nikula
Reviewed-by: Daniel Vetter
> ---
> drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 62
> +++---
> 1 file changed, 22 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> index 1f9c80d21904..f0116a6c14cd 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> @@ -317,18 +317,30 @@ static const char *sequence_name(enum mipi_seq seq_id)
> return "(unknown)";
> }
>
> -static void generic_exec_sequence(struct intel_dsi *intel_dsi, const u8
> *data)
> +static void generic_exec_sequence(struct drm_panel *panel, enum mipi_seq
> seq_id)
> {
> + struct vbt_panel *vbt_panel = to_vbt_panel(panel);
> + struct intel_dsi *intel_dsi = vbt_panel->intel_dsi;
> struct drm_i915_private *dev_priv = to_i915(intel_dsi->base.base.dev);
> + const u8 *data;
> fn_mipi_elem_exec mipi_elem_exec;
>
> - if (!data)
> + if (WARN_ON(seq_id >= ARRAY_SIZE(dev_priv->vbt.dsi.sequence)))
> return;
>
> - DRM_DEBUG_DRIVER("Starting MIPI sequence %u - %s\n",
> - *data, sequence_name(*data));
> + data = dev_priv->vbt.dsi.sequence[seq_id];
> + if (!data) {
> + DRM_DEBUG_KMS("MIPI sequence %d - %s not available\n",
> + seq_id, sequence_name(seq_id));
> + return;
> + }
>
> - /* go to the first element of the sequence */
> + WARN_ON(*data != seq_id);
> +
> + DRM_DEBUG_KMS("Starting MIPI sequence %d - %s\n",
> + seq_id, sequence_name(seq_id));
> +
> + /* Skip Sequence Byte. */
> data++;
>
> /* Skip Size of Sequence. */
> @@ -370,59 +382,29 @@ static void generic_exec_sequence(struct intel_dsi
> *intel_dsi, const u8 *data)
>
> static int vbt_panel_prepare(struct drm_panel *panel)
> {
> - struct vbt_panel *vbt_panel = to_vbt_panel(panel);
> - struct intel_dsi *intel_dsi = vbt_panel->intel_dsi;
> - struct drm_device *dev = intel_dsi->base.base.dev;
> - struct drm_i915_private *dev_priv = dev->dev_private;
> - const u8 *sequence;
> -
> - sequence = dev_priv->vbt.dsi.sequence[MIPI_SEQ_ASSERT_RESET];
> - generic_exec_sequence(intel_dsi, sequence);
> -
> - sequence = dev_priv->vbt.dsi.sequence[MIPI_SEQ_INIT_OTP];
> - generic_exec_sequence(intel_dsi, sequence);
> + generic_exec_sequence(panel, MIPI_SEQ_ASSERT_RESET);
> + generic_exec_sequence(panel, MIPI_SEQ_INIT_OTP);
>
> return 0;
> }
>
> static int vbt_panel_unprepare(struct drm_panel *panel)
> {
> - struct vbt_panel *vbt_panel = to_vbt_panel(panel);
> - struct intel_dsi *intel_dsi = vbt_panel->intel_dsi;
> - struct drm_device *dev = intel_dsi->base.base.dev;
> - struct drm_i915_private *dev_priv = dev->dev_private;
> - const u8 *sequence;
> -
> - sequence = dev_priv->vbt.dsi.sequence[MIPI_SEQ_DEASSERT_RESET];
> - generic_exec_sequence(intel_dsi, sequence);
> + generic_exec_sequence(panel, MIPI_SEQ_DEASSERT_RESET);
>
> return 0;
> }
>
> static int vbt_panel_enable(struct drm_panel *panel)
> {
> - struct vbt_panel *vbt_panel = to_vbt_panel(panel);
> - struct intel_dsi *intel_dsi = vbt_panel->intel_dsi;
> - struct drm_device *dev = intel_dsi->base.base.dev;
> - struct drm_i915_private *dev_priv = dev->dev_private;
> - const u8 *sequence;
> -
> - sequence = dev_priv->vbt.dsi.sequence[MIPI_SEQ_DISPLAY_ON];
> - generic_exec_sequence(intel_dsi, sequence);
> + generic_exec_sequence(panel, MIPI_SEQ_DISPLAY_ON);
>
> return 0;
> }
>
> static int vbt_panel_disable(struct drm_panel *panel)
> {
> - struct vbt_panel *vbt_panel = to_vbt_panel(panel);
> - struct intel_dsi *intel_dsi = vbt_panel->intel_dsi;
> - struct drm_device *dev = intel_dsi->base.base.dev;
> - struct drm_i915_private *dev_priv = dev->dev_private;
> - const u8 *sequence;
> -
> - sequence = dev_priv->vbt.dsi.sequence[MIPI_SEQ_DISPLAY_OFF];
> - generic_exec_sequence(intel_dsi, sequence);
> + generic_exec_sequence(panel, MIPI_SEQ_DISPLAY_OFF);
>
> return 0;
> }
> --
> 2.1.4
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ success: Fi.CI.BAT
== Summary == Built on 05ade905f2fda5416476677509e016ef830d181a drm-intel-nightly: 2016y-01m-05d-13h-00m-24s UTC integration manifest Test gem_storedw_loop: Subgroup basic-render: dmesg-warn -> PASS (bdw-nuci7) Test kms_flip: Subgroup basic-flip-vs-modeset: dmesg-warn -> PASS (bsw-nuc-2) UNSTABLE Test kms_pipe_crc_basic: Subgroup read-crc-pipe-a: dmesg-warn -> PASS (skl-i7k-2) UNSTABLE Subgroup read-crc-pipe-a-frame-sequence: pass -> DMESG-WARN (byt-nuc) UNSTABLE Subgroup read-crc-pipe-b: dmesg-warn -> PASS (skl-i5k-2) UNSTABLE dmesg-warn -> PASS (snb-dellxps) UNSTABLE Subgroup suspend-read-crc-pipe-a: dmesg-warn -> PASS (snb-x220t) UNSTABLE Subgroup suspend-read-crc-pipe-b: pass -> DMESG-WARN (snb-x220t) UNSTABLE dmesg-warn -> PASS (skl-i7k-2) Test pm_rpm: Subgroup basic-rte: dmesg-warn -> PASS (byt-nuc) UNSTABLE bdw-nuci7total:132 pass:122 dwarn:1 dfail:0 fail:0 skip:9 bdw-ultratotal:132 pass:124 dwarn:2 dfail:0 fail:0 skip:6 bsw-nuc-2total:135 pass:114 dwarn:1 dfail:0 fail:0 skip:20 byt-nuc total:135 pass:120 dwarn:2 dfail:0 fail:0 skip:13 hsw-brixbox total:135 pass:126 dwarn:2 dfail:0 fail:0 skip:7 hsw-gt2 total:135 pass:130 dwarn:1 dfail:0 fail:0 skip:4 hsw-xps12total:132 pass:125 dwarn:3 dfail:0 fail:0 skip:4 ilk-hp8440p total:135 pass:100 dwarn:0 dfail:0 fail:0 skip:35 ivb-t430stotal:135 pass:127 dwarn:2 dfail:0 fail:0 skip:6 skl-i5k-2total:135 pass:125 dwarn:2 dfail:0 fail:0 skip:8 skl-i7k-2total:135 pass:125 dwarn:2 dfail:0 fail:0 skip:8 snb-dellxps total:135 pass:122 dwarn:1 dfail:0 fail:0 skip:12 snb-x220ttotal:135 pass:121 dwarn:2 dfail:0 fail:1 skip:11 Results at /archive/results/CI_IGT_test/Patchwork_1084/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [4.1.7-rt8][report] Very high cyclictest latency during glmark2 on i915 gpu
On Wed, Dec 23, 2015 at 01:40:06PM +0100, Christoph Mathys wrote:
> On Tue, Dec 22, 2015 at 4:37 PM, Sebastian Andrzej Siewior
> wrote:
> > I have to admit, the i915 tries very hard to avoid running on -RT. Could
> > you try the s/local_irq_disable();/local_irq_disable_nort();/ patch
> > mentioned in the thread?
>
> It did not crash so far, I did some usual work and executed glmark2
> several times. The BUG has not surfaced again. BUT, the latency is
> still far from ideal, it takes only seconds for the maximum latency to
> spike into the range of milliseconds. By the way, this is now
> 4.1.15-rt17 with the changes from here:
> http://www.spinics.net/lists/linux-rt-users/msg13543.html
>
> During some tests with cyclictests --breaktrace I've seen more BUG
> messages, but they might relate to the kernel tracing. Unfortunately,
> I could not make much of the --breaktrace.
Waking the GT takes a while, and for a bunch of reasons we need to be able
to do this from irq context, hence spin_lock_irq. Doesn't RT convert that
to a mutex?
Otherwise not sure what would prevent interrupts in your backtrace ...
-Daniel
>
> [ 4629.753864] BUG: sleeping function called from invalid context at
> kernel/locking/rtmutex.c:917
> [ 4629.753865] in_atomic(): 1, irqs_disabled(): 0, pid: 1913, name: Xorg
> [ 4629.753866] 2 locks held by Xorg/1913:
> [ 4629.753879] #0: (crtc_ww_class_acquire){+.+.+.}, at:
> [] drm_modeset_lock_crtc+0x4c/0x100 [drm]
> [ 4629.753887] #1: (crtc_ww_class_mutex){+.+.+.}, at:
> [] drm_modeset_lock+0x41/0x130 [drm]
> [ 4629.753888] CPU: 4 PID: 1913 Comm: Xorg Tainted: GE
> 4.1.15-i915-patch-realtime-1-rt17+ #4
> [ 4629.753889] Hardware name: Komax AG, Dierikon Komax-PC/KMX-B75,
> BIOS DD3-1-1D 08/21/2013
> [ 4629.753891] 81c862ce 8803e92538b8 81805313
> 5b10
> [ 4629.753892] 8803e8baa660 8803e92538e8 8108813a
> 8803e92538d8
> [ 4629.753893] 8803fe6d0070 8803fe6d0070 8803fe6d0068
> 8803e9253918
> [ 4629.753893] Call Trace:
> [ 4629.753897] [] dump_stack+0x4a/0x61
> [ 4629.753899] [] ___might_sleep+0x13a/0x200
> [ 4629.753901] [] rt_spin_lock+0x24/0x60
> [ 4629.753916] [] gen6_read32+0x46/0x380 [i915]
> [ 4629.753925] [] gm45_get_vblank_counter+0x32/0x40 [i915]
> [ 4629.753934] []
> ftrace_raw_event_i915_pipe_update_start+0x65/0xb0 [i915]
> [ 4629.753944] [] intel_pipe_update_start+0x2a3/0x640
> [i915]
> [ 4629.753946] [] ? prepare_to_wait_event+0x130/0x130
> [ 4629.753956] [] intel_begin_crtc_commit+0x166/0x1e0
> [i915]
> [ 4629.753961] []
> drm_plane_helper_commit+0x112/0x2c0 [drm_kms_helper]
> [ 4629.753963] [] drm_plane_helper_update+0x9a/0xf0
> [drm_kms_helper]
> [ 4629.753970] [] __setplane_internal+0x248/0x350 [drm]
> [ 4629.753975] [] drm_mode_cursor_universal+0x125/0x210
> [drm]
> [ 4629.753981] [] drm_mode_cursor_common+0x7f/0x1b0 [drm]
> [ 4629.753987] [] drm_mode_cursor_ioctl+0x41/0x50 [drm]
> [ 4629.753992] [] drm_ioctl+0x349/0x690 [drm]
> [ 4629.753998] [] ? drm_mode_setcrtc+0x630/0x630 [drm]
> [ 4629.753999] [] ? local_clock+0x25/0x30
> [ 4629.754001] [] ?
> lock_release_holdtime.part.31+0xd3/0x1a0
> [ 4629.754002] [] do_vfs_ioctl+0x328/0x5e0
> [ 4629.754004] [] ? __fget+0x5/0x210
> [ 4629.754004] [] ? __fget_light+0x2a/0xa0
> [ 4629.754005] [] SyS_ioctl+0x81/0xa0
> [ 4629.754007] [] tracesys_phase2+0x88/0x8d
>
> [ 361.526236] BUG: sleeping function called from invalid context at
> kernel/locking/rtmutex.c:917
> [ 361.526237] in_atomic(): 1, irqs_disabled(): 0, pid: 667, name: irq/51-i915
> [ 361.526237] no locks held by irq/51-i915/667.
> [ 361.526239] CPU: 4 PID: 667 Comm: irq/51-i915 Tainted: G
> E 4.1.15-i915-patch-realtime-1-rt17+ #4
> [ 361.526240] Hardware name: Komax AG, Dierikon Komax-PC/KMX-B75,
> BIOS DD3-1-1D 08/21/2013
> [ 361.526242] 81c862ce 8803fe6e7b58 81805313
>
> [ 361.526243] 8803ff3d4cc0 8803fe6e7b88 8108813a
> 0296
> [ 361.526244] 8803fe6d0070 8803fe6d0070 8803fe6d0068
> 8803fe6e7bb8
> [ 361.526244] Call Trace:
> [ 361.526249] [] dump_stack+0x4a/0x61
> [ 361.526251] [] ___might_sleep+0x13a/0x200
> [ 361.526253] [] rt_spin_lock+0x24/0x60
> [ 361.526281] [] gen6_read32+0x46/0x380 [i915]
> [ 361.526283] [] ?
> trace_event_buffer_lock_reserve+0x40/0x80
> [ 361.526293] [] gen6_ring_get_seqno+0x2f/0x40 [i915]
> [ 361.526301] []
> ftrace_raw_event_i915_gem_request_notify+0x5f/0x90 [i915]
> [ 361.526309] [] notify_ring.isra.12+0xcd/0x240 [i915]
> [ 361.526316] [] snb_gt_irq_handler+0x12c/0x180 [i915]
> [ 361.526323] [] ironlake_irq_handler+0x1c7/0xfe0 [i915]
> [ 361.526324] [] ? finish_task_switch+0x4d/0x140
> [ 361.526325] [] irq_forced_thread_fn+0x27/0x70
> [ 361.526326] [] irq_thread+0x149/0x1f0
> [ 361.526327] [] ? irq_thread_fn+0x40/0x40
> [ 361.526328] [] ? wake_threads_waitq+0x30/0x30
> [ 361.526329] [] ? irq_thread_check_aff
Re: [Intel-gfx] [4.1.7-rt8][report] Very high cyclictest latency during glmark2 on i915 gpu
On Tue, Dec 22, 2015 at 04:37:26PM +0100, Sebastian Andrzej Siewior wrote: > * Christoph Mathys | 2015-12-21 14:19:10 [+0100]: > > >While playing with 4.1.13-rt15 I stumbled across the following thread > >where Luis reports the same problem with i915 gpu: > >i915: sleeping function called from invalid context at > >intel_pipe_update_start/end > >http://www.spinics.net/lists/linux-rt-users/msg13543.html > > > >Sebastian suggested to set i915.use_mmio_flip to -1. I tried this, and > >this avoids the callstack that I've posted before > >(intel_mmio_flip_work). The BUG below is now the dominant one: > perfect. > > |BUG: sleeping function called from invalid context at > kernel/locking/rtmutex.c:917 > |in_atomic(): 0, irqs_disabled(): 1, pid: 2109, name: Xorg > |hardirqs last disabled at (23744596): [] > intel_pipe_update_start+0x113/0x640 [i915] > |Call Trace: > | [] dump_stack+0x4a/0x61 > | [] ___might_sleep+0x13a/0x200 > | [] rt_spin_lock+0x24/0x60 > | [] ? migrate_disable+0x6c/0xe0 > | [] prepare_to_wait+0x2b/0xa0 > | [] intel_pipe_update_start+0x1c8/0x640 [i915] > | [] ? prepare_to_wait_event+0x130/0x130 > | [] intel_begin_crtc_commit+0x166/0x1e0 [i915] > | [] drm_plane_helper_commit+0x112/0x2c0 [drm_kms_helper] > | [] drm_plane_helper_update+0x9a/0xf0 > > I have to admit, the i915 tries very hard to avoid running on -RT. Could > you try the s/local_irq_disable();/local_irq_disable_nort();/ patch > mentioned in the thread? > > Anyone of the i915 hackers an idea how could get the i915 working > without disabling interrupts? Is really required? It's a correctness problem - if we don't disable everything we might miss the timeframe and your screen will tear. Hence why we essentially need to run this little section of code with hard-rt semantics. There's other display chips with proper design which don't need hacks like this one here. Now of course you might want to accept a broken screen in exchange for non-broken hard rt for the things you really care about, but I'm not sure how to encode this. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/bxt: Fix wm calculation for pixel format change
On Wed, Dec 23, 2015 at 10:52:36AM +0530, Nabendu Maiti wrote: > Recalculate watermark when there is a change in pixel format. > > Signed-off-by: Nabendu Maiti This seems like something that's been broken recently in the watermark shuffling. Please dig out the patch which broke this (using git blame works best usually) and add a Fixes: line per Documentation/SubmittingPatches. Please then also add the patch author, reviwers and anyone else Cc'ed on that patch to your patch here using Cc: lines in the s-o-b section, and then please resend. Also, do we have an igt testcase for this issue? If not, we should definitely have one. -Daniel > --- > drivers/gpu/drm/i915/intel_display.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 2d0b006..4bd5080 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -11819,6 +11819,7 @@ static bool intel_wm_need_update(struct drm_plane > *plane, > return false; > > if (cur->base.fb->modifier[0] != new->base.fb->modifier[0] || > + cur->base.fb->pixel_format != new->base.fb->pixel_format || > cur->base.rotation != new->base.rotation || > drm_rect_width(&new->src) != drm_rect_width(&cur->src) || > drm_rect_height(&new->src) != drm_rect_height(&cur->src) || > -- > 1.9.1 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/docs: more leftovers from the big vtable documentation pile
On Mon, Jan 04, 2016 at 07:53:36AM +0100, Daniel Vetter wrote:
> Another pile of vfuncs from the old gpu.tmpl xml documentation that
> I've forgotten to delete. I spotted a few more things to
> clarify/extend in the new kerneldoc while going through this once
> more.
>
> v2: Spelling fixes (Thierry).
Found a couple more.
> diff --git a/include/drm/drm_modeset_helper_vtables.h
> b/include/drm/drm_modeset_helper_vtables.h
> index 29e0dc50031d..ce5cbf63b5cf 100644
> --- a/include/drm/drm_modeset_helper_vtables.h
> +++ b/include/drm/drm_modeset_helper_vtables.h
> @@ -131,6 +131,12 @@ struct drm_crtc_helper_funcs {
>* Atomic drivers which need to inspect and adjust more state should
>* instead use the @atomic_check callback.
>*
> + * Also beware that the neither core nor helpers filter modes before
I'd drop the "the" above: "... that neither core nor helpers..."
> + * passing the to the driver. More specifically modes rejected by the
"passing them"?
> + * ->mode_valid callback from #drm_connector_helper_funcs can still be
> + * requested by userspace and therefore also need to be rejected in
> + * either this hook, or the one in #drm_encoder_helper_funcs.
> + *
>* RETURNS:
>*
>* True if an acceptable configuration is possible, false if the modeset
> @@ -188,7 +194,9 @@ struct drm_crtc_helper_funcs {
>* This callback is used by the legacy CRTC helpers to set a new
>* framebuffer and scanout position. It is optional and used as an
>* optimized fast-path instead of a full mode set operation with all the
> - * resulting flickering. Since it can't update other planes it's
> + * resulting flickering. If it is not present
> + * drm_crtc_helper_set_config() will fall back to a full modeset, using
> + * the ->mode_set() callbac. Since it can't update other planes it's
"callback"
>* incompatible with atomic modeset support.
>*
>* This callback is only used by the CRTC helpers and deprecated.
> @@ -439,6 +447,12 @@ struct drm_encoder_helper_funcs {
>* Atomic drivers which need to inspect and adjust more state should
>* instead use the @atomic_check callback.
>*
> + * Also beware that the neither core nor helpers filter modes before
> + * passing the to the driver. More specifically modes rejected by the
Same as above.
Thierry
signature.asc
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC] drm/i915/bxt: Add pipe_src size property
On Mon, Jan 04, 2016 at 07:06:15PM +0200, Ville Syrjälä wrote: > On Mon, Jan 04, 2016 at 11:16:39AM +0100, Maarten Lankhorst wrote: > > Hey, > > > > Op 23-12-15 om 12:05 schreef Nabendu Maiti: > > > This patch is adding pipesource size as property as intel property.User > > > application is allowed to change the pipe source size in runtime on > > > BXT/SKL. > > > Added the property as inteli crtc property. > > > > > > Comments and suggestions are requested for whether we can keep the > > > property as intel crtc property or move to drm level. > > > > > This property would get lost on a modeset. But why do you need a pipe_src > > property? > > It's needed if we want to use the panel fitter with non-eDP/LVDS/DSI > displays. > > Last time this came up I decided that we want to expose this via a new > "fixed_mode" property. Ie. userspace can choose the actual display > timings by setting the "fixed_mode" property with a specific mode, and > then the normal mode property will basically just control just the pipe > src size just like it already does for eDP/LVDS/DSI where we already have > the fixed_mode internally (just not exposed to usersapce). If the > fixed_mode is not specified, things will work just as they do right > now. Obviously for eDP/LVDS/DSI we will reject any request to > change/unset the fixed mode. > > The benefit of that approach is that things will work exactly the same > way as before unless the user explicitly sets the fixed_mode, and once > it's set thigns will work exactly the same way as they have worked so > far for eDP/LVDS/DSI. Also it keeps the policy of choosing the fixed > mode strictly is userspace for external displays. > > And as a bonus we will also expose the eDP/LVDS/DSI fixed_mode to > userspace giving userspace more information on what the actual panel > timings are. Currently userspace has to more or less guess that one > of the modes the connector claims to support has the actual panel > timings. > > So far I've not heard any opposing opinions to this plan. Hm yeah, pipe_src would run into all kinds of fun in conjunction with the existing fixed mode stuff we have. Just exposing the fixed as a prop might be simpler. But then we need to figure out what to do wrt the clock in the mode passed in through userspace - currently the fixed mode always wins, but for manual DRRS it would be nice if userspace could control it, and doesn't have to know that there's a fixed_mode. So semantically I think only exposing the pipe src to expose panel fitters would be cleaner. Then we'd need to deal with some internal troubles to make sure we combine everything correctly, but that shouldn't be too hard really. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 08/15] drm/i915/dsi: be defensive about out of bounds operation byte
On Tue, 05 Jan 2016, Daniel Vetter wrote:
> On Mon, Dec 21, 2015 at 03:10:59PM +0200, Jani Nikula wrote:
>> Untie the VBT based generic panel driver from the VBT parsing, so that
>> the two don't have to be updated in lockstep.
>>
>> Signed-off-by: Jani Nikula
>> ---
>> drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 22 +-
>> 1 file changed, 9 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
>> b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
>> index 45512e0df57a..ba5355506590 100644
>> --- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
>> +++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
>> @@ -232,11 +232,9 @@ static const u8 *mipi_exec_gpio(struct intel_dsi
>> *intel_dsi, const u8 *data)
>> typedef const u8 * (*fn_mipi_elem_exec)(struct intel_dsi *intel_dsi,
>> const u8 *data);
>> static const fn_mipi_elem_exec exec_elem[] = {
>> -NULL, /* reserved */
>> -mipi_exec_send_packet,
>> -mipi_exec_delay,
>> -mipi_exec_gpio,
>> -NULL, /* status read; later */
>> +[MIPI_SEQ_ELEM_SEND_PKT] = mipi_exec_send_packet,
>> +[MIPI_SEQ_ELEM_DELAY] = mipi_exec_delay,
>> +[MIPI_SEQ_ELEM_GPIO] = mipi_exec_gpio,
>> };
>>
>> /*
>> @@ -264,7 +262,6 @@ static const char *sequence_name(enum mipi_seq seq_id)
>> static void generic_exec_sequence(struct intel_dsi *intel_dsi, const u8
>> *data)
>> {
>> fn_mipi_elem_exec mipi_elem_exec;
>> -int index;
>>
>> if (!data)
>> return;
>> @@ -277,15 +274,14 @@ static void generic_exec_sequence(struct intel_dsi
>> *intel_dsi, const u8 *data)
>>
>> /* parse each byte till we reach end of sequence byte - 0x00 */
>> while (1) {
>> -index = *data;
>> -mipi_elem_exec = exec_elem[index];
>> -if (!mipi_elem_exec) {
>> -DRM_ERROR("Unsupported MIPI element, skipping sequence
>> execution\n");
>> +u8 operation_byte = *data++;
>> +if (operation_byte >= ARRAY_SIZE(exec_elem) ||
>> +!exec_elem[operation_byte]) {
>> +DRM_ERROR("Unsupported MIPI operation byte %u\n",
>
> Maybe DRM_ERROR in the previous patch too? Just for ocd consistency.
>
> Reviewed-by: Daniel Vetter
Pushed up to and including this patch. I took the liberty of pushing
as-is, leaving the nitpicks for follow-up. Thanks for the review.
BR,
Jani.
>
>> + operation_byte);
>> return;
>> }
>> -
>> -/* goto element payload */
>> -data++;
>> +mipi_elem_exec = exec_elem[operation_byte];
>>
>> /* execute the element specific rotines */
>> data = mipi_elem_exec(intel_dsi, data);
>> --
>> 2.1.4
>>
>> ___
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 14/15] drm/i915/dsi: skip unknown elements for sequence block v3+
On Tue, 05 Jan 2016, Daniel Vetter wrote:
> On Mon, Dec 21, 2015 at 03:11:05PM +0200, Jani Nikula wrote:
>> The sequence block has sizes of elements after the operation byte since
>> sequence block v3. Use it to skip elements we don't support yet.
>>
>> Signed-off-by: Jani Nikula
>> ---
>> drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 43
>> +-
>> 1 file changed, 24 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
>> b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
>> index eabfd9eb9cc0..1f9c80d21904 100644
>> --- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
>> +++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
>> @@ -335,31 +335,36 @@ static void generic_exec_sequence(struct intel_dsi
>> *intel_dsi, const u8 *data)
>> if (dev_priv->vbt.dsi.seq_version >= 3)
>> data += 4;
>>
>> -/* parse each byte till we reach end of sequence byte - 0x00 */
>> while (1) {
>> u8 operation_byte = *data++;
>> -if (operation_byte >= ARRAY_SIZE(exec_elem) ||
>> -!exec_elem[operation_byte]) {
>> +u8 operation_size = 0;
>> +
>> +if (operation_byte == MIPI_SEQ_ELEM_END)
>> +break;
>> +
>> +if (operation_byte < ARRAY_SIZE(exec_elem) &&
>> +exec_elem[operation_byte])
>
> && exec_elem[operation_byte] is redundant since you assing it to NULL
> anyway in the else clause. While I bikeshed: Might look prettier with ?:
Silly me.
I might prefer an if-else if the ?: doesn't fit on one line.
>
>
>> +mipi_elem_exec = exec_elem[operation_byte];
>> +else
>> +mipi_elem_exec = NULL;
>> +
>> +/* Size of Operation. */
>> +if (dev_priv->vbt.dsi.seq_version >= 3)
>> +operation_size = *data++;
>> +
>> +if (mipi_elem_exec) {
>> +data = mipi_elem_exec(intel_dsi, data);
>> +} else if (operation_size) {
>> +/* We have size, skip. */
>> +DRM_DEBUG_KMS("Unsupported MIPI operation byte %u\n",
>> + operation_byte);
>
> DRM_ERROR, like in the other cases we fail to parse the sequence fully?
I suppose we could add that in intel_bios.c to do it a limited number of
times at driver load, but here it would keep spewing out the errors
every modeset and possibly more. This is not necessarily a showstopper
with sequence v3, as we have the size to move on to the next one.
BR,
Jani.
>
> Either way on both bikesheds: Reviewed-by: Daniel Vetter
>
>
>
>> +data += operation_size;
>> +} else {
>> +/* No size, can't skip without parsing. */
>> DRM_ERROR("Unsupported MIPI operation byte %u\n",
>>operation_byte);
>> return;
>> }
>> -mipi_elem_exec = exec_elem[operation_byte];
>> -
>> -/* Skip Size of Operation. */
>> -if (dev_priv->vbt.dsi.seq_version >= 3)
>> -data++;
>> -
>> -/* execute the element specific rotines */
>> -data = mipi_elem_exec(intel_dsi, data);
>> -
>> -/*
>> - * After processing the element, data should point to
>> - * next element or end of sequence
>> - * check if have we reached end of sequence
>> - */
>> -if (*data == 0x00)
>> -break;
>> }
>> }
>>
>> --
>> 2.1.4
>>
>> ___
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 1/3] drm/i915: Enable lockless lookup of request tracking via RCU
On Wed, Dec 23, 2015 at 01:35:54PM +, Chris Wilson wrote:
> If we enable RCU for the requests (providing a grace period where we can
> inspect a "dead" request before it is freed), we can allow callers to
> carefully perform lockless lookup of an active request.
>
> However, by enabling deferred freeing of requests, we can potentially
> hog a lot of memory when dealing with tens of thousands of requests per
> second - with a quick insertion of the a synchronize_rcu() inside our
> shrinker callback, that issue disappears.
>
> Signed-off-by: Chris Wilson
> ---
> drivers/gpu/drm/i915/i915_gem.c | 3 ++-
> drivers/gpu/drm/i915/i915_gem_request.c | 2 +-
> drivers/gpu/drm/i915/i915_gem_request.h | 24 +++-
> drivers/gpu/drm/i915/i915_gem_shrinker.c | 1 +
> 4 files changed, 27 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index c169574758d5..696ada3891ed 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4222,7 +4222,8 @@ i915_gem_load(struct drm_device *dev)
> dev_priv->requests =
> kmem_cache_create("i915_gem_request",
> sizeof(struct drm_i915_gem_request), 0,
> - SLAB_HWCACHE_ALIGN,
> + SLAB_HWCACHE_ALIGN |
> + SLAB_DESTROY_BY_RCU,
> NULL);
>
> INIT_LIST_HEAD(&dev_priv->context_list);
[snip i915 private changes, leave just slab/shrinker changes]
> diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c
> b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> index c561ed2b8287..03a8bbb3e31e 100644
> --- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
> +++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> @@ -142,6 +142,7 @@ i915_gem_shrink(struct drm_i915_private *dev_priv,
> }
>
> i915_gem_retire_requests(dev_priv->dev);
> + synchronize_rcu(); /* expedite the grace period to free the requests */
Shouldn't the slab subsystem do this for us if we request it delays the
actual kfree? Seems like a core bug to me ... Adding more folks.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 1/3] drm/i915: Enable lockless lookup of request tracking via RCU
On Tue, Jan 05, 2016 at 03:59:51PM +0100, Daniel Vetter wrote:
> On Wed, Dec 23, 2015 at 01:35:54PM +, Chris Wilson wrote:
> > If we enable RCU for the requests (providing a grace period where we can
> > inspect a "dead" request before it is freed), we can allow callers to
> > carefully perform lockless lookup of an active request.
> >
> > However, by enabling deferred freeing of requests, we can potentially
> > hog a lot of memory when dealing with tens of thousands of requests per
> > second - with a quick insertion of the a synchronize_rcu() inside our
> > shrinker callback, that issue disappears.
> >
> > Signed-off-by: Chris Wilson
> > ---
> > drivers/gpu/drm/i915/i915_gem.c | 3 ++-
> > drivers/gpu/drm/i915/i915_gem_request.c | 2 +-
> > drivers/gpu/drm/i915/i915_gem_request.h | 24 +++-
> > drivers/gpu/drm/i915/i915_gem_shrinker.c | 1 +
> > 4 files changed, 27 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c
> > b/drivers/gpu/drm/i915/i915_gem.c
> > index c169574758d5..696ada3891ed 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -4222,7 +4222,8 @@ i915_gem_load(struct drm_device *dev)
> > dev_priv->requests =
> > kmem_cache_create("i915_gem_request",
> > sizeof(struct drm_i915_gem_request), 0,
> > - SLAB_HWCACHE_ALIGN,
> > + SLAB_HWCACHE_ALIGN |
> > + SLAB_DESTROY_BY_RCU,
> > NULL);
> >
> > INIT_LIST_HEAD(&dev_priv->context_list);
>
> [snip i915 private changes, leave just slab/shrinker changes]
>
> > diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c
> > b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> > index c561ed2b8287..03a8bbb3e31e 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> > @@ -142,6 +142,7 @@ i915_gem_shrink(struct drm_i915_private *dev_priv,
> > }
> >
> > i915_gem_retire_requests(dev_priv->dev);
> > + synchronize_rcu(); /* expedite the grace period to free the requests */
>
> Shouldn't the slab subsystem do this for us if we request it delays the
> actual kfree? Seems like a core bug to me ... Adding more folks.
note that sync_rcu() can take a terribly long time.. but yes, I seem to
remember Paul talking about adding this to reclaim paths for just this
reason. Not sure that ever happened thouhg.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2] drm/i915/bios: add support for MIPI sequence block v3
The changes since the sequence block v2 are:
* The whole MIPI bios data block has a separate 32-bit size field since
v3, stored after the version. This facilitates big sequences.
* The size of the panel specific sequence blocks has grown to 32
bits. This facilitates big sequences.
* The elements within sequences now have an 8-bit size field following
the operation byte. This facilitates skipping unknown new operation
bytes, i.e. forward compatibility.
v2 (of the patch): use DRM_ERROR for unknown operation byte
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/intel_bios.c | 84 ++
drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 9
2 files changed, 84 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_bios.c
b/drivers/gpu/drm/i915/intel_bios.c
index 69040a73c09a..6c09b5643942 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -754,21 +754,30 @@ parse_mipi_config(struct drm_i915_private *dev_priv,
/* Find the sequence block and size for the given panel. */
static const u8 *
find_panel_sequence_block(const struct bdb_mipi_sequence *sequence,
- u16 panel_id, u16 *seq_size)
+ u16 panel_id, u32 *seq_size)
{
u32 total = get_blocksize(sequence);
const u8 *data = &sequence->data[0];
u8 current_id;
- u16 current_size;
+ u32 current_size;
int index = 0;
int i;
+ /* skip new block size */
+ if (sequence->version >= 3)
+ data += 4;
+
for (i = 0; i < MAX_MIPI_CONFIGURATIONS && index + 3 < total; i++) {
current_id = *(data + index);
index++;
- current_size = *((const u16 *)(data + index));
- index += 2;
+ if (sequence->version >= 3) {
+ current_size = *((const u32 *)(data + index));
+ index += 4;
+ } else {
+ current_size = *((const u16 *)(data + index));
+ index += 2;
+ }
if (index + current_size > total) {
DRM_ERROR("Invalid sequence block\n");
@@ -826,13 +835,66 @@ static int goto_next_sequence(const u8 *data, int index,
int total)
return 0;
}
+static int goto_next_sequence_v3(const u8 *data, int index, int total)
+{
+ int seq_end;
+ u16 len;
+
+ /*
+* Could skip sequence based on Size of Sequence alone, but also do some
+* checking on the structure.
+*/
+ seq_end = index + *((const u32 *)(data + 1));
+ if (seq_end > total) {
+ DRM_ERROR("Invalid sequence size\n");
+ return 0;
+ }
+
+ /* Skip Sequence Byte and Size of Sequence. */
+ for (index = index + 5; index < total; index += len) {
+ u8 operation_byte = *(data + index);
+ index++;
+
+ if (operation_byte == MIPI_SEQ_ELEM_END) {
+ if (index != seq_end) {
+ DRM_ERROR("Invalid element structure\n");
+ return 0;
+ }
+ return index;
+ }
+
+ len = *(data + index);
+ index++;
+
+ /*
+* FIXME: Would be nice to check elements like for v1/v2 in
+* goto_next_sequence() above.
+*/
+ switch (operation_byte) {
+ case MIPI_SEQ_ELEM_SEND_PKT:
+ case MIPI_SEQ_ELEM_DELAY:
+ case MIPI_SEQ_ELEM_GPIO:
+ case MIPI_SEQ_ELEM_I2C:
+ case MIPI_SEQ_ELEM_SPI:
+ case MIPI_SEQ_ELEM_PMIC:
+ break;
+ default:
+ DRM_ERROR("Unknown operation byte %u\n",
+ operation_byte);
+ break;
+ }
+ }
+
+ return 0;
+}
+
static void
parse_mipi_sequence(struct drm_i915_private *dev_priv,
const struct bdb_header *bdb)
{
const struct bdb_mipi_sequence *sequence;
const u8 *seq_data;
- u16 seq_size;
+ u32 seq_size;
u8 *data;
int index = 0;
@@ -847,12 +909,13 @@ parse_mipi_sequence(struct drm_i915_private *dev_priv,
}
/* Fail gracefully for forward incompatible sequence block. */
- if (sequence->version >= 3) {
- DRM_ERROR("Unable to parse MIPI Sequence Block v3+\n");
+ if (sequence->version >= 4) {
+ DRM_ERROR("Unable to parse MIPI Sequence Block v%u\n",
+ sequence->version);
return;
}
- DRM_DEBUG_DRIVER("Found MIPI sequence block\n");
+ DRM_DEBUG_DRIVER("Found MIPI sequence block v%u\n", sequence->version);
seq_data = find_panel_s
[Intel-gfx] [PATCH v2] drm/i915/dsi: skip unknown elements for sequence block v3+
The sequence block has sizes of elements after the operation byte since
sequence block v3. Use it to skip elements we don't support yet.
v2: remove redundant exec_elem[operation_byte] check (Daniel)
Reviewed-by: Daniel Vetter
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 42 --
1 file changed, 23 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
index 7f67749eb0ef..4a6f9a593ea2 100644
--- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
+++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
@@ -335,31 +335,35 @@ static void generic_exec_sequence(struct intel_dsi
*intel_dsi, const u8 *data)
if (dev_priv->vbt.dsi.seq_version >= 3)
data += 4;
- /* parse each byte till we reach end of sequence byte - 0x00 */
while (1) {
u8 operation_byte = *data++;
- if (operation_byte >= ARRAY_SIZE(exec_elem) ||
- !exec_elem[operation_byte]) {
+ u8 operation_size = 0;
+
+ if (operation_byte == MIPI_SEQ_ELEM_END)
+ break;
+
+ if (operation_byte < ARRAY_SIZE(exec_elem))
+ mipi_elem_exec = exec_elem[operation_byte];
+ else
+ mipi_elem_exec = NULL;
+
+ /* Size of Operation. */
+ if (dev_priv->vbt.dsi.seq_version >= 3)
+ operation_size = *data++;
+
+ if (mipi_elem_exec) {
+ data = mipi_elem_exec(intel_dsi, data);
+ } else if (operation_size) {
+ /* We have size, skip. */
+ DRM_DEBUG_KMS("Unsupported MIPI operation byte %u\n",
+ operation_byte);
+ data += operation_size;
+ } else {
+ /* No size, can't skip without parsing. */
DRM_ERROR("Unsupported MIPI operation byte %u\n",
operation_byte);
return;
}
- mipi_elem_exec = exec_elem[operation_byte];
-
- /* Skip Size of Operation. */
- if (dev_priv->vbt.dsi.seq_version >= 3)
- data++;
-
- /* execute the element specific rotines */
- data = mipi_elem_exec(intel_dsi, data);
-
- /*
-* After processing the element, data should point to
-* next element or end of sequence
-* check if have we reached end of sequence
-*/
- if (*data == 0x00)
- break;
}
}
--
2.1.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 1/3] drm/i915: Enable lockless lookup of request tracking via RCU
On Tue, Jan 05, 2016 at 04:02:13PM +0100, Peter Zijlstra wrote: > > Shouldn't the slab subsystem do this for us if we request it delays the > > actual kfree? Seems like a core bug to me ... Adding more folks. > > note that sync_rcu() can take a terribly long time.. but yes, I seem to > remember Paul talking about adding this to reclaim paths for just this > reason. Not sure that ever happened thouhg. Also, you might be wanting rcu_barrier() instead, that not only waits for a GP to complete, but also for all pending callbacks to be processed. Without the latter there might still not be anything to free after it. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] igt_core: Fix logging to display extended line
On Wed, Dec 23, 2015 at 03:34:48PM +, Derek Morton wrote: > line[strlen(line)] will always evaluate to NULL so line_continuation > was always true. That prevented the program name, pid and log level > ever being printed. > Changed to [strlen(line) - 1] so the last character before the null > terminator is compared with '\n' to determine line_continuation. > > Signed-off-by: Derek Morton Ooops. Applied, thanks for the patch. -Daniel > --- > lib/igt_core.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/lib/igt_core.c b/lib/igt_core.c > index 84cf8d2..221ed7e 100644 > --- a/lib/igt_core.c > +++ b/lib/igt_core.c > @@ -1748,7 +1748,7 @@ void igt_vlog(const char *domain, enum igt_log_level > level, const char *format, > goto out; > } > > - line_continuation = line[strlen(line)] != '\n'; > + line_continuation = line[strlen(line) - 1] != '\n'; > > /* append log buffer */ > _igt_log_buffer_append(formatted_line); > -- > 1.9.1 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/dsi: add debug printing of the new sequence block names
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
index 70fc11e0869d..a5817bef2d71 100644
--- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
+++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
@@ -307,6 +307,12 @@ static const char * const seq_name[] = {
[MIPI_SEQ_DISPLAY_ON] = "MIPI_SEQ_DISPLAY_ON",
[MIPI_SEQ_DISPLAY_OFF] = "MIPI_SEQ_DISPLAY_OFF",
[MIPI_SEQ_DEASSERT_RESET] = "MIPI_SEQ_DEASSERT_RESET",
+ [MIPI_SEQ_BACKLIGHT_ON] = "MIPI_SEQ_BACKLIGHT_ON",
+ [MIPI_SEQ_BACKLIGHT_OFF] = "MIPI_SEQ_BACKLIGHT_OFF",
+ [MIPI_SEQ_TEAR_ON] = "MIPI_SEQ_TEAR_ON",
+ [MIPI_SEQ_TEAR_OFF] = "MIPI_SEQ_TEAR_OFF",
+ [MIPI_SEQ_POWER_ON] = "MIPI_SEQ_POWER_ON",
+ [MIPI_SEQ_POWER_OFF] = "MIPI_SEQ_POWER_OFF",
};
static const char *sequence_name(enum mipi_seq seq_id)
--
2.1.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/docs: more leftovers from the big vtable documentation pile
On Mon, Jan 04, 2016 at 07:49:41AM +0100, Daniel Vetter wrote:
> On Mon, Dec 28, 2015 at 11:22:52AM +0100, Thierry Reding wrote:
> > On Wed, Dec 16, 2015 at 06:18:25PM +0100, Daniel Vetter wrote:
> > > Another pile of vfuncs from the old gpu.tmpl xml documentation that
> > > I've forgotten to delete. I spotted a few more things to
> > > clarify/extend in the new kerneldoc while going through this once
> > > more.
> > >
> > > Cc: Laurent Pinchart
> > > Cc: Thierry Reding
> > > Signed-off-by: Daniel Vetter
> > > ---
> > > Documentation/DocBook/gpu.tmpl | 188
> > > ---
> > > include/drm/drm_modeset_helper_vtables.h | 28 -
> > > 2 files changed, 25 insertions(+), 191 deletions(-)
> > >
> > > diff --git a/Documentation/DocBook/gpu.tmpl
> > > b/Documentation/DocBook/gpu.tmpl
> > > index a3764291c826..c0fa21c797fe 100644
> > > --- a/Documentation/DocBook/gpu.tmpl
> > > +++ b/Documentation/DocBook/gpu.tmpl
> > > @@ -1579,194 +1579,6 @@ void intel_crt_init(struct drm_device *dev)
> > >entities.
> > >
> > >
> > > - Legacy CRTC Helper Operations
> > > -
> > > -
> > > - bool (*mode_fixup)(struct drm_crtc *crtc,
> > > - const struct drm_display_mode *mode,
> > > - struct drm_display_mode
> > > *adjusted_mode);
> > > -
> > > -Let CRTCs adjust the requested mode or reject it completely.
> > > This
> > > -operation returns true if the mode is accepted (possibly
> > > after being
> > > -adjusted) or false if it is rejected.
> > > -
> > > -
> > > -The mode_fixup operation should
> > > reject the
> > > -mode if it can't reasonably use it. The definition of
> > > "reasonable"
> > > -is currently fuzzy in this context. One possible behaviour
> > > would be
> > > -to set the adjusted mode to the panel timings when a
> > > fixed-mode
> > > -panel is used with hardware capable of scaling. Another
> > > behaviour
> > > -would be to accept any input mode and adjust it to the
> > > closest mode
> > > -supported by the hardware (FIXME: This needs to be
> > > clarified).
> > > -
> > > -
> >
> > I just noticed that this deviates somewhat from what is now in the new
> > documentation in include/drm/drm_modeset_helper_vtables.h. The new
> > documentation suggests that ->mode_fixup() should not modify
> > adjusted_mode but instead reject the mode if the conversion from mode to
> > adjusted_mode can't be supported. The new definition sounds much saner
> > to me, because if we allowed the CRTC's ->mode_fixup() to modify the
> > adjusted_mode, we'd need to make sure that the encoder (and bridge)
> > still accept it. And we'd need to potentially reiterate until everybody
> > agrees.
>
> Yeah, I simply addressed the FIXME while typing the new docs and made this
> a strong recommendation (that's why I picked "should" instead of "must").
> There's some drivers (at least i915's TV-out) where the input mode is
> massively mangled, but no one will ever fix this.
Okay, fine with me.
> > > diff --git a/include/drm/drm_modeset_helper_vtables.h
> > > b/include/drm/drm_modeset_helper_vtables.h
> > > index 29e0dc50031d..b995d5ec50a0 100644
> > > --- a/include/drm/drm_modeset_helper_vtables.h
> > > +++ b/include/drm/drm_modeset_helper_vtables.h
> > > @@ -131,6 +131,12 @@ struct drm_crtc_helper_funcs {
> > >* Atomic drivers which need to inspect and adjust more state should
> > >* instead use the @atomic_check callback.
> > >*
> > > + * Also beware that the core nor helpers filters mode before passing the
> >
> > "... neither the core nor the helpers filter modes before passing them ..."?
> >
> > > + * to the driver. More specifically modes rejected by the ->mode_valid
> > > + * callback from #drm_connector_helper_funcs can still be requested by
> > > + * userspace and therefore also need to be rejected in either this hook,
> > > + * or the one in #drm_encoder_helper_funcs.
> >
> > Hmm... that's odd. Why would one want to allow a mode that the connector
> > can't support? Also looking at drm_helper_probe_single_connector_modes()
> > this isn't quite true. That helper is used by all connectors except
> > vmwgfx. It also calls drm_mode_prune_invalid(), which will remove all
> > modes from the connector's mode list that don't have status == MODE_OK.
> > As far as I can see after they've been removed from the connector's mode
> > list they will no longer be exposed to userspace.
>
> Maybe I need to hammer this in more, but it's a common misconception that
> userspace can only ask for modes in the connector list. Generally that's
> what's happening, but there's not restrictions on userspace to ask for a
> mode which is _not_ in the connector's mode list.
>
> If you don't believe that, look at xrandr --addmode and try
Re: [Intel-gfx] [RFC] drm/i915/bxt: Add pipe_src size property
On Tue, Jan 05, 2016 at 03:50:38PM +0100, Daniel Vetter wrote: > On Mon, Jan 04, 2016 at 07:06:15PM +0200, Ville Syrjälä wrote: > > On Mon, Jan 04, 2016 at 11:16:39AM +0100, Maarten Lankhorst wrote: > > > Hey, > > > > > > Op 23-12-15 om 12:05 schreef Nabendu Maiti: > > > > This patch is adding pipesource size as property as intel property.User > > > > application is allowed to change the pipe source size in runtime on > > > > BXT/SKL. > > > > Added the property as inteli crtc property. > > > > > > > > Comments and suggestions are requested for whether we can keep the > > > > property as intel crtc property or move to drm level. > > > > > > > This property would get lost on a modeset. But why do you need a pipe_src > > > property? > > > > It's needed if we want to use the panel fitter with non-eDP/LVDS/DSI > > displays. > > > > Last time this came up I decided that we want to expose this via a new > > "fixed_mode" property. Ie. userspace can choose the actual display > > timings by setting the "fixed_mode" property with a specific mode, and > > then the normal mode property will basically just control just the pipe > > src size just like it already does for eDP/LVDS/DSI where we already have > > the fixed_mode internally (just not exposed to usersapce). If the > > fixed_mode is not specified, things will work just as they do right > > now. Obviously for eDP/LVDS/DSI we will reject any request to > > change/unset the fixed mode. > > > > The benefit of that approach is that things will work exactly the same > > way as before unless the user explicitly sets the fixed_mode, and once > > it's set thigns will work exactly the same way as they have worked so > > far for eDP/LVDS/DSI. Also it keeps the policy of choosing the fixed > > mode strictly is userspace for external displays. > > > > And as a bonus we will also expose the eDP/LVDS/DSI fixed_mode to > > userspace giving userspace more information on what the actual panel > > timings are. Currently userspace has to more or less guess that one > > of the modes the connector claims to support has the actual panel > > timings. > > > > So far I've not heard any opposing opinions to this plan. > > Hm yeah, pipe_src would run into all kinds of fun in conjunction with the > existing fixed mode stuff we have. Just exposing the fixed as a prop might > be simpler. But then we need to figure out what to do wrt the clock in the > mode passed in through userspace - currently the fixed mode always wins, > but for manual DRRS it would be nice if userspace could control it, and > doesn't have to know that there's a fixed_mode. We could have the user mode vrefresh indicate the desired refresh rate of the fixed mode as well. In fact I've been wanting to add a check to make sure the user mode refresh rate isn't too far off from the fixed/downlclock mode vrefresh, but didn't get around to it yet. > > So semantically I think only exposing the pipe src to expose panel fitters > would be cleaner. Then we'd need to deal with some internal troubles to > make sure we combine everything correctly, but that shouldn't be too hard > really. I think it's quite a bit more work since we have to redo all the fb size checks and whatnot to use the property when specified. I once outlined a more detailed plan for his approach too (too lazy to dig up the mail), but I think the fixed mode prop is a simpler approach and won't actually require much in the way of userspace changes either. It'll be enough to set the property once and then even the legacy modeset ioctls will work exactly like they do now for eDP/LVDS/DSI. -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/docs: more leftovers from the big vtable documentation pile
Another pile of vfuncs from the old gpu.tmpl xml documentation that I've forgotten to delete. I spotted a few more things to clarify/extend in the new kerneldoc while going through this once more. v2: Spelling fixes (Thierry). v3: More spelling fixes and use Thierry's proposal to clarify why drivers need to validate modes both in ->mode_fixup and ->mode_valid. Cc: Laurent Pinchart Cc: Thierry Reding Acked-by: Thierry Reding Signed-off-by: Daniel Vetter --- Documentation/DocBook/gpu.tmpl | 188 --- include/drm/drm_modeset_helper_vtables.h | 44 +++- 2 files changed, 41 insertions(+), 191 deletions(-) diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl index 225a246c5f53..faa5e0d4208d 100644 --- a/Documentation/DocBook/gpu.tmpl +++ b/Documentation/DocBook/gpu.tmpl @@ -1579,194 +1579,6 @@ void intel_crt_init(struct drm_device *dev) entities. - Legacy CRTC Helper Operations - - - bool (*mode_fixup)(struct drm_crtc *crtc, - const struct drm_display_mode *mode, - struct drm_display_mode *adjusted_mode); - -Let CRTCs adjust the requested mode or reject it completely. This -operation returns true if the mode is accepted (possibly after being -adjusted) or false if it is rejected. - - -The mode_fixup operation should reject the -mode if it can't reasonably use it. The definition of "reasonable" -is currently fuzzy in this context. One possible behaviour would be -to set the adjusted mode to the panel timings when a fixed-mode -panel is used with hardware capable of scaling. Another behaviour -would be to accept any input mode and adjust it to the closest mode -supported by the hardware (FIXME: This needs to be clarified). - - - - int (*mode_set_base)(struct drm_crtc *crtc, int x, int y, - struct drm_framebuffer *old_fb) - -Move the CRTC on the current frame buffer (stored in -crtc->fb) to position (x,y). Any of the frame -buffer, x position or y position may have been modified. - - -This helper operation is optional. If not provided, the -drm_crtc_helper_set_config function will fall -back to the mode_set helper operation. - - -FIXME: Why are x and y passed as arguments, as they can be accessed -through crtc->x and -crtc->y? - - - - void (*prepare)(struct drm_crtc *crtc); - -Prepare the CRTC for mode setting. This operation is called after -validating the requested mode. Drivers use it to perform -device-specific operations required before setting the new mode. - - - - int (*mode_set)(struct drm_crtc *crtc, struct drm_display_mode *mode, -struct drm_display_mode *adjusted_mode, int x, int y, -struct drm_framebuffer *old_fb); - -Set a new mode, position and frame buffer. Depending on the device -requirements, the mode can be stored internally by the driver and -applied in the commit operation, or -programmed to the hardware immediately. - - -The mode_set operation returns 0 on success - or a negative error code if an error occurs. - - - - void (*commit)(struct drm_crtc *crtc); - -Commit a mode. This operation is called after setting the new mode. -Upon return the device must use the new mode and be fully -operational. - - - - - - Encoder Helper Operations - - - bool (*mode_fixup)(struct drm_encoder *encoder, - const struct drm_display_mode *mode, - struct drm_display_mode *adjusted_mode); - -Let encoders adjust the requested mode or reject it completely. This -operation returns true if the mode is accepted (possibly after being -adjusted) or false if it is rejected. See the -mode_fixup CRTC helper -operation for an explanation of the allowed adjustments. - - - - void (*prepare)(struct drm_encoder *encoder); - -Prepare the encoder for mode setting. This operation is called after -validating the requested mode. Drivers use it to perform -device-specific operations required before setting the new mode. - - - - void (*mode_set)(struct drm_encoder *encoder, - struct drm_d
Re: [Intel-gfx] [PATCH] drm/docs: more leftovers from the big vtable documentation pile
On Tue, Jan 05, 2016 at 04:22:15PM +0100, Daniel Vetter wrote: > Another pile of vfuncs from the old gpu.tmpl xml documentation that > I've forgotten to delete. I spotted a few more things to > clarify/extend in the new kerneldoc while going through this once > more. > > v2: Spelling fixes (Thierry). > > v3: More spelling fixes and use Thierry's proposal to clarify why > drivers need to validate modes both in ->mode_fixup and ->mode_valid. > > Cc: Laurent Pinchart > Cc: Thierry Reding > Acked-by: Thierry Reding > Signed-off-by: Daniel Vetter ... and pulled into drm-misc. -Daniel > --- > Documentation/DocBook/gpu.tmpl | 188 > --- > include/drm/drm_modeset_helper_vtables.h | 44 +++- > 2 files changed, 41 insertions(+), 191 deletions(-) > > diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl > index 225a246c5f53..faa5e0d4208d 100644 > --- a/Documentation/DocBook/gpu.tmpl > +++ b/Documentation/DocBook/gpu.tmpl > @@ -1579,194 +1579,6 @@ void intel_crt_init(struct drm_device *dev) >entities. > > > - Legacy CRTC Helper Operations > - > - > - bool (*mode_fixup)(struct drm_crtc *crtc, > - const struct drm_display_mode *mode, > - struct drm_display_mode *adjusted_mode); > - > -Let CRTCs adjust the requested mode or reject it completely. This > -operation returns true if the mode is accepted (possibly after > being > -adjusted) or false if it is rejected. > - > - > -The mode_fixup operation should reject > the > -mode if it can't reasonably use it. The definition of > "reasonable" > -is currently fuzzy in this context. One possible behaviour would > be > -to set the adjusted mode to the panel timings when a fixed-mode > -panel is used with hardware capable of scaling. Another behaviour > -would be to accept any input mode and adjust it to the closest > mode > -supported by the hardware (FIXME: This needs to be clarified). > - > - > - > - int (*mode_set_base)(struct drm_crtc *crtc, int x, int y, > - struct drm_framebuffer *old_fb) > - > -Move the CRTC on the current frame buffer (stored in > -crtc->fb) to position (x,y). Any of the > frame > -buffer, x position or y position may have been modified. > - > - > -This helper operation is optional. If not provided, the > -drm_crtc_helper_set_config function will > fall > -back to the mode_set helper operation. > - > - > -FIXME: Why are x and y passed as arguments, as they can be > accessed > -through crtc->x and > -crtc->y? > - > - > - > - void (*prepare)(struct drm_crtc *crtc); > - > -Prepare the CRTC for mode setting. This operation is called after > -validating the requested mode. Drivers use it to perform > -device-specific operations required before setting the new mode. > - > - > - > - int (*mode_set)(struct drm_crtc *crtc, struct > drm_display_mode *mode, > -struct drm_display_mode *adjusted_mode, int x, int y, > -struct drm_framebuffer *old_fb); > - > -Set a new mode, position and frame buffer. Depending on the > device > -requirements, the mode can be stored internally by the driver and > -applied in the commit operation, or > -programmed to the hardware immediately. > - > - > -The mode_set operation returns 0 on > success > - or a negative error code if an error occurs. > - > - > - > - void (*commit)(struct drm_crtc *crtc); > - > -Commit a mode. This operation is called after setting the new > mode. > -Upon return the device must use the new mode and be fully > -operational. > - > - > - > - > - > - Encoder Helper Operations > - > - > - bool (*mode_fixup)(struct drm_encoder *encoder, > - const struct drm_display_mode *mode, > - struct drm_display_mode *adjusted_mode); > - > -Let encoders adjust the requested mode or reject it completely. > This > -operation returns true if the mode is accepted (possibly after > being > -adjusted) or false if it is rejected. See the > -mode_fixup CRTC helper > -operation for an explanation of the allowed adjustments. > - > - > - > - void (*prepare)(struct drm_encode
[Intel-gfx] [PATCH] drm/i915: Update Skylake DDI translation table for HDMI.
When debuging an intermittent corrupted screen I suspected on DDI
translation table and checked we are out of date with the spec.
I'm not sure this will fix my bug yet, but it is always good to follow
the spec.
Signed-off-by: Rodrigo Vivi
---
drivers/gpu/drm/i915/intel_ddi.c | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index e6408e5..c7e3114 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -225,27 +225,27 @@ static const struct ddi_buf_trans
skl_ddi_translations_hdmi[] = {
{ 0x7011, 0x0088, 0x0 },
{ 0x0018, 0x00A1, 0x0 },
{ 0x0018, 0x0098, 0x0 },
- { 0x4013, 0x0088, 0x0 },
- { 0x6012, 0x0087, 0x0 },
+ { 0x4013, 0x0088, 0x1 },
+ { 0x80006012, 0x00CD, 0x0 },
{ 0x0018, 0x00DF, 0x0 },
- { 0x3015, 0x0087, 0x0 },/* Default */
- { 0x3015, 0x00C7, 0x0 },
- { 0x0018, 0x00C7, 0x0 },
+ { 0x80003015, 0x00CD, 0x1 },/* Default */
+ { 0x80003015, 0x00C0, 0x1 },
+ { 0x8018, 0x00C0, 0x1 },
};
/* Skylake Y */
static const struct ddi_buf_trans skl_y_ddi_translations_hdmi[] = {
{ 0x0018, 0x00A1, 0x0 },
{ 0x5012, 0x00DF, 0x0 },
- { 0x7011, 0x0084, 0x0 },
+ { 0x80007011, 0x00CB, 0x3 },
{ 0x0018, 0x00A4, 0x0 },
{ 0x0018, 0x009D, 0x0 },
{ 0x4013, 0x0080, 0x0 },
- { 0x6013, 0x00C7, 0x0 },
+ { 0x80006013, 0x00C0, 0x3 },
{ 0x0018, 0x008A, 0x0 },
- { 0x3015, 0x00C7, 0x0 },/* Default */
- { 0x80003015, 0x00C7, 0x7 },/* Uses I_boost level 0x7 */
- { 0x0018, 0x00C7, 0x0 },
+ { 0x80003015, 0x00C0, 0x3 },/* Default */
+ { 0x80003015, 0x00C0, 0x3 },
+ { 0x8018, 0x00C0, 0x3 },
};
struct bxt_ddi_buf_trans {
--
2.4.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Update Skylake DDI translation table for DP.
When reviewing DDI translation table I noticed few changes we
haven't incorporated yet and it is always good to follow latest
spec.
Signed-off-by: Rodrigo Vivi
---
drivers/gpu/drm/i915/intel_ddi.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index c7e3114..8f16b13 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -145,7 +145,7 @@ static const struct ddi_buf_trans skl_ddi_translations_dp[]
= {
static const struct ddi_buf_trans skl_u_ddi_translations_dp[] = {
{ 0x201B, 0x00A2, 0x0 },
{ 0x5012, 0x0088, 0x0 },
- { 0x7011, 0x0087, 0x0 },
+ { 0x80007011, 0x00CD, 0x0 },
{ 0x80009010, 0x00C0, 0x1 },/* Uses I_boost level 0x1 */
{ 0x201B, 0x009D, 0x0 },
{ 0x80005012, 0x00C0, 0x1 },/* Uses I_boost level 0x1 */
@@ -158,7 +158,7 @@ static const struct ddi_buf_trans
skl_u_ddi_translations_dp[] = {
static const struct ddi_buf_trans skl_y_ddi_translations_dp[] = {
{ 0x0018, 0x00A2, 0x0 },
{ 0x5012, 0x0088, 0x0 },
- { 0x7011, 0x0087, 0x0 },
+ { 0x80007011, 0x00CD, 0x0 },
{ 0x80009010, 0x00C0, 0x3 },/* Uses I_boost level 0x3 */
{ 0x0018, 0x009D, 0x0 },
{ 0x80005012, 0x00C0, 0x3 },/* Uses I_boost level 0x3 */
--
2.4.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ failure: Fi.CI.BAT
== Summary == HEAD is now at 865e245 drm-intel-nightly: 2016y-01m-05d-15h-23m-53s UTC integration manifest Applying: drm/i915/bios: add proper documentation for the Video BIOS Table (VBT) Using index info to reconstruct a base tree... M Documentation/DocBook/gpu.tmpl M drivers/gpu/drm/i915/intel_bios.c M drivers/gpu/drm/i915/intel_bios.h Falling back to patching base and 3-way merge... Auto-merging drivers/gpu/drm/i915/intel_bios.h CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/intel_bios.h Auto-merging drivers/gpu/drm/i915/intel_bios.c CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/intel_bios.c Patch failed at 0001 drm/i915/bios: add proper documentation for the Video BIOS Table (VBT) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t] tests/gem_guc_loading: Adding simple GuC loading test
Test check GuC debugfs file for successful loading confirmation
Signed-off-by: Lukasz Fiedorowicz
---
tests/Makefile.sources | 1 +
tests/gem_guc_loading.c | 89 +
2 files changed, 90 insertions(+)
create mode 100644 tests/gem_guc_loading.c
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index d594038..331234f 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -36,6 +36,7 @@ TESTS_progs_M = \
gem_flink_basic \
gem_flink_race \
gem_linear_blits \
+ gem_guc_loading \
gem_madvise \
gem_mmap \
gem_mmap_gtt \
diff --git a/tests/gem_guc_loading.c b/tests/gem_guc_loading.c
new file mode 100644
index 000..fd53a46
--- /dev/null
+++ b/tests/gem_guc_loading.c
@@ -0,0 +1,89 @@
+/*
+ * Copyright © 2015 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ *Lukasz Fiedorowicz
+ *
+ */
+
+#include
+#include
+#include
+#include
+
+#include "igt.h"
+
+IGT_TEST_DESCRIPTION("GuC firmware loading test.");
+
+#define LOAD_STATUS_BUF_SIZE 96
+
+enum guc_status { GUC_ENABLED, GUC_DISABLED };
+
+int guc_status_fd;
+
+static void open_guc_status(void)
+{
+ guc_status_fd = igt_debugfs_open("i915_guc_load_status", O_RDONLY);
+ igt_assert_f(guc_status_fd >= 0, "Can't open i915_guc_load_status\n");
+}
+
+static enum guc_status get_guc_status(void)
+{
+ char buf[LOAD_STATUS_BUF_SIZE];
+
+ FILE *fp = fdopen(guc_status_fd, "r");
+ igt_assert_f(fp != NULL, "Can't open i915_guc_load_status file\n");
+
+ while (fgets(buf, LOAD_STATUS_BUF_SIZE, fp))
+ if ((strstr(buf, "\tload: SUCCESS\n")))
+ return GUC_ENABLED;
+
+ return GUC_DISABLED;
+}
+
+static void close_guc_status(void)
+{
+ close(guc_status_fd);
+}
+
+static void test_guc_loaded()
+{
+ igt_assert_f(get_guc_status() == GUC_ENABLED, "GuC is disabled\n");
+}
+
+igt_main
+{
+ int gfx_fd = 0;
+ int gen = 0;
+
+ igt_fixture
+ {
+ gfx_fd = drm_open_driver(DRIVER_INTEL);
+ gen = intel_gen(intel_get_drm_devid(gfx_fd));
+ igt_require(gen >= 9);
+ open_guc_status();
+ }
+
+ igt_subtest("guc_loaded") test_guc_loaded();
+
+ igt_fixture close_guc_status();
+}
--
2.4.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 01/10] drm/i915: Allow use of get_dma_address for stolen backed objects
On Tue, Dec 22, 2015 at 10:39:38AM +, Chris Wilson wrote: > On Tue, Dec 22, 2015 at 10:23:11AM +, Tvrtko Ursulin wrote: > > > > > > On 22/12/15 06:20, ankitprasad.r.sha...@intel.com wrote: > > >From: Ankitprasad Sharma > > > > > >i915_gem_object_get_dma_address function is used to retrieve the dma > > >address > > >of a particular page so as to map it in a given GTT entry for CPU access. > > >This function would be used for stolen backed objects also for tasks like > > >pwrite, clearing of the pages etc. So the obj->get_page.sg needs to be > > >initialized for the stolen objects also. > > > > > >Signed-off-by: Ankitprasad Sharma > > >--- > > > drivers/gpu/drm/i915/i915_gem_stolen.c | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > >diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c > > >b/drivers/gpu/drm/i915/i915_gem_stolen.c > > >index 598ed2f..5384767 100644 > > >--- a/drivers/gpu/drm/i915/i915_gem_stolen.c > > >+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c > > >@@ -569,6 +569,9 @@ _i915_gem_object_create_stolen(struct drm_device *dev, > > > if (obj->pages == NULL) > > > goto cleanup; > > > > > >+ obj->get_page.sg = obj->pages->sgl; > > >+ obj->get_page.last = 0; > > >+ > > > i915_gem_object_pin_pages(obj); > > > obj->stolen = stolen; > > > > > > > > > > As the last time, > > > > Reviewed-by: Tvrtko Ursulin > > Please do pull in r-b on reposting patches, if you haven't changed the > patch significant (err on the side of caution, if you have made logic > changes either drop the r-b, or note that the r-b was for a previous > version (only if minor changes again)). Yup please do so, that avoids duplicated work by reworkers and maintainers. Queued for -next, thanks for the patch. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Why idle_freq is set to RPn and not RPe
Hello Chris, Happy New Year! Thanks for answers so far. I have some additional questions. You wrote that this change was made to take a defensive position to ensure minimum power consumption - did we do any power measurements to confirm the benefit? How does this change affect user experience and overall performance? Did we do any performance impact measurements? Is same change done in Windows implementation? Was this change in the algorithm discussed with HW team? Thanks! Radek > -Original Message- > From: Kamble, Sagar A > Sent: Thursday, December 31, 2015 6:38 PM > To: Chris Wilson; Szwichtenberg, Radoslaw; S, Deepak; Intel Graphics > Development; Goel, Akash > Subject: Re: Why idle_freq is set to RPn and not RPe > > > > On 12/30/2015 4:20 PM, Chris Wilson wrote: > > On Wed, Dec 30, 2015 at 04:09:46PM +0530, Kamble, Sagar A wrote: > >> Turbo frequency range is Rpe to Rp0 when GPU is active as, on > workload > >> submission frequency is taken to Rpe. > >> > >> Does the HW require us to drop to RPn before entering RC6? > >> If we can enter RC6 even with other frequencies I think we can keep > >> running at Rpe on Idle. > > Remember that we quite frequently prevent the hardware going into RC6, > I assume this is threshold times in TO/EI mode for which GT is idle but not > power gated. > > and that it has been known for the hardware to fail to enter RC6 > > itself (through driver error or whatnot). > And assume this is because of forcewake/rc6 setup errors in driver paths > which should not happen in best case :) Agree that running at Rpn makes > sense. > > Going to the extreme, why wouldn't > > you set Rp0 on idle, since that will give the best restart latency? > True. We can have different logic that starts from Rp0 and comes down if > perf is met. > > -Chris > > > Thanks for the inputs Chris. Intel Technology Poland sp. z o.o. ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN. Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek przegladanie lub rozpowszechnianie jest zabronione. This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by others is strictly prohibited. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Tune down rpm wakelock debug checks
They're causing massive amounts of dmesg noise and hence CI noise all
over the place. Enabling them for a bit was good enough to refresh our
task list of what's still needed to enable rpm by default.
To make sure we're not forgetting to make this noisy again add a FIXME
comment.
Fixes: da5827c36607 ("drm/i915: add assert_rpm_wakelock_held helper")
Cc: Imre Deak
Cc: drm-intel-fi...@lists.freedesktop.org
Cc: Chris Wilson
Acked-by: Chris Wilson
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/i915/intel_drv.h | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 187f632aa0ee..d6a8a4730b91 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1453,8 +1453,10 @@ static inline void
assert_rpm_wakelock_held(struct drm_i915_private *dev_priv)
{
assert_rpm_device_not_suspended(dev_priv);
- WARN_ONCE(!atomic_read(&dev_priv->pm.wakeref_count),
- "RPM wakelock ref not held during HW access");
+ /* FIXME: Needs to be converted back to WARN_ONCE, but currently causes
+* too much noise. */
+ if (!atomic_read(&dev_priv->pm.wakeref_count))
+ DRM_DEBUG_DRIVER("RPM wakelock ref not held during HW access");
}
static inline int
--
2.6.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ warning: Fi.CI.BAT
== Summary == Built on bc303261a81a96298b2f9e02734aeaa0a25421a6 drm-intel-nightly: 2016y-01m-05d-16h-47m-54s UTC integration manifest Test kms_flip: Subgroup basic-flip-vs-modeset: dmesg-warn -> PASS (bdw-nuci7) UNSTABLE Subgroup basic-flip-vs-wf_vblank: dmesg-warn -> PASS (snb-x220t) UNSTABLE Subgroup basic-plain-flip: pass -> DMESG-WARN (bdw-ultra) Test kms_pipe_crc_basic: Subgroup hang-read-crc-pipe-a: pass -> DMESG-WARN (snb-x220t) Subgroup read-crc-pipe-a: pass -> DMESG-WARN (snb-dellxps) Subgroup read-crc-pipe-b: dmesg-warn -> PASS (skl-i5k-2) UNSTABLE dmesg-warn -> PASS (snb-dellxps) UNSTABLE Test kms_psr_sink_crc: Subgroup psr_basic: dmesg-warn -> PASS (bdw-ultra) Test pm_rpm: Subgroup basic-rte: pass -> DMESG-WARN (byt-nuc) UNSTABLE bdw-nuci7total:132 pass:122 dwarn:1 dfail:0 fail:0 skip:9 bdw-ultratotal:132 pass:124 dwarn:2 dfail:0 fail:0 skip:6 bsw-nuc-2total:135 pass:114 dwarn:1 dfail:0 fail:0 skip:20 byt-nuc total:135 pass:120 dwarn:2 dfail:0 fail:0 skip:13 hsw-brixbox total:135 pass:126 dwarn:2 dfail:0 fail:0 skip:7 hsw-gt2 total:135 pass:130 dwarn:1 dfail:0 fail:0 skip:4 ilk-hp8440p total:135 pass:100 dwarn:0 dfail:0 fail:0 skip:35 ivb-t430stotal:135 pass:127 dwarn:2 dfail:0 fail:0 skip:6 skl-i5k-2total:135 pass:125 dwarn:2 dfail:0 fail:0 skip:8 skl-i7k-2total:135 pass:124 dwarn:3 dfail:0 fail:0 skip:8 snb-dellxps total:135 pass:121 dwarn:2 dfail:0 fail:0 skip:12 snb-x220ttotal:135 pass:121 dwarn:2 dfail:0 fail:1 skip:11 Results at /archive/results/CI_IGT_test/Patchwork_1088/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] [trivial] drm/i915 Fix typos in i915_gem_fence.c
On 01/04/16 19:29, Masanari Iida wrote: > This patch fix some spelling typos found in Documentation/Docbook > gpu/ch04s03.html. This file was generated from comments within > source, so I have to fix typos in i915_gem_fence.c. > > Signed-off-by: Masanari Iida > --- > drivers/gpu/drm/i915/i915_gem_fence.c | 10 +- > 1 file changed, 5 insertions(+), 5 deletions(-) > Acked-by: Randy Dunlap Thanks. -- ~Randy ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 5/5] drm/i915: Use CSB helper in debugfs
Since we extracted it for use in error state, we may as well use it in debugfs
too.
Signed-off-by: Ben Widawsky
---
drivers/gpu/drm/i915/i915_debugfs.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 3b05bd1..0b829fa 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2099,8 +2099,7 @@ static int i915_execlists(struct seq_file *m, void *data)
read_pointer, write_pointer);
for (i = 0; i < GEN8_CSB_ENTRIES; i++) {
- status = I915_READ(RING_CONTEXT_STATUS_BUF_LO(ring, i));
- ctx_id = I915_READ(RING_CONTEXT_STATUS_BUF_HI(ring, i));
+ intel_lrc_get_context_status(ring, i, &status, &ctx_id);
seq_printf(m, "\tStatus buffer %d: 0x%08X, context:
%u\n",
i, status, ctx_id);
--
2.6.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/5] drm/i915: Change WARN to ERROR in CSB count
There is no point in emitting a WARN since the backtrace will always be the
same. Errors have actually become easier to spot given the large number of WARNs
which exist today in modesetting paths.
Signed-off-by: Ben Widawsky
---
drivers/gpu/drm/i915/intel_lrc.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 7fb2035..14affaa 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -556,7 +556,9 @@ void intel_lrc_irq_handler(struct intel_engine_cs *ring)
spin_unlock(&ring->execlist_lock);
- WARN(submit_contexts > 2, "More than two context complete events?\n");
+ if (unlikely(submit_contexts > 2))
+ DRM_ERROR("More than two context complete events?\n");
+
ring->next_context_status_buffer = write_pointer % GEN8_CSB_ENTRIES;
/* Update the read pointer to the old write pointer. Manual ringbuffer
--
2.6.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/5] drm/i915: Extract CSB status read
This is a useful thing to have around as a function because the mechanism may
change in the future.
There is a net increase in LOC here, and it will continue to be the case on GEN8
and GEN9 - but future GENs may have an alternate mechanism for doing this.
Signed-off-by: Ben Widawsky
---
drivers/gpu/drm/i915/intel_lrc.c | 19 ---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 14affaa..23839ff 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -496,6 +496,19 @@ static bool execlists_check_remove_request(struct
intel_engine_cs *ring,
return false;
}
+static void get_context_status(struct intel_engine_cs *ring,
+ u8 read_pointer,
+ u32 *status, u32 *context_id)
+{
+ struct drm_i915_private *dev_priv = ring->dev->dev_private;
+
+ if (WARN_ON(read_pointer >= GEN8_CSB_ENTRIES))
+ return;
+
+ *status = I915_READ(RING_CONTEXT_STATUS_BUF_LO(ring, read_pointer));
+ *context_id = I915_READ(RING_CONTEXT_STATUS_BUF_HI(ring, read_pointer));
+}
+
/**
* intel_lrc_irq_handler() - handle Context Switch interrupts
* @ring: Engine Command Streamer to handle.
@@ -523,9 +536,9 @@ void intel_lrc_irq_handler(struct intel_engine_cs *ring)
spin_lock(&ring->execlist_lock);
while (read_pointer < write_pointer) {
- read_pointer++;
- status = I915_READ(RING_CONTEXT_STATUS_BUF_LO(ring,
read_pointer % GEN8_CSB_ENTRIES));
- status_id = I915_READ(RING_CONTEXT_STATUS_BUF_HI(ring,
read_pointer % GEN8_CSB_ENTRIES));
+
+ get_context_status(ring, ++read_pointer % GEN8_CSB_ENTRIES,
+ &status, &status_id);
if (status & GEN8_CTX_STATUS_IDLE_ACTIVE)
continue;
--
2.6.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 0/5] Some minor CSB/execlist stuff
While doing some debug in simulation, I came across a handful of patches which I think are beneficial today. Mostly this just has some minor cleanups and error state additions. They're pretty optional, though I have a private branch that depends on some of this stuff, so it'd be nice to land as much as possible - but I'll live without it. Ben Widawsky (5): drm/i915: Cleanup some of the CSB handling drm/i915: change WARN to ERROR in CSB count drm/i915: Extract CSB status read drm/i915: Add basic execlist info to error state drm/i915: Use CSB helper in debugfs drivers/gpu/drm/i915/i915_debugfs.c | 9 - drivers/gpu/drm/i915/i915_drv.h | 7 ++- drivers/gpu/drm/i915/i915_gpu_error.c | 23 + drivers/gpu/drm/i915/intel_lrc.c | 38 ++- drivers/gpu/drm/i915/intel_lrc.h | 22 ++-- 5 files changed, 81 insertions(+), 18 deletions(-) -- 2.6.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/5] drm/i915: Add basic execlist info to error state
Sample output:
...
waiting: yes
ring->head: 0x
ring->tail: 0x0c50
ring->next_context_status_buffer: 0x5
CSB Pointer: 0x0405
Context 0 Status: 0x0001
Context 1 Status: 0x009d0018
Context 2 Status: 0x0001
Context 3 Status: 0x00010018
Context 4 Status: 0x0001
Context 5 Status: 0x009d0018
hangcheck: hung [40]
bsd command stream:
START: 0x00039000
HEAD: 0x0018
...
Signed-off-by: Ben Widawsky
---
drivers/gpu/drm/i915/i915_drv.h | 7 ++-
drivers/gpu/drm/i915/i915_gpu_error.c | 23 +++
drivers/gpu/drm/i915/intel_lrc.c | 10 +-
drivers/gpu/drm/i915/intel_lrc.h | 4
4 files changed, 38 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c6dd4db..c79e869 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -526,6 +526,7 @@ struct drm_i915_error_state {
u32 cpu_ring_tail;
u32 semaphore_seqno[I915_NUM_RINGS - 1];
+ u32 semaphore_mboxes[I915_NUM_RINGS - 1];
/* Register state */
u32 start;
@@ -545,7 +546,11 @@ struct drm_i915_error_state {
u32 fault_reg;
u64 faddr;
u32 rc_psmi; /* sleep state */
- u32 semaphore_mboxes[I915_NUM_RINGS - 1];
+
+ /* execlist state */
+ u32 csb_ptr;
+ u8 next_context_status_buffer;
+ u64 context_status[GEN8_CSB_ENTRIES];
struct drm_i915_error_object {
int page_count;
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c
b/drivers/gpu/drm/i915/i915_gpu_error.c
index 06ca408..20a5daa 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -301,6 +301,18 @@ static void i915_ring_error_state(struct
drm_i915_error_state_buf *m,
err_printf(m, " waiting: %s\n", yesno(ring->waiting));
err_printf(m, " ring->head: 0x%08x\n", ring->cpu_ring_head);
err_printf(m, " ring->tail: 0x%08x\n", ring->cpu_ring_tail);
+
+ if (i915.enable_execlists) {
+ int j;
+ err_printf(m, " ring->next_context_status_buffer: 0x%d\n",
+ ring->next_context_status_buffer);
+ err_printf(m, " CSB Pointer: 0x%08x\n", ring->csb_ptr);
+ for (j = 0; j < GEN8_CSB_ENTRIES; j++) {
+ err_printf(m, "Context %d Status: 0x%016llx\n",
+ j, ring->context_status[j]);
+ }
+ }
+
err_printf(m, " hangcheck: %s [%d]\n",
hangcheck_action_to_str(ring->hangcheck_action),
ring->hangcheck_score);
@@ -1042,6 +1054,8 @@ static void i915_gem_record_rings(struct drm_device *dev,
}
if (i915.enable_execlists) {
+ int j;
+
/* TODO: This is only a small fix to keep basic error
* capture working, but we need to add more information
* for it to be useful (e.g. dump the context being
@@ -1051,6 +1065,15 @@ static void i915_gem_record_rings(struct drm_device *dev,
rbuf = request->ctx->engine[ring->id].ringbuf;
else
rbuf =
ring->default_context->engine[ring->id].ringbuf;
+
+ error->ring[i].csb_ptr =
I915_READ(RING_CONTEXT_STATUS_PTR(ring));
+ error->ring[i].next_context_status_buffer =
ring->next_context_status_buffer;
+ for (j = 0; j < GEN8_CSB_ENTRIES; j++) {
+ u32 status, id;
+ intel_lrc_get_context_status(ring, j, &status,
&id);
+ error->ring[i].context_status[j] =
((__u64)id<<32)|(__u64)status;
+ }
+
} else
rbuf = ring->buffer;
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 23839ff..a118146 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -496,9 +496,9 @@ static bool execlists_check_remove_request(struct
intel_engine_cs *ring,
return false;
}
-static void get_context_status(struct intel_engine_cs *ring,
- u8 read_pointer,
- u32 *status, u32 *context_id)
+void intel_lrc_get_context_status(struct intel_engine_cs *ring,
+ u8 read_pointer,
+ u32 *status, u32 *context_id)
{
struct drm_i915_private *dev_priv = ring->dev->dev_private;
@@ -537,8 +537,8 @@ void intel_lrc_irq_handler(struct intel_engine_cs *ring)
w
[Intel-gfx] [PATCH 1/5] drm/i915: Cleanup some of the CSB handling
I think this patch is a worthwhile cleanup even if it might look only marginally
useful. It gets more useful in upcoming patches and for handling of future GEN
platforms.
The only non-mechanical part of this is the removal of the extra & operation on
the ring->next_context_status_buffer. This is safe because right above this, we
already did a modulus operation.
Signed-off-by: Ben Widawsky
---
drivers/gpu/drm/i915/i915_debugfs.c | 6 +++---
drivers/gpu/drm/i915/intel_lrc.c| 15 +--
drivers/gpu/drm/i915/intel_lrc.h| 18 --
3 files changed, 28 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 0fc38bb..3b05bd1 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2092,13 +2092,13 @@ static int i915_execlists(struct seq_file *m, void
*data)
seq_printf(m, "\tStatus pointer: 0x%08X\n", status_pointer);
read_pointer = ring->next_context_status_buffer;
- write_pointer = status_pointer & 0x07;
+ write_pointer = GEN8_CSB_WRITE_PTR(status_pointer);
if (read_pointer > write_pointer)
- write_pointer += 6;
+ write_pointer += GEN8_CSB_ENTRIES;
seq_printf(m, "\tRead pointer: 0x%08X, write pointer 0x%08X\n",
read_pointer, write_pointer);
- for (i = 0; i < 6; i++) {
+ for (i = 0; i < GEN8_CSB_ENTRIES; i++) {
status = I915_READ(RING_CONTEXT_STATUS_BUF_LO(ring, i));
ctx_id = I915_READ(RING_CONTEXT_STATUS_BUF_HI(ring, i));
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 8096c6a..7fb2035 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -516,7 +516,7 @@ void intel_lrc_irq_handler(struct intel_engine_cs *ring)
status_pointer = I915_READ(RING_CONTEXT_STATUS_PTR(ring));
read_pointer = ring->next_context_status_buffer;
- write_pointer = status_pointer & GEN8_CSB_PTR_MASK;
+ write_pointer = GEN8_CSB_WRITE_PTR(status_pointer);
if (read_pointer > write_pointer)
write_pointer += GEN8_CSB_ENTRIES;
@@ -559,10 +559,11 @@ void intel_lrc_irq_handler(struct intel_engine_cs *ring)
WARN(submit_contexts > 2, "More than two context complete events?\n");
ring->next_context_status_buffer = write_pointer % GEN8_CSB_ENTRIES;
+ /* Update the read pointer to the old write pointer. Manual ringbuffer
+* management ftw */
I915_WRITE(RING_CONTEXT_STATUS_PTR(ring),
- _MASKED_FIELD(GEN8_CSB_PTR_MASK << 8,
-((u32)ring->next_context_status_buffer &
- GEN8_CSB_PTR_MASK) << 8));
+ _MASKED_FIELD(GEN8_CSB_READ_PTR_MASK,
+ring->next_context_status_buffer << 8));
}
static int execlists_context_queue(struct drm_i915_gem_request *request)
@@ -1506,9 +1507,11 @@ static int gen8_init_common_ring(struct intel_engine_cs
*ring)
* | Suspend-to-idle (freeze) | Suspend-to-RAM (mem) |
* BDW | CSB regs not reset | CSB regs reset |
* CHT | CSB regs not reset | CSB regs not reset |
+* SKL | ?| ?|
+* BXT | ?| ?|
*/
- next_context_status_buffer_hw =
(I915_READ(RING_CONTEXT_STATUS_PTR(ring))
- & GEN8_CSB_PTR_MASK);
+ next_context_status_buffer_hw =
+ GEN8_CSB_WRITE_PTR(I915_READ(RING_CONTEXT_STATUS_PTR(ring)));
/*
* When the CSB registers are reset (also after power-up / gpu reset),
diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
index ae90f86..de41ad6 100644
--- a/drivers/gpu/drm/i915/intel_lrc.h
+++ b/drivers/gpu/drm/i915/intel_lrc.h
@@ -25,8 +25,6 @@
#define _INTEL_LRC_H_
#define GEN8_LR_CONTEXT_ALIGN 4096
-#define GEN8_CSB_ENTRIES 6
-#define GEN8_CSB_PTR_MASK 0x07
/* Execlists regs */
#define RING_ELSP(ring)_MMIO((ring)->mmio_base
+ 0x230)
@@ -40,6 +38,22 @@
#define RING_CONTEXT_STATUS_BUF_HI(ring, i)_MMIO((ring)->mmio_base + 0x370
+ (i) * 8 + 4)
#define RING_CONTEXT_STATUS_PTR(ring) _MMIO((ring)->mmio_base + 0x3a0)
+/* The docs specify that the write pointer wraps around after 5h, "After status
+ * is written out to the last available status QW at offset 5h, this pointer
+ * wraps to 0."
+ *
+ * Therefore, one must infer than even though there are 3 bits available, 6 and
+ * 7 appear to be * reserved.
+ */
+#define GEN8_CSB_ENTRIES 6
+#define GEN8_CSB_PTR_MASK 0x7
+#define GEN8_CSB_READ_PTR_MASK (GEN8_CSB_PTR_MASK << 8)
+#define
Re: [Intel-gfx] [PATCH i-g-t] tests/gem_softpin: Use offset addresses in canonical form
-Original Message-
From: Thierry, Michel
Sent: Tuesday, January 5, 2016 10:02 AM
To: Belgaumkar, Vinay
Subject: RE: [Intel-gfx] [PATCH i-g-t] tests/gem_softpin: Use offset addresses
in canonical form
> -Original Message-
> From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf
> Of Michel Thierry
> Sent: Friday, December 11, 2015 2:14 PM
> To: intel-gfx@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH i-g-t] tests/gem_softpin: Use offset addresses
> in
> canonical form
>
> i915 validates that requested offset is in canonical form, so tests need
> to convert the offsets as required.
>
> Also add test to verify non-canonical 48-bit address will be rejected.
>
> Signed-off-by: Michel Thierry
> Reviewed-by: Vinay Belgaumkar
> ---
> tests/gem_softpin.c | 66 +---
> -
> 1 file changed, 46 insertions(+), 20 deletions(-)
>
> diff --git a/tests/gem_softpin.c b/tests/gem_softpin.c
> index 7bee16b..2981b30 100644
> --- a/tests/gem_softpin.c
> +++ b/tests/gem_softpin.c
> @@ -67,7 +67,7 @@ static void *create_mem_buffer(uint64_t size);
> static int gem_call_userptr_ioctl(int fd, i915_gem_userptr *userptr);
> static void gem_pin_userptr_test(void);
> static void gem_pin_bo_test(void);
> -static void gem_pin_invalid_vma_test(bool test_decouple_flags);
> +static void gem_pin_invalid_vma_test(bool test_decouple_flags, bool
> test_canonical_offset);
> static void gem_pin_overlap_test(void);
> static void gem_pin_high_address_test(void);
>
> @@ -198,6 +198,15 @@ static void setup_exec_obj(struct
> drm_i915_gem_exec_object2 *exec,
> exec->offset = offset;
> }
>
> +/* gen8_canonical_addr
> + * Used to convert any address into canonical form, i.e. [63:48] == [47].
> + * @address - a virtual address
> +*/
> +static uint64_t gen8_canonical_addr(uint64_t address)
> +{
> + return ((int64_t)address << 16) >> 16;
> +}
> +
> /* gem_store_data_svm
> * populate batch buffer with MI_STORE_DWORD_IMM command
> * @fd: drm file descriptor
> @@ -630,6 +639,7 @@ static void gem_pin_overlap_test(void)
> * Share with GPU using userptr ioctl
> * Create batch buffer to write DATA in first element of each buffer
> * Pin each buffer to varying addresses starting from 0x8000 going
> below
> + * (requires offsets in canonical form)
> * Execute Batch Buffer on Blit ring STRESS_NUM_LOOPS times
> * Validate every buffer has DATA in first element
> * Rinse and Repeat on Render ring
> @@ -637,7 +647,7 @@ static void gem_pin_overlap_test(void)
> #define STRESS_NUM_BUFFERS 10
> #define STRESS_NUM_LOOPS 100
> #define STRESS_STORE_COMMANDS 4 * STRESS_NUM_BUFFERS
> -
> +#define STRESS_START_ADDRESS 0x8000
> static void gem_softpin_stress_test(void)
> {
> i915_gem_userptr userptr;
> @@ -650,7 +660,7 @@ static void gem_softpin_stress_test(void)
> uint32_t batch_buf_handle;
> int ring, len;
> int buf, loop;
> - uint64_t pinning_offset = 0x8000;
> + uint64_t pinning_offset = STRESS_START_ADDRESS;
>
> fd = drm_open_driver(DRIVER_INTEL);
> igt_require(uses_full_ppgtt(fd, FULL_48_BIT_PPGTT));
> @@ -680,10 +690,10 @@ static void gem_softpin_stress_test(void)
> setup_exec_obj(&exec_object2[buf], shared_handle[buf],
> EXEC_OBJECT_PINNED |
> EXEC_OBJECT_SUPPORTS_48B_ADDRESS,
> -pinning_offset);
> +gen8_canonical_addr(pinning_offset));
> len += gem_store_data_svm(fd, batch_buffer + (len/4),
> - pinning_offset, buf,
> - (buf == STRESS_NUM_BUFFERS-1)?
> \
> +
> gen8_canonical_addr(pinning_offset),
> + buf, (buf ==
> STRESS_NUM_BUFFERS-1)? \
> true:false);
>
> /* decremental 4K aligned address */
> @@ -705,10 +715,11 @@ static void gem_softpin_stress_test(void)
> for (loop = 0; loop < STRESS_NUM_LOOPS; loop++) {
> submit_and_sync(fd, &execbuf, batch_buf_handle);
> /* Set pinning offset back to original value */
> - pinning_offset = 0x8000;
> + pinning_offset = STRESS_START_ADDRESS;
> for(buf = 0; buf < STRESS_NUM_BUFFERS; buf++) {
> gem_userptr_sync(fd, shared_handle[buf]);
> - igt_assert(exec_object2[buf].offset ==
> pinning_offset);
> + igt_assert(exec_object2[buf].offset ==
> +
> gen8_canonical_addr(pinning_offset));
> igt_fail_on_f(*shared_buffer[buf] != buf, \
> "Mismatch in buffer %d, iteration %d:
> 0x%08X\n", \
> buf, loop, *shared_buffer[buf]);
> @@ -727,
[Intel-gfx] [PATCH v3] drm/i915/guc: Expose (intel)_lr_context_size()
From: Dave Gordon
The GuC code needs to know the size of a logical context, so we
expose get_lr_context_size(), renaming it intel_lr_context__size()
to fit the naming conventions for nonstatic functions.
Add comments or kerneldoc (Daniel Vetter)
For: VIZ-2021
Signed-off-by: Dave Gordon
Signed-off-by: Alex Dai
---
drivers/gpu/drm/i915/intel_lrc.c | 13 +++--
drivers/gpu/drm/i915/intel_lrc.h | 1 +
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index e095058..9be9835 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -2399,7 +2399,16 @@ void intel_lr_context_free(struct intel_context *ctx)
}
}
-static uint32_t get_lr_context_size(struct intel_engine_cs *ring)
+/**
+ * intel_lr_context_size() - get the LRC state pages size
+ * @ring: engine to be used to get ring id
+ *
+ * The LRC state pages size varies for different engines. This function is used
+ * in ExecList / GuC mode to get LRC state pages size.
+ *
+ * Return: size of the LRC state pages. zero on unknown engine.
+ */
+uint32_t intel_lr_context_size(struct intel_engine_cs *ring)
{
int ret = 0;
@@ -2467,7 +2476,7 @@ int intel_lr_context_deferred_alloc(struct intel_context
*ctx,
WARN_ON(ctx->legacy_hw_ctx.rcs_state != NULL);
WARN_ON(ctx->engine[ring->id].state);
- context_size = round_up(get_lr_context_size(ring), 4096);
+ context_size = round_up(intel_lr_context_size(ring), 4096);
/* One extra page as the sharing data between driver and GuC */
context_size += PAGE_SIZE * LRC_PPHWSP_PN;
diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
index 0b821b9..ae90f86 100644
--- a/drivers/gpu/drm/i915/intel_lrc.h
+++ b/drivers/gpu/drm/i915/intel_lrc.h
@@ -84,6 +84,7 @@ static inline void intel_logical_ring_emit_reg(struct
intel_ringbuffer *ringbuf,
#define LRC_STATE_PN (LRC_PPHWSP_PN + 1)
void intel_lr_context_free(struct intel_context *ctx);
+uint32_t intel_lr_context_size(struct intel_engine_cs *ring);
int intel_lr_context_deferred_alloc(struct intel_context *ctx,
struct intel_engine_cs *ring);
void intel_lr_context_unpin(struct drm_i915_gem_request *req);
--
2.5.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Update Skylake DDI translation table for HDMI.
On Tue, Jan 05, 2016 at 07:32:59AM -0800, Rodrigo Vivi wrote:
> When debuging an intermittent corrupted screen I suspected on DDI
> translation table and checked we are out of date with the spec.
>
> I'm not sure this will fix my bug yet, but it is always good to follow
> the spec.
>
> Signed-off-by: Rodrigo Vivi
> ---
> drivers/gpu/drm/i915/intel_ddi.c | 20 ++--
> 1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> b/drivers/gpu/drm/i915/intel_ddi.c
> index e6408e5..c7e3114 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -225,27 +225,27 @@ static const struct ddi_buf_trans
> skl_ddi_translations_hdmi[] = {
> { 0x7011, 0x0088, 0x0 },
> { 0x0018, 0x00A1, 0x0 },
> { 0x0018, 0x0098, 0x0 },
> - { 0x4013, 0x0088, 0x0 },
> - { 0x6012, 0x0087, 0x0 },
> + { 0x4013, 0x0088, 0x1 },
^^^
0x0
> + { 0x80006012, 0x00CD, 0x0 },
^^^
0x1
The rest looks fine.
> { 0x0018, 0x00DF, 0x0 },
> - { 0x3015, 0x0087, 0x0 },/* Default */
> - { 0x3015, 0x00C7, 0x0 },
> - { 0x0018, 0x00C7, 0x0 },
> + { 0x80003015, 0x00CD, 0x1 },/* Default */
> + { 0x80003015, 0x00C0, 0x1 },
> + { 0x8018, 0x00C0, 0x1 },
> };
>
> /* Skylake Y */
> static const struct ddi_buf_trans skl_y_ddi_translations_hdmi[] = {
> { 0x0018, 0x00A1, 0x0 },
> { 0x5012, 0x00DF, 0x0 },
> - { 0x7011, 0x0084, 0x0 },
> + { 0x80007011, 0x00CB, 0x3 },
> { 0x0018, 0x00A4, 0x0 },
> { 0x0018, 0x009D, 0x0 },
> { 0x4013, 0x0080, 0x0 },
> - { 0x6013, 0x00C7, 0x0 },
> + { 0x80006013, 0x00C0, 0x3 },
> { 0x0018, 0x008A, 0x0 },
> - { 0x3015, 0x00C7, 0x0 },/* Default */
> - { 0x80003015, 0x00C7, 0x7 },/* Uses I_boost level 0x7 */
Since you're removing this pointless "Uses I_boost..." comment,
maybe remove all such comments as a followup?
> - { 0x0018, 0x00C7, 0x0 },
> + { 0x80003015, 0x00C0, 0x3 },/* Default */
> + { 0x80003015, 0x00C0, 0x3 },
> + { 0x8018, 0x00C0, 0x3 },
> };
>
> struct bxt_ddi_buf_trans {
> --
> 2.4.3
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
