Re: [PATCH 1/2] drm/mediatek: Switch the hdmi bridge ops to the atomic versions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On 25.03.21 11:43, Enric Balletbo i Serra wrote:
Hi Dafna,

Thank you for your patch. It'd be nice if you can cc the linux-mediatek ML for
next version, so Mediatek people is more aware of this change. IMHO cc'ing the
lkml is also a good practice.

ok, I added all the mails from get_maintainer.pl. linux-mediatek was not there,
I can add it.


On 24/3/21 20:12, Dafna Hirschfeld wrote:
The bridge operation '.enable' and the audio cb '.get_eld'
access hdmi->conn. In the future we will want to support
the flag DRM_BRIDGE_ATTACH_NO_CONNECTOR and then we will
not have direct access to the connector.
The atomic version '.atomic_enable' allows accessing the
current connector from the state.
This patch switches the bridge to the atomic version and
saves the current connector in a new field 'curr_conn'.

Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@xxxxxxxxxxxxx>
---
  drivers/gpu/drm/mediatek/mtk_hdmi.c | 41 ++++++++++++++++++++---------
  1 file changed, 29 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c b/drivers/gpu/drm/mediatek/mtk_hdmi.c
index 8ee55f9e2954..9f415c508b33 100644
--- a/drivers/gpu/drm/mediatek/mtk_hdmi.c
+++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c
@@ -154,6 +154,7 @@ struct mtk_hdmi {
  	struct drm_bridge bridge;
  	struct drm_bridge *next_bridge;
  	struct drm_connector conn;
+	struct drm_connector *curr_conn;

Adding a new drm_connector (curr_conn) is something that surprised me at the
beginning, then I saw that in the second patch you remove the first
drm_connector (conn). I didn't test it yet, but did you test this patch alone?
without the second one, maybe I'm missing something but looks to me that this
patch alone breaks more functionalities of the driver? (and I know that the
driver is broken right now)

Hi, I didn't test it alone, but indeed this patch was wirrten with the intention
that 'conn' will be removed next patch. But I don't think that patch should break
functionality.


Is really needed to create a new drm_connector to switch to the atomic versions?

No, indeed this is the wrong patch to add curr_conn, I should add curr_conn to the
second patch or maybe to a seperate last patch so not to overload the patch.

Thanks,
Dafna



  	struct device *dev;
  	const struct mtk_hdmi_conf *conf;
  	struct phy *phy;
@@ -974,7 +975,7 @@ static int mtk_hdmi_setup_avi_infoframe(struct mtk_hdmi *hdmi,
  	ssize_t err;
err = drm_hdmi_avi_infoframe_from_display_mode(&frame,
-						       &hdmi->conn, mode);
+						       hdmi->curr_conn, mode);
  	if (err < 0) {
  		dev_err(hdmi->dev,
  			"Failed to get AVI infoframe from mode: %zd\n", err);
@@ -1054,7 +1055,7 @@ static int mtk_hdmi_setup_vendor_specific_infoframe(struct mtk_hdmi *hdmi,
  	ssize_t err;
err = drm_hdmi_vendor_infoframe_from_display_mode(&frame,
-							  &hdmi->conn, mode);
+							  hdmi->curr_conn, mode);
  	if (err) {
  		dev_err(hdmi->dev,
  			"Failed to get vendor infoframe from mode: %zd\n", err);
@@ -1357,7 +1358,8 @@ static bool mtk_hdmi_bridge_mode_fixup(struct drm_bridge *bridge,
  	return true;
  }
-static void mtk_hdmi_bridge_disable(struct drm_bridge *bridge)
+static void mtk_hdmi_bridge_atomic_disable(struct drm_bridge *bridge,
+					   struct drm_bridge_state *old_bridge_state)
  {
  	struct mtk_hdmi *hdmi = hdmi_ctx_from_bridge(bridge);
@@ -1368,10 +1370,13 @@ static void mtk_hdmi_bridge_disable(struct drm_bridge *bridge)
  	clk_disable_unprepare(hdmi->clk[MTK_HDMI_CLK_HDMI_PIXEL]);
  	clk_disable_unprepare(hdmi->clk[MTK_HDMI_CLK_HDMI_PLL]);
+ hdmi->curr_conn = NULL;
+
  	hdmi->enabled = false;
  }
-static void mtk_hdmi_bridge_post_disable(struct drm_bridge *bridge)
+static void mtk_hdmi_bridge_atomic_post_disable(struct drm_bridge *bridge,
+						struct drm_bridge_state *old_state)
  {
  	struct mtk_hdmi *hdmi = hdmi_ctx_from_bridge(bridge);
@@ -1406,7 +1411,8 @@ static void mtk_hdmi_bridge_mode_set(struct drm_bridge *bridge,
  	drm_mode_copy(&hdmi->mode, adjusted_mode);
  }
-static void mtk_hdmi_bridge_pre_enable(struct drm_bridge *bridge)
+static void mtk_hdmi_bridge_atomic_pre_enable(struct drm_bridge *bridge,
+					      struct drm_bridge_state *old_state)
  {
  	struct mtk_hdmi *hdmi = hdmi_ctx_from_bridge(bridge);
@@ -1426,10 +1432,16 @@ static void mtk_hdmi_send_infoframe(struct mtk_hdmi *hdmi,
  		mtk_hdmi_setup_vendor_specific_infoframe(hdmi, mode);
  }
-static void mtk_hdmi_bridge_enable(struct drm_bridge *bridge)
+static void mtk_hdmi_bridge_atomic_enable(struct drm_bridge *bridge,
+					  struct drm_bridge_state *old_state)
  {
+	struct drm_atomic_state *state = old_state->base.state;
  	struct mtk_hdmi *hdmi = hdmi_ctx_from_bridge(bridge);
+ /* Retrieve the connector through the atomic state. */
+	hdmi->curr_conn = drm_atomic_get_new_connector_for_encoder(state,
+								   bridge->encoder);
+
  	mtk_hdmi_output_set_display_mode(hdmi, &hdmi->mode);
  	clk_prepare_enable(hdmi->clk[MTK_HDMI_CLK_HDMI_PLL]);
  	clk_prepare_enable(hdmi->clk[MTK_HDMI_CLK_HDMI_PIXEL]);
@@ -1440,13 +1452,16 @@ static void mtk_hdmi_bridge_enable(struct drm_bridge *bridge)
  }
static const struct drm_bridge_funcs mtk_hdmi_bridge_funcs = {
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_reset = drm_atomic_helper_bridge_reset,
  	.attach = mtk_hdmi_bridge_attach,
  	.mode_fixup = mtk_hdmi_bridge_mode_fixup,
-	.disable = mtk_hdmi_bridge_disable,
-	.post_disable = mtk_hdmi_bridge_post_disable,
+	.atomic_disable = mtk_hdmi_bridge_atomic_disable,
+	.atomic_post_disable = mtk_hdmi_bridge_atomic_post_disable,
  	.mode_set = mtk_hdmi_bridge_mode_set,
-	.pre_enable = mtk_hdmi_bridge_pre_enable,
-	.enable = mtk_hdmi_bridge_enable,
+	.atomic_pre_enable = mtk_hdmi_bridge_atomic_pre_enable,
+	.atomic_enable = mtk_hdmi_bridge_atomic_enable,
  };
static int mtk_hdmi_dt_parse_pdata(struct mtk_hdmi *hdmi,
@@ -1662,8 +1677,10 @@ static int mtk_hdmi_audio_get_eld(struct device *dev, void *data, uint8_t *buf,
  {
  	struct mtk_hdmi *hdmi = dev_get_drvdata(dev);
- memcpy(buf, hdmi->conn.eld, min(sizeof(hdmi->conn.eld), len));
-
+	if (hdmi->curr_conn)
+		memcpy(buf, hdmi->curr_conn->eld, min(sizeof(hdmi->curr_conn->eld), len));
+	else
+		memset(buf, 0, len);
  	return 0;
  }
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux