On Mon, Apr 04, 2016 at 11:45:26AM +0300, Jani Nikula wrote: > On Mon, 04 Apr 2016, Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxx> > >> > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > >> > --- > >> > 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@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
