From: "Souza, Jose" <jose.souza@intel.com>
To: "Deak, Imre" <imre.deak@intel.com>
Cc: "intel-gfx@lists.freedesktop.org"
<intel-gfx@lists.freedesktop.org>,
"De Marchi, Lucas" <lucas.demarchi@intel.com>
Subject: Re: [PATCH v4 1/4] drm/i915/tc: Update DP_MODE programming
Date: Thu, 26 Sep 2019 19:35:16 +0000 [thread overview]
Message-ID: <b290308c9eef178fe6f7c19ce512f11b37bf14db.camel@intel.com> (raw)
In-Reply-To: <20190926120200.GA20817@ideak-desk.fi.intel.com>
On Thu, 2019-09-26 at 15:02 +0300, Imre Deak wrote:
> On Wed, Sep 25, 2019 at 04:45:06PM -0700, José Roberto de Souza
> wrote:
> > From: Clinton A Taylor <clinton.a.taylor@intel.com>
> >
> > BSpec was updated(r146548) with a new MG_DP_MODE Programming table,
> > now taking in consideration the pin assignment and allowing us to
> > optimize power by shutting down available but not needed lanes.
> >
> > It was tested on ICL and TGL, with adaptors that used pin
> > assignment
> > C and B, reversing the connector and going to different modes
> > testing
> > the not needed lane shutdown.
> >
> > BSpec: 21735
> > BSpec: 49292
> >
> > Cc: Imre Deak <imre.deak@intel.com>
> > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > Signed-off-by: Clinton A Taylor <clinton.a.taylor@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> > drivers/gpu/drm/i915/display/intel_ddi.c | 82 +++++++++++++-------
> > ----
> > drivers/gpu/drm/i915/display/intel_tc.c | 15 +++++
> > drivers/gpu/drm/i915/display/intel_tc.h | 1 +
> > drivers/gpu/drm/i915/i915_reg.h | 5 ++
> > 4 files changed, 66 insertions(+), 37 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> > b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index aa470c70a198..316cedb16935 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -3095,7 +3095,8 @@ static void icl_program_mg_dp_mode(struct
> > intel_digital_port *intel_dig_port)
> > {
> > struct drm_i915_private *dev_priv = to_i915(intel_dig_port-
> > >base.base.dev);
> > enum port port = intel_dig_port->base.port;
> > - u32 ln0, ln1, lane_mask;
> > + u32 ln0, ln1, pin_assignment;
> > + u8 width;
> >
> > if (intel_dig_port->tc_mode == TC_PORT_TBT_ALT)
> > return;
> > @@ -3103,50 +3104,57 @@ static void icl_program_mg_dp_mode(struct
> > intel_digital_port *intel_dig_port)
> > ln0 = I915_READ(MG_DP_MODE(0, port));
> > ln1 = I915_READ(MG_DP_MODE(1, port));
> >
> > - switch (intel_dig_port->tc_mode) {
> > - case TC_PORT_DP_ALT:
> > - ln0 &= ~(MG_DP_MODE_CFG_DP_X1_MODE |
> > MG_DP_MODE_CFG_DP_X2_MODE);
> > - ln1 &= ~(MG_DP_MODE_CFG_DP_X1_MODE |
> > MG_DP_MODE_CFG_DP_X2_MODE);
> > + ln0 &= ~(MG_DP_MODE_CFG_DP_X1_MODE |
> > MG_DP_MODE_CFG_DP_X1_MODE);
> > + ln1 &= ~(MG_DP_MODE_CFG_DP_X1_MODE |
> > MG_DP_MODE_CFG_DP_X2_MODE);
> >
> > - lane_mask =
> > intel_tc_port_get_lane_mask(intel_dig_port);
> > + /* DPPATC */
> > + pin_assignment =
> > intel_tc_port_get_pin_assignment_mask(intel_dig_port);
> > + width = intel_dig_port->dp.lane_count;
>
> Should be crtc_state->lane_count. (dp.lane_count makes no sense for
> HDMI
> and it's also only set for link training)
Done
>
> >
> > - switch (lane_mask) {
> > - case 0x1:
> > - case 0x4:
> > - break;
> > - case 0x2:
> > + switch (pin_assignment) {
> > + case 0x0:
> > + WARN_ON(intel_dig_port->tc_mode != TC_PORT_LEGACY);
> > + if (width == 1) {
> > + ln1 |= MG_DP_MODE_CFG_DP_X1_MODE;
> > + } else {
> > + ln0 |= MG_DP_MODE_CFG_DP_X2_MODE;
> > + ln1 |= MG_DP_MODE_CFG_DP_X2_MODE;
> > + }
> > + break;
> > + case 0x1:
> > + if (width == 4) {
> > + ln0 |= MG_DP_MODE_CFG_DP_X2_MODE;
> > + ln1 |= MG_DP_MODE_CFG_DP_X2_MODE;
> > + }
> > + break;
> > + case 0x2:
> > + if (width == 2) {
> > + ln0 |= MG_DP_MODE_CFG_DP_X2_MODE;
> > + ln1 |= MG_DP_MODE_CFG_DP_X2_MODE;
> > + }
>
> nit: WARN_ON(width==4)?
This lane assignment only has 2 lanes, so it will never be 4.
>
> > + break;
> > + case 0x3:
> > + case 0x5:
> > + if (width == 1) {
> > ln0 |= MG_DP_MODE_CFG_DP_X1_MODE;
> > - break;
> > - case 0x3:
> > - ln0 |= MG_DP_MODE_CFG_DP_X1_MODE |
> > - MG_DP_MODE_CFG_DP_X2_MODE;
> > - break;
> > - case 0x8:
> > ln1 |= MG_DP_MODE_CFG_DP_X1_MODE;
> > - break;
> > - case 0xC:
> > - ln1 |= MG_DP_MODE_CFG_DP_X1_MODE |
> > - MG_DP_MODE_CFG_DP_X2_MODE;
> > - break;
> > - case 0xF:
> > - ln0 |= MG_DP_MODE_CFG_DP_X1_MODE |
> > - MG_DP_MODE_CFG_DP_X2_MODE;
> > - ln1 |= MG_DP_MODE_CFG_DP_X1_MODE |
> > - MG_DP_MODE_CFG_DP_X2_MODE;
> > - break;
> > - default:
> > - MISSING_CASE(lane_mask);
> > + } else {
> > + ln0 |= MG_DP_MODE_CFG_DP_X2_MODE;
> > + ln1 |= MG_DP_MODE_CFG_DP_X2_MODE;
> > }
> > break;
> > -
> > - case TC_PORT_LEGACY:
> > - ln0 |= MG_DP_MODE_CFG_DP_X1_MODE |
> > MG_DP_MODE_CFG_DP_X2_MODE;
> > - ln1 |= MG_DP_MODE_CFG_DP_X1_MODE |
> > MG_DP_MODE_CFG_DP_X2_MODE;
> > + case 0x4:
> > + case 0x6:
> > + if (width == 1) {
> > + ln0 |= MG_DP_MODE_CFG_DP_X1_MODE;
> > + ln1 |= MG_DP_MODE_CFG_DP_X1_MODE;
> > + } else {
> > + ln0 |= MG_DP_MODE_CFG_DP_X2_MODE;
> > + ln1 |= MG_DP_MODE_CFG_DP_X2_MODE;
> > + }
>
> WARN_ON(width==4)?
Same as above.
>
> With the lane_count fix:
> Reviewed-by: Imre Deak <imre.deak@intel.com>
Thanks
>
> > break;
> > -
> > default:
> > - MISSING_CASE(intel_dig_port->tc_mode);
> > - return;
> > + MISSING_CASE(pin_assignment);
> > }
> >
> > I915_WRITE(MG_DP_MODE(0, port), ln0);
> > diff --git a/drivers/gpu/drm/i915/display/intel_tc.c
> > b/drivers/gpu/drm/i915/display/intel_tc.c
> > index f923f9cbd33c..7773169b7331 100644
> > --- a/drivers/gpu/drm/i915/display/intel_tc.c
> > +++ b/drivers/gpu/drm/i915/display/intel_tc.c
> > @@ -67,6 +67,21 @@ u32 intel_tc_port_get_lane_mask(struct
> > intel_digital_port *dig_port)
> > return lane_mask >> DP_LANE_ASSIGNMENT_SHIFT(dig_port-
> > >tc_phy_fia_idx);
> > }
> >
> > +u32 intel_tc_port_get_pin_assignment_mask(struct
> > intel_digital_port *dig_port)
> > +{
> > + struct drm_i915_private *i915 = to_i915(dig_port-
> > >base.base.dev);
> > + struct intel_uncore *uncore = &i915->uncore;
> > + u32 pin_mask;
> > +
> > + pin_mask = intel_uncore_read(uncore,
> > + PORT_TX_DFLEXPA1(dig_port-
> > >tc_phy_fia));
> > +
> > + WARN_ON(pin_mask == 0xffffffff);
> > +
> > + return (pin_mask & DP_PIN_ASSIGNMENT_MASK(dig_port-
> > >tc_phy_fia_idx)) >>
> > + DP_PIN_ASSIGNMENT_SHIFT(dig_port->tc_phy_fia_idx);
> > +}
> > +
> > int intel_tc_port_fia_max_lane_count(struct intel_digital_port
> > *dig_port)
> > {
> > struct drm_i915_private *i915 = to_i915(dig_port-
> > >base.base.dev);
> > diff --git a/drivers/gpu/drm/i915/display/intel_tc.h
> > b/drivers/gpu/drm/i915/display/intel_tc.h
> > index 783d75531435..463f1b3c836f 100644
> > --- a/drivers/gpu/drm/i915/display/intel_tc.h
> > +++ b/drivers/gpu/drm/i915/display/intel_tc.h
> > @@ -13,6 +13,7 @@ struct intel_digital_port;
> >
> > bool intel_tc_port_connected(struct intel_digital_port *dig_port);
> > u32 intel_tc_port_get_lane_mask(struct intel_digital_port
> > *dig_port);
> > +u32 intel_tc_port_get_pin_assignment_mask(struct
> > intel_digital_port *dig_port);
> > int intel_tc_port_fia_max_lane_count(struct intel_digital_port
> > *dig_port);
> > void intel_tc_port_set_fia_lane_count(struct intel_digital_port
> > *dig_port,
> > int required_lanes);
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index e752de9470bd..bcf449c1d152 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -11857,6 +11857,11 @@ enum skl_power_gate {
> > #define PORT_TX_DFLEXDPCSSS(fia) _MMIO_FIA((fia),
> > 0x00894)
> > #define DP_PHY_MODE_STATUS_NOT_SAFE(idx) (1 << (idx))
> >
> > +#define PORT_TX_DFLEXPA1(fia) _MMIO_FIA((fia)
> > , 0x00880)
> > +#define DP_PIN_ASSIGNMENT_SHIFT(idx) ((idx) * 4)
> > +#define DP_PIN_ASSIGNMENT_MASK(idx) (0xf << ((idx)
> > * 4))
> > +#define DP_PIN_ASSIGNMENT(idx, x) ((x) << ((idx)
> > * 4))
> > +
> > /* This register controls the Display State Buffer (DSB) engines.
> > */
> > #define _DSBSL_INSTANCE_BASE 0x70B00
> > #define DSBSL_INSTANCE(pipe, id) (_DSBSL_INSTANCE_BASE + \
> > --
> > 2.23.0
> >
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2019-09-26 19:35 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-25 23:45 [PATCH v4 0/4] TGL TC enabling v4 José Roberto de Souza
2019-09-25 23:45 ` [PATCH v4 1/4] drm/i915/tc: Update DP_MODE programming José Roberto de Souza
2019-09-26 12:02 ` Imre Deak
2019-09-26 19:35 ` Souza, Jose [this message]
2019-09-26 19:41 ` Imre Deak
2019-09-25 23:45 ` [PATCH v4 2/4] drm/i915/tgl: Add dkl phy programming sequences José Roberto de Souza
2019-09-26 15:34 ` Imre Deak
2019-09-26 21:02 ` Souza, Jose
2019-09-25 23:45 ` [PATCH v4 3/4] drm/i915/tgl: Fix dkl link training José Roberto de Souza
2019-09-26 13:06 ` Imre Deak
2019-09-25 23:45 ` [PATCH v4 4/4] drm/i915/tgl: initialize TC and TBT ports José Roberto de Souza
2019-09-26 0:58 ` ✓ Fi.CI.BAT: success for TGL TC enabling (rev4) Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=b290308c9eef178fe6f7c19ce512f11b37bf14db.camel@intel.com \
--to=jose.souza@intel.com \
--cc=imre.deak@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=lucas.demarchi@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.