[PATCH RFC 3/6] drm/vkms: Extract vkms_plane header

José Expósito jose.exposito89 at gmail.com
Tue Aug 20 15:28:50 UTC 2024


> In order to properly split vkms_output function, extract all
> its function to its own header.
> 
> Signed-off-by: Louis Chauvet <louis.chauvet at bootlin.com>

Reviewed-by: José Expósito <jose.exposito89 at gmail.com>

> ---
>  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 at 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 at 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;
>  
> 


More information about the dri-devel mailing list