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@xxxxxxxxxxxx> > >>>> --- > >>>> 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@xxxxxxxxxxxx> > >>>> --- > >>>> 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@xxxxxxxxxxxxxxxxxxxxx > > http://lists.freedesktop.org/mailman/listinfo/dri-devel > > > > _______________________________________________ > dri-devel mailing list > dri-devel@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel