Re: [PATCH 09/17] drm/msm/dp: move parity calculation to dp_catalog

2024-01-27 Thread Paloma Arellano 



On 1/25/2024 1:32 PM, Dmitry Baryshkov wrote:

On 25/01/2024 21:38, Paloma Arellano wrote:

Parity calculation is necessary for VSC SDP implementation, therefore
move it to dp_catalog so it usable by both SDP programming and
dp_audio.c

Signed-off-by: Paloma Arellano 
---
  drivers/gpu/drm/msm/dp/dp_audio.c   | 100 
  drivers/gpu/drm/msm/dp/dp_catalog.h |  72 
  2 files changed, 86 insertions(+), 86 deletions(-)


There is nothing catalog-uish in the parity calculation. Just add 
dp_utils.c. Another options is to push it to the drm/display/


LGTM otherwise.

Ack. Will move to dp_utils.c




diff --git a/drivers/gpu/drm/msm/dp/dp_audio.c 
b/drivers/gpu/drm/msm/dp/dp_audio.c

index 4a2e479723a85..7aa785018155a 100644
--- a/drivers/gpu/drm/msm/dp/dp_audio.c
+++ b/drivers/gpu/drm/msm/dp/dp_audio.c
@@ -16,13 +16,6 @@
  #include "dp_panel.h"
  #include "dp_display.h"
  -#define HEADER_BYTE_2_BIT 0
-#define PARITY_BYTE_2_BIT 8
-#define HEADER_BYTE_1_BIT    16
-#define PARITY_BYTE_1_BIT    24
-#define HEADER_BYTE_3_BIT    16
-#define PARITY_BYTE_3_BIT    24
-
  struct dp_audio_private {
  struct platform_device *audio_pdev;
  struct platform_device *pdev;
@@ -36,71 +29,6 @@ struct dp_audio_private {
  struct dp_audio dp_audio;
  };
  -static u8 dp_audio_get_g0_value(u8 data)
-{
-    u8 c[4];
-    u8 g[4];
-    u8 ret_data = 0;
-    u8 i;
-
-    for (i = 0; i < 4; i++)
-    c[i] = (data >> i) & 0x01;
-
-    g[0] = c[3];
-    g[1] = c[0] ^ c[3];
-    g[2] = c[1];
-    g[3] = c[2];
-
-    for (i = 0; i < 4; i++)
-    ret_data = ((g[i] & 0x01) << i) | ret_data;
-
-    return ret_data;
-}
-
-static u8 dp_audio_get_g1_value(u8 data)
-{
-    u8 c[4];
-    u8 g[4];
-    u8 ret_data = 0;
-    u8 i;
-
-    for (i = 0; i < 4; i++)
-    c[i] = (data >> i) & 0x01;
-
-    g[0] = c[0] ^ c[3];
-    g[1] = c[0] ^ c[1] ^ c[3];
-    g[2] = c[1] ^ c[2];
-    g[3] = c[2] ^ c[3];
-
-    for (i = 0; i < 4; i++)
-    ret_data = ((g[i] & 0x01) << i) | ret_data;
-
-    return ret_data;
-}
-
-static u8 dp_audio_calculate_parity(u32 data)
-{
-    u8 x0 = 0;
-    u8 x1 = 0;
-    u8 ci = 0;
-    u8 iData = 0;
-    u8 i = 0;
-    u8 parity_byte;
-    u8 num_byte = (data & 0xFF00) > 0 ? 8 : 2;
-
-    for (i = 0; i < num_byte; i++) {
-    iData = (data >> i*4) & 0xF;
-
-    ci = iData ^ x1;
-    x1 = x0 ^ dp_audio_get_g1_value(ci);
-    x0 = dp_audio_get_g0_value(ci);
-    }
-
-    parity_byte = x1 | (x0 << 4);
-
-    return parity_byte;
-}
-
  static u32 dp_audio_get_header(struct dp_catalog *catalog,
  enum dp_catalog_audio_sdp_type sdp,
  enum dp_catalog_audio_header_type header)
@@ -134,7 +62,7 @@ static void dp_audio_stream_sdp(struct 
dp_audio_private *audio)

  DP_AUDIO_SDP_STREAM, DP_AUDIO_SDP_HEADER_1);
    new_value = 0x02;
-    parity_byte = dp_audio_calculate_parity(new_value);
+    parity_byte = dp_catalog_calculate_parity(new_value);
  value |= ((new_value << HEADER_BYTE_1_BIT)
  | (parity_byte << PARITY_BYTE_1_BIT));
  drm_dbg_dp(audio->drm_dev,
@@ -147,7 +75,7 @@ static void dp_audio_stream_sdp(struct 
dp_audio_private *audio)

  value = dp_audio_get_header(catalog,
  DP_AUDIO_SDP_STREAM, DP_AUDIO_SDP_HEADER_2);
  new_value = value;
-    parity_byte = dp_audio_calculate_parity(new_value);
+    parity_byte = dp_catalog_calculate_parity(new_value);
  value |= ((new_value << HEADER_BYTE_2_BIT)
  | (parity_byte << PARITY_BYTE_2_BIT));
  drm_dbg_dp(audio->drm_dev,
@@ -162,7 +90,7 @@ static void dp_audio_stream_sdp(struct 
dp_audio_private *audio)

  DP_AUDIO_SDP_STREAM, DP_AUDIO_SDP_HEADER_3);
    new_value = audio->channels - 1;
-    parity_byte = dp_audio_calculate_parity(new_value);
+    parity_byte = dp_catalog_calculate_parity(new_value);
  value |= ((new_value << HEADER_BYTE_3_BIT)
  | (parity_byte << PARITY_BYTE_3_BIT));
  drm_dbg_dp(audio->drm_dev,
@@ -184,7 +112,7 @@ static void dp_audio_timestamp_sdp(struct 
dp_audio_private *audio)

  DP_AUDIO_SDP_TIMESTAMP, DP_AUDIO_SDP_HEADER_1);
    new_value = 0x1;
-    parity_byte = dp_audio_calculate_parity(new_value);
+    parity_byte = dp_catalog_calculate_parity(new_value);
  value |= ((new_value << HEADER_BYTE_1_BIT)
  | (parity_byte << PARITY_BYTE_1_BIT));
  drm_dbg_dp(audio->drm_dev,
@@ -198,7 +126,7 @@ static void dp_audio_timestamp_sdp(struct 
dp_audio_private *audio)

  DP_AUDIO_SDP_TIMESTAMP, DP_AUDIO_SDP_HEADER_2);
    new_value = 0x17;
-    parity_byte = dp_audio_calculate_parity(new_value);
+    parity_byte = dp_catalog_calculate_parity(new_value);
  value |= ((new_value << HEADER_BYTE_2_BIT)
  | (parity_byte << PARITY_BYTE_2_BIT));
  drm_dbg_dp(audio->drm_dev,
@@ -212,7 +140,7 @@ static void dp_audio_timestamp_sdp(struct 
dp_audio_private *audio)

 

Re: [PATCH 09/17] drm/msm/dp: move parity calculation to dp_catalog

2024-01-25 Thread Dmitry Baryshkov 

On 25/01/2024 21:38, Paloma Arellano wrote:

Parity calculation is necessary for VSC SDP implementation, therefore
move it to dp_catalog so it usable by both SDP programming and
dp_audio.c

Signed-off-by: Paloma Arellano 
---
  drivers/gpu/drm/msm/dp/dp_audio.c   | 100 
  drivers/gpu/drm/msm/dp/dp_catalog.h |  72 
  2 files changed, 86 insertions(+), 86 deletions(-)


There is nothing catalog-uish in the parity calculation. Just add 
dp_utils.c. Another options is to push it to the drm/display/


LGTM otherwise.



diff --git a/drivers/gpu/drm/msm/dp/dp_audio.c 
b/drivers/gpu/drm/msm/dp/dp_audio.c
index 4a2e479723a85..7aa785018155a 100644
--- a/drivers/gpu/drm/msm/dp/dp_audio.c
+++ b/drivers/gpu/drm/msm/dp/dp_audio.c
@@ -16,13 +16,6 @@
  #include "dp_panel.h"
  #include "dp_display.h"
  
-#define HEADER_BYTE_2_BIT	 0

-#define PARITY_BYTE_2_BIT   8
-#define HEADER_BYTE_1_BIT  16
-#define PARITY_BYTE_1_BIT  24
-#define HEADER_BYTE_3_BIT  16
-#define PARITY_BYTE_3_BIT  24
-
  struct dp_audio_private {
struct platform_device *audio_pdev;
struct platform_device *pdev;
@@ -36,71 +29,6 @@ struct dp_audio_private {
struct dp_audio dp_audio;
  };
  
-static u8 dp_audio_get_g0_value(u8 data)

-{
-   u8 c[4];
-   u8 g[4];
-   u8 ret_data = 0;
-   u8 i;
-
-   for (i = 0; i < 4; i++)
-   c[i] = (data >> i) & 0x01;
-
-   g[0] = c[3];
-   g[1] = c[0] ^ c[3];
-   g[2] = c[1];
-   g[3] = c[2];
-
-   for (i = 0; i < 4; i++)
-   ret_data = ((g[i] & 0x01) << i) | ret_data;
-
-   return ret_data;
-}
-
-static u8 dp_audio_get_g1_value(u8 data)
-{
-   u8 c[4];
-   u8 g[4];
-   u8 ret_data = 0;
-   u8 i;
-
-   for (i = 0; i < 4; i++)
-   c[i] = (data >> i) & 0x01;
-
-   g[0] = c[0] ^ c[3];
-   g[1] = c[0] ^ c[1] ^ c[3];
-   g[2] = c[1] ^ c[2];
-   g[3] = c[2] ^ c[3];
-
-   for (i = 0; i < 4; i++)
-   ret_data = ((g[i] & 0x01) << i) | ret_data;
-
-   return ret_data;
-}
-
-static u8 dp_audio_calculate_parity(u32 data)
-{
-   u8 x0 = 0;
-   u8 x1 = 0;
-   u8 ci = 0;
-   u8 iData = 0;
-   u8 i = 0;
-   u8 parity_byte;
-   u8 num_byte = (data & 0xFF00) > 0 ? 8 : 2;
-
-   for (i = 0; i < num_byte; i++) {
-   iData = (data >> i*4) & 0xF;
-
-   ci = iData ^ x1;
-   x1 = x0 ^ dp_audio_get_g1_value(ci);
-   x0 = dp_audio_get_g0_value(ci);
-   }
-
-   parity_byte = x1 | (x0 << 4);
-
-   return parity_byte;
-}
-
  static u32 dp_audio_get_header(struct dp_catalog *catalog,
enum dp_catalog_audio_sdp_type sdp,
enum dp_catalog_audio_header_type header)
@@ -134,7 +62,7 @@ static void dp_audio_stream_sdp(struct dp_audio_private 
*audio)
DP_AUDIO_SDP_STREAM, DP_AUDIO_SDP_HEADER_1);
  
  	new_value = 0x02;

-   parity_byte = dp_audio_calculate_parity(new_value);
+   parity_byte = dp_catalog_calculate_parity(new_value);
value |= ((new_value << HEADER_BYTE_1_BIT)
| (parity_byte << PARITY_BYTE_1_BIT));
drm_dbg_dp(audio->drm_dev,
@@ -147,7 +75,7 @@ static void dp_audio_stream_sdp(struct dp_audio_private 
*audio)
value = dp_audio_get_header(catalog,
DP_AUDIO_SDP_STREAM, DP_AUDIO_SDP_HEADER_2);
new_value = value;
-   parity_byte = dp_audio_calculate_parity(new_value);
+   parity_byte = dp_catalog_calculate_parity(new_value);
value |= ((new_value << HEADER_BYTE_2_BIT)
| (parity_byte << PARITY_BYTE_2_BIT));
drm_dbg_dp(audio->drm_dev,
@@ -162,7 +90,7 @@ static void dp_audio_stream_sdp(struct dp_audio_private 
*audio)
DP_AUDIO_SDP_STREAM, DP_AUDIO_SDP_HEADER_3);
  
  	new_value = audio->channels - 1;

-   parity_byte = dp_audio_calculate_parity(new_value);
+   parity_byte = dp_catalog_calculate_parity(new_value);
value |= ((new_value << HEADER_BYTE_3_BIT)
| (parity_byte << PARITY_BYTE_3_BIT));
drm_dbg_dp(audio->drm_dev,
@@ -184,7 +112,7 @@ static void dp_audio_timestamp_sdp(struct dp_audio_private 
*audio)
DP_AUDIO_SDP_TIMESTAMP, DP_AUDIO_SDP_HEADER_1);
  
  	new_value = 0x1;

-   parity_byte = dp_audio_calculate_parity(new_value);
+   parity_byte = dp_catalog_calculate_parity(new_value);
value |= ((new_value << HEADER_BYTE_1_BIT)
| (parity_byte << PARITY_BYTE_1_BIT));
drm_dbg_dp(audio->drm_dev,
@@ -198,7 +126,7 @@ static void dp_audio_timestamp_sdp(struct dp_audio_private 
*audio)
DP_AUDIO_SDP_TIMESTAMP, DP_AUDIO_SDP_HEADER_2);
  
  	new_value = 0x17;

-   parity_byte = dp_audio_calculate_parity(new_value);
+   parity_byte = dp_catalog_calculate_parity(new_value);
value

[PATCH 09/17] drm/msm/dp: move parity calculation to dp_catalog

2024-01-25 Thread Paloma Arellano 
Parity calculation is necessary for VSC SDP implementation, therefore
move it to dp_catalog so it usable by both SDP programming and
dp_audio.c

Signed-off-by: Paloma Arellano 
---
 drivers/gpu/drm/msm/dp/dp_audio.c   | 100 
 drivers/gpu/drm/msm/dp/dp_catalog.h |  72 
 2 files changed, 86 insertions(+), 86 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_audio.c 
b/drivers/gpu/drm/msm/dp/dp_audio.c
index 4a2e479723a85..7aa785018155a 100644
--- a/drivers/gpu/drm/msm/dp/dp_audio.c
+++ b/drivers/gpu/drm/msm/dp/dp_audio.c
@@ -16,13 +16,6 @@
 #include "dp_panel.h"
 #include "dp_display.h"
 
-#define HEADER_BYTE_2_BIT   0
-#define PARITY_BYTE_2_BIT   8
-#define HEADER_BYTE_1_BIT  16
-#define PARITY_BYTE_1_BIT  24
-#define HEADER_BYTE_3_BIT  16
-#define PARITY_BYTE_3_BIT  24
-
 struct dp_audio_private {
struct platform_device *audio_pdev;
struct platform_device *pdev;
@@ -36,71 +29,6 @@ struct dp_audio_private {
struct dp_audio dp_audio;
 };
 
-static u8 dp_audio_get_g0_value(u8 data)
-{
-   u8 c[4];
-   u8 g[4];
-   u8 ret_data = 0;
-   u8 i;
-
-   for (i = 0; i < 4; i++)
-   c[i] = (data >> i) & 0x01;
-
-   g[0] = c[3];
-   g[1] = c[0] ^ c[3];
-   g[2] = c[1];
-   g[3] = c[2];
-
-   for (i = 0; i < 4; i++)
-   ret_data = ((g[i] & 0x01) << i) | ret_data;
-
-   return ret_data;
-}
-
-static u8 dp_audio_get_g1_value(u8 data)
-{
-   u8 c[4];
-   u8 g[4];
-   u8 ret_data = 0;
-   u8 i;
-
-   for (i = 0; i < 4; i++)
-   c[i] = (data >> i) & 0x01;
-
-   g[0] = c[0] ^ c[3];
-   g[1] = c[0] ^ c[1] ^ c[3];
-   g[2] = c[1] ^ c[2];
-   g[3] = c[2] ^ c[3];
-
-   for (i = 0; i < 4; i++)
-   ret_data = ((g[i] & 0x01) << i) | ret_data;
-
-   return ret_data;
-}
-
-static u8 dp_audio_calculate_parity(u32 data)
-{
-   u8 x0 = 0;
-   u8 x1 = 0;
-   u8 ci = 0;
-   u8 iData = 0;
-   u8 i = 0;
-   u8 parity_byte;
-   u8 num_byte = (data & 0xFF00) > 0 ? 8 : 2;
-
-   for (i = 0; i < num_byte; i++) {
-   iData = (data >> i*4) & 0xF;
-
-   ci = iData ^ x1;
-   x1 = x0 ^ dp_audio_get_g1_value(ci);
-   x0 = dp_audio_get_g0_value(ci);
-   }
-
-   parity_byte = x1 | (x0 << 4);
-
-   return parity_byte;
-}
-
 static u32 dp_audio_get_header(struct dp_catalog *catalog,
enum dp_catalog_audio_sdp_type sdp,
enum dp_catalog_audio_header_type header)
@@ -134,7 +62,7 @@ static void dp_audio_stream_sdp(struct dp_audio_private 
*audio)
DP_AUDIO_SDP_STREAM, DP_AUDIO_SDP_HEADER_1);
 
new_value = 0x02;
-   parity_byte = dp_audio_calculate_parity(new_value);
+   parity_byte = dp_catalog_calculate_parity(new_value);
value |= ((new_value << HEADER_BYTE_1_BIT)
| (parity_byte << PARITY_BYTE_1_BIT));
drm_dbg_dp(audio->drm_dev,
@@ -147,7 +75,7 @@ static void dp_audio_stream_sdp(struct dp_audio_private 
*audio)
value = dp_audio_get_header(catalog,
DP_AUDIO_SDP_STREAM, DP_AUDIO_SDP_HEADER_2);
new_value = value;
-   parity_byte = dp_audio_calculate_parity(new_value);
+   parity_byte = dp_catalog_calculate_parity(new_value);
value |= ((new_value << HEADER_BYTE_2_BIT)
| (parity_byte << PARITY_BYTE_2_BIT));
drm_dbg_dp(audio->drm_dev,
@@ -162,7 +90,7 @@ static void dp_audio_stream_sdp(struct dp_audio_private 
*audio)
DP_AUDIO_SDP_STREAM, DP_AUDIO_SDP_HEADER_3);
 
new_value = audio->channels - 1;
-   parity_byte = dp_audio_calculate_parity(new_value);
+   parity_byte = dp_catalog_calculate_parity(new_value);
value |= ((new_value << HEADER_BYTE_3_BIT)
| (parity_byte << PARITY_BYTE_3_BIT));
drm_dbg_dp(audio->drm_dev,
@@ -184,7 +112,7 @@ static void dp_audio_timestamp_sdp(struct dp_audio_private 
*audio)
DP_AUDIO_SDP_TIMESTAMP, DP_AUDIO_SDP_HEADER_1);
 
new_value = 0x1;
-   parity_byte = dp_audio_calculate_parity(new_value);
+   parity_byte = dp_catalog_calculate_parity(new_value);
value |= ((new_value << HEADER_BYTE_1_BIT)
| (parity_byte << PARITY_BYTE_1_BIT));
drm_dbg_dp(audio->drm_dev,
@@ -198,7 +126,7 @@ static void dp_audio_timestamp_sdp(struct dp_audio_private 
*audio)
DP_AUDIO_SDP_TIMESTAMP, DP_AUDIO_SDP_HEADER_2);
 
new_value = 0x17;
-   parity_byte = dp_audio_calculate_parity(new_value);
+   parity_byte = dp_catalog_calculate_parity(new_value);
value |= ((new_value << HEADER_BYTE_2_BIT)
| (parity_byte << PARITY_BYTE_2_BIT));
drm_dbg_dp(audio->drm_dev,
@@ -212,7 +140,7 @@ static void