linux-kernel - Re: [PATCH] adv7511: Added mode_fixup function.

Open Source and information security mailing list archives

Message-ID: <56B3B42C.7020804@metafoo.de>
Date:	Thu, 4 Feb 2016 21:27:24 +0100
From:	Lars-Peter Clausen <lars@...afoo.de>
To:	Carlos Palminha <CARLOS.PALMINHA@...opsys.com>,
	Laurent Pinchart <laurent.pinchart@...asonboard.com>,
	David Airlie <airlied@...ux.ie>
Cc:	Laurent Pinchart <laurent.pinchart+renesas@...asonboard.com>,
	linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org
Subject: Re: [PATCH] adv7511: Added mode_fixup function.

On 02/04/2016 04:22 PM, Carlos Palminha wrote:
> Hi guys,
> 
> any feedback? patch will be accepted for adv7511 driver?

Hi,

Thanks for the patch, but please try to find and fix the call site that is
trying to invoke the callback even though it does not exist.

This is most likely drm_i2c_encoder_mode_fixup().

- Lars

> 
> Regards,
> C.Palminha
> 
> On 01-02-2016 12:37, Carlos Palminha wrote:
>> Hi Laurent
>>
>> On 29-01-2016 17:48, Laurent Pinchart wrote:
>>> Hi Carlos,
>>>
>>> Thank you for the patch.
>>>
>>> On Friday 29 January 2016 10:33:47 Carlos Palminha wrote:
>>>> The mode_fixup is necessary when using it in a DRM FB driver pipeline.
>>>
>>> Instead of implementing stubs in encoder drivers, wouldn't it be better to 
>>> make mode_fixup optional ?
>> Probably you are right but i don't have enough knowledge or time to do that for the DRM framework. :(
>> I limited myself to do what the other drivers already implement.
>>
>> The patch is mandatory to have to the ADV working or else will get some NULL pointer crash.
>>
>> Regards,
>> C.Palminha
>>
>>>
>>>> Signed-off-by: Carlos Palminha <palminha@...opsys.com>
>>>> ---
>>>>  drivers/gpu/drm/i2c/adv7511.c | 8 ++++++++
>>>>  1 file changed, 8 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/i2c/adv7511.c b/drivers/gpu/drm/i2c/adv7511.c
>>>> index 533d1e3..90082d2 100644
>>>> --- a/drivers/gpu/drm/i2c/adv7511.c
>>>> +++ b/drivers/gpu/drm/i2c/adv7511.c
>>>> @@ -648,6 +648,13 @@ adv7511_encoder_detect(struct drm_encoder *encoder,
>>>>  	return status;
>>>>  }
>>>>
>>>> +static bool adv7511_encoder_mode_fixup(struct drm_encoder *encoder,
>>>> +					const struct drm_display_mode *mode,
>>>> +					struct drm_display_mode *adjusted_mode)
>>>> +{
>>>> +	return true;
>>>> +}
>>>> +
>>>>  static int adv7511_encoder_mode_valid(struct drm_encoder *encoder,
>>>>  				      struct drm_display_mode *mode)
>>>>  {
>>>> @@ -754,6 +761,7 @@ static void adv7511_encoder_mode_set(struct drm_encoder
>>>> *encoder,
>>>>
>>>>  static const struct drm_encoder_slave_funcs adv7511_encoder_funcs = {
>>>>  	.dpms = adv7511_encoder_dpms,
>>>> +	.mode_fixup = adv7511_encoder_mode_fixup,
>>>>  	.mode_valid = adv7511_encoder_mode_valid,
>>>>  	.mode_set = adv7511_encoder_mode_set,
>>>>  	.detect = adv7511_encoder_detect,
>>>
>>
>>
>> On 29-01-2016 17:48, Laurent Pinchart wrote:
>>> Hi Carlos,
>>>
>>> Thank you for the patch.
>>>
>>> On Friday 29 January 2016 10:33:47 Carlos Palminha wrote:
>>>> The mode_fixup is necessary when using it in a DRM FB driver pipeline.
>>>
>>> Instead of implementing stubs in encoder drivers, wouldn't it be better to 
>>> make mode_fixup optional ?
>>>
>>>> Signed-off-by: Carlos Palminha <palminha@...opsys.com>
>>>> ---
>>>>  drivers/gpu/drm/i2c/adv7511.c | 8 ++++++++
>>>>  1 file changed, 8 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/i2c/adv7511.c b/drivers/gpu/drm/i2c/adv7511.c
>>>> index 533d1e3..90082d2 100644
>>>> --- a/drivers/gpu/drm/i2c/adv7511.c
>>>> +++ b/drivers/gpu/drm/i2c/adv7511.c
>>>> @@ -648,6 +648,13 @@ adv7511_encoder_detect(struct drm_encoder *encoder,
>>>>  	return status;
>>>>  }
>>>>
>>>> +static bool adv7511_encoder_mode_fixup(struct drm_encoder *encoder,
>>>> +					const struct drm_display_mode *mode,
>>>> +					struct drm_display_mode *adjusted_mode)
>>>> +{
>>>> +	return true;
>>>> +}
>>>> +
>>>>  static int adv7511_encoder_mode_valid(struct drm_encoder *encoder,
>>>>  				      struct drm_display_mode *mode)
>>>>  {
>>>> @@ -754,6 +761,7 @@ static void adv7511_encoder_mode_set(struct drm_encoder
>>>> *encoder,
>>>>
>>>>  static const struct drm_encoder_slave_funcs adv7511_encoder_funcs = {
>>>>  	.dpms = adv7511_encoder_dpms,
>>>> +	.mode_fixup = adv7511_encoder_mode_fixup,
>>>>  	.mode_valid = adv7511_encoder_mode_valid,
>>>>  	.mode_set = adv7511_encoder_mode_set,
>>>>  	.detect = adv7511_encoder_detect,
>>>
> _______________________________________________
> dri-devel mailing list
> dri-devel@...ts.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

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