[PATCH] drm/i915/cnl: Add information to missing case.

[PATCH] drm/i915/cnl: Add information to missing case.

[Rodrigo Vivi]

This missing case could be reached out on missing
type or missing voltage. So let's add a debug
message to make our lives easier whenever this
might happen.

Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 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;
 	}
-- 
1.9.1

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

Re: [PATCH] drm/i915/cnl: Add information to missing case.

[Pandiyan, Dhinakaran]

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

>  So let's add a debug
> message to make our lives easier whenever this
> might happen.
> 
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  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;
>  	}

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

Re: [PATCH] drm/i915/cnl: Add information to missing case.

[Manasi Navare]

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 or lets say the type is something that is not
supported on this platform.


> >  So let's add a debug
> > message to make our lives easier whenever this
> > might happen.
> > 
> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > ---
> >  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?

Manasi


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

Re: [PATCH] drm/i915/cnl: Add information to missing case.

[Pandiyan, Dhinakaran]

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? 

> 
> > >  So let's add a debug
> > > message to make our lives easier whenever this
> > > might happen.
> > > 
> > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > ---
> > >  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?
> 
> Manasi
> 
> 
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH] drm/i915/cnl: Add information to missing case.

[Vivi, Rodrigo]

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.

> 
> > 
> > > >  So let's add a debug
> > > > message to make our lives easier whenever this
> > > > might happen.
> > > > 
> > > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > ---
> > > >  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@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 

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

✓ Fi.CI.BAT: success for drm/i915/cnl: Add information to missing case.

[Patchwork]

== Series Details ==

Series: drm/i915/cnl: Add information to missing case.
URL   : https://patchwork.freedesktop.org/series/25937/
State : success

== Summary ==

Series 25937v1 drm/i915/cnl: Add information to missing case.
https://patchwork.freedesktop.org/api/1.0/series/25937/revisions/1/mbox/

Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-uc:
                pass       -> FAIL       (fi-snb-2600) fdo#100007
Test gem_exec_suspend:
        Subgroup basic-s4-devices:
                dmesg-warn -> PASS       (fi-kbl-r) fdo#100125
Test kms_cursor_legacy:
        Subgroup basic-busy-flip-before-cursor-atomic:
                pass       -> FAIL       (fi-snb-2600) fdo#100215 +1

fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007
fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125
fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time:467s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time:483s
fi-bsw-n3050     total:278  pass:242  dwarn:0   dfail:0   fail:0   skip:36  time:574s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:541s
fi-byt-j1900     total:278  pass:254  dwarn:0   dfail:0   fail:0   skip:24  time:487s
fi-byt-n2820     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:495s
fi-glk-2a        total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:587s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:437s
fi-hsw-4770r     total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:16  time:417s
fi-ilk-650       total:278  pass:227  dwarn:0   dfail:0   fail:0   skip:50  time:463s
fi-ivb-3520m     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:18  time:498s
fi-ivb-3770      total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:18  time:516s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:470s
fi-kbl-7560u     total:278  pass:267  dwarn:1   dfail:0   fail:0   skip:10  time:566s
fi-kbl-r         total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:576s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:474s
fi-skl-6700hq    total:278  pass:224  dwarn:5   dfail:16  fail:11  skip:22  time:474s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time:508s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:503s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time:506s
fi-snb-2520m     total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:28  time:621s
fi-snb-2600      total:278  pass:246  dwarn:0   dfail:0   fail:2   skip:29  time:412s

473e5a3f4cc108756ffeeb6f4ccf9337e9050648 drm-tip: 2017y-06m-16d-20h-10m-15s UTC integration manifest
bdc418d drm/i915/cnl: Add information to missing case.

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4976/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/cnl: Add information to missing case.

[Pandiyan, Dhinakaran]

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()



> > 
> > > 
> > > > >  So let's add a debug
> > > > > message to make our lives easier whenever this
> > > > > might happen.
> > > > > 
> > > > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > > > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > > ---
> > > > >  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@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH] drm/i915/cnl: Add information to missing case.

[Navare, Manasi D]

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@intel.com>
> > > > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > > ---
> > > > >  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@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH] drm/i915/cnl: Add information to missing case.

[Vivi, Rodrigo]

On Fri, 2017-06-16 at 21:54 +0000, Navare, Manasi D wrote:
> 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()

this is why I didn't get the missing case on the type if/else ;)

we should never reach that case, but in case there is the missing case
an extra debug message doesn't hurt...

> 
> 
> 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@intel.com>
> > > > > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > > > ---
> > > > > >  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@lists.freedesktop.org
> > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > > _______________________________________________
> > > > Intel-gfx mailing list
> > > > Intel-gfx@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > 
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH] drm/i915/cnl: Add information to missing case.

[Dhinakaran Pandiyan]

On Friday, June 16, 2017 10:18:28 PM PDT Vivi, Rodrigo wrote:
> On Fri, 2017-06-16 at 21:54 +0000, Navare, Manasi D wrote:
> 
> > 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()
> 
> this is why I didn't get the missing case on the type if/else ;)
> 
> we should never reach that case, but in case there is the missing case
> an extra debug message doesn't hurt...
> 

We should not be writing to registers in the caller for invalid encoder types. 
And also debug log a bit sooner. How about?

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/
intel_ddi.c
index db80938..81cdffb 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1858,14 +1858,18 @@ static void cnl_ddi_vswing_sequence(struct 
intel_encoder *encoder, u32 level)
        u32 val;
        int ln = 0;
 
-       if ((intel_dp) && (type == INTEL_OUTPUT_EDP || type == 
INTEL_OUTPUT_DP)) {
+       if (intel_dp && (type == INTEL_OUTPUT_EDP || type == INTEL_OUTPUT_DP)) 
{
                width = intel_dp->lane_count;
                rate = intel_dp->link_rate;
-       } else {
+       } else if (type == INTEL_OUTPUT_HDMI){
                width = 4;
                /* Rate is always < than 6GHz for HDMI */
+       } else {
+               DRM_DEBUG_KMS("vswing sequence undefined for encoder type %d
\n", type);
+               return;
        }
 
+
        /*
         * 1. If port type is eDP or DP,
         * set PORT_PCS_DW1 cmnkeeper_enable to 1b,

-DK

> 
> > 
> > 
> > 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@intel.com>
> > > > > > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > > > > ---
> > > > > > > 
> > > > > > >  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@lists.freedesktop.org
> > > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > > > 
> > > > > _______________________________________________
> > > > > Intel-gfx mailing list
> > > > > Intel-gfx@lists.freedesktop.org
> > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > > 
> > > > 
> > > 
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx


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

Re: [PATCH] drm/i915/cnl: Add information to missing case.

[Vivi, Rodrigo]

On Sat, 2017-06-17 at 01:31 -0700, Dhinakaran Pandiyan wrote:
> On Friday, June 16, 2017 10:18:28 PM PDT Vivi, Rodrigo wrote:
> > On Fri, 2017-06-16 at 21:54 +0000, Navare, Manasi D wrote:
> > 
> > > 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()
> > 
> > this is why I didn't get the missing case on the type if/else ;)
> > 
> > we should never reach that case, but in case there is the missing case
> > an extra debug message doesn't hurt...
> > 
> 
> We should not be writing to registers in the caller for invalid encoder types. 
> And also debug log a bit sooner. How about?

I liked it, with one bikeshed below:

> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/
> intel_ddi.c
> index db80938..81cdffb 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1858,14 +1858,18 @@ static void cnl_ddi_vswing_sequence(struct 
> intel_encoder *encoder, u32 level)
>         u32 val;
>         int ln = 0;
>  
> -       if ((intel_dp) && (type == INTEL_OUTPUT_EDP || type == 
> INTEL_OUTPUT_DP)) {
> +       if (intel_dp && (type == INTEL_OUTPUT_EDP || type == INTEL_OUTPUT_DP)) 
> {
>                 width = intel_dp->lane_count;
>                 rate = intel_dp->link_rate;
> -       } else {
> +       } else if (type == INTEL_OUTPUT_HDMI){
>                 width = 4;
>                 /* Rate is always < than 6GHz for HDMI */
> +       } else {
> +               DRM_DEBUG_KMS("vswing sequence undefined for encoder type %d
> \n", type);

this should be 

	MISSING_CASE(type);

instead.

> +               return;
>         }
>  
> +
>         /*
>          * 1. If port type is eDP or DP,
>          * set PORT_PCS_DW1 cmnkeeper_enable to 1b,
> 
> -DK

I think it is your patch now ;)
But let me know if you want me to do anything here..

> 
> > 
> > > 
> > > 
> > > 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@intel.com>
> > > > > > > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > > > > > ---
> > > > > > > > 
> > > > > > > >  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@lists.freedesktop.org
> > > > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > > > > 
> > > > > > _______________________________________________
> > > > > > Intel-gfx mailing list
> > > > > > Intel-gfx@lists.freedesktop.org
> > > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > > > 
> > > > > 
> > > > 
> > > > 
> > > > _______________________________________________
> > > > Intel-gfx mailing list
> > > > Intel-gfx@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > 
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 

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

Re: [PATCH] drm/i915/cnl: Add information to missing case.

[Navare, Manasi D]

On Sat, 2017-06-17 at 01:31 -0700, Dhinakaran Pandiyan wrote:
> On Friday, June 16, 2017 10:18:28 PM PDT Vivi, Rodrigo wrote:
> > On Fri, 2017-06-16 at 21:54 +0000, Navare, Manasi D wrote:
> > 
> > > 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()
> > 
> > this is why I didn't get the missing case on the type if/else ;)
> > 
> > we should never reach that case, but in case there is the missing 
> > case an extra debug message doesn't hurt...
> > 
> 
> We should not be writing to registers in the caller for invalid encoder types. 
> And also debug log a bit sooner. How about?

I liked it, with one bikeshed below:

> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/ 
> intel_ddi.c index db80938..81cdffb 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1858,14 +1858,18 @@ static void cnl_ddi_vswing_sequence(struct 
> intel_encoder *encoder, u32 level)
>         u32 val;
>         int ln = 0;
>  
> -       if ((intel_dp) && (type == INTEL_OUTPUT_EDP || type == 
> INTEL_OUTPUT_DP)) {
> +       if (intel_dp && (type == INTEL_OUTPUT_EDP || type == 
> + INTEL_OUTPUT_DP))
> {
>                 width = intel_dp->lane_count;
>                 rate = intel_dp->link_rate;
> -       } else {
> +       } else if (type == INTEL_OUTPUT_HDMI){
>                 width = 4;
>                 /* Rate is always < than 6GHz for HDMI */
> +       } else {
> +               DRM_DEBUG_KMS("vswing sequence undefined for encoder 
> + type %d
> \n", type);

this should be 

	MISSING_CASE(type);

instead.

> +               return;
>         }
>  
> +
>         /*
>          * 1. If port type is eDP or DP,
>          * set PORT_PCS_DW1 cmnkeeper_enable to 1b,
> 
> -DK

I think it is your patch now ;)
But let me know if you want me to do anything here..

I still think that there is a chance of running into a situation where the combination of the type
And the read vccio voltage is invalid and where we would need the debug print.
Also because I hate to sprinkle debug prints after a bug is filed would rather have it there
Making the debug easier. That is just my 2 cents....but if some people are too opinionated about
The debug print then go ahead remove it.

Manasi


> 
> > 
> > > 
> > > 
> > > 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@intel.com>
> > > > > > > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > > > > > ---
> > > > > > > > 
> > > > > > > >  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@lists.freedesktop.org 
> > > > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > > > > 
> > > > > > _______________________________________________
> > > > > > Intel-gfx mailing list
> > > > > > Intel-gfx@lists.freedesktop.org 
> > > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > > > 
> > > > > 
> > > > 
> > > > 
> > > > _______________________________________________
> > > > Intel-gfx mailing list
> > > > Intel-gfx@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > 
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 

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