On Tue, 29 Mar 2016, Ramalingam C <ramalingam.c@xxxxxxxxx> wrote: > At BXT DSI, PIPE registers are inactive. So we can't get the > PIPE's mode parameters from them. The possible option is > retriving them from the PORT registers. > > The required changes are added for BXT in intel_dsi_get_config > (encoder->get_config). > > Signed-off-by: Ramalingam C <ramalingam.c@xxxxxxxxx> > Signed-off-by: Uma Shankar <uma.shankar@xxxxxxxxx> > --- > Previously reviewed at https://patchwork.freedesktop.org/patch/75301/ > > drivers/gpu/drm/i915/i915_reg.h | 1 + > drivers/gpu/drm/i915/intel_display.c | 44 ++++++++++++++ > drivers/gpu/drm/i915/intel_dsi.c | 104 ++++++++++++++++++++++++++++++++++ > 3 files changed, 149 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index c839ce9..da3cdef 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -8210,6 +8210,7 @@ enum skl_disp_power_wells { > #define BXT_PIPE_SELECT_SHIFT 7 > #define BXT_PIPE_SELECT_MASK (7 << 7) > #define BXT_PIPE_SELECT(pipe) ((pipe) << 7) > +#define BXT_PORT_TO_PIPE(ctrl) ((ctrl & BXT_PIPE_SELECT_MASK) >> 7) > > #define _MIPIA_DATA_ADDRESS (dev_priv->mipi_mmio_base + 0xb108) > #define _MIPIC_DATA_ADDRESS (dev_priv->mipi_mmio_base + 0xb908) > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 29aa64b..c0627d6 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -9941,11 +9941,40 @@ static void haswell_get_ddi_port_state(struct intel_crtc *crtc, > } > } > > +struct intel_encoder *bxt_get_dsi_encoder_for_crtc(struct intel_crtc *crtc, > + struct intel_crtc_state *pipe_config) > +{ > + struct drm_device *dev = crtc->base.dev; > + struct drm_i915_private *dev_priv = dev->dev_private; > + struct intel_encoder *intel_encoder; > + struct intel_dsi *intel_dsi; > + enum port port; > + u32 tmp; > + > + for_each_intel_encoder(dev, intel_encoder) { > + if (intel_encoder->type == INTEL_OUTPUT_DSI) { > + intel_dsi = enc_to_intel_dsi(&intel_encoder->base); > + for_each_dsi_port(port, intel_dsi->ports) { > + if (!(I915_READ(BXT_MIPI_PORT_CTRL(port)) & > + DPI_ENABLE)) > + break; > + > + tmp = I915_READ(MIPI_CTRL(port)); > + if ((tmp & BXT_PIPE_SELECT_MASK) == > + BXT_PIPE_SELECT(crtc->pipe)) > + return intel_encoder; > + } > + } > + } > + return NULL; > +} > + > static bool haswell_get_pipe_config(struct intel_crtc *crtc, > struct intel_crtc_state *pipe_config) > { > struct drm_device *dev = crtc->base.dev; > struct drm_i915_private *dev_priv = dev->dev_private; > + struct intel_encoder *intel_encoder, *attached_encoder = NULL; > enum intel_display_power_domain power_domain; > unsigned long power_domain_mask; > bool active; > @@ -9965,6 +9994,21 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc, > WARN_ON(active && pipe_config->has_dsi_encoder); > if (pipe_config->has_dsi_encoder) > active = true; > + > + for_each_encoder_on_crtc(dev, &crtc->base, intel_encoder) > + attached_encoder = intel_encoder; > + > + /* > + * attached_encoder will be NULL, if there is no modeset from > + * the kernel bootup. > + */ > + if (!attached_encoder && pipe_config->has_dsi_encoder) > + attached_encoder = > + bxt_get_dsi_encoder_for_crtc(crtc, pipe_config); > + > + if (attached_encoder && attached_encoder->get_config) > + attached_encoder->get_config(attached_encoder, > + pipe_config); No, you must not add a new call to the encoder->get_config() hook. haswell_get_pipe_config() is called through the dev_priv->display.get_pipe_config() function pointer. This happens in check_crtc_state() and intel_modeset_readout_hw_state(). In both places, encoder->get_config() is called afterwards, if encoder->get_hw_state() returns true for the encoder. The infrastructure is there, you only need to update DSI ->get_config(). > } > > if (!active) > diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c > index 0de74e1..69a801e 100644 > --- a/drivers/gpu/drm/i915/intel_dsi.c > +++ b/drivers/gpu/drm/i915/intel_dsi.c > @@ -740,14 +740,118 @@ out_put_power: > return active; > } > > +/* return pixels equvalent to txbyteclkhs */ > +static u16 pixels_from_txbyteclkhs(u16 clk_hs, int bpp, int lane_count, > + u16 burst_mode_ratio) > +{ > + return DIV_ROUND_UP((clk_hs * lane_count * 8 * 100), > + (bpp * burst_mode_ratio)); > +} > + > +static void bxt_dsi_get_pipe_config(struct intel_encoder *encoder, > + struct intel_crtc_state *pipe_config) > +{ > + struct drm_device *dev = encoder->base.dev; > + struct drm_i915_private *dev_priv = dev->dev_private; > + struct drm_display_mode *adjusted_mode = > + &pipe_config->base.adjusted_mode; > + struct intel_dsi *intel_dsi = > + enc_to_intel_dsi(&encoder->base); > + unsigned int bpp = mipi_dsi_pixel_format_to_bpp(intel_dsi->pixel_format); > + unsigned int lane_count = intel_dsi->lane_count; > + enum port port; > + enum pipe dsi_pipe; > + u16 hactive, hfp, hsync, hbp, vfp, vsync, vbp; > + uint32_t tmp; > + > + for_each_dsi_port(port, intel_dsi->ports) { > + if (!(I915_READ(BXT_MIPI_PORT_CTRL(port)) & DPI_ENABLE)) > + continue; Please use the loop just for figuring out the port to use for reading the registers. After that, you can do the rest at a lower indentation level, and it's clearer the rest only gets done once, for one port. > + > + /* In terms of pixels */ > + adjusted_mode->crtc_hdisplay = > + I915_READ(BXT_MIPI_TRANS_HACTIVE(port)); > + adjusted_mode->crtc_vdisplay = > + I915_READ(BXT_MIPI_TRANS_VACTIVE(port)); > + adjusted_mode->crtc_vtotal = > + I915_READ(BXT_MIPI_TRANS_VTOTAL(port)); > + > + hactive = adjusted_mode->crtc_hdisplay; > + hfp = I915_READ(MIPI_HFP_COUNT(port)); > + > + /* > + * meaningful for video mode non-burst sync pulse mode only, > + * can be zero for non-burst sync events and burst modes > + */ > + hsync = I915_READ(MIPI_HSYNC_PADDING_COUNT(port)); > + hbp = I915_READ(MIPI_HBP_COUNT(port)); > + > + /* horizontal values are in terms of high speed byte clock */ > + hfp = pixels_from_txbyteclkhs(hfp, bpp, lane_count, > + intel_dsi->burst_mode_ratio); > + hsync = pixels_from_txbyteclkhs(hsync, bpp, lane_count, > + intel_dsi->burst_mode_ratio); > + hbp = pixels_from_txbyteclkhs(hbp, bpp, lane_count, > + intel_dsi->burst_mode_ratio); > + > + if (intel_dsi->dual_link) { > + hfp *= 2; > + hsync *= 2; > + hbp *= 2; > + } > + > + /* vertical values are in terms of lines */ > + vfp = I915_READ(MIPI_VFP_COUNT(port)); > + vsync = I915_READ(MIPI_VSYNC_PADDING_COUNT(port)); > + vbp = I915_READ(MIPI_VBP_COUNT(port)); > + > + adjusted_mode->crtc_htotal = hactive + hfp + hsync + hbp; > + adjusted_mode->crtc_hsync_start = > + hfp + adjusted_mode->crtc_hdisplay; > + adjusted_mode->crtc_hsync_end = > + hsync + adjusted_mode->crtc_hsync_start; > + adjusted_mode->crtc_hblank_start = adjusted_mode->crtc_hdisplay; > + adjusted_mode->crtc_hblank_end = adjusted_mode->crtc_htotal; > + > + adjusted_mode->crtc_vsync_start = > + vfp + adjusted_mode->crtc_vdisplay; > + adjusted_mode->crtc_vsync_end = > + vsync + adjusted_mode->crtc_vsync_start; > + adjusted_mode->crtc_vblank_start = adjusted_mode->crtc_vdisplay; > + adjusted_mode->crtc_vblank_end = adjusted_mode->crtc_vtotal; > + > + pipe_config->pipe_bpp = > + mipi_dsi_pixel_format_to_bpp(intel_dsi->pixel_format); I guess this should be read from the register. > + > + dsi_pipe = BXT_PORT_TO_PIPE(I915_READ(MIPI_CTRL(port))); > + if (dsi_pipe > PIPE_C) { > + DRM_ERROR("Invalid PIPE configured\n"); > + break; > + } > + > + tmp = I915_READ(PIPESRC(dsi_pipe)); > + pipe_config->pipe_src_h = (tmp & 0xffff) + 1; > + pipe_config->pipe_src_w = ((tmp >> 16) & 0xffff) + 1; > + > + pipe_config->base.mode.vdisplay = pipe_config->pipe_src_h; > + pipe_config->base.mode.hdisplay = pipe_config->pipe_src_w; This part is already covered by intel_get_pipe_src_size() for DSI. BR, Jani. > + break; > + } > +} > + > + > static void intel_dsi_get_config(struct intel_encoder *encoder, > struct intel_crtc_state *pipe_config) > { > + struct drm_device *dev = encoder->base.dev; > u32 pclk; > DRM_DEBUG_KMS("\n"); > > pipe_config->has_dsi_encoder = true; > > + if (IS_BROXTON(dev)) > + bxt_dsi_get_pipe_config(encoder, pipe_config); > + > /* > * DPLL_MD is not used in case of DSI, reading will get some default value > * set dpll_md = 0 -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
