| Message ID | 1444670703-644-3-git-send-email-uma.shankar@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Oct 12, 2015 at 10:55:02PM +0530, Uma Shankar wrote: > For BXT DSI, vtotal, vactive, hactive registers are different. > Making changes to intel_crtc_mode_get() and get_pipe_timings(), > to read the correct registers for BXT DSI. > > Signed-off-by: Uma Shankar <uma.shankar@intel.com> > Signed-off-by: Vandana Kannan <vandana.kannan@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 48 +++++++++++++++++++++++++++++++--- > 1 file changed, 45 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 75c60b8..2717823 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -7708,6 +7708,7 @@ static void intel_get_pipe_timings(struct intel_crtc *crtc, > struct drm_device *dev = crtc->base.dev; > struct drm_i915_private *dev_priv = dev->dev_private; > enum transcoder cpu_transcoder = pipe_config->cpu_transcoder; > + bool is_dsi = intel_pipe_has_type(crtc, INTEL_OUTPUT_DSI); > uint32_t tmp; > > tmp = I915_READ(HTOTAL(cpu_transcoder)); > @@ -7736,6 +7737,26 @@ static void intel_get_pipe_timings(struct intel_crtc *crtc, > pipe_config->base.adjusted_mode.crtc_vblank_end += 1; > } > > + if (IS_BROXTON(dev) && is_dsi) { > + struct intel_encoder *encoder; > + > + for_each_encoder_on_crtc(dev, &crtc->base, encoder) { > + struct intel_dsi *intel_dsi = > + enc_to_intel_dsi(&encoder->base); > + enum port port; > + > + for_each_dsi_port(port, intel_dsi->ports) { > + pipe_config->base.adjusted_mode.crtc_hdisplay = > + I915_READ(BXT_MIPI_TRANS_HACTIVE(port)); > + pipe_config->base.adjusted_mode.crtc_vdisplay = > + I915_READ(BXT_MIPI_TRANS_VACTIVE(port)); > + pipe_config->base.adjusted_mode.crtc_vtotal = > + I915_READ(BXT_MIPI_TRANS_VTOTAL(port)); > + } > + } > + > + } I think I already asked this earlier when this patch was posted; Don't the normal pipe timing registers contain the same values? And if so, why would you need to read them out from the DSI speciific registers? > + > tmp = I915_READ(PIPESRC(crtc->pipe)); > pipe_config->pipe_src_h = (tmp & 0xffff) + 1; > pipe_config->pipe_src_w = ((tmp >> 16) & 0xffff) + 1; > @@ -10664,6 +10685,7 @@ struct drm_display_mode *intel_crtc_mode_get(struct drm_device *dev, > int vtot = I915_READ(VTOTAL(cpu_transcoder)); > int vsync = I915_READ(VSYNC(cpu_transcoder)); > enum pipe pipe = intel_crtc->pipe; > + bool is_dsi = intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DSI); > > mode = kzalloc(sizeof(*mode), GFP_KERNEL); > if (!mode) > @@ -10684,15 +10706,35 @@ struct drm_display_mode *intel_crtc_mode_get(struct drm_device *dev, > i9xx_crtc_clock_get(intel_crtc, &pipe_config); > > mode->clock = pipe_config.port_clock / pipe_config.pixel_multiplier; > - mode->hdisplay = (htot & 0xffff) + 1; > mode->htotal = ((htot & 0xffff0000) >> 16) + 1; > mode->hsync_start = (hsync & 0xffff) + 1; > mode->hsync_end = ((hsync & 0xffff0000) >> 16) + 1; > - mode->vdisplay = (vtot & 0xffff) + 1; > - mode->vtotal = ((vtot & 0xffff0000) >> 16) + 1; > mode->vsync_start = (vsync & 0xffff) + 1; > mode->vsync_end = ((vsync & 0xffff0000) >> 16) + 1; > > + if (IS_BROXTON(dev) && is_dsi) { > + struct intel_encoder *encoder; > + > + for_each_encoder_on_crtc(dev, &intel_crtc->base, encoder) { > + struct intel_dsi *intel_dsi = > + enc_to_intel_dsi(&encoder->base); > + enum port port; > + > + for_each_dsi_port(port, intel_dsi->ports) { > + mode->vtotal = > + I915_READ(BXT_MIPI_TRANS_VTOTAL(port)); > + mode->hdisplay = > + I915_READ(BXT_MIPI_TRANS_HACTIVE(port)); > + mode->vdisplay = > + I915_READ(BXT_MIPI_TRANS_VACTIVE(port)); > + } > + } > + } else { > + mode->vtotal = ((vtot & 0xffff0000) >> 16) + 1; > + mode->hdisplay = (htot & 0xffff) + 1; > + mode->vdisplay = (vtot & 0xffff) + 1; > + } > + > drm_mode_set_name(mode); > > return mode; > -- > 1.7.9.5 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>-----Original Message----- >From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] >Sent: Monday, October 12, 2015 10:54 PM >To: Shankar, Uma >Cc: intel-gfx@lists.freedesktop.org; Kumar, Shobhit >Subject: Re: [Intel-gfx] [BXT DSI timing fixes v1 2/3] drm/i915/bxt: Get pipe >timing for BXT DSI > >On Mon, Oct 12, 2015 at 10:55:02PM +0530, Uma Shankar wrote: >> For BXT DSI, vtotal, vactive, hactive registers are different. >> Making changes to intel_crtc_mode_get() and get_pipe_timings(), to >> read the correct registers for BXT DSI. >> >> Signed-off-by: Uma Shankar <uma.shankar@intel.com> >> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com> >> --- >> drivers/gpu/drm/i915/intel_display.c | 48 >+++++++++++++++++++++++++++++++--- >> 1 file changed, 45 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_display.c >> b/drivers/gpu/drm/i915/intel_display.c >> index 75c60b8..2717823 100644 >> --- a/drivers/gpu/drm/i915/intel_display.c >> +++ b/drivers/gpu/drm/i915/intel_display.c >> @@ -7708,6 +7708,7 @@ static void intel_get_pipe_timings(struct intel_crtc >*crtc, >> struct drm_device *dev = crtc->base.dev; >> struct drm_i915_private *dev_priv = dev->dev_private; >> enum transcoder cpu_transcoder = pipe_config->cpu_transcoder; >> + bool is_dsi = intel_pipe_has_type(crtc, INTEL_OUTPUT_DSI); >> uint32_t tmp; >> >> tmp = I915_READ(HTOTAL(cpu_transcoder)); >> @@ -7736,6 +7737,26 @@ static void intel_get_pipe_timings(struct intel_crtc >*crtc, >> pipe_config->base.adjusted_mode.crtc_vblank_end += 1; >> } >> >> + if (IS_BROXTON(dev) && is_dsi) { >> + struct intel_encoder *encoder; >> + >> + for_each_encoder_on_crtc(dev, &crtc->base, encoder) { >> + struct intel_dsi *intel_dsi = >> + enc_to_intel_dsi(&encoder->base); >> + enum port port; >> + >> + for_each_dsi_port(port, intel_dsi->ports) { >> + pipe_config->base.adjusted_mode.crtc_hdisplay >= >> + > I915_READ(BXT_MIPI_TRANS_HACTIVE(port)); >> + pipe_config->base.adjusted_mode.crtc_vdisplay >= >> + > I915_READ(BXT_MIPI_TRANS_VACTIVE(port)); >> + pipe_config->base.adjusted_mode.crtc_vtotal = >> + > I915_READ(BXT_MIPI_TRANS_VTOTAL(port)); >> + } >> + } >> + >> + } > >I think I already asked this earlier when this patch was posted; Don't the normal >pipe timing registers contain the same values? And if so, why would you need to >read them out from the DSI speciific registers? > Normal pipe timing registers like HTOTAL etc. is not used by MIPI DSI for BXT. Hence getting the data from DSI specific registers for BXT. Separate MIPI transcoder is used for BXT. Regards, Uma Shankar >> + >> tmp = I915_READ(PIPESRC(crtc->pipe)); >> pipe_config->pipe_src_h = (tmp & 0xffff) + 1; >> pipe_config->pipe_src_w = ((tmp >> 16) & 0xffff) + 1; @@ -10664,6 >> +10685,7 @@ struct drm_display_mode *intel_crtc_mode_get(struct >drm_device *dev, >> int vtot = I915_READ(VTOTAL(cpu_transcoder)); >> int vsync = I915_READ(VSYNC(cpu_transcoder)); >> enum pipe pipe = intel_crtc->pipe; >> + bool is_dsi = intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DSI); >> >> mode = kzalloc(sizeof(*mode), GFP_KERNEL); >> if (!mode) >> @@ -10684,15 +10706,35 @@ struct drm_display_mode >*intel_crtc_mode_get(struct drm_device *dev, >> i9xx_crtc_clock_get(intel_crtc, &pipe_config); >> >> mode->clock = pipe_config.port_clock / pipe_config.pixel_multiplier; >> - mode->hdisplay = (htot & 0xffff) + 1; >> mode->htotal = ((htot & 0xffff0000) >> 16) + 1; >> mode->hsync_start = (hsync & 0xffff) + 1; >> mode->hsync_end = ((hsync & 0xffff0000) >> 16) + 1; >> - mode->vdisplay = (vtot & 0xffff) + 1; >> - mode->vtotal = ((vtot & 0xffff0000) >> 16) + 1; >> mode->vsync_start = (vsync & 0xffff) + 1; >> mode->vsync_end = ((vsync & 0xffff0000) >> 16) + 1; >> >> + if (IS_BROXTON(dev) && is_dsi) { >> + struct intel_encoder *encoder; >> + >> + for_each_encoder_on_crtc(dev, &intel_crtc->base, encoder) { >> + struct intel_dsi *intel_dsi = >> + enc_to_intel_dsi(&encoder->base); >> + enum port port; >> + >> + for_each_dsi_port(port, intel_dsi->ports) { >> + mode->vtotal = >> + > I915_READ(BXT_MIPI_TRANS_VTOTAL(port)); >> + mode->hdisplay = >> + > I915_READ(BXT_MIPI_TRANS_HACTIVE(port)); >> + mode->vdisplay = >> + > I915_READ(BXT_MIPI_TRANS_VACTIVE(port)); >> + } >> + } >> + } else { >> + mode->vtotal = ((vtot & 0xffff0000) >> 16) + 1; >> + mode->hdisplay = (htot & 0xffff) + 1; >> + mode->vdisplay = (vtot & 0xffff) + 1; >> + } >> + >> drm_mode_set_name(mode); >> >> return mode; >> -- >> 1.7.9.5 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > >-- >Ville Syrjälä >Intel OTC
On Tue, Oct 13, 2015 at 11:03:27AM +0000, Shankar, Uma wrote: > > > >-----Original Message----- > >From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] > >Sent: Monday, October 12, 2015 10:54 PM > >To: Shankar, Uma > >Cc: intel-gfx@lists.freedesktop.org; Kumar, Shobhit > >Subject: Re: [Intel-gfx] [BXT DSI timing fixes v1 2/3] drm/i915/bxt: Get pipe > >timing for BXT DSI > > > >On Mon, Oct 12, 2015 at 10:55:02PM +0530, Uma Shankar wrote: > >> For BXT DSI, vtotal, vactive, hactive registers are different. > >> Making changes to intel_crtc_mode_get() and get_pipe_timings(), to > >> read the correct registers for BXT DSI. > >> > >> Signed-off-by: Uma Shankar <uma.shankar@intel.com> > >> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com> > >> --- > >> drivers/gpu/drm/i915/intel_display.c | 48 > >+++++++++++++++++++++++++++++++--- > >> 1 file changed, 45 insertions(+), 3 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/intel_display.c > >> b/drivers/gpu/drm/i915/intel_display.c > >> index 75c60b8..2717823 100644 > >> --- a/drivers/gpu/drm/i915/intel_display.c > >> +++ b/drivers/gpu/drm/i915/intel_display.c > >> @@ -7708,6 +7708,7 @@ static void intel_get_pipe_timings(struct intel_crtc > >*crtc, > >> struct drm_device *dev = crtc->base.dev; > >> struct drm_i915_private *dev_priv = dev->dev_private; > >> enum transcoder cpu_transcoder = pipe_config->cpu_transcoder; > >> + bool is_dsi = intel_pipe_has_type(crtc, INTEL_OUTPUT_DSI); > >> uint32_t tmp; > >> > >> tmp = I915_READ(HTOTAL(cpu_transcoder)); > >> @@ -7736,6 +7737,26 @@ static void intel_get_pipe_timings(struct intel_crtc > >*crtc, > >> pipe_config->base.adjusted_mode.crtc_vblank_end += 1; > >> } > >> > >> + if (IS_BROXTON(dev) && is_dsi) { > >> + struct intel_encoder *encoder; > >> + > >> + for_each_encoder_on_crtc(dev, &crtc->base, encoder) { > >> + struct intel_dsi *intel_dsi = > >> + enc_to_intel_dsi(&encoder->base); > >> + enum port port; > >> + > >> + for_each_dsi_port(port, intel_dsi->ports) { > >> + pipe_config->base.adjusted_mode.crtc_hdisplay > >= > >> + > > I915_READ(BXT_MIPI_TRANS_HACTIVE(port)); > >> + pipe_config->base.adjusted_mode.crtc_vdisplay > >= > >> + > > I915_READ(BXT_MIPI_TRANS_VACTIVE(port)); > >> + pipe_config->base.adjusted_mode.crtc_vtotal = > >> + > > I915_READ(BXT_MIPI_TRANS_VTOTAL(port)); > >> + } > >> + } > >> + > >> + } > > > >I think I already asked this earlier when this patch was posted; Don't the normal > >pipe timing registers contain the same values? And if so, why would you need to > >read them out from the DSI speciific registers? > > > > Normal pipe timing registers like HTOTAL etc. is not used by MIPI DSI for BXT. Hence getting the data from > DSI specific registers for BXT. Separate MIPI transcoder is used for BXT. So calling intel_set_pipe_timings() is pointless (apart from PIPESRC)? If that's the case then it seems the readback should also read all of it from the DSI registers instead of reading some from the pipe timings and some from DSI registers. > > Regards, > Uma Shankar > > >> + > >> tmp = I915_READ(PIPESRC(crtc->pipe)); > >> pipe_config->pipe_src_h = (tmp & 0xffff) + 1; > >> pipe_config->pipe_src_w = ((tmp >> 16) & 0xffff) + 1; @@ -10664,6 > >> +10685,7 @@ struct drm_display_mode *intel_crtc_mode_get(struct > >drm_device *dev, > >> int vtot = I915_READ(VTOTAL(cpu_transcoder)); > >> int vsync = I915_READ(VSYNC(cpu_transcoder)); > >> enum pipe pipe = intel_crtc->pipe; > >> + bool is_dsi = intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DSI); > >> > >> mode = kzalloc(sizeof(*mode), GFP_KERNEL); > >> if (!mode) > >> @@ -10684,15 +10706,35 @@ struct drm_display_mode > >*intel_crtc_mode_get(struct drm_device *dev, > >> i9xx_crtc_clock_get(intel_crtc, &pipe_config); > >> > >> mode->clock = pipe_config.port_clock / pipe_config.pixel_multiplier; > >> - mode->hdisplay = (htot & 0xffff) + 1; > >> mode->htotal = ((htot & 0xffff0000) >> 16) + 1; > >> mode->hsync_start = (hsync & 0xffff) + 1; > >> mode->hsync_end = ((hsync & 0xffff0000) >> 16) + 1; > >> - mode->vdisplay = (vtot & 0xffff) + 1; > >> - mode->vtotal = ((vtot & 0xffff0000) >> 16) + 1; > >> mode->vsync_start = (vsync & 0xffff) + 1; > >> mode->vsync_end = ((vsync & 0xffff0000) >> 16) + 1; > >> > >> + if (IS_BROXTON(dev) && is_dsi) { > >> + struct intel_encoder *encoder; > >> + > >> + for_each_encoder_on_crtc(dev, &intel_crtc->base, encoder) { > >> + struct intel_dsi *intel_dsi = > >> + enc_to_intel_dsi(&encoder->base); > >> + enum port port; > >> + > >> + for_each_dsi_port(port, intel_dsi->ports) { > >> + mode->vtotal = > >> + > > I915_READ(BXT_MIPI_TRANS_VTOTAL(port)); > >> + mode->hdisplay = > >> + > > I915_READ(BXT_MIPI_TRANS_HACTIVE(port)); > >> + mode->vdisplay = > >> + > > I915_READ(BXT_MIPI_TRANS_VACTIVE(port)); > >> + } > >> + } > >> + } else { > >> + mode->vtotal = ((vtot & 0xffff0000) >> 16) + 1; > >> + mode->hdisplay = (htot & 0xffff) + 1; > >> + mode->vdisplay = (vtot & 0xffff) + 1; > >> + } > >> + > >> drm_mode_set_name(mode); > >> > >> return mode; > >> -- > >> 1.7.9.5 > >> > >> _______________________________________________ > >> Intel-gfx mailing list > >> Intel-gfx@lists.freedesktop.org > >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > >-- > >Ville Syrjälä > >Intel OTC
>-----Original Message----- >From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] >Sent: Tuesday, October 13, 2015 4:54 PM >To: Shankar, Uma >Cc: intel-gfx@lists.freedesktop.org; Kumar, Shobhit >Subject: Re: [Intel-gfx] [BXT DSI timing fixes v1 2/3] drm/i915/bxt: Get pipe >timing for BXT DSI > >On Tue, Oct 13, 2015 at 11:03:27AM +0000, Shankar, Uma wrote: >> >> >> >-----Original Message----- >> >From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] >> >Sent: Monday, October 12, 2015 10:54 PM >> >To: Shankar, Uma >> >Cc: intel-gfx@lists.freedesktop.org; Kumar, Shobhit >> >Subject: Re: [Intel-gfx] [BXT DSI timing fixes v1 2/3] drm/i915/bxt: >> >Get pipe timing for BXT DSI >> > >> >On Mon, Oct 12, 2015 at 10:55:02PM +0530, Uma Shankar wrote: >> >> For BXT DSI, vtotal, vactive, hactive registers are different. >> >> Making changes to intel_crtc_mode_get() and get_pipe_timings(), to >> >> read the correct registers for BXT DSI. >> >> >> >> Signed-off-by: Uma Shankar <uma.shankar@intel.com> >> >> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com> >> >> --- >> >> drivers/gpu/drm/i915/intel_display.c | 48 >> >+++++++++++++++++++++++++++++++--- >> >> 1 file changed, 45 insertions(+), 3 deletions(-) >> >> >> >> diff --git a/drivers/gpu/drm/i915/intel_display.c >> >> b/drivers/gpu/drm/i915/intel_display.c >> >> index 75c60b8..2717823 100644 >> >> --- a/drivers/gpu/drm/i915/intel_display.c >> >> +++ b/drivers/gpu/drm/i915/intel_display.c >> >> @@ -7708,6 +7708,7 @@ static void intel_get_pipe_timings(struct >> >> intel_crtc >> >*crtc, >> >> struct drm_device *dev = crtc->base.dev; >> >> struct drm_i915_private *dev_priv = dev->dev_private; >> >> enum transcoder cpu_transcoder = pipe_config->cpu_transcoder; >> >> + bool is_dsi = intel_pipe_has_type(crtc, INTEL_OUTPUT_DSI); >> >> uint32_t tmp; >> >> >> >> tmp = I915_READ(HTOTAL(cpu_transcoder)); >> >> @@ -7736,6 +7737,26 @@ static void intel_get_pipe_timings(struct >> >> intel_crtc >> >*crtc, >> >> pipe_config->base.adjusted_mode.crtc_vblank_end += 1; >> >> } >> >> >> >> + if (IS_BROXTON(dev) && is_dsi) { >> >> + struct intel_encoder *encoder; >> >> + >> >> + for_each_encoder_on_crtc(dev, &crtc->base, encoder) { >> >> + struct intel_dsi *intel_dsi = >> >> + enc_to_intel_dsi(&encoder->base); >> >> + enum port port; >> >> + >> >> + for_each_dsi_port(port, intel_dsi->ports) { >> >> + pipe_config->base.adjusted_mode.crtc_hdisplay >> >= >> >> + >> > I915_READ(BXT_MIPI_TRANS_HACTIVE(port)); >> >> + pipe_config->base.adjusted_mode.crtc_vdisplay >> >= >> >> + >> > I915_READ(BXT_MIPI_TRANS_VACTIVE(port)); >> >> + pipe_config->base.adjusted_mode.crtc_vtotal = >> >> + >> > I915_READ(BXT_MIPI_TRANS_VTOTAL(port)); >> >> + } >> >> + } >> >> + >> >> + } >> > >> >I think I already asked this earlier when this patch was posted; >> >Don't the normal pipe timing registers contain the same values? And >> >if so, why would you need to read them out from the DSI speciific registers? >> > >> >> Normal pipe timing registers like HTOTAL etc. is not used by MIPI DSI >> for BXT. Hence getting the data from DSI specific registers for BXT. Separate >MIPI transcoder is used for BXT. > >So calling intel_set_pipe_timings() is pointless (apart from PIPESRC)? > >If that's the case then it seems the readback should also read all of it from the >DSI registers instead of reading some from the pipe timings and some from DSI >registers. > Yes set_pipe_timings is doing only the pipesrc programming which is useful for DSI, rest is not needed. Readback can be rectified to read all other details as well from DSI specific registers apart from the ones currently being used. >> >> Regards, >> Uma Shankar >> >> >> + >> >> tmp = I915_READ(PIPESRC(crtc->pipe)); >> >> pipe_config->pipe_src_h = (tmp & 0xffff) + 1; >> >> pipe_config->pipe_src_w = ((tmp >> 16) & 0xffff) + 1; @@ -10664,6 >> >> +10685,7 @@ struct drm_display_mode *intel_crtc_mode_get(struct >> >drm_device *dev, >> >> int vtot = I915_READ(VTOTAL(cpu_transcoder)); >> >> int vsync = I915_READ(VSYNC(cpu_transcoder)); >> >> enum pipe pipe = intel_crtc->pipe; >> >> + bool is_dsi = intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DSI); >> >> >> >> mode = kzalloc(sizeof(*mode), GFP_KERNEL); >> >> if (!mode) >> >> @@ -10684,15 +10706,35 @@ struct drm_display_mode >> >*intel_crtc_mode_get(struct drm_device *dev, >> >> i9xx_crtc_clock_get(intel_crtc, &pipe_config); >> >> >> >> mode->clock = pipe_config.port_clock / pipe_config.pixel_multiplier; >> >> - mode->hdisplay = (htot & 0xffff) + 1; >> >> mode->htotal = ((htot & 0xffff0000) >> 16) + 1; >> >> mode->hsync_start = (hsync & 0xffff) + 1; >> >> mode->hsync_end = ((hsync & 0xffff0000) >> 16) + 1; >> >> - mode->vdisplay = (vtot & 0xffff) + 1; >> >> - mode->vtotal = ((vtot & 0xffff0000) >> 16) + 1; >> >> mode->vsync_start = (vsync & 0xffff) + 1; >> >> mode->vsync_end = ((vsync & 0xffff0000) >> 16) + 1; >> >> >> >> + if (IS_BROXTON(dev) && is_dsi) { >> >> + struct intel_encoder *encoder; >> >> + >> >> + for_each_encoder_on_crtc(dev, &intel_crtc->base, encoder) { >> >> + struct intel_dsi *intel_dsi = >> >> + enc_to_intel_dsi(&encoder->base); >> >> + enum port port; >> >> + >> >> + for_each_dsi_port(port, intel_dsi->ports) { >> >> + mode->vtotal = >> >> + >> > I915_READ(BXT_MIPI_TRANS_VTOTAL(port)); >> >> + mode->hdisplay = >> >> + >> > I915_READ(BXT_MIPI_TRANS_HACTIVE(port)); >> >> + mode->vdisplay = >> >> + >> > I915_READ(BXT_MIPI_TRANS_VACTIVE(port)); >> >> + } >> >> + } >> >> + } else { >> >> + mode->vtotal = ((vtot & 0xffff0000) >> 16) + 1; >> >> + mode->hdisplay = (htot & 0xffff) + 1; >> >> + mode->vdisplay = (vtot & 0xffff) + 1; >> >> + } >> >> + >> >> drm_mode_set_name(mode); >> >> >> >> return mode; >> >> -- >> >> 1.7.9.5 >> >> >> >> _______________________________________________ >> >> 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
On Tue, Oct 13, 2015 at 01:24:27PM +0000, Shankar, Uma wrote: > > > >-----Original Message----- > >From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] > >Sent: Tuesday, October 13, 2015 4:54 PM > >To: Shankar, Uma > >Cc: intel-gfx@lists.freedesktop.org; Kumar, Shobhit > >Subject: Re: [Intel-gfx] [BXT DSI timing fixes v1 2/3] drm/i915/bxt: Get pipe > >timing for BXT DSI > > > >On Tue, Oct 13, 2015 at 11:03:27AM +0000, Shankar, Uma wrote: > >> > >> > >> >-----Original Message----- > >> >From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] > >> >Sent: Monday, October 12, 2015 10:54 PM > >> >To: Shankar, Uma > >> >Cc: intel-gfx@lists.freedesktop.org; Kumar, Shobhit > >> >Subject: Re: [Intel-gfx] [BXT DSI timing fixes v1 2/3] drm/i915/bxt: > >> >Get pipe timing for BXT DSI > >> > > >> >On Mon, Oct 12, 2015 at 10:55:02PM +0530, Uma Shankar wrote: > >> >> For BXT DSI, vtotal, vactive, hactive registers are different. > >> >> Making changes to intel_crtc_mode_get() and get_pipe_timings(), to > >> >> read the correct registers for BXT DSI. > >> >> > >> >> Signed-off-by: Uma Shankar <uma.shankar@intel.com> > >> >> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com> > >> >> --- > >> >> drivers/gpu/drm/i915/intel_display.c | 48 > >> >+++++++++++++++++++++++++++++++--- > >> >> 1 file changed, 45 insertions(+), 3 deletions(-) > >> >> > >> >> diff --git a/drivers/gpu/drm/i915/intel_display.c > >> >> b/drivers/gpu/drm/i915/intel_display.c > >> >> index 75c60b8..2717823 100644 > >> >> --- a/drivers/gpu/drm/i915/intel_display.c > >> >> +++ b/drivers/gpu/drm/i915/intel_display.c > >> >> @@ -7708,6 +7708,7 @@ static void intel_get_pipe_timings(struct > >> >> intel_crtc > >> >*crtc, > >> >> struct drm_device *dev = crtc->base.dev; > >> >> struct drm_i915_private *dev_priv = dev->dev_private; > >> >> enum transcoder cpu_transcoder = pipe_config->cpu_transcoder; > >> >> + bool is_dsi = intel_pipe_has_type(crtc, INTEL_OUTPUT_DSI); > >> >> uint32_t tmp; > >> >> > >> >> tmp = I915_READ(HTOTAL(cpu_transcoder)); > >> >> @@ -7736,6 +7737,26 @@ static void intel_get_pipe_timings(struct > >> >> intel_crtc > >> >*crtc, > >> >> pipe_config->base.adjusted_mode.crtc_vblank_end += 1; > >> >> } > >> >> > >> >> + if (IS_BROXTON(dev) && is_dsi) { > >> >> + struct intel_encoder *encoder; > >> >> + > >> >> + for_each_encoder_on_crtc(dev, &crtc->base, encoder) { > >> >> + struct intel_dsi *intel_dsi = > >> >> + enc_to_intel_dsi(&encoder->base); > >> >> + enum port port; > >> >> + > >> >> + for_each_dsi_port(port, intel_dsi->ports) { > >> >> + pipe_config->base.adjusted_mode.crtc_hdisplay > >> >= > >> >> + > >> > I915_READ(BXT_MIPI_TRANS_HACTIVE(port)); > >> >> + pipe_config->base.adjusted_mode.crtc_vdisplay > >> >= > >> >> + > >> > I915_READ(BXT_MIPI_TRANS_VACTIVE(port)); > >> >> + pipe_config->base.adjusted_mode.crtc_vtotal = > >> >> + > >> > I915_READ(BXT_MIPI_TRANS_VTOTAL(port)); > >> >> + } > >> >> + } > >> >> + > >> >> + } > >> > > >> >I think I already asked this earlier when this patch was posted; > >> >Don't the normal pipe timing registers contain the same values? And > >> >if so, why would you need to read them out from the DSI speciific registers? > >> > > >> > >> Normal pipe timing registers like HTOTAL etc. is not used by MIPI DSI > >> for BXT. Hence getting the data from DSI specific registers for BXT. Separate > >MIPI transcoder is used for BXT. > > > >So calling intel_set_pipe_timings() is pointless (apart from PIPESRC)? > > > >If that's the case then it seems the readback should also read all of it from the > >DSI registers instead of reading some from the pipe timings and some from DSI > >registers. > > > > Yes set_pipe_timings is doing only the pipesrc programming which is useful for DSI, rest is not needed. > > Readback can be rectified to read all other details as well from DSI specific registers apart from the ones > currently being used. Also this kind of is_dsi check is a bit a hack and won't really work for fastboot case (intel_pipe_has_type doesn't work at that point yet). Instead you need to push this into a dsi-specific readout function. -Daniel > > >> > >> Regards, > >> Uma Shankar > >> > >> >> + > >> >> tmp = I915_READ(PIPESRC(crtc->pipe)); > >> >> pipe_config->pipe_src_h = (tmp & 0xffff) + 1; > >> >> pipe_config->pipe_src_w = ((tmp >> 16) & 0xffff) + 1; @@ -10664,6 > >> >> +10685,7 @@ struct drm_display_mode *intel_crtc_mode_get(struct > >> >drm_device *dev, > >> >> int vtot = I915_READ(VTOTAL(cpu_transcoder)); > >> >> int vsync = I915_READ(VSYNC(cpu_transcoder)); > >> >> enum pipe pipe = intel_crtc->pipe; > >> >> + bool is_dsi = intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DSI); > >> >> > >> >> mode = kzalloc(sizeof(*mode), GFP_KERNEL); > >> >> if (!mode) > >> >> @@ -10684,15 +10706,35 @@ struct drm_display_mode > >> >*intel_crtc_mode_get(struct drm_device *dev, > >> >> i9xx_crtc_clock_get(intel_crtc, &pipe_config); > >> >> > >> >> mode->clock = pipe_config.port_clock / pipe_config.pixel_multiplier; > >> >> - mode->hdisplay = (htot & 0xffff) + 1; > >> >> mode->htotal = ((htot & 0xffff0000) >> 16) + 1; > >> >> mode->hsync_start = (hsync & 0xffff) + 1; > >> >> mode->hsync_end = ((hsync & 0xffff0000) >> 16) + 1; > >> >> - mode->vdisplay = (vtot & 0xffff) + 1; > >> >> - mode->vtotal = ((vtot & 0xffff0000) >> 16) + 1; > >> >> mode->vsync_start = (vsync & 0xffff) + 1; > >> >> mode->vsync_end = ((vsync & 0xffff0000) >> 16) + 1; > >> >> > >> >> + if (IS_BROXTON(dev) && is_dsi) { > >> >> + struct intel_encoder *encoder; > >> >> + > >> >> + for_each_encoder_on_crtc(dev, &intel_crtc->base, encoder) { > >> >> + struct intel_dsi *intel_dsi = > >> >> + enc_to_intel_dsi(&encoder->base); > >> >> + enum port port; > >> >> + > >> >> + for_each_dsi_port(port, intel_dsi->ports) { > >> >> + mode->vtotal = > >> >> + > >> > I915_READ(BXT_MIPI_TRANS_VTOTAL(port)); > >> >> + mode->hdisplay = > >> >> + > >> > I915_READ(BXT_MIPI_TRANS_HACTIVE(port)); > >> >> + mode->vdisplay = > >> >> + > >> > I915_READ(BXT_MIPI_TRANS_VACTIVE(port)); > >> >> + } > >> >> + } > >> >> + } else { > >> >> + mode->vtotal = ((vtot & 0xffff0000) >> 16) + 1; > >> >> + mode->hdisplay = (htot & 0xffff) + 1; > >> >> + mode->vdisplay = (vtot & 0xffff) + 1; > >> >> + } > >> >> + > >> >> drm_mode_set_name(mode); > >> >> > >> >> return mode; > >> >> -- > >> >> 1.7.9.5 > >> >> > >> >> _______________________________________________ > >> >> 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
On Mon, 2015-10-12 at 22:55 +0530, Uma Shankar wrote: > For BXT DSI, vtotal, vactive, hactive registers are different. > Making changes to intel_crtc_mode_get() and get_pipe_timings(), > to read the correct registers for BXT DSI. > Tested-by: Mika Kahola <mika.kahola@intel.com> > Signed-off-by: Uma Shankar <uma.shankar@intel.com> > Signed-off-by: Vandana Kannan <vandana.kannan@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 48 +++++++++++++++++++++++++++++++--- > 1 file changed, 45 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 75c60b8..2717823 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -7708,6 +7708,7 @@ static void intel_get_pipe_timings(struct intel_crtc *crtc, > struct drm_device *dev = crtc->base.dev; > struct drm_i915_private *dev_priv = dev->dev_private; > enum transcoder cpu_transcoder = pipe_config->cpu_transcoder; > + bool is_dsi = intel_pipe_has_type(crtc, INTEL_OUTPUT_DSI); > uint32_t tmp; > > tmp = I915_READ(HTOTAL(cpu_transcoder)); > @@ -7736,6 +7737,26 @@ static void intel_get_pipe_timings(struct intel_crtc *crtc, > pipe_config->base.adjusted_mode.crtc_vblank_end += 1; > } > > + if (IS_BROXTON(dev) && is_dsi) { > + struct intel_encoder *encoder; > + > + for_each_encoder_on_crtc(dev, &crtc->base, encoder) { > + struct intel_dsi *intel_dsi = > + enc_to_intel_dsi(&encoder->base); > + enum port port; > + > + for_each_dsi_port(port, intel_dsi->ports) { > + pipe_config->base.adjusted_mode.crtc_hdisplay = > + I915_READ(BXT_MIPI_TRANS_HACTIVE(port)); > + pipe_config->base.adjusted_mode.crtc_vdisplay = > + I915_READ(BXT_MIPI_TRANS_VACTIVE(port)); > + pipe_config->base.adjusted_mode.crtc_vtotal = > + I915_READ(BXT_MIPI_TRANS_VTOTAL(port)); > + } > + } > + > + } > + > tmp = I915_READ(PIPESRC(crtc->pipe)); > pipe_config->pipe_src_h = (tmp & 0xffff) + 1; > pipe_config->pipe_src_w = ((tmp >> 16) & 0xffff) + 1; > @@ -10664,6 +10685,7 @@ struct drm_display_mode *intel_crtc_mode_get(struct drm_device *dev, > int vtot = I915_READ(VTOTAL(cpu_transcoder)); > int vsync = I915_READ(VSYNC(cpu_transcoder)); > enum pipe pipe = intel_crtc->pipe; > + bool is_dsi = intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DSI); > > mode = kzalloc(sizeof(*mode), GFP_KERNEL); > if (!mode) > @@ -10684,15 +10706,35 @@ struct drm_display_mode *intel_crtc_mode_get(struct drm_device *dev, > i9xx_crtc_clock_get(intel_crtc, &pipe_config); > > mode->clock = pipe_config.port_clock / pipe_config.pixel_multiplier; > - mode->hdisplay = (htot & 0xffff) + 1; > mode->htotal = ((htot & 0xffff0000) >> 16) + 1; > mode->hsync_start = (hsync & 0xffff) + 1; > mode->hsync_end = ((hsync & 0xffff0000) >> 16) + 1; > - mode->vdisplay = (vtot & 0xffff) + 1; > - mode->vtotal = ((vtot & 0xffff0000) >> 16) + 1; > mode->vsync_start = (vsync & 0xffff) + 1; > mode->vsync_end = ((vsync & 0xffff0000) >> 16) + 1; > > + if (IS_BROXTON(dev) && is_dsi) { > + struct intel_encoder *encoder; > + > + for_each_encoder_on_crtc(dev, &intel_crtc->base, encoder) { > + struct intel_dsi *intel_dsi = > + enc_to_intel_dsi(&encoder->base); > + enum port port; > + > + for_each_dsi_port(port, intel_dsi->ports) { > + mode->vtotal = > + I915_READ(BXT_MIPI_TRANS_VTOTAL(port)); > + mode->hdisplay = > + I915_READ(BXT_MIPI_TRANS_HACTIVE(port)); > + mode->vdisplay = > + I915_READ(BXT_MIPI_TRANS_VACTIVE(port)); > + } > + } > + } else { > + mode->vtotal = ((vtot & 0xffff0000) >> 16) + 1; > + mode->hdisplay = (htot & 0xffff) + 1; > + mode->vdisplay = (vtot & 0xffff) + 1; > + } > + > drm_mode_set_name(mode); > > return mode;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 75c60b8..2717823 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -7708,6 +7708,7 @@ static void intel_get_pipe_timings(struct intel_crtc *crtc, struct drm_device *dev = crtc->base.dev; struct drm_i915_private *dev_priv = dev->dev_private; enum transcoder cpu_transcoder = pipe_config->cpu_transcoder; + bool is_dsi = intel_pipe_has_type(crtc, INTEL_OUTPUT_DSI); uint32_t tmp; tmp = I915_READ(HTOTAL(cpu_transcoder)); @@ -7736,6 +7737,26 @@ static void intel_get_pipe_timings(struct intel_crtc *crtc, pipe_config->base.adjusted_mode.crtc_vblank_end += 1; } + if (IS_BROXTON(dev) && is_dsi) { + struct intel_encoder *encoder; + + for_each_encoder_on_crtc(dev, &crtc->base, encoder) { + struct intel_dsi *intel_dsi = + enc_to_intel_dsi(&encoder->base); + enum port port; + + for_each_dsi_port(port, intel_dsi->ports) { + pipe_config->base.adjusted_mode.crtc_hdisplay = + I915_READ(BXT_MIPI_TRANS_HACTIVE(port)); + pipe_config->base.adjusted_mode.crtc_vdisplay = + I915_READ(BXT_MIPI_TRANS_VACTIVE(port)); + pipe_config->base.adjusted_mode.crtc_vtotal = + I915_READ(BXT_MIPI_TRANS_VTOTAL(port)); + } + } + + } + tmp = I915_READ(PIPESRC(crtc->pipe)); pipe_config->pipe_src_h = (tmp & 0xffff) + 1; pipe_config->pipe_src_w = ((tmp >> 16) & 0xffff) + 1; @@ -10664,6 +10685,7 @@ struct drm_display_mode *intel_crtc_mode_get(struct drm_device *dev, int vtot = I915_READ(VTOTAL(cpu_transcoder)); int vsync = I915_READ(VSYNC(cpu_transcoder)); enum pipe pipe = intel_crtc->pipe; + bool is_dsi = intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DSI); mode = kzalloc(sizeof(*mode), GFP_KERNEL); if (!mode) @@ -10684,15 +10706,35 @@ struct drm_display_mode *intel_crtc_mode_get(struct drm_device *dev, i9xx_crtc_clock_get(intel_crtc, &pipe_config); mode->clock = pipe_config.port_clock / pipe_config.pixel_multiplier; - mode->hdisplay = (htot & 0xffff) + 1; mode->htotal = ((htot & 0xffff0000) >> 16) + 1; mode->hsync_start = (hsync & 0xffff) + 1; mode->hsync_end = ((hsync & 0xffff0000) >> 16) + 1; - mode->vdisplay = (vtot & 0xffff) + 1; - mode->vtotal = ((vtot & 0xffff0000) >> 16) + 1; mode->vsync_start = (vsync & 0xffff) + 1; mode->vsync_end = ((vsync & 0xffff0000) >> 16) + 1; + if (IS_BROXTON(dev) && is_dsi) { + struct intel_encoder *encoder; + + for_each_encoder_on_crtc(dev, &intel_crtc->base, encoder) { + struct intel_dsi *intel_dsi = + enc_to_intel_dsi(&encoder->base); + enum port port; + + for_each_dsi_port(port, intel_dsi->ports) { + mode->vtotal = + I915_READ(BXT_MIPI_TRANS_VTOTAL(port)); + mode->hdisplay = + I915_READ(BXT_MIPI_TRANS_HACTIVE(port)); + mode->vdisplay = + I915_READ(BXT_MIPI_TRANS_VACTIVE(port)); + } + } + } else { + mode->vtotal = ((vtot & 0xffff0000) >> 16) + 1; + mode->hdisplay = (htot & 0xffff) + 1; + mode->vdisplay = (vtot & 0xffff) + 1; + } + drm_mode_set_name(mode); return mode;