| Message ID | 1454502456-10556-1-git-send-email-ramalingam.c@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 03 Feb 2016, Ramalingam C <ramalingam.c@intel.com> wrote: > From: Deepak M <m.deepak@intel.com> > > The bpp value which is used while calulating the txbyteclkhs values > should be wrt the pixel format value. Currently bpp is coming > from pipe config to calculate txbyteclkhs. Fix it in this patch. > > Signed-off-by: Deepak M <m.deepak@intel.com> > Signed-off-by: Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@intel.com> > Signed-off-by: Ramalingam C <ramalingam.c@intel.com> > --- > drivers/gpu/drm/i915/intel_dsi.c | 5 ++--- > drivers/gpu/drm/i915/intel_dsi.h | 1 + > drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 1 + > 3 files changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c > index 91cef35..aa11293 100644 > --- a/drivers/gpu/drm/i915/intel_dsi.c > +++ b/drivers/gpu/drm/i915/intel_dsi.c > @@ -775,10 +775,9 @@ static void set_dsi_timings(struct drm_encoder *encoder, > { > struct drm_device *dev = encoder->dev; > struct drm_i915_private *dev_priv = dev->dev_private; > - struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc); > struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder); > enum port port; > - unsigned int bpp = intel_crtc->config->pipe_bpp; > + unsigned int bpp = intel_dsi->dsi_bpp; > unsigned int lane_count = intel_dsi->lane_count; > > u16 hactive, hfp, hsync, hbp, vfp, vsync, vbp; > @@ -849,7 +848,7 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder) > struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder); > const struct drm_display_mode *adjusted_mode = &intel_crtc->config->base.adjusted_mode; > enum port port; > - unsigned int bpp = intel_crtc->config->pipe_bpp; > + unsigned int bpp = intel_dsi->dsi_bpp; > u32 val, tmp; > u16 mode_hdisplay; > > diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h > index de7be7f..9bf6fa1d 100644 > --- a/drivers/gpu/drm/i915/intel_dsi.h > +++ b/drivers/gpu/drm/i915/intel_dsi.h > @@ -64,6 +64,7 @@ struct intel_dsi { > > /* video mode pixel format for MIPI_DSI_FUNC_PRG register */ > u32 pixel_format; > + u32 dsi_bpp; Please never add extra state for things that can trivially be derived from existing information. Given the dsi_pixel_format_bpp() in intel_dsi_pll.c, this should always hold, right: intel_dsi->dsi_bpp == dsi_pixel_format_bpp(intel_dsi->pixel_format) Please just make dsi_pixel_format_bpp() available and use it. As a nice bonus, this becomes self-documenting code on why pipe config is not used where the bpp based on pixel format is used. BR, Jani. > > /* video mode format for MIPI_VIDEO_MODE_FORMAT register */ > u32 video_mode_format; > diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c > index 1d43e6f..bf266cb 100644 > --- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c > +++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c > @@ -435,6 +435,7 @@ struct drm_panel *vbt_panel_init(struct intel_dsi *intel_dsi, u16 panel_id) > intel_dsi->bw_timer = mipi_config->dbi_bw_timer; > intel_dsi->video_frmt_cfg_bits = > mipi_config->bta_enabled ? DISABLE_VIDEO_BTA : 0; > + intel_dsi->dsi_bpp = bits_per_pixel; > > pclk = mode->clock;
On Thursday 04 February 2016 06:43 PM, Jani Nikula wrote: > On Wed, 03 Feb 2016, Ramalingam C <ramalingam.c@intel.com> wrote: >> From: Deepak M <m.deepak@intel.com> >> >> The bpp value which is used while calulating the txbyteclkhs values >> should be wrt the pixel format value. Currently bpp is coming >> from pipe config to calculate txbyteclkhs. Fix it in this patch. >> >> Signed-off-by: Deepak M <m.deepak@intel.com> >> Signed-off-by: Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@intel.com> >> Signed-off-by: Ramalingam C <ramalingam.c@intel.com> >> --- >> drivers/gpu/drm/i915/intel_dsi.c | 5 ++--- >> drivers/gpu/drm/i915/intel_dsi.h | 1 + >> drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 1 + >> 3 files changed, 4 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c >> index 91cef35..aa11293 100644 >> --- a/drivers/gpu/drm/i915/intel_dsi.c >> +++ b/drivers/gpu/drm/i915/intel_dsi.c >> @@ -775,10 +775,9 @@ static void set_dsi_timings(struct drm_encoder *encoder, >> { >> struct drm_device *dev = encoder->dev; >> struct drm_i915_private *dev_priv = dev->dev_private; >> - struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc); >> struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder); >> enum port port; >> - unsigned int bpp = intel_crtc->config->pipe_bpp; >> + unsigned int bpp = intel_dsi->dsi_bpp; >> unsigned int lane_count = intel_dsi->lane_count; >> >> u16 hactive, hfp, hsync, hbp, vfp, vsync, vbp; >> @@ -849,7 +848,7 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder) >> struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder); >> const struct drm_display_mode *adjusted_mode = &intel_crtc->config->base.adjusted_mode; >> enum port port; >> - unsigned int bpp = intel_crtc->config->pipe_bpp; >> + unsigned int bpp = intel_dsi->dsi_bpp; >> u32 val, tmp; >> u16 mode_hdisplay; >> >> diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h >> index de7be7f..9bf6fa1d 100644 >> --- a/drivers/gpu/drm/i915/intel_dsi.h >> +++ b/drivers/gpu/drm/i915/intel_dsi.h >> @@ -64,6 +64,7 @@ struct intel_dsi { >> >> /* video mode pixel format for MIPI_DSI_FUNC_PRG register */ >> u32 pixel_format; >> + u32 dsi_bpp; > Please never add extra state for things that can trivially be derived > from existing information. > > Given the dsi_pixel_format_bpp() in intel_dsi_pll.c, this should always > hold, right: > > intel_dsi->dsi_bpp == dsi_pixel_format_bpp(intel_dsi->pixel_format) > > Please just make dsi_pixel_format_bpp() available and use it. As a nice > bonus, this becomes self-documenting code on why pipe config is not used > where the bpp based on pixel format is used. Agreed. Implemented in the next version. Thanks > > BR, > Jani. > > >> >> /* video mode format for MIPI_VIDEO_MODE_FORMAT register */ >> u32 video_mode_format; >> diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c >> index 1d43e6f..bf266cb 100644 >> --- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c >> +++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c >> @@ -435,6 +435,7 @@ struct drm_panel *vbt_panel_init(struct intel_dsi *intel_dsi, u16 panel_id) >> intel_dsi->bw_timer = mipi_config->dbi_bw_timer; >> intel_dsi->video_frmt_cfg_bits = >> mipi_config->bta_enabled ? DISABLE_VIDEO_BTA : 0; >> + intel_dsi->dsi_bpp = bits_per_pixel; >> >> pclk = mode->clock;
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c index 91cef35..aa11293 100644 --- a/drivers/gpu/drm/i915/intel_dsi.c +++ b/drivers/gpu/drm/i915/intel_dsi.c @@ -775,10 +775,9 @@ static void set_dsi_timings(struct drm_encoder *encoder, { struct drm_device *dev = encoder->dev; struct drm_i915_private *dev_priv = dev->dev_private; - struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc); struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder); enum port port; - unsigned int bpp = intel_crtc->config->pipe_bpp; + unsigned int bpp = intel_dsi->dsi_bpp; unsigned int lane_count = intel_dsi->lane_count; u16 hactive, hfp, hsync, hbp, vfp, vsync, vbp; @@ -849,7 +848,7 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder) struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder); const struct drm_display_mode *adjusted_mode = &intel_crtc->config->base.adjusted_mode; enum port port; - unsigned int bpp = intel_crtc->config->pipe_bpp; + unsigned int bpp = intel_dsi->dsi_bpp; u32 val, tmp; u16 mode_hdisplay; diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h index de7be7f..9bf6fa1d 100644 --- a/drivers/gpu/drm/i915/intel_dsi.h +++ b/drivers/gpu/drm/i915/intel_dsi.h @@ -64,6 +64,7 @@ struct intel_dsi { /* video mode pixel format for MIPI_DSI_FUNC_PRG register */ u32 pixel_format; + u32 dsi_bpp; /* video mode format for MIPI_VIDEO_MODE_FORMAT register */ u32 video_mode_format; diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c index 1d43e6f..bf266cb 100644 --- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c +++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c @@ -435,6 +435,7 @@ struct drm_panel *vbt_panel_init(struct intel_dsi *intel_dsi, u16 panel_id) intel_dsi->bw_timer = mipi_config->dbi_bw_timer; intel_dsi->video_frmt_cfg_bits = mipi_config->bta_enabled ? DISABLE_VIDEO_BTA : 0; + intel_dsi->dsi_bpp = bits_per_pixel; pclk = mode->clock;