| Message ID | 20230227122108.117279-1-contact@emersion.fr (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm: document TV margin properties | expand |
On Mon, Feb 27, 2023 at 12:21:16PM +0000, Simon Ser wrote: > Add docs for "{left,right,top,bottom} margin" properties. > > Signed-off-by: Simon Ser <contact@emersion.fr> > Cc: Maxime Ripard <maxime@cerno.tech> > Cc: Daniel Vetter <daniel@ffwll.ch> > Cc: Pekka Paalanen <ppaalanen@gmail.com> > Cc: Noralf Trønnes <noralf@tronnes.org> > Cc: Mateusz Kwiatkowski <kfyatek+publicgit@gmail.com> Acked-by: Maxime Ripard <maxime@cerno.tech> Maxime
On Mon, 27 Feb 2023 at 12:21, Simon Ser <contact@emersion.fr> wrote: > > Add docs for "{left,right,top,bottom} margin" properties. > > Signed-off-by: Simon Ser <contact@emersion.fr> > Cc: Maxime Ripard <maxime@cerno.tech> > Cc: Daniel Vetter <daniel@ffwll.ch> > Cc: Pekka Paalanen <ppaalanen@gmail.com> > Cc: Noralf Trønnes <noralf@tronnes.org> > Cc: Mateusz Kwiatkowski <kfyatek+publicgit@gmail.com> This certainly matches my understanding of the properties. Thanks. Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com> > --- > drivers/gpu/drm/drm_connector.c | 14 ++++++++++---- > 1 file changed, 10 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c > index 9d0250c28e9b..65a586680940 100644 > --- a/drivers/gpu/drm/drm_connector.c > +++ b/drivers/gpu/drm/drm_connector.c > @@ -1590,10 +1590,6 @@ EXPORT_SYMBOL(drm_connector_attach_dp_subconnector_property); > > /* > * TODO: Document the properties: > - * - left margin > - * - right margin > - * - top margin > - * - bottom margin > * - brightness > * - contrast > * - flicker reduction > @@ -1651,6 +1647,16 @@ EXPORT_SYMBOL(drm_connector_attach_dp_subconnector_property); > * > * Drivers can set up this property by calling > * drm_mode_create_tv_properties(). > + * > + * left margin, right margin, top margin, bottom margin: > + * Add margins to the connector's viewport. > + * > + * The value is the size in pixels of the black border which will be > + * added. The attached CRTC's content will be scaled to fill the whole > + * area inside the margin. > + * > + * Drivers can set up these properties by calling > + * drm_mode_create_tv_margin_properties(). > */ > > /** > -- > 2.39.2 > >
On Mon, 27 Feb 2023 12:21:16 +0000 Simon Ser <contact@emersion.fr> wrote: > Add docs for "{left,right,top,bottom} margin" properties. > > Signed-off-by: Simon Ser <contact@emersion.fr> > Cc: Maxime Ripard <maxime@cerno.tech> > Cc: Daniel Vetter <daniel@ffwll.ch> > Cc: Pekka Paalanen <ppaalanen@gmail.com> > Cc: Noralf Trønnes <noralf@tronnes.org> > Cc: Mateusz Kwiatkowski <kfyatek+publicgit@gmail.com> > --- > drivers/gpu/drm/drm_connector.c | 14 ++++++++++---- > 1 file changed, 10 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c > index 9d0250c28e9b..65a586680940 100644 > --- a/drivers/gpu/drm/drm_connector.c > +++ b/drivers/gpu/drm/drm_connector.c > @@ -1590,10 +1590,6 @@ EXPORT_SYMBOL(drm_connector_attach_dp_subconnector_property); > > /* > * TODO: Document the properties: > - * - left margin > - * - right margin > - * - top margin > - * - bottom margin > * - brightness > * - contrast > * - flicker reduction > @@ -1651,6 +1647,16 @@ EXPORT_SYMBOL(drm_connector_attach_dp_subconnector_property); > * > * Drivers can set up this property by calling > * drm_mode_create_tv_properties(). > + * > + * left margin, right margin, top margin, bottom margin: > + * Add margins to the connector's viewport. > + * > + * The value is the size in pixels of the black border which will be > + * added. The attached CRTC's content will be scaled to fill the whole > + * area inside the margin. > + * > + * Drivers can set up these properties by calling > + * drm_mode_create_tv_margin_properties(). > */ > > /** Hi Simon, can these be negative as well, to achieve overscan and not just underscan? Did I get overscan and underscan right... these are related to under/overscan, aren't they? Hmm, no, I guess that doesn't make sense, there is no room in the signal to have negative margins, it would result in clipping the framebuffer while scaling up. So this can only be used to scale framebuffer *down*, add borders, and the TV then scales it back up again? Looks like neither my Intel nor AMD cards support these, I don't see the properties. More things to the list of KMS properties Weston needs to explicitly control. Oh, it seems vc4 exclusive for now. Where does this text appear in the HTML kernel docs? I tried to look at drm_connector.c but I cannot find the spot where this patch applies. Is this actually a connector property? How does that work, should this not be a CRTC property? Is this instead not scaling anything but simply sending metadata through the connector? Or are there underlying requirements that this connector property is actually affecting the CRTC, which means that it is fundamentally impossible to use multiple connectors with different values on the same CRTC? And drivers will reject any attempt, so there is no need to define what conflicting settings will do? IOW, does simply the existence of these properties on a connector guarantee that the connector must be the only one on a CRTC? Thanks, pq
On Tuesday, February 28th, 2023 at 09:46, Pekka Paalanen <ppaalanen@gmail.com> wrote: > can these be negative as well, to achieve overscan and not just > underscan? Did I get overscan and underscan right... these are related > to under/overscan, aren't they? > > Hmm, no, I guess that doesn't make sense, there is no room in the > signal to have negative margins, it would result in clipping the > framebuffer while scaling up. So this can only be used to scale > framebuffer down, add borders, and the TV then scales it back up > again? Correct. > Looks like neither my Intel nor AMD cards support these, I don't see > the properties. More things to the list of KMS properties Weston needs > to explicitly control. Oh, it seems vc4 exclusive for now. i915 does support it but for TV connectors only (i915/display/intel_tv.c). gud also supports it. > Where does this text appear in the HTML kernel docs? I tried to look at > drm_connector.c but I cannot find the spot where this patch applies. Here: https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#analog-tv-specific-connector-properties > Is this actually a connector property? How does that work, should this > not be a CRTC property? Yeah, it's a connector property for some reason. > Is this instead not scaling anything but simply sending metadata > through the connector? No metadata is sent. This is purely equivalent to setting up CRTC_* properties to scale the planes. > Or are there underlying requirements that this connector property is > actually affecting the CRTC, which means that it is fundamentally > impossible to use multiple connectors with different values on the same > CRTC? And drivers will reject any attempt, so there is no need to > define what conflicting settings will do? I don't think any driver above supports cloning CRTCs for these connector types. i915 sets clonable = false for these connectors. vc4 picks the first connector's TV margins, always. > IOW, does simply the existence of these properties on a connector > guarantee that the connector must be the only one on a CRTC? I suppose that there could exist some hardware capable of applying margins post-CRTC? Such driver could support cloning CRTCs and still applying the connector margins correctly.
On Tue, 28 Feb 2023 09:53:47 +0000 Simon Ser <contact@emersion.fr> wrote: > On Tuesday, February 28th, 2023 at 09:46, Pekka Paalanen <ppaalanen@gmail.com> wrote: > > > can these be negative as well, to achieve overscan and not just > > underscan? Did I get overscan and underscan right... these are related > > to under/overscan, aren't they? > > > > Hmm, no, I guess that doesn't make sense, there is no room in the > > signal to have negative margins, it would result in clipping the > > framebuffer while scaling up. So this can only be used to scale > > framebuffer down, add borders, and the TV then scales it back up > > again? > > Correct. > > > Looks like neither my Intel nor AMD cards support these, I don't see > > the properties. More things to the list of KMS properties Weston needs > > to explicitly control. Oh, it seems vc4 exclusive for now. > > i915 does support it but for TV connectors only (i915/display/intel_tv.c). > gud also supports it. > > > Where does this text appear in the HTML kernel docs? I tried to look at > > drm_connector.c but I cannot find the spot where this patch applies. > > Here: > https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#analog-tv-specific-connector-properties Analog TV properties? So this does not apply to e.g. HDMI? I believe HDMI TVs do have the problems that margins could mitigate. > > Is this actually a connector property? How does that work, should this > > not be a CRTC property? > > Yeah, it's a connector property for some reason. > > > Is this instead not scaling anything but simply sending metadata > > through the connector? > > No metadata is sent. This is purely equivalent to setting up CRTC_* > properties to scale the planes. Oh! That would be really good to mention in the doc. Maybe even prefer plane props over this? Or is this for analog TV, and plane props for digital TV? The above are the important comments. All below is just musings you can ignore if you wish. > > Or are there underlying requirements that this connector property is > > actually affecting the CRTC, which means that it is fundamentally > > impossible to use multiple connectors with different values on the same > > CRTC? And drivers will reject any attempt, so there is no need to > > define what conflicting settings will do? > > I don't think any driver above supports cloning CRTCs for these > connector types. i915 sets clonable = false for these connectors. > vc4 picks the first connector's TV margins, always. Sounds like i915 does it right, and vc4 does not, assuming vc4 does not prevent cloning. > > > IOW, does simply the existence of these properties on a connector > > guarantee that the connector must be the only one on a CRTC? > > I suppose that there could exist some hardware capable of applying > margins post-CRTC? Such driver could support cloning CRTCs and still > applying the connector margins correctly. Yeah, theoretically. But in the KMS object model, is there the idea that connectors do not do image manipulation, they can only convert the signal type at most and send metadata? Thanks, pq
On Tue, Feb 28, 2023 at 12:12:22PM +0200, Pekka Paalanen wrote: > On Tue, 28 Feb 2023 09:53:47 +0000 > Simon Ser <contact@emersion.fr> wrote: > > > On Tuesday, February 28th, 2023 at 09:46, Pekka Paalanen <ppaalanen@gmail.com> wrote: > > > > > can these be negative as well, to achieve overscan and not just > > > underscan? Did I get overscan and underscan right... these are related > > > to under/overscan, aren't they? > > > > > > Hmm, no, I guess that doesn't make sense, there is no room in the > > > signal to have negative margins, it would result in clipping the > > > framebuffer while scaling up. So this can only be used to scale > > > framebuffer down, add borders, and the TV then scales it back up > > > again? > > > > Correct. > > > > > Looks like neither my Intel nor AMD cards support these, I don't see > > > the properties. More things to the list of KMS properties Weston needs > > > to explicitly control. Oh, it seems vc4 exclusive for now. > > > > i915 does support it but for TV connectors only (i915/display/intel_tv.c). I also have some patches to add it for HDMI and DP on i915. But those are a bit stalled due to more important stuff taking up my time. Some fairly old version here: https://github.com/vsyrjala/linux.git hdmi_margins_3 > > > Is this instead not scaling anything but simply sending metadata > > > through the connector? > > > > No metadata is sent. This is purely equivalent to setting up CRTC_* > > properties to scale the planes. My wip HDMI/DP patches do set the AVI inforame "bars" based on this. I think vc4 is already doing that as well. > > Oh! That would be really good to mention in the doc. Maybe even prefer > plane props over this? Or is this for analog TV, and plane props for > digital TV? Plane properties would be pointless for this. CRTC properties might make sense. But what is more accurate kinda depends on the hardware design. > The above are the important comments. All below is just musings you can > ignore if you wish. > > > > Or are there underlying requirements that this connector property is > > > actually affecting the CRTC, which means that it is fundamentally > > > impossible to use multiple connectors with different values on the same > > > CRTC? And drivers will reject any attempt, so there is no need to > > > define what conflicting settings will do? > > > > I don't think any driver above supports cloning CRTCs for these > > connector types. i915 sets clonable = false for these connectors. > > vc4 picks the first connector's TV margins, always. > > Sounds like i915 does it right, and vc4 does not, assuming vc4 does not > prevent cloning. For i915 my plan was to reject even HDMI+HDMI/VGA cloning (which otherwise is allowed) when these properties are set. And that's because they are connector properties and thus could disagree between the cloned connectors. If they were CRTC properties that would work fine. The main reason i915 rejects cloning with many output types is that generating the correct clock for each output becomes difficult/impossible. And on HSW+ cloning is no longer supported by the hardware at all. > > > > > > IOW, does simply the existence of these properties on a connector > > > guarantee that the connector must be the only one on a CRTC? > > > > I suppose that there could exist some hardware capable of applying > > margins post-CRTC? Such driver could support cloning CRTCs and still > > applying the connector margins correctly. > > Yeah, theoretically. But in the KMS object model, is there the idea > that connectors do not do image manipulation, they can only convert the > signal type at most and send metadata? No such rule. Some hardware has scalers and all kinds of fancy stuff in the encoder essentially. Quite common in old TV encoder chips. That's pretty much where these properties came from I think. And eDP/LVDS/etc. also do scaling in the connector in the current model since that's where the 'scaling mode' property lives.
Hi Pekka On Tue, 28 Feb 2023 at 10:12, Pekka Paalanen <ppaalanen@gmail.com> wrote: > > On Tue, 28 Feb 2023 09:53:47 +0000 > Simon Ser <contact@emersion.fr> wrote: > > > On Tuesday, February 28th, 2023 at 09:46, Pekka Paalanen <ppaalanen@gmail.com> wrote: > > > > > can these be negative as well, to achieve overscan and not just > > > underscan? Did I get overscan and underscan right... these are related > > > to under/overscan, aren't they? > > > > > > Hmm, no, I guess that doesn't make sense, there is no room in the > > > signal to have negative margins, it would result in clipping the > > > framebuffer while scaling up. So this can only be used to scale > > > framebuffer down, add borders, and the TV then scales it back up > > > again? > > > > Correct. > > > > > Looks like neither my Intel nor AMD cards support these, I don't see > > > the properties. More things to the list of KMS properties Weston needs > > > to explicitly control. Oh, it seems vc4 exclusive for now. I've started a discussion with Simon with regard overscan handling [1]. There would be nothing stopping Weston ignoring the DRM properties if Weston/userspace provides a way to reduce the destintation rectangle on the display device. Using that would also be renderer agnostic. [1] https://gitlab.freedesktop.org/wlroots/wlroots/-/issues/3597 > > i915 does support it but for TV connectors only (i915/display/intel_tv.c). > > gud also supports it. > > > > > Where does this text appear in the HTML kernel docs? I tried to look at > > > drm_connector.c but I cannot find the spot where this patch applies. > > > > Here: > > https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#analog-tv-specific-connector-properties > > Analog TV properties? So this does not apply to e.g. HDMI? > > I believe HDMI TVs do have the problems that margins could mitigate. Correct. Particularly on TVs instead of monitors, it's not uncommon to find the HDMI output gets overscanned. > > > Is this actually a connector property? How does that work, should this > > > not be a CRTC property? > > > > Yeah, it's a connector property for some reason. > > > > > Is this instead not scaling anything but simply sending metadata > > > through the connector? > > > > No metadata is sent. This is purely equivalent to setting up CRTC_* > > properties to scale the planes. > > Oh! That would be really good to mention in the doc. Maybe even prefer > plane props over this? Or is this for analog TV, and plane props for > digital TV? > > > The above are the important comments. All below is just musings you can > ignore if you wish. > > > > Or are there underlying requirements that this connector property is > > > actually affecting the CRTC, which means that it is fundamentally > > > impossible to use multiple connectors with different values on the same > > > CRTC? And drivers will reject any attempt, so there is no need to > > > define what conflicting settings will do? > > > > I don't think any driver above supports cloning CRTCs for these > > connector types. i915 sets clonable = false for these connectors. > > vc4 picks the first connector's TV margins, always. > > Sounds like i915 does it right, and vc4 does not, assuming vc4 does not > prevent cloning. The cloneable flag is in struct intel_encoder, not core. vc4 doesn't support cloning of a single CRTC to multiple connectors at all, and I believe sets up the possible_crtc bitmasks correctly to denote that. Dave > > > > > IOW, does simply the existence of these properties on a connector > > > guarantee that the connector must be the only one on a CRTC? > > > > I suppose that there could exist some hardware capable of applying > > margins post-CRTC? Such driver could support cloning CRTCs and still > > applying the connector margins correctly. > > Yeah, theoretically. But in the KMS object model, is there the idea > that connectors do not do image manipulation, they can only convert the > signal type at most and send metadata? > > > Thanks, > pq
On Tue, Feb 28, 2023 at 11:37:38AM +0000, Dave Stevenson wrote: > Hi Pekka > > On Tue, 28 Feb 2023 at 10:12, Pekka Paalanen <ppaalanen@gmail.com> wrote: > > > > On Tue, 28 Feb 2023 09:53:47 +0000 > > Simon Ser <contact@emersion.fr> wrote: > > > > > On Tuesday, February 28th, 2023 at 09:46, Pekka Paalanen <ppaalanen@gmail.com> wrote: > > > > > > > can these be negative as well, to achieve overscan and not just > > > > underscan? Did I get overscan and underscan right... these are related > > > > to under/overscan, aren't they? > > > > > > > > Hmm, no, I guess that doesn't make sense, there is no room in the > > > > signal to have negative margins, it would result in clipping the > > > > framebuffer while scaling up. So this can only be used to scale > > > > framebuffer down, add borders, and the TV then scales it back up > > > > again? > > > > > > Correct. > > > > > > > Looks like neither my Intel nor AMD cards support these, I don't see > > > > the properties. More things to the list of KMS properties Weston needs > > > > to explicitly control. Oh, it seems vc4 exclusive for now. > > I've started a discussion with Simon with regard overscan handling [1]. > There would be nothing stopping Weston ignoring the DRM properties if > Weston/userspace provides a way to reduce the destintation rectangle > on the display device. Using that would also be renderer agnostic. > > [1] https://gitlab.freedesktop.org/wlroots/wlroots/-/issues/3597 > > > > i915 does support it but for TV connectors only (i915/display/intel_tv.c). > > > gud also supports it. > > > > > > > Where does this text appear in the HTML kernel docs? I tried to look at > > > > drm_connector.c but I cannot find the spot where this patch applies. > > > > > > Here: > > > https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#analog-tv-specific-connector-properties > > > > Analog TV properties? So this does not apply to e.g. HDMI? > > > > I believe HDMI TVs do have the problems that margins could mitigate. > > Correct. Particularly on TVs instead of monitors, it's not uncommon to > find the HDMI output gets overscanned. > > > > > Is this actually a connector property? How does that work, should this > > > > not be a CRTC property? > > > > > > Yeah, it's a connector property for some reason. > > > > > > > Is this instead not scaling anything but simply sending metadata > > > > through the connector? > > > > > > No metadata is sent. This is purely equivalent to setting up CRTC_* > > > properties to scale the planes. > > > > Oh! That would be really good to mention in the doc. Maybe even prefer > > plane props over this? Or is this for analog TV, and plane props for > > digital TV? > > > > > > The above are the important comments. All below is just musings you can > > ignore if you wish. > > > > > > Or are there underlying requirements that this connector property is > > > > actually affecting the CRTC, which means that it is fundamentally > > > > impossible to use multiple connectors with different values on the same > > > > CRTC? And drivers will reject any attempt, so there is no need to > > > > define what conflicting settings will do? > > > > > > I don't think any driver above supports cloning CRTCs for these > > > connector types. i915 sets clonable = false for these connectors. > > > vc4 picks the first connector's TV margins, always. > > > > Sounds like i915 does it right, and vc4 does not, assuming vc4 does not > > prevent cloning. > > The cloneable flag is in struct intel_encoder, not core. It gets converted into the core thing by intel_encoder_possible_clones().
On Tue, 28 Feb 2023 at 11:45, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > > On Tue, Feb 28, 2023 at 11:37:38AM +0000, Dave Stevenson wrote: > > Hi Pekka > > > > On Tue, 28 Feb 2023 at 10:12, Pekka Paalanen <ppaalanen@gmail.com> wrote: > > > > > > On Tue, 28 Feb 2023 09:53:47 +0000 > > > Simon Ser <contact@emersion.fr> wrote: > > > > > > > On Tuesday, February 28th, 2023 at 09:46, Pekka Paalanen <ppaalanen@gmail.com> wrote: > > > > > > > > > can these be negative as well, to achieve overscan and not just > > > > > underscan? Did I get overscan and underscan right... these are related > > > > > to under/overscan, aren't they? > > > > > > > > > > Hmm, no, I guess that doesn't make sense, there is no room in the > > > > > signal to have negative margins, it would result in clipping the > > > > > framebuffer while scaling up. So this can only be used to scale > > > > > framebuffer down, add borders, and the TV then scales it back up > > > > > again? > > > > > > > > Correct. > > > > > > > > > Looks like neither my Intel nor AMD cards support these, I don't see > > > > > the properties. More things to the list of KMS properties Weston needs > > > > > to explicitly control. Oh, it seems vc4 exclusive for now. > > > > I've started a discussion with Simon with regard overscan handling [1]. > > There would be nothing stopping Weston ignoring the DRM properties if > > Weston/userspace provides a way to reduce the destintation rectangle > > on the display device. Using that would also be renderer agnostic. > > > > [1] https://gitlab.freedesktop.org/wlroots/wlroots/-/issues/3597 > > > > > > i915 does support it but for TV connectors only (i915/display/intel_tv.c). > > > > gud also supports it. > > > > > > > > > Where does this text appear in the HTML kernel docs? I tried to look at > > > > > drm_connector.c but I cannot find the spot where this patch applies. > > > > > > > > Here: > > > > https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#analog-tv-specific-connector-properties > > > > > > Analog TV properties? So this does not apply to e.g. HDMI? > > > > > > I believe HDMI TVs do have the problems that margins could mitigate. > > > > Correct. Particularly on TVs instead of monitors, it's not uncommon to > > find the HDMI output gets overscanned. > > > > > > > Is this actually a connector property? How does that work, should this > > > > > not be a CRTC property? > > > > > > > > Yeah, it's a connector property for some reason. > > > > > > > > > Is this instead not scaling anything but simply sending metadata > > > > > through the connector? > > > > > > > > No metadata is sent. This is purely equivalent to setting up CRTC_* > > > > properties to scale the planes. > > > > > > Oh! That would be really good to mention in the doc. Maybe even prefer > > > plane props over this? Or is this for analog TV, and plane props for > > > digital TV? > > > > > > > > > The above are the important comments. All below is just musings you can > > > ignore if you wish. > > > > > > > > Or are there underlying requirements that this connector property is > > > > > actually affecting the CRTC, which means that it is fundamentally > > > > > impossible to use multiple connectors with different values on the same > > > > > CRTC? And drivers will reject any attempt, so there is no need to > > > > > define what conflicting settings will do? > > > > > > > > I don't think any driver above supports cloning CRTCs for these > > > > connector types. i915 sets clonable = false for these connectors. > > > > vc4 picks the first connector's TV margins, always. > > > > > > Sounds like i915 does it right, and vc4 does not, assuming vc4 does not > > > prevent cloning. > > > > The cloneable flag is in struct intel_encoder, not core. > > It gets converted into the core thing by intel_encoder_possible_clones(). Thanks Ville. vc4 never adds additional possible_clones, therefore I believe it is still doing the right thing. Dave
On Tue, Feb 28, 2023 at 11:52:54AM +0000, Dave Stevenson wrote: > On Tue, 28 Feb 2023 at 11:45, Ville Syrjälä > <ville.syrjala@linux.intel.com> wrote: > > > > On Tue, Feb 28, 2023 at 11:37:38AM +0000, Dave Stevenson wrote: > > > Hi Pekka > > > > > > On Tue, 28 Feb 2023 at 10:12, Pekka Paalanen <ppaalanen@gmail.com> wrote: > > > > > > > > On Tue, 28 Feb 2023 09:53:47 +0000 > > > > Simon Ser <contact@emersion.fr> wrote: > > > > > > > > > On Tuesday, February 28th, 2023 at 09:46, Pekka Paalanen <ppaalanen@gmail.com> wrote: > > > > > > > > > > > can these be negative as well, to achieve overscan and not just > > > > > > underscan? Did I get overscan and underscan right... these are related > > > > > > to under/overscan, aren't they? > > > > > > > > > > > > Hmm, no, I guess that doesn't make sense, there is no room in the > > > > > > signal to have negative margins, it would result in clipping the > > > > > > framebuffer while scaling up. So this can only be used to scale > > > > > > framebuffer down, add borders, and the TV then scales it back up > > > > > > again? > > > > > > > > > > Correct. > > > > > > > > > > > Looks like neither my Intel nor AMD cards support these, I don't see > > > > > > the properties. More things to the list of KMS properties Weston needs > > > > > > to explicitly control. Oh, it seems vc4 exclusive for now. > > > > > > I've started a discussion with Simon with regard overscan handling [1]. > > > There would be nothing stopping Weston ignoring the DRM properties if > > > Weston/userspace provides a way to reduce the destintation rectangle > > > on the display device. Using that would also be renderer agnostic. > > > > > > [1] https://gitlab.freedesktop.org/wlroots/wlroots/-/issues/3597 > > > > > > > > i915 does support it but for TV connectors only (i915/display/intel_tv.c). > > > > > gud also supports it. > > > > > > > > > > > Where does this text appear in the HTML kernel docs? I tried to look at > > > > > > drm_connector.c but I cannot find the spot where this patch applies. > > > > > > > > > > Here: > > > > > https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#analog-tv-specific-connector-properties > > > > > > > > Analog TV properties? So this does not apply to e.g. HDMI? > > > > > > > > I believe HDMI TVs do have the problems that margins could mitigate. > > > > > > Correct. Particularly on TVs instead of monitors, it's not uncommon to > > > find the HDMI output gets overscanned. > > > > > > > > > Is this actually a connector property? How does that work, should this > > > > > > not be a CRTC property? > > > > > > > > > > Yeah, it's a connector property for some reason. > > > > > > > > > > > Is this instead not scaling anything but simply sending metadata > > > > > > through the connector? > > > > > > > > > > No metadata is sent. This is purely equivalent to setting up CRTC_* > > > > > properties to scale the planes. > > > > > > > > Oh! That would be really good to mention in the doc. Maybe even prefer > > > > plane props over this? Or is this for analog TV, and plane props for > > > > digital TV? > > > > > > > > > > > > The above are the important comments. All below is just musings you can > > > > ignore if you wish. > > > > > > > > > > Or are there underlying requirements that this connector property is > > > > > > actually affecting the CRTC, which means that it is fundamentally > > > > > > impossible to use multiple connectors with different values on the same > > > > > > CRTC? And drivers will reject any attempt, so there is no need to > > > > > > define what conflicting settings will do? > > > > > > > > > > I don't think any driver above supports cloning CRTCs for these > > > > > connector types. i915 sets clonable = false for these connectors. > > > > > vc4 picks the first connector's TV margins, always. > > > > > > > > Sounds like i915 does it right, and vc4 does not, assuming vc4 does not > > > > prevent cloning. > > > > > > The cloneable flag is in struct intel_encoder, not core. > > > > It gets converted into the core thing by intel_encoder_possible_clones(). > > Thanks Ville. > vc4 never adds additional possible_clones, therefore I believe it is > still doing the right thing. Yeah, should be fine. These days drivers can just leave/set possible_clones=0 if they don't support cloning, and fixup_encoder_possible_clones() will fix it up later to follow the rule that the encoder itself must be included in the bitmask.
On Tuesday, February 28th, 2023 at 12:28, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > > > > Is this instead not scaling anything but simply sending metadata > > > > through the connector? > > > > > > No metadata is sent. This is purely equivalent to setting up CRTC_* > > > properties to scale the planes. > > My wip HDMI/DP patches do set the AVI inforame "bars" based on > this. I think vc4 is already doing that as well. Eh. Indeed it does, via drm_hdmi_avi_infoframe_bars().
On Tue, 28 Feb 2023 13:28:48 +0200 Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > On Tue, Feb 28, 2023 at 12:12:22PM +0200, Pekka Paalanen wrote: > > Oh! That would be really good to mention in the doc. Maybe even prefer > > plane props over this? Or is this for analog TV, and plane props for > > digital TV? > > Plane properties would be pointless for this. CRTC properties might > make sense. But what is more accurate kinda depends on the hardware > design. I meant the existing plane properties CRTC_X,Y,W,H. They can already describe e.g. a primary plane that does not cover the whole CRTC area, which is essentially the same as margins, scaling included even. > Some hardware has scalers and all kinds of fancy stuff in the > encoder essentially. Quite common in old TV encoder chips. > That's pretty much where these properties came from I think. > > And eDP/LVDS/etc. also do scaling in the connector in the > current model since that's where the 'scaling mode' property > lives. Ok, that makes sense. Thanks, pq
On Tue, Feb 28, 2023 at 02:24:23PM +0200, Pekka Paalanen wrote: > On Tue, 28 Feb 2023 13:28:48 +0200 > Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > > > On Tue, Feb 28, 2023 at 12:12:22PM +0200, Pekka Paalanen wrote: > > > > Oh! That would be really good to mention in the doc. Maybe even prefer > > > plane props over this? Or is this for analog TV, and plane props for > > > digital TV? > > > > Plane properties would be pointless for this. CRTC properties might > > make sense. But what is more accurate kinda depends on the hardware > > design. > > I meant the existing plane properties CRTC_X,Y,W,H. They can already > describe e.g. a primary plane that does not cover the whole CRTC area, > which is essentially the same as margins, scaling included even. Yeah that can work, assuming you have hardware that supports it.
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 9d0250c28e9b..65a586680940 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -1590,10 +1590,6 @@ EXPORT_SYMBOL(drm_connector_attach_dp_subconnector_property); /* * TODO: Document the properties: - * - left margin - * - right margin - * - top margin - * - bottom margin * - brightness * - contrast * - flicker reduction @@ -1651,6 +1647,16 @@ EXPORT_SYMBOL(drm_connector_attach_dp_subconnector_property); * * Drivers can set up this property by calling * drm_mode_create_tv_properties(). + * + * left margin, right margin, top margin, bottom margin: + * Add margins to the connector's viewport. + * + * The value is the size in pixels of the black border which will be + * added. The attached CRTC's content will be scaled to fill the whole + * area inside the margin. + * + * Drivers can set up these properties by calling + * drm_mode_create_tv_margin_properties(). */ /**
Add docs for "{left,right,top,bottom} margin" properties. Signed-off-by: Simon Ser <contact@emersion.fr> Cc: Maxime Ripard <maxime@cerno.tech> Cc: Daniel Vetter <daniel@ffwll.ch> Cc: Pekka Paalanen <ppaalanen@gmail.com> Cc: Noralf Trønnes <noralf@tronnes.org> Cc: Mateusz Kwiatkowski <kfyatek+publicgit@gmail.com> --- drivers/gpu/drm/drm_connector.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)