public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Detect if MIPI panel based on VBT and initialize only if present
@ 2014-05-23 16:09 Shobhit Kumar
  2014-05-27 11:32 ` Damien Lespiau
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Shobhit Kumar @ 2014-05-23 16:09 UTC (permalink / raw)
  To: intel-gfx; +Cc: Jani Nikula, Daniel Vetter

It seems by default the VBT has MIPI configuration block as well. The
Generic driver will assume always MIPI if MIPI configuration block is found.
This is causing probelm when actually there is eDP. Fix this by looking
into general definition block which will have device configurations. From here
we can figure out what is the LFP type and initialize MIPI only if MIPI
is found.

Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |  2 ++
 drivers/gpu/drm/i915/intel_bios.c    | 20 +++++++++++++++++++-
 drivers/gpu/drm/i915/intel_display.c |  4 +++-
 3 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8e78703..5a5225b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1229,7 +1229,9 @@ struct intel_vbt_data {
 	} backlight;
 
 	/* MIPI DSI */
+	int is_mipi;
 	struct {
+		u16 port;
 		u16 panel_id;
 		struct mipi_config *config;
 		struct mipi_pps_data *pps;
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index 6b65096..b825c80 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -1059,6 +1059,20 @@ parse_device_mapping(struct drm_i915_private *dev_priv,
 			/* skip the device block if device type is invalid */
 			continue;
 		}
+
+#define MIPI_PORT_A	0x15
+#define MIPI_PORT_B	0x16
+#define MIPI_PORT_C	0x17
+#define MIPI_PORT_D	0x18
+		if (p_child->common.dvo_port >= MIPI_PORT_A && p_child->common.dvo_port <= MIPI_PORT_D) {
+			/* check the device type and confirm its MIPI */
+			if (p_child->common.device_type & DEVICE_TYPE_MIPI_OUTPUT) {
+				DRM_DEBUG_KMS("Found MIPI as LFP\n");
+				dev_priv->vbt.is_mipi = 1;
+				dev_priv->vbt.dsi.port = p_child->common.dvo_port;
+			}
+		}
+
 		child_dev_ptr = dev_priv->vbt.child_dev + count;
 		count++;
 		memcpy((void *)child_dev_ptr, (void *)p_child,
@@ -1230,7 +1244,11 @@ intel_parse_bios(struct drm_device *dev)
 	parse_device_mapping(dev_priv, bdb);
 	parse_driver_features(dev_priv, bdb);
 	parse_edp(dev_priv, bdb);
-	parse_mipi(dev_priv, bdb);
+
+	/* parse MIPI blocks only if LFP type is MIPI */
+	if (dev_priv->vbt.is_mipi)
+		parse_mipi(dev_priv, bdb);
+
 	parse_ddi_ports(dev_priv, bdb);
 
 	if (bios)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3da73ef..a55fa41 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11151,7 +11151,9 @@ static void intel_setup_outputs(struct drm_device *dev)
 			}
 		}
 
-		intel_dsi_init(dev);
+		/* There is no detection method for MIPI so rely on VBT */
+		if (dev_priv->vbt.is_mipi)
+			intel_dsi_init(dev);
 	} else if (SUPPORTS_DIGITAL_OUTPUTS(dev)) {
 		bool found = false;
 
-- 
1.8.3.2

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH] drm/i915: Detect if MIPI panel based on VBT and initialize only if present
  2014-05-23 16:09 [PATCH] drm/i915: Detect if MIPI panel based on VBT and initialize only if present Shobhit Kumar
@ 2014-05-27 11:32 ` Damien Lespiau
  2014-05-27 12:10   ` Kumar, Shobhit
  2014-05-27 11:40 ` Daniel Vetter
  2014-05-27 12:56 ` [v2] " Shobhit Kumar
  2 siblings, 1 reply; 15+ messages in thread
From: Damien Lespiau @ 2014-05-27 11:32 UTC (permalink / raw)
  To: Shobhit Kumar; +Cc: Jani Nikula, Daniel Vetter, intel-gfx

On Fri, May 23, 2014 at 09:39:28PM +0530, Shobhit Kumar wrote:
> It seems by default the VBT has MIPI configuration block as well. The
> Generic driver will assume always MIPI if MIPI configuration block is found.
> This is causing probelm when actually there is eDP. Fix this by looking
> into general definition block which will have device configurations. From here
> we can figure out what is the LFP type and initialize MIPI only if MIPI
> is found.
> 
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h      |  2 ++
>  drivers/gpu/drm/i915/intel_bios.c    | 20 +++++++++++++++++++-
>  drivers/gpu/drm/i915/intel_display.c |  4 +++-
>  3 files changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 8e78703..5a5225b 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1229,7 +1229,9 @@ struct intel_vbt_data {
>  	} backlight;
>  
>  	/* MIPI DSI */
> +	int is_mipi;

There's a "feature" bitfield near the start of the structure, I think
you should add that field there and maybe rename it to has_mipi.

>  	struct {
> +		u16 port;

We store the port but never use it, is it needed?

>  		u16 panel_id;
>  		struct mipi_config *config;
>  		struct mipi_pps_data *pps;
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index 6b65096..b825c80 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -1059,6 +1059,20 @@ parse_device_mapping(struct drm_i915_private *dev_priv,
>  			/* skip the device block if device type is invalid */
>  			continue;
>  		}
> +
> +#define MIPI_PORT_A	0x15
> +#define MIPI_PORT_B	0x16
> +#define MIPI_PORT_C	0x17
> +#define MIPI_PORT_D	0x18

These should go with the other valid defines for dvo_port in
intel_bios.h and be renamed to match, eg. DEVICE_PORT_MIPI_A.

Also this is not what I have in my copy of the VBT documentation, I have
(in decimal) MIPI PORT_A = 11, .., PORT_D = 14

> +		if (p_child->common.dvo_port >= MIPI_PORT_A && p_child->common.dvo_port <= MIPI_PORT_D) {
> +			/* check the device type and confirm its MIPI */
> +			if (p_child->common.device_type & DEVICE_TYPE_MIPI_OUTPUT) {
> +				DRM_DEBUG_KMS("Found MIPI as LFP\n");
> +				dev_priv->vbt.is_mipi = 1;
> +				dev_priv->vbt.dsi.port = p_child->common.dvo_port;
> +			}
> +		}
> +
>  		child_dev_ptr = dev_priv->vbt.child_dev + count;
>  		count++;
>  		memcpy((void *)child_dev_ptr, (void *)p_child,
> @@ -1230,7 +1244,11 @@ intel_parse_bios(struct drm_device *dev)
>  	parse_device_mapping(dev_priv, bdb);
>  	parse_driver_features(dev_priv, bdb);
>  	parse_edp(dev_priv, bdb);
> -	parse_mipi(dev_priv, bdb);
> +
> +	/* parse MIPI blocks only if LFP type is MIPI */
> +	if (dev_priv->vbt.is_mipi)
> +		parse_mipi(dev_priv, bdb);

Let's put that check inside parse_mipi so we don't break the symetry of
those parse_* calls.

> +
>  	parse_ddi_ports(dev_priv, bdb);
>  
>  	if (bios)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 3da73ef..a55fa41 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11151,7 +11151,9 @@ static void intel_setup_outputs(struct drm_device *dev)
>  			}
>  		}
>  
> -		intel_dsi_init(dev);
> +		/* There is no detection method for MIPI so rely on VBT */
> +		if (dev_priv->vbt.is_mipi)
> +			intel_dsi_init(dev);

Can we have that check inside intel_dsi_init() so we don't have to do it
for every platform with DSI?

>  	} else if (SUPPORTS_DIGITAL_OUTPUTS(dev)) {
>  		bool found = false;
>  
> -- 
> 1.8.3.2
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] drm/i915: Detect if MIPI panel based on VBT and initialize only if present
  2014-05-23 16:09 [PATCH] drm/i915: Detect if MIPI panel based on VBT and initialize only if present Shobhit Kumar
  2014-05-27 11:32 ` Damien Lespiau
@ 2014-05-27 11:40 ` Daniel Vetter
  2014-05-27 11:43   ` Kumar, Shobhit
  2014-05-27 12:56 ` [v2] " Shobhit Kumar
  2 siblings, 1 reply; 15+ messages in thread
From: Daniel Vetter @ 2014-05-27 11:40 UTC (permalink / raw)
  To: Shobhit Kumar; +Cc: Jani Nikula, Daniel Vetter, intel-gfx

On Fri, May 23, 2014 at 09:39:28PM +0530, Shobhit Kumar wrote:
> It seems by default the VBT has MIPI configuration block as well. The
> Generic driver will assume always MIPI if MIPI configuration block is found.
> This is causing probelm when actually there is eDP. Fix this by looking
> into general definition block which will have device configurations. From here
> we can figure out what is the LFP type and initialize MIPI only if MIPI
> is found.
> 
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>

Will the mipi patch I've just merged without this one here break machines?
If so we need to get this one here into shape asap, for otherwise I need
to kick out the mipi patch again. Which we really don't want.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_drv.h      |  2 ++
>  drivers/gpu/drm/i915/intel_bios.c    | 20 +++++++++++++++++++-
>  drivers/gpu/drm/i915/intel_display.c |  4 +++-
>  3 files changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 8e78703..5a5225b 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1229,7 +1229,9 @@ struct intel_vbt_data {
>  	} backlight;
>  
>  	/* MIPI DSI */
> +	int is_mipi;
>  	struct {
> +		u16 port;
>  		u16 panel_id;
>  		struct mipi_config *config;
>  		struct mipi_pps_data *pps;
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index 6b65096..b825c80 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -1059,6 +1059,20 @@ parse_device_mapping(struct drm_i915_private *dev_priv,
>  			/* skip the device block if device type is invalid */
>  			continue;
>  		}
> +
> +#define MIPI_PORT_A	0x15
> +#define MIPI_PORT_B	0x16
> +#define MIPI_PORT_C	0x17
> +#define MIPI_PORT_D	0x18
> +		if (p_child->common.dvo_port >= MIPI_PORT_A && p_child->common.dvo_port <= MIPI_PORT_D) {
> +			/* check the device type and confirm its MIPI */
> +			if (p_child->common.device_type & DEVICE_TYPE_MIPI_OUTPUT) {
> +				DRM_DEBUG_KMS("Found MIPI as LFP\n");
> +				dev_priv->vbt.is_mipi = 1;
> +				dev_priv->vbt.dsi.port = p_child->common.dvo_port;
> +			}
> +		}
> +
>  		child_dev_ptr = dev_priv->vbt.child_dev + count;
>  		count++;
>  		memcpy((void *)child_dev_ptr, (void *)p_child,
> @@ -1230,7 +1244,11 @@ intel_parse_bios(struct drm_device *dev)
>  	parse_device_mapping(dev_priv, bdb);
>  	parse_driver_features(dev_priv, bdb);
>  	parse_edp(dev_priv, bdb);
> -	parse_mipi(dev_priv, bdb);
> +
> +	/* parse MIPI blocks only if LFP type is MIPI */
> +	if (dev_priv->vbt.is_mipi)
> +		parse_mipi(dev_priv, bdb);
> +
>  	parse_ddi_ports(dev_priv, bdb);
>  
>  	if (bios)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 3da73ef..a55fa41 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11151,7 +11151,9 @@ static void intel_setup_outputs(struct drm_device *dev)
>  			}
>  		}
>  
> -		intel_dsi_init(dev);
> +		/* There is no detection method for MIPI so rely on VBT */
> +		if (dev_priv->vbt.is_mipi)
> +			intel_dsi_init(dev);
>  	} else if (SUPPORTS_DIGITAL_OUTPUTS(dev)) {
>  		bool found = false;
>  
> -- 
> 1.8.3.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] drm/i915: Detect if MIPI panel based on VBT and initialize only if present
  2014-05-27 11:40 ` Daniel Vetter
@ 2014-05-27 11:43   ` Kumar, Shobhit
  0 siblings, 0 replies; 15+ messages in thread
From: Kumar, Shobhit @ 2014-05-27 11:43 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: Jani Nikula, Daniel Vetter, intel-gfx

On 5/27/2014 5:10 PM, Daniel Vetter wrote:
> On Fri, May 23, 2014 at 09:39:28PM +0530, Shobhit Kumar wrote:
>> It seems by default the VBT has MIPI configuration block as well. The
>> Generic driver will assume always MIPI if MIPI configuration block is found.
>> This is causing probelm when actually there is eDP. Fix this by looking
>> into general definition block which will have device configurations. From here
>> we can figure out what is the LFP type and initialize MIPI only if MIPI
>> is found.
>>
>> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
>
> Will the mipi patch I've just merged without this one here break machines?
> If so we need to get this one here into shape asap, for otherwise I need
> to kick out the mipi patch again. Which we really don't want.

Yes it will break. I am working on this patch right now

Regards
Shobhit


> -Daniel
>
>> ---
>>   drivers/gpu/drm/i915/i915_drv.h      |  2 ++
>>   drivers/gpu/drm/i915/intel_bios.c    | 20 +++++++++++++++++++-
>>   drivers/gpu/drm/i915/intel_display.c |  4 +++-
>>   3 files changed, 24 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 8e78703..5a5225b 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -1229,7 +1229,9 @@ struct intel_vbt_data {
>>   	} backlight;
>>
>>   	/* MIPI DSI */
>> +	int is_mipi;
>>   	struct {
>> +		u16 port;
>>   		u16 panel_id;
>>   		struct mipi_config *config;
>>   		struct mipi_pps_data *pps;
>> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
>> index 6b65096..b825c80 100644
>> --- a/drivers/gpu/drm/i915/intel_bios.c
>> +++ b/drivers/gpu/drm/i915/intel_bios.c
>> @@ -1059,6 +1059,20 @@ parse_device_mapping(struct drm_i915_private *dev_priv,
>>   			/* skip the device block if device type is invalid */
>>   			continue;
>>   		}
>> +
>> +#define MIPI_PORT_A	0x15
>> +#define MIPI_PORT_B	0x16
>> +#define MIPI_PORT_C	0x17
>> +#define MIPI_PORT_D	0x18
>> +		if (p_child->common.dvo_port >= MIPI_PORT_A && p_child->common.dvo_port <= MIPI_PORT_D) {
>> +			/* check the device type and confirm its MIPI */
>> +			if (p_child->common.device_type & DEVICE_TYPE_MIPI_OUTPUT) {
>> +				DRM_DEBUG_KMS("Found MIPI as LFP\n");
>> +				dev_priv->vbt.is_mipi = 1;
>> +				dev_priv->vbt.dsi.port = p_child->common.dvo_port;
>> +			}
>> +		}
>> +
>>   		child_dev_ptr = dev_priv->vbt.child_dev + count;
>>   		count++;
>>   		memcpy((void *)child_dev_ptr, (void *)p_child,
>> @@ -1230,7 +1244,11 @@ intel_parse_bios(struct drm_device *dev)
>>   	parse_device_mapping(dev_priv, bdb);
>>   	parse_driver_features(dev_priv, bdb);
>>   	parse_edp(dev_priv, bdb);
>> -	parse_mipi(dev_priv, bdb);
>> +
>> +	/* parse MIPI blocks only if LFP type is MIPI */
>> +	if (dev_priv->vbt.is_mipi)
>> +		parse_mipi(dev_priv, bdb);
>> +
>>   	parse_ddi_ports(dev_priv, bdb);
>>
>>   	if (bios)
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 3da73ef..a55fa41 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -11151,7 +11151,9 @@ static void intel_setup_outputs(struct drm_device *dev)
>>   			}
>>   		}
>>
>> -		intel_dsi_init(dev);
>> +		/* There is no detection method for MIPI so rely on VBT */
>> +		if (dev_priv->vbt.is_mipi)
>> +			intel_dsi_init(dev);
>>   	} else if (SUPPORTS_DIGITAL_OUTPUTS(dev)) {
>>   		bool found = false;
>>
>> --
>> 1.8.3.2
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] drm/i915: Detect if MIPI panel based on VBT and initialize only if present
  2014-05-27 11:32 ` Damien Lespiau
@ 2014-05-27 12:10   ` Kumar, Shobhit
  2014-05-27 12:27     ` Damien Lespiau
  0 siblings, 1 reply; 15+ messages in thread
From: Kumar, Shobhit @ 2014-05-27 12:10 UTC (permalink / raw)
  To: Damien Lespiau; +Cc: Jani Nikula, Daniel Vetter, intel-gfx

On 5/27/2014 5:02 PM, Damien Lespiau wrote:
> On Fri, May 23, 2014 at 09:39:28PM +0530, Shobhit Kumar wrote:
>> It seems by default the VBT has MIPI configuration block as well. The
>> Generic driver will assume always MIPI if MIPI configuration block is found.
>> This is causing probelm when actually there is eDP. Fix this by looking
>> into general definition block which will have device configurations. From here
>> we can figure out what is the LFP type and initialize MIPI only if MIPI
>> is found.
>>
>> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_drv.h      |  2 ++
>>   drivers/gpu/drm/i915/intel_bios.c    | 20 +++++++++++++++++++-
>>   drivers/gpu/drm/i915/intel_display.c |  4 +++-
>>   3 files changed, 24 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 8e78703..5a5225b 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -1229,7 +1229,9 @@ struct intel_vbt_data {
>>   	} backlight;
>>
>>   	/* MIPI DSI */
>> +	int is_mipi;
>
> There's a "feature" bitfield near the start of the structure, I think
> you should add that field there and maybe rename it to has_mipi.

will do.

>
>>   	struct {
>> +		u16 port;
>
> We store the port but never use it, is it needed?

Just wanted to store as we might want to use later while enabling. 
Shouldn't harm ? In fact we have a need to enable PORT C internally.

>
>>   		u16 panel_id;
>>   		struct mipi_config *config;
>>   		struct mipi_pps_data *pps;
>> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
>> index 6b65096..b825c80 100644
>> --- a/drivers/gpu/drm/i915/intel_bios.c
>> +++ b/drivers/gpu/drm/i915/intel_bios.c
>> @@ -1059,6 +1059,20 @@ parse_device_mapping(struct drm_i915_private *dev_priv,
>>   			/* skip the device block if device type is invalid */
>>   			continue;
>>   		}
>> +
>> +#define MIPI_PORT_A	0x15
>> +#define MIPI_PORT_B	0x16
>> +#define MIPI_PORT_C	0x17
>> +#define MIPI_PORT_D	0x18
>
> These should go with the other valid defines for dvo_port in
> intel_bios.h and be renamed to match, eg. DEVICE_PORT_MIPI_A.

Will do

>
> Also this is not what I have in my copy of the VBT documentation, I have
> (in decimal) MIPI PORT_A = 11, .., PORT_D = 14

What I have say 21..24. Again this is the problem we need to fix.

>
>> +		if (p_child->common.dvo_port >= MIPI_PORT_A && p_child->common.dvo_port <= MIPI_PORT_D) {
>> +			/* check the device type and confirm its MIPI */
>> +			if (p_child->common.device_type & DEVICE_TYPE_MIPI_OUTPUT) {
>> +				DRM_DEBUG_KMS("Found MIPI as LFP\n");
>> +				dev_priv->vbt.is_mipi = 1;
>> +				dev_priv->vbt.dsi.port = p_child->common.dvo_port;
>> +			}
>> +		}
>> +
>>   		child_dev_ptr = dev_priv->vbt.child_dev + count;
>>   		count++;
>>   		memcpy((void *)child_dev_ptr, (void *)p_child,
>> @@ -1230,7 +1244,11 @@ intel_parse_bios(struct drm_device *dev)
>>   	parse_device_mapping(dev_priv, bdb);
>>   	parse_driver_features(dev_priv, bdb);
>>   	parse_edp(dev_priv, bdb);
>> -	parse_mipi(dev_priv, bdb);
>> +
>> +	/* parse MIPI blocks only if LFP type is MIPI */
>> +	if (dev_priv->vbt.is_mipi)
>> +		parse_mipi(dev_priv, bdb);
>
> Let's put that check inside parse_mipi so we don't break the symetry of
> those parse_* calls.

Will do.

>
>> +
>>   	parse_ddi_ports(dev_priv, bdb);
>>
>>   	if (bios)
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 3da73ef..a55fa41 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -11151,7 +11151,9 @@ static void intel_setup_outputs(struct drm_device *dev)
>>   			}
>>   		}
>>
>> -		intel_dsi_init(dev);
>> +		/* There is no detection method for MIPI so rely on VBT */
>> +		if (dev_priv->vbt.is_mipi)
>> +			intel_dsi_init(dev);
>
> Can we have that check inside intel_dsi_init() so we don't have to do it
> for every platform with DSI?

Will do

regards
Shobhit

>
>>   	} else if (SUPPORTS_DIGITAL_OUTPUTS(dev)) {
>>   		bool found = false;
>>
>> --
>> 1.8.3.2
>>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] drm/i915: Detect if MIPI panel based on VBT and initialize only if present
  2014-05-27 12:10   ` Kumar, Shobhit
@ 2014-05-27 12:27     ` Damien Lespiau
  0 siblings, 0 replies; 15+ messages in thread
From: Damien Lespiau @ 2014-05-27 12:27 UTC (permalink / raw)
  To: Kumar, Shobhit; +Cc: Jani Nikula, Daniel Vetter, intel-gfx

On Tue, May 27, 2014 at 05:40:23PM +0530, Kumar, Shobhit wrote:
> >>  	struct {
> >>+		u16 port;
> >
> >We store the port but never use it, is it needed?
> 
> Just wanted to store as we might want to use later while enabling.
> Shouldn't harm ? In fact we have a need to enable PORT C internally.

Sure, no harm there.

> >Also this is not what I have in my copy of the VBT documentation, I have
> >(in decimal) MIPI PORT_A = 11, .., PORT_D = 14
> 
> What I have say 21..24. Again this is the problem we need to fix.

Mind sending me your version?

-- 
Damien

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [v2] drm/i915: Detect if MIPI panel based on VBT and initialize only if present
  2014-05-23 16:09 [PATCH] drm/i915: Detect if MIPI panel based on VBT and initialize only if present Shobhit Kumar
  2014-05-27 11:32 ` Damien Lespiau
  2014-05-27 11:40 ` Daniel Vetter
@ 2014-05-27 12:56 ` Shobhit Kumar
  2014-05-27 13:18   ` Damien Lespiau
  2014-05-27 14:03   ` [v3] " Shobhit Kumar
  2 siblings, 2 replies; 15+ messages in thread
From: Shobhit Kumar @ 2014-05-27 12:56 UTC (permalink / raw)
  To: intel-gfx; +Cc: Jani Nikula, Daniel Vetter

It seems by default the VBT has MIPI configuration block as well. The
Generic driver will assume always MIPI if MIPI configuration block is found.
This is causing probelm when actually there is eDP. Fix this by looking
into general definition block which will have device configurations. From here
we can figure out what is the LFP type and initialize MIPI only if MIPI
is found.

v2: Addressed review comments by Damien
    - Moved PORT definitions to intel_bios.h and renamed as DVO_PORT_MIPIA
    - renamed is_mipi to has_mipi and moved definition as suggested
    - Check hs_mipi inside parse_mipi and intel_dsi_init insted of outside

Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h   |  2 ++
 drivers/gpu/drm/i915/intel_bios.c | 14 ++++++++++++++
 drivers/gpu/drm/i915/intel_bios.h |  4 ++++
 drivers/gpu/drm/i915/intel_dsi.c  |  4 ++++
 4 files changed, 24 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8e78703..dac9ceb 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1209,6 +1209,7 @@ struct intel_vbt_data {
 	unsigned int fdi_rx_polarity_inverted:1;
 	int lvds_ssc_freq;
 	unsigned int bios_lvds_val; /* initial [PCH_]LVDS reg val in VBIOS */
+	int has_mipi;
 
 	enum drrs_support_type drrs_type;
 
@@ -1230,6 +1231,7 @@ struct intel_vbt_data {
 
 	/* MIPI DSI */
 	struct {
+		u16 port;
 		u16 panel_id;
 		struct mipi_config *config;
 		struct mipi_pps_data *pps;
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index 6b65096..6f6b6c6 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -744,6 +744,10 @@ parse_mipi(struct drm_i915_private *dev_priv, struct bdb_header *bdb)
 	int i, panel_id, seq_size;
 	u16 block_size;
 
+	/* parse MIPI blocks only if LFP type is MIPI */
+	if (!dev_priv->vbt.has_mipi)
+		return;
+
 	/* Initialize this to undefined indicating no generic MIPI support */
 	dev_priv->vbt.dsi.panel_id = MIPI_DSI_UNDEFINED_PANEL_ID;
 
@@ -1059,6 +1063,16 @@ parse_device_mapping(struct drm_i915_private *dev_priv,
 			/* skip the device block if device type is invalid */
 			continue;
 		}
+
+		if (p_child->common.dvo_port >= DVO_PORT_MIPIA && p_child->common.dvo_port <= DVO_PORT_MIPID) {
+			/* check the device type and confirm its MIPI */
+			if (p_child->common.device_type & DEVICE_TYPE_MIPI_OUTPUT) {
+				DRM_DEBUG_KMS("Found MIPI as LFP\n");
+				dev_priv->vbt.has_mipi = 1;
+				dev_priv->vbt.dsi.port = p_child->common.dvo_port;
+			}
+		}
+
 		child_dev_ptr = dev_priv->vbt.child_dev + count;
 		count++;
 		memcpy((void *)child_dev_ptr, (void *)p_child,
diff --git a/drivers/gpu/drm/i915/intel_bios.h b/drivers/gpu/drm/i915/intel_bios.h
index 6009deb..b986677 100644
--- a/drivers/gpu/drm/i915/intel_bios.h
+++ b/drivers/gpu/drm/i915/intel_bios.h
@@ -743,6 +743,10 @@ int intel_parse_bios(struct drm_device *dev);
 #define DVO_PORT_DPC	8
 #define DVO_PORT_DPD	9
 #define DVO_PORT_DPA	10
+#define DVO_PORT_MIPIA	21
+#define DVO_PORT_MIPIB	22
+#define DVO_PORT_MIPIC	23
+#define DVO_PORT_MIPID	24
 
 /* Block 52 contains MIPI Panel info
  * 6 such enteries will there. Index into correct
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index e73bec6..944a421 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -660,6 +660,10 @@ bool intel_dsi_init(struct drm_device *dev)
 
 	DRM_DEBUG_KMS("\n");
 
+	/* There is no detection method for MIPI so rely on VBT */
+	if (!dev_priv->vbt.has_mipi)
+		return false;
+
 	intel_dsi = kzalloc(sizeof(*intel_dsi), GFP_KERNEL);
 	if (!intel_dsi)
 		return false;
-- 
1.8.3.2

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [v2] drm/i915: Detect if MIPI panel based on VBT and initialize only if present
  2014-05-27 12:56 ` [v2] " Shobhit Kumar
@ 2014-05-27 13:18   ` Damien Lespiau
  2014-05-27 14:03   ` [v3] " Shobhit Kumar
  1 sibling, 0 replies; 15+ messages in thread
From: Damien Lespiau @ 2014-05-27 13:18 UTC (permalink / raw)
  To: Shobhit Kumar; +Cc: Jani Nikula, Daniel Vetter, intel-gfx

On Tue, May 27, 2014 at 06:26:22PM +0530, Shobhit Kumar wrote:
> It seems by default the VBT has MIPI configuration block as well. The
> Generic driver will assume always MIPI if MIPI configuration block is found.
> This is causing probelm when actually there is eDP. Fix this by looking
> into general definition block which will have device configurations. From here
> we can figure out what is the LFP type and initialize MIPI only if MIPI
> is found.
> 
> v2: Addressed review comments by Damien
>     - Moved PORT definitions to intel_bios.h and renamed as DVO_PORT_MIPIA
>     - renamed is_mipi to has_mipi and moved definition as suggested
>     - Check hs_mipi inside parse_mipi and intel_dsi_init insted of outside
> 
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h   |  2 ++
>  drivers/gpu/drm/i915/intel_bios.c | 14 ++++++++++++++
>  drivers/gpu/drm/i915/intel_bios.h |  4 ++++
>  drivers/gpu/drm/i915/intel_dsi.c  |  4 ++++
>  4 files changed, 24 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 8e78703..dac9ceb 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1209,6 +1209,7 @@ struct intel_vbt_data {
>  	unsigned int fdi_rx_polarity_inverted:1;
>  	int lvds_ssc_freq;
>  	unsigned int bios_lvds_val; /* initial [PCH_]LVDS reg val in VBIOS */
> +	int has_mipi;

I meant this can be added in the bitfield (after
fdi_rx_polarity_inverted) so it only takes 1 bit

Otherwise:

Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

--
Damien

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [v3] drm/i915: Detect if MIPI panel based on VBT and initialize only if present
  2014-05-27 12:56 ` [v2] " Shobhit Kumar
  2014-05-27 13:18   ` Damien Lespiau
@ 2014-05-27 14:03   ` Shobhit Kumar
  2014-05-27 14:17     ` Damien Lespiau
  2014-05-27 17:10     ` Daniel Vetter
  1 sibling, 2 replies; 15+ messages in thread
From: Shobhit Kumar @ 2014-05-27 14:03 UTC (permalink / raw)
  To: intel-gfx; +Cc: Daniel Vetter

It seems by default the VBT has MIPI configuration block as well. The
Generic driver will assume always MIPI if MIPI configuration block is found.
This is causing probelm when actually there is eDP. Fix this by looking
into general definition block which will have device configurations. From here
we can figure out what is the LFP type and initialize MIPI only if MIPI
is found.

v2: Addressed review comments by Damien
    - Moved PORT definitions to intel_bios.h and renamed as DVO_PORT_MIPIA
    - renamed is_mipi to has_mipi and moved definition as suggested
    - Check has_mipi inside parse_mipi and intel_dsi_init insted of outside

v3: Make has_mipi as a bitfield as suggested

Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h   |  2 ++
 drivers/gpu/drm/i915/intel_bios.c | 14 ++++++++++++++
 drivers/gpu/drm/i915/intel_bios.h |  4 ++++
 drivers/gpu/drm/i915/intel_dsi.c  |  4 ++++
 4 files changed, 24 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 9ef6712..ef7ab0a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1208,6 +1208,7 @@ struct intel_vbt_data {
 	unsigned int lvds_use_ssc:1;
 	unsigned int display_clock_mode:1;
 	unsigned int fdi_rx_polarity_inverted:1;
+	unsigned int has_mipi:1;
 	int lvds_ssc_freq;
 	unsigned int bios_lvds_val; /* initial [PCH_]LVDS reg val in VBIOS */
 
@@ -1231,6 +1232,7 @@ struct intel_vbt_data {
 
 	/* MIPI DSI */
 	struct {
+		u16 port;
 		u16 panel_id;
 		struct mipi_config *config;
 		struct mipi_pps_data *pps;
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index 6b65096..6f6b6c6 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -744,6 +744,10 @@ parse_mipi(struct drm_i915_private *dev_priv, struct bdb_header *bdb)
 	int i, panel_id, seq_size;
 	u16 block_size;
 
+	/* parse MIPI blocks only if LFP type is MIPI */
+	if (!dev_priv->vbt.has_mipi)
+		return;
+
 	/* Initialize this to undefined indicating no generic MIPI support */
 	dev_priv->vbt.dsi.panel_id = MIPI_DSI_UNDEFINED_PANEL_ID;
 
@@ -1059,6 +1063,16 @@ parse_device_mapping(struct drm_i915_private *dev_priv,
 			/* skip the device block if device type is invalid */
 			continue;
 		}
+
+		if (p_child->common.dvo_port >= DVO_PORT_MIPIA && p_child->common.dvo_port <= DVO_PORT_MIPID) {
+			/* check the device type and confirm its MIPI */
+			if (p_child->common.device_type & DEVICE_TYPE_MIPI_OUTPUT) {
+				DRM_DEBUG_KMS("Found MIPI as LFP\n");
+				dev_priv->vbt.has_mipi = 1;
+				dev_priv->vbt.dsi.port = p_child->common.dvo_port;
+			}
+		}
+
 		child_dev_ptr = dev_priv->vbt.child_dev + count;
 		count++;
 		memcpy((void *)child_dev_ptr, (void *)p_child,
diff --git a/drivers/gpu/drm/i915/intel_bios.h b/drivers/gpu/drm/i915/intel_bios.h
index 6009deb..b986677 100644
--- a/drivers/gpu/drm/i915/intel_bios.h
+++ b/drivers/gpu/drm/i915/intel_bios.h
@@ -743,6 +743,10 @@ int intel_parse_bios(struct drm_device *dev);
 #define DVO_PORT_DPC	8
 #define DVO_PORT_DPD	9
 #define DVO_PORT_DPA	10
+#define DVO_PORT_MIPIA	21
+#define DVO_PORT_MIPIB	22
+#define DVO_PORT_MIPIC	23
+#define DVO_PORT_MIPID	24
 
 /* Block 52 contains MIPI Panel info
  * 6 such enteries will there. Index into correct
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index e73bec6..944a421 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -660,6 +660,10 @@ bool intel_dsi_init(struct drm_device *dev)
 
 	DRM_DEBUG_KMS("\n");
 
+	/* There is no detection method for MIPI so rely on VBT */
+	if (!dev_priv->vbt.has_mipi)
+		return false;
+
 	intel_dsi = kzalloc(sizeof(*intel_dsi), GFP_KERNEL);
 	if (!intel_dsi)
 		return false;
-- 
1.8.3.2

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [v3] drm/i915: Detect if MIPI panel based on VBT and initialize only if present
  2014-05-27 14:03   ` [v3] " Shobhit Kumar
@ 2014-05-27 14:17     ` Damien Lespiau
  2014-05-27 14:31       ` Ville Syrjälä
  2014-05-27 14:31       ` Kumar, Shobhit
  2014-05-27 17:10     ` Daniel Vetter
  1 sibling, 2 replies; 15+ messages in thread
From: Damien Lespiau @ 2014-05-27 14:17 UTC (permalink / raw)
  To: Shobhit Kumar; +Cc: Daniel Vetter, intel-gfx

Sorry to be such a bore but:

On Tue, May 27, 2014 at 07:33:59PM +0530, Shobhit Kumar wrote:
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -660,6 +660,10 @@ bool intel_dsi_init(struct drm_device *dev)
>  
>  	DRM_DEBUG_KMS("\n");
>  
> +	/* There is no detection method for MIPI so rely on VBT */
> +	if (!dev_priv->vbt.has_mipi)
> +		return false;
> +

Huum, if we can intel_dsi_init() on VLV, but we don't have a MIPI panel,
shouldn't return true here? ie. "intel_dsi_init() was successful, we
just don't have a MIPI panel.

-- 
Damien

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [v3] drm/i915: Detect if MIPI panel based on VBT and initialize only if present
  2014-05-27 14:17     ` Damien Lespiau
@ 2014-05-27 14:31       ` Ville Syrjälä
  2014-05-27 14:35         ` Damien Lespiau
  2014-05-27 14:31       ` Kumar, Shobhit
  1 sibling, 1 reply; 15+ messages in thread
From: Ville Syrjälä @ 2014-05-27 14:31 UTC (permalink / raw)
  To: Damien Lespiau; +Cc: intel-gfx, Daniel Vetter

On Tue, May 27, 2014 at 03:17:15PM +0100, Damien Lespiau wrote:
> Sorry to be such a bore but:
> 
> On Tue, May 27, 2014 at 07:33:59PM +0530, Shobhit Kumar wrote:
> > --- a/drivers/gpu/drm/i915/intel_dsi.c
> > +++ b/drivers/gpu/drm/i915/intel_dsi.c
> > @@ -660,6 +660,10 @@ bool intel_dsi_init(struct drm_device *dev)
> >  
> >  	DRM_DEBUG_KMS("\n");
> >  
> > +	/* There is no detection method for MIPI so rely on VBT */
> > +	if (!dev_priv->vbt.has_mipi)
> > +		return false;
> > +
> 
> Huum, if we can intel_dsi_init() on VLV, but we don't have a MIPI panel,
> shouldn't return true here? ie. "intel_dsi_init() was successful, we
> just don't have a MIPI panel.

Why does it even have a return value? Either it added the
connector and encoder or it didn't.

-- 
Ville Syrjälä
Intel OTC

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [v3] drm/i915: Detect if MIPI panel based on VBT and initialize only if present
  2014-05-27 14:17     ` Damien Lespiau
  2014-05-27 14:31       ` Ville Syrjälä
@ 2014-05-27 14:31       ` Kumar, Shobhit
  1 sibling, 0 replies; 15+ messages in thread
From: Kumar, Shobhit @ 2014-05-27 14:31 UTC (permalink / raw)
  To: Damien Lespiau; +Cc: Daniel Vetter, intel-gfx

On 5/27/2014 7:47 PM, Damien Lespiau wrote:
> Sorry to be such a bore but:
>
> On Tue, May 27, 2014 at 07:33:59PM +0530, Shobhit Kumar wrote:
>> --- a/drivers/gpu/drm/i915/intel_dsi.c
>> +++ b/drivers/gpu/drm/i915/intel_dsi.c
>> @@ -660,6 +660,10 @@ bool intel_dsi_init(struct drm_device *dev)
>>
>>   	DRM_DEBUG_KMS("\n");
>>
>> +	/* There is no detection method for MIPI so rely on VBT */
>> +	if (!dev_priv->vbt.has_mipi)
>> +		return false;
>> +
>
> Huum, if we can intel_dsi_init() on VLV, but we don't have a MIPI panel,
> shouldn't return true here? ie. "intel_dsi_init() was successful, we
> just don't have a MIPI panel.

This check just determines that the design has probably eDP and not MIPI 
panel attached. Assuming of course that on any design will have either 
eDP or MIPI as LFP. So I was checking in terms of the OEM design and not 
platform capability to have DSI.

In fact even in intel_dp_init when in edp_init_connector fails to read 
DPCD we return false. In fact why we really need to have a return value 
when we don't even check it and for example intel_dp_init is void 
intel_dp_init

Regards
Shobhit

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [v3] drm/i915: Detect if MIPI panel based on VBT and initialize only if present
  2014-05-27 14:31       ` Ville Syrjälä
@ 2014-05-27 14:35         ` Damien Lespiau
  2014-05-27 17:05           ` Daniel Vetter
  0 siblings, 1 reply; 15+ messages in thread
From: Damien Lespiau @ 2014-05-27 14:35 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx, Daniel Vetter

On Tue, May 27, 2014 at 05:31:42PM +0300, Ville Syrjälä wrote:
> On Tue, May 27, 2014 at 03:17:15PM +0100, Damien Lespiau wrote:
> > Sorry to be such a bore but:
> > 
> > On Tue, May 27, 2014 at 07:33:59PM +0530, Shobhit Kumar wrote:
> > > --- a/drivers/gpu/drm/i915/intel_dsi.c
> > > +++ b/drivers/gpu/drm/i915/intel_dsi.c
> > > @@ -660,6 +660,10 @@ bool intel_dsi_init(struct drm_device *dev)
> > >  
> > >  	DRM_DEBUG_KMS("\n");
> > >  
> > > +	/* There is no detection method for MIPI so rely on VBT */
> > > +	if (!dev_priv->vbt.has_mipi)
> > > +		return false;
> > > +
> > 
> > Huum, if we can intel_dsi_init() on VLV, but we don't have a MIPI panel,
> > shouldn't return true here? ie. "intel_dsi_init() was successful, we
> > just don't have a MIPI panel.
> 
> Why does it even have a return value? Either it added the
> connector and encoder or it didn't.

That's even better. I guess we can have a patch on top once this one has
landed.

-- 
Damien

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [v3] drm/i915: Detect if MIPI panel based on VBT and initialize only if present
  2014-05-27 14:35         ` Damien Lespiau
@ 2014-05-27 17:05           ` Daniel Vetter
  0 siblings, 0 replies; 15+ messages in thread
From: Daniel Vetter @ 2014-05-27 17:05 UTC (permalink / raw)
  To: Damien Lespiau; +Cc: Daniel Vetter, intel-gfx

On Tue, May 27, 2014 at 03:35:44PM +0100, Damien Lespiau wrote:
> On Tue, May 27, 2014 at 05:31:42PM +0300, Ville Syrjälä wrote:
> > On Tue, May 27, 2014 at 03:17:15PM +0100, Damien Lespiau wrote:
> > > Sorry to be such a bore but:
> > > 
> > > On Tue, May 27, 2014 at 07:33:59PM +0530, Shobhit Kumar wrote:
> > > > --- a/drivers/gpu/drm/i915/intel_dsi.c
> > > > +++ b/drivers/gpu/drm/i915/intel_dsi.c
> > > > @@ -660,6 +660,10 @@ bool intel_dsi_init(struct drm_device *dev)
> > > >  
> > > >  	DRM_DEBUG_KMS("\n");
> > > >  
> > > > +	/* There is no detection method for MIPI so rely on VBT */
> > > > +	if (!dev_priv->vbt.has_mipi)
> > > > +		return false;
> > > > +
> > > 
> > > Huum, if we can intel_dsi_init() on VLV, but we don't have a MIPI panel,
> > > shouldn't return true here? ie. "intel_dsi_init() was successful, we
> > > just don't have a MIPI panel.
> > 
> > Why does it even have a return value? Either it added the
> > connector and encoder or it didn't.
> 
> That's even better. I guess we can have a patch on top once this one has
> landed.

dp_init has this since edp init can fail and that decides what we will do
with it occasionally. But that's about it wrt reasons for a return value
for a encoder init function. Please remove.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [v3] drm/i915: Detect if MIPI panel based on VBT and initialize only if present
  2014-05-27 14:03   ` [v3] " Shobhit Kumar
  2014-05-27 14:17     ` Damien Lespiau
@ 2014-05-27 17:10     ` Daniel Vetter
  1 sibling, 0 replies; 15+ messages in thread
From: Daniel Vetter @ 2014-05-27 17:10 UTC (permalink / raw)
  To: Shobhit Kumar; +Cc: Daniel Vetter, intel-gfx

On Tue, May 27, 2014 at 07:33:59PM +0530, Shobhit Kumar wrote:
> It seems by default the VBT has MIPI configuration block as well. The
> Generic driver will assume always MIPI if MIPI configuration block is found.
> This is causing probelm when actually there is eDP. Fix this by looking
> into general definition block which will have device configurations. From here
> we can figure out what is the LFP type and initialize MIPI only if MIPI
> is found.
> 
> v2: Addressed review comments by Damien
>     - Moved PORT definitions to intel_bios.h and renamed as DVO_PORT_MIPIA
>     - renamed is_mipi to has_mipi and moved definition as suggested
>     - Check has_mipi inside parse_mipi and intel_dsi_init insted of outside
> 
> v3: Make has_mipi as a bitfield as suggested
> 
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
> Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h   |  2 ++
>  drivers/gpu/drm/i915/intel_bios.c | 14 ++++++++++++++
>  drivers/gpu/drm/i915/intel_bios.h |  4 ++++
>  drivers/gpu/drm/i915/intel_dsi.c  |  4 ++++
>  4 files changed, 24 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 9ef6712..ef7ab0a 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1208,6 +1208,7 @@ struct intel_vbt_data {
>  	unsigned int lvds_use_ssc:1;
>  	unsigned int display_clock_mode:1;
>  	unsigned int fdi_rx_polarity_inverted:1;
> +	unsigned int has_mipi:1;
>  	int lvds_ssc_freq;
>  	unsigned int bios_lvds_val; /* initial [PCH_]LVDS reg val in VBIOS */
>  
> @@ -1231,6 +1232,7 @@ struct intel_vbt_data {
>  
>  	/* MIPI DSI */
>  	struct {
> +		u16 port;
>  		u16 panel_id;
>  		struct mipi_config *config;
>  		struct mipi_pps_data *pps;
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index 6b65096..6f6b6c6 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -744,6 +744,10 @@ parse_mipi(struct drm_i915_private *dev_priv, struct bdb_header *bdb)
>  	int i, panel_id, seq_size;
>  	u16 block_size;
>  
> +	/* parse MIPI blocks only if LFP type is MIPI */
> +	if (!dev_priv->vbt.has_mipi)
> +		return;
> +
>  	/* Initialize this to undefined indicating no generic MIPI support */
>  	dev_priv->vbt.dsi.panel_id = MIPI_DSI_UNDEFINED_PANEL_ID;
>  
> @@ -1059,6 +1063,16 @@ parse_device_mapping(struct drm_i915_private *dev_priv,
>  			/* skip the device block if device type is invalid */
>  			continue;
>  		}
> +
> +		if (p_child->common.dvo_port >= DVO_PORT_MIPIA && p_child->common.dvo_port <= DVO_PORT_MIPID) {

checkpatch thinks you should break this line. I agree. And folding in the
nested if will make everything neatly fit below 80 chars, so I've done
that. Queued for -next, thanks for the patch.
-Daniel

> +			/* check the device type and confirm its MIPI */
> +			if (p_child->common.device_type & DEVICE_TYPE_MIPI_OUTPUT) {
> +				DRM_DEBUG_KMS("Found MIPI as LFP\n");
> +				dev_priv->vbt.has_mipi = 1;
> +				dev_priv->vbt.dsi.port = p_child->common.dvo_port;
> +			}
> +		}
> +
>  		child_dev_ptr = dev_priv->vbt.child_dev + count;
>  		count++;
>  		memcpy((void *)child_dev_ptr, (void *)p_child,
> diff --git a/drivers/gpu/drm/i915/intel_bios.h b/drivers/gpu/drm/i915/intel_bios.h
> index 6009deb..b986677 100644
> --- a/drivers/gpu/drm/i915/intel_bios.h
> +++ b/drivers/gpu/drm/i915/intel_bios.h
> @@ -743,6 +743,10 @@ int intel_parse_bios(struct drm_device *dev);
>  #define DVO_PORT_DPC	8
>  #define DVO_PORT_DPD	9
>  #define DVO_PORT_DPA	10
> +#define DVO_PORT_MIPIA	21
> +#define DVO_PORT_MIPIB	22
> +#define DVO_PORT_MIPIC	23
> +#define DVO_PORT_MIPID	24
>  
>  /* Block 52 contains MIPI Panel info
>   * 6 such enteries will there. Index into correct
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index e73bec6..944a421 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -660,6 +660,10 @@ bool intel_dsi_init(struct drm_device *dev)
>  
>  	DRM_DEBUG_KMS("\n");
>  
> +	/* There is no detection method for MIPI so rely on VBT */
> +	if (!dev_priv->vbt.has_mipi)
> +		return false;
> +
>  	intel_dsi = kzalloc(sizeof(*intel_dsi), GFP_KERNEL);
>  	if (!intel_dsi)
>  		return false;
> -- 
> 1.8.3.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2014-05-27 17:10 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-05-23 16:09 [PATCH] drm/i915: Detect if MIPI panel based on VBT and initialize only if present Shobhit Kumar
2014-05-27 11:32 ` Damien Lespiau
2014-05-27 12:10   ` Kumar, Shobhit
2014-05-27 12:27     ` Damien Lespiau
2014-05-27 11:40 ` Daniel Vetter
2014-05-27 11:43   ` Kumar, Shobhit
2014-05-27 12:56 ` [v2] " Shobhit Kumar
2014-05-27 13:18   ` Damien Lespiau
2014-05-27 14:03   ` [v3] " Shobhit Kumar
2014-05-27 14:17     ` Damien Lespiau
2014-05-27 14:31       ` Ville Syrjälä
2014-05-27 14:35         ` Damien Lespiau
2014-05-27 17:05           ` Daniel Vetter
2014-05-27 14:31       ` Kumar, Shobhit
2014-05-27 17:10     ` Daniel Vetter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox