On Tue, Feb 20, 2018 at 05:30:51PM +0000, Chris Wilson wrote:
> Quoting Ville Syrjala (2018-02-20 17:05:23)
> > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> >
> > Just store function pointers that give us the correct register offsets
> > instead of storing the register offsets themselves. Slightly less
> > efficient perhaps but saves a few bytes and better matches how we do
> > things elsewhere.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/intel_dp.c | 81 +++++++++++++++++++---------------------
> > drivers/gpu/drm/i915/intel_drv.h | 5 ++-
> > 2 files changed, 41 insertions(+), 45 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index eeb8a026fd08..ea414766f82b 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -936,7 +936,7 @@ static uint32_t
> > intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool has_aux_irq)
> > {
> > struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
> > - i915_reg_t ch_ctl = intel_dp->aux_ch_ctl_reg;
> > + i915_reg_t ch_ctl = intel_dp->aux_ch_ctl_reg(intel_dp);
> > uint32_t status;
> > bool done;
> >
> > @@ -1089,7 +1089,7 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
> > struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > struct drm_i915_private *dev_priv =
> > to_i915(intel_dig_port->base.base.dev);
> > - i915_reg_t ch_ctl = intel_dp->aux_ch_ctl_reg;
> > + i915_reg_t ch_ctl = intel_dp->aux_ch_ctl_reg(intel_dp);
> > uint32_t aux_clock_divider;
> > int i, ret, recv_bytes;
> > uint32_t status;
> > @@ -1154,7 +1154,7 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
> > for (try = 0; try < 5; try++) {
> > /* Load the send data into the aux channel data registers */
> > for (i = 0; i < send_bytes; i += 4)
> > - I915_WRITE(intel_dp->aux_ch_data_reg[i >> 2],
> > + I915_WRITE(intel_dp->aux_ch_data_reg(intel_dp, i >> 2),
> > intel_dp_pack_aux(send + i,
> > send_bytes - i));
> >
> > @@ -1239,7 +1239,7 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
> > recv_bytes = recv_size;
> >
> > for (i = 0; i < recv_bytes; i += 4)
> > - intel_dp_unpack_aux(I915_READ(intel_dp->aux_ch_data_reg[i >> 2]),
> > + intel_dp_unpack_aux(I915_READ(intel_dp->aux_ch_data_reg(intel_dp, i >> 2)),
> > recv + i, recv_bytes - i);
>
> I might have created data_reg[5] locals for intel_dp_aux_ch(), and even
> consider intel_dp->aux_ch_data_regs fill the entire reg[5] array in one
> pass.
Hmm. We do use a 'ch_ctl' local for the ctl reg so something
like that wouldn't feel totally out of place.
This is the only caller so not sure if there's much point in having the
vfuncs populate the entire array.
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 005735a7fc29..787c0aa45163 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1089,7 +1089,7 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
struct drm_i915_private *dev_priv =
to_i915(intel_dig_port->base.base.dev);
- i915_reg_t ch_ctl = intel_dp->aux_ch_ctl_reg(intel_dp);
+ i915_reg_t ch_ctl, ch_data[5];
uint32_t aux_clock_divider;
int i, ret, recv_bytes;
uint32_t status;
@@ -1097,6 +1097,10 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
bool has_aux_irq = HAS_AUX_IRQ(dev_priv);
bool vdd;
+ ch_ctl = intel_dp->aux_ch_ctl_reg(intel_dp);
+ for (i = 0; i < ARRAY_SIZE(ch_data); i++)
+ ch_data[i] = intel_dp->aux_ch_data_reg(intel_dp, i);
+
pps_lock(intel_dp);
/*
@@ -1154,7 +1158,7 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
for (try = 0; try < 5; try++) {
/* Load the send data into the aux channel data registers */
for (i = 0; i < send_bytes; i += 4)
- I915_WRITE(intel_dp->aux_ch_data_reg(intel_dp, i >> 2),
+ I915_WRITE(ch_data[i >> 2],
intel_dp_pack_aux(send + i,
send_bytes - i));
@@ -1239,7 +1243,7 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
recv_bytes = recv_size;
for (i = 0; i < recv_bytes; i += 4)
- intel_dp_unpack_aux(I915_READ(intel_dp->aux_ch_data_reg(intel_dp, i >> 2)),
+ intel_dp_unpack_aux(I915_READ(ch_data[i >> 2]),
recv + i, recv_bytes - i);
ret = recv_bytes;
--
2.13.6
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
