Re: [PATCH 5/6] drm/rcar_du: use drm_encoder pointer for drm_writeback_connector

[Abhinav Kumar <quic_abhinavk@quicinc.com>]

Hi Laurent

On 3/10/2022 11:28 PM, Laurent Pinchart wrote:
> Hi Abhinav
> 
> Thank you for the patch.
> 
> On Thu, Mar 10, 2022 at 05:49:59PM -0800, Abhinav Kumar wrote:
>> Make changes to rcar_du driver to start using drm_encoder pointer
>> for drm_writeback_connector.
>>
>> Co-developed-by: Kandpal Suraj <suraj.kandpal@intel.com>
>> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
>> ---
>>   drivers/gpu/drm/rcar-du/rcar_du_writeback.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_writeback.c b/drivers/gpu/drm/rcar-du/rcar_du_writeback.c
>> index c79d125..03930ad 100644
>> --- a/drivers/gpu/drm/rcar-du/rcar_du_writeback.c
>> +++ b/drivers/gpu/drm/rcar-du/rcar_du_writeback.c
>> @@ -200,7 +200,8 @@ int rcar_du_writeback_init(struct rcar_du_device *rcdu,
>>   {
>>   	struct drm_writeback_connector *wb_conn = &rcrtc->writeback;
>>   
>> -	wb_conn->encoder.possible_crtcs = 1 << drm_crtc_index(&rcrtc->crtc);
>> +	wb_conn->encoder = kzalloc(sizeof(struct drm_encoder), GFP_KERNEL);
> 
> Where is this freed ?

You are right, this isnt. Looking more into this, it seems like moving 
the allocation of encoder to drm_writeback.c for cases which dont pass a 
real encoder is much better so that I will not have to add alloc() / 
free() code for all the vendor driver changes which is what I originally 
thought in my RFC but changed my mind because of below.

> 
>> +	wb_conn->encoder->possible_crtcs = 1 << drm_crtc_index(&rcrtc->crtc);

Do you think we can just move usage of wb_conn->encoder->possible_crtcs 
just right after drm_writeback_connector_init() so that it wont crash?

198 int rcar_du_writeback_init(struct rcar_du_device *rcdu,
199 			   struct rcar_du_crtc *rcrtc)
200 {
201 	struct drm_writeback_connector *wb_conn = &rcrtc->writeback;
202
203 	wb_conn->encoder.possible_crtcs = 1 << drm_crtc_index(&rcrtc->crtc);
204 	drm_connector_helper_add(&wb_conn->base,
205 				 &rcar_du_wb_conn_helper_funcs);
206
207 	return drm_writeback_connector_init(&rcdu->ddev, wb_conn,
208 					    &rcar_du_wb_conn_funcs,
209 					    &rcar_du_wb_enc_helper_funcs,
210 					    writeback_formats,
211 					    ARRAY_SIZE(writeback_formats));
212 }

>>   	drm_connector_helper_add(&wb_conn->base,
>>   				 &rcar_du_wb_conn_helper_funcs);
>>   
>