| Message ID | 20200616163406.27387-1-matthew.s.atwood@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Apply Wa_14011264657:gen11+ | expand |
On Tue, Jun 16, 2020 at 09:34:06AM -0700, Matt Atwood wrote: > Add minimum width to planes, variable with specific formats, for gen11+. How did this suddenly become gen11+? Wasn't it rkl only before? > > Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com> > --- > drivers/gpu/drm/i915/display/intel_display.c | 55 +++++++++++++++++--- > 1 file changed, 47 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > index 7457813ef273..d4fdad6cb3b1 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -3760,6 +3760,45 @@ static int glk_max_plane_width(const struct drm_framebuffer *fb, > } > } > > +static int icl_min_plane_width(struct drm_i915_private *dev_priv, > + const struct drm_framebuffer *fb) > +{ > + /* Wa_14011264657, Wa_14011050563 */ > + switch (fb->format->format) { > + case DRM_FORMAT_C8: > + return 18; > + case DRM_FORMAT_RGB565: > + return 10; > + case DRM_FORMAT_XRGB8888: > + case DRM_FORMAT_XBGR8888: > + case DRM_FORMAT_ARGB8888: > + case DRM_FORMAT_ABGR8888: > + case DRM_FORMAT_XRGB2101010: > + case DRM_FORMAT_XBGR2101010: > + case DRM_FORMAT_ARGB2101010: > + case DRM_FORMAT_ABGR2101010: > + case DRM_FORMAT_XVYU2101010: > + case DRM_FORMAT_Y212: > + case DRM_FORMAT_Y216: > + return 6; > + case DRM_FORMAT_NV12: > + return 20; > + case DRM_FORMAT_P010: > + case DRM_FORMAT_P012: > + case DRM_FORMAT_P016: > + return 12; > + case DRM_FORMAT_XRGB16161616F: > + case DRM_FORMAT_XBGR16161616F: > + case DRM_FORMAT_ARGB16161616F: > + case DRM_FORMAT_ABGR16161616F: > + case DRM_FORMAT_XVYU12_16161616: > + case DRM_FORMAT_XVYU16161616: > + return 4; > + default: > + return 1; > + } > +} > + > static int icl_max_plane_width(const struct drm_framebuffer *fb, > int color_plane, > unsigned int rotation) > @@ -3831,15 +3870,15 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state) > int y = plane_state->uapi.src.y1 >> 16; > int w = drm_rect_width(&plane_state->uapi.src) >> 16; > int h = drm_rect_height(&plane_state->uapi.src) >> 16; > - int max_width; > - int max_height; > - u32 alignment; > - u32 offset; > + int max_width, min_width = 1, max_height; > + u32 alignment, offset; > int aux_plane = intel_main_to_aux_plane(fb, 0); > u32 aux_offset = plane_state->color_plane[aux_plane].offset; > > - if (INTEL_GEN(dev_priv) >= 11) > + if (INTEL_GEN(dev_priv) >= 11) { > max_width = icl_max_plane_width(fb, 0, rotation); > + min_width = icl_min_plane_width(dev_priv, fb); > + } > else if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) > max_width = glk_max_plane_width(fb, 0, rotation); > else > @@ -3850,10 +3889,10 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state) > else > max_height = skl_max_plane_height(); > > - if (w > max_width || h > max_height) { > + if (w > max_width || w < min_width || h > max_height) { > drm_dbg_kms(&dev_priv->drm, > - "requested Y/RGB source size %dx%d too big (limit %dx%d)\n", > - w, h, max_width, max_height); > + "requested Y/RGB source size %dx%d outside limits (min: %dx1 max: %dx%d)\n", > + w, h, min_width, max_width, max_height); > return -EINVAL; > } > > -- > 2.21.3 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, Jun 16, 2020 at 07:39:09PM +0300, Ville Syrjälä wrote: > On Tue, Jun 16, 2020 at 09:34:06AM -0700, Matt Atwood wrote: > > Add minimum width to planes, variable with specific formats, for gen11+. > > How did this suddenly become gen11+? Wasn't it rkl only before? gen11 platforms were currently in pending, that has changed. > > > > > Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_display.c | 55 +++++++++++++++++--- > > 1 file changed, 47 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > > index 7457813ef273..d4fdad6cb3b1 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > @@ -3760,6 +3760,45 @@ static int glk_max_plane_width(const struct drm_framebuffer *fb, > > } > > } > > > > +static int icl_min_plane_width(struct drm_i915_private *dev_priv, > > + const struct drm_framebuffer *fb) > > +{ > > + /* Wa_14011264657, Wa_14011050563 */ > > + switch (fb->format->format) { > > + case DRM_FORMAT_C8: > > + return 18; > > + case DRM_FORMAT_RGB565: > > + return 10; > > + case DRM_FORMAT_XRGB8888: > > + case DRM_FORMAT_XBGR8888: > > + case DRM_FORMAT_ARGB8888: > > + case DRM_FORMAT_ABGR8888: > > + case DRM_FORMAT_XRGB2101010: > > + case DRM_FORMAT_XBGR2101010: > > + case DRM_FORMAT_ARGB2101010: > > + case DRM_FORMAT_ABGR2101010: > > + case DRM_FORMAT_XVYU2101010: > > + case DRM_FORMAT_Y212: > > + case DRM_FORMAT_Y216: > > + return 6; > > + case DRM_FORMAT_NV12: > > + return 20; > > + case DRM_FORMAT_P010: > > + case DRM_FORMAT_P012: > > + case DRM_FORMAT_P016: > > + return 12; > > + case DRM_FORMAT_XRGB16161616F: > > + case DRM_FORMAT_XBGR16161616F: > > + case DRM_FORMAT_ARGB16161616F: > > + case DRM_FORMAT_ABGR16161616F: > > + case DRM_FORMAT_XVYU12_16161616: > > + case DRM_FORMAT_XVYU16161616: > > + return 4; > > + default: > > + return 1; > > + } > > +} > > + > > static int icl_max_plane_width(const struct drm_framebuffer *fb, > > int color_plane, > > unsigned int rotation) > > @@ -3831,15 +3870,15 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state) > > int y = plane_state->uapi.src.y1 >> 16; > > int w = drm_rect_width(&plane_state->uapi.src) >> 16; > > int h = drm_rect_height(&plane_state->uapi.src) >> 16; > > - int max_width; > > - int max_height; > > - u32 alignment; > > - u32 offset; > > + int max_width, min_width = 1, max_height; > > + u32 alignment, offset; > > int aux_plane = intel_main_to_aux_plane(fb, 0); > > u32 aux_offset = plane_state->color_plane[aux_plane].offset; > > > > - if (INTEL_GEN(dev_priv) >= 11) > > + if (INTEL_GEN(dev_priv) >= 11) { > > max_width = icl_max_plane_width(fb, 0, rotation); > > + min_width = icl_min_plane_width(dev_priv, fb); > > + } > > else if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) > > max_width = glk_max_plane_width(fb, 0, rotation); > > else > > @@ -3850,10 +3889,10 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state) > > else > > max_height = skl_max_plane_height(); > > > > - if (w > max_width || h > max_height) { > > + if (w > max_width || w < min_width || h > max_height) { > > drm_dbg_kms(&dev_priv->drm, > > - "requested Y/RGB source size %dx%d too big (limit %dx%d)\n", > > - w, h, max_width, max_height); > > + "requested Y/RGB source size %dx%d outside limits (min: %dx1 max: %dx%d)\n", > > + w, h, min_width, max_width, max_height); > > return -EINVAL; > > } > > > > -- > > 2.21.3 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Ville Syrjälä > Intel
On Tue, Jun 16, 2020 at 10:01:40AM -0700, Matt Atwood wrote: > On Tue, Jun 16, 2020 at 07:39:09PM +0300, Ville Syrjälä wrote: > > On Tue, Jun 16, 2020 at 09:34:06AM -0700, Matt Atwood wrote: > > > Add minimum width to planes, variable with specific formats, for gen11+. > > > > How did this suddenly become gen11+? Wasn't it rkl only before? > gen11 platforms were currently in pending, that has changed. What does "in pending" mean? > > > > > > > > Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com> > > > --- > > > drivers/gpu/drm/i915/display/intel_display.c | 55 +++++++++++++++++--- > > > 1 file changed, 47 insertions(+), 8 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > > > index 7457813ef273..d4fdad6cb3b1 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > > @@ -3760,6 +3760,45 @@ static int glk_max_plane_width(const struct drm_framebuffer *fb, > > > } > > > } > > > > > > +static int icl_min_plane_width(struct drm_i915_private *dev_priv, > > > + const struct drm_framebuffer *fb) > > > +{ > > > + /* Wa_14011264657, Wa_14011050563 */ > > > + switch (fb->format->format) { > > > + case DRM_FORMAT_C8: > > > + return 18; > > > + case DRM_FORMAT_RGB565: > > > + return 10; > > > + case DRM_FORMAT_XRGB8888: > > > + case DRM_FORMAT_XBGR8888: > > > + case DRM_FORMAT_ARGB8888: > > > + case DRM_FORMAT_ABGR8888: > > > + case DRM_FORMAT_XRGB2101010: > > > + case DRM_FORMAT_XBGR2101010: > > > + case DRM_FORMAT_ARGB2101010: > > > + case DRM_FORMAT_ABGR2101010: > > > + case DRM_FORMAT_XVYU2101010: > > > + case DRM_FORMAT_Y212: > > > + case DRM_FORMAT_Y216: > > > + return 6; > > > + case DRM_FORMAT_NV12: > > > + return 20; > > > + case DRM_FORMAT_P010: > > > + case DRM_FORMAT_P012: > > > + case DRM_FORMAT_P016: > > > + return 12; > > > + case DRM_FORMAT_XRGB16161616F: > > > + case DRM_FORMAT_XBGR16161616F: > > > + case DRM_FORMAT_ARGB16161616F: > > > + case DRM_FORMAT_ABGR16161616F: > > > + case DRM_FORMAT_XVYU12_16161616: > > > + case DRM_FORMAT_XVYU16161616: > > > + return 4; > > > + default: > > > + return 1; > > > + } > > > +} > > > + > > > static int icl_max_plane_width(const struct drm_framebuffer *fb, > > > int color_plane, > > > unsigned int rotation) > > > @@ -3831,15 +3870,15 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state) > > > int y = plane_state->uapi.src.y1 >> 16; > > > int w = drm_rect_width(&plane_state->uapi.src) >> 16; > > > int h = drm_rect_height(&plane_state->uapi.src) >> 16; > > > - int max_width; > > > - int max_height; > > > - u32 alignment; > > > - u32 offset; > > > + int max_width, min_width = 1, max_height; > > > + u32 alignment, offset; > > > int aux_plane = intel_main_to_aux_plane(fb, 0); > > > u32 aux_offset = plane_state->color_plane[aux_plane].offset; > > > > > > - if (INTEL_GEN(dev_priv) >= 11) > > > + if (INTEL_GEN(dev_priv) >= 11) { > > > max_width = icl_max_plane_width(fb, 0, rotation); > > > + min_width = icl_min_plane_width(dev_priv, fb); > > > + } > > > else if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) > > > max_width = glk_max_plane_width(fb, 0, rotation); > > > else > > > @@ -3850,10 +3889,10 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state) > > > else > > > max_height = skl_max_plane_height(); > > > > > > - if (w > max_width || h > max_height) { > > > + if (w > max_width || w < min_width || h > max_height) { > > > drm_dbg_kms(&dev_priv->drm, > > > - "requested Y/RGB source size %dx%d too big (limit %dx%d)\n", > > > - w, h, max_width, max_height); > > > + "requested Y/RGB source size %dx%d outside limits (min: %dx1 max: %dx%d)\n", > > > + w, h, min_width, max_width, max_height); > > > return -EINVAL; > > > } > > > > > > -- > > > 2.21.3 > > > > > > _______________________________________________ > > > Intel-gfx mailing list > > > Intel-gfx@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > -- > > Ville Syrjälä > > Intel
On Tue, Jun 16, 2020 at 09:34:06AM -0700, Matt Atwood wrote: > Add minimum width to planes, variable with specific formats, for gen11+. > > Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com> > --- > drivers/gpu/drm/i915/display/intel_display.c | 55 +++++++++++++++++--- > 1 file changed, 47 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > index 7457813ef273..d4fdad6cb3b1 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -3760,6 +3760,45 @@ static int glk_max_plane_width(const struct drm_framebuffer *fb, > } > } > > +static int icl_min_plane_width(struct drm_i915_private *dev_priv, > + const struct drm_framebuffer *fb) > +{ > + /* Wa_14011264657, Wa_14011050563 */ > + switch (fb->format->format) { > + case DRM_FORMAT_C8: > + return 18; > + case DRM_FORMAT_RGB565: > + return 10; > + case DRM_FORMAT_XRGB8888: > + case DRM_FORMAT_XBGR8888: > + case DRM_FORMAT_ARGB8888: > + case DRM_FORMAT_ABGR8888: > + case DRM_FORMAT_XRGB2101010: > + case DRM_FORMAT_XBGR2101010: > + case DRM_FORMAT_ARGB2101010: > + case DRM_FORMAT_ABGR2101010: > + case DRM_FORMAT_XVYU2101010: > + case DRM_FORMAT_Y212: > + case DRM_FORMAT_Y216: > + return 6; > + case DRM_FORMAT_NV12: > + return 20; > + case DRM_FORMAT_P010: > + case DRM_FORMAT_P012: > + case DRM_FORMAT_P016: > + return 12; > + case DRM_FORMAT_XRGB16161616F: > + case DRM_FORMAT_XBGR16161616F: > + case DRM_FORMAT_ARGB16161616F: > + case DRM_FORMAT_ABGR16161616F: > + case DRM_FORMAT_XVYU12_16161616: > + case DRM_FORMAT_XVYU16161616: > + return 4; > + default: > + return 1; > + } if (semiplanar) { switch (cpp[0]) { case 1: return 20; case 2: return 12; } } else { switch (cpp[0]) { case 1: return 18; case 2: return 10; case 4: return 6; case 8: return 4; } } Actually if we fully reverse engineer this we are left with just: if (semiplanar) return 16/cpp[0] + 4; else return 16/cpp[0] + 2; I'd much prefer calculating this since then it's fully divorced from defining new pixel formats. Can we get a confirmation from the hw folks if that is in fact the formula (or if there's a different formula how they came up with these magic numbers)? > +} > + > static int icl_max_plane_width(const struct drm_framebuffer *fb, > int color_plane, > unsigned int rotation) > @@ -3831,15 +3870,15 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state) > int y = plane_state->uapi.src.y1 >> 16; > int w = drm_rect_width(&plane_state->uapi.src) >> 16; > int h = drm_rect_height(&plane_state->uapi.src) >> 16; > - int max_width; > - int max_height; > - u32 alignment; > - u32 offset; > + int max_width, min_width = 1, max_height; > + u32 alignment, offset; > int aux_plane = intel_main_to_aux_plane(fb, 0); > u32 aux_offset = plane_state->color_plane[aux_plane].offset; > > - if (INTEL_GEN(dev_priv) >= 11) > + if (INTEL_GEN(dev_priv) >= 11) { > max_width = icl_max_plane_width(fb, 0, rotation); > + min_width = icl_min_plane_width(dev_priv, fb); > + } > else if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) Missing curly braces on all the branches. Feels like dejavu... I'd also do the min_width=1 assignment in each branch to make it clear what's what. > max_width = glk_max_plane_width(fb, 0, rotation); > else > @@ -3850,10 +3889,10 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state) > else > max_height = skl_max_plane_height(); > > - if (w > max_width || h > max_height) { > + if (w > max_width || w < min_width || h > max_height) { > drm_dbg_kms(&dev_priv->drm, > - "requested Y/RGB source size %dx%d too big (limit %dx%d)\n", > - w, h, max_width, max_height); > + "requested Y/RGB source size %dx%d outside limits (min: %dx1 max: %dx%d)\n", > + w, h, min_width, max_width, max_height); > return -EINVAL; > } > > -- > 2.21.3 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On 6/16/20 10:34 AM, Ville Syrjälä wrote: > On Tue, Jun 16, 2020 at 09:34:06AM -0700, Matt Atwood wrote: >> Add minimum width to planes, variable with specific formats, for gen11+. >> >> Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com> >> --- >> drivers/gpu/drm/i915/display/intel_display.c | 55 +++++++++++++++++--- >> 1 file changed, 47 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c >> index 7457813ef273..d4fdad6cb3b1 100644 >> --- a/drivers/gpu/drm/i915/display/intel_display.c >> +++ b/drivers/gpu/drm/i915/display/intel_display.c >> @@ -3760,6 +3760,45 @@ static int glk_max_plane_width(const struct drm_framebuffer *fb, >> } >> } >> >> +static int icl_min_plane_width(struct drm_i915_private *dev_priv, >> + const struct drm_framebuffer *fb) >> +{ >> + /* Wa_14011264657, Wa_14011050563 */ >> + switch (fb->format->format) { >> + case DRM_FORMAT_C8: >> + return 18; >> + case DRM_FORMAT_RGB565: >> + return 10; >> + case DRM_FORMAT_XRGB8888: >> + case DRM_FORMAT_XBGR8888: >> + case DRM_FORMAT_ARGB8888: >> + case DRM_FORMAT_ABGR8888: >> + case DRM_FORMAT_XRGB2101010: >> + case DRM_FORMAT_XBGR2101010: >> + case DRM_FORMAT_ARGB2101010: >> + case DRM_FORMAT_ABGR2101010: >> + case DRM_FORMAT_XVYU2101010: >> + case DRM_FORMAT_Y212: >> + case DRM_FORMAT_Y216: >> + return 6; >> + case DRM_FORMAT_NV12: >> + return 20; >> + case DRM_FORMAT_P010: >> + case DRM_FORMAT_P012: >> + case DRM_FORMAT_P016: >> + return 12; >> + case DRM_FORMAT_XRGB16161616F: >> + case DRM_FORMAT_XBGR16161616F: >> + case DRM_FORMAT_ARGB16161616F: >> + case DRM_FORMAT_ABGR16161616F: >> + case DRM_FORMAT_XVYU12_16161616: >> + case DRM_FORMAT_XVYU16161616: >> + return 4; >> + default: >> + return 1; >> + } > > if (semiplanar) { > switch (cpp[0]) { > case 1: > return 20; > case 2: > return 12; > } > } else { > switch (cpp[0]) { > case 1: > return 18; > case 2: > return 10; > case 4: > return 6; > case 8: > return 4; > } > } > > Actually if we fully reverse engineer this we are left with just: > if (semiplanar) > return 16/cpp[0] + 4; > else > return 16/cpp[0] + 2; > > I'd much prefer calculating this since then it's fully divorced from > defining new pixel formats. Can we get a confirmation from the hw > folks if that is in fact the formula (or if there's a different formula > how they came up with these magic numbers)? > >> +} >> + >> static int icl_max_plane_width(const struct drm_framebuffer *fb, >> int color_plane, >> unsigned int rotation) >> @@ -3831,15 +3870,15 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state) >> int y = plane_state->uapi.src.y1 >> 16; >> int w = drm_rect_width(&plane_state->uapi.src) >> 16; >> int h = drm_rect_height(&plane_state->uapi.src) >> 16; >> - int max_width; >> - int max_height; >> - u32 alignment; >> - u32 offset; >> + int max_width, min_width = 1, max_height; >> + u32 alignment, offset; >> int aux_plane = intel_main_to_aux_plane(fb, 0); >> u32 aux_offset = plane_state->color_plane[aux_plane].offset; >> >> - if (INTEL_GEN(dev_priv) >= 11) >> + if (INTEL_GEN(dev_priv) >= 11) { >> max_width = icl_max_plane_width(fb, 0, rotation); >> + min_width = icl_min_plane_width(dev_priv, fb); To add to Ville's comments, there is no need for dev_priv and while we are at it, I don't understand the significance of passing parameters rotation if we are not going to use it? Aditya >> + } >> else if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) > > Missing curly braces on all the branches. Feels like dejavu... > > I'd also do the min_width=1 assignment in each branch to make it > clear what's what. > >> max_width = glk_max_plane_width(fb, 0, rotation); >> else >> @@ -3850,10 +3889,10 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state) >> else >> max_height = skl_max_plane_height(); >> >> - if (w > max_width || h > max_height) { >> + if (w > max_width || w < min_width || h > max_height) { >> drm_dbg_kms(&dev_priv->drm, >> - "requested Y/RGB source size %dx%d too big (limit %dx%d)\n", >> - w, h, max_width, max_height); >> + "requested Y/RGB source size %dx%d outside limits (min: %dx1 max: %dx%d)\n", >> + w, h, min_width, max_width, max_height); >> return -EINVAL; >> } >> >> -- >> 2.21.3 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx >
On Tue, Jun 16, 2020 at 08:34:07PM +0300, Ville Syrjälä wrote: > On Tue, Jun 16, 2020 at 09:34:06AM -0700, Matt Atwood wrote: > > Add minimum width to planes, variable with specific formats, for gen11+. > > > > Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_display.c | 55 +++++++++++++++++--- > > 1 file changed, 47 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > > index 7457813ef273..d4fdad6cb3b1 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > @@ -3760,6 +3760,45 @@ static int glk_max_plane_width(const struct drm_framebuffer *fb, > > } > > } > > > > +static int icl_min_plane_width(struct drm_i915_private *dev_priv, > > + const struct drm_framebuffer *fb) > > +{ > > + /* Wa_14011264657, Wa_14011050563 */ > > + switch (fb->format->format) { > > + case DRM_FORMAT_C8: > > + return 18; > > + case DRM_FORMAT_RGB565: > > + return 10; > > + case DRM_FORMAT_XRGB8888: > > + case DRM_FORMAT_XBGR8888: > > + case DRM_FORMAT_ARGB8888: > > + case DRM_FORMAT_ABGR8888: > > + case DRM_FORMAT_XRGB2101010: > > + case DRM_FORMAT_XBGR2101010: > > + case DRM_FORMAT_ARGB2101010: > > + case DRM_FORMAT_ABGR2101010: > > + case DRM_FORMAT_XVYU2101010: > > + case DRM_FORMAT_Y212: > > + case DRM_FORMAT_Y216: > > + return 6; > > + case DRM_FORMAT_NV12: > > + return 20; > > + case DRM_FORMAT_P010: > > + case DRM_FORMAT_P012: > > + case DRM_FORMAT_P016: > > + return 12; > > + case DRM_FORMAT_XRGB16161616F: > > + case DRM_FORMAT_XBGR16161616F: > > + case DRM_FORMAT_ARGB16161616F: > > + case DRM_FORMAT_ABGR16161616F: > > + case DRM_FORMAT_XVYU12_16161616: > > + case DRM_FORMAT_XVYU16161616: > > + return 4; > > + default: > > + return 1; > > + } > > if (semiplanar) { > switch (cpp[0]) { > case 1: > return 20; > case 2: > return 12; > } > } else { > switch (cpp[0]) { > case 1: > return 18; > case 2: > return 10; > case 4: > return 6; > case 8: > return 4; > } > } > > Actually if we fully reverse engineer this we are left with just: > if (semiplanar) > return 16/cpp[0] + 4; > else > return 16/cpp[0] + 2; > > I'd much prefer calculating this since then it's fully divorced from > defining new pixel formats. Can we get a confirmation from the hw > folks if that is in fact the formula (or if there's a different formula > how they came up with these magic numbers)? Verified with hardware folks the above doesnt always work for some of the cases that we need to support. > > > +} > > + > > static int icl_max_plane_width(const struct drm_framebuffer *fb, > > int color_plane, > > unsigned int rotation) > > @@ -3831,15 +3870,15 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state) > > int y = plane_state->uapi.src.y1 >> 16; > > int w = drm_rect_width(&plane_state->uapi.src) >> 16; > > int h = drm_rect_height(&plane_state->uapi.src) >> 16; > > - int max_width; > > - int max_height; > > - u32 alignment; > > - u32 offset; > > + int max_width, min_width = 1, max_height; > > + u32 alignment, offset; > > int aux_plane = intel_main_to_aux_plane(fb, 0); > > u32 aux_offset = plane_state->color_plane[aux_plane].offset; > > > > - if (INTEL_GEN(dev_priv) >= 11) > > + if (INTEL_GEN(dev_priv) >= 11) { > > max_width = icl_max_plane_width(fb, 0, rotation); > > + min_width = icl_min_plane_width(dev_priv, fb); > > + } > > else if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) > > Missing curly braces on all the branches. Feels like dejavu... > > I'd also do the min_width=1 assignment in each branch to make it > clear what's what. > > > max_width = glk_max_plane_width(fb, 0, rotation); > > else > > @@ -3850,10 +3889,10 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state) > > else > > max_height = skl_max_plane_height(); > > > > - if (w > max_width || h > max_height) { > > + if (w > max_width || w < min_width || h > max_height) { > > drm_dbg_kms(&dev_priv->drm, > > - "requested Y/RGB source size %dx%d too big (limit %dx%d)\n", > > - w, h, max_width, max_height); > > + "requested Y/RGB source size %dx%d outside limits (min: %dx1 max: %dx%d)\n", > > + w, h, min_width, max_width, max_height); > > return -EINVAL; > > } > > > > -- > > 2.21.3 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Ville Syrjälä > Intel
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 7457813ef273..d4fdad6cb3b1 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -3760,6 +3760,45 @@ static int glk_max_plane_width(const struct drm_framebuffer *fb, } } +static int icl_min_plane_width(struct drm_i915_private *dev_priv, + const struct drm_framebuffer *fb) +{ + /* Wa_14011264657, Wa_14011050563 */ + switch (fb->format->format) { + case DRM_FORMAT_C8: + return 18; + case DRM_FORMAT_RGB565: + return 10; + case DRM_FORMAT_XRGB8888: + case DRM_FORMAT_XBGR8888: + case DRM_FORMAT_ARGB8888: + case DRM_FORMAT_ABGR8888: + case DRM_FORMAT_XRGB2101010: + case DRM_FORMAT_XBGR2101010: + case DRM_FORMAT_ARGB2101010: + case DRM_FORMAT_ABGR2101010: + case DRM_FORMAT_XVYU2101010: + case DRM_FORMAT_Y212: + case DRM_FORMAT_Y216: + return 6; + case DRM_FORMAT_NV12: + return 20; + case DRM_FORMAT_P010: + case DRM_FORMAT_P012: + case DRM_FORMAT_P016: + return 12; + case DRM_FORMAT_XRGB16161616F: + case DRM_FORMAT_XBGR16161616F: + case DRM_FORMAT_ARGB16161616F: + case DRM_FORMAT_ABGR16161616F: + case DRM_FORMAT_XVYU12_16161616: + case DRM_FORMAT_XVYU16161616: + return 4; + default: + return 1; + } +} + static int icl_max_plane_width(const struct drm_framebuffer *fb, int color_plane, unsigned int rotation) @@ -3831,15 +3870,15 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state) int y = plane_state->uapi.src.y1 >> 16; int w = drm_rect_width(&plane_state->uapi.src) >> 16; int h = drm_rect_height(&plane_state->uapi.src) >> 16; - int max_width; - int max_height; - u32 alignment; - u32 offset; + int max_width, min_width = 1, max_height; + u32 alignment, offset; int aux_plane = intel_main_to_aux_plane(fb, 0); u32 aux_offset = plane_state->color_plane[aux_plane].offset; - if (INTEL_GEN(dev_priv) >= 11) + if (INTEL_GEN(dev_priv) >= 11) { max_width = icl_max_plane_width(fb, 0, rotation); + min_width = icl_min_plane_width(dev_priv, fb); + } else if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) max_width = glk_max_plane_width(fb, 0, rotation); else @@ -3850,10 +3889,10 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state) else max_height = skl_max_plane_height(); - if (w > max_width || h > max_height) { + if (w > max_width || w < min_width || h > max_height) { drm_dbg_kms(&dev_priv->drm, - "requested Y/RGB source size %dx%d too big (limit %dx%d)\n", - w, h, max_width, max_height); + "requested Y/RGB source size %dx%d outside limits (min: %dx1 max: %dx%d)\n", + w, h, min_width, max_width, max_height); return -EINVAL; }
Add minimum width to planes, variable with specific formats, for gen11+. Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com> --- drivers/gpu/drm/i915/display/intel_display.c | 55 +++++++++++++++++--- 1 file changed, 47 insertions(+), 8 deletions(-)