From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754186AbcBAMhV (ORCPT ); Mon, 1 Feb 2016 07:37:21 -0500 Received: from us01smtprelay-2.synopsys.com ([198.182.60.111]:51716 "EHLO smtprelay.synopsys.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753671AbcBAMhS (ORCPT ); Mon, 1 Feb 2016 07:37:18 -0500 Subject: Re: [PATCH] adv7511: Added mode_fixup function. To: Laurent Pinchart , Carlos Palminha References: <1454063627-12219-1-git-send-email-palminha@synopsys.com> <1948106.WC7MId72vp@avalon> CC: , , "David Airlie" , Laurent Pinchart From: Carlos Palminha Message-ID: <56AF5176.80603@synopsys.com> Date: Mon, 1 Feb 2016 12:37:10 +0000 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: <1948106.WC7MId72vp@avalon> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.107.25.157] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 >> --- >> 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 >> --- >> 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, >