Re: [PATCH v2 6/6] drm/msm/dp: Use function arguments for audio operations

2024-04-09 Thread Bjorn Andersson 
On Mon, Apr 08, 2024 at 02:31:45PM -0700, Abhinav Kumar wrote:
> On 3/28/2024 7:40 AM, Bjorn Andersson wrote:
> > From: Bjorn Andersson 
> > 
> > The dp_audio read and write operations uses members in struct dp_catalog
> > for passing arguments and return values. This adds unnecessary
> > complexity to the implementation, as it turns out after detangling the
> > logic that no state is actually held in these variables.
> > 
> > Clean this up by using function arguments and return values for passing
> > the data.
> > 
> > Reviewed-by: Dmitry Baryshkov 
> > Signed-off-by: Bjorn Andersson 
> > ---
> >   drivers/gpu/drm/msm/dp/dp_audio.c   | 20 +--
> >   drivers/gpu/drm/msm/dp/dp_catalog.c | 39 
> > +
> >   drivers/gpu/drm/msm/dp/dp_catalog.h | 18 +
> >   3 files changed, 28 insertions(+), 49 deletions(-)
> > 
> 
> One quick question, was DP audio re-tested after this patch?

No, sorry for not being explicit about that. I don't have any target
with working DP audio...

Regards,
Bjorn

Re: [PATCH v2 6/6] drm/msm/dp: Use function arguments for audio operations

2024-04-08 Thread Abhinav Kumar 




On 3/28/2024 7:40 AM, Bjorn Andersson wrote:

From: Bjorn Andersson 

The dp_audio read and write operations uses members in struct dp_catalog
for passing arguments and return values. This adds unnecessary
complexity to the implementation, as it turns out after detangling the
logic that no state is actually held in these variables.

Clean this up by using function arguments and return values for passing
the data.

Reviewed-by: Dmitry Baryshkov 
Signed-off-by: Bjorn Andersson 
---
  drivers/gpu/drm/msm/dp/dp_audio.c   | 20 +--
  drivers/gpu/drm/msm/dp/dp_catalog.c | 39 +
  drivers/gpu/drm/msm/dp/dp_catalog.h | 18 +
  3 files changed, 28 insertions(+), 49 deletions(-)



One quick question, was DP audio re-tested after this patch?

[PATCH v2 6/6] drm/msm/dp: Use function arguments for audio operations

2024-03-28 Thread Bjorn Andersson 
From: Bjorn Andersson 

The dp_audio read and write operations uses members in struct dp_catalog
for passing arguments and return values. This adds unnecessary
complexity to the implementation, as it turns out after detangling the
logic that no state is actually held in these variables.

Clean this up by using function arguments and return values for passing
the data.

Reviewed-by: Dmitry Baryshkov 
Signed-off-by: Bjorn Andersson 
---
 drivers/gpu/drm/msm/dp/dp_audio.c   | 20 +--
 drivers/gpu/drm/msm/dp/dp_catalog.c | 39 +
 drivers/gpu/drm/msm/dp/dp_catalog.h | 18 +
 3 files changed, 28 insertions(+), 49 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_audio.c 
b/drivers/gpu/drm/msm/dp/dp_audio.c
index 7fd0c1793ba3..a599fc5d63c5 100644
--- a/drivers/gpu/drm/msm/dp/dp_audio.c
+++ b/drivers/gpu/drm/msm/dp/dp_audio.c
@@ -32,11 +32,7 @@ static u32 dp_audio_get_header(struct dp_catalog *catalog,
enum dp_catalog_audio_sdp_type sdp,
enum dp_catalog_audio_header_type header)
 {
-   catalog->sdp_type = sdp;
-   catalog->sdp_header = header;
-   dp_catalog_audio_get_header(catalog);
-
-   return catalog->audio_data;
+   return dp_catalog_audio_get_header(catalog, sdp, header);
 }
 
 static void dp_audio_set_header(struct dp_catalog *catalog,
@@ -44,10 +40,7 @@ static void dp_audio_set_header(struct dp_catalog *catalog,
enum dp_catalog_audio_sdp_type sdp,
enum dp_catalog_audio_header_type header)
 {
-   catalog->sdp_type = sdp;
-   catalog->sdp_header = header;
-   catalog->audio_data = data;
-   dp_catalog_audio_set_header(catalog);
+   dp_catalog_audio_set_header(catalog, sdp, header, data);
 }
 
 static void dp_audio_stream_sdp(struct dp_audio_private *audio)
@@ -317,8 +310,7 @@ static void dp_audio_setup_acr(struct dp_audio_private 
*audio)
break;
}
 
-   catalog->audio_data = select;
-   dp_catalog_audio_config_acr(catalog);
+   dp_catalog_audio_config_acr(catalog, select);
 }
 
 static void dp_audio_safe_to_exit_level(struct dp_audio_private *audio)
@@ -344,16 +336,14 @@ static void dp_audio_safe_to_exit_level(struct 
dp_audio_private *audio)
break;
}
 
-   catalog->audio_data = safe_to_exit_level;
-   dp_catalog_audio_sfe_level(catalog);
+   dp_catalog_audio_sfe_level(catalog, safe_to_exit_level);
 }
 
 static void dp_audio_enable(struct dp_audio_private *audio, bool enable)
 {
struct dp_catalog *catalog = audio->catalog;
 
-   catalog->audio_data = enable;
-   dp_catalog_audio_enable(catalog);
+   dp_catalog_audio_enable(catalog, enable);
 }
 
 static struct dp_audio_private *dp_audio_get_data(struct platform_device *pdev)
diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c 
b/drivers/gpu/drm/msm/dp/dp_catalog.c
index 00ad3ebaa5a1..970d62e1610c 100644
--- a/drivers/gpu/drm/msm/dp/dp_catalog.c
+++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
@@ -1159,34 +1159,28 @@ struct dp_catalog *dp_catalog_get(struct device *dev)
return >dp_catalog;
 }
 
-void dp_catalog_audio_get_header(struct dp_catalog *dp_catalog)
+u32 dp_catalog_audio_get_header(struct dp_catalog *dp_catalog,
+   enum dp_catalog_audio_sdp_type sdp,
+   enum dp_catalog_audio_header_type header)
 {
struct dp_catalog_private *catalog;
u32 (*sdp_map)[DP_AUDIO_SDP_HEADER_MAX];
-   enum dp_catalog_audio_sdp_type sdp;
-   enum dp_catalog_audio_header_type header;
-
-   if (!dp_catalog)
-   return;
 
catalog = container_of(dp_catalog,
struct dp_catalog_private, dp_catalog);
 
sdp_map = catalog->audio_map;
-   sdp = dp_catalog->sdp_type;
-   header  = dp_catalog->sdp_header;
 
-   dp_catalog->audio_data = dp_read_link(catalog,
-   sdp_map[sdp][header]);
+   return dp_read_link(catalog, sdp_map[sdp][header]);
 }
 
-void dp_catalog_audio_set_header(struct dp_catalog *dp_catalog)
+void dp_catalog_audio_set_header(struct dp_catalog *dp_catalog,
+enum dp_catalog_audio_sdp_type sdp,
+enum dp_catalog_audio_header_type header,
+u32 data)
 {
struct dp_catalog_private *catalog;
u32 (*sdp_map)[DP_AUDIO_SDP_HEADER_MAX];
-   enum dp_catalog_audio_sdp_type sdp;
-   enum dp_catalog_audio_header_type header;
-   u32 data;
 
if (!dp_catalog)
return;
@@ -1195,17 +1189,14 @@ void dp_catalog_audio_set_header(struct dp_catalog 
*dp_catalog)
struct dp_catalog_private, dp_catalog);
 
sdp_map = catalog->audio_map;
-   sdp = dp_catalog->sdp_type;
-   header  = dp_catalog->sdp_header;
-   data= dp_catalog->audio_data;
 
dp_write_link(catalog,