> In order to properly split vkms_output function, extract all > its function to its own header. > > Signed-off-by: Louis Chauvet <louis.chauvet@xxxxxxxxxxx> Reviewed-by: José Expósito <jose.exposito89@xxxxxxxxx> > --- > drivers/gpu/drm/vkms/vkms_crtc.c | 2 ++ > drivers/gpu/drm/vkms/vkms_drv.h | 56 ------------------------------ > drivers/gpu/drm/vkms/vkms_formats.c | 3 +- > drivers/gpu/drm/vkms/vkms_output.c | 2 +- > drivers/gpu/drm/vkms/vkms_plane.c | 3 ++ > drivers/gpu/drm/vkms/vkms_plane.h | 65 +++++++++++++++++++++++++++++++++++ > drivers/gpu/drm/vkms/vkms_writeback.c | 1 - > drivers/gpu/drm/vkms/vkms_writeback.h | 1 + > 8 files changed, 74 insertions(+), 59 deletions(-) > > diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c > index 06e28305d660..6a4de8f7a678 100644 > --- a/drivers/gpu/drm/vkms/vkms_crtc.c > +++ b/drivers/gpu/drm/vkms/vkms_crtc.c > @@ -6,8 +6,10 @@ > #include <drm/drm_atomic_helper.h> > #include <drm/drm_probe_helper.h> > #include <drm/drm_vblank.h> > +#include <drm/drm_print.h> Seems unrelated to this change? Anyway, it'd be nice to place it after drm_probe_helper.h. > > #include "vkms_drv.h" > +#include "vkms_plane.h" > > static enum hrtimer_restart vkms_vblank_simulate(struct hrtimer *timer) > { > diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h > index 46daa2fab6e8..ea73f01fcc74 100644 > --- a/drivers/gpu/drm/vkms/vkms_drv.h > +++ b/drivers/gpu/drm/vkms/vkms_drv.h > @@ -27,48 +27,6 @@ > > #define VKMS_LUT_SIZE 256 > > -/** > - * struct vkms_frame_info - Structure to store the state of a frame > - * > - * @fb: backing drm framebuffer > - * @src: source rectangle of this frame in the source framebuffer, stored in 16.16 fixed-point form > - * @dst: destination rectangle in the crtc buffer, stored in whole pixel units > - * @map: see drm_shadow_plane_state@data > - * @rotation: rotation applied to the source. > - * > - * @src and @dst should have the same size modulo the rotation. > - */ > -struct vkms_frame_info { > - struct drm_framebuffer *fb; > - struct drm_rect src, dst; > - struct iosys_map map[DRM_FORMAT_MAX_PLANES]; > - unsigned int rotation; > -}; > - > - > -struct vkms_plane_state; I guess this forward declaration was added by a previous patch in a different series, but it is not required. Mentioning just in case you'd like to fix it in the correct patch. > - > - > - > -/** > - * struct vkms_plane_state - Driver specific plane state > - * @base: base plane state > - * @frame_info: data required for composing computation > - * @pixel_read_line: function to read a pixel line in this plane. The creator of a > - * struct vkms_plane_state must ensure that this pointer is valid > - * @conversion_matrix: matrix used for yuv formats to convert to rgb > - */ > -struct vkms_plane_state { > - struct drm_shadow_plane_state base; > - struct vkms_frame_info *frame_info; > - pixel_read_line_t pixel_read_line; > - struct conversion_matrix conversion_matrix; > -}; > - > -struct vkms_plane { > - struct drm_plane base; > -}; > - > /** > * struct vkms_crtc_state - Driver specific CRTC state > * > @@ -174,9 +132,6 @@ struct vkms_device { > #define to_vkms_crtc_state(target)\ > container_of(target, struct vkms_crtc_state, base) > > -#define to_vkms_plane_state(target)\ > - container_of(target, struct vkms_plane_state, base.base) > - > /** > * vkms_crtc_init() - Initialize a crtc for vkms > * @dev: drm_device associated with the vkms buffer > @@ -196,17 +151,6 @@ int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc, > > int vkms_output_init(struct vkms_device *vkmsdev, int possible_crtc_index); > > -/** > - * vkms_plane_init() - Initialize a plane > - * > - * @vkmsdev: vkms device containing the plane > - * @type: type of plane to initialize > - * @possible_crtc_index: Crtc which can be attached to the plane. The caller must ensure that > - * possible_crtc_index is positive and less or equals to 31. > - */ > -struct vkms_plane *vkms_plane_init(struct vkms_device *vkmsdev, > - enum drm_plane_type type, int possible_crtc_index); > - > /* CRC Support */ > const char *const *vkms_get_crc_sources(struct drm_crtc *crtc, > size_t *count); > diff --git a/drivers/gpu/drm/vkms/vkms_formats.c b/drivers/gpu/drm/vkms/vkms_formats.c > index cbfa7943e948..4e8494d4ade4 100644 > --- a/drivers/gpu/drm/vkms/vkms_formats.c > +++ b/drivers/gpu/drm/vkms/vkms_formats.c > @@ -11,8 +11,9 @@ > > #include <kunit/visibility.h> > > -#include "vkms_writeback.h" > +#include "vkms_plane.h" > #include "vkms_formats.h" > +#include "vkms_writeback.h" The #include "vkms_writeback.h" was added by the previous patch. We can avoid moving it here by adding it at the bottom. plane.h should go after formats.h. > > /** > * packed_pixels_offset() - Get the offset of the block containing the pixel at coordinates x/y > diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c > index 0c55682337a4..09fcf242ecf7 100644 > --- a/drivers/gpu/drm/vkms/vkms_output.c > +++ b/drivers/gpu/drm/vkms/vkms_output.c > @@ -1,11 +1,11 @@ > // SPDX-License-Identifier: GPL-2.0+ > > -#include "vkms_drv.h" > #include <drm/drm_atomic_helper.h> > #include <drm/drm_edid.h> > #include <drm/drm_probe_helper.h> > > #include "vkms_writeback.h" > +#include "vkms_plane.h" #include "vkms_plane.h" #include "vkms_writeback.h" > > static const struct drm_connector_funcs vkms_connector_funcs = { > .fill_modes = drm_helper_probe_single_connector_modes, > diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c > index 9d85464ee0e9..de2c83e1b02c 100644 > --- a/drivers/gpu/drm/vkms/vkms_plane.c > +++ b/drivers/gpu/drm/vkms/vkms_plane.c > @@ -8,9 +8,12 @@ > #include <drm/drm_fourcc.h> > #include <drm/drm_gem_atomic_helper.h> > #include <drm/drm_gem_framebuffer_helper.h> > +#include <drm/drm_framebuffer.h> > +#include <drm/drm_print.h> #include <drm/drm_fourcc.h> +#include <drm/drm_framebuffer.h> #include <drm/drm_gem_atomic_helper.h> #include <drm/drm_gem_framebuffer_helper.h> +#include <drm/drm_print.h> > > #include "vkms_drv.h" > #include "vkms_formats.h" > +#include "vkms_plane.h" > > static const u32 vkms_formats[] = { > DRM_FORMAT_ARGB8888, > diff --git a/drivers/gpu/drm/vkms/vkms_plane.h b/drivers/gpu/drm/vkms/vkms_plane.h > new file mode 100644 > index 000000000000..161b44da0240 > --- /dev/null > +++ b/drivers/gpu/drm/vkms/vkms_plane.h > @@ -0,0 +1,65 @@ > +/* SPDX-License-Identifier: GPL-2.0+ */ > + > +#ifndef _VKMS_PLANE_H > +#define _VKMS_PLANE_H #ifndef _VKMS_PLANE_H_ #define _VKMS_PLANE_H_ > + > +#include <drm/drm_framebuffer.h> > +#include <drm/drm_plane.h> > +#include <drm/drm_gem_atomic_helper.h> > +#include <linux/iosys-map.h> > + > +#include "vkms_drv.h" > +#include "vkms_formats.h" While including "vkms_drv.h" from other headers should be fine > + > +struct vkms_plane { > + struct drm_plane base; > +}; > + > +/** > + * struct vkms_plane_state - Driver specific plane state > + * @base: base plane state > + * @frame_info: data required for composing computation > + * @pixel_read_line: function to read a pixel line in this plane. The creator of a vkms_plane_state > + * must ensure that this pointer is valid > + * @conversion_matrix: matrix used for yuv formats to convert to rgb > + */ > +struct vkms_plane_state { > + struct drm_shadow_plane_state base; > + struct vkms_frame_info *frame_info; > + pixel_read_line_t pixel_read_line; > + struct conversion_matrix conversion_matrix; > +}; > + > +/** > + * struct vkms_frame_info - structure to store the state of a frame > + * > + * @fb: backing drm framebuffer > + * @src: source rectangle of this frame in the source framebuffer, stored in 16.16 fixed-point form > + * @dst: destination rectangle in the crtc buffer, stored in whole pixel units > + * @map: see drm_shadow_plane_state@data > + * @rotation: rotation applied to the source. > + * > + * @src and @dst should have the same size modulo the rotation. > + */ > +struct vkms_frame_info { > + struct drm_framebuffer *fb; > + struct drm_rect src, dst; > + struct iosys_map map[DRM_FORMAT_MAX_PLANES]; > + unsigned int rotation; > +}; > + > +/** > + * vkms_plane_init() - Initialize a plane > + * > + * @vkmsdev: vkms device containing the plane > + * @type: type of plane to initialize > + * @possible_crtc_index: Crtc which can be attached to the plane. The caller must ensure that > + * possible_crtc_index is positive and less or equals to 31. > + */ > +struct vkms_plane *vkms_plane_init(struct vkms_device *vkmsdev, > + enum drm_plane_type type, int possible_crtc_index); > + > +#define to_vkms_plane_state(target)\ > + container_of(target, struct vkms_plane_state, base.base) > + > +#endif //_VKMS_PLANE_H #endif /* _VKMS_PLANE_H_ */ > diff --git a/drivers/gpu/drm/vkms/vkms_writeback.c b/drivers/gpu/drm/vkms/vkms_writeback.c > index 4a830a4c4d64..740d9e2f3d71 100644 > --- a/drivers/gpu/drm/vkms/vkms_writeback.c > +++ b/drivers/gpu/drm/vkms/vkms_writeback.c > @@ -12,7 +12,6 @@ > #include <drm/drm_gem_shmem_helper.h> > #include <drm/drm_framebuffer.h> > > -#include "vkms_drv.h" > #include "vkms_writeback.h" > #include "vkms_formats.h" > > diff --git a/drivers/gpu/drm/vkms/vkms_writeback.h b/drivers/gpu/drm/vkms/vkms_writeback.h > index 70f0c4c26c23..44dff15faff6 100644 > --- a/drivers/gpu/drm/vkms/vkms_writeback.h > +++ b/drivers/gpu/drm/vkms/vkms_writeback.h > @@ -5,6 +5,7 @@ > > #include "vkms_drv.h" > #include "vkms_formats.h" > +#include "vkms_plane.h" > > struct vkms_crtc; > >