Re: [Intel-gfx] [PATCH 2/2] drm/i915: Add rotation readout for plane initial config

2018-11-21 Thread Ville Syrjälä 
On Tue, Nov 20, 2018 at 02:20:14PM -0800, Rodrigo Vivi wrote:
> On Tue, Nov 20, 2018 at 03:54:50PM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä 
> > 
> > If we need to force a full plane update before userspace/fbdev
> > have given us a proper plane state we should try to maintain the
> > current plane state as much as possible (apart from the parts
> > of the state we're trying to fix up with the plane update).
> > To that end add basic readout for the plane rotation and
> > maintain it during the initial fb takeover.
> > 
> > Cc: Hans de Goede 
> > Fixes: 516a49cc1946 ("drm/i915: Fix assert_plane() warning on bootup with 
> > external display")
> > Signed-off-by: Ville Syrjälä 
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 27 +++
> >  drivers/gpu/drm/i915/intel_drv.h |  1 +
> >  2 files changed, 28 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c 
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 60c1e54285c1..4d339f54559c 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -2831,6 +2831,7 @@ intel_find_initial_plane_obj(struct intel_crtc 
> > *intel_crtc,
> > return;
> >  
> >  valid_fb:
> > +   intel_state->base.rotation = plane_config->rotation;
> > intel_fill_fb_ggtt_view(_state->view, fb,
> > intel_state->base.rotation);
> > intel_state->color_plane[0].stride =
> > @@ -7787,8 +7788,15 @@ i9xx_get_initial_plane_config(struct intel_crtc 
> > *crtc,
> > plane_config->tiling = I915_TILING_X;
> > fb->modifier = I915_FORMAT_MOD_X_TILED;
> > }
> > +
> > +   if (val & DISPPLANE_ROTATE_180)
> > +   plane_config->rotation = DRM_MODE_ROTATE_180;
> > }
> >  
> > +   if (IS_CHERRYVIEW(dev_priv) && pipe == PIPE_B &&
> > +   val & DISPPLANE_MIRROR)
> > +   plane_config->rotation |= DRM_MODE_REFLECT_X;
> > +
> > pixel_format = val & DISPPLANE_PIXFORMAT_MASK;
> > fourcc = i9xx_format_to_fourcc(pixel_format);
> > fb->format = drm_format_info(fourcc);
> > @@ -8898,6 +8906,25 @@ skylake_get_initial_plane_config(struct intel_crtc 
> > *crtc,
> > goto error;
> > }
> >  
> > +   switch (val & PLANE_CTL_ROTATE_MASK) {
> > +   case PLANE_CTL_ROTATE_0:
> > +   plane_config->rotation = DRM_MODE_ROTATE_0;
> > +   break;
> > +   case PLANE_CTL_ROTATE_90:
> > +   plane_config->rotation = DRM_MODE_ROTATE_270;
> 
> we should add that comment that DRM_MODE_ROTATE_ is counter clockwise
> to avoid later confusion.

I copied over the comment from the other place, and pushed the series
to dinq. Thanks for the reviews, testing, and tracking down the bug
initially.

> 
> Also I wonder if we shouldn't move all the cases to a unique
> map function.
> 
> Anyways,
> 
> 
> Reviewed-by: Rodrigo Vivi 
> 
> 
> 
> > +   break;
> > +   case PLANE_CTL_ROTATE_180:
> > +   plane_config->rotation = DRM_MODE_ROTATE_180;
> > +   break;
> > +   case PLANE_CTL_ROTATE_270:
> > +   plane_config->rotation = DRM_MODE_ROTATE_90;
> > +   break;
> > +   }
> > +
> > +   if (INTEL_GEN(dev_priv) >= 10 &&
> > +   val & PLANE_CTL_FLIP_HORIZONTAL)
> > +   plane_config->rotation |= DRM_MODE_REFLECT_X;
> > +
> > base = I915_READ(PLANE_SURF(pipe, plane_id)) & 0xf000;
> > plane_config->base = base;
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h 
> > b/drivers/gpu/drm/i915/intel_drv.h
> > index f575ba2a59da..a7d9ac912125 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -572,6 +572,7 @@ struct intel_initial_plane_config {
> > unsigned int tiling;
> > int size;
> > u32 base;
> > +   u8 rotation;
> >  };
> >  
> >  #define SKL_MIN_SRC_W 8
> > -- 
> > 2.18.1
> > 
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/2] drm/i915: Add rotation readout for plane initial config

2018-11-21 Thread Maarten Lankhorst 
Op 20-11-18 om 23:20 schreef Rodrigo Vivi:
> On Tue, Nov 20, 2018 at 03:54:50PM +0200, Ville Syrjala wrote:
>> From: Ville Syrjälä 
>>
>> If we need to force a full plane update before userspace/fbdev
>> have given us a proper plane state we should try to maintain the
>> current plane state as much as possible (apart from the parts
>> of the state we're trying to fix up with the plane update).
>> To that end add basic readout for the plane rotation and
>> maintain it during the initial fb takeover.
>>
>> Cc: Hans de Goede 
>> Fixes: 516a49cc1946 ("drm/i915: Fix assert_plane() warning on bootup with 
>> external display")
>> Signed-off-by: Ville Syrjälä 
>> ---
>>  drivers/gpu/drm/i915/intel_display.c | 27 +++
>>  drivers/gpu/drm/i915/intel_drv.h |  1 +
>>  2 files changed, 28 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c 
>> b/drivers/gpu/drm/i915/intel_display.c
>> index 60c1e54285c1..4d339f54559c 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -2831,6 +2831,7 @@ intel_find_initial_plane_obj(struct intel_crtc 
>> *intel_crtc,
>>  return;
>>  
>>  valid_fb:
>> +intel_state->base.rotation = plane_config->rotation;
>>  intel_fill_fb_ggtt_view(_state->view, fb,
>>  intel_state->base.rotation);
>>  intel_state->color_plane[0].stride =
>> @@ -7787,8 +7788,15 @@ i9xx_get_initial_plane_config(struct intel_crtc *crtc,
>>  plane_config->tiling = I915_TILING_X;
>>  fb->modifier = I915_FORMAT_MOD_X_TILED;
>>  }
>> +
>> +if (val & DISPPLANE_ROTATE_180)
>> +plane_config->rotation = DRM_MODE_ROTATE_180;
>>  }
>>  
>> +if (IS_CHERRYVIEW(dev_priv) && pipe == PIPE_B &&
>> +val & DISPPLANE_MIRROR)
>> +plane_config->rotation |= DRM_MODE_REFLECT_X;
>> +
>>  pixel_format = val & DISPPLANE_PIXFORMAT_MASK;
>>  fourcc = i9xx_format_to_fourcc(pixel_format);
>>  fb->format = drm_format_info(fourcc);
>> @@ -8898,6 +8906,25 @@ skylake_get_initial_plane_config(struct intel_crtc 
>> *crtc,
>>  goto error;
>>  }
>>  
>> +switch (val & PLANE_CTL_ROTATE_MASK) {
>> +case PLANE_CTL_ROTATE_0:
>> +plane_config->rotation = DRM_MODE_ROTATE_0;
>> +break;
>> +case PLANE_CTL_ROTATE_90:
>> +plane_config->rotation = DRM_MODE_ROTATE_270;
> we should add that comment that DRM_MODE_ROTATE_ is counter clockwise
> to avoid later confusion.
>
> Also I wonder if we shouldn't move all the cases to a unique
> map function.
>
> Anyways,
>
>
> Reviewed-by: Rodrigo Vivi 
>
>
>
>> +break;
>> +case PLANE_CTL_ROTATE_180:
>> +plane_config->rotation = DRM_MODE_ROTATE_180;
>> +break;
>> +case PLANE_CTL_ROTATE_270:
>> +plane_config->rotation = DRM_MODE_ROTATE_90;
>> +break;
>> +}
>> +
>> +if (INTEL_GEN(dev_priv) >= 10 &&
>> +val & PLANE_CTL_FLIP_HORIZONTAL)
>> +plane_config->rotation |= DRM_MODE_REFLECT_X;
>> +
>>  base = I915_READ(PLANE_SURF(pipe, plane_id)) & 0xf000;
>>  plane_config->base = base;
>>  
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h 
>> b/drivers/gpu/drm/i915/intel_drv.h
>> index f575ba2a59da..a7d9ac912125 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -572,6 +572,7 @@ struct intel_initial_plane_config {
>>  unsigned int tiling;
>>  int size;
>>  u32 base;
>> +u8 rotation;
>>  };
>>  
>>  #define SKL_MIN_SRC_W 8
>> -- 
>> 2.18.1
>>
>> ___
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Not sure if pushed yet.

Reviewed-by: Maarten Lankhorst 

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/2] drm/i915: Add rotation readout for plane initial config

2018-11-20 Thread Rodrigo Vivi 
On Tue, Nov 20, 2018 at 03:54:50PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä 
> 
> If we need to force a full plane update before userspace/fbdev
> have given us a proper plane state we should try to maintain the
> current plane state as much as possible (apart from the parts
> of the state we're trying to fix up with the plane update).
> To that end add basic readout for the plane rotation and
> maintain it during the initial fb takeover.
> 
> Cc: Hans de Goede 
> Fixes: 516a49cc1946 ("drm/i915: Fix assert_plane() warning on bootup with 
> external display")
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/i915/intel_display.c | 27 +++
>  drivers/gpu/drm/i915/intel_drv.h |  1 +
>  2 files changed, 28 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index 60c1e54285c1..4d339f54559c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2831,6 +2831,7 @@ intel_find_initial_plane_obj(struct intel_crtc 
> *intel_crtc,
>   return;
>  
>  valid_fb:
> + intel_state->base.rotation = plane_config->rotation;
>   intel_fill_fb_ggtt_view(_state->view, fb,
>   intel_state->base.rotation);
>   intel_state->color_plane[0].stride =
> @@ -7787,8 +7788,15 @@ i9xx_get_initial_plane_config(struct intel_crtc *crtc,
>   plane_config->tiling = I915_TILING_X;
>   fb->modifier = I915_FORMAT_MOD_X_TILED;
>   }
> +
> + if (val & DISPPLANE_ROTATE_180)
> + plane_config->rotation = DRM_MODE_ROTATE_180;
>   }
>  
> + if (IS_CHERRYVIEW(dev_priv) && pipe == PIPE_B &&
> + val & DISPPLANE_MIRROR)
> + plane_config->rotation |= DRM_MODE_REFLECT_X;
> +
>   pixel_format = val & DISPPLANE_PIXFORMAT_MASK;
>   fourcc = i9xx_format_to_fourcc(pixel_format);
>   fb->format = drm_format_info(fourcc);
> @@ -8898,6 +8906,25 @@ skylake_get_initial_plane_config(struct intel_crtc 
> *crtc,
>   goto error;
>   }
>  
> + switch (val & PLANE_CTL_ROTATE_MASK) {
> + case PLANE_CTL_ROTATE_0:
> + plane_config->rotation = DRM_MODE_ROTATE_0;
> + break;
> + case PLANE_CTL_ROTATE_90:
> + plane_config->rotation = DRM_MODE_ROTATE_270;

we should add that comment that DRM_MODE_ROTATE_ is counter clockwise
to avoid later confusion.

Also I wonder if we shouldn't move all the cases to a unique
map function.

Anyways,


Reviewed-by: Rodrigo Vivi 



> + break;
> + case PLANE_CTL_ROTATE_180:
> + plane_config->rotation = DRM_MODE_ROTATE_180;
> + break;
> + case PLANE_CTL_ROTATE_270:
> + plane_config->rotation = DRM_MODE_ROTATE_90;
> + break;
> + }
> +
> + if (INTEL_GEN(dev_priv) >= 10 &&
> + val & PLANE_CTL_FLIP_HORIZONTAL)
> + plane_config->rotation |= DRM_MODE_REFLECT_X;
> +
>   base = I915_READ(PLANE_SURF(pipe, plane_id)) & 0xf000;
>   plane_config->base = base;
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h 
> b/drivers/gpu/drm/i915/intel_drv.h
> index f575ba2a59da..a7d9ac912125 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -572,6 +572,7 @@ struct intel_initial_plane_config {
>   unsigned int tiling;
>   int size;
>   u32 base;
> + u8 rotation;
>  };
>  
>  #define SKL_MIN_SRC_W 8
> -- 
> 2.18.1
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx