[PATCH v8 02/10] drm/hisilicon: Add hisilicon kirin drm master driver
On 13 April 2016 at 20:15, Emil Velikov wrote:
> Hi Xinliang,
>
> On 11 April 2016 at 09:55, Xinliang Liu wrote:
>
>> +static int kirin_drm_connectors_register(struct drm_device *dev)
>> +{
>> + struct drm_connector *connector;
>> + struct drm_connector *failed_connector;
>> + int ret;
>> +
>> + mutex_lock(&dev->mode_config.mutex);
>> + drm_for_each_connector(connector, dev) {
>> + ret = drm_connector_register(connector);
>> + if (ret) {
>> + failed_connector = connector;
>> + goto err;
>> + }
>> + }
>> + mutex_unlock(&dev->mode_config.mutex);
>> +
>> + return 0;
>> +
>> +err:
>> + drm_for_each_connector(connector, dev) {
>> + if (failed_connector == connector)
>> + break;
>> + drm_connector_unregister(connector);
>> + }
>> + mutex_unlock(&dev->mode_config.mutex);
>> +
>> + return ret;
>> +}
>> +
> Iirc we have new drm_connector_{un,}register_all() helpers.You might
> want to use it once they are in (i.e. not sure what your base is and
> if they have landed yet).
>
>
>> +static struct device_node *kirin_get_remote_node(struct device_node *np)
>> +{
>> + struct device_node *endpoint, *remote;
>> +
>> + /* get the first endpoint, in our case only one remote node
>> +* is connected to display controller.
>> +*/
>> + endpoint = of_graph_get_next_endpoint(np, NULL);
>> + if (!endpoint) {
>> + DRM_ERROR("no valid endpoint node\n");
>> + return ERR_PTR(-ENODEV);
>> + }
>> + of_node_put(endpoint);
>> +
>> + remote = of_graph_get_remote_port_parent(endpoint);
>> + if (!remote) {
>> + DRM_ERROR("no valid remote node\n");
>> + return ERR_PTR(-ENODEV);
>> + }
>> + of_node_put(remote);
>> +
>> + if (!of_device_is_available(remote)) {
>> + DRM_ERROR("not available for remote node\n");
>> + return ERR_PTR(-ENODEV);
>> + }
>> +
> This seems like a common pattern in many platform DRM drivers. Yet
> some tend to differ in subtle ways - I'm leaning that they might be
> bugs, but one cannot be too sure.
>
> A friendly request:
> Can you please follow up by adding a helper and removing the
> duplication thoughout ?
Hi emil, recently I found that there is already a helper to do such work.
It is drm_of_component_probe.
-xinliang
>
> Thanks
> Emil
[PATCH v8 02/10] drm/hisilicon: Add hisilicon kirin drm master driver
Hi Emil,
On 13 April 2016 at 20:15, Emil Velikov wrote:
> Hi Xinliang,
>
> On 11 April 2016 at 09:55, Xinliang Liu wrote:
>
>> +static int kirin_drm_connectors_register(struct drm_device *dev)
>> +{
>> + struct drm_connector *connector;
>> + struct drm_connector *failed_connector;
>> + int ret;
>> +
>> + mutex_lock(&dev->mode_config.mutex);
>> + drm_for_each_connector(connector, dev) {
>> + ret = drm_connector_register(connector);
>> + if (ret) {
>> + failed_connector = connector;
>> + goto err;
>> + }
>> + }
>> + mutex_unlock(&dev->mode_config.mutex);
>> +
>> + return 0;
>> +
>> +err:
>> + drm_for_each_connector(connector, dev) {
>> + if (failed_connector == connector)
>> + break;
>> + drm_connector_unregister(connector);
>> + }
>> + mutex_unlock(&dev->mode_config.mutex);
>> +
>> + return ret;
>> +}
>> +
> Iirc we have new drm_connector_{un,}register_all() helpers.You might
> want to use it once they are in (i.e. not sure what your base is and
> if they have landed yet).
I can't find these drm_connector_{un,}register_all() helpers in latest
tag v4.6-rc3.
>
>
>> +static struct device_node *kirin_get_remote_node(struct device_node *np)
>> +{
>> + struct device_node *endpoint, *remote;
>> +
>> + /* get the first endpoint, in our case only one remote node
>> +* is connected to display controller.
>> +*/
>> + endpoint = of_graph_get_next_endpoint(np, NULL);
>> + if (!endpoint) {
>> + DRM_ERROR("no valid endpoint node\n");
>> + return ERR_PTR(-ENODEV);
>> + }
>> + of_node_put(endpoint);
>> +
>> + remote = of_graph_get_remote_port_parent(endpoint);
>> + if (!remote) {
>> + DRM_ERROR("no valid remote node\n");
>> + return ERR_PTR(-ENODEV);
>> + }
>> + of_node_put(remote);
>> +
>> + if (!of_device_is_available(remote)) {
>> + DRM_ERROR("not available for remote node\n");
>> + return ERR_PTR(-ENODEV);
>> + }
>> +
> This seems like a common pattern in many platform DRM drivers. Yet
> some tend to differ in subtle ways - I'm leaning that they might be
> bugs, but one cannot be too sure.
>
> A friendly request:
> Can you please follow up by adding a helper and removing the
> duplication thoughout ?
Yes, I will create another patch set for this. I think I need to
figure out other drivers requirements.
Maybe, I need to add one more parameter for this helper. That's
endpoint index to get which endpoint remote node.
I will follow up this.
Thanks,
-xinliang
>
> Thanks
> Emil
[PATCH v8 02/10] drm/hisilicon: Add hisilicon kirin drm master driver
Hi Xinliang,
On 11 April 2016 at 09:55, Xinliang Liu wrote:
> +static int kirin_drm_connectors_register(struct drm_device *dev)
> +{
> + struct drm_connector *connector;
> + struct drm_connector *failed_connector;
> + int ret;
> +
> + mutex_lock(&dev->mode_config.mutex);
> + drm_for_each_connector(connector, dev) {
> + ret = drm_connector_register(connector);
> + if (ret) {
> + failed_connector = connector;
> + goto err;
> + }
> + }
> + mutex_unlock(&dev->mode_config.mutex);
> +
> + return 0;
> +
> +err:
> + drm_for_each_connector(connector, dev) {
> + if (failed_connector == connector)
> + break;
> + drm_connector_unregister(connector);
> + }
> + mutex_unlock(&dev->mode_config.mutex);
> +
> + return ret;
> +}
> +
Iirc we have new drm_connector_{un,}register_all() helpers.You might
want to use it once they are in (i.e. not sure what your base is and
if they have landed yet).
> +static struct device_node *kirin_get_remote_node(struct device_node *np)
> +{
> + struct device_node *endpoint, *remote;
> +
> + /* get the first endpoint, in our case only one remote node
> +* is connected to display controller.
> +*/
> + endpoint = of_graph_get_next_endpoint(np, NULL);
> + if (!endpoint) {
> + DRM_ERROR("no valid endpoint node\n");
> + return ERR_PTR(-ENODEV);
> + }
> + of_node_put(endpoint);
> +
> + remote = of_graph_get_remote_port_parent(endpoint);
> + if (!remote) {
> + DRM_ERROR("no valid remote node\n");
> + return ERR_PTR(-ENODEV);
> + }
> + of_node_put(remote);
> +
> + if (!of_device_is_available(remote)) {
> + DRM_ERROR("not available for remote node\n");
> + return ERR_PTR(-ENODEV);
> + }
> +
This seems like a common pattern in many platform DRM drivers. Yet
some tend to differ in subtle ways - I'm leaning that they might be
bugs, but one cannot be too sure.
A friendly request:
Can you please follow up by adding a helper and removing the
duplication thoughout ?
Thanks
Emil
[PATCH v8 02/10] drm/hisilicon: Add hisilicon kirin drm master driver
Add kirin DRM master driver for hi6220 SoC which used in HiKey board.
Add dumb buffer feature.
Add prime dmabuf feature.
v8: None.
v7:
- Add config.mutex protection when accessing mode_config.connector_list.
- Clean up match data getting.
v6: None.
v5: None.
v4: None.
v3:
- Move and rename all the files to kirin sub-directory.
So that we could separate different seires SoCs' driver.
- Replace drm_platform_init, load, unload implementation.
v2:
- Remove abtraction layer.
Signed-off-by: Xinliang Liu
Signed-off-by: Xinwei Kong
---
drivers/gpu/drm/Kconfig | 2 +
drivers/gpu/drm/Makefile| 1 +
drivers/gpu/drm/hisilicon/Kconfig | 5 +
drivers/gpu/drm/hisilicon/Makefile | 5 +
drivers/gpu/drm/hisilicon/kirin/Kconfig | 9 +
drivers/gpu/drm/hisilicon/kirin/Makefile| 3 +
drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 309
drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.h | 20 ++
8 files changed, 354 insertions(+)
create mode 100644 drivers/gpu/drm/hisilicon/Kconfig
create mode 100644 drivers/gpu/drm/hisilicon/Makefile
create mode 100644 drivers/gpu/drm/hisilicon/kirin/Kconfig
create mode 100644 drivers/gpu/drm/hisilicon/kirin/Makefile
create mode 100644 drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
create mode 100644 drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.h
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 84c587a1fa93..838f99acc2c0 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -289,3 +289,5 @@ source "drivers/gpu/drm/imx/Kconfig"
source "drivers/gpu/drm/vc4/Kconfig"
source "drivers/gpu/drm/etnaviv/Kconfig"
+
+source "drivers/gpu/drm/hisilicon/Kconfig"
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 6eb94fc561dc..327d29638137 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -78,3 +78,4 @@ obj-y += panel/
obj-y += bridge/
obj-$(CONFIG_DRM_FSL_DCU) += fsl-dcu/
obj-$(CONFIG_DRM_ETNAVIV) += etnaviv/
+obj-y += hisilicon/
diff --git a/drivers/gpu/drm/hisilicon/Kconfig
b/drivers/gpu/drm/hisilicon/Kconfig
new file mode 100644
index ..558c61b1b8e8
--- /dev/null
+++ b/drivers/gpu/drm/hisilicon/Kconfig
@@ -0,0 +1,5 @@
+#
+# hisilicon drm device configuration.
+# Please keep this list sorted alphabetically
+
+source "drivers/gpu/drm/hisilicon/kirin/Kconfig"
diff --git a/drivers/gpu/drm/hisilicon/Makefile
b/drivers/gpu/drm/hisilicon/Makefile
new file mode 100644
index ..e3f6d493c996
--- /dev/null
+++ b/drivers/gpu/drm/hisilicon/Makefile
@@ -0,0 +1,5 @@
+#
+# Makefile for hisilicon drm drivers.
+# Please keep this list sorted alphabetically
+
+obj-$(CONFIG_DRM_HISI_KIRIN) += kirin/
diff --git a/drivers/gpu/drm/hisilicon/kirin/Kconfig
b/drivers/gpu/drm/hisilicon/kirin/Kconfig
new file mode 100644
index ..3ac4b8edeac1
--- /dev/null
+++ b/drivers/gpu/drm/hisilicon/kirin/Kconfig
@@ -0,0 +1,9 @@
+config DRM_HISI_KIRIN
+ tristate "DRM Support for Hisilicon Kirin series SoCs Platform"
+ depends on DRM
+ select DRM_KMS_HELPER
+ select DRM_GEM_CMA_HELPER
+ select DRM_KMS_CMA_HELPER
+ help
+ Choose this option if you have a hisilicon Kirin chipsets(hi6220).
+ If M is selected the module will be called kirin-drm.
diff --git a/drivers/gpu/drm/hisilicon/kirin/Makefile
b/drivers/gpu/drm/hisilicon/kirin/Makefile
new file mode 100644
index ..cb346de47d48
--- /dev/null
+++ b/drivers/gpu/drm/hisilicon/kirin/Makefile
@@ -0,0 +1,3 @@
+kirin-drm-y := kirin_drm_drv.o
+
+obj-$(CONFIG_DRM_HISI_KIRIN) += kirin-drm.o
diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
new file mode 100644
index ..976c9b1a3fd3
--- /dev/null
+++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
@@ -0,0 +1,309 @@
+/*
+ * Hisilicon Kirin SoCs drm master driver
+ *
+ * Copyright (c) 2016 Linaro Limited.
+ * Copyright (c) 2014-2016 Hisilicon Limited.
+ *
+ * Author:
+ * Xinliang Liu
+ * Xinliang Liu
+ * Xinwei Kong
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include
+#include
+#include
+
+#include
+#include
+#include
+#include
+
+#include "kirin_drm_drv.h"
+
+static struct kirin_dc_ops *dc_ops;
+
+static int kirin_drm_kms_cleanup(struct drm_device *dev)
+{
+ dc_ops->cleanup(dev);
+ drm_mode_config_cleanup(dev);
+
+ return 0;
+}
+
+static const struct drm_mode_config_funcs kirin_drm_mode_config_funcs = {
+ .fb_create = drm_fb_cma_create,
+ .atomic_check = drm_atomic_helper_check,
+ .atomic_commit = drm_atomic_helper_commit,
+};
+
+static void kirin_drm_mode_config_init(struct drm_device *dev)
+{
