Re: [PATCH 1/6] drm/i915/bxt: VBT changes for hpd as wakeup feature

[Animesh Manna <animesh.manna@intel.com>]

On 4/5/2016 7:29 PM, Jani Nikula wrote:
> On Tue, 05 Apr 2016, Animesh Manna <animesh.manna@intel.com> wrote:
>> To support hpd during sleep a new feature flag is
>> added in vbt and also in dev_priv for enabling/disabling
>> inside deiver. By default this feature will be
>> diabled and based on oem request this feature can
>> be enabled by changing vbt feature flag.
>>
>> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
>> Signed-off-by: A.Sunil Kamath <sunil.kamath@intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_drv.h       | 8 ++++++++
>>   drivers/gpu/drm/i915/i915_reg.h       | 1 +
>>   drivers/gpu/drm/i915/intel_vbt_defs.h | 3 ++-
>>   3 files changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index dd18772..445b80b 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -1429,6 +1429,11 @@ enum psr_lines_to_wait {
>>   	PSR_8_LINES_TO_WAIT
>>   };
>>   
>> +enum hpd_wakeup_state {
>> +	DISABLE_HOT_PLUG_AS_WAKE_EVENT = 0,
>> +	ENABLE_HOT_PLUG_AS_WAKE_EVENT
>> +};
> Any reason to use an enum when a bool will do? In fact, you use it as a
> bool in the following patches.

Thanks Jani for review.
Agree, previously we are using more than 2 state so used enum ... but after
design discussion concluded to have only two state and bool can be used
instead of enum. Will update in my next patchset.

>
>> +
>>   struct intel_vbt_data {
>>   	struct drm_display_mode *lfp_lvds_vbt_mode; /* if any */
>>   	struct drm_display_mode *sdvo_lvds_vbt_mode; /* if any */
>> @@ -1485,6 +1490,9 @@ struct intel_vbt_data {
>>   		const u8 *sequence[MIPI_SEQ_MAX];
>>   	} dsi;
>>   
>> +	/* HPD as wakesoure for DC9 BXT */
>> +	enum hpd_wakeup_state hpd_wakeup_enabled;
>> +
> Why don't you initialize the field in this patch? Patch 6/6 should be
> part of this patch.

Don't want to enable this feature at the first place, bcoz subsequent patches
will have the actual feature implementation, but as we have vbt control we can
add here, will update in next patchset.

>
>>   	int crt_ddc_pin;
>>   
>>   	int child_dev_num;
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index 12f5103..cc42bd9 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -6119,6 +6119,7 @@ enum skl_disp_power_wells {
>>   				 SDE_PORTB_HOTPLUG |	\
>>   				 SDE_PORTC_HOTPLUG |	\
>>   				 SDE_PORTD_HOTPLUG)
>> +
> Superfluous whitespace change.

Will correct it.

>
>>   #define SDE_TRANSB_CRC_DONE	(1 << 5)
>>   #define SDE_TRANSB_CRC_ERR	(1 << 4)
>>   #define SDE_TRANSB_FIFO_UNDER	(1 << 3)
>> diff --git a/drivers/gpu/drm/i915/intel_vbt_defs.h b/drivers/gpu/drm/i915/intel_vbt_defs.h
>> index 749dcea..8e2b765 100644
>> --- a/drivers/gpu/drm/i915/intel_vbt_defs.h
>> +++ b/drivers/gpu/drm/i915/intel_vbt_defs.h
>> @@ -547,7 +547,8 @@ struct bdb_driver_features {
>>   	u16 tbt_enabled:1;
>>   	u16 psr_enabled:1;
>>   	u16 ips_enabled:1;
>> -	u16 reserved3:4;
>> +	u16 reserved3:3;
>> +	u16 hpd_wakeup_source:1;
>>   	u16 pc_feature_valid:1;
>>   } __packed;

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