| Message ID | 0187832f758eb489258212b0ec917d5013366fe5.1494347165.git.joabreu@synopsys.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, May 09, 2017 at 06:00:08PM +0100, Jose Abreu wrote: > This adds a new callback to crtc, encoder and bridge helper functions > called mode_valid(). This callback shall be implemented if the > corresponding component has some sort of restriction in the modes > that can be displayed. A NULL callback implicates that the component > can display all the modes. > > We also change the description of connector->mode_valid() callback > so that it matches the existing behaviour: It is never called in > atomic check phase. > > Only the callbacks were implemented to simplify review process, > following patches will make use of them. > > Signed-off-by: Jose Abreu <joabreu@synopsys.com> > Cc: Carlos Palminha <palminha@synopsys.com> > Cc: Alexey Brodkin <abrodkin@synopsys.com> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > Cc: Dave Airlie <airlied@linux.ie> > Cc: Andrzej Hajda <a.hajda@samsung.com> > Cc: Archit Taneja <architt@codeaurora.org> > --- > > Changes v1->v2: > - Change description of connector->mode_valid() (Daniel) > > include/drm/drm_bridge.h | 20 ++++++++++++++ > include/drm/drm_modeset_helper_vtables.h | 45 ++++++++++++++++++++++++++++++++ > 2 files changed, 65 insertions(+) > > diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h > index fdd82fc..00c6c36 100644 > --- a/include/drm/drm_bridge.h > +++ b/include/drm/drm_bridge.h > @@ -59,6 +59,26 @@ struct drm_bridge_funcs { > void (*detach)(struct drm_bridge *bridge); > > /** > + * @mode_valid: > + * > + * This callback is used to check if a specific mode is valid in this > + * bridge. This should be implemented if the bridge has some sort of > + * restriction in the modes it can display. For example, a given bridge > + * may be responsible to set a clock value. If the clock can not > + * produce all the values for the available modes then this callback > + * can be used to restrict the number of modes to only the ones that > + * can be displayed. > + * > + * This is called at mode probe and at atomic check phase. > + * > + * RETURNS: > + * > + * drm_mode_status Enum > + */ > + enum drm_mode_status (*mode_valid)(struct drm_bridge *crtc, > + const struct drm_display_mode *mode); > + > + /** > * @mode_fixup: > * > * This callback is used to validate and adjust a mode. The paramater > diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h > index c01c328..eec2c70 100644 > --- a/include/drm/drm_modeset_helper_vtables.h > +++ b/include/drm/drm_modeset_helper_vtables.h > @@ -106,6 +106,26 @@ struct drm_crtc_helper_funcs { > void (*commit)(struct drm_crtc *crtc); > > /** > + * @mode_valid: > + * > + * This callback is used to check if a specific mode is valid in this > + * crtc. This should be implemented if the crtc has some sort of > + * restriction in the modes it can display. For example, a given crtc > + * may be responsible to set a clock value. If the clock can not > + * produce all the values for the available modes then this callback > + * can be used to restrict the number of modes to only the ones that > + * can be displayed. > + * > + * This is called at mode probe and at atomic check phase. > + * > + * RETURNS: > + * > + * drm_mode_status Enum > + */ > + enum drm_mode_status (*mode_valid)(struct drm_crtc *crtc, > + const struct drm_display_mode *mode); > + > + /** > * @mode_fixup: > * > * This callback is used to validate a mode. The parameter mode is the > @@ -457,6 +477,26 @@ struct drm_encoder_helper_funcs { > void (*dpms)(struct drm_encoder *encoder, int mode); > > /** > + * @mode_valid: > + * > + * This callback is used to check if a specific mode is valid in this > + * encoder. This should be implemented if the encoder has some sort > + * of restriction in the modes it can display. For example, a given > + * encoder may be responsible to set a clock value. If the clock can > + * not produce all the values for the available modes then this callback > + * can be used to restrict the number of modes to only the ones that > + * can be displayed. > + * > + * This is called at mode probe and at atomic check phase. > + * > + * RETURNS: > + * > + * drm_mode_status Enum > + */ > + enum drm_mode_status (*mode_valid)(struct drm_encoder *crtc, > + const struct drm_display_mode *mode); > + > + /** > * @mode_fixup: > * > * This callback is used to validate and adjust a mode. The parameter > @@ -795,6 +835,11 @@ struct drm_connector_helper_funcs { > * (which is usually derived from the EDID data block from the sink). > * See e.g. drm_helper_probe_single_connector_modes(). > * > + * This callback is never called in atomic check phase so that userspace > + * can override kernel sink checks in case of broken EDID with wrong > + * limits from the sink. You can use the remaining mode_valid() > + * callbacks to validate the mode against your video path. > + * > * NOTE: > * > * This only filters the mode list supplied to userspace in the Kerneldoc review seems to still be missing. One case that needs to be updated is this note here. But there's a pile of other places where we reference one of the mode_valid or mode_fixup functions, and they should all be updated. Also, it'd be good to explain what to put into mode_valid and what to put into mode_fixup, for objects which have both. I can help with this, but I think it'd be good if you make a first round, since that might catch some interactions we've missed. Thanks, Daniel
Hi Daniel, On 10-05-2017 09:03, Daniel Vetter wrote: > On Tue, May 09, 2017 at 06:00:08PM +0100, Jose Abreu wrote: >> This adds a new callback to crtc, encoder and bridge helper functions >> called mode_valid(). This callback shall be implemented if the >> corresponding component has some sort of restriction in the modes >> that can be displayed. A NULL callback implicates that the component >> can display all the modes. >> >> We also change the description of connector->mode_valid() callback >> so that it matches the existing behaviour: It is never called in >> atomic check phase. >> >> Only the callbacks were implemented to simplify review process, >> following patches will make use of them. >> >> Signed-off-by: Jose Abreu <joabreu@synopsys.com> >> Cc: Carlos Palminha <palminha@synopsys.com> >> Cc: Alexey Brodkin <abrodkin@synopsys.com> >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> >> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> >> Cc: Dave Airlie <airlied@linux.ie> >> Cc: Andrzej Hajda <a.hajda@samsung.com> >> Cc: Archit Taneja <architt@codeaurora.org> >> --- >> >> Changes v1->v2: >> - Change description of connector->mode_valid() (Daniel) >> >> include/drm/drm_bridge.h | 20 ++++++++++++++ >> include/drm/drm_modeset_helper_vtables.h | 45 ++++++++++++++++++++++++++++++++ >> 2 files changed, 65 insertions(+) >> >> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h >> index fdd82fc..00c6c36 100644 >> --- a/include/drm/drm_bridge.h >> +++ b/include/drm/drm_bridge.h >> @@ -59,6 +59,26 @@ struct drm_bridge_funcs { >> void (*detach)(struct drm_bridge *bridge); >> >> /** >> + * @mode_valid: >> + * >> + * This callback is used to check if a specific mode is valid in this >> + * bridge. This should be implemented if the bridge has some sort of >> + * restriction in the modes it can display. For example, a given bridge >> + * may be responsible to set a clock value. If the clock can not >> + * produce all the values for the available modes then this callback >> + * can be used to restrict the number of modes to only the ones that >> + * can be displayed. >> + * >> + * This is called at mode probe and at atomic check phase. >> + * >> + * RETURNS: >> + * >> + * drm_mode_status Enum >> + */ >> + enum drm_mode_status (*mode_valid)(struct drm_bridge *crtc, >> + const struct drm_display_mode *mode); >> + >> + /** >> * @mode_fixup: >> * >> * This callback is used to validate and adjust a mode. The paramater >> diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h >> index c01c328..eec2c70 100644 >> --- a/include/drm/drm_modeset_helper_vtables.h >> +++ b/include/drm/drm_modeset_helper_vtables.h >> @@ -106,6 +106,26 @@ struct drm_crtc_helper_funcs { >> void (*commit)(struct drm_crtc *crtc); >> >> /** >> + * @mode_valid: >> + * >> + * This callback is used to check if a specific mode is valid in this >> + * crtc. This should be implemented if the crtc has some sort of >> + * restriction in the modes it can display. For example, a given crtc >> + * may be responsible to set a clock value. If the clock can not >> + * produce all the values for the available modes then this callback >> + * can be used to restrict the number of modes to only the ones that >> + * can be displayed. >> + * >> + * This is called at mode probe and at atomic check phase. >> + * >> + * RETURNS: >> + * >> + * drm_mode_status Enum >> + */ >> + enum drm_mode_status (*mode_valid)(struct drm_crtc *crtc, >> + const struct drm_display_mode *mode); >> + >> + /** >> * @mode_fixup: >> * >> * This callback is used to validate a mode. The parameter mode is the >> @@ -457,6 +477,26 @@ struct drm_encoder_helper_funcs { >> void (*dpms)(struct drm_encoder *encoder, int mode); >> >> /** >> + * @mode_valid: >> + * >> + * This callback is used to check if a specific mode is valid in this >> + * encoder. This should be implemented if the encoder has some sort >> + * of restriction in the modes it can display. For example, a given >> + * encoder may be responsible to set a clock value. If the clock can >> + * not produce all the values for the available modes then this callback >> + * can be used to restrict the number of modes to only the ones that >> + * can be displayed. >> + * >> + * This is called at mode probe and at atomic check phase. >> + * >> + * RETURNS: >> + * >> + * drm_mode_status Enum >> + */ >> + enum drm_mode_status (*mode_valid)(struct drm_encoder *crtc, >> + const struct drm_display_mode *mode); >> + >> + /** >> * @mode_fixup: >> * >> * This callback is used to validate and adjust a mode. The parameter >> @@ -795,6 +835,11 @@ struct drm_connector_helper_funcs { >> * (which is usually derived from the EDID data block from the sink). >> * See e.g. drm_helper_probe_single_connector_modes(). >> * >> + * This callback is never called in atomic check phase so that userspace >> + * can override kernel sink checks in case of broken EDID with wrong >> + * limits from the sink. You can use the remaining mode_valid() >> + * callbacks to validate the mode against your video path. >> + * >> * NOTE: >> * >> * This only filters the mode list supplied to userspace in the > Kerneldoc review seems to still be missing. One case that needs to be > updated is this note here. But there's a pile of other places where we > reference one of the mode_valid or mode_fixup functions, and they should > all be updated. > > Also, it'd be good to explain what to put into mode_valid and what to put > into mode_fixup, for objects which have both. I can help with this, but I > think it'd be good if you make a first round, since that might catch some > interactions we've missed. > > Thanks, Daniel Ok, I will need some time to review and update this. I think until the end of this week I will have another version to send. Thanks! Best regards, Jose Miguel Abreu
Hi Daniel, On Wednesday 10 May 2017 10:03:37 Daniel Vetter wrote: > On Tue, May 09, 2017 at 06:00:08PM +0100, Jose Abreu wrote: > > This adds a new callback to crtc, encoder and bridge helper functions > > called mode_valid(). This callback shall be implemented if the > > corresponding component has some sort of restriction in the modes > > that can be displayed. A NULL callback implicates that the component > > can display all the modes. > > > > We also change the description of connector->mode_valid() callback > > so that it matches the existing behaviour: It is never called in > > atomic check phase. > > > > Only the callbacks were implemented to simplify review process, > > following patches will make use of them. > > > > Signed-off-by: Jose Abreu <joabreu@synopsys.com> > > Cc: Carlos Palminha <palminha@synopsys.com> > > Cc: Alexey Brodkin <abrodkin@synopsys.com> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > > Cc: Dave Airlie <airlied@linux.ie> > > Cc: Andrzej Hajda <a.hajda@samsung.com> > > Cc: Archit Taneja <architt@codeaurora.org> > > --- > > > > Changes v1->v2: > > - Change description of connector->mode_valid() (Daniel) > > > > include/drm/drm_bridge.h | 20 ++++++++++++++ > > include/drm/drm_modeset_helper_vtables.h | 45 +++++++++++++++++++++++++++ > > 2 files changed, 65 insertions(+) > > > > diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h > > index fdd82fc..00c6c36 100644 > > --- a/include/drm/drm_bridge.h > > +++ b/include/drm/drm_bridge.h > > @@ -59,6 +59,26 @@ struct drm_bridge_funcs { > > void (*detach)(struct drm_bridge *bridge); > > > > /** > > + * @mode_valid: > > + * > > + * This callback is used to check if a specific mode is valid in this > > + * bridge. This should be implemented if the bridge has some sort of > > + * restriction in the modes it can display. For example, a given bridge > > + * may be responsible to set a clock value. If the clock can not > > + * produce all the values for the available modes then this callback > > + * can be used to restrict the number of modes to only the ones that > > + * can be displayed. > > + * > > + * This is called at mode probe and at atomic check phase. > > + * > > + * RETURNS: > > + * > > + * drm_mode_status Enum > > + */ > > + enum drm_mode_status (*mode_valid)(struct drm_bridge *crtc, > > + const struct drm_display_mode *mode); > > + > > + /** > > * @mode_fixup: > > * > > * This callback is used to validate and adjust a mode. The paramater > > diff --git a/include/drm/drm_modeset_helper_vtables.h > > b/include/drm/drm_modeset_helper_vtables.h index c01c328..eec2c70 100644 > > --- a/include/drm/drm_modeset_helper_vtables.h > > +++ b/include/drm/drm_modeset_helper_vtables.h > > @@ -106,6 +106,26 @@ struct drm_crtc_helper_funcs { > > void (*commit)(struct drm_crtc *crtc); > > > > /** > > + * @mode_valid: > > + * > > + * This callback is used to check if a specific mode is valid in this > > + * crtc. This should be implemented if the crtc has some sort of > > + * restriction in the modes it can display. For example, a given crtc > > + * may be responsible to set a clock value. If the clock can not > > + * produce all the values for the available modes then this callback > > + * can be used to restrict the number of modes to only the ones that > > + * can be displayed. > > + * > > + * This is called at mode probe and at atomic check phase. > > + * > > + * RETURNS: > > + * > > + * drm_mode_status Enum > > + */ > > + enum drm_mode_status (*mode_valid)(struct drm_crtc *crtc, > > + const struct drm_display_mode *mode); > > + > > + /** > > * @mode_fixup: > > * > > * This callback is used to validate a mode. The parameter mode is the > > @@ -457,6 +477,26 @@ struct drm_encoder_helper_funcs { > > void (*dpms)(struct drm_encoder *encoder, int mode); > > > > /** > > + * @mode_valid: > > + * > > + * This callback is used to check if a specific mode is valid in this > > + * encoder. This should be implemented if the encoder has some sort > > + * of restriction in the modes it can display. For example, a given > > + * encoder may be responsible to set a clock value. If the clock can > > + * not produce all the values for the available modes then this callback > > + * can be used to restrict the number of modes to only the ones that > > + * can be displayed. > > + * > > + * This is called at mode probe and at atomic check phase. > > + * > > + * RETURNS: > > + * > > + * drm_mode_status Enum > > + */ > > + enum drm_mode_status (*mode_valid)(struct drm_encoder *crtc, > > + const struct drm_display_mode *mode); > > + > > + /** > > * @mode_fixup: > > * > > * This callback is used to validate and adjust a mode. The parameter > > @@ -795,6 +835,11 @@ struct drm_connector_helper_funcs { > > * (which is usually derived from the EDID data block from the sink). > > * See e.g. drm_helper_probe_single_connector_modes(). > > * > > + * This callback is never called in atomic check phase so that userspace > > + * can override kernel sink checks in case of broken EDID with wrong > > + * limits from the sink. You can use the remaining mode_valid() > > + * callbacks to validate the mode against your video path. > > + * > > * NOTE: > > * > > * This only filters the mode list supplied to userspace in the > > Kerneldoc review seems to still be missing. One case that needs to be > updated is this note here. But there's a pile of other places where we > reference one of the mode_valid or mode_fixup functions, and they should > all be updated. > > Also, it'd be good to explain what to put into mode_valid and what to put > into mode_fixup, for objects which have both. I can help with this, but I > think it'd be good if you make a first round, since that might catch some > interactions we've missed. I was going to mention that. Interactions between mode_valid and mode_fixup are not defined clearly. Additionally, even though it might be a bit out of scope for this patch series, I think we should also define what mode_fixup is allowed to fix and what it should reject straight away. Thinking about it, do we really need two separate operations ? As I understand it, mode_fixup is mostly (only ? - this is where we need documentation) used to fixup the pixel clock frequency as clock generators usually have limitations in their dividers. We assume that the sync won't care too much, and happily feed it with a mode that is slightly different from what userspace requested. Given that mode_valid should accepts mode for which the exact pixel clock frequency can't bee achieved, and that the atomic commit will fixup that frequency anyway, can't we apply the same processing to modes enumerated by the connector, and merge the mode_valid and mode_fixup operations ?
On Fri, May 12, 2017 at 11:24:12AM +0300, Laurent Pinchart wrote: > Hi Daniel, > > On Wednesday 10 May 2017 10:03:37 Daniel Vetter wrote: > > On Tue, May 09, 2017 at 06:00:08PM +0100, Jose Abreu wrote: > > > This adds a new callback to crtc, encoder and bridge helper functions > > > called mode_valid(). This callback shall be implemented if the > > > corresponding component has some sort of restriction in the modes > > > that can be displayed. A NULL callback implicates that the component > > > can display all the modes. > > > > > > We also change the description of connector->mode_valid() callback > > > so that it matches the existing behaviour: It is never called in > > > atomic check phase. > > > > > > Only the callbacks were implemented to simplify review process, > > > following patches will make use of them. > > > > > > Signed-off-by: Jose Abreu <joabreu@synopsys.com> > > > Cc: Carlos Palminha <palminha@synopsys.com> > > > Cc: Alexey Brodkin <abrodkin@synopsys.com> > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > > > Cc: Dave Airlie <airlied@linux.ie> > > > Cc: Andrzej Hajda <a.hajda@samsung.com> > > > Cc: Archit Taneja <architt@codeaurora.org> > > > --- > > > > > > Changes v1->v2: > > > - Change description of connector->mode_valid() (Daniel) > > > > > > include/drm/drm_bridge.h | 20 ++++++++++++++ > > > include/drm/drm_modeset_helper_vtables.h | 45 +++++++++++++++++++++++++++ > > > 2 files changed, 65 insertions(+) > > > > > > diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h > > > index fdd82fc..00c6c36 100644 > > > --- a/include/drm/drm_bridge.h > > > +++ b/include/drm/drm_bridge.h > > > @@ -59,6 +59,26 @@ struct drm_bridge_funcs { > > > void (*detach)(struct drm_bridge *bridge); > > > > > > /** > > > + * @mode_valid: > > > + * > > > + * This callback is used to check if a specific mode is valid in this > > > + * bridge. This should be implemented if the bridge has some sort of > > > + * restriction in the modes it can display. For example, a given > bridge > > > + * may be responsible to set a clock value. If the clock can not > > > + * produce all the values for the available modes then this callback > > > + * can be used to restrict the number of modes to only the ones that > > > + * can be displayed. > > > + * > > > + * This is called at mode probe and at atomic check phase. > > > + * > > > + * RETURNS: > > > + * > > > + * drm_mode_status Enum > > > + */ > > > + enum drm_mode_status (*mode_valid)(struct drm_bridge *crtc, > > > + const struct drm_display_mode > *mode); > > > + > > > + /** > > > * @mode_fixup: > > > * > > > * This callback is used to validate and adjust a mode. The paramater > > > diff --git a/include/drm/drm_modeset_helper_vtables.h > > > b/include/drm/drm_modeset_helper_vtables.h index c01c328..eec2c70 100644 > > > --- a/include/drm/drm_modeset_helper_vtables.h > > > +++ b/include/drm/drm_modeset_helper_vtables.h > > > @@ -106,6 +106,26 @@ struct drm_crtc_helper_funcs { > > > void (*commit)(struct drm_crtc *crtc); > > > > > > /** > > > + * @mode_valid: > > > + * > > > + * This callback is used to check if a specific mode is valid in this > > > + * crtc. This should be implemented if the crtc has some sort of > > > + * restriction in the modes it can display. For example, a given crtc > > > + * may be responsible to set a clock value. If the clock can not > > > + * produce all the values for the available modes then this callback > > > + * can be used to restrict the number of modes to only the ones that > > > + * can be displayed. > > > + * > > > + * This is called at mode probe and at atomic check phase. > > > + * > > > + * RETURNS: > > > + * > > > + * drm_mode_status Enum > > > + */ > > > + enum drm_mode_status (*mode_valid)(struct drm_crtc *crtc, > > > + const struct drm_display_mode > *mode); > > > + > > > + /** > > > * @mode_fixup: > > > * > > > * This callback is used to validate a mode. The parameter mode is the > > > @@ -457,6 +477,26 @@ struct drm_encoder_helper_funcs { > > > void (*dpms)(struct drm_encoder *encoder, int mode); > > > > > > /** > > > + * @mode_valid: > > > + * > > > + * This callback is used to check if a specific mode is valid in this > > > + * encoder. This should be implemented if the encoder has some sort > > > + * of restriction in the modes it can display. For example, a given > > > + * encoder may be responsible to set a clock value. If the clock can > > > + * not produce all the values for the available modes then this > callback > > > + * can be used to restrict the number of modes to only the ones that > > > + * can be displayed. > > > + * > > > + * This is called at mode probe and at atomic check phase. > > > + * > > > + * RETURNS: > > > + * > > > + * drm_mode_status Enum > > > + */ > > > + enum drm_mode_status (*mode_valid)(struct drm_encoder *crtc, > > > + const struct drm_display_mode > *mode); > > > + > > > + /** > > > * @mode_fixup: > > > * > > > * This callback is used to validate and adjust a mode. The parameter > > > @@ -795,6 +835,11 @@ struct drm_connector_helper_funcs { > > > * (which is usually derived from the EDID data block from the sink). > > > * See e.g. drm_helper_probe_single_connector_modes(). > > > * > > > + * This callback is never called in atomic check phase so that > userspace > > > + * can override kernel sink checks in case of broken EDID with wrong > > > + * limits from the sink. You can use the remaining mode_valid() > > > + * callbacks to validate the mode against your video path. > > > + * > > > * NOTE: > > > * > > > * This only filters the mode list supplied to userspace in the > > > > Kerneldoc review seems to still be missing. One case that needs to be > > updated is this note here. But there's a pile of other places where we > > reference one of the mode_valid or mode_fixup functions, and they should > > all be updated. > > > > Also, it'd be good to explain what to put into mode_valid and what to put > > into mode_fixup, for objects which have both. I can help with this, but I > > think it'd be good if you make a first round, since that might catch some > > interactions we've missed. > > I was going to mention that. Interactions between mode_valid and mode_fixup > are not defined clearly. Additionally, even though it might be a bit out of > scope for this patch series, I think we should also define what mode_fixup is > allowed to fix and what it should reject straight away. > > Thinking about it, do we really need two separate operations ? As I understand > it, mode_fixup is mostly (only ? - this is where we need documentation) used > to fixup the pixel clock frequency as clock generators usually have > limitations in their dividers. We assume that the sync won't care too much, > and happily feed it with a mode that is slightly different from what userspace > requested. Given that mode_valid should accepts mode for which the exact pixel > clock frequency can't bee achieved, and that the atomic commit will fixup that > frequency anyway, can't we apply the same processing to modes enumerated by > the connector, and merge the mode_valid and mode_fixup operations ? They are fairly similar, except mode_fixup also has the adjusted mode, and needs to fill that one out. With atomic there's also the complication that drivers with not-so-simple checks use atomic_check, and we really can't fake the entire atomic states. So even if we could merge mode_fixup and mode_valid somehow, we'll be left with atomic_check. -Daniel
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h index fdd82fc..00c6c36 100644 --- a/include/drm/drm_bridge.h +++ b/include/drm/drm_bridge.h @@ -59,6 +59,26 @@ struct drm_bridge_funcs { void (*detach)(struct drm_bridge *bridge); /** + * @mode_valid: + * + * This callback is used to check if a specific mode is valid in this + * bridge. This should be implemented if the bridge has some sort of + * restriction in the modes it can display. For example, a given bridge + * may be responsible to set a clock value. If the clock can not + * produce all the values for the available modes then this callback + * can be used to restrict the number of modes to only the ones that + * can be displayed. + * + * This is called at mode probe and at atomic check phase. + * + * RETURNS: + * + * drm_mode_status Enum + */ + enum drm_mode_status (*mode_valid)(struct drm_bridge *crtc, + const struct drm_display_mode *mode); + + /** * @mode_fixup: * * This callback is used to validate and adjust a mode. The paramater diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h index c01c328..eec2c70 100644 --- a/include/drm/drm_modeset_helper_vtables.h +++ b/include/drm/drm_modeset_helper_vtables.h @@ -106,6 +106,26 @@ struct drm_crtc_helper_funcs { void (*commit)(struct drm_crtc *crtc); /** + * @mode_valid: + * + * This callback is used to check if a specific mode is valid in this + * crtc. This should be implemented if the crtc has some sort of + * restriction in the modes it can display. For example, a given crtc + * may be responsible to set a clock value. If the clock can not + * produce all the values for the available modes then this callback + * can be used to restrict the number of modes to only the ones that + * can be displayed. + * + * This is called at mode probe and at atomic check phase. + * + * RETURNS: + * + * drm_mode_status Enum + */ + enum drm_mode_status (*mode_valid)(struct drm_crtc *crtc, + const struct drm_display_mode *mode); + + /** * @mode_fixup: * * This callback is used to validate a mode. The parameter mode is the @@ -457,6 +477,26 @@ struct drm_encoder_helper_funcs { void (*dpms)(struct drm_encoder *encoder, int mode); /** + * @mode_valid: + * + * This callback is used to check if a specific mode is valid in this + * encoder. This should be implemented if the encoder has some sort + * of restriction in the modes it can display. For example, a given + * encoder may be responsible to set a clock value. If the clock can + * not produce all the values for the available modes then this callback + * can be used to restrict the number of modes to only the ones that + * can be displayed. + * + * This is called at mode probe and at atomic check phase. + * + * RETURNS: + * + * drm_mode_status Enum + */ + enum drm_mode_status (*mode_valid)(struct drm_encoder *crtc, + const struct drm_display_mode *mode); + + /** * @mode_fixup: * * This callback is used to validate and adjust a mode. The parameter @@ -795,6 +835,11 @@ struct drm_connector_helper_funcs { * (which is usually derived from the EDID data block from the sink). * See e.g. drm_helper_probe_single_connector_modes(). * + * This callback is never called in atomic check phase so that userspace + * can override kernel sink checks in case of broken EDID with wrong + * limits from the sink. You can use the remaining mode_valid() + * callbacks to validate the mode against your video path. + * * NOTE: * * This only filters the mode list supplied to userspace in the
This adds a new callback to crtc, encoder and bridge helper functions called mode_valid(). This callback shall be implemented if the corresponding component has some sort of restriction in the modes that can be displayed. A NULL callback implicates that the component can display all the modes. We also change the description of connector->mode_valid() callback so that it matches the existing behaviour: It is never called in atomic check phase. Only the callbacks were implemented to simplify review process, following patches will make use of them. Signed-off-by: Jose Abreu <joabreu@synopsys.com> Cc: Carlos Palminha <palminha@synopsys.com> Cc: Alexey Brodkin <abrodkin@synopsys.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: Dave Airlie <airlied@linux.ie> Cc: Andrzej Hajda <a.hajda@samsung.com> Cc: Archit Taneja <architt@codeaurora.org> --- Changes v1->v2: - Change description of connector->mode_valid() (Daniel) include/drm/drm_bridge.h | 20 ++++++++++++++ include/drm/drm_modeset_helper_vtables.h | 45 ++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+)