On Wed, Aug 31, 2016 at 12:09 PM, Daniel Vetter <daniel.vetter@xxxxxxxx> wrote:
> Just pure code movement, cleanup and polish will happen in later
> patches.
>
> v2: Don't forget all the ioctl! To extract those cleanly I decided to
> put check_src_coords into drm_framebuffer.c (and give it a
> drm_framebuffer_ prefix), since that just checks framebuffer
> constraints.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx>
> ---
> Documentation/gpu/drm-kms.rst | 12 +
> drivers/gpu/drm/Makefile | 3 +-
> drivers/gpu/drm/drm_crtc.c | 939 +-----------------------------------
> drivers/gpu/drm/drm_crtc_internal.h | 38 +-
> drivers/gpu/drm/drm_framebuffer.c | 26 +
> drivers/gpu/drm/drm_plane.c | 937 +++++++++++++++++++++++++++++++++++
> include/drm/drm_atomic.h | 154 ++++++
> include/drm/drm_crtc.h | 583 +---------------------
> include/drm/drm_plane.h | 470 ++++++++++++++++++
> 9 files changed, 1628 insertions(+), 1534 deletions(-)
> create mode 100644 drivers/gpu/drm/drm_plane.c
> create mode 100644 include/drm/drm_plane.h
>
> diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
> index f9a991bb87d4..33181be97151 100644
> --- a/Documentation/gpu/drm-kms.rst
> +++ b/Documentation/gpu/drm-kms.rst
> @@ -110,6 +110,18 @@ Note that dumb objects may not be used for gpu acceleration, as has been
> attempted on some ARM embedded platforms. Such drivers really must have
> a hardware-specific ioctl to allocate suitable buffer objects.
>
> +Plane Abstraction
> +=================
> +
> +Plane Functions Reference
> +-------------------------
> +
> +.. kernel-doc:: include/drm/drm_plane.h
> + :internal:
> +
> +.. kernel-doc:: drivers/gpu/drm/drm_plane.c
> + :export:
> +
> Display Modes Function Reference
> ================================
>
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 439d89b25ae0..8eeb07a35798 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -14,7 +14,8 @@ drm-y := drm_auth.o drm_bufs.o drm_cache.o \
> drm_rect.o drm_vma_manager.o drm_flip_work.o \
> drm_modeset_lock.o drm_atomic.o drm_bridge.o \
> drm_framebuffer.o drm_connector.o drm_blend.o \
> - drm_encoder.o drm_mode_object.o drm_property.o
> + drm_encoder.o drm_mode_object.o drm_property.o \
> + drm_plane.o
>
> drm-$(CONFIG_COMPAT) += drm_ioc32.o
> drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 0fad433f4d2d..513ab4729683 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
<snip>
> -
> -static int check_src_coords(uint32_t src_x, uint32_t src_y,
> - uint32_t src_w, uint32_t src_h,
> - const struct drm_framebuffer *fb)
> -{
> - unsigned int fb_width, fb_height;
> -
> - fb_width = fb->width << 16;
> - fb_height = fb->height << 16;
> -
> - /* Make sure source coordinates are inside the fb. */
> - if (src_w > fb_width ||
> - src_x > fb_width - src_w ||
> - src_h > fb_height ||
> - src_y > fb_height - src_h) {
> - DRM_DEBUG_KMS("Invalid source coordinates "
> - "%u.%06ux%u.%06u+%u.%06u+%u.%06u\n",
> - src_w >> 16, ((src_w & 0xffff) * 15625) >> 10,
> - src_h >> 16, ((src_h & 0xffff) * 15625) >> 10,
> - src_x >> 16, ((src_x & 0xffff) * 15625) >> 10,
> - src_y >> 16, ((src_y & 0xffff) * 15625) >> 10);
> - return -ENOSPC;
> - }
> -
> - return 0;
> -}
I'm good with this change, but I'd argue that it probably belongs in
its own patch.
<snip>
> /**
> - * drm_mode_page_flip_ioctl - schedule an asynchronous fb update
> - * @dev: DRM device
> - * @data: ioctl data
> - * @file_priv: DRM file info
> - *
> - * This schedules an asynchronous update on a given CRTC, called page flip.
> - * Optionally a drm event is generated to signal the completion of the event.
> - * Generic drivers cannot assume that a pageflip with changed framebuffer
> - * properties (including driver specific metadata like tiling layout) will work,
> - * but some drivers support e.g. pixel format changes through the pageflip
> - * ioctl.
> - *
> - * Called by the user via ioctl.
> - *
> - * Returns:
> - * Zero on success, negative errno on failure.
> - */
> -int drm_mode_page_flip_ioctl(struct drm_device *dev,
> - void *data, struct drm_file *file_priv)
IMO, this makes more sense where it is (it's a crtc operation since
the ioctl data doesn't even reference planes). Perhaps it should be
sent out on an icefloat with setplane and other legacy ABI in some
corner.
Sean
<snip>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
