Re: [Intel-gfx] [PATCH 05/23] drm: Add local 'plane' variable for primary/cursor planes

[Daniel Vetter]

On Thu, Mar 22, 2018 at 05:22:55PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä 
> 
> Make the code a bit more readable by storing the plane pointer in a
> local variable rather than having to do crtc->{primary,cursor} all the
> time.
> 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/drm_crtc.c  | 32 +++-
>  drivers/gpu/drm/drm_plane.c | 32 ++--
>  2 files changed, 37 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 7a973ada7195..8552ed419056 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -402,6 +402,7 @@ int drm_mode_getcrtc(struct drm_device *dev,
>  {
>   struct drm_mode_crtc *crtc_resp = data;
>   struct drm_crtc *crtc;
> + struct drm_plane *plane;
>  
>   if (!drm_core_check_feature(dev, DRIVER_MODESET))
>   return -EINVAL;
> @@ -410,21 +411,23 @@ int drm_mode_getcrtc(struct drm_device *dev,
>   if (!crtc)
>   return -ENOENT;
>  
> + plane = crtc->primary;
> +
>   crtc_resp->gamma_size = crtc->gamma_size;
>  
> - drm_modeset_lock(>primary->mutex, NULL);
> - if (crtc->primary->state && crtc->primary->state->fb)
> - crtc_resp->fb_id = crtc->primary->state->fb->base.id;
> - else if (!crtc->primary->state && crtc->primary->fb)
> - crtc_resp->fb_id = crtc->primary->fb->base.id;
> + drm_modeset_lock(>mutex, NULL);
> + if (plane->state && plane->state->fb)
> + crtc_resp->fb_id = plane->state->fb->base.id;
> + else if (!plane->state && plane->fb)
> + crtc_resp->fb_id = plane->fb->base.id;
>   else
>   crtc_resp->fb_id = 0;
>  
> - if (crtc->primary->state) {
> - crtc_resp->x = crtc->primary->state->src_x >> 16;
> - crtc_resp->y = crtc->primary->state->src_y >> 16;
> + if (plane->state) {
> + crtc_resp->x = plane->state->src_x >> 16;
> + crtc_resp->y = plane->state->src_y >> 16;
>   }
> - drm_modeset_unlock(>primary->mutex);
> + drm_modeset_unlock(>mutex);
>  
>   drm_modeset_lock(>mutex, NULL);
>   if (crtc->state) {
> @@ -554,6 +557,7 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
>   struct drm_mode_config *config = >mode_config;
>   struct drm_mode_crtc *crtc_req = data;
>   struct drm_crtc *crtc;
> + struct drm_plane *plane;
>   struct drm_connector **connector_set = NULL, *connector;
>   struct drm_framebuffer *fb = NULL;
>   struct drm_display_mode *mode = NULL;
> @@ -580,6 +584,8 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,

Could do the same for setconfig_internal's tmp->primary, if you're bored.

>   }
>   DRM_DEBUG_KMS("[CRTC:%d:%s]\n", crtc->base.id, crtc->name);
>  
> + plane = crtc->primary;
> +
>   mutex_lock(>dev->mode_config.mutex);
>   drm_modeset_acquire_init(, DRM_MODESET_ACQUIRE_INTERRUPTIBLE);
>  retry:
> @@ -590,12 +596,12 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
>   /* If we have a mode we need a framebuffer. */
>   /* If we pass -1, set the mode with the currently bound fb */
>   if (crtc_req->fb_id == -1) {
> - if (!crtc->primary->fb) {
> + if (!plane->fb) {
>   DRM_DEBUG_KMS("CRTC doesn't have current FB\n");
>   ret = -EINVAL;
>   goto out;
>   }
> - fb = crtc->primary->fb;
> + fb = plane->fb;
>   /* Make refcounting symmetric with the lookup path. */
>   drm_framebuffer_get(fb);
>   } else {
> @@ -627,8 +633,8 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
>* match real hardware capabilities. Skip the check in that
>* case.
>*/
> - if (!crtc->primary->format_default) {
> - ret = drm_plane_check_pixel_format(crtc->primary,
> + if (!plane->format_default) {
> + ret = drm_plane_check_pixel_format(plane,
>  fb->format->format,
>  fb->modifier);
>   if (ret) {
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index 6d2a6e428a3e..38e2a628bfa2 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -756,6 +756,7 @@ static int drm_mode_cursor_universal(struct drm_crtc 
> *crtc,
>struct drm_modeset_acquire_ctx *ctx)
>  {
>   struct drm_device *dev = crtc->dev;
> + struct drm_plane *plane = crtc->cursor;
>   struct drm_framebuffer *fb = 

[Intel-gfx] [PATCH 05/23] drm: Add local 'plane' variable for primary/cursor planes

[Ville Syrjala]

From: Ville Syrjälä 

Make the code a bit more readable by storing the plane pointer in a
local variable rather than having to do crtc->{primary,cursor} all the
time.

Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/drm_crtc.c  | 32 +++-
 drivers/gpu/drm/drm_plane.c | 32 ++--
 2 files changed, 37 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 7a973ada7195..8552ed419056 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -402,6 +402,7 @@ int drm_mode_getcrtc(struct drm_device *dev,
 {
struct drm_mode_crtc *crtc_resp = data;
struct drm_crtc *crtc;
+   struct drm_plane *plane;
 
if (!drm_core_check_feature(dev, DRIVER_MODESET))
return -EINVAL;
@@ -410,21 +411,23 @@ int drm_mode_getcrtc(struct drm_device *dev,
if (!crtc)
return -ENOENT;
 
+   plane = crtc->primary;
+
crtc_resp->gamma_size = crtc->gamma_size;
 
-   drm_modeset_lock(>primary->mutex, NULL);
-   if (crtc->primary->state && crtc->primary->state->fb)
-   crtc_resp->fb_id = crtc->primary->state->fb->base.id;
-   else if (!crtc->primary->state && crtc->primary->fb)
-   crtc_resp->fb_id = crtc->primary->fb->base.id;
+   drm_modeset_lock(>mutex, NULL);
+   if (plane->state && plane->state->fb)
+   crtc_resp->fb_id = plane->state->fb->base.id;
+   else if (!plane->state && plane->fb)
+   crtc_resp->fb_id = plane->fb->base.id;
else
crtc_resp->fb_id = 0;
 
-   if (crtc->primary->state) {
-   crtc_resp->x = crtc->primary->state->src_x >> 16;
-   crtc_resp->y = crtc->primary->state->src_y >> 16;
+   if (plane->state) {
+   crtc_resp->x = plane->state->src_x >> 16;
+   crtc_resp->y = plane->state->src_y >> 16;
}
-   drm_modeset_unlock(>primary->mutex);
+   drm_modeset_unlock(>mutex);
 
drm_modeset_lock(>mutex, NULL);
if (crtc->state) {
@@ -554,6 +557,7 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
struct drm_mode_config *config = >mode_config;
struct drm_mode_crtc *crtc_req = data;
struct drm_crtc *crtc;
+   struct drm_plane *plane;
struct drm_connector **connector_set = NULL, *connector;
struct drm_framebuffer *fb = NULL;
struct drm_display_mode *mode = NULL;
@@ -580,6 +584,8 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
}
DRM_DEBUG_KMS("[CRTC:%d:%s]\n", crtc->base.id, crtc->name);
 
+   plane = crtc->primary;
+
mutex_lock(>dev->mode_config.mutex);
drm_modeset_acquire_init(, DRM_MODESET_ACQUIRE_INTERRUPTIBLE);
 retry:
@@ -590,12 +596,12 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
/* If we have a mode we need a framebuffer. */
/* If we pass -1, set the mode with the currently bound fb */
if (crtc_req->fb_id == -1) {
-   if (!crtc->primary->fb) {
+   if (!plane->fb) {
DRM_DEBUG_KMS("CRTC doesn't have current FB\n");
ret = -EINVAL;
goto out;
}
-   fb = crtc->primary->fb;
+   fb = plane->fb;
/* Make refcounting symmetric with the lookup path. */
drm_framebuffer_get(fb);
} else {
@@ -627,8 +633,8 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
 * match real hardware capabilities. Skip the check in that
 * case.
 */
-   if (!crtc->primary->format_default) {
-   ret = drm_plane_check_pixel_format(crtc->primary,
+   if (!plane->format_default) {
+   ret = drm_plane_check_pixel_format(plane,
   fb->format->format,
   fb->modifier);
if (ret) {
diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index 6d2a6e428a3e..38e2a628bfa2 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -756,6 +756,7 @@ static int drm_mode_cursor_universal(struct drm_crtc *crtc,
 struct drm_modeset_acquire_ctx *ctx)
 {
struct drm_device *dev = crtc->dev;
+   struct drm_plane *plane = crtc->cursor;
struct drm_framebuffer *fb = NULL;
struct drm_mode_fb_cmd2 fbreq = {
.width = req->width,
@@ -769,8 +770,8 @@ static int drm_mode_cursor_universal(struct drm_crtc *crtc,
uint32_t src_w = 0, src_h = 0;
int ret =