linux-kernel - Re: [PATCH v2 2/8] drm: Add drm_crtc_mode

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <8af70047-7ab8-9beb-5258-ebaec5a245d8@synopsys.com>
Date:   Wed, 10 May 2017 09:57:30 +0100
From:   Jose Abreu <Jose.Abreu@...opsys.com>
To:     Daniel Vetter <daniel.vetter@...ll.ch>
CC:     Jose Abreu <Jose.Abreu@...opsys.com>,
        <dri-devel@...ts.freedesktop.org>, <linux-kernel@...r.kernel.org>,
        Carlos Palminha <CARLOS.PALMINHA@...opsys.com>,
        Alexey Brodkin <Alexey.Brodkin@...opsys.com>,
        Ville Syrjälä <ville.syrjala@...ux.intel.com>,
        Dave Airlie <airlied@...ux.ie>,
        Andrzej Hajda <a.hajda@...sung.com>,
        Archit Taneja <architt@...eaurora.org>
Subject: Re: [PATCH v2 2/8] drm: Add drm_crtc_mode_valid()

Hi Daniel,


On 10-05-2017 08:59, Daniel Vetter wrote:
> On Tue, May 09, 2017 at 06:00:09PM +0100, Jose Abreu wrote:
>> Add a new helper to call crtc->mode_valid callback.
>>
>> Suggested-by: Ville Syrjälä <ville.syrjala@...ux.intel.com>
>> Signed-off-by: Jose Abreu <joabreu@...opsys.com>
>> Cc: Carlos Palminha <palminha@...opsys.com>
>> Cc: Alexey Brodkin <abrodkin@...opsys.com>
>> Cc: Ville Syrjälä <ville.syrjala@...ux.intel.com>
>> Cc: Daniel Vetter <daniel.vetter@...ll.ch>
>> Cc: Dave Airlie <airlied@...ux.ie>
>> Cc: Andrzej Hajda <a.hajda@...sung.com>
>> Cc: Archit Taneja <architt@...eaurora.org>
>> ---
>>  drivers/gpu/drm/drm_crtc.c          | 22 ++++++++++++++++++++++
>>  drivers/gpu/drm/drm_crtc_internal.h |  3 +++
>>  2 files changed, 25 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
>> index 5af25ce..07ae705 100644
>> --- a/drivers/gpu/drm/drm_crtc.c
>> +++ b/drivers/gpu/drm/drm_crtc.c
>> @@ -38,6 +38,7 @@
>>  #include <drm/drm_crtc.h>
>>  #include <drm/drm_edid.h>
>>  #include <drm/drm_fourcc.h>
>> +#include <drm/drm_modeset_helper_vtables.h>
>>  #include <drm/drm_modeset_lock.h>
>>  #include <drm/drm_atomic.h>
>>  #include <drm/drm_auth.h>
>> @@ -741,3 +742,24 @@ int drm_mode_crtc_set_obj_prop(struct drm_mode_object *obj,
>>  
>>  	return ret;
>>  }
>> +
>> +/**
>> + * drm_crtc_mode_valid - call crtc->mode_valid callback, if any.
>> + * @crtc: crtc
>> + * @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_crtc_mode_valid(struct drm_crtc *crtc,
>> +					 const struct drm_display_mode *mode)
>> +{
>> +	const struct drm_crtc_helper_funcs *crtc_funcs = crtc->helper_private;
> This is clearly a helper func, but you place it into the core and
> EXPORT_SYMBOL it. Imo this should be entirely internal to the helpers,
> perhaps just stuff them all into drm_probe_helpers.c? Header file would be
> drm_crtc_helper_internal.h.

Yeah, at first I was not planning to export it but then I saw
that drm_bridge_mode_fixup() is exported (and is in drm_bridge.c)
so it kind of felt right to place this in drm_crtc.c. Anyway, I
will move them to drm_probe_helpers.c, indeed there is no point
in exporting this.

>
> That also means no need for kernel-doc (only the driver api is formally
> documented) and then these 3 patches are so tiny it's better to squash
> them into the patch that adds their users.

Ok, will remove the docs but I think its better to have a single
patch which adds all the helpers so that I can use the
suggested-by tag. Thanks!

Best regards,
Jose Miguel Abreu

>
> Thanks, Daniel
>> +
>> +	if (!crtc_funcs || !crtc_funcs->mode_valid)
>> +		return MODE_OK;
>> +
>> +	return crtc_funcs->mode_valid(crtc, mode);
>> +}
>> +EXPORT_SYMBOL(drm_crtc_mode_valid);
>> diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h
>> index d077c54..3800abd 100644
>> --- a/drivers/gpu/drm/drm_crtc_internal.h
>> +++ b/drivers/gpu/drm/drm_crtc_internal.h
>> @@ -45,6 +45,9 @@ int drm_crtc_check_viewport(const struct drm_crtc *crtc,
>>  
>>  struct dma_fence *drm_crtc_create_fence(struct drm_crtc *crtc);
>>  
>> +enum drm_mode_status drm_crtc_mode_valid(struct drm_crtc *crtc,
>> +					 const struct drm_display_mode *mode);
>> +
>>  /* IOCTLs */
>>  int drm_mode_getcrtc(struct drm_device *dev,
>>  		     void *data, struct drm_file *file_priv);
>> -- 
>> 1.9.1
>>
>>