mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "José Expósito" <jose.exposito89@gmail.com>
To: louis.chauvet@bootlin.com
Cc: airlied@gmail.com, arthurgrillo@riseup.net, daniel@ffwll.ch,
	dri-devel@lists.freedesktop.org, hamohammed.sa@gmail.com,
	jeremie.dautheribes@bootlin.com, linux-kernel@vger.kernel.org,
	maarten.lankhorst@linux.intel.com, mairacanal@riseup.net,
	marcheu@google.com, melissa.srw@gmail.com,
	miquel.raynal@bootlin.com, mripard@kernel.org,
	nicolejadeyee@google.com, rodrigosiqueiramelo@gmail.com,
	seanpaul@google.com, thomas.petazzoni@bootlin.com,
	tzimmermann@suse.de, "José Expósito" <jose.exposito89@gmail.com>
Subject: [PATCH RFC 3/6] drm/vkms: Extract vkms_plane header
Date: Tue, 20 Aug 2024 17:28:50 +0200	[thread overview]
Message-ID: <20240820152852.2843-1-jose.exposito89@gmail.com> (raw)
In-Reply-To: <20240814-google-split-headers-v1-3-51712f088f5d@bootlin.com>

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

Reviewed-by: José Expósito <jose.exposito89@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@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;
>  
> 

  reply	other threads:[~2024-08-20 15:28 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-14  9:08 [PATCH RFC 0/6] drm/vkms: Completly split headers Louis Chauvet
2024-08-14  9:08 ` [PATCH RFC 1/6] drm/vkms: Properly extract vkms_formats header Louis Chauvet
2024-08-15 14:36   ` Maíra Canal
2024-08-20 15:28   ` José Expósito
2024-08-14  9:08 ` [PATCH RFC 2/6] drm/vkms: Extract vkms_writeback header Louis Chauvet
2024-08-20 15:28   ` José Expósito
2024-08-14  9:08 ` [PATCH RFC 3/6] drm/vkms: Extract vkms_plane header Louis Chauvet
2024-08-20 15:28   ` José Expósito [this message]
2024-08-14  9:08 ` [PATCH RFC 4/6] drm/vkms: Rename to_vkms_plane_state to avoid confusion Louis Chauvet
2024-08-20 15:29   ` José Expósito
2024-08-14  9:08 ` [PATCH RFC 5/6] drm/vkms: Extract vkms_crtc header Louis Chauvet
2024-08-20 15:29   ` José Expósito
2024-08-14  9:08 ` [PATCH RFC 6/6] drm/vkms: Extract vkms_composer header Louis Chauvet
2024-08-20 15:30   ` José Expósito

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240820152852.2843-1-jose.exposito89@gmail.com \
    --to=jose.exposito89@gmail.com \
    --cc=airlied@gmail.com \
    --cc=arthurgrillo@riseup.net \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=hamohammed.sa@gmail.com \
    --cc=jeremie.dautheribes@bootlin.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=louis.chauvet@bootlin.com \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=mairacanal@riseup.net \
    --cc=marcheu@google.com \
    --cc=melissa.srw@gmail.com \
    --cc=miquel.raynal@bootlin.com \
    --cc=mripard@kernel.org \
    --cc=nicolejadeyee@google.com \
    --cc=rodrigosiqueiramelo@gmail.com \
    --cc=seanpaul@google.com \
    --cc=thomas.petazzoni@bootlin.com \
    --cc=tzimmermann@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®