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@xxxxxxxxxxxxxxx> 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@xxxxxxxxx> wrote: > > > > > > > > On Tue, 28 Feb 2023 09:53:47 +0000 > > > > Simon Ser <contact@xxxxxxxxxxx> wrote: > > > > > > > > > On Tuesday, February 28th, 2023 at 09:46, Pekka Paalanen <ppaalanen@xxxxxxxxx> 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. -- Ville Syrjälä Intel
