2024-03-26 15:12:56

by Bjorn Andersson

[permalink] [raw]
Subject: [PATCH 0/6] drm/msm/dp: Spring cleaning

Spring is in the air, clean out some dust that has accumulated in the
Qualcomm DP driver.

Signed-off-by: Bjorn Andersson <[email protected]>
---
Bjorn Andersson (6):
drm/msm/dp: Drop unused dp_debug struct
drm/msm/dp: Removed fixed nvid "support"
drm/msm/dp: Remove unused defines and members
drm/msm/dp: Use function arguments for aux writes
drm/msm/dp: Use function arguments for timing configuration
drm/msm/dp: Use function arguments for audio operations

drivers/gpu/drm/msm/dp/dp_audio.c | 25 +++------------
drivers/gpu/drm/msm/dp/dp_aux.c | 9 ++----
drivers/gpu/drm/msm/dp/dp_catalog.c | 64 ++++++++++++++-----------------------
drivers/gpu/drm/msm/dp/dp_catalog.h | 49 +++++++++-------------------
drivers/gpu/drm/msm/dp/dp_ctrl.c | 17 +---------
drivers/gpu/drm/msm/dp/dp_ctrl.h | 1 -
drivers/gpu/drm/msm/dp/dp_debug.c | 38 ++++++----------------
drivers/gpu/drm/msm/dp/dp_debug.h | 38 +++++++---------------
drivers/gpu/drm/msm/dp/dp_display.c | 15 ++-------
drivers/gpu/drm/msm/dp/dp_display.h | 3 --
drivers/gpu/drm/msm/dp/dp_drm.c | 2 --
drivers/gpu/drm/msm/dp/dp_link.c | 4 ---
drivers/gpu/drm/msm/dp/dp_link.h | 1 -
drivers/gpu/drm/msm/dp/dp_panel.c | 14 +++++---
drivers/gpu/drm/msm/dp/dp_panel.h | 3 --
15 files changed, 80 insertions(+), 203 deletions(-)
---
base-commit: 084c8e315db34b59d38d06e684b1a0dd07d30287
change-id: 20240323-msm-dp-cleanup-f8ba32f62fb9

Best regards,
--
Bjorn Andersson <[email protected]>



2024-03-26 15:14:17

by Bjorn Andersson

[permalink] [raw]
Subject: [PATCH 4/6] drm/msm/dp: Use function arguments for aux writes

From: Bjorn Andersson <[email protected]>

The dp_aux write operations takes the data to be operated on through a
member of struct dp_catalog, rather than as an argument to the function.

No state is maintained other than across the calling of the functions,
so replace this member with a function argument.

Signed-off-by: Bjorn Andersson <[email protected]>
---
drivers/gpu/drm/msm/dp/dp_aux.c | 9 +++------
drivers/gpu/drm/msm/dp/dp_catalog.c | 8 ++++----
drivers/gpu/drm/msm/dp/dp_catalog.h | 5 ++---
3 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_aux.c b/drivers/gpu/drm/msm/dp/dp_aux.c
index adbd5a367395..2c8bcc60692a 100644
--- a/drivers/gpu/drm/msm/dp/dp_aux.c
+++ b/drivers/gpu/drm/msm/dp/dp_aux.c
@@ -87,8 +87,7 @@ static ssize_t dp_aux_write(struct dp_aux_private *aux,
/* index = 0, write */
if (i == 0)
reg |= DP_AUX_DATA_INDEX_WRITE;
- aux->catalog->aux_data = reg;
- dp_catalog_aux_write_data(aux->catalog);
+ dp_catalog_aux_write_data(aux->catalog, reg);
}

dp_catalog_aux_clear_trans(aux->catalog, false);
@@ -106,8 +105,7 @@ static ssize_t dp_aux_write(struct dp_aux_private *aux,
}

reg |= DP_AUX_TRANS_CTRL_GO;
- aux->catalog->aux_data = reg;
- dp_catalog_aux_write_trans(aux->catalog);
+ dp_catalog_aux_write_trans(aux->catalog, reg);

return len;
}
@@ -145,8 +143,7 @@ static ssize_t dp_aux_cmd_fifo_rx(struct dp_aux_private *aux,
data = DP_AUX_DATA_INDEX_WRITE; /* INDEX_WRITE */
data |= DP_AUX_DATA_READ; /* read */

- aux->catalog->aux_data = data;
- dp_catalog_aux_write_data(aux->catalog);
+ dp_catalog_aux_write_data(aux->catalog, data);

dp = msg->buffer;

diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c b/drivers/gpu/drm/msm/dp/dp_catalog.c
index 55114a6aba7e..295bd4cb72cc 100644
--- a/drivers/gpu/drm/msm/dp/dp_catalog.c
+++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
@@ -169,21 +169,21 @@ u32 dp_catalog_aux_read_data(struct dp_catalog *dp_catalog)
return dp_read_aux(catalog, REG_DP_AUX_DATA);
}

-int dp_catalog_aux_write_data(struct dp_catalog *dp_catalog)
+int dp_catalog_aux_write_data(struct dp_catalog *dp_catalog, u32 data)
{
struct dp_catalog_private *catalog = container_of(dp_catalog,
struct dp_catalog_private, dp_catalog);

- dp_write_aux(catalog, REG_DP_AUX_DATA, dp_catalog->aux_data);
+ dp_write_aux(catalog, REG_DP_AUX_DATA, data);
return 0;
}

-int dp_catalog_aux_write_trans(struct dp_catalog *dp_catalog)
+int dp_catalog_aux_write_trans(struct dp_catalog *dp_catalog, u32 data)
{
struct dp_catalog_private *catalog = container_of(dp_catalog,
struct dp_catalog_private, dp_catalog);

- dp_write_aux(catalog, REG_DP_AUX_TRANS_CTRL, dp_catalog->aux_data);
+ dp_write_aux(catalog, REG_DP_AUX_TRANS_CTRL, data);
return 0;
}

diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.h b/drivers/gpu/drm/msm/dp/dp_catalog.h
index 2c2dbeee7634..290ef8180c12 100644
--- a/drivers/gpu/drm/msm/dp/dp_catalog.h
+++ b/drivers/gpu/drm/msm/dp/dp_catalog.h
@@ -48,7 +48,6 @@ enum dp_catalog_audio_header_type {
};

struct dp_catalog {
- u32 aux_data;
u32 total;
u32 sync_start;
u32 width_blanking;
@@ -64,8 +63,8 @@ void dp_catalog_snapshot(struct dp_catalog *dp_catalog, struct msm_disp_state *d

/* AUX APIs */
u32 dp_catalog_aux_read_data(struct dp_catalog *dp_catalog);
-int dp_catalog_aux_write_data(struct dp_catalog *dp_catalog);
-int dp_catalog_aux_write_trans(struct dp_catalog *dp_catalog);
+int dp_catalog_aux_write_data(struct dp_catalog *dp_catalog, u32 data);
+int dp_catalog_aux_write_trans(struct dp_catalog *dp_catalog, u32 data);
int dp_catalog_aux_clear_trans(struct dp_catalog *dp_catalog, bool read);
int dp_catalog_aux_clear_hw_interrupts(struct dp_catalog *dp_catalog);
void dp_catalog_aux_reset(struct dp_catalog *dp_catalog);

--
2.43.0


2024-03-26 15:15:09

by Bjorn Andersson

[permalink] [raw]
Subject: [PATCH 6/6] drm/msm/dp: Use function arguments for audio operations

From: Bjorn Andersson <[email protected]>

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.

Signed-off-by: Bjorn Andersson <[email protected]>
---
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 &catalog->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, sdp_map[sdp][header], data);
}

-void dp_catalog_audio_config_acr(struct dp_catalog *dp_catalog)
+void dp_catalog_audio_config_acr(struct dp_catalog *dp_catalog, u32 select)
{
struct dp_catalog_private *catalog;
- u32 acr_ctrl, select;
+ u32 acr_ctrl;

if (!dp_catalog)
return;
@@ -1213,7 +1204,6 @@ void dp_catalog_audio_config_acr(struct dp_catalog *dp_catalog)
catalog = container_of(dp_catalog,
struct dp_catalog_private, dp_catalog);

- select = dp_catalog->audio_data;
acr_ctrl = select << 4 | BIT(31) | BIT(8) | BIT(14);

drm_dbg_dp(catalog->drm_dev, "select: %#x, acr_ctrl: %#x\n",
@@ -1222,10 +1212,9 @@ void dp_catalog_audio_config_acr(struct dp_catalog *dp_catalog)
dp_write_link(catalog, MMSS_DP_AUDIO_ACR_CTRL, acr_ctrl);
}

-void dp_catalog_audio_enable(struct dp_catalog *dp_catalog)
+void dp_catalog_audio_enable(struct dp_catalog *dp_catalog, bool enable)
{
struct dp_catalog_private *catalog;
- bool enable;
u32 audio_ctrl;

if (!dp_catalog)
@@ -1234,7 +1223,6 @@ void dp_catalog_audio_enable(struct dp_catalog *dp_catalog)
catalog = container_of(dp_catalog,
struct dp_catalog_private, dp_catalog);

- enable = !!dp_catalog->audio_data;
audio_ctrl = dp_read_link(catalog, MMSS_DP_AUDIO_CFG);

if (enable)
@@ -1329,10 +1317,10 @@ void dp_catalog_audio_init(struct dp_catalog *dp_catalog)
catalog->audio_map = sdp_map;
}

-void dp_catalog_audio_sfe_level(struct dp_catalog *dp_catalog)
+void dp_catalog_audio_sfe_level(struct dp_catalog *dp_catalog, u32 safe_to_exit_level)
{
struct dp_catalog_private *catalog;
- u32 mainlink_levels, safe_to_exit_level;
+ u32 mainlink_levels;

if (!dp_catalog)
return;
@@ -1340,7 +1328,6 @@ void dp_catalog_audio_sfe_level(struct dp_catalog *dp_catalog)
catalog = container_of(dp_catalog,
struct dp_catalog_private, dp_catalog);

- safe_to_exit_level = dp_catalog->audio_data;
mainlink_levels = dp_read_link(catalog, REG_DP_MAINLINK_LEVELS);
mainlink_levels &= 0xFE0;
mainlink_levels |= safe_to_exit_level;
diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.h b/drivers/gpu/drm/msm/dp/dp_catalog.h
index a82ab4856b50..cd1ad046a953 100644
--- a/drivers/gpu/drm/msm/dp/dp_catalog.h
+++ b/drivers/gpu/drm/msm/dp/dp_catalog.h
@@ -48,9 +48,6 @@ enum dp_catalog_audio_header_type {
};

struct dp_catalog {
- enum dp_catalog_audio_sdp_type sdp_type;
- enum dp_catalog_audio_header_type sdp_header;
- u32 audio_data;
bool wide_bus_en;
};

@@ -114,12 +111,17 @@ void dp_catalog_panel_tpg_disable(struct dp_catalog *dp_catalog);
struct dp_catalog *dp_catalog_get(struct device *dev);

/* DP Audio APIs */
-void dp_catalog_audio_get_header(struct dp_catalog *catalog);
-void dp_catalog_audio_set_header(struct dp_catalog *catalog);
-void dp_catalog_audio_config_acr(struct dp_catalog *catalog);
-void dp_catalog_audio_enable(struct dp_catalog *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);
+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);
+void dp_catalog_audio_config_acr(struct dp_catalog *catalog, u32 select);
+void dp_catalog_audio_enable(struct dp_catalog *catalog, bool enable);
void dp_catalog_audio_config_sdp(struct dp_catalog *catalog);
void dp_catalog_audio_init(struct dp_catalog *catalog);
-void dp_catalog_audio_sfe_level(struct dp_catalog *catalog);
+void dp_catalog_audio_sfe_level(struct dp_catalog *catalog, u32 safe_to_exit_level);

#endif /* _DP_CATALOG_H_ */

--
2.43.0


2024-03-26 17:31:52

by Dmitry Baryshkov

[permalink] [raw]
Subject: Re: [PATCH 4/6] drm/msm/dp: Use function arguments for aux writes

On Tue, 26 Mar 2024 at 17:06, Bjorn Andersson <[email protected]> wrote:
>
> From: Bjorn Andersson <[email protected]>
>
> The dp_aux write operations takes the data to be operated on through a
> member of struct dp_catalog, rather than as an argument to the function.
>
> No state is maintained other than across the calling of the functions,
> so replace this member with a function argument.

Definitely yes, thank you!

Reviewed-by: Dmitry Baryshkov <[email protected]>

>
> Signed-off-by: Bjorn Andersson <[email protected]>
> ---
> drivers/gpu/drm/msm/dp/dp_aux.c | 9 +++------
> drivers/gpu/drm/msm/dp/dp_catalog.c | 8 ++++----
> drivers/gpu/drm/msm/dp/dp_catalog.h | 5 ++---
> 3 files changed, 9 insertions(+), 13 deletions(-)

--
With best wishes
Dmitry

2024-03-26 18:14:08

by Dmitry Baryshkov

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

On Tue, 26 Mar 2024 at 17:06, Bjorn Andersson <[email protected]> wrote:
>
> From: Bjorn Andersson <[email protected]>
>
> 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.
>
> Signed-off-by: Bjorn Andersson <[email protected]>
> ---
> 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(-)

Reviewed-by: Dmitry Baryshkov <[email protected]>

Thanks a lot for the cleanup!

--
With best wishes
Dmitry