[PATCH 1/2] drm/i915: Parse DSI backlight/cabc ports.

[PATCH 1/2] drm/i915: Parse DSI backlight/cabc ports.

[Madhav Chauhan]

This patch parse DSI backlight/cabc ports info from
VBT and save them inside local strucutre. This saved info
can be directly used while initializing DSI for different
platforms instead of parsing for each platform.

Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h   |  2 ++
 drivers/gpu/drm/i915/intel_bios.c | 63 ++++++++++++++++++++++++++++++++-------
 2 files changed, 55 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b7cba89..fc472bb 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1751,6 +1751,8 @@ struct intel_vbt_data {
 		u8 seq_version;
 		u32 size;
 		u8 *data;
+		u16 bl_ports;
+		u16 cabc_ports;
 		const u8 *sequence[MIPI_SEQ_MAX];
 	} dsi;
 
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index 3747d8d..88a72cc 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -730,6 +730,56 @@ parse_psr(struct drm_i915_private *dev_priv, const struct bdb_header *bdb)
 	dev_priv->vbt.psr.tp2_tp3_wakeup_time = psr_table->tp2_tp3_wakeup_time;
 }
 
+static void parse_dsi_backlight_ports(struct drm_i915_private *dev_priv,
+				      u16 version, enum port port)
+{
+	if (dev_priv->vbt.dsi.config->dual_link && version < 197) {
+		/*
+		 * These fields are introduced from the VBT version 197 onwards,
+		 * so making sure that these bits are set zero in the previous
+		 * versions.
+		 */
+		dev_priv->vbt.dsi.config->dl_dcs_cabc_ports = 0;
+		dev_priv->vbt.dsi.config->dl_dcs_backlight_ports = 0;
+		dev_priv->vbt.dsi.bl_ports = 0;
+		dev_priv->vbt.dsi.cabc_ports = 0;
+		return;
+	} else if (dev_priv->vbt.dsi.config->dual_link) {
+		switch (dev_priv->vbt.dsi.config->dl_dcs_backlight_ports) {
+		case DL_DCS_PORT_A:
+			dev_priv->vbt.dsi.bl_ports = BIT(PORT_A);
+			break;
+		case DL_DCS_PORT_C:
+			dev_priv->vbt.dsi.bl_ports = BIT(PORT_C);
+			break;
+		default:
+		case DL_DCS_PORT_A_AND_C:
+			dev_priv->vbt.dsi.bl_ports = BIT(PORT_A) | BIT(PORT_C);
+			break;
+		}
+
+		switch (dev_priv->vbt.dsi.config->dl_dcs_cabc_ports) {
+		case DL_DCS_PORT_A:
+			dev_priv->vbt.dsi.cabc_ports = BIT(PORT_A);
+			break;
+		case DL_DCS_PORT_C:
+			dev_priv->vbt.dsi.cabc_ports = BIT(PORT_C);
+			break;
+		default:
+		case DL_DCS_PORT_A_AND_C:
+			dev_priv->vbt.dsi.cabc_ports =
+						BIT(PORT_A) | BIT(PORT_C);
+			break;
+		}
+	} else {
+		dev_priv->vbt.dsi.bl_ports = BIT(port);
+		dev_priv->vbt.dsi.cabc_ports = BIT(port);
+	}
+
+	if (!dev_priv->vbt.dsi.config->cabc_supported)
+		dev_priv->vbt.dsi.cabc_ports = 0;
+}
+
 static void
 parse_mipi_config(struct drm_i915_private *dev_priv,
 		  const struct bdb_header *bdb)
@@ -738,9 +788,10 @@ parse_mipi_config(struct drm_i915_private *dev_priv,
 	const struct mipi_config *config;
 	const struct mipi_pps_data *pps;
 	int panel_type = dev_priv->vbt.panel_type;
+	enum port port;
 
 	/* parse MIPI blocks only if LFP type is MIPI */
-	if (!intel_bios_is_dsi_present(dev_priv, NULL))
+	if (!intel_bios_is_dsi_present(dev_priv, &port))
 		return;
 
 	/* Initialize this to undefined indicating no generic MIPI support */
@@ -781,15 +832,7 @@ parse_mipi_config(struct drm_i915_private *dev_priv,
 		return;
 	}
 
-	/*
-	 * These fields are introduced from the VBT version 197 onwards,
-	 * so making sure that these bits are set zero in the previous
-	 * versions.
-	 */
-	if (dev_priv->vbt.dsi.config->dual_link && bdb->version < 197) {
-		dev_priv->vbt.dsi.config->dl_dcs_cabc_ports = 0;
-		dev_priv->vbt.dsi.config->dl_dcs_backlight_ports = 0;
-	}
+	parse_dsi_backlight_ports(dev_priv, bdb->version, port);
 
 	/* We have mandatory mipi config blocks. Initialize as generic panel */
 	dev_priv->vbt.dsi.panel_id = MIPI_DSI_GENERIC_PANEL_ID;
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Parse DSI backlight/cabc ports.

[Jani Nikula]

On Tue, 03 Oct 2017, Madhav Chauhan <madhav.chauhan@intel.com> wrote:
> This patch parse DSI backlight/cabc ports info from
> VBT and save them inside local strucutre. This saved info
> can be directly used while initializing DSI for different
> platforms instead of parsing for each platform.
>
> Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h   |  2 ++
>  drivers/gpu/drm/i915/intel_bios.c | 63 ++++++++++++++++++++++++++++++++-------
>  2 files changed, 55 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index b7cba89..fc472bb 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1751,6 +1751,8 @@ struct intel_vbt_data {
>  		u8 seq_version;
>  		u32 size;
>  		u8 *data;
> +		u16 bl_ports;
> +		u16 cabc_ports;

This is right in the middle of the sequence data. Please move up
e.g. between pps and seq_version.

>  		const u8 *sequence[MIPI_SEQ_MAX];
>  	} dsi;
>  
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index 3747d8d..88a72cc 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -730,6 +730,56 @@ parse_psr(struct drm_i915_private *dev_priv, const struct bdb_header *bdb)
>  	dev_priv->vbt.psr.tp2_tp3_wakeup_time = psr_table->tp2_tp3_wakeup_time;
>  }
>  
> +static void parse_dsi_backlight_ports(struct drm_i915_private *dev_priv,
> +				      u16 version, enum port port)
> +{
> +	if (dev_priv->vbt.dsi.config->dual_link && version < 197) {
> +		/*
> +		 * These fields are introduced from the VBT version 197 onwards,
> +		 * so making sure that these bits are set zero in the previous
> +		 * versions.
> +		 */
> +		dev_priv->vbt.dsi.config->dl_dcs_cabc_ports = 0;
> +		dev_priv->vbt.dsi.config->dl_dcs_backlight_ports = 0;

You could remove this in patch 2. Nobody should be looking at it
anymore.

> +		dev_priv->vbt.dsi.bl_ports = 0;
> +		dev_priv->vbt.dsi.cabc_ports = 0;

This you don't have to do anyway, it's all zeros by default.

> +		return;
> +	} else if (dev_priv->vbt.dsi.config->dual_link) {
> +		switch (dev_priv->vbt.dsi.config->dl_dcs_backlight_ports) {
> +		case DL_DCS_PORT_A:
> +			dev_priv->vbt.dsi.bl_ports = BIT(PORT_A);
> +			break;
> +		case DL_DCS_PORT_C:
> +			dev_priv->vbt.dsi.bl_ports = BIT(PORT_C);
> +			break;
> +		default:
> +		case DL_DCS_PORT_A_AND_C:
> +			dev_priv->vbt.dsi.bl_ports = BIT(PORT_A) | BIT(PORT_C);
> +			break;
> +		}
> +
> +		switch (dev_priv->vbt.dsi.config->dl_dcs_cabc_ports) {
> +		case DL_DCS_PORT_A:
> +			dev_priv->vbt.dsi.cabc_ports = BIT(PORT_A);
> +			break;
> +		case DL_DCS_PORT_C:
> +			dev_priv->vbt.dsi.cabc_ports = BIT(PORT_C);
> +			break;
> +		default:
> +		case DL_DCS_PORT_A_AND_C:
> +			dev_priv->vbt.dsi.cabc_ports =
> +						BIT(PORT_A) | BIT(PORT_C);
> +			break;
> +		}
> +	} else {
> +		dev_priv->vbt.dsi.bl_ports = BIT(port);
> +		dev_priv->vbt.dsi.cabc_ports = BIT(port);
> +	}
> +
> +	if (!dev_priv->vbt.dsi.config->cabc_supported)
> +		dev_priv->vbt.dsi.cabc_ports = 0;

Would seem reasonable to not initalize it in the first place if it's not
supported. If you do a series of early returns starting with the bdb
version check, then !dual_link, then !cabc_supported, it might be
easiest.

> +}
> +
>  static void
>  parse_mipi_config(struct drm_i915_private *dev_priv,
>  		  const struct bdb_header *bdb)
> @@ -738,9 +788,10 @@ parse_mipi_config(struct drm_i915_private *dev_priv,
>  	const struct mipi_config *config;
>  	const struct mipi_pps_data *pps;
>  	int panel_type = dev_priv->vbt.panel_type;
> +	enum port port;
>  
>  	/* parse MIPI blocks only if LFP type is MIPI */
> -	if (!intel_bios_is_dsi_present(dev_priv, NULL))
> +	if (!intel_bios_is_dsi_present(dev_priv, &port))
>  		return;
>  
>  	/* Initialize this to undefined indicating no generic MIPI support */
> @@ -781,15 +832,7 @@ parse_mipi_config(struct drm_i915_private *dev_priv,
>  		return;
>  	}
>  
> -	/*
> -	 * These fields are introduced from the VBT version 197 onwards,
> -	 * so making sure that these bits are set zero in the previous
> -	 * versions.
> -	 */
> -	if (dev_priv->vbt.dsi.config->dual_link && bdb->version < 197) {
> -		dev_priv->vbt.dsi.config->dl_dcs_cabc_ports = 0;
> -		dev_priv->vbt.dsi.config->dl_dcs_backlight_ports = 0;
> -	}
> +	parse_dsi_backlight_ports(dev_priv, bdb->version, port);
>  
>  	/* We have mandatory mipi config blocks. Initialize as generic panel */
>  	dev_priv->vbt.dsi.panel_id = MIPI_DSI_GENERIC_PANEL_ID;

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Parse DSI backlight/cabc ports.

[Chauhan, Madhav]

> -----Original Message-----
> From: Nikula, Jani
> Sent: Tuesday, October 10, 2017 12:47 PM
> To: Chauhan, Madhav <madhav.chauhan@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: Hiremath, Shashidhar <shashidhar.hiremath@intel.com>; Shankar, Uma
> <uma.shankar@intel.com>; Chauhan, Madhav
> <madhav.chauhan@intel.com>
> Subject: Re: [PATCH 1/2] drm/i915: Parse DSI backlight/cabc ports.
> 
> On Tue, 03 Oct 2017, Madhav Chauhan <madhav.chauhan@intel.com>
> wrote:
> > This patch parse DSI backlight/cabc ports info from VBT and save them
> > inside local strucutre. This saved info can be directly used while
> > initializing DSI for different platforms instead of parsing for each
> > platform.
> >
> > Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h   |  2 ++
> >  drivers/gpu/drm/i915/intel_bios.c | 63
> > ++++++++++++++++++++++++++++++++-------
> >  2 files changed, 55 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h index b7cba89..fc472bb 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1751,6 +1751,8 @@ struct intel_vbt_data {
> >  		u8 seq_version;
> >  		u32 size;
> >  		u8 *data;
> > +		u16 bl_ports;
> > +		u16 cabc_ports;
> 
> This is right in the middle of the sequence data. Please move up e.g. between
> pps and seq_version.

Ok.

> 
> >  		const u8 *sequence[MIPI_SEQ_MAX];
> >  	} dsi;
> >
> > diff --git a/drivers/gpu/drm/i915/intel_bios.c
> > b/drivers/gpu/drm/i915/intel_bios.c
> > index 3747d8d..88a72cc 100644
> > --- a/drivers/gpu/drm/i915/intel_bios.c
> > +++ b/drivers/gpu/drm/i915/intel_bios.c
> > @@ -730,6 +730,56 @@ parse_psr(struct drm_i915_private *dev_priv,
> const struct bdb_header *bdb)
> >  	dev_priv->vbt.psr.tp2_tp3_wakeup_time =
> > psr_table->tp2_tp3_wakeup_time;  }
> >
> > +static void parse_dsi_backlight_ports(struct drm_i915_private *dev_priv,
> > +				      u16 version, enum port port) {
> > +	if (dev_priv->vbt.dsi.config->dual_link && version < 197) {
> > +		/*
> > +		 * These fields are introduced from the VBT version 197
> onwards,
> > +		 * so making sure that these bits are set zero in the previous
> > +		 * versions.
> > +		 */
> > +		dev_priv->vbt.dsi.config->dl_dcs_cabc_ports = 0;
> > +		dev_priv->vbt.dsi.config->dl_dcs_backlight_ports = 0;
> 
> You could remove this in patch 2. Nobody should be looking at it anymore.

Patch 2 means?? Next version of this patch or patch 2 of this series.
Patch 2 of this series doesn't use these variables. Please clarify.

> 
> > +		dev_priv->vbt.dsi.bl_ports = 0;
> > +		dev_priv->vbt.dsi.cabc_ports = 0;
> 
> This you don't have to do anyway, it's all zeros by default.

Ok.

> 
> > +		return;
> > +	} else if (dev_priv->vbt.dsi.config->dual_link) {
> > +		switch (dev_priv->vbt.dsi.config->dl_dcs_backlight_ports) {
> > +		case DL_DCS_PORT_A:
> > +			dev_priv->vbt.dsi.bl_ports = BIT(PORT_A);
> > +			break;
> > +		case DL_DCS_PORT_C:
> > +			dev_priv->vbt.dsi.bl_ports = BIT(PORT_C);
> > +			break;
> > +		default:
> > +		case DL_DCS_PORT_A_AND_C:
> > +			dev_priv->vbt.dsi.bl_ports = BIT(PORT_A) |
> BIT(PORT_C);
> > +			break;
> > +		}
> > +
> > +		switch (dev_priv->vbt.dsi.config->dl_dcs_cabc_ports) {
> > +		case DL_DCS_PORT_A:
> > +			dev_priv->vbt.dsi.cabc_ports = BIT(PORT_A);
> > +			break;
> > +		case DL_DCS_PORT_C:
> > +			dev_priv->vbt.dsi.cabc_ports = BIT(PORT_C);
> > +			break;
> > +		default:
> > +		case DL_DCS_PORT_A_AND_C:
> > +			dev_priv->vbt.dsi.cabc_ports =
> > +						BIT(PORT_A) | BIT(PORT_C);
> > +			break;
> > +		}
> > +	} else {
> > +		dev_priv->vbt.dsi.bl_ports = BIT(port);
> > +		dev_priv->vbt.dsi.cabc_ports = BIT(port);
> > +	}
> > +
> > +	if (!dev_priv->vbt.dsi.config->cabc_supported)
> > +		dev_priv->vbt.dsi.cabc_ports = 0;
> 
> Would seem reasonable to not initalize it in the first place if it's not
> supported. 

Agree. How about putting a check condition !cabc_supported
before parsing dl_dcs_cabc_ports in switch case??

>If you do a series of early returns starting with the bdb version
> check, then !dual_link, then !cabc_supported, it might be easiest.

But even if for !dual_link, still we need to fill newly added cabc_ports and bl_ports.

Regards,
Madhav

> 
> > +}
> > +
> >  static void
> >  parse_mipi_config(struct drm_i915_private *dev_priv,
> >  		  const struct bdb_header *bdb)
> > @@ -738,9 +788,10 @@ parse_mipi_config(struct drm_i915_private
> *dev_priv,
> >  	const struct mipi_config *config;
> >  	const struct mipi_pps_data *pps;
> >  	int panel_type = dev_priv->vbt.panel_type;
> > +	enum port port;
> >
> >  	/* parse MIPI blocks only if LFP type is MIPI */
> > -	if (!intel_bios_is_dsi_present(dev_priv, NULL))
> > +	if (!intel_bios_is_dsi_present(dev_priv, &port))
> >  		return;
> >
> >  	/* Initialize this to undefined indicating no generic MIPI support
> > */ @@ -781,15 +832,7 @@ parse_mipi_config(struct drm_i915_private
> *dev_priv,
> >  		return;
> >  	}
> >
> > -	/*
> > -	 * These fields are introduced from the VBT version 197 onwards,
> > -	 * so making sure that these bits are set zero in the previous
> > -	 * versions.
> > -	 */
> > -	if (dev_priv->vbt.dsi.config->dual_link && bdb->version < 197) {
> > -		dev_priv->vbt.dsi.config->dl_dcs_cabc_ports = 0;
> > -		dev_priv->vbt.dsi.config->dl_dcs_backlight_ports = 0;
> > -	}
> > +	parse_dsi_backlight_ports(dev_priv, bdb->version, port);
> >
> >  	/* We have mandatory mipi config blocks. Initialize as generic panel
> */
> >  	dev_priv->vbt.dsi.panel_id = MIPI_DSI_GENERIC_PANEL_ID;
> 
> --
> Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Parse DSI backlight/cabc ports.

[Jani Nikula]

On Tue, 10 Oct 2017, "Chauhan, Madhav" <madhav.chauhan@intel.com> wrote:
>> -----Original Message-----
>> From: Nikula, Jani
>> Sent: Tuesday, October 10, 2017 12:47 PM
>> To: Chauhan, Madhav <madhav.chauhan@intel.com>; intel-
>> gfx@lists.freedesktop.org
>> Cc: Hiremath, Shashidhar <shashidhar.hiremath@intel.com>; Shankar, Uma
>> <uma.shankar@intel.com>; Chauhan, Madhav
>> <madhav.chauhan@intel.com>
>> Subject: Re: [PATCH 1/2] drm/i915: Parse DSI backlight/cabc ports.
>> 
>> On Tue, 03 Oct 2017, Madhav Chauhan <madhav.chauhan@intel.com>
>> wrote:
>> > This patch parse DSI backlight/cabc ports info from VBT and save them
>> > inside local strucutre. This saved info can be directly used while
>> > initializing DSI for different platforms instead of parsing for each
>> > platform.
>> >
>> > Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/i915_drv.h   |  2 ++
>> >  drivers/gpu/drm/i915/intel_bios.c | 63
>> > ++++++++++++++++++++++++++++++++-------
>> >  2 files changed, 55 insertions(+), 10 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
>> > b/drivers/gpu/drm/i915/i915_drv.h index b7cba89..fc472bb 100644
>> > --- a/drivers/gpu/drm/i915/i915_drv.h
>> > +++ b/drivers/gpu/drm/i915/i915_drv.h
>> > @@ -1751,6 +1751,8 @@ struct intel_vbt_data {
>> >  		u8 seq_version;
>> >  		u32 size;
>> >  		u8 *data;
>> > +		u16 bl_ports;
>> > +		u16 cabc_ports;
>> 
>> This is right in the middle of the sequence data. Please move up e.g. between
>> pps and seq_version.
>
> Ok.
>
>> 
>> >  		const u8 *sequence[MIPI_SEQ_MAX];
>> >  	} dsi;
>> >
>> > diff --git a/drivers/gpu/drm/i915/intel_bios.c
>> > b/drivers/gpu/drm/i915/intel_bios.c
>> > index 3747d8d..88a72cc 100644
>> > --- a/drivers/gpu/drm/i915/intel_bios.c
>> > +++ b/drivers/gpu/drm/i915/intel_bios.c
>> > @@ -730,6 +730,56 @@ parse_psr(struct drm_i915_private *dev_priv,
>> const struct bdb_header *bdb)
>> >  	dev_priv->vbt.psr.tp2_tp3_wakeup_time =
>> > psr_table->tp2_tp3_wakeup_time;  }
>> >
>> > +static void parse_dsi_backlight_ports(struct drm_i915_private *dev_priv,
>> > +				      u16 version, enum port port) {
>> > +	if (dev_priv->vbt.dsi.config->dual_link && version < 197) {
>> > +		/*
>> > +		 * These fields are introduced from the VBT version 197
>> onwards,
>> > +		 * so making sure that these bits are set zero in the previous
>> > +		 * versions.
>> > +		 */
>> > +		dev_priv->vbt.dsi.config->dl_dcs_cabc_ports = 0;
>> > +		dev_priv->vbt.dsi.config->dl_dcs_backlight_ports = 0;
>> 
>> You could remove this in patch 2. Nobody should be looking at it anymore.
>
> Patch 2 means?? Next version of this patch or patch 2 of this series.
> Patch 2 of this series doesn't use these variables. Please clarify.

After patch 2 of this series, the initialization of these two fields is
unnecessary, so please remove the above lines in patch 2.

>
>> 
>> > +		dev_priv->vbt.dsi.bl_ports = 0;
>> > +		dev_priv->vbt.dsi.cabc_ports = 0;
>> 
>> This you don't have to do anyway, it's all zeros by default.
>
> Ok.
>
>> 
>> > +		return;
>> > +	} else if (dev_priv->vbt.dsi.config->dual_link) {
>> > +		switch (dev_priv->vbt.dsi.config->dl_dcs_backlight_ports) {
>> > +		case DL_DCS_PORT_A:
>> > +			dev_priv->vbt.dsi.bl_ports = BIT(PORT_A);
>> > +			break;
>> > +		case DL_DCS_PORT_C:
>> > +			dev_priv->vbt.dsi.bl_ports = BIT(PORT_C);
>> > +			break;
>> > +		default:
>> > +		case DL_DCS_PORT_A_AND_C:
>> > +			dev_priv->vbt.dsi.bl_ports = BIT(PORT_A) |
>> BIT(PORT_C);
>> > +			break;
>> > +		}
>> > +
>> > +		switch (dev_priv->vbt.dsi.config->dl_dcs_cabc_ports) {
>> > +		case DL_DCS_PORT_A:
>> > +			dev_priv->vbt.dsi.cabc_ports = BIT(PORT_A);
>> > +			break;
>> > +		case DL_DCS_PORT_C:
>> > +			dev_priv->vbt.dsi.cabc_ports = BIT(PORT_C);
>> > +			break;
>> > +		default:
>> > +		case DL_DCS_PORT_A_AND_C:
>> > +			dev_priv->vbt.dsi.cabc_ports =
>> > +						BIT(PORT_A) | BIT(PORT_C);
>> > +			break;
>> > +		}
>> > +	} else {
>> > +		dev_priv->vbt.dsi.bl_ports = BIT(port);
>> > +		dev_priv->vbt.dsi.cabc_ports = BIT(port);
>> > +	}
>> > +
>> > +	if (!dev_priv->vbt.dsi.config->cabc_supported)
>> > +		dev_priv->vbt.dsi.cabc_ports = 0;
>> 
>> Would seem reasonable to not initalize it in the first place if it's not
>> supported. 
>
> Agree. How about putting a check condition !cabc_supported
> before parsing dl_dcs_cabc_ports in switch case??
>
>>If you do a series of early returns starting with the bdb version
>> check, then !dual_link, then !cabc_supported, it might be easiest.
>
> But even if for !dual_link, still we need to fill newly added cabc_ports and bl_ports.
>
> Regards,
> Madhav
>
>> 
>> > +}
>> > +
>> >  static void
>> >  parse_mipi_config(struct drm_i915_private *dev_priv,
>> >  		  const struct bdb_header *bdb)
>> > @@ -738,9 +788,10 @@ parse_mipi_config(struct drm_i915_private
>> *dev_priv,
>> >  	const struct mipi_config *config;
>> >  	const struct mipi_pps_data *pps;
>> >  	int panel_type = dev_priv->vbt.panel_type;
>> > +	enum port port;
>> >
>> >  	/* parse MIPI blocks only if LFP type is MIPI */
>> > -	if (!intel_bios_is_dsi_present(dev_priv, NULL))
>> > +	if (!intel_bios_is_dsi_present(dev_priv, &port))
>> >  		return;
>> >
>> >  	/* Initialize this to undefined indicating no generic MIPI support
>> > */ @@ -781,15 +832,7 @@ parse_mipi_config(struct drm_i915_private
>> *dev_priv,
>> >  		return;
>> >  	}
>> >
>> > -	/*
>> > -	 * These fields are introduced from the VBT version 197 onwards,
>> > -	 * so making sure that these bits are set zero in the previous
>> > -	 * versions.
>> > -	 */
>> > -	if (dev_priv->vbt.dsi.config->dual_link && bdb->version < 197) {
>> > -		dev_priv->vbt.dsi.config->dl_dcs_cabc_ports = 0;
>> > -		dev_priv->vbt.dsi.config->dl_dcs_backlight_ports = 0;
>> > -	}
>> > +	parse_dsi_backlight_ports(dev_priv, bdb->version, port);
>> >
>> >  	/* We have mandatory mipi config blocks. Initialize as generic panel
>> */
>> >  	dev_priv->vbt.dsi.panel_id = MIPI_DSI_GENERIC_PANEL_ID;
>> 
>> --
>> Jani Nikula, Intel Open Source Technology Center

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Parse DSI backlight/cabc ports.

[Chauhan, Madhav]

> -----Original Message-----
> From: Nikula, Jani
> Sent: Tuesday, October 10, 2017 11:27 PM
> To: Chauhan, Madhav <madhav.chauhan@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: Hiremath, Shashidhar <shashidhar.hiremath@intel.com>; Shankar, Uma
> <uma.shankar@intel.com>
> Subject: RE: [PATCH 1/2] drm/i915: Parse DSI backlight/cabc ports.
> 
> On Tue, 10 Oct 2017, "Chauhan, Madhav" <madhav.chauhan@intel.com>
> wrote:
> >> -----Original Message-----
> >> From: Nikula, Jani
> >> Sent: Tuesday, October 10, 2017 12:47 PM
> >> To: Chauhan, Madhav <madhav.chauhan@intel.com>; intel-
> >> gfx@lists.freedesktop.org
> >> Cc: Hiremath, Shashidhar <shashidhar.hiremath@intel.com>; Shankar,
> >> Uma <uma.shankar@intel.com>; Chauhan, Madhav
> >> <madhav.chauhan@intel.com>
> >> Subject: Re: [PATCH 1/2] drm/i915: Parse DSI backlight/cabc ports.
> >>
> >> On Tue, 03 Oct 2017, Madhav Chauhan <madhav.chauhan@intel.com>
> >> wrote:
> >> > This patch parse DSI backlight/cabc ports info from VBT and save
> >> > them inside local strucutre. This saved info can be directly used
> >> > while initializing DSI for different platforms instead of parsing
> >> > for each platform.
> >> >
> >> > Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
> >> > ---
> >> >  drivers/gpu/drm/i915/i915_drv.h   |  2 ++
> >> >  drivers/gpu/drm/i915/intel_bios.c | 63
> >> > ++++++++++++++++++++++++++++++++-------
> >> >  2 files changed, 55 insertions(+), 10 deletions(-)
> >> >
> >> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> >> > b/drivers/gpu/drm/i915/i915_drv.h index b7cba89..fc472bb 100644
> >> > --- a/drivers/gpu/drm/i915/i915_drv.h
> >> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> >> > @@ -1751,6 +1751,8 @@ struct intel_vbt_data {
> >> >  		u8 seq_version;
> >> >  		u32 size;
> >> >  		u8 *data;
> >> > +		u16 bl_ports;
> >> > +		u16 cabc_ports;
> >>
> >> This is right in the middle of the sequence data. Please move up e.g.
> >> between pps and seq_version.
> >
> > Ok.
> >
> >>
> >> >  		const u8 *sequence[MIPI_SEQ_MAX];
> >> >  	} dsi;
> >> >
> >> > diff --git a/drivers/gpu/drm/i915/intel_bios.c
> >> > b/drivers/gpu/drm/i915/intel_bios.c
> >> > index 3747d8d..88a72cc 100644
> >> > --- a/drivers/gpu/drm/i915/intel_bios.c
> >> > +++ b/drivers/gpu/drm/i915/intel_bios.c
> >> > @@ -730,6 +730,56 @@ parse_psr(struct drm_i915_private *dev_priv,
> >> const struct bdb_header *bdb)
> >> >  	dev_priv->vbt.psr.tp2_tp3_wakeup_time =
> >> > psr_table->tp2_tp3_wakeup_time;  }
> >> >
> >> > +static void parse_dsi_backlight_ports(struct drm_i915_private
> *dev_priv,
> >> > +				      u16 version, enum port port) {
> >> > +	if (dev_priv->vbt.dsi.config->dual_link && version < 197) {
> >> > +		/*
> >> > +		 * These fields are introduced from the VBT version 197
> >> onwards,
> >> > +		 * so making sure that these bits are set zero in the previous
> >> > +		 * versions.
> >> > +		 */
> >> > +		dev_priv->vbt.dsi.config->dl_dcs_cabc_ports = 0;
> >> > +		dev_priv->vbt.dsi.config->dl_dcs_backlight_ports = 0;
> >>
> >> You could remove this in patch 2. Nobody should be looking at it
> anymore.
> >
> > Patch 2 means?? Next version of this patch or patch 2 of this series.
> > Patch 2 of this series doesn't use these variables. Please clarify.
> 
> After patch 2 of this series, the initialization of these two fields is
> unnecessary, so please remove the above lines in patch 2.

Ok. Thanks!!

Regards,
Madhav

> 
> >
> >>
> >> > +		dev_priv->vbt.dsi.bl_ports = 0;
> >> > +		dev_priv->vbt.dsi.cabc_ports = 0;
> >>
> >> This you don't have to do anyway, it's all zeros by default.
> >
> > Ok.
> >
> >>
> >> > +		return;
> >> > +	} else if (dev_priv->vbt.dsi.config->dual_link) {
> >> > +		switch (dev_priv->vbt.dsi.config->dl_dcs_backlight_ports) {
> >> > +		case DL_DCS_PORT_A:
> >> > +			dev_priv->vbt.dsi.bl_ports = BIT(PORT_A);
> >> > +			break;
> >> > +		case DL_DCS_PORT_C:
> >> > +			dev_priv->vbt.dsi.bl_ports = BIT(PORT_C);
> >> > +			break;
> >> > +		default:
> >> > +		case DL_DCS_PORT_A_AND_C:
> >> > +			dev_priv->vbt.dsi.bl_ports = BIT(PORT_A) |
> >> BIT(PORT_C);
> >> > +			break;
> >> > +		}
> >> > +
> >> > +		switch (dev_priv->vbt.dsi.config->dl_dcs_cabc_ports) {
> >> > +		case DL_DCS_PORT_A:
> >> > +			dev_priv->vbt.dsi.cabc_ports = BIT(PORT_A);
> >> > +			break;
> >> > +		case DL_DCS_PORT_C:
> >> > +			dev_priv->vbt.dsi.cabc_ports = BIT(PORT_C);
> >> > +			break;
> >> > +		default:
> >> > +		case DL_DCS_PORT_A_AND_C:
> >> > +			dev_priv->vbt.dsi.cabc_ports =
> >> > +						BIT(PORT_A) | BIT(PORT_C);
> >> > +			break;
> >> > +		}
> >> > +	} else {
> >> > +		dev_priv->vbt.dsi.bl_ports = BIT(port);
> >> > +		dev_priv->vbt.dsi.cabc_ports = BIT(port);
> >> > +	}
> >> > +
> >> > +	if (!dev_priv->vbt.dsi.config->cabc_supported)
> >> > +		dev_priv->vbt.dsi.cabc_ports = 0;
> >>
> >> Would seem reasonable to not initalize it in the first place if it's
> >> not supported.
> >
> > Agree. How about putting a check condition !cabc_supported before
> > parsing dl_dcs_cabc_ports in switch case??
> >
> >>If you do a series of early returns starting with the bdb version
> >>check, then !dual_link, then !cabc_supported, it might be easiest.
> >
> > But even if for !dual_link, still we need to fill newly added cabc_ports and
> bl_ports.
> >
> > Regards,
> > Madhav
> >
> >>
> >> > +}
> >> > +
> >> >  static void
> >> >  parse_mipi_config(struct drm_i915_private *dev_priv,
> >> >  		  const struct bdb_header *bdb)
> >> > @@ -738,9 +788,10 @@ parse_mipi_config(struct drm_i915_private
> >> *dev_priv,
> >> >  	const struct mipi_config *config;
> >> >  	const struct mipi_pps_data *pps;
> >> >  	int panel_type = dev_priv->vbt.panel_type;
> >> > +	enum port port;
> >> >
> >> >  	/* parse MIPI blocks only if LFP type is MIPI */
> >> > -	if (!intel_bios_is_dsi_present(dev_priv, NULL))
> >> > +	if (!intel_bios_is_dsi_present(dev_priv, &port))
> >> >  		return;
> >> >
> >> >  	/* Initialize this to undefined indicating no generic MIPI
> >> > support */ @@ -781,15 +832,7 @@ parse_mipi_config(struct
> >> > drm_i915_private
> >> *dev_priv,
> >> >  		return;
> >> >  	}
> >> >
> >> > -	/*
> >> > -	 * These fields are introduced from the VBT version 197 onwards,
> >> > -	 * so making sure that these bits are set zero in the previous
> >> > -	 * versions.
> >> > -	 */
> >> > -	if (dev_priv->vbt.dsi.config->dual_link && bdb->version < 197) {
> >> > -		dev_priv->vbt.dsi.config->dl_dcs_cabc_ports = 0;
> >> > -		dev_priv->vbt.dsi.config->dl_dcs_backlight_ports = 0;
> >> > -	}
> >> > +	parse_dsi_backlight_ports(dev_priv, bdb->version, port);
> >> >
> >> >  	/* We have mandatory mipi config blocks. Initialize as generic
> >> > panel
> >> */
> >> >  	dev_priv->vbt.dsi.panel_id = MIPI_DSI_GENERIC_PANEL_ID;
> >>
> >> --
> >> Jani Nikula, Intel Open Source Technology Center
> 
> --
> Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 1/2] drm/i915: Parse DSI backlight/cabc ports.

[Madhav Chauhan]

This patch parse DSI backlight/cabc ports info from
VBT and save them inside local structure. This saved info
can be directly used while initializing DSI for different
platforms instead of parsing for each platform.

V2: Changes:
    - Typo fix in commit message.
    - Move up newly added port variables (Jani N)
    - Remove redundant initialization (Jani N)
    - Don't parse CABC ports if not supported (Jani N)

Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h   |  2 ++
 drivers/gpu/drm/i915/intel_bios.c | 58 ++++++++++++++++++++++++++++++++-------
 2 files changed, 50 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b7cba89..4fe2e13 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1748,6 +1748,8 @@ struct intel_vbt_data {
 		u16 panel_id;
 		struct mipi_config *config;
 		struct mipi_pps_data *pps;
+		u16 bl_ports;
+		u16 cabc_ports;
 		u8 seq_version;
 		u32 size;
 		u8 *data;
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index 3747d8d..dcfc6fa 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -730,6 +730,51 @@ parse_psr(struct drm_i915_private *dev_priv, const struct bdb_header *bdb)
 	dev_priv->vbt.psr.tp2_tp3_wakeup_time = psr_table->tp2_tp3_wakeup_time;
 }
 
+static void parse_dsi_backlight_ports(struct drm_i915_private *dev_priv,
+				      u16 version, enum port port)
+{
+	if (dev_priv->vbt.dsi.config->dual_link && version < 197) {
+		/*
+		 * These fields are introduced from the VBT version 197 onwards,
+		 * so don't parse in older versions
+		 */
+		return;
+	} else if (dev_priv->vbt.dsi.config->dual_link) {
+		switch (dev_priv->vbt.dsi.config->dl_dcs_backlight_ports) {
+		case DL_DCS_PORT_A:
+			dev_priv->vbt.dsi.bl_ports = BIT(PORT_A);
+			break;
+		case DL_DCS_PORT_C:
+			dev_priv->vbt.dsi.bl_ports = BIT(PORT_C);
+			break;
+		default:
+		case DL_DCS_PORT_A_AND_C:
+			dev_priv->vbt.dsi.bl_ports = BIT(PORT_A) | BIT(PORT_C);
+			break;
+		}
+
+		if (!dev_priv->vbt.dsi.config->cabc_supported)
+			return;
+
+		switch (dev_priv->vbt.dsi.config->dl_dcs_cabc_ports) {
+		case DL_DCS_PORT_A:
+			dev_priv->vbt.dsi.cabc_ports = BIT(PORT_A);
+			break;
+		case DL_DCS_PORT_C:
+			dev_priv->vbt.dsi.cabc_ports = BIT(PORT_C);
+			break;
+		default:
+		case DL_DCS_PORT_A_AND_C:
+			dev_priv->vbt.dsi.cabc_ports =
+						BIT(PORT_A) | BIT(PORT_C);
+			break;
+		}
+	} else {
+		dev_priv->vbt.dsi.bl_ports = BIT(port);
+		dev_priv->vbt.dsi.cabc_ports = BIT(port);
+	}
+}
+
 static void
 parse_mipi_config(struct drm_i915_private *dev_priv,
 		  const struct bdb_header *bdb)
@@ -738,9 +783,10 @@ parse_mipi_config(struct drm_i915_private *dev_priv,
 	const struct mipi_config *config;
 	const struct mipi_pps_data *pps;
 	int panel_type = dev_priv->vbt.panel_type;
+	enum port port;
 
 	/* parse MIPI blocks only if LFP type is MIPI */
-	if (!intel_bios_is_dsi_present(dev_priv, NULL))
+	if (!intel_bios_is_dsi_present(dev_priv, &port))
 		return;
 
 	/* Initialize this to undefined indicating no generic MIPI support */
@@ -781,15 +827,7 @@ parse_mipi_config(struct drm_i915_private *dev_priv,
 		return;
 	}
 
-	/*
-	 * These fields are introduced from the VBT version 197 onwards,
-	 * so making sure that these bits are set zero in the previous
-	 * versions.
-	 */
-	if (dev_priv->vbt.dsi.config->dual_link && bdb->version < 197) {
-		dev_priv->vbt.dsi.config->dl_dcs_cabc_ports = 0;
-		dev_priv->vbt.dsi.config->dl_dcs_backlight_ports = 0;
-	}
+	parse_dsi_backlight_ports(dev_priv, bdb->version, port);
 
 	/* We have mandatory mipi config blocks. Initialize as generic panel */
 	dev_priv->vbt.dsi.panel_id = MIPI_DSI_GENERIC_PANEL_ID;
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/2] drm/i915: Use existing DSI backlight ports info

[Madhav Chauhan]

This patch re-use already parsed DSI backlight/cabc ports
info for saving it inside struct intel_dsi rather than
parsing it at the time of DSI initialization.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
---
 drivers/gpu/drm/i915/intel_dsi.c | 37 ++++---------------------------------
 1 file changed, 4 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 20a7b00..6d21ae8 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -1746,42 +1746,13 @@ void intel_dsi_init(struct drm_i915_private *dev_priv)
 	else
 		intel_encoder->crtc_mask = BIT(PIPE_B);
 
-	if (dev_priv->vbt.dsi.config->dual_link) {
+	if (dev_priv->vbt.dsi.config->dual_link)
 		intel_dsi->ports = BIT(PORT_A) | BIT(PORT_C);
-
-		switch (dev_priv->vbt.dsi.config->dl_dcs_backlight_ports) {
-		case DL_DCS_PORT_A:
-			intel_dsi->dcs_backlight_ports = BIT(PORT_A);
-			break;
-		case DL_DCS_PORT_C:
-			intel_dsi->dcs_backlight_ports = BIT(PORT_C);
-			break;
-		default:
-		case DL_DCS_PORT_A_AND_C:
-			intel_dsi->dcs_backlight_ports = BIT(PORT_A) | BIT(PORT_C);
-			break;
-		}
-
-		switch (dev_priv->vbt.dsi.config->dl_dcs_cabc_ports) {
-		case DL_DCS_PORT_A:
-			intel_dsi->dcs_cabc_ports = BIT(PORT_A);
-			break;
-		case DL_DCS_PORT_C:
-			intel_dsi->dcs_cabc_ports = BIT(PORT_C);
-			break;
-		default:
-		case DL_DCS_PORT_A_AND_C:
-			intel_dsi->dcs_cabc_ports = BIT(PORT_A) | BIT(PORT_C);
-			break;
-		}
-	} else {
+	else
 		intel_dsi->ports = BIT(port);
-		intel_dsi->dcs_backlight_ports = BIT(port);
-		intel_dsi->dcs_cabc_ports = BIT(port);
-	}
 
-	if (!dev_priv->vbt.dsi.config->cabc_supported)
-		intel_dsi->dcs_cabc_ports = 0;
+	intel_dsi->dcs_backlight_ports = dev_priv->vbt.dsi.bl_ports;
+	intel_dsi->dcs_cabc_ports = dev_priv->vbt.dsi.cabc_ports;
 
 	/* Create a DSI host (and a device) for each port. */
 	for_each_dsi_port(port, intel_dsi->ports) {
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Parse DSI backlight/cabc ports.

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915: Parse DSI backlight/cabc ports.
URL   : https://patchwork.freedesktop.org/series/31737/
State : success

== Summary ==

Series 31737v1 series starting with [1/2] drm/i915: Parse DSI backlight/cabc ports.
https://patchwork.freedesktop.org/api/1.0/series/31737/revisions/1/mbox/

Test chamelium:
        Subgroup dp-crc-fast:
                pass       -> FAIL       (fi-kbl-7500u) fdo#102514

fdo#102514 https://bugs.freedesktop.org/show_bug.cgi?id=102514

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:454s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:477s
fi-blb-e6850     total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:391s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:568s
fi-bwr-2160      total:289  pass:183  dwarn:0   dfail:0   fail:0   skip:106 time:283s
fi-bxt-dsi       total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  time:522s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:511s
fi-byt-j1900     total:289  pass:253  dwarn:1   dfail:0   fail:0   skip:35  time:538s
fi-byt-n2820     total:289  pass:249  dwarn:1   dfail:0   fail:0   skip:39  time:524s
fi-cnl-y         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:638s
fi-elk-e7500     total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:433s
fi-glk-1         total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:598s
fi-hsw-4770r     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:443s
fi-ilk-650       total:289  pass:228  dwarn:0   dfail:0   fail:0   skip:61  time:466s
fi-ivb-3520m     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:506s
fi-ivb-3770      total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:478s
fi-kbl-7500u     total:289  pass:263  dwarn:1   dfail:0   fail:1   skip:24  time:494s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:581s
fi-kbl-7567u     total:289  pass:265  dwarn:4   dfail:0   fail:0   skip:20  time:487s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:588s
fi-pnv-d510      total:289  pass:222  dwarn:1   dfail:0   fail:0   skip:66  time:656s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:464s
fi-skl-6700hq    total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:662s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:537s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:528s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:471s
fi-snb-2520m     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:580s
fi-snb-2600      total:289  pass:249  dwarn:0   dfail:0   fail:0   skip:40  time:445s
fi-cfl-s failed to connect after reboot

2dcc40d169e600cfa63887260901c74f943e9bf5 drm-tip: 2017y-10m-11d-08h-54m-27s UTC integration manifest
9940b601ef61 drm/i915: Use existing DSI backlight ports info
5a1a95cf8e1e drm/i915: Parse DSI backlight/cabc ports.

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5989/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Parse DSI backlight/cabc ports.

[Jani Nikula]

On Wed, 11 Oct 2017, Madhav Chauhan <madhav.chauhan@intel.com> wrote:
> This patch parse DSI backlight/cabc ports info from
> VBT and save them inside local structure. This saved info
> can be directly used while initializing DSI for different
> platforms instead of parsing for each platform.
>
> V2: Changes:
>     - Typo fix in commit message.
>     - Move up newly added port variables (Jani N)
>     - Remove redundant initialization (Jani N)
>     - Don't parse CABC ports if not supported (Jani N)
>
> Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h   |  2 ++
>  drivers/gpu/drm/i915/intel_bios.c | 58 ++++++++++++++++++++++++++++++++-------
>  2 files changed, 50 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index b7cba89..4fe2e13 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1748,6 +1748,8 @@ struct intel_vbt_data {
>  		u16 panel_id;
>  		struct mipi_config *config;
>  		struct mipi_pps_data *pps;
> +		u16 bl_ports;
> +		u16 cabc_ports;
>  		u8 seq_version;
>  		u32 size;
>  		u8 *data;
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index 3747d8d..dcfc6fa 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -730,6 +730,51 @@ parse_psr(struct drm_i915_private *dev_priv, const struct bdb_header *bdb)
>  	dev_priv->vbt.psr.tp2_tp3_wakeup_time = psr_table->tp2_tp3_wakeup_time;
>  }
>  
> +static void parse_dsi_backlight_ports(struct drm_i915_private *dev_priv,
> +				      u16 version, enum port port)
> +{
> +	if (dev_priv->vbt.dsi.config->dual_link && version < 197) {
> +		/*
> +		 * These fields are introduced from the VBT version 197 onwards,
> +		 * so don't parse in older versions
> +		 */
> +		return;

You can't remove the initialization of dl_dcs_backlight_ports and
dl_dcs_cabc_ports in patch 1 because the code in intel_dsi.c still uses
them. As I said, you need to remove the initialization in patch 2 where
you stop using them.

I had something like this in mind. Please chat on IRC if you have any
questions.

if (!dual_link || version < 197) {
   initialize bl_ports
   if (cabc)
      initialize cabc_ports
   initialize dl_dcs_backlight_ports and dl_dcs_cabc_ports to 0 (remove
   this in patch 2/2)
   return
}

switch (dl_dcs_backlight_ports) {
   initialize bl_ports for dual link
}

if (!cabc)
   return

switch (dl_dcs_cabc_ports) {
   initialize cabc_ports for dual link
}


> +	} else if (dev_priv->vbt.dsi.config->dual_link) {
> +		switch (dev_priv->vbt.dsi.config->dl_dcs_backlight_ports) {
> +		case DL_DCS_PORT_A:
> +			dev_priv->vbt.dsi.bl_ports = BIT(PORT_A);
> +			break;
> +		case DL_DCS_PORT_C:
> +			dev_priv->vbt.dsi.bl_ports = BIT(PORT_C);
> +			break;
> +		default:
> +		case DL_DCS_PORT_A_AND_C:
> +			dev_priv->vbt.dsi.bl_ports = BIT(PORT_A) | BIT(PORT_C);
> +			break;
> +		}
> +
> +		if (!dev_priv->vbt.dsi.config->cabc_supported)
> +			return;
> +
> +		switch (dev_priv->vbt.dsi.config->dl_dcs_cabc_ports) {
> +		case DL_DCS_PORT_A:
> +			dev_priv->vbt.dsi.cabc_ports = BIT(PORT_A);
> +			break;
> +		case DL_DCS_PORT_C:
> +			dev_priv->vbt.dsi.cabc_ports = BIT(PORT_C);
> +			break;
> +		default:
> +		case DL_DCS_PORT_A_AND_C:
> +			dev_priv->vbt.dsi.cabc_ports =
> +						BIT(PORT_A) | BIT(PORT_C);
> +			break;
> +		}
> +	} else {
> +		dev_priv->vbt.dsi.bl_ports = BIT(port);
> +		dev_priv->vbt.dsi.cabc_ports = BIT(port);
> +	}
> +}
> +
>  static void
>  parse_mipi_config(struct drm_i915_private *dev_priv,
>  		  const struct bdb_header *bdb)
> @@ -738,9 +783,10 @@ parse_mipi_config(struct drm_i915_private *dev_priv,
>  	const struct mipi_config *config;
>  	const struct mipi_pps_data *pps;
>  	int panel_type = dev_priv->vbt.panel_type;
> +	enum port port;
>  
>  	/* parse MIPI blocks only if LFP type is MIPI */
> -	if (!intel_bios_is_dsi_present(dev_priv, NULL))
> +	if (!intel_bios_is_dsi_present(dev_priv, &port))
>  		return;
>  
>  	/* Initialize this to undefined indicating no generic MIPI support */
> @@ -781,15 +827,7 @@ parse_mipi_config(struct drm_i915_private *dev_priv,
>  		return;
>  	}
>  
> -	/*
> -	 * These fields are introduced from the VBT version 197 onwards,
> -	 * so making sure that these bits are set zero in the previous
> -	 * versions.
> -	 */
> -	if (dev_priv->vbt.dsi.config->dual_link && bdb->version < 197) {
> -		dev_priv->vbt.dsi.config->dl_dcs_cabc_ports = 0;
> -		dev_priv->vbt.dsi.config->dl_dcs_backlight_ports = 0;
> -	}
> +	parse_dsi_backlight_ports(dev_priv, bdb->version, port);
>  
>  	/* We have mandatory mipi config blocks. Initialize as generic panel */
>  	dev_priv->vbt.dsi.panel_id = MIPI_DSI_GENERIC_PANEL_ID;

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.IGT: warning for series starting with [1/2] drm/i915: Parse DSI backlight/cabc ports.

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915: Parse DSI backlight/cabc ports.
URL   : https://patchwork.freedesktop.org/series/31737/
State : warning

== Summary ==

Test prime_self_import:
        Subgroup reimport-vs-gem_close-race:
                pass       -> FAIL       (shard-hsw) fdo#102655
Test kms_plane:
        Subgroup plane-panning-bottom-right-pipe-B-planes:
                pass       -> SKIP       (shard-hsw)

fdo#102655 https://bugs.freedesktop.org/show_bug.cgi?id=102655

shard-hsw        total:2552 pass:1433 dwarn:6   dfail:0   fail:9   skip:1104 time:9570s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5989/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 1/2] drm/i915: Parse DSI backlight/cabc ports.

[Madhav Chauhan]

This patch parse DSI backlight/cabc ports info from
VBT and save them inside local structure. This saved info
can be directly used while initializing DSI for different
platforms instead of parsing for each platform.

V2: Changes:
    - Typo fix in commit message.
    - Move up newly added port variables (Jani N)
    - Remove redundant initialization (Jani N)
    - Don't parse CABC ports if not supported (Jani N)
V3: Patch restructure (Suggested by Jani N)

Credits-to: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h   |  2 ++
 drivers/gpu/drm/i915/intel_bios.c | 57 ++++++++++++++++++++++++++++++++-------
 2 files changed, 49 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b7cba89..4fe2e13 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1748,6 +1748,8 @@ struct intel_vbt_data {
 		u16 panel_id;
 		struct mipi_config *config;
 		struct mipi_pps_data *pps;
+		u16 bl_ports;
+		u16 cabc_ports;
 		u8 seq_version;
 		u32 size;
 		u8 *data;
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index 3747d8d..fb30f9a 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -730,6 +730,50 @@ parse_psr(struct drm_i915_private *dev_priv, const struct bdb_header *bdb)
 	dev_priv->vbt.psr.tp2_tp3_wakeup_time = psr_table->tp2_tp3_wakeup_time;
 }
 
+static void parse_dsi_backlight_ports(struct drm_i915_private *dev_priv,
+				      u16 version, enum port port)
+{
+	if (!dev_priv->vbt.dsi.config->dual_link || version < 197) {
+		dev_priv->vbt.dsi.bl_ports = BIT(port);
+		if (dev_priv->vbt.dsi.config->cabc_supported)
+			dev_priv->vbt.dsi.cabc_ports = BIT(port);
+
+		dev_priv->vbt.dsi.config->dl_dcs_cabc_ports = 0;
+		dev_priv->vbt.dsi.config->dl_dcs_backlight_ports = 0;
+		return;
+	}
+
+	switch (dev_priv->vbt.dsi.config->dl_dcs_backlight_ports) {
+	case DL_DCS_PORT_A:
+		dev_priv->vbt.dsi.bl_ports = BIT(PORT_A);
+		break;
+	case DL_DCS_PORT_C:
+		dev_priv->vbt.dsi.bl_ports = BIT(PORT_C);
+		break;
+	default:
+	case DL_DCS_PORT_A_AND_C:
+		dev_priv->vbt.dsi.bl_ports = BIT(PORT_A) | BIT(PORT_C);
+		break;
+	}
+
+	if (!dev_priv->vbt.dsi.config->cabc_supported)
+		return;
+
+	switch (dev_priv->vbt.dsi.config->dl_dcs_cabc_ports) {
+	case DL_DCS_PORT_A:
+		dev_priv->vbt.dsi.cabc_ports = BIT(PORT_A);
+		break;
+	case DL_DCS_PORT_C:
+		dev_priv->vbt.dsi.cabc_ports = BIT(PORT_C);
+		break;
+	default:
+	case DL_DCS_PORT_A_AND_C:
+		dev_priv->vbt.dsi.cabc_ports =
+					BIT(PORT_A) | BIT(PORT_C);
+		break;
+	}
+}
+
 static void
 parse_mipi_config(struct drm_i915_private *dev_priv,
 		  const struct bdb_header *bdb)
@@ -738,9 +782,10 @@ parse_mipi_config(struct drm_i915_private *dev_priv,
 	const struct mipi_config *config;
 	const struct mipi_pps_data *pps;
 	int panel_type = dev_priv->vbt.panel_type;
+	enum port port;
 
 	/* parse MIPI blocks only if LFP type is MIPI */
-	if (!intel_bios_is_dsi_present(dev_priv, NULL))
+	if (!intel_bios_is_dsi_present(dev_priv, &port))
 		return;
 
 	/* Initialize this to undefined indicating no generic MIPI support */
@@ -781,15 +826,7 @@ parse_mipi_config(struct drm_i915_private *dev_priv,
 		return;
 	}
 
-	/*
-	 * These fields are introduced from the VBT version 197 onwards,
-	 * so making sure that these bits are set zero in the previous
-	 * versions.
-	 */
-	if (dev_priv->vbt.dsi.config->dual_link && bdb->version < 197) {
-		dev_priv->vbt.dsi.config->dl_dcs_cabc_ports = 0;
-		dev_priv->vbt.dsi.config->dl_dcs_backlight_ports = 0;
-	}
+	parse_dsi_backlight_ports(dev_priv, bdb->version, port);
 
 	/* We have mandatory mipi config blocks. Initialize as generic panel */
 	dev_priv->vbt.dsi.panel_id = MIPI_DSI_GENERIC_PANEL_ID;
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Parse DSI backlight/cabc ports.

[Jani Nikula]

On Fri, 13 Oct 2017, Madhav Chauhan <madhav.chauhan@intel.com> wrote:
> This patch parse DSI backlight/cabc ports info from
> VBT and save them inside local structure. This saved info
> can be directly used while initializing DSI for different
> platforms instead of parsing for each platform.
>
> V2: Changes:
>     - Typo fix in commit message.
>     - Move up newly added port variables (Jani N)
>     - Remove redundant initialization (Jani N)
>     - Don't parse CABC ports if not supported (Jani N)
> V3: Patch restructure (Suggested by Jani N)
>
> Credits-to: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_drv.h   |  2 ++
>  drivers/gpu/drm/i915/intel_bios.c | 57 ++++++++++++++++++++++++++++++++-------
>  2 files changed, 49 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index b7cba89..4fe2e13 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1748,6 +1748,8 @@ struct intel_vbt_data {
>  		u16 panel_id;
>  		struct mipi_config *config;
>  		struct mipi_pps_data *pps;
> +		u16 bl_ports;
> +		u16 cabc_ports;
>  		u8 seq_version;
>  		u32 size;
>  		u8 *data;
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index 3747d8d..fb30f9a 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -730,6 +730,50 @@ parse_psr(struct drm_i915_private *dev_priv, const struct bdb_header *bdb)
>  	dev_priv->vbt.psr.tp2_tp3_wakeup_time = psr_table->tp2_tp3_wakeup_time;
>  }
>  
> +static void parse_dsi_backlight_ports(struct drm_i915_private *dev_priv,
> +				      u16 version, enum port port)
> +{
> +	if (!dev_priv->vbt.dsi.config->dual_link || version < 197) {
> +		dev_priv->vbt.dsi.bl_ports = BIT(port);
> +		if (dev_priv->vbt.dsi.config->cabc_supported)
> +			dev_priv->vbt.dsi.cabc_ports = BIT(port);
> +
> +		dev_priv->vbt.dsi.config->dl_dcs_cabc_ports = 0;
> +		dev_priv->vbt.dsi.config->dl_dcs_backlight_ports = 0;
> +		return;
> +	}
> +
> +	switch (dev_priv->vbt.dsi.config->dl_dcs_backlight_ports) {
> +	case DL_DCS_PORT_A:
> +		dev_priv->vbt.dsi.bl_ports = BIT(PORT_A);
> +		break;
> +	case DL_DCS_PORT_C:
> +		dev_priv->vbt.dsi.bl_ports = BIT(PORT_C);
> +		break;
> +	default:
> +	case DL_DCS_PORT_A_AND_C:
> +		dev_priv->vbt.dsi.bl_ports = BIT(PORT_A) | BIT(PORT_C);
> +		break;
> +	}
> +
> +	if (!dev_priv->vbt.dsi.config->cabc_supported)
> +		return;
> +
> +	switch (dev_priv->vbt.dsi.config->dl_dcs_cabc_ports) {
> +	case DL_DCS_PORT_A:
> +		dev_priv->vbt.dsi.cabc_ports = BIT(PORT_A);
> +		break;
> +	case DL_DCS_PORT_C:
> +		dev_priv->vbt.dsi.cabc_ports = BIT(PORT_C);
> +		break;
> +	default:
> +	case DL_DCS_PORT_A_AND_C:
> +		dev_priv->vbt.dsi.cabc_ports =
> +					BIT(PORT_A) | BIT(PORT_C);
> +		break;
> +	}
> +}
> +
>  static void
>  parse_mipi_config(struct drm_i915_private *dev_priv,
>  		  const struct bdb_header *bdb)
> @@ -738,9 +782,10 @@ parse_mipi_config(struct drm_i915_private *dev_priv,
>  	const struct mipi_config *config;
>  	const struct mipi_pps_data *pps;
>  	int panel_type = dev_priv->vbt.panel_type;
> +	enum port port;
>  
>  	/* parse MIPI blocks only if LFP type is MIPI */
> -	if (!intel_bios_is_dsi_present(dev_priv, NULL))
> +	if (!intel_bios_is_dsi_present(dev_priv, &port))
>  		return;
>  
>  	/* Initialize this to undefined indicating no generic MIPI support */
> @@ -781,15 +826,7 @@ parse_mipi_config(struct drm_i915_private *dev_priv,
>  		return;
>  	}
>  
> -	/*
> -	 * These fields are introduced from the VBT version 197 onwards,
> -	 * so making sure that these bits are set zero in the previous
> -	 * versions.
> -	 */
> -	if (dev_priv->vbt.dsi.config->dual_link && bdb->version < 197) {
> -		dev_priv->vbt.dsi.config->dl_dcs_cabc_ports = 0;
> -		dev_priv->vbt.dsi.config->dl_dcs_backlight_ports = 0;
> -	}
> +	parse_dsi_backlight_ports(dev_priv, bdb->version, port);
>  
>  	/* We have mandatory mipi config blocks. Initialize as generic panel */
>  	dev_priv->vbt.dsi.panel_id = MIPI_DSI_GENERIC_PANEL_ID;

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx