Re: drm connectors, tegra, and the web they weave (was Re: [PATCH 58/59] drm/todo: Add new debugfs todo)

Thierry Reding
2019-06-20T15:16:24Z

[-- Attachment #1.1: Type: text/plain, Size: 3250 bytes --]

On Tue, Jun 18, 2019 at 04:37:16PM +0100, Jon Hunter wrote:
>
> On 18/06/2019 16:19, Greg Kroah-Hartman wrote:
> > On Fri, Jun 14, 2019 at 10:36:14PM +0200, Daniel Vetter wrote:
> >> Greg is busy already, but maybe he won't do everything ...
> >>
> >> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> >> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> >> ---
> >> Documentation/gpu/todo.rst | 3 +++
> >> 1 file changed, 3 insertions(+)
> >>
> >> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
> >> index 9717540ee28f..026e55c517e1 100644
> >> --- a/Documentation/gpu/todo.rst
> >> +++ b/Documentation/gpu/todo.rst
> >> @@ -375,6 +375,9 @@ There's a bunch of issues with it:
> >> this (together with the drm_minor->drm_device move) would allow us to remove
> >> debugfs_init.
> >>
> >> +- Drop the return code and error checking from all debugfs functions. Greg KH is
> >> + working on this already.
> >
> >
> > Part of this work was to try to delete drm_debugfs_remove_files().
> >
> > There are only 4 files that currently still call this function:
> > drivers/gpu/drm/tegra/dc.c
> > drivers/gpu/drm/tegra/dsi.c
> > drivers/gpu/drm/tegra/hdmi.c
> > drivers/gpu/drm/tegra/sor.c
> >
> > For dc.c, the driver wants to add debugfs files to the struct drm_crtc
> > debugfs directory. Which is fine, but it has to do some special memory
> > allocation to get the debugfs callback to point not to the struct
> > drm_minor pointer, but rather the drm_crtc structure.
> >
> > So, to remove this call, I need to remove this special memory allocation
> > and to do that, I need to somehow be able to cast from drm_minor back to
> > the drm_crtc structure being used in this driver. And I can't figure
> > how they are related at all.
> >
> > Any pointers here (pun intended) would be appreciated.
> >
> > For the other 3 files, the situation is much the same, but I need to get
> > from a 'struct drm_minor' pointer to a 'struct drm_connector' pointer.
> >
> > I could just "open code" a bunch of calls to debugfs_create_file() for
> > these drivers, which would solve this issue, but in a more "non-drm"
> > way. Is it worth to just do that instead of overthinking the whole
> > thing and trying to squish it into the drm "model" of drm debugfs calls?
> >
> > Either way, who can test these changes? I can't even build the tegra
> > driver without digging up an arm64 cross-compiler, and can't test it as
> > I have no hardware at all.
>
> We can definitely compile and boot test these no problem. In fact
> anything that lands in -next we will boot test. However, I can do some
> quick sanity if you have something to test.
>
> Thierry may have more specific Tegra DRM tests.

We don't have any automated tests for this yet, unfortunately. Let me
work on something. In the meantime I can manually test any of the
patches that Greg sends out. These should be fairly trivial to test.
It's difficult to check for success/failure on something like the
register dump or the CRC, but I think for now we don't really need much
more than just validating that things don't crash when we read one of
these debugfs files.

Thierry

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]