On Fri, 2017-06-16 at 21:26 +0000, Vivi, Rodrigo wrote:
> On Fri, 2017-06-16 at 21:21 +0000, Pandiyan, Dhinakaran wrote:
> > On Fri, 2017-06-16 at 14:12 -0700, Manasi Navare wrote:
> > > On Fri, Jun 16, 2017 at 08:58:25PM +0000, Pandiyan, Dhinakaran wrote:
> > > > On Fri, 2017-06-16 at 13:20 -0700, Rodrigo Vivi wrote:
> > > > > This missing case could be reached out on missing type or
> > > > > missing voltage.
> > > >
> > > > Should we even reach this far with a missing DDI type?
> > > >
> > > > -DK
> > > >
> > >
> > > Yes it is possible that we get into this situation if reading of
> > > the vccio voltage from PORT_COMP_DW3 returns a garbage value due
> > > to some corruption
> >
> > MISSING_CASE already logs that.
> >
> > > or lets say the type is something that is not supported on this
> > > platform.
> > >
> > But my question is, should we even be trying to program vswing
> > without knowing the ddi type or for an invalid type?
>
> we will reach the return; and avoid vswing programming.
> And logs will give us some information of what failed during modeset.
>
Correct me if I am wrong, the only callers I could see are
intel_dp_set_signal_levels() and intel_ddi_pre_enable_hdmi(). In both cases, the encoder type should have been set appropriately. My concern is that if we are seeing some other type, we should be handling that before calling cnl_ddi_vswing_program()
But this debug message catches the condition where the type is correct but the type and corresponding vccio voltage
Combination is wrong and the ddi tables are not present for that combination.
I definitely think it is a good idea to have this Debug print here in the missing case.
Manasi
> >
> > >
> > > > > So let's add a debug
> > > > > message to make our lives easier whenever this might happen.
> > > > >
> > > > > Cc: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
> > > > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> > > > > ---
> > > > > drivers/gpu/drm/i915/intel_ddi.c | 1 +
> > > > > 1 file changed, 1 insertion(+)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> > > > > b/drivers/gpu/drm/i915/intel_ddi.c
> > > > > index e66947a..c96c8d0 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > > > > @@ -1802,6 +1802,7 @@ static void cnl_ddi_vswing_program(struct drm_i915_private *dev_priv,
> > > > > }
> > > > >
> > > > > if (ddi_translations == NULL) {
> > > > > + DRM_DEBUG_KMS("Missing DDI translation table for type %d
> > > > > +with voltage %d\n", type, voltage);
> > > > > MISSING_CASE(voltage);
> > > > > return;
> > > > > }
> > >
> > > Like Paulo suggested it would be better to use a switch statement
> > > for different vccio voltages and then have this in the default. What do you think?
>
> we don't need to iterate on voltages... and the switch for type
> wouldn't catch failures happening inside the functions where we
> actually pick the table.
>
> > >
> > > Manasi
> > >
> > >
> > > >
> > > > _______________________________________________
> > > > Intel-gfx mailing list
> > > > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
