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 <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
> 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 <joabreu@xxxxxxxxxxxx>
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
