Re: [Intel-gfx] [PATCH 01/39] drm/i915: add display sub-struct to drm_i915_private

[Jani Nikula <jani.nikula@intel.com>]

On Tue, 16 Aug 2022, "Murthy, Arun R" <arun.r.murthy@intel.com> wrote:
>> -----Original Message-----
>> From: Nikula, Jani <jani.nikula@intel.com>
>> Sent: Friday, August 12, 2022 12:10 PM
>> To: Murthy, Arun R <arun.r.murthy@intel.com>; intel-
>> gfx@lists.freedesktop.org
>> Cc: De Marchi, Lucas <lucas.demarchi@intel.com>
>> Subject: RE: [Intel-gfx] [PATCH 01/39] drm/i915: add display sub-struct to
>> drm_i915_private
>>
>> On Fri, 12 Aug 2022, "Murthy, Arun R" <arun.r.murthy@intel.com> wrote:
>> >> +++ b/drivers/gpu/drm/i915/display/intel_display_core.h
>> >> @@ -0,0 +1,38 @@
>> >> +/* SPDX-License-Identifier: MIT */
>> >> +/*
>> >> + * Copyright © 2022 Intel Corporation  */
>> >> +
>> >> +#ifndef __INTEL_DISPLAY_CORE_H__
>> >> +#define __INTEL_DISPLAY_CORE_H__
>> >> +
>> >> +#include <linux/types.h>
>> >> +
>> >> +struct intel_atomic_state;
>> >> +struct intel_crtc;
>> >> +struct intel_crtc_state;
>> >> +struct intel_initial_plane_config;
>> >> +
>> >> +struct intel_display_funcs {
>> >> +     /* Returns the active state of the crtc, and if the crtc is active,
>> >> +      * fills out the pipe-config with the hw state. */
>> > Can this be changed to multi-line commenting style.
>>
>> Yeah.
>>
>> > /*
>> >  *
>> >  */
>> >> +     bool (*get_pipe_config)(struct intel_crtc *,
>> >> +                             struct intel_crtc_state *);
>> >> +     void (*get_initial_plane_config)(struct intel_crtc *,
>> >> +                                      struct intel_initial_plane_config *);
>> >> +     void (*crtc_enable)(struct intel_atomic_state *state,
>> >> +                         struct intel_crtc *crtc);
>> >> +     void (*crtc_disable)(struct intel_atomic_state *state,
>> >> +                          struct intel_crtc *crtc);
>> >> +     void (*commit_modeset_enables)(struct intel_atomic_state
>> >> + *state);
>> >
>> > Can this be changed to something meaningful word, something like
>> > update_modeset()
>>
>> It's already borderline doing too much in one patch to rename the struct, and
>> definitely too much to rename the hook. Maybe in another patch.
>>
> Thanks for accepting, would this come as part of this series itself?

I think this series is already growing too big, I'll want to start
merging before adding new patches. But there's more things to move to
the display sub-struct too, so there'll be more patches.

BR,
Jani.

>
> Thanks and Regards,
> Arun R Murthy
> --------------------

-- 
Jani Nikula, Intel Open Source Graphics Center