Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1638291 > unrolled thread
| Started by | Jose Abreu <Jose.Abreu@synopsys.com> |
|---|---|
| First post | 2017-05-09 19:10 +0200 |
| Last post | 2017-05-09 19:10 +0200 |
| Articles | 14 on this page of 34 — 5 participants |
Back to article view | Back to linux.kernel
[PATCH v2 0/8] Introduce new mode validation callbacks Jose Abreu <Jose.Abreu@synopsys.com> - 2017-05-09 19:10 +0200
[PATCH v2 6/8] drm: Introduce drm_bridge_mode_valid() Jose Abreu <Jose.Abreu@synopsys.com> - 2017-05-09 19:10 +0200
Re: [PATCH v2 6/8] drm: Introduce drm_bridge_mode_valid() Ville Syrjälä <ville.syrjala@linux.intel.com> - 2017-05-10 15:50 +0200
Re: [PATCH v2 6/8] drm: Introduce drm_bridge_mode_valid() Jose Abreu <Jose.Abreu@synopsys.com> - 2017-05-10 16:10 +0200
Re: [PATCH v2 6/8] drm: Introduce drm_bridge_mode_valid() Jose Abreu <Jose.Abreu@synopsys.com> - 2017-05-10 16:10 +0200
Re: [PATCH v2 6/8] drm: Introduce drm_bridge_mode_valid() Daniel Vetter <daniel@ffwll.ch> - 2017-05-10 17:20 +0200
Re: [PATCH v2 6/8] drm: Introduce drm_bridge_mode_valid() Laurent Pinchart <laurent.pinchart@ideasonboard.com> - 2017-05-12 11:40 +0200
Re: [PATCH v2 6/8] drm: Introduce drm_bridge_mode_valid() Archit Taneja <architt@codeaurora.org> - 2017-05-12 13:00 +0200
Re: [PATCH v2 6/8] drm: Introduce drm_bridge_mode_valid() Laurent Pinchart <laurent.pinchart@ideasonboard.com> - 2017-05-12 13:10 +0200
Re: [PATCH v2 6/8] drm: Introduce drm_bridge_mode_valid() Archit Taneja <architt@codeaurora.org> - 2017-05-15 06:20 +0200
Re: [PATCH v2 6/8] drm: Introduce drm_bridge_mode_valid() Daniel Vetter <daniel@ffwll.ch> - 2017-05-15 08:50 +0200
[PATCH v2 4/8] drm: Add drm_connector_mode_valid() Jose Abreu <Jose.Abreu@synopsys.com> - 2017-05-09 19:10 +0200
[PATCH v2 2/8] drm: Add drm_crtc_mode_valid() Jose Abreu <Jose.Abreu@synopsys.com> - 2017-05-09 19:10 +0200
Re: [PATCH v2 2/8] drm: Add drm_crtc_mode_valid() Daniel Vetter <daniel@ffwll.ch> - 2017-05-10 10:10 +0200
Re: [PATCH v2 2/8] drm: Add drm_crtc_mode_valid() Jose Abreu <Jose.Abreu@synopsys.com> - 2017-05-10 11:00 +0200
Re: [PATCH v2 2/8] drm: Add drm_crtc_mode_valid() Daniel Vetter <daniel@ffwll.ch> - 2017-05-10 17:20 +0200
[PATCH v2 8/8] drm: arc: Use crtc->mode_valid() callback Jose Abreu <Jose.Abreu@synopsys.com> - 2017-05-09 19:10 +0200
Re: [PATCH v2 8/8] drm: arc: Use crtc->mode_valid() callback Laurent Pinchart <laurent.pinchart@ideasonboard.com> - 2017-05-12 12:00 +0200
Re: [PATCH v2 8/8] drm: arc: Use crtc->mode_valid() callback Jose Abreu <Jose.Abreu@synopsys.com> - 2017-05-15 03:50 +0200
[PATCH v2 5/8] drm: Use new mode_valid() helpers in connector probe helper Jose Abreu <Jose.Abreu@synopsys.com> - 2017-05-09 19:10 +0200
Re: [PATCH v2 5/8] drm: Use new mode_valid() helpers in connector probe helper Daniel Vetter <daniel@ffwll.ch> - 2017-05-10 10:10 +0200
Re: [PATCH v2 5/8] drm: Use new mode_valid() helpers in connector probe helper Jose Abreu <Jose.Abreu@synopsys.com> - 2017-05-10 11:10 +0200
Re: [PATCH v2 5/8] drm: Use new mode_valid() helpers in connector probe helper Laurent Pinchart <laurent.pinchart@ideasonboard.com> - 2017-05-12 11:40 +0200
Re: [PATCH v2 5/8] drm: Use new mode_valid() helpers in connector probe helper Jose Abreu <Jose.Abreu@synopsys.com> - 2017-05-12 18:10 +0200
Re: [PATCH v2 5/8] drm: Use new mode_valid() helpers in connector probe helper Laurent Pinchart <laurent.pinchart@ideasonboard.com> - 2017-05-14 13:10 +0200
Re: [PATCH v2 5/8] drm: Use new mode_valid() helpers in connector probe helper Daniel Vetter <daniel@ffwll.ch> - 2017-05-15 08:50 +0200
Re: [PATCH v2 5/8] drm: Use new mode_valid() helpers in connector probe helper Laurent Pinchart <laurent.pinchart@ideasonboard.com> - 2017-05-15 09:10 +0200
Re: [PATCH v2 5/8] drm: Use new mode_valid() helpers in connector probe helper Jose Abreu <Jose.Abreu@synopsys.com> - 2017-05-16 07:20 +0200
[PATCH v2 1/8] drm: Add crtc/encoder/bridge->mode_valid() callbacks Jose Abreu <Jose.Abreu@synopsys.com> - 2017-05-09 19:10 +0200
Re: [PATCH v2 1/8] drm: Add crtc/encoder/bridge->mode_valid() callbacks Daniel Vetter <daniel@ffwll.ch> - 2017-05-10 10:10 +0200
Re: [PATCH v2 1/8] drm: Add crtc/encoder/bridge->mode_valid() callbacks Jose Abreu <Jose.Abreu@synopsys.com> - 2017-05-10 11:10 +0200
Re: [PATCH v2 1/8] drm: Add crtc/encoder/bridge->mode_valid() callbacks Laurent Pinchart <laurent.pinchart@ideasonboard.com> - 2017-05-12 10:30 +0200
Re: [PATCH v2 1/8] drm: Add crtc/encoder/bridge->mode_valid() callbacks Daniel Vetter <daniel@ffwll.ch> - 2017-05-15 08:50 +0200
[PATCH v2 3/8] drm: Add drm_encoder_mode_valid() Jose Abreu <Jose.Abreu@synopsys.com> - 2017-05-09 19:10 +0200
Page 2 of 2 — ← Prev page 1 [2]
| From | Daniel Vetter <daniel@ffwll.ch> |
|---|---|
| Date | 2017-05-10 10:10 +0200 |
| Subject | Re: [PATCH v2 5/8] drm: Use new mode_valid() helpers in connector probe helper |
| Message-ID | <tFuR4-6F7-17@gated-at.bofh.it> |
| In reply to | #1638302 |
On Tue, May 09, 2017 at 06:00:12PM +0100, Jose Abreu wrote:
> This changes the connector probe helper function to use the new
> encoder->mode_valid() and crtc->mode_valid() helper callbacks to
> validate the modes.
>
> The new callbacks are optional so the behaviour remains the same
> if they are not implemented. If they are, then the code loops
> through all the connector's encodersXcrtcs and calls the
> callback.
>
> If at least a valid encoderXcrtc combination is found which
> accepts the mode then the function returns MODE_OK.
>
> 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:
> - Use new helpers suggested by Ville
> - Change documentation (Daniel)
>
> drivers/gpu/drm/drm_probe_helper.c | 60 ++++++++++++++++++++++++++++++++++++--
> 1 file changed, 57 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
> index 1b0c14a..de47413 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -39,6 +39,8 @@
> #include <drm/drm_fb_helper.h>
> #include <drm/drm_edid.h>
>
> +#include "drm_crtc_internal.h"
> +
> /**
> * DOC: output probing helper overview
> *
> @@ -80,6 +82,54 @@
> return MODE_OK;
> }
>
> +static enum drm_mode_status
> +drm_mode_validate_connector(struct drm_connector *connector,
> + struct drm_display_mode *mode)
> +{
> + struct drm_device *dev = connector->dev;
> + uint32_t *ids = connector->encoder_ids;
> + enum drm_mode_status ret = MODE_OK;
> + unsigned int i;
> +
> + /* Step 1: Validate against connector */
> + ret = drm_connector_mode_valid(connector, mode);
> + if (ret != MODE_OK)
> + return ret;
> +
> + /* Step 2: Validate against encoders and crtcs */
> + for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
> + struct drm_encoder *encoder = drm_encoder_find(dev, ids[i]);
> + struct drm_crtc *crtc;
> +
> + if (!encoder)
> + continue;
> +
> + ret = drm_encoder_mode_valid(encoder, mode);
> + if (ret != MODE_OK) {
> + /* No point in continuing for crtc check as this encoder
> + * will not accept the mode anyway. If all encoders
> + * reject the mode then, at exit, ret will not be
> + * MODE_OK. */
> + continue;
> + }
One thing I've forgotten the last time around: Please also check
bridge->mode_valid here. The encoder->bridge mapping is fixed.
Otherwise I think this looks good.
-Daniel
> +
> + drm_for_each_crtc(crtc, dev) {
> + if (!drm_encoder_crtc_ok(encoder, crtc))
> + continue;
> +
> + ret = drm_crtc_mode_valid(crtc, mode);
> + if (ret == MODE_OK) {
> + /* If we get to this point there is at least
> + * one combination of encoder+crtc that works
> + * for this mode. Lets return now. */
> + return ret;
> + }
> + }
> + }
> +
> + return ret;
> +}
> +
> static int drm_helper_probe_add_cmdline_mode(struct drm_connector *connector)
> {
> struct drm_cmdline_mode *cmdline_mode;
> @@ -284,7 +334,11 @@ void drm_kms_helper_poll_enable(struct drm_device *dev)
> * - drm_mode_validate_flag() checks the modes against basic connector
> * capabilities (interlace_allowed,doublescan_allowed,stereo_allowed)
> * - the optional &drm_connector_helper_funcs.mode_valid helper can perform
> - * driver and/or hardware specific checks
> + * driver and/or sink specific checks
> + * - the optional &drm_crtc_helper_funcs.mode_valid and
> + * &drm_encoder_helper_funcs.mode_valid helpers can perform driver and/or
> + * source specific checks which are also enforced by the modeset/atomic
> + * helpers
> *
> * 5. Any mode whose status is not OK is pruned from the connector's modes list,
> * accompanied by a debug message indicating the reason for the mode's
> @@ -428,8 +482,8 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector,
> if (mode->status == MODE_OK)
> mode->status = drm_mode_validate_flag(mode, mode_flags);
>
> - if (mode->status == MODE_OK && connector_funcs->mode_valid)
> - mode->status = connector_funcs->mode_valid(connector,
> + if (mode->status == MODE_OK)
> + mode->status = drm_mode_validate_connector(connector,
> mode);
> }
>
> --
> 1.9.1
>
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[toc] | [prev] | [next] | [standalone]
| From | Jose Abreu <Jose.Abreu@synopsys.com> |
|---|---|
| Date | 2017-05-10 11:10 +0200 |
| Subject | Re: [PATCH v2 5/8] drm: Use new mode_valid() helpers in connector probe helper |
| Message-ID | <tFvN9-7fl-35@gated-at.bofh.it> |
| In reply to | #1638636 |
Hi Daniel,
On 10-05-2017 09:01, Daniel Vetter wrote:
> On Tue, May 09, 2017 at 06:00:12PM +0100, Jose Abreu wrote:
>> This changes the connector probe helper function to use the new
>> encoder->mode_valid() and crtc->mode_valid() helper callbacks to
>> validate the modes.
>>
>> The new callbacks are optional so the behaviour remains the same
>> if they are not implemented. If they are, then the code loops
>> through all the connector's encodersXcrtcs and calls the
>> callback.
>>
>> If at least a valid encoderXcrtc combination is found which
>> accepts the mode then the function returns MODE_OK.
>>
>> 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:
>> - Use new helpers suggested by Ville
>> - Change documentation (Daniel)
>>
>> drivers/gpu/drm/drm_probe_helper.c | 60 ++++++++++++++++++++++++++++++++++++--
>> 1 file changed, 57 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
>> index 1b0c14a..de47413 100644
>> --- a/drivers/gpu/drm/drm_probe_helper.c
>> +++ b/drivers/gpu/drm/drm_probe_helper.c
>> @@ -39,6 +39,8 @@
>> #include <drm/drm_fb_helper.h>
>> #include <drm/drm_edid.h>
>>
>> +#include "drm_crtc_internal.h"
>> +
>> /**
>> * DOC: output probing helper overview
>> *
>> @@ -80,6 +82,54 @@
>> return MODE_OK;
>> }
>>
>> +static enum drm_mode_status
>> +drm_mode_validate_connector(struct drm_connector *connector,
>> + struct drm_display_mode *mode)
>> +{
>> + struct drm_device *dev = connector->dev;
>> + uint32_t *ids = connector->encoder_ids;
>> + enum drm_mode_status ret = MODE_OK;
>> + unsigned int i;
>> +
>> + /* Step 1: Validate against connector */
>> + ret = drm_connector_mode_valid(connector, mode);
>> + if (ret != MODE_OK)
>> + return ret;
>> +
>> + /* Step 2: Validate against encoders and crtcs */
>> + for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
>> + struct drm_encoder *encoder = drm_encoder_find(dev, ids[i]);
>> + struct drm_crtc *crtc;
>> +
>> + if (!encoder)
>> + continue;
>> +
>> + ret = drm_encoder_mode_valid(encoder, mode);
>> + if (ret != MODE_OK) {
>> + /* No point in continuing for crtc check as this encoder
>> + * will not accept the mode anyway. If all encoders
>> + * reject the mode then, at exit, ret will not be
>> + * MODE_OK. */
>> + continue;
>> + }
> One thing I've forgotten the last time around: Please also check
> bridge->mode_valid here. The encoder->bridge mapping is fixed.
Ok, will add in next version.
Best regards,
Jose Miguel Abreu
>
> Otherwise I think this looks good.
> -Daniel
>
>> +
>> + drm_for_each_crtc(crtc, dev) {
>> + if (!drm_encoder_crtc_ok(encoder, crtc))
>> + continue;
>> +
>> + ret = drm_crtc_mode_valid(crtc, mode);
>> + if (ret == MODE_OK) {
>> + /* If we get to this point there is at least
>> + * one combination of encoder+crtc that works
>> + * for this mode. Lets return now. */
>> + return ret;
>> + }
>> + }
>> + }
>> +
>> + return ret;
>> +}
>> +
>> static int drm_helper_probe_add_cmdline_mode(struct drm_connector *connector)
>> {
>> struct drm_cmdline_mode *cmdline_mode;
>> @@ -284,7 +334,11 @@ void drm_kms_helper_poll_enable(struct drm_device *dev)
>> * - drm_mode_validate_flag() checks the modes against basic connector
>> * capabilities (interlace_allowed,doublescan_allowed,stereo_allowed)
>> * - the optional &drm_connector_helper_funcs.mode_valid helper can perform
>> - * driver and/or hardware specific checks
>> + * driver and/or sink specific checks
>> + * - the optional &drm_crtc_helper_funcs.mode_valid and
>> + * &drm_encoder_helper_funcs.mode_valid helpers can perform driver and/or
>> + * source specific checks which are also enforced by the modeset/atomic
>> + * helpers
>> *
>> * 5. Any mode whose status is not OK is pruned from the connector's modes list,
>> * accompanied by a debug message indicating the reason for the mode's
>> @@ -428,8 +482,8 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector,
>> if (mode->status == MODE_OK)
>> mode->status = drm_mode_validate_flag(mode, mode_flags);
>>
>> - if (mode->status == MODE_OK && connector_funcs->mode_valid)
>> - mode->status = connector_funcs->mode_valid(connector,
>> + if (mode->status == MODE_OK)
>> + mode->status = drm_mode_validate_connector(connector,
>> mode);
>> }
>>
>> --
>> 1.9.1
>>
>>
[toc] | [prev] | [next] | [standalone]
| From | Laurent Pinchart <laurent.pinchart@ideasonboard.com> |
|---|---|
| Date | 2017-05-12 11:40 +0200 |
| Subject | Re: [PATCH v2 5/8] drm: Use new mode_valid() helpers in connector probe helper |
| Message-ID | <tGfdh-2rg-29@gated-at.bofh.it> |
| In reply to | #1638302 |
Hi Jose,
Thank you for the patch.
On Tuesday 09 May 2017 18:00:12 Jose Abreu wrote:
> This changes the connector probe helper function to use the new
> encoder->mode_valid() and crtc->mode_valid() helper callbacks to
> validate the modes.
>
> The new callbacks are optional so the behaviour remains the same
> if they are not implemented. If they are, then the code loops
> through all the connector's encodersXcrtcs and calls the
> callback.
>
> If at least a valid encoderXcrtc combination is found which
> accepts the mode then the function returns MODE_OK.
>
> 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:
> - Use new helpers suggested by Ville
> - Change documentation (Daniel)
>
> drivers/gpu/drm/drm_probe_helper.c | 60 +++++++++++++++++++++++++++++++++--
> 1 file changed, 57 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_probe_helper.c
> b/drivers/gpu/drm/drm_probe_helper.c index 1b0c14a..de47413 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -39,6 +39,8 @@
> #include <drm/drm_fb_helper.h>
> #include <drm/drm_edid.h>
>
> +#include "drm_crtc_internal.h"
> +
> /**
> * DOC: output probing helper overview
> *
> @@ -80,6 +82,54 @@
> return MODE_OK;
> }
>
> +static enum drm_mode_status
> +drm_mode_validate_connector(struct drm_connector *connector,
> + struct drm_display_mode *mode)
This does more than validating the mode against the connector, it validates it
against the whole pipeline. I would call the function
drm_mode_validate_pipeline() (or any other similar name).
> +{
> + struct drm_device *dev = connector->dev;
> + uint32_t *ids = connector->encoder_ids;
> + enum drm_mode_status ret = MODE_OK;
> + unsigned int i;
> +
> + /* Step 1: Validate against connector */
> + ret = drm_connector_mode_valid(connector, mode);
> + if (ret != MODE_OK)
> + return ret;
> +
> + /* Step 2: Validate against encoders and crtcs */
> + for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
> + struct drm_encoder *encoder = drm_encoder_find(dev, ids[i]);
> + struct drm_crtc *crtc;
> +
> + if (!encoder)
> + continue;
> +
> + ret = drm_encoder_mode_valid(encoder, mode);
> + if (ret != MODE_OK) {
> + /* No point in continuing for crtc check as this
encoder
> + * will not accept the mode anyway. If all encoders
> + * reject the mode then, at exit, ret will not be
> + * MODE_OK. */
> + continue;
> + }
> +
> + drm_for_each_crtc(crtc, dev) {
> + if (!drm_encoder_crtc_ok(encoder, crtc))
> + continue;
> +
> + ret = drm_crtc_mode_valid(crtc, mode);
> + if (ret == MODE_OK) {
> + /* If we get to this point there is at least
> + * one combination of encoder+crtc that works
> + * for this mode. Lets return now. */
> + return ret;
> + }
> + }
> + }
> +
> + return ret;
> +}
> +
> static int drm_helper_probe_add_cmdline_mode(struct drm_connector
> *connector)
> {
> struct drm_cmdline_mode *cmdline_mode;
> @@ -284,7 +334,11 @@ void drm_kms_helper_poll_enable(struct drm_device *dev)
> * - drm_mode_validate_flag() checks the modes against basic connector
> * capabilities (interlace_allowed,doublescan_allowed,stereo_allowed)
> * - the optional &drm_connector_helper_funcs.mode_valid helper can
> perform
> - * driver and/or hardware specific checks
> + * driver and/or sink specific checks
> + * - the optional &drm_crtc_helper_funcs.mode_valid and
> + * &drm_encoder_helper_funcs.mode_valid helpers can perform driver
> and/or
> + * source specific checks which are also enforced by the
> modeset/atomic
> + * helpers
> *
> * 5. Any mode whose status is not OK is pruned from the connector's modes
> list,
> * accompanied by a debug message indicating the reason for the mode's
> @@ -428,8 +482,8 @@ int
> drm_helper_probe_single_connector_modes(struct drm_connector *connector,
> if (mode->status == MODE_OK)
> mode->status = drm_mode_validate_flag(mode,
> mode_flags);
>
> - if (mode->status == MODE_OK && connector_funcs->mode_valid)
> - mode->status = connector_funcs->mode_valid(connector,
> + if (mode->status == MODE_OK)
> + mode->status = drm_mode_validate_connector(connector,
> mode);
I would reverse the arguments order to match the style of the other validation
functions.
> }
--
Regards,
Laurent Pinchart
[toc] | [prev] | [next] | [standalone]
| From | Jose Abreu <Jose.Abreu@synopsys.com> |
|---|---|
| Date | 2017-05-12 18:10 +0200 |
| Subject | Re: [PATCH v2 5/8] drm: Use new mode_valid() helpers in connector probe helper |
| Message-ID | <tGliF-71V-3@gated-at.bofh.it> |
| In reply to | #1640361 |
Hi Laurent,
On 12-05-2017 10:35, Laurent Pinchart wrote:
> Hi Jose,
>
> Thank you for the patch.
>
> On Tuesday 09 May 2017 18:00:12 Jose Abreu wrote:
>> This changes the connector probe helper function to use the new
>> encoder->mode_valid() and crtc->mode_valid() helper callbacks to
>> validate the modes.
>>
>> The new callbacks are optional so the behaviour remains the same
>> if they are not implemented. If they are, then the code loops
>> through all the connector's encodersXcrtcs and calls the
>> callback.
>>
>> If at least a valid encoderXcrtc combination is found which
>> accepts the mode then the function returns MODE_OK.
>>
>> 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:
>> - Use new helpers suggested by Ville
>> - Change documentation (Daniel)
>>
>> drivers/gpu/drm/drm_probe_helper.c | 60 +++++++++++++++++++++++++++++++++--
>> 1 file changed, 57 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_probe_helper.c
>> b/drivers/gpu/drm/drm_probe_helper.c index 1b0c14a..de47413 100644
>> --- a/drivers/gpu/drm/drm_probe_helper.c
>> +++ b/drivers/gpu/drm/drm_probe_helper.c
>> @@ -39,6 +39,8 @@
>> #include <drm/drm_fb_helper.h>
>> #include <drm/drm_edid.h>
>>
>> +#include "drm_crtc_internal.h"
>> +
>> /**
>> * DOC: output probing helper overview
>> *
>> @@ -80,6 +82,54 @@
>> return MODE_OK;
>> }
>>
>> +static enum drm_mode_status
>> +drm_mode_validate_connector(struct drm_connector *connector,
>> + struct drm_display_mode *mode)
> This does more than validating the mode against the connector, it validates it
> against the whole pipeline. I would call the function
> drm_mode_validate_pipeline() (or any other similar name).
Yeah, in previous version I had something similar but I changed
in order to address review comments. I can change again though...
>
>> +{
>> + struct drm_device *dev = connector->dev;
>> + uint32_t *ids = connector->encoder_ids;
>> + enum drm_mode_status ret = MODE_OK;
>> + unsigned int i;
>> +
>> + /* Step 1: Validate against connector */
>> + ret = drm_connector_mode_valid(connector, mode);
>> + if (ret != MODE_OK)
>> + return ret;
>> +
>> + /* Step 2: Validate against encoders and crtcs */
>> + for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
>> + struct drm_encoder *encoder = drm_encoder_find(dev, ids[i]);
>> + struct drm_crtc *crtc;
>> +
>> + if (!encoder)
>> + continue;
>> +
>> + ret = drm_encoder_mode_valid(encoder, mode);
>> + if (ret != MODE_OK) {
>> + /* No point in continuing for crtc check as this
> encoder
>> + * will not accept the mode anyway. If all encoders
>> + * reject the mode then, at exit, ret will not be
>> + * MODE_OK. */
>> + continue;
>> + }
>> +
>> + drm_for_each_crtc(crtc, dev) {
>> + if (!drm_encoder_crtc_ok(encoder, crtc))
>> + continue;
>> +
>> + ret = drm_crtc_mode_valid(crtc, mode);
>> + if (ret == MODE_OK) {
>> + /* If we get to this point there is at least
>> + * one combination of encoder+crtc that works
>> + * for this mode. Lets return now. */
>> + return ret;
>> + }
>> + }
>> + }
>> +
>> + return ret;
>> +}
>> +
>> static int drm_helper_probe_add_cmdline_mode(struct drm_connector
>> *connector)
>> {
>> struct drm_cmdline_mode *cmdline_mode;
>> @@ -284,7 +334,11 @@ void drm_kms_helper_poll_enable(struct drm_device *dev)
>> * - drm_mode_validate_flag() checks the modes against basic connector
>> * capabilities (interlace_allowed,doublescan_allowed,stereo_allowed)
>> * - the optional &drm_connector_helper_funcs.mode_valid helper can
>> perform
>> - * driver and/or hardware specific checks
>> + * driver and/or sink specific checks
>> + * - the optional &drm_crtc_helper_funcs.mode_valid and
>> + * &drm_encoder_helper_funcs.mode_valid helpers can perform driver
>> and/or
>> + * source specific checks which are also enforced by the
>> modeset/atomic
>> + * helpers
>> *
>> * 5. Any mode whose status is not OK is pruned from the connector's modes
>> list,
>> * accompanied by a debug message indicating the reason for the mode's
>> @@ -428,8 +482,8 @@ int
>> drm_helper_probe_single_connector_modes(struct drm_connector *connector,
>> if (mode->status == MODE_OK)
>> mode->status = drm_mode_validate_flag(mode,
>> mode_flags);
>>
>> - if (mode->status == MODE_OK && connector_funcs->mode_valid)
>> - mode->status = connector_funcs->mode_valid(connector,
>> + if (mode->status == MODE_OK)
>> + mode->status = drm_mode_validate_connector(connector,
>> mode);
> I would reverse the arguments order to match the style of the other validation
> functions.
Hmm, I think it makes more sense to pass connector first and then
mode ...
Best regards,
Jose Miguel Abreu
>
>> }
[toc] | [prev] | [next] | [standalone]
| From | Laurent Pinchart <laurent.pinchart@ideasonboard.com> |
|---|---|
| Date | 2017-05-14 13:10 +0200 |
| Subject | Re: [PATCH v2 5/8] drm: Use new mode_valid() helpers in connector probe helper |
| Message-ID | <tGZzs-Q7-9@gated-at.bofh.it> |
| In reply to | #1640560 |
Hi Jose,
On Friday 12 May 2017 17:06:14 Jose Abreu wrote:
> On 12-05-2017 10:35, Laurent Pinchart wrote:
> > On Tuesday 09 May 2017 18:00:12 Jose Abreu wrote:
> >> This changes the connector probe helper function to use the new
> >> encoder->mode_valid() and crtc->mode_valid() helper callbacks to
> >> validate the modes.
> >>
> >> The new callbacks are optional so the behaviour remains the same
> >> if they are not implemented. If they are, then the code loops
> >> through all the connector's encodersXcrtcs and calls the
> >> callback.
> >>
> >> If at least a valid encoderXcrtc combination is found which
> >> accepts the mode then the function returns MODE_OK.
> >>
> >> 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:
> >> - Use new helpers suggested by Ville
> >> - Change documentation (Daniel)
> >>
> >> drivers/gpu/drm/drm_probe_helper.c | 60 ++++++++++++++++++++++++++++++--
> >> 1 file changed, 57 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/drm_probe_helper.c
> >> b/drivers/gpu/drm/drm_probe_helper.c index 1b0c14a..de47413 100644
> >> --- a/drivers/gpu/drm/drm_probe_helper.c
> >> +++ b/drivers/gpu/drm/drm_probe_helper.c
[snip]
> >> +static enum drm_mode_status
> >> +drm_mode_validate_connector(struct drm_connector *connector,
> >> + struct drm_display_mode *mode)
> >
> > This does more than validating the mode against the connector, it
> > validates it against the whole pipeline. I would call the function
> > drm_mode_validate_pipeline() (or any other similar name).
>
> Yeah, in previous version I had something similar but I changed
> in order to address review comments. I can change again though...
Sorry, I haven't seen v1. I think it makes more sense to reflect in its name
the fact that the function validates the mode against the whole pipeline, but
I'll let others disagree.
> >> +{
> >> + struct drm_device *dev = connector->dev;
> >> + uint32_t *ids = connector->encoder_ids;
> >> + enum drm_mode_status ret = MODE_OK;
> >> + unsigned int i;
> >> +
> >> + /* Step 1: Validate against connector */
> >> + ret = drm_connector_mode_valid(connector, mode);
> >> + if (ret != MODE_OK)
> >> + return ret;
> >> +
> >> + /* Step 2: Validate against encoders and crtcs */
> >> + for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
> >> + struct drm_encoder *encoder = drm_encoder_find(dev, ids[i]);
> >> + struct drm_crtc *crtc;
> >> +
> >> + if (!encoder)
> >> + continue;
> >> +
> >> + ret = drm_encoder_mode_valid(encoder, mode);
> >> + if (ret != MODE_OK) {
> >> + /* No point in continuing for crtc check as this
> >
> > encoder
> >
> >> + * will not accept the mode anyway. If all encoders
> >> + * reject the mode then, at exit, ret will not be
> >> + * MODE_OK. */
> >> + continue;
> >> + }
> >> +
> >> + drm_for_each_crtc(crtc, dev) {
> >> + if (!drm_encoder_crtc_ok(encoder, crtc))
> >> + continue;
> >> +
> >> + ret = drm_crtc_mode_valid(crtc, mode);
> >> + if (ret == MODE_OK) {
> >> + /* If we get to this point there is at least
> >> + * one combination of encoder+crtc that works
> >> + * for this mode. Lets return now. */
> >> + return ret;
> >> + }
> >> + }
> >> + }
> >> +
> >> + return ret;
> >> +}
[snip]
> >> @@ -428,8 +482,8 @@ int
> >> drm_helper_probe_single_connector_modes(struct drm_connector *connector,
> >>
> >> if (mode->status == MODE_OK)
> >>
> >> mode->status = drm_mode_validate_flag(mode,
> >>
> >> mode_flags);
> >>
> >> - if (mode->status == MODE_OK && connector_funcs->mode_valid)
> >> - mode->status = connector_funcs->mode_valid(connector,
> >> + if (mode->status == MODE_OK)
> >> + mode->status = drm_mode_validate_connector(connector,
> >>
> >> mode);
> >
> > I would reverse the arguments order to match the style of the other
> > validation functions.
>
> Hmm, I think it makes more sense to pass connector first and then
> mode ...
I disagree, as this function validates a mode against a pipeline, the same way
the other validation functions validate a mode against other parameters, but
it's your patch :-)
--
Regards,
Laurent Pinchart
[toc] | [prev] | [next] | [standalone]
| From | Daniel Vetter <daniel@ffwll.ch> |
|---|---|
| Date | 2017-05-15 08:50 +0200 |
| Subject | Re: [PATCH v2 5/8] drm: Use new mode_valid() helpers in connector probe helper |
| Message-ID | <tHhZn-4vl-1@gated-at.bofh.it> |
| In reply to | #1641071 |
On Sun, May 14, 2017 at 02:04:24PM +0300, Laurent Pinchart wrote: > On Friday 12 May 2017 17:06:14 Jose Abreu wrote: > > On 12-05-2017 10:35, Laurent Pinchart wrote: > > > On Tuesday 09 May 2017 18:00:12 Jose Abreu wrote: > > >> + if (mode->status == MODE_OK) > > >> + mode->status = drm_mode_validate_connector(connector, > > >> > > >> mode); > > > > > > I would reverse the arguments order to match the style of the other > > > validation functions. > > > > Hmm, I think it makes more sense to pass connector first and then > > mode ... > > I disagree, as this function validates a mode against a pipeline, the same way > the other validation functions validate a mode against other parameters, but > it's your patch :-) Call it drm_connector_validate_mode, because the first argument is generally the object we operate on :-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[toc] | [prev] | [next] | [standalone]
| From | Laurent Pinchart <laurent.pinchart@ideasonboard.com> |
|---|---|
| Date | 2017-05-15 09:10 +0200 |
| Subject | Re: [PATCH v2 5/8] drm: Use new mode_valid() helpers in connector probe helper |
| Message-ID | <tHiiJ-4Th-1@gated-at.bofh.it> |
| In reply to | #1641309 |
On Monday 15 May 2017 08:47:49 Daniel Vetter wrote: > On Sun, May 14, 2017 at 02:04:24PM +0300, Laurent Pinchart wrote: > > On Friday 12 May 2017 17:06:14 Jose Abreu wrote: > >> On 12-05-2017 10:35, Laurent Pinchart wrote: > >>> On Tuesday 09 May 2017 18:00:12 Jose Abreu wrote: > >>>> + if (mode->status == MODE_OK) > >>>> + mode->status = drm_mode_validate_connector(connector, > >>>> > >>>> mode); > >>> > >>> I would reverse the arguments order to match the style of the other > >>> validation functions. > >> > >> Hmm, I think it makes more sense to pass connector first and then > >> mode ... > > > > I disagree, as this function validates a mode against a pipeline, the same > > way the other validation functions validate a mode against other > > parameters, but it's your patch :-) > > Call it drm_connector_validate_mode, because the first argument is > generally the object we operate on :-) But the function doesn't validate a mode for a connector, it validates a mode for a complete pipeline... -- Regards, Laurent Pinchart
[toc] | [prev] | [next] | [standalone]
| From | Jose Abreu <Jose.Abreu@synopsys.com> |
|---|---|
| Date | 2017-05-16 07:20 +0200 |
| Subject | Re: [PATCH v2 5/8] drm: Use new mode_valid() helpers in connector probe helper |
| Message-ID | <tHD3P-1lZ-3@gated-at.bofh.it> |
| In reply to | #1641316 |
Hi Laurent, On 15-05-2017 08:05, Laurent Pinchart wrote: > On Monday 15 May 2017 08:47:49 Daniel Vetter wrote: >> On Sun, May 14, 2017 at 02:04:24PM +0300, Laurent Pinchart wrote: >>> On Friday 12 May 2017 17:06:14 Jose Abreu wrote: >>>> On 12-05-2017 10:35, Laurent Pinchart wrote: >>>>> On Tuesday 09 May 2017 18:00:12 Jose Abreu wrote: >>>>>> + if (mode->status == MODE_OK) >>>>>> + mode->status = > drm_mode_validate_connector(connector, >>>>>> > mode); >>>>> I would reverse the arguments order to match the style of the other >>>>> validation functions. >>>> Hmm, I think it makes more sense to pass connector first and then >>>> mode ... >>> I disagree, as this function validates a mode against a pipeline, the same >>> way the other validation functions validate a mode against other >>> parameters, but it's your patch :-) >> Call it drm_connector_validate_mode, because the first argument is >> generally the object we operate on :-) > But the function doesn't validate a mode for a connector, it validates a mode > for a complete pipeline... > Hmm, but note that in the same function there is drm_mode_validate_size() and drm_mode_validate_flag() calls, which take as first argument the mode and then the object to validate (I hadn't seen this). So, maybe leave it as drm_mode_validate_connector() as it takes a connector as argument or change to drm_mode_validate_pipeline() as you said, or even drm_mode_validate_datapath(), drm_mode_validate_videopath(), drm_mode_validate_components() ? Best regards, Jose Miguel Abreu
[toc] | [prev] | [next] | [standalone]
| From | Jose Abreu <Jose.Abreu@synopsys.com> |
|---|---|
| Date | 2017-05-09 19:10 +0200 |
| Subject | [PATCH v2 1/8] drm: Add crtc/encoder/bridge->mode_valid() callbacks |
| Message-ID | <tFgO6-4HS-37@gated-at.bofh.it> |
| In reply to | #1638291 |
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
--
1.9.1
[toc] | [prev] | [next] | [standalone]
| From | Daniel Vetter <daniel@ffwll.ch> |
|---|---|
| Date | 2017-05-10 10:10 +0200 |
| Subject | Re: [PATCH v2 1/8] drm: Add crtc/encoder/bridge->mode_valid() callbacks |
| Message-ID | <tFuR4-6F7-25@gated-at.bofh.it> |
| In reply to | #1638303 |
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
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[toc] | [prev] | [next] | [standalone]
| From | Jose Abreu <Jose.Abreu@synopsys.com> |
|---|---|
| Date | 2017-05-10 11:10 +0200 |
| Subject | Re: [PATCH v2 1/8] drm: Add crtc/encoder/bridge->mode_valid() callbacks |
| Message-ID | <tFvN8-7fl-19@gated-at.bofh.it> |
| In reply to | #1638638 |
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
[toc] | [prev] | [next] | [standalone]
| From | Laurent Pinchart <laurent.pinchart@ideasonboard.com> |
|---|---|
| Date | 2017-05-12 10:30 +0200 |
| Subject | Re: [PATCH v2 1/8] drm: Add crtc/encoder/bridge->mode_valid() callbacks |
| Message-ID | <tGe7w-1Ju-5@gated-at.bofh.it> |
| In reply to | #1638638 |
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 ?
--
Regards,
Laurent Pinchart
[toc] | [prev] | [next] | [standalone]
| From | Daniel Vetter <daniel@ffwll.ch> |
|---|---|
| Date | 2017-05-15 08:50 +0200 |
| Subject | Re: [PATCH v2 1/8] drm: Add crtc/encoder/bridge->mode_valid() callbacks |
| Message-ID | <tHhZo-4vl-9@gated-at.bofh.it> |
| In reply to | #1640309 |
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
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[toc] | [prev] | [next] | [standalone]
| From | Jose Abreu <Jose.Abreu@synopsys.com> |
|---|---|
| Date | 2017-05-09 19:10 +0200 |
| Subject | [PATCH v2 3/8] drm: Add drm_encoder_mode_valid() |
| Message-ID | <tFgO6-4HS-35@gated-at.bofh.it> |
| In reply to | #1638291 |
Add a new helper to call encoder->mode_valid callback.
Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
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>
---
drivers/gpu/drm/drm_crtc_internal.h | 2 ++
drivers/gpu/drm/drm_encoder.c | 23 +++++++++++++++++++++++
2 files changed, 25 insertions(+)
diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h
index 3800abd..6165bc9 100644
--- a/drivers/gpu/drm/drm_crtc_internal.h
+++ b/drivers/gpu/drm/drm_crtc_internal.h
@@ -129,6 +129,8 @@ int drm_mode_obj_set_property_ioctl(struct drm_device *dev, void *data,
/* drm_encoder.c */
int drm_encoder_register_all(struct drm_device *dev);
void drm_encoder_unregister_all(struct drm_device *dev);
+enum drm_mode_status drm_encoder_mode_valid(struct drm_encoder *encoder,
+ const struct drm_display_mode *mode);
/* IOCTL */
int drm_mode_getencoder(struct drm_device *dev,
diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c
index 0708779..af75f42 100644
--- a/drivers/gpu/drm/drm_encoder.c
+++ b/drivers/gpu/drm/drm_encoder.c
@@ -23,6 +23,7 @@
#include <linux/export.h>
#include <drm/drmP.h>
#include <drm/drm_encoder.h>
+#include <drm/drm_modeset_helper_vtables.h>
#include "drm_crtc_internal.h"
@@ -239,3 +240,25 @@ int drm_mode_getencoder(struct drm_device *dev, void *data,
return 0;
}
+
+/**
+ * drm_encoder_mode_valid - call encoder->mode_valid callback, if any.
+ * @encoder: encoder
+ * @mode: mode to be validated
+ *
+ * If no mode_valid callback is available this will return MODE_OK.
+ *
+ * Returns: drm_mode_status Enum
+ */
+enum drm_mode_status drm_encoder_mode_valid(struct drm_encoder *encoder,
+ const struct drm_display_mode *mode)
+{
+ const struct drm_encoder_helper_funcs *encoder_funcs =
+ encoder->helper_private;
+
+ if (!encoder_funcs || !encoder_funcs->mode_valid)
+ return MODE_OK;
+
+ return encoder_funcs->mode_valid(encoder, mode);
+}
+EXPORT_SYMBOL(drm_encoder_mode_valid);
--
1.9.1
[toc] | [prev] | [standalone]
Page 2 of 2 — ← Prev page 1 [2]
Back to top | Article view | linux.kernel
csiph-web