>-----Original Message-----
>From: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
>Sent: Monday, October 14, 2024 7:53 PM
>To: Hermes Wu (吳佳宏) <Hermes.Wu@xxxxxxxxxx>
>Cc: Pin-yen Lin <treapking@xxxxxxxxxxxx>; Kenneth Hung (洪家倫) <Kenneth.Hung@xxxxxxxxxx>; Pet Weng (翁玉芬) <Pet.Weng@xxxxxxxxxx>; Andrzej Hajda <andrzej.hajda@xxxxxxxxx>; Neil Armstrong <neil.armstrong@xxxxxxxxxx>; Robert Foss <rfoss@xxxxxxxxxx>; Laurent Pinchart <Laurent.pinchart@xxxxxxxxxxxxxxxx>; Jonas Karlman <jonas@xxxxxxxxx>; Jernej Skrabec <jernej.skrabec@xxxxxxxxx>; Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>; Maxime Ripard <mripard@xxxxxxxxxx>; Thomas Zimmermann <tzimmermann@xxxxxxx>; David Airlie <airlied@xxxxxxxxx>; Simona Vetter <simona@xxxxxxxx>; open list:DRM DRIVERS <dri-devel@xxxxxxxxxxxxxxxxxxxxx>; open list <linux-kernel@xxxxxxxxxxxxxxx>
>Subject: Re: [PATCH v5 03/10] drm/bridge: it6505: add AUX operation for HDCP KSV list read
>
>On Tue, Oct 01, 2024 at 02:43:02PM +0800, Hermes Wu wrote:
>> From: Hermes Wu <Hermes.wu@xxxxxxxxxx>
>>
>> HDCP KSV list readback can choose to use AUX FIFO or general data register.
>> For some DisplayPort devices, the KSV list must be read in 5 byte boundaries.
>> The original AUX read command does not support these devices.
>>
>> The AUX command operation control register "REG_AUX_CMD_REQ" uses b[3:0] as AUX operacion control, and b[7:4] are status bits and read only.
>> To change KSV read operation uses "CMD_AUX_NATIVE_READ" from using the data registers to using AUX FIFO.
>> The extended command "CMD_AUX_GET_KSV_LIST" is added as "CMD_AUX_NATIVE_READ" with the 0x10 flag which selects AUX FIFO mode.
>
>Please keep the commit message wrapped at 72-75 chars.
>
>Other than that:
Should I send new v6 patches with commit message wrapped at 72-75 chars?
Thanks.
>
>Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
>
>
>>
>> Signed-off-by: Hermes Wu <Hermes.wu@xxxxxxxxxx>
>> ---
>> drivers/gpu/drm/bridge/ite-it6505.c | 16 +++++++++++-----
>> 1 file changed, 11 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/ite-it6505.c
>> b/drivers/gpu/drm/bridge/ite-it6505.c
>> index 864fab7e388f..9a022c095af4 100644
>> --- a/drivers/gpu/drm/bridge/ite-it6505.c
>> +++ b/drivers/gpu/drm/bridge/ite-it6505.c
>> @@ -126,6 +126,7 @@
>> #define REG_AUX_OUT_DATA0 0x27
>>
>> #define REG_AUX_CMD_REQ 0x2B
>> +#define M_AUX_REQ_CMD 0x0F
>> #define AUX_BUSY BIT(5)
>>
>> #define REG_AUX_DATA_0_7 0x2C
>> @@ -324,6 +325,9 @@ enum aux_cmd_type {
>> CMD_AUX_NATIVE_READ = 0x0,
>> CMD_AUX_NATIVE_WRITE = 0x5,
>> CMD_AUX_I2C_EDID_READ = 0xB,
>> +
>> + /* KSV read with AUX FIFO extend from CMD_AUX_NATIVE_READ*/
>> + CMD_AUX_GET_KSV_LIST = 0x10,
>> };
>>
>> enum aux_cmd_reply {
>> @@ -965,7 +969,8 @@ static ssize_t it6505_aux_operation(struct it6505 *it6505,
>> it6505_set_bits(it6505, REG_AUX_CTRL, AUX_USER_MODE, AUX_USER_MODE);
>>
>> aux_op_start:
>> - if (cmd == CMD_AUX_I2C_EDID_READ) {
>> + /* HW AUX FIFO supports only EDID and DCPD KSV FIFO area */
>> + if (cmd == CMD_AUX_I2C_EDID_READ || cmd == CMD_AUX_GET_KSV_LIST) {
>> /* AUX EDID FIFO has max length of AUX_FIFO_MAX_SIZE bytes. */
>> size = min_t(size_t, size, AUX_FIFO_MAX_SIZE);
>> /* Enable AUX FIFO read back and clear FIFO */ @@ -996,7 +1001,7 @@
>> static ssize_t it6505_aux_operation(struct it6505 *it6505,
>> size);
>>
>> /* Aux Fire */
>> - it6505_write(it6505, REG_AUX_CMD_REQ, cmd);
>> + it6505_write(it6505, REG_AUX_CMD_REQ, FIELD_GET(M_AUX_REQ_CMD,
>> +cmd));
>>
>> ret = it6505_aux_wait(it6505);
>> if (ret < 0)
>> @@ -1030,7 +1035,7 @@ static ssize_t it6505_aux_operation(struct it6505 *it6505,
>> goto aux_op_start;
>> }
>>
>> - if (cmd == CMD_AUX_I2C_EDID_READ) {
>> + if (cmd == CMD_AUX_I2C_EDID_READ || cmd == CMD_AUX_GET_KSV_LIST) {
>> for (i = 0; i < size; i++) {
>> ret = it6505_read(it6505, REG_AUX_DATA_FIFO);
>> if (ret < 0)
>> @@ -1055,7 +1060,7 @@ static ssize_t it6505_aux_operation(struct it6505 *it6505,
>> ret = i;
>>
>> aux_op_err:
>> - if (cmd == CMD_AUX_I2C_EDID_READ) {
>> + if (cmd == CMD_AUX_I2C_EDID_READ || cmd == CMD_AUX_GET_KSV_LIST) {
>> /* clear AUX FIFO */
>> it6505_set_bits(it6505, REG_AUX_CTRL,
>> AUX_EN_FIFO_READ | CLR_EDID_FIFO, @@ -1076,7 +1081,8 @@ static
>> ssize_t it6505_aux_do_transfer(struct it6505 *it6505,
>> size_t size, enum aux_cmd_reply *reply) {
>> int i, ret_size, ret = 0, request_size;
>> - int fifo_max_size = (cmd == CMD_AUX_I2C_EDID_READ) ? AUX_FIFO_MAX_SIZE : 4;
>> + int fifo_max_size = (cmd == CMD_AUX_I2C_EDID_READ || cmd == CMD_AUX_GET_KSV_LIST) ?
>> + AUX_FIFO_MAX_SIZE : 4;
>>
>> mutex_lock(&it6505->aux_lock);
>> i = 0;
>> --
>> 2.34.1
>>
>
>--
>With best wishes
>Dmitry
>
>
BR,
Hermes
