[1/2] drm/i915: Using the bpp value wrt the pixel format

Submitted by Ramalingam C on Feb. 3, 2016, 12:27 p.m.

Details

Message ID 1454502456-10556-1-git-send-email-ramalingam.c@intel.com
State New
Headers show
Series "Series without cover letter" ( rev: 3 2 ) in Intel GFX

Not browsing as part of any series.

Commit Message

Ramalingam C Feb. 3, 2016, 12:27 p.m.
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(-)

Patch hide | download patch | download mbox

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;
 

Comments

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;