Re: [PATCH v6 3/5] drm/i915: Parse VBT data for lspcon

[Jani Nikula <jani.nikula@linux.intel.com>]

On Fri, 14 Oct 2016, "Sharma, Shashank" <shashank.sharma@intel.com> wrote:
> Regards
>
> Shashank
>
>
> On 10/14/2016 8:02 PM, Jani Nikula wrote:
>> On Fri, 14 Oct 2016, Shashank Sharma <shashank.sharma@intel.com> wrote:
>>> Many GEN9 boards come with on-board lspcon cards.
>>> Fot these boards, VBT configuration should properly point out
>>> if a particular port contains lspcon device, so that driver can
>>> initialize it properly.
>>>
>>> This patch adds a utility function, which checks the VBT flag
>>> for lspcon bit, and tells us if a port is configured to have a
>>> lspcon device or not.
>>>
>>> V2: Fixed review comments from Ville
>>> - Do not forget PORT_D while checking lspcon for GEN9
>>>
>>> V3: Addressed review comments from Rodrigo
>>> - Create a HAS_LSPCON() macro for better use case handling.
>>> - Do not dump warnings for non-gen-9 platforms, it will be noise.
>>>
>>> V4: Rebase
>>> V5: Rebase
>>> V6: Pass dev_priv to HAS_LSPCON() macro
>>>
>>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>>> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> I was hoping you'd use the version I rebased and sent, put it first in
>> the series, and rebase the rest on that. The point is, this series has
>> taken so long that lspcon devices have proliferated all over the place,
>> and we'll be getting more and more bugs about them. If this patch was
>> first, with the debug logging, we could at least get that to 4.9, maybe
>> stable kernels, and we'd immediately know the reason. I think it'll be a
>> hard sell to get the whole series to 4.9 kernel.
>>
>> BR,
>> Jani.
> Jani,
> The patch got its first r-b since a long time.
> After that, it was waiting to be merged, for long time.
>
> Recently, when Imre was asked to test the patches, and he found one 
> issue specific to APL.
> We were trying to fix a suspend-resume issue, which was fixed with the 
> last patch.
> Now this patch is ready to be merged, just waiting for Imre's r-b.
>
> Third patch just gives information about if LSPCON is available or not, 
> which is not a big help for anything as such.
> So instead of changing the sequence, and confusing the reviewers, I 
> thought it would be better to send the whole series and
> get this merged as-it-is.

Fine, let's merge this as-is... after patch 1/5 has been posted to
dri-devel and/or has received an ack from Dave Airlie that it's fine to
merge through our tree.

In the bigger scheme of things, if this patch 3/5 had been first in the
series all along, we could have merged this *months* ago. This is how
series should be organized. Simple things first.

Having the debug information *is* valuable. You'd see that if you had to
go through our incoming bugs. We've had plenty of LSPCON bugs since the
day Skylake was launched. Yes, quite a long time now. If 3/5 was first
in the series, we could backport that to v4.9 and older and reduce our
debugging time.

BR,
Jani.






>
> Regards
> Shashank
>>
>>> ---
>>>   drivers/gpu/drm/i915/i915_drv.h   |  5 ++++
>>>   drivers/gpu/drm/i915/intel_bios.c | 49 +++++++++++++++++++++++++++++++++++++++
>>>   2 files changed, 54 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>>> index fe875b2..7bab2f1 100644
>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>> @@ -2864,6 +2864,8 @@ struct drm_i915_cmd_table {
>>>   
>>>   #define HAS_GMCH_DISPLAY(dev_priv) ((dev_priv)->info.has_gmch_display)
>>>   
>>> +#define HAS_LSPCON(dev_priv) (IS_GEN9(dev_priv))
>>> +
>>>   /* DPF == dynamic parity feature */
>>>   #define HAS_L3_DPF(dev_priv) ((dev_priv)->info.has_l3_dpf)
>>>   #define NUM_L3_SLICES(dev_priv) (IS_HSW_GT3(dev_priv) ? \
>>> @@ -3631,6 +3633,9 @@ bool intel_bios_is_port_dp_dual_mode(struct drm_i915_private *dev_priv, enum por
>>>   bool intel_bios_is_dsi_present(struct drm_i915_private *dev_priv, enum port *port);
>>>   bool intel_bios_is_port_hpd_inverted(struct drm_i915_private *dev_priv,
>>>   				     enum port port);
>>> +bool intel_bios_is_lspcon_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 83667e8..32e1def 100644
>>> --- a/drivers/gpu/drm/i915/intel_bios.c
>>> +++ b/drivers/gpu/drm/i915/intel_bios.c
>>> @@ -1763,3 +1763,52 @@ intel_bios_is_port_hpd_inverted(struct drm_i915_private *dev_priv,
>>>   
>>>   	return false;
>>>   }
>>> +
>>> +/**
>>> + * intel_bios_is_lspcon_present - if LSPCON is attached on %port
>>> + * @dev_priv:	i915 device instance
>>> + * @port:	port to check
>>> + *
>>> + * Return true if LSPCON is present on this port
>>> + */
>>> +bool
>>> +intel_bios_is_lspcon_present(struct drm_i915_private *dev_priv,
>>> +				enum port port)
>>> +{
>>> +	int i;
>>> +
>>> +	if (!HAS_LSPCON(dev_priv))
>>> +		return false;
>>> +
>>> +	for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
>>> +		if (!dev_priv->vbt.child_dev[i].common.lspcon)
>>> +			continue;
>>> +
>>> +		switch (dev_priv->vbt.child_dev[i].common.dvo_port) {
>>> +		case DVO_PORT_DPA:
>>> +		case DVO_PORT_HDMIA:
>>> +			if (port == PORT_A)
>>> +				return true;
>>> +			break;
>>> +		case DVO_PORT_DPB:
>>> +		case DVO_PORT_HDMIB:
>>> +			if (port == PORT_B)
>>> +				return true;
>>> +			break;
>>> +		case DVO_PORT_DPC:
>>> +		case DVO_PORT_HDMIC:
>>> +			if (port == PORT_C)
>>> +				return true;
>>> +			break;
>>> +		case DVO_PORT_DPD:
>>> +		case DVO_PORT_HDMID:
>>> +			if (port == PORT_D)
>>> +				return true;
>>> +			break;
>>> +		default:
>>> +			break;
>>> +		}
>>> +	}
>>> +
>>> +	return false;
>>> +}
>

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