Re: [PATCH 4/5] drm/print: Include drm_device.h

[Simona Vetter <simona.vetter@ffwll.ch>]

On Fri, Jan 24, 2025 at 09:21:25AM -0300, Gustavo Sousa wrote:
> Quoting Jani Nikula (2025-01-24 08:50:14-03:00)
> >On Thu, 23 Jan 2025, Gustavo Sousa <gustavo.sousa@intel.com> wrote:
> >> Quoting Jani Nikula (2025-01-23 12:14:31-03:00)
> >>>On Thu, 23 Jan 2025, Jani Nikula <jani.nikula@intel.com> wrote:
> >>>> From: Gustavo Sousa <gustavo.sousa@intel.com>
> >>>>
> >>>> The header drm_print.h uses members of struct drm_device pointers, as
> >>>> such, it should include drm_device.h to let the compiler know the full
> >>>> type definition.
> >>>>
> >>>> Without such include, users of drm_print.h that don't explicitly need
> >>>> drm_device.h would bump into build errors and be forced to include the
> >>>> latter.
> >>>>
> >>>> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
> >>>> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> >>>
> >>>This posting should have had
> >>>
> >>>Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> >>>
> >>>but obviously not needed if the original [1] is merged instead.
> >>>
> >>>
> >>>[1] https://lore.kernel.org/r/20250121210935.84357-1-gustavo.sousa@intel.com
> >>
> >> Hm. Since that's in the upper drm layer, I thought I was not supposed to
> >> merge it myself.
> >>
> >> Am I? In that case, is it okay to merge it via drm-intel-next?
> >
> >It's generally not okay to merge non-i915 stuff via drm-intel-next,
> >because only i915 is under our maintenance. We'd need rationale and acks
> >for that, which we occasionally do, but there's no reason here.
> >
> >But you can apply for drm-misc commit rights, similar to drm-intel, and
> >apply the patches to drm-misc-next yourself. See the maintainer tools
> >documentation.
> 
> The first bullet point in the list of criteria for drm-misc[2] says:
> 
>   "Submitted a few (5-10 as a rule of thumb) non-trivial (not just
>   simple spelling fixes and whitespace adjustment) patches that have
>   been merged already."
> 
> I believe those patches should be targeted at drm-misc, right? (meaning
> not stuff that have their own tree (e.g. drm-xe drm-intel)).
> 
> If that's the case, since this is my first patch targeting core drm, I
> believe I would not fit into the criteria right now.

So we're generally flexible about where the patches landed, but there's
also the requirement that you plan to regularly push more patches to
drm-misc. And that's probably not so much the case here.

There should be enough drm-misc committers in the intel team to do the
occasional push to drm-misc for you.
-Sima

> Decided to ask here just to make sure I don't raise a request in vain.
> 
> [2] https://drm.pages.freedesktop.org/maintainer-tools/committer/commit-access.html#drm-misc
> 
> --
> Gustavo Sousa
> 
> >
> >
> >BR,
> >Jani.
> >
> >
> >>
> >> --
> >> Gustavo Sousa
> >>
> >>>
> >>>>
> >>>> ---
> >>>>
> >>>> Including here as a dependency. May be merged independently.
> >>>> ---
> >>>>  include/drm/drm_print.h | 1 +
> >>>>  1 file changed, 1 insertion(+)
> >>>>
> >>>> diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
> >>>> index f77fe1531cf8..9732f514566d 100644
> >>>> --- a/include/drm/drm_print.h
> >>>> +++ b/include/drm/drm_print.h
> >>>> @@ -32,6 +32,7 @@
> >>>>  #include <linux/dynamic_debug.h>
> >>>>  
> >>>>  #include <drm/drm.h>
> >>>> +#include <drm/drm_device.h>
> >>>>  
> >>>>  struct debugfs_regset32;
> >>>>  struct drm_device;
> >>>
> >>>-- 
> >>>Jani Nikula, Intel
> >
> >-- 
> >Jani Nikula, Intel

-- 
Simona Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch