| Message ID | 1459771308-4405-10-git-send-email-shashank.sharma@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@intel.com> > Signed-off-by: Akashdeep Sharma <Akashdeep.sharma@intel.com> > --- > 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. > + > +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(adaptor_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
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@intel.com> >> Signed-off-by: Akashdeep Sharma <Akashdeep.sharma@intel.com> >> --- >> 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 ? >> + >> +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(adaptor_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 >
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@intel.com> > >> Signed-off-by: Akashdeep Sharma <Akashdeep.sharma@intel.com> > >> --- > >> 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. > >> + > >> +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(adaptor_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 > >
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@intel.com> >>>> Signed-off-by: Akashdeep Sharma <Akashdeep.sharma@intel.com> >>>> --- >>>> 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. >>>> + >>>> +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(adaptor_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 >>> >
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@intel.com> > > > > > Signed-off-by: Akashdeep Sharma <Akashdeep.sharma@intel.com> > > > > > --- > > > > > 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
On Wed, May 04, 2016 at 09:09:06PM +0000, Zanoni, Paulo R wrote: > 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@intel.com> > > > > > > Signed-off-by: Akashdeep Sharma <Akashdeep.sharma@intel.com> > > > > > > --- > > > > > > 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. Well, when/if that happens we'll have to change the way the detection works, eg. just leave it up to the driver or something. As always, it's hard to make predictions, especially about the future.
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; +} + +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(adaptor_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;