diff mbox

[v6,6/9] drm/hisilicon/hibmc: Add encoder for VDAC

Message ID 1477639682-22520-7-git-send-email-zourongrong@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Rongrong Zou Oct. 28, 2016, 7:27 a.m. UTC
Add encoder funcs and helpers for VDAC.

Signed-off-by: Rongrong Zou <zourongrong@gmail.com>
---
 drivers/gpu/drm/hisilicon/hibmc/Makefile         |  2 +-
 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c  |  6 ++
 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h  |  2 +
 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c | 89 ++++++++++++++++++++++++
 4 files changed, 98 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c

Comments

Sean Paul Nov. 10, 2016, 10:20 p.m. UTC | #1
On Fri, Oct 28, 2016 at 3:27 AM, Rongrong Zou <zourongrong@gmail.com> wrote:
> Add encoder funcs and helpers for VDAC.
>
> Signed-off-by: Rongrong Zou <zourongrong@gmail.com>
> ---
>  drivers/gpu/drm/hisilicon/hibmc/Makefile         |  2 +-
>  drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c  |  6 ++
>  drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h  |  2 +
>  drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c | 89 ++++++++++++++++++++++++
>  4 files changed, 98 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
>
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/Makefile b/drivers/gpu/drm/hisilicon/hibmc/Makefile
> index 72e107e..e04f114 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/Makefile
> +++ b/drivers/gpu/drm/hisilicon/hibmc/Makefile
> @@ -1,5 +1,5 @@
>  ccflags-y := -Iinclude/drm
> -hibmc-drm-y := hibmc_drm_drv.o hibmc_drm_de.o hibmc_drm_fbdev.o hibmc_drm_power.o hibmc_ttm.o
> +hibmc-drm-y := hibmc_drm_drv.o hibmc_drm_de.o hibmc_drm_vdac.o hibmc_drm_fbdev.o hibmc_drm_power.o hibmc_ttm.o
>
>  obj-$(CONFIG_DRM_HISI_HIBMC)   +=hibmc-drm.o
>  #obj-y += hibmc-drm.o
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
> index 303cd36..ba191e1 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
> @@ -125,6 +125,12 @@ static int hibmc_kms_init(struct hibmc_drm_device *hidev)
>                 return ret;
>         }
>
> +       ret = hibmc_encoder_init(hidev);
> +       if (ret) {
> +               DRM_ERROR("failed to init encoder\n");
> +               return ret;
> +       }
> +
>         return 0;
>  }
>
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
> index 5731ec2..401cea4 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
> @@ -47,6 +47,7 @@ struct hibmc_drm_device {
>         struct drm_device  *dev;
>         struct drm_plane plane;
>         struct drm_crtc crtc;
> +       struct drm_encoder encoder;

Same comment here, you don't need to keep track of this

>         bool mode_config_initialized;
>
>         /* ttm */
> @@ -87,6 +88,7 @@ static inline struct hibmc_bo *gem_to_hibmc_bo(struct drm_gem_object *gem)
>
>  int hibmc_plane_init(struct hibmc_drm_device *hidev);
>  int hibmc_crtc_init(struct hibmc_drm_device *hidev);
> +int hibmc_encoder_init(struct hibmc_drm_device *hidev);
>  int hibmc_fbdev_init(struct hibmc_drm_device *hidev);
>  void hibmc_fbdev_fini(struct hibmc_drm_device *hidev);
>
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
> new file mode 100644
> index 0000000..953f659
> --- /dev/null
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
> @@ -0,0 +1,89 @@
> +/* Hisilicon Hibmc SoC drm driver
> + *
> + * Based on the bochs drm driver.
> + *
> + * Copyright (c) 2016 Huawei Limited.
> + *
> + * Author:
> + *     Rongrong Zou <zourongrong@huawei.com>
> + *     Rongrong Zou <zourongrong@gmail.com>
> + *     Jianhua Li <lijianhua@huawei.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + */
> +
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_crtc_helper.h>
> +
> +#include "hibmc_drm_drv.h"
> +#include "hibmc_drm_regs.h"
> +
> +static int defx = 800;
> +static int defy = 600;
> +
> +module_param(defx, int, 0444);
> +module_param(defy, int, 0444);
> +MODULE_PARM_DESC(defx, "default x resolution");
> +MODULE_PARM_DESC(defy, "default y resolution");

Not used, and I'm not sure these are a good idea

> +
> +static void hibmc_encoder_disable(struct drm_encoder *encoder)
> +{
> +}
> +
> +static void hibmc_encoder_enable(struct drm_encoder *encoder)
> +{
> +}

Null-checked, no need to stub

> +
> +static void hibmc_encoder_mode_set(struct drm_encoder *encoder,
> +                                  struct drm_display_mode *mode,
> +                                  struct drm_display_mode *adj_mode)
> +{
> +       u32 reg;
> +       struct drm_device *dev = encoder->dev;
> +       struct hibmc_drm_device *hidev = dev->dev_private;
> +
> +       /* just open DISPLAY_CONTROL_HISILE register bit 3:0*/
> +       reg = readl(hidev->mmio + DISPLAY_CONTROL_HISILE);
> +       reg |= 0xf;

Can you just pull this into a #define instead of explaining in the comment?

> +       writel(reg, hidev->mmio + DISPLAY_CONTROL_HISILE);
> +}
> +
> +static int hibmc_encoder_atomic_check(struct drm_encoder *encoder,
> +                                     struct drm_crtc_state *crtc_state,
> +                                     struct drm_connector_state *conn_state)
> +{
> +       return 0;
> +}

null-checked, remove stub

> +
> +static const struct drm_encoder_helper_funcs hibmc_encoder_helper_funcs = {
> +       .mode_set = hibmc_encoder_mode_set,
> +       .disable = hibmc_encoder_disable,
> +       .enable = hibmc_encoder_enable,
> +       .atomic_check = hibmc_encoder_atomic_check,
> +};
> +
> +static const struct drm_encoder_funcs hibmc_encoder_encoder_funcs = {
> +       .destroy = drm_encoder_cleanup,
> +};
> +
> +int hibmc_encoder_init(struct hibmc_drm_device *hidev)
> +{
> +       struct drm_device *dev = hidev->dev;
> +       struct drm_encoder *encoder = &hidev->encoder;
> +       int ret;
> +
> +       encoder->possible_crtcs = 0x1;
> +       ret = drm_encoder_init(dev, encoder, &hibmc_encoder_encoder_funcs,
> +                              DRM_MODE_ENCODER_DAC, NULL);
> +       if (ret) {
> +               DRM_ERROR("failed to init encoder\n");

print ret

> +               return ret;
> +       }
> +
> +       drm_encoder_helper_add(encoder, &hibmc_encoder_helper_funcs);
> +       return 0;
> +}
> --
> 1.9.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Rongrong Zou Nov. 12, 2016, 10:36 a.m. UTC | #2
在 2016/11/11 6:20, Sean Paul 写道:
> On Fri, Oct 28, 2016 at 3:27 AM, Rongrong Zou <zourongrong@gmail.com> wrote:
>> Add encoder funcs and helpers for VDAC.
>>
>> Signed-off-by: Rongrong Zou <zourongrong@gmail.com>
>> ---
>>   drivers/gpu/drm/hisilicon/hibmc/Makefile         |  2 +-
>>   drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c  |  6 ++
>>   drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h  |  2 +
>>   drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c | 89 ++++++++++++++++++++++++
>>   4 files changed, 98 insertions(+), 1 deletion(-)
>>   create mode 100644 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
>>
>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/Makefile b/drivers/gpu/drm/hisilicon/hibmc/Makefile
>> index 72e107e..e04f114 100644
>> --- a/drivers/gpu/drm/hisilicon/hibmc/Makefile
>> +++ b/drivers/gpu/drm/hisilicon/hibmc/Makefile
>> @@ -1,5 +1,5 @@
>>   ccflags-y := -Iinclude/drm
>> -hibmc-drm-y := hibmc_drm_drv.o hibmc_drm_de.o hibmc_drm_fbdev.o hibmc_drm_power.o hibmc_ttm.o
>> +hibmc-drm-y := hibmc_drm_drv.o hibmc_drm_de.o hibmc_drm_vdac.o hibmc_drm_fbdev.o hibmc_drm_power.o hibmc_ttm.o
>>
>>   obj-$(CONFIG_DRM_HISI_HIBMC)   +=hibmc-drm.o
>>   #obj-y += hibmc-drm.o
>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
>> index 303cd36..ba191e1 100644
>> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
>> @@ -125,6 +125,12 @@ static int hibmc_kms_init(struct hibmc_drm_device *hidev)
>>                  return ret;
>>          }
>>
>> +       ret = hibmc_encoder_init(hidev);
>> +       if (ret) {
>> +               DRM_ERROR("failed to init encoder\n");
>> +               return ret;
>> +       }
>> +
>>          return 0;
>>   }
>>
>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
>> index 5731ec2..401cea4 100644
>> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
>> @@ -47,6 +47,7 @@ struct hibmc_drm_device {
>>          struct drm_device  *dev;
>>          struct drm_plane plane;
>>          struct drm_crtc crtc;
>> +       struct drm_encoder encoder;
>
> Same comment here, you don't need to keep track of this

ok, it can be dealt with like crtc.

>
>>          bool mode_config_initialized;
>>
>>          /* ttm */
>> @@ -87,6 +88,7 @@ static inline struct hibmc_bo *gem_to_hibmc_bo(struct drm_gem_object *gem)
>>
>>   int hibmc_plane_init(struct hibmc_drm_device *hidev);
>>   int hibmc_crtc_init(struct hibmc_drm_device *hidev);
>> +int hibmc_encoder_init(struct hibmc_drm_device *hidev);
>>   int hibmc_fbdev_init(struct hibmc_drm_device *hidev);
>>   void hibmc_fbdev_fini(struct hibmc_drm_device *hidev);
>>
>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
>> new file mode 100644
>> index 0000000..953f659
>> --- /dev/null
>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
>> @@ -0,0 +1,89 @@
>> +/* Hisilicon Hibmc SoC drm driver
>> + *
>> + * Based on the bochs drm driver.
>> + *
>> + * Copyright (c) 2016 Huawei Limited.
>> + *
>> + * Author:
>> + *     Rongrong Zou <zourongrong@huawei.com>
>> + *     Rongrong Zou <zourongrong@gmail.com>
>> + *     Jianhua Li <lijianhua@huawei.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + */
>> +
>> +#include <drm/drm_atomic_helper.h>
>> +#include <drm/drm_crtc_helper.h>
>> +
>> +#include "hibmc_drm_drv.h"
>> +#include "hibmc_drm_regs.h"
>> +
>> +static int defx = 800;
>> +static int defy = 600;
>> +
>> +module_param(defx, int, 0444);
>> +module_param(defy, int, 0444);
>> +MODULE_PARM_DESC(defx, "default x resolution");
>> +MODULE_PARM_DESC(defy, "default y resolution");
>
> Not used, and I'm not sure these are a good idea

it is used in following patch, i think it is put in wrong place.

>
>> +
>> +static void hibmc_encoder_disable(struct drm_encoder *encoder)
>> +{
>> +}
>> +
>> +static void hibmc_encoder_enable(struct drm_encoder *encoder)
>> +{
>> +}
>
> Null-checked, no need to stub

thanks for pointing it out.

>
>> +
>> +static void hibmc_encoder_mode_set(struct drm_encoder *encoder,
>> +                                  struct drm_display_mode *mode,
>> +                                  struct drm_display_mode *adj_mode)
>> +{
>> +       u32 reg;
>> +       struct drm_device *dev = encoder->dev;
>> +       struct hibmc_drm_device *hidev = dev->dev_private;
>> +
>> +       /* just open DISPLAY_CONTROL_HISILE register bit 3:0*/
>> +       reg = readl(hidev->mmio + DISPLAY_CONTROL_HISILE);
>> +       reg |= 0xf;
>
> Can you just pull this into a #define instead of explaining in the comment?

ok, thanks.

>
>> +       writel(reg, hidev->mmio + DISPLAY_CONTROL_HISILE);
>> +}
>> +
>> +static int hibmc_encoder_atomic_check(struct drm_encoder *encoder,
>> +                                     struct drm_crtc_state *crtc_state,
>> +                                     struct drm_connector_state *conn_state)
>> +{
>> +       return 0;
>> +}
>
> null-checked, remove stub

ok, will do.

>
>> +
>> +static const struct drm_encoder_helper_funcs hibmc_encoder_helper_funcs = {
>> +       .mode_set = hibmc_encoder_mode_set,
>> +       .disable = hibmc_encoder_disable,
>> +       .enable = hibmc_encoder_enable,
>> +       .atomic_check = hibmc_encoder_atomic_check,
>> +};
>> +
>> +static const struct drm_encoder_funcs hibmc_encoder_encoder_funcs = {
>> +       .destroy = drm_encoder_cleanup,
>> +};
>> +
>> +int hibmc_encoder_init(struct hibmc_drm_device *hidev)
>> +{
>> +       struct drm_device *dev = hidev->dev;
>> +       struct drm_encoder *encoder = &hidev->encoder;
>> +       int ret;
>> +
>> +       encoder->possible_crtcs = 0x1;
>> +       ret = drm_encoder_init(dev, encoder, &hibmc_encoder_encoder_funcs,
>> +                              DRM_MODE_ENCODER_DAC, NULL);
>> +       if (ret) {
>> +               DRM_ERROR("failed to init encoder\n");
>
> print ret

will do, thanks.

>
>> +               return ret;
>> +       }
>> +
>> +       drm_encoder_helper_add(encoder, &hibmc_encoder_helper_funcs);
>> +       return 0;
>> +}
>> --
>> 1.9.1
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> _______________________________________________
> linuxarm mailing list
> linuxarm@huawei.com
> http://rnd-openeuler.huawei.com/mailman/listinfo/linuxarm
>
> .
>
diff mbox

Patch

diff --git a/drivers/gpu/drm/hisilicon/hibmc/Makefile b/drivers/gpu/drm/hisilicon/hibmc/Makefile
index 72e107e..e04f114 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/Makefile
+++ b/drivers/gpu/drm/hisilicon/hibmc/Makefile
@@ -1,5 +1,5 @@ 
 ccflags-y := -Iinclude/drm
-hibmc-drm-y := hibmc_drm_drv.o hibmc_drm_de.o hibmc_drm_fbdev.o hibmc_drm_power.o hibmc_ttm.o
+hibmc-drm-y := hibmc_drm_drv.o hibmc_drm_de.o hibmc_drm_vdac.o hibmc_drm_fbdev.o hibmc_drm_power.o hibmc_ttm.o
 
 obj-$(CONFIG_DRM_HISI_HIBMC)	+=hibmc-drm.o
 #obj-y	+= hibmc-drm.o
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
index 303cd36..ba191e1 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
@@ -125,6 +125,12 @@  static int hibmc_kms_init(struct hibmc_drm_device *hidev)
 		return ret;
 	}
 
+	ret = hibmc_encoder_init(hidev);
+	if (ret) {
+		DRM_ERROR("failed to init encoder\n");
+		return ret;
+	}
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
index 5731ec2..401cea4 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
@@ -47,6 +47,7 @@  struct hibmc_drm_device {
 	struct drm_device  *dev;
 	struct drm_plane plane;
 	struct drm_crtc crtc;
+	struct drm_encoder encoder;
 	bool mode_config_initialized;
 
 	/* ttm */
@@ -87,6 +88,7 @@  static inline struct hibmc_bo *gem_to_hibmc_bo(struct drm_gem_object *gem)
 
 int hibmc_plane_init(struct hibmc_drm_device *hidev);
 int hibmc_crtc_init(struct hibmc_drm_device *hidev);
+int hibmc_encoder_init(struct hibmc_drm_device *hidev);
 int hibmc_fbdev_init(struct hibmc_drm_device *hidev);
 void hibmc_fbdev_fini(struct hibmc_drm_device *hidev);
 
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
new file mode 100644
index 0000000..953f659
--- /dev/null
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
@@ -0,0 +1,89 @@ 
+/* Hisilicon Hibmc SoC drm driver
+ *
+ * Based on the bochs drm driver.
+ *
+ * Copyright (c) 2016 Huawei Limited.
+ *
+ * Author:
+ *	Rongrong Zou <zourongrong@huawei.com>
+ *	Rongrong Zou <zourongrong@gmail.com>
+ *	Jianhua Li <lijianhua@huawei.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ */
+
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_crtc_helper.h>
+
+#include "hibmc_drm_drv.h"
+#include "hibmc_drm_regs.h"
+
+static int defx = 800;
+static int defy = 600;
+
+module_param(defx, int, 0444);
+module_param(defy, int, 0444);
+MODULE_PARM_DESC(defx, "default x resolution");
+MODULE_PARM_DESC(defy, "default y resolution");
+
+static void hibmc_encoder_disable(struct drm_encoder *encoder)
+{
+}
+
+static void hibmc_encoder_enable(struct drm_encoder *encoder)
+{
+}
+
+static void hibmc_encoder_mode_set(struct drm_encoder *encoder,
+				   struct drm_display_mode *mode,
+				   struct drm_display_mode *adj_mode)
+{
+	u32 reg;
+	struct drm_device *dev = encoder->dev;
+	struct hibmc_drm_device *hidev = dev->dev_private;
+
+	/* just open DISPLAY_CONTROL_HISILE register bit 3:0*/
+	reg = readl(hidev->mmio + DISPLAY_CONTROL_HISILE);
+	reg |= 0xf;
+	writel(reg, hidev->mmio + DISPLAY_CONTROL_HISILE);
+}
+
+static int hibmc_encoder_atomic_check(struct drm_encoder *encoder,
+				      struct drm_crtc_state *crtc_state,
+				      struct drm_connector_state *conn_state)
+{
+	return 0;
+}
+
+static const struct drm_encoder_helper_funcs hibmc_encoder_helper_funcs = {
+	.mode_set = hibmc_encoder_mode_set,
+	.disable = hibmc_encoder_disable,
+	.enable = hibmc_encoder_enable,
+	.atomic_check = hibmc_encoder_atomic_check,
+};
+
+static const struct drm_encoder_funcs hibmc_encoder_encoder_funcs = {
+	.destroy = drm_encoder_cleanup,
+};
+
+int hibmc_encoder_init(struct hibmc_drm_device *hidev)
+{
+	struct drm_device *dev = hidev->dev;
+	struct drm_encoder *encoder = &hidev->encoder;
+	int ret;
+
+	encoder->possible_crtcs = 0x1;
+	ret = drm_encoder_init(dev, encoder, &hibmc_encoder_encoder_funcs,
+			       DRM_MODE_ENCODER_DAC, NULL);
+	if (ret) {
+		DRM_ERROR("failed to init encoder\n");
+		return ret;
+	}
+
+	drm_encoder_helper_add(encoder, &hibmc_encoder_helper_funcs);
+	return 0;
+}