Hi Shashank, On 07-02-2017 16:09, Sharma, Shashank wrote: > Thanks for the review Jose, my comments inline. > > > Regards > > Shashank > > > On 2/7/2017 4:24 PM, Jose Abreu wrote: >> Hi Shashank, >> >> >> Sorry for the late review. >> >> >> On 06-02-2017 13:59, Shashank Sharma wrote: >>> From: Thierry Reding <treding@xxxxxxxxxx> >>> >>> SCDC is a mechanism defined in the HDMI 2.0 specification >>> that allows >>> the source and sink devices to communicate. >>> >>> This commit introduces helpers to access the SCDC and >>> provides the >>> symbolic names for the various registers defined in the >>> specification. >>> >>> Signed-off-by: Thierry Reding <treding@xxxxxxxxxx> >>> Signed-off-by: Shashank Sharma <shashank.sharma@xxxxxxxxx> >>> --- >>> Documentation/gpu/drm-kms-helpers.rst | 12 ++++ >>> drivers/gpu/drm/Makefile | 3 +- >>> drivers/gpu/drm/drm_scdc_helper.c | 111 >>> ++++++++++++++++++++++++++++ >>> include/drm/drm_scdc_helper.h | 132 >>> ++++++++++++++++++++++++++++++++++ >>> 4 files changed, 257 insertions(+), 1 deletion(-) >>> create mode 100644 drivers/gpu/drm/drm_scdc_helper.c >>> create mode 100644 include/drm/drm_scdc_helper.h >>> >>> diff --git a/Documentation/gpu/drm-kms-helpers.rst >>> b/Documentation/gpu/drm-kms-helpers.rst >>> index 03040aa..7592756 100644 >>> --- a/Documentation/gpu/drm-kms-helpers.rst >>> +++ b/Documentation/gpu/drm-kms-helpers.rst >>> @@ -217,6 +217,18 @@ EDID Helper Functions Reference >>> .. kernel-doc:: drivers/gpu/drm/drm_edid.c >>> :export: >>> +SCDC Helper Functions Reference >>> +=============================== >>> + >>> +.. kernel-doc:: drivers/gpu/drm/drm_scdc_helper.c >>> + :doc: scdc helpers >>> + >>> +.. kernel-doc:: include/drm/drm_scdc_helper.h >>> + :internal: >>> + >>> +.. kernel-doc:: drivers/gpu/drm/drm_scdc_helper.c >>> + :export: >>> + >>> Rectangle Utilities Reference >>> ============================= >>> diff --git a/drivers/gpu/drm/Makefile >>> b/drivers/gpu/drm/Makefile >>> index 92de399..ad91cd9 100644 >>> --- a/drivers/gpu/drm/Makefile >>> +++ b/drivers/gpu/drm/Makefile >>> @@ -31,7 +31,8 @@ drm-$(CONFIG_DEBUG_FS) += drm_debugfs.o >>> drm_debugfs_crc.o >>> drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o >>> drm_probe_helper.o \ >>> drm_plane_helper.o drm_dp_mst_topology.o >>> drm_atomic_helper.o \ >>> drm_kms_helper_common.o drm_dp_dual_mode_helper.o \ >>> - drm_simple_kms_helper.o drm_modeset_helper.o >>> + drm_simple_kms_helper.o drm_modeset_helper.o \ >>> + drm_scdc_helper.o >>> drm_kms_helper-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += >>> drm_edid_load.o >>> drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += >>> drm_fb_helper.o >>> diff --git a/drivers/gpu/drm/drm_scdc_helper.c >>> b/drivers/gpu/drm/drm_scdc_helper.c >>> new file mode 100644 >>> index 0000000..fe0e121 >>> --- /dev/null >>> +++ b/drivers/gpu/drm/drm_scdc_helper.c >>> @@ -0,0 +1,111 @@ >>> +/* >>> + * Copyright (c) 2015 NVIDIA Corporation. All rights reserved. >>> + * >>> + * Permission is hereby granted, free of charge, to any >>> person obtaining a >>> + * copy of this software and associated documentation files >>> (the "Software"), >>> + * to deal in the Software without restriction, including >>> without limitation >>> + * the rights to use, copy, modify, merge, publish, >>> distribute, sub license, >>> + * and/or sell copies of the Software, and to permit persons >>> to whom the >>> + * Software is furnished to do so, subject to the following >>> conditions: >>> + * >>> + * The above copyright notice and this permission notice >>> (including the >>> + * next paragraph) shall be included in all copies or >>> substantial portions >>> + * of the Software. >>> + * >>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY >>> KIND, EXPRESS OR >>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF >>> MERCHANTABILITY, >>> + * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN >>> NO EVENT SHALL >>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, >>> DAMAGES OR OTHER >>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR >>> OTHERWISE, ARISING >>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE >>> USE OR OTHER >>> + * DEALINGS IN THE SOFTWARE. >>> + */ >>> + >>> +#include <linux/slab.h> >>> + >>> +#include <drm/drm_scdc_helper.h> >>> + >>> +/** >>> + * DOC: scdc helpers >>> + * >>> + * Status and Control Data Channel (SCDC) is a mechanism >>> introduced by the >>> + * HDMI 2.0 specification. It is a point-to-point protocol >>> that allows the >>> + * HDMI source and HDMI sink to exchange data. The same I2C >>> interface that >>> + * is used to access EDID serves as the transport mechanism >>> for SCDC. >>> + */ >>> + >>> +#define SCDC_I2C_SLAVE_ADDRESS 0x54 >>> + >>> +/** >>> + * drm_scdc_read - read a block of data from SCDC >>> + * @adapter: I2C controller >>> + * @offset: start offset of block to read >>> + * @buffer: return location for the block to read >>> + * @size: size of the block to read >>> + * >>> + * Reads a block of data from SCDC, starting at a given offset. >>> + * >>> + * Returns: >>> + * The number of bytes read from SCDC or a negative error >>> code on failure. >>> + */ >>> +ssize_t drm_scdc_read(struct i2c_adapter *adapter, u8 >>> offset, void *buffer, >>> + size_t size) >>> +{ >>> + struct i2c_msg msgs[2] = { >>> + { >>> + .addr = SCDC_I2C_SLAVE_ADDRESS, >>> + .flags = 0, >>> + .len = 1, >> .len = sizeof(offset) ? > Technically correct, but wouldn't that mean that we are > expecting to have I2C offsets with length more than one byte ? I just commented this because it would be more consistent but indeed you are correct. You can disregard my comment :) Best regards, Jose Miguel Abreu >> >>> + .buf = &offset, >>> + }, { >>> + .addr = SCDC_I2C_SLAVE_ADDRESS, >>> + .flags = I2C_M_RD, >>> + .len = size, >>> + .buf = buffer, >>> + } >>> + }; >>> + >>> + return i2c_transfer(adapter, msgs, ARRAY_SIZE(msgs)); >>> +} >>> +EXPORT_SYMBOL(drm_scdc_read); >>> + >>> +/** >>> + * drm_scdc_write - write a block of data to SCDC >>> + * @adapter: I2C controller >>> + * @offset: start offset of block to write >>> + * @buffer: block of data to write >>> + * @size: size of the block to write >>> + * >>> + * Writes a block of data to SCDC, starting at a given offset. >>> + * >>> + * Returns: >>> + * The number of bytes written to SCDC or a negative error >>> code on failure. >>> + */ >>> +ssize_t drm_scdc_write(struct i2c_adapter *adapter, u8 offset, >>> + const void *buffer, size_t size) >>> +{ >>> + struct i2c_msg msg = { >>> + .addr = SCDC_I2C_SLAVE_ADDRESS, >>> + .flags = 0, >>> + .len = 1 + size, >> .len = sizeof(offset) + size ? > Same as above. >> >>> + .buf = NULL, >>> + }; >>> + void *data; >>> + int err; >>> + >>> + data = kmalloc(1 + size, GFP_TEMPORARY); >> Same as above. > So on ... >>> + if (!data) >>> + return -ENOMEM; >>> + >>> + msg.buf = data; >>> + >>> + memcpy(data, &offset, sizeof(offset)); >>> + memcpy(data + 1, buffer, size); >> Same as above. >> > So on .. >> Best regards, >> Jose Miguel Abreu > - Shashank >>> + >>> + err = i2c_transfer(adapter, &msg, 1); >>> + >>> + kfree(data); >>> + >>> + return err; >>> +} >>> +EXPORT_SYMBOL(drm_scdc_write); >>> diff --git a/include/drm/drm_scdc_helper.h >>> b/include/drm/drm_scdc_helper.h >>> new file mode 100644 >>> index 0000000..93b07bc >>> --- /dev/null >>> +++ b/include/drm/drm_scdc_helper.h >>> @@ -0,0 +1,132 @@ >>> +/* >>> + * Copyright (c) 2015 NVIDIA Corporation. All rights reserved. >>> + * >>> + * Permission is hereby granted, free of charge, to any >>> person obtaining a >>> + * copy of this software and associated documentation files >>> (the "Software"), >>> + * to deal in the Software without restriction, including >>> without limitation >>> + * the rights to use, copy, modify, merge, publish, >>> distribute, sub license, >>> + * and/or sell copies of the Software, and to permit persons >>> to whom the >>> + * Software is furnished to do so, subject to the following >>> conditions: >>> + * >>> + * The above copyright notice and this permission notice >>> (including the >>> + * next paragraph) shall be included in all copies or >>> substantial portions >>> + * of the Software. >>> + * >>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY >>> KIND, EXPRESS OR >>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF >>> MERCHANTABILITY, >>> + * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN >>> NO EVENT SHALL >>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, >>> DAMAGES OR OTHER >>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR >>> OTHERWISE, ARISING >>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE >>> USE OR OTHER >>> + * DEALINGS IN THE SOFTWARE. >>> + */ >>> + >>> +#ifndef DRM_SCDC_HELPER_H >>> +#define DRM_SCDC_HELPER_H >>> + >>> +#include <linux/i2c.h> >>> +#include <linux/types.h> >>> + >>> +#define SCDC_SINK_VERSION 0x01 >>> + >>> +#define SCDC_SOURCE_VERSION 0x02 >>> + >>> +#define SCDC_UPDATE_0 0x10 >>> +#define SCDC_READ_REQUEST_TEST (1 << 2) >>> +#define SCDC_CED_UPDATE (1 << 1) >>> +#define SCDC_STATUS_UPDATE (1 << 0) >>> + >>> +#define SCDC_UPDATE_1 0x11 >>> + >>> +#define SCDC_TMDS_CONFIG 0x20 >>> +#define SCDC_TMDS_BIT_CLOCK_RATIO_BY_40 (1 << 1) >>> +#define SCDC_TMDS_BIT_CLOCK_RATIO_BY_10 (0 << 1) >>> +#define SCDC_SCRAMBLING_ENABLE (1 << 0) >>> + >>> +#define SCDC_SCRAMBLER_STATUS 0x21 >>> +#define SCDC_SCRAMBLING_STATUS (1 << 0) >>> + >>> +#define SCDC_CONFIG_0 0x30 >>> +#define SCDC_READ_REQUEST_ENABLE (1 << 0) >>> + >>> +#define SCDC_STATUS_FLAGS_0 0x40 >>> +#define SCDC_CH2_LOCK (1 < 3) >>> +#define SCDC_CH1_LOCK (1 < 2) >>> +#define SCDC_CH0_LOCK (1 < 1) >>> +#define SCDC_CH_LOCK_MASK (SCDC_CH2_LOCK | SCDC_CH1_LOCK | >>> SCDC_CH0_LOCK) >>> +#define SCDC_CLOCK_DETECT (1 << 0) >>> + >>> +#define SCDC_STATUS_FLAGS_1 0x41 >>> + >>> +#define SCDC_ERR_DET_0_L 0x50 >>> +#define SCDC_ERR_DET_0_H 0x51 >>> +#define SCDC_ERR_DET_1_L 0x52 >>> +#define SCDC_ERR_DET_1_H 0x53 >>> +#define SCDC_ERR_DET_2_L 0x54 >>> +#define SCDC_ERR_DET_2_H 0x55 >>> +#define SCDC_CHANNEL_VALID (1 << 7) >>> + >>> +#define SCDC_ERR_DET_CHECKSUM 0x56 >>> + >>> +#define SCDC_TEST_CONFIG_0 0xc0 >>> +#define SCDC_TEST_READ_REQUEST (1 << 7) >>> +#define SCDC_TEST_READ_REQUEST_DELAY(x) ((x) & 0x7f) >>> + >>> +#define SCDC_MANUFACTURER_IEEE_OUI 0xd0 >>> +#define SCDC_MANUFACTURER_IEEE_OUI_SIZE 3 >>> + >>> +#define SCDC_DEVICE_ID 0xd3 >>> +#define SCDC_DEVICE_ID_SIZE 8 >>> + >>> +#define SCDC_DEVICE_HARDWARE_REVISION 0xdb >>> +#define SCDC_DEVICE_HARDWARE_REVISION_MAJOR(x) (((x) >> 4) >>> & 0xf) >>> +#define SCDC_DEVICE_HARDWARE_REVISION_MINOR(x) (((x) >> 0) >>> & 0xf) >>> + >>> +#define SCDC_DEVICE_SOFTWARE_MAJOR_REVISION 0xdc >>> +#define SCDC_DEVICE_SOFTWARE_MINOR_REVISION 0xdd >>> + >>> +#define SCDC_MANUFACTURER_SPECIFIC 0xde >>> +#define SCDC_MANUFACTURER_SPECIFIC_SIZE 34 >>> + >>> +ssize_t drm_scdc_read(struct i2c_adapter *adapter, u8 >>> offset, void *buffer, >>> + size_t size); >>> +ssize_t drm_scdc_write(struct i2c_adapter *adapter, u8 offset, >>> + const void *buffer, size_t size); >>> + >>> +/** >>> + * drm_scdc_readb - read a single byte from SCDC >>> + * @adapter: I2C adapter >>> + * @offset: offset of register to read >>> + * @value: return location for the register value >>> + * >>> + * Reads a single byte from SCDC. This is a convenience >>> wrapper around the >>> + * drm_scdc_read() function. >>> + * >>> + * Returns: >>> + * 0 on success or a negative error code on failure. >>> + */ >>> +static inline int drm_scdc_readb(struct i2c_adapter >>> *adapter, u8 offset, >>> + u8 *value) >>> +{ >>> + return drm_scdc_read(adapter, offset, value, >>> sizeof(*value)); >>> +} >>> + >>> +/** >>> + * drm_scdc_writeb - write a single byte to SCDC >>> + * @adapter: I2C adapter >>> + * @offset: offset of register to read >>> + * @value: return location for the register value >>> + * >>> + * Writes a single byte to SCDC. This is a convenience >>> wrapper around the >>> + * drm_scdc_write() function. >>> + * >>> + * Returns: >>> + * 0 on success or a negative error code on failure. >>> + */ >>> +static inline int drm_scdc_writeb(struct i2c_adapter >>> *adapter, u8 offset, >>> + u8 value) >>> +{ >>> + return drm_scdc_write(adapter, offset, &value, >>> sizeof(value)); >>> +} >>> + >>> +#endif > _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx