[PATCH 1/3] drm/doc: Move hdmi infoframe docs

[PATCH 1/3] drm/doc: Move hdmi infoframe docs

[Daniel Vetter]

.. next to all the other sink helpers. The rect library is more used
for handling plane clipping, so belongs to those imo.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 Documentation/gpu/drm-kms-helpers.rst | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/Documentation/gpu/drm-kms-helpers.rst b/Documentation/gpu/drm-kms-helpers.rst
index fbd11b2fe5b5..17ca7f8bf3d3 100644
--- a/Documentation/gpu/drm-kms-helpers.rst
+++ b/Documentation/gpu/drm-kms-helpers.rst
@@ -296,18 +296,6 @@ SCDC Helper Functions Reference
 .. kernel-doc:: drivers/gpu/drm/drm_scdc_helper.c
    :export:
 
-Rectangle Utilities Reference
-=============================
-
-.. kernel-doc:: include/drm/drm_rect.h
-   :doc: rect utils
-
-.. kernel-doc:: include/drm/drm_rect.h
-   :internal:
-
-.. kernel-doc:: drivers/gpu/drm/drm_rect.c
-   :export:
-
 HDMI Infoframes Helper Reference
 ================================
 
@@ -322,6 +310,18 @@ libraries and hence is also included here.
 .. kernel-doc:: drivers/video/hdmi.c
    :export:
 
+Rectangle Utilities Reference
+=============================
+
+.. kernel-doc:: include/drm/drm_rect.h
+   :doc: rect utils
+
+.. kernel-doc:: include/drm/drm_rect.h
+   :internal:
+
+.. kernel-doc:: drivers/gpu/drm/drm_rect.c
+   :export:
+
 Flip-work Helper Reference
 ==========================
 
-- 
2.20.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/3] drm/doc: Drop chapter "KMS Initialization and Cleanup"

[Daniel Vetter]

It only talks about crtc, brings up intel as an exampel and I think is
more misleading than useful really. Plus we have lots of discussion
about how your standard kms driver should be initialized/cleaned up,
so maybe better to document this when we have a better idea.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 Documentation/gpu/drm-kms.rst | 96 -----------------------------------
 1 file changed, 96 deletions(-)

diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
index 75c882e09fee..23a3c986ef6d 100644
--- a/Documentation/gpu/drm-kms.rst
+++ b/Documentation/gpu/drm-kms.rst
@@ -410,102 +410,6 @@ Encoder Functions Reference
 .. kernel-doc:: drivers/gpu/drm/drm_encoder.c
    :export:
 
-KMS Initialization and Cleanup
-==============================
-
-A KMS device is abstracted and exposed as a set of planes, CRTCs,
-encoders and connectors. KMS drivers must thus create and initialize all
-those objects at load time after initializing mode setting.
-
-CRTCs (:c:type:`struct drm_crtc <drm_crtc>`)
---------------------------------------------
-
-A CRTC is an abstraction representing a part of the chip that contains a
-pointer to a scanout buffer. Therefore, the number of CRTCs available
-determines how many independent scanout buffers can be active at any
-given time. The CRTC structure contains several fields to support this:
-a pointer to some video memory (abstracted as a frame buffer object), a
-display mode, and an (x, y) offset into the video memory to support
-panning or configurations where one piece of video memory spans multiple
-CRTCs.
-
-CRTC Initialization
-~~~~~~~~~~~~~~~~~~~
-
-A KMS device must create and register at least one struct
-:c:type:`struct drm_crtc <drm_crtc>` instance. The instance is
-allocated and zeroed by the driver, possibly as part of a larger
-structure, and registered with a call to :c:func:`drm_crtc_init()`
-with a pointer to CRTC functions.
-
-
-Cleanup
--------
-
-The DRM core manages its objects' lifetime. When an object is not needed
-anymore the core calls its destroy function, which must clean up and
-free every resource allocated for the object. Every
-:c:func:`drm_\*_init()` call must be matched with a corresponding
-:c:func:`drm_\*_cleanup()` call to cleanup CRTCs
-(:c:func:`drm_crtc_cleanup()`), planes
-(:c:func:`drm_plane_cleanup()`), encoders
-(:c:func:`drm_encoder_cleanup()`) and connectors
-(:c:func:`drm_connector_cleanup()`). Furthermore, connectors that
-have been added to sysfs must be removed by a call to
-:c:func:`drm_connector_unregister()` before calling
-:c:func:`drm_connector_cleanup()`.
-
-Connectors state change detection must be cleanup up with a call to
-:c:func:`drm_kms_helper_poll_fini()`.
-
-Output discovery and initialization example
--------------------------------------------
-
-.. code-block:: c
-
-    void intel_crt_init(struct drm_device *dev)
-    {
-        struct drm_connector *connector;
-        struct intel_output *intel_output;
-
-        intel_output = kzalloc(sizeof(struct intel_output), GFP_KERNEL);
-        if (!intel_output)
-            return;
-
-        connector = &intel_output->base;
-        drm_connector_init(dev, &intel_output->base,
-                   &intel_crt_connector_funcs, DRM_MODE_CONNECTOR_VGA);
-
-        drm_encoder_init(dev, &intel_output->enc, &intel_crt_enc_funcs,
-                 DRM_MODE_ENCODER_DAC);
-
-        drm_connector_attach_encoder(&intel_output->base,
-                          &intel_output->enc);
-
-        /* Set up the DDC bus. */
-        intel_output->ddc_bus = intel_i2c_create(dev, GPIOA, "CRTDDC_A");
-        if (!intel_output->ddc_bus) {
-            dev_printk(KERN_ERR, &dev->pdev->dev, "DDC bus registration "
-                   "failed.\n");
-            return;
-        }
-
-        intel_output->type = INTEL_OUTPUT_ANALOG;
-        connector->interlace_allowed = 0;
-        connector->doublescan_allowed = 0;
-
-        drm_encoder_helper_add(&intel_output->enc, &intel_crt_helper_funcs);
-        drm_connector_helper_add(connector, &intel_crt_connector_helper_funcs);
-
-        drm_connector_register(connector);
-    }
-
-In the example above (taken from the i915 driver), a CRTC, connector and
-encoder combination is created. A device-specific i2c bus is also
-created for fetching EDID data and performing monitor detection. Once
-the process is complete, the new connector is registered with sysfs to
-make its properties available to applications.
-
 KMS Locking
 ===========
 
-- 
2.20.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/3] drm/doc: Drop chapter "KMS Initialization and Cleanup"

[Kazlauskas, Nicholas]

On 1/30/19 11:30 AM, Daniel Vetter wrote:
> It only talks about crtc, brings up intel as an exampel and I think is

nit: Should be "example".

> more misleading than useful really. Plus we have lots of discussion
> about how your standard kms driver should be initialized/cleaned up,
> so maybe better to document this when we have a better idea.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>   Documentation/gpu/drm-kms.rst | 96 -----------------------------------
>   1 file changed, 96 deletions(-)
> 
> diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
> index 75c882e09fee..23a3c986ef6d 100644
> --- a/Documentation/gpu/drm-kms.rst
> +++ b/Documentation/gpu/drm-kms.rst
> @@ -410,102 +410,6 @@ Encoder Functions Reference
>   .. kernel-doc:: drivers/gpu/drm/drm_encoder.c
>      :export:
>   
> -KMS Initialization and Cleanup
> -==============================
> -
> -A KMS device is abstracted and exposed as a set of planes, CRTCs,
> -encoders and connectors. KMS drivers must thus create and initialize all
> -those objects at load time after initializing mode setting.
> -
> -CRTCs (:c:type:`struct drm_crtc <drm_crtc>`)
> ---------------------------------------------
> -
> -A CRTC is an abstraction representing a part of the chip that contains a
> -pointer to a scanout buffer. Therefore, the number of CRTCs available
> -determines how many independent scanout buffers can be active at any
> -given time. The CRTC structure contains several fields to support this:
> -a pointer to some video memory (abstracted as a frame buffer object), a
> -display mode, and an (x, y) offset into the video memory to support
> -panning or configurations where one piece of video memory spans multiple
> -CRTCs.

This is mostly a duplicate of what's already in the CRTC abstraction 
section but it may be worth carrying over the bit about  "the number of 
CRTCs available determining the number of independent scanout buffers 
that can be active at any given time".

> -
> -CRTC Initialization
> -~~~~~~~~~~~~~~~~~~~
> -
> -A KMS device must create and register at least one struct
> -:c:type:`struct drm_crtc <drm_crtc>` instance. The instance is
> -allocated and zeroed by the driver, possibly as part of a larger
> -structure, and registered with a call to :c:func:`drm_crtc_init()`
> -with a pointer to CRTC functions.
> -
> -
> -Cleanup
> --------
> -
> -The DRM core manages its objects' lifetime. When an object is not needed
> -anymore the core calls its destroy function, which must clean up and
> -free every resource allocated for the object. Every
> -:c:func:`drm_\*_init()` call must be matched with a corresponding
> -:c:func:`drm_\*_cleanup()` call to cleanup CRTCs
> -(:c:func:`drm_crtc_cleanup()`), planes
> -(:c:func:`drm_plane_cleanup()`), encoders
> -(:c:func:`drm_encoder_cleanup()`) and connectors
> -(:c:func:`drm_connector_cleanup()`). Furthermore, connectors that
> -have been added to sysfs must be removed by a call to
> -:c:func:`drm_connector_unregister()` before calling
> -:c:func:`drm_connector_cleanup()`.
> -
> -Connectors state change detection must be cleanup up with a call to
> -:c:func:`drm_kms_helper_poll_fini()`.
> -
> -Output discovery and initialization example
> --------------------------------------------
> -
> -.. code-block:: c
> -
> -    void intel_crt_init(struct drm_device *dev)
> -    {
> -        struct drm_connector *connector;
> -        struct intel_output *intel_output;
> -
> -        intel_output = kzalloc(sizeof(struct intel_output), GFP_KERNEL);
> -        if (!intel_output)
> -            return;
> -
> -        connector = &intel_output->base;
> -        drm_connector_init(dev, &intel_output->base,
> -                   &intel_crt_connector_funcs, DRM_MODE_CONNECTOR_VGA);
> -
> -        drm_encoder_init(dev, &intel_output->enc, &intel_crt_enc_funcs,
> -                 DRM_MODE_ENCODER_DAC);
> -
> -        drm_connector_attach_encoder(&intel_output->base,
> -                          &intel_output->enc);
> -
> -        /* Set up the DDC bus. */
> -        intel_output->ddc_bus = intel_i2c_create(dev, GPIOA, "CRTDDC_A");
> -        if (!intel_output->ddc_bus) {
> -            dev_printk(KERN_ERR, &dev->pdev->dev, "DDC bus registration "
> -                   "failed.\n");
> -            return;
> -        }
> -
> -        intel_output->type = INTEL_OUTPUT_ANALOG;
> -        connector->interlace_allowed = 0;
> -        connector->doublescan_allowed = 0;
> -
> -        drm_encoder_helper_add(&intel_output->enc, &intel_crt_helper_funcs);
> -        drm_connector_helper_add(connector, &intel_crt_connector_helper_funcs);
> -
> -        drm_connector_register(connector);
> -    }
> -
> -In the example above (taken from the i915 driver), a CRTC, connector and
> -encoder combination is created. A device-specific i2c bus is also
> -created for fetching EDID data and performing monitor detection. Once
> -the process is complete, the new connector is registered with sysfs to
> -make its properties available to applications.
> -
>   KMS Locking
>   ===========
>   
> 

Everything else looks fine to drop.

The bits about initialization/cleanup/subclassing/connectors can all be 
found in the Atomic Mode Setting section or one of the abstraction sections.

Nicholas Kazlauskas
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/3] drm/doc: Drop chapter "KMS Initialization and Cleanup"

[Daniel Vetter]

On Wed, Jan 30, 2019 at 05:33:00PM +0000, Kazlauskas, Nicholas wrote:
> On 1/30/19 11:30 AM, Daniel Vetter wrote:
> > It only talks about crtc, brings up intel as an exampel and I think is
> 
> nit: Should be "example".

Will fix when applying.

> > more misleading than useful really. Plus we have lots of discussion
> > about how your standard kms driver should be initialized/cleaned up,
> > so maybe better to document this when we have a better idea.
> > 
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > ---
> >   Documentation/gpu/drm-kms.rst | 96 -----------------------------------
> >   1 file changed, 96 deletions(-)
> > 
> > diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
> > index 75c882e09fee..23a3c986ef6d 100644
> > --- a/Documentation/gpu/drm-kms.rst
> > +++ b/Documentation/gpu/drm-kms.rst
> > @@ -410,102 +410,6 @@ Encoder Functions Reference
> >   .. kernel-doc:: drivers/gpu/drm/drm_encoder.c
> >      :export:
> >   
> > -KMS Initialization and Cleanup
> > -==============================
> > -
> > -A KMS device is abstracted and exposed as a set of planes, CRTCs,
> > -encoders and connectors. KMS drivers must thus create and initialize all
> > -those objects at load time after initializing mode setting.
> > -
> > -CRTCs (:c:type:`struct drm_crtc <drm_crtc>`)
> > ---------------------------------------------
> > -
> > -A CRTC is an abstraction representing a part of the chip that contains a
> > -pointer to a scanout buffer. Therefore, the number of CRTCs available
> > -determines how many independent scanout buffers can be active at any
> > -given time. The CRTC structure contains several fields to support this:
> > -a pointer to some video memory (abstracted as a frame buffer object), a
> > -display mode, and an (x, y) offset into the video memory to support
> > -panning or configurations where one piece of video memory spans multiple
> > -CRTCs.
> 
> This is mostly a duplicate of what's already in the CRTC abstraction 
> section but it may be worth carrying over the bit about  "the number of 
> CRTCs available determining the number of independent scanout buffers 
> that can be active at any given time".

Oh I didn't even read this. This is just plain wrong and probably predates
drm_plane (when it kinda was true-ish, if you'd ignore cursors). Now this
would map to number of drm_planes, except
- cursors can still be an exceptions
- yuv multi-planar might or might not be multiple planes per drm_plane.

Aka it's really complicated :-)

This is definitely much better explained in the kms overview section at
the top of:

https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#overview

Plus everything that's getting linked from there ofc.

> > -
> > -CRTC Initialization
> > -~~~~~~~~~~~~~~~~~~~
> > -
> > -A KMS device must create and register at least one struct
> > -:c:type:`struct drm_crtc <drm_crtc>` instance. The instance is
> > -allocated and zeroed by the driver, possibly as part of a larger
> > -structure, and registered with a call to :c:func:`drm_crtc_init()`
> > -with a pointer to CRTC functions.
> > -
> > -
> > -Cleanup
> > --------
> > -
> > -The DRM core manages its objects' lifetime. When an object is not needed
> > -anymore the core calls its destroy function, which must clean up and
> > -free every resource allocated for the object. Every
> > -:c:func:`drm_\*_init()` call must be matched with a corresponding
> > -:c:func:`drm_\*_cleanup()` call to cleanup CRTCs
> > -(:c:func:`drm_crtc_cleanup()`), planes
> > -(:c:func:`drm_plane_cleanup()`), encoders
> > -(:c:func:`drm_encoder_cleanup()`) and connectors
> > -(:c:func:`drm_connector_cleanup()`). Furthermore, connectors that
> > -have been added to sysfs must be removed by a call to
> > -:c:func:`drm_connector_unregister()` before calling
> > -:c:func:`drm_connector_cleanup()`.
> > -
> > -Connectors state change detection must be cleanup up with a call to
> > -:c:func:`drm_kms_helper_poll_fini()`.
> > -
> > -Output discovery and initialization example
> > --------------------------------------------
> > -
> > -.. code-block:: c
> > -
> > -    void intel_crt_init(struct drm_device *dev)
> > -    {
> > -        struct drm_connector *connector;
> > -        struct intel_output *intel_output;
> > -
> > -        intel_output = kzalloc(sizeof(struct intel_output), GFP_KERNEL);
> > -        if (!intel_output)
> > -            return;
> > -
> > -        connector = &intel_output->base;
> > -        drm_connector_init(dev, &intel_output->base,
> > -                   &intel_crt_connector_funcs, DRM_MODE_CONNECTOR_VGA);
> > -
> > -        drm_encoder_init(dev, &intel_output->enc, &intel_crt_enc_funcs,
> > -                 DRM_MODE_ENCODER_DAC);
> > -
> > -        drm_connector_attach_encoder(&intel_output->base,
> > -                          &intel_output->enc);
> > -
> > -        /* Set up the DDC bus. */
> > -        intel_output->ddc_bus = intel_i2c_create(dev, GPIOA, "CRTDDC_A");
> > -        if (!intel_output->ddc_bus) {
> > -            dev_printk(KERN_ERR, &dev->pdev->dev, "DDC bus registration "
> > -                   "failed.\n");
> > -            return;
> > -        }
> > -
> > -        intel_output->type = INTEL_OUTPUT_ANALOG;
> > -        connector->interlace_allowed = 0;
> > -        connector->doublescan_allowed = 0;
> > -
> > -        drm_encoder_helper_add(&intel_output->enc, &intel_crt_helper_funcs);
> > -        drm_connector_helper_add(connector, &intel_crt_connector_helper_funcs);
> > -
> > -        drm_connector_register(connector);
> > -    }
> > -
> > -In the example above (taken from the i915 driver), a CRTC, connector and
> > -encoder combination is created. A device-specific i2c bus is also
> > -created for fetching EDID data and performing monitor detection. Once
> > -the process is complete, the new connector is registered with sysfs to
> > -make its properties available to applications.
> > -
> >   KMS Locking
> >   ===========
> >   
> > 
> 
> Everything else looks fine to drop.
> 
> The bits about initialization/cleanup/subclassing/connectors can all be 
> found in the Atomic Mode Setting section or one of the abstraction sections.

With the one thing you wanted to keep also wrong - ok with this patch?

Thanks for taking a look at this.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/3] drm/doc: Drop chapter "KMS Initialization and Cleanup"

[Kazlauskas, Nicholas]

On 1/30/19 4:02 PM, Daniel Vetter wrote:
> On Wed, Jan 30, 2019 at 05:33:00PM +0000, Kazlauskas, Nicholas wrote:
>> On 1/30/19 11:30 AM, Daniel Vetter wrote:
>>> It only talks about crtc, brings up intel as an exampel and I think is
>>
>> nit: Should be "example".
> 
> Will fix when applying.
> 
>>> more misleading than useful really. Plus we have lots of discussion
>>> about how your standard kms driver should be initialized/cleaned up,
>>> so maybe better to document this when we have a better idea.
>>>
>>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
>>> ---
>>>    Documentation/gpu/drm-kms.rst | 96 -----------------------------------
>>>    1 file changed, 96 deletions(-)
>>>
>>> diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
>>> index 75c882e09fee..23a3c986ef6d 100644
>>> --- a/Documentation/gpu/drm-kms.rst
>>> +++ b/Documentation/gpu/drm-kms.rst
>>> @@ -410,102 +410,6 @@ Encoder Functions Reference
>>>    .. kernel-doc:: drivers/gpu/drm/drm_encoder.c
>>>       :export:
>>>    
>>> -KMS Initialization and Cleanup
>>> -==============================
>>> -
>>> -A KMS device is abstracted and exposed as a set of planes, CRTCs,
>>> -encoders and connectors. KMS drivers must thus create and initialize all
>>> -those objects at load time after initializing mode setting.
>>> -
>>> -CRTCs (:c:type:`struct drm_crtc <drm_crtc>`)
>>> ---------------------------------------------
>>> -
>>> -A CRTC is an abstraction representing a part of the chip that contains a
>>> -pointer to a scanout buffer. Therefore, the number of CRTCs available
>>> -determines how many independent scanout buffers can be active at any
>>> -given time. The CRTC structure contains several fields to support this:
>>> -a pointer to some video memory (abstracted as a frame buffer object), a
>>> -display mode, and an (x, y) offset into the video memory to support
>>> -panning or configurations where one piece of video memory spans multiple
>>> -CRTCs.
>>
>> This is mostly a duplicate of what's already in the CRTC abstraction
>> section but it may be worth carrying over the bit about  "the number of
>> CRTCs available determining the number of independent scanout buffers
>> that can be active at any given time".
> 
> Oh I didn't even read this. This is just plain wrong and probably predates
> drm_plane (when it kinda was true-ish, if you'd ignore cursors). Now this
> would map to number of drm_planes, except
> - cursors can still be an exceptions
> - yuv multi-planar might or might not be multiple planes per drm_plane.
> 
> Aka it's really complicated :-)
> 
> This is definitely much better explained in the kms overview section at
> the top of:
> 
> https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#overview
> 
> Plus everything that's getting linked from there ofc.
> 
>>> -
>>> -CRTC Initialization
>>> -~~~~~~~~~~~~~~~~~~~
>>> -
>>> -A KMS device must create and register at least one struct
>>> -:c:type:`struct drm_crtc <drm_crtc>` instance. The instance is
>>> -allocated and zeroed by the driver, possibly as part of a larger
>>> -structure, and registered with a call to :c:func:`drm_crtc_init()`
>>> -with a pointer to CRTC functions.
>>> -
>>> -
>>> -Cleanup
>>> --------
>>> -
>>> -The DRM core manages its objects' lifetime. When an object is not needed
>>> -anymore the core calls its destroy function, which must clean up and
>>> -free every resource allocated for the object. Every
>>> -:c:func:`drm_\*_init()` call must be matched with a corresponding
>>> -:c:func:`drm_\*_cleanup()` call to cleanup CRTCs
>>> -(:c:func:`drm_crtc_cleanup()`), planes
>>> -(:c:func:`drm_plane_cleanup()`), encoders
>>> -(:c:func:`drm_encoder_cleanup()`) and connectors
>>> -(:c:func:`drm_connector_cleanup()`). Furthermore, connectors that
>>> -have been added to sysfs must be removed by a call to
>>> -:c:func:`drm_connector_unregister()` before calling
>>> -:c:func:`drm_connector_cleanup()`.
>>> -
>>> -Connectors state change detection must be cleanup up with a call to
>>> -:c:func:`drm_kms_helper_poll_fini()`.
>>> -
>>> -Output discovery and initialization example
>>> --------------------------------------------
>>> -
>>> -.. code-block:: c
>>> -
>>> -    void intel_crt_init(struct drm_device *dev)
>>> -    {
>>> -        struct drm_connector *connector;
>>> -        struct intel_output *intel_output;
>>> -
>>> -        intel_output = kzalloc(sizeof(struct intel_output), GFP_KERNEL);
>>> -        if (!intel_output)
>>> -            return;
>>> -
>>> -        connector = &intel_output->base;
>>> -        drm_connector_init(dev, &intel_output->base,
>>> -                   &intel_crt_connector_funcs, DRM_MODE_CONNECTOR_VGA);
>>> -
>>> -        drm_encoder_init(dev, &intel_output->enc, &intel_crt_enc_funcs,
>>> -                 DRM_MODE_ENCODER_DAC);
>>> -
>>> -        drm_connector_attach_encoder(&intel_output->base,
>>> -                          &intel_output->enc);
>>> -
>>> -        /* Set up the DDC bus. */
>>> -        intel_output->ddc_bus = intel_i2c_create(dev, GPIOA, "CRTDDC_A");
>>> -        if (!intel_output->ddc_bus) {
>>> -            dev_printk(KERN_ERR, &dev->pdev->dev, "DDC bus registration "
>>> -                   "failed.\n");
>>> -            return;
>>> -        }
>>> -
>>> -        intel_output->type = INTEL_OUTPUT_ANALOG;
>>> -        connector->interlace_allowed = 0;
>>> -        connector->doublescan_allowed = 0;
>>> -
>>> -        drm_encoder_helper_add(&intel_output->enc, &intel_crt_helper_funcs);
>>> -        drm_connector_helper_add(connector, &intel_crt_connector_helper_funcs);
>>> -
>>> -        drm_connector_register(connector);
>>> -    }
>>> -
>>> -In the example above (taken from the i915 driver), a CRTC, connector and
>>> -encoder combination is created. A device-specific i2c bus is also
>>> -created for fetching EDID data and performing monitor detection. Once
>>> -the process is complete, the new connector is registered with sysfs to
>>> -make its properties available to applications.
>>> -
>>>    KMS Locking
>>>    ===========
>>>    
>>>
>>
>> Everything else looks fine to drop.
>>
>> The bits about initialization/cleanup/subclassing/connectors can all be
>> found in the Atomic Mode Setting section or one of the abstraction sections.
> 
> With the one thing you wanted to keep also wrong - ok with this patch?
> 
> Thanks for taking a look at this.
> -Daniel
> 

Sounds good.

Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>

Nicholas Kazlauskas
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/3] drm/doc: Drop chapter "KMS Initialization and Cleanup"

[Daniel Vetter]

On Wed, Jan 30, 2019 at 09:04:23PM +0000, Kazlauskas, Nicholas wrote:
> On 1/30/19 4:02 PM, Daniel Vetter wrote:
> > On Wed, Jan 30, 2019 at 05:33:00PM +0000, Kazlauskas, Nicholas wrote:
> >> On 1/30/19 11:30 AM, Daniel Vetter wrote:
> >>> It only talks about crtc, brings up intel as an exampel and I think is
> >>
> >> nit: Should be "example".
> > 
> > Will fix when applying.
> > 
> >>> more misleading than useful really. Plus we have lots of discussion
> >>> about how your standard kms driver should be initialized/cleaned up,
> >>> so maybe better to document this when we have a better idea.
> >>>
> >>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> >>> ---
> >>>    Documentation/gpu/drm-kms.rst | 96 -----------------------------------
> >>>    1 file changed, 96 deletions(-)
> >>>
> >>> diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
> >>> index 75c882e09fee..23a3c986ef6d 100644
> >>> --- a/Documentation/gpu/drm-kms.rst
> >>> +++ b/Documentation/gpu/drm-kms.rst
> >>> @@ -410,102 +410,6 @@ Encoder Functions Reference
> >>>    .. kernel-doc:: drivers/gpu/drm/drm_encoder.c
> >>>       :export:
> >>>    
> >>> -KMS Initialization and Cleanup
> >>> -==============================
> >>> -
> >>> -A KMS device is abstracted and exposed as a set of planes, CRTCs,
> >>> -encoders and connectors. KMS drivers must thus create and initialize all
> >>> -those objects at load time after initializing mode setting.
> >>> -
> >>> -CRTCs (:c:type:`struct drm_crtc <drm_crtc>`)
> >>> ---------------------------------------------
> >>> -
> >>> -A CRTC is an abstraction representing a part of the chip that contains a
> >>> -pointer to a scanout buffer. Therefore, the number of CRTCs available
> >>> -determines how many independent scanout buffers can be active at any
> >>> -given time. The CRTC structure contains several fields to support this:
> >>> -a pointer to some video memory (abstracted as a frame buffer object), a
> >>> -display mode, and an (x, y) offset into the video memory to support
> >>> -panning or configurations where one piece of video memory spans multiple
> >>> -CRTCs.
> >>
> >> This is mostly a duplicate of what's already in the CRTC abstraction
> >> section but it may be worth carrying over the bit about  "the number of
> >> CRTCs available determining the number of independent scanout buffers
> >> that can be active at any given time".
> > 
> > Oh I didn't even read this. This is just plain wrong and probably predates
> > drm_plane (when it kinda was true-ish, if you'd ignore cursors). Now this
> > would map to number of drm_planes, except
> > - cursors can still be an exceptions
> > - yuv multi-planar might or might not be multiple planes per drm_plane.
> > 
> > Aka it's really complicated :-)
> > 
> > This is definitely much better explained in the kms overview section at
> > the top of:
> > 
> > https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#overview
> > 
> > Plus everything that's getting linked from there ofc.
> > 
> >>> -
> >>> -CRTC Initialization
> >>> -~~~~~~~~~~~~~~~~~~~
> >>> -
> >>> -A KMS device must create and register at least one struct
> >>> -:c:type:`struct drm_crtc <drm_crtc>` instance. The instance is
> >>> -allocated and zeroed by the driver, possibly as part of a larger
> >>> -structure, and registered with a call to :c:func:`drm_crtc_init()`
> >>> -with a pointer to CRTC functions.
> >>> -
> >>> -
> >>> -Cleanup
> >>> --------
> >>> -
> >>> -The DRM core manages its objects' lifetime. When an object is not needed
> >>> -anymore the core calls its destroy function, which must clean up and
> >>> -free every resource allocated for the object. Every
> >>> -:c:func:`drm_\*_init()` call must be matched with a corresponding
> >>> -:c:func:`drm_\*_cleanup()` call to cleanup CRTCs
> >>> -(:c:func:`drm_crtc_cleanup()`), planes
> >>> -(:c:func:`drm_plane_cleanup()`), encoders
> >>> -(:c:func:`drm_encoder_cleanup()`) and connectors
> >>> -(:c:func:`drm_connector_cleanup()`). Furthermore, connectors that
> >>> -have been added to sysfs must be removed by a call to
> >>> -:c:func:`drm_connector_unregister()` before calling
> >>> -:c:func:`drm_connector_cleanup()`.
> >>> -
> >>> -Connectors state change detection must be cleanup up with a call to
> >>> -:c:func:`drm_kms_helper_poll_fini()`.
> >>> -
> >>> -Output discovery and initialization example
> >>> --------------------------------------------
> >>> -
> >>> -.. code-block:: c
> >>> -
> >>> -    void intel_crt_init(struct drm_device *dev)
> >>> -    {
> >>> -        struct drm_connector *connector;
> >>> -        struct intel_output *intel_output;
> >>> -
> >>> -        intel_output = kzalloc(sizeof(struct intel_output), GFP_KERNEL);
> >>> -        if (!intel_output)
> >>> -            return;
> >>> -
> >>> -        connector = &intel_output->base;
> >>> -        drm_connector_init(dev, &intel_output->base,
> >>> -                   &intel_crt_connector_funcs, DRM_MODE_CONNECTOR_VGA);
> >>> -
> >>> -        drm_encoder_init(dev, &intel_output->enc, &intel_crt_enc_funcs,
> >>> -                 DRM_MODE_ENCODER_DAC);
> >>> -
> >>> -        drm_connector_attach_encoder(&intel_output->base,
> >>> -                          &intel_output->enc);
> >>> -
> >>> -        /* Set up the DDC bus. */
> >>> -        intel_output->ddc_bus = intel_i2c_create(dev, GPIOA, "CRTDDC_A");
> >>> -        if (!intel_output->ddc_bus) {
> >>> -            dev_printk(KERN_ERR, &dev->pdev->dev, "DDC bus registration "
> >>> -                   "failed.\n");
> >>> -            return;
> >>> -        }
> >>> -
> >>> -        intel_output->type = INTEL_OUTPUT_ANALOG;
> >>> -        connector->interlace_allowed = 0;
> >>> -        connector->doublescan_allowed = 0;
> >>> -
> >>> -        drm_encoder_helper_add(&intel_output->enc, &intel_crt_helper_funcs);
> >>> -        drm_connector_helper_add(connector, &intel_crt_connector_helper_funcs);
> >>> -
> >>> -        drm_connector_register(connector);
> >>> -    }
> >>> -
> >>> -In the example above (taken from the i915 driver), a CRTC, connector and
> >>> -encoder combination is created. A device-specific i2c bus is also
> >>> -created for fetching EDID data and performing monitor detection. Once
> >>> -the process is complete, the new connector is registered with sysfs to
> >>> -make its properties available to applications.
> >>> -
> >>>    KMS Locking
> >>>    ===========
> >>>    
> >>>
> >>
> >> Everything else looks fine to drop.
> >>
> >> The bits about initialization/cleanup/subclassing/connectors can all be
> >> found in the Atomic Mode Setting section or one of the abstraction sections.
> > 
> > With the one thing you wanted to keep also wrong - ok with this patch?
> > 
> > Thanks for taking a look at this.
> > -Daniel
> > 
> 
> Sounds good.
> 
> Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>

Thanks for reviewing, patch merged to drm-misc-next.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 3/3] drm/doc: fix VRR_ENABLED casing

[Daniel Vetter]

Yes it's inconsitent with vrr_capable, but this is the actual uapi as
exercise by igt.

Fixes: ab7a664f7a2d ("drm: Document variable refresh properties")
Cc: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
Cc: Harry Wentland <harry.wentland@amd.com>
Cc: Pekka Paalanen <pekka.paalanen@collabora.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_connector.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 847539645558..e3ff73695c32 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1367,7 +1367,7 @@ EXPORT_SYMBOL(drm_mode_create_scaling_mode_property);
  *
  *	Absence of the property should indicate absence of support.
  *
- * "vrr_enabled":
+ * "VRR_ENABLED":
  *	Default &drm_crtc boolean property that notifies the driver that the
  *	content on the CRTC is suitable for variable refresh rate presentation.
  *	The driver will take this property as a hint to enable variable
-- 
2.20.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/3] drm/doc: fix VRR_ENABLED casing

[Kazlauskas, Nicholas]

On 1/30/19 11:30 AM, Daniel Vetter wrote:
> Yes it's inconsitent with vrr_capable, but this is the actual uapi as
> exercise by igt.
> 
> Fixes: ab7a664f7a2d ("drm: Document variable refresh properties")
> Cc: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
> Cc: Harry Wentland <harry.wentland@amd.com>
> Cc: Pekka Paalanen <pekka.paalanen@collabora.com>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>

Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>

It's what xf86-video-amdgpu expects too - the casing is intentional to 
match up with the other default CRTC properties.

Thanks!

Nicholas Kazlauskas

> ---
>   drivers/gpu/drm/drm_connector.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 847539645558..e3ff73695c32 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -1367,7 +1367,7 @@ EXPORT_SYMBOL(drm_mode_create_scaling_mode_property);
>    *
>    *	Absence of the property should indicate absence of support.
>    *
> - * "vrr_enabled":
> + * "VRR_ENABLED":
>    *	Default &drm_crtc boolean property that notifies the driver that the
>    *	content on the CRTC is suitable for variable refresh rate presentation.
>    *	The driver will take this property as a hint to enable variable
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/3] drm/doc: fix VRR_ENABLED casing

[Daniel Vetter]

On Wed, Jan 30, 2019 at 04:57:48PM +0000, Kazlauskas, Nicholas wrote:
> On 1/30/19 11:30 AM, Daniel Vetter wrote:
> > Yes it's inconsitent with vrr_capable, but this is the actual uapi as
> > exercise by igt.
> > 
> > Fixes: ab7a664f7a2d ("drm: Document variable refresh properties")
> > Cc: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
> > Cc: Harry Wentland <harry.wentland@amd.com>
> > Cc: Pekka Paalanen <pekka.paalanen@collabora.com>
> > Cc: Alex Deucher <alexander.deucher@amd.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> 
> Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
> 
> It's what xf86-video-amdgpu expects too - the casing is intentional to 
> match up with the other default CRTC properties.
> 
> Thanks!

Applied, thanks for your review. Can I motivate you to take a look at the
other patches in this series too?

Thanks, Daniel

> 
> Nicholas Kazlauskas
> 
> > ---
> >   drivers/gpu/drm/drm_connector.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> > index 847539645558..e3ff73695c32 100644
> > --- a/drivers/gpu/drm/drm_connector.c
> > +++ b/drivers/gpu/drm/drm_connector.c
> > @@ -1367,7 +1367,7 @@ EXPORT_SYMBOL(drm_mode_create_scaling_mode_property);
> >    *
> >    *	Absence of the property should indicate absence of support.
> >    *
> > - * "vrr_enabled":
> > + * "VRR_ENABLED":
> >    *	Default &drm_crtc boolean property that notifies the driver that the
> >    *	content on the CRTC is suitable for variable refresh rate presentation.
> >    *	The driver will take this property as a hint to enable variable
> > 
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] drm/doc: Move hdmi infoframe docs

[Kazlauskas, Nicholas]

On 1/30/19 11:30 AM, Daniel Vetter wrote:
> .. next to all the other sink helpers. The rect library is more used
> for handling plane clipping, so belongs to those imo.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>

Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>

Looks better than being sandwiched between the HDMI helpers. Flip work 
is also plane related in a sense.

Nicholas Kazlauskas

> ---
>   Documentation/gpu/drm-kms-helpers.rst | 24 ++++++++++++------------
>   1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/Documentation/gpu/drm-kms-helpers.rst b/Documentation/gpu/drm-kms-helpers.rst
> index fbd11b2fe5b5..17ca7f8bf3d3 100644
> --- a/Documentation/gpu/drm-kms-helpers.rst
> +++ b/Documentation/gpu/drm-kms-helpers.rst
> @@ -296,18 +296,6 @@ SCDC Helper Functions Reference
>   .. kernel-doc:: drivers/gpu/drm/drm_scdc_helper.c
>      :export:
>   
> -Rectangle Utilities Reference
> -=============================
> -
> -.. kernel-doc:: include/drm/drm_rect.h
> -   :doc: rect utils
> -
> -.. kernel-doc:: include/drm/drm_rect.h
> -   :internal:
> -
> -.. kernel-doc:: drivers/gpu/drm/drm_rect.c
> -   :export:
> -
>   HDMI Infoframes Helper Reference
>   ================================
>   
> @@ -322,6 +310,18 @@ libraries and hence is also included here.
>   .. kernel-doc:: drivers/video/hdmi.c
>      :export:
>   
> +Rectangle Utilities Reference
> +=============================
> +
> +.. kernel-doc:: include/drm/drm_rect.h
> +   :doc: rect utils
> +
> +.. kernel-doc:: include/drm/drm_rect.h
> +   :internal:
> +
> +.. kernel-doc:: drivers/gpu/drm/drm_rect.c
> +   :export:
> +
>   Flip-work Helper Reference
>   ==========================
>   
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] drm/doc: Move hdmi infoframe docs

[Daniel Vetter]

On Wed, Jan 30, 2019 at 05:13:13PM +0000, Kazlauskas, Nicholas wrote:
> On 1/30/19 11:30 AM, Daniel Vetter wrote:
> > .. next to all the other sink helpers. The rect library is more used
> > for handling plane clipping, so belongs to those imo.
> > 
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> 
> Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
> 
> Looks better than being sandwiched between the HDMI helpers. Flip work 
> is also plane related in a sense.

Yup, that's what I meant with plane helpers - the kms helper intro groups
all these (flip, plane helpers, rect stuff, ...) into a plane related
helpers pile.

Thanks for taking a look, patch merged.
-Daniel

> 
> Nicholas Kazlauskas
> 
> > ---
> >   Documentation/gpu/drm-kms-helpers.rst | 24 ++++++++++++------------
> >   1 file changed, 12 insertions(+), 12 deletions(-)
> > 
> > diff --git a/Documentation/gpu/drm-kms-helpers.rst b/Documentation/gpu/drm-kms-helpers.rst
> > index fbd11b2fe5b5..17ca7f8bf3d3 100644
> > --- a/Documentation/gpu/drm-kms-helpers.rst
> > +++ b/Documentation/gpu/drm-kms-helpers.rst
> > @@ -296,18 +296,6 @@ SCDC Helper Functions Reference
> >   .. kernel-doc:: drivers/gpu/drm/drm_scdc_helper.c
> >      :export:
> >   
> > -Rectangle Utilities Reference
> > -=============================
> > -
> > -.. kernel-doc:: include/drm/drm_rect.h
> > -   :doc: rect utils
> > -
> > -.. kernel-doc:: include/drm/drm_rect.h
> > -   :internal:
> > -
> > -.. kernel-doc:: drivers/gpu/drm/drm_rect.c
> > -   :export:
> > -
> >   HDMI Infoframes Helper Reference
> >   ================================
> >   
> > @@ -322,6 +310,18 @@ libraries and hence is also included here.
> >   .. kernel-doc:: drivers/video/hdmi.c
> >      :export:
> >   
> > +Rectangle Utilities Reference
> > +=============================
> > +
> > +.. kernel-doc:: include/drm/drm_rect.h
> > +   :doc: rect utils
> > +
> > +.. kernel-doc:: include/drm/drm_rect.h
> > +   :internal:
> > +
> > +.. kernel-doc:: drivers/gpu/drm/drm_rect.c
> > +   :export:
> > +
> >   Flip-work Helper Reference
> >   ==========================
> >   
> > 
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel