All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] adv7511: Added mode_fixup function.
@ 2016-01-29 10:33 ` Carlos Palminha
  0 siblings, 0 replies; 15+ messages in thread
From: Carlos Palminha @ 2016-01-29 10:33 UTC (permalink / raw)
  To: dri-devel; +Cc: Carlos Palminha, linux-kernel, David Airlie, Laurent Pinchart

The mode_fixup is necessary when using it in a DRM FB driver pipeline.

Signed-off-by: Carlos Palminha <palminha@synopsys.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,
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 15+ messages in thread
* [PATCH] adv7511: Added mode_fixup function.
@ 2016-01-29 10:33 ` Carlos Palminha
  0 siblings, 0 replies; 15+ messages in thread
From: Carlos Palminha @ 2016-01-29 10:33 UTC (permalink / raw)
  To: dri-devel; +Cc: Carlos Palminha, linux-kernel, David Airlie, Laurent Pinchart

The mode_fixup is necessary when using it in a DRM FB driver pipeline.

Signed-off-by: Carlos Palminha <palminha@synopsys.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,
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 15+ messages in thread
* Re: [PATCH] adv7511: Added mode_fixup function.
  2016-01-29 10:33 ` Carlos Palminha
@ 2016-01-29 17:48   ` Laurent Pinchart
  -1 siblings, 0 replies; 15+ messages in thread
From: Laurent Pinchart @ 2016-01-29 17:48 UTC (permalink / raw)
  To: Carlos Palminha; +Cc: dri-devel, linux-kernel, David Airlie, Laurent Pinchart

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@synopsys.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,

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH] adv7511: Added mode_fixup function.
@ 2016-01-29 17:48   ` Laurent Pinchart
  0 siblings, 0 replies; 15+ messages in thread
From: Laurent Pinchart @ 2016-01-29 17:48 UTC (permalink / raw)
  To: Carlos Palminha; +Cc: Laurent Pinchart, linux-kernel, dri-devel

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@synopsys.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,

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH] adv7511: Added mode_fixup function.
  2016-01-29 17:48   ` Laurent Pinchart
@ 2016-02-01 12:37     ` Carlos Palminha
  -1 siblings, 0 replies; 15+ messages in thread
From: Carlos Palminha @ 2016-02-01 12:37 UTC (permalink / raw)
  To: Laurent Pinchart, Carlos Palminha
  Cc: dri-devel, linux-kernel, David Airlie, Laurent Pinchart

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@synopsys.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@synopsys.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,
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH] adv7511: Added mode_fixup function.
@ 2016-02-01 12:37     ` Carlos Palminha
  0 siblings, 0 replies; 15+ messages in thread
From: Carlos Palminha @ 2016-02-01 12:37 UTC (permalink / raw)
  To: Laurent Pinchart, Carlos Palminha
  Cc: dri-devel, linux-kernel, David Airlie, Laurent Pinchart

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@synopsys.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@synopsys.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,
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH] adv7511: Added mode_fixup function.
  2016-02-01 12:37     ` Carlos Palminha
@ 2016-02-04 15:22       ` Carlos Palminha
  -1 siblings, 0 replies; 15+ messages in thread
From: Carlos Palminha @ 2016-02-04 15:22 UTC (permalink / raw)
  To: Laurent Pinchart, Carlos Palminha, David Airlie
  Cc: dri-devel, linux-kernel, Laurent Pinchart

Hi guys,

any feedback? patch will be accepted for adv7511 driver?

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@synopsys.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@synopsys.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,
>>

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH] adv7511: Added mode_fixup function.
@ 2016-02-04 15:22       ` Carlos Palminha
  0 siblings, 0 replies; 15+ messages in thread
From: Carlos Palminha @ 2016-02-04 15:22 UTC (permalink / raw)
  To: Laurent Pinchart, Carlos Palminha, David Airlie
  Cc: dri-devel, linux-kernel, Laurent Pinchart

Hi guys,

any feedback? patch will be accepted for adv7511 driver?

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@synopsys.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@synopsys.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,
>>

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH] adv7511: Added mode_fixup function.
  2016-02-04 15:22       ` Carlos Palminha
  (?)
@ 2016-02-04 20:27       ` Lars-Peter Clausen
  2016-02-09  9:24           ` Daniel Vetter
  -1 siblings, 1 reply; 15+ messages in thread
From: Lars-Peter Clausen @ 2016-02-04 20:27 UTC (permalink / raw)
  To: Carlos Palminha, Laurent Pinchart, David Airlie
  Cc: Laurent Pinchart, linux-kernel, dri-devel

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@synopsys.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@synopsys.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@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH] adv7511: Added mode_fixup function.
  2016-02-04 20:27       ` Lars-Peter Clausen
@ 2016-02-09  9:24           ` Daniel Vetter
  0 siblings, 0 replies; 15+ messages in thread
From: Daniel Vetter @ 2016-02-09  9:24 UTC (permalink / raw)
  To: Lars-Peter Clausen
  Cc: Carlos Palminha, Laurent Pinchart, David Airlie,
	Laurent Pinchart, linux-kernel, dri-devel

On Thu, Feb 04, 2016 at 09:27:24PM +0100, Lars-Peter Clausen wrote:
> 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().

Agreed, this should be fixed in the helper library, not in drivers by
copypasting piles more dummy functions.
-Daniel

> 
> - 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@synopsys.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@synopsys.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@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
> > 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH] adv7511: Added mode_fixup function.
@ 2016-02-09  9:24           ` Daniel Vetter
  0 siblings, 0 replies; 15+ messages in thread
From: Daniel Vetter @ 2016-02-09  9:24 UTC (permalink / raw)
  To: Lars-Peter Clausen
  Cc: Laurent Pinchart, linux-kernel, dri-devel, Carlos Palminha,
	Laurent Pinchart

On Thu, Feb 04, 2016 at 09:27:24PM +0100, Lars-Peter Clausen wrote:
> 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().

Agreed, this should be fixed in the helper library, not in drivers by
copypasting piles more dummy functions.
-Daniel

> 
> - 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@synopsys.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@synopsys.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@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
> > 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH] adv7511: Added mode_fixup function.
  2016-02-09  9:24           ` Daniel Vetter
@ 2016-02-10 10:19             ` Carlos Palminha
  -1 siblings, 0 replies; 15+ messages in thread
From: Carlos Palminha @ 2016-02-10 10:19 UTC (permalink / raw)
  To: Lars-Peter Clausen, Carlos Palminha, Laurent Pinchart,
	David Airlie, Laurent Pinchart, linux-kernel, dri-devel

Hi guys,

I agree that this should be fixed in the helper library.
There are already a lot of drivers that copy/paste code and several helper functions that do not avoid it.

I will start sending some patches to fix this type of issue in several places of DRM helper functions.

Regards,
C.Palminha

On 09-02-2016 09:24, Daniel Vetter wrote:
> On Thu, Feb 04, 2016 at 09:27:24PM +0100, Lars-Peter Clausen wrote:
>> 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().
> 
> Agreed, this should be fixed in the helper library, not in drivers by
> copypasting piles more dummy functions.
> -Daniel
> 
>>
>> - 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@synopsys.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@synopsys.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@lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH] adv7511: Added mode_fixup function.
@ 2016-02-10 10:19             ` Carlos Palminha
  0 siblings, 0 replies; 15+ messages in thread
From: Carlos Palminha @ 2016-02-10 10:19 UTC (permalink / raw)
  To: Lars-Peter Clausen, Carlos Palminha, Laurent Pinchart,
	David Airlie, Laurent Pinchart, linux-kernel, dri-devel

Hi guys,

I agree that this should be fixed in the helper library.
There are already a lot of drivers that copy/paste code and several helper functions that do not avoid it.

I will start sending some patches to fix this type of issue in several places of DRM helper functions.

Regards,
C.Palminha

On 09-02-2016 09:24, Daniel Vetter wrote:
> On Thu, Feb 04, 2016 at 09:27:24PM +0100, Lars-Peter Clausen wrote:
>> 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().
> 
> Agreed, this should be fixed in the helper library, not in drivers by
> copypasting piles more dummy functions.
> -Daniel
> 
>>
>> - 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@synopsys.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@synopsys.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@lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH] adv7511: Added mode_fixup function.
  2016-02-10 10:19             ` Carlos Palminha
@ 2016-02-10 12:17               ` Daniel Vetter
  -1 siblings, 0 replies; 15+ messages in thread
From: Daniel Vetter @ 2016-02-10 12:17 UTC (permalink / raw)
  To: Carlos Palminha
  Cc: Lars-Peter Clausen, Laurent Pinchart, David Airlie,
	Laurent Pinchart, linux-kernel, dri-devel

On Wed, Feb 10, 2016 at 10:19:21AM +0000, Carlos Palminha wrote:
> Hi guys,
> 
> I agree that this should be fixed in the helper library.
> There are already a lot of drivers that copy/paste code and several helper functions that do not avoid it.
> 
> I will start sending some patches to fix this type of issue in several places of DRM helper functions.

Awesome, thanks a lot for doing this.
-Daniel

> 
> Regards,
> C.Palminha
> 
> On 09-02-2016 09:24, Daniel Vetter wrote:
> > On Thu, Feb 04, 2016 at 09:27:24PM +0100, Lars-Peter Clausen wrote:
> >> 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().
> > 
> > Agreed, this should be fixed in the helper library, not in drivers by
> > copypasting piles more dummy functions.
> > -Daniel
> > 
> >>
> >> - 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@synopsys.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@synopsys.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@lists.freedesktop.org
> >>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
> >>>
> >>
> >> _______________________________________________
> >> dri-devel mailing list
> >> dri-devel@lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/dri-devel
> > 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH] adv7511: Added mode_fixup function.
@ 2016-02-10 12:17               ` Daniel Vetter
  0 siblings, 0 replies; 15+ messages in thread
From: Daniel Vetter @ 2016-02-10 12:17 UTC (permalink / raw)
  To: Carlos Palminha
  Cc: Laurent Pinchart, linux-kernel, dri-devel, Laurent Pinchart

On Wed, Feb 10, 2016 at 10:19:21AM +0000, Carlos Palminha wrote:
> Hi guys,
> 
> I agree that this should be fixed in the helper library.
> There are already a lot of drivers that copy/paste code and several helper functions that do not avoid it.
> 
> I will start sending some patches to fix this type of issue in several places of DRM helper functions.

Awesome, thanks a lot for doing this.
-Daniel

> 
> Regards,
> C.Palminha
> 
> On 09-02-2016 09:24, Daniel Vetter wrote:
> > On Thu, Feb 04, 2016 at 09:27:24PM +0100, Lars-Peter Clausen wrote:
> >> 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().
> > 
> > Agreed, this should be fixed in the helper library, not in drivers by
> > copypasting piles more dummy functions.
> > -Daniel
> > 
> >>
> >> - 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@synopsys.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@synopsys.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@lists.freedesktop.org
> >>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
> >>>
> >>
> >> _______________________________________________
> >> dri-devel mailing list
> >> dri-devel@lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/dri-devel
> > 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread
end of thread, other threads:[~2016-02-10 12:17 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-29 10:33 [PATCH] adv7511: Added mode_fixup function Carlos Palminha
2016-01-29 10:33 ` Carlos Palminha
2016-01-29 17:48 ` Laurent Pinchart
2016-01-29 17:48   ` Laurent Pinchart
2016-02-01 12:37   ` Carlos Palminha
2016-02-01 12:37     ` Carlos Palminha
2016-02-04 15:22     ` Carlos Palminha
2016-02-04 15:22       ` Carlos Palminha
2016-02-04 20:27       ` Lars-Peter Clausen
2016-02-09  9:24         ` Daniel Vetter
2016-02-09  9:24           ` Daniel Vetter
2016-02-10 10:19           ` Carlos Palminha
2016-02-10 10:19             ` Carlos Palminha
2016-02-10 12:17             ` Daniel Vetter
2016-02-10 12:17               ` Daniel Vetter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.