Re: [PATCH 1/5] drm/i915/bxt: Corrected the guid for bxt.

From: Animesh Manna <animesh.manna@intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>,
	intel-gfx@lists.freedesktop.org,
	Bharath K Veera <bharath.k.veera@intel.com>,
	Ananth Krishna R <ananth.krishna.r@intel.com>
Subject: Re: [PATCH 1/5] drm/i915/bxt: Corrected the guid for bxt.
Date: Mon, 28 Nov 2016 16:26:16 +0530	[thread overview]
Message-ID: <00cc7cf9-dde0-bc3e-e5a4-da3231d02b59@intel.com> (raw)
In-Reply-To: <20161123163221.GB17701@nuc-i3427.alporthouse.com>


On 11/23/2016 10:02 PM, Chris Wilson wrote:
> On Wed, Nov 23, 2016 at 09:48:23PM +0530, Animesh Manna wrote:
>> Guid is changed for bxt platform, so corrected the guid for bxt.
>>
>> v1: Initial version as RFC.
>>
>> v2: Based on review comment from Jani and David,
>> have kept guid as binary format.
>>
>> Signed-off-by: Ananth Krishna R <ananth.krishna.r@intel.com>
>> Signed-off-by: Bharath K Veera <bharath.k.veera@intel.com>
>> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_acpi.c | 20 ++++++++++++++++++--
>>   1 file changed, 18 insertions(+), 2 deletions(-)
>>   mode change 100644 => 100755 drivers/gpu/drm/i915/intel_acpi.c
>>
>> diff --git a/drivers/gpu/drm/i915/intel_acpi.c b/drivers/gpu/drm/i915/intel_acpi.c
>> old mode 100644
>> new mode 100755
> 3 people handled this patch and none complained about making
> intel_acpi.c executable? What does happen when you try to execute it?
oh, will correct it in the next patchset.
>
>> index eb638a1..8c878ab
>> --- a/drivers/gpu/drm/i915/intel_acpi.c
>> +++ b/drivers/gpu/drm/i915/intel_acpi.c
>> @@ -15,7 +15,7 @@
>>   	acpi_handle dhandle;
>>   } intel_dsm_priv;
>>   
>> -static const u8 intel_dsm_guid[] = {
>> +static u8 intel_dsm_guid[] = {
> Why drop the const?
intel_dsm_guid is not updated anywhere, it used to assign it to a common 
pointer based on platform check in my current implementation.
we can explicitly typecast to avoid compilation warning to a normal 
pointer which will be used during dsm probe. Hope it will be fine.
Please let me know for any concern/suggestion.

>
>>   	0xd3, 0x73, 0xd8, 0x7e,
>>   	0xd0, 0xc2,
>>   	0x4f, 0x4e,
>> @@ -23,6 +23,14 @@
>>   	0x0f, 0x13, 0x17, 0xb0, 0x1c, 0x2c
>>   };
>>   
>> +static u8 intel_dsm_guid_bxt[] = {
> Missing const.
Explained above.
>
>> +	0xc6, 0x41, 0x5b, 0x3e,
>> +	0x1d, 0xeb,
>> +	0x60, 0x42,
>> +	0x9d, 0x15,
>> +	0xc7, 0x1f, 0xba, 0xda, 0xe4, 0x14
>> +};
>> +
>>   static char *intel_dsm_port_name(u8 id)
>>   {
>>   	switch (id) {
>> @@ -113,12 +121,20 @@ static void intel_dsm_platform_mux_info(void)
>>   static bool intel_dsm_pci_probe(struct pci_dev *pdev)
>>   {
>>   	acpi_handle dhandle;
>> +	struct drm_device *dev = pci_get_drvdata(pdev);
>> +	struct drm_i915_private *dev_priv = dev->dev_private;
> dev == dev_priv, just a rose by another name. Use to_i915();
>
>> +	u8 *guid;
> Missing const.
Explained above.
>
>>   
>>   	dhandle = ACPI_HANDLE(&pdev->dev);
>>   	if (!dhandle)
>>   		return false;
>>   
>> -	if (!acpi_check_dsm(dhandle, intel_dsm_guid, INTEL_DSM_REVISION_ID,
>> +	if (IS_BROXTON(dev_priv))
>> +		guid = intel_dsm_guid_bxt;
>> +	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;
>> -- 
>> 1.9.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx