On Wed, May 21, 2014 at 09:58:35AM +0000, Lee, Chon Ming wrote: > > > > -----Original Message----- > > From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com] > > Sent: Wednesday, May 21, 2014 4:50 PM > > To: Lee, Chon Ming > > Cc: intel-gfx@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.syrj...@linux.intel.com wrote: > > > > From: Ville Syrjälä <ville.syrj...@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.syrj...@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....@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@lists.freedesktop.org > > > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > -- > > Ville Syrjälä > > Intel OTC -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
