All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/3] ast: Fix I2C corner cases wrt init/cleanup
@ 2021-12-06  9:11 Thomas Zimmermann
  2021-12-06  9:11 ` [PATCH v2 1/3] drm/ast: Handle failed I2C initialization gracefully Thomas Zimmermann
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Thomas Zimmermann @ 2021-12-06  9:11 UTC (permalink / raw)
  To: airlied, daniel, sam, kuohsiang_chou; +Cc: Thomas Zimmermann, dri-devel

The VGA connector in ast is supposed to work without I2C. Currently,
this isn't implemented correctly in several places. Fix this. Also
add managed cleanup of the I2C code and move it into separate source
file.

Tested on AST2100 hardware.

v2:
	* init edid to NULL to avoid uninitialized read
	* drop test for drm_connector_init() for now
	* move I2C code into separate source file

Thomas Zimmermann (3):
  drm/ast: Handle failed I2C initialization gracefully
  drm/ast: Convert I2C code to managed cleanup
  drm/ast: Move I2C code into separate source file

 drivers/gpu/drm/ast/Makefile   |   2 +-
 drivers/gpu/drm/ast/ast_drv.h  |   3 +
 drivers/gpu/drm/ast/ast_i2c.c  | 152 +++++++++++++++++++++++++++++++++
 drivers/gpu/drm/ast/ast_mode.c | 151 +++-----------------------------
 4 files changed, 167 insertions(+), 141 deletions(-)
 create mode 100644 drivers/gpu/drm/ast/ast_i2c.c


base-commit: 909bf926eaf382123d9b215871143d9e3cf44aa3
--
2.34.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2 1/3] drm/ast: Handle failed I2C initialization gracefully
  2021-12-06  9:11 [PATCH v2 0/3] ast: Fix I2C corner cases wrt init/cleanup Thomas Zimmermann
@ 2021-12-06  9:11 ` Thomas Zimmermann
  2021-12-06  9:11 ` [PATCH v2 2/3] drm/ast: Convert I2C code to managed cleanup Thomas Zimmermann
  2021-12-06  9:11 ` [PATCH v2 3/3] drm/ast: Move I2C code into separate source file Thomas Zimmermann
  2 siblings, 0 replies; 5+ messages in thread
From: Thomas Zimmermann @ 2021-12-06  9:11 UTC (permalink / raw)
  To: airlied, daniel, sam, kuohsiang_chou; +Cc: Thomas Zimmermann, dri-devel

I2C initialization is allowed to fail. In this case, create a connector
without DDC adapter. The current code would dereference a NULL pointer.

Reading the modes from the connector is supposed to work without I2C
adapter. Add the respective test.

v2:
	* init edid to NULL to avoid uninitialized read

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/ast/ast_mode.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index 1e30eaeb0e1b..692e7a3b3555 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -1210,9 +1210,9 @@ static int ast_get_modes(struct drm_connector *connector)
 {
 	struct ast_connector *ast_connector = to_ast_connector(connector);
 	struct ast_private *ast = to_ast_private(connector->dev);
-	struct edid *edid;
-	int ret;
+	struct edid *edid = NULL;
 	bool flags = false;
+	int ret;
 
 	if (ast->tx_chip_type == AST_TX_DP501) {
 		ast->dp501_maxclk = 0xff;
@@ -1226,7 +1226,7 @@ static int ast_get_modes(struct drm_connector *connector)
 		else
 			kfree(edid);
 	}
-	if (!flags)
+	if (!flags && ast_connector->i2c)
 		edid = drm_get_edid(connector, &ast_connector->i2c->adapter);
 	if (edid) {
 		drm_connector_update_edid_property(&ast_connector->base, edid);
@@ -1332,10 +1332,13 @@ static int ast_connector_init(struct drm_device *dev)
 	if (!ast_connector->i2c)
 		drm_err(dev, "failed to add ddc bus for connector\n");
 
-	drm_connector_init_with_ddc(dev, connector,
-				    &ast_connector_funcs,
-				    DRM_MODE_CONNECTOR_VGA,
-				    &ast_connector->i2c->adapter);
+	if (ast_connector->i2c)
+		drm_connector_init_with_ddc(dev, connector, &ast_connector_funcs,
+					    DRM_MODE_CONNECTOR_VGA,
+					    &ast_connector->i2c->adapter);
+	else
+		drm_connector_init(dev, connector, &ast_connector_funcs,
+				   DRM_MODE_CONNECTOR_VGA);
 
 	drm_connector_helper_add(connector, &ast_connector_helper_funcs);
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v2 2/3] drm/ast: Convert I2C code to managed cleanup
  2021-12-06  9:11 [PATCH v2 0/3] ast: Fix I2C corner cases wrt init/cleanup Thomas Zimmermann
  2021-12-06  9:11 ` [PATCH v2 1/3] drm/ast: Handle failed I2C initialization gracefully Thomas Zimmermann
@ 2021-12-06  9:11 ` Thomas Zimmermann
  2021-12-06  9:11 ` [PATCH v2 3/3] drm/ast: Move I2C code into separate source file Thomas Zimmermann
  2 siblings, 0 replies; 5+ messages in thread
From: Thomas Zimmermann @ 2021-12-06  9:11 UTC (permalink / raw)
  To: airlied, daniel, sam, kuohsiang_chou; +Cc: Thomas Zimmermann, dri-devel

Release the I2C adapter as part of the DRM device cleanup. Remove
ast's dedicated helper for struct drm_connector_funcs.destroy.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/ast/ast_mode.c | 36 +++++++++++++++-------------------
 1 file changed, 16 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index 692e7a3b3555..abb8a3fdd812 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -40,6 +40,7 @@
 #include <drm/drm_gem_atomic_helper.h>
 #include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/drm_gem_vram_helper.h>
+#include <drm/drm_managed.h>
 #include <drm/drm_plane_helper.h>
 #include <drm/drm_probe_helper.h>
 #include <drm/drm_simple_kms_helper.h>
@@ -48,7 +49,6 @@
 #include "ast_tables.h"
 
 static struct ast_i2c_chan *ast_i2c_create(struct drm_device *dev);
-static void ast_i2c_destroy(struct ast_i2c_chan *i2c);
 
 static inline void ast_load_palette_index(struct ast_private *ast,
 				     u8 index, u8 red, u8 green,
@@ -1300,14 +1300,6 @@ static enum drm_mode_status ast_mode_valid(struct drm_connector *connector,
 	return flags;
 }
 
-static void ast_connector_destroy(struct drm_connector *connector)
-{
-	struct ast_connector *ast_connector = to_ast_connector(connector);
-
-	ast_i2c_destroy(ast_connector->i2c);
-	drm_connector_cleanup(connector);
-}
-
 static const struct drm_connector_helper_funcs ast_connector_helper_funcs = {
 	.get_modes = ast_get_modes,
 	.mode_valid = ast_mode_valid,
@@ -1316,7 +1308,7 @@ static const struct drm_connector_helper_funcs ast_connector_helper_funcs = {
 static const struct drm_connector_funcs ast_connector_funcs = {
 	.reset = drm_atomic_helper_connector_reset,
 	.fill_modes = drm_helper_probe_single_connector_modes,
-	.destroy = ast_connector_destroy,
+	.destroy = drm_connector_cleanup,
 	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
 };
@@ -1493,6 +1485,14 @@ static void set_data(void *i2c_priv, int data)
 	}
 }
 
+static void ast_i2c_release(struct drm_device *dev, void *res)
+{
+	struct ast_i2c_chan *i2c = res;
+
+	i2c_del_adapter(&i2c->adapter);
+	kfree(i2c);
+}
+
 static struct ast_i2c_chan *ast_i2c_create(struct drm_device *dev)
 {
 	struct ast_i2c_chan *i2c;
@@ -1521,19 +1521,15 @@ static struct ast_i2c_chan *ast_i2c_create(struct drm_device *dev)
 	ret = i2c_bit_add_bus(&i2c->adapter);
 	if (ret) {
 		drm_err(dev, "Failed to register bit i2c\n");
-		goto out_free;
+		goto out_kfree;
 	}
 
+	ret = drmm_add_action_or_reset(dev, ast_i2c_release, i2c);
+	if (ret)
+		return NULL;
 	return i2c;
-out_free:
-	kfree(i2c);
-	return NULL;
-}
 
-static void ast_i2c_destroy(struct ast_i2c_chan *i2c)
-{
-	if (!i2c)
-		return;
-	i2c_del_adapter(&i2c->adapter);
+out_kfree:
 	kfree(i2c);
+	return NULL;
 }
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v2 3/3] drm/ast: Move I2C code into separate source file
  2021-12-06  9:11 [PATCH v2 0/3] ast: Fix I2C corner cases wrt init/cleanup Thomas Zimmermann
  2021-12-06  9:11 ` [PATCH v2 1/3] drm/ast: Handle failed I2C initialization gracefully Thomas Zimmermann
  2021-12-06  9:11 ` [PATCH v2 2/3] drm/ast: Convert I2C code to managed cleanup Thomas Zimmermann
@ 2021-12-06  9:11 ` Thomas Zimmermann
  2021-12-16 10:25   ` Thomas Zimmermann
  2 siblings, 1 reply; 5+ messages in thread
From: Thomas Zimmermann @ 2021-12-06  9:11 UTC (permalink / raw)
  To: airlied, daniel, sam, kuohsiang_chou; +Cc: Thomas Zimmermann, dri-devel

Move I2C code into its own source file. Makes the mode-setting
code a little less convoluted.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/ast/Makefile   |   2 +-
 drivers/gpu/drm/ast/ast_drv.h  |   3 +
 drivers/gpu/drm/ast/ast_i2c.c  | 152 +++++++++++++++++++++++++++++++++
 drivers/gpu/drm/ast/ast_mode.c | 128 ---------------------------
 4 files changed, 156 insertions(+), 129 deletions(-)
 create mode 100644 drivers/gpu/drm/ast/ast_i2c.c

diff --git a/drivers/gpu/drm/ast/Makefile b/drivers/gpu/drm/ast/Makefile
index 438a2d05b115..21f71160bc3e 100644
--- a/drivers/gpu/drm/ast/Makefile
+++ b/drivers/gpu/drm/ast/Makefile
@@ -3,6 +3,6 @@
 # Makefile for the drm device driver.  This driver provides support for the
 # Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher.
 
-ast-y := ast_drv.o ast_main.o ast_mm.o ast_mode.o ast_post.o ast_dp501.o
+ast-y := ast_drv.o ast_i2c.o ast_main.o ast_mm.o ast_mode.o ast_post.o ast_dp501.o
 
 obj-$(CONFIG_DRM_AST) := ast.o
diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index 2cfce7dc95af..00bfa41ff7cb 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -357,4 +357,7 @@ bool ast_dp501_read_edid(struct drm_device *dev, u8 *ediddata);
 u8 ast_get_dp501_max_clk(struct drm_device *dev);
 void ast_init_3rdtx(struct drm_device *dev);
 
+/* ast_i2c.c */
+struct ast_i2c_chan *ast_i2c_create(struct drm_device *dev);
+
 #endif
diff --git a/drivers/gpu/drm/ast/ast_i2c.c b/drivers/gpu/drm/ast/ast_i2c.c
new file mode 100644
index 000000000000..8e4589d74b21
--- /dev/null
+++ b/drivers/gpu/drm/ast/ast_i2c.c
@@ -0,0 +1,152 @@
+// SPDX-License: MIT
+/*
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the
+ * "Software"), to deal in the Software without restriction, including
+ * without limitation the rights to use, copy, modify, merge, publish,
+ * distribute, sub license, and/or sell copies of the Software, and to
+ * permit persons to whom the Software is furnished to do so, subject to
+ * the following conditions:
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM,
+ * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+ * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
+ * USE OR OTHER DEALINGS IN THE SOFTWARE.
+ *
+ * The above copyright notice and this permission notice (including the
+ * next paragraph) shall be included in all copies or substantial portions
+ * of the Software.
+ */
+
+#include <drm/drm_managed.h>
+#include <drm/drm_print.h>
+
+#include "ast_drv.h"
+
+static void ast_i2c_setsda(void *i2c_priv, int data)
+{
+	struct ast_i2c_chan *i2c = i2c_priv;
+	struct ast_private *ast = to_ast_private(i2c->dev);
+	int i;
+	u8 ujcrb7, jtemp;
+
+	for (i = 0; i < 0x10000; i++) {
+		ujcrb7 = ((data & 0x01) ? 0 : 1) << 2;
+		ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb7, 0xf1, ujcrb7);
+		jtemp = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb7, 0x04);
+		if (ujcrb7 == jtemp)
+			break;
+	}
+}
+
+static void ast_i2c_setscl(void *i2c_priv, int clock)
+{
+	struct ast_i2c_chan *i2c = i2c_priv;
+	struct ast_private *ast = to_ast_private(i2c->dev);
+	int i;
+	u8 ujcrb7, jtemp;
+
+	for (i = 0; i < 0x10000; i++) {
+		ujcrb7 = ((clock & 0x01) ? 0 : 1);
+		ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb7, 0xf4, ujcrb7);
+		jtemp = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb7, 0x01);
+		if (ujcrb7 == jtemp)
+			break;
+	}
+}
+
+static int ast_i2c_getsda(void *i2c_priv)
+{
+	struct ast_i2c_chan *i2c = i2c_priv;
+	struct ast_private *ast = to_ast_private(i2c->dev);
+	uint32_t val, val2, count, pass;
+
+	count = 0;
+	pass = 0;
+	val = (ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb7, 0x20) >> 5) & 0x01;
+	do {
+		val2 = (ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb7, 0x20) >> 5) & 0x01;
+		if (val == val2) {
+			pass++;
+		} else {
+			pass = 0;
+			val = (ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb7, 0x20) >> 5) & 0x01;
+		}
+	} while ((pass < 5) && (count++ < 0x10000));
+
+	return val & 1 ? 1 : 0;
+}
+
+static int ast_i2c_getscl(void *i2c_priv)
+{
+	struct ast_i2c_chan *i2c = i2c_priv;
+	struct ast_private *ast = to_ast_private(i2c->dev);
+	uint32_t val, val2, count, pass;
+
+	count = 0;
+	pass = 0;
+	val = (ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb7, 0x10) >> 4) & 0x01;
+	do {
+		val2 = (ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb7, 0x10) >> 4) & 0x01;
+		if (val == val2) {
+			pass++;
+		} else {
+			pass = 0;
+			val = (ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb7, 0x10) >> 4) & 0x01;
+		}
+	} while ((pass < 5) && (count++ < 0x10000));
+
+	return val & 1 ? 1 : 0;
+}
+
+static void ast_i2c_release(struct drm_device *dev, void *res)
+{
+	struct ast_i2c_chan *i2c = res;
+
+	i2c_del_adapter(&i2c->adapter);
+	kfree(i2c);
+}
+
+struct ast_i2c_chan *ast_i2c_create(struct drm_device *dev)
+{
+	struct ast_i2c_chan *i2c;
+	int ret;
+
+	i2c = kzalloc(sizeof(struct ast_i2c_chan), GFP_KERNEL);
+	if (!i2c)
+		return NULL;
+
+	i2c->adapter.owner = THIS_MODULE;
+	i2c->adapter.class = I2C_CLASS_DDC;
+	i2c->adapter.dev.parent = dev->dev;
+	i2c->dev = dev;
+	i2c_set_adapdata(&i2c->adapter, i2c);
+	snprintf(i2c->adapter.name, sizeof(i2c->adapter.name),
+		 "AST i2c bit bus");
+	i2c->adapter.algo_data = &i2c->bit;
+
+	i2c->bit.udelay = 20;
+	i2c->bit.timeout = 2;
+	i2c->bit.data = i2c;
+	i2c->bit.setsda = ast_i2c_setsda;
+	i2c->bit.setscl = ast_i2c_setscl;
+	i2c->bit.getsda = ast_i2c_getsda;
+	i2c->bit.getscl = ast_i2c_getscl;
+	ret = i2c_bit_add_bus(&i2c->adapter);
+	if (ret) {
+		drm_err(dev, "Failed to register bit i2c\n");
+		goto out_kfree;
+	}
+
+	ret = drmm_add_action_or_reset(dev, ast_i2c_release, i2c);
+	if (ret)
+		return NULL;
+	return i2c;
+
+out_kfree:
+	kfree(i2c);
+	return NULL;
+}
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index abb8a3fdd812..44c2aafcb7c2 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -40,7 +40,6 @@
 #include <drm/drm_gem_atomic_helper.h>
 #include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/drm_gem_vram_helper.h>
-#include <drm/drm_managed.h>
 #include <drm/drm_plane_helper.h>
 #include <drm/drm_probe_helper.h>
 #include <drm/drm_simple_kms_helper.h>
@@ -48,8 +47,6 @@
 #include "ast_drv.h"
 #include "ast_tables.h"
 
-static struct ast_i2c_chan *ast_i2c_create(struct drm_device *dev);
-
 static inline void ast_load_palette_index(struct ast_private *ast,
 				     u8 index, u8 red, u8 green,
 				     u8 blue)
@@ -1408,128 +1405,3 @@ int ast_mode_config_init(struct ast_private *ast)
 
 	return 0;
 }
-
-static int get_clock(void *i2c_priv)
-{
-	struct ast_i2c_chan *i2c = i2c_priv;
-	struct ast_private *ast = to_ast_private(i2c->dev);
-	uint32_t val, val2, count, pass;
-
-	count = 0;
-	pass = 0;
-	val = (ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb7, 0x10) >> 4) & 0x01;
-	do {
-		val2 = (ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb7, 0x10) >> 4) & 0x01;
-		if (val == val2) {
-			pass++;
-		} else {
-			pass = 0;
-			val = (ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb7, 0x10) >> 4) & 0x01;
-		}
-	} while ((pass < 5) && (count++ < 0x10000));
-
-	return val & 1 ? 1 : 0;
-}
-
-static int get_data(void *i2c_priv)
-{
-	struct ast_i2c_chan *i2c = i2c_priv;
-	struct ast_private *ast = to_ast_private(i2c->dev);
-	uint32_t val, val2, count, pass;
-
-	count = 0;
-	pass = 0;
-	val = (ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb7, 0x20) >> 5) & 0x01;
-	do {
-		val2 = (ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb7, 0x20) >> 5) & 0x01;
-		if (val == val2) {
-			pass++;
-		} else {
-			pass = 0;
-			val = (ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb7, 0x20) >> 5) & 0x01;
-		}
-	} while ((pass < 5) && (count++ < 0x10000));
-
-	return val & 1 ? 1 : 0;
-}
-
-static void set_clock(void *i2c_priv, int clock)
-{
-	struct ast_i2c_chan *i2c = i2c_priv;
-	struct ast_private *ast = to_ast_private(i2c->dev);
-	int i;
-	u8 ujcrb7, jtemp;
-
-	for (i = 0; i < 0x10000; i++) {
-		ujcrb7 = ((clock & 0x01) ? 0 : 1);
-		ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb7, 0xf4, ujcrb7);
-		jtemp = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb7, 0x01);
-		if (ujcrb7 == jtemp)
-			break;
-	}
-}
-
-static void set_data(void *i2c_priv, int data)
-{
-	struct ast_i2c_chan *i2c = i2c_priv;
-	struct ast_private *ast = to_ast_private(i2c->dev);
-	int i;
-	u8 ujcrb7, jtemp;
-
-	for (i = 0; i < 0x10000; i++) {
-		ujcrb7 = ((data & 0x01) ? 0 : 1) << 2;
-		ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb7, 0xf1, ujcrb7);
-		jtemp = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb7, 0x04);
-		if (ujcrb7 == jtemp)
-			break;
-	}
-}
-
-static void ast_i2c_release(struct drm_device *dev, void *res)
-{
-	struct ast_i2c_chan *i2c = res;
-
-	i2c_del_adapter(&i2c->adapter);
-	kfree(i2c);
-}
-
-static struct ast_i2c_chan *ast_i2c_create(struct drm_device *dev)
-{
-	struct ast_i2c_chan *i2c;
-	int ret;
-
-	i2c = kzalloc(sizeof(struct ast_i2c_chan), GFP_KERNEL);
-	if (!i2c)
-		return NULL;
-
-	i2c->adapter.owner = THIS_MODULE;
-	i2c->adapter.class = I2C_CLASS_DDC;
-	i2c->adapter.dev.parent = dev->dev;
-	i2c->dev = dev;
-	i2c_set_adapdata(&i2c->adapter, i2c);
-	snprintf(i2c->adapter.name, sizeof(i2c->adapter.name),
-		 "AST i2c bit bus");
-	i2c->adapter.algo_data = &i2c->bit;
-
-	i2c->bit.udelay = 20;
-	i2c->bit.timeout = 2;
-	i2c->bit.data = i2c;
-	i2c->bit.setsda = set_data;
-	i2c->bit.setscl = set_clock;
-	i2c->bit.getsda = get_data;
-	i2c->bit.getscl = get_clock;
-	ret = i2c_bit_add_bus(&i2c->adapter);
-	if (ret) {
-		drm_err(dev, "Failed to register bit i2c\n");
-		goto out_kfree;
-	}
-
-	ret = drmm_add_action_or_reset(dev, ast_i2c_release, i2c);
-	if (ret)
-		return NULL;
-	return i2c;
-
-out_kfree:
-	kfree(i2c);
-	return NULL;
-}
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 3/3] drm/ast: Move I2C code into separate source file
  2021-12-06  9:11 ` [PATCH v2 3/3] drm/ast: Move I2C code into separate source file Thomas Zimmermann
@ 2021-12-16 10:25   ` Thomas Zimmermann
  0 siblings, 0 replies; 5+ messages in thread
From: Thomas Zimmermann @ 2021-12-16 10:25 UTC (permalink / raw)
  To: airlied, daniel, sam, kuohsiang_chou; +Cc: dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 11481 bytes --]



Am 06.12.21 um 10:11 schrieb Thomas Zimmermann:
> Move I2C code into its own source file. Makes the mode-setting
> code a little less convoluted.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>

A-b Maxime via irc

> ---
>   drivers/gpu/drm/ast/Makefile   |   2 +-
>   drivers/gpu/drm/ast/ast_drv.h  |   3 +
>   drivers/gpu/drm/ast/ast_i2c.c  | 152 +++++++++++++++++++++++++++++++++
>   drivers/gpu/drm/ast/ast_mode.c | 128 ---------------------------
>   4 files changed, 156 insertions(+), 129 deletions(-)
>   create mode 100644 drivers/gpu/drm/ast/ast_i2c.c
> 
> diff --git a/drivers/gpu/drm/ast/Makefile b/drivers/gpu/drm/ast/Makefile
> index 438a2d05b115..21f71160bc3e 100644
> --- a/drivers/gpu/drm/ast/Makefile
> +++ b/drivers/gpu/drm/ast/Makefile
> @@ -3,6 +3,6 @@
>   # Makefile for the drm device driver.  This driver provides support for the
>   # Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher.
>   
> -ast-y := ast_drv.o ast_main.o ast_mm.o ast_mode.o ast_post.o ast_dp501.o
> +ast-y := ast_drv.o ast_i2c.o ast_main.o ast_mm.o ast_mode.o ast_post.o ast_dp501.o
>   
>   obj-$(CONFIG_DRM_AST) := ast.o
> diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
> index 2cfce7dc95af..00bfa41ff7cb 100644
> --- a/drivers/gpu/drm/ast/ast_drv.h
> +++ b/drivers/gpu/drm/ast/ast_drv.h
> @@ -357,4 +357,7 @@ bool ast_dp501_read_edid(struct drm_device *dev, u8 *ediddata);
>   u8 ast_get_dp501_max_clk(struct drm_device *dev);
>   void ast_init_3rdtx(struct drm_device *dev);
>   
> +/* ast_i2c.c */
> +struct ast_i2c_chan *ast_i2c_create(struct drm_device *dev);
> +
>   #endif
> diff --git a/drivers/gpu/drm/ast/ast_i2c.c b/drivers/gpu/drm/ast/ast_i2c.c
> new file mode 100644
> index 000000000000..8e4589d74b21
> --- /dev/null
> +++ b/drivers/gpu/drm/ast/ast_i2c.c
> @@ -0,0 +1,152 @@
> +// SPDX-License: MIT
> +/*
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the
> + * "Software"), to deal in the Software without restriction, including
> + * without limitation the rights to use, copy, modify, merge, publish,
> + * distribute, sub license, and/or sell copies of the Software, and to
> + * permit persons to whom the Software is furnished to do so, subject to
> + * the following conditions:
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
> + * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM,
> + * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
> + * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
> + * USE OR OTHER DEALINGS IN THE SOFTWARE.
> + *
> + * The above copyright notice and this permission notice (including the
> + * next paragraph) shall be included in all copies or substantial portions
> + * of the Software.
> + */
> +
> +#include <drm/drm_managed.h>
> +#include <drm/drm_print.h>
> +
> +#include "ast_drv.h"
> +
> +static void ast_i2c_setsda(void *i2c_priv, int data)
> +{
> +	struct ast_i2c_chan *i2c = i2c_priv;
> +	struct ast_private *ast = to_ast_private(i2c->dev);
> +	int i;
> +	u8 ujcrb7, jtemp;
> +
> +	for (i = 0; i < 0x10000; i++) {
> +		ujcrb7 = ((data & 0x01) ? 0 : 1) << 2;
> +		ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb7, 0xf1, ujcrb7);
> +		jtemp = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb7, 0x04);
> +		if (ujcrb7 == jtemp)
> +			break;
> +	}
> +}
> +
> +static void ast_i2c_setscl(void *i2c_priv, int clock)
> +{
> +	struct ast_i2c_chan *i2c = i2c_priv;
> +	struct ast_private *ast = to_ast_private(i2c->dev);
> +	int i;
> +	u8 ujcrb7, jtemp;
> +
> +	for (i = 0; i < 0x10000; i++) {
> +		ujcrb7 = ((clock & 0x01) ? 0 : 1);
> +		ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb7, 0xf4, ujcrb7);
> +		jtemp = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb7, 0x01);
> +		if (ujcrb7 == jtemp)
> +			break;
> +	}
> +}
> +
> +static int ast_i2c_getsda(void *i2c_priv)
> +{
> +	struct ast_i2c_chan *i2c = i2c_priv;
> +	struct ast_private *ast = to_ast_private(i2c->dev);
> +	uint32_t val, val2, count, pass;
> +
> +	count = 0;
> +	pass = 0;
> +	val = (ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb7, 0x20) >> 5) & 0x01;
> +	do {
> +		val2 = (ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb7, 0x20) >> 5) & 0x01;
> +		if (val == val2) {
> +			pass++;
> +		} else {
> +			pass = 0;
> +			val = (ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb7, 0x20) >> 5) & 0x01;
> +		}
> +	} while ((pass < 5) && (count++ < 0x10000));
> +
> +	return val & 1 ? 1 : 0;
> +}
> +
> +static int ast_i2c_getscl(void *i2c_priv)
> +{
> +	struct ast_i2c_chan *i2c = i2c_priv;
> +	struct ast_private *ast = to_ast_private(i2c->dev);
> +	uint32_t val, val2, count, pass;
> +
> +	count = 0;
> +	pass = 0;
> +	val = (ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb7, 0x10) >> 4) & 0x01;
> +	do {
> +		val2 = (ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb7, 0x10) >> 4) & 0x01;
> +		if (val == val2) {
> +			pass++;
> +		} else {
> +			pass = 0;
> +			val = (ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb7, 0x10) >> 4) & 0x01;
> +		}
> +	} while ((pass < 5) && (count++ < 0x10000));
> +
> +	return val & 1 ? 1 : 0;
> +}
> +
> +static void ast_i2c_release(struct drm_device *dev, void *res)
> +{
> +	struct ast_i2c_chan *i2c = res;
> +
> +	i2c_del_adapter(&i2c->adapter);
> +	kfree(i2c);
> +}
> +
> +struct ast_i2c_chan *ast_i2c_create(struct drm_device *dev)
> +{
> +	struct ast_i2c_chan *i2c;
> +	int ret;
> +
> +	i2c = kzalloc(sizeof(struct ast_i2c_chan), GFP_KERNEL);
> +	if (!i2c)
> +		return NULL;
> +
> +	i2c->adapter.owner = THIS_MODULE;
> +	i2c->adapter.class = I2C_CLASS_DDC;
> +	i2c->adapter.dev.parent = dev->dev;
> +	i2c->dev = dev;
> +	i2c_set_adapdata(&i2c->adapter, i2c);
> +	snprintf(i2c->adapter.name, sizeof(i2c->adapter.name),
> +		 "AST i2c bit bus");
> +	i2c->adapter.algo_data = &i2c->bit;
> +
> +	i2c->bit.udelay = 20;
> +	i2c->bit.timeout = 2;
> +	i2c->bit.data = i2c;
> +	i2c->bit.setsda = ast_i2c_setsda;
> +	i2c->bit.setscl = ast_i2c_setscl;
> +	i2c->bit.getsda = ast_i2c_getsda;
> +	i2c->bit.getscl = ast_i2c_getscl;
> +	ret = i2c_bit_add_bus(&i2c->adapter);
> +	if (ret) {
> +		drm_err(dev, "Failed to register bit i2c\n");
> +		goto out_kfree;
> +	}
> +
> +	ret = drmm_add_action_or_reset(dev, ast_i2c_release, i2c);
> +	if (ret)
> +		return NULL;
> +	return i2c;
> +
> +out_kfree:
> +	kfree(i2c);
> +	return NULL;
> +}
> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
> index abb8a3fdd812..44c2aafcb7c2 100644
> --- a/drivers/gpu/drm/ast/ast_mode.c
> +++ b/drivers/gpu/drm/ast/ast_mode.c
> @@ -40,7 +40,6 @@
>   #include <drm/drm_gem_atomic_helper.h>
>   #include <drm/drm_gem_framebuffer_helper.h>
>   #include <drm/drm_gem_vram_helper.h>
> -#include <drm/drm_managed.h>
>   #include <drm/drm_plane_helper.h>
>   #include <drm/drm_probe_helper.h>
>   #include <drm/drm_simple_kms_helper.h>
> @@ -48,8 +47,6 @@
>   #include "ast_drv.h"
>   #include "ast_tables.h"
>   
> -static struct ast_i2c_chan *ast_i2c_create(struct drm_device *dev);
> -
>   static inline void ast_load_palette_index(struct ast_private *ast,
>   				     u8 index, u8 red, u8 green,
>   				     u8 blue)
> @@ -1408,128 +1405,3 @@ int ast_mode_config_init(struct ast_private *ast)
>   
>   	return 0;
>   }
> -
> -static int get_clock(void *i2c_priv)
> -{
> -	struct ast_i2c_chan *i2c = i2c_priv;
> -	struct ast_private *ast = to_ast_private(i2c->dev);
> -	uint32_t val, val2, count, pass;
> -
> -	count = 0;
> -	pass = 0;
> -	val = (ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb7, 0x10) >> 4) & 0x01;
> -	do {
> -		val2 = (ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb7, 0x10) >> 4) & 0x01;
> -		if (val == val2) {
> -			pass++;
> -		} else {
> -			pass = 0;
> -			val = (ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb7, 0x10) >> 4) & 0x01;
> -		}
> -	} while ((pass < 5) && (count++ < 0x10000));
> -
> -	return val & 1 ? 1 : 0;
> -}
> -
> -static int get_data(void *i2c_priv)
> -{
> -	struct ast_i2c_chan *i2c = i2c_priv;
> -	struct ast_private *ast = to_ast_private(i2c->dev);
> -	uint32_t val, val2, count, pass;
> -
> -	count = 0;
> -	pass = 0;
> -	val = (ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb7, 0x20) >> 5) & 0x01;
> -	do {
> -		val2 = (ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb7, 0x20) >> 5) & 0x01;
> -		if (val == val2) {
> -			pass++;
> -		} else {
> -			pass = 0;
> -			val = (ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb7, 0x20) >> 5) & 0x01;
> -		}
> -	} while ((pass < 5) && (count++ < 0x10000));
> -
> -	return val & 1 ? 1 : 0;
> -}
> -
> -static void set_clock(void *i2c_priv, int clock)
> -{
> -	struct ast_i2c_chan *i2c = i2c_priv;
> -	struct ast_private *ast = to_ast_private(i2c->dev);
> -	int i;
> -	u8 ujcrb7, jtemp;
> -
> -	for (i = 0; i < 0x10000; i++) {
> -		ujcrb7 = ((clock & 0x01) ? 0 : 1);
> -		ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb7, 0xf4, ujcrb7);
> -		jtemp = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb7, 0x01);
> -		if (ujcrb7 == jtemp)
> -			break;
> -	}
> -}
> -
> -static void set_data(void *i2c_priv, int data)
> -{
> -	struct ast_i2c_chan *i2c = i2c_priv;
> -	struct ast_private *ast = to_ast_private(i2c->dev);
> -	int i;
> -	u8 ujcrb7, jtemp;
> -
> -	for (i = 0; i < 0x10000; i++) {
> -		ujcrb7 = ((data & 0x01) ? 0 : 1) << 2;
> -		ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb7, 0xf1, ujcrb7);
> -		jtemp = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb7, 0x04);
> -		if (ujcrb7 == jtemp)
> -			break;
> -	}
> -}
> -
> -static void ast_i2c_release(struct drm_device *dev, void *res)
> -{
> -	struct ast_i2c_chan *i2c = res;
> -
> -	i2c_del_adapter(&i2c->adapter);
> -	kfree(i2c);
> -}
> -
> -static struct ast_i2c_chan *ast_i2c_create(struct drm_device *dev)
> -{
> -	struct ast_i2c_chan *i2c;
> -	int ret;
> -
> -	i2c = kzalloc(sizeof(struct ast_i2c_chan), GFP_KERNEL);
> -	if (!i2c)
> -		return NULL;
> -
> -	i2c->adapter.owner = THIS_MODULE;
> -	i2c->adapter.class = I2C_CLASS_DDC;
> -	i2c->adapter.dev.parent = dev->dev;
> -	i2c->dev = dev;
> -	i2c_set_adapdata(&i2c->adapter, i2c);
> -	snprintf(i2c->adapter.name, sizeof(i2c->adapter.name),
> -		 "AST i2c bit bus");
> -	i2c->adapter.algo_data = &i2c->bit;
> -
> -	i2c->bit.udelay = 20;
> -	i2c->bit.timeout = 2;
> -	i2c->bit.data = i2c;
> -	i2c->bit.setsda = set_data;
> -	i2c->bit.setscl = set_clock;
> -	i2c->bit.getsda = get_data;
> -	i2c->bit.getscl = get_clock;
> -	ret = i2c_bit_add_bus(&i2c->adapter);
> -	if (ret) {
> -		drm_err(dev, "Failed to register bit i2c\n");
> -		goto out_kfree;
> -	}
> -
> -	ret = drmm_add_action_or_reset(dev, ast_i2c_release, i2c);
> -	if (ret)
> -		return NULL;
> -	return i2c;
> -
> -out_kfree:
> -	kfree(i2c);
> -	return NULL;
> -}
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-12-16 10:25 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-06  9:11 [PATCH v2 0/3] ast: Fix I2C corner cases wrt init/cleanup Thomas Zimmermann
2021-12-06  9:11 ` [PATCH v2 1/3] drm/ast: Handle failed I2C initialization gracefully Thomas Zimmermann
2021-12-06  9:11 ` [PATCH v2 2/3] drm/ast: Convert I2C code to managed cleanup Thomas Zimmermann
2021-12-06  9:11 ` [PATCH v2 3/3] drm/ast: Move I2C code into separate source file Thomas Zimmermann
2021-12-16 10:25   ` Thomas Zimmermann

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.