On Tue, 20 Dec 2016, Imre Deak <imre.deak@xxxxxxxxx> wrote:
> On ti, 2016-12-20 at 18:02 +0200, David Weinehall wrote:
>> On Tue, Dec 20, 2016 at 05:12:49PM +0200, Imre Deak wrote:
>> > There is at least one APL based system using port A in DP mode
>> > (connecting to an on-board DP->VGA adaptor). Atm we'll configure port A
>> > unconditionally as eDP which is incorrect in this case. Fix this by
>> > relying on the VBT DDI port 'internal port' flag instead on all ports
>> > GEN9 onwards. For now chicken out from doing this on earlier DDI
>> > platforms too.
>> >
>> > Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx>
>> > ---
>> > drivers/gpu/drm/i915/i915_drv.h | 1 +
>> > drivers/gpu/drm/i915/intel_bios.c | 1 +
>> > drivers/gpu/drm/i915/intel_dp.c | 3 +++
>> > 3 files changed, 5 insertions(+)
>> >
>> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> > index 7e94e506..42fb7e9 100644
>> > --- a/drivers/gpu/drm/i915/i915_drv.h
>> > +++ b/drivers/gpu/drm/i915/i915_drv.h
>> > @@ -1669,6 +1669,7 @@ struct ddi_vbt_port_info {
>> > uint8_t supports_dvi:1;
>> > uint8_t supports_hdmi:1;
>> > uint8_t supports_dp:1;
>> > + uint8_t supports_edp:1;
>> >
>> > uint8_t alternate_aux_channel;
>> > uint8_t alternate_ddc_pin;
>> > diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
>> > index 1cf2fa6..60fdd31 100644
>> > --- a/drivers/gpu/drm/i915/intel_bios.c
>> > +++ b/drivers/gpu/drm/i915/intel_bios.c
>> > @@ -1161,6 +1161,7 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
>> > info->supports_dvi = is_dvi;
>> > info->supports_hdmi = is_hdmi;
>> > info->supports_dp = is_dp;
>> > + info->supports_edp = is_edp;
>> >
>> > DRM_DEBUG_KMS("Port %c VBT info: DP:%d HDMI:%d DVI:%d EDP:%d CRT:%d\n",
>> > port_name(port), is_dp, is_hdmi, is_dvi, is_edp, is_crt);
>> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> > index d37cb00..7f3f4fa 100644
>> > --- a/drivers/gpu/drm/i915/intel_dp.c
>> > +++ b/drivers/gpu/drm/i915/intel_dp.c
>> > @@ -4983,6 +4983,9 @@ bool intel_dp_is_edp(struct drm_i915_private *dev_priv, enum port port)
>> > if (INTEL_GEN(dev_priv) < 5)
>> > return false;
>> >
>> > + if (INTEL_GEN(dev_priv) >= 9)
>> > + return dev_priv->vbt.ddi_port_info[port].supports_edp;
>> > +
>>
>> We already have intel_bios_is_port_edp() that tries to
>> discern whether the port is eDP or not.
>>
>> Wouldn't that function be usable for this purpose?
>>
>> If so, doing:
>>
>> if (port == PORT_A && INTEL_GEN(dev_priv) < 9)
>> return true;
>>
>> return intel_bios_is_port_edp(dev_priv, port);
>>
>> should be enough. If not, maybe it should be fixed instead of
>> introducing a new way of doing things.
>
> intel_bios_is_port_edp() does on old platforms what parse_ddi_port()
> for DDI platforms. I find it clearer to have all the platform checks in
> one place in intel_dp_is_edp() to decide which info to use.
>
> We could move the parsing in intel_bios_is_port_edp() to happen during
> the early VBT parsing on old platforms and use the DDI port info for
> those too not sure if it's feasible or if it's worth the effort.
IMO everything that looks at VBT for the info should be abstracted into
intel_bios_is_port_edp(). I don't like to spread out things like, if
this then look at VBT here, else look at VBT there, or something else.
BR,
Jani.
>
> --Imre
>
>>
>> > if (port == PORT_A)
>> > return true;
>>
>>
>> Kind regards, David
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
