On Thu, 13 Aug 2015, "Jindal, Sonika" <sonika.jindal@xxxxxxxxx> wrote: > On 8/13/2015 8:57 AM, Zhang, Xiong Y wrote: >>> On Wed, 2015-08-12 at 02:20 +0000, Zhang, Xiong Y wrote: >>>>> On Tue, 2015-08-11 at 07:05 +0000, Zhang, Xiong Y wrote: >>>>>>> -----Original Message----- >>>>>>> From: Vivi, Rodrigo >>>>>>> Sent: Saturday, August 8, 2015 8:34 AM >>>>>>> To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx >>>>>>> Cc: Vivi, Rodrigo; Zhang, Xiong Y >>>>>>> Subject: [PATCH 7/6] drm/i915/skl: DDI-E and DDI-A shares 4 >>>>>>> lanes. >>>>>>> >>>>>>> DDI-A and DDI-E shares the 4 lanes. So when DDI-E is present we >>>>>>> need to configure lane count propperly for both. >>>>>>> >>>>>>> This was based on Sonika's >>>>>>> [PATCH] drm/i915/skl: Select DDIA lane capability based upon vbt >>>>>>> >>>>>>> Credits-to: Sonika Jindal <sonika.jindal@xxxxxxxxx> >>>>>>> Cc: Xiong Zhang <xiong.y.zhang@xxxxxxxxx> >>>>>>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> >>>>>>> --- >>>>>>> drivers/gpu/drm/i915/intel_ddi.c | 12 ++++++++++-- >>>>>>> drivers/gpu/drm/i915/intel_dp.c | 8 +++++--- >>>>>>> 2 files changed, 15 insertions(+), 5 deletions(-) >>>>>>> >>>>>>> diff --git a/drivers/gpu/drm/i915/intel_ddi.c >>>>>>> b/drivers/gpu/drm/i915/intel_ddi.c >>>>>>> index 110d546..557cecf 100644 >>>>>>> --- a/drivers/gpu/drm/i915/intel_ddi.c >>>>>>> +++ b/drivers/gpu/drm/i915/intel_ddi.c >>>>>>> @@ -3178,7 +3178,15 @@ void intel_ddi_init(struct drm_device >>>>>>> *dev, enum port port) >>>>>>> struct intel_digital_port *intel_dig_port; >>>>>>> struct intel_encoder *intel_encoder; >>>>>>> struct drm_encoder *encoder; >>>>>>> - bool init_hdmi, init_dp; >>>>>>> + bool init_hdmi, init_dp, ddi_e_present; >>>>>>> + >>>>>>> + /* >>>>>>> + * On SKL we don't have a way to detect DDI-E so we >>>>>>> rely >>>>>>> on VBT. >>>>>>> + */ >>>>>>> + ddie_present = IS_SKYLAKE(dev) && >>>>>>> + (dev_priv >>>>>>> ->vbt.ddi_port_info[PORT_E].supports_dp >>>>>>>>> >>>>>>> + dev_priv >>>>>>> ->vbt.ddi_port_info[PORT_E].supports_dvi >>>>>>>>> >>>>>>> + dev_priv >>>>>>> ->vbt.ddi_port_info[PORT_E].supports_hdmi); >>>>>> [Zhang, Xiong Y] ddie_present should be ddi_e_present >>>>>>> >>>>>>> init_hdmi = (dev_priv >>>>>>> ->vbt.ddi_port_info[port].supports_dvi || >>>>>>> dev_priv >>>>>>> ->vbt.ddi_port_info[port].supports_hdmi); >>>>>>> @@ -3210,7 +3218,7 @@ void intel_ddi_init(struct drm_device >>>>>>> *dev, enum port port) >>>>>>> intel_dig_port->port = port; >>>>>>> intel_dig_port->saved_port_bits = >>>>>>> I915_READ(DDI_BUF_CTL(port)) & >>>>>>> >>>>>>> (DDI_BUF_PORT_REVERSAL | >>>>>>> - DDI_A_4_LANES); >>>>>>> + ddi_e_present ? 0 : >>>>>>> DDI_A_4_LANES); >>>>>> [Zhang, Xiong Y] Sonika's patch will set DDI-A to 4 lanes when >>>>>> DDI-E doesn't exist, I think your patch will do nothing. >>>>> >>>>> Actually DDI_A_4_LANES is being already set unconditionally, so >>>>> Sonika's patch has no effect. >>>> [Zhang, Xiong Y] No. Sonika's patch set DDI_A_4_LANES under many >>>> conditions. >>>> + if (IS_SKYLAKE(dev) && port == PORT_A >>>> + && !(val & DDI_BUF_CTL_ENABLE) >>>> + && !dev_priv->vbt.ddi_e_used) >>>> + I915_WRITE(DDI_BUF_CTL(port), val | DDI_A_4_LANES) >>>>> >>>>> saved_port_bits goes to intel_dp->DP that goes to DDI_BUF_CTL and >>>>> also it is used to calculate the number of lanes. >>>>> >>>>> With this patch we stop setting DDI_A_4_LANES when ddi_e is present >>>>> so DDI-A keeps with 2 lanes and let other 2 lanes for DDI-E >>>> [Zhang, Xiong Y] Yes, this patch will clear DDI_A_4_LANES when ddi_e >>>> is present. >>>> But this patch won't set DDI_A_4_LANES under following conditions >>>> which is purpose for Sonika patch 1. Bios fail to driver eDP and >>>> doesn't enable DDI_A buffer >>> >>> If DDI_A isn't enabled we don't need to set DDI_A_4_LANES >> [Zhang, Xiong Y] From commit message on Sonika patch, she want to >> set DDI_A_4_LANES on such case. Maybe she met such fail case on one high >> resolution eDP screen. Let's Sonikia explain it. >>> >>>> 2. Bios clear DDI_A_4_LANES >>>> 3. DDI_E isn't present >>> >>> I don't agree... This is already covered on current code. DDI_A_4_LANES is >>> already being set when enabling DDI_A. >>> > As Zhang mentioned and as my commit message explains, my patch is needed > when bios failed to drive edp (In my case it was an intermediate > frequency supported panel which was set to 3.24 GHz and bios didn't have > support for intermediate frequencies), it will not enable DDIA in which > case, it will not set DDI_BUF_CTL and DDI Lane capability will remain 0 > (which is DDIA with 2 lanes and DDIE with 2 lanes). > So, since the native resolution of that panel was high and couldn't work > with 2 lanes. > So, ideally we should not rely on bios to set the initial value and set > it based upon whether DDI_E is used or not. > So, my patch has some effect :) Rodrigo? Please figure out what the needed patch is, and send it. BR, Jani. >>> >>>> >>>> thanks >> > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
