| Message ID | 20230109105807.18172-4-mcanal@igalia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Check for valid framebuffer's format | expand |
The Intel counterpart is also:
Reviewed-by: Simon Ser <contact@emersion.fr>
On Mon, Jan 09, 2023 at 07:58:06AM -0300, Maíra Canal wrote: > Now that framebuffer_check() verifies that the format is properly > supported, there is no need to check it again on i915's inside > helpers. > > Therefore, remove the redundant framebuffer format check from the > intel_framebuffer_init() function, letting framebuffer_check() > perform the framebuffer validation. > > Signed-off-by: Maíra Canal <mcanal@igalia.com> > --- > drivers/gpu/drm/i915/display/intel_fb.c | 9 --------- > 1 file changed, 9 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_fb.c b/drivers/gpu/drm/i915/display/intel_fb.c > index 63137ae5ab21..230b729e42d6 100644 > --- a/drivers/gpu/drm/i915/display/intel_fb.c > +++ b/drivers/gpu/drm/i915/display/intel_fb.c > @@ -1914,15 +1914,6 @@ int intel_framebuffer_init(struct intel_framebuffer *intel_fb, > } > } > > - if (!drm_any_plane_has_format(&dev_priv->drm, > - mode_cmd->pixel_format, > - mode_cmd->modifier[0])) { > - drm_dbg_kms(&dev_priv->drm, > - "unsupported pixel format %p4cc / modifier 0x%llx\n", > - &mode_cmd->pixel_format, mode_cmd->modifier[0]); > - goto err; > - } > - This doesn't work for the legacy tiling->modifier path. > /* > * gen2/3 display engine uses the fence if present, > * so the tiling mode must match the fb modifier exactly. > -- > 2.39.0
Hi, On 1/12/23 09:43, Ville Syrjälä wrote: > On Mon, Jan 09, 2023 at 07:58:06AM -0300, Maíra Canal wrote: >> Now that framebuffer_check() verifies that the format is properly >> supported, there is no need to check it again on i915's inside >> helpers. >> >> Therefore, remove the redundant framebuffer format check from the >> intel_framebuffer_init() function, letting framebuffer_check() >> perform the framebuffer validation. >> >> Signed-off-by: Maíra Canal <mcanal@igalia.com> >> --- >> drivers/gpu/drm/i915/display/intel_fb.c | 9 --------- >> 1 file changed, 9 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_fb.c b/drivers/gpu/drm/i915/display/intel_fb.c >> index 63137ae5ab21..230b729e42d6 100644 >> --- a/drivers/gpu/drm/i915/display/intel_fb.c >> +++ b/drivers/gpu/drm/i915/display/intel_fb.c >> @@ -1914,15 +1914,6 @@ int intel_framebuffer_init(struct intel_framebuffer *intel_fb, >> } >> } >> >> - if (!drm_any_plane_has_format(&dev_priv->drm, >> - mode_cmd->pixel_format, >> - mode_cmd->modifier[0])) { >> - drm_dbg_kms(&dev_priv->drm, >> - "unsupported pixel format %p4cc / modifier 0x%llx\n", >> - &mode_cmd->pixel_format, mode_cmd->modifier[0]); >> - goto err; >> - } >> - > > This doesn't work for the legacy tiling->modifier path. Do you have any idea on how we could remove drm_any_plane_has_format() from i915? Or is it strictly necessary to validate the modifier in the legacy path? Best Regards, - Maíra Canal > >> /* >> * gen2/3 display engine uses the fence if present, >> * so the tiling mode must match the fb modifier exactly. >> -- >> 2.39.0 >
On Thu, Jan 12, 2023 at 11:07:59AM -0300, Maíra Canal wrote: > Hi, > > On 1/12/23 09:43, Ville Syrjälä wrote: > > On Mon, Jan 09, 2023 at 07:58:06AM -0300, Maíra Canal wrote: > >> Now that framebuffer_check() verifies that the format is properly > >> supported, there is no need to check it again on i915's inside > >> helpers. > >> > >> Therefore, remove the redundant framebuffer format check from the > >> intel_framebuffer_init() function, letting framebuffer_check() > >> perform the framebuffer validation. > >> > >> Signed-off-by: Maíra Canal <mcanal@igalia.com> > >> --- > >> drivers/gpu/drm/i915/display/intel_fb.c | 9 --------- > >> 1 file changed, 9 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/display/intel_fb.c b/drivers/gpu/drm/i915/display/intel_fb.c > >> index 63137ae5ab21..230b729e42d6 100644 > >> --- a/drivers/gpu/drm/i915/display/intel_fb.c > >> +++ b/drivers/gpu/drm/i915/display/intel_fb.c > >> @@ -1914,15 +1914,6 @@ int intel_framebuffer_init(struct intel_framebuffer *intel_fb, > >> } > >> } > >> > >> - if (!drm_any_plane_has_format(&dev_priv->drm, > >> - mode_cmd->pixel_format, > >> - mode_cmd->modifier[0])) { > >> - drm_dbg_kms(&dev_priv->drm, > >> - "unsupported pixel format %p4cc / modifier 0x%llx\n", > >> - &mode_cmd->pixel_format, mode_cmd->modifier[0]); > >> - goto err; > >> - } > >> - > > > > This doesn't work for the legacy tiling->modifier path. > > Do you have any idea on how we could remove drm_any_plane_has_format() from > i915? Or is it strictly necessary to validate the modifier in the legacy > path? I guess techinically we could skip it by knowing that X-tile is always supported. However that may not hold in the future so not a soution I really like. Also the drm_any_plane_has_format() call from framebuffer_check() is too early, so instead of checking X-tile vs. linear based on the tiling it's going to always assume linear.
diff --git a/drivers/gpu/drm/i915/display/intel_fb.c b/drivers/gpu/drm/i915/display/intel_fb.c index 63137ae5ab21..230b729e42d6 100644 --- a/drivers/gpu/drm/i915/display/intel_fb.c +++ b/drivers/gpu/drm/i915/display/intel_fb.c @@ -1914,15 +1914,6 @@ int intel_framebuffer_init(struct intel_framebuffer *intel_fb, } } - if (!drm_any_plane_has_format(&dev_priv->drm, - mode_cmd->pixel_format, - mode_cmd->modifier[0])) { - drm_dbg_kms(&dev_priv->drm, - "unsupported pixel format %p4cc / modifier 0x%llx\n", - &mode_cmd->pixel_format, mode_cmd->modifier[0]); - goto err; - } - /* * gen2/3 display engine uses the fence if present, * so the tiling mode must match the fb modifier exactly.
Now that framebuffer_check() verifies that the format is properly supported, there is no need to check it again on i915's inside helpers. Therefore, remove the redundant framebuffer format check from the intel_framebuffer_init() function, letting framebuffer_check() perform the framebuffer validation. Signed-off-by: Maíra Canal <mcanal@igalia.com> --- drivers/gpu/drm/i915/display/intel_fb.c | 9 --------- 1 file changed, 9 deletions(-)