Hi Shashank,
On 08-02-2017 12:59, Sharma, Shashank wrote: > Regards > > Shashank > > > On 2/8/2017 4:57 PM, Jose Abreu wrote: >> 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 <tred...@nvidia.com> >>>>> >>>>> 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 <tred...@nvidia.com> >>>>> Signed-off-by: Shashank Sharma <shashank.sha...@intel.com> >>>>> --- >>>>> 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 > Thanks, is everything else works for you, would you mind a r-b ? > - Shashank Sure, everything looks nice by me. Maybe wait for more comments. You can add: Reviewed-by: Jose Abreu <joab...@synopsys.com> 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 > _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel