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@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. 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@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
