Re: [PATCH] drm/malidp: Use struct drm_gem_object_funcs.get_sg_table internally

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Aug 7, 2020 at 4:02 PM Thomas Zimmermann <tzimmermann@xxxxxxx> wrote:
>
> Hi
>
> Am 07.08.20 um 15:12 schrieb Daniel Vetter:
> > On Fri, Aug 07, 2020 at 01:10:22PM +0200, Thomas Zimmermann wrote:
> >> The malidp driver uses GEM object functions for callbacks. Fix it to
> >> use them internally as well.
> >>
> >> Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
> >> Fixes: ecdd6474644f ("drm/malidp: Use GEM CMA object functions")
> >> Cc: Thomas Zimmermann <tzimmermann@xxxxxxx>
> >> Cc: Emil Velikov <emil.velikov@xxxxxxxxxxxxx>
> >> Cc: Liviu Dudau <liviu.dudau@xxxxxxx>
> >> Cc: Brian Starkey <brian.starkey@xxxxxxx>
> >> ---
> >>  drivers/gpu/drm/arm/malidp_planes.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/arm/malidp_planes.c b/drivers/gpu/drm/arm/malidp_planes.c
> >> index ab45ac445045..351a85088d0e 100644
> >> --- a/drivers/gpu/drm/arm/malidp_planes.c
> >> +++ b/drivers/gpu/drm/arm/malidp_planes.c
> >> @@ -346,7 +346,7 @@ static bool malidp_check_pages_threshold(struct malidp_plane_state *ms,
> >>              if (cma_obj->sgt)
> >>                      sgt = cma_obj->sgt;
> >>              else
> >> -                    sgt = obj->dev->driver->gem_prime_get_sg_table(obj);
> >> +                    sgt = obj->funcs->get_sg_table(obj);
> >
> > Uh if there's not a switch somewhere I'd just call the right function
> > directly. Or call the right wrapper for this, this feels a bit fishy ...
>
> The driver initializes the pointer via CMA helper macro to an
> CMA-internal default. Calling the actual function here is fragile if the
> CMA-internal default ever changes.
>
> But I have no strong feelings. I'll go with whatever the driver's
> maintainer prefers.

What I meant is: There should be an exported helper to get at the sgt.
Drivers using helpers shouldn't need to do this kind of stuff here.

Also the entire code is fairly suspect, getting at the sgt from
plane_check is a bit iffy. But that's a different kind of problem.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux