Re: linux-next: manual merge of the drm-misc tree with the drm-misc-fixes tree

[Tomi Valkeinen]

Hi Stephen,

On 23/04/2020 06:17, Stephen Rothwell wrote:
> Hi all,
> 
> On Tue, 21 Apr 2020 09:10:25 +0300 Tomi Valkeinen <tomi.valkeinen@xxxxxx> wrote:
>>
>> On 21/04/2020 04:52, Stephen Rothwell wrote:
>>>
>>> Today's linux-next merge of the drm-misc tree got a conflict in:he drm-misc tree with the drm-misc-fixes tree
>>>
>>>     drivers/gpu/drm/tidss/tidss_encoder.c
>>>
>>> between commit:
>>>
>>>     9da67433f64e ("drm/tidss: fix crash related to accessing freed memory")
>>>
>>> from the drm-misc-fixes tree and commit:
>>>
>>>     b28ad7deb2f2 ("drm/tidss: Use simple encoder")
>>>
>>> from the drm-misc tree.
>>>
>>> I fixed it up (I just used the latter version of this file) and can
>>
>> We need to use "drm/tidss: fix crash related to accessing freed memory" version.
>>
>>> carry the fix as necessary. This is now fixed as far as linux-next is
>>> concerned, but any non trivial conflicts should be mentioned to your
>>> upstream maintainer when your tree is submitted for merging.  You may
>>> also want to consider cooperating with the maintainer of the conflicting
>>> tree to minimise any particularly complex conflicts.
>>
>> I have fixed this with drm-misc's dim tool, so I presume the conflict goes away when drm-misc-fixes
>> is merged to drm-fixes, and drm-fixes is then at some point merged to drm-misc-next.
>>
>> It was a bit bad timing with the "drm/tidss: Use simple encoder", which removes the plumbing I
>> needed to implement the fix. So I effectively revert the "drm/tidss: Use simple encoder".
>>
>>    Tomi
>>
> 
> This is now a conflict between the drm and drm-misc-fixes trees.

The fix ("drm/tidss: fix crash related to accessing freed memory") is in v5.7-rc3, and the conflicting change ("drm/tidss: Use simple encoder") in drm-next.

The conflict resolution in linux-next drops the fix and take the change from drm-next, which causes crash on module removal.

Here's the diff I made on top of linux-next to fix it. Essentially dropping the simple-encoder change, and reapplying the fix. This should be fixed in drm-next when Dave next time pulls in Linus' branch.

diff --git a/drivers/gpu/drm/tidss/tidss_encoder.c b/drivers/gpu/drm/tidss/tidss_encoder.c
index 4c0558286f5e..e624cdcbb567 100644
--- a/drivers/gpu/drm/tidss/tidss_encoder.c
+++ b/drivers/gpu/drm/tidss/tidss_encoder.c
@@ -56,25 +56,38 @@ static int tidss_encoder_atomic_check(struct drm_encoder *encoder,
 	return 0;
 }
 
+static void tidss_encoder_destroy(struct drm_encoder *encoder)
+{
+	drm_encoder_cleanup(encoder);
+	kfree(encoder);
+}
+
 static const struct drm_encoder_helper_funcs encoder_helper_funcs = {
 	.atomic_check = tidss_encoder_atomic_check,
 };
 
+static const struct drm_encoder_funcs encoder_funcs = {
+	.destroy = tidss_encoder_destroy,
+};
+
 struct drm_encoder *tidss_encoder_create(struct tidss_device *tidss,
 					 u32 encoder_type, u32 possible_crtcs)
 {
 	struct drm_encoder *enc;
 	int ret;
 
-	enc = devm_kzalloc(tidss->dev, sizeof(*enc), GFP_KERNEL);
+	enc = kzalloc(sizeof(*enc), GFP_KERNEL);
 	if (!enc)
 		return ERR_PTR(-ENOMEM);
 
 	enc->possible_crtcs = possible_crtcs;
 
-	ret = drm_simple_encoder_init(&tidss->ddev, enc, encoder_type);
-	if (ret < 0)
+	ret = drm_encoder_init(&tidss->ddev, enc, &encoder_funcs,
+			       encoder_type, NULL);
+	if (ret < 0) {
+		kfree(enc);
 		return ERR_PTR(ret);
+	}
 
 	drm_encoder_helper_add(enc, &encoder_helper_funcs);
 


 Tomi

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki