Re: [PATCH 08/12] drm: Add lspcon (custom type2 dp->hdmi) support

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Tue, May 03, 2016 at 09:39:49PM +0530, Sharma, Shashank wrote:
> Regards
> Shashank
> 
> On 5/3/2016 9:29 PM, Ville Syrjälä wrote:
> > 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@intel.com>
> >>>> ---
> >>>>    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.
> >
> Please correct me if its not the case, but the master_xfer function 
> getting loaded in drm_dp_helper layer is drm_dp_i2c_xfer() which does a 
> lot of stuff. You mean to say i2c_xfer() function call will map to 
> drm_dp_i2c_xfer ?

Yes, assuming you pass in the correct i2c adapter.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx