On Tue, May 03, 2016 at 09:15:48PM +0530, Sharma, Shashank wrote:
> Regards
> Shashank
>
> On 5/2/2016 7:19 PM, Ville Syrjälä wrote:
> > On Mon, Apr 04, 2016 at 05:31:44PM +0530, Shashank Sharma wrote:
> >> This patch adds support for LSPCON devices in dp++ adaptor
> >> helper layer. LSPCON is DP++ type-2 adaptor with some customized
> >> functionalities, to provide additional features in Intel Gen9 HW.
> >>
> >> LSPCON needs I2C-over-aux support to read/write control and
> >> data registers. This patch adds following:
> >> - Functions for I2C over aux read/write
> >> - Function to read EDID using I2c-over-aux
> >> - Function to identify LSPCON among type-2 DP adaptors
> >>
> >> Signed-off-by: Shashank Sharma <shashank.sharma@xxxxxxxxx>
> >> ---
> >> drivers/gpu/drm/drm_dp_dual_mode_helper.c | 100 ++++++++++++++++++++++++++++++
> >> include/drm/drm_dp_dual_mode_helper.h | 10 +++
> >> 2 files changed, 110 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/drm_dp_dual_mode_helper.c b/drivers/gpu/drm/drm_dp_dual_mode_helper.c
> >> index b72b7bb..ce8e11c 100644
> >> --- a/drivers/gpu/drm/drm_dp_dual_mode_helper.c
> >> +++ b/drivers/gpu/drm/drm_dp_dual_mode_helper.c
> >> @@ -132,6 +132,99 @@ ssize_t drm_dp_dual_mode_write(struct i2c_adapter *adapter,
> >> }
> >> EXPORT_SYMBOL(drm_dp_dual_mode_write);
> >>
> >> +int drm_dp_dual_mode_get_edid(void *data,
> >> + u8 *buf, unsigned int block, size_t len)
> >> +{
> >> + struct i2c_adapter *adapter = data;
> >> + unsigned char start = block * EDID_LENGTH;
> >> + unsigned char segment = block >> 1;
> >> + unsigned char xfers = segment ? 3 : 2;
> >> + int ret, retries = 5;
> >> +
> >> + do {
> >> + struct i2c_msg msgs[] = {
> >> + {
> >> + .addr = DP_DUAL_MODE_DDC_SEGMENT_ADDR,
> >> + .flags = 0,
> >> + .len = 1,
> >> + .buf = &segment,
> >> + }, {
> >> + .addr = DDC_ADDR,
> >> + .flags = 0,
> >> + .len = 1,
> >> + .buf = &start,
> >> + }, {
> >> + .addr = DDC_ADDR,
> >> + .flags = I2C_M_RD,
> >> + .len = len,
> >> + .buf = buf,
> >> + }
> >> + };
> >> +
> >> + ret = adapter->algo->master_xfer(adapter, &msgs[3 - xfers],
> >> + xfers);
> >> +
> >> + if (ret == -ENXIO) {
> >> + DRM_ERROR("Non-existent adapter %s\n",
> >> + adapter->name);
> >> + break;
> >> + }
> >> + } while (ret != xfers && --retries);
> >> +
> >> + return ret == xfers ? 0 : -1;
> >> +}
> >> +EXPORT_SYMBOL(drm_dp_dual_mode_get_edid);
> >
> > This sort of stuff shouldn't be needed. All that should be needed is
> > passing the right i2c adapter to drm_get_edid() and whatnot.
> >
> Yeah, agree, we can optimize this.
> >> +
> >> +/*
> >> +* drm_dp_dual_mode_ioa_xfer
> >> +* Few dp->hdmi type 2 adaptors allow i2c_over_aux read/write
> >> +* to the control and status registers. These functions help
> >> +* to read/write from those.
> >> +*/
> >> +static int drm_dp_dual_mode_ioa_xfer(struct i2c_adapter *adapter,
> >> + u8 *buffer, u8 offset, u8 no_of_bytes, u8 rw_flag)
> >> +{
> >> + int err = 0;
> >> +
> >> + struct i2c_msg msgs[] = {
> >> + {
> >> + .addr = DP_DUAL_MODE_SLAVE_ADDRESS,
> >> + .flags = 0,
> >> + .len = 1,
> >> + .buf = &offset,
> >> + }, {
> >> + .addr = DP_DUAL_MODE_SLAVE_ADDRESS,
> >> + .flags = rw_flag,
> >> + .len = no_of_bytes,
> >> + .buf = buffer,
> >> + }
> >> + };
> >> +
> >> + /* I2C over AUX here */
> >> + err = adapter->algo->master_xfer(adapter, msgs, 2);
> >> + if (err < 0)
> >> + DRM_ERROR("LSPCON: Failed I2C over Aux read(addr=0x%x)\n",
> >> + (unsigned int)offset);
> >> +
> >> + return err;
> >> +}
> >> +
> >> +int drm_dp_dual_mode_ioa_read(struct i2c_adapter *adapter, u8 *buffer,
> >> + u8 offset, u8 no_of_bytes)
> >> +{
> >> + return drm_dp_dual_mode_ioa_xfer(adapter, buffer, offset,
> >> + no_of_bytes, I2C_M_RD);
> >> +}
> >> +EXPORT_SYMBOL(drm_dp_dual_mode_ioa_read);
> >> +
> >> +int drm_dp_dual_mode_ioa_write(struct i2c_adapter *adapter, u8 *buffer,
> >> + u8 offset, u8 no_of_bytes)
> >> +{
> >> + return drm_dp_dual_mode_ioa_xfer(adapter, buffer, offset,
> >> + no_of_bytes, 0);
> >> +}
> >> +EXPORT_SYMBOL(drm_dp_dual_mode_ioa_write);
> >
> > Aren't these just dupes of the dual mode helper read/write functions?
> No, I2C over aux read uses adapter->algo->master_xfer function, which is
> different in case of aux channel read/write. dp_dual_mode uses i2c_xfer.
.master_xfer() is just an internal implementation detail of the i2c
bus/algo driver. You're not meant call it directly. i2c_transfer() is
what you call, and it will end up calling .master_xfer() internally.
> >
> >> +
> >> static bool is_hdmi_adaptor(const char hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN])
> >> {
> >> static const char dp_dual_mode_hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN] =
> >> @@ -141,6 +234,11 @@ static bool is_hdmi_adaptor(const char hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN])
> >> sizeof(dp_dual_mode_hdmi_id)) == 0;
> >> }
> >>
> >> +bool is_lspcon_adaptor(const uint8_t adaptor_id)
> >> +{
> >> + return adaptor_id == 0xa8;
> >> +}
> >> +
> >> /**
> >> * drm_dp_dual_mode_detect - Identyfy the DP dual mode adaptor
> >> * adapter: I2C adapter for the DDC bus
> >> @@ -197,6 +295,8 @@ enum drm_dp_dual_mode_type drm_dp_dual_mode_detect(struct i2c_adapter *adapter)
> >> &adaptor_id, sizeof(adaptor_id));
> >> if (ret || (adaptor_id != (DP_DUAL_MODE_TYPE_TYPE2 |
> >> DP_DUAL_MODE_REV_TYPE2))) {
> >> + if (is_lspcon_adaptor(adaptor_id))
> >> + return DRM_DP_DUAL_MODE_TYPE2_LSPCON;
> >
> > We should reorganize this a bit I think. Probably should end up looking
> > somehting like:
> >
> > bool is_type2_adaptor();
> > bool is_lspcon_adaptor();
> >
> > if (ret == 0) {
> > if (is_lspcon_adaptor())
> > return TYPE2_LSPCON;
> > else if (is_type2_adaptor() && is_hdmi_adaptor)
> > return TYPE2_HDMI;
> > else if (is_type2_adaptor())
> > return TYPE2_DVI;
> > }
> >
> > if (is_hdmi...)
> > return TYPE1_HDMI;
> > else
> > return TYPE1_DVI;
> >
> >
> Yep, this is a good suggestion.
I already reogranized the type1/2 detection to match this pattern in my
repost of the dp++ stuff, so should be easy to drop this in:
https://lists.freedesktop.org/archives/dri-devel/2016-May/106535.html
> >> if (is_hdmi_adaptor(hdmi_id))
> >> return DRM_DP_DUAL_MODE_TYPE1_HDMI;
> >> else
> >> diff --git a/include/drm/drm_dp_dual_mode_helper.h b/include/drm/drm_dp_dual_mode_helper.h
> >> index 8f8a8dc..26e3dd8 100644
> >> --- a/include/drm/drm_dp_dual_mode_helper.h
> >> +++ b/include/drm/drm_dp_dual_mode_helper.h
> >> @@ -24,6 +24,7 @@
> >> #define DRM_DP_DUAL_MODE_HELPER_H
> >>
> >> #include <linux/types.h>
> >> +#include <drm/drm_edid.h>
> >>
> >> /*
> >> * Optional for type 1 DVI adaptors
> >> @@ -55,6 +56,7 @@
> >> #define DP_DUAL_MODE_CEC_ENABLE 0x01
> >> #define DP_DUAL_MODE_I2C_SPEED_CTRL 0x22
> >> #define DP_DUAL_MODE_LAST_RESERVED 0xff
> >> +#define DP_DUAL_MODE_DDC_SEGMENT_ADDR 0x30
> >>
> >> struct i2c_adapter;
> >>
> >> @@ -69,6 +71,7 @@ enum drm_dp_dual_mode_type {
> >> DRM_DP_DUAL_MODE_TYPE1_HDMI,
> >> DRM_DP_DUAL_MODE_TYPE2_DVI,
> >> DRM_DP_DUAL_MODE_TYPE2_HDMI,
> >> + DRM_DP_DUAL_MODE_TYPE2_LSPCON,
> >> };
> >>
> >> enum drm_dp_dual_mode_type drm_dp_dual_mode_detect(struct i2c_adapter *adapter);
> >> @@ -81,4 +84,11 @@ drm_dp_dual_mode_set_tmds_output(struct i2c_adapter *adapter, bool enable);
> >> const char
> >> *drm_dp_get_dual_mode_type_name(enum drm_dp_dual_mode_type type);
> >>
> >> +int drm_dp_dual_mode_ioa_read(struct i2c_adapter *adapter, u8 *buffer,
> >> + u8 offset, u8 no_of_bytes);
> >> +int drm_dp_dual_mode_ioa_write(struct i2c_adapter *adapter, u8 *buffer,
> >> + u8 offset, u8 no_of_bytes);
> >> +int drm_dp_dual_mode_get_edid(void *data,
> >> + u8 *buf, unsigned int block, size_t len);
> >> +
> >> #endif
> >> --
> >> 1.9.1
> >
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
