[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <cb55c4bb-95fe-3261-af09-669f89269d5b@suse.de>
Date: Mon, 21 Sep 2020 09:20:31 +0200
From: Thomas Zimmermann <tzimmermann@...e.de>
To: Tian Tao <tiantao6@...ilicon.com>, airlied@...ux.ie,
daniel@...ll.ch, kraxel@...hat.com, alexander.deucher@....com,
tglx@...utronix.de, dri-devel@...ts.freedesktop.org,
xinliang.liu@...aro.org, linux-kernel@...r.kernel.org
Cc: linuxarm@...wei.com
Subject: Re: [PATCH drm/hisilicon 1/3] drm/hisilicon: Support i2c driver
algorithms for bit-shift adapters
Hi
Am 21.09.20 um 05:25 schrieb Tian Tao:
> Adding driver implementation to support i2c driver algorithms for
> bit-shift adapters, so hibmc will using the interface provided by
> drm to read edid.
>
> Signed-off-by: Tian Tao <tiantao6@...ilicon.com>
> ---
> drivers/gpu/drm/hisilicon/hibmc/Makefile | 2 +-
> drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h | 21 +++++-
> drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_i2c.c | 98 +++++++++++++++++++++++++
> 3 files changed, 119 insertions(+), 2 deletions(-)
> create mode 100644 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_i2c.c
>
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/Makefile b/drivers/gpu/drm/hisilicon/hibmc/Makefile
> index f991327..684ef79 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/Makefile
> +++ b/drivers/gpu/drm/hisilicon/hibmc/Makefile
> @@ -1,4 +1,4 @@
> # SPDX-License-Identifier: GPL-2.0-only
> -hibmc-drm-y := hibmc_drm_drv.o hibmc_drm_de.o hibmc_drm_vdac.o hibmc_ttm.o
> +hibmc-drm-y := hibmc_drm_drv.o hibmc_drm_de.o hibmc_drm_vdac.o hibmc_ttm.o hibmc_drm_i2c.o
>
> obj-$(CONFIG_DRM_HISI_HIBMC) += hibmc-drm.o
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
> index 197485e..1b2edb3 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
> @@ -14,11 +14,24 @@
> #ifndef HIBMC_DRM_DRV_H
> #define HIBMC_DRM_DRV_H
>
> +#include <linux/gpio/consumer.h>
> +#include <linux/i2c-algo-bit.h>
> +#include <linux/i2c.h>
> +
> +#include <drm/drm_edid.h>
> #include <drm/drm_fb_helper.h>
> #include <drm/drm_framebuffer.h>
>
> struct drm_device;
>
> +struct hibmc_connector {
> + struct drm_connector base;
> + struct drm_device *dev;
dev is not required. There's already a dev field under base.
> +
> + struct i2c_adapter adapter;
> + struct i2c_algo_bit_data bit_data;
> +};
> +
> struct hibmc_drm_private {
> /* hw */
> void __iomem *mmio;
> @@ -31,10 +44,15 @@ struct hibmc_drm_private {
> struct drm_plane primary_plane;
> struct drm_crtc crtc;
> struct drm_encoder encoder;
> - struct drm_connector connector;
> + struct hibmc_connector connector;
> bool mode_config_initialized;
> };
>
> +static inline struct hibmc_connector *to_hibmc_connector(struct drm_connector *connector)
> +{
> + return container_of(connector, struct hibmc_connector, base);
> +}
> +
> void hibmc_set_power_mode(struct hibmc_drm_private *priv,
> unsigned int power_mode);
> void hibmc_set_current_gate(struct hibmc_drm_private *priv,
> @@ -47,6 +65,7 @@ int hibmc_mm_init(struct hibmc_drm_private *hibmc);
> void hibmc_mm_fini(struct hibmc_drm_private *hibmc);
> int hibmc_dumb_create(struct drm_file *file, struct drm_device *dev,
> struct drm_mode_create_dumb *args);
> +int hibmc_ddc_create(struct hibmc_connector *connector);
>
> extern const struct drm_mode_config_funcs hibmc_mode_funcs;
>
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_i2c.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_i2c.c
> new file mode 100644
> index 0000000..0506846
> --- /dev/null
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_i2c.c
> @@ -0,0 +1,98 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/* Hisilicon Hibmc SoC drm driver
> + *
> + * Based on the bochs drm driver.
> + *
> + * Copyright (c) 2016 Huawei Limited.
> + *
> + * Author:
> + * Tian Tao <tiantao6@...ilicon.com>
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/pci.h>
> +
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_probe_helper.h>
> +
> +#include "hibmc_drm_drv.h"
> +
> +#define GPIO_DATA 0x0802A0
> +#define GPIO_DATA_DIRECTION 0x0802A4
> +
> +#define GPIO_SCL_MASK 0x1
> +#define GPIO_SDA_MASK 0x2
Weird indention.
> +
> +static void hibmc_set_i2c_signal(void *data, u32 mask, int value)
> +{
> + struct hibmc_connector *hibmc_connector = data;
> + struct hibmc_drm_private *priv = hibmc_connector->dev->dev_private;
Oh, hibmc is still using dev_private. dev_private is on it's way out.
The least thing to do is to make little function that wraps the
conversion from struct drm_device to hibmc_drm_private. And then convert
over all uses of dev_private over to the function. That's for another
patchset.
That's all just nits, so
Reviewed-by: Thomas Zimmermann <tzimmermann@...e.de>
Best regards
Thomas
> + u32 tmp_dir = readl(priv->mmio + GPIO_DATA_DIRECTION);
> +
> + if (value) {
> + tmp_dir &= ~mask;
> + writel(tmp_dir, priv->mmio + GPIO_DATA_DIRECTION);
> + } else {
> + u32 tmp_data = readl(priv->mmio + GPIO_DATA);
> +
> + tmp_data &= ~mask;
> + writel(tmp_data, priv->mmio + GPIO_DATA);
> +
> + tmp_dir |= mask;
> + writel(tmp_dir, priv->mmio + GPIO_DATA_DIRECTION);
> + }
> +}
> +
> +static int hibmc_get_i2c_signal(void *data, u32 mask)
> +{
> + struct hibmc_connector *hibmc_connector = data;
> + struct hibmc_drm_private *priv = hibmc_connector->dev->dev_private;
> + u32 tmp_dir = readl(priv->mmio + GPIO_DATA_DIRECTION);
> +
> + if ((tmp_dir & mask) != mask) {
> + tmp_dir &= ~mask;
> + writel(tmp_dir, priv->mmio + GPIO_DATA_DIRECTION);
> + }
> +
> + return (readl(priv->mmio + GPIO_DATA) & mask) ? 1 : 0;
> +}
> +
> +static void hibmc_ddc_setsda(void *data, int state)
> +{
> + hibmc_set_i2c_signal(data, GPIO_SDA_MASK, state);
> +}
> +
> +static void hibmc_ddc_setscl(void *data, int state)
> +{
> + hibmc_set_i2c_signal(data, GPIO_SCL_MASK, state);
> +}
> +
> +static int hibmc_ddc_getsda(void *data)
> +{
> + return hibmc_get_i2c_signal(data, GPIO_SDA_MASK);
> +}
> +
> +static int hibmc_ddc_getscl(void *data)
> +{
> + return hibmc_get_i2c_signal(data, GPIO_SCL_MASK);
> +}
> +
> +int hibmc_ddc_create(struct hibmc_connector *connector)
> +{
> + connector->adapter.owner = THIS_MODULE;
> + connector->adapter.class = I2C_CLASS_DDC;
> + snprintf(connector->adapter.name, I2C_NAME_SIZE, "HIS i2c bit bus");
> + connector->adapter.dev.parent = &connector->dev->pdev->dev;
> + i2c_set_adapdata(&connector->adapter, connector);
> + connector->adapter.algo_data = &connector->bit_data;
> +
> + connector->bit_data.udelay = 20;
> + connector->bit_data.timeout = usecs_to_jiffies(2000);
> + connector->bit_data.data = connector;
> + connector->bit_data.setsda = hibmc_ddc_setsda;
> + connector->bit_data.setscl = hibmc_ddc_setscl;
> + connector->bit_data.getsda = hibmc_ddc_getsda;
> + connector->bit_data.getscl = hibmc_ddc_getscl;
> +
> + return i2c_bit_add_bus(&connector->adapter);
> +}
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
Download attachment "signature.asc" of type "application/pgp-signature" (517 bytes)
Powered by blists - more mailing lists