On Thu, May 05, 2016 at 03:24:31PM +0200, Noralf Trønnes wrote:

Add drm_fb_cma_create_with_funcs() for drivers that need to set the dirty() callback.

Cc: laurent.pinchart@ideasonboard.com Signed-off-by: Noralf Trønnes noralf@tronnes.org

---

drivers/gpu/drm/drm_fb_cma_helper.c | 29 +++++++++++++++++++++++------ include/drm/drm_fb_cma_helper.h | 3 +++ 2 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c index 086f600..7165209 100644 --- a/drivers/gpu/drm/drm_fb_cma_helper.c +++ b/drivers/gpu/drm/drm_fb_cma_helper.c @@ -161,13 +161,16 @@ static struct drm_fb_cma *drm_fb_cma_alloc(struct drm_device *dev, }

/**

• • drm_fb_cma_create() - (struct drm_mode_config_funcs *)->fb_create callback function

• • drm_fb_cma_create_with_funcs() - helper function for the

• •                              &drm_mode_config_funcs ->fb_create
    

• •                              callback function
    

• • If your hardware has special alignment or pitch requirements these should be
• • checked before calling this function.

• • This can be used to set &drm_framebuffer_funcs for drivers that need the
• • dirty() callback.

Please reference the other function in your kerneldoc using drm_fb_cma_create() syntax. That will create a hyperlink. With such sets of functions it's always good to cross link them and explain exactly when another one is more appropriate. E.g. here "If your driver does not need a custom &drm_framebuffer_funcs then just use drm_fb_cma_create() directly."

Similar, but other way round for the existing one.

Again please check with make htmldocs that it all looks good.

Otherwise lgtm. -Daniel

*/ -struct drm_framebuffer *drm_fb_cma_create(struct drm_device *dev,

• struct drm_file *file_priv, const struct drm_mode_fb_cmd2 *mode_cmd)

+struct drm_framebuffer *drm_fb_cma_create_with_funcs(struct drm_device *dev,

• struct drm_file *file_priv, const struct drm_mode_fb_cmd2 *mode_cmd,
• struct drm_framebuffer_funcs *funcs)

{ struct drm_fb_cma *fb_cma; struct drm_gem_cma_object *objs[4]; @@ -204,7 +207,7 @@ struct drm_framebuffer *drm_fb_cma_create(struct drm_device *dev, objs[i] = to_drm_gem_cma_obj(obj); }

• fb_cma = drm_fb_cma_alloc(dev, mode_cmd, objs, i, &drm_fb_cma_funcs);

• fb_cma = drm_fb_cma_alloc(dev, mode_cmd, objs, i, funcs); if (IS_ERR(fb_cma)) { ret = PTR_ERR(fb_cma); goto err_gem_object_unreference;

@@ -217,6 +220,20 @@ err_gem_object_unreference: drm_gem_object_unreference_unlocked(&objs[i]->base); return ERR_PTR(ret); } +EXPORT_SYMBOL_GPL(drm_fb_cma_create_with_funcs);

+/**

• • drm_fb_cma_create() - &drm_mode_config_funcs ->fb_create callback function
• • If your hardware has special alignment or pitch requirements these should be
• • checked before calling this function.
• */

+struct drm_framebuffer *drm_fb_cma_create(struct drm_device *dev,

• struct drm_file *file_priv, const struct drm_mode_fb_cmd2 *mode_cmd)

+{

• return drm_fb_cma_create_with_funcs(dev, file_priv, mode_cmd,

• 			    &drm_fb_cma_funcs);
  

+} EXPORT_SYMBOL_GPL(drm_fb_cma_create);

/** diff --git a/include/drm/drm_fb_cma_helper.h b/include/drm/drm_fb_cma_helper.h index c6d9c9c..1f9a8bc 100644 --- a/include/drm/drm_fb_cma_helper.h +++ b/include/drm/drm_fb_cma_helper.h @@ -31,6 +31,9 @@ void drm_fb_cma_destroy(struct drm_framebuffer *fb); int drm_fb_cma_create_handle(struct drm_framebuffer *fb, struct drm_file *file_priv, unsigned int *handle);

+struct drm_framebuffer *drm_fb_cma_create_with_funcs(struct drm_device *dev,

• struct drm_file *file_priv, const struct drm_mode_fb_cmd2 *mode_cmd,
• struct drm_framebuffer_funcs *funcs);

struct drm_framebuffer *drm_fb_cma_create(struct drm_device *dev, struct drm_file *file_priv, const struct drm_mode_fb_cmd2 *mode_cmd);

-- 2.2.2

On Fri, May 06, 2016 at 03:01:37PM +0200, Noralf Trønnes wrote:

Den 05.05.2016 18:27, skrev Daniel Vetter:

...

On Thu, May 05, 2016 at 03:24:31PM +0200, Noralf Trønnes wrote:

...

Add drm_fb_cma_create_with_funcs() for drivers that need to set the dirty() callback.

Cc: laurent.pinchart@ideasonboard.com Signed-off-by: Noralf Trønnes noralf@tronnes.org

---

drivers/gpu/drm/drm_fb_cma_helper.c | 29 +++++++++++++++++++++++------ include/drm/drm_fb_cma_helper.h | 3 +++ 2 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c index 086f600..7165209 100644 --- a/drivers/gpu/drm/drm_fb_cma_helper.c +++ b/drivers/gpu/drm/drm_fb_cma_helper.c @@ -161,13 +161,16 @@ static struct drm_fb_cma *drm_fb_cma_alloc(struct drm_device *dev, } /**

• • drm_fb_cma_create() - (struct drm_mode_config_funcs *)->fb_create callback function

• • drm_fb_cma_create_with_funcs() - helper function for the

• •                              &drm_mode_config_funcs ->fb_create
    

• •                              callback function
    

• • If your hardware has special alignment or pitch requirements these should be
• • checked before calling this function.

• • This can be used to set &drm_framebuffer_funcs for drivers that need the
• • dirty() callback.

Please reference the other function in your kerneldoc using drm_fb_cma_create() syntax. That will create a hyperlink. With such sets of functions it's always good to cross link them and explain exactly when another one is more appropriate. E.g. here "If your driver does not need a custom &drm_framebuffer_funcs then just use drm_fb_cma_create() directly."

Similar, but other way round for the existing one.

Again please check with make htmldocs that it all looks good.

Ok, I didn't understand this htmldocs stuff, I thought it picked up the docs by magic or something. It turns out that drm_fb_cma_helper isn't mentioned in gpu.tmpl so I have to make a patch for that.

Oh right, cma fb helpers aren't pulled int at all. Would be great if you can fix that.

Is there an order to things where I should put it in gpu.tmpl? (drm_simple_kms_helper also)

This is the current order:

Mode Setting Helper Functions

Atomic Modeset Helper Functions Reference Modeset Helper Reference for Common Vtables Legacy CRTC/Modeset Helper Functions Reference Output Probing Helper Functions Reference fbdev Helper Functions Reference Display Port Helper Functions Reference Display Port MST Helper Functions Reference MIPI DSI Helper Functions Reference EDID Helper Functions Reference Rectangle Utilities Reference Flip-work Helper Reference HDMI Infoframes Helper Reference Plane Helper Reference Tile group Bridges

Just add it somewhere to this list of helpers where you think it fits. We're not that structured yet.

And the code example I put in drm_fb_cma_helper DOC: looks terrible, maybe it looks better in the intel augmented version?

Yeah, in upstream it's terrible. But if you base your patch on top of drm-intel-nightly, or pull topic/kerneldoc in, and install asciidoc, then it should be fairly pretty. Note: That asciidoc support is pretty hacked up, it takes 10-20 minutes to build the gpu docs with that. We're working on something much better. -Daniel

Noralf.

...

Otherwise lgtm. -Daniel

...

*/ -struct drm_framebuffer *drm_fb_cma_create(struct drm_device *dev,

• struct drm_file *file_priv, const struct drm_mode_fb_cmd2 *mode_cmd)

+struct drm_framebuffer *drm_fb_cma_create_with_funcs(struct drm_device *dev,

• struct drm_file *file_priv, const struct drm_mode_fb_cmd2 *mode_cmd,
• struct drm_framebuffer_funcs *funcs)

{ struct drm_fb_cma *fb_cma; struct drm_gem_cma_object *objs[4]; @@ -204,7 +207,7 @@ struct drm_framebuffer *drm_fb_cma_create(struct drm_device *dev, objs[i] = to_drm_gem_cma_obj(obj); }

• fb_cma = drm_fb_cma_alloc(dev, mode_cmd, objs, i, &drm_fb_cma_funcs);

• fb_cma = drm_fb_cma_alloc(dev, mode_cmd, objs, i, funcs); if (IS_ERR(fb_cma)) { ret = PTR_ERR(fb_cma); goto err_gem_object_unreference;

@@ -217,6 +220,20 @@ err_gem_object_unreference: drm_gem_object_unreference_unlocked(&objs[i]->base); return ERR_PTR(ret); } +EXPORT_SYMBOL_GPL(drm_fb_cma_create_with_funcs);

+/**

• • drm_fb_cma_create() - &drm_mode_config_funcs ->fb_create callback function
• • If your hardware has special alignment or pitch requirements these should be
• • checked before calling this function.
• */

+struct drm_framebuffer *drm_fb_cma_create(struct drm_device *dev,

• struct drm_file *file_priv, const struct drm_mode_fb_cmd2 *mode_cmd)

+{

• return drm_fb_cma_create_with_funcs(dev, file_priv, mode_cmd,

• 			    &drm_fb_cma_funcs);
  

+} EXPORT_SYMBOL_GPL(drm_fb_cma_create); /** diff --git a/include/drm/drm_fb_cma_helper.h b/include/drm/drm_fb_cma_helper.h index c6d9c9c..1f9a8bc 100644 --- a/include/drm/drm_fb_cma_helper.h +++ b/include/drm/drm_fb_cma_helper.h @@ -31,6 +31,9 @@ void drm_fb_cma_destroy(struct drm_framebuffer *fb); int drm_fb_cma_create_handle(struct drm_framebuffer *fb, struct drm_file *file_priv, unsigned int *handle); +struct drm_framebuffer *drm_fb_cma_create_with_funcs(struct drm_device *dev,

• struct drm_file *file_priv, const struct drm_mode_fb_cmd2 *mode_cmd,
• struct drm_framebuffer_funcs *funcs);

struct drm_framebuffer *drm_fb_cma_create(struct drm_device *dev, struct drm_file *file_priv, const struct drm_mode_fb_cmd2 *mode_cmd); -- 2.2.2

---

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

On Thu, May 05, 2016 at 03:24:32PM +0200, Noralf Trønnes wrote:

Make drm_encoder_helper_funcs and it's functions optional to avoid having dummy functions.

Signed-off-by: Noralf Trønnes noralf@tronnes.org

Please also update the kerneldoc and mention there that the enable/disable hooks are optional. You can build the hmtl docs using

$ make htmldocs

to check the result. The vtables are documented in include/drm/drm_helper_vtables.h

---

drivers/gpu/drm/drm_atomic_helper.c | 11 ++++++++-- drivers/gpu/drm/drm_crtc_helper.c | 41 +++++++++++++++++++++++++++++--------

Hm, tbh I wouldn't bother at all with crtc helpers and just drop that part. Old drivers should converted to atomic, new drivers should just use atomic. No need to touch that code at all. -Daniel

2 files changed, 42 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 92e11a2..03ea049 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -696,6 +696,8 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state) continue;

funcs = encoder->helper_private;

• if (!funcs)
  

• 	continue;
  
  

  DRM_DEBUG_ATOMIC("disabling [ENCODER:%d:%s]\n", encoder->base.id, encoder->name);

@@ -711,7 +713,7 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state) funcs->prepare(encoder); else if (funcs->disable) funcs->disable(encoder);

• else
  

• else if (funcs->dpms)
  funcs->dpms(encoder, DRM_MODE_DPMS_OFF);
  
  

  drm_bridge_post_disable(encoder->bridge);

@@ -859,6 +861,9 @@ crtc_set_mode(struct drm_device *dev, struct drm_atomic_state *old_state)

encoder = connector->state->best_encoder;
funcs = encoder->helper_private;

• if (!funcs)
  

• 	continue;
  

• new_crtc_state = connector->state->crtc->state; mode = &new_crtc_state->mode; adjusted_mode = &new_crtc_state->adjusted_mode;

@@ -964,6 +969,8 @@ void drm_atomic_helper_commit_modeset_enables(struct drm_device *dev,

encoder = connector->state->best_encoder;
funcs = encoder->helper_private;

• if (!funcs)
  

• 	continue;
  
  

  DRM_DEBUG_ATOMIC("enabling [ENCODER:%d:%s]\n", encoder->base.id, encoder->name);

@@ -976,7 +983,7 @@ void drm_atomic_helper_commit_modeset_enables(struct drm_device *dev,

if (funcs->enable)
	funcs->enable(encoder);

• else
  

• else if (funcs->commit)
  funcs->commit(encoder);
  
  

  drm_bridge_enable(encoder->bridge);

diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c index 66ca313..6e6ab38 100644 --- a/drivers/gpu/drm/drm_crtc_helper.c +++ b/drivers/gpu/drm/drm_crtc_helper.c @@ -170,11 +170,14 @@ drm_encoder_disable(struct drm_encoder *encoder) { const struct drm_encoder_helper_funcs *encoder_funcs = encoder->helper_private;

• if (!encoder_funcs)

• return;
  

• drm_bridge_disable(encoder->bridge);

  if (encoder_funcs->disable) (*encoder_funcs->disable)(encoder);

• else

• else if (encoder_funcs->dpms) (*encoder_funcs->dpms)(encoder, DRM_MODE_DPMS_OFF);

  drm_bridge_post_disable(encoder->bridge);

@@ -248,6 +251,9 @@ drm_crtc_prepare_encoders(struct drm_device *dev)

drm_for_each_encoder(encoder, dev) { encoder_funcs = encoder->helper_private;

• if (!encoder_funcs)
  

• 	continue;
  

• /* Disable unused encoders */ if (encoder->crtc == NULL) drm_encoder_disable(encoder);

@@ -326,6 +332,10 @@ bool drm_crtc_helper_set_mode(struct drm_crtc *crtc, if (encoder->crtc != crtc) continue;

• encoder_funcs = encoder->helper_private;
  

• if (!encoder_funcs)
  

• 	continue;
  

• ret = drm_bridge_mode_fixup(encoder->bridge, mode, adjusted_mode); if (!ret) {

@@ -360,11 +370,15 @@ bool drm_crtc_helper_set_mode(struct drm_crtc *crtc, if (encoder->crtc != crtc) continue;

• encoder_funcs = encoder->helper_private;
  

• if (!encoder_funcs)
  

• 	continue;
  

• drm_bridge_disable(encoder->bridge);

• encoder_funcs = encoder->helper_private;
  

  /* Disable the encoders as the first thing we do. */

• encoder_funcs->prepare(encoder);
  

• if (encoder_funcs->prepare)
  

• 	encoder_funcs->prepare(encoder);
  
  

  drm_bridge_post_disable(encoder->bridge); }

@@ -385,11 +399,15 @@ bool drm_crtc_helper_set_mode(struct drm_crtc *crtc, if (encoder->crtc != crtc) continue;

• encoder_funcs = encoder->helper_private;
  

• if (!encoder_funcs)
  

• 	continue;
  

• DRM_DEBUG_KMS("[ENCODER:%d:%s] set [MODE:%d:%s]\n", encoder->base.id, encoder->name, mode->base.id, mode->name);

• encoder_funcs = encoder->helper_private;
  

• encoder_funcs->mode_set(encoder, mode, adjusted_mode);
  

• if (encoder_funcs->mode_set)
  

• 	encoder_funcs->mode_set(encoder, mode, adjusted_mode);
  
  

  drm_bridge_mode_set(encoder->bridge, mode, adjusted_mode); }

@@ -402,10 +420,14 @@ bool drm_crtc_helper_set_mode(struct drm_crtc *crtc, if (encoder->crtc != crtc) continue;

• encoder_funcs = encoder->helper_private;
  

• if (!encoder_funcs)
  

• 	continue;
  

• drm_bridge_pre_enable(encoder->bridge);

• encoder_funcs = encoder->helper_private;
  

• encoder_funcs->commit(encoder);
  

• if (encoder_funcs->commit)
  

• 	encoder_funcs->commit(encoder);
  
  

  drm_bridge_enable(encoder->bridge); }

@@ -771,12 +793,15 @@ static void drm_helper_encoder_dpms(struct drm_encoder *encoder, int mode) struct drm_bridge *bridge = encoder->bridge; const struct drm_encoder_helper_funcs *encoder_funcs;

• encoder_funcs = encoder->helper_private;
• if (!encoder_funcs)

• return;
  

• if (mode == DRM_MODE_DPMS_ON) drm_bridge_pre_enable(bridge); else drm_bridge_disable(bridge);

• encoder_funcs = encoder->helper_private; if (encoder_funcs->dpms) encoder_funcs->dpms(encoder, mode);

-- 2.2.2

On Mon, May 09, 2016 at 09:19:22PM +0200, Noralf Trønnes wrote:

Den 05.05.2016 18:23, skrev Daniel Vetter:

...

On Thu, May 05, 2016 at 03:24:32PM +0200, Noralf Trønnes wrote:

...

Make drm_encoder_helper_funcs and it's functions optional to avoid having dummy functions.

Signed-off-by: Noralf Trønnes noralf@tronnes.org

Please also update the kerneldoc and mention there that the enable/disable hooks are optional. You can build the hmtl docs using

AFAICT the kerneldoc already says that they are optional for atomic helpers:

struct drm_encoder_helper_funcs { [...] /** * @disable: [...] * This hook is used both by legacy CRTC helpers and atomic helpers. * Atomic drivers don't need to implement it if there's no need to * disable anything at the encoder level. To ensure that runtime PM [...] /** * @enable: [...] * This hook is used only by atomic helpers, for symmetry with @disable. * Atomic drivers don't need to implement it if there's no need to * enable anything at the encoder level. To ensure that runtime PM handling

Indeed, docs not matching the code. But with your patch here will soon ;-) I'll apply this one to drm-misc. -Daniel

Noralf.

...

$ make htmldocs

to check the result. The vtables are documented in include/drm/drm_helper_vtables.h

...

---

drivers/gpu/drm/drm_atomic_helper.c | 11 ++++++++-- drivers/gpu/drm/drm_crtc_helper.c | 41 +++++++++++++++++++++++++++++--------

Hm, tbh I wouldn't bother at all with crtc helpers and just drop that part. Old drivers should converted to atomic, new drivers should just use atomic. No need to touch that code at all. -Daniel

...

2 files changed, 42 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 92e11a2..03ea049 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -696,6 +696,8 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state) continue; funcs = encoder->helper_private;

• if (!funcs)
  

• 	continue;
  

  DRM_DEBUG_ATOMIC("disabling [ENCODER:%d:%s]\n", encoder->base.id, encoder->name);

@@ -711,7 +713,7 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state) funcs->prepare(encoder); else if (funcs->disable) funcs->disable(encoder);

• else
  

• else if (funcs->dpms)
  funcs->dpms(encoder, DRM_MODE_DPMS_OFF);
  

  drm_bridge_post_disable(encoder->bridge);

@@ -859,6 +861,9 @@ crtc_set_mode(struct drm_device *dev, struct drm_atomic_state *old_state) encoder = connector->state->best_encoder; funcs = encoder->helper_private;

• if (!funcs)
  

• 	continue;
  

• new_crtc_state = connector->state->crtc->state; mode = &new_crtc_state->mode; adjusted_mode = &new_crtc_state->adjusted_mode;

@@ -964,6 +969,8 @@ void drm_atomic_helper_commit_modeset_enables(struct drm_device *dev, encoder = connector->state->best_encoder; funcs = encoder->helper_private;

• if (!funcs)
  

• 	continue;
  

  DRM_DEBUG_ATOMIC("enabling [ENCODER:%d:%s]\n", encoder->base.id, encoder->name);

@@ -976,7 +983,7 @@ void drm_atomic_helper_commit_modeset_enables(struct drm_device *dev, if (funcs->enable) funcs->enable(encoder);

• else
  

• else if (funcs->commit)
  funcs->commit(encoder);
  

  drm_bridge_enable(encoder->bridge);

diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c index 66ca313..6e6ab38 100644 --- a/drivers/gpu/drm/drm_crtc_helper.c +++ b/drivers/gpu/drm/drm_crtc_helper.c @@ -170,11 +170,14 @@ drm_encoder_disable(struct drm_encoder *encoder) { const struct drm_encoder_helper_funcs *encoder_funcs = encoder->helper_private;

• if (!encoder_funcs)

• return;
  

• drm_bridge_disable(encoder->bridge); if (encoder_funcs->disable) (*encoder_funcs->disable)(encoder);

• else

• else if (encoder_funcs->dpms) (*encoder_funcs->dpms)(encoder, DRM_MODE_DPMS_OFF); drm_bridge_post_disable(encoder->bridge);

@@ -248,6 +251,9 @@ drm_crtc_prepare_encoders(struct drm_device *dev) drm_for_each_encoder(encoder, dev) { encoder_funcs = encoder->helper_private;

• if (!encoder_funcs)
  

• 	continue;
  

• /* Disable unused encoders */ if (encoder->crtc == NULL) drm_encoder_disable(encoder);

@@ -326,6 +332,10 @@ bool drm_crtc_helper_set_mode(struct drm_crtc *crtc, if (encoder->crtc != crtc) continue;

• encoder_funcs = encoder->helper_private;
  

• if (!encoder_funcs)
  

• 	continue;
  

• ret = drm_bridge_mode_fixup(encoder->bridge, mode, adjusted_mode); if (!ret) {

@@ -360,11 +370,15 @@ bool drm_crtc_helper_set_mode(struct drm_crtc *crtc, if (encoder->crtc != crtc) continue;

• encoder_funcs = encoder->helper_private;
  

• if (!encoder_funcs)
  

• 	continue;
  

• drm_bridge_disable(encoder->bridge);

• encoder_funcs = encoder->helper_private;
  

  /* Disable the encoders as the first thing we do. */

• encoder_funcs->prepare(encoder);
  

• if (encoder_funcs->prepare)
  

• 	encoder_funcs->prepare(encoder);
  

  drm_bridge_post_disable(encoder->bridge); }

@@ -385,11 +399,15 @@ bool drm_crtc_helper_set_mode(struct drm_crtc *crtc, if (encoder->crtc != crtc) continue;

• encoder_funcs = encoder->helper_private;
  

• if (!encoder_funcs)
  

• 	continue;
  

• DRM_DEBUG_KMS("[ENCODER:%d:%s] set [MODE:%d:%s]\n", encoder->base.id, encoder->name, mode->base.id, mode->name);

• encoder_funcs = encoder->helper_private;
  

• encoder_funcs->mode_set(encoder, mode, adjusted_mode);
  

• if (encoder_funcs->mode_set)
  

• 	encoder_funcs->mode_set(encoder, mode, adjusted_mode);
  

  drm_bridge_mode_set(encoder->bridge, mode, adjusted_mode); }

@@ -402,10 +420,14 @@ bool drm_crtc_helper_set_mode(struct drm_crtc *crtc, if (encoder->crtc != crtc) continue;

• encoder_funcs = encoder->helper_private;
  

• if (!encoder_funcs)
  

• 	continue;
  

• drm_bridge_pre_enable(encoder->bridge);

• encoder_funcs = encoder->helper_private;
  

• encoder_funcs->commit(encoder);
  

• if (encoder_funcs->commit)
  

• 	encoder_funcs->commit(encoder);
  

  drm_bridge_enable(encoder->bridge); }

@@ -771,12 +793,15 @@ static void drm_helper_encoder_dpms(struct drm_encoder *encoder, int mode) struct drm_bridge *bridge = encoder->bridge; const struct drm_encoder_helper_funcs *encoder_funcs;

• encoder_funcs = encoder->helper_private;
• if (!encoder_funcs)

• return;
  

• if (mode == DRM_MODE_DPMS_ON) drm_bridge_pre_enable(bridge); else drm_bridge_disable(bridge);

• encoder_funcs = encoder->helper_private; if (encoder_funcs->dpms) encoder_funcs->dpms(encoder, mode);

-- 2.2.2

On Thu, May 05, 2016 at 03:24:33PM +0200, Noralf Trønnes wrote:

Provides helper functions for drivers that have a simple display pipeline. Plane, crtc and encoder are collapsed into one entity.

Signed-off-by: Noralf Trønnes noralf@tronnes.org

When resubmitting a patch please always have a per-patch changelog of what you've done. Otherwise you force reviewers to remember they looked at this already, dig out the old mail and compare ;-)

---

drivers/gpu/drm/Kconfig | 7 ++ drivers/gpu/drm/Makefile | 1 + drivers/gpu/drm/drm_simple_kms_helper.c | 157 ++++++++++++++++++++++++++++++++ include/drm/drm_simple_kms_helper.h | 88 ++++++++++++++++++

Please also add a new section (next to all the ones for the other kms helpers) in Documentation/DocBook/gpu.tmpl and pull your kerneldoc in. Please also add a short overview kernel doc section to the .c file (Using the DOC: header) and also pull that into the overall gpu docs.

You can check the results using

$ make htmldocs

4 files changed, 253 insertions(+) create mode 100644 drivers/gpu/drm/drm_simple_kms_helper.c create mode 100644 include/drm/drm_simple_kms_helper.h

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index 16e4c21..ff9f480 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -39,6 +39,13 @@ config DRM_KMS_HELPER help CRTC helpers for KMS drivers.

+config DRM_SIMPLE_KMS_HELPER

• tristate
• depends on DRM
• select DRM_KMS_HELPER
• help

•  Helpers for very simple KMS drivers.
  

config DRM_KMS_FB_HELPER bool depends on DRM_KMS_HELPER diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index 43c2abf..7e99923 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -31,6 +31,7 @@ drm_kms_helper-$(CONFIG_DRM_KMS_CMA_HELPER) += drm_fb_cma_helper.o drm_kms_helper-$(CONFIG_DRM_DP_AUX_CHARDEV) += drm_dp_aux_dev.o

obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o +obj-$(CONFIG_DRM_SIMPLE_KMS_HELPER) += drm_simple_kms_helper.o

CFLAGS_drm_trace_points.o := -I$(src)

diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c new file mode 100644 index 0000000..c8725bb --- /dev/null +++ b/drivers/gpu/drm/drm_simple_kms_helper.c @@ -0,0 +1,157 @@ +/*

• • Copyright (C) 2016 Noralf Trønnes
• • This program is free software; you can redistribute it and/or modify
• • it under the terms of the GNU General Public License as published by
• • the Free Software Foundation; either version 2 of the License, or
• • (at your option) any later version.
• */

+#include <drm/drmP.h> +#include <drm/drm_atomic.h> +#include <drm/drm_atomic_helper.h> +#include <drm/drm_crtc_helper.h> +#include <drm/drm_plane_helper.h> +#include <drm/drm_simple_kms_helper.h> +#include <linux/slab.h>

+static const struct drm_encoder_funcs drm_simple_kms_encoder_funcs = {

• .destroy = drm_encoder_cleanup,

+};

+static void drm_simple_kms_crtc_enable(struct drm_crtc *crtc) +{

• struct drm_simple_display_pipe *pipe;
• pipe = container_of(crtc, struct drm_simple_display_pipe, crtc);
• if (!pipe->funcs || !pipe->funcs->enable)

• return;
  

• pipe->funcs->enable(pipe, crtc->state);

+}

+static void drm_simple_kms_crtc_disable(struct drm_crtc *crtc) +{

• struct drm_simple_display_pipe *pipe;
• pipe = container_of(crtc, struct drm_simple_display_pipe, crtc);
• if (!pipe->funcs || !pipe->funcs->disable)

• return;
  

• pipe->funcs->disable(pipe);

+}

+static const struct drm_crtc_helper_funcs drm_simple_kms_crtc_helper_funcs = {

• .disable = drm_simple_kms_crtc_disable,
• .enable = drm_simple_kms_crtc_enable,

+};

+static const struct drm_crtc_funcs drm_simple_kms_crtc_funcs = {

• .reset = drm_atomic_helper_crtc_reset,
• .destroy = drm_crtc_cleanup,
• .set_config = drm_atomic_helper_set_config,
• .page_flip = drm_atomic_helper_page_flip,
• .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
• .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,

+};

+static int drm_simple_kms_plane_atomic_check(struct drm_plane *plane,

• 			struct drm_plane_state *pstate)
  

+{

• struct drm_simple_display_pipe *pipe;
• struct drm_crtc_state *cstate;
• pipe = container_of(plane, struct drm_simple_display_pipe, plane);
• if (!pipe->funcs || !pipe->funcs->check)

• return 0;
  

• cstate = drm_atomic_get_existing_crtc_state(pstate->state,

• 				    &pipe->crtc);
  

• return pipe->funcs->check(pipe, pstate, cstate);

+}

+static void drm_simple_kms_plane_atomic_update(struct drm_plane *plane,

• 			struct drm_plane_state *pstate)
  

+{

• struct drm_simple_display_pipe *pipe;
• pipe = container_of(plane, struct drm_simple_display_pipe, plane);
• if (!pipe->funcs || !pipe->funcs->update)

• return;
  

• pipe->funcs->update(pipe, pstate);

+}

+static const struct drm_plane_helper_funcs drm_simple_kms_plane_helper_funcs = {

• .atomic_check = drm_simple_kms_plane_atomic_check,
• .atomic_update = drm_simple_kms_plane_atomic_update,

+};

+static const struct drm_plane_funcs drm_simple_kms_plane_funcs = {

• .update_plane = drm_atomic_helper_update_plane,
• .disable_plane = drm_atomic_helper_disable_plane,
• .destroy = drm_plane_cleanup,
• .reset = drm_atomic_helper_plane_reset,
• .atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
• .atomic_destroy_state = drm_atomic_helper_plane_destroy_state,

+};

+/**

• • drm_simple_display_pipe_init - Initialize a simple display pipeline
• • @dev: DRM device
• • @pipe: simple display pipe object to initialize
• • @funcs: callbacks for the display pipe
• • @formats: array of supported formats (%DRM_FORMAT_*)
• • @format_count: number of elements in @formats
• • @connector: connector to attach and register
• • Sets up a display pipeline which consist of a really simple
• • plane-crtc-encoder pipe coupled with the provided connector.

need to specify that @funcs is optional.

• • Returns:
• • Zero on success, error code on failure.

"_negative_ error code"

• */

+int drm_simple_display_pipe_init(struct drm_device *dev,

• 		 struct drm_simple_display_pipe *pipe,
  

• 		 struct drm_simple_display_pipe_funcs *funcs,
  

• 		 const uint32_t *formats,
  

• 		 unsigned int format_count,
  

• 		 struct drm_connector *connector)
  

+{

• struct drm_encoder *encoder = &pipe->encoder;
• struct drm_plane *plane = &pipe->plane;
• struct drm_crtc *crtc = &pipe->crtc;
• int ret;
• pipe->funcs = funcs;
• drm_plane_helper_add(plane, &drm_simple_kms_plane_helper_funcs);
• ret = drm_universal_plane_init(dev, plane, 0,

• 		       &drm_simple_kms_plane_funcs,
  

• 		       formats, format_count,
  

• 		       DRM_PLANE_TYPE_PRIMARY, NULL);
  

• if (ret)

• return ret;
  

• drm_crtc_helper_add(crtc, &drm_simple_kms_crtc_helper_funcs);
• ret = drm_crtc_init_with_planes(dev, crtc, plane, NULL,

• 			&drm_simple_kms_crtc_funcs, NULL);
  

• if (ret)

• return ret;
  

• encoder->possible_crtcs = 1 << drm_crtc_index(crtc);
• ret = drm_encoder_init(dev, encoder, &drm_simple_kms_encoder_funcs,

• 	       DRM_MODE_ENCODER_NONE, NULL);
  

• if (ret)

• return ret;
  

• ret = drm_mode_connector_attach_encoder(connector, encoder);
• if (ret)

• return ret;
  

• return drm_connector_register(connector);

+} +EXPORT_SYMBOL(drm_simple_display_pipe_init);

+MODULE_LICENSE("GPL"); diff --git a/include/drm/drm_simple_kms_helper.h b/include/drm/drm_simple_kms_helper.h new file mode 100644 index 0000000..69eac94 --- /dev/null +++ b/include/drm/drm_simple_kms_helper.h @@ -0,0 +1,88 @@ +/*

• • Copyright (C) 2016 Noralf Trønnes
• • This program is free software; you can redistribute it and/or modify
• • it under the terms of the GNU General Public License as published by
• • the Free Software Foundation; either version 2 of the License, or
• • (at your option) any later version.
• */

+#ifndef __LINUX_DRM_SIMPLE_KMS_HELPER_H +#define __LINUX_DRM_SIMPLE_KMS_HELPER_H

+struct drm_simple_display_pipe;

+/**

• • struct drm_simple_display_pipe_funcs - helper operations for a simple

• •                                    display pipeline
    

• */

+struct drm_simple_display_pipe_funcs {

• /**

• * @enable:
  

• *
  

• * This function should be used to enable the pipeline.
  

• * It is called when the underlying crtc is enabled.
  

Please add boilerplate to all these hooks along the lines of:

"This hook is optional."

Besides that little bit of polish looks all good to me. -Daniel

• */
  

• void (*enable)(struct drm_simple_display_pipe *pipe,

•        struct drm_crtc_state *crtc_state);
  

• /**

• * @disable:
  

• *
  

• * This function should be used to disable the pipeline.
  

• * It is called when the underlying crtc is disabled.
  

• */
  

• void (*disable)(struct drm_simple_display_pipe *pipe);
• /**

• * @check:
  

• *
  

• * This function is called in the check phase of an atomic update,
  

• * specifically when the underlying plane is checked.
  

• *
  

• * RETURNS:
  

• *
  

• * 0 on success, -EINVAL if the state or the transition can't be
  

• * supported, -ENOMEM on memory allocation failure and -EDEADLK if an
  

• * attempt to obtain another state object ran into a &drm_modeset_lock
  

• * deadlock.
  

• */
  

• int (*check)(struct drm_simple_display_pipe *pipe,

•      struct drm_plane_state *plane_state,
  

•      struct drm_crtc_state *crtc_state);
  

• /**

• * @update:
  

• *
  

• * This function is called when the underlying plane state is updated.
  

• */
  

• void (*update)(struct drm_simple_display_pipe *pipe,

•        struct drm_plane_state *plane_state);
  

+};

+/**

• • struct drm_simple_display_pipe - simple display pipeline
• • @crtc: CRTC control structure
• • @plane: Plane control structure
• • @encoder: Encoder control structure
• • @connector: Connector control structure
• • @funcs: Pipeline control functions (optional)
• • Simple display pipeline with plane, crtc and encoder collapsed into one
• • entity.
• */

+struct drm_simple_display_pipe {

• struct drm_crtc crtc;
• struct drm_plane plane;
• struct drm_encoder encoder;
• struct drm_connector *connector;
• struct drm_simple_display_pipe_funcs *funcs;

+};

+int drm_simple_display_pipe_init(struct drm_device *dev,

• 		 struct drm_simple_display_pipe *pipe,
  

• 		 struct drm_simple_display_pipe_funcs *funcs,
  

• 		 const uint32_t *formats,
  

• 		 unsigned int format_count,
  

• 		 struct drm_connector *connector);
  

+#endif /* __LINUX_DRM_SIMPLE_KMS_HELPER_H */

2.2.2

On Thu, May 05, 2016 at 03:24:34PM +0200, Noralf Trønnes wrote:

Add function to create a simple connector for a panel.

Signed-off-by: Noralf Trønnes noralf@tronnes.org

Like in the previous patch please also add a new section for the panel helpers to gpu.tmpl. I don't think this needs an overview section, it's so simple. But adding some cross references from the drm_panel.c kerneldoc to this and back would be real good.

---

drivers/gpu/drm/Makefile | 1 + drivers/gpu/drm/drm_panel_helper.c | 117 +++++++++++++++++++++++++++++++++++++ drivers/gpu/drm/panel/Kconfig | 7 +++ include/drm/drm_panel_helper.h | 20 +++++++ 4 files changed, 145 insertions(+) create mode 100644 drivers/gpu/drm/drm_panel_helper.c create mode 100644 include/drm/drm_panel_helper.h

diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index 7e99923..3f3696c 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -18,6 +18,7 @@ drm-$(CONFIG_COMPAT) += drm_ioc32.o drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o drm-$(CONFIG_PCI) += ati_pcigart.o drm-$(CONFIG_DRM_PANEL) += drm_panel.o +drm-$(CONFIG_DRM_PANEL_HELPER) += drm_panel_helper.o drm-$(CONFIG_OF) += drm_of.o drm-$(CONFIG_AGP) += drm_agpsupport.o

diff --git a/drivers/gpu/drm/drm_panel_helper.c b/drivers/gpu/drm/drm_panel_helper.c new file mode 100644 index 0000000..5d8b623 --- /dev/null +++ b/drivers/gpu/drm/drm_panel_helper.c @@ -0,0 +1,117 @@ +/*

• • Copyright (C) 2016 Noralf Trønnes
• • This program is free software; you can redistribute it and/or modify
• • it under the terms of the GNU General Public License as published by
• • the Free Software Foundation; either version 2 of the License, or
• • (at your option) any later version.
• */

+#include <linux/module.h> +#include <linux/slab.h>

+#include <drm/drmP.h> +#include <drm/drm_atomic_helper.h> +#include <drm/drm_crtc_helper.h> +#include <drm/drm_panel.h>

+struct drm_panel_connector {

• struct drm_connector base;
• struct drm_panel *panel;

+};

+static inline struct drm_panel_connector * +to_drm_panel_connector(struct drm_connector *connector) +{

• return container_of(connector, struct drm_panel_connector, base);

+}

+static int drm_panel_connector_get_modes(struct drm_connector *connector) +{

• return drm_panel_get_modes(to_drm_panel_connector(connector)->panel);

+}

+static struct drm_encoder * +drm_panel_connector_best_encoder(struct drm_connector *connector) +{

• return drm_encoder_find(connector->dev, connector->encoder_ids[0]);

+}

As mentioned in my previous review, this function would be extremely useful to rid tons of drivers of boilerplate - most drm_connector only support exactly 1 encoder, statically determined at driver init time.

Please put this helper into the atomic helper library, and maybe call it something like

drm_atomic_helper_best_encoder.

To make sure it's never abused by accident please also add a

WARN_ON(connector->encoder_ids[1]);

to make sure that there's really only just one encoder for this connector.

If you're bored you can also go through drivers and use that everywhere it fits, it would result in a _lot_ of deleted code. But also needs quite a bit of audit work ...

Otherwise lgtm, but please ask Thierry for an ack as the panel maintainer. -Daniel

+static const struct drm_connector_helper_funcs drm_panel_connector_hfuncs = {

• .get_modes = drm_panel_connector_get_modes,
• .best_encoder = drm_panel_connector_best_encoder,

+};

+static enum drm_connector_status +drm_panel_connector_detect(struct drm_connector *connector, bool force) +{

• if (drm_device_is_unplugged(connector->dev))

• return connector_status_disconnected;
  

• return connector->status;

+}

+static void drm_panel_connector_destroy(struct drm_connector *connector) +{

• struct drm_panel_connector *panel_connector;
• panel_connector = to_drm_panel_connector(connector);
• drm_panel_detach(panel_connector->panel);
• drm_panel_remove(panel_connector->panel);
• drm_connector_unregister(connector);
• drm_connector_cleanup(connector);
• kfree(panel_connector);

+}

+static const struct drm_connector_funcs drm_panel_connector_funcs = {

• .dpms = drm_atomic_helper_connector_dpms,
• .reset = drm_atomic_helper_connector_reset,
• .detect = drm_panel_connector_detect,
• .fill_modes = drm_helper_probe_single_connector_modes,
• .destroy = drm_panel_connector_destroy,
• .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
• .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,

+};

+/**

• • drm_panel_connector_create - Create simple connector for panel
• • @dev: DRM device
• • @panel: DRM panel
• • @connector_type: user visible type of the connector
• • Creates a simple connector for a panel.
• • The panel needs to provide a get_modes() function.
• • Returns:
• • Pointer to new connector or ERR_PTR on failure.
• */

+struct drm_connector *drm_panel_connector_create(struct drm_device *dev,

• 				 struct drm_panel *panel,
  

• 				 int connector_type)
  

+{

• struct drm_panel_connector *panel_connector;
• struct drm_connector *connector;
• int ret;
• panel_connector = kzalloc(sizeof(*panel_connector), GFP_KERNEL);
• if (!panel_connector)

• return ERR_PTR(-ENOMEM);
  

• panel_connector->panel = panel;
• connector = &panel_connector->base;
• drm_connector_helper_add(connector, &drm_panel_connector_hfuncs);
• ret = drm_connector_init(dev, connector, &drm_panel_connector_funcs,

• 		 connector_type);
  

• if (ret) {

• kfree(panel_connector);
  

• return ERR_PTR(ret);
  

• }
• connector->status = connector_status_connected;
• drm_panel_init(panel);
• drm_panel_add(panel);
• drm_panel_attach(panel, connector);
• return connector;

+} +EXPORT_SYMBOL(drm_panel_connector_create); diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig index 1500ab9..87264a3 100644 --- a/drivers/gpu/drm/panel/Kconfig +++ b/drivers/gpu/drm/panel/Kconfig @@ -4,6 +4,13 @@ config DRM_PANEL help Panel registration and lookup framework.

+config DRM_PANEL_HELPER

• bool
• depends on DRM
• select DRM_PANEL
• help

•  Panel helper.
  

menu "Display Panels" depends on DRM && DRM_PANEL

diff --git a/include/drm/drm_panel_helper.h b/include/drm/drm_panel_helper.h new file mode 100644 index 0000000..c3355e3 --- /dev/null +++ b/include/drm/drm_panel_helper.h @@ -0,0 +1,20 @@ +/*

• • Copyright (C) 2016 Noralf Trønnes
• • This program is free software; you can redistribute it and/or modify
• • it under the terms of the GNU General Public License as published by
• • the Free Software Foundation; either version 2 of the License, or
• • (at your option) any later version.
• */

+#ifndef __DRM_PANEL_HELPER_H__ +#define __DRM_PANEL_HELPER_H__

+struct drm_device; +struct drm_panel;

+struct drm_connector *drm_panel_connector_create(struct drm_device *dev,

• 				 struct drm_panel *panel,
  

• 				 int connector_type);
  

+#endif

2.2.2

On Fri, May 06, 2016 at 03:39:53PM +0200, Noralf Trønnes wrote:

Den 05.05.2016 19:03, skrev Daniel Vetter:

...

On Thu, May 05, 2016 at 03:24:34PM +0200, Noralf Trønnes wrote:

...

Add function to create a simple connector for a panel.

Signed-off-by: Noralf Trønnes noralf@tronnes.org

Like in the previous patch please also add a new section for the panel helpers to gpu.tmpl. I don't think this needs an overview section, it's so simple. But adding some cross references from the drm_panel.c kerneldoc to this and back would be real good.

drm_panel.c doesn't have any documentation and the header file has only the drm_panel_funcs struct documented, not hooked up to gpu.tmpl.

I can make a patch documenting the functions, it looks fairly straight forward, but I have no idea what to put in the DOC: section, except an xref to this helper :-)

Maybe now is a good time for me to post the below. I really should've sent this out ages ago, sorry.

Thierry --- >8 --- From 77057510413f8ca52d37da883afeabb13031ec63 Mon Sep 17 00:00:00 2001 From: Thierry Reding treding@nvidia.com Date: Tue, 4 Nov 2014 15:23:10 +0100 Subject: [PATCH] drm/panel: Flesh out kerneldoc

Write more complete kerneldoc comments for the DRM panel API and integrate the helpers in the DRM DocBook reference.

Signed-off-by: Thierry Reding treding@nvidia.com --- Documentation/DocBook/gpu.tmpl | 12 ++++++--- drivers/gpu/drm/drm_panel.c | 61 ++++++++++++++++++++++++++++++++++++++++++ include/drm/drm_panel.h | 59 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 129 insertions(+), 3 deletions(-)

diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl index 1464fb2f3c46..fb4ad6945a97 100644 --- a/Documentation/DocBook/gpu.tmpl +++ b/Documentation/DocBook/gpu.tmpl @@ -1671,17 +1671,23 @@ void intel_crt_init(struct drm_device *dev) !Pdrivers/gpu/drm/drm_crtc.c Tile group </sect2> <sect2> - <title>Bridges</title> + <title>Bridges</title> <sect3> - <title>Overview</title> + <title>Overview</title> !Pdrivers/gpu/drm/drm_bridge.c overview </sect3> <sect3> - <title>Default bridge callback sequence</title> + <title>Default bridge callback sequence</title> !Pdrivers/gpu/drm/drm_bridge.c bridge callbacks </sect3> !Edrivers/gpu/drm/drm_bridge.c </sect2> + <sect2> + <title>Panel Helper Reference</title> +!Iinclude/drm/drm_panel.h +!Edrivers/gpu/drm/drm_panel.c +!Pdrivers/gpu/drm/drm_panel.c drm panel + </sect2> </sect1>

<!-- Internals: kms properties --> diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c index 2ef988e037b7..3dfe3c886502 100644 --- a/drivers/gpu/drm/drm_panel.c +++ b/drivers/gpu/drm/drm_panel.c @@ -30,12 +30,36 @@ static DEFINE_MUTEX(panel_lock); static LIST_HEAD(panel_list);

+/** + * DOC: drm panel + * + * The DRM panel helpers allow drivers to register panel objects with a + * central registry and provide functions to retrieve those panels in display + * drivers. + */ + +/** + * drm_panel_init - initialize a panel + * @panel: DRM panel + * + * Sets up internal fields of the panel so that it can subsequently be added + * to the registry. + */ void drm_panel_init(struct drm_panel *panel) { INIT_LIST_HEAD(&panel->list); } EXPORT_SYMBOL(drm_panel_init);

+/** + * drm_panel_add - add a panel to the global registry + * @panel: panel to add + * + * Add a panel to the global registry so that it can be looked up by display + * drivers. + * + * Return: 0 on success or a negative error code on failure. + */ int drm_panel_add(struct drm_panel *panel) { mutex_lock(&panel_lock); @@ -46,6 +70,12 @@ int drm_panel_add(struct drm_panel *panel) } EXPORT_SYMBOL(drm_panel_add);

+/** + * drm_panel_remove - remove a panel from the global registry + * @panel: DRM panel + * + * Removes a panel from the global registry. + */ void drm_panel_remove(struct drm_panel *panel) { mutex_lock(&panel_lock); @@ -54,6 +84,18 @@ void drm_panel_remove(struct drm_panel *panel) } EXPORT_SYMBOL(drm_panel_remove);

+/** + * drm_panel_attach - attach a panel to a connector + * @panel: DRM panel + * @connector: DRM connector + * + * After obtaining a pointer to a DRM panel a display driver calls this + * function to attach a panel to a connector. + * + * An error is returned if the panel is already attached to another connector. + * + * Return: 0 on success or a negative error code on failure. + */ int drm_panel_attach(struct drm_panel *panel, struct drm_connector *connector) { if (panel->connector) @@ -66,6 +108,15 @@ int drm_panel_attach(struct drm_panel *panel, struct drm_connector *connector) } EXPORT_SYMBOL(drm_panel_attach);

+/** + * drm_panel_detach - detach a panel from a connector + * @panel: DRM panel + * + * Detaches a panel from the connector it is attached to. If a panel is not + * attached to any connector this is effectively a no-op. + * + * Return: 0 on success or a negative error code on failure. + */ int drm_panel_detach(struct drm_panel *panel) { panel->connector = NULL; @@ -76,6 +127,16 @@ int drm_panel_detach(struct drm_panel *panel) EXPORT_SYMBOL(drm_panel_detach);

#ifdef CONFIG_OF +/** + * of_drm_find_panel - look up a panel using a device tree node + * @np: device tree node of the panel + * + * Searches the set of registered panels for one that matches the given device + * tree node. If a matching panel is found, return a pointer to it. + * + * Return: A pointer to the panel registered for the specified device tree + * node or NULL if no panel matching the device tree node can be found. + */ struct drm_panel *of_drm_find_panel(struct device_node *np) { struct drm_panel *panel; diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h index 13ff44b28893..220d1e2b3db1 100644 --- a/include/drm/drm_panel.h +++ b/include/drm/drm_panel.h @@ -75,6 +75,14 @@ struct drm_panel_funcs { struct display_timing *timings); };

+/** + * struct drm_panel - DRM panel object + * @drm: DRM device owning the panel + * @connector: DRM connector that the panel is attached to + * @dev: parent device of the panel + * @funcs: operations that can be performed on the panel + * @list: panel entry in registry + */ struct drm_panel { struct drm_device *drm; struct drm_connector *connector; @@ -85,6 +93,17 @@ struct drm_panel { struct list_head list; };

+/** + * drm_disable_unprepare - power off a panel + * @panel: DRM panel + * + * Calling this function will completely power off a panel (assert the panel's + * reset, turn off power supplies, ...). After this function has completed, it + * is usually no longer possible to communicate with the panel until another + * call to drm_panel_prepare(). + * + * Return: 0 on success or a negative error code on failure. + */ static inline int drm_panel_unprepare(struct drm_panel *panel) { if (panel && panel->funcs && panel->funcs->unprepare) @@ -93,6 +112,16 @@ static inline int drm_panel_unprepare(struct drm_panel *panel) return panel ? -ENOSYS : -EINVAL; }

+/** + * drm_panel_disable - disable a panel + * @panel: DRM panel + * + * This will typically turn off the panel's backlight or disable the display + * drivers. For smart panels it should still be possible to communicate with + * the integrated circuitry via any command bus after this call. + * + * Return: 0 on success or a negative error code on failure. + */ static inline int drm_panel_disable(struct drm_panel *panel) { if (panel && panel->funcs && panel->funcs->disable) @@ -101,6 +130,16 @@ static inline int drm_panel_disable(struct drm_panel *panel) return panel ? -ENOSYS : -EINVAL; }

+/** + * drm_panel_prepare - power on a panel + * @panel: DRM panel + * + * Calling this function will enable power and deassert any reset signals to + * the panel. After this has completed it is possible to communicate with any + * integrated circuitry via a command bus. + * + * Return: 0 on success or a negative error code on failure. + */ static inline int drm_panel_prepare(struct drm_panel *panel) { if (panel && panel->funcs && panel->funcs->prepare) @@ -109,6 +148,16 @@ static inline int drm_panel_prepare(struct drm_panel *panel) return panel ? -ENOSYS : -EINVAL; }

+/** + * drm_panel_enable - enable a panel + * @panel: DRM panel + * + * Calling this function will cause the panel display drivers to be turned on + * and the backlight to be enabled. Content will be visible on screen after + * this call completes. + * + * Return: 0 on success or a negative error code on failure. + */ static inline int drm_panel_enable(struct drm_panel *panel) { if (panel && panel->funcs && panel->funcs->enable) @@ -117,6 +166,16 @@ static inline int drm_panel_enable(struct drm_panel *panel) return panel ? -ENOSYS : -EINVAL; }

+/** + * drm_panel_get_modes - probe the available display modes of a panel + * @panel: DRM panel + * + * The modes probed from the panel are automatically added to the connector + * that the panel is attached to. + * + * Return: The number of modes available from the panel on success or a + * negative error code on failure. + */ static inline int drm_panel_get_modes(struct drm_panel *panel) { if (panel && panel->funcs && panel->funcs->get_modes)