diff mbox series

[01/22] drm/arc: Use simple encoder

Message ID 20200305155950.2705-2-tzimmermann@suse.de (mailing list archive)
State New, archived
Headers show
Series drm: Convert drivers to drm_simple_encoder_init() | expand

Commit Message

Thomas Zimmermann March 5, 2020, 3:59 p.m. UTC 
The arc driver uses empty implementations for its encoders. Replace
the code with the generic simple encoder.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/arc/arcpgu_hdmi.c | 10 +++-------
 drivers/gpu/drm/arc/arcpgu_sim.c  |  8 ++------
 2 files changed, 5 insertions(+), 13 deletions(-)

Comments

kernel test robot March 5, 2020, 11:48 p.m. UTC | #1 
Hi Thomas,

I love your patch! Yet something to improve:

[auto build test ERROR on next-20200305]
[also build test ERROR on v5.6-rc4]
[cannot apply to rockchip/for-next shawnguo/for-next sunxi/sunxi/for-next tegra/for-next linus/master v5.6-rc4 v5.6-rc3 v5.6-rc2]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Thomas-Zimmermann/drm-Convert-drivers-to-drm_simple_encoder_init/20200306-045931
base:    47466dcf84ee66a973ea7d2fca7e582fe9328932
config: mips-randconfig-a001-20200306 (attached as .config)
compiler: mipsel-linux-gcc (GCC) 5.5.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=5.5.0 make.cross ARCH=mips 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/gpu/drm/arc/arcpgu_hdmi.c: In function 'arcpgu_drm_hdmi_init':
>> drivers/gpu/drm/arc/arcpgu_hdmi.c:34:8: error: implicit declaration of function 'drm_simple_encoder_init' [-Werror=implicit-function-declaration]
     ret = drm_simple_encoder_init(drm, encoder, DRM_MODE_ENCODER_TMDS);
           ^
   cc1: some warnings being treated as errors
--
   drivers/gpu/drm/arc/arcpgu_sim.c: In function 'arcpgu_drm_sim_init':
>> drivers/gpu/drm/arc/arcpgu_sim.c:68:8: error: implicit declaration of function 'drm_simple_encoder_init' [-Werror=implicit-function-declaration]
     ret = drm_simple_encoder_init(drm, encoder, DRM_MODE_ENCODER_VIRTUAL);
           ^
   cc1: some warnings being treated as errors

vim +/drm_simple_encoder_init +34 drivers/gpu/drm/arc/arcpgu_hdmi.c

    15	
    16	int arcpgu_drm_hdmi_init(struct drm_device *drm, struct device_node *np)
    17	{
    18		struct drm_encoder *encoder;
    19		struct drm_bridge *bridge;
    20	
    21		int ret = 0;
    22	
    23		encoder = devm_kzalloc(drm->dev, sizeof(*encoder), GFP_KERNEL);
    24		if (encoder == NULL)
    25			return -ENOMEM;
    26	
    27		/* Locate drm bridge from the hdmi encoder DT node */
    28		bridge = of_drm_find_bridge(np);
    29		if (!bridge)
    30			return -EPROBE_DEFER;
    31	
    32		encoder->possible_crtcs = 1;
    33		encoder->possible_clones = 0;
  > 34		ret = drm_simple_encoder_init(drm, encoder, DRM_MODE_ENCODER_TMDS);

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Sam Ravnborg March 6, 2020, 9:18 p.m. UTC | #2 
On Thu, Mar 05, 2020 at 04:59:29PM +0100, Thomas Zimmermann wrote:
> The arc driver uses empty implementations for its encoders. Replace
> the code with the generic simple encoder.

We should , as a follow-up patch, embed the encoder in
arcgpu_drm_private.
Then we drop the kzalloc() and avoid that life-time challenge.

This patch looks good for what it does.

Acked-by: Sam Ravnborg <sam@ravnborg.org>

> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>  drivers/gpu/drm/arc/arcpgu_hdmi.c | 10 +++-------
>  drivers/gpu/drm/arc/arcpgu_sim.c  |  8 ++------
>  2 files changed, 5 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/arc/arcpgu_hdmi.c b/drivers/gpu/drm/arc/arcpgu_hdmi.c
> index 52839934f2fb..780911765e2e 100644
> --- a/drivers/gpu/drm/arc/arcpgu_hdmi.c
> +++ b/drivers/gpu/drm/arc/arcpgu_hdmi.c
> @@ -7,15 +7,12 @@
>  
>  #include <drm/drm_bridge.h>
>  #include <drm/drm_crtc.h>
> -#include <drm/drm_encoder.h>
>  #include <drm/drm_device.h>
> +#include <drm/drm_encoder.h>
> +#include <drm/drm_simple_kms_helper.h>
>  
>  #include "arcpgu.h"
>  
> -static struct drm_encoder_funcs arcpgu_drm_encoder_funcs = {
> -	.destroy = drm_encoder_cleanup,
> -};
> -
>  int arcpgu_drm_hdmi_init(struct drm_device *drm, struct device_node *np)
>  {
>  	struct drm_encoder *encoder;
> @@ -34,8 +31,7 @@ int arcpgu_drm_hdmi_init(struct drm_device *drm, struct device_node *np)
>  
>  	encoder->possible_crtcs = 1;
>  	encoder->possible_clones = 0;
> -	ret = drm_encoder_init(drm, encoder, &arcpgu_drm_encoder_funcs,
> -			       DRM_MODE_ENCODER_TMDS, NULL);
> +	ret = drm_simple_encoder_init(drm, encoder, DRM_MODE_ENCODER_TMDS);
>  	if (ret)
>  		return ret;
>  
> diff --git a/drivers/gpu/drm/arc/arcpgu_sim.c b/drivers/gpu/drm/arc/arcpgu_sim.c
> index 37d961668dfe..66ca2c26e339 100644
> --- a/drivers/gpu/drm/arc/arcpgu_sim.c
> +++ b/drivers/gpu/drm/arc/arcpgu_sim.c
> @@ -8,6 +8,7 @@
>  #include <drm/drm_atomic_helper.h>
>  #include <drm/drm_device.h>
>  #include <drm/drm_probe_helper.h>
> +#include <drm/drm_simple_kms_helper.h>
>  
>  #include "arcpgu.h"
>  
> @@ -50,10 +51,6 @@ static const struct drm_connector_funcs arcpgu_drm_connector_funcs = {
>  	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>  };
>  
> -static struct drm_encoder_funcs arcpgu_drm_encoder_funcs = {
> -	.destroy = drm_encoder_cleanup,
> -};
> -
>  int arcpgu_drm_sim_init(struct drm_device *drm, struct device_node *np)
>  {
>  	struct arcpgu_drm_connector *arcpgu_connector;
> @@ -68,8 +65,7 @@ int arcpgu_drm_sim_init(struct drm_device *drm, struct device_node *np)
>  	encoder->possible_crtcs = 1;
>  	encoder->possible_clones = 0;
>  
> -	ret = drm_encoder_init(drm, encoder, &arcpgu_drm_encoder_funcs,
> -			       DRM_MODE_ENCODER_VIRTUAL, NULL);
> +	ret = drm_simple_encoder_init(drm, encoder, DRM_MODE_ENCODER_VIRTUAL);
>  	if (ret)
>  		return ret;
>  
> -- 
> 2.25.1
Thomas Zimmermann March 9, 2020, 7:55 a.m. UTC | #3 
Hi Sam

Am 06.03.20 um 22:18 schrieb Sam Ravnborg:
> On Thu, Mar 05, 2020 at 04:59:29PM +0100, Thomas Zimmermann wrote:
>> The arc driver uses empty implementations for its encoders. Replace
>> the code with the generic simple encoder.
> 
> We should , as a follow-up patch, embed the encoder in
> arcgpu_drm_private.
> Then we drop the kzalloc() and avoid that life-time challenge.

You're right, there's a devm_kzalloc() for the encoder. I didn't notice
before. And from what I learned from the drmm_ patches, this doesn't
work reliably.

I'll drop this patch, as the series is supposed to handle embedded
encoders. arcgpu will be fixed later when the managed API is ready.

Best regards
Thomas

> 
> This patch looks good for what it does.
> 
> Acked-by: Sam Ravnborg <sam@ravnborg.org>
> 
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
>>  drivers/gpu/drm/arc/arcpgu_hdmi.c | 10 +++-------
>>  drivers/gpu/drm/arc/arcpgu_sim.c  |  8 ++------
>>  2 files changed, 5 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/arc/arcpgu_hdmi.c b/drivers/gpu/drm/arc/arcpgu_hdmi.c
>> index 52839934f2fb..780911765e2e 100644
>> --- a/drivers/gpu/drm/arc/arcpgu_hdmi.c
>> +++ b/drivers/gpu/drm/arc/arcpgu_hdmi.c
>> @@ -7,15 +7,12 @@
>>  
>>  #include <drm/drm_bridge.h>
>>  #include <drm/drm_crtc.h>
>> -#include <drm/drm_encoder.h>
>>  #include <drm/drm_device.h>
>> +#include <drm/drm_encoder.h>
>> +#include <drm/drm_simple_kms_helper.h>
>>  
>>  #include "arcpgu.h"
>>  
>> -static struct drm_encoder_funcs arcpgu_drm_encoder_funcs = {
>> -	.destroy = drm_encoder_cleanup,
>> -};
>> -
>>  int arcpgu_drm_hdmi_init(struct drm_device *drm, struct device_node *np)
>>  {
>>  	struct drm_encoder *encoder;
>> @@ -34,8 +31,7 @@ int arcpgu_drm_hdmi_init(struct drm_device *drm, struct device_node *np)
>>  
>>  	encoder->possible_crtcs = 1;
>>  	encoder->possible_clones = 0;
>> -	ret = drm_encoder_init(drm, encoder, &arcpgu_drm_encoder_funcs,
>> -			       DRM_MODE_ENCODER_TMDS, NULL);
>> +	ret = drm_simple_encoder_init(drm, encoder, DRM_MODE_ENCODER_TMDS);
>>  	if (ret)
>>  		return ret;
>>  
>> diff --git a/drivers/gpu/drm/arc/arcpgu_sim.c b/drivers/gpu/drm/arc/arcpgu_sim.c
>> index 37d961668dfe..66ca2c26e339 100644
>> --- a/drivers/gpu/drm/arc/arcpgu_sim.c
>> +++ b/drivers/gpu/drm/arc/arcpgu_sim.c
>> @@ -8,6 +8,7 @@
>>  #include <drm/drm_atomic_helper.h>
>>  #include <drm/drm_device.h>
>>  #include <drm/drm_probe_helper.h>
>> +#include <drm/drm_simple_kms_helper.h>
>>  
>>  #include "arcpgu.h"
>>  
>> @@ -50,10 +51,6 @@ static const struct drm_connector_funcs arcpgu_drm_connector_funcs = {
>>  	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>>  };
>>  
>> -static struct drm_encoder_funcs arcpgu_drm_encoder_funcs = {
>> -	.destroy = drm_encoder_cleanup,
>> -};
>> -
>>  int arcpgu_drm_sim_init(struct drm_device *drm, struct device_node *np)
>>  {
>>  	struct arcpgu_drm_connector *arcpgu_connector;
>> @@ -68,8 +65,7 @@ int arcpgu_drm_sim_init(struct drm_device *drm, struct device_node *np)
>>  	encoder->possible_crtcs = 1;
>>  	encoder->possible_clones = 0;
>>  
>> -	ret = drm_encoder_init(drm, encoder, &arcpgu_drm_encoder_funcs,
>> -			       DRM_MODE_ENCODER_VIRTUAL, NULL);
>> +	ret = drm_simple_encoder_init(drm, encoder, DRM_MODE_ENCODER_VIRTUAL);
>>  	if (ret)
>>  		return ret;
>>  
>> -- 
>> 2.25.1
diff mbox series

Patch

diff --git a/drivers/gpu/drm/arc/arcpgu_hdmi.c b/drivers/gpu/drm/arc/arcpgu_hdmi.c
index 52839934f2fb..780911765e2e 100644
--- a/drivers/gpu/drm/arc/arcpgu_hdmi.c
+++ b/drivers/gpu/drm/arc/arcpgu_hdmi.c
@@ -7,15 +7,12 @@ 
 
 #include <drm/drm_bridge.h>
 #include <drm/drm_crtc.h>
-#include <drm/drm_encoder.h>
 #include <drm/drm_device.h>
+#include <drm/drm_encoder.h>
+#include <drm/drm_simple_kms_helper.h>
 
 #include "arcpgu.h"
 
-static struct drm_encoder_funcs arcpgu_drm_encoder_funcs = {
-	.destroy = drm_encoder_cleanup,
-};
-
 int arcpgu_drm_hdmi_init(struct drm_device *drm, struct device_node *np)
 {
 	struct drm_encoder *encoder;
@@ -34,8 +31,7 @@  int arcpgu_drm_hdmi_init(struct drm_device *drm, struct device_node *np)
 
 	encoder->possible_crtcs = 1;
 	encoder->possible_clones = 0;
-	ret = drm_encoder_init(drm, encoder, &arcpgu_drm_encoder_funcs,
-			       DRM_MODE_ENCODER_TMDS, NULL);
+	ret = drm_simple_encoder_init(drm, encoder, DRM_MODE_ENCODER_TMDS);
 	if (ret)
 		return ret;
 
diff --git a/drivers/gpu/drm/arc/arcpgu_sim.c b/drivers/gpu/drm/arc/arcpgu_sim.c
index 37d961668dfe..66ca2c26e339 100644
--- a/drivers/gpu/drm/arc/arcpgu_sim.c
+++ b/drivers/gpu/drm/arc/arcpgu_sim.c
@@ -8,6 +8,7 @@ 
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_device.h>
 #include <drm/drm_probe_helper.h>
+#include <drm/drm_simple_kms_helper.h>
 
 #include "arcpgu.h"
 
@@ -50,10 +51,6 @@  static const struct drm_connector_funcs arcpgu_drm_connector_funcs = {
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
 };
 
-static struct drm_encoder_funcs arcpgu_drm_encoder_funcs = {
-	.destroy = drm_encoder_cleanup,
-};
-
 int arcpgu_drm_sim_init(struct drm_device *drm, struct device_node *np)
 {
 	struct arcpgu_drm_connector *arcpgu_connector;
@@ -68,8 +65,7 @@  int arcpgu_drm_sim_init(struct drm_device *drm, struct device_node *np)
 	encoder->possible_crtcs = 1;
 	encoder->possible_clones = 0;
 
-	ret = drm_encoder_init(drm, encoder, &arcpgu_drm_encoder_funcs,
-			       DRM_MODE_ENCODER_VIRTUAL, NULL);
+	ret = drm_simple_encoder_init(drm, encoder, DRM_MODE_ENCODER_VIRTUAL);
 	if (ret)
 		return ret;