[Letux-kernel] LetuxOS: Kernel: m-stable 5.4.77 successful

H. Nikolaus Schaller hns at goldelico.com
Thu Nov 26 21:57:17 CET 2020


Hi Andreas,

> Am 21.11.2020 um 11:58 schrieb Andreas Kemnade <andreas at kemnade.info>:
> 
> On Sat, 21 Nov 2020 08:46:44 +0100
> "H. Nikolaus Schaller" <hns at goldelico.com> wrote:
> 
>> Hi Andreas,
>> 
>>> Am 20.11.2020 um 22:37 schrieb Andreas Kemnade <andreas at kemnade.info>:
>>> 
>>> On Fri, 20 Nov 2020 22:26:16 +0100
>>> Andreas Kemnade <andreas at kemnade.info> wrote:
>> 
>> Yes, it may be a cherry-pick with bad automatic or manual conflict resolution.
>> It may come from a patch backported from 5.6. AFAIR, 5.6 was the best Pyra kernel
>> for a while and is currently used by PyraOS. But it is no longer maintained.
>> 
> Same code area from letux 5.6 kernel:
> 
> static void omap_gem_unpin_locked(struct drm_gem_object *obj)
> {
>        struct omap_drm_private *priv = obj->dev->dev_private;
>        struct omap_gem_object *omap_obj = to_omap_bo(obj);
>        int ret;
> 
>        if (omap_gem_is_contiguous(omap_obj) || !priv->has_dmm)
>                return;
> 
>        if (refcount_dec_and_test(&omap_obj->dma_addr_cnt)) {
> 
> 
> So something was refactored upstream and the patch does not fit
> properly.

Ok,
this is upstream v5.4:

https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/gpu/drm/omapdrm/omap_gem.c?h=v5.4.80

void omap_gem_unpin(struct drm_gem_object *obj)
{
	struct omap_gem_object *omap_obj = to_omap_bo(obj);
	int ret;

	mutex_lock(&omap_obj->lock);

	if (omap_obj->dma_addr_cnt > 0) {
		omap_obj->dma_addr_cnt--;
		if (omap_obj->dma_addr_cnt == 0) {
			ret = tiler_unpin(omap_obj->block);
			if (ret) {
				dev_err(obj->dev->dev,
					"could not unpin pages: %d\n", ret);
			}
			ret = tiler_release(omap_obj->block);
			if (ret) {
				dev_err(obj->dev->dev,
					"could not release unmap: %d\n", ret);
			}
			omap_obj->dma_addr = 0;
			omap_obj->block = NULL;
		}
	}

	mutex_unlock(&omap_obj->lock);
}

This is what we have in letux-5.4:

60d81a5a4665390 drivers/gpu/drm/omapdrm/omap_gem.c (Matthijs van Duin    2018-01-02 11:34:11 +0100  874) static void _omap_gem_unpin(struct drm_gem_object *obj)
60d81a5a4665390 drivers/gpu/drm/omapdrm/omap_gem.c (Matthijs van Duin    2018-01-02 11:34:11 +0100  875) {
60d81a5a4665390 drivers/gpu/drm/omapdrm/omap_gem.c (Matthijs van Duin    2018-01-02 11:34:11 +0100  876)        struct omap_gem_object *omap_obj = to_omap_bo(obj);
60d81a5a4665390 drivers/gpu/drm/omapdrm/omap_gem.c (Matthijs van Duin    2018-01-02 11:34:11 +0100  877)        int ret;
f7f9f4536a09f9f drivers/staging/omapdrm/omap_gem.c (Rob Clark            2011-12-05 19:19:22 -0600  878) 
60d81a5a4665390 drivers/gpu/drm/omapdrm/omap_gem.c (Matthijs van Duin    2018-01-02 11:34:11 +0100  879)        if (omap_gem_is_contiguous(omap_obj))
60d81a5a4665390 drivers/gpu/drm/omapdrm/omap_gem.c (Matthijs van Duin    2018-01-02 11:34:11 +0100  880)                return;
60d81a5a4665390 drivers/gpu/drm/omapdrm/omap_gem.c (Matthijs van Duin    2018-01-02 11:34:11 +0100  881)        if (omap_obj->dma_addr_cnt--)
60d81a5a4665390 drivers/gpu/drm/omapdrm/omap_gem.c (Matthijs van Duin    2018-01-02 11:34:11 +0100  882)                return;
60d81a5a4665390 drivers/gpu/drm/omapdrm/omap_gem.c (Matthijs van Duin    2018-01-02 11:34:11 +0100  883)        if (WARN_ON(!omap_obj->block))
60d81a5a4665390 drivers/gpu/drm/omapdrm/omap_gem.c (Matthijs van Duin    2018-01-02 11:34:11 +0100  884)                return;
f7f9f4536a09f9f drivers/staging/omapdrm/omap_gem.c (Rob Clark            2011-12-05 19:19:22 -0600  885) 
a882ed5f130ce0d drivers/gpu/drm/omapdrm/omap_gem.c (Matthijs van Duin    2020-06-19 16:34:41 +0100  886)        if (refcount_dec_and_test(&omap_obj->dma_addr_cnt)) {
a882ed5f130ce0d drivers/gpu/drm/omapdrm/omap_gem.c (Matthijs van Duin    2020-06-19 16:34:41 +0100  887) 
a882ed5f130ce0d drivers/gpu/drm/omapdrm/omap_gem.c (Matthijs van Duin    2020-06-19 16:34:41 +0100  888)                if (omap_obj->sgt) {
a882ed5f130ce0d drivers/gpu/drm/omapdrm/omap_gem.c (Matthijs van Duin    2020-06-19 16:34:41 +0100  889)                        sg_free_table(omap_obj->sgt);
a882ed5f130ce0d drivers/gpu/drm/omapdrm/omap_gem.c (Matthijs van Duin    2020-06-19 16:34:41 +0100  890)                        kfree(omap_obj->sgt);
a882ed5f130ce0d drivers/gpu/drm/omapdrm/omap_gem.c (Matthijs van Duin    2020-06-19 16:34:41 +0100  891)                        omap_obj->sgt = NULL;
a882ed5f130ce0d drivers/gpu/drm/omapdrm/omap_gem.c (Matthijs van Duin    2020-06-19 16:34:41 +0100  892)                }
a882ed5f130ce0d drivers/gpu/drm/omapdrm/omap_gem.c (Matthijs van Duin    2020-06-19 16:34:41 +0100  893) 
a882ed5f130ce0d drivers/gpu/drm/omapdrm/omap_gem.c (Matthijs van Duin    2020-06-19 16:34:41 +0100  894)                ret = tiler_unpin(omap_obj->block);
a882ed5f130ce0d drivers/gpu/drm/omapdrm/omap_gem.c (Matthijs van Duin    2020-06-19 16:34:41 +0100  895)                if (ret) {
a882ed5f130ce0d drivers/gpu/drm/omapdrm/omap_gem.c (Matthijs van Duin    2020-06-19 16:34:41 +0100  896)                        dev_err(obj->dev->dev,
a882ed5f130ce0d drivers/gpu/drm/omapdrm/omap_gem.c (Matthijs van Duin    2020-06-19 16:34:41 +0100  897)                                "could not unpin pages: %d\n", ret);
a882ed5f130ce0d drivers/gpu/drm/omapdrm/omap_gem.c (Matthijs van Duin    2020-06-19 16:34:41 +0100  898)                }
a882ed5f130ce0d drivers/gpu/drm/omapdrm/omap_gem.c (Matthijs van Duin    2020-06-19 16:34:41 +0100  899)                ret = tiler_release(omap_obj->block);
a882ed5f130ce0d drivers/gpu/drm/omapdrm/omap_gem.c (Matthijs van Duin    2020-06-19 16:34:41 +0100  900)                if (ret) {
a882ed5f130ce0d drivers/gpu/drm/omapdrm/omap_gem.c (Matthijs van Duin    2020-06-19 16:34:41 +0100  901)                        dev_err(obj->dev->dev,
a882ed5f130ce0d drivers/gpu/drm/omapdrm/omap_gem.c (Matthijs van Duin    2020-06-19 16:34:41 +0100  902)                                "could not release unmap: %d\n", ret);
a882ed5f130ce0d drivers/gpu/drm/omapdrm/omap_gem.c (Matthijs van Duin    2020-06-19 16:34:41 +0100  903)                }
a882ed5f130ce0d drivers/gpu/drm/omapdrm/omap_gem.c (Matthijs van Duin    2020-06-19 16:34:41 +0100  904)                omap_obj->dma_addr = 0;
a882ed5f130ce0d drivers/gpu/drm/omapdrm/omap_gem.c (Matthijs van Duin    2020-06-19 16:34:41 +0100  905)                omap_obj->block = NULL;
cd5351f4d2b1b88 drivers/staging/omapdrm/omap_gem.c (Rob Clark            2011-11-12 12:09:40 -0600  906)        }
60d81a5a4665390 drivers/gpu/drm/omapdrm/omap_gem.c (Matthijs van Duin    2018-01-02 11:34:11 +0100  907)        ret = tiler_release(omap_obj->block);
60d81a5a4665390 drivers/gpu/drm/omapdrm/omap_gem.c (Matthijs van Duin    2018-01-02 11:34:11 +0100  908)        if (ret) {
60d81a5a4665390 drivers/gpu/drm/omapdrm/omap_gem.c (Matthijs van Duin    2018-01-02 11:34:11 +0100  909)                dev_err(obj->dev->dev,
60d81a5a4665390 drivers/gpu/drm/omapdrm/omap_gem.c (Matthijs van Duin    2018-01-02 11:34:11 +0100  910)                        "could not release unmap: %d\n", ret);
60d81a5a4665390 drivers/gpu/drm/omapdrm/omap_gem.c (Matthijs van Duin    2018-01-02 11:34:11 +0100  911)        }
60d81a5a4665390 drivers/gpu/drm/omapdrm/omap_gem.c (Matthijs van Duin    2018-01-02 11:34:11 +0100  912)        omap_obj->dma_addr = 0;
60d81a5a4665390 drivers/gpu/drm/omapdrm/omap_gem.c (Matthijs van Duin    2018-01-02 11:34:11 +0100  913)        omap_obj->block = NULL;
cd5351f4d2b1b88 drivers/staging/omapdrm/omap_gem.c (Rob Clark            2011-11-12 12:09:40 -0600  914) }
cd5351f4d2b1b88 drivers/staging/omapdrm/omap_gem.c (Rob Clark            2011-11-12 12:09:40 -0600  915) 


It seems as if a882ed5f130ce0d was merged although it should not have been this way or fixed differently.
Clearly, omap_obj->dma_addr_cnt-- is decremented twice unless it is 0.

I think we just should delete lines 881&882

       if (omap_obj->dma_addr_cnt--)
	      return;

Checking for omap_obj->block before omap_obj->dma_addr_cnt should not harm.

What I am not sure about are lines 888-892 and 907-910. They are not in upstream v5.4 or upstream v5.6. But they may be essential for making the tiler work on the Pyra.
They are in a different way in letux-5.6: https://git.goldelico.com/?p=letux-kernel.git;a=blob;f=drivers/gpu/drm/omapdrm/omap_gem.c;h=c1fe70cf98d8a3d56ab3532d85a8f9191026cdbf;hb=refs/heads/letux-5.6.y#l899

So lines 888-892 seem to be needed but 907 is a double tiler_release() besides that the error message in 910 is also a duplicate...

It looks as if a882ed5f130ce0d was inserted by a backport patched into lines 886 and 906 replacing some older tiler addition code.

In summary I think we must drop 881-882 and 907-913.

What do you think?

BR,
Nikolaus

diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c b/drivers/gpu/drm/omapdrm/omap_gem.c
index 47585dcbafb4e9..f37af7ff785aaf 100644
--- a/drivers/gpu/drm/omapdrm/omap_gem.c
+++ b/drivers/gpu/drm/omapdrm/omap_gem.c
@@ -878,8 +878,6 @@ static void _omap_gem_unpin(struct drm_gem_object *obj)
 
        if (omap_gem_is_contiguous(omap_obj))
                return;
-       if (omap_obj->dma_addr_cnt--)
-               return;
        if (WARN_ON(!omap_obj->block))
                return;
 
@@ -904,13 +902,6 @@ static void _omap_gem_unpin(struct drm_gem_object *obj)
                omap_obj->dma_addr = 0;
                omap_obj->block = NULL;
        }
-       ret = tiler_release(omap_obj->block);
-       if (ret) {
-               dev_err(obj->dev->dev,
-                       "could not release unmap: %d\n", ret);
-       }
-       omap_obj->dma_addr = 0;
-       omap_obj->block = NULL;
 }
 
 /**






More information about the Letux-kernel mailing list