Re: [RFC 1/4] drm: Add writeback connector type

[Sean Paul]

On Fri, Feb 23, 2018 at 04:48:58PM +, Liviu Dudau wrote:
> On Fri, Feb 23, 2018 at 11:43:29AM -0500, Sean Paul wrote:
> > On Fri, Feb 23, 2018 at 11:25:11AM -0500, Rob Clark wrote:
> > > On Fri, Feb 23, 2018 at 10:59 AM, Sean Paul <seanp...@chromium.org> wrote:
> > > >
> > > > Have we considered hiding writeback behind a client cap instead?
> > > 
> > > It is kinda *almost* unneeded, since the connector reports itself as
> > > disconnected.
> > > 
> > > I'm not sure what the reason was to drop the cap, but I think it would
> > > be better to have a cap so WB connectors don't show up in, for ex,
> > > xrandr
> > 
> > Yeah, the disconnected hack is kind of gross, IMO. I hate to introduce 
> > churn in
> > the patch series given that it was initially introduced with the client cap.
> 
> Haha, that's the reverse of Daniel's position:
> 
> https://lists.freedesktop.org/archives/dri-devel/2016-October/120519.html

Yeah, it happens :(. I don't think it's a dealbreaker either way, it just seems
awkward to expose a connector which is "disconnected", but available for use. I
don't think we have any other connectors which are supposed to be used in this
state.

> 
> > 
> > There are also cases where we might want to make writeback unavailable, 
> > such as
> > when content protection is enabled. In those cases, it's conceivable that we
> > might want to use disconnected as a signal to u/s. I suppose we could also 
> > just
> > fail the check, so most of this is just academic.
> 
> Not sure what other hardware out there does, but on Mali DP's case you
> would be outputing the protected content by putting the display
> processor in secure mode, which automatically disables writeback for us.
> Or to put in another way, you don't need a writeback framebuffer if you
> are in non-secure mode as you can get access to the framebuffer used for
> the plane anyway.

Yeah, I was mostly thinking about the case where you might have HDCP enabled on
the HDMI connector and be able to slurp up the content via a writeback. However
if the buffer is not secure in the first place, then it's already accessible in
userspace so I don't think that writeback presents a new security hole.

/me needs to get HDCP out of his head.

Sean


> 
> Best regards,
> Liviu
> 
> > 
> > Sean
> > 
> > 
> > > 
> > > BR,
> > > -R
> > 
> > -- 
> > Sean Paul, Software Engineer, Google / Chromium OS
> 
> -- 
> 
> | I would like to |
> | fix the world,  |
> | but they're not |
> | giving me the   |
>  \ source code!  /
>   ---
> ¯\_(ツ)_/¯

-- 
Sean Paul, Software Engineer, Google / Chromium OS
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 1/4] drm: Add writeback connector type

[Sean Paul]

On Fri, Feb 23, 2018 at 11:25:11AM -0500, Rob Clark wrote:
> On Fri, Feb 23, 2018 at 10:59 AM, Sean Paul <seanp...@chromium.org> wrote:
> >
> > Have we considered hiding writeback behind a client cap instead?
> 
> It is kinda *almost* unneeded, since the connector reports itself as
> disconnected.
> 
> I'm not sure what the reason was to drop the cap, but I think it would
> be better to have a cap so WB connectors don't show up in, for ex,
> xrandr

Yeah, the disconnected hack is kind of gross, IMO. I hate to introduce churn in
the patch series given that it was initially introduced with the client cap.

There are also cases where we might want to make writeback unavailable, such as
when content protection is enabled. In those cases, it's conceivable that we
might want to use disconnected as a signal to u/s. I suppose we could also just
fail the check, so most of this is just academic.

Sean


> 
> BR,
> -R

-- 
Sean Paul, Software Engineer, Google / Chromium OS
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 1/4] drm: Add writeback connector type

[Sean Paul]

On Fri, Feb 23, 2018 at 04:21:05PM +, Liviu Dudau wrote:
> On Fri, Feb 23, 2018 at 10:59:35AM -0500, Sean Paul wrote:
> > On Fri, Feb 23, 2018 at 08:17:51AM -0500, Rob Clark wrote:
> > > From: Brian Starkey <brian.star...@arm.com>
> > > 
> > > Writeback connectors represent writeback engines which can write the
> > > CRTC output to a memory framebuffer. Add a writeback connector type and
> > > related support functions.
> > > 
> > > Drivers should initialize a writeback connector with
> > > drm_writeback_connector_init() which takes care of setting up all the
> > > writeback-specific details on top of the normal functionality of
> > > drm_connector_init().
> > > 
> > > Writeback connectors have a WRITEBACK_FB_ID property, used to set the
> > > output framebuffer, and a WRITEBACK_PIXEL_FORMATS blob used to expose the
> > > supported writeback formats to userspace.
> > > 
> > > When a framebuffer is attached to a writeback connector with the
> > > WRITEBACK_FB_ID property, it is used only once (for the commit in which
> > > it was included), and userspace can never read back the value of
> > > WRITEBACK_FB_ID. WRITEBACK_FB_ID can only be set if the connector is
> > > attached to a CRTC.
> > > 
> > > Changes since v1:
> > >  - Added drm_writeback.c + documentation
> > >  - Added helper to initialize writeback connector in one go
> > >  - Added core checks
> > >  - Squashed into a single commit
> > >  - Dropped the client cap
> > >  - Writeback framebuffers are no longer persistent
> > > 
> > > Changes since v2:
> > >  Daniel Vetter:
> > >  - Subclass drm_connector to drm_writeback_connector
> > >  - Relax check to allow CRTC to be set without an FB
> > >  - Add some writeback_ prefixes
> > >  - Drop PIXEL_FORMATS_SIZE property, as it was unnecessary
> > >  Gustavo Padovan:
> > >  - Add drm_writeback_job to handle writeback signalling centrally
> > > 
> > > Changes since v3:
> > >  - Rebased
> > >  - Rename PIXEL_FORMATS -> WRITEBACK_PIXEL_FORMATS
> > > 
> > > Changes since v4:
> > >  - Added atomic_commit() vfunc to connector helper funcs, so that
> > >writeback jobs are committed from atomic helpers
> > > 
> > > Signed-off-by: Brian Starkey <brian.star...@arm.com>
> > > [rebased and fixed conflicts]
> > > Signed-off-by: Mihail Atanassov <mihail.atanas...@arm.com>
> > > Signed-off-by: Liviu Dudau <liviu.du...@arm.com>
> > > [rebased and added atomic_commit() vfunc for writeback jobs]
> > > Signed-off-by: Rob Clark <robdcl...@gmail.com>
> > > ---
> > >  Documentation/gpu/drm-kms.rst|   9 ++
> > >  drivers/gpu/drm/Makefile |   2 +-
> > >  drivers/gpu/drm/drm_atomic.c | 130 
> > >  drivers/gpu/drm/drm_atomic_helper.c  |  30 
> > >  drivers/gpu/drm/drm_connector.c  |   4 +-
> > >  drivers/gpu/drm/drm_writeback.c  | 257 
> > > +++
> > >  include/drm/drm_atomic.h |   3 +
> > >  include/drm/drm_connector.h  |  13 ++
> > >  include/drm/drm_mode_config.h|  14 ++
> > >  include/drm/drm_modeset_helper_vtables.h |  11 ++
> > >  include/drm/drm_writeback.h  |  89 +++
> > >  include/uapi/drm/drm_mode.h  |   1 +
> > >  12 files changed, 561 insertions(+), 2 deletions(-)
> > >  create mode 100644 drivers/gpu/drm/drm_writeback.c
> > >  create mode 100644 include/drm/drm_writeback.h
> > > 
> > > diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
> > > index 2dcf5b42015d..e7590dd2f71e 100644
> > > --- a/Documentation/gpu/drm-kms.rst
> > > +++ b/Documentation/gpu/drm-kms.rst
> > > @@ -370,6 +370,15 @@ Connector Functions Reference
> > >  .. kernel-doc:: drivers/gpu/drm/drm_connector.c
> > > :export:
> > >  
> > > +Writeback Connectors
> > > +
> > > +
> > > +.. kernel-doc:: drivers/gpu/drm/drm_writeback.c
> > > +  :doc: overview
> > > +
> > > +.. kernel-doc:: drivers/gpu/drm/drm_writeback.c
> > > +  :export:
> > > +
> > >  Encoder Abstraction
> > >  ===
> > >  
> > > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Mak

Re: [RFC 1/4] drm: Add writeback connector type

[Sean Paul]

ack_job: Writeback job for writeback connectors
> +  *
> +  * Holds the framebuffer for a writeback connector. As the writeback
> +  * completion may be asynchronous to the normal commit cycle, the
> +  * writeback job lifetime is managed separately from the normal atomic
> +  * state by this object.
> +  *
> +  * See also: drm_writeback_queue_job() and
> +  * drm_writeback_signal_completion()
> +  */
> + struct drm_writeback_job *writeback_job;
>  };
>  
>  /**
> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> index 7569f22ffef6..c012e1148ec0 100644
> --- a/include/drm/drm_mode_config.h
> +++ b/include/drm/drm_mode_config.h
> @@ -779,6 +779,20 @@ struct drm_mode_config {
>*/
>   struct drm_property *panel_orientation_property;
>  
> + /**
> +  * @writeback_fb_id_property: Property for writeback connectors, storing
> +  * the ID of the output framebuffer.
> +  * See also: drm_writeback_connector_init()
> +  */
> + struct drm_property *writeback_fb_id_property;
> + /**
> +  * @writeback_pixel_formats_property: Property for writeback connectors,
> +  * storing an array of the supported pixel formats for the writeback
> +  * engine (read-only).
> +  * See also: drm_writeback_connector_init()
> +  */
> + struct drm_property *writeback_pixel_formats_property;
> +
>   /* dumb ioctl parameters */
>   uint32_t preferred_depth, prefer_shadow;
>  
> diff --git a/include/drm/drm_modeset_helper_vtables.h 
> b/include/drm/drm_modeset_helper_vtables.h
> index 3e76ca805b0f..97d3a810bc85 100644
> --- a/include/drm/drm_modeset_helper_vtables.h
> +++ b/include/drm/drm_modeset_helper_vtables.h
> @@ -974,6 +974,17 @@ struct drm_connector_helper_funcs {
>*/
>   int (*atomic_check)(struct drm_connector *connector,
>   struct drm_connector_state *state);
> +
> + /**
> +  * @atomic_commit:
> +  *
> +  * For write-back connectors, this is the point to commit the write-
> +  * back job to hw.
> +  *
> +  * This callback is used by the atomic modeset helpers.
> +  */
> + void (*atomic_commit)(struct drm_connector *connector,
> +   struct drm_writeback_job *writeback_job);
>  };
>  
>  /**
> diff --git a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h
> new file mode 100644
> index ..0bb95fd4907d
> --- /dev/null
> +++ b/include/drm/drm_writeback.h
> @@ -0,0 +1,89 @@
> +/*
> + * (C) COPYRIGHT 2016 ARM Limited. All rights reserved.
> + * Author: Brian Starkey <brian.star...@arm.com>
> + *
> + * This program is free software and is provided to you under the terms of 
> the
> + * GNU General Public License version 2 as published by the Free Software
> + * Foundation, and any use by you of this program is subject to the terms
> + * of such GNU licence.
> + */
> +
> +#ifndef __DRM_WRITEBACK_H__
> +#define __DRM_WRITEBACK_H__
> +#include 
> +#include 
> +#include 
> +
> +struct drm_writeback_connector {
> + struct drm_connector base;
> +
> + /**
> +  * @encoder: Internal encoder used by the connector to fulfill
> +  * the DRM framework requirements. The users of the
> +  * @drm_writeback_connector control the behaviour of the @encoder
> +  * by passing the @enc_funcs parameter to drm_writeback_connector_init()
> +  * function.
> +  */
> + struct drm_encoder encoder;
> +
> + /**
> +  * @pixel_formats_blob_ptr:
> +  *
> +  * DRM blob property data for the pixel formats list on writeback
> +  * connectors
> +  * See also drm_writeback_connector_init()
> +  */
> + struct drm_property_blob *pixel_formats_blob_ptr;
> +
> + /** @job_lock: Protects job_queue */
> + spinlock_t job_lock;
> +
> + /**
> +  * @job_queue:
> +  *
> +  * Holds a list of a connector's writeback jobs; the last item is the
> +  * most recent. The first item may be either waiting for the hardware
> +  * to begin writing, or currently being written.
> +  *
> +  * See also: drm_writeback_queue_job() and
> +  * drm_writeback_signal_completion()
> +  */
> + struct list_head job_queue;
> +};
> +#define to_wb_connector(x) container_of(x, struct drm_writeback_connector, 
> base)
> +
> +struct drm_writeback_job {
> + /**
> +  * @cleanup_work:
> +  *
> +  * Used to allow drm_writeback_signal_completion to defer dropping the
> +  * framebuffer reference to a workqueue.
> +  */
> + struct work_struct cleanup_work;
> + /**
> +  * @list_entry:
> +  *
> +  * List item for the connector's @job_queue
> +  */
> + struct list_head list_entry;
> + /**
> +  * @fb:
> +  *
> +  * Framebuffer to be written to by the writeback connector. Do not set
> +  * directly, use drm_atomic_set_writeback_fb_for_connector()
> +  */
> + struct drm_framebuffer *fb;
> +};
> +
> +int drm_writeback_connector_init(struct drm_device *dev,
> +  struct drm_writeback_connector *wb_connector,
> +  const struct drm_connector_funcs *con_funcs,
> +  const struct drm_encoder_helper_funcs 
> *enc_helper_funcs,
> +  const u32 *formats, int n_formats);
> +
> +void drm_writeback_queue_job(struct drm_writeback_connector *wb_connector,
> +  struct drm_writeback_job *job);
> +
> +void drm_writeback_cleanup_job(struct drm_writeback_job *job);
> +void drm_writeback_signal_completion(struct drm_writeback_connector 
> *wb_connector);
> +#endif
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index 2c575794fb52..7b47e184e95e 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -338,6 +338,7 @@ enum drm_mode_subconnector {
>  #define DRM_MODE_CONNECTOR_VIRTUAL  15
>  #define DRM_MODE_CONNECTOR_DSI   16
>  #define DRM_MODE_CONNECTOR_DPI   17
> +#define DRM_MODE_CONNECTOR_WRITEBACK 18
>  
>  struct drm_mode_get_connector {
>  
> -- 
> 2.14.3
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] drm/todo: Add s/dev_*/DRM_DEV_*/ coversion to TODO

[Sean Paul]

On Fri, Sep 8, 2017 at 10:32 AM, Sean Paul <seanp...@chromium.org> wrote:
> Now that we have the DRM_DEV_* variants, we should use them.
>
> Signed-off-by: Sean Paul <seanp...@chromium.org>


Applied to -misc-next with danvet's R-b.

Sean

> ---
>  Documentation/gpu/todo.rst | 11 +++
>  1 file changed, 11 insertions(+)
>
> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
> index 22af55d06ab8..e3b622094bf4 100644
> --- a/Documentation/gpu/todo.rst
> +++ b/Documentation/gpu/todo.rst
> @@ -177,6 +177,17 @@ following drivers still use ``struct_mutex``: ``msm``, 
> ``omapdrm`` and
>
>  Contact: Daniel Vetter, respective driver maintainers
>
> +Convert instances of dev_info/dev_err/dev_warn to their DRM_DEV_* equivalent
> +
> +
> +For drivers which could have multiple instances, it is necessary to
> +differentiate between which is which in the logs. Since DRM_INFO/WARN/ERROR
> +don't do this, drivers used dev_info/warn/err to make this differentiation. 
> We
> +now have DRM_DEV_* variants of the drm print macros, so we can start to 
> convert
> +those drivers back to using drm-formwatted specific log messages.
> +
> +Contact: Sean Paul, Maintainer of the driver you plan to convert
> +
>  Core refactorings
>  =
>
> --
> 2.14.1.581.gf28d330327-goog
>
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] drm/todo: Add s/dev_*/DRM_DEV_*/ coversion to TODO

[Sean Paul]

On Fri, Sep 8, 2017 at 11:44 AM, Daniel Vetter <dan...@ffwll.ch> wrote:
> On Fri, Sep 08, 2017 at 10:32:07AM -0400, Sean Paul wrote:
>> Now that we have the DRM_DEV_* variants, we should use them.
>>
>> Signed-off-by: Sean Paul <seanp...@chromium.org>
>
> I think that's fairly ideal fodder for outreachy, assume we make sure the
> driver maintainer is ok with the resulting torrent of patches first :-)

I'll happily review patches that convert rockchip.

Sean

> -Daniel
>
>> ---
>>  Documentation/gpu/todo.rst | 11 +++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
>> index 22af55d06ab8..e3b622094bf4 100644
>> --- a/Documentation/gpu/todo.rst
>> +++ b/Documentation/gpu/todo.rst
>> @@ -177,6 +177,17 @@ following drivers still use ``struct_mutex``: ``msm``, 
>> ``omapdrm`` and
>>
>>  Contact: Daniel Vetter, respective driver maintainers
>>
>> +Convert instances of dev_info/dev_err/dev_warn to their DRM_DEV_* equivalent
>> +
>> +
>> +For drivers which could have multiple instances, it is necessary to
>> +differentiate between which is which in the logs. Since DRM_INFO/WARN/ERROR
>> +don't do this, drivers used dev_info/warn/err to make this differentiation. 
>> We
>> +now have DRM_DEV_* variants of the drm print macros, so we can start to 
>> convert
>> +those drivers back to using drm-formwatted specific log messages.
>> +
>> +Contact: Sean Paul, Maintainer of the driver you plan to convert
>> +
>>  Core refactorings
>>  =
>>
>> --
>> 2.14.1.581.gf28d330327-goog
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-doc" in
>> the body of a message to majord...@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] drm/todo: Add s/dev_*/DRM_DEV_*/ coversion to TODO

[Sean Paul]

Now that we have the DRM_DEV_* variants, we should use them.

Signed-off-by: Sean Paul <seanp...@chromium.org>
---
 Documentation/gpu/todo.rst | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index 22af55d06ab8..e3b622094bf4 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -177,6 +177,17 @@ following drivers still use ``struct_mutex``: ``msm``, 
``omapdrm`` and
 
 Contact: Daniel Vetter, respective driver maintainers
 
+Convert instances of dev_info/dev_err/dev_warn to their DRM_DEV_* equivalent
+
+
+For drivers which could have multiple instances, it is necessary to
+differentiate between which is which in the logs. Since DRM_INFO/WARN/ERROR
+don't do this, drivers used dev_info/warn/err to make this differentiation. We
+now have DRM_DEV_* variants of the drm print macros, so we can start to convert
+those drivers back to using drm-formwatted specific log messages.
+
+Contact: Sean Paul, Maintainer of the driver you plan to convert
+
 Core refactorings
 =
 
-- 
2.14.1.581.gf28d330327-goog

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] drm/doc: Document feature merge deadlines

[Sean Paul]

On Tue, Mar 21, 2017 at 04:52:28PM +0100, Daniel Vetter wrote:
> The discussion pretty much concluded without objections, let's
> document what we agreed on.
> 
> Cc'ing linux-doc for the new tag in Documentation/process/index.rst.
> 
> Cc: Jonathan Corbet <cor...@lwn.net>
> Cc: linux-doc@vger.kernel.org
> Cc: Dave Airlie <airl...@gmail.com>
> Cc: Sean Paul <seanp...@chromium.org>
> Cc: Jani Nikula <jani.nik...@linux.intel.com>
> Cc: Alex Deucher <alexdeuc...@gmail.com>
> Cc: Lukas Wunner <lu...@wunner.de>
> Signed-off-by: Daniel Vetter <daniel.vet...@intel.com>

Reviewed-by: Sean Paul <seanp...@chromium.org>

> ---
>  Documentation/gpu/introduction.rst | 25 +
>  Documentation/process/index.rst|  1 +
>  2 files changed, 26 insertions(+)
> 
> diff --git a/Documentation/gpu/introduction.rst 
> b/Documentation/gpu/introduction.rst
> index 1f8bd5ef5f9d..05a82bdfbca4 100644
> --- a/Documentation/gpu/introduction.rst
> +++ b/Documentation/gpu/introduction.rst
> @@ -60,3 +60,28 @@ checkpatch or sparse. We welcome such contributions.
>  
>  Anyone looking to kick it up a notch can find a list of janitorial tasks on
>  the :ref:`TODO list `.
> +
> +Contribution Process
> +
> +
> +Mostly the DRM subsystem works like any other kernel subsystem, see :ref:`the
> +main process guidelines and documentation ` for how things 
> work.
> +Here we just document some of the specialities of the GPU subsystem.
> +
> +Feature Merge Deadlines
> +---
> +
> +All feature work must be in the linux-next tree by the -rc6 release of the
> +current release cycle, otherwise they must be postponed and can't reach the 
> next
> +merge window. All patches must have landed in the drm-next tree by latest 
> -rc7,
> +but if your branch is not in linux-next then this must have happened by -rc6
> +already.
> +
> +After that point only bugfixes (like after the upstream merge window has 
> closed
> +with the -rc1 release) are allowed. No new platform enabling or new drivers 
> are
> +allowed.
> +
> +This means that there's a blackout-period of about one month where feature 
> work
> +can't be merged. The recommended way to deal with that is having a -next tree
> +that's always open, but making sure to not feed it into linux-next during the
> +blackout period. As an example, drm-misc works like that.
> diff --git a/Documentation/process/index.rst b/Documentation/process/index.rst
> index 10aa6920709a..82fc399fcd33 100644
> --- a/Documentation/process/index.rst
> +++ b/Documentation/process/index.rst
> @@ -3,6 +3,7 @@
>   \renewcommand\thesection*
>   \renewcommand\thesubsection*
>  
> +.. _process_index:
>  
>  Working with the kernel development community
>  =
> -- 
> 2.11.0

-- 
Sean Paul, Software Engineer, Google / Chromium OS
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 20/21] vgaarbiter: rst-ifiy and polish kerneldoc

[Sean Paul]

On Fri, Aug 12, 2016 at 4:48 PM, Daniel Vetter <daniel.vet...@ffwll.ch> wrote:
> Move the documentation into Documentation/gpu, link it up and pull in
> the kernel doc.
>
> No actual text changes except that I did polish the kerneldoc a bit,
> especially for vga_client_register().
>
> v2: Remove some rst from vga-switcheroo.rst that I don't understand,
> but which seems to be the reason why the new vgaarbiter.rst sometimes
> drops out of the sidebar index.
>
> v3: Drop one level of headings and clarify the vgaarb one a bit.
>
> v4: Fix some typos (Sean).
>
> Cc: Jonathan Corbet <cor...@lwn.net>
> Cc: linux-doc@vger.kernel.org
> Cc: Sean Paul <seanp...@chromium.org>
> Signed-off-by: Daniel Vetter <daniel.vet...@intel.com>

Reviewed-by: Sean Paul <seanp...@chromium.org>

> ---
>  Documentation/gpu/index.rst  |   1 +
>  Documentation/gpu/vga-switcheroo.rst |   2 -
>  Documentation/gpu/vgaarbiter.rst | 191 ++
>  Documentation/vgaarbiter.txt | 192 
> ---
>  drivers/gpu/vga/vgaarb.c | 110 +++-
>  include/linux/vgaarb.h   | 128 +++
>  6 files changed, 316 insertions(+), 308 deletions(-)
>  create mode 100644 Documentation/gpu/vgaarbiter.rst
>  delete mode 100644 Documentation/vgaarbiter.txt
>
> diff --git a/Documentation/gpu/index.rst b/Documentation/gpu/index.rst
> index fcac0fa72056..ba92f45abb76 100644
> --- a/Documentation/gpu/index.rst
> +++ b/Documentation/gpu/index.rst
> @@ -12,3 +12,4 @@ Linux GPU Driver Developer's Guide
> drm-uapi
> i915
> vga-switcheroo
> +   vgaarbiter
> diff --git a/Documentation/gpu/vga-switcheroo.rst 
> b/Documentation/gpu/vga-switcheroo.rst
> index cbbdb994f1dd..463a74fc40d1 100644
> --- a/Documentation/gpu/vga-switcheroo.rst
> +++ b/Documentation/gpu/vga-switcheroo.rst
> @@ -1,5 +1,3 @@
> -.. _vga_switcheroo:
> -
>  ==
>  VGA Switcheroo
>  ==
> diff --git a/Documentation/gpu/vgaarbiter.rst 
> b/Documentation/gpu/vgaarbiter.rst
> new file mode 100644
> index ..0b41b051d021
> --- /dev/null
> +++ b/Documentation/gpu/vgaarbiter.rst
> @@ -0,0 +1,191 @@
> +===
> +VGA Arbiter
> +===
> +
> +Graphic devices are accessed through ranges in I/O or memory space. While 
> most
> +modern devices allow relocation of such ranges, some "Legacy" VGA devices
> +implemented on PCI will typically have the same "hard-decoded" addresses as
> +they did on ISA. For more details see "PCI Bus Binding to IEEE Std 1275-1994
> +Standard for Boot (Initialization Configuration) Firmware Revision 2.1"
> +Section 7, Legacy Devices.
> +
> +The Resource Access Control (RAC) module inside the X server [0] existed for
> +the legacy VGA arbitration task (besides other bus management tasks) when 
> more
> +than one legacy device co-exists on the same machine. But the problem happens
> +when these devices are trying to be accessed by different userspace clients
> +(e.g. two server in parallel). Their address assignments conflict. Moreover,
> +ideally, being a userspace application, it is not the role of the X server to
> +control bus resources. Therefore an arbitration scheme outside of the X 
> server
> +is needed to control the sharing of these resources. This document introduces
> +the operation of the VGA arbiter implemented for the Linux kernel.
> +
> +vgaarb kernel/userspace ABI
> +---
> +
> +The vgaarb is a module of the Linux Kernel. When it is initially loaded, it
> +scans all PCI devices and adds the VGA ones inside the arbitration. The
> +arbiter then enables/disables the decoding on different devices of the VGA
> +legacy instructions. Devices which do not want/need to use the arbiter may
> +explicitly tell it by calling vga_set_legacy_decoding().
> +
> +The kernel exports a char device interface (/dev/vga_arbiter) to the clients,
> +which has the following semantics:
> +
> +open
> +Opens a user instance of the arbiter. By default, it's attached to 
> the
> +default VGA device of the system.
> +
> +close
> +Close a user instance. Release locks made by the user
> +
> +read
> +Return a string indicating the status of the target like:
> +
> +",decodes=,owns=,locks= 
> (ic,mc)"
> +
> +An IO state string is of the form {io,mem,io+mem,none}, mc and
> +ic are respectively mem and io lock counts (for debugging/
> +diagnostic only). "decodes" indicate what the card currently
> +decodes, "owns&qu

Re: [PATCH] vgaarbiter: rst-ifiy and polish kerneldoc

[Sean Paul]

On Tue, Aug 9, 2016 at 9:50 AM, Daniel Vetter  wrote:
> Move the documentation into Documentation/gpu, link it up and pull in
> the kernel doc.
>
> No actual text changes except that I did polish the kerneldoc a bit,
> especially for vga_client_register().
>
> v2: Remove some rst from vga-switcheroo.rst that I don't understand,
> but which seems to be the reason why the new vgaarbiter.rst sometimes
> drops out of the sidebar index.
>
> v3: Drop one level of headings and clarify the vgaarb one a bit.
>
> Cc: Jonathan Corbet 
> Cc: linux-doc@vger.kernel.org
> Signed-off-by: Daniel Vetter 
> ---
>  Documentation/gpu/index.rst  |   1 +
>  Documentation/gpu/vga-switcheroo.rst |   2 -
>  Documentation/gpu/vgaarbiter.rst | 191 ++
>  Documentation/vgaarbiter.txt | 192 
> ---
>  drivers/gpu/vga/vgaarb.c | 103 ++-
>  include/linux/vgaarb.h   | 128 +++
>  6 files changed, 309 insertions(+), 308 deletions(-)
>  create mode 100644 Documentation/gpu/vgaarbiter.rst
>  delete mode 100644 Documentation/vgaarbiter.txt
>
> diff --git a/Documentation/gpu/index.rst b/Documentation/gpu/index.rst
> index fcac0fa72056..ba92f45abb76 100644
> --- a/Documentation/gpu/index.rst
> +++ b/Documentation/gpu/index.rst
> @@ -12,3 +12,4 @@ Linux GPU Driver Developer's Guide
> drm-uapi
> i915
> vga-switcheroo
> +   vgaarbiter
> diff --git a/Documentation/gpu/vga-switcheroo.rst 
> b/Documentation/gpu/vga-switcheroo.rst
> index cbbdb994f1dd..463a74fc40d1 100644
> --- a/Documentation/gpu/vga-switcheroo.rst
> +++ b/Documentation/gpu/vga-switcheroo.rst
> @@ -1,5 +1,3 @@
> -.. _vga_switcheroo:
> -
>  ==
>  VGA Switcheroo
>  ==
> diff --git a/Documentation/gpu/vgaarbiter.rst 
> b/Documentation/gpu/vgaarbiter.rst
> new file mode 100644
> index ..0b41b051d021
> --- /dev/null
> +++ b/Documentation/gpu/vgaarbiter.rst
> @@ -0,0 +1,191 @@
> +===
> +VGA Arbiter
> +===
> +
> +Graphic devices are accessed through ranges in I/O or memory space. While 
> most
> +modern devices allow relocation of such ranges, some "Legacy" VGA devices
> +implemented on PCI will typically have the same "hard-decoded" addresses as
> +they did on ISA. For more details see "PCI Bus Binding to IEEE Std 1275-1994
> +Standard for Boot (Initialization Configuration) Firmware Revision 2.1"
> +Section 7, Legacy Devices.
> +
> +The Resource Access Control (RAC) module inside the X server [0] existed for
> +the legacy VGA arbitration task (besides other bus management tasks) when 
> more
> +than one legacy device co-exists on the same machine. But the problem happens
> +when these devices are trying to be accessed by different userspace clients
> +(e.g. two server in parallel). Their address assignments conflict. Moreover,
> +ideally, being a userspace application, it is not the role of the X server to
> +control bus resources. Therefore an arbitration scheme outside of the X 
> server
> +is needed to control the sharing of these resources. This document introduces
> +the operation of the VGA arbiter implemented for the Linux kernel.
> +
> +vgaarb kernel/userspace ABI
> +---
> +
> +The vgaarb is a module of the Linux Kernel. When it is initially loaded, it
> +scans all PCI devices and adds the VGA ones inside the arbitration. The
> +arbiter then enables/disables the decoding on different devices of the VGA
> +legacy instructions. Devices which do not want/need to use the arbiter may
> +explicitly tell it by calling vga_set_legacy_decoding().
> +
> +The kernel exports a char device interface (/dev/vga_arbiter) to the clients,
> +which has the following semantics:
> +
> +open
> +Opens a user instance of the arbiter. By default, it's attached to 
> the
> +default VGA device of the system.
> +
> +close
> +Close a user instance. Release locks made by the user
> +
> +read
> +Return a string indicating the status of the target like:
> +
> +",decodes=,owns=,locks= 
> (ic,mc)"
> +
> +An IO state string is of the form {io,mem,io+mem,none}, mc and
> +ic are respectively mem and io lock counts (for debugging/
> +diagnostic only). "decodes" indicate what the card currently
> +decodes, "owns" indicates what is currently enabled on it, and
> +"locks" indicates what is locked by this card. If the card is
> +unplugged, we get "invalid" then for card_ID and an -ENODEV
> +error is returned for any command until a new card is targeted.
> +
> +
> +write
> +Write a command to the arbiter. List of commands:
> +
> +target 
> +switch target to card  (see below)
> +lock 
> +acquires locks on target ("none" is an invalid io_state)
> +trylock 
> +