[RFC PATCH] drm: allow passing a real encoder object for wb connector

[RFC PATCH] drm: allow passing a real encoder object for wb connector

[Abhinav Kumar]

Instead of creating an internal encoder for the writeback
connector to satisfy DRM requirements, allow the clients
to pass a real encoder to it by changing the drm_writeback's
encoder to a pointer.

If a real encoder is not passed, drm_writeback_connector_init
will internally allocate one.

This will help the clients to manage the real encoder states
better as they will allocate and maintain the encoder.

Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
---
 drivers/gpu/drm/drm_writeback.c | 11 +++++++----
 include/drm/drm_writeback.h     |  2 +-
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c
index dccf4504..fdb7381 100644
--- a/drivers/gpu/drm/drm_writeback.c
+++ b/drivers/gpu/drm/drm_writeback.c
@@ -189,8 +189,11 @@ int drm_writeback_connector_init(struct drm_device *dev,
 	if (IS_ERR(blob))
 		return PTR_ERR(blob);
 
-	drm_encoder_helper_add(&wb_connector->encoder, enc_helper_funcs);
-	ret = drm_encoder_init(dev, &wb_connector->encoder,
+	/* allocate the internal drm encoder if a real one wasnt passed */
+	if (!wb_connector->encoder)
+		wb_connector->encoder = devm_kzalloc(dev->dev, sizeof(struct drm_encoder), GFP_KERNEL);
+	drm_encoder_helper_add(wb_connector->encoder, enc_helper_funcs);
+	ret = drm_encoder_init(dev, wb_connector->encoder,
 			       &drm_writeback_encoder_funcs,
 			       DRM_MODE_ENCODER_VIRTUAL, NULL);
 	if (ret)
@@ -204,7 +207,7 @@ int drm_writeback_connector_init(struct drm_device *dev,
 		goto connector_fail;
 
 	ret = drm_connector_attach_encoder(connector,
-						&wb_connector->encoder);
+						wb_connector->encoder);
 	if (ret)
 		goto attach_fail;
 
@@ -233,7 +236,7 @@ int drm_writeback_connector_init(struct drm_device *dev,
 attach_fail:
 	drm_connector_cleanup(connector);
 connector_fail:
-	drm_encoder_cleanup(&wb_connector->encoder);
+	drm_encoder_cleanup(wb_connector->encoder);
 fail:
 	drm_property_blob_put(blob);
 	return ret;
diff --git a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h
index 9697d27..f0d8147 100644
--- a/include/drm/drm_writeback.h
+++ b/include/drm/drm_writeback.h
@@ -31,7 +31,7 @@ struct drm_writeback_connector {
 	 * by passing the @enc_funcs parameter to drm_writeback_connector_init()
 	 * function.
 	 */
-	struct drm_encoder encoder;
+	struct drm_encoder *encoder;
 
 	/**
 	 * @pixel_formats_blob_ptr:
-- 
2.7.4

[RFC PATCH] drm: allow passing a real encoder object for wb connector

[Abhinav Kumar]

Instead of creating an internal encoder for the writeback
connector to satisfy DRM requirements, allow the clients
to pass a real encoder to it by changing the drm_writeback's
encoder to a pointer.

If a real encoder is not passed, drm_writeback_connector_init
will internally allocate one.

This will help the clients to manage the real encoder states
better as they will allocate and maintain the encoder.

Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
---
 drivers/gpu/drm/drm_writeback.c | 11 +++++++----
 include/drm/drm_writeback.h     |  2 +-
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c
index dccf4504..fdb7381 100644
--- a/drivers/gpu/drm/drm_writeback.c
+++ b/drivers/gpu/drm/drm_writeback.c
@@ -189,8 +189,11 @@ int drm_writeback_connector_init(struct drm_device *dev,
 	if (IS_ERR(blob))
 		return PTR_ERR(blob);
 
-	drm_encoder_helper_add(&wb_connector->encoder, enc_helper_funcs);
-	ret = drm_encoder_init(dev, &wb_connector->encoder,
+	/* allocate the internal drm encoder if a real one wasnt passed */
+	if (!wb_connector->encoder)
+		wb_connector->encoder = devm_kzalloc(dev->dev, sizeof(struct drm_encoder), GFP_KERNEL);
+	drm_encoder_helper_add(wb_connector->encoder, enc_helper_funcs);
+	ret = drm_encoder_init(dev, wb_connector->encoder,
 			       &drm_writeback_encoder_funcs,
 			       DRM_MODE_ENCODER_VIRTUAL, NULL);
 	if (ret)
@@ -204,7 +207,7 @@ int drm_writeback_connector_init(struct drm_device *dev,
 		goto connector_fail;
 
 	ret = drm_connector_attach_encoder(connector,
-						&wb_connector->encoder);
+						wb_connector->encoder);
 	if (ret)
 		goto attach_fail;
 
@@ -233,7 +236,7 @@ int drm_writeback_connector_init(struct drm_device *dev,
 attach_fail:
 	drm_connector_cleanup(connector);
 connector_fail:
-	drm_encoder_cleanup(&wb_connector->encoder);
+	drm_encoder_cleanup(wb_connector->encoder);
 fail:
 	drm_property_blob_put(blob);
 	return ret;
diff --git a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h
index 9697d27..f0d8147 100644
--- a/include/drm/drm_writeback.h
+++ b/include/drm/drm_writeback.h
@@ -31,7 +31,7 @@ struct drm_writeback_connector {
 	 * by passing the @enc_funcs parameter to drm_writeback_connector_init()
 	 * function.
 	 */
-	struct drm_encoder encoder;
+	struct drm_encoder *encoder;
 
 	/**
 	 * @pixel_formats_blob_ptr:
-- 
2.7.4

Re: [RFC PATCH] drm: allow passing a real encoder object for wb connector

[Laurent Pinchart]

Hi Abhinav,

Thank you for the patch.

On Thu, Jan 20, 2022 at 06:29:55PM -0800, Abhinav Kumar wrote:
> Instead of creating an internal encoder for the writeback
> connector to satisfy DRM requirements, allow the clients
> to pass a real encoder to it by changing the drm_writeback's
> encoder to a pointer.
> 
> If a real encoder is not passed, drm_writeback_connector_init
> will internally allocate one.
> 
> This will help the clients to manage the real encoder states
> better as they will allocate and maintain the encoder.

A writeback connector is a bit of a hack. It was implemented that way to
minimize the extensions to the KMS userspace API for writeback support.
There's no "encoder" there, as there's no real "connector" either. The
only reason we register a drm_encoder in the writeback implementation is
because encoders are exposed to userspace and are thus required (this is
considered a historical mistake that we can't fix anymore). Why do you
thus need to create a "real encoder" ?

> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
> ---
>  drivers/gpu/drm/drm_writeback.c | 11 +++++++----
>  include/drm/drm_writeback.h     |  2 +-
>  2 files changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c
> index dccf4504..fdb7381 100644
> --- a/drivers/gpu/drm/drm_writeback.c
> +++ b/drivers/gpu/drm/drm_writeback.c
> @@ -189,8 +189,11 @@ int drm_writeback_connector_init(struct drm_device *dev,
>  	if (IS_ERR(blob))
>  		return PTR_ERR(blob);
>  
> -	drm_encoder_helper_add(&wb_connector->encoder, enc_helper_funcs);
> -	ret = drm_encoder_init(dev, &wb_connector->encoder,
> +	/* allocate the internal drm encoder if a real one wasnt passed */
> +	if (!wb_connector->encoder)
> +		wb_connector->encoder = devm_kzalloc(dev->dev, sizeof(struct drm_encoder), GFP_KERNEL);
> +	drm_encoder_helper_add(wb_connector->encoder, enc_helper_funcs);
> +	ret = drm_encoder_init(dev, wb_connector->encoder,
>  			       &drm_writeback_encoder_funcs,
>  			       DRM_MODE_ENCODER_VIRTUAL, NULL);
>  	if (ret)
> @@ -204,7 +207,7 @@ int drm_writeback_connector_init(struct drm_device *dev,
>  		goto connector_fail;
>  
>  	ret = drm_connector_attach_encoder(connector,
> -						&wb_connector->encoder);
> +						wb_connector->encoder);
>  	if (ret)
>  		goto attach_fail;
>  
> @@ -233,7 +236,7 @@ int drm_writeback_connector_init(struct drm_device *dev,
>  attach_fail:
>  	drm_connector_cleanup(connector);
>  connector_fail:
> -	drm_encoder_cleanup(&wb_connector->encoder);
> +	drm_encoder_cleanup(wb_connector->encoder);
>  fail:
>  	drm_property_blob_put(blob);
>  	return ret;
> diff --git a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h
> index 9697d27..f0d8147 100644
> --- a/include/drm/drm_writeback.h
> +++ b/include/drm/drm_writeback.h
> @@ -31,7 +31,7 @@ struct drm_writeback_connector {
>  	 * by passing the @enc_funcs parameter to drm_writeback_connector_init()
>  	 * function.
>  	 */
> -	struct drm_encoder encoder;
> +	struct drm_encoder *encoder;
>  
>  	/**
>  	 * @pixel_formats_blob_ptr:

-- 
Regards,

Laurent Pinchart

Re: [RFC PATCH] drm: allow passing a real encoder object for wb connector

[Laurent Pinchart]

Hi Abhinav,

Thank you for the patch.

On Thu, Jan 20, 2022 at 06:29:55PM -0800, Abhinav Kumar wrote:
> Instead of creating an internal encoder for the writeback
> connector to satisfy DRM requirements, allow the clients
> to pass a real encoder to it by changing the drm_writeback's
> encoder to a pointer.
> 
> If a real encoder is not passed, drm_writeback_connector_init
> will internally allocate one.
> 
> This will help the clients to manage the real encoder states
> better as they will allocate and maintain the encoder.

A writeback connector is a bit of a hack. It was implemented that way to
minimize the extensions to the KMS userspace API for writeback support.
There's no "encoder" there, as there's no real "connector" either. The
only reason we register a drm_encoder in the writeback implementation is
because encoders are exposed to userspace and are thus required (this is
considered a historical mistake that we can't fix anymore). Why do you
thus need to create a "real encoder" ?

> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
> ---
>  drivers/gpu/drm/drm_writeback.c | 11 +++++++----
>  include/drm/drm_writeback.h     |  2 +-
>  2 files changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c
> index dccf4504..fdb7381 100644
> --- a/drivers/gpu/drm/drm_writeback.c
> +++ b/drivers/gpu/drm/drm_writeback.c
> @@ -189,8 +189,11 @@ int drm_writeback_connector_init(struct drm_device *dev,
>  	if (IS_ERR(blob))
>  		return PTR_ERR(blob);
>  
> -	drm_encoder_helper_add(&wb_connector->encoder, enc_helper_funcs);
> -	ret = drm_encoder_init(dev, &wb_connector->encoder,
> +	/* allocate the internal drm encoder if a real one wasnt passed */
> +	if (!wb_connector->encoder)
> +		wb_connector->encoder = devm_kzalloc(dev->dev, sizeof(struct drm_encoder), GFP_KERNEL);
> +	drm_encoder_helper_add(wb_connector->encoder, enc_helper_funcs);
> +	ret = drm_encoder_init(dev, wb_connector->encoder,
>  			       &drm_writeback_encoder_funcs,
>  			       DRM_MODE_ENCODER_VIRTUAL, NULL);
>  	if (ret)
> @@ -204,7 +207,7 @@ int drm_writeback_connector_init(struct drm_device *dev,
>  		goto connector_fail;
>  
>  	ret = drm_connector_attach_encoder(connector,
> -						&wb_connector->encoder);
> +						wb_connector->encoder);
>  	if (ret)
>  		goto attach_fail;
>  
> @@ -233,7 +236,7 @@ int drm_writeback_connector_init(struct drm_device *dev,
>  attach_fail:
>  	drm_connector_cleanup(connector);
>  connector_fail:
> -	drm_encoder_cleanup(&wb_connector->encoder);
> +	drm_encoder_cleanup(wb_connector->encoder);
>  fail:
>  	drm_property_blob_put(blob);
>  	return ret;
> diff --git a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h
> index 9697d27..f0d8147 100644
> --- a/include/drm/drm_writeback.h
> +++ b/include/drm/drm_writeback.h
> @@ -31,7 +31,7 @@ struct drm_writeback_connector {
>  	 * by passing the @enc_funcs parameter to drm_writeback_connector_init()
>  	 * function.
>  	 */
> -	struct drm_encoder encoder;
> +	struct drm_encoder *encoder;
>  
>  	/**
>  	 * @pixel_formats_blob_ptr:

-- 
Regards,

Laurent Pinchart

Re: [Freedreno] [RFC PATCH] drm: allow passing a real encoder object for wb connector

[Abhinav Kumar]

Hi Laurent

Thanks for the response.

On 1/20/2022 6:43 PM, Laurent Pinchart wrote:
> Hi Abhinav,
> 
> Thank you for the patch.
> 
> On Thu, Jan 20, 2022 at 06:29:55PM -0800, Abhinav Kumar wrote:
>> Instead of creating an internal encoder for the writeback
>> connector to satisfy DRM requirements, allow the clients
>> to pass a real encoder to it by changing the drm_writeback's
>> encoder to a pointer.
>>
>> If a real encoder is not passed, drm_writeback_connector_init
>> will internally allocate one.
>>
>> This will help the clients to manage the real encoder states
>> better as they will allocate and maintain the encoder.
> 
> A writeback connector is a bit of a hack. It was implemented that way to
> minimize the extensions to the KMS userspace API for writeback support.
> There's no "encoder" there, as there's no real "connector" either. The
> only reason we register a drm_encoder in the writeback implementation is
> because encoders are exposed to userspace and are thus required (this is
> considered a historical mistake that we can't fix anymore). Why do you
> thus need to create a "real encoder" ?

On some hardware, it is possible that the writeback encoder is shared.
That is, in terms of hardware resources, we can only mutually drive 
either the physical interface or the writeback one.

In that case, it would be better that drm_writeback accepts the real 
encoder that is being used instead of allocating a dummy one internally.

Moreover, the drm_writeback_connector_init() does already accept passing 
our own enc_helper_funcs to perform necessary checks on the internal 
encoder.

These hooks are provided to perform various operations on the encoder to
fit the respective needs.

In that case why shouldnt the writeback have its own real encoder?

Thanks

Abhinav


> 
>> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
>> ---
>>   drivers/gpu/drm/drm_writeback.c | 11 +++++++----
>>   include/drm/drm_writeback.h     |  2 +-
>>   2 files changed, 8 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c
>> index dccf4504..fdb7381 100644
>> --- a/drivers/gpu/drm/drm_writeback.c
>> +++ b/drivers/gpu/drm/drm_writeback.c
>> @@ -189,8 +189,11 @@ int drm_writeback_connector_init(struct drm_device *dev,
>>   	if (IS_ERR(blob))
>>   		return PTR_ERR(blob);
>>   
>> -	drm_encoder_helper_add(&wb_connector->encoder, enc_helper_funcs);
>> -	ret = drm_encoder_init(dev, &wb_connector->encoder,
>> +	/* allocate the internal drm encoder if a real one wasnt passed */
>> +	if (!wb_connector->encoder)
>> +		wb_connector->encoder = devm_kzalloc(dev->dev, sizeof(struct drm_encoder), GFP_KERNEL);
>> +	drm_encoder_helper_add(wb_connector->encoder, enc_helper_funcs);
>> +	ret = drm_encoder_init(dev, wb_connector->encoder,
>>   			       &drm_writeback_encoder_funcs,
>>   			       DRM_MODE_ENCODER_VIRTUAL, NULL);
>>   	if (ret)
>> @@ -204,7 +207,7 @@ int drm_writeback_connector_init(struct drm_device *dev,
>>   		goto connector_fail;
>>   
>>   	ret = drm_connector_attach_encoder(connector,
>> -						&wb_connector->encoder);
>> +						wb_connector->encoder);
>>   	if (ret)
>>   		goto attach_fail;
>>   
>> @@ -233,7 +236,7 @@ int drm_writeback_connector_init(struct drm_device *dev,
>>   attach_fail:
>>   	drm_connector_cleanup(connector);
>>   connector_fail:
>> -	drm_encoder_cleanup(&wb_connector->encoder);
>> +	drm_encoder_cleanup(wb_connector->encoder);
>>   fail:
>>   	drm_property_blob_put(blob);
>>   	return ret;
>> diff --git a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h
>> index 9697d27..f0d8147 100644
>> --- a/include/drm/drm_writeback.h
>> +++ b/include/drm/drm_writeback.h
>> @@ -31,7 +31,7 @@ struct drm_writeback_connector {
>>   	 * by passing the @enc_funcs parameter to drm_writeback_connector_init()
>>   	 * function.
>>   	 */
>> -	struct drm_encoder encoder;
>> +	struct drm_encoder *encoder;
>>   
>>   	/**
>>   	 * @pixel_formats_blob_ptr:
> 

Re: [Freedreno] [RFC PATCH] drm: allow passing a real encoder object for wb connector

[Abhinav Kumar]

Hi Laurent

Thanks for the response.

On 1/20/2022 6:43 PM, Laurent Pinchart wrote:
> Hi Abhinav,
> 
> Thank you for the patch.
> 
> On Thu, Jan 20, 2022 at 06:29:55PM -0800, Abhinav Kumar wrote:
>> Instead of creating an internal encoder for the writeback
>> connector to satisfy DRM requirements, allow the clients
>> to pass a real encoder to it by changing the drm_writeback's
>> encoder to a pointer.
>>
>> If a real encoder is not passed, drm_writeback_connector_init
>> will internally allocate one.
>>
>> This will help the clients to manage the real encoder states
>> better as they will allocate and maintain the encoder.
> 
> A writeback connector is a bit of a hack. It was implemented that way to
> minimize the extensions to the KMS userspace API for writeback support.
> There's no "encoder" there, as there's no real "connector" either. The
> only reason we register a drm_encoder in the writeback implementation is
> because encoders are exposed to userspace and are thus required (this is
> considered a historical mistake that we can't fix anymore). Why do you
> thus need to create a "real encoder" ?

On some hardware, it is possible that the writeback encoder is shared.
That is, in terms of hardware resources, we can only mutually drive 
either the physical interface or the writeback one.

In that case, it would be better that drm_writeback accepts the real 
encoder that is being used instead of allocating a dummy one internally.

Moreover, the drm_writeback_connector_init() does already accept passing 
our own enc_helper_funcs to perform necessary checks on the internal 
encoder.

These hooks are provided to perform various operations on the encoder to
fit the respective needs.

In that case why shouldnt the writeback have its own real encoder?

Thanks

Abhinav


> 
>> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
>> ---
>>   drivers/gpu/drm/drm_writeback.c | 11 +++++++----
>>   include/drm/drm_writeback.h     |  2 +-
>>   2 files changed, 8 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c
>> index dccf4504..fdb7381 100644
>> --- a/drivers/gpu/drm/drm_writeback.c
>> +++ b/drivers/gpu/drm/drm_writeback.c
>> @@ -189,8 +189,11 @@ int drm_writeback_connector_init(struct drm_device *dev,
>>   	if (IS_ERR(blob))
>>   		return PTR_ERR(blob);
>>   
>> -	drm_encoder_helper_add(&wb_connector->encoder, enc_helper_funcs);
>> -	ret = drm_encoder_init(dev, &wb_connector->encoder,
>> +	/* allocate the internal drm encoder if a real one wasnt passed */
>> +	if (!wb_connector->encoder)
>> +		wb_connector->encoder = devm_kzalloc(dev->dev, sizeof(struct drm_encoder), GFP_KERNEL);
>> +	drm_encoder_helper_add(wb_connector->encoder, enc_helper_funcs);
>> +	ret = drm_encoder_init(dev, wb_connector->encoder,
>>   			       &drm_writeback_encoder_funcs,
>>   			       DRM_MODE_ENCODER_VIRTUAL, NULL);
>>   	if (ret)
>> @@ -204,7 +207,7 @@ int drm_writeback_connector_init(struct drm_device *dev,
>>   		goto connector_fail;
>>   
>>   	ret = drm_connector_attach_encoder(connector,
>> -						&wb_connector->encoder);
>> +						wb_connector->encoder);
>>   	if (ret)
>>   		goto attach_fail;
>>   
>> @@ -233,7 +236,7 @@ int drm_writeback_connector_init(struct drm_device *dev,
>>   attach_fail:
>>   	drm_connector_cleanup(connector);
>>   connector_fail:
>> -	drm_encoder_cleanup(&wb_connector->encoder);
>> +	drm_encoder_cleanup(wb_connector->encoder);
>>   fail:
>>   	drm_property_blob_put(blob);
>>   	return ret;
>> diff --git a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h
>> index 9697d27..f0d8147 100644
>> --- a/include/drm/drm_writeback.h
>> +++ b/include/drm/drm_writeback.h
>> @@ -31,7 +31,7 @@ struct drm_writeback_connector {
>>   	 * by passing the @enc_funcs parameter to drm_writeback_connector_init()
>>   	 * function.
>>   	 */
>> -	struct drm_encoder encoder;
>> +	struct drm_encoder *encoder;
>>   
>>   	/**
>>   	 * @pixel_formats_blob_ptr:
> 

Re: [RFC PATCH] drm: allow passing a real encoder object for wb connector

[kernel test robot]

Hi Abhinav,

[FYI, it's a private test report for your RFC patch.]
[auto build test ERROR on drm/drm-next]
[also build test ERROR on drm-intel/for-linux-next drm-tip/drm-tip drm-exynos/exynos-drm-next tegra-drm/drm/tegra/for-next v5.16 next-20220121]
[cannot apply to airlied/drm-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Abhinav-Kumar/drm-allow-passing-a-real-encoder-object-for-wb-connector/20220121-103231
base:   git://anongit.freedesktop.org/drm/drm drm-next
config: arm64-randconfig-r011-20220120 (https://download.01.org/0day-ci/archive/20220121/202201211543.ZApV4jVl-lkp@intel.com/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project d4baf3b1322b84816aa623d8e8cb45a49cb68b84)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm64 cross compiling tool for clang build
        # apt-get install binutils-aarch64-linux-gnu
        # https://github.com/0day-ci/linux/commit/ce1d81913d9146f6e753c39f41929266a5885f99
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Abhinav-Kumar/drm-allow-passing-a-real-encoder-object-for-wb-connector/20220121-103231
        git checkout ce1d81913d9146f6e753c39f41929266a5885f99
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm64 SHELL=/bin/bash drivers/gpu/drm/arm/ drivers/gpu/drm/rcar-du/

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

All errors (new ones prefixed by >>):

>> drivers/gpu/drm/arm/malidp_mw.c:215:30: error: member reference type 'struct drm_encoder *' is a pointer; did you mean to use '->'?
           malidp->mw_connector.encoder.possible_crtcs = 1 << drm_crtc_index(&malidp->crtc);
           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
                                       ->
   1 error generated.
--
>> drivers/gpu/drm/rcar-du/rcar_du_writeback.c:203:18: error: member reference type 'struct drm_encoder *' is a pointer; did you mean to use '->'?
           wb_conn->encoder.possible_crtcs = 1 << drm_crtc_index(&rcrtc->crtc);
           ~~~~~~~~~~~~~~~~^
                           ->
   1 error generated.
--
>> drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c:158:18: error: member reference type 'struct drm_encoder *' is a pointer; did you mean to use '->'?
           wb_conn->encoder.possible_crtcs = BIT(drm_crtc_index(&kcrtc->base));
           ~~~~~~~~~~~~~~~~^
                           ->
   1 error generated.


vim +215 drivers/gpu/drm/arm/malidp_mw.c

8cbc5caf36ef7a Brian Starkey 2017-11-02  205  
8cbc5caf36ef7a Brian Starkey 2017-11-02  206  int malidp_mw_connector_init(struct drm_device *drm)
8cbc5caf36ef7a Brian Starkey 2017-11-02  207  {
8cbc5caf36ef7a Brian Starkey 2017-11-02  208  	struct malidp_drm *malidp = drm->dev_private;
8cbc5caf36ef7a Brian Starkey 2017-11-02  209  	u32 *formats;
8cbc5caf36ef7a Brian Starkey 2017-11-02  210  	int ret, n_formats;
8cbc5caf36ef7a Brian Starkey 2017-11-02  211  
8cbc5caf36ef7a Brian Starkey 2017-11-02  212  	if (!malidp->dev->hw->enable_memwrite)
8cbc5caf36ef7a Brian Starkey 2017-11-02  213  		return 0;
8cbc5caf36ef7a Brian Starkey 2017-11-02  214  
8cbc5caf36ef7a Brian Starkey 2017-11-02 @215  	malidp->mw_connector.encoder.possible_crtcs = 1 << drm_crtc_index(&malidp->crtc);
8cbc5caf36ef7a Brian Starkey 2017-11-02  216  	drm_connector_helper_add(&malidp->mw_connector.base,
8cbc5caf36ef7a Brian Starkey 2017-11-02  217  				 &malidp_mw_connector_helper_funcs);
8cbc5caf36ef7a Brian Starkey 2017-11-02  218  
8cbc5caf36ef7a Brian Starkey 2017-11-02  219  	formats = get_writeback_formats(malidp, &n_formats);
8cbc5caf36ef7a Brian Starkey 2017-11-02  220  	if (!formats)
8cbc5caf36ef7a Brian Starkey 2017-11-02  221  		return -ENOMEM;
8cbc5caf36ef7a Brian Starkey 2017-11-02  222  
8cbc5caf36ef7a Brian Starkey 2017-11-02  223  	ret = drm_writeback_connector_init(drm, &malidp->mw_connector,
8cbc5caf36ef7a Brian Starkey 2017-11-02  224  					   &malidp_mw_connector_funcs,
8cbc5caf36ef7a Brian Starkey 2017-11-02  225  					   &malidp_mw_encoder_helper_funcs,
8cbc5caf36ef7a Brian Starkey 2017-11-02  226  					   formats, n_formats);
8cbc5caf36ef7a Brian Starkey 2017-11-02  227  	kfree(formats);
8cbc5caf36ef7a Brian Starkey 2017-11-02  228  	if (ret)
8cbc5caf36ef7a Brian Starkey 2017-11-02  229  		return ret;
8cbc5caf36ef7a Brian Starkey 2017-11-02  230  
8cbc5caf36ef7a Brian Starkey 2017-11-02  231  	return 0;
8cbc5caf36ef7a Brian Starkey 2017-11-02  232  }
8cbc5caf36ef7a Brian Starkey 2017-11-02  233  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

Re: [RFC PATCH] drm: allow passing a real encoder object for wb connector

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 5160 bytes --]

Hi Abhinav,

[FYI, it's a private test report for your RFC patch.]
[auto build test ERROR on drm/drm-next]
[also build test ERROR on drm-intel/for-linux-next drm-tip/drm-tip drm-exynos/exynos-drm-next tegra-drm/drm/tegra/for-next v5.16 next-20220121]
[cannot apply to airlied/drm-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Abhinav-Kumar/drm-allow-passing-a-real-encoder-object-for-wb-connector/20220121-103231
base:   git://anongit.freedesktop.org/drm/drm drm-next
config: arm64-randconfig-r011-20220120 (https://download.01.org/0day-ci/archive/20220121/202201211543.ZApV4jVl-lkp(a)intel.com/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project d4baf3b1322b84816aa623d8e8cb45a49cb68b84)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm64 cross compiling tool for clang build
        # apt-get install binutils-aarch64-linux-gnu
        # https://github.com/0day-ci/linux/commit/ce1d81913d9146f6e753c39f41929266a5885f99
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Abhinav-Kumar/drm-allow-passing-a-real-encoder-object-for-wb-connector/20220121-103231
        git checkout ce1d81913d9146f6e753c39f41929266a5885f99
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm64 SHELL=/bin/bash drivers/gpu/drm/arm/ drivers/gpu/drm/rcar-du/

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

All errors (new ones prefixed by >>):

>> drivers/gpu/drm/arm/malidp_mw.c:215:30: error: member reference type 'struct drm_encoder *' is a pointer; did you mean to use '->'?
           malidp->mw_connector.encoder.possible_crtcs = 1 << drm_crtc_index(&malidp->crtc);
           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
                                       ->
   1 error generated.
--
>> drivers/gpu/drm/rcar-du/rcar_du_writeback.c:203:18: error: member reference type 'struct drm_encoder *' is a pointer; did you mean to use '->'?
           wb_conn->encoder.possible_crtcs = 1 << drm_crtc_index(&rcrtc->crtc);
           ~~~~~~~~~~~~~~~~^
                           ->
   1 error generated.
--
>> drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c:158:18: error: member reference type 'struct drm_encoder *' is a pointer; did you mean to use '->'?
           wb_conn->encoder.possible_crtcs = BIT(drm_crtc_index(&kcrtc->base));
           ~~~~~~~~~~~~~~~~^
                           ->
   1 error generated.


vim +215 drivers/gpu/drm/arm/malidp_mw.c

8cbc5caf36ef7a Brian Starkey 2017-11-02  205  
8cbc5caf36ef7a Brian Starkey 2017-11-02  206  int malidp_mw_connector_init(struct drm_device *drm)
8cbc5caf36ef7a Brian Starkey 2017-11-02  207  {
8cbc5caf36ef7a Brian Starkey 2017-11-02  208  	struct malidp_drm *malidp = drm->dev_private;
8cbc5caf36ef7a Brian Starkey 2017-11-02  209  	u32 *formats;
8cbc5caf36ef7a Brian Starkey 2017-11-02  210  	int ret, n_formats;
8cbc5caf36ef7a Brian Starkey 2017-11-02  211  
8cbc5caf36ef7a Brian Starkey 2017-11-02  212  	if (!malidp->dev->hw->enable_memwrite)
8cbc5caf36ef7a Brian Starkey 2017-11-02  213  		return 0;
8cbc5caf36ef7a Brian Starkey 2017-11-02  214  
8cbc5caf36ef7a Brian Starkey 2017-11-02 @215  	malidp->mw_connector.encoder.possible_crtcs = 1 << drm_crtc_index(&malidp->crtc);
8cbc5caf36ef7a Brian Starkey 2017-11-02  216  	drm_connector_helper_add(&malidp->mw_connector.base,
8cbc5caf36ef7a Brian Starkey 2017-11-02  217  				 &malidp_mw_connector_helper_funcs);
8cbc5caf36ef7a Brian Starkey 2017-11-02  218  
8cbc5caf36ef7a Brian Starkey 2017-11-02  219  	formats = get_writeback_formats(malidp, &n_formats);
8cbc5caf36ef7a Brian Starkey 2017-11-02  220  	if (!formats)
8cbc5caf36ef7a Brian Starkey 2017-11-02  221  		return -ENOMEM;
8cbc5caf36ef7a Brian Starkey 2017-11-02  222  
8cbc5caf36ef7a Brian Starkey 2017-11-02  223  	ret = drm_writeback_connector_init(drm, &malidp->mw_connector,
8cbc5caf36ef7a Brian Starkey 2017-11-02  224  					   &malidp_mw_connector_funcs,
8cbc5caf36ef7a Brian Starkey 2017-11-02  225  					   &malidp_mw_encoder_helper_funcs,
8cbc5caf36ef7a Brian Starkey 2017-11-02  226  					   formats, n_formats);
8cbc5caf36ef7a Brian Starkey 2017-11-02  227  	kfree(formats);
8cbc5caf36ef7a Brian Starkey 2017-11-02  228  	if (ret)
8cbc5caf36ef7a Brian Starkey 2017-11-02  229  		return ret;
8cbc5caf36ef7a Brian Starkey 2017-11-02  230  
8cbc5caf36ef7a Brian Starkey 2017-11-02  231  	return 0;
8cbc5caf36ef7a Brian Starkey 2017-11-02  232  }
8cbc5caf36ef7a Brian Starkey 2017-11-02  233  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

Re: [RFC PATCH] drm: allow passing a real encoder object for wb connector

[kernel test robot]

Hi Abhinav,

[FYI, it's a private test report for your RFC patch.]
[auto build test ERROR on drm/drm-next]
[also build test ERROR on drm-intel/for-linux-next drm-tip/drm-tip drm-exynos/exynos-drm-next tegra-drm/drm/tegra/for-next v5.16 next-20220121]
[cannot apply to airlied/drm-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Abhinav-Kumar/drm-allow-passing-a-real-encoder-object-for-wb-connector/20220121-103231
base:   git://anongit.freedesktop.org/drm/drm drm-next
config: hexagon-randconfig-r005-20220120 (https://download.01.org/0day-ci/archive/20220121/202201211623.L8JFrG3B-lkp@intel.com/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project d4baf3b1322b84816aa623d8e8cb45a49cb68b84)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/ce1d81913d9146f6e753c39f41929266a5885f99
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Abhinav-Kumar/drm-allow-passing-a-real-encoder-object-for-wb-connector/20220121-103231
        git checkout ce1d81913d9146f6e753c39f41929266a5885f99
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash drivers/gpu/drm/vkms/

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

All errors (new ones prefixed by >>):

>> drivers/gpu/drm/vkms/vkms_writeback.c:143:38: error: member reference type 'struct drm_encoder *' is a pointer; did you mean to use '->'?
           vkmsdev->output.wb_connector.encoder.possible_crtcs = 1;
           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
                                               ->
   1 error generated.


vim +143 drivers/gpu/drm/vkms/vkms_writeback.c

dbd9d80c1b2e03 Rodrigo Siqueira 2020-08-30  138  
dbd9d80c1b2e03 Rodrigo Siqueira 2020-08-30  139  int vkms_enable_writeback_connector(struct vkms_device *vkmsdev)
dbd9d80c1b2e03 Rodrigo Siqueira 2020-08-30  140  {
dbd9d80c1b2e03 Rodrigo Siqueira 2020-08-30  141  	struct drm_writeback_connector *wb = &vkmsdev->output.wb_connector;
dbd9d80c1b2e03 Rodrigo Siqueira 2020-08-30  142  
dbd9d80c1b2e03 Rodrigo Siqueira 2020-08-30 @143  	vkmsdev->output.wb_connector.encoder.possible_crtcs = 1;

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

Re: [RFC PATCH] drm: allow passing a real encoder object for wb connector

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 2827 bytes --]

Hi Abhinav,

[FYI, it's a private test report for your RFC patch.]
[auto build test ERROR on drm/drm-next]
[also build test ERROR on drm-intel/for-linux-next drm-tip/drm-tip drm-exynos/exynos-drm-next tegra-drm/drm/tegra/for-next v5.16 next-20220121]
[cannot apply to airlied/drm-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Abhinav-Kumar/drm-allow-passing-a-real-encoder-object-for-wb-connector/20220121-103231
base:   git://anongit.freedesktop.org/drm/drm drm-next
config: hexagon-randconfig-r005-20220120 (https://download.01.org/0day-ci/archive/20220121/202201211623.L8JFrG3B-lkp(a)intel.com/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project d4baf3b1322b84816aa623d8e8cb45a49cb68b84)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/ce1d81913d9146f6e753c39f41929266a5885f99
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Abhinav-Kumar/drm-allow-passing-a-real-encoder-object-for-wb-connector/20220121-103231
        git checkout ce1d81913d9146f6e753c39f41929266a5885f99
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash drivers/gpu/drm/vkms/

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

All errors (new ones prefixed by >>):

>> drivers/gpu/drm/vkms/vkms_writeback.c:143:38: error: member reference type 'struct drm_encoder *' is a pointer; did you mean to use '->'?
           vkmsdev->output.wb_connector.encoder.possible_crtcs = 1;
           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
                                               ->
   1 error generated.


vim +143 drivers/gpu/drm/vkms/vkms_writeback.c

dbd9d80c1b2e03 Rodrigo Siqueira 2020-08-30  138  
dbd9d80c1b2e03 Rodrigo Siqueira 2020-08-30  139  int vkms_enable_writeback_connector(struct vkms_device *vkmsdev)
dbd9d80c1b2e03 Rodrigo Siqueira 2020-08-30  140  {
dbd9d80c1b2e03 Rodrigo Siqueira 2020-08-30  141  	struct drm_writeback_connector *wb = &vkmsdev->output.wb_connector;
dbd9d80c1b2e03 Rodrigo Siqueira 2020-08-30  142  
dbd9d80c1b2e03 Rodrigo Siqueira 2020-08-30 @143  	vkmsdev->output.wb_connector.encoder.possible_crtcs = 1;

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

Re: [RFC PATCH] drm: allow passing a real encoder object for wb connector

[Jani Nikula]

On Thu, 20 Jan 2022, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote:
> Instead of creating an internal encoder for the writeback
> connector to satisfy DRM requirements, allow the clients
> to pass a real encoder to it by changing the drm_writeback's
> encoder to a pointer.
>
> If a real encoder is not passed, drm_writeback_connector_init
> will internally allocate one.
>
> This will help the clients to manage the real encoder states
> better as they will allocate and maintain the encoder.

See also the thread starting at [1], and please try to coordinate.

I don't know what the end result should be like, I'm just saying please
collaborate instead of racing to get one set of changes in.

BR,
Jani.


[1] https://patchwork.freedesktop.org/patch/msgid/20220111101801.28310-1-suraj.kandpal@intel.com

>
> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
> ---
>  drivers/gpu/drm/drm_writeback.c | 11 +++++++----
>  include/drm/drm_writeback.h     |  2 +-
>  2 files changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c
> index dccf4504..fdb7381 100644
> --- a/drivers/gpu/drm/drm_writeback.c
> +++ b/drivers/gpu/drm/drm_writeback.c
> @@ -189,8 +189,11 @@ int drm_writeback_connector_init(struct drm_device *dev,
>  	if (IS_ERR(blob))
>  		return PTR_ERR(blob);
>  
> -	drm_encoder_helper_add(&wb_connector->encoder, enc_helper_funcs);
> -	ret = drm_encoder_init(dev, &wb_connector->encoder,
> +	/* allocate the internal drm encoder if a real one wasnt passed */
> +	if (!wb_connector->encoder)
> +		wb_connector->encoder = devm_kzalloc(dev->dev, sizeof(struct drm_encoder), GFP_KERNEL);
> +	drm_encoder_helper_add(wb_connector->encoder, enc_helper_funcs);
> +	ret = drm_encoder_init(dev, wb_connector->encoder,
>  			       &drm_writeback_encoder_funcs,
>  			       DRM_MODE_ENCODER_VIRTUAL, NULL);
>  	if (ret)
> @@ -204,7 +207,7 @@ int drm_writeback_connector_init(struct drm_device *dev,
>  		goto connector_fail;
>  
>  	ret = drm_connector_attach_encoder(connector,
> -						&wb_connector->encoder);
> +						wb_connector->encoder);
>  	if (ret)
>  		goto attach_fail;
>  
> @@ -233,7 +236,7 @@ int drm_writeback_connector_init(struct drm_device *dev,
>  attach_fail:
>  	drm_connector_cleanup(connector);
>  connector_fail:
> -	drm_encoder_cleanup(&wb_connector->encoder);
> +	drm_encoder_cleanup(wb_connector->encoder);
>  fail:
>  	drm_property_blob_put(blob);
>  	return ret;
> diff --git a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h
> index 9697d27..f0d8147 100644
> --- a/include/drm/drm_writeback.h
> +++ b/include/drm/drm_writeback.h
> @@ -31,7 +31,7 @@ struct drm_writeback_connector {
>  	 * by passing the @enc_funcs parameter to drm_writeback_connector_init()
>  	 * function.
>  	 */
> -	struct drm_encoder encoder;
> +	struct drm_encoder *encoder;
>  
>  	/**
>  	 * @pixel_formats_blob_ptr:

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [RFC PATCH] drm: allow passing a real encoder object for wb connector

[Jani Nikula]

On Thu, 20 Jan 2022, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote:
> Instead of creating an internal encoder for the writeback
> connector to satisfy DRM requirements, allow the clients
> to pass a real encoder to it by changing the drm_writeback's
> encoder to a pointer.
>
> If a real encoder is not passed, drm_writeback_connector_init
> will internally allocate one.
>
> This will help the clients to manage the real encoder states
> better as they will allocate and maintain the encoder.

See also the thread starting at [1], and please try to coordinate.

I don't know what the end result should be like, I'm just saying please
collaborate instead of racing to get one set of changes in.

BR,
Jani.


[1] https://patchwork.freedesktop.org/patch/msgid/20220111101801.28310-1-suraj.kandpal@intel.com

>
> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
> ---
>  drivers/gpu/drm/drm_writeback.c | 11 +++++++----
>  include/drm/drm_writeback.h     |  2 +-
>  2 files changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c
> index dccf4504..fdb7381 100644
> --- a/drivers/gpu/drm/drm_writeback.c
> +++ b/drivers/gpu/drm/drm_writeback.c
> @@ -189,8 +189,11 @@ int drm_writeback_connector_init(struct drm_device *dev,
>  	if (IS_ERR(blob))
>  		return PTR_ERR(blob);
>  
> -	drm_encoder_helper_add(&wb_connector->encoder, enc_helper_funcs);
> -	ret = drm_encoder_init(dev, &wb_connector->encoder,
> +	/* allocate the internal drm encoder if a real one wasnt passed */
> +	if (!wb_connector->encoder)
> +		wb_connector->encoder = devm_kzalloc(dev->dev, sizeof(struct drm_encoder), GFP_KERNEL);
> +	drm_encoder_helper_add(wb_connector->encoder, enc_helper_funcs);
> +	ret = drm_encoder_init(dev, wb_connector->encoder,
>  			       &drm_writeback_encoder_funcs,
>  			       DRM_MODE_ENCODER_VIRTUAL, NULL);
>  	if (ret)
> @@ -204,7 +207,7 @@ int drm_writeback_connector_init(struct drm_device *dev,
>  		goto connector_fail;
>  
>  	ret = drm_connector_attach_encoder(connector,
> -						&wb_connector->encoder);
> +						wb_connector->encoder);
>  	if (ret)
>  		goto attach_fail;
>  
> @@ -233,7 +236,7 @@ int drm_writeback_connector_init(struct drm_device *dev,
>  attach_fail:
>  	drm_connector_cleanup(connector);
>  connector_fail:
> -	drm_encoder_cleanup(&wb_connector->encoder);
> +	drm_encoder_cleanup(wb_connector->encoder);
>  fail:
>  	drm_property_blob_put(blob);
>  	return ret;
> diff --git a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h
> index 9697d27..f0d8147 100644
> --- a/include/drm/drm_writeback.h
> +++ b/include/drm/drm_writeback.h
> @@ -31,7 +31,7 @@ struct drm_writeback_connector {
>  	 * by passing the @enc_funcs parameter to drm_writeback_connector_init()
>  	 * function.
>  	 */
> -	struct drm_encoder encoder;
> +	struct drm_encoder *encoder;
>  
>  	/**
>  	 * @pixel_formats_blob_ptr:

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [RFC PATCH] drm: allow passing a real encoder object for wb connector

[Abhinav Kumar]

Hi Jani

On 1/21/2022 1:17 AM, Jani Nikula wrote:
> On Thu, 20 Jan 2022, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote:
>> Instead of creating an internal encoder for the writeback
>> connector to satisfy DRM requirements, allow the clients
>> to pass a real encoder to it by changing the drm_writeback's
>> encoder to a pointer.
>>
>> If a real encoder is not passed, drm_writeback_connector_init
>> will internally allocate one.
>>
>> This will help the clients to manage the real encoder states
>> better as they will allocate and maintain the encoder.
> 
> See also the thread starting at [1], and please try to coordinate.
> 
> I don't know what the end result should be like, I'm just saying please
> collaborate instead of racing to get one set of changes in.
> 
> BR,
> Jani.
> 
> 
> [1] https://patchwork.freedesktop.org/patch/msgid/20220111101801.28310-1-suraj.kandpal@intel.com
> 
Thanks for pointing to this thread. Since 
https://patchwork.freedesktop.org/patch/469090/ has been posted earlier 
and is more complete in terms of handling other vendor changes, we can 
continue on that one.

But I dont see any comments on that one yet.

Hi Laurent

In that case can you please check the 
https://patchwork.freedesktop.org/patch/469090/ thread , we can continue 
our discussion there.

We also have the same issue too. Our encoder also maintains its own 
struct drm_encoder.

Thanks

Abhinav
>>
>> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
>> ---
>>   drivers/gpu/drm/drm_writeback.c | 11 +++++++----
>>   include/drm/drm_writeback.h     |  2 +-
>>   2 files changed, 8 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c
>> index dccf4504..fdb7381 100644
>> --- a/drivers/gpu/drm/drm_writeback.c
>> +++ b/drivers/gpu/drm/drm_writeback.c
>> @@ -189,8 +189,11 @@ int drm_writeback_connector_init(struct drm_device *dev,
>>   	if (IS_ERR(blob))
>>   		return PTR_ERR(blob);
>>   
>> -	drm_encoder_helper_add(&wb_connector->encoder, enc_helper_funcs);
>> -	ret = drm_encoder_init(dev, &wb_connector->encoder,
>> +	/* allocate the internal drm encoder if a real one wasnt passed */
>> +	if (!wb_connector->encoder)
>> +		wb_connector->encoder = devm_kzalloc(dev->dev, sizeof(struct drm_encoder), GFP_KERNEL);
>> +	drm_encoder_helper_add(wb_connector->encoder, enc_helper_funcs);
>> +	ret = drm_encoder_init(dev, wb_connector->encoder,
>>   			       &drm_writeback_encoder_funcs,
>>   			       DRM_MODE_ENCODER_VIRTUAL, NULL);
>>   	if (ret)
>> @@ -204,7 +207,7 @@ int drm_writeback_connector_init(struct drm_device *dev,
>>   		goto connector_fail;
>>   
>>   	ret = drm_connector_attach_encoder(connector,
>> -						&wb_connector->encoder);
>> +						wb_connector->encoder);
>>   	if (ret)
>>   		goto attach_fail;
>>   
>> @@ -233,7 +236,7 @@ int drm_writeback_connector_init(struct drm_device *dev,
>>   attach_fail:
>>   	drm_connector_cleanup(connector);
>>   connector_fail:
>> -	drm_encoder_cleanup(&wb_connector->encoder);
>> +	drm_encoder_cleanup(wb_connector->encoder);
>>   fail:
>>   	drm_property_blob_put(blob);
>>   	return ret;
>> diff --git a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h
>> index 9697d27..f0d8147 100644
>> --- a/include/drm/drm_writeback.h
>> +++ b/include/drm/drm_writeback.h
>> @@ -31,7 +31,7 @@ struct drm_writeback_connector {
>>   	 * by passing the @enc_funcs parameter to drm_writeback_connector_init()
>>   	 * function.
>>   	 */
>> -	struct drm_encoder encoder;
>> +	struct drm_encoder *encoder;
>>   
>>   	/**
>>   	 * @pixel_formats_blob_ptr:
> 

Re: [RFC PATCH] drm: allow passing a real encoder object for wb connector

[Abhinav Kumar]

Hi Jani

On 1/21/2022 1:17 AM, Jani Nikula wrote:
> On Thu, 20 Jan 2022, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote:
>> Instead of creating an internal encoder for the writeback
>> connector to satisfy DRM requirements, allow the clients
>> to pass a real encoder to it by changing the drm_writeback's
>> encoder to a pointer.
>>
>> If a real encoder is not passed, drm_writeback_connector_init
>> will internally allocate one.
>>
>> This will help the clients to manage the real encoder states
>> better as they will allocate and maintain the encoder.
> 
> See also the thread starting at [1], and please try to coordinate.
> 
> I don't know what the end result should be like, I'm just saying please
> collaborate instead of racing to get one set of changes in.
> 
> BR,
> Jani.
> 
> 
> [1] https://patchwork.freedesktop.org/patch/msgid/20220111101801.28310-1-suraj.kandpal@intel.com
> 
Thanks for pointing to this thread. Since 
https://patchwork.freedesktop.org/patch/469090/ has been posted earlier 
and is more complete in terms of handling other vendor changes, we can 
continue on that one.

But I dont see any comments on that one yet.

Hi Laurent

In that case can you please check the 
https://patchwork.freedesktop.org/patch/469090/ thread , we can continue 
our discussion there.

We also have the same issue too. Our encoder also maintains its own 
struct drm_encoder.

Thanks

Abhinav
>>
>> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
>> ---
>>   drivers/gpu/drm/drm_writeback.c | 11 +++++++----
>>   include/drm/drm_writeback.h     |  2 +-
>>   2 files changed, 8 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c
>> index dccf4504..fdb7381 100644
>> --- a/drivers/gpu/drm/drm_writeback.c
>> +++ b/drivers/gpu/drm/drm_writeback.c
>> @@ -189,8 +189,11 @@ int drm_writeback_connector_init(struct drm_device *dev,
>>   	if (IS_ERR(blob))
>>   		return PTR_ERR(blob);
>>   
>> -	drm_encoder_helper_add(&wb_connector->encoder, enc_helper_funcs);
>> -	ret = drm_encoder_init(dev, &wb_connector->encoder,
>> +	/* allocate the internal drm encoder if a real one wasnt passed */
>> +	if (!wb_connector->encoder)
>> +		wb_connector->encoder = devm_kzalloc(dev->dev, sizeof(struct drm_encoder), GFP_KERNEL);
>> +	drm_encoder_helper_add(wb_connector->encoder, enc_helper_funcs);
>> +	ret = drm_encoder_init(dev, wb_connector->encoder,
>>   			       &drm_writeback_encoder_funcs,
>>   			       DRM_MODE_ENCODER_VIRTUAL, NULL);
>>   	if (ret)
>> @@ -204,7 +207,7 @@ int drm_writeback_connector_init(struct drm_device *dev,
>>   		goto connector_fail;
>>   
>>   	ret = drm_connector_attach_encoder(connector,
>> -						&wb_connector->encoder);
>> +						wb_connector->encoder);
>>   	if (ret)
>>   		goto attach_fail;
>>   
>> @@ -233,7 +236,7 @@ int drm_writeback_connector_init(struct drm_device *dev,
>>   attach_fail:
>>   	drm_connector_cleanup(connector);
>>   connector_fail:
>> -	drm_encoder_cleanup(&wb_connector->encoder);
>> +	drm_encoder_cleanup(wb_connector->encoder);
>>   fail:
>>   	drm_property_blob_put(blob);
>>   	return ret;
>> diff --git a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h
>> index 9697d27..f0d8147 100644
>> --- a/include/drm/drm_writeback.h
>> +++ b/include/drm/drm_writeback.h
>> @@ -31,7 +31,7 @@ struct drm_writeback_connector {
>>   	 * by passing the @enc_funcs parameter to drm_writeback_connector_init()
>>   	 * function.
>>   	 */
>> -	struct drm_encoder encoder;
>> +	struct drm_encoder *encoder;
>>   
>>   	/**
>>   	 * @pixel_formats_blob_ptr:
>