Re: [PATCH 3/5] drm/i915/bxt: Added _DSM call to set HPD_CTL.

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Mon, Nov 28, 2016 at 09:36:51PM +0530, Animesh Manna wrote:
> 
> 
> On 11/23/2016 11:47 PM, Ville Syrjälä wrote:
> > On Wed, Nov 23, 2016 at 09:48:25PM +0530, Animesh Manna wrote:
> >> _DSM is added to program HPD_CTL(0x1094) register
> >> of PMC from i915 driver which will be called
> >> based on driver feature flag. PMC hpd control register
> >> programming will enable PMC to get hpd interrupt
> >> during dc9.
> >>
> >> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> >> ---
> >>   drivers/gpu/drm/i915/intel_acpi.c | 44 ++++++++++++++++++++++++++++++++-------
> >>   1 file changed, 37 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_acpi.c b/drivers/gpu/drm/i915/intel_acpi.c
> >> index 8c878ab..15d3b84 100755
> >> --- a/drivers/gpu/drm/i915/intel_acpi.c
> >> +++ b/drivers/gpu/drm/i915/intel_acpi.c
> >> @@ -10,6 +10,8 @@
> >>   
> >>   #define INTEL_DSM_REVISION_ID 1 /* For Calpella anyway... */
> >>   #define INTEL_DSM_FN_PLATFORM_MUX_INFO 1 /* No args */
> >> +#define INTEL_DSM_SET_HPD_WAKEUP 17
> >> +#define HPD_WAKEUP_EN_VAL 0xFCF0
> >>   
> >>   static struct intel_dsm_priv {
> >>   	acpi_handle dhandle;
> >> @@ -118,6 +120,25 @@ static void intel_dsm_platform_mux_info(void)
> >>   	ACPI_FREE(pkg);
> >>   }
> >>   
> >> +static void intel_dsm_set_hpd_wakeup(u8 *guid)
> >> +{
> >> +	union acpi_object *obj;
> >> +	union acpi_object argv4 = {
> >> +		.integer.type = ACPI_TYPE_INTEGER,
> >> +		.integer.value = HPD_WAKEUP_EN_VAL,
> >> +	};
> >> +
> >> +	obj = acpi_evaluate_dsm_typed(intel_dsm_priv.dhandle, guid,
> >> +			INTEL_DSM_REVISION_ID, INTEL_DSM_SET_HPD_WAKEUP,
> >> +			&argv4, ACPI_TYPE_INTEGER);
> >> +
> >> +	if (!obj)
> >> +		DRM_DEBUG_DRIVER("failed to evaluate _DSM\n");
> >> +
> >> +	ACPI_FREE(obj);
> >> +}
> >> +
> >> +
> >>   static bool intel_dsm_pci_probe(struct pci_dev *pdev)
> >>   {
> >>   	acpi_handle dhandle;
> >> @@ -134,14 +155,23 @@ static bool intel_dsm_pci_probe(struct pci_dev *pdev)
> >>   	else
> >>   		guid = intel_dsm_guid;
> >>   
> >> -	if (!acpi_check_dsm(dhandle, guid, INTEL_DSM_REVISION_ID,
> >> -			    1 << INTEL_DSM_FN_PLATFORM_MUX_INFO)) {
> >> -		DRM_DEBUG_KMS("no _DSM method for intel device\n");
> >> -		return false;
> >> -	}
> >> -
> >>   	intel_dsm_priv.dhandle = dhandle;
> >> -	intel_dsm_platform_mux_info();
> >> +
> >> +	if (acpi_check_dsm(dhandle, guid, INTEL_DSM_REVISION_ID,
> >> +				1 << INTEL_DSM_FN_PLATFORM_MUX_INFO))
> >> +		intel_dsm_platform_mux_info();
> >> +	else
> >> +		DRM_DEBUG_KMS("no _DSM method for mux-info\n");
> >> +
> >> +	/* Need to ensure vbt parsing is completed. */
> > Eh?
> There is a feature flag for oem to enable/disable HPD as wake feature in 
> vbt and on the basis of that PMC_HPD_CTL register will be programmed.
> So added a comment just to have that info captured in code-comment, for 
> any concern let me know, will update accordingly.

The comment is making me thing the VBT parsing would happen
asynchronously somehow, and this would need then to block until it's
finsiehd. Doesn't make any sense to me.

> >
> >> +	if (dev_priv->vbt.hpd_wakeup_enabled &&
> >> +		acpi_check_dsm(dhandle, guid, INTEL_DSM_REVISION_ID,
> >> +				1 << INTEL_DSM_SET_HPD_WAKEUP))
> >> +		intel_dsm_set_hpd_wakeup(guid);
> > So we're permanently routing hpds to the pcu? Won't that mess up
> > stuff like short pulse detection?
> >
> > I was expecting that we'd switch between the PCU and not during
> > runtime suspend/resume.
> Routing hpd to the pcu will only happen during S0ix. As per my 
> understanding for both HDMI and DP we will be getting long pulse during 
> hotplug at S0ix. So I think it will not mess up.
> Correct me if I am missing something.

I have no idea. I don't have the hardware to verify any of that. Please
do that yourself, and add a convicing comment/commit message with your
findings documented.

> 
> - Animesh
> >
> >> +	else {
> >> +		dev_priv->vbt.hpd_wakeup_enabled = false;
> >> +		DRM_DEBUG_KMS("no _DSM method for hpd-enabling\n");
> >> +	}
> >>   
> >>   	return true;
> >>   }
> >> -- 
> >> 1.9.1
> >>
> >> _______________________________________________
> >> Intel-gfx mailing list
> >> Intel-gfx@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx