From mboxrd@z Thu Jan 1 00:00:00 1970 From: Egbert Eich Subject: Re: [PATCH 2/2] drm/i915: Set up SDVO encoder type only at detect Date: Wed, 16 Apr 2014 07:58:48 +0200 Message-ID: <21326.7192.130642.36744@linux-qknr.fritz.box> References: <1397496369-2746-1-git-send-email-eich@suse.de> <1397496369-2746-3-git-send-email-eich@suse.de> <20140415191215.GD1023@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mx2.suse.de (cantor2.suse.de [195.135.220.15]) by gabe.freedesktop.org (Postfix) with ESMTP id B4F446E9E5 for ; Tue, 15 Apr 2014 23:01:28 -0700 (PDT) In-Reply-To: daniel@ffwll.ch wrote on Tuesday, 15 April 2014 at 21:12:15 +0200 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Daniel Vetter Cc: Egbert Eich , intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org Daniel Vetter writes: > On Mon, Apr 14, 2014 at 07:26:09PM +0200, Egbert Eich wrote: > > Depending on the SDVO output_flags SDVO may have multiple connectors. > > These are all initialized in intel_sdvo_output_setup(). The connector > > that is initialized later will override the encoder_type that has been > > set up by an earlier connector type initialization. Eventually the > > one that comes last wins. > > Eventually when intel_sdvo_detect() is called the active connector is > > determined. > > Delay encoder type initialization until the active connector is known > > and set it to the type that corresponds to this connector. > > = > > Signed-off-by: Egbert Eich > = > Hm, has this any effect on the code itself? I think if we want to fix th= is > just for optics we should add a new DRM_MODE_ENCODER_MULTI. At least I > have an sdvo here which has a dac, hdmi and tv-out ... With the present logic the last connector in the list matching a flag bit will win the encoder type. The encoder type is presently just used for (debug) messages, so it is cosmetic. > = > This also reminds that I should finally polish of the multi-sdvo fixes I > have around here - currently the last encoder detected wins on a > multi-encoder chip, which means if you have an lvds+hdmi combo and plug = in > a screeen you'll never be able to use the lvds again until the hdmi is > unplugged. You know, from looking at the code I was wondering already if this was = possible at all. = I stayed away tinkering with this - there doesn't seem to be documentation = on the SDVO command set publically available. Moreover I'm moslty dealing = with cash registers here - so I doubt they have all the SVIDEO connectors = they advertise ;) But I can't tell as I don't even have physical access! As I read the current code it seems like bad things could happen = if more than one bit is set in intel_sdvo->attached_output: after all tv-out should have quite different timing requirements than a TMDS. > = > Much worse if there's a tv-out detect issue ;-) ;p Cheers, Egbert. > = > Cheers, Daniel > = > > --- > > drivers/gpu/drm/i915/intel_sdvo.c | 23 ++++++++++++++++++----- > > 1 file changed, 18 insertions(+), 5 deletions(-) > > = > > diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/= intel_sdvo.c > > index d27155a..5043f16 100644 > > --- a/drivers/gpu/drm/i915/intel_sdvo.c > > +++ b/drivers/gpu/drm/i915/intel_sdvo.c > > @@ -154,6 +154,9 @@ struct intel_sdvo_connector { > > /* Mark the type of connector */ > > uint16_t output_flag; > > = > > + /* store encoder type for convenience */ > > + int encoder_type; > > + > > enum hdmi_force_audio force_audio; > > = > > /* This contains all current supported TV format */ > > @@ -1746,6 +1749,7 @@ intel_sdvo_detect(struct drm_connector *connecto= r, bool force) > > if (response =3D=3D 0) > > return connector_status_disconnected; > > = > > + intel_sdvo->base.base.encoder_type =3D intel_sdvo_connector->encoder= _type; > > intel_sdvo->attached_output =3D response; > > = > > intel_sdvo->has_hdmi_monitor =3D false; > > @@ -2489,7 +2493,9 @@ intel_sdvo_dvi_init(struct intel_sdvo *intel_sdv= o, int device) > > } else { > > intel_connector->polled =3D DRM_CONNECTOR_POLL_CONNECT | DRM_CONNEC= TOR_POLL_DISCONNECT; > > } > > - encoder->encoder_type =3D DRM_MODE_ENCODER_TMDS; > > + /* delay encoder_type setting until detection */ > > + intel_sdvo_connector->encoder_type =3D DRM_MODE_ENCODER_TMDS; > > + encoder->encoder_type =3D DRM_MODE_ENCODER_NONE; > > connector->connector_type =3D DRM_MODE_CONNECTOR_DVID; > > = > > if (intel_sdvo_is_hdmi_connector(intel_sdvo, device)) { > > @@ -2524,7 +2530,9 @@ intel_sdvo_tv_init(struct intel_sdvo *intel_sdvo= , int type) > > = > > intel_connector =3D &intel_sdvo_connector->base; > > connector =3D &intel_connector->base; > > - encoder->encoder_type =3D DRM_MODE_ENCODER_TVDAC; > > + /* delay encoder_type setting until detection */ > > + intel_sdvo_connector->encoder_type =3D DRM_MODE_ENCODER_TVDAC; > > + encoder->encoder_type =3D DRM_MODE_ENCODER_NONE; > > connector->connector_type =3D DRM_MODE_CONNECTOR_SVIDEO; > > = > > intel_sdvo->controlled_output |=3D type; > > @@ -2568,7 +2576,9 @@ intel_sdvo_analog_init(struct intel_sdvo *intel_= sdvo, int device) > > intel_connector =3D &intel_sdvo_connector->base; > > connector =3D &intel_connector->base; > > intel_connector->polled =3D DRM_CONNECTOR_POLL_CONNECT; > > - encoder->encoder_type =3D DRM_MODE_ENCODER_DAC; > > + /* delay encoder_type setting until detection */ > > + intel_sdvo_connector->encoder_type =3D DRM_MODE_ENCODER_DAC; > > + encoder->encoder_type =3D DRM_MODE_ENCODER_NONE; > > connector->connector_type =3D DRM_MODE_CONNECTOR_VGA; > > = > > if (device =3D=3D 0) { > > @@ -2603,7 +2613,9 @@ intel_sdvo_lvds_init(struct intel_sdvo *intel_sd= vo, int device) > > = > > intel_connector =3D &intel_sdvo_connector->base; > > connector =3D &intel_connector->base; > > - encoder->encoder_type =3D DRM_MODE_ENCODER_LVDS; > > + /* delay encoder_type setting until detection */ > > + intel_sdvo_connector->encoder_type =3D DRM_MODE_ENCODER_LVDS; > > + encoder->encoder_type =3D DRM_MODE_ENCODER_NONE; > > connector->connector_type =3D DRM_MODE_CONNECTOR_LVDS; > > = > > if (device =3D=3D 0) { > > @@ -2984,7 +2996,8 @@ bool intel_sdvo_init(struct drm_device *dev, uin= t32_t sdvo_reg, bool is_sdvob) > > /* encoder type will be decided later */ > > intel_encoder =3D &intel_sdvo->base; > > intel_encoder->type =3D INTEL_OUTPUT_SDVO; > > - drm_encoder_init(dev, &intel_encoder->base, &intel_sdvo_enc_funcs, 0= ); > > + drm_encoder_init(dev, &intel_encoder->base, &intel_sdvo_enc_funcs, > > + DRM_MODE_ENCODER_NONE); > > = > > /* Read the regs to test if we can talk to the device */ > > for (i =3D 0; i < 0x40; i++) { > > -- = > > 1.8.4.5 > > = > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > = > -- = > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- = Egbert Eich (Res. & Dev.) SUSE LINUX Products GmbH SUSE Labs KMS / DRM / X Window System Development Tel: +49 911-740 53 0 http://www.suse.de = ----------------------------------------------------------------- SUSE LINUX Products GmbH, GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffe= r, HRB 16746 (AG N=FCrnberg) = Maxfeldstra=DFe 5 = 90409 N=FCrnberg = Germany =