[PATCH] drm/i915/ddi: Silence compiler warning for unknown output type

[PATCH] drm/i915/ddi: Silence compiler warning for unknown output type

[Chris Wilson]

Silences

	src/drivers/gpu/drm/i915/intel_ddi.c: warning: 'port' may be used uninitialized in this function [-Wuninitialized]

Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_ddi.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 766156f88ef4..921edf183d22 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -315,6 +315,9 @@ static void ddi_get_encoder_port(struct intel_encoder *intel_encoder,
 		*dig_port = enc_to_mst(encoder)->primary;
 		*port = (*dig_port)->port;
 		break;
+	default:
+		WARN(1, "Invalid DDI encoder type %d\n", intel_encoder->type);
+		/* fallthrough and treat as unknown */
 	case INTEL_OUTPUT_DISPLAYPORT:
 	case INTEL_OUTPUT_EDP:
 	case INTEL_OUTPUT_HDMI:
@@ -326,9 +329,6 @@ static void ddi_get_encoder_port(struct intel_encoder *intel_encoder,
 		*dig_port = NULL;
 		*port = PORT_E;
 		break;
-	default:
-		WARN(1, "Invalid DDI encoder type %d\n", intel_encoder->type);
-		break;
 	}
 }
 
-- 
2.8.0.rc3

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

Re: [PATCH] drm/i915/ddi: Silence compiler warning for unknown output type

[Joonas Lahtinen]

On su, 2016-04-03 at 21:59 +0100, Chris Wilson wrote:
> Silences
> 
> 	src/drivers/gpu/drm/i915/intel_ddi.c: warning: 'port' may be used uninitialized in this function [-Wuninitialized]
> 
> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 766156f88ef4..921edf183d22 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -315,6 +315,9 @@ static void ddi_get_encoder_port(struct intel_encoder *intel_encoder,
>  		*dig_port = enc_to_mst(encoder)->primary;
>  		*port = (*dig_port)->port;
>  		break;
> +	default:
> +		WARN(1, "Invalid DDI encoder type %d\n", intel_encoder->type);
> +		/* fallthrough and treat as unknown */

While touching it, might as well fix into MISSING_CASE. With that;

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

>  	case INTEL_OUTPUT_DISPLAYPORT:
>  	case INTEL_OUTPUT_EDP:
>  	case INTEL_OUTPUT_HDMI:
> @@ -326,9 +329,6 @@ static void ddi_get_encoder_port(struct intel_encoder *intel_encoder,
>  		*dig_port = NULL;
>  		*port = PORT_E;
>  		break;
> -	default:
> -		WARN(1, "Invalid DDI encoder type %d\n", intel_encoder->type);
> -		break;
>  	}
>  }
>  
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915/ddi: Silence compiler warning for unknown output type

[Patchwork]

== Series Details ==

Series: drm/i915/ddi: Silence compiler warning for unknown output type
URL   : https://patchwork.freedesktop.org/series/5247/
State : success

== Summary ==

Series 5247v1 drm/i915/ddi: Silence compiler warning for unknown output type
http://patchwork.freedesktop.org/api/1.0/series/5247/revisions/1/mbox/

Test drv_hangman:
        Subgroup error-state-basic:
                fail       -> PASS       (ilk-hp8440p)
Test drv_module_reload_basic:
                pass       -> SKIP       (ivb-t430s)
Test kms_flip:
        Subgroup basic-flip-vs-dpms:
                pass       -> DMESG-WARN (ilk-hp8440p) UNSTABLE
        Subgroup basic-flip-vs-modeset:
                dmesg-warn -> PASS       (ilk-hp8440p) UNSTABLE

bdw-nuci7        total:196  pass:184  dwarn:0   dfail:0   fail:0   skip:12 
bdw-ultra        total:196  pass:175  dwarn:0   dfail:0   fail:0   skip:21 
bsw-nuc-2        total:196  pass:159  dwarn:0   dfail:0   fail:0   skip:37 
byt-nuc          total:196  pass:161  dwarn:0   dfail:0   fail:0   skip:35 
hsw-brixbox      total:196  pass:174  dwarn:0   dfail:0   fail:0   skip:22 
hsw-gt2          total:196  pass:179  dwarn:0   dfail:0   fail:0   skip:17 
ilk-hp8440p      total:196  pass:131  dwarn:1   dfail:0   fail:0   skip:64 
ivb-t430s        total:196  pass:170  dwarn:0   dfail:0   fail:0   skip:26 
skl-i7k-2        total:196  pass:173  dwarn:0   dfail:0   fail:0   skip:23 
snb-dellxps      total:196  pass:162  dwarn:0   dfail:0   fail:0   skip:34 
snb-x220t        total:196  pass:162  dwarn:0   dfail:0   fail:1   skip:33 

Results at /archive/results/CI_IGT_test/Patchwork_1784/

5633965ef152b136c22fe9858723be52116f8686 drm-intel-nightly: 2016y-04m-03d-14h-10m-49s UTC integration manifest
aa4e21fe8519eaaf6679f7eb2ba110c0726f79f6 drm/i915/ddi: Silence compiler warning for unknown output type

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

Re: [PATCH] drm/i915/ddi: Silence compiler warning for unknown output type

[Chris Wilson]

On Mon, Apr 04, 2016 at 09:20:27AM +0300, Joonas Lahtinen wrote:
> On su, 2016-04-03 at 21:59 +0100, Chris Wilson wrote:
> > Silences
> > 
> > 	src/drivers/gpu/drm/i915/intel_ddi.c: warning: 'port' may be used uninitialized in this function [-Wuninitialized]
> > 
> > Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> >  drivers/gpu/drm/i915/intel_ddi.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > index 766156f88ef4..921edf183d22 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -315,6 +315,9 @@ static void ddi_get_encoder_port(struct intel_encoder *intel_encoder,
> >  		*dig_port = enc_to_mst(encoder)->primary;
> >  		*port = (*dig_port)->port;
> >  		break;
> > +	default:
> > +		WARN(1, "Invalid DDI encoder type %d\n", intel_encoder->type);
> > +		/* fallthrough and treat as unknown */
> 
> While touching it, might as well fix into MISSING_CASE. With that;

I was tempted, I wasn't sure if not using MISSING_CASE was deliberate
(trying not to confuse semantics between adding support for a new
generation versus an unknown DDI)? So I left it for whoever has to make
that fix (and kept the change as small as possible for a minor buglet).
 
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Thanks,
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/ddi: Silence compiler warning for unknown output type

[Jani Nikula]

On Mon, 04 Apr 2016, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Mon, Apr 04, 2016 at 09:20:27AM +0300, Joonas Lahtinen wrote:
>> On su, 2016-04-03 at 21:59 +0100, Chris Wilson wrote:
>> > Silences
>> > 
>> > 	src/drivers/gpu/drm/i915/intel_ddi.c: warning: 'port' may be used uninitialized in this function [-Wuninitialized]
>> > 
>> > Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
>> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>> > ---
>> >  drivers/gpu/drm/i915/intel_ddi.c | 6 +++---
>> >  1 file changed, 3 insertions(+), 3 deletions(-)
>> > 
>> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
>> > index 766156f88ef4..921edf183d22 100644
>> > --- a/drivers/gpu/drm/i915/intel_ddi.c
>> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
>> > @@ -315,6 +315,9 @@ static void ddi_get_encoder_port(struct intel_encoder *intel_encoder,
>> >  		*dig_port = enc_to_mst(encoder)->primary;
>> >  		*port = (*dig_port)->port;
>> >  		break;
>> > +	default:
>> > +		WARN(1, "Invalid DDI encoder type %d\n", intel_encoder->type);
>> > +		/* fallthrough and treat as unknown */
>> 
>> While touching it, might as well fix into MISSING_CASE. With that;
>
> I was tempted, I wasn't sure if not using MISSING_CASE was deliberate
> (trying not to confuse semantics between adding support for a new
> generation versus an unknown DDI)? So I left it for whoever has to make
> that fix (and kept the change as small as possible for a minor
> buglet).

Hitting MISSING_CASE here would, IMO, imply that the fix is to add the
missing case, but we have plenty of intel_output_type enums that simply
should not be handled here at all, and we should ensure we don't end up
here with the wrong output types. Thus I think keeping the WARN is the
way to go.

I'm just slightly concerned about the fix here though. So we're screwed
anyway if the output type is not handled, and we might use an
uninitialized variable... but the assumption that using *dig_port =
enc_to_dig_port(encoder) will result in any more meaningful result is
wrong.

BR,
Jani.


>  
>> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>
> Thanks,
> -Chris

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/ddi: Silence compiler warning for unknown output type

[Chris Wilson]

On Mon, Apr 04, 2016 at 11:45:26AM +0300, Jani Nikula wrote:
> On Mon, 04 Apr 2016, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > On Mon, Apr 04, 2016 at 09:20:27AM +0300, Joonas Lahtinen wrote:
> >> On su, 2016-04-03 at 21:59 +0100, Chris Wilson wrote:
> >> > Silences
> >> > 
> >> > 	src/drivers/gpu/drm/i915/intel_ddi.c: warning: 'port' may be used uninitialized in this function [-Wuninitialized]
> >> > 
> >> > Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> >> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> >> > ---
> >> >  drivers/gpu/drm/i915/intel_ddi.c | 6 +++---
> >> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >> > 
> >> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> >> > index 766156f88ef4..921edf183d22 100644
> >> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> >> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> >> > @@ -315,6 +315,9 @@ static void ddi_get_encoder_port(struct intel_encoder *intel_encoder,
> >> >  		*dig_port = enc_to_mst(encoder)->primary;
> >> >  		*port = (*dig_port)->port;
> >> >  		break;
> >> > +	default:
> >> > +		WARN(1, "Invalid DDI encoder type %d\n", intel_encoder->type);
> >> > +		/* fallthrough and treat as unknown */
> >> 
> >> While touching it, might as well fix into MISSING_CASE. With that;
> >
> > I was tempted, I wasn't sure if not using MISSING_CASE was deliberate
> > (trying not to confuse semantics between adding support for a new
> > generation versus an unknown DDI)? So I left it for whoever has to make
> > that fix (and kept the change as small as possible for a minor
> > buglet).
> 
> Hitting MISSING_CASE here would, IMO, imply that the fix is to add the
> missing case, but we have plenty of intel_output_type enums that simply
> should not be handled here at all, and we should ensure we don't end up
> here with the wrong output types. Thus I think keeping the WARN is the
> way to go.
> 
> I'm just slightly concerned about the fix here though. So we're screwed
> anyway if the output type is not handled, and we might use an
> uninitialized variable... but the assumption that using *dig_port =
> enc_to_dig_port(encoder) will result in any more meaningful result is
> wrong.

Hobson's choice, we're fubar anyway. The only question is whether or not
we can make a choice that prevents us from locking the machine up during
boot. I was tempted by using INTEL_OUTPUT_VGA instead, would that be
better?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/ddi: Silence compiler warning for unknown output type

[Joonas Lahtinen]

On ma, 2016-04-04 at 10:00 +0100, Chris Wilson wrote:
> On Mon, Apr 04, 2016 at 11:45:26AM +0300, Jani Nikula wrote:
> > 
> > On Mon, 04 Apr 2016, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > > 
> > > On Mon, Apr 04, 2016 at 09:20:27AM +0300, Joonas Lahtinen wrote:
> > > > 
> > > > On su, 2016-04-03 at 21:59 +0100, Chris Wilson wrote:
> > > > > 
> > > > > Silences
> > > > > 
> > > > > 	src/drivers/gpu/drm/i915/intel_ddi.c: warning: 'port' may be used uninitialized in this function [-Wuninitialized]
> > > > > 
> > > > > Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > > > ---
> > > > >  drivers/gpu/drm/i915/intel_ddi.c | 6 +++---
> > > > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > > > > index 766156f88ef4..921edf183d22 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > > > > @@ -315,6 +315,9 @@ static void ddi_get_encoder_port(struct intel_encoder *intel_encoder,
> > > > >  		*dig_port = enc_to_mst(encoder)->primary;
> > > > >  		*port = (*dig_port)->port;
> > > > >  		break;
> > > > > +	default:
> > > > > +		WARN(1, "Invalid DDI encoder type %d\n", intel_encoder->type);
> > > > > +		/* fallthrough and treat as unknown */
> > > > While touching it, might as well fix into MISSING_CASE. With that;
> > > I was tempted, I wasn't sure if not using MISSING_CASE was deliberate
> > > (trying not to confuse semantics between adding support for a new
> > > generation versus an unknown DDI)? So I left it for whoever has to make
> > > that fix (and kept the change as small as possible for a minor
> > > buglet).
> > Hitting MISSING_CASE here would, IMO, imply that the fix is to add the
> > missing case, but we have plenty of intel_output_type enums that simply
> > should not be handled here at all, and we should ensure we don't end up
> > here with the wrong output types. Thus I think keeping the WARN is the
> > way to go.
> > 
> > I'm just slightly concerned about the fix here though. So we're screwed
> > anyway if the output type is not handled, and we might use an
> > uninitialized variable... but the assumption that using *dig_port =
> > enc_to_dig_port(encoder) will result in any more meaningful result is
> > wrong.

Well, enumeration values *known* not to be acceptable should cause a
ERR_PTR(-EINVAL) or along that route, and there should be a
MISSING_CASE for rest totally unexpected values.

How to actually handle that situation outside of the function to make
system bootable, is an entirely different question :P

Regards, Joonas

> Hobson's choice, we're fubar anyway. The only question is whether or not
> we can make a choice that prevents us from locking the machine up during
> boot. I was tempted by using INTEL_OUTPUT_VGA instead, would that be
> better?
> -Chris
> 
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/ddi: Silence compiler warning for unknown output type

[Jani Nikula]

On Mon, 04 Apr 2016, Joonas Lahtinen <joonas.lahtinen@linux.intel.com> wrote:
> Well, enumeration values *known* not to be acceptable should cause a
> ERR_PTR(-EINVAL) or along that route, and there should be a
> MISSING_CASE for rest totally unexpected values.

In this case, they're all the same. No use returning errors when this
shouldn't happen to begin with.

J.


-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx