On Mon, 08 Feb 2016, "Thulasimani, Sivakumar" <[email protected]> 
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 <[email protected]>
>> ---
>>   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
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Reply via email to