Re: [PATCH 10/12] drm/i915: Add lspcon core functions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Em Ter, 2016-05-03 às 21:44 +0530, Sharma, Shashank escreveu:
> 
> On 5/3/2016 9:39 PM, Ville Syrjälä wrote:
> > 
> > On Tue, May 03, 2016 at 09:18:49PM +0530, Sharma, Shashank wrote:
> > > 
> > > Regards
> > > Shashank
> > > 
> > > On 5/2/2016 7:21 PM, Ville Syrjälä wrote:
> > > > 
> > > > On Mon, Apr 04, 2016 at 05:31:46PM +0530, Shashank Sharma
> > > > wrote:
> > > > > 
> > > > > This patch adds lspcon's internal functions, which work
> > > > > on the probe layer, and indicate the working status of
> > > > > lspcon, which are mostly:
> > > > > 
> > > > > probe: A lspcon device is probed only once, during boot
> > > > > time, as its always present with the device, next to port.
> > > > > So the i2c_over_aux channel is alwyas read/writeable if DC is
> > > > > powered on. If VBT says that this port contains lspcon, we
> > > > > check and probe the HW to verify and initialize it.
> > > > > 
> > > > > get_mode: This function indicates the current mode of
> > > > > operation
> > > > > of lspcon (ls or pcon mode)
> > > > > 
> > > > > change_mode: This function can change the lspcon's mode of
> > > > > operation to desired mode.
> > > > > 
> > > > > Signed-off-by: Shashank Sharma <shashank.sharma@xxxxxxxxx>
> > > > > Signed-off-by: Akashdeep Sharma <Akashdeep.sharma@xxxxxxxxx>
> > > > > ---
> > > > >    drivers/gpu/drm/i915/intel_lspcon.c | 145
> > > > > ++++++++++++++++++++++++++++++++++++
> > > > >    1 file changed, 145 insertions(+)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/i915/intel_lspcon.c
> > > > > b/drivers/gpu/drm/i915/intel_lspcon.c
> > > > > index c3c1cd2..617fe3f 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_lspcon.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_lspcon.c
> > > > > @@ -61,6 +61,144 @@ static struct intel_lspcon
> > > > >    	return
> > > > > enc_to_lspcon(&intel_attached_encoder(connector)->base);
> > > > >    }
> > > > > 
> > > > > +enum lspcon_mode lspcon_get_current_mode(struct intel_lspcon
> > > > > *lspcon)
> > > > > +{
> > > > > +	u8 data;
> > > > > +	int err = 0;
> > > > > +	struct intel_digital_port *dig_port =
> > > > > lspcon_to_dig_port(lspcon);
> > > > > +	struct i2c_adapter *adapter = &dig_port->dp.aux.ddc;
> > > > > +
> > > > > +	/* Read Status: i2c over aux */
> > > > > +	err = drm_dp_dual_mode_ioa_read(adapter, &data,
> > > > > +		LSPCON_MODE_CHECK_OFFSET, sizeof(data));
> > > > > +	if (err < 0) {
> > > > > +		DRM_ERROR("LSPCON read mode ioa (0x80, 0x41)
> > > > > failed\n");
> > > > > +		return lspcon_mode_invalid;
> > > > > +	}
> > > > > +
> > > > > +	DRM_DEBUG_DRIVER("LSPCON mode (0x80, 0x41) = %x\n",
> > > > > (unsigned int)data);
> > > > > +	return data & LSPCON_MODE_MASK ? lspcon_mode_pcon :
> > > > > lspcon_mode_ls;
> > > > > +}
> > > > > +
> > > > > +int lspcon_change_mode(struct intel_lspcon *lspcon,
> > > > > +	enum lspcon_mode mode, bool force)
> > > > > +{
> > > > > +	u8 data;
> > > > > +	int err;
> > > > > +	int time_out = 200;
> > > > > +	enum lspcon_mode current_mode;
> > > > > +	struct intel_digital_port *dig_port =
> > > > > lspcon_to_dig_port(lspcon);
> > > > > +
> > > > > +	current_mode = lspcon_get_current_mode(lspcon);
> > > > > +	if (current_mode == lspcon_mode_invalid) {
> > > > > +		DRM_ERROR("Failed to get current LSPCON
> > > > > mode\n");
> > > > > +		return -EFAULT;
> > > > > +	}
> > > > > +
> > > > > +	if (current_mode == mode && !force) {
> > > > > +		DRM_DEBUG_DRIVER("Current mode = desired
> > > > > LSPCON mode\n");
> > > > > +		return 0;
> > > > > +	}
> > > > > +
> > > > > +	if (mode == lspcon_mode_ls)
> > > > > +		data = ~LSPCON_MODE_MASK;
> > > > > +	else
> > > > > +		data = LSPCON_MODE_MASK;
> > > > > +
> > > > > +	/* Change mode */
> > > > > +	err = drm_dp_dual_mode_ioa_write(&dig_port-
> > > > > >dp.aux.ddc, &data,
> > > > > +			LSPCON_MODE_CHANGE_OFFSET,
> > > > > sizeof(data));
> > > > > +	if (err < 0) {
> > > > > +		DRM_ERROR("LSPCON mode change failed\n");
> > > > > +		return err;
> > > > > +	}
> > > > > +
> > > > > +	/*
> > > > > +	* Confirm mode change by reading the status bit.
> > > > > +	* Sometimes, it takes a while to change the mode,
> > > > > +	* so wait and retry until time out or done.
> > > > > +	*/
> > > > > +	while (time_out) {
> > > > > +		current_mode =
> > > > > lspcon_get_current_mode(lspcon);
> > > > > +		if (current_mode != mode) {
> > > > > +			mdelay(10);
> > > > > +			time_out -= 10;
> > > > > +		} else {
> > > > > +			lspcon->mode_of_op = mode;
> > > > > +			DRM_DEBUG_DRIVER("LSPCON mode
> > > > > changed to %s\n",
> > > > > +				mode == lspcon_mode_ls ?
> > > > > "LS" : "PCON");
> > > > > +			return 0;
> > > > > +		}
> > > > > +	}
> > > > > +
> > > > > +	DRM_ERROR("LSPCON mode change timed out\n");
> > > > > +	return -EFAULT;
> > > > > +}
> > > > I think we probably want to put most of this stuff into the
> > > > helper. We
> > > > already have the LSPCON identification there, so having a few
> > > > helpers
> > > > to set/get the ls vs. pconn mode would seem appropriate.
> > > > 
> > > Actually handling of LS/PCON modes are specific to MCA chips, and
> > > some
> > > of the mode change mechanism and few other control stuff may not
> > > be same
> > > on parade or some other vendor's chip, so I am not sure if we
> > > should
> > > create a helper for something which is specific to this chip. You
> > > suggest so ?
> > Well, if we already put the detection into the helper we already
> > have
> > some vendor specific stuff there, no? Or would the other vendor's
> > chips
> > be identifiable in the same way?
> > 
> > Anyways, I presume someone else than Intel might want to use these
> > same
> > chips in their products, so having the support in a central place
> > would
> > seem like a good idea. If we start to see incompatble LSPCON chips
> > from
> > multiple vendors we'll need to rethink how we support them all, but
> > until we know how exactly they differ it's rather impossible to
> > design
> > the helper to deal with them. So as a first step I'd stuff it all
> > into
> > the helper.
> > 
> Yes, makes more sense this way. I will try to go through parade
> specs 
> once, and see if I can find something which needs immediate
> discussion 
> here. Else, I will move this into the central layer.

The big problem with LSPCON is that it uses bits that are reserved by
the VESA specs, such as bit 3 of I2C address 80h offset 10h. Nothing
prevents VESA from defining an actual meaning to that bit that will be
incompatible with LSPCON.

> > 
> > > 
> > > > 
> > > > > 
> > > > > +
> > > > > +bool lspcon_detect_identifier(struct intel_lspcon *lspcon)
> > > > > +{
> > > > > +	enum drm_dp_dual_mode_type adaptor_type;
> > > > > +	struct intel_digital_port *dig_port =
> > > > > lspcon_to_dig_port(lspcon);
> > > > > +	struct i2c_adapter *adapter = &dig_port->dp.aux.ddc;
> > > > > +
> > > > > +	/* Lets probe the adaptor and check its type */
> > > > > +	adaptor_type = drm_dp_dual_mode_detect(adapter);
> > > > > +	if (adaptor_type != DRM_DP_DUAL_MODE_TYPE2_LSPCON) {
> > > > > +		DRM_DEBUG_DRIVER("No LSPCON detected, found
> > > > > %s\n",
> > > > > +			drm_dp_get_dual_mode_type_name(adapt
> > > > > or_type));
> > > > > +		return false;
> > > > > +	}
> > > > > +
> > > > > +	/* Yay ... got a LSPCON device */
> > > > > +	DRM_DEBUG_DRIVER("LSPCON detected\n");
> > > > > +	return true;
> > > > > +}
> > > > > +
> > > > > +enum lspcon_mode lspcon_probe(struct intel_lspcon *lspcon)
> > > > > +{
> > > > > +	enum lspcon_mode current_mode;
> > > > > +
> > > > > +	/* Detect a valid lspcon */
> > > > > +	if (!lspcon_detect_identifier(lspcon)) {
> > > > > +		DRM_DEBUG_DRIVER("Failed to find LSPCON
> > > > > identifier\n");
> > > > > +		return false;
> > > > > +	}
> > > > > +
> > > > > +	/* LSPCON's mode of operation */
> > > > > +	current_mode = lspcon_get_current_mode(lspcon);
> > > > > +	if (current_mode == lspcon_mode_invalid) {
> > > > > +		DRM_ERROR("Failed to read LSPCON mode\n");
> > > > > +		return false;
> > > > > +	}
> > > > > +
> > > > > +	/* All is well */
> > > > > +	lspcon->mode_of_op = current_mode;
> > > > > +	lspcon->active = true;
> > > > > +	return current_mode;
> > > > > +}
> > > > > +
> > > > > +bool lspcon_device_init(struct intel_lspcon *lspcon)
> > > > > +{
> > > > > +
> > > > > +	/* Lets check LSPCON now, probe the HW status */
> > > > > +	lspcon->active = false;
> > > > > +	lspcon->mode_of_op = lspcon_mode_invalid;
> > > > > +	if (!lspcon_probe(lspcon)) {
> > > > > +		DRM_ERROR("Failed to probe lspcon");
> > > > > +		return false;
> > > > > +	}
> > > > > +
> > > > > +	/* We wish to keep LSPCON in LS mode */
> > > > > +	if (lspcon->active && lspcon->mode_of_op !=
> > > > > lspcon_mode_ls) {
> > > > > +		if (lspcon_change_mode(lspcon,
> > > > > lspcon_mode_ls, true) < 0) {
> > > > > +			DRM_ERROR("LSPCON mode change to LS
> > > > > failed\n");
> > > > > +			return false;
> > > > > +		}
> > > > > +	}
> > > > > +	DRM_DEBUG_DRIVER("LSPCON init success\n");
> > > > > +	return true;
> > > > > +}
> > > > > +
> > > > >    struct edid *lspcon_get_edid(struct intel_lspcon *lspcon,
> > > > > struct drm_connector
> > > > >    						*connector
> > > > > )
> > > > >    {
> > > > > @@ -233,6 +371,7 @@ int intel_lspcon_init_connector(struct
> > > > > intel_digital_port *intel_dig_port)
> > > > >    	struct intel_encoder *intel_encoder =
> > > > > &intel_dig_port->base;
> > > > >    	struct drm_device *dev = intel_encoder->base.dev;
> > > > >    	struct drm_i915_private *dev_priv = dev-
> > > > > >dev_private;
> > > > > +	struct intel_lspcon *lspcon = &intel_dig_port-
> > > > > >lspcon;
> > > > >    	struct intel_connector *intel_connector;
> > > > >    	struct drm_connector *connector;
> > > > >    	enum port port = intel_dig_port->port;
> > > > > @@ -314,6 +453,12 @@ int intel_lspcon_init_connector(struct
> > > > > intel_digital_port *intel_dig_port)
> > > > >    		I915_WRITE(PEG_BAND_GAP_DATA, (temp &
> > > > > ~0xf) | 0xd);
> > > > >    	}
> > > > > 
> > > > > +	/* Now initialize the LSPCON device */
> > > > > +	if (!lspcon_device_init(lspcon)) {
> > > > > +		DRM_ERROR("LSPCON device init failed\n");
> > > > > +		goto fail;
> > > > > +	}
> > > > > +
> > > > >    	DRM_DEBUG_DRIVER("Success: LSPCON connector
> > > > > init\n");
> > > > >    	return 0;
> > > > > 
> > > > > --
> > > > > 1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux