Re: [PATCH 1/2] drm/i915: introduce pipe_config->ddi_personality

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Oct 28, 2014 at 04:30:41PM +0200, Ville Syrjälä wrote:
> On Tue, Oct 28, 2014 at 11:26:51AM -0200, Paulo Zanoni wrote:
> > >> +             /*
> > >> +              * We can't change the DDI type if we already have a connected
> > >> +              * device on this port. The first time a DDI is used though
> > >> +              * (encoder_type is INTEL_OUTPUT_UNKNOWN) and we force a
> > >> +              * connector to be connected (either trought the kernel command
> > >> +              * line or KMS) we need to comply.
> > >> +              */
> > >> +              if (encoder->type != INTEL_OUTPUT_UNKNOWN &&
> > >> +                  connector->base.status == connector_status_connected) {
> > >> +                      DRM_DEBUG_KMS("Can't set DDI %c personality to %s, it has a connected %s device\n",
> > >> +                                     port_name(port),
> > >> +                                     intel_output_name(encoder->type),
> > >> +                                     intel_output_name(pipe_config->ddi_personality));
> > >> +                      return false;
> > >> +             }
> > >
> > > I think this part is better done with Ville's more general "do we have
> > > both hdmi and dp on the same dig_port?" check. Care to review Ville's
> > > patch instead? Thomas Wood is signed up for it on the review board but I
> > > guess you can steal that task.
> > 
> > Ville's patch solves a different problem. I just reviewed it, but we
> > still need the check above. The code above is in case, for example,
> > there's a DP connector actually connected (but without a mode set),
> > and then the user tries to set a mode on the HDMI connector of this
> > encoder.

My comment was _only_ about the check quoted above, the other part of the
loop is imo the entire point of adding ddi_personality.

> Should we even care about that issue? Forcing a HDMI mode on a port even
> when there's a DP monitor plugged in does make it easier to test things
> without having to yank out the cables all the time. Also your patch
> wouldn't fix that problem for non-ddi platforms.
> 
> I would suggest that we should allow this behaviour unless there's some
> risk of causing problems to the sink. Another reason for rejecting it
> would be if aux would stop working, but I don't think that should be the
> case since we can do both gmbus and aux during probing anyway.

This is useful for injection arbitrary modeset configs, something Thomas
Wood is slowly chipping away on. And I think we definitely want that for
increased automated test coverage.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx