[Intel-gfx] [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 at 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 at linux-m68k.org>
> >> > Signed-off-by: Chris Wilson <chris at 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