Hi Am 28.12.19 um 01:59 schrieb Tian Tao: > In the previous version, the callback function mode_valid of > drm_connector_helper_funcs directly returned MODE_OK. Now we will > ensure that the resolution is correct and return MODE_OK, otherwise > return MODE_NOMODE. > > Signed-off-by: Tian Tao <[email protected]> > Signed-off-by: Gong junjie <[email protected]> > --- > drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c | 41 > ++++++++++++++++++++++-- > 1 file changed, 39 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c > b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c > index 6d98fdc..3d08210 100644 > --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c > +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c > @@ -11,22 +11,59 @@ > * Jianhua Li <[email protected]> > */ > > +#include <drm/drm_gem_vram_helper.h> > #include <drm/drm_atomic_helper.h> > #include <drm/drm_probe_helper.h> > +#include <drm/drm_crtc_helper.h> > #include <drm/drm_print.h> > > #include "hibmc_drm_drv.h" > #include "hibmc_drm_regs.h" > > +static const struct hibmc_resolution { > + int w; > + int h; > +} hibmc_modetables[] = { > + {640, 480}, {800, 600}, {1024, 768}, {1152, 864}, {1280, 768}, > + {1280, 720}, {1280, 960}, {1280, 1024}, {1440, 900}, {1600, 900}, > + {1600, 1200}, {1920, 1080}, {1920, 1200} > +};
There's already a mode table for PLL programming in hibmc_drm_de.c.
Rather than duplicating the information, maybe make the existing table
available for the mode-checking code.
> +
> +static int hibmc_valid_mode(int w, int h)
> +{
> + int size = sizeof(hibmc_modetables) / sizeof(struct hibmc_resolution);
> + int i;
> +
> + for (i = 0; i < size; i++) {
> + if (hibmc_modetables[i].w == w && hibmc_modetables[i].h == h)
> + return 0;
> + }
> +
> + return -1;
> +}
> +
> static int hibmc_connector_get_modes(struct drm_connector *connector)
> {
> - return drm_add_modes_noedid(connector, 800, 600);
> + int count;
> +
> + drm_connector_update_edid_property(connector, NULL);
> + count = drm_add_modes_noedid(connector, 1920, 1200);
> + drm_set_preferred_mode(connector, 1024, 768);
> +
> + return count;
> }
>
> static enum drm_mode_status hibmc_connector_mode_valid(struct drm_connector
> *connector,
> struct drm_display_mode *mode)
> {
> - return MODE_OK;
> + int vrefresh = drm_mode_vrefresh(mode);
> +
> + if (vrefresh < 59 || vrefresh > 61)
> + return MODE_NOMODE;
> + else if (hibmc_valid_mode(mode->hdisplay, mode->vdisplay) != 0)
I'd rather drop hibmc_valid_mode() from the patch and do its tests right
here.
More generally speaking, are these tests really related to the
connector? Both, refresh and resolution are limits of the CRTC. I think
they should be validated in the CRTC code.
Best regards
Thomas
> + return MODE_NOMODE;
> + else
> + return MODE_OK;
> }
>
> static const struct drm_connector_helper_funcs
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
signature.asc
Description: OpenPGP digital signature
_______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
