* [PATCH RESEND 2/5] drm/i915: move VBT based LVDS presence check to intel_bios.c
2016-02-05 10:36 [PATCH RESEND 1/5] drm/i915: move VBT based TV presence check to intel_bios.c Jani Nikula
@ 2016-02-05 10:36 ` Jani Nikula
2016-02-08 3:00 ` Thulasimani, Sivakumar
2016-02-05 10:36 ` [PATCH RESEND 3/5] drm/i915: move VBT based eDP port " Jani Nikula
` (4 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Jani Nikula @ 2016-02-05 10:36 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula
Hide knowledge about VBT child devices in intel_bios.c.
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/intel_bios.c | 50 ++++++++++++++++++++++++++++++++++++
drivers/gpu/drm/i915/intel_lvds.c | 53 +--------------------------------------
3 files changed, 52 insertions(+), 52 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 715f200cfbf5..d70ef71d2538 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3398,6 +3398,7 @@ extern void intel_i2c_reset(struct drm_device *dev);
int intel_bios_init(struct drm_i915_private *dev_priv);
bool intel_bios_is_valid_vbt(const void *buf, size_t size);
bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv);
+bool intel_bios_is_lvds_present(struct drm_i915_private *dev_priv, u8 *i2c_pin);
/* intel_opregion.c */
#ifdef CONFIG_ACPI
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index 2800ae50465a..f3f4f8e687cf 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -1469,3 +1469,53 @@ bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv)
return false;
}
+
+/**
+ * intel_bios_is_lvds_present - is LVDS present in VBT
+ * @dev_priv: i915 device instance
+ * @i2c_pin: i2c pin for LVDS if present
+ *
+ * Return true if LVDS is present. If no child devices were parsed from VBT,
+ * assume LVDS is present.
+ */
+bool intel_bios_is_lvds_present(struct drm_i915_private *dev_priv, u8 *i2c_pin)
+{
+ int i;
+
+ if (!dev_priv->vbt.child_dev_num)
+ return true;
+
+ for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
+ union child_device_config *uchild = dev_priv->vbt.child_dev + i;
+ struct old_child_dev_config *child = &uchild->old;
+
+ /* If the device type is not LFP, continue.
+ * We have to check both the new identifiers as well as the
+ * old for compatibility with some BIOSes.
+ */
+ if (child->device_type != DEVICE_TYPE_INT_LFP &&
+ child->device_type != DEVICE_TYPE_LFP)
+ continue;
+
+ if (intel_gmbus_is_valid_pin(dev_priv, child->i2c_pin))
+ *i2c_pin = child->i2c_pin;
+
+ /* However, we cannot trust the BIOS writers to populate
+ * the VBT correctly. Since LVDS requires additional
+ * information from AIM blocks, a non-zero addin offset is
+ * a good indicator that the LVDS is actually present.
+ */
+ if (child->addin_offset)
+ return true;
+
+ /* But even then some BIOS writers perform some black magic
+ * and instantiate the device without reference to any
+ * additional data. Trust that if the VBT was written into
+ * the OpRegion then they have validated the LVDS's existence.
+ */
+ if (dev_priv->opregion.vbt)
+ return true;
+ }
+
+ return false;
+}
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 0da0240caf81..80f8684e5137 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -772,57 +772,6 @@ static const struct dmi_system_id intel_no_lvds[] = {
{ } /* terminating entry */
};
-/*
- * Enumerate the child dev array parsed from VBT to check whether
- * the LVDS is present.
- * If it is present, return 1.
- * If it is not present, return false.
- * If no child dev is parsed from VBT, it assumes that the LVDS is present.
- */
-static bool lvds_is_present_in_vbt(struct drm_device *dev,
- u8 *i2c_pin)
-{
- struct drm_i915_private *dev_priv = dev->dev_private;
- int i;
-
- if (!dev_priv->vbt.child_dev_num)
- return true;
-
- for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
- union child_device_config *uchild = dev_priv->vbt.child_dev + i;
- struct old_child_dev_config *child = &uchild->old;
-
- /* If the device type is not LFP, continue.
- * We have to check both the new identifiers as well as the
- * old for compatibility with some BIOSes.
- */
- if (child->device_type != DEVICE_TYPE_INT_LFP &&
- child->device_type != DEVICE_TYPE_LFP)
- continue;
-
- if (intel_gmbus_is_valid_pin(dev_priv, child->i2c_pin))
- *i2c_pin = child->i2c_pin;
-
- /* However, we cannot trust the BIOS writers to populate
- * the VBT correctly. Since LVDS requires additional
- * information from AIM blocks, a non-zero addin offset is
- * a good indicator that the LVDS is actually present.
- */
- if (child->addin_offset)
- return true;
-
- /* But even then some BIOS writers perform some black magic
- * and instantiate the device without reference to any
- * additional data. Trust that if the VBT was written into
- * the OpRegion then they have validated the LVDS's existence.
- */
- if (dev_priv->opregion.vbt)
- return true;
- }
-
- return false;
-}
-
static int intel_dual_link_lvds_callback(const struct dmi_system_id *id)
{
DRM_INFO("Forcing lvds to dual link mode on %s\n", id->ident);
@@ -979,7 +928,7 @@ void intel_lvds_init(struct drm_device *dev)
}
pin = GMBUS_PIN_PANEL;
- if (!lvds_is_present_in_vbt(dev, &pin)) {
+ if (!intel_bios_is_lvds_present(dev_priv, &pin)) {
if ((lvds & LVDS_PORT_EN) == 0) {
DRM_DEBUG_KMS("LVDS is not present in VBT\n");
return;
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH RESEND 2/5] drm/i915: move VBT based LVDS presence check to intel_bios.c
2016-02-05 10:36 ` [PATCH RESEND 2/5] drm/i915: move VBT based LVDS " Jani Nikula
@ 2016-02-08 3:00 ` Thulasimani, Sivakumar
2016-02-09 15:12 ` Jani Nikula
0 siblings, 1 reply; 12+ messages in thread
From: Thulasimani, Sivakumar @ 2016-02-08 3:00 UTC (permalink / raw)
To: Jani Nikula, intel-gfx
On 2/5/2016 4:06 PM, Jani Nikula wrote:
> Hide knowledge about VBT child devices in intel_bios.c.
>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 1 +
> drivers/gpu/drm/i915/intel_bios.c | 50 ++++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/intel_lvds.c | 53 +--------------------------------------
> 3 files changed, 52 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 715f200cfbf5..d70ef71d2538 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3398,6 +3398,7 @@ extern void intel_i2c_reset(struct drm_device *dev);
> int intel_bios_init(struct drm_i915_private *dev_priv);
> bool intel_bios_is_valid_vbt(const void *buf, size_t size);
> bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv);
> +bool intel_bios_is_lvds_present(struct drm_i915_private *dev_priv, u8 *i2c_pin);
>
> /* intel_opregion.c */
> #ifdef CONFIG_ACPI
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index 2800ae50465a..f3f4f8e687cf 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -1469,3 +1469,53 @@ bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv)
>
> return false;
> }
> +
> +/**
> + * intel_bios_is_lvds_present - is LVDS present in VBT
> + * @dev_priv: i915 device instance
> + * @i2c_pin: i2c pin for LVDS if present
> + *
> + * Return true if LVDS is present. If no child devices were parsed from VBT,
> + * assume LVDS is present.
> + */
> +bool intel_bios_is_lvds_present(struct drm_i915_private *dev_priv, u8 *i2c_pin)
> +{
> + int i;
> +
> + if (!dev_priv->vbt.child_dev_num)
> + return true;
> +
we check if eDP is supported through following code before the call to
this function. please add this check here so we can avoid vbt
referencing for that.
intel_lvds.c
972 if (HAS_PCH_SPLIT(dev)) {
973 if ((lvds & LVDS_DETECTED) == 0)
974 return;
975 if (dev_priv->vbt.edp_support) {
976 DRM_DEBUG_KMS("disable LVDS for eDP
support\n");
977 return;
978 }
979 }
Sivakumar
> + for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
> + union child_device_config *uchild = dev_priv->vbt.child_dev + i;
> + struct old_child_dev_config *child = &uchild->old;
> +
> + /* If the device type is not LFP, continue.
> + * We have to check both the new identifiers as well as the
> + * old for compatibility with some BIOSes.
> + */
> + if (child->device_type != DEVICE_TYPE_INT_LFP &&
> + child->device_type != DEVICE_TYPE_LFP)
> + continue;
> +
> + if (intel_gmbus_is_valid_pin(dev_priv, child->i2c_pin))
> + *i2c_pin = child->i2c_pin;
> +
> + /* However, we cannot trust the BIOS writers to populate
> + * the VBT correctly. Since LVDS requires additional
> + * information from AIM blocks, a non-zero addin offset is
> + * a good indicator that the LVDS is actually present.
> + */
> + if (child->addin_offset)
> + return true;
> +
> + /* But even then some BIOS writers perform some black magic
> + * and instantiate the device without reference to any
> + * additional data. Trust that if the VBT was written into
> + * the OpRegion then they have validated the LVDS's existence.
> + */
> + if (dev_priv->opregion.vbt)
> + return true;
> + }
> +
> + return false;
> +}
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index 0da0240caf81..80f8684e5137 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -772,57 +772,6 @@ static const struct dmi_system_id intel_no_lvds[] = {
> { } /* terminating entry */
> };
>
> -/*
> - * Enumerate the child dev array parsed from VBT to check whether
> - * the LVDS is present.
> - * If it is present, return 1.
> - * If it is not present, return false.
> - * If no child dev is parsed from VBT, it assumes that the LVDS is present.
> - */
> -static bool lvds_is_present_in_vbt(struct drm_device *dev,
> - u8 *i2c_pin)
> -{
> - struct drm_i915_private *dev_priv = dev->dev_private;
> - int i;
> -
> - if (!dev_priv->vbt.child_dev_num)
> - return true;
> -
> - for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
> - union child_device_config *uchild = dev_priv->vbt.child_dev + i;
> - struct old_child_dev_config *child = &uchild->old;
> -
> - /* If the device type is not LFP, continue.
> - * We have to check both the new identifiers as well as the
> - * old for compatibility with some BIOSes.
> - */
> - if (child->device_type != DEVICE_TYPE_INT_LFP &&
> - child->device_type != DEVICE_TYPE_LFP)
> - continue;
> -
> - if (intel_gmbus_is_valid_pin(dev_priv, child->i2c_pin))
> - *i2c_pin = child->i2c_pin;
> -
> - /* However, we cannot trust the BIOS writers to populate
> - * the VBT correctly. Since LVDS requires additional
> - * information from AIM blocks, a non-zero addin offset is
> - * a good indicator that the LVDS is actually present.
> - */
> - if (child->addin_offset)
> - return true;
> -
> - /* But even then some BIOS writers perform some black magic
> - * and instantiate the device without reference to any
> - * additional data. Trust that if the VBT was written into
> - * the OpRegion then they have validated the LVDS's existence.
> - */
> - if (dev_priv->opregion.vbt)
> - return true;
> - }
> -
> - return false;
> -}
> -
> static int intel_dual_link_lvds_callback(const struct dmi_system_id *id)
> {
> DRM_INFO("Forcing lvds to dual link mode on %s\n", id->ident);
> @@ -979,7 +928,7 @@ void intel_lvds_init(struct drm_device *dev)
> }
>
> pin = GMBUS_PIN_PANEL;
> - if (!lvds_is_present_in_vbt(dev, &pin)) {
> + if (!intel_bios_is_lvds_present(dev_priv, &pin)) {
> if ((lvds & LVDS_PORT_EN) == 0) {
> DRM_DEBUG_KMS("LVDS is not present in VBT\n");
> return;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH RESEND 2/5] drm/i915: move VBT based LVDS presence check to intel_bios.c
2016-02-08 3:00 ` Thulasimani, Sivakumar
@ 2016-02-09 15:12 ` Jani Nikula
0 siblings, 0 replies; 12+ messages in thread
From: Jani Nikula @ 2016-02-09 15:12 UTC (permalink / raw)
To: Thulasimani, Sivakumar, intel-gfx
On Mon, 08 Feb 2016, "Thulasimani, Sivakumar" <sivakumar.thulasimani@intel.com> wrote:
> On 2/5/2016 4:06 PM, Jani Nikula wrote:
>> Hide knowledge about VBT child devices in intel_bios.c.
>>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_drv.h | 1 +
>> drivers/gpu/drm/i915/intel_bios.c | 50 ++++++++++++++++++++++++++++++++++++
>> drivers/gpu/drm/i915/intel_lvds.c | 53 +--------------------------------------
>> 3 files changed, 52 insertions(+), 52 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 715f200cfbf5..d70ef71d2538 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -3398,6 +3398,7 @@ extern void intel_i2c_reset(struct drm_device *dev);
>> int intel_bios_init(struct drm_i915_private *dev_priv);
>> bool intel_bios_is_valid_vbt(const void *buf, size_t size);
>> bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv);
>> +bool intel_bios_is_lvds_present(struct drm_i915_private *dev_priv, u8 *i2c_pin);
>>
>> /* intel_opregion.c */
>> #ifdef CONFIG_ACPI
>> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
>> index 2800ae50465a..f3f4f8e687cf 100644
>> --- a/drivers/gpu/drm/i915/intel_bios.c
>> +++ b/drivers/gpu/drm/i915/intel_bios.c
>> @@ -1469,3 +1469,53 @@ bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv)
>>
>> return false;
>> }
>> +
>> +/**
>> + * intel_bios_is_lvds_present - is LVDS present in VBT
>> + * @dev_priv: i915 device instance
>> + * @i2c_pin: i2c pin for LVDS if present
>> + *
>> + * Return true if LVDS is present. If no child devices were parsed from VBT,
>> + * assume LVDS is present.
>> + */
>> +bool intel_bios_is_lvds_present(struct drm_i915_private *dev_priv, u8 *i2c_pin)
>> +{
>> + int i;
>> +
>> + if (!dev_priv->vbt.child_dev_num)
>> + return true;
>> +
> we check if eDP is supported through following code before the call to
> this function. please add this check here so we can avoid vbt
> referencing for that.
Unfortunately, that would change the logic and I want to avoid making
that change at this point. For PCH split, we respect
dev_priv->vbt.edp_support, and bail out early, but otherwise we ignore
VBT if the BIOS has enabled LVDS anyway. I can only guess it's some
hackery to work around some weird setups out there.
BR,
Jani.
>
> intel_lvds.c
> 972 if (HAS_PCH_SPLIT(dev)) {
> 973 if ((lvds & LVDS_DETECTED) == 0)
> 974 return;
> 975 if (dev_priv->vbt.edp_support) {
> 976 DRM_DEBUG_KMS("disable LVDS for eDP
> support\n");
> 977 return;
> 978 }
> 979 }
>
> Sivakumar
>> + for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
>> + union child_device_config *uchild = dev_priv->vbt.child_dev + i;
>> + struct old_child_dev_config *child = &uchild->old;
>> +
>> + /* If the device type is not LFP, continue.
>> + * We have to check both the new identifiers as well as the
>> + * old for compatibility with some BIOSes.
>> + */
>> + if (child->device_type != DEVICE_TYPE_INT_LFP &&
>> + child->device_type != DEVICE_TYPE_LFP)
>> + continue;
>> +
>> + if (intel_gmbus_is_valid_pin(dev_priv, child->i2c_pin))
>> + *i2c_pin = child->i2c_pin;
>> +
>> + /* However, we cannot trust the BIOS writers to populate
>> + * the VBT correctly. Since LVDS requires additional
>> + * information from AIM blocks, a non-zero addin offset is
>> + * a good indicator that the LVDS is actually present.
>> + */
>> + if (child->addin_offset)
>> + return true;
>> +
>> + /* But even then some BIOS writers perform some black magic
>> + * and instantiate the device without reference to any
>> + * additional data. Trust that if the VBT was written into
>> + * the OpRegion then they have validated the LVDS's existence.
>> + */
>> + if (dev_priv->opregion.vbt)
>> + return true;
>> + }
>> +
>> + return false;
>> +}
>> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
>> index 0da0240caf81..80f8684e5137 100644
>> --- a/drivers/gpu/drm/i915/intel_lvds.c
>> +++ b/drivers/gpu/drm/i915/intel_lvds.c
>> @@ -772,57 +772,6 @@ static const struct dmi_system_id intel_no_lvds[] = {
>> { } /* terminating entry */
>> };
>>
>> -/*
>> - * Enumerate the child dev array parsed from VBT to check whether
>> - * the LVDS is present.
>> - * If it is present, return 1.
>> - * If it is not present, return false.
>> - * If no child dev is parsed from VBT, it assumes that the LVDS is present.
>> - */
>> -static bool lvds_is_present_in_vbt(struct drm_device *dev,
>> - u8 *i2c_pin)
>> -{
>> - struct drm_i915_private *dev_priv = dev->dev_private;
>> - int i;
>> -
>> - if (!dev_priv->vbt.child_dev_num)
>> - return true;
>> -
>> - for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
>> - union child_device_config *uchild = dev_priv->vbt.child_dev + i;
>> - struct old_child_dev_config *child = &uchild->old;
>> -
>> - /* If the device type is not LFP, continue.
>> - * We have to check both the new identifiers as well as the
>> - * old for compatibility with some BIOSes.
>> - */
>> - if (child->device_type != DEVICE_TYPE_INT_LFP &&
>> - child->device_type != DEVICE_TYPE_LFP)
>> - continue;
>> -
>> - if (intel_gmbus_is_valid_pin(dev_priv, child->i2c_pin))
>> - *i2c_pin = child->i2c_pin;
>> -
>> - /* However, we cannot trust the BIOS writers to populate
>> - * the VBT correctly. Since LVDS requires additional
>> - * information from AIM blocks, a non-zero addin offset is
>> - * a good indicator that the LVDS is actually present.
>> - */
>> - if (child->addin_offset)
>> - return true;
>> -
>> - /* But even then some BIOS writers perform some black magic
>> - * and instantiate the device without reference to any
>> - * additional data. Trust that if the VBT was written into
>> - * the OpRegion then they have validated the LVDS's existence.
>> - */
>> - if (dev_priv->opregion.vbt)
>> - return true;
>> - }
>> -
>> - return false;
>> -}
>> -
>> static int intel_dual_link_lvds_callback(const struct dmi_system_id *id)
>> {
>> DRM_INFO("Forcing lvds to dual link mode on %s\n", id->ident);
>> @@ -979,7 +928,7 @@ void intel_lvds_init(struct drm_device *dev)
>> }
>>
>> pin = GMBUS_PIN_PANEL;
>> - if (!lvds_is_present_in_vbt(dev, &pin)) {
>> + if (!intel_bios_is_lvds_present(dev_priv, &pin)) {
>> if ((lvds & LVDS_PORT_EN) == 0) {
>> DRM_DEBUG_KMS("LVDS is not present in VBT\n");
>> return;
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH RESEND 3/5] drm/i915: move VBT based eDP port check to intel_bios.c
2016-02-05 10:36 [PATCH RESEND 1/5] drm/i915: move VBT based TV presence check to intel_bios.c Jani Nikula
2016-02-05 10:36 ` [PATCH RESEND 2/5] drm/i915: move VBT based LVDS " Jani Nikula
@ 2016-02-05 10:36 ` Jani Nikula
2016-02-05 10:36 ` [PATCH RESEND 4/5] drm/i915: move VBT based DSI presence " Jani Nikula
` (3 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Jani Nikula @ 2016-02-05 10:36 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula
Hide knowledge about VBT child devices in intel_bios.c.
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/intel_bios.c | 33 +++++++++++++++++++++++++++++++++
drivers/gpu/drm/i915/intel_dp.c | 21 +--------------------
3 files changed, 35 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d70ef71d2538..234f33708a31 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3399,6 +3399,7 @@ int intel_bios_init(struct drm_i915_private *dev_priv);
bool intel_bios_is_valid_vbt(const void *buf, size_t size);
bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv);
bool intel_bios_is_lvds_present(struct drm_i915_private *dev_priv, u8 *i2c_pin);
+bool intel_bios_is_port_edp(struct drm_i915_private *dev_priv, enum port port);
/* intel_opregion.c */
#ifdef CONFIG_ACPI
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index f3f4f8e687cf..1429d8214885 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -1519,3 +1519,36 @@ bool intel_bios_is_lvds_present(struct drm_i915_private *dev_priv, u8 *i2c_pin)
return false;
}
+
+/**
+ * intel_bios_is_port_edp - is the device in given port eDP
+ * @dev_priv: i915 device instance
+ * @port: port to check
+ *
+ * Return true if the device in %port is eDP.
+ */
+bool intel_bios_is_port_edp(struct drm_i915_private *dev_priv, enum port port)
+{
+ union child_device_config *p_child;
+ static const short port_mapping[] = {
+ [PORT_B] = DVO_PORT_DPB,
+ [PORT_C] = DVO_PORT_DPC,
+ [PORT_D] = DVO_PORT_DPD,
+ [PORT_E] = DVO_PORT_DPE,
+ };
+ int i;
+
+ if (!dev_priv->vbt.child_dev_num)
+ return false;
+
+ for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
+ p_child = dev_priv->vbt.child_dev + i;
+
+ if (p_child->common.dvo_port == port_mapping[port] &&
+ (p_child->common.device_type & DEVICE_TYPE_eDP_BITS) ==
+ (DEVICE_TYPE_eDP & DEVICE_TYPE_eDP_BITS))
+ return true;
+ }
+
+ return false;
+}
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index a073f04a5330..9a4cfdf0cd70 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -5065,14 +5065,6 @@ put_power:
bool intel_dp_is_edp(struct drm_device *dev, enum port port)
{
struct drm_i915_private *dev_priv = dev->dev_private;
- union child_device_config *p_child;
- int i;
- static const short port_mapping[] = {
- [PORT_B] = DVO_PORT_DPB,
- [PORT_C] = DVO_PORT_DPC,
- [PORT_D] = DVO_PORT_DPD,
- [PORT_E] = DVO_PORT_DPE,
- };
/*
* eDP not supported on g4x. so bail out early just
@@ -5084,18 +5076,7 @@ bool intel_dp_is_edp(struct drm_device *dev, enum port port)
if (port == PORT_A)
return true;
- if (!dev_priv->vbt.child_dev_num)
- return false;
-
- for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
- p_child = dev_priv->vbt.child_dev + i;
-
- if (p_child->common.dvo_port == port_mapping[port] &&
- (p_child->common.device_type & DEVICE_TYPE_eDP_BITS) ==
- (DEVICE_TYPE_eDP & DEVICE_TYPE_eDP_BITS))
- return true;
- }
- return false;
+ return intel_bios_is_port_edp(dev_priv, port);
}
void
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH RESEND 4/5] drm/i915: move VBT based DSI presence check to intel_bios.c
2016-02-05 10:36 [PATCH RESEND 1/5] drm/i915: move VBT based TV presence check to intel_bios.c Jani Nikula
2016-02-05 10:36 ` [PATCH RESEND 2/5] drm/i915: move VBT based LVDS " Jani Nikula
2016-02-05 10:36 ` [PATCH RESEND 3/5] drm/i915: move VBT based eDP port " Jani Nikula
@ 2016-02-05 10:36 ` Jani Nikula
2016-02-08 3:17 ` Thulasimani, Sivakumar
2016-02-05 10:36 ` [PATCH RESEND 5/5] drm/i915/panel: setup pwm backlight based on connector type Jani Nikula
` (2 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Jani Nikula @ 2016-02-05 10:36 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula
Hide knowledge about VBT child devices in intel_bios.c.
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 2 +-
drivers/gpu/drm/i915/intel_bios.c | 33 ++++++++++++++++++++++++++++++++-
drivers/gpu/drm/i915/intel_dsi.c | 23 ++++++++++++++---------
3 files changed, 47 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 234f33708a31..cd2595c25f8d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1516,7 +1516,6 @@ struct intel_vbt_data {
/* MIPI DSI */
struct {
- u16 port;
u16 panel_id;
struct mipi_config *config;
struct mipi_pps_data *pps;
@@ -3400,6 +3399,7 @@ bool intel_bios_is_valid_vbt(const void *buf, size_t size);
bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv);
bool intel_bios_is_lvds_present(struct drm_i915_private *dev_priv, u8 *i2c_pin);
bool intel_bios_is_port_edp(struct drm_i915_private *dev_priv, enum port port);
+bool intel_bios_is_dsi_present(struct drm_i915_private *dev_priv, enum port *port);
/* intel_opregion.c */
#ifdef CONFIG_ACPI
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index 1429d8214885..b0cf33234c33 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -1237,7 +1237,6 @@ parse_device_mapping(struct drm_i915_private *dev_priv,
&&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;
@@ -1552,3 +1551,35 @@ bool intel_bios_is_port_edp(struct drm_i915_private *dev_priv, enum port port)
return false;
}
+
+/**
+ * intel_bios_is_dsi_present - is DSI present in VBT
+ * @dev_priv: i915 device instance
+ * @port: port for DSI if present
+ *
+ * Return true if DSI is present, and return the port in %port.
+ */
+bool intel_bios_is_dsi_present(struct drm_i915_private *dev_priv,
+ enum port *port)
+{
+ union child_device_config *p_child;
+ int i;
+
+ for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
+ p_child = dev_priv->vbt.child_dev + i;
+
+ if (!(p_child->common.device_type & DEVICE_TYPE_MIPI_OUTPUT))
+ continue;
+
+ switch (p_child->common.dvo_port) {
+ case DVO_PORT_MIPIA:
+ case DVO_PORT_MIPIB:
+ case DVO_PORT_MIPIC:
+ case DVO_PORT_MIPID:
+ *port = p_child->common.dvo_port - DVO_PORT_MIPIA;
+ return true;
+ }
+ }
+
+ return false;
+}
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 378f879f4015..5b6ec567e3ce 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -1112,9 +1112,15 @@ void 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)
+ if (!intel_bios_is_dsi_present(dev_priv, &port))
return;
+ if (port != PORT_A && port != PORT_C) {
+ DRM_DEBUG_KMS("VBT has unsupported DSI port %c\n",
+ port_name(port));
+ return;
+ }
+
if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
dev_priv->mipi_mmio_base = VLV_MIPI_BASE;
} else {
@@ -1153,16 +1159,15 @@ void intel_dsi_init(struct drm_device *dev)
intel_connector->unregister = intel_connector_unregister;
/* Pipe A maps to MIPI DSI port A, pipe B maps to MIPI DSI port C */
- if (dev_priv->vbt.dsi.port == DVO_PORT_MIPIA) {
- intel_encoder->crtc_mask = (1 << PIPE_A);
- intel_dsi->ports = (1 << PORT_A);
- } else if (dev_priv->vbt.dsi.port == DVO_PORT_MIPIC) {
- intel_encoder->crtc_mask = (1 << PIPE_B);
- intel_dsi->ports = (1 << PORT_C);
- }
+ if (port == PORT_A)
+ intel_encoder->crtc_mask = 1 << PIPE_A;
+ else
+ intel_encoder->crtc_mask = 1 << PIPE_B;
if (dev_priv->vbt.dsi.config->dual_link)
- intel_dsi->ports = ((1 << PORT_A) | (1 << PORT_C));
+ intel_dsi->ports = (1 << PORT_A) | (1 << PORT_C);
+ else
+ intel_dsi->ports = 1 << port;
/* Create a DSI host (and a device) for each port. */
for_each_dsi_port(port, intel_dsi->ports) {
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH RESEND 4/5] drm/i915: move VBT based DSI presence check to intel_bios.c
2016-02-05 10:36 ` [PATCH RESEND 4/5] drm/i915: move VBT based DSI presence " Jani Nikula
@ 2016-02-08 3:17 ` Thulasimani, Sivakumar
2016-02-09 15:16 ` Jani Nikula
0 siblings, 1 reply; 12+ messages in thread
From: Thulasimani, Sivakumar @ 2016-02-08 3:17 UTC (permalink / raw)
To: Jani Nikula, intel-gfx
On 2/5/2016 4:06 PM, Jani Nikula wrote:
> Hide knowledge about VBT child devices in intel_bios.c.
>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 2 +-
> drivers/gpu/drm/i915/intel_bios.c | 33 ++++++++++++++++++++++++++++++++-
> drivers/gpu/drm/i915/intel_dsi.c | 23 ++++++++++++++---------
> 3 files changed, 47 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 234f33708a31..cd2595c25f8d 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1516,7 +1516,6 @@ struct intel_vbt_data {
>
> /* MIPI DSI */
> struct {
> - u16 port;
> u16 panel_id;
> struct mipi_config *config;
> struct mipi_pps_data *pps;
> @@ -3400,6 +3399,7 @@ bool intel_bios_is_valid_vbt(const void *buf, size_t size);
> bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv);
> bool intel_bios_is_lvds_present(struct drm_i915_private *dev_priv, u8 *i2c_pin);
> bool intel_bios_is_port_edp(struct drm_i915_private *dev_priv, enum port port);
> +bool intel_bios_is_dsi_present(struct drm_i915_private *dev_priv, enum port *port);
>
> /* intel_opregion.c */
> #ifdef CONFIG_ACPI
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index 1429d8214885..b0cf33234c33 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -1237,7 +1237,6 @@ parse_device_mapping(struct drm_i915_private *dev_priv,
> &&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;
> @@ -1552,3 +1551,35 @@ bool intel_bios_is_port_edp(struct drm_i915_private *dev_priv, enum port port)
>
> return false;
> }
> +
> +/**
> + * intel_bios_is_dsi_present - is DSI present in VBT
> + * @dev_priv: i915 device instance
> + * @port: port for DSI if present
> + *
> + * Return true if DSI is present, and return the port in %port.
> + */
> +bool intel_bios_is_dsi_present(struct drm_i915_private *dev_priv,
> + enum port *port)
> +{
> + union child_device_config *p_child;
> + int i;
> +
> + for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
> + p_child = dev_priv->vbt.child_dev + i;
> +
> + if (!(p_child->common.device_type & DEVICE_TYPE_MIPI_OUTPUT))
> + continue;
> +
> + switch (p_child->common.dvo_port) {
> + case DVO_PORT_MIPIA:
> + case DVO_PORT_MIPIB:
> + case DVO_PORT_MIPIC:
> + case DVO_PORT_MIPID:
> + *port = p_child->common.dvo_port - DVO_PORT_MIPIA;
> + return true;
> + }
> + }
can we add platform check & vbt.has_mipi check ? it will avoid the need
to loop through the array.
Also since all platforms so far (VLV/CHV/BXT) has only MIPI A & MIPI C
is B & D needed here ? we can directly fail here for unsupported ports
rather than in dsi_init as special check.
> +
> + return false;
> +}
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 378f879f4015..5b6ec567e3ce 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -1112,9 +1112,15 @@ void 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)
> + if (!intel_bios_is_dsi_present(dev_priv, &port))
> return;
>
> + if (port != PORT_A && port != PORT_C) {
> + DRM_DEBUG_KMS("VBT has unsupported DSI port %c\n",
> + port_name(port));
> + return;
> + }
> +
> if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
> dev_priv->mipi_mmio_base = VLV_MIPI_BASE;
> } else {
> @@ -1153,16 +1159,15 @@ void intel_dsi_init(struct drm_device *dev)
> intel_connector->unregister = intel_connector_unregister;
>
> /* Pipe A maps to MIPI DSI port A, pipe B maps to MIPI DSI port C */
> - if (dev_priv->vbt.dsi.port == DVO_PORT_MIPIA) {
> - intel_encoder->crtc_mask = (1 << PIPE_A);
> - intel_dsi->ports = (1 << PORT_A);
> - } else if (dev_priv->vbt.dsi.port == DVO_PORT_MIPIC) {
> - intel_encoder->crtc_mask = (1 << PIPE_B);
> - intel_dsi->ports = (1 << PORT_C);
> - }
> + if (port == PORT_A)
> + intel_encoder->crtc_mask = 1 << PIPE_A;
> + else
> + intel_encoder->crtc_mask = 1 << PIPE_B;
>
The above limitation applies only to CHT/VLV.
> if (dev_priv->vbt.dsi.config->dual_link)
> - intel_dsi->ports = ((1 << PORT_A) | (1 << PORT_C));
> + intel_dsi->ports = (1 << PORT_A) | (1 << PORT_C);
> + else
> + intel_dsi->ports = 1 << port;
>
> /* Create a DSI host (and a device) for each port. */
> for_each_dsi_port(port, intel_dsi->ports) {
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH RESEND 4/5] drm/i915: move VBT based DSI presence check to intel_bios.c
2016-02-08 3:17 ` Thulasimani, Sivakumar
@ 2016-02-09 15:16 ` Jani Nikula
0 siblings, 0 replies; 12+ messages in thread
From: Jani Nikula @ 2016-02-09 15:16 UTC (permalink / raw)
To: Thulasimani, Sivakumar, intel-gfx
On Mon, 08 Feb 2016, "Thulasimani, Sivakumar" <sivakumar.thulasimani@intel.com> wrote:
> On 2/5/2016 4:06 PM, Jani Nikula wrote:
>> Hide knowledge about VBT child devices in intel_bios.c.
>>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_drv.h | 2 +-
>> drivers/gpu/drm/i915/intel_bios.c | 33 ++++++++++++++++++++++++++++++++-
>> drivers/gpu/drm/i915/intel_dsi.c | 23 ++++++++++++++---------
>> 3 files changed, 47 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 234f33708a31..cd2595c25f8d 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -1516,7 +1516,6 @@ struct intel_vbt_data {
>>
>> /* MIPI DSI */
>> struct {
>> - u16 port;
>> u16 panel_id;
>> struct mipi_config *config;
>> struct mipi_pps_data *pps;
>> @@ -3400,6 +3399,7 @@ bool intel_bios_is_valid_vbt(const void *buf, size_t size);
>> bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv);
>> bool intel_bios_is_lvds_present(struct drm_i915_private *dev_priv, u8 *i2c_pin);
>> bool intel_bios_is_port_edp(struct drm_i915_private *dev_priv, enum port port);
>> +bool intel_bios_is_dsi_present(struct drm_i915_private *dev_priv, enum port *port);
>>
>> /* intel_opregion.c */
>> #ifdef CONFIG_ACPI
>> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
>> index 1429d8214885..b0cf33234c33 100644
>> --- a/drivers/gpu/drm/i915/intel_bios.c
>> +++ b/drivers/gpu/drm/i915/intel_bios.c
>> @@ -1237,7 +1237,6 @@ parse_device_mapping(struct drm_i915_private *dev_priv,
>> &&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;
>> @@ -1552,3 +1551,35 @@ bool intel_bios_is_port_edp(struct drm_i915_private *dev_priv, enum port port)
>>
>> return false;
>> }
>> +
>> +/**
>> + * intel_bios_is_dsi_present - is DSI present in VBT
>> + * @dev_priv: i915 device instance
>> + * @port: port for DSI if present
>> + *
>> + * Return true if DSI is present, and return the port in %port.
>> + */
>> +bool intel_bios_is_dsi_present(struct drm_i915_private *dev_priv,
>> + enum port *port)
>> +{
>> + union child_device_config *p_child;
>> + int i;
>> +
>> + for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
>> + p_child = dev_priv->vbt.child_dev + i;
>> +
>> + if (!(p_child->common.device_type & DEVICE_TYPE_MIPI_OUTPUT))
>> + continue;
>> +
>> + switch (p_child->common.dvo_port) {
>> + case DVO_PORT_MIPIA:
>> + case DVO_PORT_MIPIB:
>> + case DVO_PORT_MIPIC:
>> + case DVO_PORT_MIPID:
>> + *port = p_child->common.dvo_port - DVO_PORT_MIPIA;
>> + return true;
>> + }
>> + }
> can we add platform check & vbt.has_mipi check ? it will avoid the need
> to loop through the array.
I'm actually going to remove has_mipi altogether as unnecessary
duplication of information. We run the loop very few times anyway. Also,
I don't want to duplicate the information about platforms, that stuff is
in intel_setup_outputs and in the encoder inits. Single point of truth.
> Also since all platforms so far (VLV/CHV/BXT) has only MIPI A & MIPI C
> is B & D needed here ? we can directly fail here for unsupported ports
> rather than in dsi_init as special check.
Agreed.
>> +
>> + return false;
>> +}
>> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
>> index 378f879f4015..5b6ec567e3ce 100644
>> --- a/drivers/gpu/drm/i915/intel_dsi.c
>> +++ b/drivers/gpu/drm/i915/intel_dsi.c
>> @@ -1112,9 +1112,15 @@ void 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)
>> + if (!intel_bios_is_dsi_present(dev_priv, &port))
>> return;
>>
>> + if (port != PORT_A && port != PORT_C) {
>> + DRM_DEBUG_KMS("VBT has unsupported DSI port %c\n",
>> + port_name(port));
>> + return;
>> + }
>> +
>> if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
>> dev_priv->mipi_mmio_base = VLV_MIPI_BASE;
>> } else {
>> @@ -1153,16 +1159,15 @@ void intel_dsi_init(struct drm_device *dev)
>> intel_connector->unregister = intel_connector_unregister;
>>
>> /* Pipe A maps to MIPI DSI port A, pipe B maps to MIPI DSI port C */
>> - if (dev_priv->vbt.dsi.port == DVO_PORT_MIPIA) {
>> - intel_encoder->crtc_mask = (1 << PIPE_A);
>> - intel_dsi->ports = (1 << PORT_A);
>> - } else if (dev_priv->vbt.dsi.port == DVO_PORT_MIPIC) {
>> - intel_encoder->crtc_mask = (1 << PIPE_B);
>> - intel_dsi->ports = (1 << PORT_C);
>> - }
>> + if (port == PORT_A)
>> + intel_encoder->crtc_mask = 1 << PIPE_A;
>> + else
>> + intel_encoder->crtc_mask = 1 << PIPE_B;
>>
> The above limitation applies only to CHT/VLV.
True, but I'm again reluctant to make the change at this point. It
should be added as part of BXT DSI instead.
BR,
Jani.
>> if (dev_priv->vbt.dsi.config->dual_link)
>> - intel_dsi->ports = ((1 << PORT_A) | (1 << PORT_C));
>> + intel_dsi->ports = (1 << PORT_A) | (1 << PORT_C);
>> + else
>> + intel_dsi->ports = 1 << port;
>>
>> /* Create a DSI host (and a device) for each port. */
>> for_each_dsi_port(port, intel_dsi->ports) {
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH RESEND 5/5] drm/i915/panel: setup pwm backlight based on connector type
2016-02-05 10:36 [PATCH RESEND 1/5] drm/i915: move VBT based TV presence check to intel_bios.c Jani Nikula
` (2 preceding siblings ...)
2016-02-05 10:36 ` [PATCH RESEND 4/5] drm/i915: move VBT based DSI presence " Jani Nikula
@ 2016-02-05 10:36 ` Jani Nikula
2016-02-05 11:14 ` ✗ Fi.CI.BAT: failure for series starting with [RESEND,1/5] drm/i915: move VBT based TV presence check to intel_bios.c Patchwork
2016-02-08 2:52 ` [PATCH RESEND 1/5] " Thulasimani, Sivakumar
5 siblings, 0 replies; 12+ messages in thread
From: Jani Nikula @ 2016-02-05 10:36 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula
Use the connector type instead of VBT directly to decide which backlight
mechanism to use on VLV/CHV. (Indirectly, this is the same thing, but
hides the VBT use.)
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
drivers/gpu/drm/i915/intel_panel.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index 21ee6477bf98..f7fbb7c223b1 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -1745,7 +1745,7 @@ intel_panel_init_backlight_funcs(struct intel_panel *panel)
panel->backlight.get = pch_get_backlight;
panel->backlight.hz_to_pwm = pch_hz_to_pwm;
} else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
- if (dev_priv->vbt.has_mipi) {
+ if (connector->base.connector_type == DRM_MODE_CONNECTOR_DSI) {
panel->backlight.setup = pwm_setup_backlight;
panel->backlight.enable = pwm_enable_backlight;
panel->backlight.disable = pwm_disable_backlight;
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread* ✗ Fi.CI.BAT: failure for series starting with [RESEND,1/5] drm/i915: move VBT based TV presence check to intel_bios.c
2016-02-05 10:36 [PATCH RESEND 1/5] drm/i915: move VBT based TV presence check to intel_bios.c Jani Nikula
` (3 preceding siblings ...)
2016-02-05 10:36 ` [PATCH RESEND 5/5] drm/i915/panel: setup pwm backlight based on connector type Jani Nikula
@ 2016-02-05 11:14 ` Patchwork
2016-02-08 2:52 ` [PATCH RESEND 1/5] " Thulasimani, Sivakumar
5 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2016-02-05 11:14 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx
== Summary ==
Series 3121v1 Series without cover letter
http://patchwork.freedesktop.org/api/1.0/series/3121/revisions/1/mbox/
Test drv_module_reload_basic:
pass -> DMESG-WARN (ilk-hp8440p)
Test gem_mmap_gtt:
Subgroup basic-small-bo:
pass -> DMESG-WARN (ilk-hp8440p)
Test gem_sync:
Subgroup basic-bsd:
pass -> DMESG-FAIL (ilk-hp8440p)
Test kms_flip:
Subgroup basic-flip-vs-dpms:
dmesg-warn -> PASS (ilk-hp8440p) UNSTABLE
Test pm_rpm:
Subgroup basic-pci-d3-state:
dmesg-warn -> PASS (skl-i5k-2)
bdw-nuci7 total:161 pass:152 dwarn:0 dfail:0 fail:0 skip:9
bdw-ultra total:164 pass:152 dwarn:0 dfail:0 fail:0 skip:12
byt-nuc total:164 pass:141 dwarn:0 dfail:0 fail:0 skip:23
hsw-brixbox total:164 pass:151 dwarn:0 dfail:0 fail:0 skip:13
hsw-gt2 total:164 pass:154 dwarn:0 dfail:0 fail:0 skip:10
ilk-hp8440p total:164 pass:113 dwarn:2 dfail:1 fail:0 skip:48
ivb-t430s total:164 pass:150 dwarn:0 dfail:0 fail:0 skip:14
skl-i5k-2 total:164 pass:149 dwarn:1 dfail:0 fail:0 skip:14
skl-i7k-2 total:164 pass:149 dwarn:1 dfail:0 fail:0 skip:14
snb-dellxps total:164 pass:142 dwarn:0 dfail:0 fail:0 skip:22
snb-x220t total:164 pass:142 dwarn:0 dfail:0 fail:1 skip:21
Results at /archive/results/CI_IGT_test/Patchwork_1368/
57e229193395068adcb34c5266d54194e652869f drm-intel-nightly: 2016y-02m-04d-18h-38m-55s UTC integration manifest
31400d242652b6705a98f9539c685daf4f8d46c9 drm/i915/panel: setup pwm backlight based on connector type
fb6c1a1a33ed8c55450251bace69f42b98f23e9b drm/i915: move VBT based DSI presence check to intel_bios.c
dbc3f4f96e24ae58849c4775f8c1cae3c0d70065 drm/i915: move VBT based eDP port check to intel_bios.c
3389e0bfcd2611ac99db098fddb936f9eb6f9259 drm/i915: move VBT based LVDS presence check to intel_bios.c
4a1453567944519983f85e212a8eb93084834656 drm/i915: move VBT based TV presence check to intel_bios.c
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH RESEND 1/5] drm/i915: move VBT based TV presence check to intel_bios.c
2016-02-05 10:36 [PATCH RESEND 1/5] drm/i915: move VBT based TV presence check to intel_bios.c Jani Nikula
` (4 preceding siblings ...)
2016-02-05 11:14 ` ✗ Fi.CI.BAT: failure for series starting with [RESEND,1/5] drm/i915: move VBT based TV presence check to intel_bios.c Patchwork
@ 2016-02-08 2:52 ` Thulasimani, Sivakumar
2016-02-09 15:18 ` Jani Nikula
5 siblings, 1 reply; 12+ messages in thread
From: Thulasimani, Sivakumar @ 2016-02-08 2:52 UTC (permalink / raw)
To: Jani Nikula, intel-gfx
On 2/5/2016 4:06 PM, Jani Nikula wrote:
> Hide knowledge about VBT child devices in intel_bios.c.
>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 1 +
> drivers/gpu/drm/i915/intel_bios.c | 38 ++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/intel_tv.c | 43 +--------------------------------------
> 3 files changed, 40 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 77227a39f3d5..715f200cfbf5 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3397,6 +3397,7 @@ extern void intel_i2c_reset(struct drm_device *dev);
> /* intel_bios.c */
> int intel_bios_init(struct drm_i915_private *dev_priv);
> bool intel_bios_is_valid_vbt(const void *buf, size_t size);
> +bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv);
>
> /* intel_opregion.c */
> #ifdef CONFIG_ACPI
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index bf62a19c8f69..2800ae50465a 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -1431,3 +1431,41 @@ intel_bios_init(struct drm_i915_private *dev_priv)
>
> return 0;
> }
> +
> +/**
> + * intel_bios_is_tv_present - is integrated TV present in VBT
> + * @dev_priv: i915 device instance
> + *
> + * Return true if TV is present. If no child devices were parsed from VBT,
> + * assume TV is present.
> + */
> +bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv)
> +{
> + union child_device_config *p_child;
> + int i;
> +
> + if (!dev_priv->vbt.child_dev_num)
> + return true;
> +
This is old code moved here, but should we return true
when child_dev_num == 0 ? I understand we shouldn't make
functional changes when moving code but just pointing out
to check if this can be fixed in the further patches.
> + for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
> + p_child = dev_priv->vbt.child_dev + i;
> + /*
> + * If the device type is not TV, continue.
> + */
> + switch (p_child->old.device_type) {
> + case DEVICE_TYPE_INT_TV:
> + case DEVICE_TYPE_TV:
> + case DEVICE_TYPE_TV_SVIDEO_COMPOSITE:
> + break;
> + default:
> + continue;
> + }
> + /* Only when the addin_offset is non-zero, it is regarded
> + * as present.
> + */
> + if (p_child->old.addin_offset)
> + return true;
> + }
> +
> + return false;
> +}
> diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
> index 5034b0055169..8417fcad02d2 100644
> --- a/drivers/gpu/drm/i915/intel_tv.c
> +++ b/drivers/gpu/drm/i915/intel_tv.c
> @@ -1530,47 +1530,6 @@ static const struct drm_encoder_funcs intel_tv_enc_funcs = {
> .destroy = intel_encoder_destroy,
> };
>
> -/*
> - * Enumerate the child dev array parsed from VBT to check whether
> - * the integrated TV is present.
> - * If it is present, return 1.
> - * If it is not present, return false.
> - * If no child dev is parsed from VBT, it assumes that the TV is present.
> - */
> -static int tv_is_present_in_vbt(struct drm_device *dev)
> -{
> - struct drm_i915_private *dev_priv = dev->dev_private;
> - union child_device_config *p_child;
> - int i, ret;
> -
> - if (!dev_priv->vbt.child_dev_num)
> - return 1;
> -
> - ret = 0;
> - for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
> - p_child = dev_priv->vbt.child_dev + i;
> - /*
> - * If the device type is not TV, continue.
> - */
> - switch (p_child->old.device_type) {
> - case DEVICE_TYPE_INT_TV:
> - case DEVICE_TYPE_TV:
> - case DEVICE_TYPE_TV_SVIDEO_COMPOSITE:
> - break;
> - default:
> - continue;
> - }
> - /* Only when the addin_offset is non-zero, it is regarded
> - * as present.
> - */
> - if (p_child->old.addin_offset) {
> - ret = 1;
> - break;
> - }
> - }
> - return ret;
> -}
> -
> void
> intel_tv_init(struct drm_device *dev)
> {
> @@ -1586,7 +1545,7 @@ intel_tv_init(struct drm_device *dev)
> if ((I915_READ(TV_CTL) & TV_FUSE_STATE_MASK) == TV_FUSE_STATE_DISABLED)
> return;
>
> - if (!tv_is_present_in_vbt(dev)) {
> + if (!intel_bios_is_tv_present(dev_priv)) {
> DRM_DEBUG_KMS("Integrated TV is not present.\n");
> return;
> }
saw below line immediately after the tv_is_present_in_vbt() call.
can you move this inside the new function as well since we
consider it as well before enumerating tv encoder.
1593 /* Even if we have an encoder we may not have a connector */
1594 if (!dev_priv->vbt.int_tv_support)
1595 return;
1596
regards,
Sivakumar
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH RESEND 1/5] drm/i915: move VBT based TV presence check to intel_bios.c
2016-02-08 2:52 ` [PATCH RESEND 1/5] " Thulasimani, Sivakumar
@ 2016-02-09 15:18 ` Jani Nikula
0 siblings, 0 replies; 12+ messages in thread
From: Jani Nikula @ 2016-02-09 15:18 UTC (permalink / raw)
To: Thulasimani, Sivakumar, intel-gfx
On Mon, 08 Feb 2016, "Thulasimani, Sivakumar" <sivakumar.thulasimani@intel.com> wrote:
> On 2/5/2016 4:06 PM, Jani Nikula wrote:
>> Hide knowledge about VBT child devices in intel_bios.c.
>>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_drv.h | 1 +
>> drivers/gpu/drm/i915/intel_bios.c | 38 ++++++++++++++++++++++++++++++++++
>> drivers/gpu/drm/i915/intel_tv.c | 43 +--------------------------------------
>> 3 files changed, 40 insertions(+), 42 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 77227a39f3d5..715f200cfbf5 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -3397,6 +3397,7 @@ extern void intel_i2c_reset(struct drm_device *dev);
>> /* intel_bios.c */
>> int intel_bios_init(struct drm_i915_private *dev_priv);
>> bool intel_bios_is_valid_vbt(const void *buf, size_t size);
>> +bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv);
>>
>> /* intel_opregion.c */
>> #ifdef CONFIG_ACPI
>> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
>> index bf62a19c8f69..2800ae50465a 100644
>> --- a/drivers/gpu/drm/i915/intel_bios.c
>> +++ b/drivers/gpu/drm/i915/intel_bios.c
>> @@ -1431,3 +1431,41 @@ intel_bios_init(struct drm_i915_private *dev_priv)
>>
>> return 0;
>> }
>> +
>> +/**
>> + * intel_bios_is_tv_present - is integrated TV present in VBT
>> + * @dev_priv: i915 device instance
>> + *
>> + * Return true if TV is present. If no child devices were parsed from VBT,
>> + * assume TV is present.
>> + */
>> +bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv)
>> +{
>> + union child_device_config *p_child;
>> + int i;
>> +
>> + if (!dev_priv->vbt.child_dev_num)
>> + return true;
>> +
> This is old code moved here, but should we return true
> when child_dev_num == 0 ? I understand we shouldn't make
> functional changes when moving code but just pointing out
> to check if this can be fixed in the further patches.
I have no idea, but I'm guessing it's there for historical reasons. No
reason to change it now as the code gets run only for some gen 3/4
hardware.
>> + for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
>> + p_child = dev_priv->vbt.child_dev + i;
>> + /*
>> + * If the device type is not TV, continue.
>> + */
>> + switch (p_child->old.device_type) {
>> + case DEVICE_TYPE_INT_TV:
>> + case DEVICE_TYPE_TV:
>> + case DEVICE_TYPE_TV_SVIDEO_COMPOSITE:
>> + break;
>> + default:
>> + continue;
>> + }
>> + /* Only when the addin_offset is non-zero, it is regarded
>> + * as present.
>> + */
>> + if (p_child->old.addin_offset)
>> + return true;
>> + }
>> +
>> + return false;
>> +}
>> diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
>> index 5034b0055169..8417fcad02d2 100644
>> --- a/drivers/gpu/drm/i915/intel_tv.c
>> +++ b/drivers/gpu/drm/i915/intel_tv.c
>> @@ -1530,47 +1530,6 @@ static const struct drm_encoder_funcs intel_tv_enc_funcs = {
>> .destroy = intel_encoder_destroy,
>> };
>>
>> -/*
>> - * Enumerate the child dev array parsed from VBT to check whether
>> - * the integrated TV is present.
>> - * If it is present, return 1.
>> - * If it is not present, return false.
>> - * If no child dev is parsed from VBT, it assumes that the TV is present.
>> - */
>> -static int tv_is_present_in_vbt(struct drm_device *dev)
>> -{
>> - struct drm_i915_private *dev_priv = dev->dev_private;
>> - union child_device_config *p_child;
>> - int i, ret;
>> -
>> - if (!dev_priv->vbt.child_dev_num)
>> - return 1;
>> -
>> - ret = 0;
>> - for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
>> - p_child = dev_priv->vbt.child_dev + i;
>> - /*
>> - * If the device type is not TV, continue.
>> - */
>> - switch (p_child->old.device_type) {
>> - case DEVICE_TYPE_INT_TV:
>> - case DEVICE_TYPE_TV:
>> - case DEVICE_TYPE_TV_SVIDEO_COMPOSITE:
>> - break;
>> - default:
>> - continue;
>> - }
>> - /* Only when the addin_offset is non-zero, it is regarded
>> - * as present.
>> - */
>> - if (p_child->old.addin_offset) {
>> - ret = 1;
>> - break;
>> - }
>> - }
>> - return ret;
>> -}
>> -
>> void
>> intel_tv_init(struct drm_device *dev)
>> {
>> @@ -1586,7 +1545,7 @@ intel_tv_init(struct drm_device *dev)
>> if ((I915_READ(TV_CTL) & TV_FUSE_STATE_MASK) == TV_FUSE_STATE_DISABLED)
>> return;
>>
>> - if (!tv_is_present_in_vbt(dev)) {
>> + if (!intel_bios_is_tv_present(dev_priv)) {
>> DRM_DEBUG_KMS("Integrated TV is not present.\n");
>> return;
>> }
> saw below line immediately after the tv_is_present_in_vbt() call.
> can you move this inside the new function as well since we
> consider it as well before enumerating tv encoder.
Makes sense.
BR,
Jani.
>
> 1593 /* Even if we have an encoder we may not have a connector */
> 1594 if (!dev_priv->vbt.int_tv_support)
> 1595 return;
> 1596
>
> regards,
> Sivakumar
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread