| Message ID | 20180829112847.3641-3-stanislav.lisovskiy@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add XYUV format support | expand |
On Wed, Aug 29, 2018 at 02:28:47PM +0300, Stanislav Lisovskiy wrote: > PLANE_CTL_FORMAT_AYUV is already supported, according to hardware > specification. > > v2: Edited commit message, removed redundant whitespaces. > > v3: Fixed fallthrough logic for the format switch cases. > > v4: Yet again fixed fallthrough logic, to reuse code from other case > labels. > > v5: Started to use XYUV instead of AYUV, as we don't use alpha. > > v6: Removed unneeded initializer for new XYUV format. > > v7: Added scaling support for DRM_FORMAT_XYUV > > Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 8 ++++++++ > drivers/gpu/drm/i915/intel_sprite.c | 1 + > 2 files changed, 9 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 30fdfd1a3037..3c96fa3a2b61 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -86,6 +86,7 @@ static const uint32_t skl_primary_formats[] = { > DRM_FORMAT_YVYU, > DRM_FORMAT_UYVY, > DRM_FORMAT_VYUY, > + DRM_FORMAT_XYUV, > }; > > static const uint32_t skl_pri_planar_formats[] = { > @@ -102,6 +103,7 @@ static const uint32_t skl_pri_planar_formats[] = { > DRM_FORMAT_UYVY, > DRM_FORMAT_VYUY, > DRM_FORMAT_NV12, > + DRM_FORMAT_XYUV, I would keep the NV12 at the end so that the arrays are easier to compare visually. > }; > > static const uint64_t skl_format_modifiers_noccs[] = { > @@ -3501,6 +3503,8 @@ static u32 skl_plane_ctl_format(uint32_t pixel_format) > return PLANE_CTL_FORMAT_XRGB_2101010; > case DRM_FORMAT_XBGR2101010: > return PLANE_CTL_ORDER_RGBX | PLANE_CTL_FORMAT_XRGB_2101010; > + case DRM_FORMAT_XYUV: > + return PLANE_CTL_FORMAT_AYUV; We should probably rename that define to XYUV as well since it doesn't support per-pixel alpha. Since you've only implemented this for skl+ you chould mention that in the commit msg. IVB+ support should be equally trivial to implement (wink wink). > case DRM_FORMAT_YUYV: > return PLANE_CTL_FORMAT_YUV422 | PLANE_CTL_YUV422_YUYV; > case DRM_FORMAT_YVYU: > @@ -4959,6 +4963,7 @@ static int skl_update_scaler_plane(struct intel_crtc_state *crtc_state, > case DRM_FORMAT_UYVY: > case DRM_FORMAT_VYUY: > case DRM_FORMAT_NV12: > + case DRM_FORMAT_XYUV: > break; > default: > DRM_DEBUG_KMS("[PLANE:%d:%s] FB:%d unsupported scaling format 0x%x\n", > @@ -13399,6 +13404,7 @@ static bool skl_plane_format_mod_supported(struct drm_plane *_plane, > } > > switch (format) { > + Bogus whitespace. > case DRM_FORMAT_XRGB8888: > case DRM_FORMAT_XBGR8888: > case DRM_FORMAT_ARGB8888: > @@ -13414,6 +13420,7 @@ static bool skl_plane_format_mod_supported(struct drm_plane *_plane, > case DRM_FORMAT_UYVY: > case DRM_FORMAT_VYUY: > case DRM_FORMAT_NV12: > + case DRM_FORMAT_XYUV: > if (modifier == I915_FORMAT_MOD_Yf_TILED) > return true; > /* fall through */ > @@ -14540,6 +14547,7 @@ static int intel_framebuffer_init(struct intel_framebuffer *intel_fb, > goto err; > } > break; > + case DRM_FORMAT_XYUV: That's not the right spot if we want the platform checks to match your choice of skl+. > case DRM_FORMAT_YUYV: > case DRM_FORMAT_UYVY: > case DRM_FORMAT_YVYU: > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > index c286dda625e4..2ac9471974c2 100644 > --- a/drivers/gpu/drm/i915/intel_sprite.c > +++ b/drivers/gpu/drm/i915/intel_sprite.c > @@ -1420,6 +1420,7 @@ static bool skl_plane_format_mod_supported(struct drm_plane *_plane, > case DRM_FORMAT_UYVY: > case DRM_FORMAT_VYUY: > case DRM_FORMAT_NV12: > + case DRM_FORMAT_XYUV: > if (modifier == I915_FORMAT_MOD_Yf_TILED) > return true; > /* fall through */ > -- > 2.17.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Wed, 2018-08-29 at 21:10 +0300, Ville Syrjälä wrote: > On Wed, Aug 29, 2018 at 02:28:47PM +0300, Stanislav Lisovskiy wrote: > > PLANE_CTL_FORMAT_AYUV is already supported, according to hardware > > specification. > > > > v2: Edited commit message, removed redundant whitespaces. > > > > v3: Fixed fallthrough logic for the format switch cases. > > > > v4: Yet again fixed fallthrough logic, to reuse code from other > > case > > labels. > > > > v5: Started to use XYUV instead of AYUV, as we don't use alpha. > > > > v6: Removed unneeded initializer for new XYUV format. > > > > v7: Added scaling support for DRM_FORMAT_XYUV I don't see yuv formats in skl_format_to_fourcc(), any idea why? Also, shouldn't plane_color_ctl_alpha() be updated? > > > > Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> > > --- > > drivers/gpu/drm/i915/intel_display.c | 8 ++++++++ > > drivers/gpu/drm/i915/intel_sprite.c | 1 + > > 2 files changed, 9 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c > > b/drivers/gpu/drm/i915/intel_display.c > > index 30fdfd1a3037..3c96fa3a2b61 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -86,6 +86,7 @@ static const uint32_t skl_primary_formats[] = { > > DRM_FORMAT_YVYU, > > DRM_FORMAT_UYVY, > > DRM_FORMAT_VYUY, > > + DRM_FORMAT_XYUV, > > }; > > > > static const uint32_t skl_pri_planar_formats[] = { > > @@ -102,6 +103,7 @@ static const uint32_t skl_pri_planar_formats[] > > = { > > DRM_FORMAT_UYVY, > > DRM_FORMAT_VYUY, > > DRM_FORMAT_NV12, > > + DRM_FORMAT_XYUV, > > I would keep the NV12 at the end so that the arrays are easier to > compare visually. > > > }; > > > > static const uint64_t skl_format_modifiers_noccs[] = { > > @@ -3501,6 +3503,8 @@ static u32 skl_plane_ctl_format(uint32_t > > pixel_format) > > return PLANE_CTL_FORMAT_XRGB_2101010; > > case DRM_FORMAT_XBGR2101010: > > return PLANE_CTL_ORDER_RGBX | > > PLANE_CTL_FORMAT_XRGB_2101010; > > + case DRM_FORMAT_XYUV: > > + return PLANE_CTL_FORMAT_AYUV; > > We should probably rename that define to XYUV as well since it > doesn't > support per-pixel alpha. > > Since you've only implemented this for skl+ you chould mention that > in the commit msg. IVB+ support should be equally trivial to > implement (wink wink). > > > case DRM_FORMAT_YUYV: > > return PLANE_CTL_FORMAT_YUV422 | > > PLANE_CTL_YUV422_YUYV; > > case DRM_FORMAT_YVYU: > > @@ -4959,6 +4963,7 @@ static int skl_update_scaler_plane(struct > > intel_crtc_state *crtc_state, > > case DRM_FORMAT_UYVY: > > case DRM_FORMAT_VYUY: > > case DRM_FORMAT_NV12: > > + case DRM_FORMAT_XYUV: > > break; > > default: > > DRM_DEBUG_KMS("[PLANE:%d:%s] FB:%d unsupported > > scaling format 0x%x\n", > > @@ -13399,6 +13404,7 @@ static bool > > skl_plane_format_mod_supported(struct drm_plane *_plane, > > } > > > > switch (format) { > > + > > Bogus whitespace. > > > case DRM_FORMAT_XRGB8888: > > case DRM_FORMAT_XBGR8888: > > case DRM_FORMAT_ARGB8888: > > @@ -13414,6 +13420,7 @@ static bool > > skl_plane_format_mod_supported(struct drm_plane *_plane, > > case DRM_FORMAT_UYVY: > > case DRM_FORMAT_VYUY: > > case DRM_FORMAT_NV12: > > + case DRM_FORMAT_XYUV: > > if (modifier == I915_FORMAT_MOD_Yf_TILED) > > return true; > > /* fall through */ > > @@ -14540,6 +14547,7 @@ static int intel_framebuffer_init(struct > > intel_framebuffer *intel_fb, > > goto err; > > } > > break; > > + case DRM_FORMAT_XYUV: > > That's not the right spot if we want the platform checks to match > your > choice of skl+. > > > case DRM_FORMAT_YUYV: > > case DRM_FORMAT_UYVY: > > case DRM_FORMAT_YVYU: > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c > > b/drivers/gpu/drm/i915/intel_sprite.c > > index c286dda625e4..2ac9471974c2 100644 > > --- a/drivers/gpu/drm/i915/intel_sprite.c > > +++ b/drivers/gpu/drm/i915/intel_sprite.c > > @@ -1420,6 +1420,7 @@ static bool > > skl_plane_format_mod_supported(struct drm_plane *_plane, > > case DRM_FORMAT_UYVY: > > case DRM_FORMAT_VYUY: > > case DRM_FORMAT_NV12: > > + case DRM_FORMAT_XYUV: > > if (modifier == I915_FORMAT_MOD_Yf_TILED) > > return true; > > /* fall through */ > > -- > > 2.17.0 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > >
On Wed, 2018-08-29 at 12:16 -0700, Dhinakaran Pandiyan wrote: > > On Wed, 2018-08-29 at 21:10 +0300, Ville Syrjälä wrote: > > On Wed, Aug 29, 2018 at 02:28:47PM +0300, Stanislav Lisovskiy > > wrote: > > > PLANE_CTL_FORMAT_AYUV is already supported, according to hardware > > > specification. > > > > > > v2: Edited commit message, removed redundant whitespaces. > > > > > > v3: Fixed fallthrough logic for the format switch cases. > > > > > > v4: Yet again fixed fallthrough logic, to reuse code from other > > > case > > > labels. > > > > > > v5: Started to use XYUV instead of AYUV, as we don't use alpha. > > > > > > v6: Removed unneeded initializer for new XYUV format. > > > > > > v7: Added scaling support for DRM_FORMAT_XYUV > > I don't see yuv formats in skl_format_to_fourcc(), any idea why? Good point. I guess would be nice idea to add at least XYUV there now. I can add rest of the formats with a separate patch afterwards. > > Also, shouldn't plane_color_ctl_alpha() be updated? I guess not, as we don't support alpha in that case. > > > > > > > Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com > > > > > > > --- > > > drivers/gpu/drm/i915/intel_display.c | 8 ++++++++ > > > drivers/gpu/drm/i915/intel_sprite.c | 1 + > > > 2 files changed, 9 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c > > > b/drivers/gpu/drm/i915/intel_display.c > > > index 30fdfd1a3037..3c96fa3a2b61 100644 > > > --- a/drivers/gpu/drm/i915/intel_display.c > > > +++ b/drivers/gpu/drm/i915/intel_display.c > > > @@ -86,6 +86,7 @@ static const uint32_t skl_primary_formats[] = { > > > DRM_FORMAT_YVYU, > > > DRM_FORMAT_UYVY, > > > DRM_FORMAT_VYUY, > > > + DRM_FORMAT_XYUV, > > > }; > > > > > > static const uint32_t skl_pri_planar_formats[] = { > > > @@ -102,6 +103,7 @@ static const uint32_t > > > skl_pri_planar_formats[] > > > = { > > > DRM_FORMAT_UYVY, > > > DRM_FORMAT_VYUY, > > > DRM_FORMAT_NV12, > > > + DRM_FORMAT_XYUV, > > > > I would keep the NV12 at the end so that the arrays are easier to > > compare visually. > > > > > }; > > > > > > static const uint64_t skl_format_modifiers_noccs[] = { > > > @@ -3501,6 +3503,8 @@ static u32 skl_plane_ctl_format(uint32_t > > > pixel_format) > > > return PLANE_CTL_FORMAT_XRGB_2101010; > > > case DRM_FORMAT_XBGR2101010: > > > return PLANE_CTL_ORDER_RGBX | > > > PLANE_CTL_FORMAT_XRGB_2101010; > > > + case DRM_FORMAT_XYUV: > > > + return PLANE_CTL_FORMAT_AYUV; > > > > We should probably rename that define to XYUV as well since it > > doesn't > > support per-pixel alpha. > > > > Since you've only implemented this for skl+ you chould mention that > > in the commit msg. IVB+ support should be equally trivial to > > implement (wink wink). > > > > > case DRM_FORMAT_YUYV: > > > return PLANE_CTL_FORMAT_YUV422 | > > > PLANE_CTL_YUV422_YUYV; > > > case DRM_FORMAT_YVYU: > > > @@ -4959,6 +4963,7 @@ static int skl_update_scaler_plane(struct > > > intel_crtc_state *crtc_state, > > > case DRM_FORMAT_UYVY: > > > case DRM_FORMAT_VYUY: > > > case DRM_FORMAT_NV12: > > > + case DRM_FORMAT_XYUV: > > > break; > > > default: > > > DRM_DEBUG_KMS("[PLANE:%d:%s] FB:%d unsupported > > > scaling format 0x%x\n", > > > @@ -13399,6 +13404,7 @@ static bool > > > skl_plane_format_mod_supported(struct drm_plane *_plane, > > > } > > > > > > switch (format) { > > > + > > > > Bogus whitespace. > > > > > case DRM_FORMAT_XRGB8888: > > > case DRM_FORMAT_XBGR8888: > > > case DRM_FORMAT_ARGB8888: > > > @@ -13414,6 +13420,7 @@ static bool > > > skl_plane_format_mod_supported(struct drm_plane *_plane, > > > case DRM_FORMAT_UYVY: > > > case DRM_FORMAT_VYUY: > > > case DRM_FORMAT_NV12: > > > + case DRM_FORMAT_XYUV: > > > if (modifier == I915_FORMAT_MOD_Yf_TILED) > > > return true; > > > /* fall through */ > > > @@ -14540,6 +14547,7 @@ static int intel_framebuffer_init(struct > > > intel_framebuffer *intel_fb, > > > goto err; > > > } > > > break; > > > + case DRM_FORMAT_XYUV: > > > > That's not the right spot if we want the platform checks to match > > your > > choice of skl+. > > > > > case DRM_FORMAT_YUYV: > > > case DRM_FORMAT_UYVY: > > > case DRM_FORMAT_YVYU: > > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c > > > b/drivers/gpu/drm/i915/intel_sprite.c > > > index c286dda625e4..2ac9471974c2 100644 > > > --- a/drivers/gpu/drm/i915/intel_sprite.c > > > +++ b/drivers/gpu/drm/i915/intel_sprite.c > > > @@ -1420,6 +1420,7 @@ static bool > > > skl_plane_format_mod_supported(struct drm_plane *_plane, > > > case DRM_FORMAT_UYVY: > > > case DRM_FORMAT_VYUY: > > > case DRM_FORMAT_NV12: > > > + case DRM_FORMAT_XYUV: > > > if (modifier == I915_FORMAT_MOD_Yf_TILED) > > > return true; > > > /* fall through */ > > > -- > > > 2.17.0 > > > > > > _______________________________________________ > > > Intel-gfx mailing list > > > Intel-gfx@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > >
On Thu, 2018-08-30 at 13:57 +0100, Lisovskiy, Stanislav wrote: > On Wed, 2018-08-29 at 12:16 -0700, Dhinakaran Pandiyan wrote: > > > > On Wed, 2018-08-29 at 21:10 +0300, Ville Syrjälä wrote: > > > On Wed, Aug 29, 2018 at 02:28:47PM +0300, Stanislav Lisovskiy > > > wrote: > > > > PLANE_CTL_FORMAT_AYUV is already supported, according to > > > > hardware > > > > specification. > > > > > > > > v2: Edited commit message, removed redundant whitespaces. > > > > > > > > v3: Fixed fallthrough logic for the format switch cases. > > > > > > > > v4: Yet again fixed fallthrough logic, to reuse code from other > > > > case > > > > labels. > > > > > > > > v5: Started to use XYUV instead of AYUV, as we don't use alpha. Curious what the reason is. Is it because the hardware does not support alpha with this format? > > > > > > > > v6: Removed unneeded initializer for new XYUV format. > > > > > > > > v7: Added scaling support for DRM_FORMAT_XYUV > > > > I don't see yuv formats in skl_format_to_fourcc(), any idea why? > > Good point. I guess would be nice idea to add at least XYUV there > now. > I can add rest of the formats with a separate patch afterwards. Wonder if the expectation is BIOS not use yuv formats. Ville? > > > > > Also, shouldn't plane_color_ctl_alpha() be updated? > > I guess not, as we don't support alpha in that case. Right, the default case should take care of setting PLANE_CTL_ALPHA_DISABLE. I misread it. > > > > > > > > > > > Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.c > > > > om > > > > > > > > > > > > > --- > > > > drivers/gpu/drm/i915/intel_display.c | 8 ++++++++ > > > > drivers/gpu/drm/i915/intel_sprite.c | 1 + > > > > 2 files changed, 9 insertions(+) > > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c > > > > b/drivers/gpu/drm/i915/intel_display.c > > > > index 30fdfd1a3037..3c96fa3a2b61 100644 > > > > --- a/drivers/gpu/drm/i915/intel_display.c > > > > +++ b/drivers/gpu/drm/i915/intel_display.c > > > > @@ -86,6 +86,7 @@ static const uint32_t skl_primary_formats[] = > > > > { > > > > DRM_FORMAT_YVYU, > > > > DRM_FORMAT_UYVY, > > > > DRM_FORMAT_VYUY, > > > > + DRM_FORMAT_XYUV, > > > > }; > > > > > > > > static const uint32_t skl_pri_planar_formats[] = { > > > > @@ -102,6 +103,7 @@ static const uint32_t > > > > skl_pri_planar_formats[] > > > > = { > > > > DRM_FORMAT_UYVY, > > > > DRM_FORMAT_VYUY, > > > > DRM_FORMAT_NV12, > > > > + DRM_FORMAT_XYUV, > > > > > > I would keep the NV12 at the end so that the arrays are easier to > > > compare visually. > > > > > > > }; > > > > > > > > static const uint64_t skl_format_modifiers_noccs[] = { > > > > @@ -3501,6 +3503,8 @@ static u32 skl_plane_ctl_format(uint32_t > > > > pixel_format) > > > > return PLANE_CTL_FORMAT_XRGB_2101010; > > > > case DRM_FORMAT_XBGR2101010: > > > > return PLANE_CTL_ORDER_RGBX | > > > > PLANE_CTL_FORMAT_XRGB_2101010; > > > > + case DRM_FORMAT_XYUV: > > > > + return PLANE_CTL_FORMAT_AYUV; > > > > > > We should probably rename that define to XYUV as well since it > > > doesn't > > > support per-pixel alpha. > > > > > > Since you've only implemented this for skl+ you chould mention > > > that > > > in the commit msg. IVB+ support should be equally trivial to > > > implement (wink wink). > > > > > > > case DRM_FORMAT_YUYV: > > > > return PLANE_CTL_FORMAT_YUV422 | > > > > PLANE_CTL_YUV422_YUYV; > > > > case DRM_FORMAT_YVYU: > > > > @@ -4959,6 +4963,7 @@ static int skl_update_scaler_plane(struct > > > > intel_crtc_state *crtc_state, > > > > case DRM_FORMAT_UYVY: > > > > case DRM_FORMAT_VYUY: > > > > case DRM_FORMAT_NV12: > > > > + case DRM_FORMAT_XYUV: > > > > break; > > > > default: > > > > DRM_DEBUG_KMS("[PLANE:%d:%s] FB:%d unsupported > > > > scaling format 0x%x\n", > > > > @@ -13399,6 +13404,7 @@ static bool > > > > skl_plane_format_mod_supported(struct drm_plane *_plane, > > > > } > > > > > > > > switch (format) { > > > > + > > > > > > Bogus whitespace. > > > > > > > case DRM_FORMAT_XRGB8888: > > > > case DRM_FORMAT_XBGR8888: > > > > case DRM_FORMAT_ARGB8888: > > > > @@ -13414,6 +13420,7 @@ static bool > > > > skl_plane_format_mod_supported(struct drm_plane *_plane, > > > > case DRM_FORMAT_UYVY: > > > > case DRM_FORMAT_VYUY: > > > > case DRM_FORMAT_NV12: > > > > + case DRM_FORMAT_XYUV: > > > > if (modifier == I915_FORMAT_MOD_Yf_TILED) > > > > return true; > > > > /* fall through */ > > > > @@ -14540,6 +14547,7 @@ static int > > > > intel_framebuffer_init(struct > > > > intel_framebuffer *intel_fb, > > > > goto err; > > > > } > > > > break; > > > > + case DRM_FORMAT_XYUV: > > > > > > That's not the right spot if we want the platform checks to match > > > your > > > choice of skl+. > > > > > > > case DRM_FORMAT_YUYV: > > > > case DRM_FORMAT_UYVY: > > > > case DRM_FORMAT_YVYU: > > > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c > > > > b/drivers/gpu/drm/i915/intel_sprite.c > > > > index c286dda625e4..2ac9471974c2 100644 > > > > --- a/drivers/gpu/drm/i915/intel_sprite.c > > > > +++ b/drivers/gpu/drm/i915/intel_sprite.c > > > > @@ -1420,6 +1420,7 @@ static bool > > > > skl_plane_format_mod_supported(struct drm_plane *_plane, > > > > case DRM_FORMAT_UYVY: > > > > case DRM_FORMAT_VYUY: > > > > case DRM_FORMAT_NV12: > > > > + case DRM_FORMAT_XYUV: > > > > if (modifier == I915_FORMAT_MOD_Yf_TILED) > > > > return true; > > > > /* fall through */ > > > > -- > > > > 2.17.0 > > > > > > > > _______________________________________________ > > > > Intel-gfx mailing list > > > > Intel-gfx@lists.freedesktop.org > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > > > > > -- > Best Regards, > > Lisovskiy Stanislav
On Thu, 2018-08-30 at 11:15 -0700, Dhinakaran Pandiyan wrote: > On Thu, 2018-08-30 at 13:57 +0100, Lisovskiy, Stanislav wrote: > > On Wed, 2018-08-29 at 12:16 -0700, Dhinakaran Pandiyan wrote: > > > > > > On Wed, 2018-08-29 at 21:10 +0300, Ville Syrjälä wrote: > > > > On Wed, Aug 29, 2018 at 02:28:47PM +0300, Stanislav Lisovskiy > > > > wrote: > > > > > PLANE_CTL_FORMAT_AYUV is already supported, according to > > > > > hardware > > > > > specification. > > > > > > > > > > v2: Edited commit message, removed redundant whitespaces. > > > > > > > > > > v3: Fixed fallthrough logic for the format switch cases. > > > > > > > > > > v4: Yet again fixed fallthrough logic, to reuse code from > > > > > other > > > > > case > > > > > labels. > > > > > > > > > > v5: Started to use XYUV instead of AYUV, as we don't use > > > > > alpha. > > Curious what the reason is. Is it because the hardware does not > support > alpha with this format? As I understood yes, this is a hardware limitation. > > > > > > > > > > > v6: Removed unneeded initializer for new XYUV format. > > > > > > > > > > v7: Added scaling support for DRM_FORMAT_XYUV > > > > > > I don't see yuv formats in skl_format_to_fourcc(), any idea why? > > > > Good point. I guess would be nice idea to add at least XYUV there > > now. > > I can add rest of the formats with a separate patch afterwards. > > Wonder if the expectation is BIOS not use yuv formats. Ville? I talked to Ville yesterday, I think that was basically what he said. > > > > > > > > > Also, shouldn't plane_color_ctl_alpha() be updated? > > > > I guess not, as we don't support alpha in that case. > > Right, the default case should take care of setting > PLANE_CTL_ALPHA_DISABLE. I misread it. > > > > > > > > > > > > > > > > Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel > > > > > .c > > > > > om > > > > > > > > > > > > > > > > --- > > > > > drivers/gpu/drm/i915/intel_display.c | 8 ++++++++ > > > > > drivers/gpu/drm/i915/intel_sprite.c | 1 + > > > > > 2 files changed, 9 insertions(+) > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c > > > > > b/drivers/gpu/drm/i915/intel_display.c > > > > > index 30fdfd1a3037..3c96fa3a2b61 100644 > > > > > --- a/drivers/gpu/drm/i915/intel_display.c > > > > > +++ b/drivers/gpu/drm/i915/intel_display.c > > > > > @@ -86,6 +86,7 @@ static const uint32_t skl_primary_formats[] > > > > > = > > > > > { > > > > > DRM_FORMAT_YVYU, > > > > > DRM_FORMAT_UYVY, > > > > > DRM_FORMAT_VYUY, > > > > > + DRM_FORMAT_XYUV, > > > > > }; > > > > > > > > > > static const uint32_t skl_pri_planar_formats[] = { > > > > > @@ -102,6 +103,7 @@ static const uint32_t > > > > > skl_pri_planar_formats[] > > > > > = { > > > > > DRM_FORMAT_UYVY, > > > > > DRM_FORMAT_VYUY, > > > > > DRM_FORMAT_NV12, > > > > > + DRM_FORMAT_XYUV, > > > > > > > > I would keep the NV12 at the end so that the arrays are easier > > > > to > > > > compare visually. > > > > > > > > > }; > > > > > > > > > > static const uint64_t skl_format_modifiers_noccs[] = { > > > > > @@ -3501,6 +3503,8 @@ static u32 > > > > > skl_plane_ctl_format(uint32_t > > > > > pixel_format) > > > > > return PLANE_CTL_FORMAT_XRGB_2101010; > > > > > case DRM_FORMAT_XBGR2101010: > > > > > return PLANE_CTL_ORDER_RGBX | > > > > > PLANE_CTL_FORMAT_XRGB_2101010; > > > > > + case DRM_FORMAT_XYUV: > > > > > + return PLANE_CTL_FORMAT_AYUV; > > > > > > > > We should probably rename that define to XYUV as well since it > > > > doesn't > > > > support per-pixel alpha. > > > > > > > > Since you've only implemented this for skl+ you chould mention > > > > that > > > > in the commit msg. IVB+ support should be equally trivial to > > > > implement (wink wink). > > > > > > > > > case DRM_FORMAT_YUYV: > > > > > return PLANE_CTL_FORMAT_YUV422 | > > > > > PLANE_CTL_YUV422_YUYV; > > > > > case DRM_FORMAT_YVYU: > > > > > @@ -4959,6 +4963,7 @@ static int > > > > > skl_update_scaler_plane(struct > > > > > intel_crtc_state *crtc_state, > > > > > case DRM_FORMAT_UYVY: > > > > > case DRM_FORMAT_VYUY: > > > > > case DRM_FORMAT_NV12: > > > > > + case DRM_FORMAT_XYUV: > > > > > break; > > > > > default: > > > > > DRM_DEBUG_KMS("[PLANE:%d:%s] FB:%d > > > > > unsupported > > > > > scaling format 0x%x\n", > > > > > @@ -13399,6 +13404,7 @@ static bool > > > > > skl_plane_format_mod_supported(struct drm_plane *_plane, > > > > > } > > > > > > > > > > switch (format) { > > > > > + > > > > > > > > Bogus whitespace. > > > > > > > > > case DRM_FORMAT_XRGB8888: > > > > > case DRM_FORMAT_XBGR8888: > > > > > case DRM_FORMAT_ARGB8888: > > > > > @@ -13414,6 +13420,7 @@ static bool > > > > > skl_plane_format_mod_supported(struct drm_plane *_plane, > > > > > case DRM_FORMAT_UYVY: > > > > > case DRM_FORMAT_VYUY: > > > > > case DRM_FORMAT_NV12: > > > > > + case DRM_FORMAT_XYUV: > > > > > if (modifier == I915_FORMAT_MOD_Yf_TILED) > > > > > return true; > > > > > /* fall through */ > > > > > @@ -14540,6 +14547,7 @@ static int > > > > > intel_framebuffer_init(struct > > > > > intel_framebuffer *intel_fb, > > > > > goto err; > > > > > } > > > > > break; > > > > > + case DRM_FORMAT_XYUV: > > > > > > > > That's not the right spot if we want the platform checks to > > > > match > > > > your > > > > choice of skl+. > > > > > > > > > case DRM_FORMAT_YUYV: > > > > > case DRM_FORMAT_UYVY: > > > > > case DRM_FORMAT_YVYU: > > > > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c > > > > > b/drivers/gpu/drm/i915/intel_sprite.c > > > > > index c286dda625e4..2ac9471974c2 100644 > > > > > --- a/drivers/gpu/drm/i915/intel_sprite.c > > > > > +++ b/drivers/gpu/drm/i915/intel_sprite.c > > > > > @@ -1420,6 +1420,7 @@ static bool > > > > > skl_plane_format_mod_supported(struct drm_plane *_plane, > > > > > case DRM_FORMAT_UYVY: > > > > > case DRM_FORMAT_VYUY: > > > > > case DRM_FORMAT_NV12: > > > > > + case DRM_FORMAT_XYUV: > > > > > if (modifier == I915_FORMAT_MOD_Yf_TILED) > > > > > return true; > > > > > /* fall through */ > > > > > -- > > > > > 2.17.0 > > > > > > > > > > _______________________________________________ > > > > > Intel-gfx mailing list > > > > > Intel-gfx@lists.freedesktop.org > > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > > > > > > > > > -- > > Best Regards, > > > > Lisovskiy Stanislav
On Fri, Aug 31, 2018 at 07:24:48AM +0000, Lisovskiy, Stanislav wrote: > On Thu, 2018-08-30 at 11:15 -0700, Dhinakaran Pandiyan wrote: > > On Thu, 2018-08-30 at 13:57 +0100, Lisovskiy, Stanislav wrote: > > > On Wed, 2018-08-29 at 12:16 -0700, Dhinakaran Pandiyan wrote: > > > > > > > > On Wed, 2018-08-29 at 21:10 +0300, Ville Syrjälä wrote: > > > > > On Wed, Aug 29, 2018 at 02:28:47PM +0300, Stanislav Lisovskiy > > > > > wrote: > > > > > > PLANE_CTL_FORMAT_AYUV is already supported, according to > > > > > > hardware > > > > > > specification. > > > > > > > > > > > > v2: Edited commit message, removed redundant whitespaces. > > > > > > > > > > > > v3: Fixed fallthrough logic for the format switch cases. > > > > > > > > > > > > v4: Yet again fixed fallthrough logic, to reuse code from > > > > > > other > > > > > > case > > > > > > labels. > > > > > > > > > > > > v5: Started to use XYUV instead of AYUV, as we don't use > > > > > > alpha. > > > > Curious what the reason is. Is it because the hardware does not > > support > > alpha with this format? > > As I understood yes, this is a hardware limitation. > > > > > > > > > > > > > > > v6: Removed unneeded initializer for new XYUV format. > > > > > > > > > > > > v7: Added scaling support for DRM_FORMAT_XYUV > > > > > > > > I don't see yuv formats in skl_format_to_fourcc(), any idea why? > > > > > > Good point. I guess would be nice idea to add at least XYUV there > > > now. > > > I can add rest of the formats with a separate patch afterwards. > > > > Wonder if the expectation is BIOS not use yuv formats. Ville? > > I talked to Ville yesterday, I think that was basically what he said. Yes. Although I have this dream of full plane state readout (which we could then use for verification purposes at least), so adding the missing formats there would be a decent idea.
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 30fdfd1a3037..3c96fa3a2b61 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -86,6 +86,7 @@ static const uint32_t skl_primary_formats[] = { DRM_FORMAT_YVYU, DRM_FORMAT_UYVY, DRM_FORMAT_VYUY, + DRM_FORMAT_XYUV, }; static const uint32_t skl_pri_planar_formats[] = { @@ -102,6 +103,7 @@ static const uint32_t skl_pri_planar_formats[] = { DRM_FORMAT_UYVY, DRM_FORMAT_VYUY, DRM_FORMAT_NV12, + DRM_FORMAT_XYUV, }; static const uint64_t skl_format_modifiers_noccs[] = { @@ -3501,6 +3503,8 @@ static u32 skl_plane_ctl_format(uint32_t pixel_format) return PLANE_CTL_FORMAT_XRGB_2101010; case DRM_FORMAT_XBGR2101010: return PLANE_CTL_ORDER_RGBX | PLANE_CTL_FORMAT_XRGB_2101010; + case DRM_FORMAT_XYUV: + return PLANE_CTL_FORMAT_AYUV; case DRM_FORMAT_YUYV: return PLANE_CTL_FORMAT_YUV422 | PLANE_CTL_YUV422_YUYV; case DRM_FORMAT_YVYU: @@ -4959,6 +4963,7 @@ static int skl_update_scaler_plane(struct intel_crtc_state *crtc_state, case DRM_FORMAT_UYVY: case DRM_FORMAT_VYUY: case DRM_FORMAT_NV12: + case DRM_FORMAT_XYUV: break; default: DRM_DEBUG_KMS("[PLANE:%d:%s] FB:%d unsupported scaling format 0x%x\n", @@ -13399,6 +13404,7 @@ static bool skl_plane_format_mod_supported(struct drm_plane *_plane, } switch (format) { + case DRM_FORMAT_XRGB8888: case DRM_FORMAT_XBGR8888: case DRM_FORMAT_ARGB8888: @@ -13414,6 +13420,7 @@ static bool skl_plane_format_mod_supported(struct drm_plane *_plane, case DRM_FORMAT_UYVY: case DRM_FORMAT_VYUY: case DRM_FORMAT_NV12: + case DRM_FORMAT_XYUV: if (modifier == I915_FORMAT_MOD_Yf_TILED) return true; /* fall through */ @@ -14540,6 +14547,7 @@ static int intel_framebuffer_init(struct intel_framebuffer *intel_fb, goto err; } break; + case DRM_FORMAT_XYUV: case DRM_FORMAT_YUYV: case DRM_FORMAT_UYVY: case DRM_FORMAT_YVYU: diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index c286dda625e4..2ac9471974c2 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -1420,6 +1420,7 @@ static bool skl_plane_format_mod_supported(struct drm_plane *_plane, case DRM_FORMAT_UYVY: case DRM_FORMAT_VYUY: case DRM_FORMAT_NV12: + case DRM_FORMAT_XYUV: if (modifier == I915_FORMAT_MOD_Yf_TILED) return true; /* fall through */
PLANE_CTL_FORMAT_AYUV is already supported, according to hardware specification. v2: Edited commit message, removed redundant whitespaces. v3: Fixed fallthrough logic for the format switch cases. v4: Yet again fixed fallthrough logic, to reuse code from other case labels. v5: Started to use XYUV instead of AYUV, as we don't use alpha. v6: Removed unneeded initializer for new XYUV format. v7: Added scaling support for DRM_FORMAT_XYUV Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> --- drivers/gpu/drm/i915/intel_display.c | 8 ++++++++ drivers/gpu/drm/i915/intel_sprite.c | 1 + 2 files changed, 9 insertions(+)