| Message ID | 1481792500-30863-3-git-send-email-madhav.chauhan@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Dec 15, 2016 at 02:31:33PM +0530, Madhav Chauhan wrote: > From: Deepak M <m.deepak@intel.com> > > Program the clk lane and tlpx time count registers > to configure DSI PHY. > > v2: Addressed Jani's Review comments(renamed bit field macros) > > Signed-off-by: Deepak M <m.deepak@intel.com> > Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com> > --- > drivers/gpu/drm/i915/i915_reg.h | 18 ++++++++++++++++++ > drivers/gpu/drm/i915/intel_dsi.c | 15 +++++++++++++++ > 2 files changed, 33 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 8e47b59..03858f9 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -8550,6 +8550,24 @@ enum { > #define LP_BYTECLK_SHIFT 0 > #define LP_BYTECLK_MASK (0xffff << 0) > > +#define _MIPIA_TLPX_TIME_COUNT (dev_priv->mipi_mmio_base + 0xb0a4) > +#define _MIPIC_TLPX_TIME_COUNT (dev_priv->mipi_mmio_base + 0xb8a4) > +#define MIPI_TLPX_TIME_COUNT(port) _MMIO_MIPI(port, _MIPIA_TLPX_TIME_COUNT, _MIPIC_TLPX_TIME_COUNT) > +#define GLK_DPHY_TLPX_TIME_CNT_SHIFT 0 > +#define GLK_DPHY_TLPX_TIME_CNT_MASK (0xff << 0) > + > +#define _MIPIA_CLK_LANE_TIMING (dev_priv->mipi_mmio_base + 0xb098) > +#define _MIPIC_CLK_LANE_TIMING (dev_priv->mipi_mmio_base + 0xb898) > +#define MIPI_CLK_LANE_TIMING(port) _MMIO_MIPI(port, _MIPIA_CLK_LANE_TIMING, _MIPIC_CLK_LANE_TIMING) > +#define GLK_MIPI_CLK_LANE_HS_PREP_SHIFT 0 > +#define GLK_MIPI_CLK_LANE_HS_PREP_MASK (0xff << 0) > +#define GLK_MIPI_CLK_LANE_HS_ZERO_SHIFT 8 > +#define GLK_MIPI_CLK_LANE_HS_ZERO_MASK (0xff00 << 0) > +#define GLK_MIPI_CLK_LANE_HS_TRAIL_SHIFT 16 > +#define GLK_MIPI_CLK_LANE_HS_TRAIL_MASK (0xff0000 << 0) > +#define GLK_MIPI_CLK_LANE_HS_EXIT_SHIFT 24 > +#define GLK_MIPI_CLK_LANE_HS_EXIT_MASK (0xff000000 << 0) > + > /* bits 31:0 */ > #define _MIPIA_LP_GEN_DATA (dev_priv->mipi_mmio_base + 0xb064) > #define _MIPIC_LP_GEN_DATA (dev_priv->mipi_mmio_base + 0xb864) > diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c > index 6b63355..b78c686 100644 > --- a/drivers/gpu/drm/i915/intel_dsi.c > +++ b/drivers/gpu/drm/i915/intel_dsi.c > @@ -1123,6 +1123,7 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder, > struct drm_i915_private *dev_priv = to_i915(dev); > struct intel_crtc *intel_crtc = to_intel_crtc(pipe_config->base.crtc); > struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder); > + struct mipi_config *mipi_config = dev_priv->vbt.dsi.config; > const struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode; > enum port port; > unsigned int bpp = mipi_dsi_pixel_format_to_bpp(intel_dsi->pixel_format); > @@ -1278,6 +1279,20 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder, > */ > I915_WRITE(MIPI_LP_BYTECLK(port), intel_dsi->lp_byte_clk); > > + if (IS_GEMINILAKE(dev_priv)) { > + I915_WRITE(MIPI_TLPX_TIME_COUNT(port), > + intel_dsi->lp_byte_clk); > + val = ((mipi_config->ths_prepare << > + GLK_MIPI_CLK_LANE_HS_PREP_SHIFT) | > + (mipi_config->ths_prepare_hszero << > + GLK_MIPI_CLK_LANE_HS_ZERO_SHIFT) | > + (mipi_config->ths_trail << > + GLK_MIPI_CLK_LANE_HS_TRAIL_SHIFT) | > + (mipi_config->ths_exit << > + GLK_MIPI_CLK_LANE_HS_EXIT_SHIFT)); I don't think anything else here uses the mipi_config block directly. Rather everything gets duplicated in intel_dsi. This here looks like the same (or similar) thing to dphy_reg. Probably should reuse that, or split it up into its component pieces if we want to keep duplicated information between the mipi_config and intel_dsi. > + I915_WRITE(MIPI_CLK_LANE_TIMING(port), val); > + } > + > /* the bw essential for transmitting 16 long packets containing > * 252 bytes meant for dcs write memory command is programmed in > * this register in terms of byte clocks. based on dsi transfer > -- > 1.9.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, 22 Dec 2016, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > On Thu, Dec 15, 2016 at 02:31:33PM +0530, Madhav Chauhan wrote: >> From: Deepak M <m.deepak@intel.com> >> >> Program the clk lane and tlpx time count registers >> to configure DSI PHY. >> >> v2: Addressed Jani's Review comments(renamed bit field macros) >> >> Signed-off-by: Deepak M <m.deepak@intel.com> >> Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com> >> --- >> drivers/gpu/drm/i915/i915_reg.h | 18 ++++++++++++++++++ >> drivers/gpu/drm/i915/intel_dsi.c | 15 +++++++++++++++ >> 2 files changed, 33 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h >> index 8e47b59..03858f9 100644 >> --- a/drivers/gpu/drm/i915/i915_reg.h >> +++ b/drivers/gpu/drm/i915/i915_reg.h >> @@ -8550,6 +8550,24 @@ enum { >> #define LP_BYTECLK_SHIFT 0 >> #define LP_BYTECLK_MASK (0xffff << 0) >> >> +#define _MIPIA_TLPX_TIME_COUNT (dev_priv->mipi_mmio_base + 0xb0a4) >> +#define _MIPIC_TLPX_TIME_COUNT (dev_priv->mipi_mmio_base + 0xb8a4) >> +#define MIPI_TLPX_TIME_COUNT(port) _MMIO_MIPI(port, _MIPIA_TLPX_TIME_COUNT, _MIPIC_TLPX_TIME_COUNT) >> +#define GLK_DPHY_TLPX_TIME_CNT_SHIFT 0 >> +#define GLK_DPHY_TLPX_TIME_CNT_MASK (0xff << 0) >> + >> +#define _MIPIA_CLK_LANE_TIMING (dev_priv->mipi_mmio_base + 0xb098) >> +#define _MIPIC_CLK_LANE_TIMING (dev_priv->mipi_mmio_base + 0xb898) >> +#define MIPI_CLK_LANE_TIMING(port) _MMIO_MIPI(port, _MIPIA_CLK_LANE_TIMING, _MIPIC_CLK_LANE_TIMING) >> +#define GLK_MIPI_CLK_LANE_HS_PREP_SHIFT 0 >> +#define GLK_MIPI_CLK_LANE_HS_PREP_MASK (0xff << 0) >> +#define GLK_MIPI_CLK_LANE_HS_ZERO_SHIFT 8 >> +#define GLK_MIPI_CLK_LANE_HS_ZERO_MASK (0xff00 << 0) >> +#define GLK_MIPI_CLK_LANE_HS_TRAIL_SHIFT 16 >> +#define GLK_MIPI_CLK_LANE_HS_TRAIL_MASK (0xff0000 << 0) >> +#define GLK_MIPI_CLK_LANE_HS_EXIT_SHIFT 24 >> +#define GLK_MIPI_CLK_LANE_HS_EXIT_MASK (0xff000000 << 0) >> + >> /* bits 31:0 */ >> #define _MIPIA_LP_GEN_DATA (dev_priv->mipi_mmio_base + 0xb064) >> #define _MIPIC_LP_GEN_DATA (dev_priv->mipi_mmio_base + 0xb864) >> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c >> index 6b63355..b78c686 100644 >> --- a/drivers/gpu/drm/i915/intel_dsi.c >> +++ b/drivers/gpu/drm/i915/intel_dsi.c >> @@ -1123,6 +1123,7 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder, >> struct drm_i915_private *dev_priv = to_i915(dev); >> struct intel_crtc *intel_crtc = to_intel_crtc(pipe_config->base.crtc); >> struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder); >> + struct mipi_config *mipi_config = dev_priv->vbt.dsi.config; >> const struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode; >> enum port port; >> unsigned int bpp = mipi_dsi_pixel_format_to_bpp(intel_dsi->pixel_format); >> @@ -1278,6 +1279,20 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder, >> */ >> I915_WRITE(MIPI_LP_BYTECLK(port), intel_dsi->lp_byte_clk); >> >> + if (IS_GEMINILAKE(dev_priv)) { >> + I915_WRITE(MIPI_TLPX_TIME_COUNT(port), >> + intel_dsi->lp_byte_clk); >> + val = ((mipi_config->ths_prepare << >> + GLK_MIPI_CLK_LANE_HS_PREP_SHIFT) | >> + (mipi_config->ths_prepare_hszero << >> + GLK_MIPI_CLK_LANE_HS_ZERO_SHIFT) | >> + (mipi_config->ths_trail << >> + GLK_MIPI_CLK_LANE_HS_TRAIL_SHIFT) | >> + (mipi_config->ths_exit << >> + GLK_MIPI_CLK_LANE_HS_EXIT_SHIFT)); > > I don't think anything else here uses the mipi_config block directly. > Rather everything gets duplicated in intel_dsi. This here looks > like the same (or similar) thing to dphy_reg. Probably should > reuse that, or split it up into its component pieces if we want > to keep duplicated information between the mipi_config and intel_dsi. My thoughts exactly. I've just been thinking about Hans' patches, and whether we should give in and split intel_dsi.c and intel_dsi_panel_vbt.c differently. BR, Jani. > >> + I915_WRITE(MIPI_CLK_LANE_TIMING(port), val); >> + } >> + >> /* the bw essential for transmitting 16 long packets containing >> * 252 bytes meant for dcs write memory command is programmed in >> * this register in terms of byte clocks. based on dsi transfer >> -- >> 1.9.1 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, 15 Dec 2016, Madhav Chauhan <madhav.chauhan@intel.com> wrote: > From: Deepak M <m.deepak@intel.com> > > Program the clk lane and tlpx time count registers > to configure DSI PHY. > > v2: Addressed Jani's Review comments(renamed bit field macros) > > Signed-off-by: Deepak M <m.deepak@intel.com> > Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com> > --- > drivers/gpu/drm/i915/i915_reg.h | 18 ++++++++++++++++++ > drivers/gpu/drm/i915/intel_dsi.c | 15 +++++++++++++++ > 2 files changed, 33 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 8e47b59..03858f9 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -8550,6 +8550,24 @@ enum { > #define LP_BYTECLK_SHIFT 0 > #define LP_BYTECLK_MASK (0xffff << 0) > > +#define _MIPIA_TLPX_TIME_COUNT (dev_priv->mipi_mmio_base + 0xb0a4) > +#define _MIPIC_TLPX_TIME_COUNT (dev_priv->mipi_mmio_base + 0xb8a4) > +#define MIPI_TLPX_TIME_COUNT(port) _MMIO_MIPI(port, _MIPIA_TLPX_TIME_COUNT, _MIPIC_TLPX_TIME_COUNT) > +#define GLK_DPHY_TLPX_TIME_CNT_SHIFT 0 > +#define GLK_DPHY_TLPX_TIME_CNT_MASK (0xff << 0) > + > +#define _MIPIA_CLK_LANE_TIMING (dev_priv->mipi_mmio_base + 0xb098) > +#define _MIPIC_CLK_LANE_TIMING (dev_priv->mipi_mmio_base + 0xb898) > +#define MIPI_CLK_LANE_TIMING(port) _MMIO_MIPI(port, _MIPIA_CLK_LANE_TIMING, _MIPIC_CLK_LANE_TIMING) > +#define GLK_MIPI_CLK_LANE_HS_PREP_SHIFT 0 > +#define GLK_MIPI_CLK_LANE_HS_PREP_MASK (0xff << 0) > +#define GLK_MIPI_CLK_LANE_HS_ZERO_SHIFT 8 > +#define GLK_MIPI_CLK_LANE_HS_ZERO_MASK (0xff00 << 0) 0xff << 8 > +#define GLK_MIPI_CLK_LANE_HS_TRAIL_SHIFT 16 > +#define GLK_MIPI_CLK_LANE_HS_TRAIL_MASK (0xff0000 << 0) 0xff << 16 > +#define GLK_MIPI_CLK_LANE_HS_EXIT_SHIFT 24 > +#define GLK_MIPI_CLK_LANE_HS_EXIT_MASK (0xff000000 << 0) 0xff << 24 > + > /* bits 31:0 */ > #define _MIPIA_LP_GEN_DATA (dev_priv->mipi_mmio_base + 0xb064) > #define _MIPIC_LP_GEN_DATA (dev_priv->mipi_mmio_base + 0xb864) > diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c > index 6b63355..b78c686 100644 > --- a/drivers/gpu/drm/i915/intel_dsi.c > +++ b/drivers/gpu/drm/i915/intel_dsi.c > @@ -1123,6 +1123,7 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder, > struct drm_i915_private *dev_priv = to_i915(dev); > struct intel_crtc *intel_crtc = to_intel_crtc(pipe_config->base.crtc); > struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder); > + struct mipi_config *mipi_config = dev_priv->vbt.dsi.config; > const struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode; > enum port port; > unsigned int bpp = mipi_dsi_pixel_format_to_bpp(intel_dsi->pixel_format); > @@ -1278,6 +1279,20 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder, > */ > I915_WRITE(MIPI_LP_BYTECLK(port), intel_dsi->lp_byte_clk); > > + if (IS_GEMINILAKE(dev_priv)) { > + I915_WRITE(MIPI_TLPX_TIME_COUNT(port), > + intel_dsi->lp_byte_clk); > + val = ((mipi_config->ths_prepare << > + GLK_MIPI_CLK_LANE_HS_PREP_SHIFT) | > + (mipi_config->ths_prepare_hszero << > + GLK_MIPI_CLK_LANE_HS_ZERO_SHIFT) | > + (mipi_config->ths_trail << > + GLK_MIPI_CLK_LANE_HS_TRAIL_SHIFT) | > + (mipi_config->ths_exit << > + GLK_MIPI_CLK_LANE_HS_EXIT_SHIFT)); > + I915_WRITE(MIPI_CLK_LANE_TIMING(port), val); > + } Please fix this as Ville suggested, i.e. don't look at mipi_config directly in intel_dsi.c. See what gets done with dphy_reg. We may change this later, but for now I think this is the right thing to do. BR, Jani. > + > /* the bw essential for transmitting 16 long packets containing > * 252 bytes meant for dcs write memory command is programmed in > * this register in terms of byte clocks. based on dsi transfer
> -----Original Message----- > From: Nikula, Jani > Sent: Friday, December 23, 2016 7:27 PM > To: Chauhan, Madhav <madhav.chauhan@intel.com>; intel- > gfx@lists.freedesktop.org > Cc: Conselvan De Oliveira, Ander <ander.conselvan.de.oliveira@intel.com>; > Saarinen, Jani <jani.saarinen@intel.com>; Konduru, Chandra > <chandra.konduru@intel.com>; Shankar, Uma <uma.shankar@intel.com>; > Mukherjee, Indranil <indranil.mukherjee@intel.com>; Kumar, Shobhit > <shobhit.kumar@intel.com>; Deepak M <m.deepak@intel.com>; Chauhan, > Madhav <madhav.chauhan@intel.com> > Subject: Re: [GLK MIPI DSI V2 2/9] drm/i915/glk: Program new MIPI DSI PHY > registers for GLK > > On Thu, 15 Dec 2016, Madhav Chauhan <madhav.chauhan@intel.com> > wrote: > > From: Deepak M <m.deepak@intel.com> > > > > Program the clk lane and tlpx time count registers to configure DSI > > PHY. > > > > v2: Addressed Jani's Review comments(renamed bit field macros) > > > > Signed-off-by: Deepak M <m.deepak@intel.com> > > Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com> > > --- > > drivers/gpu/drm/i915/i915_reg.h | 18 ++++++++++++++++++ > > drivers/gpu/drm/i915/intel_dsi.c | 15 +++++++++++++++ > > 2 files changed, 33 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > b/drivers/gpu/drm/i915/i915_reg.h index 8e47b59..03858f9 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -8550,6 +8550,24 @@ enum { > > #define LP_BYTECLK_SHIFT 0 > > #define LP_BYTECLK_MASK (0xffff << 0) > > > > +#define _MIPIA_TLPX_TIME_COUNT (dev_priv->mipi_mmio_base > + 0xb0a4) > > +#define _MIPIC_TLPX_TIME_COUNT (dev_priv->mipi_mmio_base > + 0xb8a4) > > +#define MIPI_TLPX_TIME_COUNT(port) _MMIO_MIPI(port, > _MIPIA_TLPX_TIME_COUNT, _MIPIC_TLPX_TIME_COUNT) > > +#define GLK_DPHY_TLPX_TIME_CNT_SHIFT 0 > > +#define GLK_DPHY_TLPX_TIME_CNT_MASK (0xff << 0) > > + > > +#define _MIPIA_CLK_LANE_TIMING (dev_priv->mipi_mmio_base > + 0xb098) > > +#define _MIPIC_CLK_LANE_TIMING (dev_priv->mipi_mmio_base > + 0xb898) > > +#define MIPI_CLK_LANE_TIMING(port) _MMIO_MIPI(port, > _MIPIA_CLK_LANE_TIMING, _MIPIC_CLK_LANE_TIMING) > > +#define GLK_MIPI_CLK_LANE_HS_PREP_SHIFT 0 > > +#define GLK_MIPI_CLK_LANE_HS_PREP_MASK (0xff > << 0) > > +#define GLK_MIPI_CLK_LANE_HS_ZERO_SHIFT 8 > > +#define GLK_MIPI_CLK_LANE_HS_ZERO_MASK > (0xff00 << 0) > > 0xff << 8 > > > +#define GLK_MIPI_CLK_LANE_HS_TRAIL_SHIFT 16 > > +#define GLK_MIPI_CLK_LANE_HS_TRAIL_MASK > (0xff0000 << 0) > > 0xff << 16 > > > +#define GLK_MIPI_CLK_LANE_HS_EXIT_SHIFT 24 > > +#define GLK_MIPI_CLK_LANE_HS_EXIT_MASK > (0xff000000 << 0) > > 0xff << 24 > > > + > > /* bits 31:0 */ > > #define _MIPIA_LP_GEN_DATA (dev_priv->mipi_mmio_base > + 0xb064) > > #define _MIPIC_LP_GEN_DATA (dev_priv->mipi_mmio_base > + 0xb864) > > diff --git a/drivers/gpu/drm/i915/intel_dsi.c > > b/drivers/gpu/drm/i915/intel_dsi.c > > index 6b63355..b78c686 100644 > > --- a/drivers/gpu/drm/i915/intel_dsi.c > > +++ b/drivers/gpu/drm/i915/intel_dsi.c > > @@ -1123,6 +1123,7 @@ static void intel_dsi_prepare(struct intel_encoder > *intel_encoder, > > struct drm_i915_private *dev_priv = to_i915(dev); > > struct intel_crtc *intel_crtc = to_intel_crtc(pipe_config->base.crtc); > > struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder); > > + struct mipi_config *mipi_config = dev_priv->vbt.dsi.config; > > const struct drm_display_mode *adjusted_mode = &pipe_config- > >base.adjusted_mode; > > enum port port; > > unsigned int bpp = > > mipi_dsi_pixel_format_to_bpp(intel_dsi->pixel_format); > > @@ -1278,6 +1279,20 @@ static void intel_dsi_prepare(struct > intel_encoder *intel_encoder, > > */ > > I915_WRITE(MIPI_LP_BYTECLK(port), intel_dsi->lp_byte_clk); > > > > + if (IS_GEMINILAKE(dev_priv)) { > > + I915_WRITE(MIPI_TLPX_TIME_COUNT(port), > > + intel_dsi->lp_byte_clk); > > + val = ((mipi_config->ths_prepare << > > + GLK_MIPI_CLK_LANE_HS_PREP_SHIFT) | > > + (mipi_config->ths_prepare_hszero << > > + GLK_MIPI_CLK_LANE_HS_ZERO_SHIFT) | > > + (mipi_config->ths_trail << > > + GLK_MIPI_CLK_LANE_HS_TRAIL_SHIFT) | > > + (mipi_config->ths_exit << > > + GLK_MIPI_CLK_LANE_HS_EXIT_SHIFT)); > > + I915_WRITE(MIPI_CLK_LANE_TIMING(port), val); > > + } > > Please fix this as Ville suggested, i.e. don't look at mipi_config directly in > intel_dsi.c. See what gets done with dphy_reg. > > We may change this later, but for now I think this is the right thing to do. Thanks. Will send the patch in next series, early next week. > > BR, > Jani. > > > + > > /* the bw essential for transmitting 16 long packets > containing > > * 252 bytes meant for dcs write memory command is > programmed in > > * this register in terms of byte clocks. based on dsi transfer > > -- > Jani Nikula, Intel Open Source Technology Center
> -----Original Message----- > From: Chauhan, Madhav > Sent: Saturday, December 24, 2016 12:53 AM > To: Nikula, Jani <jani.nikula@intel.com>; intel-gfx@lists.freedesktop.org > Cc: Conselvan De Oliveira, Ander <ander.conselvan.de.oliveira@intel.com>; > Saarinen, Jani <jani.saarinen@intel.com>; Konduru, Chandra > <chandra.konduru@intel.com>; Shankar, Uma <uma.shankar@intel.com>; > Mukherjee, Indranil <indranil.mukherjee@intel.com>; Kumar, Shobhit > <shobhit.kumar@intel.com>; Deepak M <m.deepak@intel.com> > Subject: RE: [GLK MIPI DSI V2 2/9] drm/i915/glk: Program new MIPI DSI PHY > registers for GLK > > > -----Original Message----- > > From: Nikula, Jani > > Sent: Friday, December 23, 2016 7:27 PM > > To: Chauhan, Madhav <madhav.chauhan@intel.com>; intel- > > gfx@lists.freedesktop.org > > Cc: Conselvan De Oliveira, Ander > > <ander.conselvan.de.oliveira@intel.com>; > > Saarinen, Jani <jani.saarinen@intel.com>; Konduru, Chandra > > <chandra.konduru@intel.com>; Shankar, Uma <uma.shankar@intel.com>; > > Mukherjee, Indranil <indranil.mukherjee@intel.com>; Kumar, Shobhit > > <shobhit.kumar@intel.com>; Deepak M <m.deepak@intel.com>; Chauhan, > > Madhav <madhav.chauhan@intel.com> > > Subject: Re: [GLK MIPI DSI V2 2/9] drm/i915/glk: Program new MIPI DSI > > PHY registers for GLK > > > > On Thu, 15 Dec 2016, Madhav Chauhan <madhav.chauhan@intel.com> > > wrote: > > > From: Deepak M <m.deepak@intel.com> > > > > > > Program the clk lane and tlpx time count registers to configure DSI > > > PHY. > > > > > > v2: Addressed Jani's Review comments(renamed bit field macros) > > > > > > Signed-off-by: Deepak M <m.deepak@intel.com> > > > Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com> > > > --- > > > drivers/gpu/drm/i915/i915_reg.h | 18 ++++++++++++++++++ > > > drivers/gpu/drm/i915/intel_dsi.c | 15 +++++++++++++++ > > > 2 files changed, 33 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > > b/drivers/gpu/drm/i915/i915_reg.h index 8e47b59..03858f9 100644 > > > --- a/drivers/gpu/drm/i915/i915_reg.h > > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > > @@ -8550,6 +8550,24 @@ enum { > > > #define LP_BYTECLK_SHIFT 0 > > > #define LP_BYTECLK_MASK (0xffff << 0) > > > > > > +#define _MIPIA_TLPX_TIME_COUNT (dev_priv- > >mipi_mmio_base > > + 0xb0a4) > > > +#define _MIPIC_TLPX_TIME_COUNT (dev_priv- > >mipi_mmio_base > > + 0xb8a4) > > > +#define MIPI_TLPX_TIME_COUNT(port) _MMIO_MIPI(port, > > _MIPIA_TLPX_TIME_COUNT, _MIPIC_TLPX_TIME_COUNT) > > > +#define GLK_DPHY_TLPX_TIME_CNT_SHIFT 0 > > > +#define GLK_DPHY_TLPX_TIME_CNT_MASK (0xff > << 0) > > > + > > > +#define _MIPIA_CLK_LANE_TIMING (dev_priv- > >mipi_mmio_base > > + 0xb098) > > > +#define _MIPIC_CLK_LANE_TIMING (dev_priv- > >mipi_mmio_base > > + 0xb898) > > > +#define MIPI_CLK_LANE_TIMING(port) _MMIO_MIPI(port, > > _MIPIA_CLK_LANE_TIMING, _MIPIC_CLK_LANE_TIMING) > > > +#define GLK_MIPI_CLK_LANE_HS_PREP_SHIFT 0 > > > +#define GLK_MIPI_CLK_LANE_HS_PREP_MASK (0xff > > << 0) > > > +#define GLK_MIPI_CLK_LANE_HS_ZERO_SHIFT 8 > > > +#define GLK_MIPI_CLK_LANE_HS_ZERO_MASK > > (0xff00 << 0) > > > > 0xff << 8 > > > > > +#define GLK_MIPI_CLK_LANE_HS_TRAIL_SHIFT 16 > > > +#define GLK_MIPI_CLK_LANE_HS_TRAIL_MASK > > (0xff0000 << 0) > > > > 0xff << 16 > > > > > +#define GLK_MIPI_CLK_LANE_HS_EXIT_SHIFT 24 > > > +#define GLK_MIPI_CLK_LANE_HS_EXIT_MASK > > (0xff000000 << 0) > > > > 0xff << 24 > > > > > + > > > /* bits 31:0 */ > > > #define _MIPIA_LP_GEN_DATA (dev_priv->mipi_mmio_base > > + 0xb064) > > > #define _MIPIC_LP_GEN_DATA (dev_priv->mipi_mmio_base > > + 0xb864) > > > diff --git a/drivers/gpu/drm/i915/intel_dsi.c > > > b/drivers/gpu/drm/i915/intel_dsi.c > > > index 6b63355..b78c686 100644 > > > --- a/drivers/gpu/drm/i915/intel_dsi.c > > > +++ b/drivers/gpu/drm/i915/intel_dsi.c > > > @@ -1123,6 +1123,7 @@ static void intel_dsi_prepare(struct > > > intel_encoder > > *intel_encoder, > > > struct drm_i915_private *dev_priv = to_i915(dev); > > > struct intel_crtc *intel_crtc = to_intel_crtc(pipe_config->base.crtc); > > > struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder); > > > + struct mipi_config *mipi_config = dev_priv->vbt.dsi.config; > > > const struct drm_display_mode *adjusted_mode = &pipe_config- > > >base.adjusted_mode; > > > enum port port; > > > unsigned int bpp = > > > mipi_dsi_pixel_format_to_bpp(intel_dsi->pixel_format); > > > @@ -1278,6 +1279,20 @@ static void intel_dsi_prepare(struct > > intel_encoder *intel_encoder, > > > */ > > > I915_WRITE(MIPI_LP_BYTECLK(port), intel_dsi->lp_byte_clk); > > > > > > + if (IS_GEMINILAKE(dev_priv)) { > > > + I915_WRITE(MIPI_TLPX_TIME_COUNT(port), > > > + intel_dsi->lp_byte_clk); > > > + val = ((mipi_config->ths_prepare << > > > + GLK_MIPI_CLK_LANE_HS_PREP_SHIFT) | > > > + (mipi_config->ths_prepare_hszero << > > > + GLK_MIPI_CLK_LANE_HS_ZERO_SHIFT) | > > > + (mipi_config->ths_trail << > > > + GLK_MIPI_CLK_LANE_HS_TRAIL_SHIFT) | > > > + (mipi_config->ths_exit << > > > + GLK_MIPI_CLK_LANE_HS_EXIT_SHIFT)); > > > + I915_WRITE(MIPI_CLK_LANE_TIMING(port), val); > > > + } > > > > Please fix this as Ville suggested, i.e. don't look at mipi_config > > directly in intel_dsi.c. See what gets done with dphy_reg. > > > > We may change this later, but for now I think this is the right thing to do. > > Thanks. Will send the patch in next series, early next week. For GLK, DPHY_PARAM_REG parameters have to be programmed in terms of HS byte clock count. Till BXT it used to be programmed in HS DDR Clock count, BSPEC is getting updated for this. Due to above, MIPI_CLK_LANE_TIMING is shadow register of DPHY_PARAM_REG and both have to programmed same value. Check this by printing the values inside driver for these registers. Hence we can use intel_dsi->dphy_reg for both. > > > > > BR, > > Jani. > > > > > + > > > /* the bw essential for transmitting 16 long packets > > containing > > > * 252 bytes meant for dcs write memory command is > > programmed in > > > * this register in terms of byte clocks. based on dsi transfer > > > > -- > > Jani Nikula, Intel Open Source Technology Center
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 8e47b59..03858f9 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -8550,6 +8550,24 @@ enum { #define LP_BYTECLK_SHIFT 0 #define LP_BYTECLK_MASK (0xffff << 0) +#define _MIPIA_TLPX_TIME_COUNT (dev_priv->mipi_mmio_base + 0xb0a4) +#define _MIPIC_TLPX_TIME_COUNT (dev_priv->mipi_mmio_base + 0xb8a4) +#define MIPI_TLPX_TIME_COUNT(port) _MMIO_MIPI(port, _MIPIA_TLPX_TIME_COUNT, _MIPIC_TLPX_TIME_COUNT) +#define GLK_DPHY_TLPX_TIME_CNT_SHIFT 0 +#define GLK_DPHY_TLPX_TIME_CNT_MASK (0xff << 0) + +#define _MIPIA_CLK_LANE_TIMING (dev_priv->mipi_mmio_base + 0xb098) +#define _MIPIC_CLK_LANE_TIMING (dev_priv->mipi_mmio_base + 0xb898) +#define MIPI_CLK_LANE_TIMING(port) _MMIO_MIPI(port, _MIPIA_CLK_LANE_TIMING, _MIPIC_CLK_LANE_TIMING) +#define GLK_MIPI_CLK_LANE_HS_PREP_SHIFT 0 +#define GLK_MIPI_CLK_LANE_HS_PREP_MASK (0xff << 0) +#define GLK_MIPI_CLK_LANE_HS_ZERO_SHIFT 8 +#define GLK_MIPI_CLK_LANE_HS_ZERO_MASK (0xff00 << 0) +#define GLK_MIPI_CLK_LANE_HS_TRAIL_SHIFT 16 +#define GLK_MIPI_CLK_LANE_HS_TRAIL_MASK (0xff0000 << 0) +#define GLK_MIPI_CLK_LANE_HS_EXIT_SHIFT 24 +#define GLK_MIPI_CLK_LANE_HS_EXIT_MASK (0xff000000 << 0) + /* bits 31:0 */ #define _MIPIA_LP_GEN_DATA (dev_priv->mipi_mmio_base + 0xb064) #define _MIPIC_LP_GEN_DATA (dev_priv->mipi_mmio_base + 0xb864) diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c index 6b63355..b78c686 100644 --- a/drivers/gpu/drm/i915/intel_dsi.c +++ b/drivers/gpu/drm/i915/intel_dsi.c @@ -1123,6 +1123,7 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder, struct drm_i915_private *dev_priv = to_i915(dev); struct intel_crtc *intel_crtc = to_intel_crtc(pipe_config->base.crtc); struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder); + struct mipi_config *mipi_config = dev_priv->vbt.dsi.config; const struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode; enum port port; unsigned int bpp = mipi_dsi_pixel_format_to_bpp(intel_dsi->pixel_format); @@ -1278,6 +1279,20 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder, */ I915_WRITE(MIPI_LP_BYTECLK(port), intel_dsi->lp_byte_clk); + if (IS_GEMINILAKE(dev_priv)) { + I915_WRITE(MIPI_TLPX_TIME_COUNT(port), + intel_dsi->lp_byte_clk); + val = ((mipi_config->ths_prepare << + GLK_MIPI_CLK_LANE_HS_PREP_SHIFT) | + (mipi_config->ths_prepare_hszero << + GLK_MIPI_CLK_LANE_HS_ZERO_SHIFT) | + (mipi_config->ths_trail << + GLK_MIPI_CLK_LANE_HS_TRAIL_SHIFT) | + (mipi_config->ths_exit << + GLK_MIPI_CLK_LANE_HS_EXIT_SHIFT)); + I915_WRITE(MIPI_CLK_LANE_TIMING(port), val); + } + /* the bw essential for transmitting 16 long packets containing * 252 bytes meant for dcs write memory command is programmed in * this register in terms of byte clocks. based on dsi transfer