| Message ID | 20190224153405.8447-1-digetx@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v1] drm/tegra: plane: Remove format-modifier checking | expand |
24.02.2019 18:34, Dmitry Osipenko пишет: > Tiling modifier can't be applied to YV12 video overlay because all tiling > modifiers are filtered out for multi-plane formats. AFAIK, all modifiers > should work with all of formats, hence the checking is incorrect and > simply not needed. > > Fixes: e90124cb46bdb ("drm/tegra: plane: Support format modifiers") > Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > --- For the reference.. this patch needs a stable tag, I did the same mistake in [0]. Thierry, please let me know if you're okay with the change and if you could add the tag while applying the patch, thanks. [0] https://lkml.org/lkml/2019/2/26/277
27.02.2019 18:08, Dmitry Osipenko пишет: > 24.02.2019 18:34, Dmitry Osipenko пишет: >> Tiling modifier can't be applied to YV12 video overlay because all tiling >> modifiers are filtered out for multi-plane formats. AFAIK, all modifiers >> should work with all of formats, hence the checking is incorrect and >> simply not needed. >> >> Fixes: e90124cb46bdb ("drm/tegra: plane: Support format modifiers") >> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> >> --- > > For the reference.. this patch needs a stable tag, I did the same mistake in [0]. Thierry, please let me know if you're okay with the change and if you could add the tag while applying the patch, thanks. > > [0] https://lkml.org/lkml/2019/2/26/277 > Thierry, please don't forget about this patch.
On Sun, Feb 24, 2019 at 06:34:05PM +0300, Dmitry Osipenko wrote: > Tiling modifier can't be applied to YV12 video overlay because all tiling > modifiers are filtered out for multi-plane formats. AFAIK, all modifiers > should work with all of formats, hence the checking is incorrect and > simply not needed. > > Fixes: e90124cb46bdb ("drm/tegra: plane: Support format modifiers") > Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > --- > drivers/gpu/drm/tegra/plane.c | 16 ---------------- > 1 file changed, 16 deletions(-) I'm hesitant to apply this because we don't really have a good way to test that this is actually the case. I vaguely recall that at least for some of the block-linear formats supported on Tegra124 and later there are additional restrictions on when they can be used. There's also the problem that using these block-linear formats, and I think this even applies to the TILED format on older Tegra SoCs, results in higher bandwidth requirements. Bandwidth requirements is something that we don't really concern ourselves with, and that's bad enough as it is. I suspect that once we blindly allow all format modifiers we could easily run into situations where the display controllers underflow. Now, regardless of which way you look at this, it boils down to testing. We don't have a good way of testing various combinations of format modifiers to verify that they work. You say yourself that "AFAIK, all modifiers should work with all formats", but can you really know for certain? Until we're able to properly test this, we really can't. Given all of the above, I think it's better to be prudent and only allow format/modifier combinations that we've actually tested. I'm not aware of a good way to test planar formats, so we don't have a good way to get the results that we need. I'm all ears if you know of a good way to test this. It doesn't have to be anything fully automated. Automated testing is especially difficult to do for display because it usually needs visual inspection. But that's okay, I'm willing to settle for something that we can roll into a script and run manually after boot until we can find a way to automatically do this type of test. Thierry > diff --git a/drivers/gpu/drm/tegra/plane.c b/drivers/gpu/drm/tegra/plane.c > index d068e8aa3553..5a8a3387f5ee 100644 > --- a/drivers/gpu/drm/tegra/plane.c > +++ b/drivers/gpu/drm/tegra/plane.c > @@ -72,21 +72,6 @@ static void tegra_plane_atomic_destroy_state(struct drm_plane *plane, > kfree(state); > } > > -static bool tegra_plane_format_mod_supported(struct drm_plane *plane, > - uint32_t format, > - uint64_t modifier) > -{ > - const struct drm_format_info *info = drm_format_info(format); > - > - if (modifier == DRM_FORMAT_MOD_LINEAR) > - return true; > - > - if (info->num_planes == 1) > - return true; > - > - return false; > -} > - > const struct drm_plane_funcs tegra_plane_funcs = { > .update_plane = drm_atomic_helper_update_plane, > .disable_plane = drm_atomic_helper_disable_plane, > @@ -94,7 +79,6 @@ const struct drm_plane_funcs tegra_plane_funcs = { > .reset = tegra_plane_reset, > .atomic_duplicate_state = tegra_plane_atomic_duplicate_state, > .atomic_destroy_state = tegra_plane_atomic_destroy_state, > - .format_mod_supported = tegra_plane_format_mod_supported, > }; > > int tegra_plane_state_add(struct tegra_plane *plane, > -- > 2.20.1 >
diff --git a/drivers/gpu/drm/tegra/plane.c b/drivers/gpu/drm/tegra/plane.c index d068e8aa3553..5a8a3387f5ee 100644 --- a/drivers/gpu/drm/tegra/plane.c +++ b/drivers/gpu/drm/tegra/plane.c @@ -72,21 +72,6 @@ static void tegra_plane_atomic_destroy_state(struct drm_plane *plane, kfree(state); } -static bool tegra_plane_format_mod_supported(struct drm_plane *plane, - uint32_t format, - uint64_t modifier) -{ - const struct drm_format_info *info = drm_format_info(format); - - if (modifier == DRM_FORMAT_MOD_LINEAR) - return true; - - if (info->num_planes == 1) - return true; - - return false; -} - const struct drm_plane_funcs tegra_plane_funcs = { .update_plane = drm_atomic_helper_update_plane, .disable_plane = drm_atomic_helper_disable_plane, @@ -94,7 +79,6 @@ const struct drm_plane_funcs tegra_plane_funcs = { .reset = tegra_plane_reset, .atomic_duplicate_state = tegra_plane_atomic_duplicate_state, .atomic_destroy_state = tegra_plane_atomic_destroy_state, - .format_mod_supported = tegra_plane_format_mod_supported, }; int tegra_plane_state_add(struct tegra_plane *plane,
Tiling modifier can't be applied to YV12 video overlay because all tiling modifiers are filtered out for multi-plane formats. AFAIK, all modifiers should work with all of formats, hence the checking is incorrect and simply not needed. Fixes: e90124cb46bdb ("drm/tegra: plane: Support format modifiers") Signed-off-by: Dmitry Osipenko <digetx@gmail.com> --- drivers/gpu/drm/tegra/plane.c | 16 ---------------- 1 file changed, 16 deletions(-)