[PATCH] drm/i915/gvt: Init PHY related registers for BXT

[Colin Xu]

On 9/12/18 11:07 AM, Zhenyu Wang wrote:
> On 2018.09.12 10:19:05 +0800, Colin Xu wrote:
>> Recent patch fixed the call trace
>> "ERROR Port B enabled but PHY powered down? (PHY_CTL 00000000)".
>> but introduced another similar call trace shown as:
>> "ERROR Port C enabled but PHY powered down? (PHY_CTL 00000200)".
>> The call trace will appear when host and guest enabled different ports,
>> i.e. host using PORT C or neither PORT is enabled, while guest is always
>> using PORT B as simulated by gvt. The issue is actually covered previously
>> before the commit and reverals now when the commit do the right thing.
>>
>> On BXT, some PHY registers are initialized by vbios, before i915 loaded.
>> Later i915 will re-program some, or skip some based on the implementation.
>> The initialized mmio for guest i915 is done by gvt, based on the snapshot
>> taken from host. If host and guest have different PORT enabled, some
>> DPIO PHY mmios that gvt initialized for guest i915 will not match the
>> simualted monitor for guest, which leads to guest i915 print the calltrace
>> when it's trying to enable PHY and PORT.
>>
>> The solution is to init these DPIO PHY registers to default value, then
>> guest i915 will program them to reasonable value based on the default
>> powerwell table and enabled PORT. Together with the old patch, all similar
>> call trace in guest kernel on BXT can be resolved.
>>
> Nice write-up! Commit message is really meant to tell a good story,
> this one tells it clearly. And I think this is the right approach.
> Min, any comments?
>
>> Fixes: c8ab5ac30ccc ("drm/i915/gvt: Make correct handling to vreg
>> BXT_PHY_CTL_FAMILY")
> Shouldn't this be on same line?

I have to split it since the checkpatch.pl script reports long line warning due to more than 80.
Should the readability is considered prior to checkpatch result, in similar case like this one?
Sometimes I'm struggling in chosing  whether to split into multiple lines or not..

>> Signed-off-by: Colin Xu <colin.xu at intel.com>
>> ---
>>   drivers/gpu/drm/i915/gvt/display.c | 19 +++++++++++++++++++
>>   1 file changed, 19 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/gvt/display.c b/drivers/gpu/drm/i915/gvt/display.c
>> index df1e14145747..dc17f1c13d09 100644
>> --- a/drivers/gpu/drm/i915/gvt/display.c
>> +++ b/drivers/gpu/drm/i915/gvt/display.c
>> @@ -172,6 +172,25 @@ static void emulate_monitor_status_change(struct intel_vgpu *vgpu)
>>   	int pipe;
>>   
>>   	if (IS_BROXTON(dev_priv)) {
>> +		vgpu_vreg_t(vgpu, BXT_P_CR_GT_DISP_PWRON) &= ~(BIT(0) | BIT(1));
>> +		vgpu_vreg_t(vgpu, BXT_PORT_CL1CM_DW0(DPIO_PHY0)) &=
>> +			    ~PHY_POWER_GOOD;
>> +		vgpu_vreg_t(vgpu, BXT_PORT_CL1CM_DW0(DPIO_PHY1)) &=
>> +			    ~PHY_POWER_GOOD;
>> +		vgpu_vreg_t(vgpu, BXT_PHY_CTL_FAMILY(DPIO_PHY0)) &= ~BIT(30);
>> +		vgpu_vreg_t(vgpu, BXT_PHY_CTL_FAMILY(DPIO_PHY1)) &= ~BIT(30);
>> +		vgpu_vreg_t(vgpu, BXT_PHY_CTL(PORT_A)) &= ~BXT_PHY_LANE_ENABLED;
>> +		vgpu_vreg_t(vgpu, BXT_PHY_CTL(PORT_A)) |=
>> +			    BXT_PHY_CMNLANE_POWERDOWN_ACK |
>> +			    BXT_PHY_LANE_POWERDOWN_ACK;
>> +		vgpu_vreg_t(vgpu, BXT_PHY_CTL(PORT_B)) &= ~BXT_PHY_LANE_ENABLED;
>> +		vgpu_vreg_t(vgpu, BXT_PHY_CTL(PORT_B)) |=
>> +			    BXT_PHY_CMNLANE_POWERDOWN_ACK |
>> +			    BXT_PHY_LANE_POWERDOWN_ACK;
>> +		vgpu_vreg_t(vgpu, BXT_PHY_CTL(PORT_C)) &= ~BXT_PHY_LANE_ENABLED;
>> +		vgpu_vreg_t(vgpu, BXT_PHY_CTL(PORT_C)) |=
>> +			    BXT_PHY_CMNLANE_POWERDOWN_ACK |
>> +			    BXT_PHY_LANE_POWERDOWN_ACK;
>>   		vgpu_vreg_t(vgpu, GEN8_DE_PORT_ISR) &= ~(BXT_DE_PORT_HP_DDIA |
>>   			BXT_DE_PORT_HP_DDIB |
>>   			BXT_DE_PORT_HP_DDIC);
>> -- 
>> 2.18.0
>>
>> _______________________________________________
>> intel-gvt-dev mailing list
>> intel-gvt-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev

-- 
Best Regards,
Colin Xu