| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 17 May 2016 14:00:45 +0200
From: Noralf Trønnes <noralf@...nnes.org>
To: Daniel Vetter <daniel@...ll.ch>
Cc: Ville Syrjälä <ville.syrjala@...ux.intel.com>,
jsarha@...com, dri-devel@...ts.freedesktop.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 3/3] drm: Add helper for simple display pipeline
Den 17.05.2016 09:59, skrev Daniel Vetter:
> On Tue, May 17, 2016 at 10:46:51AM +0300, Ville Syrjälä wrote:
>> On Tue, May 17, 2016 at 09:05:01AM +0200, Daniel Vetter wrote:
>>> On Thu, May 12, 2016 at 09:36:14PM +0300, Ville Syrjälä wrote:
>>>> On Thu, May 12, 2016 at 08:25:23PM +0200, Noralf Trønnes wrote:
>>>>> Provides helper functions for drivers that have a simple display
>>>>> pipeline. Plane, crtc and encoder are collapsed into one entity.
>>>>>
>>>>> Cc: jsarha@...com
>>>>> Signed-off-by: Noralf Trønnes <noralf@...nnes.org>
>>>>> ---
>>>>>
>>>>> Changes since v3:
>>>>> - (struct drm_simple_display_pipe *)->funcs should be const
>>>>>
>>>>> Changes since v2:
>>>>> - Drop Kconfig knob DRM_KMS_HELPER
>>>>> - Expand documentation
>>>>>
>>>>> Changes since v1:
>>>>> - Add DOC header and add to gpu.tmpl
>>>>> - Fix docs: @funcs is optional, "negative error code",
>>>>> "This hook is optional."
>>>>> - Add checks to drm_simple_kms_plane_atomic_check()
>>>>>
>>>>> Documentation/DocBook/gpu.tmpl | 6 +
>>>>> drivers/gpu/drm/Makefile | 2 +-
>>>>> drivers/gpu/drm/drm_simple_kms_helper.c | 208 ++++++++++++++++++++++++++++++++
>>>>> include/drm/drm_simple_kms_helper.h | 94 +++++++++++++++
>>>>> 4 files changed, 309 insertions(+), 1 deletion(-)
>>>>> create mode 100644 drivers/gpu/drm/drm_simple_kms_helper.c
>>>>> create mode 100644 include/drm/drm_simple_kms_helper.h
>>>>>
>>>>> diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl
>>>>> index 4a0c599..cf3f5a8 100644
>>>>> --- a/Documentation/DocBook/gpu.tmpl
>>>>> +++ b/Documentation/DocBook/gpu.tmpl
>>>>> @@ -1693,6 +1693,12 @@ void intel_crt_init(struct drm_device *dev)
>>>>> !Edrivers/gpu/drm/drm_panel.c
>>>>> !Pdrivers/gpu/drm/drm_panel.c drm panel
>>>>> </sect2>
>>>>> + <sect2>
>>>>> + <title>Simple KMS Helper Reference</title>
>>>>> +!Iinclude/drm/drm_simple_kms_helper.h
>>>>> +!Edrivers/gpu/drm/drm_simple_kms_helper.c
>>>>> +!Pdrivers/gpu/drm/drm_simple_kms_helper.c overview
>>>>> + </sect2>
>>>>> </sect1>
>>>>>
>>>>> <!-- Internals: kms properties -->
>>>>> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
>>>>> index 2bd3e5a..31b85df5 100644
>>>>> --- a/drivers/gpu/drm/Makefile
>>>>> +++ b/drivers/gpu/drm/Makefile
>>>>> @@ -23,7 +23,7 @@ drm-$(CONFIG_AGP) += drm_agpsupport.o
>>>>>
>>>>> drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_probe_helper.o \
>>>>> drm_plane_helper.o drm_dp_mst_topology.o drm_atomic_helper.o \
>>>>> - drm_kms_helper_common.o
>>>>> + drm_kms_helper_common.o drm_simple_kms_helper.o
>>>>>
>>>>> drm_kms_helper-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o
>>>>> drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fb_helper.o
>>>>> diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c
>>>>> new file mode 100644
>>>>> index 0000000..d45417a
>>>>> --- /dev/null
>>>>> +++ b/drivers/gpu/drm/drm_simple_kms_helper.c
>>>>> @@ -0,0 +1,208 @@
>>>>> +/*
>>>>> + * Copyright (C) 2016 Noralf Trønnes
>>>>> + *
>>>>> + * This program is free software; you can redistribute it and/or modify
>>>>> + * it under the terms of the GNU General Public License as published by
>>>>> + * the Free Software Foundation; either version 2 of the License, or
>>>>> + * (at your option) any later version.
>>>>> + */
>>>>> +
>>>>> +#include <drm/drmP.h>
>>>>> +#include <drm/drm_atomic.h>
>>>>> +#include <drm/drm_atomic_helper.h>
>>>>> +#include <drm/drm_crtc_helper.h>
>>>>> +#include <drm/drm_plane_helper.h>
>>>>> +#include <drm/drm_simple_kms_helper.h>
>>>>> +#include <linux/slab.h>
>>>>> +
>>>>> +/**
>>>>> + * DOC: overview
>>>>> + *
>>>>> + * This helper library provides helpers for drivers for simple display
>>>>> + * hardware.
>>>>> + *
>>>>> + * drm_simple_display_pipe_init() initializes a simple display pipeline
>>>>> + * which has only one full-screen scanout buffer feeding one output. The
>>>>> + * pipeline is represented by struct &drm_simple_display_pipe and binds
>>>>> + * together &drm_plane, &drm_crtc and &drm_encoder structures into one fixed
>>>>> + * entity. Some flexibility for code reuse is provided through a separately
>>>>> + * allocated &drm_connector object and supporting optional &drm_bridge
>>>>> + * encoder drivers.
>>>>> + */
>>>>> +
>>>>> +static const struct drm_encoder_funcs drm_simple_kms_encoder_funcs = {
>>>>> + .destroy = drm_encoder_cleanup,
>>>>> +};
>>>>> +
>>>>> +static void drm_simple_kms_crtc_enable(struct drm_crtc *crtc)
>>>>> +{
>>>>> + struct drm_simple_display_pipe *pipe;
>>>>> +
>>>>> + pipe = container_of(crtc, struct drm_simple_display_pipe, crtc);
>>>>> + if (!pipe->funcs || !pipe->funcs->enable)
>>>>> + return;
>>>>> +
>>>>> + pipe->funcs->enable(pipe, crtc->state);
>>>>> +}
>>>>> +
>>>>> +static void drm_simple_kms_crtc_disable(struct drm_crtc *crtc)
>>>>> +{
>>>>> + struct drm_simple_display_pipe *pipe;
>>>>> +
>>>>> + pipe = container_of(crtc, struct drm_simple_display_pipe, crtc);
>>>>> + if (!pipe->funcs || !pipe->funcs->disable)
>>>>> + return;
>>>>> +
>>>>> + pipe->funcs->disable(pipe);
>>>>> +}
>>>>> +
>>>>> +static const struct drm_crtc_helper_funcs drm_simple_kms_crtc_helper_funcs = {
>>>>> + .disable = drm_simple_kms_crtc_disable,
>>>>> + .enable = drm_simple_kms_crtc_enable,
>>>>> +};
>>>>> +
>>>>> +static const struct drm_crtc_funcs drm_simple_kms_crtc_funcs = {
>>>>> + .reset = drm_atomic_helper_crtc_reset,
>>>>> + .destroy = drm_crtc_cleanup,
>>>>> + .set_config = drm_atomic_helper_set_config,
>>>>> + .page_flip = drm_atomic_helper_page_flip,
>>>>> + .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
>>>>> + .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
>>>>> +};
>>>>> +
>>>>> +static int drm_simple_kms_plane_atomic_check(struct drm_plane *plane,
>>>>> + struct drm_plane_state *plane_state)
>>>>> +{
>>>>> + struct drm_rect src = {
>>>>> + .x1 = plane_state->src_x,
>>>>> + .y1 = plane_state->src_y,
>>>>> + .x2 = plane_state->src_x + plane_state->src_w,
>>>>> + .y2 = plane_state->src_y + plane_state->src_h,
>>>>> + };
>>>>> + struct drm_rect dest = {
>>>>> + .x1 = plane_state->crtc_x,
>>>>> + .y1 = plane_state->crtc_y,
>>>>> + .x2 = plane_state->crtc_x + plane_state->crtc_w,
>>>>> + .y2 = plane_state->crtc_y + plane_state->crtc_h,
>>>>> + };
>>>>> + struct drm_rect clip = { 0 };
>>>>> + struct drm_simple_display_pipe *pipe;
>>>>> + struct drm_crtc_state *crtc_state;
>>>>> + bool visible;
>>>>> + int ret;
>>>>> +
>>>>> + pipe = container_of(plane, struct drm_simple_display_pipe, plane);
>>>>> + crtc_state = drm_atomic_get_existing_crtc_state(plane_state->state,
>>>>> + &pipe->crtc);
>>>>> + if (crtc_state->enable != !!plane_state->crtc)
>>>>> + return -EINVAL; /* plane must match crtc enable state */
>>>>> +
>>>>> + if (!crtc_state->enable)
>>>>> + return 0; /* nothing to check when disabling or disabled */
>>>>> +
>>>>> + clip.x2 = crtc_state->adjusted_mode.hdisplay;
>>>>> + clip.y2 = crtc_state->adjusted_mode.vdisplay;
>>>>> + ret = drm_plane_helper_check_update(plane, &pipe->crtc,
>>>>> + plane_state->fb,
>>>>> + &src, &dest, &clip,
>>>>> + DRM_PLANE_HELPER_NO_SCALING,
>>>>> + DRM_PLANE_HELPER_NO_SCALING,
>>>>> + false, true, &visible);
>>>>> + if (ret)
>>>>> + return ret;
>>>>> +
>>>>> + if (!visible)
>>>>> + return -EINVAL;
>>>>> +
>>>>> + if (!pipe->funcs || !pipe->funcs->check)
>>>>> + return 0;
>>>>> +
>>>>> + return pipe->funcs->check(pipe, plane_state, crtc_state);
>>>>> +}
>>>> What's anyone supposed to do with this when the clipped coordinates
>>>> aren't even passed/stored anywhere?
>>> It disallows positioning and scaling, so shouldn't ever need to have the
>>> clipped area?
>> You can still configure a larger area that gets clipped to the
>> fullscreen dimensions.
> Oh right. Noralf, sounds like we need to feed back the clipped rectangle.
> Probably best if we add clipped plane coordinates to drm_plane_state.
Both src and crtc or only src?
if (!visible)
return -EINVAL;
+
+ plane_state->src_x = src.x1;
+ plane_state->src_y = src.y1;
+ plane_state->src_w = drm_rect_width(&src);
+ plane_state->src_h = drm_rect_height(&src);
+
+ plane_state->crtc_x = dest.x1;
+ plane_state->crtc_y = dest.y1;
+ plane_state->crtc_w = drm_rect_width(&dest);
+ plane_state->crtc_h = drm_rect_height(&dest);
if (!pipe->funcs || !pipe->funcs->check)
return 0;
Noralf.
Powered by blists - more mailing lists