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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> -----Original Message-----
> From: Ville Syrjälä [mailto:ville.syrjala@xxxxxxxxxxxxxxx]
> Sent: Wednesday, May 21, 2014 4:50 PM
> To: Lee, Chon Ming
> Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> Subject: Re:  [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@xxxxxxxxxxxxxxx wrote:
> > > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > >
> > > 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@xxxxxxxxxxxxxxx>
> > > ---
> > >  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.

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@xxxxxxxxx>
> >
> >
> > > + * |---------------|---------------| 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@xxxxxxxxxxxxxxxxxxxxx
> > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> --
> Ville Syrjälä
> Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx





[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux