[Intel-gfx] [PATCH 2/4] drm/i915: fix include order in intel_tc.*

[Michal Wajdeczko]

On Thu, 04 Jul 2019 19:43:12 +0200, Imre Deak <imre.deak at intel.com> wrote:

> On Thu, Jul 04, 2019 at 07:21:38PM +0200, Michal Wajdeczko wrote:
>> On Thu, 04 Jul 2019 02:06:47 +0200, Lucas De Marchi
>> <lucas.demarchi at intel.com> wrote:
>>
>> > Make intel_tc.h the first include so we guarantee it's self-contained.
>> > Sort the rest. Same principle applies for includes in the header.
>> >
>> > Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/display/intel_tc.c | 5 +++--
>> >  drivers/gpu/drm/i915/display/intel_tc.h | 5 +++--
>> >  2 files changed, 6 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/display/intel_tc.c
>> > b/drivers/gpu/drm/i915/display/intel_tc.c
>> > index 1a9dd32fb0a5..e6e6163c1232 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_tc.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_tc.c
>> > @@ -3,10 +3,11 @@
>> >   * Copyright © 2019 Intel Corporation
>> >   */
>> > +#include "intel_tc.h"
>> > +
>> > +#include "i915_drv.h"
>>
>> this master header will likely include everything, so while
>> your .h is now self-contained, .c remains unclean
>>
>> >  #include "intel_display.h"
>> >  #include "intel_dp_mst.h"
>> > -#include "i915_drv.h"
>> > -#include "intel_tc.h"
>> > static const char *tc_port_mode_name(enum tc_port_mode mode)
>> >  {
>> > diff --git a/drivers/gpu/drm/i915/display/intel_tc.h
>> > b/drivers/gpu/drm/i915/display/intel_tc.h
>> > index 0d8411d4a91d..45ae30537b78 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_tc.h
>> > +++ b/drivers/gpu/drm/i915/display/intel_tc.h
>> > @@ -6,10 +6,11 @@
>> >  #ifndef __INTEL_TC_H__
>> >  #define __INTEL_TC_H__
>> > -#include <linux/types.h>
>> > -#include <linux/mutex.h>
>> >  #include "intel_drv.h"
>>
>> are you sure you want this giant header to be included here?
>> note that it will #include almost everything (i915_drv.h too)
>> and may introduce hard to resolve circular dependencies
>>
>> > +#include <linux/mutex.h>
>>
>> we don't need mutex definitions here in current header file
>>
>> > +#include <linux/types.h>
>> > +
>>
>> so you need only above types.h and then add forward decl for:
>>
>> struct intel_digital_port;
>
> Both the mutex and intel_digital_port definitions are needed by
> intel_tc_port_ref_held() in this header.

oops, sorry, didn't scroll down to see intel_tc_port_ref_held() inline.

but wait, it is used only once in intel_display_power.c so
maybe this inline can be moved to one of the .c file ?

also, maybe it's time to move struct intel_digital_port definition
out of intel_drv.h ?

>
>>
>> >  bool intel_tc_port_connected(struct intel_digital_port *dig_port);
>> >  u32 intel_tc_port_get_lane_mask(struct intel_digital_port *dig_port);
>> >  int intel_tc_port_fia_max_lane_count(struct intel_digital_port
>> > *dig_port);
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx