* [PATCH 1/3] drm: add crtc helper drm_crtc_from_index()
2016-12-29 12:41 [PATCH 0/3] Add CRTC helper drm_crtc_from_index() Shawn Guo
@ 2016-12-29 12:41 ` Shawn Guo
2016-12-30 11:14 ` Daniel Vetter
2016-12-29 12:41 ` [PATCH 2/3] drm: zte: use " Shawn Guo
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Shawn Guo @ 2016-12-29 12:41 UTC (permalink / raw)
To: dri-devel; +Cc: Daniel Vetter, Shawn Guo
From: Shawn Guo <shawn.guo@linaro.org>
It adds a crtc helper drm_crtc_from_index() to find the registered CRTC
with a given index, just like drm_plane_from_index().
Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
drivers/gpu/drm/drm_crtc.c | 20 ++++++++++++++++++++
include/drm/drm_crtc.h | 1 +
2 files changed, 21 insertions(+)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index e75f62cd8a65..5c1bb1f34697 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -47,6 +47,26 @@
#include "drm_internal.h"
/**
+ * drm_crtc_from_index - find the registered CRTC at an index
+ * @dev: DRM device
+ * @idx: index of registered CRTC to find for
+ *
+ * Given a CRTC index, return the registered CRTC from DRM device's
+ * list of CRTCs with matching index.
+ */
+struct drm_crtc *drm_crtc_from_index(struct drm_device *dev, int idx)
+{
+ struct drm_crtc *crtc;
+
+ drm_for_each_crtc(crtc, dev)
+ if (idx == crtc->index)
+ return crtc;
+
+ return NULL;
+}
+EXPORT_SYMBOL(drm_crtc_from_index);
+
+/**
* drm_crtc_force_disable - Forcibly turn off a CRTC
* @crtc: CRTC to turn off
*
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 946672f97e1e..a5627eb8621f 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -830,6 +830,7 @@ void drm_crtc_get_hv_timing(const struct drm_display_mode *mode,
int drm_crtc_force_disable_all(struct drm_device *dev);
int drm_mode_set_config_internal(struct drm_mode_set *set);
+struct drm_crtc *drm_crtc_from_index(struct drm_device *dev, int idx);
/* Helpers */
static inline struct drm_crtc *drm_crtc_find(struct drm_device *dev,
--
1.9.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH 1/3] drm: add crtc helper drm_crtc_from_index()
2016-12-29 12:41 ` [PATCH 1/3] drm: add crtc " Shawn Guo
@ 2016-12-30 11:14 ` Daniel Vetter
0 siblings, 0 replies; 8+ messages in thread
From: Daniel Vetter @ 2016-12-30 11:14 UTC (permalink / raw)
To: Shawn Guo; +Cc: Daniel Vetter, dri-devel
On Thu, Dec 29, 2016 at 08:41:28PM +0800, Shawn Guo wrote:
> From: Shawn Guo <shawn.guo@linaro.org>
>
> It adds a crtc helper drm_crtc_from_index() to find the registered CRTC
> with a given index, just like drm_plane_from_index().
>
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> ---
> drivers/gpu/drm/drm_crtc.c | 20 ++++++++++++++++++++
> include/drm/drm_crtc.h | 1 +
> 2 files changed, 21 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index e75f62cd8a65..5c1bb1f34697 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -47,6 +47,26 @@
> #include "drm_internal.h"
>
> /**
> + * drm_crtc_from_index - find the registered CRTC at an index
> + * @dev: DRM device
> + * @idx: index of registered CRTC to find for
> + *
> + * Given a CRTC index, return the registered CRTC from DRM device's
> + * list of CRTCs with matching index.
> + */
Might be useful to go through al the drm_*_inde and drm_*_from_index
functions we have and a sentence to each likt "This is the inverse of
drm_crtc_index()." to cross link them a bit.
Also here a sentence explaining that this is useful in the vblank code,
e.g.
"This is useful in the vblank callbacks (like &drm_driver.enable_vblank or
&drm_driver.disable_vblank), since that still deals with indices instead
of pointers to &struct drm_crtc."
Follow-up pach, since I applied these here already.
-Daniel
> +struct drm_crtc *drm_crtc_from_index(struct drm_device *dev, int idx)
> +{
> + struct drm_crtc *crtc;
> +
> + drm_for_each_crtc(crtc, dev)
> + if (idx == crtc->index)
> + return crtc;
> +
> + return NULL;
> +}
> +EXPORT_SYMBOL(drm_crtc_from_index);
> +
> +/**
> * drm_crtc_force_disable - Forcibly turn off a CRTC
> * @crtc: CRTC to turn off
> *
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 946672f97e1e..a5627eb8621f 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -830,6 +830,7 @@ void drm_crtc_get_hv_timing(const struct drm_display_mode *mode,
> int drm_crtc_force_disable_all(struct drm_device *dev);
>
> int drm_mode_set_config_internal(struct drm_mode_set *set);
> +struct drm_crtc *drm_crtc_from_index(struct drm_device *dev, int idx);
>
> /* Helpers */
> static inline struct drm_crtc *drm_crtc_find(struct drm_device *dev,
> --
> 1.9.1
>
--
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/3] drm: zte: use crtc helper drm_crtc_from_index()
2016-12-29 12:41 [PATCH 0/3] Add CRTC helper drm_crtc_from_index() Shawn Guo
2016-12-29 12:41 ` [PATCH 1/3] drm: add crtc " Shawn Guo
@ 2016-12-29 12:41 ` Shawn Guo
2016-12-29 12:41 ` [PATCH 3/3] drm: rockchip: " Shawn Guo
2016-12-30 11:16 ` [PATCH 0/3] Add CRTC " Daniel Vetter
3 siblings, 0 replies; 8+ messages in thread
From: Shawn Guo @ 2016-12-29 12:41 UTC (permalink / raw)
To: dri-devel; +Cc: Daniel Vetter, Shawn Guo
From: Shawn Guo <shawn.guo@linaro.org>
Function zx_find_crtc() does the exactly same thing as what crtc helper
drm_crtc_from_index() provides. Use the helper to save some code.
Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
drivers/gpu/drm/zte/zx_vou.c | 15 ++-------------
1 file changed, 2 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/zte/zx_vou.c b/drivers/gpu/drm/zte/zx_vou.c
index 73fe15c17c32..a86e3a5852a2 100644
--- a/drivers/gpu/drm/zte/zx_vou.c
+++ b/drivers/gpu/drm/zte/zx_vou.c
@@ -355,17 +355,6 @@ static int zx_crtc_init(struct drm_device *drm, struct zx_vou_hw *vou,
return 0;
}
-static inline struct drm_crtc *zx_find_crtc(struct drm_device *drm, int pipe)
-{
- struct drm_crtc *crtc;
-
- list_for_each_entry(crtc, &drm->mode_config.crtc_list, head)
- if (crtc->index == pipe)
- return crtc;
-
- return NULL;
-}
-
int zx_vou_enable_vblank(struct drm_device *drm, unsigned int pipe)
{
struct drm_crtc *crtc;
@@ -373,7 +362,7 @@ int zx_vou_enable_vblank(struct drm_device *drm, unsigned int pipe)
struct zx_vou_hw *vou;
u32 int_frame_mask;
- crtc = zx_find_crtc(drm, pipe);
+ crtc = drm_crtc_from_index(drm, pipe);
if (!crtc)
return 0;
@@ -393,7 +382,7 @@ void zx_vou_disable_vblank(struct drm_device *drm, unsigned int pipe)
struct zx_crtc *zcrtc;
struct zx_vou_hw *vou;
- crtc = zx_find_crtc(drm, pipe);
+ crtc = drm_crtc_from_index(drm, pipe);
if (!crtc)
return;
--
1.9.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 3/3] drm: rockchip: use crtc helper drm_crtc_from_index()
2016-12-29 12:41 [PATCH 0/3] Add CRTC helper drm_crtc_from_index() Shawn Guo
2016-12-29 12:41 ` [PATCH 1/3] drm: add crtc " Shawn Guo
2016-12-29 12:41 ` [PATCH 2/3] drm: zte: use " Shawn Guo
@ 2016-12-29 12:41 ` Shawn Guo
2016-12-30 11:16 ` [PATCH 0/3] Add CRTC " Daniel Vetter
3 siblings, 0 replies; 8+ messages in thread
From: Shawn Guo @ 2016-12-29 12:41 UTC (permalink / raw)
To: dri-devel; +Cc: Daniel Vetter, Shawn Guo
From: Shawn Guo <shawn.guo@linaro.org>
Function rockchip_crtc_from_pipe() does the exactly same thing as what
crtc helper drm_crtc_from_index() provides. Use the helper to save
some code.
Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 17 ++---------------
1 file changed, 2 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
index 2390c8577617..c30d649cb147 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
@@ -99,24 +99,11 @@ void rockchip_unregister_crtc_funcs(struct drm_crtc *crtc)
priv->crtc_funcs[pipe] = NULL;
}
-static struct drm_crtc *rockchip_crtc_from_pipe(struct drm_device *drm,
- int pipe)
-{
- struct drm_crtc *crtc;
- int i = 0;
-
- list_for_each_entry(crtc, &drm->mode_config.crtc_list, head)
- if (i++ == pipe)
- return crtc;
-
- return NULL;
-}
-
static int rockchip_drm_crtc_enable_vblank(struct drm_device *dev,
unsigned int pipe)
{
struct rockchip_drm_private *priv = dev->dev_private;
- struct drm_crtc *crtc = rockchip_crtc_from_pipe(dev, pipe);
+ struct drm_crtc *crtc = drm_crtc_from_index(dev, pipe);
if (crtc && priv->crtc_funcs[pipe] &&
priv->crtc_funcs[pipe]->enable_vblank)
@@ -129,7 +116,7 @@ static void rockchip_drm_crtc_disable_vblank(struct drm_device *dev,
unsigned int pipe)
{
struct rockchip_drm_private *priv = dev->dev_private;
- struct drm_crtc *crtc = rockchip_crtc_from_pipe(dev, pipe);
+ struct drm_crtc *crtc = drm_crtc_from_index(dev, pipe);
if (crtc && priv->crtc_funcs[pipe] &&
priv->crtc_funcs[pipe]->enable_vblank)
--
1.9.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH 0/3] Add CRTC helper drm_crtc_from_index()
2016-12-29 12:41 [PATCH 0/3] Add CRTC helper drm_crtc_from_index() Shawn Guo
` (2 preceding siblings ...)
2016-12-29 12:41 ` [PATCH 3/3] drm: rockchip: " Shawn Guo
@ 2016-12-30 11:16 ` Daniel Vetter
2017-01-09 9:03 ` Shawn Guo
3 siblings, 1 reply; 8+ messages in thread
From: Daniel Vetter @ 2016-12-30 11:16 UTC (permalink / raw)
To: Shawn Guo; +Cc: Daniel Vetter, dri-devel
On Thu, Dec 29, 2016 at 08:41:27PM +0800, Shawn Guo wrote:
> From: Shawn Guo <shawn.guo@linaro.org>
>
> This is a suggestion from Sean Paul. ZTE and Rockchip DRM drivers have
> the similar code to find registered CRTC by given index. The series
> create a CRTC helper drm_crtc_from_index() to do the job, just like
> drm_plane_from_index() to search plane, and change ZTE, Rockchip driver
> to use the helper.
>
> Shawn Guo (3):
> drm: add crtc helper drm_crtc_from_index()
> drm: zte: use crtc helper drm_crtc_from_index()
> drm: rockchip: use crtc helper drm_crtc_from_index()
Entire series applied. I suspect that there's more drivers open-coding
something like this in their vblank code, might be worth it to grep for
them all and do a quick audit.
Thanks a lot,
-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
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH 0/3] Add CRTC helper drm_crtc_from_index()
2016-12-30 11:16 ` [PATCH 0/3] Add CRTC " Daniel Vetter
@ 2017-01-09 9:03 ` Shawn Guo
2017-01-09 9:58 ` Daniel Vetter
0 siblings, 1 reply; 8+ messages in thread
From: Shawn Guo @ 2017-01-09 9:03 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Daniel Vetter, dri-devel
On Fri, Dec 30, 2016 at 12:16:43PM +0100, Daniel Vetter wrote:
> Entire series applied. I suspect that there's more drivers open-coding
> something like this in their vblank code, might be worth it to grep for
> them all and do a quick audit.
I did a round of audit on all drivers vblank code, and found there are
basically 3 categories:
1. Drivers that only support one CRTC. In this case, the 'pipe' is not
used at all in vblank code. So we need to do thing about them.
Examples: arm, atmel-hlcdc, mxsfb, etc.
2. Drivers that maintain CRTC pointers and number in the private
structure. And the fields are used in somewhere else beside vblank
code. The cleanup will need a bit surgery on the driver code. I would
leave them out for driver owners/maintainers. Examples: armada, msm,
sti, etc.
3. Drivers that can use the helper for benefit, including exynos, kirin,
mediatek, nouveau, tegra and vc4. I have already prepared a series for
these drivers, and will send it out for review shortly.
Shawn
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/3] Add CRTC helper drm_crtc_from_index()
2017-01-09 9:03 ` Shawn Guo
@ 2017-01-09 9:58 ` Daniel Vetter
0 siblings, 0 replies; 8+ messages in thread
From: Daniel Vetter @ 2017-01-09 9:58 UTC (permalink / raw)
To: Shawn Guo; +Cc: Daniel Vetter, dri-devel
On Mon, Jan 09, 2017 at 05:03:35PM +0800, Shawn Guo wrote:
> On Fri, Dec 30, 2016 at 12:16:43PM +0100, Daniel Vetter wrote:
> > Entire series applied. I suspect that there's more drivers open-coding
> > something like this in their vblank code, might be worth it to grep for
> > them all and do a quick audit.
>
> I did a round of audit on all drivers vblank code, and found there are
> basically 3 categories:
>
> 1. Drivers that only support one CRTC. In this case, the 'pipe' is not
> used at all in vblank code. So we need to do thing about them.
> Examples: arm, atmel-hlcdc, mxsfb, etc.
>
> 2. Drivers that maintain CRTC pointers and number in the private
> structure. And the fields are used in somewhere else beside vblank
> code. The cleanup will need a bit surgery on the driver code. I would
> leave them out for driver owners/maintainers. Examples: armada, msm,
> sti, etc.
>
> 3. Drivers that can use the helper for benefit, including exynos, kirin,
> mediatek, nouveau, tegra and vc4. I have already prepared a series for
> these drivers, and will send it out for review shortly.
Awesome, thanks a lot for doing this. One of my long-term goals is to
switch the vblank helper callbacks over from int pipe to struct drm_crtc
*, stuff like this helps a lot in prepping the ground.
-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
^ permalink raw reply [flat|nested] 8+ messages in thread