Message ID | 20230525191809.3524-1-harry.wentland@amd.com (mailing list archive) |
---|---|
Headers | show |
Series | Enable Colorspace connector property in amdgpu | expand |
On Thu, 25 May 2023 15:17:56 -0400 Harry Wentland <harry.wentland@amd.com> wrote: > This patchset is based on Joshua's previous patchset [1], as well > as my previous patchset [2]. > > It is > - enabling support for the colorspace property in amdgpu, as well as > - allowing drivers to specify the supported set of colorspaces, and > > Colorspace, Infoframes, and YCbCr matrix > --------------------------------------- > > Even though the initial intent of the colorspace property was to set the > colorspace field in the respective HDMI AVI and DP SDP infoframes that > is not sufficient in all scenarios. For DP the colorspace information > also affects the MSA (main stream attribute) packet. For YUV output the > colorspace affects the RGB-to-YCbCr conversion matrix. The colorspace > field of the infopackets also depends on the encoding used, which is > something that is decided by the driver and not known to userspace. Hi Harry, the "deprecation" of BT2020 RGB vs. YCC is now dropped completely. Should there still be a patch that adds some UAPI documentation only, saying that drivers are free to swap e.g. between BT2020 RGB and YCC based which encoding they actually chose? Even if just BT2020 variant specifically. I have nothing against with this series now. Thanks, pq > > For these reasons a driver will need to be able to select the supported > colorspaces at property creation. > > Note: There seems to be an understanding that the colorspace property > should ONLY modify the infoframe. While this is current behavior and > sufficient in some cases it is nowhere specified that this should be the > only use of this property. As outlined above this limitation is not > going to work in all cases. > > This patchset does not affect current behavior for the drivers that > implement this property: i915 and vc4. > > In the future we might want to give userspace control over the encoding > format on the wire, in particular to avoid use of YUV420 when image > fidelity is important. This work would likely go hand in hand with a > min_bpc property and wouldn't conflict with the work done in this > patchset. I would expect this future work to tag along with a drm_crtc > or drm_connector's Color Pipeline, similar to the one propsed for > drm_plane [3]. > > Colorspace on crtc or connector? > -------------------------------- > > There have been suggestions of programming 'colorspace' on the drm_crtc > but I don't think the crtc is the right place for this property. The > drm_plane and drm_crtc will be used to offload color processing that > would normally be done via the GFX or other pipelines. The drm_connector > controls the signalling with the display and ensures the wire format is > appropriate for the encoding by programming the RGB-to-YCbCr matrix. > > [1] https://patchwork.freedesktop.org/series/113632/ > [2] https://patchwork.freedesktop.org/series/111865/ > [3] https://lists.freedesktop.org/archives/dri-devel/2023-May/403173.html
On Fri, May 26, 2023 at 3:16 PM Pekka Paalanen <ppaalanen@gmail.com> wrote: > > On Thu, 25 May 2023 15:17:56 -0400 > Harry Wentland <harry.wentland@amd.com> wrote: > > > This patchset is based on Joshua's previous patchset [1], as well > > as my previous patchset [2]. > > > > It is > > - enabling support for the colorspace property in amdgpu, as well as > > - allowing drivers to specify the supported set of colorspaces, and > > > > Colorspace, Infoframes, and YCbCr matrix > > --------------------------------------- > > > > Even though the initial intent of the colorspace property was to set the > > colorspace field in the respective HDMI AVI and DP SDP infoframes that > > is not sufficient in all scenarios. For DP the colorspace information > > also affects the MSA (main stream attribute) packet. For YUV output the > > colorspace affects the RGB-to-YCbCr conversion matrix. The colorspace > > field of the infopackets also depends on the encoding used, which is > > something that is decided by the driver and not known to userspace. > > Hi Harry, > > the "deprecation" of BT2020 RGB vs. YCC is now dropped completely. > Should there still be a patch that adds some UAPI documentation only, > saying that drivers are free to swap e.g. between BT2020 RGB and YCC > based which encoding they actually chose? Yes please! > Even if just BT2020 variant specifically. > > I have nothing against with this series now. > > > Thanks, > pq > > > > > For these reasons a driver will need to be able to select the supported > > colorspaces at property creation. > > > > Note: There seems to be an understanding that the colorspace property > > should ONLY modify the infoframe. While this is current behavior and > > sufficient in some cases it is nowhere specified that this should be the > > only use of this property. As outlined above this limitation is not > > going to work in all cases. > > > > This patchset does not affect current behavior for the drivers that > > implement this property: i915 and vc4. > > > > In the future we might want to give userspace control over the encoding > > format on the wire, in particular to avoid use of YUV420 when image > > fidelity is important. This work would likely go hand in hand with a > > min_bpc property and wouldn't conflict with the work done in this > > patchset. I would expect this future work to tag along with a drm_crtc > > or drm_connector's Color Pipeline, similar to the one propsed for > > drm_plane [3]. > > > > Colorspace on crtc or connector? > > -------------------------------- > > > > There have been suggestions of programming 'colorspace' on the drm_crtc > > but I don't think the crtc is the right place for this property. The > > drm_plane and drm_crtc will be used to offload color processing that > > would normally be done via the GFX or other pipelines. The drm_connector > > controls the signalling with the display and ensures the wire format is > > appropriate for the encoding by programming the RGB-to-YCbCr matrix. > > > > [1] https://patchwork.freedesktop.org/series/113632/ > > [2] https://patchwork.freedesktop.org/series/111865/ > > [3] https://lists.freedesktop.org/archives/dri-devel/2023-May/403173.html
With the documentation about RGB and YCC variants added the drm core patches are Reviewed-by: Sebastian Wick <sebastian.wick@redhat.com> On Fri, May 26, 2023 at 6:24 PM Sebastian Wick <sebastian.wick@redhat.com> wrote: > > On Fri, May 26, 2023 at 3:16 PM Pekka Paalanen <ppaalanen@gmail.com> wrote: > > > > On Thu, 25 May 2023 15:17:56 -0400 > > Harry Wentland <harry.wentland@amd.com> wrote: > > > > > This patchset is based on Joshua's previous patchset [1], as well > > > as my previous patchset [2]. > > > > > > It is > > > - enabling support for the colorspace property in amdgpu, as well as > > > - allowing drivers to specify the supported set of colorspaces, and > > > > > > Colorspace, Infoframes, and YCbCr matrix > > > --------------------------------------- > > > > > > Even though the initial intent of the colorspace property was to set the > > > colorspace field in the respective HDMI AVI and DP SDP infoframes that > > > is not sufficient in all scenarios. For DP the colorspace information > > > also affects the MSA (main stream attribute) packet. For YUV output the > > > colorspace affects the RGB-to-YCbCr conversion matrix. The colorspace > > > field of the infopackets also depends on the encoding used, which is > > > something that is decided by the driver and not known to userspace. > > > > Hi Harry, > > > > the "deprecation" of BT2020 RGB vs. YCC is now dropped completely. > > Should there still be a patch that adds some UAPI documentation only, > > saying that drivers are free to swap e.g. between BT2020 RGB and YCC > > based which encoding they actually chose? > > Yes please! > > > Even if just BT2020 variant specifically. > > > > I have nothing against with this series now. > > > > > > Thanks, > > pq > > > > > > > > For these reasons a driver will need to be able to select the supported > > > colorspaces at property creation. > > > > > > Note: There seems to be an understanding that the colorspace property > > > should ONLY modify the infoframe. While this is current behavior and > > > sufficient in some cases it is nowhere specified that this should be the > > > only use of this property. As outlined above this limitation is not > > > going to work in all cases. > > > > > > This patchset does not affect current behavior for the drivers that > > > implement this property: i915 and vc4. > > > > > > In the future we might want to give userspace control over the encoding > > > format on the wire, in particular to avoid use of YUV420 when image > > > fidelity is important. This work would likely go hand in hand with a > > > min_bpc property and wouldn't conflict with the work done in this > > > patchset. I would expect this future work to tag along with a drm_crtc > > > or drm_connector's Color Pipeline, similar to the one propsed for > > > drm_plane [3]. > > > > > > Colorspace on crtc or connector? > > > -------------------------------- > > > > > > There have been suggestions of programming 'colorspace' on the drm_crtc > > > but I don't think the crtc is the right place for this property. The > > > drm_plane and drm_crtc will be used to offload color processing that > > > would normally be done via the GFX or other pipelines. The drm_connector > > > controls the signalling with the display and ensures the wire format is > > > appropriate for the encoding by programming the RGB-to-YCbCr matrix. > > > > > > [1] https://patchwork.freedesktop.org/series/113632/ > > > [2] https://patchwork.freedesktop.org/series/111865/ > > > [3] https://lists.freedesktop.org/archives/dri-devel/2023-May/403173.html