Lyude Paul wrote a nice intro to vblank in the following mail: https://lore.kernel.org/dri-devel/faf63d8a9ed23c16af69762f59d0dca6b2bf085f.c...
Reading this I managed to spot a glimmer of hope that I one day would understand some of the fuzz around vblank. To let others benefit from the description I went ahead and added the description to drm_vblank.c.
Lyude - I added a "Co-developed-by: ..." Can I get your s-o-b on the patch, to document that you are OK with this to go in.
When checking the output with "make htmldocs" I noticed several drm related warnings. I went ahead a fixed most of them resulting in a few extre patches.
There are some warnings in amdgpu land - I have left them for the AMD people to figure out: amdgpu_vm.c:92: warning: Function parameter or member 'vm' not described in 'amdgpu_vm_eviction_lock' amdgpu_xgmi.c:1: warning: no structured comments found amdgpu_ras.c:1: warning: no structured comments found amdgpu_dm.h:305: warning: Function parameter or member 'hdcp_workqueue' not described in 'amdgpu_display_manager'
Sam
Sam Ravnborg (6): drm/vblank: Add intro to documentation drm/fb: fix kernel-doc in drm_framebuffer.h drm/sched: fix kernel-doc in gpu_scheduler.h drm: writeback: document callbacks drm/dp_mst: add kernel-doc for drm_dp_mst_port.fec_capable drm/bridge: fix kernel-doc warning in panel.c
drivers/gpu/drm/bridge/panel.c | 1 + drivers/gpu/drm/drm_vblank.c | 15 +++++++++++++++ include/drm/drm_dp_mst_helper.h | 4 ++++ include/drm/drm_framebuffer.h | 4 ++-- include/drm/drm_modeset_helper_vtables.h | 31 +++++++++++++++++++++++++++++++ include/drm/gpu_scheduler.h | 1 + 6 files changed, 54 insertions(+), 2 deletions(-)
Lyude Paul wrote a very good intro to vblank here: https://lore.kernel.org/dri-devel/faf63d8a9ed23c16af69762f59d0dca6b2bf085f.c...
Add this to the intro chapter in drm_vblank.c so others can benefit from it too.
Signed-off-by: Sam Ravnborg sam@ravnborg.org Co-developed-by: Lyude Paul lyude@redhat.com Cc: Lyude Paul lyude@redhat.com Cc: Daniel Vetter daniel@ffwll.ch Cc: Maarten Lankhorst maarten.lankhorst@linux.intel.com Cc: Maxime Ripard mripard@kernel.org Cc: Thomas Zimmermann tzimmermann@suse.de Cc: David Airlie airlied@linux.ie --- drivers/gpu/drm/drm_vblank.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c index bcf346b3e486..95cac22d59d1 100644 --- a/drivers/gpu/drm/drm_vblank.c +++ b/drivers/gpu/drm/drm_vblank.c @@ -41,6 +41,21 @@ /** * DOC: vblank handling * + * From the perspective of a computer, every time a computer monitor displays + * a new frame it's done by "scanning out" the display image from top to + * bottom, one row of pixels at a time. which row of pixels we're on is + * referred to as the scanline. + * Additionally, there's usually a couple of extra scanlines which we + * scan out, but aren't actually displayed on the screen (these sometimes + * get used by HDMI audio and friends, but that's another story). + * The period where we're on these scanlines is referred to as the vblank. + * + * On a lot of display hardware, programming needs to take effect during the + * vertical blanking period so that settings like gamma, what frame we're + * scanning out, etc. can be safely changed without showing visual tearing + * on the screen. In some unforgiving hardware, some of this programming has + * to both start and end in the same vblank - vertical blanking. + * * Vertical blanking plays a major role in graphics rendering. To achieve * tear-free display, users must synchronize page flips and/or rendering to * vertical blanking. The DRM API offers ioctls to perform page flips
Hi Sam and Lyude,
thanks for improving the documentation. Below are a few points that I'd found more understandable. I'm no native speaker, though.
Am 28.03.20 um 14:20 schrieb Sam Ravnborg:
Lyude Paul wrote a very good intro to vblank here: https://lore.kernel.org/dri-devel/faf63d8a9ed23c16af69762f59d0dca6b2bf085f.c...
Add this to the intro chapter in drm_vblank.c so others can benefit from it too.
Signed-off-by: Sam Ravnborg sam@ravnborg.org Co-developed-by: Lyude Paul lyude@redhat.com Cc: Lyude Paul lyude@redhat.com Cc: Daniel Vetter daniel@ffwll.ch Cc: Maarten Lankhorst maarten.lankhorst@linux.intel.com Cc: Maxime Ripard mripard@kernel.org Cc: Thomas Zimmermann tzimmermann@suse.de Cc: David Airlie airlied@linux.ie
drivers/gpu/drm/drm_vblank.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c index bcf346b3e486..95cac22d59d1 100644 --- a/drivers/gpu/drm/drm_vblank.c +++ b/drivers/gpu/drm/drm_vblank.c @@ -41,6 +41,21 @@ /**
- DOC: vblank handling
- From the perspective of a computer, every time a computer monitor displays
Possibly change from dative case to genitive:
"From the computer's perspective," ...
- a new frame it's done by "scanning out" the display image from top to
- bottom, one row of pixels at a time. which row of pixels we're on is
s/which/Which
The text changes from third person (the computer) to first person (we're). Makes it harder to read. I'd remove both, "we" and "computer", and speak of display hardware or scanout engine.
- referred to as the scanline.
I'd say a scanline is any of them. Maybe say "current scanline"?
- Additionally, there's usually a couple of extra scanlines which we
"In addition to the display's visible area, there's usually a couple of extra scanlines that" ...
- scan out, but aren't actually displayed on the screen (these sometimes
- get used by HDMI audio and friends, but that's another story).
- The period where we're on these scanlines is referred to as the vblank.
I'd replace vblank with "vertical blanking period." That term is required in the next paragraph.
The time when the hardware operates on these invisible scanlines is referred to as vertical blanking period, or simply vblank.
- On a lot of display hardware, programming needs to take effect during the
- vertical blanking period so that settings like gamma, what frame we're
"we" again
- scanning out, etc. can be safely changed without showing visual tearing
- on the screen. In some unforgiving hardware, some of this programming has
- to both start and end in the same vblank - vertical blanking.
- Vertical blanking plays a major role in graphics rendering. To achieve
- tear-free display, users must synchronize page flips and/or rendering to
- vertical blanking. The DRM API offers ioctls to perform page flips
In any case
Acked-by: Thomas Zimmermann tzimmermann@suse.de
Best regards Thomas
Hi Thomas.
On Mon, Mar 30, 2020 at 01:29:16PM +0200, Thomas Zimmermann wrote:
Hi Sam and Lyude,
thanks for improving the documentation. Below are a few points that I'd found more understandable. I'm no native speaker, though.
Am 28.03.20 um 14:20 schrieb Sam Ravnborg:
Lyude Paul wrote a very good intro to vblank here: https://lore.kernel.org/dri-devel/faf63d8a9ed23c16af69762f59d0dca6b2bf085f.c...
Add this to the intro chapter in drm_vblank.c so others can benefit from it too.
Signed-off-by: Sam Ravnborg sam@ravnborg.org Co-developed-by: Lyude Paul lyude@redhat.com Cc: Lyude Paul lyude@redhat.com Cc: Daniel Vetter daniel@ffwll.ch Cc: Maarten Lankhorst maarten.lankhorst@linux.intel.com Cc: Maxime Ripard mripard@kernel.org Cc: Thomas Zimmermann tzimmermann@suse.de Cc: David Airlie airlied@linux.ie
drivers/gpu/drm/drm_vblank.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c index bcf346b3e486..95cac22d59d1 100644 --- a/drivers/gpu/drm/drm_vblank.c +++ b/drivers/gpu/drm/drm_vblank.c @@ -41,6 +41,21 @@ /**
- DOC: vblank handling
- From the perspective of a computer, every time a computer monitor displays
Possibly change from dative case to genitive:
"From the computer's perspective," ...
- a new frame it's done by "scanning out" the display image from top to
- bottom, one row of pixels at a time. which row of pixels we're on is
s/which/Which
The text changes from third person (the computer) to first person (we're). Makes it harder to read. I'd remove both, "we" and "computer", and speak of display hardware or scanout engine.
- referred to as the scanline.
I'd say a scanline is any of them. Maybe say "current scanline"?
- Additionally, there's usually a couple of extra scanlines which we
"In addition to the display's visible area, there's usually a couple of extra scanlines that" ...
- scan out, but aren't actually displayed on the screen (these sometimes
- get used by HDMI audio and friends, but that's another story).
- The period where we're on these scanlines is referred to as the vblank.
I'd replace vblank with "vertical blanking period." That term is required in the next paragraph.
The time when the hardware operates on these invisible scanlines is referred to as vertical blanking period, or simply vblank.
- On a lot of display hardware, programming needs to take effect during the
- vertical blanking period so that settings like gamma, what frame we're
"we" again
- scanning out, etc. can be safely changed without showing visual tearing
- on the screen. In some unforgiving hardware, some of this programming has
- to both start and end in the same vblank - vertical blanking.
- Vertical blanking plays a major role in graphics rendering. To achieve
- tear-free display, users must synchronize page flips and/or rendering to
- vertical blanking. The DRM API offers ioctls to perform page flips
In any case
Acked-by: Thomas Zimmermann tzimmermann@suse.de
Thanks for the detailed feedabck - I have tried to reword it so it better fits the context and have taking into account your suggetions. See other mail for the updated patch.
Sam
Lyude Paul wrote a very good intro to vblank here: https://lore.kernel.org/dri-devel/faf63d8a9ed23c16af69762f59d0dca6b2bf085f.c...
Add this to the intro chapter in drm_vblank.c so others can benefit from it too.
v2: - Reworded to improve readability (Thomas)
Signed-off-by: Sam Ravnborg sam@ravnborg.org Co-developed-by: Lyude Paul lyude@redhat.com Cc: Lyude Paul lyude@redhat.com Acked-by: Thomas Zimmermann tzimmermann@suse.de Cc: Thomas Zimmermann tzimmermann@suse.de Cc: Daniel Vetter daniel@ffwll.ch Cc: Maarten Lankhorst maarten.lankhorst@linux.intel.com Cc: Maxime Ripard mripard@kernel.org Cc: Thomas Zimmermann tzimmermann@suse.de Cc: David Airlie airlied@linux.ie --- drivers/gpu/drm/drm_vblank.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)
diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c index bcf346b3e486..ec2c2083b186 100644 --- a/drivers/gpu/drm/drm_vblank.c +++ b/drivers/gpu/drm/drm_vblank.c @@ -41,6 +41,23 @@ /** * DOC: vblank handling * + * From the computer's perspective, every time the monitor displays + * a new frame the scanout engine have "scanned out" the display image + * from top to bottom, one row of pixels at a time. + * The current row of pixels is referred to as the current scanline. + * + * In addition to the display's visible area, there's usually a couple of + * extra scanlines which aren't actually displayed on the screen + * (the extra scanlines are sometimes used by HDMI audio and friends). + * The period where the extra scanlines are "scanned out" is referred + * to as the vertical blanking period (vblank for short). + * + * On a lot of display hardware, programming needs to take effect during the + * vertical blanking period so that settings like gamma, what frame being + * scanned out, etc. can be safely changed without showing visual tearing + * on the screen. In some unforgiving hardware, some of this programming has + * to both start and end in the same vblank - vertical blanking period. + * * Vertical blanking plays a major role in graphics rendering. To achieve * tear-free display, users must synchronize page flips and/or rendering to * vertical blanking. The DRM API offers ioctls to perform page flips
I am glad that my explanation of vblanks made sense! Some comments below on things I think we could improve here
On Mon, 2020-03-30 at 20:57 +0200, Sam Ravnborg wrote:
Lyude Paul wrote a very good intro to vblank here:
https://lore.kernel.org/dri-devel/faf63d8a9ed23c16af69762f59d0dca6b2bf085f.c...
Add this to the intro chapter in drm_vblank.c so others can benefit from it too.
v2:
- Reworded to improve readability (Thomas)
Signed-off-by: Sam Ravnborg sam@ravnborg.org Co-developed-by: Lyude Paul lyude@redhat.com Cc: Lyude Paul lyude@redhat.com Acked-by: Thomas Zimmermann tzimmermann@suse.de Cc: Thomas Zimmermann tzimmermann@suse.de Cc: Daniel Vetter daniel@ffwll.ch Cc: Maarten Lankhorst maarten.lankhorst@linux.intel.com Cc: Maxime Ripard mripard@kernel.org Cc: Thomas Zimmermann tzimmermann@suse.de Cc: David Airlie airlied@linux.ie
drivers/gpu/drm/drm_vblank.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)
diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c index bcf346b3e486..ec2c2083b186 100644 --- a/drivers/gpu/drm/drm_vblank.c +++ b/drivers/gpu/drm/drm_vblank.c @@ -41,6 +41,23 @@ /**
- DOC: vblank handling
- From the computer's perspective, every time the monitor displays
- a new frame the scanout engine have "scanned out" the display image
- from top to bottom, one row of pixels at a time.
- The current row of pixels is referred to as the current scanline.
- In addition to the display's visible area, there's usually a couple of
- extra scanlines which aren't actually displayed on the screen
- (the extra scanlines are sometimes used by HDMI audio and friends).
- The period where the extra scanlines are "scanned out" is referred
- to as the vertical blanking period (vblank for short).
I'd reword this, starting from "(the extra scanlines…" (I'd also remove the paranthesis):
These extra scanlines don't contain image data, and are occasionally used for features like audio and infoframes. The region made up of these scanlines is referred to as the vertical blanking region, or vblank for short.
I'd also add a simple ascii-art diagram here, since this might make a lot more sense to some people if there's a visual reference. Probably something like this (feel free to get a little creative)
⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽ | | | | | New frame | | | |↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓| |~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~| ← Scanline, updates the |↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓| frame as it travels | | down (AKA "scans out") | | | | | Old frame | | | | | | | | | | | physical bottom of |⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽| ← display ┆xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx┆ ┆xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx┆ ← vertical blanking ┆xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx┆ region ------------------------------------------------
- On a lot of display hardware, programming needs to take effect during
the
- vertical blanking period so that settings like gamma, what frame being
s/what frame being/which frame is being/
- scanned out, etc. can be safely changed without showing visual tearing
- on the screen. In some unforgiving hardware, some of this programming
has
- to both start and end in the same vblank - vertical blanking period.
You can just say vblank here, since we already explained what the vertical blanking period is up above.
Alex Deucher pointed out to me that it's probably also worth mentioning that not all hardware actually fires off the vblank interrupt at the start of the vertical blank, depending on the hardware the interrupt could also happen a few scanlines after the start of vblank, a few scanlines before the start, somewhere in the middle, at the end of the vblank, etc.
Other then that, this looks great so far! Feel free to cc me in the respin for this patch and I'll be happy to give my r-b.
- Vertical blanking plays a major role in graphics rendering. To achieve
- tear-free display, users must synchronize page flips and/or rendering to
- vertical blanking. The DRM API offers ioctls to perform page flips
Hi,
just a few more nits below.
Am 30.03.20 um 23:51 schrieb Lyude Paul:
I am glad that my explanation of vblanks made sense! Some comments below on things I think we could improve here
On Mon, 2020-03-30 at 20:57 +0200, Sam Ravnborg wrote:
Lyude Paul wrote a very good intro to vblank here:
https://lore.kernel.org/dri-devel/faf63d8a9ed23c16af69762f59d0dca6b2bf085f.c...
Add this to the intro chapter in drm_vblank.c so others can benefit from it too.
v2:
- Reworded to improve readability (Thomas)
Signed-off-by: Sam Ravnborg sam@ravnborg.org Co-developed-by: Lyude Paul lyude@redhat.com Cc: Lyude Paul lyude@redhat.com Acked-by: Thomas Zimmermann tzimmermann@suse.de Cc: Thomas Zimmermann tzimmermann@suse.de Cc: Daniel Vetter daniel@ffwll.ch Cc: Maarten Lankhorst maarten.lankhorst@linux.intel.com Cc: Maxime Ripard mripard@kernel.org Cc: Thomas Zimmermann tzimmermann@suse.de Cc: David Airlie airlied@linux.ie
drivers/gpu/drm/drm_vblank.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)
diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c index bcf346b3e486..ec2c2083b186 100644 --- a/drivers/gpu/drm/drm_vblank.c +++ b/drivers/gpu/drm/drm_vblank.c @@ -41,6 +41,23 @@ /**
- DOC: vblank handling
- From the computer's perspective, every time the monitor displays
- a new frame the scanout engine have "scanned out" the display image
- from top to bottom, one row of pixels at a time.
- The current row of pixels is referred to as the current scanline.
- In addition to the display's visible area, there's usually a couple of
- extra scanlines which aren't actually displayed on the screen
- (the extra scanlines are sometimes used by HDMI audio and friends).
- The period where the extra scanlines are "scanned out" is referred
- to as the vertical blanking period (vblank for short).
I'd reword this, starting from "(the extra scanlines…" (I'd also remove the paranthesis):
These extra scanlines don't contain image data, and are occasionally used for features like audio and infoframes. The region made up of these scanlines is referred to as the vertical blanking region, or vblank for short.I'd also add a simple ascii-art diagram here, since this might make a lot more sense to some people if there's a visual reference. Probably something like this (feel free to get a little creative)
⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽ | | | | | New frame | | | |↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓| |~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~| ← Scanline, updates the |↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓| frame as it travels | | down (AKA "scans out") | | | | | Old frame | | | | | | | | | | | physical bottom of |⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽| ← display ┆xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx┆ ┆xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx┆ ← vertical blanking ┆xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx┆ region ------------------------------------------------
- On a lot of display hardware, programming needs to take effect during
the
- vertical blanking period so that settings like gamma, what frame being
s/what frame being/which frame is being/
I still had read it twice in either variant. Maybe:
s/what frame being scanned out/the displayed image buffer
- scanned out, etc. can be safely changed without showing visual tearing
I think tearing refers specifically to mid-frame buffer flips. Maybe
s/tearing/artifacts
or
s/tearing/distortion
Best regards Thomas
- on the screen. In some unforgiving hardware, some of this programming
has
- to both start and end in the same vblank - vertical blanking period.
You can just say vblank here, since we already explained what the vertical blanking period is up above.
Alex Deucher pointed out to me that it's probably also worth mentioning that not all hardware actually fires off the vblank interrupt at the start of the vertical blank, depending on the hardware the interrupt could also happen a few scanlines after the start of vblank, a few scanlines before the start, somewhere in the middle, at the end of the vblank, etc.
Other then that, this looks great so far! Feel free to cc me in the respin for this patch and I'll be happy to give my r-b.
- Vertical blanking plays a major role in graphics rendering. To achieve
- tear-free display, users must synchronize page flips and/or rendering to
- vertical blanking. The DRM API offers ioctls to perform page flips
On Mon, Mar 30, 2020 at 05:51:26PM -0400, Lyude Paul wrote:
I am glad that my explanation of vblanks made sense! Some comments below on things I think we could improve here
On Mon, 2020-03-30 at 20:57 +0200, Sam Ravnborg wrote:
Lyude Paul wrote a very good intro to vblank here:
https://lore.kernel.org/dri-devel/faf63d8a9ed23c16af69762f59d0dca6b2bf085f.c...
Add this to the intro chapter in drm_vblank.c so others can benefit from it too.
v2:
- Reworded to improve readability (Thomas)
Signed-off-by: Sam Ravnborg sam@ravnborg.org Co-developed-by: Lyude Paul lyude@redhat.com Cc: Lyude Paul lyude@redhat.com Acked-by: Thomas Zimmermann tzimmermann@suse.de Cc: Thomas Zimmermann tzimmermann@suse.de Cc: Daniel Vetter daniel@ffwll.ch Cc: Maarten Lankhorst maarten.lankhorst@linux.intel.com Cc: Maxime Ripard mripard@kernel.org Cc: Thomas Zimmermann tzimmermann@suse.de Cc: David Airlie airlied@linux.ie
drivers/gpu/drm/drm_vblank.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)
diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c index bcf346b3e486..ec2c2083b186 100644 --- a/drivers/gpu/drm/drm_vblank.c +++ b/drivers/gpu/drm/drm_vblank.c @@ -41,6 +41,23 @@ /**
- DOC: vblank handling
- From the computer's perspective, every time the monitor displays
- a new frame the scanout engine have "scanned out" the display image
- from top to bottom, one row of pixels at a time.
- The current row of pixels is referred to as the current scanline.
- In addition to the display's visible area, there's usually a couple of
- extra scanlines which aren't actually displayed on the screen
- (the extra scanlines are sometimes used by HDMI audio and friends).
- The period where the extra scanlines are "scanned out" is referred
- to as the vertical blanking period (vblank for short).
I'd reword this, starting from "(the extra scanlines…" (I'd also remove the paranthesis):
These extra scanlines don't contain image data, and are occasionally used for features like audio and infoframes. The region made up of these scanlines is referred to as the vertical blanking region, or vblank for short.I'd also add a simple ascii-art diagram here, since this might make a lot more sense to some people if there's a visual reference. Probably something like this (feel free to get a little creative)
⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽ | | | | | New frame | | | |↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓| |~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~| ← Scanline, updates the |↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓| frame as it travels | | down (AKA "scans out") | | | | | Old frame | | | | | | | | | | | physical bottom of |⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽| ← display ┆xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx┆ ┆xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx┆ ← vertical blanking ┆xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx┆ region ------------------------------------------------
Oh if we go with a nice asci-art picture, can we please also make a note that top of the frame is where the corrected/high-precision timestamp should match? -Daniel
- On a lot of display hardware, programming needs to take effect during
the
- vertical blanking period so that settings like gamma, what frame being
s/what frame being/which frame is being/
- scanned out, etc. can be safely changed without showing visual tearing
- on the screen. In some unforgiving hardware, some of this programming
has
- to both start and end in the same vblank - vertical blanking period.
You can just say vblank here, since we already explained what the vertical blanking period is up above.
Alex Deucher pointed out to me that it's probably also worth mentioning that not all hardware actually fires off the vblank interrupt at the start of the vertical blank, depending on the hardware the interrupt could also happen a few scanlines after the start of vblank, a few scanlines before the start, somewhere in the middle, at the end of the vblank, etc.
Other then that, this looks great so far! Feel free to cc me in the respin for this patch and I'll be happy to give my r-b.
- Vertical blanking plays a major role in graphics rendering. To achieve
- tear-free display, users must synchronize page flips and/or rendering to
- vertical blanking. The DRM API offers ioctls to perform page flips
-- Cheers, Lyude Paul (she/her) Associate Software Engineer at Red Hat
Fix following warnings: drm_framebuffer.h:342: warning: Function parameter or member 'block_width' not described in 'drm_afbc_framebuffer' drm_framebuffer.h:342: warning: Function parameter or member 'block_height' not described in 'drm_afbc_framebuffer'
Trivial spelling mistakes.
Signed-off-by: Sam Ravnborg sam@ravnborg.org Fixes: 55f7f72753ab ("drm/core: Add drm_afbc_framebuffer and a corresponding helper") Cc: Andrzej Pietrasiewicz andrzej.p@collabora.com Cc: Emil Velikov emil.velikov@collabora.com Cc: James Qian Wang james.qian.wang@arm.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Maarten Lankhorst maarten.lankhorst@linux.intel.com Cc: Maxime Ripard mripard@kernel.org Cc: Thomas Zimmermann tzimmermann@suse.de Cc: David Airlie airlied@linux.ie Cc: Daniel Vetter daniel@ffwll.ch --- include/drm/drm_framebuffer.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/drm/drm_framebuffer.h b/include/drm/drm_framebuffer.h index e9f1b0e2968d..b53c0332f040 100644 --- a/include/drm/drm_framebuffer.h +++ b/include/drm/drm_framebuffer.h @@ -308,11 +308,11 @@ struct drm_afbc_framebuffer { */ struct drm_framebuffer base; /** - * @block_widht: width of a single afbc block + * @block_width: width of a single afbc block */ u32 block_width; /** - * @block_widht: height of a single afbc block + * @block_height: height of a single afbc block */ u32 block_height; /**
W dniu 28.03.2020 o 14:20, Sam Ravnborg pisze:
Fix following warnings: drm_framebuffer.h:342: warning: Function parameter or member 'block_width' not described in 'drm_afbc_framebuffer' drm_framebuffer.h:342: warning: Function parameter or member 'block_height' not described in 'drm_afbc_framebuffer'
Trivial spelling mistakes.
Signed-off-by: Sam Ravnborg sam@ravnborg.org Fixes: 55f7f72753ab ("drm/core: Add drm_afbc_framebuffer and a corresponding helper") Cc: Andrzej Pietrasiewicz andrzej.p@collabora.com Cc: Emil Velikov emil.velikov@collabora.com Cc: James Qian Wang james.qian.wang@arm.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Maarten Lankhorst maarten.lankhorst@linux.intel.com Cc: Maxime Ripard mripard@kernel.org Cc: Thomas Zimmermann tzimmermann@suse.de Cc: David Airlie airlied@linux.ie Cc: Daniel Vetter daniel@ffwll.ch
include/drm/drm_framebuffer.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/drm/drm_framebuffer.h b/include/drm/drm_framebuffer.h index e9f1b0e2968d..b53c0332f040 100644 --- a/include/drm/drm_framebuffer.h +++ b/include/drm/drm_framebuffer.h @@ -308,11 +308,11 @@ struct drm_afbc_framebuffer { */ struct drm_framebuffer base; /**
* @block_widht: width of a single afbc block
* @block_width: width of a single afbc block
* @block_widht: height of a single afbc block
* @block_height: height of a single afbc block
Acked-by: Andrzej Pietrasiewicz andrzej.p@collabora.com
Hi Andrzej
On Mon, Mar 30, 2020 at 01:35:18PM +0200, Andrzej Pietrasiewicz wrote:
W dniu 28.03.2020 o 14:20, Sam Ravnborg pisze:
Fix following warnings: drm_framebuffer.h:342: warning: Function parameter or member 'block_width' not described in 'drm_afbc_framebuffer' drm_framebuffer.h:342: warning: Function parameter or member 'block_height' not described in 'drm_afbc_framebuffer'
Trivial spelling mistakes.
Signed-off-by: Sam Ravnborg sam@ravnborg.org Fixes: 55f7f72753ab ("drm/core: Add drm_afbc_framebuffer and a corresponding helper") Cc: Andrzej Pietrasiewicz andrzej.p@collabora.com
...
Acked-by: Andrzej Pietrasiewicz andrzej.p@collabora.com
Thanks. Added a-b and applied to drm-misc-next.
Sam
Fix following warning: gpu_scheduler.h:103: warning: Function parameter or member 'priority' not described in 'drm_sched_entity'
Signed-off-by: Sam Ravnborg sam@ravnborg.org Cc: Nirmoy Das nirmoy.das@amd.com Cc: David Airlie airlied@linux.ie Cc: Daniel Vetter daniel@ffwll.ch --- include/drm/gpu_scheduler.h | 1 + 1 file changed, 1 insertion(+)
diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h index 26b04ff62676..a21b3b92135a 100644 --- a/include/drm/gpu_scheduler.h +++ b/include/drm/gpu_scheduler.h @@ -56,6 +56,7 @@ enum drm_sched_priority { * Jobs from this entity can be scheduled on any scheduler * on this list. * @num_sched_list: number of drm_gpu_schedulers in the sched_list. + * @priority: priority of the entity * @rq_lock: lock to modify the runqueue to which this entity belongs. * @job_queue: the list of jobs of this entity. * @fence_seq: a linearly increasing seqno incremented with each
On Sat, Mar 28, 2020 at 02:20:22PM +0100, Sam Ravnborg wrote:
Fix following warning: gpu_scheduler.h:103: warning: Function parameter or member 'priority' not described in 'drm_sched_entity'
Signed-off-by: Sam Ravnborg sam@ravnborg.org Cc: Nirmoy Das nirmoy.das@amd.com Cc: David Airlie airlied@linux.ie Cc: Daniel Vetter daniel@ffwll.ch
Inline struct comments would be really nice here. Otherwise
Acked-by: Daniel Vetter daniel.vetter@ffwll.ch
include/drm/gpu_scheduler.h | 1 + 1 file changed, 1 insertion(+)
diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h index 26b04ff62676..a21b3b92135a 100644 --- a/include/drm/gpu_scheduler.h +++ b/include/drm/gpu_scheduler.h @@ -56,6 +56,7 @@ enum drm_sched_priority {
Jobs from this entity can be scheduled on any scheduleron this list.- @num_sched_list: number of drm_gpu_schedulers in the sched_list.
- @priority: priority of the entity
- @rq_lock: lock to modify the runqueue to which this entity belongs.
- @job_queue: the list of jobs of this entity.
- @fence_seq: a linearly increasing seqno incremented with each
-- 2.20.1
On Sat, Mar 28, 2020 at 02:20:22PM +0100, Sam Ravnborg wrote:
Fix following warning: gpu_scheduler.h:103: warning: Function parameter or member 'priority' not described in 'drm_sched_entity'
Signed-off-by: Sam Ravnborg sam@ravnborg.org Cc: Nirmoy Das nirmoy.das@amd.com Cc: David Airlie airlied@linux.ie Cc: Daniel Vetter daniel@ffwll.ch
include/drm/gpu_scheduler.h | 1 + 1 file changed, 1 insertion(+)
Committed to drm-misc-next
diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h index 26b04ff62676..a21b3b92135a 100644 --- a/include/drm/gpu_scheduler.h +++ b/include/drm/gpu_scheduler.h @@ -56,6 +56,7 @@ enum drm_sched_priority {
Jobs from this entity can be scheduled on any scheduleron this list.- @num_sched_list: number of drm_gpu_schedulers in the sched_list.
- @priority: priority of the entity
- @rq_lock: lock to modify the runqueue to which this entity belongs.
- @job_queue: the list of jobs of this entity.
- @fence_seq: a linearly increasing seqno incremented with each
-- 2.20.1
Document the callbacks: drm_connector_helper_funcs.prepare_writeback_job drm_connector_helper_funcs.cleanup_writeback_job
The documentation was pulled from the changelong introducing the callbacks, originally written by Laurent.
Addign the missing documentation fixes the following warnings: drm_modeset_helper_vtables.h:1052: warning: Function parameter or member 'prepare_writeback_job' not described in 'drm_connector_helper_funcs' drm_modeset_helper_vtables.h:1052: warning: Function parameter or member 'cleanup_writeback_job' not described in 'drm_connector_helper_funcs'
Signed-off-by: Sam Ravnborg sam@ravnborg.org Cc: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com Cc: Liviu Dudau liviu.dudau@arm.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Maarten Lankhorst maarten.lankhorst@linux.intel.com Cc: Maxime Ripard mripard@kernel.org Cc: Thomas Zimmermann tzimmermann@suse.de Cc: David Airlie airlied@linux.ie --- include/drm/drm_modeset_helper_vtables.h | 31 ++++++++++++++++++++++++ 1 file changed, 31 insertions(+)
diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h index 7c20b1c8b6a7..c51bca1ffec7 100644 --- a/include/drm/drm_modeset_helper_vtables.h +++ b/include/drm/drm_modeset_helper_vtables.h @@ -1075,8 +1075,39 @@ struct drm_connector_helper_funcs { void (*atomic_commit)(struct drm_connector *connector, struct drm_connector_state *state);
+ /** + * @prepare_writeback_job: + * + * As writeback jobs contain a framebuffer, drivers may need to + * prepare and cleanup them the same way they can prepare and + * cleanup framebuffers for planes. + * This optional connector operation is used to support the + * preparation of writeback jobs. + * The job prepare operation is called from + * drm_atomic_helper_prepare_planes() to avoid a new atomic commit + * helper that would need to be called by all drivers not using + * drm_atomic_helper_commit(). + * + * This hook is optional. + * + * This callback is used by the atomic modeset helpers. + */ int (*prepare_writeback_job)(struct drm_writeback_connector *connector, struct drm_writeback_job *job); + /** + * @cleanup_writeback_job: + * + * This optional connector operation is used to support the + * cleanup of writeback jobs. + * The job cleanup operation is called from the existing + * drm_writeback_cleanup_job() function, invoked both when + * destroying the job as part of a aborted commit, or when + * the job completes. + * + * This hook is optional. + * + * This callback is used by the atomic modeset helpers. + */ void (*cleanup_writeback_job)(struct drm_writeback_connector *connector, struct drm_writeback_job *job); };
On Sat, Mar 28, 2020 at 02:20:23PM +0100, Sam Ravnborg wrote:
Document the callbacks: drm_connector_helper_funcs.prepare_writeback_job drm_connector_helper_funcs.cleanup_writeback_job
The documentation was pulled from the changelong introducing the callbacks, originally written by Laurent.
Addign the missing documentation fixes the following warnings: drm_modeset_helper_vtables.h:1052: warning: Function parameter or member 'prepare_writeback_job' not described in 'drm_connector_helper_funcs' drm_modeset_helper_vtables.h:1052: warning: Function parameter or member 'cleanup_writeback_job' not described in 'drm_connector_helper_funcs'
Signed-off-by: Sam Ravnborg sam@ravnborg.org Cc: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com Cc: Liviu Dudau liviu.dudau@arm.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Maarten Lankhorst maarten.lankhorst@linux.intel.com Cc: Maxime Ripard mripard@kernel.org Cc: Thomas Zimmermann tzimmermann@suse.de Cc: David Airlie airlied@linux.ie
include/drm/drm_modeset_helper_vtables.h | 31 ++++++++++++++++++++++++ 1 file changed, 31 insertions(+)
diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h index 7c20b1c8b6a7..c51bca1ffec7 100644 --- a/include/drm/drm_modeset_helper_vtables.h +++ b/include/drm/drm_modeset_helper_vtables.h @@ -1075,8 +1075,39 @@ struct drm_connector_helper_funcs { void (*atomic_commit)(struct drm_connector *connector, struct drm_connector_state *state);
- /**
* @prepare_writeback_job:
Formatting looks funny, your linebreaks here won't go into the generated html and are a bit unusual. I'd remove them and just flow this as a full paragraph.
** As writeback jobs contain a framebuffer, drivers may need to* prepare and cleanup them the same way they can prepare and* cleanup framebuffers for planes.* This optional connector operation is used to support the* preparation of writeback jobs.* The job prepare operation is called from* drm_atomic_helper_prepare_planes() to avoid a new atomic commit* helper that would need to be called by all drivers not using* drm_atomic_helper_commit().
I'd delete "to avoid a new ..." until the end of the sentence. That feels more like stuff in the commit message/review than kernel docs for driver writers.
Instead maybe add "... for struct &drm_writeback_connector connectors only." This gives us a nice link to the writeback docs, and makes it clear that this isn't some general prep/cleanup thing. Similar addition below.
** This hook is optional.** This callback is used by the atomic modeset helpers.*/- /**
* @cleanup_writeback_job:** This optional connector operation is used to support the* cleanup of writeback jobs.* The job cleanup operation is called from the existing* drm_writeback_cleanup_job() function, invoked both when* destroying the job as part of a aborted commit, or when* the job completes.** This hook is optional.** This callback is used by the atomic modeset helpers.*/
With the bikesheds addressed as you see fit:
Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch
Also Laurent owes you one, I've been pestering to fill this gap in his docs since forever ...
Cheers, Daniel
};
2.20.1
Hi Sam,
Thank you for the patch.
On Tue, Mar 31, 2020 at 09:53:18AM +0200, Daniel Vetter wrote:
On Sat, Mar 28, 2020 at 02:20:23PM +0100, Sam Ravnborg wrote:
Document the callbacks: drm_connector_helper_funcs.prepare_writeback_job drm_connector_helper_funcs.cleanup_writeback_job
The documentation was pulled from the changelong introducing the callbacks, originally written by Laurent.
Addign the missing documentation fixes the following warnings: drm_modeset_helper_vtables.h:1052: warning: Function parameter or member 'prepare_writeback_job' not described in 'drm_connector_helper_funcs' drm_modeset_helper_vtables.h:1052: warning: Function parameter or member 'cleanup_writeback_job' not described in 'drm_connector_helper_funcs'
Signed-off-by: Sam Ravnborg sam@ravnborg.org Cc: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com Cc: Liviu Dudau liviu.dudau@arm.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Maarten Lankhorst maarten.lankhorst@linux.intel.com Cc: Maxime Ripard mripard@kernel.org Cc: Thomas Zimmermann tzimmermann@suse.de Cc: David Airlie airlied@linux.ie
include/drm/drm_modeset_helper_vtables.h | 31 ++++++++++++++++++++++++ 1 file changed, 31 insertions(+)
diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h index 7c20b1c8b6a7..c51bca1ffec7 100644 --- a/include/drm/drm_modeset_helper_vtables.h +++ b/include/drm/drm_modeset_helper_vtables.h @@ -1075,8 +1075,39 @@ struct drm_connector_helper_funcs { void (*atomic_commit)(struct drm_connector *connector, struct drm_connector_state *state);
- /**
* @prepare_writeback_job:Formatting looks funny, your linebreaks here won't go into the generated html and are a bit unusual. I'd remove them and just flow this as a full paragraph.
** As writeback jobs contain a framebuffer, drivers may need to* prepare and cleanup them the same way they can prepare and
"cleanup them" or "clean them up" ?
* cleanup framebuffers for planes.
This would be "clean up" too.
* This optional connector operation is used to support the* preparation of writeback jobs.* The job prepare operation is called from* drm_atomic_helper_prepare_planes() to avoid a new atomic commit* helper that would need to be called by all drivers not using* drm_atomic_helper_commit().I'd delete "to avoid a new ..." until the end of the sentence. That feels more like stuff in the commit message/review than kernel docs for driver writers.
Instead maybe add "... for struct &drm_writeback_connector connectors only." This gives us a nice link to the writeback docs, and makes it clear that this isn't some general prep/cleanup thing. Similar addition below.
** This hook is optional.** This callback is used by the atomic modeset helpers.
"hook" or "callback", you decide, but let's be consistent :-) I'd go for "operation" personally as that's what is used above. Same for the cleanup_writeback_job operation below.
*/- /**
* @cleanup_writeback_job:** This optional connector operation is used to support the* cleanup of writeback jobs.* The job cleanup operation is called from the existing* drm_writeback_cleanup_job() function, invoked both when* destroying the job as part of a aborted commit, or when
s/a aborted/an aborted/
* the job completes.** This hook is optional.** This callback is used by the atomic modeset helpers.*/With the bikesheds addressed as you see fit:
Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch
Reviewed-by: Laurent Pinchart laurent.pinchart@ideasonboard.com
Also Laurent owes you one, I've been pestering to fill this gap in his docs since forever ...
I do. Sorry for letting you fix it.
};
Fix kernel-doc warnings for drm_dp_mst_port.fec_capable. This fixed the following warning: drm_dp_mst_helper.h:162: warning: Function parameter or member 'fec_capable' not described in 'drm_dp_mst_port'
Signed-off-by: Sam Ravnborg sam@ravnborg.org Cc: David Francis David.Francis@amd.com Cc: Lyude Paul lyude@redhat.com Cc: Harry Wentland harry.wentland@amd.com Cc: Mikita Lipski mikita.lipski@amd.com Cc: Alex Deucher alexander.deucher@amd.com Cc: Maarten Lankhorst maarten.lankhorst@linux.intel.com Cc: Maxime Ripard mripard@kernel.org Cc: Thomas Zimmermann tzimmermann@suse.de Cc: David Airlie airlied@linux.ie Cc: Daniel Vetter daniel@ffwll.ch --- include/drm/drm_dp_mst_helper.h | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h index bf5e65d2303e..d93e628ebc84 100644 --- a/include/drm/drm_dp_mst_helper.h +++ b/include/drm/drm_dp_mst_helper.h @@ -157,6 +157,10 @@ struct drm_dp_mst_port { */ bool has_audio;
+ /** + * @fec_capable: bool indicating if FEC can be supported + * up to that point in the MST network. + */ bool fec_capable; };
On Sat, 2020-03-28 at 14:20 +0100, Sam Ravnborg wrote:
Fix kernel-doc warnings for drm_dp_mst_port.fec_capable. This fixed the following warning: drm_dp_mst_helper.h:162: warning: Function parameter or member 'fec_capable' not described in 'drm_dp_mst_port'
Signed-off-by: Sam Ravnborg sam@ravnborg.org Cc: David Francis David.Francis@amd.com Cc: Lyude Paul lyude@redhat.com Cc: Harry Wentland harry.wentland@amd.com Cc: Mikita Lipski mikita.lipski@amd.com Cc: Alex Deucher alexander.deucher@amd.com Cc: Maarten Lankhorst maarten.lankhorst@linux.intel.com Cc: Maxime Ripard mripard@kernel.org Cc: Thomas Zimmermann tzimmermann@suse.de Cc: David Airlie airlied@linux.ie Cc: Daniel Vetter daniel@ffwll.ch
include/drm/drm_dp_mst_helper.h | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h index bf5e65d2303e..d93e628ebc84 100644 --- a/include/drm/drm_dp_mst_helper.h +++ b/include/drm/drm_dp_mst_helper.h @@ -157,6 +157,10 @@ struct drm_dp_mst_port { */ bool has_audio;
- /**
* @fec_capable: bool indicating if FEC can be supported* up to that point in the MST network.
s/network/topology, but I can just fix that locally and push this in just a moment. Thanks!
Reviewed-by: Lyude Paul lyude@redhat.com
*/};
Hi Lyude.
On Mon, Mar 30, 2020 at 11:01:12AM -0400, Lyude Paul wrote:
On Sat, 2020-03-28 at 14:20 +0100, Sam Ravnborg wrote:
Fix kernel-doc warnings for drm_dp_mst_port.fec_capable. This fixed the following warning: drm_dp_mst_helper.h:162: warning: Function parameter or member 'fec_capable' not described in 'drm_dp_mst_port'
Signed-off-by: Sam Ravnborg sam@ravnborg.org Cc: David Francis David.Francis@amd.com Cc: Lyude Paul lyude@redhat.com Cc: Harry Wentland harry.wentland@amd.com Cc: Mikita Lipski mikita.lipski@amd.com Cc: Alex Deucher alexander.deucher@amd.com Cc: Maarten Lankhorst maarten.lankhorst@linux.intel.com Cc: Maxime Ripard mripard@kernel.org Cc: Thomas Zimmermann tzimmermann@suse.de Cc: David Airlie airlied@linux.ie Cc: Daniel Vetter daniel@ffwll.ch
include/drm/drm_dp_mst_helper.h | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h index bf5e65d2303e..d93e628ebc84 100644 --- a/include/drm/drm_dp_mst_helper.h +++ b/include/drm/drm_dp_mst_helper.h @@ -157,6 +157,10 @@ struct drm_dp_mst_port { */ bool has_audio;
- /**
* @fec_capable: bool indicating if FEC can be supported* up to that point in the MST network.s/network/topology, but I can just fix that locally and push this in just a moment. Thanks!
Reviewed-by: Lyude Paul lyude@redhat.com
Thanks for fixing and applying!
Can you also take a look at PATCH 1/6 and if OK provide your s-o-b that should follow the Co-developed-by: ... I know the text has seen a few changes but the original source came from you.
Sam
Add missing documentation to fix following warning: panel.c:303: warning: Function parameter or member 'bridge' not described in 'drm_panel_bridge_connector'
Signed-off-by: Sam Ravnborg sam@ravnborg.org Cc: Laurent Pinchart laurent.pinchart@ideasonboard.com Cc: Boris Brezillon boris.brezillon@collabora.com Cc: Mihail Atanassov Mihail.Atanassov@arm.com Cc: Andrzej Hajda a.hajda@samsung.com Cc: Neil Armstrong narmstrong@baylibre.com Cc: Jonas Karlman jonas@kwiboo.se Cc: Jernej Skrabec jernej.skrabec@siol.net --- drivers/gpu/drm/bridge/panel.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c index 8461ee8304ba..e933f1c47f5d 100644 --- a/drivers/gpu/drm/bridge/panel.c +++ b/drivers/gpu/drm/bridge/panel.c @@ -311,6 +311,7 @@ EXPORT_SYMBOL(devm_drm_panel_bridge_add_typed);
/** * drm_panel_bridge_connector - return the connector for the panel bridge + * @bridge: The drm_bridge. * * drm_panel_bridge creates the connector. * This function gives external access to the connector.
On Sat, Mar 28, 2020 at 02:20:25PM +0100, Sam Ravnborg wrote:
Add missing documentation to fix following warning: panel.c:303: warning: Function parameter or member 'bridge' not described in 'drm_panel_bridge_connector'
Signed-off-by: Sam Ravnborg sam@ravnborg.org Cc: Laurent Pinchart laurent.pinchart@ideasonboard.com Cc: Boris Brezillon boris.brezillon@collabora.com Cc: Mihail Atanassov Mihail.Atanassov@arm.com Cc: Andrzej Hajda a.hajda@samsung.com Cc: Neil Armstrong narmstrong@baylibre.com Cc: Jonas Karlman jonas@kwiboo.se Cc: Jernej Skrabec jernej.skrabec@siol.net
Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch
drivers/gpu/drm/bridge/panel.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c index 8461ee8304ba..e933f1c47f5d 100644 --- a/drivers/gpu/drm/bridge/panel.c +++ b/drivers/gpu/drm/bridge/panel.c @@ -311,6 +311,7 @@ EXPORT_SYMBOL(devm_drm_panel_bridge_add_typed);
/**
- drm_panel_bridge_connector - return the connector for the panel bridge
- @bridge: The drm_bridge.
- drm_panel_bridge creates the connector.
- This function gives external access to the connector.
-- 2.20.1
On Sat, Mar 28, 2020 at 02:20:25PM +0100, Sam Ravnborg wrote:
Add missing documentation to fix following warning: panel.c:303: warning: Function parameter or member 'bridge' not described in 'drm_panel_bridge_connector'
Signed-off-by: Sam Ravnborg sam@ravnborg.org Cc: Laurent Pinchart laurent.pinchart@ideasonboard.com Cc: Boris Brezillon boris.brezillon@collabora.com Cc: Mihail Atanassov Mihail.Atanassov@arm.com Cc: Andrzej Hajda a.hajda@samsung.com Cc: Neil Armstrong narmstrong@baylibre.com Cc: Jonas Karlman jonas@kwiboo.se Cc: Jernej Skrabec jernej.skrabec@siol.net
drivers/gpu/drm/bridge/panel.c | 1 + 1 file changed, 1 insertion(+)
Committed to drm-misc-next
diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c index 8461ee8304ba..e933f1c47f5d 100644 --- a/drivers/gpu/drm/bridge/panel.c +++ b/drivers/gpu/drm/bridge/panel.c @@ -311,6 +311,7 @@ EXPORT_SYMBOL(devm_drm_panel_bridge_add_typed);
/**
- drm_panel_bridge_connector - return the connector for the panel bridge
- @bridge: The drm_bridge.
- drm_panel_bridge creates the connector.
- This function gives external access to the connector.
-- 2.20.1
dri-devel@lists.freedesktop.org
-
Andrzej Pietrasiewicz -
Daniel Vetter -
Laurent Pinchart -
Lyude Paul -
Sam Ravnborg -
Thomas Zimmermann