[Intel-gfx] [PATCH v2 2/3] drm/i915: Add a brief description of the VLV display PHY internals

Ville Syrjälä ville.syrjala at linux.intel.com
Wed May 21 12:31:52 CEST 2014


On Wed, May 21, 2014 at 09:58:35AM +0000, Lee, Chon Ming wrote:
> 
> 
> > -----Original Message-----
> > From: Ville Syrjälä [mailto:ville.syrjala at linux.intel.com]
> > Sent: Wednesday, May 21, 2014 4:50 PM
> > To: Lee, Chon Ming
> > Cc: intel-gfx at lists.freedesktop.org
> > Subject: Re: [Intel-gfx] [PATCH v2 2/3] drm/i915: Add a brief description of
> > the VLV display PHY internals
> > 
> > On Wed, May 21, 2014 at 04:31:37PM +0800, Lee, Chon Ming wrote:
> > > On 04/25 20:14, ville.syrjala at linux.intel.com wrote:
> > > > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > > >
> > > > Document the internal structure of the VLV display PHY a bit to help
> > > > people understand how the different register blocks relate to each
> > > > other.
> > > >
> > > > v2: Add a bit more text
> > > >     Make it a DOC: comment, but leave the ascii art out since
> > > >     it would get mangled
> > > >
> > > > Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > > > ---
> > > >  Documentation/DocBook/drm.tmpl  |  4 ++
> > > > drivers/gpu/drm/i915/i915_reg.h | 85
> > > > +++++++++++++++++++++++++++++++++++++++--
> > > >  2 files changed, 85 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/Documentation/DocBook/drm.tmpl
> > > > b/Documentation/DocBook/drm.tmpl index 4a955b4..e361ccd 100644
> > > > --- a/Documentation/DocBook/drm.tmpl
> > > > +++ b/Documentation/DocBook/drm.tmpl
> > > > @@ -2942,6 +2942,10 @@ int num_ioctls;</synopsis>
> > > >  	  probing, so those sections fully apply.
> > > >          </para>
> > > >        </sect2>
> > > > +      <sect2>
> > > > +        <title>DPIO</title>
> > > > +!Pdrivers/gpu/drm/i915/i915_reg.h DPIO
> > > > +      </sect2>
> > > >      </sect1>
> > > >
> > > >      <sect1>
> > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > > > b/drivers/gpu/drm/i915/i915_reg.h index b6d5045..8e18e8f 100644
> > > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > > @@ -566,12 +566,89 @@ enum punit_power_well {
> > > >  #define  DSI_PLL_M1_DIV_MASK			(0x1ff << 0)
> > > >  #define CCK_DISPLAY_CLOCK_CONTROL		0x6b
> > > >
> > > > -/*
> > > > - * DPIO - a special bus for various display related registers to
> > > > hide behind
> > > > +/**
> 
> This is something I miss out.  The /** should be /*   to match what you want to do in the first patch.

No, this time around we really want this comment in the generated docs.

> 
> Regards,
> Chon Ming
> 
> > > > + * DOC: DPIO
> > > > + *
> > > > + * VLV and CHV have slightly peculiar display PHYs for driving
> > > > +DP/HDMI
> > > > + * ports. DPIO is the name given to such a display PHY. These PHYs
> > > > + * don't follow the standard programming model using direct MMIO
> > > > + * registers, and instead their registers must be accessed trough
> > > > +IOSF
> > > > + * sideband. VLV has one such PHY for driving ports B and C, and
> > > > +CHV
> > > > + * adds another PHY for driving port D. Each PHY responds to
> > > > +specific
> > > > + * IOSF-SB port.
> > > > + *
> > > > + * Each display PHY is made up of one or two channels. Each channel
> > > > + * houses a common lane part which contains the PLL and other
> > > > +common
> > > > + * logic. CH0 common lane also contains the IOSF-SB logic for the
> > > > + * Common Register Interface (CRI) ie. the DPIO registers. CRI
> > > > +clock
> > > > + * must be running when any DPIO registers are accessed.
> > > > + *
> > > > + * In addition to having their own registers, the PHYs are also
> > > > + * controlled through some dedicated signals from the display
> > > > + * controller. These include PLL reference clock enable, PLL
> > > > +enable,
> > > > + * and CRI clock selection, for example.
> > > > + *
> > > > + * Eeach channel also has two splines (also called data lanes), and
> > > > + * each spline is made up of one Physical Access Coding Sub-Layer
> > > > + * (PCS) block and two TX lanes. So each channel has two PCS blocks
> > > > + * and four TX lanes. The TX lanes are used as DP lanes or TMDS
> > > > + * data/clock pairs depending on the output type.
> > > > + *
> > > > + * Additionally the PHY also contains an AUX lane with AUX blocks
> > > > + * for each channel. This is used for DP AUX communication, but
> > > > + * this fact isn't really relevant for the driver since AUX is
> > > > + * controlled from the display controller side. No DPIO registers
> > > > + * need to be accessed during AUX communication,
> > > > + *
> > > > + * Generally the common lane corresponds to the pipe and
> > > > + * the spline (PCS/TX) correponds to the port.
> > > > + *
> > > > + * For dual channel PHY (VLV/CHV):
> > > > + *
> > > > + *  pipe A == CMN/PLL/REF CH0
> > > >   *
> > > > - * DPIO is VLV only.
> > > > + *  pipe B == CMN/PLL/REF CH1
> > > > + *
> > > > + *  port B == PCS/TX CH0
> > > > + *
> > > > + *  port C == PCS/TX CH1
> > > > + *
> > > > + * This is especially important when we cross the streams
> > > > + * ie. drive port B with pipe B, or port C with pipe A.
> > > > + *
> > >
> > >
> > > Do you want to add something like the PHY actually allow PLL CH0 to
> > > supply clock to both ports, same as PLL CH1. But this is something
> > > i915 not supported yet, for power saving purpose.
> > 
> > Maybe we can add the comment if and when we implement it.
> > 
> > I'm not even sure if we should implement it since it might lead to blinking
> > displays if we have to reroute the PLLs for active pipes.
> > Although maybe it's possible to swap over to the pipe's own PLL from the
> > shared case w/o blinking, but that would require special code in the modeset
> > path since we'd need to make sure we fire up the new PLL first, then swap
> > the PLLs, and only then can reprogram the old PLL.
> > 
> > So unless the power savings are really significant I'm not sure anyone will
> > bother with this.
> > 
> > OTOH we already have potential display blinking due to the cdclk change logic,
> > so maybe we don't care that much.
> > 
> > >
> > > > + * For single channel PHY (CHV):
> > > > + *
> > > > + *  pipe C == CMN/PLL/REF CH0
> > > > + *
> > > > + *  port D == PCS/TX CH0
> > > > + *
> > > > + * Note: digital port B is DDI0, digital port C is DDI1,
> > > > + * digital port D is DDI2
> > > > + */
> > > > +/*
> > > > + * Dual channel PHY (VLV/CHV)
> > > > + * ---------------------------------
> > > > + * |      CH0      |      CH1      |
> > > > + * |  CMN/PLL/REF  |  CMN/PLL/REF  |
> > >
> > > There is a AUX for both CH0 and CH1.
> > 
> > I left out AUX since the fact that it's part of the PHY isn't really significant for
> > us. I did mention it in the text.
> > 
> > >
> > > Other than this,
> > >
> > > Reviewed-by: Chon Ming Lee <chon.ming.lee at intel.com>
> > >
> > >
> > > > + * |---------------|---------------| Display PHY
> > > > + * | PCS01 | PCS23 | PCS01 | PCS23 |
> > > > + * |-------|-------|-------|-------|
> > > > + * |TX0|TX1|TX2|TX3|TX0|TX1|TX2|TX3|
> > > > + * ---------------------------------
> > > > + * |     DDI0      |     DDI1      | DP/HDMI ports
> > > > + * ---------------------------------
> > > >   *
> > > > - * Note: digital port B is DDI0, digital pot C is DDI1
> > > > + * Single channel PHY (CHV)
> > > > + * -----------------
> > > > + * |      CH0      |
> > > > + * |  CMN/PLL/REF  |
> > > > + * |---------------| Display PHY
> > > > + * | PCS01 | PCS23 |
> > > > + * |-------|-------|
> > > > + * |TX0|TX1|TX2|TX3|
> > > > + * -----------------
> > > > + * |     DDI2      | DP/HDMI port
> > > > + * -----------------
> > > >   */
> > > >  #define DPIO_DEVFN			0
> > > >  #define DPIO_OPCODE_REG_WRITE		1
> > > > --
> > > > 1.8.3.2
> > > >
> > > > _______________________________________________
> > > > Intel-gfx mailing list
> > > > Intel-gfx at lists.freedesktop.org
> > > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> > --
> > Ville Syrjälä
> > Intel OTC

-- 
Ville Syrjälä
Intel OTC



More information about the Intel-gfx mailing list