* [PATCH v3 0/2] Fix ycbcr_420_allowed inconsistency for HDMI bridges
@ 2024-12-16 22:54 Cristian Ciocaltea
2024-12-16 22:54 ` [PATCH v3 1/2] drm/bridge-connector: Prioritize supported_formats over ycbcr_420_allowed Cristian Ciocaltea
2024-12-16 22:54 ` [PATCH v3 2/2] drm/connector: hdmi: Validate supported_formats matches ycbcr_420_allowed Cristian Ciocaltea
0 siblings, 2 replies; 10+ messages in thread
From: Cristian Ciocaltea @ 2024-12-16 22:54 UTC (permalink / raw)
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Dmitry Baryshkov
Cc: kernel, dri-devel, linux-kernel
Bridges with DRM_BRIDGE_OP_HDMI set in drm_bridge->ops are expected to
rely on drm_bridge->supported_formats to advertise the supported
colorspaces, including HDMI_COLORSPACE_YUV420.
However, when drm_bridge_connector gets initialised, only
drm_bridge->ycbcr_420_allowed is considered in the process of adjusting
the drm_connector->ycbcr_420_allowed, which effectively discards the
formats advertised by the HDMI bridge.
This patchset tries to address the issue by prioritizing
supported_formats over ycbcr_420_allowed.
Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
Changes in v3:
- Simplified the inconsistency handling by overwriting ycbcr_420_allowed
for HDMI bridges before adding them to the global bridge list
- Added a 2nd patch to check if supported_formats matches
ycbcr_420_allowed on HDMI connector initialization (Dmitry)
- Link to v2: https://lore.kernel.org/r/20241206-bridge-conn-fmt-prio-v2-1-85c817529b88@collabora.com
Changes in v2:
- Wrapped HDMI_COLORSPACE_YUV420 flag in the BIT() macro to properly
check its presence in supported_formats
- Ensured YUV420 gets removed from the bitmask passed to
drmm_connector_hdmi_init() when ycbcr_420_allowed is not set
- Link to v1: https://lore.kernel.org/r/20241130-bridge-conn-fmt-prio-v1-1-146b663f17f3@collabora.com
---
Cristian Ciocaltea (2):
drm/bridge-connector: Prioritize supported_formats over ycbcr_420_allowed
drm/connector: hdmi: Validate supported_formats matches ycbcr_420_allowed
drivers/gpu/drm/display/drm_bridge_connector.c | 8 ++++++--
drivers/gpu/drm/drm_bridge.c | 4 ++++
drivers/gpu/drm/drm_connector.c | 3 +++
3 files changed, 13 insertions(+), 2 deletions(-)
---
base-commit: f486c8aa16b8172f63bddc70116a0c897a7f3f02
change-id: 20241130-bridge-conn-fmt-prio-c517c1407ed5
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH v3 1/2] drm/bridge-connector: Prioritize supported_formats over ycbcr_420_allowed 2024-12-16 22:54 [PATCH v3 0/2] Fix ycbcr_420_allowed inconsistency for HDMI bridges Cristian Ciocaltea @ 2024-12-16 22:54 ` Cristian Ciocaltea 2024-12-16 23:27 ` Dmitry Baryshkov 2024-12-16 22:54 ` [PATCH v3 2/2] drm/connector: hdmi: Validate supported_formats matches ycbcr_420_allowed Cristian Ciocaltea 1 sibling, 1 reply; 10+ messages in thread From: Cristian Ciocaltea @ 2024-12-16 22:54 UTC (permalink / raw) To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Dmitry Baryshkov Cc: kernel, dri-devel, linux-kernel Bridges having the DRM_BRIDGE_OP_HDMI flag set in drm_bridge->ops are supposed to rely on drm_bridge->supported_formats bitmask to advertise the supported colorspaces, including HDMI_COLORSPACE_YUV420. Therefore, the newly introduced drm_bridge->ycbcr_420_allowed flag becomes redundant in this particular context. Moreover, when drm_bridge_connector gets initialised, only drm_bridge->ycbcr_420_allowed is considered in the process of adjusting the equivalent property of the base drm_connector, which effectively discards the formats advertised by the HDMI bridge. Handle the inconsistency by overwriting drm_bridge->ycbcr_420_allowed for HDMI bridges according to drm_bridge->supported_formats, before adding them to the global bridge list. Additionally, ensure the YUV420 related bit is removed from the bitmask passed to drmm_connector_hdmi_init() when the final ycbcr_420_allowed flag for the connector ends up not being set (i.e. the case of having at least one non-HDMI bridge in the pipeline that didn't enable it). Fixes: 3ced1c687512 ("drm/display: bridge_connector: handle ycbcr_420_allowed") Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com> --- drivers/gpu/drm/display/drm_bridge_connector.c | 8 ++++++-- drivers/gpu/drm/drm_bridge.c | 4 ++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c index 320c297008aaa8b6ef5b1f4c71928849b202e8ac..3a5a4f92c979accaa2a8f79ca0f15396dd579429 100644 --- a/drivers/gpu/drm/display/drm_bridge_connector.c +++ b/drivers/gpu/drm/display/drm_bridge_connector.c @@ -459,7 +459,10 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, if (connector_type == DRM_MODE_CONNECTOR_Unknown) return ERR_PTR(-EINVAL); - if (bridge_connector->bridge_hdmi) + if (bridge_connector->bridge_hdmi) { + if (!connector->ycbcr_420_allowed) + supported_formats &= ~BIT(HDMI_COLORSPACE_YUV420); + ret = drmm_connector_hdmi_init(drm, connector, bridge_connector->bridge_hdmi->vendor, bridge_connector->bridge_hdmi->product, @@ -468,10 +471,11 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, connector_type, ddc, supported_formats, max_bpc); - else + } else { ret = drmm_connector_init(drm, connector, &drm_bridge_connector_funcs, connector_type, ddc); + } if (ret) return ERR_PTR(ret); diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c index c6af46dd02bfa9e15b59e4c460debdd7fd84be44..241a384ebce39b4a3db58c208af27960904fc662 100644 --- a/drivers/gpu/drm/drm_bridge.c +++ b/drivers/gpu/drm/drm_bridge.c @@ -207,6 +207,10 @@ void drm_bridge_add(struct drm_bridge *bridge) { mutex_init(&bridge->hpd_mutex); + if (bridge->ops & DRM_BRIDGE_OP_HDMI) + bridge->ycbcr_420_allowed = !!(bridge->supported_formats & + BIT(HDMI_COLORSPACE_YUV420)); + mutex_lock(&bridge_lock); list_add_tail(&bridge->list, &bridge_list); mutex_unlock(&bridge_lock); -- 2.47.0 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/2] drm/bridge-connector: Prioritize supported_formats over ycbcr_420_allowed 2024-12-16 22:54 ` [PATCH v3 1/2] drm/bridge-connector: Prioritize supported_formats over ycbcr_420_allowed Cristian Ciocaltea @ 2024-12-16 23:27 ` Dmitry Baryshkov 2024-12-17 0:38 ` Cristian Ciocaltea 0 siblings, 1 reply; 10+ messages in thread From: Dmitry Baryshkov @ 2024-12-16 23:27 UTC (permalink / raw) To: Cristian Ciocaltea Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, kernel, dri-devel, linux-kernel On Tue, Dec 17, 2024 at 12:54:07AM +0200, Cristian Ciocaltea wrote: > Bridges having the DRM_BRIDGE_OP_HDMI flag set in drm_bridge->ops are > supposed to rely on drm_bridge->supported_formats bitmask to advertise > the supported colorspaces, including HDMI_COLORSPACE_YUV420. Therefore, > the newly introduced drm_bridge->ycbcr_420_allowed flag becomes > redundant in this particular context. > > Moreover, when drm_bridge_connector gets initialised, only > drm_bridge->ycbcr_420_allowed is considered in the process of adjusting > the equivalent property of the base drm_connector, which effectively > discards the formats advertised by the HDMI bridge. > > Handle the inconsistency by overwriting drm_bridge->ycbcr_420_allowed > for HDMI bridges according to drm_bridge->supported_formats, before > adding them to the global bridge list. > > Additionally, ensure the YUV420 related bit is removed from the bitmask > passed to drmm_connector_hdmi_init() when the final ycbcr_420_allowed > flag for the connector ends up not being set (i.e. the case of having at > least one non-HDMI bridge in the pipeline that didn't enable it). > > Fixes: 3ced1c687512 ("drm/display: bridge_connector: handle ycbcr_420_allowed") > Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com> > --- > drivers/gpu/drm/display/drm_bridge_connector.c | 8 ++++++-- > drivers/gpu/drm/drm_bridge.c | 4 ++++ > 2 files changed, 10 insertions(+), 2 deletions(-) I think the second patch in the series is enough, it ensures that connector's state is consistent, no matter if the drm_bridge_connector is being used or a normal drm_connector. Nevertheless, I'd leave the final decision to DRM maintainers. -- With best wishes Dmitry ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/2] drm/bridge-connector: Prioritize supported_formats over ycbcr_420_allowed 2024-12-16 23:27 ` Dmitry Baryshkov @ 2024-12-17 0:38 ` Cristian Ciocaltea 2024-12-20 1:05 ` Dmitry Baryshkov 0 siblings, 1 reply; 10+ messages in thread From: Cristian Ciocaltea @ 2024-12-17 0:38 UTC (permalink / raw) To: Dmitry Baryshkov Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, kernel, dri-devel, linux-kernel On 12/17/24 1:27 AM, Dmitry Baryshkov wrote: > On Tue, Dec 17, 2024 at 12:54:07AM +0200, Cristian Ciocaltea wrote: >> Bridges having the DRM_BRIDGE_OP_HDMI flag set in drm_bridge->ops are >> supposed to rely on drm_bridge->supported_formats bitmask to advertise >> the supported colorspaces, including HDMI_COLORSPACE_YUV420. Therefore, >> the newly introduced drm_bridge->ycbcr_420_allowed flag becomes >> redundant in this particular context. >> >> Moreover, when drm_bridge_connector gets initialised, only >> drm_bridge->ycbcr_420_allowed is considered in the process of adjusting >> the equivalent property of the base drm_connector, which effectively >> discards the formats advertised by the HDMI bridge. >> >> Handle the inconsistency by overwriting drm_bridge->ycbcr_420_allowed >> for HDMI bridges according to drm_bridge->supported_formats, before >> adding them to the global bridge list. >> >> Additionally, ensure the YUV420 related bit is removed from the bitmask >> passed to drmm_connector_hdmi_init() when the final ycbcr_420_allowed >> flag for the connector ends up not being set (i.e. the case of having at >> least one non-HDMI bridge in the pipeline that didn't enable it). >> >> Fixes: 3ced1c687512 ("drm/display: bridge_connector: handle ycbcr_420_allowed") >> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com> >> --- >> drivers/gpu/drm/display/drm_bridge_connector.c | 8 ++++++-- >> drivers/gpu/drm/drm_bridge.c | 4 ++++ >> 2 files changed, 10 insertions(+), 2 deletions(-) > > I think the second patch in the series is enough, it ensures that > connector's state is consistent, no matter if the drm_bridge_connector > is being used or a normal drm_connector. > > Nevertheless, I'd leave the final decision to DRM maintainers. This patch has 2 parts, maybe I should have put them into separate patches as they kind of relate to distinct problems. The 1st part makes sure that drm_bridge->ycbcr_420_allowed is automatically set when HDMI_COLORSPACE_YUV420 is provided in drm_bridge->supported_formats, to avoid the need of requiring redundant information on HDMI bridges initialization. This implicitly ensures the consistency needed to further allow relying on ->ycbcr_420_allowed internally. While the 1st part could be dropped (assuming redundancy & consistency is not really something we want/need to handle), the 2nd part I think is mandatory, i.e. we must adjust supported_formats before calling drmm_connector_hdmi_init() to ensure the presence of HDMI_COLORSPACE_YUV420 reflects the status of the computed connector->ycbcr_420_allowed, which might end up being different than what the HDMI bridge advertised, i.e. the case of having an HDMI bridge in the pipeline advertising YUV420 via supported_formats and a non-HDMI one that didn't enable ycbcr_420_allowed. Thanks, Cristian ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/2] drm/bridge-connector: Prioritize supported_formats over ycbcr_420_allowed 2024-12-17 0:38 ` Cristian Ciocaltea @ 2024-12-20 1:05 ` Dmitry Baryshkov 2024-12-24 18:37 ` Cristian Ciocaltea 0 siblings, 1 reply; 10+ messages in thread From: Dmitry Baryshkov @ 2024-12-20 1:05 UTC (permalink / raw) To: Cristian Ciocaltea Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, kernel, dri-devel, linux-kernel On Tue, Dec 17, 2024 at 02:38:55AM +0200, Cristian Ciocaltea wrote: > On 12/17/24 1:27 AM, Dmitry Baryshkov wrote: > > On Tue, Dec 17, 2024 at 12:54:07AM +0200, Cristian Ciocaltea wrote: > >> Bridges having the DRM_BRIDGE_OP_HDMI flag set in drm_bridge->ops are > >> supposed to rely on drm_bridge->supported_formats bitmask to advertise > >> the supported colorspaces, including HDMI_COLORSPACE_YUV420. Therefore, > >> the newly introduced drm_bridge->ycbcr_420_allowed flag becomes > >> redundant in this particular context. > >> > >> Moreover, when drm_bridge_connector gets initialised, only > >> drm_bridge->ycbcr_420_allowed is considered in the process of adjusting > >> the equivalent property of the base drm_connector, which effectively > >> discards the formats advertised by the HDMI bridge. > >> > >> Handle the inconsistency by overwriting drm_bridge->ycbcr_420_allowed > >> for HDMI bridges according to drm_bridge->supported_formats, before > >> adding them to the global bridge list. > >> > >> Additionally, ensure the YUV420 related bit is removed from the bitmask > >> passed to drmm_connector_hdmi_init() when the final ycbcr_420_allowed > >> flag for the connector ends up not being set (i.e. the case of having at > >> least one non-HDMI bridge in the pipeline that didn't enable it). > >> > >> Fixes: 3ced1c687512 ("drm/display: bridge_connector: handle ycbcr_420_allowed") > >> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com> > >> --- > >> drivers/gpu/drm/display/drm_bridge_connector.c | 8 ++++++-- > >> drivers/gpu/drm/drm_bridge.c | 4 ++++ > >> 2 files changed, 10 insertions(+), 2 deletions(-) > > > > I think the second patch in the series is enough, it ensures that > > connector's state is consistent, no matter if the drm_bridge_connector > > is being used or a normal drm_connector. > > > > Nevertheless, I'd leave the final decision to DRM maintainers. > > This patch has 2 parts, maybe I should have put them into separate patches > as they kind of relate to distinct problems. > > The 1st part makes sure that drm_bridge->ycbcr_420_allowed is automatically > set when HDMI_COLORSPACE_YUV420 is provided in drm_bridge->supported_formats, > to avoid the need of requiring redundant information on HDMI bridges > initialization. This implicitly ensures the consistency needed to further > allow relying on ->ycbcr_420_allowed internally. > > While the 1st part could be dropped (assuming redundancy & consistency is > not really something we want/need to handle), the 2nd part I think is > mandatory, i.e. we must adjust supported_formats before calling > drmm_connector_hdmi_init() to ensure the presence of HDMI_COLORSPACE_YUV420 > reflects the status of the computed connector->ycbcr_420_allowed, which > might end up being different than what the HDMI bridge advertised, i.e. the > case of having an HDMI bridge in the pipeline advertising YUV420 via > supported_formats and a non-HDMI one that didn't enable ycbcr_420_allowed. Please split it into two patches. I don't have a strong opinion upon the first one (I'd change it to dev_warn() maybe), while the second one (removing HDMI_COLORSPACE_YUV420 if connector->ycbcr_420_allowed is false) is definitely a correct change. > > Thanks, > Cristian -- With best wishes Dmitry ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/2] drm/bridge-connector: Prioritize supported_formats over ycbcr_420_allowed 2024-12-20 1:05 ` Dmitry Baryshkov @ 2024-12-24 18:37 ` Cristian Ciocaltea 0 siblings, 0 replies; 10+ messages in thread From: Cristian Ciocaltea @ 2024-12-24 18:37 UTC (permalink / raw) To: Dmitry Baryshkov Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, kernel, dri-devel, linux-kernel On 12/20/24 3:05 AM, Dmitry Baryshkov wrote: > On Tue, Dec 17, 2024 at 02:38:55AM +0200, Cristian Ciocaltea wrote: >> On 12/17/24 1:27 AM, Dmitry Baryshkov wrote: >>> On Tue, Dec 17, 2024 at 12:54:07AM +0200, Cristian Ciocaltea wrote: >>>> Bridges having the DRM_BRIDGE_OP_HDMI flag set in drm_bridge->ops are >>>> supposed to rely on drm_bridge->supported_formats bitmask to advertise >>>> the supported colorspaces, including HDMI_COLORSPACE_YUV420. Therefore, >>>> the newly introduced drm_bridge->ycbcr_420_allowed flag becomes >>>> redundant in this particular context. >>>> >>>> Moreover, when drm_bridge_connector gets initialised, only >>>> drm_bridge->ycbcr_420_allowed is considered in the process of adjusting >>>> the equivalent property of the base drm_connector, which effectively >>>> discards the formats advertised by the HDMI bridge. >>>> >>>> Handle the inconsistency by overwriting drm_bridge->ycbcr_420_allowed >>>> for HDMI bridges according to drm_bridge->supported_formats, before >>>> adding them to the global bridge list. >>>> >>>> Additionally, ensure the YUV420 related bit is removed from the bitmask >>>> passed to drmm_connector_hdmi_init() when the final ycbcr_420_allowed >>>> flag for the connector ends up not being set (i.e. the case of having at >>>> least one non-HDMI bridge in the pipeline that didn't enable it). >>>> >>>> Fixes: 3ced1c687512 ("drm/display: bridge_connector: handle ycbcr_420_allowed") >>>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com> >>>> --- >>>> drivers/gpu/drm/display/drm_bridge_connector.c | 8 ++++++-- >>>> drivers/gpu/drm/drm_bridge.c | 4 ++++ >>>> 2 files changed, 10 insertions(+), 2 deletions(-) >>> >>> I think the second patch in the series is enough, it ensures that >>> connector's state is consistent, no matter if the drm_bridge_connector >>> is being used or a normal drm_connector. >>> >>> Nevertheless, I'd leave the final decision to DRM maintainers. >> >> This patch has 2 parts, maybe I should have put them into separate patches >> as they kind of relate to distinct problems. >> >> The 1st part makes sure that drm_bridge->ycbcr_420_allowed is automatically >> set when HDMI_COLORSPACE_YUV420 is provided in drm_bridge->supported_formats, >> to avoid the need of requiring redundant information on HDMI bridges >> initialization. This implicitly ensures the consistency needed to further >> allow relying on ->ycbcr_420_allowed internally. >> >> While the 1st part could be dropped (assuming redundancy & consistency is >> not really something we want/need to handle), the 2nd part I think is >> mandatory, i.e. we must adjust supported_formats before calling >> drmm_connector_hdmi_init() to ensure the presence of HDMI_COLORSPACE_YUV420 >> reflects the status of the computed connector->ycbcr_420_allowed, which >> might end up being different than what the HDMI bridge advertised, i.e. the >> case of having an HDMI bridge in the pipeline advertising YUV420 via >> supported_formats and a non-HDMI one that didn't enable ycbcr_420_allowed. > > Please split it into two patches. I don't have a strong opinion upon the > first one (I'd change it to dev_warn() maybe), while the second one > (removing HDMI_COLORSPACE_YUV420 if connector->ycbcr_420_allowed is > false) is definitely a correct change. Split done in v4: https://lore.kernel.org/lkml/20241224-bridge-conn-fmt-prio-v4-2-a9ceb5671379@collabora.com/ Thanks, Cristian ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 2/2] drm/connector: hdmi: Validate supported_formats matches ycbcr_420_allowed 2024-12-16 22:54 [PATCH v3 0/2] Fix ycbcr_420_allowed inconsistency for HDMI bridges Cristian Ciocaltea 2024-12-16 22:54 ` [PATCH v3 1/2] drm/bridge-connector: Prioritize supported_formats over ycbcr_420_allowed Cristian Ciocaltea @ 2024-12-16 22:54 ` Cristian Ciocaltea 2024-12-16 23:26 ` Dmitry Baryshkov 2024-12-17 15:25 ` Maxime Ripard 1 sibling, 2 replies; 10+ messages in thread From: Cristian Ciocaltea @ 2024-12-16 22:54 UTC (permalink / raw) To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Dmitry Baryshkov Cc: kernel, dri-devel, linux-kernel Ensure HDMI connector initialization fails when the presence of HDMI_COLORSPACE_YUV420 in the given supported_formats bitmask doesn't match the value of drm_connector->ycbcr_420_allowed. Suggested-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com> --- drivers/gpu/drm/drm_connector.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index fc35f47e2849ed6786d6223ac9c69e1c359fc648..ca7f43c8d6f1b31ef9d3a1ee05f4df930ecffac4 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -507,6 +507,9 @@ int drmm_connector_hdmi_init(struct drm_device *dev, if (!supported_formats || !(supported_formats & BIT(HDMI_COLORSPACE_RGB))) return -EINVAL; + if (connector->ycbcr_420_allowed != !!(supported_formats & BIT(HDMI_COLORSPACE_YUV420))) + return -EINVAL; + if (!(max_bpc == 8 || max_bpc == 10 || max_bpc == 12)) return -EINVAL; -- 2.47.0 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/2] drm/connector: hdmi: Validate supported_formats matches ycbcr_420_allowed 2024-12-16 22:54 ` [PATCH v3 2/2] drm/connector: hdmi: Validate supported_formats matches ycbcr_420_allowed Cristian Ciocaltea @ 2024-12-16 23:26 ` Dmitry Baryshkov 2024-12-17 15:25 ` Maxime Ripard 1 sibling, 0 replies; 10+ messages in thread From: Dmitry Baryshkov @ 2024-12-16 23:26 UTC (permalink / raw) To: Cristian Ciocaltea Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, kernel, dri-devel, linux-kernel On Tue, Dec 17, 2024 at 12:54:08AM +0200, Cristian Ciocaltea wrote: > Ensure HDMI connector initialization fails when the presence of > HDMI_COLORSPACE_YUV420 in the given supported_formats bitmask doesn't > match the value of drm_connector->ycbcr_420_allowed. > > Suggested-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com> > --- > drivers/gpu/drm/drm_connector.c | 3 +++ > 1 file changed, 3 insertions(+) > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> -- With best wishes Dmitry ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/2] drm/connector: hdmi: Validate supported_formats matches ycbcr_420_allowed 2024-12-16 22:54 ` [PATCH v3 2/2] drm/connector: hdmi: Validate supported_formats matches ycbcr_420_allowed Cristian Ciocaltea 2024-12-16 23:26 ` Dmitry Baryshkov @ 2024-12-17 15:25 ` Maxime Ripard 2024-12-24 18:35 ` Cristian Ciocaltea 1 sibling, 1 reply; 10+ messages in thread From: Maxime Ripard @ 2024-12-17 15:25 UTC (permalink / raw) To: Cristian Ciocaltea Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Thomas Zimmermann, David Airlie, Simona Vetter, Dmitry Baryshkov, kernel, dri-devel, linux-kernel [-- Attachment #1: Type: text/plain, Size: 1237 bytes --] On Tue, Dec 17, 2024 at 12:54:08AM +0200, Cristian Ciocaltea wrote: > Ensure HDMI connector initialization fails when the presence of > HDMI_COLORSPACE_YUV420 in the given supported_formats bitmask doesn't > match the value of drm_connector->ycbcr_420_allowed. > > Suggested-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com> > --- > drivers/gpu/drm/drm_connector.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c > index fc35f47e2849ed6786d6223ac9c69e1c359fc648..ca7f43c8d6f1b31ef9d3a1ee05f4df930ecffac4 100644 > --- a/drivers/gpu/drm/drm_connector.c > +++ b/drivers/gpu/drm/drm_connector.c > @@ -507,6 +507,9 @@ int drmm_connector_hdmi_init(struct drm_device *dev, > if (!supported_formats || !(supported_formats & BIT(HDMI_COLORSPACE_RGB))) > return -EINVAL; > > + if (connector->ycbcr_420_allowed != !!(supported_formats & BIT(HDMI_COLORSPACE_YUV420))) > + return -EINVAL; > + > if (!(max_bpc == 8 || max_bpc == 10 || max_bpc == 12)) > return -EINVAL; The patch looks fine to me, but we need to have unit tests to cover this case. Maxime [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 273 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/2] drm/connector: hdmi: Validate supported_formats matches ycbcr_420_allowed 2024-12-17 15:25 ` Maxime Ripard @ 2024-12-24 18:35 ` Cristian Ciocaltea 0 siblings, 0 replies; 10+ messages in thread From: Cristian Ciocaltea @ 2024-12-24 18:35 UTC (permalink / raw) To: Maxime Ripard Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Thomas Zimmermann, David Airlie, Simona Vetter, Dmitry Baryshkov, kernel, dri-devel, linux-kernel On 12/17/24 5:25 PM, Maxime Ripard wrote: > On Tue, Dec 17, 2024 at 12:54:08AM +0200, Cristian Ciocaltea wrote: >> Ensure HDMI connector initialization fails when the presence of >> HDMI_COLORSPACE_YUV420 in the given supported_formats bitmask doesn't >> match the value of drm_connector->ycbcr_420_allowed. >> >> Suggested-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com> >> --- >> drivers/gpu/drm/drm_connector.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c >> index fc35f47e2849ed6786d6223ac9c69e1c359fc648..ca7f43c8d6f1b31ef9d3a1ee05f4df930ecffac4 100644 >> --- a/drivers/gpu/drm/drm_connector.c >> +++ b/drivers/gpu/drm/drm_connector.c >> @@ -507,6 +507,9 @@ int drmm_connector_hdmi_init(struct drm_device *dev, >> if (!supported_formats || !(supported_formats & BIT(HDMI_COLORSPACE_RGB))) >> return -EINVAL; >> >> + if (connector->ycbcr_420_allowed != !!(supported_formats & BIT(HDMI_COLORSPACE_YUV420))) >> + return -EINVAL; >> + >> if (!(max_bpc == 8 || max_bpc == 10 || max_bpc == 12)) >> return -EINVAL; > > The patch looks fine to me, but we need to have unit tests to cover this case. Unit tests added in v4: https://lore.kernel.org/lkml/20241224-bridge-conn-fmt-prio-v4-4-a9ceb5671379@collabora.com/ Thanks, Cristian ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2024-12-24 18:37 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-12-16 22:54 [PATCH v3 0/2] Fix ycbcr_420_allowed inconsistency for HDMI bridges Cristian Ciocaltea 2024-12-16 22:54 ` [PATCH v3 1/2] drm/bridge-connector: Prioritize supported_formats over ycbcr_420_allowed Cristian Ciocaltea 2024-12-16 23:27 ` Dmitry Baryshkov 2024-12-17 0:38 ` Cristian Ciocaltea 2024-12-20 1:05 ` Dmitry Baryshkov 2024-12-24 18:37 ` Cristian Ciocaltea 2024-12-16 22:54 ` [PATCH v3 2/2] drm/connector: hdmi: Validate supported_formats matches ycbcr_420_allowed Cristian Ciocaltea 2024-12-16 23:26 ` Dmitry Baryshkov 2024-12-17 15:25 ` Maxime Ripard 2024-12-24 18:35 ` Cristian Ciocaltea
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®