public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
From: Jani Nikula <jani.nikula@linux.intel.com>
To: "Lee, Chon Ming" <chon.ming.lee@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915: Add additional pipe parameter for vlv_dpio_read and vlv_dpio_write.
Date: Thu, 05 Sep 2013 12:04:07 +0300	[thread overview]
Message-ID: <8738pjiqrc.fsf@intel.com> (raw)
In-Reply-To: <20130905084024.GA31537@ubuntu.png.intel.com>

On Thu, 05 Sep 2013, "Lee, Chon Ming" <chon.ming.lee@intel.com> wrote:
> On 09/05 11:35, Jani Nikula wrote:
>> On Thu, 05 Sep 2013, Chon Ming Lee <chon.ming.lee@intel.com> wrote:
>> > The additional pipe parameter will use to select which phy to target
>> > for.
>> >
>> > Signed-off-by: Chon Ming Lee <chon.ming.lee@intel.com>
>> > ---
>> 
>> [snip]
>> 
>> > diff --git a/drivers/gpu/drm/i915/intel_sideband.c b/drivers/gpu/drm/i915/intel_sideband.c
>> > index 0a41670..12bbc28 100644
>> > --- a/drivers/gpu/drm/i915/intel_sideband.c
>> > +++ b/drivers/gpu/drm/i915/intel_sideband.c
>> > @@ -157,19 +157,29 @@ void vlv_gps_core_write(struct drm_i915_private *dev_priv, u32 reg, u32 val)
>> >  			PUNIT_OPCODE_REG_WRITE, reg, &val);
>> >  }
>> >  
>> > -u32 vlv_dpio_read(struct drm_i915_private *dev_priv, int reg)
>> > +u32 vlv_get_phy_port(enum pipe pipe)

This should be static.

>> > +{
>> > +	u32 port = IOSF_PORT_DPIO;
>> > +
>> > +	if (!((pipe == PIPE_A) || (pipe == PIPE_B))) {
>> > +		DRM_ERROR("Invalid pipe detected\n");
>> > +	}

Could be WARN_ON(pipe != PIPE_A && pipe != PIPE_B); 

>> > +
>> > +	return port;
>> > +}
>> 
>> So this patch doesn't actually have any functional changes. It would be
>> good to say that in the commit message.
>> 
>> It would also be interesting to see the intended follow-up patch using
>> the pipe parameter for something, in order to assess the usefulness of
>> this change.
>> 
>> BR,
>> Jani.
>> 
> Yup, will update the message.  This patch is to actually prepare for
> some future platform.  Suggested by Daniel to upstream this first.

Ah, right. With the fixes, you get,

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


>
>> 
>> 
>> > +
>> > +u32 vlv_dpio_read(struct drm_i915_private *dev_priv, enum pipe pipe, int reg)
>> >  {
>> >  	u32 val = 0;
>> >  
>> > -	vlv_sideband_rw(dev_priv, DPIO_DEVFN, IOSF_PORT_DPIO,
>> > +	vlv_sideband_rw(dev_priv, DPIO_DEVFN, vlv_get_phy_port(pipe),
>> >  			DPIO_OPCODE_REG_READ, reg, &val);
>> > -
>> >  	return val;
>> >  }
>> >  
>> > -void vlv_dpio_write(struct drm_i915_private *dev_priv, int reg, u32 val)
>> > +void vlv_dpio_write(struct drm_i915_private *dev_priv, enum pipe pipe, int reg, u32 val)
>> >  {
>> > -	vlv_sideband_rw(dev_priv, DPIO_DEVFN, IOSF_PORT_DPIO,
>> > +	vlv_sideband_rw(dev_priv, DPIO_DEVFN, vlv_get_phy_port(pipe),
>> >  			DPIO_OPCODE_REG_WRITE, reg, &val);
>> >  }
>> >  
>> > -- 
>> > 1.7.7.6
>> >
>> > _______________________________________________
>> > Intel-gfx mailing list
>> > Intel-gfx@lists.freedesktop.org
>> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> 
>> -- 
>> Jani Nikula, Intel Open Source Technology Center

-- 
Jani Nikula, Intel Open Source Technology Center

  parent reply	other threads:[~2013-09-05  9:02 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-09-05  5:33 [PATCH] drm/i915: Add additional pipe parameter for vlv_dpio_read and vlv_dpio_write Chon Ming Lee
2013-09-05  8:35 ` Jani Nikula
2013-09-05  8:40   ` Lee, Chon Ming
2013-09-05  8:51     ` Daniel Vetter
2013-09-05  9:04     ` Jani Nikula [this message]
2013-09-05  8:56 ` Ville Syrjälä

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=8738pjiqrc.fsf@intel.com \
    --to=jani.nikula@linux.intel.com \
    --cc=chon.ming.lee@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox